Skip to content

Commit

Permalink
[FIRRTL] LowerToHW: guard against folded operations (#7358) (#7590)
Browse files Browse the repository at this point in the history
In LowerToHW we copy the name from FIRRTL ops to the lowered HW ops.  We
often call `createOrFold` when creating the HW ops, which can mean that
there is no operation to copy the name to.  We were not properly
detecting this scenario, which could cause us to try to copy the name to
a null operation.  Most ops were already covered under a guard in a
generic code path, but div-like and subaccess-like operations needed to
be guarded.  Some of these problems are impossible to test right now as
we require folders to be implemented for some operations.

This backports #7358 to 1.62.0 for Chisel 6 users.

Co-authored-by: Andrew Young <youngar17@gmail.com>
  • Loading branch information
dtzSiFive and youngar authored Sep 6, 2024
1 parent ccdef17 commit ba60dce
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
12 changes: 8 additions & 4 deletions lib/Conversion/FIRRTLToHW/LowerToHW.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2604,7 +2604,8 @@ FailureOr<Value> FIRRTLLowering::lowerSubindex(SubindexOp op, Value input) {
result = builder.createOrFold<sv::ArrayIndexInOutOp>(input, iIdx);
else
result = builder.createOrFold<hw::ArrayGetOp>(input, iIdx);
tryCopyName(result.getDefiningOp(), op);
if (auto *definingOp = result.getDefiningOp())
tryCopyName(definingOp, op);
return result;
}

Expand All @@ -2627,7 +2628,8 @@ FailureOr<Value> FIRRTLLowering::lowerSubaccess(SubaccessOp op, Value input) {
result = builder.createOrFold<sv::ArrayIndexInOutOp>(input, valueIdx);
else
result = createArrayIndexing(input, valueIdx);
tryCopyName(result.getDefiningOp(), op);
if (auto *definingOp = result.getDefiningOp())
tryCopyName(definingOp, op);
return result;
}

Expand All @@ -2647,7 +2649,8 @@ FailureOr<Value> FIRRTLLowering::lowerSubfield(SubfieldOp op, Value input) {
result = builder.createOrFold<sv::StructFieldInOutOp>(input, field);
else
result = builder.createOrFold<hw::StructExtractOp>(input, field);
tryCopyName(result.getDefiningOp(), op);
if (auto *definingOp = result.getDefiningOp())
tryCopyName(definingOp, op);
return result;
}

Expand Down Expand Up @@ -3500,7 +3503,8 @@ LogicalResult FIRRTLLowering::lowerDivLikeOp(Operation *op) {
else
result = builder.createOrFold<UnsignedOp>(lhs, rhs, true);

tryCopyName(result.getDefiningOp(), op);
if (auto *definingOp = result.getDefiningOp())
tryCopyName(definingOp, op);

if (resultType == opType)
return setLowering(op->getResult(0), result);
Expand Down
6 changes: 5 additions & 1 deletion test/Conversion/FIRRTLToHW/lower-to-hw.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,11 @@ firrtl.circuit "Simple" attributes {annotations = [{class =
// CHECK-NEXT: = comb.extract [[SHIFT]] from 0 : (i14) -> i4
%31 = firrtl.dshr %25, %18 : (!firrtl.sint<4>, !firrtl.uint<14>) -> !firrtl.sint<4>

// CHECK-NEXT: comb.icmp bin ule {{.*}}, {{.*}} : i4
// Noop.
%c0_ui1 = firrtl.constant 0 : !firrtl.const.uint<1>
%32 = firrtl.dshr %in1, %c0_ui1 { name = "test" } : (!firrtl.uint<4>, !firrtl.const.uint<1>) -> !firrtl.uint<4>

// CHECK: comb.icmp bin ule {{.*}}, {{.*}} : i4
%41 = firrtl.leq %in1, %4 : (!firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<1>
// CHECK-NEXT: comb.icmp bin ult {{.*}}, {{.*}} : i4
%42 = firrtl.lt %in1, %4 : (!firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<1>
Expand Down

0 comments on commit ba60dce

Please sign in to comment.