[flang] use runRegionDCE instead of a custom DCE in cg-rewrite

[jeanPerier]

Summary

The custom DCE in cg-rewrite is meant to get rid of fir.shape, fir.shift,
fir.shape_shift and fir.slice ops as well as their unused operands
before codegen (that does not lower those abstract operation to LLVM).

However, it turned out to be flowed in case some fir.shape operands were
unused outside of fir.shape and appeared several times as operands:
they were erased at the first appearance, causing the further attemp
to erase it to segfault (since the op IR storage was deallocated).

Instead of trying to fixing the custom DCE code, use mlir::runRegionDCE.

[clementval]

Nice cleanup! LGTM

flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp

 
 #include "CGOps.h"
 #include "flang/Optimizer/Builder/Todo.h" // remove when TODO's are done
 #include "flang/Optimizer/Dialect/FIRDialect.h"
 #include "flang/Optimizer/Dialect/FIROps.h"
 #include "flang/Optimizer/Dialect/FIRType.h"
 #include "flang/Optimizer/Support/FIRContext.h"
 #include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/RegionUtils.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 
 namespace fir {
 #define GEN_PASS_DEF_CODEGENREWRITE
 #include "flang/Optimizer/CodeGen/CGPasses.h.inc"
 } // namespace fir
 
matchAndRewrite(fir::DeclareOp declareOp,
     return mlir::success();
   }
 };
 
 class CodeGenRewrite : public fir::impl::CodeGenRewriteBase<CodeGenRewrite> {
 public:
   void runOn(mlir::Operation *op, mlir::Region &region) {
     auto &context = getContext();
-    mlir::OpBuilder rewriter(&context);
     mlir::ConversionTarget target(context);
     target.addLegalDialect<mlir::arith::ArithDialect, fir::FIROpsDialect,
                            fir::FIRCodeGenDialect, mlir::func::FuncDialect>();
     target.addIllegalOp<fir::ArrayCoorOp>();
     target.addIllegalOp<fir::ReboxOp>();
     target.addIllegalOp<fir::DeclareOp>();
     target.addDynamicallyLegalOp<fir::EmboxOp>([](fir::EmboxOp embox) {
       return !(embox.getShape() || embox.getType()
                                        .cast<fir::BaseBoxType>()
                                        .getEleTy()
                                        .isa<fir::SequenceType>());
     });
     mlir::RewritePatternSet patterns(&context);
     patterns.insert<EmboxConversion, ArrayCoorConversion, ReboxConversion,
                     DeclareOpConversion>(&context);
     if (mlir::failed(
             mlir::applyPartialConversion(op, target, std::move(patterns)))) {
       mlir::emitError(mlir::UnknownLoc::get(&context),
                       "error in running the pre-codegen conversions");
       signalPassFailure();
+      return;
     }
-    // Erase any residual.
-    simplifyRegion(region);
+    // Erase any residual (fir.shape, fir.slice...).
+    mlir::IRRewriter rewriter(&context);
+    (void)mlir::runRegionDCE(rewriter, op->getRegions());
   }
 
   void runOnOperation() override final {
     // Call runOn on all top level regions that may contain emboxOp/arrayCoorOp.
     auto mod = getOperation();
     for (auto func : mod.getOps<mlir::func::FuncOp>())
       runOn(func, func.getBody());
     for (auto global : mod.getOps<fir::GlobalOp>())
       runOn(global, global.getRegion());
   }
-
-  // Clean up the region.
-  void simplifyRegion(mlir::Region &region) {
-    for (auto &block : region.getBlocks())
-      for (auto &op : block.getOperations()) {
-        for (auto &reg : op.getRegions())
-          simplifyRegion(reg);
-        maybeEraseOp(&op);
-      }
-    doDCE();
-  }
-
-  /// Run a simple DCE cleanup to remove any dead code after the rewrites.
-  void doDCE() {
-    std::vector<mlir::Operation *> workList;
-    workList.swap(opsToErase);
-    while (!workList.empty()) {
-      for (auto *op : workList) {
-        std::vector<mlir::Value> opOperands(op->operand_begin(),
-                                            op->operand_end());
-        LLVM_DEBUG(llvm::dbgs() << "DCE on " << *op << '\n');
-        ++numDCE;
-        op->erase();
-        for (auto opnd : opOperands)
-          maybeEraseOp(opnd.getDefiningOp());
-      }
-      workList.clear();
-      workList.swap(opsToErase);
-    }
-  }
-
-  void maybeEraseOp(mlir::Operation *op) {
-    if (!op)
-      return;
-    if (op->hasTrait<mlir::OpTrait::IsTerminator>())
-      return;
-    if (mlir::isOpTriviallyDead(op))
-      opsToErase.push_back(op);
-  }
-
-private:
-  std::vector<mlir::Operation *> opsToErase;
 };
 
 } // namespace
 
 std::unique_ptr<mlir::Pass> fir::createFirCodeGenRewritePass() {
   return std::make_unique<CodeGenRewrite>();
 }

flang/test/Fir/declare-codegen.fir

func.func @test(%arg0: !fir.ref<!fir.array<12x23xi32>>) {
   return
 }
 func.func private @bar(%arg0: !fir.ref<!fir.array<12x23xi32>>)
 
 
 // CHECK-LABEL: func.func @test(
 // CHECK-SAME: %[[arg0:.*]]: !fir.ref<!fir.array<12x23xi32>>) {
 // CHECK-NEXT: fir.call @bar(%[[arg0]]) : (!fir.ref<!fir.array<12x23xi32>>) -> ()
+
+func.func @useless_shape_with_duplicate_extent_operand(%arg0: !fir.ref<!fir.array<3x3xf32>>) {
+  %c3 = arith.constant 3 : index
+  %1 = fir.shape %c3, %c3 : (index, index) -> !fir.shape<2>
+  %2 = fir.declare %arg0(%1) {uniq_name = "u"} : (!fir.ref<!fir.array<3x3xf32>>, !fir.shape<2>) -> !fir.ref<!fir.array<3x3xf32>>
+  return
+}
+
+// CHECK-LABEL: func.func @useless_shape_with_duplicate_extent_operand(
+// CHECK-NEXT: return