Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Seq] Add clock gate operation #5082

Merged
merged 1 commit into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions include/circt/Dialect/Seq/SeqOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,37 @@ def WritePortOp : SeqOp<"write", [
let results = (outs);
let hasCustomAssemblyFormat = 1;
}

//===----------------------------------------------------------------------===//
// Clock Gate
//===----------------------------------------------------------------------===//

def ClockGateOp : SeqOp<"clock_gate", [Pure]> {
let summary = "Safely gates a clock with an enable signal";
let description = [{
The `seq.clock_gate` enables and disables a clock safely, without glitches,
based on a boolean enable value. If the enable operand is 1, the output
clock produced by the clock gate is identical to the input clock. If the
enable operand is 0, the output clock is a constant zero.

The `enable` operand is sampled at the rising edge of the input clock; any
changes on the enable before or after that edge are ignored and do not
affect the output clock.

The `test_enable` operand is optional and if present is OR'd together with
the `enable` operand to determine whether the output clock is gated or not.

```
%gatedClock = seq.clock_gate %clock, %enable : i1
%gatedClock = seq.clock_gate %clock, %enable, %test_enable : i1, i1
```
}];

let arguments = (ins I1:$input, I1:$enable, Optional<I1>:$test_enable);
let results = (outs I1:$output);
let hasFolder = 1;
let hasCanonicalizeMethod = 1;
let assemblyFormat = [{
$input `,` $enable (`,` $test_enable^)? attr-dict
}];
}
61 changes: 61 additions & 0 deletions lib/Dialect/Seq/SeqOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,29 @@ using namespace mlir;
using namespace circt;
using namespace seq;

/// Determine the integer value of a constant operand.
static std::optional<APInt> getConstantInt(Attribute operand) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like these are useful in general (and likely copied from combfolds). Would it make sense to put them in a shared utility header?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find any such op in the comb folds yet. But we should definitely add such a header to reduce code duplication.

if (!operand)
return {};
if (auto attr = dyn_cast<IntegerAttr>(operand))
return attr.getValue();
return {};
}

/// Determine whether a constant operand is a zero value.
static bool isConstantZero(Attribute operand) {
if (auto cst = getConstantInt(operand))
return cst->isZero();
return false;
}

/// Determine whether a constant operand is a one value.
static bool isConstantOne(Attribute operand) {
if (auto cst = getConstantInt(operand))
return cst->isOne();
return false;
}

bool circt::seq::isValidIndexValues(Value hlmemHandle, ValueRange addresses) {
auto memType = hlmemHandle.getType().cast<seq::HLMemType>();
auto shape = memType.getShape();
Expand Down Expand Up @@ -597,6 +620,44 @@ OpFoldResult FirRegOp::fold(FoldAdaptor adaptor) {
return IntegerAttr::get(intType, 0);
}

//===----------------------------------------------------------------------===//
// ClockGateOp
//===----------------------------------------------------------------------===//

OpFoldResult ClockGateOp::fold(FoldAdaptor adaptor) {
// Forward the clock if one of the enables is always true.
if (isConstantOne(adaptor.getEnable()) ||
isConstantOne(adaptor.getTestEnable()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to check for existence of the enable first?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's done in isConstantOne. The adaptor will return null if the enable isn't there, and the function handles that case.

return getInput();

// Fold to a constant zero clock if the enables are always false.
if (isConstantZero(adaptor.getEnable()) &&
(!getTestEnable() || isConstantZero(adaptor.getTestEnable())))
return IntegerAttr::get(IntegerType::get(getContext(), 1), 0);

// Forward constant zero clocks.
if (isConstantZero(adaptor.getInput()))
return IntegerAttr::get(IntegerType::get(getContext(), 1), 0);

return {};
}

LogicalResult ClockGateOp::canonicalize(ClockGateOp op,
PatternRewriter &rewriter) {
// Remove constant false test enable.
if (auto testEnable = op.getTestEnable()) {
if (auto constOp = testEnable.getDefiningOp<hw::ConstantOp>()) {
if (constOp.getValue().isZero()) {
rewriter.updateRootInPlace(op,
[&] { op.getTestEnableMutable().clear(); });
return success();
}
}
}

return failure();
}

//===----------------------------------------------------------------------===//
// TableGen generated logic.
//===----------------------------------------------------------------------===//
Expand Down
33 changes: 33 additions & 0 deletions test/Dialect/Seq/canonicalization.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,36 @@ hw.module @UninitializedArrayElement(%a: i1, %clock: i1) -> (b: !hw.array<2xi1>)
%1 = hw.array_create %0, %a : i1
hw.output %r : !hw.array<2xi1>
}

// CHECK-LABEL: hw.module @ClockGate
hw.module @ClockGate(%clock: i1, %enable: i1, %testEnable: i1) {
// CHECK-NEXT: hw.constant false
%false = hw.constant false
%true = hw.constant true

// CHECK-NEXT: %zeroClock = hw.wire %false sym @zeroClock
%0 = seq.clock_gate %false, %enable
%zeroClock = hw.wire %0 sym @zeroClock : i1

// CHECK-NEXT: %alwaysOff1 = hw.wire %false sym @alwaysOff1
// CHECK-NEXT: %alwaysOff2 = hw.wire %false sym @alwaysOff2
%1 = seq.clock_gate %clock, %false
%2 = seq.clock_gate %clock, %false, %false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using true for the enable would provide more coverage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are separate always-on tests for the constant true case. The problem here is if I pass one of the enables as constant true, it will just make the clock gate fold away (the enables are ORed).

%alwaysOff1 = hw.wire %1 sym @alwaysOff1 : i1
%alwaysOff2 = hw.wire %2 sym @alwaysOff2 : i1

// CHECK-NEXT: %alwaysOn1 = hw.wire %clock sym @alwaysOn1
// CHECK-NEXT: %alwaysOn2 = hw.wire %clock sym @alwaysOn2
// CHECK-NEXT: %alwaysOn3 = hw.wire %clock sym @alwaysOn3
%3 = seq.clock_gate %clock, %true
%4 = seq.clock_gate %clock, %true, %testEnable
%5 = seq.clock_gate %clock, %enable, %true
%alwaysOn1 = hw.wire %3 sym @alwaysOn1 : i1
%alwaysOn2 = hw.wire %4 sym @alwaysOn2 : i1
%alwaysOn3 = hw.wire %5 sym @alwaysOn3 : i1

// CHECK-NEXT: [[TMP:%.+]] = seq.clock_gate %clock, %enable
// CHECK-NEXT: %dropTestEnable = hw.wire [[TMP]] sym @dropTestEnable
%6 = seq.clock_gate %clock, %enable, %false
%dropTestEnable = hw.wire %6 sym @dropTestEnable : i1
}
8 changes: 8 additions & 0 deletions test/Dialect/Seq/round-trip.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,11 @@ hw.module @d0(%clk : i1, %rst : i1) -> () {
%myMemory_rdata = seq.read %myMemory[%c0_i0] rden %c1_i1 { latency = 0} : !seq.hlmem<1xi32>
hw.output
}

// CHECK-LABEL: hw.module @ClockGate
hw.module @ClockGate(%clock: i1, %enable: i1, %test_enable: i1) {
// CHECK-NEXT: seq.clock_gate %clock, %enable
// CHECK-NEXT: seq.clock_gate %clock, %enable, %test_enable
%cg0 = seq.clock_gate %clock, %enable
%cg1 = seq.clock_gate %clock, %enable, %test_enable
}