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
[apint] instcombine miscompilation #1633
Comments
Testcase: define i16 @test(i31 %zzz) { |
The differences are these (main ones highlighted with *** 360c360
|
Mine. |
Leo, Please don't file any more of these InstCombine bugs. We know that This bug is merely a reflection of SimplifyDemandedBits not supporting i31. |
*** This bug has been marked as a duplicate of 1205 *** |
It seems this is a bug after all. After bug 1205 was closed (Instcombine changes define i16 @test(i31 %zzz) { i.e. the "sext" changed to zext without any other compensating changes. This needs to be investigated freshly to determine if the zext is okay or if |
This is clearly a bug. The sign bit in the 31st bit (extended bit) is still live |
SimplifyDemandedBits is computing a DemandedMask that doesn't include the high |
The Demanded Mask makes these transitions: ret i16 %D 0xFFFF (all bits demanded in %D) Despite my previous comment, this looks correct to me. The high bit is not
Presumably this is because zext is cheaper than sext on some hardware. So, I'm claiming that the sext -> zext is correct and appropriate. |
This xform is definitely correct. |
mentioned in issue llvm/llvm-bugzilla-archive#20377 |
…f1aec93d3d482cd3d1d95c537aefe [lldb] Enable inheriting TCC permissions in lldb-test
Extended Description
I found an error in instcombine pass. Please look at the b.ll file attached. At
line 956, the extension operation is sext. After instcombine, it has been
changed to zext which causes semantic errors.
The text was updated successfully, but these errors were encountered: