FPGAwars / icestudio

:snowflake: Visual editor for open FPGA boards
https://icestudio.io
GNU General Public License v2.0
1.69k stars 244 forks source link

Verify fails but build works when using always block for combinational #482

Open LBertrandDC opened 3 years ago

LBertrandDC commented 3 years ago

In icestudio 0.5.0, when using an always block for combinational logic, I get an error with verify: y is not a valid l=value

always1_verify-l-value

But the design works just fine if I build then upload, or just go directly to upload.

module main_v99b0f2 (
    input x,
    output y
);
    always @(*) begin
        y = ~x;
    end
endmodule

But if I declare reg y before the always block, the verify passes (the design works on the FPGA):

    reg y;
    always @(*) begin
        y = ~x;
    end

But strangely, wire y instead of reg y also doesn't verify but builds and works on the FPGA.

This seems to me that it should work, or maybe my understanding of Verilog is shaky?

icestudio 0.5.0 Windows 10 Enterprise or Mac OS X 10.14.6 (Mojave) TinyFPGA BX

Thanks in advance Louis Bertrand Durham College (Canada)

cavearr commented 3 years ago

Download the latest nightly version at https://icestudio.io

Uninstall 0.5.0 version, install the last nightly and tell us.

----- Mensaje original ----- De: Louis Bertrand notifications@github.com Para: FPGAwars/icestudio icestudio@noreply.github.com CC: Subscribed subscribed@noreply.github.com Enviado: Sat, 16 Jan 2021 23:00:14 +0100 (CET) Asunto: [FPGAwars/icestudio] Verify fails but build works when using always block for combinational (#482)

In icestudio 0.5.0, when using an always block for combinational logic, I get an error with verify: y is not a valid l=value

always1_verify-l-value

But the design works just fine if I build then upload, or just go directly to upload.

module main_v99b0f2 (
    input x,
    output y
);
    always @(*) begin
        y = ~x;
    end
endmodule

But if I declare reg y before the always block, the verify passes (the design works on the FPGA):

    reg y;
    always @(*) begin
        y = ~x;
    end

But strangely, wire y instead of reg y also doesn't verify but builds and works on the FPGA.

This seems to me that it should work, or maybe my understanding of Verilog is shaky?

icestudio 0.5.0 Windows 10 Enterprise or Mac OS X 10.14.6 (Mojave) TinyFPGA BX

Thanks in advance Louis Bertrand Durham College (Canada)

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/FPGAwars/icestudio/issues/482

LBertrandDC commented 3 years ago

Hola, The problem is still there on 0.5.1n210107 with APIO 0.6.0 on Mac OS X (I don't want to change my working version on Windows for school).

It seems that verify and build invoke different commands with different errors and warnings Verify:


iverilog -B "/Users/louis/.icestudio/apio/packages/toolchain-iverilog/lib/ivl" -o hardware.out -D VCD_OUTPUT= /Users/louis/.icestudio/apio/packages/toolchain-yosys/share/yosys/ice40/cells_sim.v main.v
main.v:31: error: y is not a valid l-value in main.va24517.
main.v:28:      : y is declared here as wire.
1 error(s) during elaboration.
scons: *** [hardware.out] Error 1

Build:


[Sun Jan 17 08:08:03 2021] Processing TinyFPGA-BX
--------------------------------------------------------------------------------
yosys -p "synth_ice40 -json hardware.json" -q main.v
Warning: wire '\y' is assigned in a block at main.v:31.

Gracias --Louis

John-Westlake commented 3 years ago

Louis,

Could / should bit "inversion" be considered a procedural type block? - is so then the output has to be declared as a reg type:

always

https://www.javatpoint.com/verilog-always-block

But, please dont take too much notice of me as I've only been working with Verilog since Dec21st ...

LBertrandDC commented 3 years ago

Hi John, Yes the always block is mostly used for synchronous clocked logic but can synthesize combinational blocks too. For example, an if-else is can implement a 2:1 mux in a self documenting style, but the if-else has to be inside the always block. Only registers can be assigned inside always, and the (apparently) redundant redeclaration of output y to pass verify overrides the default type directive near the top of the generated code file. That's where I got confused. It seems to me that we should be able to specify type reg in the icestudio code element output list to override the default and avoid the redeclaration. Anyway, icestudio is a brilliant little gem to teach first year students. My own understanding is a bit shaky since the last time I used a HDL was with VHDL, quite a different beast. Gracias! --Louis

John-Westlake commented 3 years ago

Hi Louis,

Well I'm a first "few weeks student" here - but indeed icestudio does indeed seem like a "little Gem" - I'm very impressed ! (A massive thank you to the team who brought this to the community) - Gracias!

I've still alot to learn, but I worry I'm running into the limitations of icestudio - such as being able to optimizes the max operating frequency, and being able to define balanced / matched complementry I/O ports (such as LVDS) from within icestudio - maybe its possible but as I say I'm learning step by step here...

cavearr commented 3 years ago

Hello guys! These days I am very busy, but tomorrow I will be working on your problems.

Apart from this, all the ideas you have, do not hesitate to transmit them, we will take them into account. My goal is for Icestudio to be a tool with all the potential possible and not just a tool to learn fpgas.

Thank you for your comments!

John-Westlake commented 3 years ago

Hi Carlos,

It be brilliant if iCEStudio could be a tool for "Power users" as I struggle to see how I could progress from the iCEStudio's brilliant and logical GUI method to "HardCore" methods... its does not make sense why the GUI style is not the obvious working method - its gives a clear and simple overview of how all blocks are interconnected and takes care of the interconnected Blocks "wiring" and easily compartmentalizes the design "blocks".

So a "Dream" wish list of features that would help me:-

  1. Global Block unlock

When I'm working on a design and editing blocks, I always forget to unlock a block, and as soon as I start to edit the unlocked block it returns me to the Top layer... very frustrating and time consuming, I especially forget to unlock blocks when I'm tired... and when its even more frustrating :)

  1. Global Buffer input pins (not just for main clock)

It be good to be able to define Global buffer inputs - not just for main clock - maybe the GB interconnect colour could be changed to highlight the GB signal paths

  1. Differential I/O FPGA I/O (LVDS etc)

Working with IO primitives is hard work, it be elegantly simple if I/O FPGA pins existed for Differential I/O. This options should also apply for Global Buffer inputs.

UsingDifferentialIOLVDSSubLVDSiniCE40Devices.pdf

SBTICETechnologyLibrary201504.pdf

  1. Option to optimize clock frequency over all other criteria

I'm not sure if the toolchain has any options that can tell the synthesis / layout tools to optimize clock frequency over all other criteria? If there is, then it be great to have an option in iCEStudio to Max operating Clock frequency aver all other considerations (operating current / size etc).

  1. View the generated RTL...

Again I suspect a function of the later toolchain, but it be brilliant to be able to view the generated RTL to aid operating speed optimization (and understand the repercussions of ones "coding style").

  1. Easy method to define & add "custom" boards

One could then added there own PCB / hardware design (with I/O defined) to really integrate design / product development.

  1. Virtual Logic analyzer probe pins

I know your working on this, but it be BRILLIANT have simple Logic Analyzer pins that could be dropped into signal lines as "Probes" to Pulseview (I use DSLogic, but I understand this is based on much the same original code).

  1. A Simple method to create your own library of "Block parts"

A simple method to add "Blocks" ' Gates, BUS'es etc from Collections and the "Import as Block"option. These could be added to a "Sidebar" so one could quickly added to the working design "layout" much like component selection in PCB layout tools.

It be also great to be able to import iCEStudio design into ICEcube2 - I tried to import the Verilog code .V file but clock info was missing and when I run the imported Verilog code in iCEcube2 only the top layer elements where seen so the design has the IO ports etc, but parts of the LOGIC design missing... maybe my error??? how to import from iCEstudio to iCEcube2?

### Known bugs, listed in Critical order:-

  1. Issue #481 iCEStudio Verifys / Builds / Saves an earlier edited design then currently displayed

  2. Issue #473 Win8 UI issue & I/O Port define corruption also odd "FPGA pin" set behavior

  3. Issue #480 Large code block UI lower taskbar rendering issue Win8

Once again THANK you Carlos for all your brilliant HARD work - its allowed this retired 50 year old to work with FPGA's...!

Gracias!

TimRudy commented 1 year ago

Hi, these ideas are some gems and they're hidden here, not related to the issue title :-) Shall we create official issues for wish list?

More issues for wishlist, transcribed from a reference above, from @SiccoDwars:

  1. A button to cancel an ongoing Build or Upload. Quite often I realize seconds after pressing ‘Upload’ that I made a typo or coding error. But then I have to wait until the ongoing action is completed. That can be minutes. Being able to cancel and go straight back into editing would be much appreciated. The Verify/Build/Upload pushed to the background with a local copy of the relevant source files maybe? Just so that the editor isn’t blocked while compilation/synthesis/programming is in progress.

  2. A ‘replace block’ option. Often I have a Code block with many inputs and many outputs an parameters that got a tweak. When I do such a ‘block replace’ action now, I delete the old block and then do ‘File’, ‘Add as block..’, But then I need to rewire… [If my experience is correct, ‘replace block’ works in Icestudio now - does it work brilliantly?!]

  3. A way to let a WIDTH (# bits) local parameter trickle into a Code block so that array input and output busses automatically get the widths that they need. For example I have generic Verilog code for a N bits adder block. I want a generic pre-cooked Collection with a 4 bits, 8 bits, 12 bits, 16 bits, 24 bits and a 32 bits version of that binary adder. Verilog as such allows for that flexibility. As in one and the same Verilog module can do all those different array WIDTHs. But if I make Icestudio blocks, there’s a lot of editing to do as I need to edit each [array] input and output width, both in the specification dialog ‘Enter the input ports, Enter the output ports’, and then again in the Input and Output spec when wrapping such a Code Block into a .ice file for Importing as a Block later on. [Somewhat related to https://github.com/FPGAwars/icestudio/issues/449?]
    [Related to https://github.com/FPGAwars/icestudio/issues/340]

TimRudy commented 1 year ago

From comment by Joaquim @jojo535275:

  1. One day it will be nice to have a warning to the user somewhere when we load a design which contains old blocks versions (compared to the last version available in the ice collections [that are loaded in Icestudio or present in External Collections])

By the way, in reply to issue 5, I hope you love this:

digitaljs.tilk.eu by Prof @tilk This tool basically wraps available technology, i.e. yosys, in Icestudio's toolchain, which whips up synthesized logic of gates on the fly, and renders them. I think that is so impressive and useful, plus extremely simple - Input (paste Verilog) - Output (look at result), I wish I were plugging those features into Icestudio. Whoever does it will be a groundbreaker! and it will be relatively easy.