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.

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

[RFC] Bit Pattern Support in coreir #946

Open
leonardt opened this issue Aug 20, 2020 · 6 comments
Open

[RFC] Bit Pattern Support in coreir #946

leonardt opened this issue Aug 20, 2020 · 6 comments
Assignees

Comments

@leonardt
Copy link
Collaborator

Magma is looking to add support for the notion of a Bit Pattern ala chisel (see https://github.com/ucb-bar/riscv-mini/blob/release/src/main/scala/Instructions.scala#L8-L71 for an example).

This would allow us to emulate the variety of case statements found in verilog (see phanrahan/magma#229 for some context and related issues). In order to this, CoreIR needs to support some representation of these patterns in the IR (and so will the verilog code generator).

One natural option might be to extend the coreir const primitive to be a more general bit pattern (a const as we know it now could be considered a simple bit pattern). Or we could add another primitive specifically for this (this might be simpler to implement as it doesn't require changing const in anyway). Any thoughts?

@rdaly525
Copy link
Owner

rdaly525 commented Aug 24, 2020

Seems like we have a couple options:
a) represent a number like 4'b1?1?' as a coreir BitVector constant
a1) extend BitVector Values to allow '?'
a2) extend BitVector Values to be '4 valued' (X,Z,0, 1) and use "X" to mean "?" in the context of the case-muxes.
b) use the string "1?1?" and do parsing/determine semantics in the generator

We should not define a new primitive.

This also somewhat relates to a goal of mine to replace all modargs/configargs with 'Const' types in the module's interface.

@leonardt
Copy link
Collaborator Author

For b), would that mean that the const generator understands how to parse a string argument? I think that would be fine and much simpler to implement for synthesis (i.e. verilog codegen). I think a) would be the way to go for supporting it in the simulator (i.e. the BitVector constant would need to support "don't care" or have to special case the primitive in some way) and is probably a better long term solution, but I would imagine is quite a bit more work.

One thing I noticed in the Chisel reference is that they use a notion of a literal value (with don't care as 0) and a mask (can be used to mask off the significant bits of interest when comparing and ignoring the don't cares). So it seems like for now, in the front-end we could implement the literal+mask logic as a temporary solution (especially if we want to go with option a since that seems like it will take some time).

@rsetaluri
Copy link
Collaborator

I think overloading the existing const generator to parse "1?1?" might be stuffing too much functionality into the same place. The real use of these bit patterns is for case's right? Perhaps we could have generators specifically for that application? In magma, we could have nicer syntax of course.

Another thought, looking at the chisel example: it looks like the real use-case is to do like "instruction decode". If we're designing for this use case from the ground up, it seems like the peak-style is what we want to evangelize? I.e. tagged unions + "assemblers". It might be better to present this kind of abstraction, and then generate bit-patterns in the verilog. It's possible that you may want bit-patterns independent of this use-case. Just a thought.

@rdaly525
Copy link
Owner

We would not need to extend const generator. We could just use a genarg/modarg with a CoreIR::Value

@leonardt
Copy link
Collaborator Author

I think there's more use cases than just instruction decode, such as a priority encoder, but I'm not sure how many they're are, @rdaly525 is probably more familiar whether this is a more general pattern beyond decode.

@rdaly525
Copy link
Owner

rdaly525 commented Sep 2, 2020

I think having a case-mux generator is a good idea even if we also separately pursue peak-style assembling. Also, in case this was not clear I was advocating the case-mux generator itself take in an genarg or modarg of "1?1?" instead of using a coreir.const.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants