SpinalHDL icon indicating copy to clipboard operation
SpinalHDL copied to clipboard

changes for output assigns

Open lsteveol opened this issue 5 years ago • 2 comments

This is a hopeful solution to https://github.com/SpinalHDL/SpinalHDL/issues/251 that I raised.

ComponentEmitterVerilog.scala

  • Added a check where if an output of a subcomponent was connect to an output of the current component it would set the referencesOverrides() to the component port. This removes the need for the additional assign statement.
  • subcomponent outputs that are only used internally to the current component should still produce a wire as expected.
  • Added formatting to the portMap in emitEntity. Before adding to the portMaps, it will figure up the spacing.

Spinal.scala

  • Added a config parameter called 'removeOutputAssigns'. Defaults to false, keeping the original functionality where outputs are set through assigns.

Here is an example of the input Spinal and corresponding output Verilogs

class MyBB extends BlackBox {
  val thisin  = in Bool
  val thisout = out Bool
}

class MyBottom extends MrvnComponent{
  val somein = in Bool
  val somein2 = in Bool
  val someout = out Bool
  val someout2 = out Bool
  
  someout  := somein & somein2
  someout2 := somein | somein2
  
  val bb = new MyBB
  bb.thisin := somein
}

class MyTop extends MrvnComponent{
  val in1 = in Bool
  val in2 = in Bool
  val in3 = in Bool
  
  val out1 = out Bool
  val out2 = out Bool
  val out3 = out Bool
  val out4 = out Bool

  val bot1 = new MyBottom
  val bot2 = new MyBottom
  
  bot1.somein   := in1
  bot1.somein2  := in2
  out1 := bot1.someout
  
  bot2.somein   := in2
  bot2.somein2  := in3
  out2 := bot2.someout
  
  out3 := bot1.someout2 ^ bot2.someout2
  
  out4 := bot1.someout2
}

Normal/Default - removeOutputAssigns=false

module MyBottom (
      input    somein,
      input    somein2,
      output   someout,
      output   someout2);
  wire  bb_thisout;
  MyBB bb ( 
    .thisin  ( somein     ),
    .thisout ( bb_thisout ) 
  );
  assign someout = (somein && somein2);
  assign someout2 = (somein || somein2);
endmodule


//MyBottom_1_ remplaced by MyBottom

module MyTop (
      input    in1,
      input    in2,
      input    in3,
      output   out1,
      output   out2,
      output   out3,
      output   out4);
  wire  bot1_someout;
  wire  bot1_someout2;
  wire  bot2_someout;
  wire  bot2_someout2;
  MyBottom bot1 ( 
    .somein   ( in1           ),
    .somein2  ( in2           ),
    .someout  ( bot1_someout  ),
    .someout2 ( bot1_someout2 ) 
  );
  MyBottom bot2 ( 
    .somein   ( in2           ),
    .somein2  ( in3           ),
    .someout  ( bot2_someout  ),
    .someout2 ( bot2_someout2 ) 
  );
  assign out1 = bot1_someout;
  assign out2 = bot2_someout;
  assign out3 = (bot1_someout2 ^ bot2_someout2);
  assign out4 = bot1_someout2;
endmodule

removeOutputAssigns=true

module MyBottom (
      input    somein,
      input    somein2,
      output   someout,
      output   someout2);
  wire  bb_thisout;
  MyBB bb ( 
    .thisin  ( somein     ),
    .thisout ( bb_thisout ) 
  );
  assign someout = (somein && somein2);
  assign someout2 = (somein || somein2);
endmodule


//MyBottom_1_ remplaced by MyBottom

module MyTop (
      input    in1,
      input    in2,
      input    in3,
      output   out1,
      output   out2,
      output   out3,
      output   out4);
  wire  bot2_someout2;
  MyBottom bot1 ( 
    .somein   ( in1  ),
    .somein2  ( in2  ),
    .someout  ( out1 ),
    .someout2 ( out4 ) 
  );
  MyBottom bot2 ( 
    .somein   ( in2           ),
    .somein2  ( in3           ),
    .someout  ( out2          ),
    .someout2 ( bot2_someout2 ) 
  );
  assign out3 = (out4 ^ bot2_someout2);
endmodule

Fixes #251

lsteveol avatar Feb 06 '20 19:02 lsteveol

For additional race conditions I'll say that in a traditional ASIC flow we carefully construct the clock trees in such a way that should prevent ... we carefully construct the clock trees in such a way that should prevent, or at least significantly reduce the possibility of a delta cycle issue.

I was talking about race condition / delta cycle issue only from a digital simulation perspective, it seem you were interpreting it more in a backend / design perspective, or i miss something ?

  output wire dac, //analog
  input  wire vcc,  //analog
  input  wire vss //analog

Then we could imagine setting a specific property on those signal in SpinalHDL to say "That's analog stuff, avoid intermediate signals", it would have the advantage to be explicit and could be reuse for other purposes. I mean, VHDL/Verilog implicit inferation is probably similar to the bilion $$$ null mistake in software XD, some say "lipstick on a pig" :D

Are you ok with this feature provided we keep the original methodology (output assigns) as the default?

I'm not against as long that's disabled by default (because of the delta cycle potential issues in simulation), but maybe we can do better (explicit analog signal tags for signal in SpinalHDL itself) Then we can mix digital / analog in a clean and safe way

What is your opinon ?

Dolu1990 avatar Feb 17 '20 10:02 Dolu1990

Constructing the clock tree in that manner will usually avoid the delta cycle issue in digital simulation as well.

I have been working on a DSL that sits on top of Spinal. Right now I have signals that I declare as a certain type of analog (not the Spinal built in Analog). I mainly do this for creating set_dont_touch lists after elaboration.

I will modify the current pull request to resolve the issues you've brought up and give some thought to how we can specify intermingling digital and analog signals. I mainly don't want to bloat Spinal with something that could be handled with an additional DSL and/or some custom DSL as many ASIC companies have their own methodologies with regards to how they handled mixed signal designs.

lsteveol avatar Feb 17 '20 13:02 lsteveol

Hi Pathfinder @lsteveol, any news about this PR and the linked issue?

numero-744 avatar Oct 03 '22 17:10 numero-744

This PR is old and has conflicts so I close it, but it is still linked to #251 so that people can read it to fix #251.

numero-744 avatar Nov 15 '22 11:11 numero-744