Navigation Menu

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

Jump to bottom

Instsimplify: uadd_overflow(X, undef) is not undef #41554

Closed
nunoplopes opened this issue Jun 9, 2019 · 5 comments
Closed

Instsimplify: uadd_overflow(X, undef) is not undef #41554

nunoplopes opened this issue Jun 9, 2019 · 5 comments
Assignees
@LebedevRI
Labels
bugzilla Issues migrated from bugzilla

Comments

@nunoplopes
Copy link
Member

Bugzilla Link 42209
Resolution FIXED
Resolved on Sep 01, 2019 04:12
Version trunk
OS All
CC @LebedevRI,@RKSimon,@nikic,@regehr,@sanjoy,@rotateright
Fixed by commit(s) 363522

Extended Description

test/Transforms/InstSimplify/call.ll contains a few incorrect transformations when undef is involved.

For example, when %v is zero, there's no overflow, so can't fold this to undef:

define {i8, i1} @​test_uadd3(i8 %v) {
%0:
%result = uadd_overflow i8 %v, undef
ret {i8, i1} %result
}
=>
define {i8, i1} @​test_uadd3(i8 %v) {
%0:
ret {i8, i1} undef
}
Transformation doesn't verify!
ERROR: Value mismatch

Example:
i8 %v = #x00 (0)

Source:
{i8, i1} %result = { ?, #x0 }

Target:
Source value: { ?, #x0 }
Target value: { #x00, #x1 }
@nunoplopes
Copy link
Member Author

assigned to @LebedevRI

@nikic
Copy link
Contributor

nikic commented Jun 9, 2019

https://reviews.llvm.org/D63065

@LebedevRI
Copy link
Member

https://reviews.llvm.org/D63065

Yep, undef is uh, mindmelting.

@​Nuno - to summarize, and check myself - am i correct that we
can fold the entire {u,s}{add,sub}.with.overflow(%a, undef) to {undef, 0}?

To me, we can as per AliveToolkit/alive2#71 (comment)
But is that checking what i think it is checking?
If not, alive is missing insertvalue i guess?

@nunoplopes
Copy link
Member Author

https://reviews.llvm.org/D63065

Yep, undef is uh, mindmelting.

@​Nuno - to summarize, and check myself - am i correct that we
can fold the entire {u,s}{add,sub}.with.overflow(%a, undef) to {undef, 0}?

To me, we can as per
AliveToolkit/alive2#71 (comment)
But is that checking what i think it is checking?
If not, alive is missing insertvalue i guess?

It is proving exactly what you want; the transformation you mention is correct per the undef semantics. You can fold to {undef, 0}.

@LebedevRI
Copy link
Member

I didn't check with alive-tv, so if this suddenly is still incorrect, do CC me in the new bug (:

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@LebedevRI LebedevRI

Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@LebedevRI @nikic @nunoplopes