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
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 :) Marcello 2014-07-01 23:01 GMT+02:00 David Menendez <davemm at cs.rutgers.edu>:> 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 > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140702/4d49f68d/attachment.html>
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> On Jul 1, 2014, at 6:10 PM, Marcello Maggioni <hayarms at gmail.com> wrote: > > 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 :) > > Marcello > > > > > 2014-07-01 23:01 GMT+02:00 David Menendez <davemm at cs.rutgers.edu>: > 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 > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140701/2dd30108/attachment.html>