wzab / agwb

Support for automatic address map generation and address decoding logic for Wishbone connected hierachical systems
12 stars 6 forks source link

Problem with the newest version of GHDL #46

Closed wzab closed 3 years ago

wzab commented 3 years ago

During the final tests, I have found a strange thing in the GHDL. It looks like during the elaboration it attempts to perform all assignments?

When using the "test" design (from commit https://github.com/wzab/agwb/commit/f42e076deec93c982f0a118f0b586f60b619095b ), I get the following error:

general-cores/modules/wishbone/wb_crossbar/xwb_crossbar.vhd:114:7:@0ms:(report note): Mapping slave #33[0xf800/0x1fc00]
general-cores/modules/wishbone/wb_crossbar/xwb_crossbar.vhd:114:7:@0ms:(report note): Mapping slave #34[0xf400/0x1fc00]
general-cores/modules/wishbone/wb_crossbar/xwb_crossbar.vhd:114:7:@0ms:(report note): Mapping slave #35[0xf000/0x1fc00]
general-cores/modules/wishbone/wb_crossbar/xwb_crossbar.vhd:114:7:@0ms:(report note): Mapping slave #36[0x10000/0x10000]
general-cores/modules/wishbone/wb_crossbar/xwb_crossbar.vhd:114:7:@0ms:(report note): Mapping slave #0[0x0/0x0]
./wb_test_top_tb:error: index (9) out of bounds (0 to 8) at gen/SYS1.vhd:216
in process .wb_test_top_tb(test).dut@wb_test_top(simul).main_1@main(rtl).gl1(0).sys1_1@sys1(rtl).sys1_1@sys1(gener).P6
./wb_test_top_tb:error: error during elaboration

I have checked line 216: obraz

It shouldn't be excuted, as the generic g_ENABLEs_size is set to 9!

So I have modified the generated SYS1.vhd: obraz

Now that line should be never executed! After compilation and starting the simulation, I get:

general-cores/modules/wishbone/wb_crossbar/xwb_crossbar.vhd:114:7:@0ms:(report note): Mapping slave #34[0xf400/0x1fc00]
general-cores/modules/wishbone/wb_crossbar/xwb_crossbar.vhd:114:7:@0ms:(report note): Mapping slave #35[0xf000/0x1fc00]
general-cores/modules/wishbone/wb_crossbar/xwb_crossbar.vhd:114:7:@0ms:(report note): Mapping slave #36[0x10000/0x10000]
general-cores/modules/wishbone/wb_crossbar/xwb_crossbar.vhd:114:7:@0ms:(report note): Mapping slave #0[0x0/0x0]
./wb_test_top_tb:error: index (9) out of bounds (0 to 8) at gen/SYS1.vhd:216
in process .wb_test_top_tb(test).dut@wb_test_top(simul).main_1@main(rtl).gl1(0).sys1_1@sys1(rtl).sys1_1@sys1(gener).P6
./wb_test_top_tb:error: error during elaboration

The error occurs during the elaboration, at time 0. Of course, I can use the for loop instead of if's. It makes the generated VHDL much smaller, but less legible. Anyway the fact that generics and if (in the process, it works for if-generate) do not allow skipping the non-existing parts of an array is a little annoying.

I have verified, that the problem does not occur in the earlier version of GHDL.

wzab commented 3 years ago

It seems that the generated code didn't comply to the VHDL LMR. It is explained in the GHDL issue. I have rewritten the code that generates access to the registers. Now it uses for-loop instead of if-then to select the accessible range of registers. Please see commit c3b6aeed3f78d237edc81674b6dd2852989b594a (not fully tested yet!)

The new code generator handles each register or vector of registers in a single loop (below an example of a control and status registers):

            for i in 0 to g_ENABLEs_size - 1 loop
              if int_addr = std_logic_vector(to_unsigned(3 + i, 4)) then
                int_regs_wb_m_i.dat <= (others => '0');
                int_regs_wb_m_i.dat(31 downto 0) <= std_logic_vector(int_ENABLEs_o( i ));
                if int_regs_wb_m_o.we = '1' then
                  int_ENABLEs_o( i ) <= std_logic_vector(int_regs_wb_m_o.dat(31 downto 0));
                end if;
                int_regs_wb_m_i.ack <= '1';
                int_regs_wb_m_i.err <= '0';
              end if;
            end loop; -- g_ENABLEs_size
            for i in 0 to g_STATUS_size - 1 loop
              if int_addr = std_logic_vector(to_unsigned(13 + i, 4)) then
                int_regs_wb_m_i.dat <= (others => '0');
                int_regs_wb_m_i.dat(31 downto 0) <= std_logic_vector(STATUS_i);
                if int_regs_wb_m_i.ack = '0' then
                   STATUS_i_ack <= '1';
                end if;
                int_regs_wb_m_i.ack <= '1';
                int_regs_wb_m_i.err <= '0';
              end if;
            end loop; -- g_STATUS_size

The solution still requires thorough testing.

wzab commented 3 years ago

I have similarly modified the code generator for connecting sub-buses (in https://github.com/wzab/agwb/commit/30efd0f211d98400229738c7c1fb5df7015cc32e )

    bg1: if g_EXTHUGE_size > 0 generate
      wb_m_i(0) <= EXTHUGE_wb_m_i;
      EXTHUGE_wb_m_o  <= wb_m_o(0);
    end generate; -- g_EXTHUGE_size
    bg2: if g_EXTERN_size > 0 generate
      bg3: for i in 0 to g_EXTERN_size - 1 generate
        wb_m_i(1 + i) <= EXTERN_wb_m_i(i);
        EXTERN_wb_m_o(i)  <= wb_m_o(1 + i);
      end generate; -- for g_EXTERN_size
    end generate; -- if g_EXTERN_size
    bg4: if g_LINKS_size > 0 generate
      bg5: for i in 0 to g_LINKS_size - 1 generate
        wb_m_i(5 + i) <= LINKS_wb_m_i(i);
        LINKS_wb_m_o(i)  <= wb_m_o(5 + i);
      end generate; -- for g_LINKS_size
    end generate; -- if g_LINKS_size
    wb_m_i(36) <= int_regs_wb_m_i;
    int_regs_wb_m_o  <= wb_m_o(36);

Probably it may be further simplified by avoiding if-generate and relying on empty ranges in for-generate Hopefully, new synthesis tools should support empty ranges - https://www.xilinx.com/support/answers/53503.html

wzab commented 3 years ago

The simplified version of code generator for connecting sub-buses (in https://github.com/wzab/agwb/commit/529dbcfe79e163d6f8bad9d764c0859566343a99 ). Now the generated code looks like below:

    bg1: if g_EXTHUGE_size > 0 generate
      wb_m_i(0) <= EXTHUGE_wb_m_i;
      EXTHUGE_wb_m_o  <= wb_m_o(0);
    end generate; -- g_EXTHUGE_size
    bg2: for i in 0 to g_EXTERN_size - 1 generate
      wb_m_i(1 + i) <= EXTERN_wb_m_i(i);
      EXTERN_wb_m_o(i)  <= wb_m_o(1 + i);
    end generate; -- for g_EXTERN_size
    bg3: for i in 0 to g_LINKS_size - 1 generate
      wb_m_i(5 + i) <= LINKS_wb_m_i(i);
      LINKS_wb_m_o(i)  <= wb_m_o(5 + i);
    end generate; -- for g_LINKS_size
    wb_m_i(36) <= int_regs_wb_m_i;
    int_regs_wb_m_o  <= wb_m_o(36);

The EXTHUGE bus connects a single sub-block and may be controlled by used attribute. Therefore, it uses if-generate. The EXTERN and LINKS buses connect vectors of blocks and are controlled by reps attribute. Therefore, they use for-generate.

wzab commented 3 years ago

It seems that the problem is solved.