Skip to content

Commit 64a4391

Browse files
rwy7KelvinChung2000
authored andcommitted
[FirRegLowering] Add limit to number of ifs generated (llvm#8313)
When lowering a firreg to SV, we structure the IR so that the register is driven from inside an if-op tree. We build this structure by examining the register's next value, translating mux ops into if-ops. This PR sets a limit of 1024 on the number of if-ops generated for each register. While it is unlikely to hit this limit in the common case, it is possible to cause firtool to produce a very large if-op tree from a relatively small number of mux ops. The goal is to prevent firtool from hanging. If we exceed the limit, we stop translating muxes to nested if-ops, and just drive the register with the original mux op result instead. This PR changes the work stack to a work queue, so that we create if-ops in a breadth-first order, so that the budget is allocated evenly across the branches of the if-op tree.
1 parent 607beb8 commit 64a4391

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

lib/Conversion/SeqToSV/FirRegLowering.cpp

+18-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#include "llvm/ADT/DenseSet.h"
1414
#include "llvm/Support/Debug.h"
1515

16+
#include <deque>
17+
1618
using namespace circt;
1719
using namespace hw;
1820
using namespace seq;
@@ -396,6 +398,12 @@ FirRegLowering::tryRestoringSubaccess(OpBuilder &builder, Value reg, Value term,
396398

397399
void FirRegLowering::createTree(OpBuilder &builder, Value reg, Value term,
398400
Value next) {
401+
// If-then-else tree limit.
402+
constexpr size_t limit = 1024;
403+
404+
// Count of emitted if-then-else ops.
405+
size_t counter = 0;
406+
399407
// Get the fanout from this register before we build the tree. While we are
400408
// creating the tree of if/else statements from muxes, we only want to turn
401409
// muxes that are on the register's fanout into if/else statements. This is
@@ -405,9 +413,9 @@ void FirRegLowering::createTree(OpBuilder &builder, Value reg, Value term,
405413
// enable.
406414
auto firReg = term.getDefiningOp<seq::FirRegOp>();
407415

408-
SmallVector<std::tuple<Block *, Value, Value, Value>> worklist;
416+
std::deque<std::tuple<Block *, Value, Value, Value>> worklist;
409417
auto addToWorklist = [&](Value reg, Value term, Value next) {
410-
worklist.push_back({builder.getBlock(), reg, term, next});
418+
worklist.emplace_back(builder.getBlock(), reg, term, next);
411419
};
412420

413421
auto getArrayIndex = [&](Value reg, Value idx) {
@@ -423,7 +431,9 @@ void FirRegLowering::createTree(OpBuilder &builder, Value reg, Value term,
423431
OpBuilder::InsertionGuard guard(builder);
424432
Block *block;
425433
Value reg, term, next;
426-
std::tie(block, reg, term, next) = worklist.pop_back_val();
434+
std::tie(block, reg, term, next) = worklist.front();
435+
worklist.pop_front();
436+
427437
builder.setInsertionPointToEnd(block);
428438
if (areEquivalentValues(term, next))
429439
continue;
@@ -433,10 +443,15 @@ void FirRegLowering::createTree(OpBuilder &builder, Value reg, Value term,
433443
auto mux = next.getDefiningOp<comb::MuxOp>();
434444
if (mux && mux.getTwoState() &&
435445
reachableMuxes->isMuxReachableFrom(firReg, mux)) {
446+
if (counter >= limit) {
447+
builder.create<sv::PAssignOp>(term.getLoc(), reg, next);
448+
continue;
449+
}
436450
addToIfBlock(
437451
builder, mux.getCond(),
438452
[&]() { addToWorklist(reg, term, mux.getTrueValue()); },
439453
[&]() { addToWorklist(reg, term, mux.getFalseValue()); });
454+
++counter;
440455
continue;
441456
}
442457
// If the next value is an array creation, split the value into

test/Dialect/Seq/firreg.mlir

+3-3
Original file line numberDiff line numberDiff line change
@@ -606,8 +606,8 @@ hw.module @ArrayElements(in %a: !hw.array<2xi1>, in %clock: !seq.clock, in %cond
606606
// CHECK-NEXT: %[[r2:.+]] = sv.array_index_inout %r[%true] : !hw.inout<array<2xi1>>, i1
607607
// CHECK: sv.always posedge %clock {
608608
// CHECK-NEXT: sv.if %cond {
609-
// CHECK-NEXT: sv.passign %[[r1]], %1 : i1
610609
// CHECK-NEXT: sv.passign %[[r2]], %0 : i1
610+
// CHECK-NEXT: sv.passign %[[r1]], %1 : i1
611611
// CHECK-NEXT: } else {
612612
// CHECK-NEXT: }
613613
// CHECK-NEXT: }
@@ -711,9 +711,9 @@ hw.module @NestedSubaccess(in %clock: !seq.clock, in %en_0: i1, in %en_1: i1, in
711711
%31 = comb.mux bin %en_1, %27, %30 : !hw.array<3xi32>
712712
%32 = hw.array_create %26, %24, %22 : i32
713713
%33 = comb.mux bin %en_0, %31, %32 : !hw.array<3xi32>
714-
// CHECK: %[[IDX1:.+]] = sv.array_index_inout %r[%addr_0] : !hw.inout<array<3xi32>>, i2
715-
// CHECK: %[[IDX2:.+]] = sv.array_index_inout %r[%addr_1] : !hw.inout<array<3xi32>>, i2
716714
// CHECK: %[[IDX3:.+]] = sv.array_index_inout %r[%addr_2] : !hw.inout<array<3xi32>>, i2
715+
// CHECK: %[[IDX2:.+]] = sv.array_index_inout %r[%addr_1] : !hw.inout<array<3xi32>>, i2
716+
// CHECK: %[[IDX1:.+]] = sv.array_index_inout %r[%addr_0] : !hw.inout<array<3xi32>>, i2
717717
// CHECK: %[[IDX4:.+]] = sv.array_index_inout %r[%addr_3] : !hw.inout<array<3xi32>>, i2
718718
// CHECK: sv.always posedge %clock {
719719
// CHECK-NEXT: sv.if %en_0 {

0 commit comments

Comments
 (0)