[llvm] r215818 - InstCombine: Fix a potential bug in 0 - (X sdiv C) -> (X sdiv -C)

[David Majnemer]

Author: majnemer
Date: Sat Aug 16 04:23:42 2014
New Revision: 215818

URL: http://llvm.org/viewvc/llvm-project?rev=215818&view=rev
Log:
InstCombine: Fix a potential bug in 0 - (X sdiv C)  -> (X sdiv -C)

While *most* (X sdiv 1) operations will get caught by InstSimplify, it
is still possible for a sdiv to appear in the worklist which hasn't been
simplified yet.

This means that it is possible for 0 - (X sdiv 1) to get transformed
into (X sdiv -1); dividing by -1 can make the transform produce undef
values instead of the proper result.

Sorry for the lack of testcase, it's a bit problematic because it relies
on the exact order of operations in the worklist.

Modified:
    llvm/trunk/include/llvm/IR/Constant.h
    llvm/trunk/lib/IR/Constants.cpp
    llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp

Modified: llvm/trunk/include/llvm/IR/Constant.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Constant.h?rev=215818&r1=215817&r2=215818&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Constant.h (original)
+++ llvm/trunk/include/llvm/IR/Constant.h Sat Aug 16 04:23:42 2014
@@ -53,6 +53,9 @@ public:
   /// getNullValue.
   bool isNullValue() const;
 
+  /// \brief Returns true if the value is one.
+  bool isOneValue() const;
+
   /// isAllOnesValue - Return true if this is the value that would be returned by
   /// getAllOnesValue.
   bool isAllOnesValue() const;

Modified: llvm/trunk/lib/IR/Constants.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Constants.cpp?rev=215818&r1=215817&r2=215818&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Constants.cpp (original)
+++ llvm/trunk/lib/IR/Constants.cpp Sat Aug 16 04:23:42 2014
@@ -107,6 +107,28 @@ bool Constant::isAllOnesValue() const {
   return false;
 }
 
+bool Constant::isOneValue() const {
+  // Check for 1 integers
+  if (const ConstantInt *CI = dyn_cast<ConstantInt>(this))
+    return CI->isOne();
+
+  // Check for FP which are bitcasted from 1 integers
+  if (const ConstantFP *CFP = dyn_cast<ConstantFP>(this))
+    return CFP->getValueAPF().bitcastToAPInt() == 1;
+
+  // Check for constant vectors which are splats of 1 values.
+  if (const ConstantVector *CV = dyn_cast<ConstantVector>(this))
+    if (Constant *Splat = CV->getSplatValue())
+      return Splat->isOneValue();
+
+  // Check for constant vectors which are splats of 1 values.
+  if (const ConstantDataVector *CV = dyn_cast<ConstantDataVector>(this))
+    if (Constant *Splat = CV->getSplatValue())
+      return Splat->isOneValue();
+
+  return false;
+}
+
 bool Constant::isMinSignedValue() const {
   // Check for INT_MIN integers
   if (const ConstantInt *CI = dyn_cast<ConstantInt>(this))

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=215818&r1=215817&r2=215818&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp Sat Aug 16 04:23:42 2014
@@ -1583,7 +1583,7 @@ Instruction *InstCombiner::visitSub(Bina
 
     // 0 - (X sdiv C)  -> (X sdiv -C)  provided the negation doesn't overflow.
     if (match(Op1, m_SDiv(m_Value(X), m_Constant(C))) && match(Op0, m_Zero()) &&
-        !C->isMinSignedValue())
+        !C->isMinSignedValue() && !C->isOneValue())
       return BinaryOperator::CreateSDiv(X, ConstantExpr::getNeg(C));
 
     // 0 - (X << Y)  -> (-X << Y)   when X is freely negatable.