Microwatt emits lots of metavalue assertions when simulating in GHDL #272
Comments
|
I started looking at these assertions. Ideally we'd like to get to a point where we could enable ghdls One thing I am struggling with is how to avoid assertions during reset. Take for example icache_tb.vhdl. We initialise I can fix this by initialising the signal: and: But that's a lot of boilerplate for something that is reset in the first cycle. I'm also loath to add more signal initialisation to microwatt because it was a source of issues in our sky130 ASIC work. @umarcor any thoughts? (FYI the testbench is currently failing, but that is a separate issue I need to chase) |
|
With regard to initialising the signals, I do agree that'd be more cumbersome than helpful, particularly given that microwatt is being used for ASICs, and not only for FPGA targets. Hence, I believe this should be fixed/handled on the GHDL side. However, I've not really gone into the details of the verbosity during the first cycles of the simulations. During the last year, there have been discussions about it in the VHDL Analysis and Standardisation group because there are mixed opinions about what the default verbosity should be. Traditionally, the motto is "better print too much and let the user filter, rather than risk not printing something meaningful". You might want to bring the issue in https://gitlab.com/IEEE-P1076/VHDL-Issues/-/issues. With regard to the specific case of |
|
@umarcor I didn't know about I'm still seeing some issues where the signal is resolved on the first positive edge of the clock, eg:
The assert is 5ns in, at negative clock edge. |
|
I guess that's because the reset is synchronous, not asynchronous: Lines 47 to 51 in 05fe709 --asserts and --ieee-asserts to disable all IEEE asserts (always, not just at 0).
|
|
I was looking at this again this morning. We could add an option to ghdl to ignore asserts until a specific time, something like However that wont resolve some of the things we are doing in microwatt: This is in the data path, and sometimes is fed with U/X state. Ideally we'd want to propagate the U state through to_integer(), instead of raising an assertion (which if we ignore the assertion it gets sanitized to 0 and if we dont ignore we have to add extra logic to the data path to avoid). While fighting all of this, it does make me think we should just async reset every FF vs playing the U/X state whack a mole game. |
|
@antonblanchard I suggest to open an issue in https://gitlab.com/IEEE-P1076/VHDL-Issues/-/issues, in order to discuss this topic in the VHDL community. I recall it being discussed in the mailing list a couple of years ago. Overall, there is some friction between "I/we prefer excessive verbosity so that no warning/error goes unreported" and "I/we all know that doing 'math' with U/X does not work". The workaround might be done at the simulator level (GHDL); however, since these warnings are coming from the (standard) IEEE libs, we might want to have it "fixed" there (i.e. have a switch to control reporting metavalue related issues). |
|
Using From a mathematical point of view, an invalid input can't be converted to an integer. An invalid integer can't be expressed by VHDL. Moreover, an array access with an invalid pointer should lead to an invalid data word. for i in 0 to 7 loop
if is_X(r2.byte_index(i)) then
data_permuted(i * 8 + 7 downto i * 8) := (others => 'X');
else
j := to_integer(r2.byte_index(i)) * 8;
data_permuted(i * 8 + 7 downto i * 8) := d_in.data(j + 7 downto j);
end if;
end loop;This could be improved by a handwritten If your tool might create logic for the package utils is
-- Distinguishes simulation from synthesis
constant SIMULATION : boolean; -- deferred constant declaration
end package;
package body utils is
function is_simulation return boolean is
variable ret : boolean := false;
begin
--synthesis translate_off
ret := true;
--synthesis translate_on
return ret;
end function;
-- deferred constant assignment
constant SIMULATION : boolean := is_simulation;
end package body; |
|
Thanks @Paebbels and @umarcor. I can see why we can't propagate U/X state through integers, and we either need to modify the code to return U/X state as @Paebbels highlights, or avoid feeding U/X state to conversion functions in the first place. I'm working on that now. While I have you both here - I hacked up some python to parse our records and ports, and spit out VHDL assertions such that we can catch places we propagate U/X state across module boundaries. In general we've been too lax and I'm finding it difficult to root cause gate level issues after the code goes through ghdl -> yosys -> verilog -> yosys (in openroad) with so much U/X state floating around. My script parses records, eg: and ports, eg To create: I wanted to use pyGHDL to do the parsing instead my hand coded stuff. I found the example at https://libraries.io/pypi/pyVHDLModel but it fails on a lot of our files (eg https://github.com/antonblanchard/microwatt/blob/master/icache.vhdl): Are we using newer features not implemented yet, or am I doing something wrong? |
|
Translating all of GHDLs internal AST into the pyVHDLModel is a huge task. I would say like 50..60% is done, but some parts like If you can provide your code, I can do 2 things in my vacation:
General note:
@umarcor |
|
We currently test against |
|
Thanks @Paebbels. I'm not sure it warrants a bunch of your vacation time since I have a pretty hacky solution. The first argument is the file to get the port information from, and all subsequent ones are files to get record information from. |
After getting GHDL to emit some more information about metavalue assertions, the worst offenders are (the first column is the number of times I saw the assertion):
The text was updated successfully, but these errors were encountered: