Improper dependency detection for some VHDL constructs

jvanstraten commented 5 years ago

vhdeps' define/use matching is currently very simplistic, perhaps overly. For instance, it doesn't detect the following properly:

components that are defined without the optional "is" keyword (there may be more like this)
usage of components that don't have a port or generic map
any recognized construct embedded in a string
...?

This all has to do with the fact that vhdeps' matching is entirely context-insensitive. For instance, without context sensitivity, component usage like above would require matching anything of the form : <ident> ;, which leads to a lot of false positives, for instance in signal declarations. To handle this properly, vhdeps' matcher would need to be aware of where blocks start and end at least.

vhdeps also doesn't currently support configurations, multiple architectures per entity, etc. in any way, mostly because I've never used them personally and have rarely seen them be used in the FPGA world.

EraYaN commented 5 years ago

This also applies to any HLS ip generated by Vivado, if it has any sub components vhdeps ignores those. xilinx_com_hls_Float_1_0.zip

The entry point is hdl/vhdl/Float.vhd (this correctly picked up)

Although this does seem to use the IS keyword.

EDIT: https://github.com/abs-tudelft/vhdeps/blob/master/vhdeps/vhdl.py#L53 that just might need to have Is/IS etc in there.

jvanstraten commented 5 years ago

I don't see anything wrong with dependency generation for the files in the zip file you sent. From the root of that zip, I get:

$ vhdeps dump
Including the current working directory recursively by default...
ResolutionError: while resolving package floating_point_v7_1_6.floating_point_v7_1_6 in /tmp/vhdeps-issue-6/hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:
could not find package floating_point_v7_1_6.floating_point_v7_1_6

which makes sense... because

$ grep -ri package
doc/ReleaseNotes.txt:Package      : -ffva1156

there is no such package defined anywhere. If I add a dummy file;

$ echo "package floating_point_v7_1_6 is" > test.vhd
$ vhdeps dump
Including the current working directory recursively by default...
ResolutionError: while resolving package floating_point_v7_1_6.floating_point_v7_1_6 in /tmp/vhdeps-issue-6/hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:
could not find package floating_point_v7_1_6.floating_point_v7_1_6

it still doesn't work, because you have to tell vhdeps which library the includes belong to or it assumes work (just like every other VHDL compiler). You can do that by prefixing include paths with the library name, separated by a colon:

$ vhdeps -i floating_point_v7_1_6:. dump
ResolutionError: while resolving component floating_point_v7_1_6 in /tmp/vhdeps-issue-6/hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:
black box: could not find entity floating_point_v7_1_6.floating_point_v7_1_6

That's also correct, because that entity doesn't exist either:

