nvc icon indicating copy to clipboard operation
nvc copied to clipboard

Multiple sources error while assigning the output ports of a view array

Open davidgussler opened this issue 1 month ago • 8 comments

I've just started to dive into the new VHDL'19 features, so forgive me if this issue is caused my lack of understanding of the new LRM, rather than a tool bug. I don't have access to any other VHDL'19 simulators, so I can't try to reproduce with a different simulator.

OS: Ubuntu24.04 NVC Version: Compiled from main branch nvc 1.19-devel (1.18.0.r80.gce2345655) (Using LLVM 18.1.3)

Minimal code to reproduce the error:

-- File : axis_pkg.vhd

library ieee;
use ieee.std_logic_1164.all;

package axis_pkg is

  -- AXI-Stream element type
  type axis_t is record
    tready     : std_ulogic;
    tvalid     : std_ulogic;
    tdata      : std_ulogic_vector;
  end record;

  -- AXI-Stream array
  type axis_arr_t is array (natural range <>) of axis_t;

  -- Manager view
  view m_axis_v of axis_t is
    tready : in;
    tvalid : out;
    tdata  : out;
  end view;

  -- Subordinate view
  alias s_axis_v is m_axis_v'converse;

end package;
-- File: axis_mux.vhd

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use work.axis_pkg.all;

entity axis_mux is
  port (
    s_axis : view (s_axis_v) of axis_arr_t;
    m_axis : view m_axis_v;
    sel    : in integer range s_axis'range
  );
end entity;

architecture rtl of axis_mux is
begin

  select_comb : process(all) begin

    m_axis.tvalid <= s_axis(sel).tvalid;
    m_axis.tdata  <= s_axis(sel).tdata;

    for i in s_axis'range loop
      -- Removing this line stops the error from printing
      s_axis(i).tready <= '1' when m_axis.tready = '1' and (sel = i) else '0';
    end loop;

  end process;

end architecture;
-- File: axis_mux_tb.vhd

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

use work.axis_pkg.all;

entity axis_mux_tb is
end;

architecture tb of axis_mux_tb is

  signal s_axis : axis_arr_t(0 to 3)(tdata(15 downto 0));
  signal m_axis : axis_t(tdata(15 downto 0));
  signal sel    : integer range s_axis'range := 0;

begin

  main : process begin
    wait for 10 us;
    assert false
      report "Simulation done"
      severity failure;
  end process;

  u_axis_mux : entity work.axis_mux
  port map (
    s_axis => s_axis,
    m_axis => m_axis,
    sel    => sel
  );

end architecture;

Running this script:

# File: nvc.sh
nvc --std=2019 -a axis_pkg.vhd axis_mux.vhd axis_mux_tb.vhd
nvc --std=2019 -e axis_mux_tb
nvc --std=2019 -r axis_mux_tb

Produces this output:

** Fatal: (init): element TVALID of signal S_AXIS has multiple sources
    > axis_mux.vhd:10
    |
 10 |     s_axis : view (s_axis_v) of axis_arr_t;
    |     ^^^^^^ composite signal S_AXIS declared with unresolved type AXIS_ARR_T
 ...
 20 |   select_comb : process(all) begin
    |   ^ driven by process :axis_mux_tb:u_axis_mux:select_comb
    |
    = Note: connected to signal S_AXIS
        > axis_mux_tb.vhd:14
    = Note: element TVALID declared here
        > axis_pkg.vhd:11

Which is confusing to me, because the input s_axis(i).tvalid signal is clearly never assigned by the entity, so its hard to see what multiple sources means here.

If I remove this line s_axis(i).tready <= '1' when m_axis.tready = '1' and (sel = i) else '0'; then the error message goes away, which makes it look like nvc does not allow assigning output view elements of record arrays.

As a another test, I created a minimal pass-through module, that only uses one input element, rather than an array of input elements, and nvc ran the design as expected, without any errors:

