[Mlir-commits] [mlir] [mlir][cf] Do not access erased operation in `cf.cond_br` lowering (PR #148358)

[Matthias Springer]

https://github.com/matthias-springer created https://github.com/llvm/llvm-project/pull/148358

Do not access the erased `cf.cond_br` 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`.


>From 409723b850575ec2589ee81d033b5fea3f88eb58 Mon Sep 17 00:00:00 2001
From: Matthias Springer <me at m-sp.org>
Date: Sat, 12 Jul 2025 11:05:59 +0000
Subject: [PATCH] [mlir][cf] Do not access erased operation in `cf.cond_br`
 lowering

---
 .../ControlFlowToLLVM/ControlFlowToLLVM.cpp     | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp b/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp
index 88a8b7fb185c5..716ab140ad6c7 100644
--- a/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp
+++ b/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp
@@ -138,11 +138,12 @@ struct BranchOpLowering : public ConvertOpToLLVMPattern<cf::BranchOp> {
                           TypeRange(adaptor.getOperands()));
     if (failed(convertedBlock))
       return failure();
+    auto attrs = op->getAttrDictionary();
     Operation *newOp = rewriter.replaceOpWithNewOp<LLVM::BrOp>(
         op, adaptor.getOperands(), *convertedBlock);
     // TODO: We should not just forward all attributes like that. But there are
     // existing Flang tests that depend on this behavior.
-    newOp->setAttrs(op->getAttrDictionary());
+    newOp->setAttrs(attrs);
     return success();
   }
 };
@@ -166,18 +167,14 @@ struct CondBranchOpLowering : public ConvertOpToLLVMPattern<cf::CondBranchOp> {
                           TypeRange(adaptor.getFalseDestOperands()));
     if (failed(convertedFalseBlock))
       return failure();
+    auto attrs = op->getAttrDictionary();
     auto newOp = rewriter.replaceOpWithNewOp<LLVM::CondBrOp>(
-        op, adaptor.getCondition(), *convertedTrueBlock,
-        adaptor.getTrueDestOperands(), *convertedFalseBlock,
-        adaptor.getFalseDestOperands());
-    ArrayRef<int32_t> weights = op.getWeights();
-    if (!weights.empty()) {
-      newOp.setWeights(weights);
-      op.removeBranchWeightsAttr();
-    }
+        op, adaptor.getCondition(), adaptor.getTrueDestOperands(),
+        adaptor.getFalseDestOperands(), op.getBranchWeightsAttr(),
+        *convertedTrueBlock, *convertedFalseBlock);
     // TODO: We should not just forward all attributes like that. But there are
     // existing Flang tests that depend on this behavior.
-    newOp->setAttrs(op->getAttrDictionary());
+    newOp->setAttrs(attrs);
     return success();
   }
 };