From 02157b2879ab955c4a06cb0a2c51805183be9ad9 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Thu, 27 Jul 2023 15:19:15 -0400 Subject: [PATCH] [HW] Consistent HWMemSimImpl ignore-read-enable Fix an inconsistency between the handling of read ports and read-write ports when running HWMemSimImpl with the "ignore-read-enable" option (firtool option "-ignore-read-enable-mem"). After #5700, this would cause the enable line of a read-write read port to have no effect. I.e., the memory would always read even if not enabled. Change this to align with the behavior of read ports where the memory always returns the last read address. Signed-off-by: Schuyler Eldridge --- lib/Dialect/SV/Transforms/HWMemSimImpl.cpp | 20 ++++++++++++++++---- test/Dialect/SV/hw-memsim.mlir | 13 ++++++++++--- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/Dialect/SV/Transforms/HWMemSimImpl.cpp b/lib/Dialect/SV/Transforms/HWMemSimImpl.cpp index a6d36cdff96f..3f1a95cd2082 100644 --- a/lib/Dialect/SV/Transforms/HWMemSimImpl.cpp +++ b/lib/Dialect/SV/Transforms/HWMemSimImpl.cpp @@ -318,10 +318,22 @@ void HWMemSimImpl::generateMemory(HWModuleOp op, FirMemory mem) { addPipelineStages(b, moduleNamespace, numCommonStages, clock, wmode); // Add read-only pipeline stages. - auto read_addr = addPipelineStages( - b, moduleNamespace, numReadStages - numCommonStages, clock, addr); - auto read_en = addPipelineStages( - b, moduleNamespace, numReadStages - numCommonStages, clock, en); + Value read_addr = addr; + Value read_en = en; + if (ignoreReadEnable) { + for (size_t j = 0, e = mem.readLatency; j != e; ++j) { + auto enLast = en; + if (j < e - 1) + read_en = addPipelineStages(b, moduleNamespace, 1, clock, en); + read_addr = + addPipelineStages(b, moduleNamespace, 1, clock, addr, enLast); + } + } else { + read_addr = addPipelineStages( + b, moduleNamespace, numReadStages - numCommonStages, clock, addr); + read_en = addPipelineStages(b, moduleNamespace, + numReadStages - numCommonStages, clock, en); + } auto read_wmode = addPipelineStages( b, moduleNamespace, numReadStages - numCommonStages, clock, wmode); diff --git a/test/Dialect/SV/hw-memsim.mlir b/test/Dialect/SV/hw-memsim.mlir index 994952457c08..d7e455038021 100644 --- a/test/Dialect/SV/hw-memsim.mlir +++ b/test/Dialect/SV/hw-memsim.mlir @@ -218,9 +218,16 @@ hw.module.generated @FIRRTLMemTwoAlways, @FIRRTLMem( %wo_addr_0: i4, %wo_en_0: i // CHECK-NEXT: sv.passign %[[v220]], %[[v15]] : i8 // IGNORE: %[[Memory:.+]] = sv.reg : !hw.inout> - // IGNORE: %[[slot:.+]] = sv.array_index_inout %Memory - // IGNORE-NEXT: %[[result:.+]] = sv.read_inout %[[slot]] - // IGNORE: hw.output %[[result]] + // IGNORE-NEXT: %[[read_addr_inout:.+]] = sv.reg {{.*}} : !hw.inout + // IGNORE-NEXT: sv.always posedge %R0_clk { + // IGNORE-NEXT: sv.if %R0_en { + // IGNORE-NEXT: sv.passign %[[read_addr_inout]], %R0_addr + // IGNORE-NEXT: } + // IGNORE-NEXT: } + // IGNORE-NEXT: %[[read_addr:.+]] = sv.read_inout %[[read_addr_inout]] + // IGNORE-NEXT: %[[slot_read:.+]] = sv.array_index_inout %Memory[%[[read_addr]]] + // IGNORE-NEXT: %[[result_read:.+]] = sv.read_inout %[[slot_read]] + // IGNORE: hw.output %[[result_read]] hw.module.generated @FIRRTLMem_1_1_0_32_16_1_1_0_1_b, @FIRRTLMem(%R0_addr: i4, %R0_en: i1, %R0_clk: i1, %W0_addr: i4, %W0_en: i1, %W0_clk: i1, %W0_data: i32, %W0_mask: i2) -> (R0_data: i32) attributes {depth = 16 : i64, maskGran = 16 : ui32, numReadPorts = 1 : ui32, numReadWritePorts = 0 : ui32, numWritePorts = 1 : ui32, readLatency = 2 : ui32, readUnderWrite = 0 : i32, width = 32 : ui32, writeClockIDs = [0 : i32], writeLatency = 3 : ui32, writeUnderWrite = 1 : i32, initFilename = "", initIsBinary = false, initIsInline = false} hw.module @memTestBar(%clock: i1, %rAddr: i4, %rEn: i1, %wAddr: i4, %wEn: i1, %wMask: i2, %wData: i32) -> (rData: i32) attributes {firrtl.moduleHierarchyFile = #hw.output_file<"testharness_hier.json", excludeFromFileList>} {