transformations: (csl) use constants instead of params

[dk949]

csl.params allow for flexibility we do not utilise whilst providing a possibility of accidentally overriding a value which was not intended to be overridden. In the ModuleOp.get_param_value function we even use the value of the param as if it will always be there.

After this PR: csl_wrapper.ParamAttribute can no longer omit its value (now referred to as "value" not "default")

In the layout module:

• All params are now arith.constants

In the program module:

• All params from the csl_wrapper.ParamAttribute are now arith.constants
• Params from the layout module yield arguments are still csl.params.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.02%. Comparing base (71aecc8) to head (e4a9c96).
Report is 79 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3319      +/-   ##
==========================================
- Coverage   90.04%   90.02%   -0.02%     
==========================================
  Files         445      445              
  Lines       56219    56214       -5     
  Branches     5398     5396       -2     
==========================================
- Hits        50620    50609      -11     
- Misses       4180     4184       +4     
- Partials     1419     1421       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[n-io]

Yes, unfortunately our programs are currently not general enough to support freely modifying the csl.params. I still retain some hope that we (or others in other contexts?) may eventually get there, and this discrepancy may not be the fault of the param translation. Translating ParamAttribute to arith.constant seems the right fix for us, but semantically - and long-term - translating ParamAttribute to csl.ParamOp appears logical.

Introducing a ConstAttribute alongside the param attribute might be cleaner but also cumbersome. We could potentially introduce a flag to lower-csl-wrapper e.g. a boolean lower-params-as-const that enables lowering params with a default/value to consts when set?

[dk949]

I have a suggestion then: at the moment, we rename ParamAttribute to ConstAttribute and if/when we ever need to pass params to the layout module, we can reintroduce the ParamAttribute.

Note that it's still possible to pass params to the program module by yielding from the layout region.

[superlopuh]

What's the latest on this? Can we close the PR until a solution is found?

[dk949]

What's the latest on this? Can we close the PR until a solution is found?

Yes, I think this can be sorted out at some other time