5 ms·

You are right, I missed that. Thanks for pointing it out! It's hard to change your mindset from a programming language to a hardware description language and n

by hrvach 8y ago

You are right, I missed that. Thanks for pointing it out! It's hard to change your mindset from a programming language to a hardware description language and not make mistakes like these, especially when just starting to learn Verilog. :-)

alain94040 8y ago
One way to restructure your code is to move all the default assignments to the top of the always block, and then let if conditions change the ones that don't apply for that cycle. This code would work just fine (many people would argue it's not very readable -- but it saves you a lot of else branches): // Default timeout <= timeout + 1'b1; // Special cases if(~old_download && ioctl_download) begin cnt <= 8'b0; timeout <= 32'b0; end
- hrvach 8y ago
  Thank You for the advice! I'm still very much a beginner and since I find the "blink a led" kind of projects too boring, this is basically the first thing I ever wrote in Verilog. I hope my next project will have cleaner code, be more readable and have less issues like these. HDL languages seem to have a rather steep learning curve and it takes some effort to distance yourself from the usual programming mindset.
  - Taniwha 8y ago
    In general you should only use a <= to write a reg once within a clock (remember that the RHS of a <= is evaluated when the code is evaluated, the assignment happens later) And you are mixing <= and = in the same code (look in execute() ) this should never happen. In general use <= in places that are gated by an "always @(posedge clk)" and = in things gated by "always @(*)" (with the exception that it's OK to assign a temp variable in a clocked always with '=' provided its lifetime doesn't extend past the block's execution (your use of SKIP_FLAG is an example of this being done correctly)
    - hrvach 8y ago
      Thank you, I'll have to do some refactoring to avoid mixing the two assignment principles. But it is ok to do something like: cnt <= cnt + 1; if (cnt > 100) cnt <= 0; If not, how else should something like this be done? Thanks for your help, I'm finding HDL to be anything but easy. :-)
      - Taniwha 8y ago
        probably not a good idea, because you are sampling cnt after think you have incremented it and you will get the wrong value Essentially what happens is: tmp_cnt1 = cnt + 1; if (cnt > 100) tmp_cnt2 = 0; some time later: cnt = tmp_cnt1; and after that (if cnt >100) cnt = tmp_cnt2; Better to say: if (cnt >= 99) { cnt <= 0; } else { cnt <= cnt+1; }
        hrvach 8y ago
        Thanks for pointing it out, I'll try to stick to a simple if-else construct in the future!
        FullyFunctional 8y ago
        There is certainly no consensus that it's the preferred style. Some designers prefer this explicit (and more verbose) style but others prefer the less verbose one (with default first, business logic next, and reset values at the end).
        Taniwha 8y ago
        BTW: as an onetime verilog implementer those temporary storage locations that are made behind your back by the compiler when you use <= are potentially quite expensive, the compiler can optimise the normal case of: always @(posedge clk) r <= v; and in some more complex cases where r is only set once in one always statement (or once in any path through an always statement) - but something like: always @(*) r <= c; is a nightmare that essentially means that r can have many changes scheduled in the same instant of time, more importantly it's a number of changes that can't be determined at compile time (could be 1000s of transitions) - resulting in code that's mallocing space to store all those changes - simulation can slow down if you use <= in a non-clocked place because the behaviour can't be determined statically Also using <= a smart compiler can merge multiple always statements: always @(posedge clk) r1 <= c1; always @(posedge clk) r2 <= c2; ...... into a single always @(posedge clk) { r1 <= c1; r2 <= c2; ...... } behind your back, effectively converting 2*N simulation events into 2 (a very good thing)
tasty_freeze 8y ago
Here is another suggestion that is part stylistic, but may save you debugging effort. When performing boolean condition testing, eg "if (~a & b)", always use "!" for negation instead of "~", "&&" for logical and, and "||" for logical or for exactly the same reasons you should do it for C code. If the operands are all 1 bit, either option works fine. But if you ever accidentally use a vector where you intended a bit, one generates a warning and the other doesn't.

by hrvach 8y ago

alain94040 8y ago

One way to restructure your code is to move all the default assignments to the top of the always block, and then let if conditions change the ones that don't apply for that cycle. This code would work just fine (many people would argue it's not very readable -- but it saves you a lot of else branches): // Default timeout <= timeout + 1'b1; // Special cases if(~old_download && ioctl_download) begin cnt <= 8'b0; timeout <= 32'b0; end

hrvach 8y ago
Thank You for the advice! I'm still very much a beginner and since I find the "blink a led" kind of projects too boring, this is basically the first thing I ever wrote in Verilog. I hope my next project will have cleaner code, be more readable and have less issues like these. HDL languages seem to have a rather steep learning curve and it takes some effort to distance yourself from the usual programming mindset.
- Taniwha 8y ago
  In general you should only use a <= to write a reg once within a clock (remember that the RHS of a <= is evaluated when the code is evaluated, the assignment happens later) And you are mixing <= and = in the same code (look in execute() ) this should never happen. In general use <= in places that are gated by an "always @(posedge clk)" and = in things gated by "always @(*)" (with the exception that it's OK to assign a temp variable in a clocked always with '=' provided its lifetime doesn't extend past the block's execution (your use of SKIP_FLAG is an example of this being done correctly)
  - hrvach 8y ago
    Thank you, I'll have to do some refactoring to avoid mixing the two assignment principles. But it is ok to do something like: cnt <= cnt + 1; if (cnt > 100) cnt <= 0; If not, how else should something like this be done? Thanks for your help, I'm finding HDL to be anything but easy. :-)
    - Taniwha 8y ago
      probably not a good idea, because you are sampling cnt after think you have incremented it and you will get the wrong value Essentially what happens is: tmp_cnt1 = cnt + 1; if (cnt > 100) tmp_cnt2 = 0; some time later: cnt = tmp_cnt1; and after that (if cnt >100) cnt = tmp_cnt2; Better to say: if (cnt >= 99) { cnt <= 0; } else { cnt <= cnt+1; }
      - hrvach 8y ago
        Thanks for pointing it out, I'll try to stick to a simple if-else construct in the future!
        FullyFunctional 8y ago
        There is certainly no consensus that it's the preferred style. Some designers prefer this explicit (and more verbose) style but others prefer the less verbose one (with default first, business logic next, and reset values at the end).
      - Taniwha 8y ago
        BTW: as an onetime verilog implementer those temporary storage locations that are made behind your back by the compiler when you use <= are potentially quite expensive, the compiler can optimise the normal case of: always @(posedge clk) r <= v; and in some more complex cases where r is only set once in one always statement (or once in any path through an always statement) - but something like: always @(*) r <= c; is a nightmare that essentially means that r can have many changes scheduled in the same instant of time, more importantly it's a number of changes that can't be determined at compile time (could be 1000s of transitions) - resulting in code that's mallocing space to store all those changes - simulation can slow down if you use <= in a non-clocked place because the behaviour can't be determined statically Also using <= a smart compiler can merge multiple always statements: always @(posedge clk) r1 <= c1; always @(posedge clk) r2 <= c2; ...... into a single always @(posedge clk) { r1 <= c1; r2 <= c2; ...... } behind your back, effectively converting 2*N simulation events into 2 (a very good thing)

tasty_freeze 8y ago

Here is another suggestion that is part stylistic, but may save you debugging effort. When performing boolean condition testing, eg "if (~a & b)", always use "!" for negation instead of "~", "&&" for logical and, and "||" for logical or for exactly the same reasons you should do it for C code. If the operands are all 1 bit, either option works fine. But if you ever accidentally use a vector where you intended a bit, one generates a warning and the other doesn't.