Skip to content
This repository has been archived by the owner on Aug 20, 2024. It is now read-only.

Module inlining runs after optimizations #1298

Closed
3 of 5 tasks
jackkoenig opened this issue Jan 8, 2020 · 4 comments
Closed
3 of 5 tasks

Module inlining runs after optimizations #1298

jackkoenig opened this issue Jan 8, 2020 · 4 comments

Comments

@jackkoenig
Copy link
Contributor

This can prevent lots of optimizations. It's only problematic if you have multiple instances of the module being inlined which prevents cross-module Constant Propagation.

This should be trivial to fix with the Dependency API (see #1123) assuming there isn't any good reason for it running after optimizations.

Checklist

  • Did you specify the current behavior?
  • Did you specify the expected behavior?
  • Did you provide a code example showing the problem?
  • Did you describe your environment?
  • Did you specify relevant external information?

What is the current behavior?

Inlining runs after optimizations

What is the expected desired behavior?

Inlining runs before optimizations

Steps to Reproduce

Have a module instantiated multiple times marked for inlining that has constants driving its ports for only one instance.

External Information

Nope

@seldridge
Copy link
Member

seldridge commented Jan 8, 2020

Note: this is a property of any inputForm=LowForm transform. Slightly more accurately, low form transforms will run right before the emitter. Consequently, a low form transform will run after optimizations only if a compiler that wants optimizations is used. The behavior thereby changes for low FIRRTL emitter, minimum verilog emitter, or verilog/system verilog emitter. 🤷‍♀️

See #1165 for more info trying to document this.

Note that #1123 preserves this behavior if dependencies are specified using input/outputForm.

@jackkoenig
Copy link
Contributor Author

Ah yeah, thanks for the clarification @seldridge! I was just noting this and hopefully we can resolve this issue by leveraging your hard work 🙂

@seldridge
Copy link
Member

I think this is fixed on master. There's no optionalPrerequisite defined for InlineIntsances now. Consequently, this runs before optimizations now:

./utils/bin/firrtl -i regress/RocketCore.fir --compiler verilog --target-dir test_run_dir --log-level info --inline RocketCore.IBuf
# Computed transform order in: 516.5 ms
# Determined Transform order that will be executed:
#   firrtl.stage.transforms.Compiler
#   ├── firrtl.passes.CheckChirrtl$
#   ├── firrtl.passes.CInferTypes$
#   ├── firrtl.passes.CInferMDir$
#   ├── firrtl.passes.RemoveCHIRRTL$
#   ├── firrtl.passes.ToWorkingIR$
#   ├── firrtl.passes.CheckHighForm$
#   ├── firrtl.passes.ResolveKinds$
#   ├── firrtl.passes.InferTypes$
#   ├── firrtl.passes.CheckTypes$
#   ├── firrtl.passes.Uniquify$
#   ├── firrtl.stage.TransformManager
#   │   ├── firrtl.passes.ResolveKinds$
#   │   └── firrtl.passes.InferTypes$
#   ├── firrtl.passes.ResolveFlows$
#   ├── firrtl.passes.CheckFlows$
#   ├── firrtl.passes.InferBinaryPoints
#   ├── firrtl.passes.TrimIntervals
#   ├── firrtl.passes.InferWidths
#   ├── firrtl.passes.CheckWidths$
#   ├── firrtl.transforms.InferResets
#   ├── firrtl.stage.TransformManager
#   │   └── firrtl.passes.CheckTypes$
#   ├── firrtl.transforms.DedupModules
#   ├── firrtl.passes.PullMuxes$
#   ├── firrtl.passes.ReplaceAccesses$
#   ├── firrtl.passes.ExpandConnects$
#   ├── firrtl.passes.ZeroLengthVecs$
#   ├── firrtl.passes.RemoveAccesses$
#   ├── firrtl.stage.TransformManager
#   │   ├── firrtl.passes.Uniquify$
#   │   └── firrtl.stage.TransformManager
#   │       ├── firrtl.passes.ResolveKinds$
#   │       └── firrtl.passes.InferTypes$
#   ├── firrtl.passes.ExpandWhensAndCheck
#   ├── firrtl.stage.TransformManager
#   │   ├── firrtl.passes.ResolveKinds$
#   │   ├── firrtl.passes.InferTypes$
#   │   ├── firrtl.passes.ResolveFlows$
#   │   └── firrtl.passes.InferWidths
#   ├── firrtl.passes.RemoveIntervals
#   ├── firrtl.passes.ConvertFixedToSInt$
#   ├── firrtl.passes.ZeroWidth$
#   ├── firrtl.stage.TransformManager
#   │   └── firrtl.passes.InferTypes$
#   ├── firrtl.passes.LowerTypes$
#   ├── firrtl.stage.TransformManager
#   │   ├── firrtl.passes.ResolveKinds$
#   │   ├── firrtl.passes.InferTypes$
#   │   ├── firrtl.passes.ResolveFlows$
#   │   └── firrtl.passes.InferWidths
#   ├── firrtl.passes.Legalize$
#   ├── firrtl.transforms.RemoveReset$
#   ├── firrtl.stage.TransformManager
#   │   └── firrtl.passes.ResolveFlows$
#   ├── firrtl.transforms.CheckCombLoops
#   ├── firrtl.checks.CheckResets
#   ├── firrtl.transforms.RemoveWires
#   ├── firrtl.passes.InlineInstances <<<<<<<<<<<<<<<<<<<<<<<<< Inlining
#   ├── firrtl.stage.TransformManager
#   │   └── firrtl.passes.ResolveKinds$
#   ├── firrtl.passes.RemoveValidIf$
#   ├── firrtl.transforms.ConstantPropagation <<<<<<<<<<<<<<<<< Optimization
#   ├── firrtl.passes.PadWidths$
#   ├── firrtl.stage.TransformManager
#   │   ├── firrtl.transforms.ConstantPropagation
#   │   └── firrtl.passes.Legalize$
#   ├── firrtl.passes.memlib.VerilogMemDelays$
#   ├── firrtl.stage.TransformManager
#   │   ├── firrtl.transforms.ConstantPropagation
#   │   └── firrtl.stage.TransformManager
#   │       └── firrtl.passes.Legalize$
#   ├── firrtl.passes.SplitExpressions$
#   ├── firrtl.transforms.CombineCats
#   ├── firrtl.passes.CommonSubexpressionElimination$
#   ├── firrtl.transforms.DeadCodeElimination
#   └── firrtl.VerilogEmitter
#       ├── firrtl.transforms.BlackBoxSourceHelper
#       ├── firrtl.transforms.FixAddingNegativeLiterals
#       ├── firrtl.transforms.ReplaceTruncatingArithmetic
#       ├── firrtl.transforms.InlineBitExtractionsTransform
#       ├── firrtl.transforms.PropagatePresetAnnotations
#       ├── firrtl.transforms.InlineCastsTransform
#       ├── firrtl.transforms.LegalizeClocksTransform
#       ├── firrtl.transforms.FlattenRegUpdate
#       ├── firrtl.transforms.DeadCodeElimination
#       ├── firrtl.passes.VerilogModulusCleanup$
#       ├── firrtl.transforms.VerilogRename
#       ├── firrtl.passes.VerilogPrep$
#       └── firrtl.AddDescriptionNodes

@seldridge
Copy link
Member

Closed via 39d76a0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants