-
Notifications
You must be signed in to change notification settings - Fork 15.7k
Description
| Bugzilla Link | 33367 |
| Resolution | FIXED |
| Resolved on | Jan 10, 2018 14:38 |
| Version | trunk |
| OS | Linux |
| Blocks | #30343 #35152 |
| Attachments | Bitcode file containing the failing function |
| Reporter | LLVM Bugzilla Contributor |
Extended Description
Since Alex B. highlighted that most of the newgvn code had landed, we started testing it immediately in our Unisys JIT, which is accelerating emulation of one of our company's legacy architectures. Our performance is heavily dependent on good scalar optimization, so we were looking forward to using the new GVN in our product, as soon as it is ready for prime time.
Unfortunately, we discovered a problem in our first production test run.
We isolated it to a particular generated function (attached) and tentatively identified the problematic area of code:
"BB3.000316011731#1.1": ; preds = %"BB3.000316011731#1"
%shifts194 = zext i16 %shifts to i64
%val = call i64 @llvm.x86.bmi.bextr.64(i64 %rmem191, i64 %shifts194)
%preg195 = getelementptr inbounds [64 x i64], [64 x i64]* %pCurrentGrs, i64 0, i64 12
store i64 %val, i64* %preg195, align 32, !tbaa !32
The final store uploads a new value to a location in the emulated machine model, which is essential to correct behavior.
The old gvn optimizes this code sequence to:
"BB3.000316011731#1.1": ; preds = %"BB3.000316011731#1"
%shifts194 = zext i16 %shifts to i64
%val = tail call i64 @llvm.x86.bmi.bextr.64(i64 %rmem191, i64 %shifts194)
store i64 %val, i64* %preg98, align 32, !tbaa !32
The %preg98 substitution is a CSE usage from an earlier point in the function. The new GVN deletes this code fragment entirely, and also some code a little farther down that also uses %val. This second deletion appears to indicate that newGVN has determined that %val is always zero (which it isn't). But this isn't consistent with eliminating the store described above. A little later, these incorrect values cause a branch destination to be computed incorrectly, which is the cause of our emulator failure that allowed us to notice the problem originally.
We default our optimization level to -O3, and we were able to reproduce the problem with the attached bitcode file using the opt command as follows:
Original GVN pass: ../llvm-tot-install/bin/opt -O3 -S if000316011717_2.bc > if000316011717_2.s
Using NewGVN: ../llvm-tot-install/bin/opt -O3 -enable-newgvn -S if000316011717_2.bc > if000316011717_2_newgvn.s
We don't see anywhere obvious that this code is employing any undefined behavior or other improper usage, and has worked with the existing GVN for a couple years. But if you discover a problem with the original source IR that is triggering this behavior, we're happy to change it.
Please let us know if you need any additional info to reproduce or diagnose this problem.