[llvm] r212164 - InstCombine: Don't turn -(x/INT_MIN) -> x/INT_MIN

[Nuno Lopes]

On Wednesday 02 July 2014 06:07:09 David Majnemer wrote:
> Author: majnemer
> Date: Wed Jul  2 01:07:09 2014
> New Revision: 212164
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=212164&view=rev
> Log:
> InstCombine: Don't turn -(x/INT_MIN) -> x/INT_MIN
> 
> It is not safe to negate the smallest signed integer, doing so yields
> the same number back.
> 
> This fixes PR20186.
> 
> Modified:
>     llvm/trunk/include/llvm/IR/Constant.h
>     llvm/trunk/lib/IR/Constants.cpp
>     llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>     llvm/trunk/test/Transforms/InstCombine/sub.ll
> 
> Modified: llvm/trunk/include/llvm/IR/Constant.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Constant.h?r
> ev=212164&r1=212163&r2=212164&view=diff
> ===========================================================================
> === --- llvm/trunk/include/llvm/IR/Constant.h (original)
> +++ llvm/trunk/include/llvm/IR/Constant.h Wed Jul  2 01:07:09 2014
> @@ -64,6 +64,9 @@ public:
>    /// Return true if the value is negative zero or null value.
>    bool isZeroValue() const;
> 
> +  /// \brief Return true if the value is the smallest signed value.
> +  bool isMinSignedValue() const;
> +
>    /// canTrap - Return true if evaluation of this constant could trap. 
> This is /// true for things like constant expressions that could divide by
> zero. bool canTrap() const;
> 
> Modified: llvm/trunk/lib/IR/Constants.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Constants.cpp?rev=212
> 164&r1=212163&r2=212164&view=diff
> ===========================================================================
> === --- llvm/trunk/lib/IR/Constants.cpp (original)
> +++ llvm/trunk/lib/IR/Constants.cpp Wed Jul  2 01:07:09 2014
> @@ -107,6 +107,28 @@ bool Constant::isAllOnesValue() const {
>    return false;
>  }
> 
> +bool Constant::isMinSignedValue() const {
> +  // Check for INT_MIN integers
> +  if (const ConstantInt *CI = dyn_cast<ConstantInt>(this))
> +    return CI->isMinValue(/*isSigned=*/true);
> +
> +  // Check for FP which are bitcasted from INT_MIN integers
> +  if (const ConstantFP *CFP = dyn_cast<ConstantFP>(this))
> +    return CFP->getValueAPF().bitcastToAPInt().isMinSignedValue();
> +
> +  // Check for constant vectors which are splats of INT_MIN values.
> +  if (const ConstantVector *CV = dyn_cast<ConstantVector>(this))
> +    if (Constant *Splat = CV->getSplatValue())
> +      return Splat->isMinSignedValue();
> +
> +  // Check for constant vectors which are splats of INT_MIN values.
> +  if (const ConstantDataVector *CV = dyn_cast<ConstantDataVector>(this))
> +    if (Constant *Splat = CV->getSplatValue())
> +      return Splat->isMinSignedValue();
> +
> +  return false;
> +}
> +
>  // Constructor to create a '0' constant of arbitrary type...
>  Constant *Constant::getNullValue(Type *Ty) {
>    switch (Ty->getTypeID()) {
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/I
> nstCombineAddSub.cpp?rev=212164&r1=212163&r2=212164&view=diff
> ===========================================================================
> === --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> (original) +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> Wed Jul  2 01:07:09 2014 @@ -1553,9 +1553,9 @@ Instruction
> *InstCombiner::visitSub(Bina
>        return BinaryOperator::CreateAnd(Op0,
>                                    Builder->CreateNot(Y, Y->getName() +
> ".not"));
> 
> -    // 0 - (X sdiv C)  -> (X sdiv -C)
> -    if (match(Op1, m_SDiv(m_Value(X), m_Constant(C))) &&
> -        match(Op0, m_Zero()))
> +    // 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())
>        return BinaryOperator::CreateSDiv(X, ConstantExpr::getNeg(C));
> 
>      // 0 - (X << Y)  -> (-X << Y)   when X is freely negatable.

This transformation is not correct when C == 1.
We'll get this:
X sdiv 1 == X
0 - X  == -X

And after the transformation:
X sdiv -1 == undefined if X == INT_MIN


In theory, 'X sdiv 1' should be folded before, but I'm not sure we can 
guarantee that, can we?  (we don't really control the order in the work list)

Nuno


> Modified: llvm/trunk/test/Transforms/InstCombine/sub.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/
> sub.ll?rev=212164&r1=212163&r2=212164&view=diff
> ===========================================================================
> === --- llvm/trunk/test/Transforms/InstCombine/sub.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/sub.ll Wed Jul  2 01:07:09 2014
> @@ -444,3 +444,22 @@ define <2 x i64> @test36(<2 x i64> %A) {
>  ; CHECK-NEXT: %sub = mul <2 x i64> %A, <i64 7, i64 15>
>  ; CHECK-NEXT: ret <2 x i64> %sub
>  }
> +
> +define <2 x i64> @test37(<2 x i64> %A) {
> +  %shl = shl <2 x i64> %A, <i64 3, i64 4>
> +  %sub = sub <2 x i64> %shl, %A
> +  ret <2 x i64> %sub
> +; CHECK-LABEL: @test37(
> +; CHECK-NEXT: %sub = mul <2 x i64> %A, <i64 7, i64 15>
> +; CHECK-NEXT: ret <2 x i64> %sub
> +}
> +
> +define i32 @test38(i32 %A) {
> +  %div = sdiv i32 %A, -2147483648
> +  %sub = sub nsw i32 0, %div
> +  ret i32 %sub
> +; CHECK-LABEL: @test38(
> +; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 %A, -2147483648
> +; CHECK-NEXT: [[SUB:%.*]] = sub nsw i32 0, [[DIV]]
> +; CHECK-NEXT: ret i32 [[SUB]]
> +}