Re: [GHC] #11731: Simplifier: Inlining trivial let can lose sharing

30 Mar 2016

      #11731: Simplifier: Inlining trivial let can lose sharing
-------------------------------------+-------------------------------------
        Reporter:  nomeata           |                Owner:
            Type:  bug               |               Status:  patch
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):  Phab:D2064
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by nomeata):
...
A small change to exprIsTrivial fixes this
And almost validates, with one exception. There is an increase of
 allocations in T5549 by 10%. The -ddump-simpl differs only slightly: In
 the code of

 {{{#!hs
     aux (_,0) _ = ([],0)
     aux _ (_,0) = ([],0)
     aux (a@(ha:as),la) (b@(hb:bs), lb)
       | ha == hb  = let (s,n) = aux (as,la-1) (bs,lb-1) in (ha : s, n+1)
       | otherwise =
         let (sa,na) = aux (as,la-1) (b,lb) -- ←
             (sb,nb) = aux (a,la) (bs,lb-1) in
         if na > nb then (sa,na) else (sb,nb)
 }}}
 the marked recursive call to `aux` re-assembles the second tuple argument,
 instead of passing the tuple that was passed to `aux`; this is CSE
 failing.

 CSE uses `exprIsTrivial` in `tryForCSE`. From my reading of the code, in
 the context of
 {{{
 case foo of foo'
   { (x [Dmd=], y) ->
      ... case x of z { ... (x,y) ... }
 }}}
 it would previously *not* CSE the inner mention of `x` to `z`, as `x` is
 trivial, and then successfully replace `(x,y)` with `foo'`. But after my
 change, `x` is no longer trivial, so it does CSE it to `z`, and then leave
 `(z,y)` as it is.

 Digging further into it, I found some code smell in CSE. Note `Case
 binders 2` says that with trivial scrutinees, we want a mapping `case
 binder -> scrutinee` instead of the usual `scrutinee -> case binder`.
 Nevertheless, `cseExpr` in the case for `Case` adds the unwanted mapping
 via `cseRhs`. Only the check for `exprIsTrivial` in `tryForCSE` prevents
 this entry from being used. Fixing this code smell (I’ve just updated
 Phab:D2064 in a moment) fixes this particular regression.

--
Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11731#comment:19
GHC http://www.haskell.org/ghc/
The Glasgow Haskell Compiler