[llvm-bugs] [Bug 33367] New: Essential code deleted by new GVN

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=33367

            Bug ID: 33367
           Summary: Essential code deleted by new GVN
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P
         Component: Scalar Optimizations
          Assignee: unassignedbugs at nondot.org
          Reporter: Kevin.Harris at unisys.com
                CC: llvm-bugs at lists.llvm.org

Created attachment 18599
  --> https://bugs.llvm.org/attachment.cgi?id=18599&action=edit
Bitcode file containing the failing function

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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20170608/b52c87fa/attachment.html>