[InstCombine] Don't replace unused `atomicrmw xchg` with `atomic store`

Following the discussion from https://reviews.llvm.org/D141277 and in
particular Ralf Jung's comment at
https://reviews.llvm.org/D141277#inline-1365148, replacing an unused `atomicrmw
xchg` into an `atomic store` is illegal even for release ordering.

Quoting Connor Horman from the rust lang discussion linked in that comment:
"An acquire operation A only synchronizes-with a release operation R if it
takes its value from R, or any store in the release sequence headed by R, which
is R, followed by the longest continuous sequence of read-modify-write
operations.
A regular store following R in the modification order would break the release
sequence, and if an acquire operation reads that store or something later, then
it loses any synchronization it might have already had."

This fixes https://github.com/llvm/llvm-project/issues/60418

Differential Revision: https://reviews.llvm.org/D142097

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
index e73667f..7b0860b 100644 (file)
--- a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
@@ -121,19 +121,6 @@ Instruction *InstCombinerImpl::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
          Ordering != AtomicOrdering::Unordered &&
          "AtomicRMWs don't make sense with Unordered or NotAtomic");
 
-  // Any atomicrmw xchg with no uses can be converted to a atomic store if the
-  // ordering is compatible.
-  if (RMWI.getOperation() == AtomicRMWInst::Xchg &&
-      RMWI.use_empty()) {
-    if (Ordering != AtomicOrdering::Release &&
-        Ordering != AtomicOrdering::Monotonic)
-      return nullptr;
-    new StoreInst(RMWI.getValOperand(), RMWI.getPointerOperand(),
-                  /*isVolatile*/ false, RMWI.getAlign(), Ordering,
-                  RMWI.getSyncScopeID(), &RMWI);
-    return eraseInstFromFunction(RMWI);
-  }
-
   if (!isIdempotentRMW(RMWI))
     return nullptr;
 

diff --git a/llvm/test/Transforms/InstCombine/atomicrmw.ll b/llvm/test/Transforms/InstCombine/atomicrmw.ll
index cb2ebf7..0c35904 100644 (file)
--- a/llvm/test/Transforms/InstCombine/atomicrmw.ll
+++ b/llvm/test/Transforms/InstCombine/atomicrmw.ll
@@ -273,7 +273,7 @@ define double @sat_fsub_nan(ptr %addr) {
 
 define void @sat_fsub_nan_unused(ptr %addr) {
 ; CHECK-LABEL: @sat_fsub_nan_unused(
-; CHECK-NEXT:    store atomic double 0x7FF00000FFFFFFFF, ptr [[ADDR:%.*]] monotonic, align 8
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw xchg ptr [[ADDR:%.*]], double 0x7FF00000FFFFFFFF monotonic, align 8
 ; CHECK-NEXT:    ret void
 ;
   atomicrmw fsub ptr %addr, double 0x7FF00000FFFFFFFF monotonic
@@ -282,7 +282,7 @@ define void @sat_fsub_nan_unused(ptr %addr) {
 
 define void @xchg_unused_monotonic(ptr %addr) {
 ; CHECK-LABEL: @xchg_unused_monotonic(
-; CHECK-NEXT:    store atomic i32 0, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw xchg ptr [[ADDR:%.*]], i32 0 monotonic, align 4
 ; CHECK-NEXT:    ret void
 ;
   atomicrmw xchg ptr %addr, i32 0 monotonic
@@ -291,7 +291,7 @@ define void @xchg_unused_monotonic(ptr %addr) {
 
 define void @xchg_unused_release(ptr %addr) {
 ; CHECK-LABEL: @xchg_unused_release(
-; CHECK-NEXT:    store atomic i32 -1, ptr [[ADDR:%.*]] release, align 4
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw xchg ptr [[ADDR:%.*]], i32 -1 release, align 4
 ; CHECK-NEXT:    ret void
 ;
   atomicrmw xchg ptr %addr, i32 -1 release
@@ -300,7 +300,7 @@ define void @xchg_unused_release(ptr %addr) {
 
 define void @xchg_unused_under_aligned(ptr %addr) {
 ; CHECK-LABEL: @xchg_unused_under_aligned(
-; CHECK-NEXT:    store atomic i32 -1, ptr [[ADDR:%.*]] release, align 1
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw xchg ptr [[ADDR:%.*]], i32 -1 release, align 1
 ; CHECK-NEXT:    ret void
 ;
   atomicrmw xchg ptr %addr, i32 -1 release, align 1
@@ -309,7 +309,7 @@ define void @xchg_unused_under_aligned(ptr %addr) {
 
 define void @xchg_unused_over_aligned(ptr %addr) {
 ; CHECK-LABEL: @xchg_unused_over_aligned(
-; CHECK-NEXT:    store atomic i32 -1, ptr [[ADDR:%.*]] release, align 8
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw xchg ptr [[ADDR:%.*]], i32 -1 release, align 8
 ; CHECK-NEXT:    ret void
 ;
   atomicrmw xchg ptr %addr, i32 -1 release, align 8
@@ -336,7 +336,7 @@ define void @xchg_unused_volatile(ptr %addr) {
 
 define void @sat_or_allones_unused(ptr %addr) {
 ; CHECK-LABEL: @sat_or_allones_unused(
-; CHECK-NEXT:    store atomic i32 -1, ptr [[ADDR:%.*]] monotonic, align 4
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw xchg ptr [[ADDR:%.*]], i32 -1 monotonic, align 4
 ; CHECK-NEXT:    ret void
 ;
   atomicrmw or ptr %addr, i32 -1 monotonic