Skip to content

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Aug 23, 2025

This is about code readability. The operands in the disjunction forming the combined predicate in mergeConditionalStoreToAddress could sometimes be negated twice. This patch addresses that.

2 tests needed updating because they exposed the double negation and now they don’t.

Copy link
Member Author

mtrofin commented Aug 23, 2025

@mtrofin mtrofin requested a review from jmolloy August 23, 2025 02:12
@mtrofin mtrofin marked this pull request as ready for review August 23, 2025 02:20
@llvmbot
Copy link
Member

llvmbot commented Aug 23, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

This is about code readability. The operands in the disjunction forming the combined predicate in mergeConditionalStoreToAddress could sometimes be negated twice. This patch addresses that.

2 tests needed updating because they exposed the double negation and now they don’t.


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

3 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+10-10)
  • (modified) llvm/test/Transforms/LoopVectorize/AArch64/masked-call.ll (+8-12)
  • (modified) llvm/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll (+16-43)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 055e8cadaab76..1dbb8856dab3d 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -4387,10 +4387,12 @@ static bool mergeConditionalStoreToAddress(
 
   // OK, we're going to sink the stores to PostBB. The store has to be
   // conditional though, so first create the predicate.
-  Value *PCond = cast<BranchInst>(PFB->getSinglePredecessor()->getTerminator())
-                     ->getCondition();
-  Value *QCond = cast<BranchInst>(QFB->getSinglePredecessor()->getTerminator())
-                     ->getCondition();
+  BranchInst *const PBranch =
+      cast<BranchInst>(PFB->getSinglePredecessor()->getTerminator());
+  BranchInst *const QBranch =
+      cast<BranchInst>(QFB->getSinglePredecessor()->getTerminator());
+  Value *const PCond = PBranch->getCondition();
+  Value *const QCond = QBranch->getCondition();
 
   Value *PPHI = ensureValueAvailableInSuccessor(PStore->getValueOperand(),
                                                 PStore->getParent());
@@ -4401,13 +4403,11 @@ static bool mergeConditionalStoreToAddress(
   IRBuilder<> QB(PostBB, PostBBFirst);
   QB.SetCurrentDebugLocation(PostBBFirst->getStableDebugLoc());
 
-  Value *PPred = PStore->getParent() == PTB ? PCond : QB.CreateNot(PCond);
-  Value *QPred = QStore->getParent() == QTB ? QCond : QB.CreateNot(QCond);
+  InvertPCond = (PStore->getParent() == PTB) ^ InvertPCond;
+  InvertQCond = (QStore->getParent() == QTB) ^ InvertQCond;
+  Value *const PPred = InvertPCond ? PCond : QB.CreateNot(PCond);
+  Value *const QPred = InvertQCond ? QCond : QB.CreateNot(QCond);
 
-  if (InvertPCond)
-    PPred = QB.CreateNot(PPred);
-  if (InvertQCond)
-    QPred = QB.CreateNot(QPred);
   Value *CombinedPred = QB.CreateOr(PPred, QPred);
 
   BasicBlock::iterator InsertPt = QB.GetInsertPoint();
diff --git a/llvm/test/Transforms/LoopVectorize/AArch64/masked-call.ll b/llvm/test/Transforms/LoopVectorize/AArch64/masked-call.ll
index 11bb4d234f3f3..87b3c838c9fe4 100644
--- a/llvm/test/Transforms/LoopVectorize/AArch64/masked-call.ll
+++ b/llvm/test/Transforms/LoopVectorize/AArch64/masked-call.ll
@@ -973,9 +973,9 @@ define void @test_widen_exp_v2(ptr noalias %p2, ptr noalias %p, i64 %n) #5 {
 ; TFA_INTERLEAVE-NEXT:    [[ACTIVE_LANE_MASK_ENTRY1:%.*]] = icmp ult i64 1, [[TMP0]]
 ; TFA_INTERLEAVE-NEXT:    br label %[[VECTOR_BODY:.*]]
 ; TFA_INTERLEAVE:       [[VECTOR_BODY]]:
-; TFA_INTERLEAVE-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, %[[ENTRY]] ], [ [[INDEX_NEXT:%.*]], %[[TMP19:.*]] ]
-; TFA_INTERLEAVE-NEXT:    [[ACTIVE_LANE_MASK:%.*]] = phi i1 [ [[ACTIVE_LANE_MASK_ENTRY]], %[[ENTRY]] ], [ [[ACTIVE_LANE_MASK_NEXT:%.*]], %[[TMP19]] ]
-; TFA_INTERLEAVE-NEXT:    [[ACTIVE_LANE_MASK2:%.*]] = phi i1 [ [[ACTIVE_LANE_MASK_ENTRY1]], %[[ENTRY]] ], [ [[ACTIVE_LANE_MASK_NEXT6:%.*]], %[[TMP19]] ]
+; TFA_INTERLEAVE-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, %[[ENTRY]] ], [ [[INDEX_NEXT:%.*]], %[[TMP15:.*]] ]
+; TFA_INTERLEAVE-NEXT:    [[ACTIVE_LANE_MASK:%.*]] = phi i1 [ [[ACTIVE_LANE_MASK_ENTRY]], %[[ENTRY]] ], [ [[ACTIVE_LANE_MASK_NEXT:%.*]], %[[TMP15]] ]
+; TFA_INTERLEAVE-NEXT:    [[ACTIVE_LANE_MASK2:%.*]] = phi i1 [ [[ACTIVE_LANE_MASK_ENTRY1]], %[[ENTRY]] ], [ [[ACTIVE_LANE_MASK_NEXT6:%.*]], %[[TMP15]] ]
 ; TFA_INTERLEAVE-NEXT:    [[TMP4:%.*]] = load double, ptr [[P2]], align 8
 ; TFA_INTERLEAVE-NEXT:    [[TMP5:%.*]] = tail call double @llvm.exp.f64(double [[TMP4]]) #[[ATTR7:[0-9]+]]
 ; TFA_INTERLEAVE-NEXT:    [[TMP6:%.*]] = tail call double @llvm.exp.f64(double [[TMP4]]) #[[ATTR7]]
