[PATCH] D159056: [InstCombine] Make `isFreeToInvert` check recursively.

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/include/llvm/Transforms/InstCombine/InstCombiner.h:273
+    Value *A, *B;
+    // Selects/min/max with invertible operands are freely invertible
+    if (match(V, m_Select(PatternMatch::m_Value(), PatternMatch::m_Value(A),
----------------
Allen wrote:
> does it make sense to restrict the condition of recursion call?  then the case **predicated_or_dominates_reduction **will not be touched.
> ```
> +    if (match(V,
> +              m_Select(PatternMatch::m_Value(), m_Not(PatternMatch::m_Value(A)),
> +                       m_Not(PatternMatch::m_Value(B)))) ||
> +        match(V, m_MaxOrMin(m_Not(PatternMatch::m_Value(A)),
> +                            m_Not(PatternMatch::m_Value(B)))))
> ```
Think that somewhat defeats the purpose of the change.
Have D159327 and D159328 which hopefully should fix the regression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159056