nickg / nvc

VHDL compiler and simulator
https://www.nickg.me.uk/nvc/
GNU General Public License v3.0
631 stars 77 forks source link

Sensitivity lists and aggregates #825

Closed JimLewis closed 8 months ago

JimLewis commented 8 months ago

For the following assignment, I think Index should be on the sensitivity list of the equivalent process:

SLV3 <= (SLV3'left downto Index+1 => '0') & '1' & (Index-1 downto 0 => '0'); 

Unfortuately it is not.

For 1076-2008, from 11.6 Condition e it says:

e) If the concurrent signal assignment statement is a guarded assignment, or if any expression (other than a time expression) within the concurrent signal assignment statement references a signal, then the process statement contains a final wait statement with an explicit sensitivity clause. The sensitivity clause is constructed by taking the union of the sets constructed by applying the rule of 10.2 to each of the aforementioned expressions.

From 10.2

The sensitivity set is initially empty. For each primary in the condition of the condition clause, if the primary is — A simple name that denotes a signal, add the longest static prefix of the name to the sensitivity set. . . . — An aggregate, apply this rule to every expression appearing after the choices and the =>, if any, in every element association. — A function call, apply this rule to every actual designator in every parameter association.

This assignment is has a primary that is an aggregate, so we apply the rule to every expression after the choices and find that Index is a signal and goes in the sensitivity list. Curious I have tested in 4 simulators. I only one of them put index in the sensitivity list.

JimLewis commented 8 months ago
library IEEE ;
use ieee.std_logic_1164.all ; 
use ieee.numeric_std_unsigned.all ;

-- library osvvm ; 
-- context osvvm.OsvvmContext ;

entity aggregate_issue is 
end entity aggregate_issue ; 
architecture test of aggregate_issue is 

--  signal TestDone : BarrierType ; 
  signal SLV1, SLV2 : std_logic_vector(7 downto 0) ;
  signal Index : integer := 0 ; 
begin
--   ControlProc : process
--   begin
--     SetTestName("aggregate_issue") ; 
--     SetLogEnable(PASSED, TRUE) ; 
--     WaitForBarrier(TestDone, 1 ms) ;
--     EndofTestReports ;  
--     std.env.stop ; 
--     wait ;
--   end process ControlProc ; 

  SLV1 <= (SLV1'left downto Index+1 => '0') & '1' & (Index-1 downto 0 => '0'); 

  RtlProc : process(Index)
  begin
    SLV2 <= (SLV2'left downto Index+1 => '0') & '1' & (Index-1 downto 0 => '0'); 
  end process RtlProc;

  TestProc : process
    variable expected : std_logic_vector(7 downto 0) ;

  begin
    wait for 0 ns ;  -- Allow SetLogEnable to be set 
    wait for 0 ns ;

    for i in SLV1'range loop 
      Index <= i ;
      wait for 10 ns ; 
      Expected := to_slv(2**Index, SLV1'length) ; 
      -- AffirmIfEqual(SLV1, Expected, "SLV1:") ;
      if SLV1 = expected then 
        report "SLV1: " & to_hstring(SLV1) & ",  expected: " & to_hstring(expected) severity note ; 
      else 
        report "SLV1: " & to_hstring(SLV1) & ",  expected: " & to_hstring(expected) severity error ; 
      end if ; 
      -- AffirmIfEqual(SLV2, Expected, "SLV2:") ;
      if SLV2 = expected then 
        report "SLV2: " & to_hstring(SLV2) & ",  expected: " & to_hstring(expected) severity note ; 
      else 
        report "SLV2: " & to_hstring(SLV2) & ",  expected: " & to_hstring(expected) severity error ; 
      end if ; 
    end loop ; 

    -- WaitForBarrier(TestDone) ;
    wait ; 

  end process TestProc ; 
end architecture test ; 
nickg commented 8 months ago

Yes I think you are right.

JimLewis commented 8 months ago

Don't feel bad, in my test only one out of four got it.

nickg commented 8 months ago

At least it's an easy fix. Try this one: https://github.com/nickg/nvc/actions/runs/7481507006/artifacts/1160916434

JimLewis commented 8 months ago

That got it. Thanks.

JimLewis commented 8 months ago

That was about 1 hour from reporting to me testing the installed fix. :)