[SystemVerilog] parameter vs. localparam in packages

[vogelpi]

I recently observed that in the OpenTitan codebase, we are not using parameter or localparam consistently when it comes to constants that are defined inside a package, but that are used outside the package in the corresponding module.

For example, the auto-generated register packages such as aes_reg_pkg.sv define all constants as parameter as they are used mainly outside the package. In contrast, some manually-written packages like hmac_pkg.sv we use localparam for constants that are used outside the package but inside the hmac RTL.

Our style guide currently is ambiguous regarding this. On one hand it says in https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#constants:

When declaring a constant:

• within a package use parameter.
• within a module or class use localparam.

The preferred method of defining constants is to declare a package and declare all constants as a parameter within that package. If the constants are to be used in only one file, it is acceptable to keep them defined within that file rather than a separate package.

On the other hand, it also recommends in https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#parameterized-objects-modules-etc:

• Use parameter to parameterize, and localparam to declare module-scoped constants.
  • Use package parameters to transmit global constants through a hierarchy instead of parameters. To declare a constant whose scope is internal to the module, use localparam instead.

To me, this guideline is quite clear when it comes to packages: use parameter for everything that is used also outside the package and use localparam only if the constant is used within the package exclusively. Or did I miss something?

Maybe we should clarify in the latter section of the style guide that by "module-scoped"/"module" the SystemVerilog module and not the IP core is meant. WDYT?

[imphil]

localparam and parameter within a package have the same meaning, localparam doesn't imply "package-local scope". That's why the current guideline ways "parameter only in packages" to accurately convey that meaning. I think the style guide is correct as it is today, but the wording could potentially be improved. And of course our use within OT.

[tomeroberts]

It could be argued that the difference between localparam and parameter is mutability, not scope. We use this in module parameter declarations in some places to make it clear what is a parameter that the "user" can change (parameter), and what is a derived constant (localparam). By that argument, everything in packages should probably be localparam.

Having said that, it makes no difference which term is used as Philipp says, and we have already established the use of parameter so we should probably stick to it, and make it clear in the style guide.

[tjaychen]

so is it decided then? we should convert (when we can) all localparam usage in packages to parameter? I honestly don't care, so the sooner this is settled the better for me.

Or do you guys prefer to get more feedback?

On Tue, Apr 21, 2020 at 5:43 AM Tom Roberts notifications@github.com wrote:

It could be argued that the difference between localparam and parameter is mutability, not scope. We use this in module parameter declarations in some places to make it clear what is a parameter that the "user" can change (parameter), and what is a derived constant (localparam). By that argument, everything in packages should probably be localparam.

Having said that, it makes no difference which term is used as Philipp says, and we have already established the use of parameter so we should probably stick to it, and make it clear in the style guide.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/style-guides/issues/35#issuecomment-617155494, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSVSFYJXFCUVQQ6OZ3TRNWIHDANCNFSM4MNHIFIA .

[eunchan]

I also don't care the usage inside package as it doesn't change the usage of the parameters outside of the module. So, feel free to make an agreement without me. I will follow.

On Tue, Apr 21, 2020 at 07:01:02PM -0700, tjaychen wrote:

so is it decided then? we should convert (when we can) all localparam usage in packages to parameter? I honestly don't care, so the sooner this is settled the better for me.

Or do you guys prefer to get more feedback?

On Tue, Apr 21, 2020 at 5:43 AM Tom Roberts notifications@github.com wrote:

It could be argued that the difference between localparam and parameter is mutability, not scope. We use this in module parameter declarations in some places to make it clear what is a parameter that the "user" can change (parameter), and what is a derived constant (localparam). By that argument, everything in packages should probably be localparam.

Having said that, it makes no difference which term is used as Philipp says, and we have already established the use of parameter so we should probably stick to it, and make it clear in the style guide.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/style-guides/issues/35#issuecomment-617155494, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSVSFYJXFCUVQQ6OZ3TRNWIHDANCNFSM4MNHIFIA .

-- You are receiving this because you were assigned. Reply to this email directly or view it on GitHub: https://github.com/lowRISC/style-guides/issues/35#issuecomment-617502256

[vogelpi]

Thanks guys for your feedback. It's good we are on the same page.

I prepared a minimal PR to emphasize that within packages, parameter should be used instead of localparam.