Questa Sim 2022.4_2 changes default bit ordering in unconstrained arrays. Test tb_uart_lib.tb_uart_rx.test_receives_one_byte broken.

[nicdes]

Updating questasim from 2020.1_1 to 2022.4_2 breakes the test 'tb_uart_lib.tb_uart_rx.test_receives_one_byte':

#      73810000 ps - check                        -   ERROR - Got 1110_1110 (238). Expected 0111_0111 (119).

And I expect many others. Note that the got value is the swapped version of the expected value.

The reason is that this procedure https://github.com/VUnit/vunit/blob/868eea10d187854a0b1adb02d2ff33df4b88293a/examples/vhdl/uart/src/test/tb_uart_rx.vhd#L54 ends up assigning the value x"77" to the unconstrained data port in the uart_master verification component: https://github.com/VUnit/vunit/blob/868eea10d187854a0b1adb02d2ff33df4b88293a/vunit/vhdl/verification_components/src/uart_master.vhd#L29

Questa Sim has changed the bit ordering of expressions like x"77" when assigned to unconstrained std_logic_vector ports.

My fix is replacing https://github.com/VUnit/vunit/blob/868eea10d187854a0b1adb02d2ff33df4b88293a/vunit/vhdl/verification_components/src/uart_master.vhd#L43 with:

      for i in data'length-1 downto 0 loop

VUnit version: 4.6.0 Simulator: Questa Sim-64 vsim 2022.4_2 Simulator 2022.12 Dec 9 2022

[LarsAsplund]

The first thing push_string does is to normalize the input. In this case to 7 downto 0. When the UART master is looping 0 to data'length-1 it is sending the bits in 0x77 LSB first as it should. If you look at the master output you should see a start bit 0 and then 11101110. There are many other places where things can flip. To know I would have to try to recreate locally.

Flipping the order of the loop may fix things for your version of Questa but it will fail on all other versions and simulators

[nicdes]

There are many other places where things can flip. To know I would have to try to recreate locally.

Indeed, I just observed the flip in the uart_master VC component output. I've run the whole vhdl test suite (vunit/examples/vhdl/docker_runall.sh) and no other errors showed up. Not what I would expect if the flip happens elsewhere.

Flipping the order of the loop may fix things for your version of Questa but it will fail on all other versions and simulators

Using get_simulator_name() from the py class could be a way to get around it. I wonder if this new questasim feature violates the LRM: then it's a bug.

[nicdes]

I added a swap_bit_order argument to the uart_master: https://github.com/VUnit/vunit/pull/891 this keeps the code clean on my side.

[nicdes]

Ignore anything previously said and have a look at the following: https://github.com/VUnit/vunit/pull/892/files

Hard to believe.

[LarsAsplund]

I also had a look at this and agree that it is a bug in Questa. It "optimizes" the code in a way that effectively skips the normalization step to 7 downto 0 and continues to use the 0 to 7 range of the input x"77". I consider this as a fairly serious bug that needs to be fixed by Siemens. This is a common approach to normalizing data. The reason you might not see it in reality is that you typically do not push literals like x"77" which has an implicit to direction. Typically you push vectors with an explicit range. Such vectors work regardless if they are defined as downto or to. It also works with literals if you change the constant to a variable. That might seem like a reasonable workaround but the problem is all other places that would also need fixing. From a maintenance point of view it would be a nightmare to

1. Consistently workaround this bug in all places we do normalization. Now and in the future.
2. Update tests to explicitly verify literal behavior to make sure we follow that rule (although we often test with both directions, negative indices, and indices not aligned to 0)

Do you have a support account with Siemens? If so, I suggest you create a minimal example of this problem and file a bug.

[nicdes]

Do you have a support account with Siemens? If so, I suggest you create a minimal example of this problem and file a bug.

I will do that and report here.

[roynil]

It seems to still be the same behaviour in Questa 2023.4.

[LarsAsplund]

@nicdes Any word from Siemens?

[m42uko]

I just opened a support case with Siemens with the following minimal example that also reproduces the error. (The root of the problem seems to lie in their procedure/function optimization routines as it works fine if you normalize "directly" in the process.). I'll keep you posted if I hear anything back.

library ieee;
use ieee.std_logic_1164.all;

entity foo is
end entity foo;

architecture a of foo is
begin
     test: process is
    procedure print_vector(constant c: std_logic_vector) is
    begin
             for i in c'range loop
                 report integer'image(i) & ": " & std_logic'image(c(i));
             end loop;      
    end procedure;
        procedure print_normalized_vector(constant c: std_logic_vector) is
        constant v : std_logic_vector(c'length - 1 downto 0) := c;
        begin
        print_vector(v);
        end procedure;

    constant slv : std_logic_vector := x"80";
    constant slv_normalized : std_logic_vector(slv'length - 1 downto 0) := slv;
     begin
        print_normalized_vector(slv);
    report "";
    print_vector(slv_normalized);
        wait;
     end process;
end architecture;

-- nvc -a foo.vhd -e foo -r
-- vcom +acc foo.vhd && vsim  -voptargs="+acc" -c -do "run -all; exit" 'work.foo(a)'

In Questa 2023.4, this evaluates to

# ** Note: 0: '1'
# ** Note: 1: '0'
# ** Note: 2: '0'
# ** Note: 3: '0'
# ** Note: 4: '0'
# ** Note: 5: '0'
# ** Note: 6: '0'
# ** Note: 7: '0'
# ** Note: 
# ** Note: 7: '1'
# ** Note: 6: '0'
# ** Note: 5: '0'
# ** Note: 4: '0'
# ** Note: 3: '0'
# ** Note: 2: '0'
# ** Note: 1: '0'
# ** Note: 0: '0'

even though both vectors should be the exact same.

[m42uko]

So Siemens just confirmed the bug, and they will look into fixing it for future versions of Questa.

Until then, they found the following workaround of just replacing the constant used for normalization with a variable. (I'm not saying we need to change anything upstream, although this would be a comparatively "clean" workaround if we wanted to restore QuestaSim support, assuming this is the only place that needs changing.)

diff --git a/vunit/vhdl/verification_components/src/stream_master_pkg-body.vhd b/vunit/vhdl/verification_components/src/stream_master_pkg-body.vhd
index 708596d5..1270f65c 100644
--- a/vunit/vhdl/verification_components/src/stream_master_pkg-body.vhd
+++ b/vunit/vhdl/verification_components/src/stream_master_pkg-body.vhd
@@ -21,7 +21,7 @@ package body stream_master_pkg is
                         data : std_logic_vector;
                         last : boolean := false) is
     variable msg : msg_t := new_msg(stream_push_msg);
-    constant normalized_data : std_logic_vector(data'length-1 downto 0) := data;
+    variable normalized_data : std_logic_vector(data'length-1 downto 0) := data;
   begin
     push_std_ulogic_vector(msg, normalized_data);
     push_boolean(msg, last);

[LarsAsplund]

@m42uko Good that they confirmed the bug and that the workaround we also found works in general. I will change this specific instance such that the example works for the affected simulator versions. There are probably other VUnit APIs that would fail for the same reason if the input is a literal. It's not reasonable to find and fix all of those so we will simply have to remember this issue and recommend people having problems to assign their literals to intermediate variables before calling the API.