$ grep -ri floating_point_v7_1_6
hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:LIBRARY floating_point_v7_1_6;
hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:USE floating_point_v7_1_6.floating_point_v7_1_6;
hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:  COMPONENT floating_point_v7_1_6 IS
hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:  END COMPONENT floating_point_v7_1_6;
hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:  ATTRIBUTE X_CORE_INFO OF Float_ap_hmul_1_max_dsp_16_arch: ARCHITECTURE IS "floating_point_v7_1_6,Vivado 2018.1";
hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:  ATTRIBUTE CHECK_LICENSE_TYPE OF Float_ap_hmul_1_max_dsp_16_arch : ARCHITECTURE IS "Float_ap_hmul_1_max_dsp_16,floating_point_v7_1_6,{}";
hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:  ATTRIBUTE CORE_GENERATION_INFO OF Float_ap_hmul_1_max_dsp_16_arch: ARCHITECTURE IS "Float_ap_hmul_1_max_dsp_16,floating_point_v7_1_6,{x_ipProduct=Vivado 2018.1,x_ipVendor=xilinx.com,x_ipLibrary=ip,x_ipName=floating_point,x_ipVersion=7.1,x_ipCoreRevision=6,x_ipLanguage=VHDL,x_ipSimLanguage=MIXED,C_XDEVICEFAMILY=virtexu,C_HAS_ADD=0,C_HAS_SUBTRACT=0,C_HAS_MULTIPLY=1,C_HAS_DIVIDE=0,C_HAS_SQRT=0,C_HAS_COMPARE=0,C_HAS_FIX_TO_FLT=0,C_HAS_FLT_TO_FIX=0,C_HAS_FLT_TO_FLT=0,C_HAS_RECIP=0,C_HAS_RECIP_SQRT=0,C_HAS_ABSOLUTE=0,C_HAS_LOGARITHM=0,C_HAS_EXPONENTIAL=0,C_HAS_FMA=0,C_HAS_FMS=0,C_HAS" & 
hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd:  U0 : floating_point_v7_1_6
hdl/ip/Float_ap_dmul_3_max_dsp_64.vhd:LIBRARY floating_point_v7_1_6;
hdl/ip/Float_ap_dmul_3_max_dsp_64.vhd:USE floating_point_v7_1_6.floating_point_v7_1_6;
hdl/ip/Float_ap_dmul_3_max_dsp_64.vhd:  COMPONENT floating_point_v7_1_6 IS
hdl/ip/Float_ap_dmul_3_max_dsp_64.vhd:  END COMPONENT floating_point_v7_1_6;
hdl/ip/Float_ap_dmul_3_max_dsp_64.vhd:  ATTRIBUTE X_CORE_INFO OF Float_ap_dmul_3_max_dsp_64_arch: ARCHITECTURE IS "floating_point_v7_1_6,Vivado 2018.1";
hdl/ip/Float_ap_dmul_3_max_dsp_64.vhd:  ATTRIBUTE CHECK_LICENSE_TYPE OF Float_ap_dmul_3_max_dsp_64_arch : ARCHITECTURE IS "Float_ap_dmul_3_max_dsp_64,floating_point_v7_1_6,{}";
hdl/ip/Float_ap_dmul_3_max_dsp_64.vhd:  ATTRIBUTE CORE_GENERATION_INFO OF Float_ap_dmul_3_max_dsp_64_arch: ARCHITECTURE IS "Float_ap_dmul_3_max_dsp_64,floating_point_v7_1_6,{x_ipProduct=Vivado 2018.1,x_ipVendor=xilinx.com,x_ipLibrary=ip,x_ipName=floating_point,x_ipVersion=7.1,x_ipCoreRevision=6,x_ipLanguage=VHDL,x_ipSimLanguage=MIXED,C_XDEVICEFAMILY=virtexu,C_HAS_ADD=0,C_HAS_SUBTRACT=0,C_HAS_MULTIPLY=1,C_HAS_DIVIDE=0,C_HAS_SQRT=0,C_HAS_COMPARE=0,C_HAS_FIX_TO_FLT=0,C_HAS_FLT_TO_FIX=0,C_HAS_FLT_TO_FLT=0,C_HAS_RECIP=0,C_HAS_RECIP_SQRT=0,C_HAS_ABSOLUTE=0,C_HAS_LOGARITHM=0,C_HAS_EXPONENTIAL=0,C_HAS_FMA=0,C_HAS_FMS=0,C_HAS" & 
hdl/ip/Float_ap_dmul_3_max_dsp_64.vhd:  U0 : floating_point_v7_1_6
hdl/ip/Float_ap_fmul_0_max_dsp_32.vhd:LIBRARY floating_point_v7_1_6;
hdl/ip/Float_ap_fmul_0_max_dsp_32.vhd:USE floating_point_v7_1_6.floating_point_v7_1_6;
hdl/ip/Float_ap_fmul_0_max_dsp_32.vhd:  COMPONENT floating_point_v7_1_6 IS
hdl/ip/Float_ap_fmul_0_max_dsp_32.vhd:  END COMPONENT floating_point_v7_1_6;
hdl/ip/Float_ap_fmul_0_max_dsp_32.vhd:  ATTRIBUTE X_CORE_INFO OF Float_ap_fmul_0_max_dsp_32_arch: ARCHITECTURE IS "floating_point_v7_1_6,Vivado 2018.1";
hdl/ip/Float_ap_fmul_0_max_dsp_32.vhd:  ATTRIBUTE CHECK_LICENSE_TYPE OF Float_ap_fmul_0_max_dsp_32_arch : ARCHITECTURE IS "Float_ap_fmul_0_max_dsp_32,floating_point_v7_1_6,{}";
hdl/ip/Float_ap_fmul_0_max_dsp_32.vhd:  ATTRIBUTE CORE_GENERATION_INFO OF Float_ap_fmul_0_max_dsp_32_arch: ARCHITECTURE IS "Float_ap_fmul_0_max_dsp_32,floating_point_v7_1_6,{x_ipProduct=Vivado 2018.1,x_ipVendor=xilinx.com,x_ipLibrary=ip,x_ipName=floating_point,x_ipVersion=7.1,x_ipCoreRevision=6,x_ipLanguage=VHDL,x_ipSimLanguage=MIXED,C_XDEVICEFAMILY=virtexu,C_HAS_ADD=0,C_HAS_SUBTRACT=0,C_HAS_MULTIPLY=1,C_HAS_DIVIDE=0,C_HAS_SQRT=0,C_HAS_COMPARE=0,C_HAS_FIX_TO_FLT=0,C_HAS_FLT_TO_FIX=0,C_HAS_FLT_TO_FLT=0,C_HAS_RECIP=0,C_HAS_RECIP_SQRT=0,C_HAS_ABSOLUTE=0,C_HAS_LOGARITHM=0,C_HAS_EXPONENTIAL=0,C_HAS_FMA=0,C_HAS_FMS=0,C_HAS" & 
hdl/ip/Float_ap_fmul_0_max_dsp_32.vhd:  U0 : floating_point_v7_1_6
test.vhd:package floating_point_v7_1_6 is

