Implementation fails

Was working on a project, and randomly, I get these errors when trying to build.
ERROR: [Vivado 12-13638] Failed runs(s) : ‘impl_1’
Bin file (/home/work/projects/000_Xilinx_Alchitry/led_stuff/build/vivado/led_stuff.runs/impl_1/alchitry_top.bin) could not be found! The build likely failed.

I have been trying to fix it for a while now, and it appears nothing with the syntax is wrong, and this happens with all projects.

I got a similar error on building, due to a bug in Lucid V2 when generating Verilog code, see this issue

@Random_FPGA_Guy could you post a project that fails to build?

@dheijl I saw this report and will dig into it soon.

OK, this is a verilog module, its wired up to the lucid main module:
module leds (input clk, input rst, output reg [7:0] leds);
reg [2:0] pointer;
reg [25:0] delay;
reg onoff = 1’b1;
initial
begin
leds <= 8’b00000000;
end
always @(posedge rst)
begin
leds <= 8’b00000000;
onoff <= 1’b1;
end
always @*
begin
if(onoff == 1’b1)
begin
leds[pointer] = 1’b1;
end
if(onoff == 1’b0)
begin
leds[pointer] = 1’b0;
end
end
always @(posedge clk)
begin
if(delay < 26’b101111101011110000100000000)
begin
delay <= delay + 1’b1;
end
if(delay >= 26’b101111101011110000100000000)
begin
delay <= 26’b0;
pointer <= pointer + 1’b1;
end
if(pointer == 3’b111)
begin
onoff <= ~onoff;
pointer <= 3’b000;
end
end
endmodule

Heres the lucid module:

module alchitry_top (
input clk, // 100MHz clock
input rst_n, // Reset button (active low)
output led[8], // 8 user controllable LEDs
input usb_rx, // USB->Serial input
output usb_tx // USB->Serial output
) {

sig rst                 // reset signal
.clk(clk) {
    // The reset conditioner is used to synchronize the reset signal to the FPGA
    // clock. This ensures the entire FPGA comes out of reset at the same time.
    reset_conditioner reset_cond
    leds verilog
}

always {
    reset_cond.in = ~rst_n  // input raw inverted reset signal
    rst = reset_cond.out    // conditioned reset
    verilog.rst = ~rst_n
    led[7:0] = verilog.leds
    usb_tx = usb_rx         // echo the serial data
}

}

Sorry, I cant indent the modules.

I’ll take a look at this more tomorrow, but given that it’s Verilog, it’s not uncommon for the .bin to fail without an error in the IDE. Verilog is only lightly supported right now.

Could you post the build messages? I’m sure there’s an error in there hinting at what’s wrong.

Sure. I will post it and we can take a look tomorrow.

Here are the build errors and critacal warnings in order:

(Here it complains about the led outputs in the Verilog module having multiple drivers. (critical warnings))

(Yet again, it complains about the led outputs in the verilog module having multiple drivers.(errors))

ERROR: [Vivado 12-13638] Failed runs(s) : ‘impl_1’

ERROR: [Common 17-39] ‘wait_on_runs’ failed due to earlier errors.

while executing"wait_on_run impl_1" (file “/home/work/projects/000_Xilinx_Alchitry/led_stuff/build/project.tcl” line 17)INFO: [Common 17-206] Exiting Vivado at Tue Nov 19 20:12:58 2024…Vivado exited.

Bin file (/home/work/projects/000_Xilinx_Alchitry/led_stuff/build/vivado/led_stuff.runs/impl_1/alchitry_top.bin) could not be found! The build likely failed.

Just checked, and alchitry_top.bin is not there. So that is the issue

I was digging around, and I think its something with the Verilog that is the problem. I made modules of lucid and Verilog, and as long as i gave the outputs a value, they built. I was just turning LEDs on. I do not know how incorrect Verilog would lead to a missing bin file, maybe the multi net driven LEDs had something to do with it. I am going to try and find the Verilog issue. Labs V2 has limited Verilog support, so maybe that is another factor. So anyways, once I find the error, I will make a mention of it here.

This version builds:

module leds (input clk, input rst, output reg [7:0] leds);
    reg [2:0] pointer;
    reg [25:0] delay;
    reg onoff = 1'b1;
initial
    begin
        leds <= 8'b00000000;
    end
/*
always @(posedge rst)
    begin
        leds <= 8'b00000000;
        onoff <= 1'b1;
    end
*/
always @*
    begin
        if(onoff == 1'b1)
            leds[pointer] = 1'b1;
        else if(onoff == 1'b0)
            leds[pointer] = 1'b0;
    end
always @(posedge clk)
    begin
        if(delay < 26'b101111101011110000100000000)
            delay <= delay + 1'b1;
        else if(delay >= 26'b101111101011110000100000000)
        begin
            delay <= 26'b0;
            pointer <= pointer + 1'b1;
        end
        if(pointer == 3'b111)
        begin
            onoff <= ~onoff;
            pointer <= 3'b000;
        end
    end
endmodule

You get someVivado warnings though:

WARNING: [DRC PDRC-153] Gated clock check: Net verilog/leds_reg[0]_i_1_n_0 is a gated clock net sourced by a combinational pin verilog/leds_reg[0]_i_1/O, cell verilog/leds_reg[0]_i_1. This is not good design practice and will likely impact performance. For SLICE registers, for example, use the CE pin to control the loading of data.
WARNING: [DRC PDRC-153] Gated clock check: Net verilog/leds_reg[1]_i_1_n_0 is a gated clock net sourced by a combinational pin verilog/leds_reg[1]_i_1/O, cell verilog/leds_reg[1]_i_1. This is not good design practice and will likely impact performance. For SLICE registers, for example, use the CE pin to control the loading of data.
WARNING: [DRC PDRC-153] Gated clock check: Net verilog/leds_reg[2]_i_1_n_0 is a gated clock net sourced by a combinational pin verilog/leds_reg[2]_i_1/O, cell verilog/leds_reg[2]_i_1. This is not good design practice and will likely impact performance. For SLICE registers, for example, use the CE pin to control the loading of data.
WARNING: [DRC PDRC-153] Gated clock check: Net verilog/leds_reg[3]_i_1_n_0 is a gated clock net sourced by a combinational pin verilog/leds_reg[3]_i_1/O, cell verilog/leds_reg[3]_i_1. This is not good design practice and will likely impact performance. For SLICE registers, for example, use the CE pin to control the loading of data.
WARNING: [DRC PDRC-153] Gated clock check: Net verilog/p_0_in[4] is a gated clock net sourced by a combinational pin verilog/leds_reg[4]_i_1/O, cell verilog/leds_reg[4]_i_1. This is not good design practice and will likely impact performance. For SLICE registers, for example, use the CE pin to control the loading of data.
WARNING: [DRC PDRC-153] Gated clock check: Net verilog/p_0_in[5] is a gated clock net sourced by a combinational pin verilog/leds_reg[5]_i_1/O, cell verilog/leds_reg[5]_i_1. This is not good design practice and will likely impact performance. For SLICE registers, for example, use the CE pin to control the loading of data.
WARNING: [DRC PDRC-153] Gated clock check: Net verilog/p_0_in[6] is a gated clock net sourced by a combinational pin verilog/leds_reg[6]_i_1/O, cell verilog/leds_reg[6]_i_1. This is not good design practice and will likely impact performance. For SLICE registers, for example, use the CE pin to control the loading of data.
WARNING: [DRC PDRC-153] Gated clock check: Net verilog/p_0_in[7] is a gated clock net sourced by a combinational pin verilog/leds_reg[7]_i_1/O, cell verilog/leds_reg[7]_i_1. This is not good design practice and will likely impact performance. For SLICE registers, for example, use the CE pin to control the loading of data.

And also this:

Issues detected in alchitry_top.luc:    Warning at line 9 offset 4: The signal "rst" is never read.

I know next to nothing of FPGAs and Verilog, so don’t ask me for an explanation.

Thank you, that is probably because Labs is not too happy about 2 always blocks controlling one register. I will look at that once I have time.

So this version of the rst block does not build:

always @(posedge rst)
begin
leds <= 8’b00000000;
onoff <= 1’b1;
end

but this version does:

always @(rst)
begin
leds <= 8’b00000000;
onoff <= 1’b1;
end

See, I knew I could do something by myself :smirk:

Note: it builds but does not work, need to rework the logic.

You should never drive a signal from multiple always blocks. This will create multiple drivers (as the warning says). You’re basically connecting two outputs together. What happens when one says it should be 0 and the other says it should be 1?

If you want an asynchronous reset, you need to add the reset signal to the sensitivity list and use it in the same always block. You generally shouldn’t use asynchronous resets in an FPGA though due to timing.

It’s unclear to me what is supposed to be DFFs in your code and what isn’t. Verilog’s proclivity to allow this kind of mistake was the original motivation for creating Lucid.

The way this module is written I don’t think it can ever be reliable since not everything is clocked together. Even if it builds and even simulates fine, timing will be violated in come cases causing issues in hardware.

Generally, you should either have one always block with @(posedge clk) with everything in it or you should have one @(posedge clk) with all your DFFs explicitly assigned and another @* block with only combinational logic.

The first style leads to easily messing up and accidentally creating latches, which often fail to work due again to timing. The second style is very verbose.

Lucid implements things in the second style but all the extra verboseness is taken care of for you.

It’s important to remember that reg isn’t special in Verilog. It doesn’t actually make a DFF. The reason your code is having issues is because you’re assigning values to things with various triggers. Any signal should either have an explicit value at all times (so it is essentially just a wire) or have its value only change at the edge of a clock (so it’s a DFF).

Here is a working version of your Verilog. Note that your constants were actually 27 bits so I dropped the LSB (rightmost 0).

module leds (input clk, input rst, output reg [7:0] leds);
    reg [2:0] pointer;
    reg [25:0] delay;
    reg onoff = 1'b1;

    initial begin
        leds <= 8'b00000000;
    end
    
    always @(posedge clk) begin
        if(onoff == 1'b1) begin
            leds[pointer] = 1'b1;
        end

        if(onoff == 1'b0) begin
            leds[pointer] = 1'b0;
        end

        if(delay < 26'b10111110101111000010000000) begin
            delay <= delay + 1'b1;
        end

        if(delay >= 26'b10111110101111000010000000) begin
            delay <= 26'b0;
            pointer <= pointer + 1'b1;
        end

        if(pointer == 3'b111) begin
            onoff <= ~onoff;
            pointer <= 3'b000;
        end

        if (rst) begin
            leds <= 8'b00000000;
            onoff <= 1'b1;
        end
    end
endmodule

Here is how I would’ve written it in Lucid.

module leds_lucid (
    input clk,  // clock
    input rst,  // reset
    output leds[8]
) {
    
    .clk(clk), .rst(rst) {
        dff led_reg[8]
        dff pointer[3]
        dff delay[26]
    }
    
    always {
        delay.d = delay.q + 1
        
        if (delay.q >=  26b10111110101111000010000000) {
            delay.d = 0
            pointer.d = pointer.q + 1
            led_reg.d[pointer.q] = ~led_reg.q[pointer.q]
        }
        
        leds = led_reg.q
    }
}

And just for completeness, here’s the generated SystemVerilog.

module leds_lucid (
        input wire clk,
        input wire rst,
        output reg [7:0] leds
    );
    
    logic [7:0] D_led_reg_d, D_led_reg_q = 0;
    logic [2:0] D_pointer_d, D_pointer_q = 0;
    logic [25:0] D_delay_d, D_delay_q = 0;
    
    always @* begin
        D_delay_d = D_delay_q;
        D_pointer_d = D_pointer_q;
        D_led_reg_d = D_led_reg_q;
        
        D_delay_d = D_delay_q + 1'h1;
        if (D_delay_q >= 26'h2faf080) begin
            D_delay_d = 1'h0;
            D_pointer_d = D_pointer_q + 1'h1;
            D_led_reg_d[D_pointer_q] = ~D_led_reg_q[D_pointer_q];
        end
        leds = D_led_reg_q;
    end
    
    always @(posedge (clk)) begin
        if ((rst) == 1'b1) begin
            D_led_reg_q <= 0;
            D_pointer_q <= 0;
            D_delay_q <= 0;
        end else begin
            D_led_reg_q <= D_led_reg_d;
            D_pointer_q <= D_pointer_d;
            D_delay_q <= D_delay_d;
        end
    end
endmodule
2 Likes

Thank you, will be cleaning up my code as soon as I know how.
Also, thanks for explaining the 2 always blocks thing to me. I did not really understand why you could not until now.