Skip to content

Commit 2aaa416

Browse files
jack2bsJackToubes
andauthored
[FIRRTL] Fix when flattening with layerblocks domination issue (#10650)
When flattening a when block, statements like asserts are merged with ltlAndWithCondition and ltlImplicationWithCondition. In the process of flattening, flattened ops that are identical become shared as an optimization strategy. However, an op created inside of a layerblock should not be shared by sibling layerblocks because an op defined in one layerblock's region does not dominate a sibling layerblock. This fixes the issue by saving a copy of cache before processing the layer block. It then restores the copy after it finishes processing the layerblock. Assisted-by: Claude Code Co-authored-by: Jack Toubes <jtoubes@berkeley.edu>
1 parent c7f81c5 commit 2aaa416

2 files changed

Lines changed: 42 additions & 0 deletions

File tree

lib/Dialect/FIRRTL/Transforms/ExpandWhens.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "mlir/Pass/Pass.h"
2323
#include "llvm/ADT/MapVector.h"
2424
#include "llvm/ADT/STLExtras.h"
25+
#include "llvm/Support/SaveAndRestore.h"
2526

2627
namespace circt {
2728
namespace firrtl {
@@ -706,6 +707,15 @@ void WhenOpVisitor::visitStmt(WhenOp whenOp) {
706707

707708
// NOLINTNEXTLINE(misc-no-recursion)
708709
void WhenOpVisitor::visitStmt(LayerBlockOp layerBlockOp) {
710+
// Operations created inside a layerblock are not accessible in sibling
711+
// layerblocks. Snapshot the LTL op caches before entering and restore them
712+
// afterward so that ops created inside one layerblock are not reused by a
713+
// sibling, which would cause dominance violations. Parent-scope cached ops
714+
// remain available inside the layerblock since they dominate it.
715+
llvm::SaveAndRestore savedLTLAndOps(createdLTLAndOps);
716+
llvm::SaveAndRestore savedLTLImplicationOps(createdLTLImplicationOps);
717+
llvm::SaveAndRestore savedLTLClockOps(createdLTLClockOps);
718+
709719
process(*layerBlockOp.getBody());
710720
}
711721

test/Dialect/FIRRTL/expand-whens.mlir

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,4 +776,36 @@ firrtl.module @instance_choice_test(in %a : !firrtl.uint<1>,
776776
}
777777
}
778778

779+
780+
// Test that sibling layerblocks inside a when each get their own LTL ops and do
781+
// not share cached values across layerblock boundaries (which would cause
782+
// dominance violations). See https://github.com/llvm/circt/issues/10104.
783+
firrtl.layer @LayerBar bind attributes {sym_visibility = "private"} {}
784+
// CHECK-LABEL: firrtl.module @SiblingLayerblocksImplication
785+
firrtl.module @SiblingLayerblocksImplication() {
786+
%c0_ui1 = firrtl.constant 0 : !firrtl.uint<1>
787+
// CHECK: firrtl.layerblock @LayerBar {
788+
// CHECK: firrtl.int.ltl.implication
789+
// CHECK: [[AND1:%.+]] = firrtl.int.ltl.and %c0_ui1, %c0_ui1
790+
// CHECK: [[IMPL1:%.+]] = firrtl.int.ltl.implication [[AND1]], %c0_ui1
791+
// CHECK: firrtl.int.verif.assert [[IMPL1]], %c0_ui1
792+
// CHECK: }
793+
// CHECK: firrtl.layerblock @LayerBar {
794+
// CHECK: firrtl.int.ltl.implication
795+
// CHECK: [[AND2:%.+]] = firrtl.int.ltl.and %c0_ui1, %c0_ui1
796+
// CHECK: [[IMPL2:%.+]] = firrtl.int.ltl.implication [[AND2]], %c0_ui1
797+
// CHECK: firrtl.int.verif.assert [[IMPL2]], %c0_ui1
798+
// CHECK: }
799+
firrtl.when %c0_ui1 : !firrtl.uint<1> {
800+
firrtl.layerblock @LayerBar {
801+
%0 = firrtl.int.ltl.implication %c0_ui1, %c0_ui1 : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1>
802+
firrtl.int.verif.assert %0, %c0_ui1 : !firrtl.uint<1>, !firrtl.uint<1>
803+
}
804+
firrtl.layerblock @LayerBar {
805+
%0 = firrtl.int.ltl.implication %c0_ui1, %c0_ui1 : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1>
806+
firrtl.int.verif.assert %0, %c0_ui1 : !firrtl.uint<1>, !firrtl.uint<1>
807+
}
808+
}
809+
}
810+
779811
}

0 commit comments

Comments
 (0)