From 52a53ca8191cfa1823a7d5949f3181148cc01801 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Sat, 16 Mar 2024 02:55:19 -0400 Subject: [PATCH] [InferReadWrite] Set builder insertion point to ensure dominance (#6836) This fixes a bug in InferReadWrite, the builder was not respecting dominance when updating the IR. Ensure that the builder creates the constant at the start of the module. The bug was introduced in #6818 --- lib/Dialect/FIRRTL/Transforms/InferReadWrite.cpp | 2 ++ test/Dialect/FIRRTL/inferRW.mlir | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/InferReadWrite.cpp b/lib/Dialect/FIRRTL/Transforms/InferReadWrite.cpp index 03eaae0da3cd..0d4c62d8eeec 100644 --- a/lib/Dialect/FIRRTL/Transforms/InferReadWrite.cpp +++ b/lib/Dialect/FIRRTL/Transforms/InferReadWrite.cpp @@ -438,6 +438,8 @@ struct InferReadWritePass : public InferReadWriteBase { if (enableDriver && wmodeDriver) { ImplicitLocOpBuilder builder(memOp.getLoc(), memOp); + builder.setInsertionPointToStart( + memOp->getParentOfType().getBodyBlock()); auto constOne = builder.create( UIntType::get(builder.getContext(), 1), APInt(1, 1)); setEnable(enableDriver, wmodeDriver, constOne); diff --git a/test/Dialect/FIRRTL/inferRW.mlir b/test/Dialect/FIRRTL/inferRW.mlir index 61a2d479e418..4f3c5181ce1e 100644 --- a/test/Dialect/FIRRTL/inferRW.mlir +++ b/test/Dialect/FIRRTL/inferRW.mlir @@ -285,6 +285,10 @@ firrtl.circuit "TLRAM" { // CHECK-LABEL: firrtl.module @SimplifyWMODE firrtl.module @SimplifyWMODE(in %rwPort_enable: !firrtl.uint<1>, in %rwPort_isWrite: !firrtl.uint<1>) attributes {} { + %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1> + %18 = firrtl.mux(%rwPort_enable, %rwPort_isWrite, %c0_ui1) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> + // CHECK: %[[c1_ui1:.+]] = firrtl.constant 1 : !firrtl.uint<1> + // CHECK: %[[v7:.+]] = firrtl.mux(%[[c1_ui1]], %rwPort_isWrite, %c0_ui1) %mem_rwPort_readData_rw = firrtl.mem Undefined {depth = 64 : i64, name = "t", portNames = ["rw"], prefix = "", readLatency = 1 : i32, writeLatency = 1 : i32} : !firrtl.bundle, en: uint<1>, clk: clock, rdata flip: uint<10>, wmode: uint<1>, wdata: uint<10>, wmask: uint<5>> %mem_rwPort_readData_rw_wmode = firrtl.wire : !firrtl.uint<1> %0 = firrtl.subfield %mem_rwPort_readData_rw[addr] : !firrtl.bundle, en: uint<1>, clk: clock, rdata flip: uint<10>, wmode: uint<1>, wdata: uint<10>, wmask: uint<5>> @@ -296,10 +300,6 @@ firrtl.circuit "TLRAM" { firrtl.strictconnect %6, %mem_rwPort_readData_rw_wmode : !firrtl.uint<1> %7 = firrtl.subfield %mem_rwPort_readData_rw[wdata] : !firrtl.bundle, en: uint<1>, clk: clock, rdata flip: uint<10>, wmode: uint<1>, wdata: uint<10>, wmask: uint<5>> %9 = firrtl.subfield %mem_rwPort_readData_rw[wmask] : !firrtl.bundle, en: uint<1>, clk: clock, rdata flip: uint<10>, wmode: uint<1>, wdata: uint<10>, wmask: uint<5>> - %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1> - %18 = firrtl.mux(%rwPort_enable, %rwPort_isWrite, %c0_ui1) : (!firrtl.uint<1>, !firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1> - // CHECK: %[[c1_ui1:.+]] = firrtl.constant 1 : !firrtl.uint<1> - // CHECK: %[[v7:.+]] = firrtl.mux(%[[c1_ui1]], %rwPort_isWrite, %c0_ui1) firrtl.strictconnect %mem_rwPort_readData_rw_wmode, %18 : !firrtl.uint<1> }