Skip to content

AtomicExpand: Stop trying to prune cmpxchg extractvalue users #103211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Aug 13, 2024

The expansion for cmpxchg was trying to tidy up extractvalue users
to directly use the lowered pieces, and then erasing the now dead
extractvalues. This was making an assumption about the iteration
order did not depend on those user instructions.

Continue doing the replacement, but just leave the dead extractvalues.
This is a minor regression, but it is of no importance since the
dead instructions will just get dropped during codegen anyway.

Copy link
Contributor Author

arsenm commented Aug 13, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @arsenm and the rest of your teammates on Graphite Graphite

@arsenm arsenm marked this pull request as ready for review August 13, 2024 14:24
@Northbadge Northbadge removed their request for review August 13, 2024 14:32
Base automatically changed from users/arsenm/atomic-expand-stop-precollecting-atomic-instrs to main August 13, 2024 15:51
@arsenm arsenm force-pushed the users/arsenm/atomic-expand-remove-prune branch from 6c3af75 to eade1f6 Compare August 13, 2024 15:55
@llvmbot
Copy link
Member

llvmbot commented Aug 13, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Matt Arsenault (arsenm)

Changes

The expansion for cmpxchg was trying to tidy up extractvalue users
to directly use the lowered pieces, and then erasing the now dead
extractvalues. This was making an assumption about the iteration
order did not depend on those user instructions.

Continue doing the replacement, but just leave the dead extractvalues.
This is a minor regression, but it is of no importance since the
dead instructions will just get dropped during codegen anyway.


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/AtomicExpandPass.cpp (-7)
  • (modified) llvm/test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll (+142-145)
diff --git a/llvm/lib/CodeGen/AtomicExpandPass.cpp b/llvm/lib/CodeGen/AtomicExpandPass.cpp
index d8f33c42a8a14c..b6344e0c27c4e6 100644
--- a/llvm/lib/CodeGen/AtomicExpandPass.cpp
+++ b/llvm/lib/CodeGen/AtomicExpandPass.cpp
@@ -1514,7 +1514,6 @@ bool AtomicExpandImpl::expandAtomicCmpXchg(AtomicCmpXchgInst *CI) {
 
   // Look for any users of the cmpxchg that are just comparing the loaded value
   // against the desired one, and replace them with the CFG-derived version.
-  SmallVector<ExtractValueInst *, 2> PrunedInsts;
   for (auto *User : CI->users()) {
     ExtractValueInst *EV = dyn_cast<ExtractValueInst>(User);
     if (!EV)
@@ -1527,14 +1526,8 @@ bool AtomicExpandImpl::expandAtomicCmpXchg(AtomicCmpXchgInst *CI) {
       EV->replaceAllUsesWith(Loaded);
     else
       EV->replaceAllUsesWith(Success);
-
-    PrunedInsts.push_back(EV);
   }
 
-  // We can remove the instructions now we're no longer iterating through them.
-  for (auto *EV : PrunedInsts)
-    EV->eraseFromParent();
-
   if (!CI->use_empty()) {
     // Some use of the full struct return that we don't understand has happened,
     // so we've got to reconstruct it properly.
diff --git a/llvm/test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll b/llvm/test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll
index 23aa57e18ecc5a..d1d6f89bccade1 100644
--- a/llvm/test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll
+++ b/llvm/test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll
@@ -1,169 +1,166 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt -passes=atomic-expand -codegen-opt-level=1 -S -mtriple=thumbv7s-apple-ios7.0 %s | FileCheck %s
 
-define i32 @test_cmpxchg_seq_cst(ptr %addr, i32 %desired, i32 %new) {
-; CHECK-LABEL: @test_cmpxchg_seq_cst
 ; Intrinsic for "dmb ishst" is then expected
-; CHECK:     br label %[[START:.*]]
-
-; CHECK: [[START]]:
-; CHECK:     [[LOADED:%.*]] = call i32 @llvm.arm.ldrex.p0(ptr elementtype(i32) %addr)
-; CHECK:     [[SHOULD_STORE:%.*]] = icmp eq i32 [[LOADED]], %desired
-; CHECK:     br i1 [[SHOULD_STORE]], label %[[FENCED_STORE:.*]], label %[[NO_STORE_BB:.*]]
-
-; CHECK: [[FENCED_STORE]]:
-; CHECK:     call void @llvm.arm.dmb(i32 10)
-; CHECK:     br label %[[TRY_STORE:.*]]
-
-; CHECK: [[TRY_STORE]]:
-; CHECK:     [[LOADED_TRYSTORE:%.*]] = phi i32 [ [[LOADED]], %[[FENCED_STORE]] ]
-; CHECK:     [[STREX:%.*]] = call i32 @llvm.arm.strex.p0(i32 %new, ptr elementtype(i32) %addr)
-; CHECK:     [[SUCCESS:%.*]] = icmp eq i32 [[STREX]], 0
-; CHECK:     br i1 [[SUCCESS]], label %[[SUCCESS_BB:.*]], label %[[FAILURE_BB:.*]]
-
-; CHECK: [[SUCCESS_BB]]:
-; CHECK:     call void @llvm.arm.dmb(i32 11)
-; CHECK:     br label %[[END:.*]]
-
-; CHECK: [[NO_STORE_BB]]:
-; CHECK:     [[LOADED_NOSTORE:%.*]] = phi i32 [ [[LOADED]], %[[START]] ]
-; CHECK:     call void @llvm.arm.clrex()
-; CHECK:     br label %[[FAILURE_BB]]
-
-; CHECK: [[FAILURE_BB]]:
-; CHECK:     [[LOADED_FAILURE:%.*]] = phi i32 [ [[LOADED_NOSTORE]], %[[NO_STORE_BB]] ], [ [[LOADED_TRYSTORE]], %[[TRY_STORE]] ]
-; CHECK:     call void @llvm.arm.dmb(i32 11)
-; CHECK:     br label %[[END]]
-
-; CHECK: [[END]]:
-; CHECK:     [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[SUCCESS_BB]] ], [ [[LOADED_FAILURE]], %[[FAILURE_BB]] ]
-; CHECK:     [[SUCCESS:%.*]] = phi i1 [ true, %[[SUCCESS_BB]] ], [ false, %[[FAILURE_BB]] ]
-; CHECK:     ret i32 [[LOADED_EXIT]]
-
+define i32 @test_cmpxchg_seq_cst(ptr %addr, i32 %desired, i32 %new) {
+; CHECK-LABEL: define i32 @test_cmpxchg_seq_cst(
+; CHECK-SAME: ptr [[ADDR:%.*]], i32 [[DESIRED:%.*]], i32 [[NEW:%.*]]) {
+; CHECK-NEXT:    br label %[[CMPXCHG_START:.*]]
+; CHECK:       [[CMPXCHG_START]]:
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.arm.ldrex.p0(ptr elementtype(i32) [[ADDR]])
+; CHECK-NEXT:    [[SHOULD_STORE:%.*]] = icmp eq i32 [[TMP1]], [[DESIRED]]
+; CHECK-NEXT:    br i1 [[SHOULD_STORE]], label %[[CMPXCHG_FENCEDSTORE:.*]], label %[[CMPXCHG_NOSTORE:.*]]
+; CHECK:       [[CMPXCHG_FENCEDSTORE]]:
+; CHECK-NEXT:    call void @llvm.arm.dmb(i32 10)
+; CHECK-NEXT:    br label %[[CMPXCHG_TRYSTORE:.*]]
+; CHECK:       [[CMPXCHG_TRYSTORE]]:
+; CHECK-NEXT:    [[LOADED_TRYSTORE:%.*]] = phi i32 [ [[TMP1]], %[[CMPXCHG_FENCEDSTORE]] ]
+; CHECK-NEXT:    [[TMP2:%.*]] = call i32 @llvm.arm.strex.p0(i32 [[NEW]], ptr elementtype(i32) [[ADDR]])
+; CHECK-NEXT:    [[SUCCESS:%.*]] = icmp eq i32 [[TMP2]], 0
+; CHECK-NEXT:    br i1 [[SUCCESS]], label %[[CMPXCHG_SUCCESS:.*]], label %[[CMPXCHG_FAILURE:.*]]
+; CHECK:       [[CMPXCHG_RELEASEDLOAD:.*:]]
+; CHECK-NEXT:    unreachable
+; CHECK:       [[CMPXCHG_SUCCESS]]:
+; CHECK-NEXT:    call void @llvm.arm.dmb(i32 11)
+; CHECK-NEXT:    br label %[[CMPXCHG_END:.*]]
+; CHECK:       [[CMPXCHG_NOSTORE]]:
+; CHECK-NEXT:    [[LOADED_NOSTORE:%.*]] = phi i32 [ [[TMP1]], %[[CMPXCHG_START]] ]
+; CHECK-NEXT:    call void @llvm.arm.clrex()
+; CHECK-NEXT:    br label %[[CMPXCHG_FAILURE]]
+; CHECK:       [[CMPXCHG_FAILURE]]:
+; CHECK-NEXT:    [[LOADED_FAILURE:%.*]] = phi i32 [ [[LOADED_NOSTORE]], %[[CMPXCHG_NOSTORE]] ], [ [[LOADED_TRYSTORE]], %[[CMPXCHG_TRYSTORE]] ]
+; CHECK-NEXT:    call void @llvm.arm.dmb(i32 11)
+; CHECK-NEXT:    br label %[[CMPXCHG_END]]
+; CHECK:       [[CMPXCHG_END]]:
+; CHECK-NEXT:    [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[CMPXCHG_SUCCESS]] ], [ [[LOADED_FAILURE]], %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[SUCCESS1:%.*]] = phi i1 [ true, %[[CMPXCHG_SUCCESS]] ], [ false, %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = insertvalue { i32, i1 } poison, i32 [[LOADED_EXIT]], 0
+; CHECK-NEXT:    [[TMP4:%.*]] = insertvalue { i32, i1 } [[TMP3]], i1 [[SUCCESS1]], 1
+; CHECK-NEXT:    [[OLDVAL:%.*]] = extractvalue { i32, i1 } [[TMP4]], 0
+; CHECK-NEXT:    ret i32 [[LOADED_EXIT]]
+;
   %pair = cmpxchg weak ptr %addr, i32 %desired, i32 %new seq_cst seq_cst
   %oldval = extractvalue { i32, i1 } %pair, 0
   ret i32 %oldval
 }
 
 define i1 @test_cmpxchg_weak_fail(ptr %addr, i32 %desired, i32 %new) {
-; CHECK-LABEL: @test_cmpxchg_weak_fail
-; CHECK:     br label %[[START:.*]]
-
-; CHECK: [[START]]:
-; CHECK:     [[LOADED:%.*]] = call i32 @llvm.arm.ldrex.p0(ptr elementtype(i32) %addr)
-; CHECK:     [[SHOULD_STORE:%.*]] = icmp eq i32 [[LOADED]], %desired
-; CHECK:     br i1 [[SHOULD_STORE]], label %[[FENCED_STORE:.*]], label %[[NO_STORE_BB:.*]]
-
-; CHECK: [[FENCED_STORE]]:
-; CHECK:     call void @llvm.arm.dmb(i32 10)
-; CHECK:     br label %[[TRY_STORE:.*]]
-
-; CHECK: [[TRY_STORE]]:
-; CHECK:     [[STREX:%.*]] = call i32 @llvm.arm.strex.p0(i32 %new, ptr elementtype(i32) %addr)
-; CHECK:     [[SUCCESS:%.*]] = icmp eq i32 [[STREX]], 0
-; CHECK:     br i1 [[SUCCESS]], label %[[SUCCESS_BB:.*]], label %[[FAILURE_BB:.*]]
-
-; CHECK: [[SUCCESS_BB]]:
-; CHECK:     call void @llvm.arm.dmb(i32 11)
-; CHECK:     br label %[[END:.*]]
-
-; CHECK: [[NO_STORE_BB]]:
-; CHECK:     call void @llvm.arm.clrex()
-; CHECK:     br label %[[FAILURE_BB]]
-
-; CHECK: [[FAILURE_BB]]:
-; CHECK-NOT: dmb
-; CHECK:     br label %[[END]]
-
-; CHECK: [[END]]:
-; CHECK:     [[SUCCESS:%.*]] = phi i1 [ true, %[[SUCCESS_BB]] ], [ false, %[[FAILURE_BB]] ]
-; CHECK:     ret i1 [[SUCCESS]]
-
+; CHECK-LABEL: define i1 @test_cmpxchg_weak_fail(
+; CHECK-SAME: ptr [[ADDR:%.*]], i32 [[DESIRED:%.*]], i32 [[NEW:%.*]]) {
+; CHECK-NEXT:    br label %[[CMPXCHG_START:.*]]
+; CHECK:       [[CMPXCHG_START]]:
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.arm.ldrex.p0(ptr elementtype(i32) [[ADDR]])
+; CHECK-NEXT:    [[SHOULD_STORE:%.*]] = icmp eq i32 [[TMP1]], [[DESIRED]]
+; CHECK-NEXT:    br i1 [[SHOULD_STORE]], label %[[CMPXCHG_FENCEDSTORE:.*]], label %[[CMPXCHG_NOSTORE:.*]]
+; CHECK:       [[CMPXCHG_FENCEDSTORE]]:
+; CHECK-NEXT:    call void @llvm.arm.dmb(i32 10)
+; CHECK-NEXT:    br label %[[CMPXCHG_TRYSTORE:.*]]
+; CHECK:       [[CMPXCHG_TRYSTORE]]:
+; CHECK-NEXT:    [[LOADED_TRYSTORE:%.*]] = phi i32 [ [[TMP1]], %[[CMPXCHG_FENCEDSTORE]] ]
+; CHECK-NEXT:    [[TMP2:%.*]] = call i32 @llvm.arm.strex.p0(i32 [[NEW]], ptr elementtype(i32) [[ADDR]])
+; CHECK-NEXT:    [[SUCCESS:%.*]] = icmp eq i32 [[TMP2]], 0
+; CHECK-NEXT:    br i1 [[SUCCESS]], label %[[CMPXCHG_SUCCESS:.*]], label %[[CMPXCHG_FAILURE:.*]]
+; CHECK:       [[CMPXCHG_RELEASEDLOAD:.*:]]
+; CHECK-NEXT:    unreachable
+; CHECK:       [[CMPXCHG_SUCCESS]]:
+; CHECK-NEXT:    call void @llvm.arm.dmb(i32 11)
+; CHECK-NEXT:    br label %[[CMPXCHG_END:.*]]
+; CHECK:       [[CMPXCHG_NOSTORE]]:
+; CHECK-NEXT:    [[LOADED_NOSTORE:%.*]] = phi i32 [ [[TMP1]], %[[CMPXCHG_START]] ]
+; CHECK-NEXT:    call void @llvm.arm.clrex()
+; CHECK-NEXT:    br label %[[CMPXCHG_FAILURE]]
+; CHECK:       [[CMPXCHG_FAILURE]]:
+; CHECK-NEXT:    [[LOADED_FAILURE:%.*]] = phi i32 [ [[LOADED_NOSTORE]], %[[CMPXCHG_NOSTORE]] ], [ [[LOADED_TRYSTORE]], %[[CMPXCHG_TRYSTORE]] ]
+; CHECK-NEXT:    br label %[[CMPXCHG_END]]
+; CHECK:       [[CMPXCHG_END]]:
+; CHECK-NEXT:    [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[CMPXCHG_SUCCESS]] ], [ [[LOADED_FAILURE]], %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[SUCCESS1:%.*]] = phi i1 [ true, %[[CMPXCHG_SUCCESS]] ], [ false, %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = insertvalue { i32, i1 } poison, i32 [[LOADED_EXIT]], 0
+; CHECK-NEXT:    [[TMP4:%.*]] = insertvalue { i32, i1 } [[TMP3]], i1 [[SUCCESS1]], 1
+; CHECK-NEXT:    [[OLDVAL:%.*]] = extractvalue { i32, i1 } [[TMP4]], 1
+; CHECK-NEXT:    ret i1 [[SUCCESS1]]
+;
   %pair = cmpxchg weak ptr %addr, i32 %desired, i32 %new seq_cst monotonic
   %oldval = extractvalue { i32, i1 } %pair, 1
   ret i1 %oldval
 }
 
 define i32 @test_cmpxchg_monotonic(ptr %addr, i32 %desired, i32 %new) {
-; CHECK-LABEL: @test_cmpxchg_monotonic
-; CHECK-NOT: dmb
-; CHECK:     br label %[[START:.*]]
-
-; CHECK: [[START]]:
-; CHECK:     [[LOADED:%.*]] = call i32 @llvm.arm.ldrex.p0(ptr elementtype(i32) %addr)
-; CHECK:     [[SHOULD_STORE:%.*]] = icmp eq i32 [[LOADED]], %desired
-; CHECK:     br i1 [[SHOULD_STORE]], label %[[FENCED_STORE:.*]], label %[[NO_STORE_BB:.*]]
-
-; CHECK: [[FENCED_STORE]]:
-; CHECK-NEXT: br label %[[TRY_STORE]]
-
-; CHECK: [[TRY_STORE]]:
-; CHECK:     [[LOADED_TRYSTORE:%.*]] = phi i32 [ [[LOADED]], %[[FENCED_STORE]] ]
-; CHECK:     [[STREX:%.*]] = call i32 @llvm.arm.strex.p0(i32 %new, ptr elementtype(i32) %addr)
-; CHECK:     [[SUCCESS:%.*]] = icmp eq i32 [[STREX]], 0
-; CHECK:     br i1 [[SUCCESS]], label %[[SUCCESS_BB:.*]], label %[[FAILURE_BB:.*]]
-
-; CHECK: [[SUCCESS_BB]]:
-; CHECK-NOT: dmb
-; CHECK:     br label %[[END:.*]]
-
-; CHECK: [[NO_STORE_BB]]:
-; CHECK:     [[LOADED_NOSTORE:%.*]] = phi i32 [ [[LOADED]], %[[START]] ]
-; CHECK:     call void @llvm.arm.clrex()
-; CHECK:     br label %[[FAILURE_BB]]
-
-; CHECK: [[FAILURE_BB]]:
-; CHECK:     [[LOADED_FAILURE:%.*]] = phi i32 [ [[LOADED_NOSTORE]], %[[NO_STORE_BB]] ], [ [[LOADED_TRYSTORE]], %[[TRY_STORE]] ]
-; CHECK-NOT: dmb
-; CHECK:     br label %[[END]]
-
-; CHECK: [[END]]:
-; CHECK:     [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[SUCCESS_BB]] ], [ [[LOADED_FAILURE]], %[[FAILURE_BB]] ]
-; CHECK:     [[SUCCESS:%.*]] = phi i1 [ true, %[[SUCCESS_BB]] ], [ false, %[[FAILURE_BB]] ]
-; CHECK:     ret i32 [[LOADED_EXIT]]
-
+; CHECK-LABEL: define i32 @test_cmpxchg_monotonic(
+; CHECK-SAME: ptr [[ADDR:%.*]], i32 [[DESIRED:%.*]], i32 [[NEW:%.*]]) {
+; CHECK-NEXT:    br label %[[CMPXCHG_START:.*]]
+; CHECK:       [[CMPXCHG_START]]:
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.arm.ldrex.p0(ptr elementtype(i32) [[ADDR]])
+; CHECK-NEXT:    [[SHOULD_STORE:%.*]] = icmp eq i32 [[TMP1]], [[DESIRED]]
+; CHECK-NEXT:    br i1 [[SHOULD_STORE]], label %[[CMPXCHG_FENCEDSTORE:.*]], label %[[CMPXCHG_NOSTORE:.*]]
+; CHECK:       [[CMPXCHG_FENCEDSTORE]]:
+; CHECK-NEXT:    br label %[[CMPXCHG_TRYSTORE:.*]]
+; CHECK:       [[CMPXCHG_TRYSTORE]]:
+; CHECK-NEXT:    [[LOADED_TRYSTORE:%.*]] = phi i32 [ [[TMP1]], %[[CMPXCHG_FENCEDSTORE]] ]
+; CHECK-NEXT:    [[TMP2:%.*]] = call i32 @llvm.arm.strex.p0(i32 [[NEW]], ptr elementtype(i32) [[ADDR]])
+; CHECK-NEXT:    [[SUCCESS:%.*]] = icmp eq i32 [[TMP2]], 0
+; CHECK-NEXT:    br i1 [[SUCCESS]], label %[[CMPXCHG_SUCCESS:.*]], label %[[CMPXCHG_FAILURE:.*]]
+; CHECK:       [[CMPXCHG_RELEASEDLOAD:.*:]]
+; CHECK-NEXT:    unreachable
+; CHECK:       [[CMPXCHG_SUCCESS]]:
+; CHECK-NEXT:    br label %[[CMPXCHG_END:.*]]
+; CHECK:       [[CMPXCHG_NOSTORE]]:
+; CHECK-NEXT:    [[LOADED_NOSTORE:%.*]] = phi i32 [ [[TMP1]], %[[CMPXCHG_START]] ]
+; CHECK-NEXT:    call void @llvm.arm.clrex()
+; CHECK-NEXT:    br label %[[CMPXCHG_FAILURE]]
+; CHECK:       [[CMPXCHG_FAILURE]]:
+; CHECK-NEXT:    [[LOADED_FAILURE:%.*]] = phi i32 [ [[LOADED_NOSTORE]], %[[CMPXCHG_NOSTORE]] ], [ [[LOADED_TRYSTORE]], %[[CMPXCHG_TRYSTORE]] ]
+; CHECK-NEXT:    br label %[[CMPXCHG_END]]
+; CHECK:       [[CMPXCHG_END]]:
+; CHECK-NEXT:    [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[CMPXCHG_SUCCESS]] ], [ [[LOADED_FAILURE]], %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[SUCCESS1:%.*]] = phi i1 [ true, %[[CMPXCHG_SUCCESS]] ], [ false, %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = insertvalue { i32, i1 } poison, i32 [[LOADED_EXIT]], 0
+; CHECK-NEXT:    [[TMP4:%.*]] = insertvalue { i32, i1 } [[TMP3]], i1 [[SUCCESS1]], 1
+; CHECK-NEXT:    [[OLDVAL:%.*]] = extractvalue { i32, i1 } [[TMP4]], 0
+; CHECK-NEXT:    ret i32 [[LOADED_EXIT]]
+;
   %pair = cmpxchg weak ptr %addr, i32 %desired, i32 %new monotonic monotonic
   %oldval = extractvalue { i32, i1 } %pair, 0
   ret i32 %oldval
 }
 
 define i32 @test_cmpxchg_seq_cst_minsize(ptr %addr, i32 %desired, i32 %new) minsize {
-; CHECK-LABEL: @test_cmpxchg_seq_cst_minsize
-; CHECK:     br label %[[START:.*]]
-
-; CHECK: [[START]]:
-; CHECK:     [[LOADED:%.*]] = call i32 @llvm.arm.ldrex.p0(ptr elementtype(i32) %addr)
-; CHECK:     [[SHOULD_STORE:%.*]] = icmp eq i32 [[LOADED]], %desired
-; CHECK:     br i1 [[SHOULD_STORE]], label %[[FENCED_STORE:.*]], label %[[NO_STORE_BB:.*]]
-
-; CHECK: [[FENCED_STORE]]:
-; CHECK:     call void @llvm.arm.dmb(i32 10)
-; CHECK:     br label %[[TRY_STORE:.*]]
-
-; CHECK: [[TRY_STORE]]:
-; CHECK:     [[LOADED_TRYSTORE:%.*]] = phi i32 [ [[LOADED]], %[[FENCED_STORE]] ]
-; CHECK:     [[STREX:%.*]] = call i32 @llvm.arm.strex.p0(i32 %new, ptr elementtype(i32) %addr)
-; CHECK:     [[SUCCESS:%.*]] = icmp eq i32 [[STREX]], 0
-; CHECK:     br i1 [[SUCCESS]], label %[[SUCCESS_BB:.*]], label %[[FAILURE_BB:.*]]
-
-; CHECK: [[SUCCESS_BB]]:
-; CHECK:     call void @llvm.arm.dmb(i32 11)
-; CHECK:     br label %[[END:.*]]
-
-; CHECK: [[NO_STORE_BB]]:
-; CHECK:     [[LOADED_NOSTORE:%.*]] = phi i32 [ [[LOADED]], %[[START]] ]
-; CHECK:     call void @llvm.arm.clrex()
-; CHECK:     br label %[[FAILURE_BB]]
-
-; CHECK: [[FAILURE_BB]]:
-; CHECK:     [[LOADED_FAILURE:%.*]] = phi i32 [ [[LOADED_NOSTORE]], %[[NO_STORE_BB]] ], [ [[LOADED_TRYSTORE]], %[[TRY_STORE]] ]
-; CHECK:     call void @llvm.arm.dmb(i32 11)
-; CHECK:     br label %[[END]]
-
-; CHECK: [[END]]:
-; CHECK:     [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[SUCCESS_BB]] ], [ [[LOADED_FAILURE]], %[[FAILURE_BB]] ]
-; CHECK:     [[SUCCESS:%.*]] = phi i1 [ true, %[[SUCCESS_BB]] ], [ false, %[[FAILURE_BB]] ]
-; CHECK:     ret i32 [[LOADED_EXIT]]
-
+; CHECK-LABEL: define i32 @test_cmpxchg_seq_cst_minsize(
+; CHECK-SAME: ptr [[ADDR:%.*]], i32 [[DESIRED:%.*]], i32 [[NEW:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:    br label %[[CMPXCHG_START:.*]]
+; CHECK:       [[CMPXCHG_START]]:
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.arm.ldrex.p0(ptr elementtype(i32) [[ADDR]])
+; CHECK-NEXT:    [[SHOULD_STORE:%.*]] = icmp eq i32 [[TMP1]], [[DESIRED]]
+; CHECK-NEXT:    br i1 [[SHOULD_STORE]], label %[[CMPXCHG_FENCEDSTORE:.*]], label %[[CMPXCHG_NOSTORE:.*]]
+; CHECK:       [[CMPXCHG_FENCEDSTORE]]:
+; CHECK-NEXT:    call void @llvm.arm.dmb(i32 10)
+; CHECK-NEXT:    br label %[[CMPXCHG_TRYSTORE:.*]]
+; CHECK:       [[CMPXCHG_TRYSTORE]]:
+; CHECK-NEXT:    [[LOADED_TRYSTORE:%.*]] = phi i32 [ [[TMP1]], %[[CMPXCHG_FENCEDSTORE]] ]
+; CHECK-NEXT:    [[TMP2:%.*]] = call i32 @llvm.arm.strex.p0(i32 [[NEW]], ptr elementtype(i32) [[ADDR]])
+; CHECK-NEXT:    [[SUCCESS:%.*]] = icmp eq i32 [[TMP2]], 0
+; CHECK-NEXT:    br i1 [[SUCCESS]], label %[[CMPXCHG_SUCCESS:.*]], label %[[CMPXCHG_FAILURE:.*]]
+; CHECK:       [[CMPXCHG_RELEASEDLOAD:.*:]]
+; CHECK-NEXT:    unreachable
+; CHECK:       [[CMPXCHG_SUCCESS]]:
+; CHECK-NEXT:    call void @llvm.arm.dmb(i32 11)
+; CHECK-NEXT:    br label %[[CMPXCHG_END:.*]]
+; CHECK:       [[CMPXCHG_NOSTORE]]:
+; CHECK-NEXT:    [[LOADED_NOSTORE:%.*]] = phi i32 [ [[TMP1]], %[[CMPXCHG_START]] ]
+; CHECK-NEXT:    call void @llvm.arm.clrex()
+; CHECK-NEXT:    br label %[[CMPXCHG_FAILURE]]
+; CHECK:       [[CMPXCHG_FAILURE]]:
+; CHECK-NEXT:    [[LOADED_FAILURE:%.*]] = phi i32 [ [[LOADED_NOSTORE]], %[[CMPXCHG_NOSTORE]] ], [ [[LOADED_TRYSTORE]], %[[CMPXCHG_TRYSTORE]] ]
+; CHECK-NEXT:    call void @llvm.arm.dmb(i32 11)
+; CHECK-NEXT:    br label %[[CMPXCHG_END]]
+; CHECK:       [[CMPXCHG_END]]:
+; CHECK-NEXT:    [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[CMPXCHG_SUCCESS]] ], [ [[LOADED_FAILURE]], %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[SUCCESS1:%.*]] = phi i1 [ true, %[[CMPXCHG_SUCCESS]] ], [ false, %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = insertvalue { i32, i1 } poison, i32 [[LOADED_EXIT]], 0
+; CHECK-NEXT:    [[TMP4:%.*]] = insertvalue { i32, i1 } [[TMP3]], i1 [[SUCCESS1]], 1
+; CHECK-NEXT:    [[OLDVAL:%.*]] = extractvalue { i32, i1 } [[TMP4]], 0
+; CHECK-NEXT:    ret i32 [[LOADED_EXIT]]
+;
   %pair = cmpxchg weak ptr %addr, i32 %desired, i32 %new seq_cst seq_cst
   %oldval = extractvalue { i32, i1 } %pair, 0
   ret i32 %oldval

The expansion for cmpxchg was trying to tidy up extractvalue users
to directly use the lowered pieces, and then erasing the now dead
extractvalues. This was making an assumption about the iteration
order did not depend on those user instructions.

Continue doing the replacement, but just leave the dead extractvalues.
This is a minor regression, but it is of no importance since the
dead instructions will just get dropped during codegen anyway.
@arsenm arsenm force-pushed the users/arsenm/atomic-expand-remove-prune branch from eade1f6 to 43d4cb7 Compare August 13, 2024 17:08
@arsenm
Copy link
Contributor Author

arsenm commented Aug 13, 2024

Turns out I don't seem to need this

@arsenm arsenm closed this Aug 13, 2024
@arsenm arsenm deleted the users/arsenm/atomic-expand-remove-prune branch August 13, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants