[flang-commits] [flang] [flang] de-duplicate AbstractResult pass (PR #88867)

via flang-commits flang-commits at lists.llvm.org
Thu Apr 18 10:52:07 PDT 2024


================
@@ -304,9 +330,7 @@ inline void createDebugPasses(
 inline void createDefaultFIRCodeGenPassPipeline(
     mlir::PassManager &pm, MLIRToLLVMPassPipelineConfig config) {
   fir::addBoxedProcedurePass(pm);
-  pm.addNestedPass<mlir::func::FuncOp>(
-      fir::createAbstractResultOnFuncOptPass());
-  pm.addNestedPass<fir::GlobalOp>(fir::createAbstractResultOnGlobalOptPass());
+  addNestedPassToAllTopLevelOperations(pm, fir::createAbstractResultOptPass);
----------------
jeanPerier wrote:

> canScheduleOn guards against running pipelines on operation types which the pass is not intended for.

In that case, I think it should be runnable on any op (the ReturnOp handling prevents that currently since you noticed it does not work on ModuleOp).

> We have to implement canScheduleOn because it is pure virtual in mlir::Pass.

You do not need to in that case because AbstractResultOptPass actually inherits from `mlir::OperationPass<>` that defines canScheduleOn [here](https://github.com/llvm/llvm-project/blob/74e07ab523122d6a8347b25770062ab331b6bb84/mlir/include/mlir/Pass/Pass.h#L373) in a way that it would say true to any operation it is being schedule on. The .td `Pass<>` syntax actually creates a pass inheriting from `mlir::OperationPass<>`, not directly from `mlir::Pass` (see [here](https://github.com/llvm/llvm-project/blob/98427264cab558d285c7222f1f25b904124c6b93/mlir/include/mlir/Pass/PassBase.td#L94C3-L94C38)).

So all in all, I am OK with your patch, it is an improvement, and it could be further improved by modifying the `ReturnOp` handling and removing the canScheduleOn restriction.

https://github.com/llvm/llvm-project/pull/88867


More information about the flang-commits mailing list