@@ -988,16 +988,12 @@ define void @test_widen_exp_v2(ptr noalias %p2, ptr noalias %p, i64 %n) #5 {
 ; TFA_INTERLEAVE-NEXT:    [[PREDPHI:%.*]] = select i1 [[TMP11]], double 1.000000e+00, double 0.000000e+00
 ; TFA_INTERLEAVE-NEXT:    [[PREDPHI3:%.*]] = select i1 [[TMP12]], double 1.000000e+00, double 0.000000e+00
 ; TFA_INTERLEAVE-NEXT:    [[SPEC_SELECT:%.*]] = select i1 [[ACTIVE_LANE_MASK2]], double [[PREDPHI3]], double [[PREDPHI]]
-; TFA_INTERLEAVE-NEXT:    [[TMP13:%.*]] = xor i1 [[ACTIVE_LANE_MASK]], true
-; TFA_INTERLEAVE-NEXT:    [[TMP14:%.*]] = xor i1 [[ACTIVE_LANE_MASK2]], true
-; TFA_INTERLEAVE-NEXT:    [[TMP15:%.*]] = xor i1 [[TMP13]], true
-; TFA_INTERLEAVE-NEXT:    [[TMP16:%.*]] = xor i1 [[TMP14]], true
-; TFA_INTERLEAVE-NEXT:    [[TMP17:%.*]] = or i1 [[TMP15]], [[TMP16]]
-; TFA_INTERLEAVE-NEXT:    br i1 [[TMP17]], label %[[BB18:.*]], label %[[TMP19]]
-; TFA_INTERLEAVE:       [[BB18]]:
+; TFA_INTERLEAVE-NEXT:    [[TMP13:%.*]] = or i1 [[ACTIVE_LANE_MASK]], [[ACTIVE_LANE_MASK2]]
+; TFA_INTERLEAVE-NEXT:    br i1 [[TMP13]], label %[[BB14:.*]], label %[[TMP15]]
+; TFA_INTERLEAVE:       [[BB14]]:
 ; TFA_INTERLEAVE-NEXT:    store double [[SPEC_SELECT]], ptr [[P]], align 8
