Integrate new transforms with firrtl.stage.Forms #1754
Conversation
b16ed48 to
cfe6002
Compare
I have a question to this. My question is possible to remove this after some a milestone like fedora get this update? I think distributions like Ubuntu or CentOS should not affect our development since they contains a very bad package maintaining policy. |
|
Generally look good to me:) |
a2172aa to
e9f40d9
Compare
Move new transforms, recently added, into existing or new sets of transforms (defined in firrtl.stage.Forms). One transform is a mandatory low FIRRTL optimization: - firrtl.transforms.LegalizeAndReductionsTransform Previously, this was included as a prerequisite of all Verilog emitters (minimum, normal, and SystemVerilog). Two transforms associated with converting and removing the new verification statements are moved into a new set of transforms, AssertsRemoved: - firrtl.transforms.formal.ConvertAsserts - firrtl.transforms.formal.RemoveVerificationStatements Previously, these transforms were directly added as prerequisites to the minimum Verilog and normal Verilog emitter, but not the SystemVerilog emitter. The designation of inputForm=LowForm for legacy, custom transforms is updated to include assertion removal transforms as part of their optionalPrerequisites. This has the effect of continuing to cause inputForm=LowForm transforms to run as late as possible (right before the low FIRRTL, minimum Verilog, Verilog, or SystemVeriog emitter). Tests are updated to reflect the new order in both CustomTransformSpec and LoweringCompilersSpec. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
|
@sequencer: Can you take a look at this again? This is just rebased onto |
|
I missed the earlier comment about |
Move new transforms, recently added, into existing or new sets of
transforms (defined in firrtl.stage.Forms).
One transform is a mandatory low FIRRTL optimization:
Previously, this was included as a prerequisite of all Verilog
emitters (minimum, normal, and SystemVerilog).
Two transforms associated with converting and removing the new
verification statements are moved into a new set of transforms,
AssertsRemoved:
Previously, these transforms were directly added as prerequisites to
the minimum Verilog and normal Verilog emitter, but not the
SystemVerilog emitter.
The designation of inputForm=LowForm for legacy, custom transforms is
updated to include assertion removal transforms as part of their
optionalPrerequisites. This has the effect of continuing to cause
inputForm=LowForm transforms to run as late as possible (right before
the low FIRRTL, minimum Verilog, Verilog, or SystemVeriog emitter).
Tests are updated to reflect the new order in both CustomTransformSpec
and LoweringCompilersSpec. This commit also simplifies the test used
in the CustomTransformSpec to assert that inputForm=LowForm legacy
transforms run right before the emitter. The new test looks only for a
list of (customTransform, emitter) in a sliding, size-2 window of the
flattened transform order. Previously, this was looking for a match
before and after the custom transform. The old implementation
necessitate busywork updates of the test when new transforms are added
that changed the transform running before the custom transform.
Depends on #1753.
This should not be backported due to references to 1.4 verification statements.
Contributor Checklist
Type of Improvement
API Impact
This adds
firrtl.stage.Forms.AssertRemovedto the API.Backend Code Generation Impact
None.
Desired Merge Strategy
Release Notes
None
Reviewer Checklist (only modified by reviewer)
Please Merge?