Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Arc] Implement memory initializers #7559

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

[Arc] Implement memory initializers #7559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d1a0b77
Implemented fill memory initializer
fzi-hielscher Aug 28, 2024
7ee3da8
Implemented randomized mem init.
fzi-hielscher Aug 28, 2024
729bacc
Fix constraint
fzi-hielscher Aug 28, 2024
a25fffb
Add integration test
fzi-hielscher Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 119 additions & 2 deletions include/circt/Dialect/Arc/ArcOps.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,10 +281,57 @@ def CallOp : ArcOp<"call", [
}];
}

def MemoryOp : ArcOp<"memory", [MemoryEffects<[MemAlloc]>]> {
class OptionalMemoryInitializerIsCompatible
<string predicate, string mem, string init> : PredOpTrait<
"initializer type is compatible with memory type",
CPred< !strconcat("!(", predicate, ") ||",
"(::llvm::cast<::circt::arc::MemoryInitializerType>($",
init, ".getType()).isCompatible(",
"::llvm::cast<::circt::arc::MemoryType>($",
mem, ".getType())))" )>
>;

class MemoryInitializerIsCompatible<string mem, string init>
: OptionalMemoryInitializerIsCompatible <"true", mem, init>;

def MemoryOp : ArcOp<"memory", [
MemoryEffects<[MemAlloc]>,
OptionalMemoryInitializerIsCompatible<
"$_op.getNumOperands() > 0", "memory", "initializer">
]> {
let summary = "Memory";
let arguments = (ins Optional<MemoryInitializerType>:$initializer);
let results = (outs MemoryType:$memory);
let assemblyFormat = "type($memory) attr-dict";
let assemblyFormat = [{
type($memory) attr-dict (`initial` $initializer^ `:` type($initializer))?
}];

let builders = [
OpBuilder<(ins "mlir::Type":$memType), [{
build($_builder, $_state, memType, {});
}]>];
}

def InitMemoryFilledOp : ArcOp<"initmem.filled", [Pure]> {
let arguments = (ins APIntAttr:$value, UnitAttr:$repeat);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if making the word type optional and having this repeat attribute here is a good choice.
Is the motivation just to be able to reuse the same initmem.filled for memories with different word types or is there more?
I don't think having one such op per memory word type is a problem, it's a very simple op that doesn't even have a nested region.
Having the type explicitly specified seems like a bigger benefit to me and I'd also drop the repeat attribute.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for explicit types. I expect every memory is statically typed determined in Arc dialect level.

let results = (outs GenericMemoryInitializerType:$result);
let hasCustomAssemblyFormat = true;
}

def InitMemoryRandomizedOp : ArcOp<"initmem.randomized", [Pure]> {
let results = (outs GenericMemoryInitializerType:$result);
let assemblyFormat = "attr-dict";
}

def InitializeMemoryOp : ArcOp<"initialize_memory", [
MemoryEffects<[MemWrite]>,
MemoryInitializerIsCompatible<"memory", "initializer">
]> {
let arguments = (ins MemoryInitializerType:$initializer, MemoryType:$memory);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to learn more about your rationale of introducing separate init mem operations for filled, randomized, etc. vs. just having a function (either a func.func or some custom version) that as as argument the memory index/address and returns a value of the word type. So, for filled it would just return a hw.constant result and for randomized we'd have a new random operation that could even live in the seq of HW dialect to be also usable in seq initializers to model firreg with compreg and get rid of the earlier.
This approach would be more general as your returned value can depend on the memory address (but we could also not have this arg if we want it more restricted).

I'd imaging that this initializer function could then also be used to read a mem file and initialize the memory according to that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, there wasn't much of a rationale other than having something to start with that is easy to implement. I'm having a lot of trouble reasoning about the flexibility and generality we may or may not need without having anything in the fronted dialects to connect to. So, maybe this PR was a tad too premature.

Having said that, an operation like you have described (kind of like Scala's tabulate if I understand you correctly) also makes perfect sense to me. But having that on top of the arc.initmem.array I've sketched above would likely be redundant and we would have to see which one aligns better with Seq.

Random initialization is a rabbit hole on its own. I haven't made my way through the entirety of the SystemVerilog LRM's Chapter 18 yet, but it gives an impression on what we could theoretically support. Initializing memories via a single op would e.g., allow us to easily provide a PRNG seed attribute for each memory instance. Then again, I would not want to engineer this separately from register randomization.

Finally, .mem files are not really suited for indexed/addressed access. While in practice they will often simply contain one word per line, they can be quite a bit more complex. We'd have to parse the entire file into a dedicated buffer first (and somehow manage the file descriptor), and at that point I don't see the benefit over just 'atomically' parsing it into the state memory. But maybe I'm missing/misunderstanding something?

let assemblyFormat = [{
$initializer `->` $memory attr-dict
`:` qualified(type($initializer)) `,` qualified(type($memory))
}];
}

class MemoryAndDataTypesMatch<string mem, string data> : TypesMatchWith<
Expand Down Expand Up @@ -905,4 +952,74 @@ def VectorizeReturnOp : ArcOp<"vectorize.return", [
let assemblyFormat = "operands attr-dict `:` qualified(type(operands))";
}

def EnvironmentCallOp : ArcOp<"environment_call", [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not func.func? I think we can just create func.func @_arc_env_fill_randomized somewhere in the pipeline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now there is no difference. But when toying around with the runtime library implementation I encountered several things that could require separating environment calls from other functions:

  • It could be helpful to export a list of the environment calls a specific model needs to the JSON file. Potentially just as an indicator that the model does require the runtime library.
  • Having them separate gives us more control over the eventual lowering to LLVM IR functions and calls. This could come in handy to specify the linkage or deal with calls that are 'exceptional', e.g., terminating simulation.
  • At some point I probably want to do type marshaling, i.e, allow environment calls with "arbitrary" IR types and potentially multiple return values. This will require a dedicated transformation pass before LLVM lowering.

This is all purely hypothetical at the moment. And I'll move the EnvironmentCallOp and CallEnvironmentOp to another PR before landing this one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that makes sense. I think the current use case in LowerMemoryInitializer can be representable by func.func so I would first implement with func.func.

This is all purely hypothetical at the moment. And I'll move the EnvironmentCallOp and CallEnvironmentOp to another PR before landing this one.

Thanks, I appreciated that :)

FunctionOpInterface, IsolatedFromAbove, HasParent<"mlir::ModuleOp">
]> {

let arguments = (ins
SymbolNameAttr:$sym_name,
TypeAttrOf<FunctionType>:$function_type,
OptionalAttr<DictArrayAttr>:$arg_attrs,
OptionalAttr<DictArrayAttr>:$res_attrs
);
let regions = (region AnyRegion:$body);

let extraClassDeclaration = [{

static constexpr ::llvm::StringLiteral fillRandomizedSymName =
"_arc_env_fill_randomized";

static ::mlir::FunctionType getFillRandomizedType(::mlir::MLIRContext *ctx);

//===------------------------------------------------------------------===//
// FunctionOpInterface Methods
//===------------------------------------------------------------------===//

/// Returns the argument types of this function.
ArrayRef<Type> getArgumentTypes() { return getFunctionType().getInputs(); }

/// Returns the result types of this function.
ArrayRef<Type> getResultTypes() { return getFunctionType().getResults(); }

/// Returns the region on the function operation that is callable.
Region *getCallableRegion() { return nullptr; }
}];

let hasCustomAssemblyFormat = 1;
}

def CallEnvironmentOp : ArcOp<"call_environment", [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference from func.call (or sim.dpi.call) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

Pure,
CallOpInterface,
DeclareOpInterfaceMethods<SymbolUserOpInterface>
]> {

let arguments = (ins FlatSymbolRefAttr:$callee, Variadic<AnyType>:$operands);
let results = (outs Variadic<AnyType>);

let extraClassDeclaration = [{

operand_range getArgOperands() {
return getOperands();
}
MutableOperandRange getArgOperandsMutable() {
return getOperandsMutable();
}

mlir::CallInterfaceCallable getCallableForCallee() {
return (*this)->getAttrOfType<mlir::FlatSymbolRefAttr>("callee");
}

/// Set the callee for this operation.
void setCalleeFromCallable(mlir::CallInterfaceCallable callee) {
(*this)->setAttr(getCalleeAttrName(), callee.get<mlir::SymbolRefAttr>());
}
}];

let assemblyFormat = [{
$callee `(` $operands `)` attr-dict `:` functional-type($operands, results)
}];

}

#endif // CIRCT_DIALECT_ARC_ARCOPS_TD
10 changes: 9 additions & 1 deletion include/circt/Dialect/Arc/ArcPasses.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,14 @@ def LowerLUT : Pass<"arc-lower-lut", "arc::DefineOp"> {
let dependentDialects = ["hw::HWDialect", "comb::CombDialect"];
}

def LowerMemoryInitializers : Pass<"arc-lower-memory-initializers",
"mlir::ModuleOp"> {
let summary = "Converts InitializeMemory ops to state writes";
let dependentDialects = [
"arc::ArcDialect", "mlir::arith::ArithDialect", "mlir::scf::SCFDialect"
];
}

def LowerState : Pass<"arc-lower-state", "mlir::ModuleOp"> {
let summary = "Split state into read and write ops grouped by clock tree";
let constructor = "circt::arc::createLowerStatePass()";
Expand All @@ -219,7 +227,7 @@ def LowerVectorizations : Pass<"arc-lower-vectorizations", "mlir::ModuleOp"> {
This pass lowers `arc.vectorize` operations. By default, the operation will
be fully lowered (i.e., the op disappears in the IR). Alternatively, it can
be partially lowered.

The "mode" pass option allows to only lower the boundary, only the body, or
only inline the body given that both the boundary and the body are already
lowered.
Expand Down
31 changes: 31 additions & 0 deletions include/circt/Dialect/Arc/ArcTypes.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,37 @@ def MemoryType : ArcTypeDef<"Memory"> {
}];
}

def MemoryInitializerType : ArcTypeDef<"MemoryInitializer"> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im curious if we can just use !seq.initial<!hw.array<...>> type (or maybe just !hw.array<...>) as an initializer. With that approach we don't have to introduce special types and operations for memory initializer. I can work on migrating !seq.initial+!hw.array later so I don't block the PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment below.

let mnemonic = "memory_initializer";

let parameters = (ins OptionalParameter<"unsigned">:$numWords,
OptionalParameter<"::mlir::IntegerType">:$wordType);
Comment on lines +49 to +50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By not specifying the respective attribute the initializer can be used for memories of all depths and/or word types.


let hasCustomAssemblyFormat = true;

let extraClassDeclaration = [{
bool isCompatible(::circt::arc::MemoryType memType) const {
if (getNumWords() > 0 && getNumWords() != memType.getNumWords())
return false;
if (!!getWordType() && getWordType() != memType.getWordType())
return false;
return true;
}

bool isGeneric() const { return getNumWords() == 0 && !getWordType(); }

}];
}

