[GHC] #13077: Worker/wrapper can break the let-app invariant

[GHC]

#13077: Worker/wrapper can break the let-app invariant
-------------------------------------+-------------------------------------
        Reporter:  simonpj           |                Owner:
            Type:  bug               |               Status:  closed
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.0.1
      Resolution:  fixed             |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
                                     |  stranal/should_compile/T13077,
                                     |  T13077a
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by simonpj):

 * status:  new => closed
 * testcase:   => stranal/should_compile/T13077, T13077a
 * resolution:   => fixed


Comment:

 Fixed by
 {{{
 commit 596dece7866006d699969f775fd97bd306aad85b
 Author: Simon Peyton Jones <simonpj at microsoft.com>
 Date:   Fri Jan 13 08:56:53 2017 +0000

     Record evaluated-ness on workers and wrappers

     Summary:
     This patch is a refinement of the original commit (which
     was reverted):

       commit 6b976eb89fe72827f226506d16d3721ba4e28bab
       Date:   Fri Jan 13 08:56:53 2017 +0000
           Record evaluated-ness on workers and wrappers

     In Trac #13027, comment:20, I noticed that wrappers created after
     demand analysis weren't recording the evaluated-ness of strict
     constructor arguments.  In the ticket that led to a (debatable)
     Lint error but in general the more we know about evaluated-ness
     the better we can optimise.

     This commit adds that info
       * both in the worker (on args)
       * and in the wrapper (on CPR result patterns).
     See Note [Record evaluated-ness in worker/wrapper] in WwLib

     On the way I defined Id.setCaseBndrEvald, and used it to shorten
     the code in a few other places

     Then I added test T13077a to test the CPR aspect of this patch,
     but I found that Lint failed!

     Reason: simpleOptExpr was discarding evaluated-ness info on
     lambda binders because zapFragileIdInfo was discarding an
     Unfolding of (OtherCon _).  But actually that's a robust
     unfolding; there is no need to discard it. To fix this:

     * zapFragileIdInfo only zaps fragile unfoldings

     * Replace isClosedUnfolding with isFragileUnfolding (the latter
       is just the negation of the former, but the nomenclature is
       more consistent).  Better documentation too
            Note [Fragile unfoldings]

     * And Simplify.simplLamBndr can now look at isFragileUnfolding
       to decide whether to use the longer route of simplUnfolding.

     For some reason perf/compiler/T9233 improves in compile-time
     allocation by 10%.  Hooray

     Nofib: essentially no change:

 --------------------------------------------------------------------------------
             Program           Size    Allocs   Runtime   Elapsed  TotalMem
 --------------------------------------------------------------------------------
           cacheprof          +0.0%     -0.3%     +0.9%     +0.4%     +0.0%
 --------------------------------------------------------------------------------
                 Min          +0.0%     -0.3%     -2.4%     -2.4%     +0.0%
                 Max          +0.0%     +0.0%     +9.8%    +11.4%     +2.4%
      Geometric Mean          +0.0%     -0.0%     +1.1%     +1.0%     +0.0%
 }}}

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/13077#comment:2>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler