Skip to content
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

validate component and declaration naming #1468

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

NikLeberg
Copy link
Contributor

related #1463

Context, Motivation & Description

Spinal now checks the names of components and declarations for validity in the currently active language.
If an invalid name is found, it fails the generation.
If an invalid name in another than the currently active language is found, it reports a warning.

Impact on code generation

Spinal will now refuse to generate code that has invalid identifiers. Previously Spinal did just generate it and at a later step the user had to find out (through GHDL, Quartus, etc.) that the generated code was invalid.

Checklist

  • Unit tests were added
  • API changes are or will be documented: -> no changes to API

@Dolu1990
Copy link
Member

Dolu1990 commented Jul 1, 2024

Thanks :D

The one thing i'm a bit worrying is the execution overhead of regex.
I realy have no idea how much processing it cost.
Do you have any idea ?

@Readon
Copy link
Collaborator

Readon commented Jul 1, 2024

A rough comparison can be carried by testing time of SpinalHDL.
This is normal one.
1719825239830

Here is the one that test time of this PR.
1719825220732

Slightly increase but not that much.

@NikLeberg
Copy link
Contributor Author

Generally the overhead of the RegEx(es) should be low. It does not use any repeats (i.e. + or * in the pattern) and as such does not have to do backtracking. And backtracking is the costly thing in regex. So it should be fine I guess?

I tried to give a quantitative answer and ran multiple iterations of a Spinal generation, with and without the proposed changes. But the JIT compilation of Scala optimized it so well that it resulted in roughly identical runtimes for both cases. 🤷‍♂️

@Dolu1990 Dolu1990 merged commit ffb9a45 into SpinalHDL:dev Jul 3, 2024
11 checks passed
@Dolu1990
Copy link
Member

Dolu1990 commented Jul 3, 2024

Thanks ^^

@NikLeberg NikLeberg deleted the phase_naming_check branch July 3, 2024 09:43
@NikLeberg
Copy link
Contributor Author

It's a pleasure :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants