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

Document mux limitation to 4096 #128

Open
1 of 4 tasks
numero-744 opened this issue Oct 19, 2022 · 6 comments
Open
1 of 4 tasks

Document mux limitation to 4096 #128

numero-744 opened this issue Oct 19, 2022 · 6 comments

Comments

@numero-744
Copy link
Collaborator

numero-744 commented Oct 19, 2022

Ancestor: SpinalHDL/SpinalHDL#230

@Dolu1990
Copy link
Member

It isn't specific to mux i think, by default, there is a safety check to not generate signal larger than 4096 bits.
This can be increase via SpinalConfig https://github.com/SpinalHDL/SpinalHDL/blob/dev/core/src/main/scala/spinal/core/Spinal.scala#L168

@numero-744
Copy link
Collaborator Author

Okay, I found (zsh % grep -r "bitVectorWidthMax" **/src) it is used only in Phase.scala:1285. (I see also Phase.scala:1297 and I'm wondering why the literal 4096 is used here 😅)

What about not adding it to RTD, but instead pointing to the configuration parameter in the error message?

Also, I'm curious about why this limit is needed.

@Readon
Copy link
Collaborator

Readon commented Oct 21, 2022

Okay, I found (zsh % grep -r "bitVectorWidthMax" **/src) it is used only in Phase.scala:1285. (I see also Phase.scala:1297 and I'm wondering why the literal 4096 is used here 😅)

What about not adding it to RTD, but instead pointing to the configuration parameter in the error message?

Also, I'm curious about why this limit is needed.

The large bit width might cause a bundle of difficulties in timing closure.
So limit it by default, and allow user to change it as their requirement is a good policy.
Just like to say "These can break the design, and if you notice the consequences and insist, we'll provide ways to change it."

@dlmiles
Copy link
Contributor

dlmiles commented May 30, 2023

Would we be ok to have this as a System.property ? -Pspinal.core.SpinalConfig.bitVectorWidthMax=4096

I have a new RTD page in the queue Java Environment Variables and Properties to help document such things in one place.

Are you saying:

  • both locations above should be based on the same setting.
  • that default setting is unlikely to be modified by many users.
  • the default is a good value at 4096
  • the setting needs to be a positive integer value > 0

The error message when exceeding the limit would explain the current limit, the value observed (that is above the limit) and the mechanism to override it that exists.

I have found a number of areas where at multiple levels there is a default-value/configurable-value not always propagated consistently to every place, or require two different methods of configuration to achieve a consistent setting.

Agree with Readon's comments.

@Dolu1990
Copy link
Member

Dolu1990 commented Jun 3, 2023

Hmm i hink i would prefer a environnement variable directly, instead of a JVM specific thing ?

@dlmiles
Copy link
Contributor

dlmiles commented Jun 7, 2023

I use both but understand if you don't use properties much at the moment, the introduction of the first would seem out of place and alien to the project.

A JVM specific thing should be ok as SpinalHDL only works on a JVM. Properties are not limited to use just with the JVM itself, everything uses properties as a method of configuration, such that .properties files exist of course to manage things.

Please suggest a name then for this new ENV ? that is now not immediately clear. SPINAL_BITVECTORWIDTHMAX ?

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

No branches or pull requests

4 participants