-; TFA_INTERLEAVE-NEXT:    br label %[[TMP19]]
-; TFA_INTERLEAVE:       [[TMP19]]:
+; TFA_INTERLEAVE-NEXT:    br label %[[TMP15]]
+; TFA_INTERLEAVE:       [[TMP15]]:
 ; TFA_INTERLEAVE-NEXT:    [[INDEX_NEXT]] = add i64 [[INDEX]], 2
 ; TFA_INTERLEAVE-NEXT:    [[TMP20:%.*]] = add i64 [[INDEX]], 1
 ; TFA_INTERLEAVE-NEXT:    [[ACTIVE_LANE_MASK_NEXT]] = icmp ult i64 [[INDEX]], [[TMP3]]
diff --git a/llvm/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll b/llvm/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll
index e16cbae9e0edf..394fa9acb683d 100644
--- a/llvm/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll
+++ b/llvm/test/Transforms/SimplifyCFG/merge-cond-stores-2.ll
@@ -1,4 +1,4 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --version 5
 ; RUN: opt -S < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -simplifycfg-merge-cond-stores=true -simplifycfg-merge-cond-stores-aggressively=false -phi-node-folding-threshold=1 | FileCheck %s
 
 target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
@@ -9,9 +9,10 @@ target triple = "armv7--linux-gnueabihf"
 ; block and there should be no PHIs.
 
 define i32 @f(ptr %b) {
-; CHECK-LABEL: @f(
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[B:%.*]], align 4
+; CHECK-LABEL: define i32 @f(
+; CHECK-SAME: ptr [[B:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[B]], align 4
 ; CHECK-NEXT:    [[AND:%.*]] = and i32 [[TMP0]], 1
 ; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp eq i32 [[AND]], 0
 ; CHECK-NEXT:    [[OR:%.*]] = or i32 [[TMP0]], -2147483648
@@ -27,119 +28,91 @@ define i32 @f(ptr %b) {
 ; CHECK-NEXT:    [[TOBOOL7:%.*]] = icmp eq i32 [[AND6]], 0
 ; CHECK-NEXT:    [[OR9:%.*]] = or i32 [[TMP2]], 536870912
 ; CHECK-NEXT:    [[SPEC_SELECT:%.*]] = select i1 [[TOBOOL7]], i32 [[TMP2]], i32 [[OR9]]
-; CHECK-NEXT:    [[TMP6:%.*]] = xor i1 [[TMP5]], true
 ; CHECK-NEXT:    [[TMP7:%.*]] = xor i1 [[TOBOOL7]], true
-; CHECK-NEXT:    [[TMP8:%.*]] = xor i1 [[TMP6]], true
-; CHECK-NEXT:    [[TMP9:%.*]] = or i1 [[TMP8]], [[TMP7]]
+; CHECK-NEXT:    [[TMP12:%.*]] = or i1 [[TMP5]], [[TMP7]]
 ; CHECK-NEXT:    [[AND11:%.*]] = and i32 [[SPEC_SELECT]], 8
 ; CHECK-NEXT:    [[TOBOOL12:%.*]] = icmp eq i32 [[AND11]], 0
 ; CHECK-NEXT:    [[OR14:%.*]] = or i32 [[SPEC_SELECT]], 268435456
 ; CHECK-NEXT:    [[SPEC_SELECT1:%.*]] = select i1 [[TOBOOL12]], i32 [[SPEC_SELECT]], i32 [[OR14]]
-; CHECK-NEXT:    [[TMP10:%.*]] = xor i1 [[TMP9]], true
 ; CHECK-NEXT:    [[TMP11:%.*]] = xor i1 [[TOBOOL12]], true
-; CHECK-NEXT:    [[TMP12:%.*]] = xor i1 [[TMP10]], true
 ; CHECK-NEXT:    [[TMP13:%.*]] = or i1 [[TMP12]], [[TMP11]]
 ; CHECK-NEXT:    [[AND16:%.*]] = and i32 [[SPEC_SELECT1]], 16
 ; CHECK-NEXT:    [[TOBOOL17:%.*]] = icmp eq i32 [[AND16]], 0
 ; CHECK-NEXT:    [[OR19:%.*]] = or i32 [[SPEC_SELECT1]], 134217728
 ; CHECK-NEXT:    [[SPEC_SELECT2:%.*]] = select i1 [[TOBOOL17]], i32 [[SPEC_SELECT1]], i32 [[OR19]]
-; CHECK-NEXT:    [[TMP14:%.*]] = xor i1 [[TMP13]], true
 ; CHECK-NEXT:    [[TMP15:%.*]] = xor i1 [[TOBOOL17]], true
-; CHECK-NEXT:    [[TMP16:%.*]] = xor i1 [[TMP14]], true
-; CHECK-NEXT:    [[TMP17:%.*]] = or i1 [[TMP16]], [[TMP15]]
+; CHECK-NEXT:    [[TMP20:%.*]] = or i1 [[TMP13]], [[TMP15]]
 ; CHECK-NEXT:    [[AND21:%.*]] = and i32 [[SPEC_SELECT2]], 32
 ; CHECK-NEXT:    [[TOBOOL22:%.*]] = icmp eq i32 [[AND21]], 0
 ; CHECK-NEXT:    [[OR24:%.*]] = or i32 [[SPEC_SELECT2]], 67108864
 ; CHECK-NEXT:    [[SPEC_SELECT3:%.*]] = select i1 [[TOBOOL22]], i32 [[SPEC_SELECT2]], i32 [[OR24]]
-; CHECK-NEXT:    [[TMP18:%.*]] = xor i1 [[TMP17]], true
 ; CHECK-NEXT:    [[TMP19:%.*]] = xor i1 [[TOBOOL22]], true
-; CHECK-NEXT:    [[TMP20:%.*]] = xor i1 [[TMP18]], true
 ; CHECK-NEXT:    [[TMP21:%.*]] = or i1 [[TMP20]], [[TMP19]]
 ; CHECK-NEXT:    [[AND26:%.*]] = and i32 [[SPEC_SELECT3]], 64
 ; CHECK-NEXT:    [[TOBOOL27:%.*]] = icmp eq i32 [[AND26]], 0
 ; CHECK-NEXT:    [[OR29:%.*]] = or i32 [[SPEC_SELECT3]], 33554432
 ; CHECK-NEXT:    [[SPEC_SELECT4:%.*]] = select i1 [[TOBOOL27]], i32 [[SPEC_SELECT3]], i32 [[OR29]]
-; CHECK-NEXT:    [[TMP22:%.*]] = xor i1 [[TMP21]], true
 ; CHECK-NEXT:    [[TMP23:%.*]] = xor i1 [[TOBOOL27]], true
-; CHECK-NEXT:    [[TMP24:%.*]] = xor i1 [[TMP22]], true
-; CHECK-NEXT:    [[TMP25:%.*]] = or i1 [[TMP24]], [[TMP23]]
+; CHECK-NEXT:    [[TMP28:%.*]] = or i1 [[TMP21]], [[TMP23]]
 ; CHECK-NEXT:    [[AND31:%.*]] = and i32 [[SPEC_SELECT4]], 256
 ; CHECK-NEXT:    [[TOBOOL32:%.*]] = icmp eq i32 [[AND31]], 0
 ; CHECK-NEXT:    [[OR34:%.*]] = or i32 [[SPEC_SELECT4]], 8388608
 ; CHECK-NEXT:    [[SPEC_SELECT5:%.*]] = select i1 [[TOBOOL32]], i32 [[SPEC_SELECT4]], i32 [[OR34]]
-; CHECK-NEXT:    [[TMP26:%.*]] = xor i1 [[TMP25]], true
 ; CHECK-NEXT:    [[TMP27:%.*]] = xor i1 [[TOBOOL32]], true
-; CHECK-NEXT:    [[TMP28:%.*]] = xor i1 [[TMP26]], true
 ; CHECK-NEXT:    [[TMP29:%.*]] = or i1 [[TMP28]], [[TMP27]]
 ; CHECK-NEXT:    [[AND36:%.*]] = and i32 [[SPEC_SELECT5]], 512
 ; CHECK-NEXT:    [[TOBOOL37:%.*]] = icmp eq i32 [[AND36]], 0
 ; CHECK-NEXT:    [[OR39:%.*]] = or i32 [[SPEC_SELECT5]], 4194304
 ; CHECK-NEXT:    [[SPEC_SELECT6:%.*]] = select i1 [[TOBOOL37]], i32 [[SPEC_SELECT5]], i32 [[OR39]]
-; CHECK-NEXT:    [[TMP30:%.*]] = xor i1 [[TMP29]], true
 ; CHECK-NEXT:    [[TMP31:%.*]] = xor i1 [[TOBOOL37]], true
-; CHECK-NEXT:    [[TMP32:%.*]] = xor i1 [[TMP30]], true
-; CHECK-NEXT:    [[TMP33:%.*]] = or i1 [[TMP32]], [[TMP31]]
+; CHECK-NEXT:    [[TMP36:%.*]] = or i1 [[TMP29]], [[TMP31]]
 ; CHECK-NEXT:    [[AND41:%.*]] = and i32 [[SPEC_SELECT6]], 1024
 ; CHECK-NEXT:    [[TOBOOL42:%.*]] = icmp eq i32 [[AND41]], 0
 ; CHECK-NEXT:    [[OR44:%.*]] = or i32 [[SPEC_SELECT6]], 2097152
 ; CHECK-NEXT:    [[SPEC_SELECT7:%.*]] = select i1 [[TOBOOL42]], i32 [[SPEC_SELECT6]], i32 [[OR44]]
-; CHECK-NEXT:    [[TMP34:%.*]] = xor i1 [[TMP33]], true
 ; CHECK-NEXT:    [[TMP35:%.*]] = xor i1 [[TOBOOL42]], true
-; CHECK-NEXT:    [[TMP36:%.*]] = xor i1 [[TMP34]], true
 ; CHECK-NEXT:    [[TMP37:%.*]] = or i1 [[TMP36]], [[TMP35]]
 ; CHECK-NEXT:    [[AND46:%.*]] = and i32 [[SPEC_SELECT7]], 2048
 ; CHECK-NEXT:    [[TOBOOL47:%.*]] = icmp eq i32 [[AND46]], 0
 ; CHECK-NEXT:    [[OR49:%.*]] = or i32 [[SPEC_SELECT7]], 1048576
 ; CHECK-NEXT:    [[SPEC_SELECT8:%.*]] = select i1 [[TOBOOL47]], i32 [[SPEC_SELECT7]], i32 [[OR49]]
-; CHECK-NEXT:    [[TMP38:%.*]] = xor i1 [[TMP37]], true
 ; CHECK-NEXT:    [[TMP39:%.*]] = xor i1 [[TOBOOL47]], true
-; CHECK-NEXT:    [[TMP40:%.*]] = xor i1 [[TMP38]], true
-; CHECK-NEXT:    [[TMP41:%.*]] = or i1 [[TMP40]], [[TMP39]]
+; CHECK-NEXT:    [[TMP44:%.*]] = or i1 [[TMP37]], [[TMP39]]
 ; CHECK-NEXT:    [[AND51:%.*]] = and i32 [[SPEC_SELECT8]], 4096
 ; CHECK-NEXT:    [[TOBOOL52:%.*]] = icmp eq i32 [[AND51]], 0
 ; CHECK-NEXT:    [[OR54:%.*]] = or i32 [[SPEC_SELECT8]], 524288
 ; CHECK-NEXT:    [[SPEC_SELECT9:%.*]] = select i1 [[TOBOOL52]], i32 [[SPEC_SELECT8]], i32 [[OR54]]
-; CHECK-NEXT:    [[TMP42:%.*]] = xor i1 [[TMP41]], true
 ; CHECK-NEXT:    [[TMP43:%.*]] = xor i1 [[TOBOOL52]], true
-; CHECK-NEXT:    [[TMP44:%.*]] = xor i1 [[TMP42]], true
 ; CHECK-NEXT:    [[TMP45:%.*]] = or i1 [[TMP44]], [[TMP43]]
 ; CHECK-NEXT:    [[AND56:%.*]] = and i32 [[SPEC_SELECT9]], 8192
 ; CHECK-NEXT:    [[TOBOOL57:%.*]] = icmp eq i32 [[AND56]], 0
 ; CHECK-NEXT:    [[OR59:%.*]] = or i32 [[SPEC_SELECT9]], 262144
 ; CHECK-NEXT:    [[SPEC_SELECT10:%.*]] = select i1 [[TOBOOL57]], i32 [[SPEC_SELECT9]], i32 [[OR59]]
-; CHECK-NEXT:    [[TMP46:%.*]] = xor i1 [[TMP45]], true
 ; CHECK-NEXT:    [[TMP47:%.*]] = xor i1 [[TOBOOL57]], true
-; CHECK-NEXT:    [[TMP48:%.*]] = xor i1 [[TMP46]], true
-; CHECK-NEXT:    [[TMP49:%.*]] = or i1 [[TMP48]], [[TMP47]]
+; CHECK-NEXT:    [[TMP52:%.*]] = or i1 [[TMP45]], [[TMP47]]
 ; CHECK-NEXT:    [[AND61:%.*]] = and i32 [[SPEC_SELECT10]], 16384
 ; CHECK-NEXT:    [[TOBOOL62:%.*]] = icmp eq i32 [[AND61]], 0
 ; CHECK-NEXT:    [[OR64:%.*]] = or i32 [[SPEC_SELECT10]], 131072
 ; CHECK-NEXT:    [[SPEC_SELECT11:%.*]] = select i1 [[TOBOOL62]], i32 [[SPEC_SELECT10]], i32 [[OR64]]
-; CHECK-NEXT:    [[TMP50:%.*]] = xor i1 [[TMP49]], true
 ; CHECK-NEXT:    [[TMP51:%.*]] = xor i1 [[TOBOOL62]], true
-; CHECK-NEXT:    [[TMP52:%.*]] = xor i1 [[TMP50]], true
 ; CHECK-NEXT:    [[TMP53:%.*]] = or i1 [[TMP52]], [[TMP51]]
 ; CHECK-NEXT:    [[AND66:%.*]] = and i32 [[SPEC_SELECT11]], 32768
 ; CHECK-NEXT:    [[TOBOOL67:%.*]] = icmp eq i32 [[AND66]], 0
 ; CHECK-NEXT:    [[OR69:%.*]] = or i32 [[SPEC_SELECT11]], 65536
 ; CHECK-NEXT:    [[SPEC_SELECT12:%.*]] = select i1 [[TOBOOL67]], i32 [[SPEC_SELECT11]], i32 [[OR69]]
-; CHECK-NEXT:    [[TMP54:%.*]] = xor i1 [[TMP53]], true
 ; CHECK-NEXT:    [[TMP55:%.*]] = xor i1 [[TOBOOL67]], true
-; CHECK-NEXT:    [[TMP56:%.*]] = xor i1 [[TMP54]], true
-; CHECK-NEXT:    [[TMP57:%.*]] = or i1 [[TMP56]], [[TMP55]]
+; CHECK-NEXT:    [[TMP60:%.*]] = or i1 [[TMP53]], [[TMP55]]
 ; CHECK-NEXT:    [[AND71:%.*]] = and i32 [[SPEC_SELECT12]], 128
 ; CHECK-NEXT:    [[TOBOOL72:%.*]] = icmp eq i32 [[AND71]], 0
 ; CHECK-NEXT:    [[OR74:%.*]] = or i32 [[SPEC_SELECT12]], 16777216
 ; CHECK-NEXT:    [[SPEC_SELECT13:%.*]] = select i1 [[TOBOOL72]], i32 [[SPEC_SELECT12]], i32 [[OR74]]
-; CHECK-NEXT:    [[TMP58:%.*]] = xor i1 [[TMP57]], true
 ; CHECK-NEXT:    [[TMP59:%.*]] = xor i1 [[TOBOOL72]], true
-; CHECK-NEXT:    [[TMP60:%.*]] = xor i1 [[TMP58]], true
 ; CHECK-NEXT:    [[TMP61:%.*]] = or i1 [[TMP60]], [[TMP59]]
-; CHECK-NEXT:    br i1 [[TMP61]], label [[TMP62:%.*]], label [[TMP63:%.*]]
-; CHECK:       62:
+; CHECK-NEXT:    br i1 [[TMP61]], label %[[BB34:.*]], label %[[BB35:.*]]
+; CHECK:       [[BB34]]:
 ; CHECK-NEXT:    store i32 [[SPEC_SELECT13]], ptr [[B]], align 4
-; CHECK-NEXT:    br label [[TMP63]]
-; CHECK:       63:
+; CHECK-NEXT:    br label %[[BB35]]
+; CHECK:       [[BB35]]:
 ; CHECK-NEXT:    ret i32 0
 ;
 entry:

@mtrofin mtrofin force-pushed the users/mtrofin/08-23-_nfc_simplifycfg_simplify_operators_for_the_combined_predicate_in_mergeconditionalstoretoaddress_ branch from d6bf02f to d52a459 Compare August 25, 2025 20:15
@@ -4418,13 +4420,11 @@ static bool mergeConditionalStoreToAddress(
IRBuilder<> QB(PostBB, PostBBFirst);
QB.SetCurrentDebugLocation(PostBBFirst->getStableDebugLoc());

Value *PPred = PStore->getParent() == PTB ? PCond : QB.CreateNot(PCond);
Value *QPred = QStore->getParent() == QTB ? QCond : QB.CreateNot(QCond);
InvertPCond = (PStore->getParent() == PTB) ^ InvertPCond;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
InvertPCond = (PStore->getParent() == PTB) ^ InvertPCond;
InvertPCond ^= PStore->getParent() == PTB;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's too late here, but shouldn't this be !=? We want to match the condition where we need to add an extra invert, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh true!

@mtrofin mtrofin force-pushed the users/mtrofin/08-23-_nfc_simplifycfg_simplify_operators_for_the_combined_predicate_in_mergeconditionalstoretoaddress_ branch from d52a459 to 30960f9 Compare August 25, 2025 20:44
Comment on lines 4425 to 4426
Value *const PPred = InvertPCond ? PCond : QB.CreateNot(PCond);
Value *const QPred = InvertQCond ? QCond : QB.CreateNot(QCond);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Value *const PPred = InvertPCond ? PCond : QB.CreateNot(PCond);
Value *const QPred = InvertQCond ? QCond : QB.CreateNot(QCond);
Value *const PPred = InvertPCond ? QB.CreateNot(PCond) : PCond;
Value *const QPred = InvertQCond ? QB.CreateNot(QCond) : QCond;

Missed this bit before -- the logic was double-inverted previously, so it ended up being correct, and now it's wrong...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol. And I followed convinced I didn't see it.

OK, clearly an argument for the readability aspect!

@mtrofin mtrofin force-pushed the users/mtrofin/08-23-_nfc_simplifycfg_simplify_operators_for_the_combined_predicate_in_mergeconditionalstoretoaddress_ branch from 30960f9 to 74a231d Compare August 25, 2025 21:32
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Member Author

mtrofin commented Aug 26, 2025

Merge activity

  • Aug 26, 2:06 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 26, 2:08 PM UTC: @mtrofin merged this pull request with Graphite.

@mtrofin mtrofin merged commit 3af4597 into main Aug 26, 2025
9 checks passed
@mtrofin mtrofin deleted the users/mtrofin/08-23-_nfc_simplifycfg_simplify_operators_for_the_combined_predicate_in_mergeconditionalstoretoaddress_ branch August 26, 2025 14:08
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.

3 participants