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

DspReal usage #92

Open
shunshou opened this issue Mar 30, 2017 · 10 comments
Open

DspReal usage #92

shunshou opened this issue Mar 30, 2017 · 10 comments
Assignees

Comments

@shunshou
Copy link
Member

Somewhat on the lines of what @stevobailey and @grebe did -- If we're using Verilator or VCS as the main mode of testing with Dsp-y things, it might make sense to have all DspReal nodes be 1 bit ports (since there's no notion of signed/unsigned there). The nodes should then be (via annotation or otherwise w/ the IfDef stuff) typed as "real" or not for simulation vs. synthesis.

I don't think it's correct to do this on an AnalogType, because, really, AnalogType is more talking about directionality rather than how to represent the data.

Opinions?

@stevobailey
Copy link
Contributor

For those playing the Verilog game at home, this makes DspReal basically the same as Verilog real. As before, it's not synthesizeable, but it is testable in VCS/NCSim.

Also, AnalogType has somewhat to do with directionality, but I think of it more as how the tools handle the signal. AnalogTypes should never be buffered or modified.

@shunshou
Copy link
Member Author

shunshou commented Apr 2, 2017

@chick Would this be possible to do? Add some kind of DspOption (annotation) that turns all DspReal's into 1 wire UInts (to Chisel), and in the Verilog emitter, (another option) output as in/out real port_name?

We need 1 wire for synthesis and we need 1 wire + "real" for VCS simulation using 1 wire (rather than 64). In doing so, it'd also be necessary to feed in a different BlackBoxFloat that doesn't have all of the fromBits/toBits stuff (it should just stay in the real domain for non-synthesizable simulation).

I need something like this ASAP so I can easily translate my Chisel tests to something that's actually compatible with tapeout...

@chick
Copy link
Contributor

chick commented Apr 3, 2017

@shunshou It sounds do-able, I'm looking into it.

@chick
Copy link
Contributor

chick commented Apr 5, 2017

@shunshou I am going to say yes, I think this is possible and I will work on it right away. Can you give me any example code with some identifying comments on what things this new transform should be applied to. An example of the verilog output desired would be helpful too

@grebe
Copy link
Contributor

grebe commented Apr 5, 2017

Perhaps the type in the DspReal blackboxes should just be Analog. Here is what we did to firrtl to make Analog work with real in simulation (warning: it's a hack).

I added a var that controlled verilog emission for firrtl analog types here. This should be done in a more clean way- maybe we should pull in @azidar here. I think the right solution is to replace Analog nodes with some working IR types that have control over what verilog they emit.

Then we added a transform+annotation to barstools here (it probably belongs somewhere else). We define a mixin here that adds `ifndef SYNTHESIS real `endif to the declaration of anything analog.

This is all pretty hackish and I would really like to have a clean way to control verilog emission via annotations.

On a side note, is there a good function for barstools to call to look at annotations and load the appropriate transform classes? I'm sure it must exist in firrtl or chisel, I just don't know where.

@shunshou
Copy link
Member Author

shunshou commented Apr 5, 2017

I'm pretty sure, from our discussion last night, that you just need the line from the Driver that we looked at and it should work. I think most of the Generator stuff should be overhauled though... because Firrtl is a lot friendlier than when it was made.

@chick
Copy link
Contributor

chick commented Apr 5, 2017

@shunshou what line from the Driver, and which Generator are you referring to.

@shunshou
Copy link
Member Author

shunshou commented Apr 6, 2017

@chick
Copy link
Contributor

chick commented Apr 6, 2017

@grebe @shunshou I have a branch of firrtl that ports your hack and cleans it up a little. I don't think I quite see how the ifdef SYNTHESIS works, it looks to me like all that is replaced is the type name so If I do that substitution. When I test with this I get

circuit Test :
  module Test :
    input a : Analog<16>
    output b : Analog<16>
    wire x : UInt<32>

becomes

module Test(
  input
`ifndef SYNTHESIS
  real
`endif
         [15:0] a,
  input
`ifndef SYNTHESIS
  real
`endif
         [15:0] b
);
  wire [31:0] x;
  assign x = 32'h0;
endmodule

I am probably missing something here. I also am not that clear about whether this solves @shunshou 's urgent problem. Please advise

@shunshou
Copy link
Member Author

shunshou commented Apr 6, 2017 via email

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

4 participants