Skip to content

[mlir][TOSA] Do not access erased operation #148374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matthias-springer
Copy link
Member

@matthias-springer matthias-springer commented Jul 12, 2025

This commit fixes two occurrences where an erased op was accessed at a later point of time. That won't work anymore in a One-Shot Dialect Conversion and triggers a use-after-free sanitizer error.

After the One-Shot Dialect Conversion refactoring, a ConversionPatternRewriter will behave more like a normal PatternRewriter.

@llvmbot
Copy link
Member

llvmbot commented Jul 12, 2025

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-linalg

Author: Matthias Springer (matthias-springer)

Changes

Do not access the erased MaxPool2dOp operation in the lowering pattern. That won't work anymore in a One-Shot Dialect Conversion and triggers a use-after-free sanitizer error.

After the One-Shot Dialect Conversion refactoring, a ConversionPatternRewriter will behave more like a normal PatternRewriter.


Full diff: https://github.com/llvm/llvm-project/pull/148374.diff

1 Files Affected:

  • (modified) mlir/lib/Conversion/TosaToLinalg/TosaToLinalgNamed.cpp (+6-6)
diff --git a/mlir/lib/Conversion/TosaToLinalg/TosaToLinalgNamed.cpp b/mlir/lib/Conversion/TosaToLinalg/TosaToLinalgNamed.cpp
index c1f40dcbd5ca0..4f62dc13e33b9 100644
--- a/mlir/lib/Conversion/TosaToLinalg/TosaToLinalgNamed.cpp
+++ b/mlir/lib/Conversion/TosaToLinalg/TosaToLinalgNamed.cpp
@@ -808,6 +808,7 @@ class MaxPool2dConverter : public OpConversionPattern<tosa::MaxPool2dOp> {
         dilationAttr);
 
     rewriter.setInsertionPointAfter(op);
+    auto nanMode = op.getNanMode();
     rewriter.replaceOp(op, resultOp);
 
     // NaN propagation has no meaning for non floating point types.
@@ -821,11 +822,10 @@ class MaxPool2dConverter : public OpConversionPattern<tosa::MaxPool2dOp> {
     // we've already produced a named op we will just take its body and modify
     // it to include the appropriate checks. If the current value is NaN the
     // old value of pool will be taken otherwise we use the result.
-    if (const auto nanMode = op.getNanMode(); nanMode == "IGNORE") {
+    if (nanMode == "IGNORE") {
       auto genericOp = rewriter.create<linalg::GenericOp>(
-          op->getLoc(), resultOp.getType(0), resultOp.getInputs(),
-          resultOp.getOutputs(), resultOp.getIndexingMapsArray(),
-          resultOp.getIteratorTypesArray(),
+          loc, resultOp.getType(0), resultOp.getInputs(), resultOp.getOutputs(),
+          resultOp.getIndexingMapsArray(), resultOp.getIteratorTypesArray(),
           [&](OpBuilder &opBuilder, Location loc, ValueRange blockArgs) {
             IRMapping map;
             auto oldBlock = resultOp.getRegion().begin();
@@ -834,10 +834,10 @@ class MaxPool2dConverter : public OpConversionPattern<tosa::MaxPool2dOp> {
             map.map(oldArgs, blockArgs);
             auto *newOp = opBuilder.clone(oldMaxOp, map);
             Value isNaN = opBuilder.create<arith::CmpFOp>(
-                op->getLoc(), arith::CmpFPredicate::UNO, blockArgs.front(),
+                loc, arith::CmpFPredicate::UNO, blockArgs.front(),
                 blockArgs.front());
             auto selectOp = opBuilder.create<arith::SelectOp>(
-                op->getLoc(), isNaN, blockArgs.back(), newOp->getResult(0));
+                loc, isNaN, blockArgs.back(), newOp->getResult(0));
             opBuilder.create<linalg::YieldOp>(loc, selectOp.getResult());
           });
       rewriter.replaceOp(resultOp, genericOp);

@matthias-springer matthias-springer force-pushed the users/matthias-springer/tosa_use_after_free branch from c6c23ad to aceb407 Compare July 12, 2025 14:01
@matthias-springer matthias-springer changed the title [mlir][TOSA] Do not access erased op in MaxPool2dOp lowering [mlir][TOSA] Do not access erased operation Jul 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants