[Mlir-commits] [mlir] [mlir][SPIRV] Do not access erased op in SPIRV->LLVM lowering (PR #148373)

[Matthias Springer]

https://github.com/matthias-springer updated https://github.com/llvm/llvm-project/pull/148373

>From b67efe61d2b9a93c2930680da81ce35710ea104d Mon Sep 17 00:00:00 2001
From: Matthias Springer <me at m-sp.org>
Date: Sat, 12 Jul 2025 13:56:49 +0000
Subject: [PATCH 1/2] [mlir][SPIRV] Do not access erased op in SPIRV->LLVM
 lowering

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`.
---
 mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
index cea9d1fdec809..e3aabb7e7e625 100644
--- a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
+++ b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
@@ -779,13 +779,15 @@ class GlobalVariablePattern
     auto linkage = storageClass == spirv::StorageClass::Private
                        ? LLVM::Linkage::Private
                        : LLVM::Linkage::External;
+    auto locationAttrName = op.getLocationAttrName();
+    auto locationAttr = op.getLocationAttr();
     auto newGlobalOp = rewriter.replaceOpWithNewOp<LLVM::GlobalOp>(
         op, dstType, isConstant, linkage, op.getSymName(), Attribute(),
         /*alignment=*/0, storageClassToAddressSpace(clientAPI, storageClass));
 
     // Attach location attribute if applicable
-    if (op.getLocationAttr())
-      newGlobalOp->setAttr(op.getLocationAttrName(), op.getLocationAttr());
+    if (locationAttr)
+      newGlobalOp->setAttr(locationAttrName, locationAttr);
 
     return success();
   }
@@ -1426,7 +1428,6 @@ class SelectionPattern : public SPIRVToLLVMConversion<spirv::SelectionOp> {
         headerBlock->getOperations().front());
     if (!condBrOp)
       return failure();
-    rewriter.eraseBlock(headerBlock);
 
     // Branch from merge block to continue block.
     auto *mergeBlock = op.getMergeBlock();
@@ -1444,6 +1445,7 @@ class SelectionPattern : public SPIRVToLLVMConversion<spirv::SelectionOp> {
                                     falseBlock,
                                     condBrOp.getFalseTargetOperands());
 
+    rewriter.eraseBlock(headerBlock);
     rewriter.inlineRegionBefore(op.getBody(), continueBlock);
     rewriter.replaceOp(op, continueBlock->getArguments());
     return success();

>From 882d28ce5d73b8808736bd875d7d859f4b94a28c Mon Sep 17 00:00:00 2001
From: Matthias Springer <me at m-sp.org>
Date: Sat, 12 Jul 2025 19:31:06 +0200
Subject: [PATCH 2/2] Update mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Markus Böck <markus.boeck02 at gmail.com>
---
 mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
index e3aabb7e7e625..1d92b5d5562b5 100644
--- a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
+++ b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
@@ -779,8 +779,8 @@ class GlobalVariablePattern
     auto linkage = storageClass == spirv::StorageClass::Private
                        ? LLVM::Linkage::Private
                        : LLVM::Linkage::External;
-    auto locationAttrName = op.getLocationAttrName();
-    auto locationAttr = op.getLocationAttr();
+    StringAttr locationAttrName = op.getLocationAttrName();
+    IntegerAttr locationAttr = op.getLocationAttr();
     auto newGlobalOp = rewriter.replaceOpWithNewOp<LLVM::GlobalOp>(
         op, dstType, isConstant, linkage, op.getSymName(), Attribute(),
         /*alignment=*/0, storageClassToAddressSpace(clientAPI, storageClass));