[PATCH] D101720: [InstCombine] generalize select + select/and/or folding using implied conditions

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2596
+  assert(SI.getType() == CondVal->getType() &&
+         "foldAndOrOfSelectUsingImpliedCond deals with i1 typed operands only");
+
----------------
nikic wrote:
> nikic wrote:
> > I'm not sure about this assert. Can't you end up with an  i1 CondVal, but an <n x i1> SI here?
> To clarify, I do think you need to check for that case, but as a normal condition, not an assert. You do check this before calling the helper for the and/or case, but I don't think anything ensures it for the case where the root instruction is a select.
> 
> Could you please add a test for this kind of scalar/vector mismatch?
I think removing runtime check is fine: isImpliedCondition is handling this case well (simply returning None currently). Also, the created SelectInst does not contain Cond as well.
If isImpliesCondition is enhanced to deal with vector+nonvector ops, this code gets benefit as well.

I added the scalar/vector code mismatch test as well (a_true_implies_b_true_vec).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101720/new/

https://reviews.llvm.org/D101720