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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2464
+  Value *X, *Y;
+  if (!match(&I, m_Not(m_OneUse(m_Xor(m_Value(X), m_Value(Y))))))
+    return nullptr;
----------------
spatel wrote:
> The one-use here is not strictly necessary? There are questions about how to best deal with inverted compares, so I don't think it needs to hold this patch up, but it's worth a code comment if I'm seeing it correctly. 
> 
> This reminded me of the still open:
> D35182
> 
Hm. I think it is needed, else we get:
```
diff --git a/test/Transforms/InstCombine/demorgan-sink-not-into-xor.ll b/test/Transforms/InstCombine/demorgan-sink-not-into-xor.ll
index c378033eef7..c5809b7726b 100644
--- a/test/Transforms/InstCombine/demorgan-sink-not-into-xor.ll
+++ b/test/Transforms/InstCombine/demorgan-sink-not-into-xor.ll
@@ -85,19 +85,20 @@ define i1 @oneuse_easyinvert_0(i8 %y) {
   ret i1 %tmp4
 }
 
 define i1 @oneuse_easyinvert_1(i8 %y) {
 ; CHECK-LABEL: @oneuse_easyinvert_1(
 ; CHECK-NEXT:    [[TMP1:%.*]] = call i1 @gen1()
 ; CHECK-NEXT:    [[TMP2:%.*]] = icmp slt i8 [[Y:%.*]], 0
 ; CHECK-NEXT:    [[TMP3:%.*]] = xor i1 [[TMP1]], [[TMP2]]
 ; CHECK-NEXT:    call void @use1(i1 [[TMP3]])
-; CHECK-NEXT:    [[TMP4:%.*]] = xor i1 [[TMP3]], true
+; CHECK-NEXT:    [[TMP2_NOT:%.*]] = xor i1 [[TMP2]], true
+; CHECK-NEXT:    [[TMP4:%.*]] = xor i1 [[TMP1]], [[TMP2_NOT]]
 ; CHECK-NEXT:    ret i1 [[TMP4]]
 ;
   %tmp1 = call i1 @gen1()
   %tmp2 = icmp slt i8 %y, 0
   %tmp3 = xor i1 %tmp1, %tmp2
   call void @use1(i1 %tmp3)
   %tmp4 = xor i1 %tmp3, true
   ret i1 %tmp4
 }
```


Repository:
  rL LLVM

https://reviews.llvm.org/D50301