nickg / nvc

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

Type issue in conditional expression #568

Closed amb5l closed 1 year ago

amb5l commented 1 year ago

Compiling this source produces an error during analysis:

nvc --std=2008 --work=work -a --relaxed video_out_timing.vhd
** Error: no matching operator "not" [BOOLEAN return STD_ULOGIC]
     > C:/work/tyto2_upstream/src/common/video_out/video_out_timing.vhd:278
     |
 278 |       elsif (run = '1') or (not hold) then
     |                             ^^^^^^^^

run is a std_logic signal, hold is a boolean generic. Initially I added parentheses around not hold but the or operator is still looking for 2 STD_ULOGICs...?

nickg commented 1 year ago

This is a bit strange. Is there a makefile target that can reproduce it? I tried analysing that file and the dependencies I could find but it didn't report an error:

$ nvc --std=2008 --work=work -a video_out_timing.vhd
$ ll work/
total 52K
-rw-r--r-- 1 nick nick  139 Nov  9 22:07 _index
-rwxr-xr-x 1 nick nick   14 Nov  9 22:05 _NVC_LIB
-rw-r--r-- 1 nick nick  623 Nov  9 22:06 WORK.SYNC_REG
-rw-r--r-- 1 nick nick  705 Nov  9 22:06 WORK.SYNC_REG_PKG
-rw-r--r-- 1 nick nick 2.0K Nov  9 22:06 WORK.SYNC_REG-STRUCTURAL
-rw-r--r-- 1 nick nick  371 Nov  9 22:05 WORK.TYTO_UTILS_PKG
-rw-r--r-- 1 nick nick  714 Nov  9 22:05 WORK.TYTO_UTILS_PKG-body
-rw-r--r-- 1 nick nick 1.3K Nov  9 22:07 WORK.VIDEO_OUT_TIMING
-rw-r--r-- 1 nick nick 1.3K Nov  9 22:07 WORK.VIDEO_OUT_TIMING_PKG
-rw-r--r-- 1 nick nick  16K Nov  9 22:07 WORK.VIDEO_OUT_TIMING-SYNTH

I'm using the latest master plus 00ac278bf5849 from your repository but I've not changed anything recently that would affect this.

My guess is it's some bug in the conditional conversion feature for VHDL-2008, so maybe try --std=1993 to see if that also fails.

amb5l commented 1 year ago

You should be able to look at this using my latest commit. First try a known good case: navigate to build\saa5050\tb_saa5050 and run make nvc. Then navigate to build\hdmi_tpg\hdmi_tpg_digilent_nexys_video and run make nvc to see the issue. Here's a full transcript:

C:\work\tyto2\build\hdmi_tpg\hdmi_tpg_digilent_nexys_video>make nvc
mkdir -p .nvc
cd .nvc && nvc --std=2008 --work=work -a --relaxed C:/work/tyto2/src/common/tyto_types_pkg.vhd
mkdir -p .nvc/.sct/src/common/ && touch .nvc/.sct/src/common/tyto_types_pkg.vhd
cd .nvc && nvc --std=2008 --work=work -a --relaxed C:/work/tyto2/src/common/tyto_utils_pkg.vhd
mkdir -p .nvc/.sct/src/common/ && touch .nvc/.sct/src/common/tyto_utils_pkg.vhd
cd .nvc && nvc --std=2008 --work=work -a --relaxed C:/work/tyto2/src/common/basic/sync_reg.vhd
mkdir -p .nvc/.sct/src/common/basic/ && touch .nvc/.sct/src/common/basic/sync_reg.vhd
cd .nvc && nvc --std=2008 --work=work -a --relaxed C:/work/tyto2/src/common/video_out/video_mode.vhd
mkdir -p .nvc/.sct/src/common/video_out/ && touch .nvc/.sct/src/common/video_out/video_mode.vhd
cd .nvc && nvc --std=2008 --work=work -a --relaxed C:/work/tyto2/src/common/video_out/xilinx_7series/video_out_clock.vhd
mkdir -p .nvc/.sct/src/common/video_out/xilinx_7series/ && touch .nvc/.sct/src/common/video_out/xilinx_7series/video_out_clock.vhd
cd .nvc && nvc --std=2008 --work=work -a --relaxed C:/work/tyto2/src/common/video_out/video_out_timing.vhd
** Error: no matching operator "not" [BOOLEAN return STD_ULOGIC]
     > C:/work/tyto2/src/common/video_out/video_out_timing.vhd:278
     |
 278 |       elsif run = '1' or not hold then
     |                          ^^^^^^^^
make: *** [C:/work/tyto2/build/build.mak:138: .nvc/.sct/src/common/video_out/video_out_timing.vhd] Error 1
amb5l commented 1 year ago

BTW I tried switching to VHDL-93 (I edited the nvc command in build/build.mak). The build failed on the previous source as shown below so I can't yet confirm your guess. FWIW GHDL agrees with NVC about these errors, but Vivado tolerates them.

nvc --std=1993 --work=work -a --relaxed C:/work/tyto2/src/common/video_out/xilinx_7series/video_out_clock.vhd
** Error: case expression must have locally static subtype
     > C:/work/tyto2/src/common/video_out/xilinx_7series/video_out_clock.vhd:123
     |
 123 |         case '0' & addr is
     |              ^^^^^^^^^^
** Error: actual associated with port D of mode IN must be a globally static expression or static name
     > C:/work/tyto2/src/common/video_out/xilinx_7series/video_out_clock.vhd:319
     |
 319 |       d(0)  => rsto_req or not locked or mmcm_rst,
     |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
nickg commented 1 year ago

Here's a minimal reproducer. It analyses ok with --std=1993 but not with --std=2008:

package pack is
    function "=" (L: bit; R: bit) return bit;
end package;

use work.pack.all;

entity issue568 is
    generic (x : boolean);
end entity;

architecture test of issue568 is
    signal y : bit;
begin
    p1: assert (y = '1') or (not x);
end architecture;
nickg commented 1 year ago

Should be fixed now.

amb5l commented 1 year ago

Looks like it's working. This is a monster simulation, it runs for hours. Thanks as always for the support.