-- File: axis_passthru.vhd

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use work.axis_pkg.all;

entity axis_passthru is
  port (
    s_axis : view s_axis_v;
    m_axis : view m_axis_v
  );
end entity;

architecture rtl of axis_passthru is

begin

  m_axis.tvalid <= s_axis.tvalid;
  m_axis.tdata  <= s_axis.tdata;
  s_axis.tready <= m_axis.tready;

end architecture;

davidgussler avatar Dec 01 '25 20:12 davidgussler

I narrowed it down a bit further. The problem seems to stem from assigning the array from a for loop inside of a process. The error message is not shown when manually assigning each element of the array from a process or when assigning it from a generate-for loop.

-- File: axis_mux.vhd

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use work.axis_pkg.all;

entity axis_mux is
  port (
    s_axis : view (s_axis_v) of axis_arr_t;
    m_axis : view m_axis_v;
    sel    : in integer range s_axis'range
  );
end entity;

architecture rtl of axis_mux is
begin

  select_comb : process(all) begin

    m_axis.tvalid <= s_axis(sel).tvalid;
    m_axis.tdata  <= s_axis(sel).tdata;

    -- This works!
    -- s_axis(0).tready <= '1' when m_axis.tready = '1' and (sel = 0) else '0';
    -- s_axis(1).tready <= '1' when m_axis.tready = '1' and (sel = 1) else '0';
    -- s_axis(2).tready <= '1' when m_axis.tready = '1' and (sel = 2) else '0';
    -- s_axis(3).tready <= '1' when m_axis.tready = '1' and (sel = 3) else '0';

    -- This does not work
    -- for i in s_axis'range loop
    --   s_axis(i).tready <= '1' when m_axis.tready = '1' and (sel = i) else '0';
    -- end loop;

    -- Neither does this
    -- for i in 0 to 3 loop
    --   s_axis(i).tready <= '1' when m_axis.tready = '1' and (sel = i) else '0';
    -- end loop;

    -- Neither does this
    -- for i in 0 to 3 loop
    --   s_axis(i).tready <= '1';
    -- end loop;

  end process;

  -- This also works!!
  gen_ready : for i in s_axis'range generate
    s_axis(i).tready <= '1' when m_axis.tready = '1' and (sel = i) else '0';
  end generate;

end architecture;

davidgussler avatar Dec 01 '25 20:12 davidgussler

I believe this error is correct (but surprising). A process creates drivers for every sub-element in the longest static prefix of each signal assignment statement. For s_axis(i).tready <= '1' the longest static prefix is s_axis since the loop variable i is non-static. This includes all the elements with in mode from the view and so those are now driven by this process as well as the input port, which produces the error above.

There's no rule in the LRM which says that drivers should not be created for inputs. Prior to 2019 this would have been unnecessary since every sub-element in the target of a signal assignment would have the same mode, and assignments to inputs are illegal. I suspect this was an oversight when interfaces were added.

@Paebbels @JimLewis who are on the IEEE committee and maybe can comment further.

One awkward workaround is to do the loop and assignment in a procedure which takes a parameter with the same view mode. Drivers are then only created for the out/inout elements passed to the procedure.

See also the discussion in #856 where this came up before.

nickg avatar Dec 02 '25 09:12 nickg

This turns out to be a complex issue with respect to for loops and driver issues.

In the VHDL working group, we have talked about a similar VHDL issue here: https://gitlab.com/IEEE-P1076/VHDL-Issues/-/issues/275

Feel free to take a look at it. Fresh perspective is always welcome.

I think the general solution to your problem is to handle in input side and output side of the AXI stream mux independently. Then for the output side, as you observed, use for generate as its loop index is globally static where for loop indices are not. See the VHDL issue referenced above. The discussion will give you some insight into why - and unsuccessfully tries to find a work around.

JimLewis avatar Dec 02 '25 11:12 JimLewis

