[PATCH] D50301: [InstCombine] De Morgan: sink 'not' into 'xor' (PR38446)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D50301#1191081, @spatel wrote:

> I may be starting a substantial yak shaving with the inline question, but I wonder if it wouldn't be better to canonicalize with the 'not' after the 'xor' (ie, the inverse of this patch). I don't have a motivating example yet, but that just seems more natural rather than semi-arbitralily choosing an operand to invert.


Quite possibly. I don't know what the right thing to do here. See inline comments.



================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2467-2469
+  // If X is not free to invert, but Y is, then swap them.
+  if (!IsFreeToInvert(X, X->hasOneUse()) && IsFreeToInvert(Y, Y->hasOneUse()))
+    std::swap(X, Y);
----------------
If we do not want to do this always, then this can be:
```
if (IsFreeToInvert(X, X->hasOneUse()))
  // ok
else if (IsFreeToInvert(Y, Y->hasOneUse()))
  std::swap(X, Y);
else
  return nullptr; // Only transform if we will be able to get rid of 'not'.
```


================
Comment at: test/Transforms/InstCombine/demorgan-sink-not-into-xor.ll:37-49
 define i1 @positive_easyinvert(i8 %y) {
 ; CHECK-LABEL: @positive_easyinvert(
 ; CHECK-NEXT:    [[TMP1:%.*]] = call i1 @gen1()
-; CHECK-NEXT:    [[TMP2:%.*]] = icmp slt i8 [[Y:%.*]], 0
-; CHECK-NEXT:    [[TMP3:%.*]] = xor i1 [[TMP1]], [[TMP2]]
-; CHECK-NEXT:    [[TMP4:%.*]] = xor i1 [[TMP3]], true
+; CHECK-NEXT:    [[TMP2:%.*]] = icmp sgt i8 [[Y:%.*]], -1
+; CHECK-NEXT:    [[TMP4:%.*]] = xor i1 [[TMP1]], [[TMP2]]
 ; CHECK-NEXT:    ret i1 [[TMP4]]
 ;
----------------
I don't know about the other cases, but do you agree that this is an improvement?
This is the motivational case here, the only one i care about here.


Repository:
  rL LLVM

https://reviews.llvm.org/D50301