VHDL-LS
Add support for ignored components
Our team uses primarily Xilinx parts with heavy use of XPM components. For unisim, the vcomponents package can be added since it's in VHDL. For XPM, they are all SV so there isn't currently a way around the issue. In general there are probably lots of reasons why someone would have to instantiate some existing verilog modules into their vhdl.
I would suggest adding an optional ignore specification to the toml file. Something like this
[ignores]
components = [ "XPM_FIFO", "XPM_CDC_GRAY"]
This would allow for things other than components to be ignored later on, like maybe a specific error code etc.
Then if rust_hdl encounters an instantiation matching that name, it blackboxes it and does not emit any errors beneath that point. This is how most CDC tools handle vendor libraries.
I'll admit ignorance of the LRM as to whether this should be called components, entities, or something else. But hopefully the meaning is clear.
I'm not sure if a line like this would also be required?
[ignores]
libraries = [ "unisim", "xpm"]
or if that could somehow be inferred in the case of entity instantiation. Probably better to be explicit and require ignoring both the components and the libraries.
See chat here as well.
It should be easy to add. I was hoping someone would step up and do it as it is a good first issue. I do not want to be the only one contributing. I focus my time on overloading and typechecking now.
Question to people asking for this. Why don't you just add a fake xpm library with an empty entity for each thing you would like to ignore? Then at least you get type checking?
@kraigher If it is still up, i can jump in and implement this! I would probably need a jump start as to in which section of code it should be! Btw, I'd be interested to contribute more to the project that just this in the future!
@patgro1 Great I can help point you in the right direction.
The first step is however to design the UX for the feature. You can write a proposal for how it should be configured in this issue. The best way is to make an explicit example of how it should work from the users perspective with an example code snippet.
One thing to consider is if it uses the toml file or a comment pragma?
One thing to consider is if it uses the toml file or a comment pragma?
Personally I’d be a strong advocate for the toml way. « Polluting » code base with pragmas that not everyone would use/understand is not the best way to do it. I would not want to add these pragmas in my work code base knowing we would be only 2-3 persons over the full 20-25 persons that would have a need for them.
Maybe configure it on the library level? Mark an empty library in the toml file as blackbox.
That would make sense! Let me write a couple of possible formats and I’ll propose then all here.
We could potentially blackbox a full library using something like this:
[libraries]
my_third_party_lib = []
That would 100% should intent of blackboxing the lib. However, it would not fix the problem when someone imports
use my_third_party_lib.all
-- Code cut
inst_of_a_component_from_third_party_lib the component();
This would still be flag at the component level. Same for records that would be defined in packages. Another way of doing it could be to provide an ignore section as @nfrancque suggested for libs and DU.
[ignore]
libraries = [my_third_library a]
components = [fifo_from_third_party_lib_b]
Any thoughts?
EDIT: I would really like for the user to be forced to put as many details as possible... while it could be tedious, it would also make sure the user knows exactly what he is ignoring and why he is.
I think that the ignore section is more elegant. It also could have a ignore file section.
El sáb., 18 mar. 2023 19:49, patgro1 @.***> escribió:
We could potentially blackbox a full library using something like this:
[libraries]my_third_party_lib = []
That would 100% should intent of blackboxing the lib. However, it would not fix the problem when someone imports
use my_third_party_lib.all-- Code cut inst_of_a_component_from_third_party_lib the component();
This would still be flag at the component level. Same for records that would be defined in packages. Another way of doing it could be to provide an ignore section as @nfrancque https://github.com/nfrancque suggested for libs and DU.
[ignore]libraries = [my_third_library a]components = [fifo_from_third_party_lib_b]
Any thoughts?
— Reply to this email directly, view it on GitHub https://github.com/VHDL-LS/rust_hdl/issues/118#issuecomment-1474953300, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNN4RZZFXOQBXGADAD6GHTW4X7R5ANCNFSM5B6UUPKQ . You are receiving this because you commented.Message ID: @.***>
I think that the ignore section is more elegant. It also could have a ignore file section.
I’m not sure about the use case of an ignore file option. In theory, if the file is not in the libraries section it’ll be ignore by default no?
I'm thinking on the integration with other tools. Imagine a project manager where the user add files (as Vivado) and this tool creates the toml file automatically.
El sáb., 18 mar. 2023 20:35, patgro1 @.***> escribió:
I think that the ignore section is more elegant. It also could have a ignore file section.
I’m not sure about the use case of an ignore file option. In theory, if the file is not in the libraries section it’ll be ignore by default no?
— Reply to this email directly, view it on GitHub https://github.com/VHDL-LS/rust_hdl/issues/118#issuecomment-1474967572, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNN4R2BPPTGBDNJGZTMULTW4YFBJANCNFSM5B6UUPKQ . You are receiving this because you commented.Message ID: @.***>
I think it is better to have an explicit ignore section. The risk with having an empty list as an implicit ignore is that is very non-obvious and could be a mistake by the user in either manually or automatically generated vhdl_ls.toml file.
Note taht it will be quite hard to implement support for the following without explicitly listing the components to ignore:
library ignored:
use ignored.all;
inst: entity missing -- Is this part of ignored or just another missing unit that is a bug, how do we know?
port map(
You have to think carefully about what should be able to be ignored. The more the ignored stuff leaks into other code the harder it will be to implement. It will not be possible to have one catch all implementation. Maybe you can think of a real use case for this, what library that exists in the wild would you like to ignore? In that way it can be narrowed down what is necessary? The alternative for the user is to always to create a dummy library themselves with empty entity or component definitions. If they have to mention each component/entity explicitly in the toml files they could almost just as well create the dummy library.
Yeah that was my concern too! It is not just hard, it might be impossible. How do you differentiate between a type in a name and a component in an ignored lib. I think we’d have to think in term of best practices too! Even if it’s possible, I do not think that importing a full library regardless of what is in it is a good idea. You open yourself to a lot of issues as name clashing and such. However, I also think that giving a user the opportunity to ignore some DU is not a bad idea either. In the case of vendors such as Xilinx and Altera for example, you might always want to exclude a fifo from the list. I could see one import the full libs because that’s the way Xilinx show it. However it makes a question pop in my head… Xilinx release libraries with versions included in the name… do we want to support patterns in the ignored stuff too? That could be a good case for components ignoring…
[ignores]
libraries = [ another_ignored_lib ]
components = [ xpm_fifo ]
IMO it is better this way since adding patterns could hide bugs by ignoring stuff that should not. However I’d probably not go further than libs and components.
For xpm you could always make stubs for the components with the bonus that you get more type checking of the ports. Seems people have already done it https://github.com/fransschreuder/xpm_vhdl/blob/master/src/xpm/xpm_VCOMP.vhd
That works too! I do not see another big use case of components ignoring. So do we settle on library ignoring and a seperate section without pattern matching?
I think the use case is not very clear. When is this feature useful instead of just stubbing the component? Will people be able to use a library ignore feature even with the limitations or will they still end up stubbing?
This is why I think a real world example is needed. It brings conceptual clarity to show where the proposed feature adds value compared to just stubbing.
I think it’s a matter of taste and practicability…
for me personally, I’m rarely using vendors libs. When I do, it’s mostly dedicated components (transceivers, pll, mmcm, buf) and it’s usually localized in a single file… I don’t think I’d stub them just because it does not makes sense for me to maintain stubs with upgrading versions just to prevents errors from showing in my editor. Without that feature, I’d juste let the error live and that’s it. Now if I could just ignore these things, I’d do it because that way you could filter out these errors from any project diagnostics list you could build to close s design.
if you are using the components everywhere, then maybe I see to case to use stub. For my usage it does not make that much sense.
Could you write down an example how how you would use it? How would your vhdl_ls.toml file look like with this feature and how would the instantiation statements look like? Would they use fully qualified names inst: libname.ent or would you first have a use libname.ent?
library ieee;
use ieee.std_logic_1164.all;
library my_lib;
use my_lib.module_a_pkg.all;
use my_lib.module_b_pkg.all;
-- Xilinx components
library unisim;
entity top is
port (
clk_i: in std_logic;
rst_i: in std_logic;
data_i: in std_logic_vector(31 downto 0);
data_o: out std_logic
);
end entity top;
architecture rtl of top is
signal clk: std_logic;
signal rst: std_logic;
begin
inst_clk_bufg: unisim.vcomponents.bufg
port map(
I => clk_i,
O => clk
);
inst_rst_bufg: unisim.vcomponents.bufg
port map(
I => rst_i,
O => rst
);
end architecture;
with toml has follows:
[libraries]
my_lib.files = ["src/my_lib/module_a/module_a_pkg.vhd", "src/my_lib/module_a/module_a.vhd",
"src/my_lib/module_b/module_b_pkg.vhd", "src/my_lib/module_b/module_b.vhd"
]
[ignore]
libraries = [ "unisim" ]
in my opinion should not return any error. There is always the case for the
vhdl use unisim.vcomponents.all but IMO it could be dealt with in a seperate issue because the implications are bigger.
Two thoughts:
- The unisim library and vcomponents is acually part of the xilinx install and can just be added to the project. So the example does not really mandate this feature.
- Having a comment pragma to just mute an error would actually solve the problem even for
use ignored_library.all;as you could just have a comment pragma above every instantiation statement to ignore the missing design unit.
The unisim library and vcomponents is acually part of the xilinx install and can just be added to the project. So the example does not really mandate this feature.
Yeah maybe it was not a great example with Vivado but imagine just a second that these libs were encrypted (which is the case with high end ips) then it would not be possible to add them in the list because you would not be able to de-encrypt and analyse these files
Having a comment pragma to just mute an error would actually solve the problem even for use ignored_library.all; as you could just have a comment pragma above every instantiation statement to ignore the missing design unit.
As I said earlier, I’m not sure my team would approve adding pragmas in our code base for a tool I am using. Our dev environment (imo) should not impact the code base itself.
The code for parsing the toml file is here: https://github.com/VHDL-LS/rust_hdl/blob/master/vhdl_lang/src/config.rs#L127
The analysis of instantiation statements are here: https://github.com/VHDL-LS/rust_hdl/blob/master/vhdl_lang/src/analysis/concurrent.rs#L246
You can start looking at those files. The DesignRoot holds the design hierarchy. You need to make the ignore information accessible somewhere there to have it accessible during analysis.
I don't want to weigh in on the discussion but just inform that Xilinx provides VHDL bindings for xpm components. I recently had the same issue and discovered that they are under /path/to/Vivado/data/ip/xpm/xpm_VCOMP.vhd. It's a single package that you can simply add to the list of xpm packages.
I could be mistaken, but I think that this issue is solved by the is_third_party library config that was added a while back and can be closed now.
