[FIRRTL] Make GroupID memory attribute unsigned. NFC (#2975)

Make the GroupID memory attribute as unsigned int, instead of signed.
It is being used to distinguish memories that should not be Deduped.
There are two passes currently updating the GroupID:
PrefixModules pass assigns unique id for each prefix.
CreateSifiveMetadata pass assigns unique id for each memory under
the testharness hierarchy.
This change ensures consistent assumptions and a convention.
This commit is contained in:
Prithayan Barua 2022-04-26 22:18:23 -07:00 committed by GitHub
parent 24fd17d0c9
commit cdf32d1bc4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 18 additions and 18 deletions

View File

@ -103,7 +103,7 @@ def MemOp : FIRRTLOp<"mem", [HasCustomSSAName]> {
AnnotationArrayAttr:$annotations,
PortAnnotationsAttr:$portAnnotations,
OptionalAttr<SymbolNameAttr>:$inner_sym,
OptionalAttr<I32Attr>:$groupID);
OptionalAttr<UI32Attr>:$groupID);
let results = (outs Variadic<FIRRTLType>:$results);
let assemblyFormat = [{

View File

@ -157,13 +157,13 @@ struct FirMemory {
SmallVector<int32_t> writeClockIDs;
StringAttr modName;
bool isMasked;
size_t groupID;
uint32_t groupID;
// Location is carried along but not considered part of the identity of this.
Location loc;
std::tuple<size_t, size_t, size_t, size_t, size_t, size_t, size_t, size_t,
size_t, hw::WUW, SmallVector<int32_t>, size_t>
size_t, hw::WUW, SmallVector<int32_t>, uint32_t>
getTuple() const {
return std::tie(numReadPorts, numWritePorts, numReadWritePorts, dataWidth,
depth, readLatency, writeLatency, maskBits, readUnderWrite,

View File

@ -1843,9 +1843,9 @@ FirMemory MemOp::getSummary() {
op.emitError("'firrtl.mem' should have simple type and known width");
width = 0;
}
size_t groupID = 0;
uint32_t groupID = 0;
if (auto gID = op.groupIDAttr())
groupID = gID.getInt();
groupID = gID.getUInt();
StringAttr modName;
if (op->hasAttr("modName"))
modName = op->getAttrOfType<StringAttr>("modName");

View File

@ -65,13 +65,13 @@ void CreateSiFiveMetadataPass::renameMemory(CircuitOp circuitOp) {
// This is a random number to start the groupIDs at, large enough to not
// conflict with existing IDs.
// TODO: Move this logic out of this pass.
unsigned baseGroupID = std::numeric_limits<unsigned>::max();
uint32_t baseGroupID = std::numeric_limits<uint32_t>::max();
for (auto mod : circuitOp.getOps<FModuleOp>()) {
bool isTestHarness = !dutModuleSet.contains(mod);
for (auto memOp : mod.getBody()->getOps<MemOp>()) {
if (isTestHarness)
memOp.groupIDAttr(
IntegerAttr::get(IntegerType::get(ctxt, 32), --baseGroupID));
memOp.groupIDAttr(IntegerAttr::get(
IntegerType::get(ctxt, 32, IntegerType::Unsigned), --baseGroupID));
memOpList.push_back(memOp);
auto firMem = memOp.getSummary();

View File

@ -105,7 +105,7 @@ class PrefixModulesPass : public PrefixModulesBase<PrefixModulesPass> {
/// This is a map from a module name to new prefixes to be applied.
PrefixMap prefixMap;
DenseMap<StringRef, size_t> prefixIdMap;
DenseMap<StringRef, uint32_t> prefixIdMap;
/// A map of Grand Central interface ID to prefix.
DenseMap<Attribute, std::string> interfacePrefixMap;
@ -125,7 +125,7 @@ class PrefixModulesPass : public PrefixModulesBase<PrefixModulesPass> {
/// any referenced module in the prefix map.
void PrefixModulesPass::renameModuleBody(std::string prefix, FModuleOp module) {
auto *context = module.getContext();
size_t groupID = 0;
uint32_t groupID = 0;
auto iter = prefixIdMap.find(prefix);
if (iter == prefixIdMap.end()) {
groupID = prefixIdMap.size() + 1;
@ -176,8 +176,8 @@ void PrefixModulesPass::renameModuleBody(std::string prefix, FModuleOp module) {
if (auto memOp = dyn_cast<MemOp>(op)) {
// Memories will be turned into modules and should be prefixed.
memOp.nameAttr(StringAttr::get(context, prefix + memOp.name()));
memOp.groupIDAttr(
IntegerAttr::get(IntegerType::get(context, 32), groupID));
memOp.groupIDAttr(IntegerAttr::get(
IntegerType::get(context, 32, IntegerType::Unsigned), groupID));
} else if (auto instanceOp = dyn_cast<InstanceOp>(op)) {
auto target = dyn_cast<FModuleLike>(
*instanceGraph->getReferencedModule(instanceOp));

View File

@ -6,7 +6,7 @@ firrtl.circuit "TLRAM" {
firrtl.module @TLRAM(in %clock: !firrtl.clock, in %reset: !firrtl.asyncreset, in %index: !firrtl.uint<4>, in %index2: !firrtl.uint<4>, in %data_0: !firrtl.uint<8>, in %wen: !firrtl.uint<1>, in %_T_29: !firrtl.uint<1>, out %auto_0: !firrtl.uint<8>) {
%mem_MPORT_en = firrtl.wire : !firrtl.uint<1>
%mem_MPORT_data_0 = firrtl.wire : !firrtl.uint<8>
%mem_0_MPORT, %mem_0_MPORT_1 = firrtl.mem Undefined {depth = 16 : i64, groupID = 2 : i32, name = "mem_0", portNames = ["MPORT", "MPORT_1"], readLatency = 1 : i32, writeLatency = 1 : i32} : !firrtl.bundle<addr: uint<4>, en: uint<1>, clk: clock, data flip: uint<8>>, !firrtl.bundle<addr: uint<4>, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>
%mem_0_MPORT, %mem_0_MPORT_1 = firrtl.mem Undefined {depth = 16 : i64, groupID = 2 : ui32, name = "mem_0", portNames = ["MPORT", "MPORT_1"], readLatency = 1 : i32, writeLatency = 1 : i32} : !firrtl.bundle<addr: uint<4>, en: uint<1>, clk: clock, data flip: uint<8>>, !firrtl.bundle<addr: uint<4>, en: uint<1>, clk: clock, data: uint<8>, mask: uint<1>>
%0 = firrtl.subfield %mem_0_MPORT(0) : (!firrtl.bundle<addr: uint<4>, en: uint<1>, clk: clock, data flip: uint<8>>) -> !firrtl.uint<4>
firrtl.connect %0, %index2 : !firrtl.uint<4>, !firrtl.uint<4>
%1 = firrtl.subfield %mem_0_MPORT(1) : (!firrtl.bundle<addr: uint<4>, en: uint<1>, clk: clock, data flip: uint<8>>) -> !firrtl.uint<1>
@ -35,7 +35,7 @@ firrtl.circuit "TLRAM" {
%11 = firrtl.mux(%REG, %mem_MPORT_data_0, %r_0) : (!firrtl.uint<1>, !firrtl.uint<8>, !firrtl.uint<8>) -> !firrtl.uint<8>
firrtl.connect %auto_0, %11 : !firrtl.uint<8>, !firrtl.uint<8>
// CHECK: %mem_0_rw = firrtl.mem Undefined {depth = 16 : i64, groupID = 2 : i32, name = "mem_0", portNames = ["rw"], readLatency = 1 : i32, writeLatency = 1 : i32} : !firrtl.bundle<addr: uint<4>, en: uint<1>, clk: clock, rdata flip: uint<8>, wmode: uint<1>, wdata: uint<8>, wmask: uint<1>>
// CHECK: %mem_0_rw = firrtl.mem Undefined {depth = 16 : i64, groupID = 2 : ui32, name = "mem_0", portNames = ["rw"], readLatency = 1 : i32, writeLatency = 1 : i32} : !firrtl.bundle<addr: uint<4>, en: uint<1>, clk: clock, rdata flip: uint<8>, wmode: uint<1>, wdata: uint<8>, wmask: uint<1>>
// CHECK: %[[v7:.+]] = firrtl.mux(%[[writeEnable:.+]], %[[writeAddr:.+]], %[[readAddr:.+]]) : (!firrtl.uint<1>, !firrtl.uint<4>, !firrtl.uint<4>) -> !firrtl.uint<4>
// CHECK: firrtl.strictconnect %[[v0:.+]], %[[v7]] : !firrtl.uint<4>
// CHECK: %[[v8:.+]] = firrtl.or %[[readEnable:.+]], %[[writeEnable]] : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<1>

View File

@ -1488,14 +1488,14 @@ firrtl.module private @Issue2315(in %x: !firrtl.vector<uint<10>, 5>, in %source:
// Ensure 0 bit fields are handled properly.
firrtl.module @ZeroWideMem(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>) {
%ram_MPORT = firrtl.mem Undefined {depth = 4 : i64, groupID = 1 : i32, name = "ram", portNames = ["MPORT"], readLatency = 0 : i32, writeLatency = 1 : i32} : !firrtl.bundle<addr: uint<2>, en: uint<1>, clk: clock, data: bundle<entry: bundle<a: uint<0>, b: uint<20>, c: uint<42>>>, mask: bundle<entry: bundle<a: uint<1>, b: uint<1>, c: uint<1>>>>
%ram_MPORT = firrtl.mem Undefined {depth = 4 : i64, groupID = 1 : ui32, name = "ram", portNames = ["MPORT"], readLatency = 0 : i32, writeLatency = 1 : i32} : !firrtl.bundle<addr: uint<2>, en: uint<1>, clk: clock, data: bundle<entry: bundle<a: uint<0>, b: uint<20>, c: uint<42>>>, mask: bundle<entry: bundle<a: uint<1>, b: uint<1>, c: uint<1>>>>
//FLATTEN %ram_MPORT = firrtl.mem Undefined {depth = 4 : i64, name = "ram", portNames = ["MPORT"], readLatency = 0 : i32, writeLatency = 1 : i32} : !firrtl.bundle<addr: uint<2>, en: uint<1>, clk: clock, data: uint<62>, mask: uint<31>>
}
firrtl.module @ZeroBitMasks(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>, in %io: !firrtl.bundle<a: uint<0>, b: uint<20>>) {
%invalid = firrtl.invalidvalue : !firrtl.bundle<a: uint<1>, b: uint<1>>
%invalid_0 = firrtl.invalidvalue : !firrtl.bundle<a: uint<0>, b: uint<20>>
%ram_MPORT = firrtl.mem Undefined {depth = 1 : i64, groupID = 1 : i32, name = "ram", portNames = ["MPORT"], readLatency = 0 : i32, writeLatency = 1 : i32} : !firrtl.bundle<addr: uint<1>, en: uint<1>, clk: clock, data: bundle<a: uint<0>, b: uint<20>>, mask: bundle<a: uint<1>, b: uint<1>>>
%ram_MPORT = firrtl.mem Undefined {depth = 1 : i64, groupID = 1 : ui32, name = "ram", portNames = ["MPORT"], readLatency = 0 : i32, writeLatency = 1 : i32} : !firrtl.bundle<addr: uint<1>, en: uint<1>, clk: clock, data: bundle<a: uint<0>, b: uint<20>>, mask: bundle<a: uint<1>, b: uint<1>>>
%3 = firrtl.subfield %ram_MPORT(3) : (!firrtl.bundle<addr: uint<1>, en: uint<1>, clk: clock, data: bundle<a: uint<0>, b: uint<20>>, mask: bundle<a: uint<1>, b: uint<1>>>) -> !firrtl.bundle<a: uint<0>, b: uint<20>>
firrtl.strictconnect %3, %invalid_0 : !firrtl.bundle<a: uint<0>, b: uint<20>>
%4 = firrtl.subfield %ram_MPORT(4) : (!firrtl.bundle<addr: uint<1>, en: uint<1>, clk: clock, data: bundle<a: uint<0>, b: uint<20>>, mask: bundle<a: uint<1>, b: uint<1>>>) -> !firrtl.bundle<a: uint<1>, b: uint<1>>

View File

@ -114,9 +114,9 @@ circuit Foo : %[[
readData <= _readData_T
; CHECK-LABEL: firrtl.module private @prefix1_Bar
; CHECK: = firrtl.mem Undefined {depth = 8 : i64, groupID = -2 : i32, modName = "prefix1_mem_ext", name = "prefix1_mem", portNames = ["MPORT", "readData_MPORT"], readLatency = 1 : i32, writeLatency = 1 : i32}
; CHECK: = firrtl.mem Undefined {depth = 8 : i64, groupID = 4294967294 : ui32, modName = "prefix1_mem_ext", name = "prefix1_mem", portNames = ["MPORT", "readData_MPORT"], readLatency = 1 : i32, writeLatency = 1 : i32}
; CHECK-LABEL: firrtl.module private @prefix2_Baz
; CHECK: = firrtl.mem Undefined {depth = 8 : i64, groupID = -3 : i32, modName = "prefix2_mem_ext", name = "prefix2_mem", portNames = ["MPORT", "readData_MPORT"], readLatency = 1 : i32, writeLatency = 1 : i32}
; CHECK: = firrtl.mem Undefined {depth = 8 : i64, groupID = 4294967293 : ui32, modName = "prefix2_mem_ext", name = "prefix2_mem", portNames = ["MPORT", "readData_MPORT"], readLatency = 1 : i32, writeLatency = 1 : i32}
; HW-LABEL: hw.module @prefix2_mem_ext(%R0_addr: i3, %R0_en: i1, %R0_clk: i1, %W0_addr: i3, %W0_en: i1, %W0_clk: i1, %W0_data: i1) -> (R0_data: i1)
; HW-LABEL: hw.module @prefix1_mem_ext(%R0_addr: i3, %R0_en: i1, %R0_clk: i1, %W0_addr: i3, %W0_en: i1, %W0_clk: i1, %W0_data: i1) -> (R0_data: i1)