From 87604c796a7dfea497265032d9e5783068e123af Mon Sep 17 00:00:00 2001
From: ASDAlexander77 <duzhar@googlemail.com>
Date: Mon, 14 Sep 2026 17:49:43 +0100
Subject: [PATCH] Make canonicalization patterns report failure when they
 change nothing

MLIR's greedy rewrite driver treats every successful pattern as an IR change
and re-scans the region, up to 10 times. NormalizeCast, RemoveUnused and
RemoveUnusedAccessor returned success even when they left the IR untouched
(a cast they do not rewrite, an op that still has uses), so canonicalization
never converged: on a small test every canonicalizer run hit the iteration
limit ("did not converge after scanning 10 times", 9 times), with 714 false
successes from NormalizeCast and 470 from RemoveUnused<LoadOp> alone.

They now return failure when nothing was rewritten or erased. On that test the
driver processes 699 operations instead of 4523, with no convergence warnings.
The emitted LLVM IR is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 tslang/lib/TypeScript/TypeScriptOps.cpp | 29 ++++++++++++++-----------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/tslang/lib/TypeScript/TypeScriptOps.cpp b/tslang/lib/TypeScript/TypeScriptOps.cpp
index 6e9aecade..82db55cb6 100644
--- a/tslang/lib/TypeScript/TypeScriptOps.cpp
+++ b/tslang/lib/TypeScript/TypeScriptOps.cpp
@@ -305,11 +305,13 @@ template <typename T> struct RemoveUnused : public OpRewritePattern<T>
 
     LogicalResult matchAndRewrite(T op, PatternRewriter &rewriter) const override
     {
-        if (op->getResult(0).use_empty())
+        // report failure when nothing was erased: the greedy driver re-scans after every success
+        if (!op->getResult(0).use_empty())
         {
-            rewriter.eraseOp(op);
+            return failure();
         }
 
+        rewriter.eraseOp(op);
         return success();
     }
 };
@@ -359,16 +361,13 @@ template <typename T> struct RemoveUnusedAccessor : public OpRewritePattern<T>
 
     LogicalResult matchAndRewrite(T op, PatternRewriter &rewriter) const override
     {
-        if (op.getSetValue())
+        // a setter call has side effects; otherwise erase only an unused getter
+        if (op.getSetValue() || !op.getValue().use_empty())
         {
-            return success();
-        }
-
-        if (op.getValue().use_empty())
-        {
-            rewriter.eraseOp(op);
+            return failure();
         }
 
+        rewriter.eraseOp(op);
         return success();
     }
 };
@@ -762,9 +761,11 @@ struct NormalizeCast : public OpRewritePattern<mlir_ts::CastOp>
 
         // union support
         // TODO: review this code, should it be in "cast" logic?
+        // a pattern must report failure when it leaves the IR unchanged, otherwise the greedy driver
+        // treats every cast as a rewrite and keeps re-running until its iteration limit
         if (isa<mlir_ts::AnyType>(res.getType()))
         {
-            return success();
+            return failure();
         }
 
         auto resUnionType = dyn_cast<mlir_ts::UnionType>(res.getType());
@@ -778,9 +779,10 @@ struct NormalizeCast : public OpRewritePattern<mlir_ts::CastOp>
                 auto typeOfValue = rewriter.create<mlir_ts::TypeOfOp>(loc, mlir_ts::StringType::get(rewriter.getContext()), in);
                 auto unionValue = rewriter.create<mlir_ts::CreateUnionInstanceOp>(loc, res.getType(), in, typeOfValue);
                 rewriter.replaceOp(castOp, ValueRange{unionValue});
+                return success();
             }
 
-            return success();
+            return failure();
         }
 
         // TODO: review it, if you still need it as we are should be using "safeCast"
@@ -791,12 +793,13 @@ struct NormalizeCast : public OpRewritePattern<mlir_ts::CastOp>
             {
                 auto value = rewriter.create<mlir_ts::GetValueFromUnionOp>(loc, res.getType(), in);
                 rewriter.replaceOp(castOp, ValueRange{value});
+                return success();
             }
 
-            return success();
+            return failure();
         }
 
-        return success();
+        return failure();
     }
 };