clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.44k stars 154 forks source link

BlockRam initialisation on Verilog not executing properly. #331

Closed anmolsahoo25 closed 6 years ago

anmolsahoo25 commented 6 years ago

Initialising a blockRam in Clash gives the following output -

// blockRam begin                                                                                                                                            
  reg signed [31:0] RAM [0:128-1];                                                                                                                             

  reg [4095:0] ram_init;                                                                                                                                       
  integer i;                                                                                                                                                   
  initial begin                                                                                                                                                
    ram_init = xs;                                                                                                                                             
    for (i=0; i < 128; i = i + 1) begin                                                                                                                        
      RAM[128-1-i] = ram_init[i*32+:32];                                                                                                                       
    end                                                                                                                                                        
  end                                                                                                                                                          

  always @(posedge CLK) begin                                                                                                                                  
      ram_init <= xs;                                                                                                                                          
  end                                                                                                                                                          

  always @(posedge CLK) begin : memElement_blockRam                                                                                                            
    if (\#app_arg ) begin                                                                                                                                      
      RAM[(wild_0)] <= y;                                                                                                                                      
    end                                                                                                                                                        
    RD_DATA <= RAM[(wild)];                                                                                                                                    
  end                                                                                                                                                          
  // blockRam end

But as can be seen in this part,

initial begin                                                                                                                                                
    ram_init = xs;                                                                                                                                             
    for (i=0; i < 128; i = i + 1) begin                                                                                                                        
      RAM[128-1-i] = ram_init[i*32+:32];                                                                                                                       
    end                                                                                                                                                        
  end 

The initial value of wires is always z (High impedance), thus for Verilog Simulators, this does not work as subsequent cycles all have the value of RAM instantiated to z.

One possible solution is to add this code

always @(posedge clock) begin
    ram_init <= xs;
end

Could anyone comment on this? I am using iVerilog on my machine, simulation using vvp and plotting the signal output using GTKwave.

christiaanb commented 6 years ago

Clash should actually error out when the initial value of a block ram is not a literal. It does that when generating VHDL or SystemVerilog, but not for Verilog.

anmolsahoo25 commented 6 years ago

I instantiated the blockRam as,

blockRamInit = blockRam (iterate d128 (+1) (0 :: Signed 32))

Is that the why the initialisation is not from a literal? Generation to VHDL crashes. I'll try and work on this to fix it for Verilog.

anmolsahoo25 commented 6 years ago

Yeah. Generating the initial data for the RAM from a function actually transforms it into a generate loop in Verilog, which assigns to a wire and in turn the register is not allotted from a literal. Thank you for pointing that out. I will try and see if I can fix the code.

ggreif commented 6 years ago

@anmolsahoo25 you can probably generate the vector by -XTemplateHaskell, in which case your module will see a literal value.

christiaanb commented 6 years ago

@anmolsahoo25: @ggreif is correct, you can probably do:

blockRamInit = blockRam $(lift (iterate d128 (+1) (0 :: Signed 32)))
anmolsahoo25 commented 6 years ago

@christiaanb @ggreif Thank you. That seems like a good solution for now, wasn't aware of Template Haskell. I'll try looking around if I can fix the Verilog Backend for this bug.

leonschoorl commented 6 years ago

The bug is in the primitive definition of blockRam: https://github.com/clash-lang/clash-compiler/blob/52deb67fb67cbae08837f2fc0a89be845b38a9b1/clash-lib/prims/verilog/Clash_Explicit_BlockRam.json#L21 it should assert that initial state(ARG[2]) is a literal:

  ~SYM[2] = ~LIT[2];

Just like the SystemVerilog version: https://github.com/clash-lang/clash-compiler/blob/52deb67fb67cbae08837f2fc0a89be845b38a9b1/clash-lib/prims/systemverilog/Clash_Explicit_BlockRam.json#L18 And the VHDL version: https://github.com/clash-lang/clash-compiler/blob/52deb67fb67cbae08837f2fc0a89be845b38a9b1/clash-lib/prims/vhdl/Clash_Explicit_BlockRam.json#L16