[PATCH] D118094: [ValueTracking] Checking haveNoCommonBitsSet for (x & y) and ~(x | y)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:290
+    Value *A, *B;
+    if (match(LHS, m_c_And(m_Value(A), m_Value(B))) &&
+        match(RHS, m_Not(m_c_Or(m_Specific(A), m_Specific(B)))))
----------------
If I'm seeing it correctly, these lines could use "m_And" rather than "m_c_And"?


================
Comment at: llvm/test/Transforms/InstCombine/pr53357.ll:19-20
 
 ; (x & y) + ~(x | y)
 define i32 @src_thwart(i32 %0, i32 %1) {
 ; CHECK-LABEL: @src_thwart(
----------------
This test does not add any value to the previous test. Let's try to simplify things.

IIUC, there are exactly 4 commutative patterns that we want to match:
  (x & y) + ~(x | y)
  (x & y) + ~(y | x) 
  ~(x | y) + (x & y) 
  ~(y | x) + (x & y)

(We can swap the names of x and y, and it doesn't change any logic in the matching code.)

1. Create tests that correspond to those patterns (including using the same value names as in the test comments):

```
; (x & y) + ~(x | y)
define i32 @no_common_bits_and_nor(i32 %x, i32 %y) {
  %and = and i32 %x, %y
  %or = or i32 %x, %y
  %nor = xor i32 %or, -1
  %r = add i32 %and, %nor
  ret i32 %r
}
```

2. You can vary the types while checking the commutes, so some of the tests can use vectors (with or without an undef in the 'not' constant).

3. I'm not sure what the tests with extra 'not' instructions are showing. As we can see in the baseline CHECK lines, those are transformed before we reach the transform in this patch. If we want to show that this patch can or can't handle an unreduced logic pattern, then you have to add extra uses of the intermediate values to prevent the other transforms from altering the instructions in the tests.


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

https://reviews.llvm.org/D118094