sylefeb / Silice

Silice is an easy-to-learn, powerful hardware description language, that simplifies designing hardware algorithms with parallelism and pipelines.
Other
1.3k stars 78 forks source link

Unexpected code execution. #249

Closed at91rm9200 closed 1 year ago

at91rm9200 commented 1 year ago

Hello,

I would like to show a case here that I had trouble debugging as a non-advanced user:

unit main(output uint5 leds, inout uint8 pmod)
{

   algorithm
   {

      subroutine delay_cycles(input uint24 cycle_counter)
      {
         uint24 ctr = 0;

         ctr = cycle_counter;
         while (ctr > 0) {
            ctr = ctr - 1;
         }
      }

      // removed Silice code.

      while (1) {
         // Code execution should stop here
         // for debugging purposes
      }

      // Execution continues and skips the following subroutine
      () <- delay_cycles <- (12000048); 

      while (1) {
         leds = 1;   
      }

   }
}

For debugging purposes, I wanted to stop the code with the while loop. But this is not the case. The code continues to run, the subroutine is not executed. The LED lights up immediately. Completely unexpected for me. I have not tested how the new draft variant of Silice behaves.

Through the thread #248 I learned that two consecutive while loops are merged. This will probably be the case here. Ignoring the subroutine could be a bug.

Regards, Bernd.

sylefeb commented 1 year ago

Hi Bernd,

Thanks for the report. Using draft (5d1b92e8), the generated Verilog looks all good. The 'infinite loop' state is (formatted for clarity):

2: begin
  if (1) begin
    _d__idx_fsm0 = 2;
  end else begin
    _d_i_delay_cycles_cycle_counter = 12000048;
    _d__idx_fsm0 = 3;
    _d_caller = 0;
  end
end

The else is never reached due to if (1).

However, I tried in hardware (on icebreaker) and got the same result as you. I then realized that no default value is given to the LED output. Using a default value fixes the issue, this is done by writing output uint5 leds(0).

(I do not know whether the LED turning on without default is due to hardware considerations or yosys/nextpnr detecting the output is left untouched).

at91rm9200 commented 1 year ago

Hello Sylvain,

I am sorry for the false alarm. When I initialize leds, it also runs with the master variant of Silice.

I only came across this behavior by accident. I am debugging far more complex code right now and using leds to output error conditions. Just setting the leds had/has an effect on the code that I cannot explain. Then I thinned out the code more and more until I came across the phenomenon described above. I think the report can be closed.

Regards, Bernd.

sylefeb commented 1 year ago

No worries at all -- good luck with debugging on LEDs (I have been there many times!). If possible, it is well worth it to setup a simulation environment with verilator and/or icarus.

Best regards - Sylvain

at91rm9200 commented 1 year ago

Hello Sylvain,

found it. I guess I had to learn the hard way that asynchronous inputs have to be registered (probably always).

I had intentionally not registered an input to save a register and I hadn't seen (and actually still don't see) the impact that could have. The design actually ran smoothly, but the very smallest changes caused it to run amok. I still have so much to learn...

Thanks for Silice. It is a wonderful tool.

Regards, Bernd.