[Mlir-commits] [mlir] 88fbccd - Revert "Fix canonicalizer to copy the entire GreedyRewriteConfig instead of selected fields"

[Mehdi Amini]

Author: Mehdi Amini
Date: 2023-08-09T21:28:24-07:00
New Revision: 88fbccd7ffcc1f99f7f52dbdfe82438b880a976e

URL: https://github.com/llvm/llvm-project/commit/88fbccd7ffcc1f99f7f52dbdfe82438b880a976e
DIFF: https://github.com/llvm/llvm-project/commit/88fbccd7ffcc1f99f7f52dbdfe82438b880a976e.diff

LOG: Revert "Fix canonicalizer to copy the entire GreedyRewriteConfig instead of selected fields"

This reverts commit e468c60c96e1af1945179c2ca80b7a9dfcd38398.

Flang is broken, investigating...

Added: 
    

Modified: 
    mlir/include/mlir/IR/OpImplementation.h
    mlir/lib/Transforms/Canonicalizer.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index 2131fe313f8c59..0eeb8bb1ec8da5 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -715,20 +715,18 @@ class AsmParser {
   //===--------------------------------------------------------------------===//
 
   /// This class represents a StringSwitch like class that is useful for parsing
-  /// expected keywords. On construction, unless a non-empty keyword is
-  /// provided, it invokes `parseKeyword` and processes each of the provided
-  /// cases statements until a match is hit. The provided `ResultT` must be
-  /// assignable from `failure()`.
+  /// expected keywords. On construction, it invokes `parseKeyword` and
+  /// processes each of the provided cases statements until a match is hit. The
+  /// provided `ResultT` must be assignable from `failure()`.
   template <typename ResultT = ParseResult>
   class KeywordSwitch {
   public:
-    KeywordSwitch(AsmParser &parser, StringRef *keyword = nullptr)
+    KeywordSwitch(AsmParser &parser)
         : parser(parser), loc(parser.getCurrentLocation()) {
-      if (keyword && !keyword->empty())
-        this->keyword = *keyword;
-      else if (failed(parser.parseKeywordOrCompletion(&this->keyword)))
+      if (failed(parser.parseKeywordOrCompletion(&keyword)))
         result = failure();
     }
+
     /// Case that uses the provided value when true.
     KeywordSwitch &Case(StringLiteral str, ResultT value) {
       return Case(str, [&](StringRef, SMLoc) { return std::move(value); });

diff  --git a/mlir/lib/Transforms/Canonicalizer.cpp b/mlir/lib/Transforms/Canonicalizer.cpp
index d50019bd6aee55..b4ad85c7c7dad2 100644
--- a/mlir/lib/Transforms/Canonicalizer.cpp
+++ b/mlir/lib/Transforms/Canonicalizer.cpp
@@ -29,8 +29,7 @@ struct Canonicalizer : public impl::CanonicalizerBase<Canonicalizer> {
   Canonicalizer() = default;
   Canonicalizer(const GreedyRewriteConfig &config,
                 ArrayRef<std::string> disabledPatterns,
-                ArrayRef<std::string> enabledPatterns)
-      : config(config) {
+                ArrayRef<std::string> enabledPatterns) {
     this->topDownProcessingEnabled = config.useTopDownTraversal;
     this->enableRegionSimplification = config.enableRegionSimplification;
     this->maxIterations = config.maxIterations;
@@ -42,31 +41,30 @@ struct Canonicalizer : public impl::CanonicalizerBase<Canonicalizer> {
   /// Initialize the canonicalizer by building the set of patterns used during
   /// execution.
   LogicalResult initialize(MLIRContext *context) override {
-    // Set the config from possible pass options set in the meantime.
-    config.useTopDownTraversal = topDownProcessingEnabled;
-    config.enableRegionSimplification = enableRegionSimplification;
-    config.maxIterations = maxIterations;
-    config.maxNumRewrites = maxNumRewrites;
-
     RewritePatternSet owningPatterns(context);
     for (auto *dialect : context->getLoadedDialects())
       dialect->getCanonicalizationPatterns(owningPatterns);
     for (RegisteredOperationName op : context->getRegisteredOperations())
       op.getCanonicalizationPatterns(owningPatterns, context);
 
-    patterns = std::make_shared<FrozenRewritePatternSet>(
-        std::move(owningPatterns), disabledPatterns, enabledPatterns);
+    patterns = FrozenRewritePatternSet(std::move(owningPatterns),
+                                       disabledPatterns, enabledPatterns);
     return success();
   }
   void runOnOperation() override {
+    GreedyRewriteConfig config;
+    config.useTopDownTraversal = topDownProcessingEnabled;
+    config.enableRegionSimplification = enableRegionSimplification;
+    config.maxIterations = maxIterations;
+    config.maxNumRewrites = maxNumRewrites;
     LogicalResult converged =
-        applyPatternsAndFoldGreedily(getOperation(), *patterns, config);
+        applyPatternsAndFoldGreedily(getOperation(), patterns, config);
     // Canonicalization is best-effort. Non-convergence is not a pass failure.
     if (testConvergence && failed(converged))
       signalPassFailure();
   }
-  GreedyRewriteConfig config;
-  std::shared_ptr<const FrozenRewritePatternSet> patterns;
+
+  FrozenRewritePatternSet patterns;
 };
 } // namespace