Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instcombine turns -(x/INT_MIN) to x/INT_MIN #20560

Closed
llvmbot opened this issue Jul 1, 2014 · 4 comments
Closed

Instcombine turns -(x/INT_MIN) to x/INT_MIN #20560

llvmbot opened this issue Jul 1, 2014 · 4 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 1, 2014

Bugzilla Link 20186
Resolution FIXED
Resolved on Jul 02, 2014 01:08
Version trunk
OS All
Reporter LLVM Bugzilla Contributor
CC @majnemer,@nunoplopes,@regehr,@zygoloid

Extended Description

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));

@llvmbot
Copy link
Collaborator Author

llvmbot commented Jul 1, 2014

assigned to @majnemer

@regehr
Copy link
Contributor

regehr commented Jul 1, 2014

I checked this against a recent LLVM on x86-64 and believe it is a real bug.

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Jul 2, 2014

We should transform X/INT_MIN to X==INT_MIN instead.

@majnemer
Copy link
Mannequin

majnemer mannequin commented Jul 2, 2014

Fixed in r212164.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

2 participants