[llvm-branch-commits] [llvm] 374ef57 - [InstCombine] 'hoist xor-by-constant from xor-by-value': completely give up on constant exprs

[Roman Lebedev via llvm-branch-commits]

Author: Roman Lebedev
Date: 2020-12-29T16:28:18+03:00
New Revision: 374ef57f1379d3d3bbe5bfb19f1d2ea7e79b6db9

URL: https://github.com/llvm/llvm-project/commit/374ef57f1379d3d3bbe5bfb19f1d2ea7e79b6db9
DIFF: https://github.com/llvm/llvm-project/commit/374ef57f1379d3d3bbe5bfb19f1d2ea7e79b6db9.diff

LOG: [InstCombine] 'hoist xor-by-constant from xor-by-value': completely give up on constant exprs

As Mikael Holmén is noting in the post-commit review for the first fix
https://reviews.llvm.org/rGd4ccef38d0bb#967466
not hoisting constantexprs is not enough,
because if the xor originally was a constantexpr (i.e. X is a constantexpr).
`SimplifyAssociativeOrCommutative()` in `visitXor()` will immediately
undo this transform, thus again causing an infinite combine loop.

This transform has resulted in a surprising number of constantexpr failures.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    llvm/test/Transforms/InstCombine/hoist-xor-by-constant-from-xor-by-value.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index c0823c950368..15dcf2d19c15 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -3459,9 +3459,11 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
 
   // Otherwise, if all else failed, try to hoist the xor-by-constant:
   //   (X ^ C) ^ Y --> (X ^ Y) ^ C
-  // Just like we do in other places, we avoid the fold for constantexprs,
-  // at least to avoid endless combine loop.
-  if (match(&I, m_c_Xor(m_OneUse(m_Xor(m_Value(X), m_ImmConstant(C1))),
+  // Just like we do in other places, we completely avoid the fold
+  // for constantexprs, at least to avoid endless combine loop.
+  if (match(&I, m_c_Xor(m_OneUse(m_Xor(m_CombineAnd(m_Value(X),
+                                                    m_Unless(m_ConstantExpr())),
+                                       m_ImmConstant(C1))),
                         m_Value(Y))))
     return BinaryOperator::CreateXor(Builder.CreateXor(X, Y), C1);
 

diff  --git a/llvm/test/Transforms/InstCombine/hoist-xor-by-constant-from-xor-by-value.ll b/llvm/test/Transforms/InstCombine/hoist-xor-by-constant-from-xor-by-value.ll
index 14227d304df7..46bdb1a6092e 100644
--- a/llvm/test/Transforms/InstCombine/hoist-xor-by-constant-from-xor-by-value.ll
+++ b/llvm/test/Transforms/InstCombine/hoist-xor-by-constant-from-xor-by-value.ll
@@ -87,3 +87,23 @@ entry:
   %r = xor i8 %or, xor (i8 xor (i8 ptrtoint (i32* @global_constant to i8), i8 -1), i8 xor (i8 ptrtoint (i32* @global_constant2 to i8), i8 -1))
   ret i8 %r
 }
+
+ at global_constant3 = external global [6 x [1 x i64]], align 1
+ at global_constant4 = external global i64, align 1
+ at global_constant5 = external global i16*, align 1
+
+define i16 @constantexpr2() {
+; CHECK-LABEL: @constantexpr2(
+; CHECK-NEXT:    [[I2:%.*]] = load i16*, i16** @global_constant5, align 1
+; CHECK-NEXT:    [[I3:%.*]] = load i16, i16* [[I2]], align 1
+; CHECK-NEXT:    [[I5:%.*]] = xor i16 [[I3]], xor (i16 zext (i1 icmp ne (i64* getelementptr inbounds ([6 x [1 x i64]], [6 x [1 x i64]]* @global_constant3, i64 0, i64 5, i64 0), i64* @global_constant4) to i16), i16 -1)
+; CHECK-NEXT:    ret i16 [[I5]]
+;
+  %i0 = icmp ne i64* getelementptr inbounds ([6 x [1 x i64]], [6 x [1 x i64]]* @global_constant3, i16 0, i16 5, i16 0), @global_constant4
+  %i1 = zext i1 %i0 to i16
+  %i2 = load i16*, i16** @global_constant5, align 1
+  %i3 = load i16, i16* %i2, align 1
+  %i4 = xor i16 %i3, %i1
+  %i5 = xor i16 %i4, -1
+  ret i16 %i5
+}