[PATCH] D49919: [InstCombine] Fold Select with OR condition

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:65-87
+static Value *foldSelectInstBinaryOp(SelectInst &Sel) {
+  Value *A, *B;
+  Value *TrueVal = Sel.getTrueValue();
+  Value *FalseVal = Sel.getFalseValue();
+  if (match(Sel.getCondition(), m_c_Or(m_Value(A), m_Value(B)))) {
+    CmpInst::Predicate Pred1, Pred2;
+    Value *T1, *T2;
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > One more thing - this is slightly too verbose. I suspect it may miss some commutative cases, too.
> > Please make better use of pattern matchers.
> > 
> > How about:
> > ```
> > Value *foldSelectInstBinaryOp(SelectInst &Sel) {
> >   Value *X, *V1, *V2;
> >   CmpInst::Predicate Pred1, Pred2;
> >   if (!match(&Sel, m_Select(m_c_Or(m_ICmp(Pred1, m_Value(X), m_Value(V1)),
> >                                    m_c_ICmp(Pred2, m_Deferred(X), m_Value(V2))),
> >                             m_Deferred(X), m_Deferred(V1))) ||
> >       !(Pred1 == Pred2 && Pred1 == ICmpInst::ICMP_NE))
> >     return nullptr;
> >   return X;
> > }
> > 
> > ```
> Actually, you won't even need `V2` **right now**, i think, so,
> ```
> static Value *foldSelectInstBinaryOp(SelectInst &Sel) {
>   Value *X, *V1;
>   CmpInst::Predicate Pred1, Pred2;
>   if (!match(&Sel, m_Select(m_c_Or(m_ICmp(Pred1, m_Value(X), m_Value(V1)),
>                                    m_c_ICmp(Pred2, m_Deferred(X), m_Value())),
>                             m_Deferred(X), m_Deferred(V1))) ||
>       !(Pred1 == Pred2 && Pred1 == ICmpInst::ICMP_NE))
>     return nullptr;
>   return X;
> }
> ```
Wow, nice simplification :) Thanks


Repository:
  rL LLVM

https://reviews.llvm.org/D49919