CmmPipeline: add a second pass of CmmCommonBlockElim
ClosedPublic

Authored by michalt on Feb 17 2018, 11:32 AM.

Details

Summary

The sinking pass often gets rid of unnecessary registers
registers/assignements exposing more opportunities for CBE, so this
commit adds a second round of CBE after the sinking pass and should
fix Trac #12915 (and some examples in Trac #14226).

Nofib results:

  • Binary size: 0.9% reduction on average
  • Compile allocations: 0.7% increase on average
  • Runtime: noisy, two separate runs of nofib showed a tiny reduction on average, (~0.2-0.3%), but I think this is mostly noise
  • Compile time: very noisy, but generally within +/- 0.5% (one run faster, one slower)

One interesting part of this change is that running CBE invalidates
results of proc-point analysis. But instead of re-doing the whole
analysis, we can use the map that CBE creates for replacing/comparing
block labels (maps a redundant label to a useful one) to update the
results of proc-point analysis. This lowers the overhead compared to the
previous experiment in Trac #12915.

Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>

Test Plan

./validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
michalt created this revision.Feb 17 2018, 11:32 AM
simonmar accepted this revision.Feb 19 2018, 3:45 AM

LGTM, I'll take a 1% binary size reduction in exchange for <1% at compile time.

This revision is now accepted and ready to land.Feb 19 2018, 3:45 AM
bgamari added inline comments.Feb 20 2018, 12:21 PM
compiler/cmm/CmmPipeline.hs
129

I'll admit that I'm a bit confused by this. Above we treat the case of splitting_proc_points == True specially by computing the minimalProcPointSet of call_pps. However, here it looks like we undo that. I'm quite confused by this and it could at very least use a comment.

michalt updated this revision to Diff 15521.Feb 20 2018, 1:42 PM
  • Fix computing new_proc_points
compiler/cmm/CmmPipeline.hs
129

Oops, that was not intended. Fixed, thanks!

bgamari accepted this revision.Mar 26 2018, 2:18 PM

Looks good to me!

This revision was automatically updated to reflect the committed changes.
osa1 added a subscriber: osa1.Apr 8 2018, 4:10 AM

I'm getting some new slow validate errors which may be related with this patch:

ghc: panic! (the 'impossible' happened)
  (GHC version 8.5.20180407 for x86_64-unknown-linux):
    Each block should be reachable from only one ProcPoint