From 629abefea21ee1aac4156bb67d7f17a6c2860e33 Mon Sep 17 00:00:00 2001 From: Fabian Schuiki Date: Mon, 14 Oct 2024 09:57:07 -0700 Subject: [PATCH] [Arc] Make arc.model have an SSACFG region (#7701) Remove the `RegionKindInterface` from `arc.model` such that its body region becomes an SSACFG region. The current implementation of the LowerState pass already creates models which honor SSA dominance, and future improvements to the Arc dialect will benefit from models enforcing dominance. --- include/circt/Dialect/Arc/ArcOps.td | 5 +- .../Dialect/Arc/group-resets-and-enables.mlir | 60 +++++++++---------- .../Arc/legalize-state-update-error.mlir | 6 +- test/Dialect/Arc/legalize-state-update.mlir | 14 ++--- 4 files changed, 41 insertions(+), 44 deletions(-) diff --git a/include/circt/Dialect/Arc/ArcOps.td b/include/circt/Dialect/Arc/ArcOps.td index 0842d66b6cda..addde7f02632 100644 --- a/include/circt/Dialect/Arc/ArcOps.td +++ b/include/circt/Dialect/Arc/ArcOps.td @@ -669,7 +669,7 @@ def TapOp : ArcOp<"tap"> { } def ModelOp : ArcOp<"model", [ - RegionKindInterface, IsolatedFromAbove, NoTerminator, Symbol, + IsolatedFromAbove, NoTerminator, Symbol, DeclareOpInterfaceMethods ]> { let summary = "A model with stratified clocks"; @@ -691,9 +691,6 @@ def ModelOp : ArcOp<"model", [ }]; let extraClassDeclaration = [{ - static mlir::RegionKind getRegionKind(unsigned index) { - return mlir::RegionKind::Graph; - } mlir::Block &getBodyBlock() { return getBody().front(); } }]; diff --git a/test/Dialect/Arc/group-resets-and-enables.mlir b/test/Dialect/Arc/group-resets-and-enables.mlir index 80cbacf559bc..a3d41e7c1eff 100644 --- a/test/Dialect/Arc/group-resets-and-enables.mlir +++ b/test/Dialect/Arc/group-resets-and-enables.mlir @@ -9,6 +9,10 @@ arc.model @BasicResetGrouping io !hw.modty !arc.state %in_reset0 = arc.root_input "reset0", %arg0 : (!arc.storage) -> !arc.state %in_reset1 = arc.root_input "reset1", %arg0 : (!arc.storage) -> !arc.state + // CHECK: [[FOO_ALLOC:%.+]] = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state + // CHECK: [[BAR_ALLOC:%.+]] = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state + %1 = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state + %2 = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state %0 = arc.state_read %in_clock : // Group resets: arc.clock_tree %0 { @@ -16,8 +20,8 @@ arc.model @BasicResetGrouping io !hw.modty // CHECK-NEXT: scf.if [[IN_RESET0]] { scf.if %3 { - // CHECK-NEXT: arc.state_write [[FOO_ALLOC:%.+]] = %c0_i4 - // CHECK-NEXT: arc.state_write [[BAR_ALLOC:%.+]] = %c0_i4 + // CHECK-NEXT: arc.state_write [[FOO_ALLOC]] = %c0_i4 + // CHECK-NEXT: arc.state_write [[BAR_ALLOC]] = %c0_i4 arc.state_write %1 = %c0_i4 : // CHECK-NEXT: } else { } else { @@ -131,8 +135,8 @@ arc.model @BasicResetGrouping io !hw.modty // CHECK-NEXT: scf.if [[IN_RESET0]] { scf.if %15 { - // CHECK-NEXT: arc.state_write [[FOO_ALLOC:%.+]] = %c0_i4 - // CHECK-NEXT: arc.state_write [[BAR_ALLOC:%.+]] = %c0_i4 + // CHECK-NEXT: arc.state_write [[FOO_ALLOC]] = %c0_i4 + // CHECK-NEXT: arc.state_write [[BAR_ALLOC]] = %c0_i4 arc.state_write %1 = %c0_i4 : // CHECK-NEXT: } else { } else { @@ -147,10 +151,6 @@ arc.model @BasicResetGrouping io !hw.modty !arc.state - // CHECK-NEXT: [[BAR_ALLOC]] = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state - %1 = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state - %2 = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state } // CHECK-LABEL: arc.model @BasicEnableGrouping @@ -162,13 +162,17 @@ arc.model @BasicEnableGrouping io !hw.modty !arc.state %in_en0 = arc.root_input "en0", %arg0 : (!arc.storage) -> !arc.state %in_en1 = arc.root_input "en1", %arg0 : (!arc.storage) -> !arc.state + // CHECK: [[FOO_ALLOC:%.+]] = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state + // CHECK: [[BAR_ALLOC:%.+]] = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state + %1 = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state + %2 = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state %0 = arc.state_read %in_clock : // Group enables: arc.clock_tree %0 { // CHECK: [[IN_EN0:%.+]] = arc.state_read %in_en0 %3 = arc.state_read %in_en0 : - // CHECK-NEXT: arc.state_write [[FOO_ALLOC:%.+]] = %c0_i4 - // CHECK-NEXT: arc.state_write [[BAR_ALLOC:%.+]] = %c0_i4 + // CHECK-NEXT: arc.state_write [[FOO_ALLOC]] = %c0_i4 + // CHECK-NEXT: arc.state_write [[BAR_ALLOC]] = %c0_i4 arc.state_write %1 = %c0_i4 : arc.state_write %2 = %c0_i4 : // CHECK-NEXT: scf.if [[IN_EN0]] { @@ -204,10 +208,6 @@ arc.model @BasicEnableGrouping io !hw.modty // CHECK-NEXT: } } - // CHECK-NEXT: [[FOO_ALLOC]] = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state - // CHECK-NEXT: [[BAR_ALLOC]] = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state - %1 = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state - %2 = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state } // CHECK-LABEL: arc.model @GroupAssignmentsInIfTesting @@ -218,6 +218,10 @@ arc.model @GroupAssignmentsInIfTesting io !hw.modty !arc.state %in_cond0 = arc.root_input "cond0", %arg0 : (!arc.storage) -> !arc.state %in_cond1 = arc.root_input "cond1", %arg0 : (!arc.storage) -> !arc.state + // CHECK: [[FOO_ALLOC:%.+]] = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state + // CHECK: [[BAR_ALLOC:%.+]] = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state + %1 = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state + %2 = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state %0 = arc.state_read %in_clock : // Do pull value in (1st and 2nd layer) arc.clock_tree %0 { @@ -228,14 +232,14 @@ arc.model @GroupAssignmentsInIfTesting io !hw.modty // CHECK: [[IN_COND1:%.+]] = arc.state_read %in_cond1 %6 = arc.state_read %in_cond1 : // CHECK-NEXT: scf.if [[IN_COND1]] { scf.if %6 { // CHECK-NEXT: [[IN_I2:%.+]] = arc.state_read %in_i2 - // CHECK-NEXT: arc.state_write [[BAR_ALLOC:%.+]] = [[IN_I2]] + // CHECK-NEXT: arc.state_write [[BAR_ALLOC]] = [[IN_I2]] arc.state_write %2 = %5 : // CHECK-NEXT: } } @@ -251,11 +255,11 @@ arc.model @GroupAssignmentsInIfTesting io !hw.modty // CHECK-NEXT: scf.if [[IN_COND0]] { scf.if %5 { - // CHECK-NEXT: arc.state_write [[FOO_ALLOC:%.+]] = [[IN_I1]] + // CHECK-NEXT: arc.state_write [[FOO_ALLOC]] = [[IN_I1]] arc.state_write %1 = %6 : // CHECK-NEXT: } } - // CHECK-NEXT: arc.state_write [[BAR_ALLOC:%.+]] = [[IN_I1]] + // CHECK-NEXT: arc.state_write [[BAR_ALLOC]] = [[IN_I1]] arc.state_write %2 = %6 : // CHECK-NEXT: } } @@ -268,12 +272,12 @@ arc.model @GroupAssignmentsInIfTesting io !hw.modty // CHECK-NEXT: [[IN_COND1:%.+]] = arc.state_read %in_cond1 // CHECK-NEXT: scf.if [[IN_COND1]] { scf.if %6 { - // CHECK-NEXT: arc.state_write [[BAR_ALLOC:%.+]] = [[IN_I1]] + // CHECK-NEXT: arc.state_write [[BAR_ALLOC]] = [[IN_I1]] arc.state_write %2 = %7 : // CHECK-NEXT: } } @@ -281,10 +285,6 @@ arc.model @GroupAssignmentsInIfTesting io !hw.modty !arc.state - // CHECK-NEXT: [[BAR_ALLOC]] = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state - %1 = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state - %2 = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state } // CHECK-LABEL: arc.model @ResetAndEnableGrouping @@ -297,6 +297,10 @@ arc.model @ResetAndEnableGrouping io !hw.modty !arc.state %in_en0 = arc.root_input "en0", %arg0 : (!arc.storage) -> !arc.state %in_en1 = arc.root_input "en1", %arg0 : (!arc.storage) -> !arc.state + // CHECK: [[FOO_ALLOC:%.+]] = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state + // CHECK: [[BAR_ALLOC:%.+]] = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state + %1 = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state + %2 = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state %0 = arc.state_read %in_clock : // Group enables inside resets (and pull in reads): arc.clock_tree %0 { @@ -304,8 +308,8 @@ arc.model @ResetAndEnableGrouping io !hw.modty // CHECK-NEXT: scf.if [[IN_RESET]] { scf.if %3 { - // CHECK-NEXT: arc.state_write [[FOO_ALLOC:%.+]] = %c0_i4 - // CHECK-NEXT: arc.state_write [[BAR_ALLOC:%.+]] = %c0_i4 + // CHECK-NEXT: arc.state_write [[FOO_ALLOC]] = %c0_i4 + // CHECK-NEXT: arc.state_write [[BAR_ALLOC]] = %c0_i4 arc.state_write %1 = %c0_i4 : arc.state_write %2 = %c0_i4 : // CHECK-NEXT: } else { @@ -389,8 +393,4 @@ arc.model @ResetAndEnableGrouping io !hw.modty !arc.state - // CHECK-NEXT: [[BAR_ALLOC]] = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state - %1 = arc.alloc_state %arg0 {name = "foo"} : (!arc.storage) -> !arc.state - %2 = arc.alloc_state %arg0 {name = "bar"} : (!arc.storage) -> !arc.state } diff --git a/test/Dialect/Arc/legalize-state-update-error.mlir b/test/Dialect/Arc/legalize-state-update-error.mlir index 271f3365f041..b5fb061786a7 100644 --- a/test/Dialect/Arc/legalize-state-update-error.mlir +++ b/test/Dialect/Arc/legalize-state-update-error.mlir @@ -3,6 +3,9 @@ arc.model @Memory io !hw.modty<> { ^bb0(%arg0: !arc.storage): %false = hw.constant false + %mem1 = arc.alloc_memory %arg0 : (!arc.storage) -> !arc.memory<2 x i32, i1> + %mem2 = arc.alloc_memory %arg0 : (!arc.storage) -> !arc.memory<2 x i32, i1> + %s1 = arc.alloc_state %arg0 : (!arc.storage) -> !arc.state arc.clock_tree %false attributes {ct4} { %r1 = arc.state_read %s1 : scf.if %false { @@ -16,7 +19,4 @@ arc.model @Memory io !hw.modty<> { %mr1 = arc.memory_read %mem2[%false] : <2 x i32, i1> } } - %mem1 = arc.alloc_memory %arg0 : (!arc.storage) -> !arc.memory<2 x i32, i1> - %mem2 = arc.alloc_memory %arg0 : (!arc.storage) -> !arc.memory<2 x i32, i1> - %s1 = arc.alloc_state %arg0 : (!arc.storage) -> !arc.state } diff --git a/test/Dialect/Arc/legalize-state-update.mlir b/test/Dialect/Arc/legalize-state-update.mlir index 78d7afdaf9c5..7dc390ccb681 100644 --- a/test/Dialect/Arc/legalize-state-update.mlir +++ b/test/Dialect/Arc/legalize-state-update.mlir @@ -191,12 +191,17 @@ arc.model @DontLeakThroughClockTreeOrPassthrough io !hw.modty { ^bb0(%arg0: !arc.storage): %false = hw.constant false + // CHECK: [[MEM1:%.+]] = arc.alloc_memory %arg0 : + // CHECK: [[MEM2:%.+]] = arc.alloc_memory %arg0 : + %mem1 = arc.alloc_memory %arg0 : (!arc.storage) -> !arc.memory<2 x i32, i1> + %mem2 = arc.alloc_memory %arg0 : (!arc.storage) -> !arc.memory<2 x i32, i1> + %s1 = arc.alloc_state %arg0 : (!arc.storage) -> !arc.state // CHECK: arc.clock_tree %false attributes {ct1} arc.clock_tree %false attributes {ct1} { // CHECK-NEXT: arc.state_read - // CHECK-NEXT: arc.memory_read [[MEM1:%.+]][%false] + // CHECK-NEXT: arc.memory_read [[MEM1]][%false] // CHECK-NEXT: arc.memory_write [[MEM1]] - // CHECK-NEXT: arc.memory_read [[MEM2:%.+]][%false] + // CHECK-NEXT: arc.memory_read [[MEM2]][%false] // CHECK-NEXT: arc.memory_write [[MEM2]] %r1 = arc.state_read %s1 : arc.memory_write %mem2[%false], %r1 : <2 x i32, i1> @@ -245,9 +250,4 @@ arc.model @Memory io !hw.modty<> { %mr2 = arc.memory_read %mem2[%false] : <2 x i32, i1> // CHECK-NEXT: } } - // CHECK: [[MEM1]] = arc.alloc_memory %arg0 : - // CHECK: [[MEM2]] = arc.alloc_memory %arg0 : - %mem1 = arc.alloc_memory %arg0 : (!arc.storage) -> !arc.memory<2 x i32, i1> - %mem2 = arc.alloc_memory %arg0 : (!arc.storage) -> !arc.memory<2 x i32, i1> - %s1 = arc.alloc_state %arg0 : (!arc.storage) -> !arc.state }