]> granicus.if.org Git - llvm/commitdiff
[InstCombine] don't use DeMorgan's Law on integer constants
authorSanjay Patel <spatel@rotateright.com>
Tue, 2 May 2017 14:31:30 +0000 (14:31 +0000)
committerSanjay Patel <spatel@rotateright.com>
Tue, 2 May 2017 14:31:30 +0000 (14:31 +0000)
This is the fold that causes the infinite loop in BoringSSL
(https://github.com/google/boringssl/blob/master/crypto/cipher/e_rc2.c)
when we fix instcombine demanded bits to prefer 'not' ops as in D32255.

There are 2 or 3 problems with dyn_castNotVal, and I don't think we can
reinstate D32255 until dyn_castNotVal is completely eliminated.
1. As shown here, it transforms 'not' into random xor. This transform is
   harmful to SCEV and codegen because 'not' can often be folded while
   random xor cannot.
2. It does not transform vector constants. This is actually a good thing,
   but if you don't believe the above argument, then we shouldn't have
   excluded vectors.
3. It tries to avoid transforming not(not(X)). That's nice, but it doesn't
   match the greedy nature of instcombine. If we DeMorganize a pattern
   that has an extra 'not' in it:
   ~(~(~X) & Y) --> (~X | ~Y)

   That's just another case of DeMorgan, so we should trust that we'll fold
   that pattern too:
   (~X | ~ Y) --> ~(X & Y)

Differential Revision: https://reviews.llvm.org/D32665

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@301923 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
test/Transforms/InstCombine/assume2.ll
test/Transforms/InstCombine/demorgan.ll

index 41ae37ee127b6a01b130a4bbc875d624ccf67bab..c7092bf3a398b87e34c82acdcc2a2e9be192002b 100644 (file)
@@ -2433,29 +2433,32 @@ Instruction *InstCombiner::visitXor(BinaryOperator &I) {
   if (Value *V = SimplifyBSwap(I))
     return replaceInstUsesWith(I, V);
 
+  // Apply DeMorgan's Law for 'nand' / 'nor' logic with an inverted operand.
+  Value *X, *Y;
+
+  // We must eliminate the and/or (one-use) for these transforms to not increase
+  // the instruction count.
+  // ~(~X & Y) --> (X | ~Y)
+  // ~(Y & ~X) --> (X | ~Y)
+  if (match(&I, m_Not(m_OneUse(m_c_And(m_Not(m_Value(X)), m_Value(Y)))))) {
+    Value *NotY = Builder->CreateNot(Y, Y->getName() + ".not");
+    return BinaryOperator::CreateOr(X, NotY);
+  }
+  // ~(~X | Y) --> (X & ~Y)
+  // ~(Y | ~X) --> (X & ~Y)
+  if (match(&I, m_Not(m_OneUse(m_c_Or(m_Not(m_Value(X)), m_Value(Y)))))) {
+    Value *NotY = Builder->CreateNot(Y, Y->getName() + ".not");
+    return BinaryOperator::CreateAnd(X, NotY);
+  }
+
   // Is this a 'not' (~) fed by a binary operator?
   BinaryOperator *NotOp;
   if (match(&I, m_Not(m_BinOp(NotOp)))) {
     if (NotOp->getOpcode() == Instruction::And ||
         NotOp->getOpcode() == Instruction::Or) {
-      // We must eliminate the and/or for this transform to not increase the
-      // instruction count.
-      if (NotOp->hasOneUse()) {
-        // ~(~X & Y) --> (X | ~Y) - De Morgan's Law
-        // ~(~X | Y) === (X & ~Y) - De Morgan's Law
-        if (dyn_castNotVal(NotOp->getOperand(1)))
-          NotOp->swapOperands();
-        if (Value *Op0NotVal = dyn_castNotVal(NotOp->getOperand(0))) {
-          Value *NotY = Builder->CreateNot(
-              NotOp->getOperand(1), NotOp->getOperand(1)->getName() + ".not");
-          if (NotOp->getOpcode() == Instruction::And)
-            return BinaryOperator::CreateOr(Op0NotVal, NotY);
-          return BinaryOperator::CreateAnd(Op0NotVal, NotY);
-        }
-      }
-
-      // ~(X & Y) --> (~X | ~Y) - De Morgan's Law
-      // ~(X | Y) === (~X & ~Y) - De Morgan's Law
+      // Apply DeMorgan's Law when inverts are free:
+      // ~(X & Y) --> (~X | ~Y)
+      // ~(X | Y) --> (~X & ~Y)
       if (IsFreeToInvert(NotOp->getOperand(0),
                          NotOp->getOperand(0)->hasOneUse()) &&
           IsFreeToInvert(NotOp->getOperand(1),
index e8fbc049f41aa8e333f3e21b386d0af9850517c8..8dc8831fffa5f67009ae41b9bf9c909bd776fee0 100644 (file)
@@ -21,8 +21,8 @@ define i32 @test1(i32 %a) #0 {
 
 define i32 @test2(i32 %a) #0 {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:    [[A_NOT:%.*]] = or i32 [[A:%.*]], -16
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[A_NOT]], -6
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[A:%.*]], 15
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[AND]], 10
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP]])
 ; CHECK-NEXT:    ret i32 2
 ;