So:

$ echo "entity floating_point_v7_1_6 is" >> test.vhd
$ vhdeps -i floating_point_v7_1_6:. dump
top floating_point_v7_1_6 2008 /tmp/vhdeps-issue-6/hdl/vhdl/Float.vhd
dep floating_point_v7_1_6 2008 /tmp/vhdeps-issue-6/test.vhd
top floating_point_v7_1_6 2008 /tmp/vhdeps-issue-6/hdl/ip/Float_ap_dmul_3_max_dsp_64.vhd
top floating_point_v7_1_6 2008 /tmp/vhdeps-issue-6/hdl/ip/Float_ap_fmul_0_max_dsp_32.vhd
top floating_point_v7_1_6 2008 /tmp/vhdeps-issue-6/hdl/ip/Float_ap_hmul_1_max_dsp_16.vhd
top floating_point_v7_1_6 2008 /tmp/vhdeps-issue-6/hdl/vhdl/Float_dmul_64ns_6cud.vhd
top floating_point_v7_1_6 2008 /tmp/vhdeps-issue-6/hdl/vhdl/Float_fmul_32ns_3bkb.vhd
top floating_point_v7_1_6 2008 /tmp/vhdeps-issue-6/hdl/vhdl/Float_hmul_16ns_1dEe.vhd

Looks good. For future reference:

$ vhdeps --vhdeps-version
vhdeps 0.2.0

Vivado using libraries to keep IPs apart is unfortunate for vhdeps since you need to put more effort into making things work, but of course it makes perfect sense to namespace IP blocks that way. There is no mechanism in VHDL files to detect which library a file belongs to by just reading the file, so aside from adding a pragma to set it (which might be nice in some cases, but doesn't work here since the files are generated) I don't see a way to make this more ergonomic.

With regards to your edit, the entire VHDL file is lowercased in memory before anything else is applied to it, so the matching is case insensitive.

ETA: Maybe have vhdeps check for some kind of marker file while recursively scanning directories? Like .vhdeps.library with the library name as the file contents, which would override the default work library name and previous marker files up the filesystem hierarchy? Then you'd only need a script to place those files based on Vivado's filesystem layout, instead of painstakingly changing the vhdeps command line every time you add or remove an IP core. Though of course you could also just call vhdeps with said script each time.

ETA 2: after IRL discussion it seems that the actual problem is that vhdeps is not detecting some instantiations at all (which I also didn't see while testing myself). See #26.

abs-tudelft / vhdeps

Improper dependency detection for some VHDL constructs #6