Skip to content

Commit

Permalink
Revert "[InstCombine] Fold nested selects"
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
LebedevRI committed Dec 20, 2022
1 parent 3a8e009 commit d73383c
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 200 deletions.
82 changes: 0 additions & 82 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
Expand Up @@ -2685,85 +2685,6 @@ foldRoundUpIntegerWithPow2Alignment(SelectInst &SI,
return R;
}

namespace {
struct DecomposedSelect {
Value *Cond = nullptr;
Value *TrueVal = nullptr;
Value *FalseVal = nullptr;
};
} // namespace

/// Look for patterns like
/// %outer.cond = select i1 %inner.cond, i1 %alt.cond, i1 false
/// %inner.sel = select i1 %inner.cond, i8 %inner.sel.t, i8 %inner.sel.f
/// %outer.sel = select i1 %outer.cond, i8 %outer.sel.t, i8 %inner.sel
/// and rewrite it as
/// %inner.sel = select i1 %cond.alternative, i8 %sel.outer.t, i8 %sel.inner.t
/// %sel.outer = select i1 %cond.inner, i8 %inner.sel, i8 %sel.inner.f
static Instruction *foldNestedSelects(SelectInst &OuterSelVal,
InstCombiner::BuilderTy &Builder) {
// We must start with a `select`.
DecomposedSelect OuterSel;
match(&OuterSelVal,
m_Select(m_Value(OuterSel.Cond), m_Value(OuterSel.TrueVal),
m_Value(OuterSel.FalseVal)));

// Canonicalize inversion of the outermost `select`'s condition.
if (match(OuterSel.Cond, m_Not(m_Value(OuterSel.Cond))))
std::swap(OuterSel.TrueVal, OuterSel.FalseVal);

// The condition of the outermost select must be an `and`/`or`.
if (!match(OuterSel.Cond, m_c_LogicalOp(m_Value(), m_Value())))
return nullptr;

// Depending on the logical op, inner select might be in different hand.
bool IsAndVariant = match(OuterSel.Cond, m_LogicalAnd());
Value *InnerSelVal = IsAndVariant ? OuterSel.FalseVal : OuterSel.TrueVal;

// Profitability check - avoid increasing instruction count.
if (none_of(ArrayRef<Value *>({OuterSelVal.getCondition(), InnerSelVal}),
[](Value *V) { return V->hasOneUse(); }))
return nullptr;

// The appropriate hand of the outermost `select` must be a select itself.
DecomposedSelect InnerSel;
if (!match(InnerSelVal,
m_Select(m_Value(InnerSel.Cond), m_Value(InnerSel.TrueVal),
m_Value(InnerSel.FalseVal))))
return nullptr;

// Canonicalize inversion of the innermost `select`'s condition.
if (match(InnerSel.Cond, m_Not(m_Value(InnerSel.Cond))))
std::swap(InnerSel.TrueVal, InnerSel.FalseVal);

Value *AltCond = nullptr;
auto matchOuterCond = [OuterSel, &AltCond](auto m_InnerCond) {
return match(OuterSel.Cond, m_c_LogicalOp(m_InnerCond, m_Value(AltCond)));
};

// Finally, match the condition that was driving the outermost `select`,
// it should be a logical operation between the condition that was driving
// the innermost `select` (after accounting for the possible inversions
// of the condition), and some other condition.
if (matchOuterCond(m_Specific(InnerSel.Cond))) {
// Done!
} else if (Value * NotInnerCond; matchOuterCond(m_CombineAnd(
m_Not(m_Specific(InnerSel.Cond)), m_Value(NotInnerCond)))) {
// Done!
std::swap(InnerSel.TrueVal, InnerSel.FalseVal);
InnerSel.Cond = NotInnerCond;
} else // Not the pattern we were looking for.
return nullptr;

Value *SelInner = Builder.CreateSelect(
AltCond, IsAndVariant ? OuterSel.TrueVal : InnerSel.FalseVal,
IsAndVariant ? InnerSel.TrueVal : OuterSel.FalseVal);
SelInner->takeName(InnerSelVal);
return SelectInst::Create(InnerSel.Cond,
IsAndVariant ? SelInner : InnerSel.TrueVal,
!IsAndVariant ? SelInner : InnerSel.FalseVal);
}

Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
Value *CondVal = SI.getCondition();
Value *TrueVal = SI.getTrueValue();
Expand Down Expand Up @@ -3430,9 +3351,6 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
}
}

if (Instruction *I = foldNestedSelects(SI, Builder))
return I;

// Match logical variants of the pattern,
// and transform them iff that gets rid of inversions.
// (~x) | y --> ~(x & (~y))
Expand Down

0 comments on commit d73383c

Please sign in to comment.