[PATCH] D139080: [Instcombine] fold logic ops to select

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2973-2974
 
+  if (match(&I, m_c_BinOp(m_And(m_Value(A), m_Value(C)),
+                          m_Not(m_Or(m_Value(B), m_Value(D)))))) {
+    if (Op0->hasOneUse() || Op1->hasOneUse()) {
----------------
spatel wrote:
> This is more general than needed. Complexity canonicalization guarantees that the 'not' is operand 1.
> 
> We could also check for the common operand out here for efficiency?
> 
>   if (match(Op0, m_And(m_Value(A), m_Value(C))) &&
>       match(Op1, m_Not(m_Or(m_Value(B), m_Value(D)))) &&
>       hasCommonOperand(A, C, B, D) && (Op0->hasOneUse() || Op1->hasOneUse())) {
> 
If we use `hasCommonOperand` we needn't call `matchSelectFromAndOr` I think.
`matchSelectFromAndOr` will peek through the bitcast. And `getSelectCondition` consider more about the pattern with constant. For now I haven't add the support of these complex patterns. But maybe we can add them in the future. So I prefer to reuse the current code.


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

https://reviews.llvm.org/D139080