[GHC] #16197: Strictness is not preserved under -O1

GHC ghc-devs at haskell.org
Thu Jan 17 09:33:09 UTC 2019


#16197: Strictness is not preserved under -O1
-------------------------------------+-------------------------------------
        Reporter:  alang9            |                Owner:  (none)
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.4.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
 Type of failure:  Incorrect result  |  Unknown/Multiple
  at runtime                         |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 I can reproduce this with 8.6, but not with HEAD, so I think perhaps it is
 fixed.  Hooray.

 Indeed we have made some recent changes to the handling of strictness on
 data constructors:
 {{{
 commit d77501cd5b9060e38acd50e11e0c5aae89d75b65
 Author: Simon Peyton Jones <simonpj at microsoft.com>
 Date:   Wed Dec 12 17:22:07 2018 +0000

     Improvements to demand analysis

     This patch collects a few improvements triggered by Trac #15696,
     and fixing Trac #16029

     * Stop making toCleanDmd behave specially for unlifted types.
       This special case was the cause of stupid behaviour in Trac
       #16029.  And to my joy I discovered the let/app invariant
       rendered it unnecessary.  (Maybe the special case pre-dated
       the let/app invariant.)

       Result: less special-case handling in the compiler, and
       better perf for the compiled code.

     * In WwLib.mkWWstr_one, treat seqDmd like U(AAA).  It was not
       being so treated before, which again led to stupid code.

     * Update and improve Notes

     There are .stderr test wibbles because we get slightly different
     strictness signatures for an argumment of unlifted type:
         <L,U> rather than <S,U>        for Int#
         <S,U> rather than <S(S),U(U)>  for Int

 commit d549c081f19925dd0e4c70d45bded0497c649d49
 Author: Ben Gamari <bgamari.foss at gmail.com>
 Date:   Tue Dec 11 13:34:47 2018 -0500

     dmdAnal: Move handling of datacon strictness to mkWWstr_one

     Previously datacon strictness was accounted for when we demand
 analysed a case
     analysis. However, this results in pessimistic demands in some cases.
 For
     instance, consider the program (from T10482)

         data family Bar a
         data instance Bar (a, b) = BarPair !(Bar a) !(Bar b)
         newtype instance Bar Int = Bar Int

         foo :: Bar ((Int, Int), Int) -> Int -> Int
         foo f k =
           case f of
             BarPair x y -> case burble of
                               True -> case x of
                                         BarPair p q -> ...
                               False -> ...

     We really should be able to assume that `p` is already evaluated since
 it came
     from a strict field of BarPair.

     However, as written the demand analyser can not conclude this since we
 may end
     up in the False branch of the case on `burble` (which places no demand
 on `x`).
     By accounting for the data con strictness later, applied to the demand
 of the
     RHS, we get the strict demand signature we want.

     See Note [Add demands for strict constructors] for a more
 comprehensive
     discussion.


 commit c5b477c29127d8375b3f23d37f877278b52547f6
 Author: Ömer Sinan Ağacan <omeragacan at gmail.com>
 Date:   Mon Oct 15 13:48:53 2018 -0400

     Fix cardinality change of fields in addDataConStrictness
 }}}
 And indeed the strictness of `f` is now
 {{{
  Str=<S,1*U(U)><S,1*U(U)>m,
 }}}
 I had previously regarded these patches as clean-ups, improving the
 generated code, rather than actual bug fixes.  So your example is very
 interesting: it shows that the "clean-up" actually fixed a bug.

 Ben/Omer: could you add comment:4 as a regression test?

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


More information about the ghc-tickets mailing list