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
Incorrect fold of uadd.with.overflow with undef #42533
Comments
assigned to @LebedevRI |
Uhm, i thought we fixed that already? |
Just realized this is related to #42209. Not sure constprop has a different code path than that bug. |
Seems to originate from https://reviews.llvm.org/D55950 |
r370608 |
I've just added support for struct consts in Alive2, so we can verify these now :) This is still incorrect. It has to be { undef, 1 }. You cannot get, for example, {0, 0} in the original program, while the optimized one can produce that. |
Then instsimplify is wrong too :) |
I'm getting flashbacks to https://reviews.llvm.org/D63065 and AliveToolkit/alive2#71 here. I've been arguing that the proposed { undef, false } fold is illegal because alive's modeling was buggy (at the time). The original revision also suggests the results we should be using instead, namely { -1, 0 } for add an { 0, 0 } for sub. |
Sorry for the delay; I was refactoring things in Alive so I could run all of overflow tests. define {i8, i1} @uadd_undef() { define {i8, i1} @usub_undef() { define {i8, i1} @sadd_undef() { define {i8, i1} @ssub_undef() { Ok, so {undef, 0} is not a correct result because you cannot have something like: Because that requires an overflow. |
Ah, your proposal sounds good, yes. It's basically saturating the operation. |
I tried, and i'm not sure how to feed alive constant struct, like "{ undef, 0 }" |
Sorry, parsing in the alive tool isn't implemented yet. The only way to experiment with this is either using the alive-tv tool (takes 2 LLVM IR files), or the opt plugin. |
The remaining issue is fixed by f094d65. |
Extended Description
Alive2 complains about a transformation in Transforms/ConstProp/overflow-ops.ll:
define {i8, i1} @uadd_undef() {
%0:
%t = uadd_overflow i8 142, undef
ret {i8, i1} %t
}
=>
define {i8, i1} @uadd_undef() {
%0:
ret {i8, i1} undef
}
Transformation doesn't verify!
ERROR: Value mismatch
Example:
Source:
{i8, i1} %t = { #x8e (142, -114), #x0 (0) } [based on undef value]
Target:
Source value: { #x8e (142, -114), #x0 (0) } [based on undef value]
Target value: { #x00 (0), #x0 (0) }
In summary, there's no value in the source that the undef can take that allows uadd to return {0, 0}. To return 0, it has to overflow (unsigned), and hence the 2nd value would be 1.
Two valid return values I can think off: {%a, 0}; {undef, 1}.
The text was updated successfully, but these errors were encountered: