[ESI] Make RAM delarations' addresses unsigned (#8658)

Using signless ints as the address was just wrong. It was probably done
to avoid incomplete lowerings, which have been addressed. Changes RAM
declaration addresses to use unsigned integer types throughout the
ESI dialect and its tests, updating both the code generator and
expected outputs.

- Switch address ports to unsigned (uiN) in MLIR tests and service
  definitions.
- Update service port generation and SystemVerilog memory
  instantiation to use unsigned address types, inserting bitcasts to
  signless for indexing.
- Adjust PyCDE front end and software integration tests to use
  UInt addressing.
This commit is contained in:
John Demme 2025-07-07 12:43:12 -07:00 committed by GitHub
parent 137b13de04
commit 8c1865c5a9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 42 additions and 30 deletions

View File

@ -36,7 +36,7 @@ assert dummy_info is not None
def read(addr: int) -> bytearray: def read(addr: int) -> bytearray:
mem_read_addr.write([addr]) mem_read_addr.write(addr)
resp = cast(bytearray, mem_read_data.read()) resp = cast(bytearray, mem_read_data.read())
print(f"resp: {resp}") print(f"resp: {resp}")
return resp return resp
@ -45,7 +45,7 @@ def read(addr: int) -> bytearray:
# The contents of address 3 are continuously updated to the contents of address # The contents of address 3 are continuously updated to the contents of address
# 2 by the accelerator. # 2 by the accelerator.
data = bytearray([random.randint(0, 2**8 - 1) for _ in range(8)]) data = bytearray([random.randint(0, 2**8 - 1) for _ in range(8)])
mem_write.write({"address": [2], "data": data}) mem_write.write({"address": 2, "data": data})
resp = read(2) resp = read(2)
try_count = 0 try_count = 0
@ -64,6 +64,6 @@ assert resp == data
# Check this by writing to address 3 and reading from it. Shouldn't have # Check this by writing to address 3 and reading from it. Shouldn't have
# changed. # changed.
zeros = bytearray([0] * 8) zeros = bytearray([0] * 8)
mem_write.write({"address": [3], "data": zeros}) mem_write.write({"address": 3, "data": zeros})
resp = read(3) resp = read(3)
assert resp == data assert resp == data

View File

