From 6f7fa77fe07ebc34dbfc8d504a6c5ef891748ed2 Mon Sep 17 00:00:00 2001 From: Poseydon42 Date: Sun, 8 Sep 2024 17:29:48 +0100 Subject: [PATCH 1/8] Precommit tests --- ...phi-with-multiple-unsimplifiable-values.ll | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll diff --git a/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll b/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll new file mode 100644 index 0000000000000..1c68817b2a999 --- /dev/null +++ b/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll @@ -0,0 +1,33 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 +; RUN: opt < %s -passes=instcombine -S | FileCheck %s + +define i1 @icmp_of_phi_of_scmp_with_constant(i1 %c, i16 %x, i16 %y) +; CHECK-LABEL: define i1 @icmp_of_phi_of_scmp_with_constant( +; CHECK-SAME: i1 [[C:%.*]], i16 [[X:%.*]], i16 [[Y:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C]], label %[[TRUE:.*]], label %[[FALSE:.*]] +; CHECK: [[TRUE]]: +; CHECK-NEXT: [[CMP1:%.*]] = call i8 @llvm.scmp.i8.i16(i16 [[X]], i16 [[Y]]) +; CHECK-NEXT: br label %[[EXIT:.*]] +; CHECK: [[FALSE]]: +; CHECK-NEXT: [[CMP2:%.*]] = call i8 @llvm.scmp.i8.i16(i16 [[Y]], i16 [[X]]) +; CHECK-NEXT: br label %[[EXIT]] +; CHECK: [[EXIT]]: +; CHECK-NEXT: [[PHI:%.*]] = phi i8 [ [[CMP1]], %[[TRUE]] ], [ [[CMP2]], %[[FALSE]] ] +; CHECK-NEXT: [[R:%.*]] = icmp slt i8 [[PHI]], 0 +; CHECK-NEXT: ret i1 [[R]] +; +{ +entry: + br i1 %c, label %true, label %false +true: + %cmp1 = call i8 @llvm.scmp(i16 %x, i16 %y) + br label %exit +false: + %cmp2 = call i8 @llvm.scmp(i16 %y, i16 %x) + br label %exit +exit: + %phi = phi i8 [%cmp1, %true], [%cmp2, %false] + %r = icmp slt i8 %phi, 0 + ret i1 %r +} From c3424e7febef45a43a13d2ea184829166842d796 Mon Sep 17 00:00:00 2001 From: Poseydon42 Date: Sun, 8 Sep 2024 17:30:53 +0100 Subject: [PATCH 2/8] Implement the optimization --- .../InstCombine/InstructionCombining.cpp | 27 +++++++++++++++++++ ...phi-with-multiple-unsimplifiable-values.ll | 7 +++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 8195e0539305c..4c27359752002 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1809,6 +1809,7 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { // Check to see whether the instruction can be folded into each phi operand. // If there is one operand that does not fold, remember the BB it is in. SmallVector NewPhiValues; + SmallVector OpsToMoveUseTo; BasicBlock *NonSimplifiedBB = nullptr; Value *NonSimplifiedInVal = nullptr; for (unsigned i = 0; i != NumPHIValues; ++i) { @@ -1820,6 +1821,16 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { continue; } + // If the only use of phi is comparing it with a constant then we can + // put this comparison in the incoming BB directly after a ucmp/scmp call + // because we know that it will simplify to a single icmp. + if (isa(InVal) && + match(&I, m_c_ICmp(m_Specific(PN), m_Constant()))) { + OpsToMoveUseTo.push_back(i); + NewPhiValues.push_back(nullptr); + continue; + } + if (NonSimplifiedBB) return nullptr; // More than one non-simplified value. NonSimplifiedBB = InBB; @@ -1851,6 +1862,22 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { return nullptr; } + // Clone the instruction that uses the phi node and move it into the incoming + // BB because we know that the next iteration of InstCombine will simplify it. + for (auto OpIndex : OpsToMoveUseTo) { + Instruction *Clone = I.clone(); + Value *Op = PN->getIncomingValue(OpIndex); + BasicBlock *OpBB = PN->getIncomingBlock(OpIndex); + for (Use &U : Clone->operands()) { + if (U == PN) + U = Op; + else + U = U->DoPHITranslation(PN->getParent(), OpBB); + } + Clone = InsertNewInstBefore(Clone, OpBB->getTerminator()->getIterator()); + NewPhiValues[OpIndex] = Clone; + } + // Okay, we can do the transformation: create the new PHI node. PHINode *NewPN = PHINode::Create(I.getType(), PN->getNumIncomingValues()); InsertNewInstBefore(NewPN, PN->getIterator()); diff --git a/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll b/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll index 1c68817b2a999..b91d8fc6e0570 100644 --- a/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll +++ b/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll @@ -7,14 +7,13 @@ define i1 @icmp_of_phi_of_scmp_with_constant(i1 %c, i16 %x, i16 %y) ; CHECK-NEXT: [[ENTRY:.*:]] ; CHECK-NEXT: br i1 [[C]], label %[[TRUE:.*]], label %[[FALSE:.*]] ; CHECK: [[TRUE]]: -; CHECK-NEXT: [[CMP1:%.*]] = call i8 @llvm.scmp.i8.i16(i16 [[X]], i16 [[Y]]) +; CHECK-NEXT: [[TMP0:%.*]] = icmp slt i16 [[X]], [[Y]] ; CHECK-NEXT: br label %[[EXIT:.*]] ; CHECK: [[FALSE]]: -; CHECK-NEXT: [[CMP2:%.*]] = call i8 @llvm.scmp.i8.i16(i16 [[Y]], i16 [[X]]) +; CHECK-NEXT: [[TMP1:%.*]] = icmp slt i16 [[Y]], [[X]] ; CHECK-NEXT: br label %[[EXIT]] ; CHECK: [[EXIT]]: -; CHECK-NEXT: [[PHI:%.*]] = phi i8 [ [[CMP1]], %[[TRUE]] ], [ [[CMP2]], %[[FALSE]] ] -; CHECK-NEXT: [[R:%.*]] = icmp slt i8 [[PHI]], 0 +; CHECK-NEXT: [[R:%.*]] = phi i1 [ [[TMP0]], %[[TRUE]] ], [ [[TMP1]], %[[FALSE]] ] ; CHECK-NEXT: ret i1 [[R]] ; { From 833e168141052b2b7c5d57095b84b61cc20c90dd Mon Sep 17 00:00:00 2001 From: Poseydon42 Date: Sun, 8 Sep 2024 18:29:27 +0100 Subject: [PATCH 3/8] Address review comments --- .../InstCombine/InstructionCombining.cpp | 60 +++++++------------ 1 file changed, 22 insertions(+), 38 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 4c27359752002..5ee639ef502cc 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1810,8 +1810,7 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { // If there is one operand that does not fold, remember the BB it is in. SmallVector NewPhiValues; SmallVector OpsToMoveUseTo; - BasicBlock *NonSimplifiedBB = nullptr; - Value *NonSimplifiedInVal = nullptr; + bool SeenNonSimplifiedInVal = false; for (unsigned i = 0; i != NumPHIValues; ++i) { Value *InVal = PN->getIncomingValue(i); BasicBlock *InBB = PN->getIncomingBlock(i); @@ -1824,29 +1823,31 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { // If the only use of phi is comparing it with a constant then we can // put this comparison in the incoming BB directly after a ucmp/scmp call // because we know that it will simplify to a single icmp. - if (isa(InVal) && - match(&I, m_c_ICmp(m_Specific(PN), m_Constant()))) { + const APInt *Ignored; + if (isa(InVal) && InVal->hasOneUse() && + match(&I, m_c_ICmp(m_Specific(PN), m_APInt(Ignored)))) { OpsToMoveUseTo.push_back(i); NewPhiValues.push_back(nullptr); continue; } - if (NonSimplifiedBB) return nullptr; // More than one non-simplified value. + if (SeenNonSimplifiedInVal) + return nullptr; // More than one non-simplified value. + SeenNonSimplifiedInVal = true; - NonSimplifiedBB = InBB; - NonSimplifiedInVal = InVal; NewPhiValues.push_back(nullptr); + OpsToMoveUseTo.push_back(i); // If the InVal is an invoke at the end of the pred block, then we can't // insert a computation after it without breaking the edge. if (isa(InVal)) - if (cast(InVal)->getParent() == NonSimplifiedBB) + if (cast(InVal)->getParent() == InBB) return nullptr; // Do not push the operation across a loop backedge. This could result in // an infinite combine loop, and is generally non-profitable (especially // if the operation was originally outside the loop). - if (isBackEdge(NonSimplifiedBB, PN->getParent())) + if (isBackEdge(InBB, PN->getParent())) return nullptr; } @@ -1855,19 +1856,18 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { // inserting the computation on some other paths (e.g. inside a loop). Only // do this if the pred block is unconditionally branching into the phi block. // Also, make sure that the pred block is not dead code. - if (NonSimplifiedBB != nullptr) { - BranchInst *BI = dyn_cast(NonSimplifiedBB->getTerminator()); - if (!BI || !BI->isUnconditional() || - !DT.isReachableFromEntry(NonSimplifiedBB)) - return nullptr; - } - - // Clone the instruction that uses the phi node and move it into the incoming - // BB because we know that the next iteration of InstCombine will simplify it. + // After checking for all of the above, clone the instruction that uses the + // phi node and move it into the incoming BB because we know that the next + // iteration of InstCombine will simplify it. for (auto OpIndex : OpsToMoveUseTo) { - Instruction *Clone = I.clone(); Value *Op = PN->getIncomingValue(OpIndex); BasicBlock *OpBB = PN->getIncomingBlock(OpIndex); + + BranchInst *BI = dyn_cast(OpBB->getTerminator()); + if (!BI || !BI->isUnconditional() || !DT.isReachableFromEntry(OpBB)) + return nullptr; + + Instruction *Clone = I.clone(); for (Use &U : Clone->operands()) { if (U == PN) U = Op; @@ -1884,30 +1884,14 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { NewPN->takeName(PN); NewPN->setDebugLoc(PN->getDebugLoc()); - // If we are going to have to insert a new computation, do so right before the - // predecessor's terminator. - Instruction *Clone = nullptr; - if (NonSimplifiedBB) { - Clone = I.clone(); - for (Use &U : Clone->operands()) { - if (U == PN) - U = NonSimplifiedInVal; - else - U = U->DoPHITranslation(PN->getParent(), NonSimplifiedBB); - } - InsertNewInstBefore(Clone, NonSimplifiedBB->getTerminator()->getIterator()); - } - for (unsigned i = 0; i != NumPHIValues; ++i) { - if (NewPhiValues[i]) - NewPN->addIncoming(NewPhiValues[i], PN->getIncomingBlock(i)); - else - NewPN->addIncoming(Clone, PN->getIncomingBlock(i)); + NewPN->addIncoming(NewPhiValues[i], PN->getIncomingBlock(i)); } for (User *U : make_early_inc_range(PN->users())) { Instruction *User = cast(U); - if (User == &I) continue; + if (User == &I) + continue; replaceInstUsesWith(*User, NewPN); eraseInstFromFunction(*User); } From bbeded81059350daeffacf6ddfafe18a48fe367d Mon Sep 17 00:00:00 2001 From: Poseydon42 Date: Mon, 9 Sep 2024 10:21:55 +0100 Subject: [PATCH 4/8] Address review comments --- .../InstCombine/InstructionCombining.cpp | 23 +++--- ...phi-with-multiple-unsimplifiable-values.ll | 73 +++++++++++++++++++ 2 files changed, 84 insertions(+), 12 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 5ee639ef502cc..8b82d130de9fe 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1835,6 +1835,15 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { return nullptr; // More than one non-simplified value. SeenNonSimplifiedInVal = true; + // If there is exactly one non-simplified value, we can insert a copy of the + // operation in that block. However, if this is a critical edge, we would + // be inserting the computation on some other paths (e.g. inside a loop). + // Only do this if the pred block is unconditionally branching into the phi + // block. Also, make sure that the pred block is not dead code. + BranchInst *BI = dyn_cast(InBB->getTerminator()); + if (!BI || !BI->isUnconditional() || !DT.isReachableFromEntry(InBB)) + return nullptr; + NewPhiValues.push_back(nullptr); OpsToMoveUseTo.push_back(i); @@ -1851,22 +1860,12 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { return nullptr; } - // If there is exactly one non-simplified value, we can insert a copy of the - // operation in that block. However, if this is a critical edge, we would be - // inserting the computation on some other paths (e.g. inside a loop). Only - // do this if the pred block is unconditionally branching into the phi block. - // Also, make sure that the pred block is not dead code. - // After checking for all of the above, clone the instruction that uses the - // phi node and move it into the incoming BB because we know that the next - // iteration of InstCombine will simplify it. + // Clone the instruction that uses the phi node and move it into the incoming + // BB because we know that the next iteration of InstCombine will simplify it. for (auto OpIndex : OpsToMoveUseTo) { Value *Op = PN->getIncomingValue(OpIndex); BasicBlock *OpBB = PN->getIncomingBlock(OpIndex); - BranchInst *BI = dyn_cast(OpBB->getTerminator()); - if (!BI || !BI->isUnconditional() || !DT.isReachableFromEntry(OpBB)) - return nullptr; - Instruction *Clone = I.clone(); for (Use &U : Clone->operands()) { if (U == PN) diff --git a/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll b/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll index b91d8fc6e0570..6bad949a8ef1e 100644 --- a/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll +++ b/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll @@ -1,6 +1,11 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s +declare void @use(i8 %value); + +; Since we know that any comparison of ucmp/scmp with a constant will result in +; a comparison of ucmp/scmp's operands, we can propagate such a comparison +; through the phi node and let the next iteration of instcombine simplify it. define i1 @icmp_of_phi_of_scmp_with_constant(i1 %c, i16 %x, i16 %y) ; CHECK-LABEL: define i1 @icmp_of_phi_of_scmp_with_constant( ; CHECK-SAME: i1 [[C:%.*]], i16 [[X:%.*]], i16 [[Y:%.*]]) { @@ -30,3 +35,71 @@ exit: %r = icmp slt i8 %phi, 0 ret i1 %r } + +; Negative test: the RHS of comparison that uses the phi node is not constant +define i1 @icmp_of_phi_of_scmp_with_non_constant(i1 %c, i16 %x, i16 %y, i8 %cmp) +; CHECK-LABEL: define i1 @icmp_of_phi_of_scmp_with_non_constant( +; CHECK-SAME: i1 [[C:%.*]], i16 [[X:%.*]], i16 [[Y:%.*]], i8 [[CMP:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C]], label %[[TRUE:.*]], label %[[FALSE:.*]] +; CHECK: [[TRUE]]: +; CHECK-NEXT: [[CMP1:%.*]] = call i8 @llvm.scmp.i8.i16(i16 [[X]], i16 [[Y]]) +; CHECK-NEXT: br label %[[EXIT:.*]] +; CHECK: [[FALSE]]: +; CHECK-NEXT: [[CMP2:%.*]] = call i8 @llvm.scmp.i8.i16(i16 [[Y]], i16 [[X]]) +; CHECK-NEXT: br label %[[EXIT]] +; CHECK: [[EXIT]]: +; CHECK-NEXT: [[PHI:%.*]] = phi i8 [ [[CMP1]], %[[TRUE]] ], [ [[CMP2]], %[[FALSE]] ] +; CHECK-NEXT: [[R:%.*]] = icmp slt i8 [[PHI]], [[CMP]] +; CHECK-NEXT: ret i1 [[R]] +; +{ +entry: + br i1 %c, label %true, label %false +true: + %cmp1 = call i8 @llvm.scmp(i16 %x, i16 %y) + br label %exit +false: + %cmp2 = call i8 @llvm.scmp(i16 %y, i16 %x) + br label %exit +exit: + %phi = phi i8 [%cmp1, %true], [%cmp2, %false] + %r = icmp slt i8 %phi, %cmp + ret i1 %r +} + +; Negative test: more than one incoming value of the phi node is not one-use +define i1 @icmp_of_phi_of_scmp_with_constant_not_one_use(i1 %c, i16 %x, i16 %y) +; CHECK-LABEL: define i1 @icmp_of_phi_of_scmp_with_constant_not_one_use( +; CHECK-SAME: i1 [[C:%.*]], i16 [[X:%.*]], i16 [[Y:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C]], label %[[TRUE:.*]], label %[[FALSE:.*]] +; CHECK: [[TRUE]]: +; CHECK-NEXT: [[CMP1:%.*]] = call i8 @llvm.scmp.i8.i16(i16 [[X]], i16 [[Y]]) +; CHECK-NEXT: call void @use(i8 [[CMP1]]) +; CHECK-NEXT: br label %[[EXIT:.*]] +; CHECK: [[FALSE]]: +; CHECK-NEXT: [[CMP2:%.*]] = call i8 @llvm.scmp.i8.i16(i16 [[Y]], i16 [[X]]) +; CHECK-NEXT: call void @use(i8 [[CMP2]]) +; CHECK-NEXT: br label %[[EXIT]] +; CHECK: [[EXIT]]: +; CHECK-NEXT: [[PHI:%.*]] = phi i8 [ [[CMP1]], %[[TRUE]] ], [ [[CMP2]], %[[FALSE]] ] +; CHECK-NEXT: [[R:%.*]] = icmp slt i8 [[PHI]], 0 +; CHECK-NEXT: ret i1 [[R]] +; +{ +entry: + br i1 %c, label %true, label %false +true: + %cmp1 = call i8 @llvm.scmp(i16 %x, i16 %y) + call void @use(i8 %cmp1) + br label %exit +false: + %cmp2 = call i8 @llvm.scmp(i16 %y, i16 %x) + call void @use(i8 %cmp2) + br label %exit +exit: + %phi = phi i8 [%cmp1, %true], [%cmp2, %false] + %r = icmp slt i8 %phi, 0 + ret i1 %r +} From 3a8efdf218197b097137ddd0891d576667514079 Mon Sep 17 00:00:00 2001 From: Poseydon42 Date: Mon, 9 Sep 2024 14:32:01 +0100 Subject: [PATCH 5/8] Address review comments 2 --- .../Transforms/InstCombine/InstructionCombining.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 8b82d130de9fe..16a840b54e28a 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1809,7 +1809,7 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { // Check to see whether the instruction can be folded into each phi operand. // If there is one operand that does not fold, remember the BB it is in. SmallVector NewPhiValues; - SmallVector OpsToMoveUseTo; + SmallVector OpsToMoveUseToIncomingBB; bool SeenNonSimplifiedInVal = false; for (unsigned i = 0; i != NumPHIValues; ++i) { Value *InVal = PN->getIncomingValue(i); @@ -1825,8 +1825,8 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { // because we know that it will simplify to a single icmp. const APInt *Ignored; if (isa(InVal) && InVal->hasOneUse() && - match(&I, m_c_ICmp(m_Specific(PN), m_APInt(Ignored)))) { - OpsToMoveUseTo.push_back(i); + match(&I, m_ICmp(m_Specific(PN), m_APInt(Ignored)))) { + OpsToMoveUseToIncomingBB.push_back(i); NewPhiValues.push_back(nullptr); continue; } @@ -1845,7 +1845,7 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { return nullptr; NewPhiValues.push_back(nullptr); - OpsToMoveUseTo.push_back(i); + OpsToMoveUseToIncomingBB.push_back(i); // If the InVal is an invoke at the end of the pred block, then we can't // insert a computation after it without breaking the edge. @@ -1862,7 +1862,7 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { // Clone the instruction that uses the phi node and move it into the incoming // BB because we know that the next iteration of InstCombine will simplify it. - for (auto OpIndex : OpsToMoveUseTo) { + for (auto OpIndex : OpsToMoveUseToIncomingBB) { Value *Op = PN->getIncomingValue(OpIndex); BasicBlock *OpBB = PN->getIncomingBlock(OpIndex); From a7a2930c79f8d6d43f5d325889e2aa4ab042bb37 Mon Sep 17 00:00:00 2001 From: Poseydon42 Date: Wed, 11 Sep 2024 13:02:05 +0100 Subject: [PATCH 6/8] Add test case with one incoming value being ucmp/scmp --- ...phi-with-multiple-unsimplifiable-values.ll | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll b/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll index 6bad949a8ef1e..2b75d5c547511 100644 --- a/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll +++ b/llvm/test/Transforms/InstCombine/phi-with-multiple-unsimplifiable-values.ll @@ -36,6 +36,36 @@ exit: ret i1 %r } +; When one of the incoming values is ucmp/scmp and the other is not we can still perform the transformation +define i1 @icmp_of_phi_of_one_scmp_with_constant(i1 %c, i16 %x, i16 %y, i8 %false_val) +; CHECK-LABEL: define i1 @icmp_of_phi_of_one_scmp_with_constant( +; CHECK-SAME: i1 [[C:%.*]], i16 [[X:%.*]], i16 [[Y:%.*]], i8 [[FALSE_VAL:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br i1 [[C]], label %[[TRUE:.*]], label %[[FALSE:.*]] +; CHECK: [[TRUE]]: +; CHECK-NEXT: [[TMP0:%.*]] = icmp slt i16 [[X]], [[Y]] +; CHECK-NEXT: br label %[[EXIT:.*]] +; CHECK: [[FALSE]]: +; CHECK-NEXT: [[TMP1:%.*]] = icmp slt i8 [[FALSE_VAL]], 0 +; CHECK-NEXT: br label %[[EXIT]] +; CHECK: [[EXIT]]: +; CHECK-NEXT: [[PHI:%.*]] = phi i1 [ [[TMP0]], %[[TRUE]] ], [ [[TMP1]], %[[FALSE]] ] +; CHECK-NEXT: ret i1 [[PHI]] +; +{ +entry: + br i1 %c, label %true, label %false +true: + %cmp1 = call i8 @llvm.scmp(i16 %x, i16 %y) + br label %exit +false: + br label %exit +exit: + %phi = phi i8 [%cmp1, %true], [%false_val, %false] + %r = icmp slt i8 %phi, 0 + ret i1 %r +} + ; Negative test: the RHS of comparison that uses the phi node is not constant define i1 @icmp_of_phi_of_scmp_with_non_constant(i1 %c, i16 %x, i16 %y, i8 %cmp) ; CHECK-LABEL: define i1 @icmp_of_phi_of_scmp_with_non_constant( From b5fe2cc55eb4d7e0d44b85db1cd8344d5ae89bac Mon Sep 17 00:00:00 2001 From: Poseydon42 Date: Wed, 11 Sep 2024 14:13:38 +0100 Subject: [PATCH 7/8] Remove unnecessary braces --- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 16a840b54e28a..f3f8eb95fe8c4 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1883,9 +1883,8 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { NewPN->takeName(PN); NewPN->setDebugLoc(PN->getDebugLoc()); - for (unsigned i = 0; i != NumPHIValues; ++i) { + for (unsigned i = 0; i != NumPHIValues; ++i) NewPN->addIncoming(NewPhiValues[i], PN->getIncomingBlock(i)); - } for (User *U : make_early_inc_range(PN->users())) { Instruction *User = cast(U); From 45157a802c604f39def93198a100502e42788428 Mon Sep 17 00:00:00 2001 From: Poseydon42 Date: Fri, 13 Sep 2024 19:49:40 +0100 Subject: [PATCH 8/8] Add comment about single-use check --- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index f3f8eb95fe8c4..473e51bb39fc4 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1823,6 +1823,9 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) { // If the only use of phi is comparing it with a constant then we can // put this comparison in the incoming BB directly after a ucmp/scmp call // because we know that it will simplify to a single icmp. + // NOTE: the single-use check here is not only to ensure that the + // optimization is profitable, but also to avoid creating a potentially + // invalid phi node when we have a multi-edge in the CFG. const APInt *Ignored; if (isa(InVal) && InVal->hasOneUse() && match(&I, m_ICmp(m_Specific(PN), m_APInt(Ignored)))) {