def GenericMemoryInitializerType : DialectType<ArcDialect,
CPred<[{
::llvm::isa<::circt::arc::MemoryInitializerType>($_self) &&
::llvm::cast<::circt::arc::MemoryInitializerType>($_self).isGeneric()
}]>, "must be a generic memory initializer type">,
BuildableType<
"::circt::arc::MemoryInitializerType::get($_builder.getContext(), 0, {})"
> {}

def StorageType : ArcTypeDef<"Storage"> {
let mnemonic = "storage";
let parameters = (ins OptionalParameter<"unsigned">:$size);
Expand Down
155 changes: 155 additions & 0 deletions integration_test/arcilator/JIT/initial-ram.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
// RUN: arcilator %s --run --jit-entry=main | FileCheck %s
// REQUIRES: arcilator-jit

// Lit testing random values is iffy, but the runtime environment should ensure
// reproducible results across runs and platforms.

// CHECK-LABEL: - addr = 21
// CHECK-NEXT: rndA = 707ca895977cf11
// CHECK-NEXT: rndB = 28e9cfdfcf6b898
// CHECK-NEXT: fill = cafe
// CHECK-NEXT: rept = 8000400020001
// CHECK-NEXT: - addr = 0
// CHECK-NEXT: rndA = 5160879eac03cbb
// CHECK-NEXT: rndB = d78aeb0b84b4823
// CHECK-NEXT: fill = cafe
// CHECK-NEXT: rept = 8000400020001
// CHECK-NEXT: - addr = 1ff
// CHECK-NEXT: rndA = 198ecb046b4841d
// CHECK-NEXT: rndB = 357020a9a09635b
// CHECK-NEXT: fill = cafe
// CHECK-NEXT: rept = 8000400020001
// CHECK-NEXT: - addr = aa
// CHECK-NEXT: rndA = 16b4a44c8c8ce64
// CHECK-NEXT: rndB = 476fc6a9fd6fb83
// CHECK-NEXT: fill = cafe
// CHECK-NEXT: rept = 8000400020001

module {
arc.define @mem_write(%arg0: i9, %arg1: i60, %arg2: i1) -> (i9, i60, i1) {
arc.output %arg0, %arg1, %arg2 : i9, i60, i1
}
hw.module @SyncRAM(
in %clk : i1, in %reset : i1, in %en : i1, in %addr : i9, in %din : i60, in %wen : i1,
out dout0 : i60, out dout1 : i60, out dout2 : i60, out dout3 : i60, out addrOut : i9) {
%clock = seq.to_clock %clk

%cst33_i9 = hw.constant 33 : i9

%randInit = arc.initmem.randomized
// Check that identical memories get different initial values
%mem0 = arc.memory <512 x i60, i9> initial %randInit : !arc.memory_initializer<* x *>
%mem3 = arc.memory <512 x i60, i9> initial %randInit : !arc.memory_initializer<* x *>

%fillInit = arc.initmem.filled 0xcafe : i16
%mem1 = arc.memory <512 x i60, i9> initial %fillInit : !arc.memory_initializer<* x *>

%repeatInit = arc.initmem.filled repeat 1 : i17
%mem2 = arc.memory <512 x i60, i9> initial %repeatInit : !arc.memory_initializer<* x *>



%addrReg = seq.compreg %0, %clock powerOn %cst33_i9 : i9
%0 = comb.mux bin %en, %addr, %addrReg : i9

%3 = seq.compreg %en, %clock : i1

%rd0 = arc.memory_read_port %mem0[%addrReg] : <512 x i60, i9>
%rd1 = arc.memory_read_port %mem1[%addrReg] : <512 x i60, i9>
%rd2 = arc.memory_read_port %mem2[%addrReg] : <512 x i60, i9>
%rd3 = arc.memory_read_port %mem3[%addrReg] : <512 x i60, i9>

%c0_i60 = hw.constant 0 : i60

arc.memory_write_port %mem0, @mem_write(%addrReg, %din, %wen) clock %clock enable latency 1 : <512 x i60, i9>, i9, i60, i1
arc.memory_write_port %mem1, @mem_write(%addrReg, %din, %wen) clock %clock enable latency 1 : <512 x i60, i9>, i9, i60, i1
arc.memory_write_port %mem2, @mem_write(%addrReg, %din, %wen) clock %clock enable latency 1 : <512 x i60, i9>, i9, i60, i1
arc.memory_write_port %mem3, @mem_write(%addrReg, %din, %wen) clock %clock enable latency 1 : <512 x i60, i9>, i9, i60, i1

hw.output %rd0, %rd1, %rd2, %rd3, %addrReg : i60, i60, i60, i60 , i9
}

func.func @main() {
%cst0 = arith.constant 0 : i9
%cst1ff = arith.constant 0x1FF : i9
%cstaa = arith.constant 0xAA : i9

%false = arith.constant 0 : i1
%true = arith.constant 1 : i1

arc.sim.instantiate @SyncRAM as %model {
%addr0 = arc.sim.get_port %model, "addrOut" : i9, !arc.sim.instance<@SyncRAM>
%res0_0 = arc.sim.get_port %model, "dout0" : i60, !arc.sim.instance<@SyncRAM>
%res1_0 = arc.sim.get_port %model, "dout1" : i60, !arc.sim.instance<@SyncRAM>
%res2_0 = arc.sim.get_port %model, "dout2" : i60, !arc.sim.instance<@SyncRAM>
%res3_0 = arc.sim.get_port %model, "dout3" : i60, !arc.sim.instance<@SyncRAM>
arc.sim.emit " - addr", %addr0 : i9
arc.sim.emit "rndA", %res0_0 : i60
arc.sim.emit "rndB", %res3_0 : i60
arc.sim.emit "fill", %res1_0 : i60
arc.sim.emit "rept", %res2_0 : i60

arc.sim.set_input %model, "en" = %true : i1, !arc.sim.instance<@SyncRAM>
arc.sim.set_input %model, "wen" = %false : i1, !arc.sim.instance<@SyncRAM>
arc.sim.set_input %model, "reset" = %false : i1, !arc.sim.instance<@SyncRAM>

arc.sim.set_input %model, "addr" = %cst0 : i9, !arc.sim.instance<@SyncRAM>

arc.sim.set_input %model, "clk" = %false : i1, !arc.sim.instance<@SyncRAM>
arc.sim.step %model : !arc.sim.instance<@SyncRAM>

arc.sim.set_input %model, "clk" = %true : i1, !arc.sim.instance<@SyncRAM>
arc.sim.step %model : !arc.sim.instance<@SyncRAM>
arc.sim.set_input %model, "clk" = %false : i1, !arc.sim.instance<@SyncRAM>
arc.sim.step %model : !arc.sim.instance<@SyncRAM>

%addr1 = arc.sim.get_port %model, "addrOut" : i9, !arc.sim.instance<@SyncRAM>
%res0_1 = arc.sim.get_port %model, "dout0" : i60, !arc.sim.instance<@SyncRAM>
%res1_1 = arc.sim.get_port %model, "dout1" : i60, !arc.sim.instance<@SyncRAM>
%res2_1 = arc.sim.get_port %model, "dout2" : i60, !arc.sim.instance<@SyncRAM>
%res3_1 = arc.sim.get_port %model, "dout3" : i60, !arc.sim.instance<@SyncRAM>
arc.sim.emit " - addr", %addr1 : i9
arc.sim.emit "rndA", %res0_1 : i60
arc.sim.emit "rndB", %res3_1 : i60
arc.sim.emit "fill", %res1_1 : i60
arc.sim.emit "rept", %res2_1 : i60

arc.sim.set_input %model, "addr" = %cst1ff : i9, !arc.sim.instance<@SyncRAM>

arc.sim.set_input %model, "clk" = %true : i1, !arc.sim.instance<@SyncRAM>
arc.sim.step %model : !arc.sim.instance<@SyncRAM>
arc.sim.set_input %model, "clk" = %false : i1, !arc.sim.instance<@SyncRAM>
arc.sim.step %model : !arc.sim.instance<@SyncRAM>

%addr2 = arc.sim.get_port %model, "addrOut" : i9, !arc.sim.instance<@SyncRAM>
%res0_2 = arc.sim.get_port %model, "dout0" : i60, !arc.sim.instance<@SyncRAM>
%res1_2 = arc.sim.get_port %model, "dout1" : i60, !arc.sim.instance<@SyncRAM>
%res2_2 = arc.sim.get_port %model, "dout2" : i60, !arc.sim.instance<@SyncRAM>
%res3_2 = arc.sim.get_port %model, "dout3" : i60, !arc.sim.instance<@SyncRAM>
arc.sim.emit " - addr", %addr2 : i9
arc.sim.emit "rndA", %res0_2 : i60
arc.sim.emit "rndB", %res3_2 : i60
arc.sim.emit "fill", %res1_2 : i60
arc.sim.emit "rept", %res2_2 : i60

arc.sim.set_input %model, "addr" = %cstaa : i9, !arc.sim.instance<@SyncRAM>

arc.sim.set_input %model, "clk" = %true : i1, !arc.sim.instance<@SyncRAM>
arc.sim.step %model : !arc.sim.instance<@SyncRAM>
arc.sim.set_input %model, "clk" = %false : i1, !arc.sim.instance<@SyncRAM>
arc.sim.step %model : !arc.sim.instance<@SyncRAM>

%addr3 = arc.sim.get_port %model, "addrOut" : i9, !arc.sim.instance<@SyncRAM>
%res0_3 = arc.sim.get_port %model, "dout0" : i60, !arc.sim.instance<@SyncRAM>
%res1_3 = arc.sim.get_port %model, "dout1" : i60, !arc.sim.instance<@SyncRAM>
%res2_3 = arc.sim.get_port %model, "dout2" : i60, !arc.sim.instance<@SyncRAM>
%res3_3 = arc.sim.get_port %model, "dout3" : i60, !arc.sim.instance<@SyncRAM>
arc.sim.emit " - addr", %addr3 : i9
arc.sim.emit "rndA", %res0_3 : i60
arc.sim.emit "rndB", %res3_3 : i60
arc.sim.emit "fill", %res1_3 : i60
arc.sim.emit "rept", %res2_3 : i60
}
return
}
}
Loading
Loading