[flang-commits] [flang] 2dfaec7 - Revert "[flang] use greedy mlir driver for stack arrays pass"

[Tom Eccles via flang-commits]

Author: Tom Eccles
Date: 2023-05-24T16:15:52Z
New Revision: 2dfaec77811effd6b4956889217522f4f1fa696a

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

LOG: Revert "[flang] use greedy mlir driver for stack arrays pass"

This reverts commit 74c2ec50f393bad8b31d0dd0bd8b2ff44d361198.

This caused a regression building spec2017 with -Ofast.

Added: 
    

Modified: 
    flang/lib/Optimizer/Transforms/StackArrays.cpp

Removed: 
    


################################################################################
diff  --git a/flang/lib/Optimizer/Transforms/StackArrays.cpp b/flang/lib/Optimizer/Transforms/StackArrays.cpp
index d09ce292c46d7..60a30d2d1ef64 100644
--- a/flang/lib/Optimizer/Transforms/StackArrays.cpp
+++ b/flang/lib/Optimizer/Transforms/StackArrays.cpp
@@ -26,7 +26,7 @@
 #include "mlir/Interfaces/LoopLikeInterface.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Support/LogicalResult.h"
-#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include "mlir/Transforms/DialectConversion.h"
 #include "mlir/Transforms/Passes.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -167,22 +167,25 @@ class StackArraysAnalysisWrapper {
 
   StackArraysAnalysisWrapper(mlir::Operation *op) {}
 
-  // returns nullptr if analysis failed
-  const AllocMemMap *getCandidateOps(mlir::Operation *func);
+  bool hasErrors() const;
+
+  const AllocMemMap &getCandidateOps(mlir::Operation *func);
 
 private:
   llvm::DenseMap<mlir::Operation *, AllocMemMap> funcMaps;
+  bool gotError = false;
 
