Skip to content

Commit

Permalink
[HW] Consistent HWMemSimImpl ignore-read-enable
Browse files Browse the repository at this point in the history
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 <schuyler.eldridge@sifive.com>
  • Loading branch information
seldridge committed Jul 27, 2023
1 parent d4b591e commit 02157b2
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
20 changes: 16 additions & 4 deletions lib/Dialect/SV/Transforms/HWMemSimImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
13 changes: 10 additions & 3 deletions test/Dialect/SV/hw-memsim.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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<uarray<16xi32>>
// 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<i4>
// 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>} {
Expand Down

0 comments on commit 02157b2

Please sign in to comment.