Thanks for all of the resources on this issue. Happy to see it was my fault, and not an issue with NVC. Its especially interesting to see that SV works the same way. I've been following Alex's development of his upgraded Ethernet cores here and noticed that he often uses the SV equivalent of this workaround pattern to deal with arrays of interfaces internally. While still not as nice as directly indexing the input array from a process, I think its a bit cleaner than passing it to a procedure.

There may be situations where you don't want to / are not able to use a generate statement to handle the output side if the interface independently, particularly if you want to have an assignment on line N override an assignment from line N-1. Here's an example, using Alex's workaround, along with showing logic flow that requires a process to implement.

-- File: axis_mux.vhd

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use work.axis_pkg.all;

entity axis_mux is
  port (
    s_axis : view (s_axis_v) of axis_arr_t;
    m_axis : view m_axis_v;
    sel    : in integer range s_axis'range
  );
end entity;

architecture rtl of axis_mux is

  type slv_arr_t is array (natural range<>) of std_ulogic_vector;

  signal s_axis_tready : std_ulogic_vector(s_axis'range);
  signal s_axis_tvalid : std_ulogic_vector(s_axis'range);
  signal s_axis_tdata  : slv_arr_t(s_axis'range)(s_axis(0).tdata'range);

begin

  gen_unpack : for i in s_axis'range generate
    s_axis_tvalid(i) <= s_axis(i).tvalid;
    s_axis_tdata(i)  <= s_axis(i).tdata;
    s_axis(i).tready <= s_axis_tready(i);
  end generate;

  select_comb : process(all) begin

    m_axis.tvalid <= s_axis_tvalid(sel);
    m_axis.tdata  <= s_axis_tdata(sel);

    s_axis_tready <= (others=>'0');
    s_axis_tready(sel) <= m_axis.tready;

  end process;

end architecture;

I took a look at the discussion you linked, @JimLewis, and most of it flew above my head, so I don't think I'll be able to meaningfully contribute to discussions of improving this for the next language standard. All I can offer right now is feedback on what feels natural from a language user's perspective.

davidgussler avatar Dec 02 '25 13:12 davidgussler

Generate is a concurrent statement - just like an architecture. So you can have a process inside of the generate statement - and the generate index is still static.

From a simulation stand point, select_comb process will run any time s_axis.tvalid, s_axis.tdata, or m_axis.tready change. From a simulation perspective, it may be more efficient to do tvalid and tdata as concurrent assignments. Maybe @nickg can clarify since he is the simulation guru.

JimLewis avatar Dec 02 '25 14:12 JimLewis

@davidgussler Are you looking to do the https://github.com/fpganinja/taxi stuff in VHDL? Be sure to look at PoC (https://github.com/VHDL/PoC) and OpenLogic (https://github.com/open-logic/open-logic) libraries - that said non-duplicated work would be great. If you intend to verify it, I can answer questions on using OSVVM.

JimLewis avatar Dec 02 '25 14:12 JimLewis

@JimLewis No, I'm not trying to make a VHDL clone of taxi. I don't think that would be a worthwhile use of time and I also think that would be impossible maintain. I also don't know how the licensing would work with that. Taxi doesn't use a fully permissive MIT/BSD-like license.

I've looked through PoC and OpenLogic. Both are great projects that I've learned a lot from. Another great one that's always improving is hdl-modules.

Thanks for offering up OSVVM help. I use it almost every day (indirectly) through VUnit, so I'm grateful for all of your work on it! The open-source VHDL ecosystem has been getting so good recently. I've been using only FOSS tools (outside of Vivado, of course) for the last year and its been really smooth. I'm just now starting to work through transitioning from GHDL to NVC to start taking advantage of interfaces.

davidgussler avatar Dec 02 '25 14:12 davidgussler

@davidgussler VUnit only provides a minor part of OSVVM's utility library. OSVVM goes well beyond that.

JimLewis avatar Dec 02 '25 14:12 JimLewis