-  mlir::LogicalResult analyseFunction(mlir::Operation *func);
+  void analyseFunction(mlir::Operation *func);
 };
 
 /// Converts a fir.allocmem to a fir.alloca
 class AllocMemConversion : public mlir::OpRewritePattern<fir::AllocMemOp> {
 public:
-  explicit AllocMemConversion(
+  using OpRewritePattern::OpRewritePattern;
+
+  AllocMemConversion(
       mlir::MLIRContext *ctx,
-      const StackArraysAnalysisWrapper::AllocMemMap &candidateOps)
-      : OpRewritePattern(ctx), candidateOps{candidateOps} {}
+      const llvm::DenseMap<mlir::Operation *, InsertionPoint> &candidateOps);
 
   mlir::LogicalResult
   matchAndRewrite(fir::AllocMemOp allocmem,
@@ -193,8 +196,9 @@ class AllocMemConversion : public mlir::OpRewritePattern<fir::AllocMemOp> {
   static InsertionPoint findAllocaInsertionPoint(fir::AllocMemOp &oldAlloc);
 
 private:
-  /// Handle to the DFA (already run)
-  const StackArraysAnalysisWrapper::AllocMemMap &candidateOps;
+  /// allocmem operations that DFA has determined are safe to move to the stack
+  /// mapping to where to insert replacement freemem operations
+  const llvm::DenseMap<mlir::Operation *, InsertionPoint> &candidateOps;
 
   /// If we failed to find an insertion point not inside a loop, see if it would
   /// be safe to use an llvm.stacksave/llvm.stackrestore inside the loop
@@ -408,8 +412,7 @@ void AllocationAnalysis::processOperation(mlir::Operation *op) {
   visitOperationImpl(op, *before, after);
 }
 
-mlir::LogicalResult
-StackArraysAnalysisWrapper::analyseFunction(mlir::Operation *func) {
+void StackArraysAnalysisWrapper::analyseFunction(mlir::Operation *func) {
   assert(mlir::isa<mlir::func::FuncOp>(func));
   mlir::DataFlowSolver solver;
   // constant propagation is required for dead code analysis, dead code analysis
@@ -423,7 +426,8 @@ StackArraysAnalysisWrapper::analyseFunction(mlir::Operation *func) {
   solver.load<AllocationAnalysis>();
   if (failed(solver.initializeAndRun(func))) {
     llvm::errs() << "DataFlowSolver failed!";
-    return mlir::failure();
+    gotError = true;
+    return;
   }
 
   LatticePoint point{func};
@@ -454,17 +458,22 @@ StackArraysAnalysisWrapper::analyseFunction(mlir::Operation *func) {
                   : candidateOps) {
     llvm::dbgs() << "StackArrays: Found candidate op: " << *allocMemOp << '\n';
   });
-  return mlir::success();
 }
 
-const StackArraysAnalysisWrapper::AllocMemMap *
+bool StackArraysAnalysisWrapper::hasErrors() const { return gotError; }
+
+const StackArraysAnalysisWrapper::AllocMemMap &
 StackArraysAnalysisWrapper::getCandidateOps(mlir::Operation *func) {
-  if (!funcMaps.contains(func))
-    if (mlir::failed(analyseFunction(func)))
-      return nullptr;
-  return &funcMaps[func];
+  if (!funcMaps.count(func))
+    analyseFunction(func);
+  return funcMaps[func];
 }
 
+AllocMemConversion::AllocMemConversion(
+    mlir::MLIRContext *ctx,
+    const llvm::DenseMap<mlir::Operation *, InsertionPoint> &candidateOps)
+    : OpRewritePattern(ctx), candidateOps(candidateOps) {}
+
 mlir::LogicalResult
 AllocMemConversion::matchAndRewrite(fir::AllocMemOp allocmem,
                                     mlir::PatternRewriter &rewriter) const {
@@ -700,31 +709,29 @@ void StackArraysPass::runOnFunc(mlir::Operation *func) {
   assert(mlir::isa<mlir::func::FuncOp>(func));
 
   auto &analysis = getAnalysis<StackArraysAnalysisWrapper>();
-  const StackArraysAnalysisWrapper::AllocMemMap *candidateOps =
-      analysis.getCandidateOps(func);
-  if (!candidateOps) {
+  const auto &candidateOps = analysis.getCandidateOps(func);
+  if (analysis.hasErrors()) {
     signalPassFailure();
     return;
   }
 
-  if (candidateOps->empty())
+  if (candidateOps.empty())
     return;
-  runCount += candidateOps->size();
-
-  llvm::SmallVector<mlir::Operation *> opsToConvert;
-  opsToConvert.reserve(candidateOps->size());
-  for (auto [op, _] : *candidateOps)
-    opsToConvert.push_back(op);
+  runCount += candidateOps.size();
 
   mlir::MLIRContext &context = getContext();
   mlir::RewritePatternSet patterns(&context);
-  mlir::GreedyRewriteConfig config;
-  // prevent the pattern driver form merging blocks
-  config.enableRegionSimplification = false;
+  mlir::ConversionTarget target(context);
+
+  target.addLegalDialect<fir::FIROpsDialect, mlir::arith::ArithDialect,
+                         mlir::func::FuncDialect>();
+  target.addDynamicallyLegalOp<fir::AllocMemOp>([&](fir::AllocMemOp alloc) {
+    return !candidateOps.count(alloc.getOperation());
+  });
 
-  patterns.insert<AllocMemConversion>(&context, *candidateOps);
-  if (mlir::failed(mlir::applyOpPatternsAndFold(opsToConvert,
-                                                std::move(patterns), config))) {
+  patterns.insert<AllocMemConversion>(&context, candidateOps);
+  if (mlir::failed(
+          mlir::applyPartialConversion(func, target, std::move(patterns)))) {
     mlir::emitError(func->getLoc(), "error in stack arrays optimization\n");
     signalPassFailure();
   }