[FIRRTL] Add a new FIRRTL annotation to specify type lowering behavior of module body

[uenoku]

Add a new annotation to control type lowering behavior for internal signals
within a module, separate from the port convention. This allows more fine-grained
control over how aggregate types are handled inside modules.

The new annotation works similarly to ConventionAnnotation but applies to
internal signals rather than module ports. It supports the same conventions
and includes an 'includeHierarchy' option to apply the setting to all
modules in the hierarchy.

[fabianschuiki]

LGTM

[seldridge]

The includeHierarchy field seems unnecessary. I also think this is unnecessary for the Convention annotation. My reasoning here is that convention is really supposed to "Port Lowering ABI". This is something that is only defined on public modules and everything else is up to the compiler to decide.

Is there a request for includeHierarchy and/or is there a way that we could avoid this and handle it entirely from Chisel, e.g., add some builder state which indicates what the convention is within a scope and then apply it to all modules under that scope?

That said, I am fine with delaying this for now as long as this never becomes a part of an implied ABI that a user relies on.

[seldridge]

Nice, exhaustive test. 👍

[uenoku]

The includeHierarchy field seems unnecessary. I also think this is unnecessary for the Convention annotation. My reasoning here is that convention is really supposed to "Port Lowering ABI". This is something that is only defined on public modules and everything else is up to the compiler to decide.

Fair point, I'm totally fine to drop includeHierachy for Convention annotation. For body type lowering, it's useful to annotate the annotation for design module and ensure that module bodies in the design is scalarized.

add some builder state which indicates what the convention is within a scope and then apply it to all modules under that scope?

That would be great. If we can mark every module in design with this annotation in Chisel, we really don't need includeHierachy.

There will never be no ABI change regarding body lowering. The intention of the annotation is more like per-module compiler option used in LowerTypes.

[uenoku]

I updated to remove includeHierarchy from convention attr and clean up the annotation application. Body type lowering annotation still has includeHierarchy since it looks not worth modifying chisel internal for this specific annotation.

[seldridge]

LGTM

[seldridge]

What is interesting about these changes is that we could move in a direction of removing the CLI options for aggregate preservation and instead rely on the annotations / attributes. As a middle ground, it may be better to change the CLI (in a follow-on) to apply the annotations or do it as a part of parsing.

This is more of a thought than a definite direction we should go.