[PATCH] D133919: [InstCombine] Fold ((x?1:4)&(y?1:4))==0 to x^y

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon requested changes to this revision.
RKSimon added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:6478
+    ) && match(Op1, m_Zero())) {
+      if ((*C & *D) == 0)
+        return BinaryOperator::CreateXor(A, B);
----------------
spatel wrote:
> marcauberer wrote:
> > @spatel do you know how to check for shared bits between C and D the best. The current solution does not work.
> > Besides that, by feeding C and D with m_APInt to the match function, the vector test case does not work properly. We might need to fall back to m_Value, do we?
> Before making enhancements to generalize the fold, we have to make sure the transform is sound. 
> 
> The Alive2 proof says that the select values must be non-zero, but that is not checked. We need more tests to verify that the matching is complete.
> 
> Once you're confident that the transform is correct for the basic pattern, look at how to make it handle other patterns:
> 1. Why are there one-use restrictions? Hint: if we're only creating 1 instruction, then we can never end up with more instructions than we started with.
> 2. Almost any fold that checks for "X == 0" alone can be generalized to handle "X != 0".
> 3. We don't need to capture all of the constants if we're looking for equivalent values; use m_Specific or m_Deferred for that. See m_ImmConstant() for a more general match than m_APInt().
You need to use m_Deferred to match later matches of C/D to their first occurrence.

Also, try to use llvm::haveNoCommonBitsSet/isKnownNonZero to match the general cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133919