Fix the removal of unnecessary stack checks
ClosedPublic

Authored by jscholl on Feb 3 2016, 10:30 AM.

Details

Summary

The module CmmLayoutStack removes stack checks if a function does not
use stack space. However, it can only recognize checks of the form
Sp < SpLim. However, these checks get sometimes rewritten to
Sp >= SpLim (with both branches swapped), so we better recognize these
checks too.

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.
jscholl updated this revision to Diff 6550.Feb 3 2016, 10:30 AM
jscholl retitled this revision from to Fix the removal of unnecessary stack checks.
jscholl updated this object.
jscholl edited the test plan for this revision. (Show Details)
jscholl set the repository for this revision to rGHC Glasgow Haskell Compiler.
jscholl updated the Trac tickets for this revision.
simonpj requested changes to this revision.Feb 3 2016, 10:37 AM
simonpj added a reviewer: simonpj.
simonpj added a subscriber: simonpj.

Nice improvement, thank you. See my notes.

Does it make any difference in practice?

compiler/cmm/CmmLayoutStack.hs
873

Very good! Can you adapt Note [Always false stack checks] to show an example of this too?

900–901

What is the semantics of CmmCondBranch on an arg that is not 1 or 0? Surely this function could give a firm answer for any CmmLit (CmmInt n) argument?

This revision now requires changes to proceed.Feb 3 2016, 10:37 AM
jscholl updated this revision to Diff 6552.Feb 3 2016, 12:12 PM
jscholl edited edge metadata.
  • Adapted Note [Always false stack checks] to include information about inverted comparisons and removed a little bit redundancy.

The difference is honestly quite small in practice. I think nofib contained maybe between 20 and 40 cases where the the stack check was incorrectly not dropped. I could not measure a performance difference, but binary size decreased by a few bytes (as one would expect). The comparison is only inverted (and thus the stack check was not removed) if the body of the function contained a loop (the block in case of sufficient stack space had more than one predecessor). But then the loopification optimisation already makes sure that we do not repeat the stack check in this case, so it was only executed once.

simonpj accepted this revision.Feb 3 2016, 3:12 PM
simonpj edited edge metadata.

Thanks. Fine modulo comments

compiler/cmm/CmmLayoutStack.hs
882

Say

-- (a)   if ((Sp + x) - y < SpLim) then .. else ..
-- or
-- (b)   if ((Sp + x) - y >= SpLim) then ... else ...
--
-- when we know that x>=y
-- Since (SpLim <= Sp), since the stack grows downward,
-- we can see that (a) yields False, and (b) yields True

and adjust the following text. I just want to have both cases shown in the Note

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