Skip to content

Commit d73383c

Browse files
committed
Revert "[InstCombine] Fold nested selects"
One of these two changes is exposing (or causing) some more miscompiles. A reproducer is in progress, so reverting until resolved. This reverts commit 9ddff66.
1 parent 3a8e009 commit d73383c

File tree

3 files changed

+141
-200
lines changed

3 files changed

+141
-200
lines changed

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -2685,85 +2685,6 @@ foldRoundUpIntegerWithPow2Alignment(SelectInst &SI,
26852685
return R;
26862686
}
26872687

2688-
namespace {
2689-
struct DecomposedSelect {
2690-
Value *Cond = nullptr;
2691-
Value *TrueVal = nullptr;
2692-
Value *FalseVal = nullptr;
2693-
};
2694-
} // namespace
2695-
2696-
/// Look for patterns like
2697-
/// %outer.cond = select i1 %inner.cond, i1 %alt.cond, i1 false
2698-
/// %inner.sel = select i1 %inner.cond, i8 %inner.sel.t, i8 %inner.sel.f
2699-
/// %outer.sel = select i1 %outer.cond, i8 %outer.sel.t, i8 %inner.sel
2700-
/// and rewrite it as
2701-
/// %inner.sel = select i1 %cond.alternative, i8 %sel.outer.t, i8 %sel.inner.t
2702-
/// %sel.outer = select i1 %cond.inner, i8 %inner.sel, i8 %sel.inner.f
2703-
static Instruction *foldNestedSelects(SelectInst &OuterSelVal,
2704-
InstCombiner::BuilderTy &Builder) {
2705-
// We must start with a `select`.
2706-
DecomposedSelect OuterSel;
2707-
match(&OuterSelVal,
2708-
m_Select(m_Value(OuterSel.Cond), m_Value(OuterSel.TrueVal),
2709-
m_Value(OuterSel.FalseVal)));
2710-
2711-
// Canonicalize inversion of the outermost `select`'s condition.
2712-
if (match(OuterSel.Cond, m_Not(m_Value(OuterSel.Cond))))
2713-
std::swap(OuterSel.TrueVal, OuterSel.FalseVal);
2714-
2715-
// The condition of the outermost select must be an `and`/`or`.
2716-
if (!match(OuterSel.Cond, m_c_LogicalOp(m_Value(), m_Value())))
2717-
return nullptr;
2718-
2719-
// Depending on the logical op, inner select might be in different hand.
2720-
bool IsAndVariant = match(OuterSel.Cond, m_LogicalAnd());
2721-
Value *InnerSelVal = IsAndVariant ? OuterSel.FalseVal : OuterSel.TrueVal;
2722-
2723-
// Profitability check - avoid increasing instruction count.
2724-
if (none_of(ArrayRef<Value *>({OuterSelVal.getCondition(), InnerSelVal}),
2725-
[](Value *V) { return V->hasOneUse(); }))
2726-
return nullptr;
2727-
2728-
// The appropriate hand of the outermost `select` must be a select itself.
2729-
DecomposedSelect InnerSel;
2730-
if (!match(InnerSelVal,
2731-
m_Select(m_Value(InnerSel.Cond), m_Value(InnerSel.TrueVal),
2732-
m_Value(InnerSel.FalseVal))))
2733-
return nullptr;
2734-
2735-
// Canonicalize inversion of the innermost `select`'s condition.
2736-
if (match(InnerSel.Cond, m_Not(m_Value(InnerSel.Cond))))
2737-
std::swap(InnerSel.TrueVal, InnerSel.FalseVal);
2738-
2739-
Value *AltCond = nullptr;
2740-
auto matchOuterCond = [OuterSel, &AltCond](auto m_InnerCond) {
2741-
return match(OuterSel.Cond, m_c_LogicalOp(m_InnerCond, m_Value(AltCond)));
2742-
};
2743-
2744-
// Finally, match the condition that was driving the outermost `select`,
2745-
// it should be a logical operation between the condition that was driving
2746-
// the innermost `select` (after accounting for the possible inversions
2747-
// of the condition), and some other condition.
2748-
if (matchOuterCond(m_Specific(InnerSel.Cond))) {
2749-
// Done!
2750-
} else if (Value * NotInnerCond; matchOuterCond(m_CombineAnd(
2751-
m_Not(m_Specific(InnerSel.Cond)), m_Value(NotInnerCond)))) {
2752-
// Done!
2753-
std::swap(InnerSel.TrueVal, InnerSel.FalseVal);
2754-
InnerSel.Cond = NotInnerCond;
2755-
} else // Not the pattern we were looking for.
2756-
return nullptr;
2757-
2758-
Value *SelInner = Builder.CreateSelect(
2759-
AltCond, IsAndVariant ? OuterSel.TrueVal : InnerSel.FalseVal,
2760-
IsAndVariant ? InnerSel.TrueVal : OuterSel.FalseVal);
2761-
SelInner->takeName(InnerSelVal);
2762-
return SelectInst::Create(InnerSel.Cond,
2763-
IsAndVariant ? SelInner : InnerSel.TrueVal,
2764-
!IsAndVariant ? SelInner : InnerSel.FalseVal);
2765-
}
2766-
27672688
Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
27682689
Value *CondVal = SI.getCondition();
27692690
Value *TrueVal = SI.getTrueValue();
@@ -3430,9 +3351,6 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
34303351
}
34313352
}
34323353

3433-
if (Instruction *I = foldNestedSelects(SI, Builder))
3434-
return I;
3435-
34363354
// Match logical variants of the pattern,
34373355
// and transform them iff that gets rid of inversions.
34383356
// (~x) | y --> ~(x & (~y))

0 commit comments

Comments
 (0)