@ -465,7 +465,7 @@ def DeclareRandomAccessMemory(inner_type: Type,
class DeclareRandomAccessMemory(ServiceDecl): class DeclareRandomAccessMemory(ServiceDecl):
__name__ = name __name__ = name
address_width = (depth - 1).bit_length() address_width = (depth - 1).bit_length()
address_type = Bits(address_width) address_type = UInt(address_width)
write_struct = StructType([('address', address_type), ('data', inner_type)]) write_struct = StructType([('address', address_type), ('data', inner_type)])
read = Bundle([ read = Bundle([

View File

@ -366,11 +366,11 @@ def Writer(type):
# CHECK: hw.module @Ram1(in %clk : !seq.clock, in %rst : i1) # CHECK: hw.module @Ram1(in %clk : !seq.clock, in %rst : i1)
# CHECK: esi.service.instance #esi.appid<"ram"> svc @ram impl as "sv"(%clk, %rst) : (!seq.clock, i1) -> () # CHECK: esi.service.instance #esi.appid<"ram"> svc @ram impl as "sv"(%clk, %rst) : (!seq.clock, i1) -> ()
# CHECK: [[WR:%.+]] = esi.service.req <@ram::@write>(#esi.appid<"ram_writer"[0]>) : !esi.bundle<[!esi.channel<!hw.struct<address: i3, data: i32>> from "req", !esi.channel<i0> to "ack"]> # CHECK: [[WR:%.+]] = esi.service.req <@ram::@write>(#esi.appid<"ram_writer"[0]>) : !esi.bundle<[!esi.channel<!hw.struct<address: ui3, data: i32>> from "req", !esi.channel<i0> to "ack"]>
# CHECK: %rawOutput, %valid = esi.unwrap.vr %req, %ready : !hw.struct<address: ui3, data: ui32> # CHECK: %rawOutput, %valid = esi.unwrap.vr %req, %ready : !hw.struct<address: ui3, data: ui32>
# CHECK: [[CASTED:%.+]] = hw.bitcast %rawOutput : (!hw.struct<address: ui3, data: ui32>) -> !hw.struct<address: i3, data: i32> # CHECK: [[CASTED:%.+]] = hw.bitcast %rawOutput : (!hw.struct<address: ui3, data: ui32>) -> !hw.struct<address: ui3, data: i32>
# CHECK: %chanOutput, %ready = esi.wrap.vr [[CASTED]], %valid : !hw.struct<address: i3, data: i32> # CHECK: %chanOutput, %ready = esi.wrap.vr [[CASTED]], %valid : !hw.struct<address: ui3, data: i32>
# CHECK: %ack = esi.bundle.unpack %chanOutput from [[WR]] : !esi.bundle<[!esi.channel<!hw.struct<address: i3, data: i32>> from "req", !esi.channel<i0> to "ack"]> # CHECK: %ack = esi.bundle.unpack %chanOutput from [[WR]] : !esi.bundle<[!esi.channel<!hw.struct<address: ui3, data: i32>> from "req", !esi.channel<i0> to "ack"]>
# CHECK: %bundle, %req = esi.bundle.pack %ack : !esi.bundle<[!esi.channel<!hw.struct<address: ui3, data: ui32>> from "req", !esi.channel<i0> to "ack"]> # CHECK: %bundle, %req = esi.bundle.pack %ack : !esi.bundle<[!esi.channel<!hw.struct<address: ui3, data: ui32>> from "req", !esi.channel<i0> to "ack"]>
# CHECK: hw.instance "Writer" sym @Writer @Writer(clk: %clk: !seq.clock, rst: %rst: i1, cmd: %bundle: !esi.bundle<[!esi.channel<!hw.struct<address: ui3, data: ui32>> from "req", !esi.channel<i0> to "ack"]>) -> () # CHECK: hw.instance "Writer" sym @Writer @Writer(clk: %clk: !seq.clock, rst: %rst: i1, cmd: %bundle: !esi.bundle<[!esi.channel<!hw.struct<address: ui3, data: ui32>> from "req", !esi.channel<i0> to "ack"]>) -> ()

View File

@ -93,7 +93,7 @@ hw.module @LoopbackArray() {
} }
esi.mem.ram @MemA i64 x 20 esi.mem.ram @MemA i64 x 20
!write = !hw.struct<address: i5, data: i64> !write = !hw.struct<address: ui5, data: i64>
!writeBundle = !esi.bundle<[!esi.channel<!write> from "req", !esi.channel<i0> to "ack"]> !writeBundle = !esi.bundle<[!esi.channel<!write> from "req", !esi.channel<i0> to "ack"]>
hw.module @MemoryAccess1(in %clk : !seq.clock, in %rst : i1) { hw.module @MemoryAccess1(in %clk : !seq.clock, in %rst : i1) {

View File

@ -271,8 +271,11 @@ instantiateSystemVerilogMemory(ServiceImplementReqOp implReq,
// Unwrap the requested address and read from that memory location. // Unwrap the requested address and read from that memory location.
auto addressUnwrap = auto addressUnwrap =
b.create<UnwrapValidReadyOp>(toServer, dataChannel.getReady()); b.create<UnwrapValidReadyOp>(toServer, dataChannel.getReady());
Value memLoc = Value unsignedAddress = addressUnwrap.getRawOutput();
b.create<sv::ArrayIndexInOutOp>(mem, addressUnwrap.getRawOutput()); Value signlessAddress = b.create<hw::BitcastOp>(
b.getIntegerType(llvm::Log2_64_Ceil(ramDecl.getDepth())),
unsignedAddress);
Value memLoc = b.create<sv::ArrayIndexInOutOp>(mem, signlessAddress);
auto readData = b.create<sv::ReadInOutOp>(memLoc); auto readData = b.create<sv::ReadInOutOp>(memLoc);
// Set the data on the response. // Set the data on the response.
@ -294,7 +297,10 @@ instantiateSystemVerilogMemory(ServiceImplementReqOp implReq,
Value a = address, d = data; // So the lambda can capture. Value a = address, d = data; // So the lambda can capture.
// If we're told to go, do the write. // If we're told to go, do the write.
b.create<sv::IfOp>(go, [&] { b.create<sv::IfOp>(go, [&] {
Value memLoc = b.create<sv::ArrayIndexInOutOp>(mem, a); Value signlessAddress = b.create<hw::BitcastOp>(
b.getIntegerType(llvm::Log2_64_Ceil(ramDecl.getDepth())), a);
Value memLoc =
b.create<sv::ArrayIndexInOutOp>(mem, signlessAddress);
b.create<sv::PAssignOp>(memLoc, d); b.create<sv::PAssignOp>(memLoc, d);
}); });
} }

View File

@ -39,7 +39,9 @@ static ServicePortInfo createReqResp(StringAttr sym, Twine name,
ServicePortInfo RandomAccessMemoryDeclOp::writePortInfo() { ServicePortInfo RandomAccessMemoryDeclOp::writePortInfo() {
auto *ctxt = getContext(); auto *ctxt = getContext();
auto addressType = IntegerType::get(ctxt, llvm::Log2_64_Ceil(getDepth())); auto addressType =
IntegerType::get(ctxt, llvm::Log2_64_Ceil(getDepth()),
IntegerType::SignednessSemantics::Unsigned);
// Write port // Write port
hw::StructType writeType = hw::StructType::get( hw::StructType writeType = hw::StructType::get(
@ -53,7 +55,9 @@ ServicePortInfo RandomAccessMemoryDeclOp::writePortInfo() {
ServicePortInfo RandomAccessMemoryDeclOp::readPortInfo() { ServicePortInfo RandomAccessMemoryDeclOp::readPortInfo() {
auto *ctxt = getContext(); auto *ctxt = getContext();
auto addressType = IntegerType::get(ctxt, llvm::Log2_64_Ceil(getDepth())); auto addressType =
IntegerType::get(ctxt, llvm::Log2_64_Ceil(getDepth()),
IntegerType::SignednessSemantics::Unsigned);
return createReqResp(getSymNameAttr(), "read", "address", addressType, "data", return createReqResp(getSymNameAttr(), "read", "address", addressType, "data",
getInnerType()); getInnerType());

View File

@ -121,22 +121,24 @@ esi.pure_module @LoopbackCosimPure {
} }
// CONN-LABEL: esi.mem.ram @MemA i64 x 20 // CONN-LABEL: esi.mem.ram @MemA i64 x 20
// CONN-LABEL: hw.module @MemoryAccess1(in %clk : !seq.clock, in %rst : i1, in %write : !esi.channel<!hw.struct<address: i5, data: i64>>, in %readAddress : !esi.channel<i5>, out readData : !esi.channel<i64>, out writeDone : !esi.channel<i0>) { // CONN-LABEL: hw.module @MemoryAccess1(in %clk : !seq.clock, in %rst : i1, in %write : !esi.channel<!hw.struct<address: ui5, data: i64>>, in %readAddress : !esi.channel<ui5>, out readData : !esi.channel<i64>, out writeDone : !esi.channel<i0>) {
// CONN: %MemA = sv.reg : !hw.inout<uarray<20xi64>> // CONN: %MemA = sv.reg : !hw.inout<uarray<20xi64>>
// CONN: %chanOutput, %ready = esi.wrap.vr %c0_i0, %write_done : i0 // CONN: %chanOutput, %ready = esi.wrap.vr %c0_i0, %write_done : i0
// CONN: %rawOutput, %valid = esi.unwrap.vr %write, %ready : !hw.struct<address: i5, data: i64> // CONN: %rawOutput, %valid = esi.unwrap.vr %write, %ready : !hw.struct<address: ui5, data: i64>
// CONN: %address = hw.struct_extract %rawOutput["address"] : !hw.struct<address: i5, data: i64> // CONN: %address = hw.struct_extract %rawOutput["address"] : !hw.struct<address: ui5, data: i64>
// CONN: %data = hw.struct_extract %rawOutput["data"] : !hw.struct<address: i5, data: i64> // CONN: %data = hw.struct_extract %rawOutput["data"] : !hw.struct<address: ui5, data: i64>
// CONN: %[[ANDVR:.*]] = comb.and %valid, %ready {sv.namehint = "write_go"} : i1 // CONN: %[[ANDVR:.*]] = comb.and %valid, %ready {sv.namehint = "write_go"} : i1
// CONN: %write_done = seq.compreg sym @write_done %[[ANDVR]], %clk reset %rst, %false : i1 // CONN: %write_done = seq.compreg sym @write_done %[[ANDVR]], %clk reset %rst, %false : i1
// CONN: %chanOutput_0, %ready_1 = esi.wrap.vr %[[MEMREAD:.*]], %valid_3 : i64 // CONN: %chanOutput_0, %ready_1 = esi.wrap.vr %[[MEMREAD:.*]], %valid_3 : i64
// CONN: %rawOutput_2, %valid_3 = esi.unwrap.vr %readAddress, %ready_1 : i5 // CONN: %rawOutput_2, %valid_3 = esi.unwrap.vr %readAddress, %ready_1 : ui5
// CONN: %[[MEMREADIO:.*]] = sv.array_index_inout %MemA[%rawOutput_2] : !hw.inout<uarray<20xi64>>, i5 // CONN: %[[ADDR:.*]] = hw.bitcast %rawOutput_2 : (ui5) -> i5
// CONN: %[[MEMREADIO:.*]] = sv.array_index_inout %MemA[%[[ADDR]]] : !hw.inout<uarray<20xi64>>, i5
// CONN: %[[MEMREAD]] = sv.read_inout %[[MEMREADIO]] : !hw.inout<i64> // CONN: %[[MEMREAD]] = sv.read_inout %[[MEMREADIO]] : !hw.inout<i64>
// CONN: %[[CLOCK:.+]] = seq.from_clock %clk // CONN: %[[CLOCK:.+]] = seq.from_clock %clk
// CONN: sv.alwaysff(posedge %[[CLOCK]]) { // CONN: sv.alwaysff(posedge %[[CLOCK]]) {
// CONN: sv.if %[[ANDVR]] { // CONN: sv.if %[[ANDVR]] {
// CONN: %[[ARRIDX:.*]] = sv.array_index_inout %MemA[%address] : !hw.inout<uarray<20xi64>>, i5 // CONN: %[[ADDR:.*]] = hw.bitcast %address : (ui5) -> i5
// CONN: %[[ARRIDX:.*]] = sv.array_index_inout %MemA[%[[ADDR]]] : !hw.inout<uarray<20xi64>>, i5
// CONN: sv.passign %[[ARRIDX]], %data : i64 // CONN: sv.passign %[[ARRIDX]], %data : i64
// CONN: } // CONN: }
// CONN: }(syncreset : posedge %rst) { // CONN: }(syncreset : posedge %rst) {
@ -144,11 +146,11 @@ esi.pure_module @LoopbackCosimPure {
// CONN: hw.output %chanOutput_0, %chanOutput : !esi.channel<i64>, !esi.channel<i0> // CONN: hw.output %chanOutput_0, %chanOutput : !esi.channel<i64>, !esi.channel<i0>
esi.mem.ram @MemA i64 x 20 esi.mem.ram @MemA i64 x 20
!write = !hw.struct<address: i5, data: i64> !write = !hw.struct<address: ui5, data: i64>
!writeBundle = !esi.bundle<[!esi.channel<!write> from "req", !esi.channel<i0> to "ack"]> !writeBundle = !esi.bundle<[!esi.channel<!write> from "req", !esi.channel<i0> to "ack"]>
!readBundle = !esi.bundle<[!esi.channel<i5> from "address", !esi.channel<i64> to "data"]> !readBundle = !esi.bundle<[!esi.channel<ui5> from "address", !esi.channel<i64> to "data"]>
hw.module @MemoryAccess1(in %clk : !seq.clock, in %rst : i1, in %write : !esi.channel<!write>, in %readAddress : !esi.channel<i5>, out readData : !esi.channel<i64>, out writeDone : !esi.channel<i0>) { hw.module @MemoryAccess1(in %clk : !seq.clock, in %rst : i1, in %write : !esi.channel<!write>, in %readAddress : !esi.channel<ui5>, out readData : !esi.channel<i64>, out writeDone : !esi.channel<i0>) {
esi.service.instance #esi.appid<"mem"> svc @MemA impl as "sv_mem" (%clk, %rst) : (!seq.clock, i1) -> () esi.service.instance #esi.appid<"mem"> svc @MemA impl as "sv_mem" (%clk, %rst) : (!seq.clock, i1) -> ()
%writeBundle = esi.service.req <@MemA::@write> (#esi.appid<"write">) : !writeBundle %writeBundle = esi.service.req <@MemA::@write> (#esi.appid<"write">) : !writeBundle
%done = esi.bundle.unpack %write from %writeBundle : !writeBundle %done = esi.bundle.unpack %write from %writeBundle : !writeBundle
@ -158,11 +160,11 @@ hw.module @MemoryAccess1(in %clk : !seq.clock, in %rst : i1, in %write : !esi.ch
hw.output %readData, %done : !esi.channel<i64>, !esi.channel<i0> hw.output %readData, %done : !esi.channel<i64>, !esi.channel<i0>
} }
// CONN-LABEL: hw.module @MemoryAccess2Read(in %clk : !seq.clock, in %rst : i1, in %write : !esi.channel<!hw.struct<address: i5, data: i64>>, in %readAddress : !esi.channel<i5>, in %readAddress2 : !esi.channel<i5>, out readData : !esi.channel<i64>, out readData2 : !esi.channel<i64>, out writeDone : !esi.channel<i0>) { // CONN-LABEL: hw.module @MemoryAccess2Read(in %clk : !seq.clock, in %rst : i1, in %write : !esi.channel<!hw.struct<address: ui5, data: i64>>, in %readAddress : !esi.channel<ui5>, in %readAddress2 : !esi.channel<ui5>, out readData : !esi.channel<i64>, out readData2 : !esi.channel<i64>, out writeDone : !esi.channel<i0>) {
// CONN: %MemA = sv.reg : !hw.inout<uarray<20xi64>> // CONN: %MemA = sv.reg : !hw.inout<uarray<20xi64>>
// CONN: hw.output %chanOutput_0, %chanOutput_4, %chanOutput : !esi.channel<i64>, !esi.channel<i64>, !esi.channel<i0> // CONN: hw.output %chanOutput_0, %chanOutput_4, %chanOutput : !esi.channel<i64>, !esi.channel<i64>, !esi.channel<i0>
hw.module @MemoryAccess2Read(in %clk: !seq.clock, in %rst: i1, in %write: !esi.channel<!write>, in %readAddress: !esi.channel<i5>, in %readAddress2: !esi.channel<i5>, out readData: !esi.channel<i64>, out readData2: !esi.channel<i64>, out writeDone: !esi.channel<i0>) { hw.module @MemoryAccess2Read(in %clk: !seq.clock, in %rst: i1, in %write: !esi.channel<!write>, in %readAddress: !esi.channel<ui5>, in %readAddress2: !esi.channel<ui5>, out readData: !esi.channel<i64>, out readData2: !esi.channel<i64>, out writeDone: !esi.channel<i0>) {
esi.service.instance #esi.appid<"mem"> svc @MemA impl as "sv_mem" (%clk, %rst) : (!seq.clock, i1) -> () esi.service.instance #esi.appid<"mem"> svc @MemA impl as "sv_mem" (%clk, %rst) : (!seq.clock, i1) -> ()
%writeBundle = esi.service.req <@MemA::@write> (#esi.appid<"write">) : !writeBundle %writeBundle = esi.service.req <@MemA::@write> (#esi.appid<"write">) : !writeBundle

View File

@ -27,10 +27,10 @@
// CHECK-SAME: in %reset : i1, // CHECK-SAME: in %reset : i1,
// CHECK-SAME: out out0 : !esi.channel<i0> // CHECK-SAME: out out0 : !esi.channel<i0>
// CHECK-SAME: ) { // CHECK-SAME: ) {
//CHECK-NEXT: [[B0:%.+]] = esi.service.req <@mem::@read>(#esi.appid<"load"[1]>) : !esi.bundle<[!esi.channel<i4> from "address", !esi.channel<i32> to "data"]> //CHECK-NEXT: [[B0:%.+]] = esi.service.req <@mem::@read>(#esi.appid<"load"[1]>) : !esi.bundle<[!esi.channel<ui4> from "address", !esi.channel<i32> to "data"]>
//CHECK-NEXT: %data = esi.bundle.unpack %main.mem_ld0.addr from [[B0]] : !esi.bundle<[!esi.channel<i4> from "address", !esi.channel<i32> to "data"]> //CHECK-NEXT: %data = esi.bundle.unpack %main.mem_ld0.addr from [[B0]] : !esi.bundle<[!esi.channel<ui4> from "address", !esi.channel<i32> to "data"]>
//CHECK-NEXT: [[B1:%.+]] = esi.service.req <@mem::@write>(#esi.appid<"store"[2]>) : !esi.bundle<[!esi.channel<!hw.struct<address: i4, data: i32>> from "req", !esi.channel<i0> to "ack"]> //CHECK-NEXT: [[B1:%.+]] = esi.service.req <@mem::@write>(#esi.appid<"store"[2]>) : !esi.bundle<[!esi.channel<!hw.struct<address: ui4, data: i32>> from "req", !esi.channel<i0> to "ack"]>
//CHECK-NEXT: %ack = esi.bundle.unpack %main.mem_st0 from [[B1]] : !esi.bundle<[!esi.channel<!hw.struct<address: i4, data: i32>> from "req", !esi.channel<i0> to "ack"]> //CHECK-NEXT: %ack = esi.bundle.unpack %main.mem_st0 from [[B1]] : !esi.bundle<[!esi.channel<!hw.struct<address: ui4, data: i32>> from "req", !esi.channel<i0> to "ack"]>
//CHECK-NEXT: %main.out0, %main.mem_ld0.addr, %main.mem_st0 = hw.instance "main" @__main_hw(arg0: %arg0: !esi.channel<i64>, arg1: %arg1: !esi.channel<i64>, v: %v: !esi.channel<i32>, mem_ld0.data: %data: !esi.channel<i32>, mem_st0.done: %ack: !esi.channel<i0>, argCtrl: %argCtrl: !esi.channel<i0>, clock: %clock: !seq.clock, reset: %reset: i1) -> (out0: !esi.channel<i0>, mem_ld0.addr: !esi.channel<i4>, mem_st0: !esi.channel<!hw.struct<address: i4, data: i32>>) //CHECK-NEXT: %main.out0, %main.mem_ld0.addr, %main.mem_st0 = hw.instance "main" @__main_hw(arg0: %arg0: !esi.channel<i64>, arg1: %arg1: !esi.channel<i64>, v: %v: !esi.channel<i32>, mem_ld0.data: %data: !esi.channel<i32>, mem_st0.done: %ack: !esi.channel<i0>, argCtrl: %argCtrl: !esi.channel<i0>, clock: %clock: !seq.clock, reset: %reset: i1) -> (out0: !esi.channel<i0>, mem_ld0.addr: !esi.channel<i4>, mem_st0: !esi.channel<!hw.struct<address: i4, data: i32>>)
//CHECK-NEXT: hw.output %main.out0 : !esi.channel<i0> //CHECK-NEXT: hw.output %main.out0 : !esi.channel<i0>
//CHECK-NEXT: } //CHECK-NEXT: }