From e1036215ecea9c1d1e500966d2cafc7da8dd6c45 Mon Sep 17 00:00:00 2001 From: Andrew Lenharth Date: Wed, 11 May 2022 06:11:23 -0700 Subject: [PATCH] When lowering memories, groupID is part of the identity of memories for uniquing, but not part of the identity for the symbol generated. Fix. --- lib/Dialect/FIRRTL/FIRRTLOps.cpp | 12 +++++----- test/Conversion/FIRRTLToHW/lower-to-hw.mlir | 26 ++++++++++----------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index 3ddf531361..7fbd04cf4e 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -1979,12 +1979,12 @@ FirMemory MemOp::getSummary() { clocks.append(Twine((char)(a + 'a')).str()); modName = StringAttr::get( op->getContext(), - llvm::formatv("FIRRTLMem_{0}_{1}_{2}_{3}_{4}_{5}_{6}_{7}_{8}_{9}{10}", - numReadPorts, numWritePorts, numReadWritePorts, - (size_t)width, op.depth(), op.readLatency(), - op.writeLatency(), op.getMaskBits(), (size_t)op.ruw(), - (unsigned)hw::WUW::PortOrder, - clocks.empty() ? "" : "_" + clocks)); + llvm::formatv( + "FIRRTLMem_{0}_{1}_{2}_{3}_{4}_{5}_{6}_{7}_{8}_{9}_{10}{11}", + numReadPorts, numWritePorts, numReadWritePorts, (size_t)width, + op.depth(), op.readLatency(), op.writeLatency(), op.getMaskBits(), + (size_t)op.ruw(), (unsigned)hw::WUW::PortOrder, groupID, + clocks.empty() ? "" : "_" + clocks)); } return {numReadPorts, numWritePorts, numReadWritePorts, (size_t)width, op.depth(), op.readLatency(), diff --git a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir index 6efd26e14f..60754ed21c 100644 --- a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir +++ b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir @@ -12,7 +12,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // This memory has two write ports where both write ports are driven by the // same clock. // - // CHECK-NEXT: hw.module.generated @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_aa, + // CHECK-NEXT: hw.module.generated @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_0_aa, // CHECK-SAME: @FIRRTLMem(%W0_addr: i4, %W0_en: i1, %W0_clk: i1, %W0_data: i8, %W1_addr: i4, %W1_en: i1, %W1_clk: i1, %W1_data: i8) // CHECK-SAME: attributes {depth = 16 : i64, maskGran = 8 : ui32, numReadPorts = 0 : ui32, // CHECK-SAME: numReadWritePorts = 0 : ui32, numWritePorts = 2 : ui32, @@ -23,7 +23,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // This memory is the same as the above memory, but each write port is driven // by a different clock. // - // CHECK-NEXT: hw.module.generated @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_ab, + // CHECK-NEXT: hw.module.generated @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_0_ab, // CHECK-SAME: @FIRRTLMem(%W0_addr: i4, %W0_en: i1, %W0_clk: i1, %W0_data: i8, %W1_addr: i4, %W1_en: i1, %W1_clk: i1, %W1_data: i8) // CHECK-SAME: attributes {depth = 16 : i64, maskGran = 8 : ui32, numReadPorts = 0 : ui32, // CHECK-SAME: numReadWritePorts = 0 : ui32, numWritePorts = 2 : ui32, @@ -31,14 +31,14 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // CHECK-SAME: width = 8 : ui32, writeClockIDs = [0 : i32, 1 : i32], // CHECK-SAME: writeLatency = 1 : ui32, writeUnderWrite = 1 : i32} // - // CHECK-NEXT: hw.module.generated @FIRRTLMem_1_0_0_32_1_0_1_0_1_1, + // CHECK-NEXT: hw.module.generated @FIRRTLMem_1_0_0_32_1_0_1_0_1_1_0, // CHECK-SAME: @FIRRTLMem(%R0_addr: i1, %R0_en: i1, %R0_clk: i1) -> (R0_data: i32) // CHECK-SAME: attributes {depth = 1 : i64, maskGran = 32 : ui32, numReadPorts = 1 : ui32, // CHECK-SAME: numReadWritePorts = 0 : ui32, numWritePorts = 0 : ui32, // CHECK-SAME: readLatency = 0 : ui32, readUnderWrite = 1 : ui32, // CHECK-SAME: width = 32 : ui32, writeClockIDs = [], // CHECK-SAME: writeLatency = 1 : ui32, writeUnderWrite = 1 : i32} - // CHECK-NEXT: hw.module.generated @FIRRTLMem_1_0_0_42_12_0_1_0_0_1, + // CHECK-NEXT: hw.module.generated @FIRRTLMem_1_0_0_42_12_0_1_0_0_1_0, // CHECK-SAME: @FIRRTLMem(%R0_addr: i4, %R0_en: i1, %R0_clk: i1) -> (R0_data: i42) // CHECK-SAME: attributes {depth = 12 : i64, maskGran = 42 : ui32, numReadPorts = 1 : ui32, // CHECK-SAME: numReadWritePorts = 0 : ui32, numWritePorts = 0 : ui32, @@ -49,9 +49,9 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // CHECK: hw.module.generated @tbMemoryKind1_ext, @FIRRTLMem // CHECK-SAME: (%R0_addr: i4, %R0_en: i1, %R0_clk: i1, %W0_addr: i4, %W0_en: i1, %W0_clk: i1, %W0_data: i8) - // CHECK-NEXT: hw.module.generated @FIRRTLMem_1_1_1_40_1022_1_1_4_0_1_a, + // CHECK-NEXT: hw.module.generated @FIRRTLMem_1_1_1_40_1022_1_1_4_0_1_0_a, // CHECK-SAME: @FIRRTLMem(%R0_addr: i10, %R0_en: i1, %R0_clk: i1, %RW0_addr: i10, %RW0_en: i1, %RW0_clk: i1, %RW0_wmode: i1, %RW0_wdata: i40, %RW0_wmask: i4, %W0_addr: i10, %W0_en: i1, %W0_clk: i1, %W0_data: i40, %W0_mask: i4) -> (R0_data: i40, RW0_rdata: i40) - // CHECK-NEXT: hw.module.generated @FIRRTLMem_1_1_1_42_12_0_1_1_0_1_a, + // CHECK-NEXT: hw.module.generated @FIRRTLMem_1_1_1_42_12_0_1_1_0_1_0_a, // CHECK-SAME: @FIRRTLMem(%R0_addr: i4, %R0_en: i1, %R0_clk: i1, %RW0_addr: i4, %RW0_en: i1, %RW0_clk: i1, %RW0_wmode: i1, %RW0_wdata: i42, %W0_addr: i4, %W0_en: i1, %W0_clk: i1, %W0_data: i42) -> (R0_data: i42, RW0_rdata: i42) // CHECK-SAME: attributes {depth = 12 : i64, maskGran = 42 : ui32, numReadPorts = 1 : ui32, // CHECK-SAME: numReadWritePorts = 1 : ui32, numWritePorts = 1 : ui32, @@ -862,7 +862,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = %c0_ui3 = firrtl.constant 0 : !firrtl.uint<3> %_M_read, %_M_rw, %_M_write = firrtl.mem Undefined {depth = 12 : i64, name = "_M", portNames = ["read", "rw", "write"], readLatency = 0 : i32, writeLatency = 1 : i32} : !firrtl.bundle, en: uint<1>, clk: clock, data flip: sint<42>>, !firrtl.bundle, en: uint<1>, clk: clock, rdata flip: sint<42>, wmode: uint<1>, wdata: sint<42>, wmask: uint<1>>, !firrtl.bundle, en: uint<1>, clk: clock, data: sint<42>, mask: uint<1>> // CHECK: %[[v2:.+]] = comb.and %inpred, %true : i1 - // CHECK: %_M_ext.R0_data, %_M_ext.RW0_rdata = hw.instance "_M_ext" @FIRRTLMem_1_1_1_42_12_0_1_1_0_1_a(R0_addr: %c0_i4: i4, R0_en: %true: i1, R0_clk: %clock1: i1, RW0_addr: %c0_i4_0: i4, RW0_en: %0: i1, RW0_clk: %clock1: i1, RW0_wmode: %true: i1, RW0_wdata: %1: i42, W0_addr: %c0_i4_1: i4, W0_en: %[[v2]]: i1, W0_clk: %clock2: i1, W0_data: %indata: i42) -> (R0_data: i42, RW0_rdata: i42) + // CHECK: %_M_ext.R0_data, %_M_ext.RW0_rdata = hw.instance "_M_ext" @FIRRTLMem_1_1_1_42_12_0_1_1_0_1_0_a(R0_addr: %c0_i4: i4, R0_en: %true: i1, R0_clk: %clock1: i1, RW0_addr: %c0_i4_0: i4, RW0_en: %0: i1, RW0_clk: %clock1: i1, RW0_wmode: %true: i1, RW0_wdata: %1: i42, W0_addr: %c0_i4_1: i4, W0_en: %[[v2]]: i1, W0_clk: %clock2: i1, W0_data: %indata: i42) -> (R0_data: i42, RW0_rdata: i42) // CHECK: hw.output %_M_ext.R0_data, %_M_ext.RW0_rdata : i42, i42 %0 = firrtl.subfield %_M_read(3) : (!firrtl.bundle, en: uint<1>, clk: clock, data flip: sint<42>>) -> !firrtl.sint<42> @@ -911,7 +911,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = %c0_ui4 = firrtl.constant 0 : !firrtl.uint<4> %c1_ui5 = firrtl.constant 1 : !firrtl.uint<5> %_M_read, %_M_rw, %_M_write = firrtl.mem Undefined {depth = 1022 : i64, name = "_M_mask", portNames = ["read", "rw", "write"], readLatency = 1 : i32, writeLatency = 1 : i32} : !firrtl.bundle, en: uint<1>, clk: clock, data flip: sint<40>>, !firrtl.bundle, en: uint<1>, clk: clock, rdata flip: sint<40>, wmode: uint<1>, wdata: sint<40>, wmask: uint<4>>, !firrtl.bundle, en: uint<1>, clk: clock, data: sint<40>, mask: uint<4>> - // CHECK: %_M_mask_ext.R0_data, %_M_mask_ext.RW0_rdata = hw.instance "_M_mask_ext" @FIRRTLMem_1_1_1_40_1022_1_1_4_0_1_a(R0_addr: %c0_i10: i10, R0_en: %true: i1, R0_clk: %clock1: i1, RW0_addr: %c0_i10: i10, RW0_en: %true: i1, RW0_clk: %clock1: i1, RW0_wmode: %true: i1, RW0_wdata: %0: i40, RW0_wmask: %c0_i4: i4, W0_addr: %c0_i10: i10, W0_en: %inpred: i1, W0_clk: %clock2: i1, W0_data: %indata: i40, W0_mask: %c0_i4: i4) -> (R0_data: i40, RW0_rdata: i40) + // CHECK: %_M_mask_ext.R0_data, %_M_mask_ext.RW0_rdata = hw.instance "_M_mask_ext" @FIRRTLMem_1_1_1_40_1022_1_1_4_0_1_0_a(R0_addr: %c0_i10: i10, R0_en: %true: i1, R0_clk: %clock1: i1, RW0_addr: %c0_i10: i10, RW0_en: %true: i1, RW0_clk: %clock1: i1, RW0_wmode: %true: i1, RW0_wdata: %0: i40, RW0_wmask: %c0_i4: i4, W0_addr: %c0_i10: i10, W0_en: %inpred: i1, W0_clk: %clock2: i1, W0_data: %indata: i40, W0_mask: %c0_i4: i4) -> (R0_data: i40, RW0_rdata: i40) // CHECK: hw.output %_M_mask_ext.R0_data, %_M_mask_ext.RW0_rdata : i40, i40 %0 = firrtl.subfield %_M_read(3) : (!firrtl.bundle, en: uint<1>, clk: clock, data flip: sint<40>>) -> !firrtl.sint<40> @@ -954,7 +954,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = %c0_ui1 = firrtl.constant 0 : !firrtl.uint<1> %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1> - // CHECK: %_M_ext.R0_data = hw.instance "_M_ext" @FIRRTLMem_1_0_0_42_12_0_1_0_0_1(R0_addr: %c0_i4: i4, R0_en: %true: i1, R0_clk: %clock1: i1) -> (R0_data: i42) + // CHECK: %_M_ext.R0_data = hw.instance "_M_ext" @FIRRTLMem_1_0_0_42_12_0_1_0_0_1_0(R0_addr: %c0_i4: i4, R0_en: %true: i1, R0_clk: %clock1: i1) -> (R0_data: i42) %_M_read = firrtl.mem Undefined {depth = 12 : i64, name = "_M", portNames = ["read"], readLatency = 0 : i32, writeLatency = 1 : i32} : !firrtl.bundle, en: uint<1>, clk: clock, data flip: sint<42>> // Read port. %6 = firrtl.subfield %_M_read(0) : (!firrtl.bundle, en: uint<1>, clk: clock, data flip: sint<42>>) -> !firrtl.uint<4> @@ -1037,7 +1037,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // CHECK-LABEL: hw.module private @MemDepth1 firrtl.module private @MemDepth1(in %clock: !firrtl.clock, in %en: !firrtl.uint<1>, in %addr: !firrtl.uint<1>, out %data: !firrtl.uint<32>) { - // CHECK: %mem0_ext.R0_data = hw.instance "mem0_ext" @FIRRTLMem_1_0_0_32_1_0_1_0_1_1(R0_addr: %addr: i1, R0_en: %en: i1, R0_clk: %clock: i1) -> (R0_data: i32) + // CHECK: %mem0_ext.R0_data = hw.instance "mem0_ext" @FIRRTLMem_1_0_0_32_1_0_1_0_1_1_0(R0_addr: %addr: i1, R0_en: %en: i1, R0_clk: %clock: i1) -> (R0_data: i32) // CHECK: hw.output %mem0_ext.R0_data : i32 %mem0_load0 = firrtl.mem Old {depth = 1 : i64, name = "mem0", portNames = ["load0"], readLatency = 0 : i32, writeLatency = 1 : i32} : !firrtl.bundle, en: uint<1>, clk: clock, data flip: uint<32>> %0 = firrtl.subfield %mem0_load0(2) : (!firrtl.bundle, en: uint<1>, clk: clock, data flip: uint<32>>) -> !firrtl.clock @@ -1155,7 +1155,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // lowered to an "aa" memory. Even if the clock is passed via different wires, // we should identify the clocks to be same. // - // CHECK: hw.instance "aa_ext" @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_aa + // CHECK: hw.instance "aa_ext" @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_0_aa %memory_aa_w0, %memory_aa_w1 = firrtl.mem Undefined {depth = 16 : i64, name = "aa", portNames = ["w0", "w1"], readLatency = 1 : i32, writeLatency = 1 : i32} : !firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>, !firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>> %clk_aa_w0 = firrtl.subfield %memory_aa_w0(2) : (!firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>) -> !firrtl.clock %clk_aa_w1 = firrtl.subfield %memory_aa_w1(2) : (!firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>) -> !firrtl.clock @@ -1169,7 +1169,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // This memory has different clocks for each write port. It should be // lowered to an "ab" memory. // - // CHECK: hw.instance "ab_ext" @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_ab + // CHECK: hw.instance "ab_ext" @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_0_ab %memory_ab_w0, %memory_ab_w1 = firrtl.mem Undefined {depth = 16 : i64, name = "ab", portNames = ["w0", "w1"], readLatency = 1 : i32, writeLatency = 1 : i32} : !firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>, !firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>> %clk_ab_w0 = firrtl.subfield %memory_ab_w0(2) : (!firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>) -> !firrtl.clock %clk_ab_w1 = firrtl.subfield %memory_ab_w1(2) : (!firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>) -> !firrtl.clock @@ -1181,7 +1181,7 @@ firrtl.circuit "Simple" attributes {annotations = [{class = // annotation blocking this from being optimized away). This should be // lowered to an "aa" since they are identical. // - // CHECK: hw.instance "ab_node_ext" @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_aa + // CHECK: hw.instance "ab_node_ext" @FIRRTLMem_0_2_0_8_16_1_1_1_0_1_0_aa %memory_ab_node_w0, %memory_ab_node_w1 = firrtl.mem Undefined {depth = 16 : i64, name = "ab_node", portNames = ["w0", "w1"], readLatency = 1 : i32, writeLatency = 1 : i32} : !firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>, !firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>> %clk_ab_node_w0 = firrtl.subfield %memory_ab_node_w0(2) : (!firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>) -> !firrtl.clock %clk_ab_node_w1 = firrtl.subfield %memory_ab_node_w1(2) : (!firrtl.bundle, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>) -> !firrtl.clock