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

[Jean Perier via flang-commits]

Author: Jean Perier
Date: 2023-02-03T10:30:16+01:00
New Revision: 2adcf1b25d03402278892f88d467c667a6fdd938

URL: https://github.com/llvm/llvm-project/commit/2adcf1b25d03402278892f88d467c667a6fdd938
DIFF: https://github.com/llvm/llvm-project/commit/2adcf1b25d03402278892f88d467c667a6fdd938.diff

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

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.

Differential Revision: https://reviews.llvm.org/D143247

Added: 
    

Modified: 
    flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
    flang/test/Fir/declare-codegen.fir

Removed: 
    


################################################################################
diff  --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
index 044c2e775a6a7..a45fed2a36798 100644
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -19,6 +19,7 @@
 #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"
 
@@ -284,7 +285,6 @@ 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>();
@@ -305,9 +305,11 @@ class CodeGenRewrite : public fir::impl::CodeGenRewriteBase<CodeGenRewrite> {
       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 {
@@ -318,48 +320,6 @@ class CodeGenRewrite : public fir::impl::CodeGenRewriteBase<CodeGenRewrite> {
     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

diff  --git a/flang/test/Fir/declare-codegen.fir b/flang/test/Fir/declare-codegen.fir
index 1b0bea88e9cff..9d68d3b2f9d4d 100644
--- a/flang/test/Fir/declare-codegen.fir
+++ b/flang/test/Fir/declare-codegen.fir
@@ -18,3 +18,13 @@ 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