Probable error in InstCombine

Project Infrastructure LLVM Dev List Archives
1

I've found what appears to be a bug in instcombine. Specifically, the transformation of -(X/C) to X/(-C) is invalid if C == INT_MIN.

Specifically, if I have

define i32 @foo(i32 %x) #0 {
entry:
%div = sdiv i32 %x, -2147483648
%sub = sub nsw i32 0, %div
ret i32 %sub
}

then opt -instcombine will produce

define i32 @foo(i32 %x) #0 {
entry:
%sub = sdiv i32 %x, -2147483648
ret i32 %sub
}

You can observe this with the following test case:

#include <stdio.h>
#include <limits.h>

int foo(int x)
{
return -(x/INT_MIN);
}

int main (void)
{
printf ("%d\n", foo(INT_MIN));
}

This will print -1 or 1, depending on the optimization level.

This appears to be the relevant code:

InstCombineAddSub.cpp:1556

    // 0 - (X sdiv C) -> (X sdiv -C)
    if (match(Op1, m_SDiv(m_Value(X), m_Constant(C))) &&
        match(Op0, m_Zero()))
      return BinaryOperator::CreateSDiv(X, ConstantExpr::getNeg(C));

- David Menendez

2

Hmm, I believe you are right there.

There is no representable respective positive value for INT_MIN in i32 , which means the transformation is not possible in this case (in particular seems ConstantExpr::getNeg(C) in this case == C itself).

It should be checked that C->isMinValue(true) does not return true before applying the optimization I believe (C should be always a ConstantInt) .
I’m not an expert on this though :slight_smile:

Marcello

3

Worth noting that if C is INTTYPE_MIN, (X/C) is equivalent to (X == INTTYPE_MIN), and CMP + SETcc/CMOV is generally cheaper than integer division.

– Steve