[CIR] Fix LLVM lowering crashes after #155244

[mmha]

This patch replaces all uses of replaceAllUseWith with replaceOp. This fixes a number of regressions post #155244.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Morris Hafner (mmha)

Changes

This patch replaces all uses of replaceAllUseWith with replaceOp. This fixes a number of regressions post #155244.

---

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

1 Files Affected:

• (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+2-4)

diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index ee9f58c829ca9..f8dac2aa02a0c 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -1355,8 +1355,7 @@ mlir::LogicalResult CIRToLLVMConstantOpLowering::matchAndRewrite(
     } else {
       const mlir::Value initVal =
           lowerCirAttrAsValue(op, op.getValue(), rewriter, typeConverter);
-      rewriter.replaceAllUsesWith(op, initVal);
-      rewriter.eraseOp(op);
+      rewriter.replaceOp(op, initVal);
       return mlir::success();
     }
   } else if (const auto recordAttr =
@@ -2653,8 +2652,7 @@ mlir::LogicalResult CIRToLLVMVTableGetVPtrOpLowering::matchAndRewrite(
   // pointer to the vptr type. Since the LLVM dialect uses opaque pointers
   // we can just replace uses of this operation with the original pointer.
   mlir::Value srcVal = adaptor.getSrc();
-  rewriter.replaceAllUsesWith(op, srcVal);
-  rewriter.eraseOp(op);
+  rewriter.replaceOp (op, srcVal);
   return mlir::success();
 }
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[andykaylor]

This looks good, but it isn't clear to me why the crashes were occurring. I see from the description of #155244 that replaceAllUsesWith now has a delayed effect in some circumstances. Was the fact that we were erasing the replaced op causing a problem?

[mmha]

My understanding is that we are not supposed to erase an op that was replaced anymore. eraseOp calls replaceOp internally which lead to the double replacement assertion failure.

I also think in our case it doesn't really matter whether we use replaceOp and replaceAllUseWith