[llvm] 3bc38fb - [InstCombine] Generalize and consolidate phi translation check (#106051)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 4 07:22:46 PDT 2024
Author: Nikita Popov
Date: 2024-09-04T16:22:43+02:00
New Revision: 3bc38fb27a12f785d8e78b8d00cbd277464ace92
URL: https://github.com/llvm/llvm-project/commit/3bc38fb27a12f785d8e78b8d00cbd277464ace92
DIFF: https://github.com/llvm/llvm-project/commit/3bc38fb27a12f785d8e78b8d00cbd277464ace92.diff
LOG: [InstCombine] Generalize and consolidate phi translation check (#106051)
The foldOpIntoPhi() transforms requires all operands to be
phi-translatable. This can be the case either because they are phi nodes
in the same block, or because the operand dominates the block.
Currently, most callers of foldOpIntoPhi() satisfy this pre-condition by
requiring a constant operand, which trivially dominates everything. Only
selects had handling for variable operands.
Move this logic into foldOpIntoPhi(), so things are handled correctly if
other callers are generalized. Also make the implementation a bit more
general by querying the dominator tree.
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/Transforms/InstCombine/phi-select-constant.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index fcd11126073bf1..66f7c4592457c2 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -1992,41 +1992,6 @@ Instruction *InstCombinerImpl::foldSelectInstWithICmp(SelectInst &SI,
return Changed ? &SI : nullptr;
}
-/// SI is a select whose condition is a PHI node (but the two may be in
-///
diff erent blocks). See if the true/false values (V) are live in all of the
-/// predecessor blocks of the PHI. For example, cases like this can't be mapped:
-///
-/// X = phi [ C1, BB1], [C2, BB2]
-/// Y = add
-/// Z = select X, Y, 0
-///
-/// because Y is not live in BB1/BB2.
-static bool canSelectOperandBeMappingIntoPredBlock(const Value *V,
- const SelectInst &SI) {
- // If the value is a non-instruction value like a constant or argument, it
- // can always be mapped.
- const Instruction *I = dyn_cast<Instruction>(V);
- if (!I) return true;
-
- // If V is a PHI node defined in the same block as the condition PHI, we can
- // map the arguments.
- const PHINode *CondPHI = cast<PHINode>(SI.getCondition());
-
- if (const PHINode *VP = dyn_cast<PHINode>(I))
- if (VP->getParent() == CondPHI->getParent())
- return true;
-
- // Otherwise, if the PHI and select are defined in the same block and if V is
- // defined in a
diff erent block, then we can transform it.
- if (SI.getParent() == CondPHI->getParent() &&
- I->getParent() != CondPHI->getParent())
- return true;
-
- // Otherwise we have a 'hard' case and we can't tell without doing more
- // detailed dominator based analysis, punt.
- return false;
-}
-
/// We have an SPF (e.g. a min or max) of an SPF of the form:
/// SPF2(SPF1(A, B), C)
Instruction *InstCombinerImpl::foldSPFofSPF(Instruction *Inner,
@@ -3929,11 +3894,8 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
// See if we can fold the select into a phi node if the condition is a select.
if (auto *PN = dyn_cast<PHINode>(SI.getCondition()))
- // The true/false values have to be live in the PHI predecessor's blocks.
- if (canSelectOperandBeMappingIntoPredBlock(TrueVal, SI) &&
- canSelectOperandBeMappingIntoPredBlock(FalseVal, SI))
- if (Instruction *NV = foldOpIntoPhi(SI, PN))
- return NV;
+ if (Instruction *NV = foldOpIntoPhi(SI, PN))
+ return NV;
if (SelectInst *TrueSI = dyn_cast<SelectInst>(TrueVal)) {
if (TrueSI->getCondition()->getType() == CondVal->getType()) {
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index ad2a620081bcd9..8195e0539305cc 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1730,8 +1730,7 @@ static Value *simplifyInstructionWithPHI(Instruction &I, PHINode *PN,
const DataLayout &DL,
const SimplifyQuery SQ) {
// NB: It is a precondition of this transform that the operands be
- // phi translatable! This is usually trivially satisfied by limiting it
- // to constant ops, and for selects we do a more sophisticated check.
+ // phi translatable!
SmallVector<Value *> Ops;
for (Value *Op : I.operands()) {
if (Op == PN)
@@ -1784,9 +1783,31 @@ Instruction *InstCombinerImpl::foldOpIntoPhi(Instruction &I, PHINode *PN) {
// Otherwise, we can replace *all* users with the new PHI we form.
}
+ // Check that all operands are phi-translatable.
+ for (Value *Op : I.operands()) {
+ if (Op == PN)
+ continue;
+
+ // Non-instructions never require phi-translation.
+ auto *I = dyn_cast<Instruction>(Op);
+ if (!I)
+ continue;
+
+ // Phi-translate can handle phi nodes in the same block.
+ if (isa<PHINode>(I))
+ if (I->getParent() == PN->getParent())
+ continue;
+
+ // Operand dominates the block, no phi-translation necessary.
+ if (DT.dominates(I, PN->getParent()))
+ continue;
+
+ // Not phi-translatable, bail out.
+ return nullptr;
+ }
+
// 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.
- // If there is more than one or if *it* is a PHI, bail out.
SmallVector<Value *> NewPhiValues;
BasicBlock *NonSimplifiedBB = nullptr;
Value *NonSimplifiedInVal = nullptr;
diff --git a/llvm/test/Transforms/InstCombine/phi-select-constant.ll b/llvm/test/Transforms/InstCombine/phi-select-constant.ll
index 9d61891127690c..81a27811ec63f5 100644
--- a/llvm/test/Transforms/InstCombine/phi-select-constant.ll
+++ b/llvm/test/Transforms/InstCombine/phi-select-constant.ll
@@ -226,17 +226,17 @@ final:
define i32 @dominating_values_select_not_same_block(i1 %c1, i1 %c2, ptr %p, ptr %p2) {
; CHECK-LABEL: @dominating_values_select_not_same_block(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[A:%.*]] = load i32, ptr [[P:%.*]], align 4
; CHECK-NEXT: [[B:%.*]] = load i32, ptr [[P2:%.*]], align 4
; CHECK-NEXT: br i1 [[C1:%.*]], label [[FINAL:%.*]], label [[DELAY:%.*]]
; CHECK: delay:
+; CHECK-NEXT: [[A:%.*]] = load i32, ptr [[P:%.*]], align 4
+; CHECK-NEXT: [[TMP0:%.*]] = select i1 [[C2:%.*]], i32 [[A]], i32 [[B]]
; CHECK-NEXT: br label [[FINAL]]
; CHECK: final:
-; CHECK-NEXT: [[USE2:%.*]] = phi i1 [ false, [[ENTRY:%.*]] ], [ [[C2:%.*]], [[DELAY]] ]
+; CHECK-NEXT: [[USE2:%.*]] = phi i32 [ [[B]], [[ENTRY:%.*]] ], [ [[TMP0]], [[DELAY]] ]
; CHECK-NEXT: br label [[SPLIT:%.*]]
; CHECK: split:
-; CHECK-NEXT: [[VALUE:%.*]] = select i1 [[USE2]], i32 [[A]], i32 [[B]]
-; CHECK-NEXT: ret i32 [[VALUE]]
+; CHECK-NEXT: ret i32 [[USE2]]
;
entry:
%a = load i32, ptr %p
More information about the llvm-commits
mailing list