From 409723b850575ec2589ee81d033b5fea3f88eb58 Mon Sep 17 00:00:00 2001 From: Matthias Springer Date: Sat, 12 Jul 2025 11:05:59 +0000 Subject: [PATCH 1/2] [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 { TypeRange(adaptor.getOperands())); if (failed(convertedBlock)) return failure(); + auto attrs = op->getAttrDictionary(); Operation *newOp = rewriter.replaceOpWithNewOp( 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 { TypeRange(adaptor.getFalseDestOperands())); if (failed(convertedFalseBlock)) return failure(); + auto attrs = op->getAttrDictionary(); auto newOp = rewriter.replaceOpWithNewOp( - op, adaptor.getCondition(), *convertedTrueBlock, - adaptor.getTrueDestOperands(), *convertedFalseBlock, - adaptor.getFalseDestOperands()); - ArrayRef 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(); } }; From 6ad7cefe5598c1decd816739b62cdfe4fcdc811c Mon Sep 17 00:00:00 2001 From: Matthias Springer Date: Sat, 12 Jul 2025 16:14:41 +0200 Subject: [PATCH 2/2] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Markus Böck --- mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp b/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp index 716ab140ad6c7..13a084407e53f 100644 --- a/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp +++ b/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp @@ -138,7 +138,7 @@ struct BranchOpLowering : public ConvertOpToLLVMPattern { TypeRange(adaptor.getOperands())); if (failed(convertedBlock)) return failure(); - auto attrs = op->getAttrDictionary(); + DictionaryAttr attrs = op->getAttrDictionary(); Operation *newOp = rewriter.replaceOpWithNewOp( op, adaptor.getOperands(), *convertedBlock); // TODO: We should not just forward all attributes like that. But there are @@ -167,7 +167,7 @@ struct CondBranchOpLowering : public ConvertOpToLLVMPattern { TypeRange(adaptor.getFalseDestOperands())); if (failed(convertedFalseBlock)) return failure(); - auto attrs = op->getAttrDictionary(); + DictionaryAttr attrs = op->getAttrDictionary(); auto newOp = rewriter.replaceOpWithNewOp( op, adaptor.getCondition(), adaptor.getTrueDestOperands(), adaptor.getFalseDestOperands(), op.getBranchWeightsAttr(),