Page MenuHomePhabricator

[MemorySSA] Don't verify MemorySSA unless VerifyMemorySSA enabled
ClosedPublic

Authored by nikic on Feb 12 2020, 1:29 PM.

Details

Summary

I noticed that MemorySSA is often taking up an unreasonable fraction of runtime in assertion enabled builds. Turns out that there is one code-path that runs verifyMemorySSA() even if VerifyMemorySSA is not enabled. This patch makes it conditional as well.

For the test case I'm looking at right now:

Before 

  12.5533 ( 21.9%)   0.1048 (  2.9%)  12.6581 ( 20.7%)  12.6539 ( 20.7%)  Memory SSA #2
   6.1540 ( 10.7%)   0.2563 (  7.0%)   6.4103 ( 10.5%)   6.4076 ( 10.5%)  Function Integration/Inlining
   2.3936 (  4.2%)   0.1117 (  3.0%)   2.5053 (  4.1%)   2.5034 (  4.1%)  Global Value Numbering
   1.9745 (  3.4%)   0.0211 (  0.6%)   1.9955 (  3.3%)   1.9949 (  3.3%)  Rotate Loops
   1.6837 (  2.9%)   0.1032 (  2.8%)   1.7869 (  2.9%)   1.7851 (  2.9%)  Induction Variable Simplification
   1.6743 (  2.9%)   0.0324 (  0.9%)   1.7068 (  2.8%)   1.7053 (  2.8%)  Memory SSA #4
   1.4166 (  2.5%)   0.0391 (  1.1%)   1.4558 (  2.4%)   1.4547 (  2.4%)  Memory SSA #5
   1.3661 (  2.4%)   0.0389 (  1.1%)   1.4050 (  2.3%)   1.4037 (  2.3%)  Memory SSA #3
    
After
 
   6.6570 ( 14.5%)   0.2266 (  6.1%)   6.8836 ( 13.9%)   6.8809 ( 13.9%)  Function Integration/Inlining
   2.7801 (  6.1%)   0.1170 (  3.1%)   2.8972 (  5.8%)   2.8947 (  5.9%)  Global Value Numbering
   2.0526 (  4.5%)   0.0416 (  1.1%)   2.0942 (  4.2%)   2.0932 (  4.2%)  Rotate Loops
   1.8899 (  4.1%)   0.1305 (  3.5%)   2.0203 (  4.1%)   2.0183 (  4.1%)  Induction Variable Simplification

There is a similar report with much more extreme numbers at https://bugs.llvm.org/show_bug.cgi?id=44238 as well.

Diff Detail

Event Timeline

nikic created this revision.Feb 12 2020, 1:29 PM
asbirlea accepted this revision.Feb 12 2020, 4:51 PM

This had been intentionally left in in order to catch potential invalidations in debug builds.
However, since the overhead is overly large, putting it under EXPENSIVE_CHECKS seems the right thing to do.

This revision is now accepted and ready to land.Feb 12 2020, 4:51 PM
This revision was automatically updated to reflect the committed changes.