@@ -50,8 +50,8 @@ define i32 @test3(i32 %a) #0 {
 
 define i32 @test4(i32 %a) #0 {
 ; CHECK-LABEL: @test4(
-; CHECK-NEXT:    [[A_NOT:%.*]] = and i32 [[A:%.*]], 15
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[A_NOT]], 10
+; CHECK-NEXT:    [[V:%.*]] = or i32 [[A:%.*]], -16
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[V]], -6
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP]])
 ; CHECK-NEXT:    ret i32 2
 ;
index fc4af3c6dad1b4d8a1573fab490c18d8d6e6428c..26c2270a3fdfe60eab2eb6a6f78c5fc364b87481 100644 (file)
@@ -367,12 +367,12 @@ define i8 @demorgan_nor_use2bc(i8 %A, i8 %B) {
   ret i8 %r2
 }
 
-; FIXME: Do not apply DeMorgan's Law to constants. We prefer 'not' ops.
+; Do not apply DeMorgan's Law to constants. We prefer 'not' ops.
 
 define i32 @demorganize_constant1(i32 %a) {
 ; CHECK-LABEL: @demorganize_constant1(
-; CHECK-NEXT:    [[A_NOT:%.*]] = or i32 %a, -16
-; CHECK-NEXT:    [[AND1:%.*]] = xor i32 [[A_NOT]], 15
+; CHECK-NEXT:    [[AND:%.*]] = and i32 %a, 15
+; CHECK-NEXT:    [[AND1:%.*]] = xor i32 [[AND]], -1
 ; CHECK-NEXT:    ret i32 [[AND1]]
 ;
   %and = and i32 %a, 15
@@ -380,12 +380,12 @@ define i32 @demorganize_constant1(i32 %a) {
   ret i32 %and1
 }
 
-; FIXME: Do not apply DeMorgan's Law to constants. We prefer 'not' ops.
+; Do not apply DeMorgan's Law to constants. We prefer 'not' ops.
 
 define i32 @demorganize_constant2(i32 %a) {
 ; CHECK-LABEL: @demorganize_constant2(
-; CHECK-NEXT:    [[A_NOT:%.*]] = and i32 %a, -16
-; CHECK-NEXT:    [[AND1:%.*]] = xor i32 [[A_NOT]], -16
+; CHECK-NEXT:    [[AND:%.*]] = or i32 %a, 15
+; CHECK-NEXT:    [[AND1:%.*]] = xor i32 [[AND]], -1
 ; CHECK-NEXT:    ret i32 [[AND1]]
 ;
   %and = or i32 %a, 15