[SV] Enhanced name validation: support IEEE standard escape name checking #8518
Conversation
…function to support IEEE compliant escaped name checking. Adjusted related calls to enable new parameter allowEscapedName.
There was a problem hiding this comment.
Thanks for starting this. I've actually wanted to use this in a couple places and thought it would require way more effort to get working.
Can you include at least one test of this behavior? E.g., this might be a good place: https://github.com/llvm/circt/blob/main/test/Conversion/ExportVerilog/name-legalize.mlir
include/circt/Dialect/SV/SVDialect.h
Outdated
| /// If \p allowEscapedName is true, IEEE-compliant escaped names are allowed to | ||
| /// pass the check. | ||
| bool isNameValid(llvm::StringRef name, bool caseInsensitiveKeywords, | ||
| bool allowEscapedName = false); |
There was a problem hiding this comment.
Escaped names are legal in both Verilog (1364-1995 2.7.1) and SystemVerilog (1800-2023 5.6.1). I don't see a good reason to make this optionally allowed given that (I think) all Verilog versions support escaped identifiers. The only difference is on what the identifiers are.
There was a problem hiding this comment.
Escaped names are legal in both Verilog (1364-1995 2.7.1) and SystemVerilog (1800-2023 5.6.1). I don't see a good reason to make this optionally allowed given that (I think) all Verilog versions support escaped identifiers. The only difference is on what the identifiers are.
Yes, I concur with your viewpoint and believe that this feature should be enabled by default. Given that my understanding of CIRCT is not particularly in-depth, I am not entirely certain whether this modification might cause any side effects. Out of caution, I have temporarily set it as an optional feature, hoping that it will at least meet my needs. After extensive validation, you, I, or anyone else can set it as a default feature, which I consider to be a prudent choice.
Thank you for your reminder. I will add some tests at a later time. |
… new module instances, moduleInstEscNameModExt and moduleInstEscNameModExt2, to ensure they handle escape characters correctly. This change enhances support for IEEE-compliant escaped names.
Change Overview
This PR updates the name validation mechanism in the SV dialect, mainly modifying the
isNameValidfunction in theSVDialect.handLegalizeNames.cppfiles to support escaped name checking in accordance with the IEEE standard.Change Reason
IEEE Standard Compliance: The original name validation function lacks the correct handling of escaped identifiers in the IEEE Verilog standard. The IEEE 1800 standard allows backslash-escaped identifiers (such as
\escaped_name), which are common in Verilog code generation.Function Enhancement: By adding a new
allowEscapedNameparameter, theisNameValidfunction can decide whether to allow escaped names based on the context, providing a more flexible name validation strategy.Backward compatibility: The modification maintains compatibility with existing APIs while expanding functionality so that callers can choose whether to enable escape name support as needed.
Code generation quality: Correct handling of escape names can avoid illegal identifiers during Verilog code generation, improving the quality and standard compliance of generated code.
Major changes
isNameValidfunction signature and addallowEscapedNameparameter