lushaylabs / lushay-code

yosys, nextpnr, apicula and openFPGALoader in vscode using OSS-CAD-Suite
https://marketplace.visualstudio.com/items?itemName=lushay-labs.lushay-code&ssr=false
MIT License
24 stars 5 forks source link

Fix formatting bug that breaks simulation #9

Closed waielal closed 4 months ago

waielal commented 4 months ago

When switching the inputs from number to string in #8, I forgot to properly format the inputs for the generated testbench which broke the simulation.

I also added code to backup the .dbgmodule of version 0.0.19 in case people want to revert back to that version.

waielal commented 4 months ago

When I was copying my changes over from my testing folder I somehow forgot to include the line in commit c8dcfae. Sorry, for the inconvenience that i caused.

lushaylabs commented 4 months ago

Looks good, I am rewriting the input part of the testbench generation to combine all inputs into a single large register as I noticed it triggers sensitivity blocks in the same time unit before all inputs can be updated, but I will keep this in mind when doing it

waielal commented 4 months ago

Try using <= (Non-Blocking Assignment) instead of = (Blocking Assignment). Verilog collects all non-blocking assignments until a next time step is reached and then applies all changes, where as blocking assignments get executes immediately. This might fix your issue without resorting to convoluted hacks.

lushaylabs commented 4 months ago

It's tricky since we want it to block and finish the assignments before printing the outputs, but I will test it out with the non blocking assignments, maybe changing the order so it updates the inputs for the next cycle, but I think it would require skipping the first clock cycle which might cause "inconsistencies" with certain modules that perform something in the first time step which would be hidden to the user.

I will open a PR once I manage to compare both options, would love to get your feedback on it

waielal commented 4 months ago

And what if you used $monitor instead of $display? You'd only need to call it once at the beginning of the initial block and then just drive the inputs afterwards.

initial begin
    $monitor("%t: a = %b", $time, a);
    #10 a = 10;
    #10 a = 7;
end

should print

10: a = 1010
20: a = 0111

Sure, just @ me in a comment :)

waielal commented 4 months ago

I read up on non-blocking assignments again. I was misremembering how they worked in testbenches. It executes each line with respective delay

begin
    #9 a <= 0;
    a <= 1;
    #5 a <= 4;
end

In that case it would execute line 2 then 3 then 1. I dont think they will help in solving your issue unfortunately.

$monitor seems to be more promising.

lushaylabs commented 4 months ago

I think I will wait with this, I opened a draft PR so you can see the change https://github.com/lushaylabs/lushay-code/pull/10

I noticed the issue when I was testing the IDES8 primitive and I saw it updated a register before all inputs were set, but it seems it isn't directly because of the assignment as when I tried to create a simplified example to reproduce it I wasn't able to reproduce the issue.

I think it might have to do with that primitive having multiple sensitivity list blocks both on pos-edge and neg-edge triggers so maybe it is something more subtle like that

But until I get to the root of the issue we can leave it as-is