Fix questa hjson file with needed options #25764
Conversation
Signed-off-by: Mostafa Hassan <[email protected]>
mostafaabdallah12
requested review from
marnovandermaas
and removed request for
a team
|
@rswarbrick I fixed the indentation issue. and added notes for the suppressed errors. |
There was a problem hiding this comment.
I started adding notes to this but realised that I was writing the same thing many times over :-)
Would you mind giving a list of examples for each waived rule? We might just be able to fix them, or otherwise should probably file issues to track fixing them properly.
| "-svext=hierbinsof", | ||
| // Questa throws error on some LRM violations & restrictions. We demoted these errors since they will not impact the flow: | ||
| //2744: It shall be illegal to have an import statement directory within a class scope. | ||
| // [DOC: IEEE Std 1800-2009 Verilog LRM - Section 26.3 "Referencing data in packages"] |
There was a problem hiding this comment.
Eek! The waiver looks perfectly sensible, but is it easy for you to generate a list of examples that currently fail? I don't have a Questa licence, but I'd be very happy to tidy this up...
There was a problem hiding this comment.
I'm working (very slowly) through this list of waivers. I think the first one will be something we can remove once #26002 is merged. (If not, thank you for testing! Please tell me what I've missed!)
| // Questa throws error on some LRM violations & restrictions. We demoted these errors since they will not impact the flow: | ||
| //2744: It shall be illegal to have an import statement directory within a class scope. | ||
| // [DOC: IEEE Std 1800-2009 Verilog LRM - Section 26.3 "Referencing data in packages"] | ||
| //2912: The specified port cannot be connected by name because it does not exist. |
There was a problem hiding this comment.
This is very surprising to me! That would presumably be a bug in the code in question? Can you send examples that are currently failing this?
There was a problem hiding this comment.
Again, please could you provide a list of examples where this doesn't work? (Or alternatively a pattern that will find them with grep?)
| //2744: It shall be illegal to have an import statement directory within a class scope. | ||
| // [DOC: IEEE Std 1800-2009 Verilog LRM - Section 26.3 "Referencing data in packages"] | ||
| //2912: The specified port cannot be connected by name because it does not exist. | ||
| //7070: An expression of string type can be assigned directly to a variable of string type. |
There was a problem hiding this comment.
Again, this looks rather surprising to me! I assume that bit [7:0] foo = "12" won't set footo8'hc` :-) Can you send a list of examples that currently fail this?
|
@rswarbrick Can we proceed with the pull request and add the change since it's needed, and I will file issues for all suppressible errors we added for with more details |
|
Sorry for not replying more quickly. I'd really appreciate at least a list of what the magic IDs mean. I don't have a Questa licence, so I don't know what the numbers mean at all. The output could be an issue (generated by me) to track the fact that this needs sorting out at our end. |
|
@rswarbrick I thought we had added notes for all IDs. I updated the last commit to have notes for all the IDs. |
| "-c", | ||
| "-sv_lib {QUESTA_HOME}/uvm-1.2/linux_x86_64/uvm_dpi", | ||
| // Questa throws error on some LRM violations & restrictions. We demoted these errors since they will not impact the flow: | ||
| //8323: dv_macros.svh has a macro printing null using %0d format specifier, Questa throws an error on this. |
There was a problem hiding this comment.
Oof, that sounds like a genuine bug in some DV code ("passing a null pointer to something that wants an integer"). Can you give a list of examples of this? I'd love to just fix them!
We've been working for the last two months to get OpenTitan to work with Questa, and we have succeeded!
We changed some setup files to add questa options and setups.
This is one of the setup files: hw/dv/tools/dvsim/questa.hjson
Once it's accepted in OpenTitan master branch we will push the remaining setup files.