[llvm-branch-commits] [llvm] d4ccef3 - [InstCombine] 'hoist xor-by-constant from xor-by-value': ignore constantexprs

[Roman Lebedev via llvm-branch-commits]

Author: Roman Lebedev
Date: 2020-12-28T20:15:20+03:00
New Revision: d4ccef38d0bbcd70f56d586b4dfc988db863e388

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

LOG: [InstCombine] 'hoist xor-by-constant from xor-by-value': ignore constantexprs

As it is being reported (in post-commit review) in
https://reviews.llvm.org/D93857
this fold (as i expected, but failed to come up with test coverage
despite trying) has issues with constant expressions.
Since we only care about true constants, which constantexprs are not,
don't perform such hoisting for constant expressions.

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 9d3b81a1cdd5..c0823c950368 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -3458,11 +3458,11 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
     return NewXor;
 
   // Otherwise, if all else failed, try to hoist the xor-by-constant:
-  // (X ^ C) ^ Y --> (X ^ Y) ^ C
-  // FIXME: does this need hardening against ConstantExpr's
-  //        to prevent infinite combine loops?
-  if (match(&I,
-            m_c_Xor(m_OneUse(m_Xor(m_Value(X), m_Constant(C1))), m_Value(Y))))
+  //   (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))),
+                        m_Value(Y))))
     return BinaryOperator::CreateXor(Builder.CreateXor(X, Y), C1);
 
   return nullptr;

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 e5ffb4622cbc..14227d304df7 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
@@ -73,3 +73,17 @@ define i8 @t5_commutativity(i8 %x) {
   %r = xor i8 %y, %i0
   ret i8 %r
 }
+
+ at global_constant = internal global i32 0, align 4
+ at global_constant2 = internal global i32 0, align 4
+
+define i8 @constantexpr(i8 %or) local_unnamed_addr #0 {
+; CHECK-LABEL: @constantexpr(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[R:%.*]] = xor i8 [[OR:%.*]], xor (i8 ptrtoint (i32* @global_constant to i8), i8 ptrtoint (i32* @global_constant2 to i8))
+; CHECK-NEXT:    ret i8 [[R]]
+;
+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
+}