dsptools icon indicating copy to clipboard operation
dsptools copied to clipboard

DspReal usage

Open shunshou opened this issue 8 years ago • 10 comments

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?

shunshou avatar Mar 30 '17 20:03 shunshou

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.

stevobailey avatar Mar 30 '17 23:03 stevobailey

@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...

shunshou avatar Apr 02 '17 19:04 shunshou

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

chick avatar Apr 03 '17 16:04 chick

@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

chick avatar Apr 05 '17 16:04 chick

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.

grebe avatar Apr 05 '17 17:04 grebe

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.

shunshou avatar Apr 05 '17 18:04 shunshou

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

chick avatar Apr 05 '17 18:04 chick

https://github.com/ucb-bar/chisel3/blob/master/src/main/scala/chisel3/Driver.scala#L151 (unrelated to DspReal)

shunshou avatar Apr 06 '17 02:04 shunshou

@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

chick avatar Apr 06 '17 21:04 chick

It doesn't. I don't want to use Analog, because I can't use the chisel testers to simulate. I want to conditionally adapt DSPReal and modify the testers to allow you to feed in real numbers, not their 64 bit representations.

On Thursday, April 6, 2017, Chick Markley [email protected] wrote:

@grebe https://github.com/grebe @shunshou https://github.com/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 https://github.com/shunshou 's urgent problem. Please advise

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ucb-bar/dsptools/issues/92#issuecomment-292336126, or mute the thread https://github.com/notifications/unsubscribe-auth/AGTTFr64_n6nfRT4Wv9WcG-JRTHK9Si0ks5rtV_UgaJpZM4Mu4Nt .

shunshou avatar Apr 06 '17 22:04 shunshou