[llvm] r200900 - [DAG] Don't pull the binary operation though the shift if the operands have opaque constants.

[Juergen Ributzka]

Author: ributzka
Date: Wed Feb  5 22:09:06 2014
New Revision: 200900

URL: http://llvm.org/viewvc/llvm-project?rev=200900&view=rev
Log:
[DAG] Don't pull the binary operation though the shift if the operands have opaque constants.

During DAGCombine visitShiftByConstant assumes that certain binary operations
with only constant operands can always be folded successfully. This is no longer
true when the constant is opaque. This commit fixes visitShiftByConstant by not
performing the optimization for opaque constants. Otherwise we would end up in
an infinite DAGCombine loop.

Added:
    llvm/trunk/test/CodeGen/Generic/2014-02-05-OpaqueConstants.ll
Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=200900&r1=200899&r2=200900&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Wed Feb  5 22:09:06 2014
@@ -3732,6 +3732,12 @@ SDValue DAGCombiner::visitXOR(SDNode *N)
 /// visitShiftByConstant - Handle transforms common to the three shifts, when
 /// the shift amount is a constant.
 SDValue DAGCombiner::visitShiftByConstant(SDNode *N, unsigned Amt) {
+  assert(isa<ConstantSDNode>(N->getOperand(1)) &&
+         "Expected an ConstantSDNode operand.");
+  // We can't and shouldn't fold opaque constants.
+  if (cast<ConstantSDNode>(N->getOperand(1))->isOpaque())
+    return SDValue();
+
   SDNode *LHS = N->getOperand(0).getNode();
   if (!LHS->hasOneUse()) return SDValue();
 
@@ -3757,9 +3763,9 @@ SDValue DAGCombiner::visitShiftByConstan
     break;
   }
 
-  // We require the RHS of the binop to be a constant as well.
+  // We require the RHS of the binop to be a constant and not opaque as well.
   ConstantSDNode *BinOpCst = dyn_cast<ConstantSDNode>(LHS->getOperand(1));
-  if (!BinOpCst) return SDValue();
+  if (!BinOpCst || BinOpCst->isOpaque()) return SDValue();
 
   // FIXME: disable this unless the input to the binop is a shift by a constant.
   // If it is not a shift, it pessimizes some common cases like:
@@ -3789,6 +3795,7 @@ SDValue DAGCombiner::visitShiftByConstan
   SDValue NewRHS = DAG.getNode(N->getOpcode(), SDLoc(LHS->getOperand(1)),
                                N->getValueType(0),
                                LHS->getOperand(1), N->getOperand(1));
+  assert(isa<ConstantSDNode>(NewRHS) && "Folding was not successful!");
 
   // Create the new shift.
   SDValue NewShift = DAG.getNode(N->getOpcode(),

Added: llvm/trunk/test/CodeGen/Generic/2014-02-05-OpaqueConstants.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Generic/2014-02-05-OpaqueConstants.ll?rev=200900&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Generic/2014-02-05-OpaqueConstants.ll (added)
+++ llvm/trunk/test/CodeGen/Generic/2014-02-05-OpaqueConstants.ll Wed Feb  5 22:09:06 2014
@@ -0,0 +1,19 @@
+; Test that opaque constants are not creating an infinite DAGCombine loop
+; RUN: llc < %s
+; XFAIL: r600, xcore
+
+ at a = common global i32* null, align 8
+ at c = common global i32 0, align 4
+ at b = common global i32* null, align 8
+
+; Function Attrs: nounwind ssp uwtable
+define void @fn() {
+  store i32* inttoptr (i64 68719476735 to i32*), i32** @a, align 8
+  %1 = load i32* @c, align 4
+  %2 = sext i32 %1 to i64
+  %3 = lshr i64 %2, 12
+  %4 = and i64 %3, 68719476735
+  %5 = getelementptr inbounds i32* null, i64 %4
+  store i32* %5, i32** @b, align 8
+  ret void
+}