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

InstructionCombining miscompiles (x < y) || (x == y) #1616

Closed
llvmbot opened this issue Mar 9, 2007 · 4 comments
Closed

InstructionCombining miscompiles (x < y) || (x == y) #1616

llvmbot opened this issue Mar 9, 2007 · 4 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla miscompilation regression

Comments

@llvmbot
Copy link
Member

llvmbot commented Mar 9, 2007

Bugzilla Link 1244
Resolution FIXED
Resolved on Mar 06, 2010 13:58
Version trunk
OS All
Reporter LLVM Bugzilla Contributor

Extended Description

hi, I found a bug in TOT (instuction combiner). It tried to optimize (a<b)||
(a=b) to (a<=b), but the new predicate is wrong.
The original code:
%tmp266.i = icmp slt i32 %c.3.i, %d.292.2.i
%tmp276.i = icmp eq i32 %c.3.i, %d.292.2.i
%sel_tmp80 = or i1 %sel_tmp78, %sel_tmp79
New code:
%sel_tmp187.demorgan.i = icmp ule i32 %c.3.i, %d.292.2.i
--(should be sle)--

I looked the source codes, I guess the bug is in these lines:
struct FoldICmpLogical {
Instruction *apply(Instruction &Log) const {
Value *RV = getICmpValue(ICmpInst::isSignedPredicate(pred),
Code, LHS, RHS);
....
}
}
It only looks the second predicate: pred = eq for this example, so it return
non-signed, but actually it's signed because the other preciate is slt. I guess
both predicates should be looked at to get the correct results. I changed the
code locally, it works
Value *RV = getICmpValue(
ICmpInst::isSignedPredicate(ICI->getPredicate())
|| ICmpInst::isSignedPredicate(cast(Log.getOperand(1))-

getPredicate()),
Code, LHS, RHS);

Thanks.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 9, 2007

assigned to @lattner

@llvmbot
Copy link
Member Author

llvmbot commented Mar 9, 2007

Yup, it looks like a bug to me. However, I don't think you need to check both
predicates. This only occurs for == and < --> <=. == is signless so you only
need to check the < case.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 9, 2007

Actually, looks like Chris was the last one in there.

@lattner
Copy link
Collaborator

lattner commented Mar 13, 2007

This is an obvious bug from the signless types change.

Testcase here:
Transforms/InstCombine/2007-03-13-CompareMerge.ll

Patch here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070312/045812.html

Thank you for expertly reducing this down!

-Chris

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
troelsbjerre pushed a commit to troelsbjerre/llvm-project that referenced this issue Jan 10, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Add missing override to Makefile
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 miscompilation regression
Projects
None yet
Development

No branches or pull requests

2 participants