[CIR][ThroughMLIR] Lower simple SwitchOp. #1871
Conversation
terapines-osc-cir
requested review from
lanza,
bcardosolopes,
xlauko and
andykaylor
as code owners
| /// Create a pass for transforming CIR operations to more 'scf' dialect-friendly | ||
| /// forms. It rewrites operations that aren't supported by 'scf', such as breaks | ||
| /// and continues. | ||
| std::unique_ptr<mlir::Pass> createMLIRLoweringPreparePass(); |
There was a problem hiding this comment.
I'm a bit skeptical of having an MLIRLoweringPrepare pass that is separate from the existing LoweringPrepare pass. For one thing, "lowering through MLIR" is a very vague concept. The fact that you're mentioning scf as the target dialect here is good. I'd like to consider what's in LoweringPrepare to see if things can be reorganized in a more general way so anyone lowering to arbitrary dialects will be able to reason about what passes they need to run to prepare.
There was a problem hiding this comment.
I agree.
I'd expect lowering to SCF to be selected at LoweringPreparePass::runOnOperation() based using -fno-direct-lowering. You could still keep the implementation on a distinct file, but following Andy's suggestion of location
| @@ -0,0 +1,112 @@ | |||
| #include "mlir/IR/BuiltinOps.h" | |||
There was a problem hiding this comment.
This needs a file header comment block.
|
|
||
| namespace cir { | ||
|
|
||
| struct MLIRLoweringPrepare |
There was a problem hiding this comment.
This should probably be in the lib/CIR/Dialects/Transforms directory. That, of course, relates to my comment above about the focus of this pass.
| /// Create a pass for transforming CIR operations to more 'scf' dialect-friendly | ||
| /// forms. It rewrites operations that aren't supported by 'scf', such as breaks | ||
| /// and continues. | ||
| std::unique_ptr<mlir::Pass> createMLIRLoweringPreparePass(); |
There was a problem hiding this comment.
I agree.
I'd expect lowering to SCF to be selected at LoweringPreparePass::runOnOperation() based using -fno-direct-lowering. You could still keep the implementation on a distinct file, but following Andy's suggestion of location
The PR 1742 contains too many conflicts with the main branch.
Got it re-patched to the current main branch.