[PATCH] D140666: [InstCombine] combine intersection for inequality icmps

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a reviewer: bcl5980.
spatel added a comment.

I'm not familiar with all of the potential folds in foldLogOpOfMaskedICmps(), so if anyone else has comments, please jump in.

I'd prefer that we add more negative tests such as mismatched icmp predicates, operands, and bitwise logic ops (even if there are shared predicates with existing transform to bail out on those patterns).

Also, please pre-commit baseline tests to main or at least locally and rebase this patch on top of that, so we can see exact diffs for tests that do change.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:681-682
+
+    assert((~*ConstB & ConstC) == 0);
+    assert((~*ConstD & ConstE) == 0);
+
----------------
Add a string message to each assert like "Expected impossible compares to be simplified".

But I'm skeptical that we can actually do that. Ie, how do we guarantee that the operands of this logic instruction were already visited before we got here? If we can't guarantee that, just bail out instead.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:688
+
+    Value *NewAnd1 = Builder.CreateAnd(B, D);
+    Value *NewAnd2 = Builder.CreateAnd(A, NewAnd1);
----------------
Why does this use B and D rather than ConstB and ConstD?
In fact, why do we create this at all - can we use ConstT in the next line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140666