-
-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
VHDL2019 assert api - partial #748
Conversation
This isn't the right way to implement it: the counters should be in C (e.g. in |
OK, I will give it some more work. So the counters will be incremented by |
OK, I see, similar to: ea457d5 |
180c20c
to
9133c66
Compare
Now ready for review |
165fd36
to
5ef70f0
Compare
lib/std.19/env-body.vhd
Outdated
begin | ||
report "not implemented" severity failure; | ||
return impl(warning) + impl(error) + impl(failure); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return impl(warning) + impl(error) + impl(failure); | |
return GetVhdlAssertCount(warning) + GetVhdlAssertCount(error) + GetVhdlAssertCount(failure); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the benefit of these two changes besides reducing number of places in the code where
the run-time is called? Isn't it going to be slower (altough negligibly, but still) to execute when
the "wrapping" function is called first instead of directly the function that returns the counter?
Or will the optimizations take care of that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The overhead of the extra call is minimal and should be inlined anyway. This way avoid duplicating the symbol name / interface of the foreign function.
test/regress/assert8.vhd
Outdated
|
||
assert (False) report "First warning" severity warning; | ||
|
||
print_asrt_state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to make the test self-checking and assert on the result of GetVhdlAssertCount
, IsVhdlAssertFailed
, etc. E.g.
assert GetVhdlAssertCount(warning) = 1;
assert IsVhdlAssertFailed;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is true, but still the test needs to have "gold" report since the asserts are expected to fail. Return
code will not distinguish correct/incorrect
a7c1785
to
037ea52
Compare
I added the format string part. |
Add partial implementation of: LCS2016-050
Still need to look at tests, some are failing.
Escaped identifiers with leading underscores are used in
std.env
to minimize "exposure" of theimplementation specific function and shared variable (instead of e.g.
assert_cnt
).