[GHC] #9390: Inlining prevents evaluation of ignored parts of unboxed tuples

[GHC]

#9390: Inlining prevents evaluation of ignored parts of unboxed tuples
-------------------------------------+-------------------------------------
              Reporter:  snoyberg    |            Owner:
                  Type:  bug         |           Status:  merge
              Priority:  normal      |        Milestone:  7.8.4
             Component:  Compiler    |          Version:  7.8.3
            Resolution:              |         Keywords:
      Operating System:  Linux       |     Architecture:  x86_64 (amd64)
       Type of failure:  Incorrect   |       Difficulty:  Unknown
  result at runtime                  |       Blocked By:
             Test Case:              |  Related Tickets:
  simplCore/should_run/T9390         |
              Blocking:              |
Differential Revisions:              |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 Oh yes thanks!

 '''First point'''.  I really don't like the reasoning though.  How would
 you explain to a client of `unsafePerformIO` what the specification is?
 Perhaps "the effects of the `unsafePerformIO` are all performed before the
 result is evaluated".  But that is not true if the effects somehow depend
 strictly on the value concerned.  If it's hard to specify, we should be
 cautious about relying on it.

 After all, the example
 {{{
       unsafeDupablePerformIO (\s -> let r = f x in
                              case writeIORef v r s of (# s1, _ #) ->
                              (# s1, r #) )
 }}}
 really '''is''' strict in `r`.  If you evaluate it, since `writeIORef`
 returns `r`, it's clear that `r` will be evaluated.  Saying "I want to
 write the reference before evaluating `r` is very delicate!".  Moreover
 you can achieve the same effect, where it matters, with less magic.  Here
 is the current code from `libraries/base/tests/Memo2.lhs`:
 {{{
 memo' f ref weak_ref = \k -> unsafePerformIO $
    do { ...blah...
        ; case lkp of
             Just res -> do { putMVar ref (size,table); return res }
             Nothing  -> do { let res = f k
                            ; ...blah...
                            ; return res }
 }}}
 Now if we  make the argument function given to `unsafePerformIO` return a
 1-tuple thus, we are good:
 {{{
 memo' f ref weak_ref = \k -> case do_effects k of {# result #) -> result
   where
     do_effects k = unsafePerformIO $
        do { ...blah...
           ; case lkp of
                Just res -> do { putMVar ref (size,table); return (# res
 #) }
                Nothing  -> do { let res = f k
                               ; ...blah...
                               ; return {# res #) }
 }}}
 Mind you (thinking aloud here), I suppose that this does rely on the
 strictness analyser not being super-clever.  If `unsafePerformIO`'s
 signature was clever enough to say "in demand d, I call my function
 argument and evaluate the second component the result with demand d" then
 we'd be back in the same boat as before.

 Another, perhaps more robust alternative, would be to say:
 {{{
 memo' f ref weak_ref = \k -> unsafePerformIO $
    do { ...blah...
        ; case lkp of
             Just res -> do { putMVar ref (size,table); return res }
             Nothing  -> do { let res = f k
                            ; ...blah...
                            ; return (lazy res) }
 }}}
 Here the `lazy` means that the `res` binding is not strict.

 Anyway the point it that this subtle stuff should be visible in the
 caller, for the rare moments when it is needed, rather than hidden in
 `unsafePerformIO`.

 All that said, there is is, and I suppose it may break things in rather
 subtle ways if we remove it.  So perhaps we should leave specifically
 leave it un-documented!

 '''Second point'''.  I think it would be clearer as
 {{{
 unsafeDupablePerformIO (IO m) = case m realWorld# of (# _, r #) -> lazy r
 }}}
 That is, make the laziness wrap the 'r' part only, which is the important
 bit here.  I tried this:
 {{{
 {-# NOINLINE u1 #-}
 u1  :: IO a -> a
 u1 (IO m) = lazy (case m realWorld# of (# _, r #) -> r)

 {-# NOINLINE u2 #-}
 u2  :: IO a -> a
 u2 (IO m) = case m realWorld# of (# _, r #) -> lazy r

 {-# NOINLINE u3 #-}
 u3  :: IO a -> a
 u3 (IO m) = case m realWorld# of (# _, r #) -> r
 }}}
 and got this in the interface file:
 {{{
   u1 :: GHC.Types.IO a -> a
     {- Arity: 1, HasNoCafRefs, Strictness: <L,1*C1(U(A,1*U))>,
        Inline: NOINLINE -}
 c9e24d21e66094c44ef0629d326e9a33
   u2 :: GHC.Types.IO a -> a
     {- Arity: 1, HasNoCafRefs, Strictness: <C(S),1*C1(U(A,U))>,
        Inline: NOINLINE -}
 83ded03238c0647421262c926910e195
   u3 :: GHC.Types.IO a -> a
     {- Arity: 1, HasNoCafRefs, Strictness: <C(S(LS)),1*C1(U(A,1*U))>,
        Inline: NOINLINE -}
 }}}
 The strictness signatures say this:
  * `u1` (the status quo) is lazy in its first argument (a function)
  * `u2` (which I advocate) calls the function and evaluates the result to
 HNF
  * `u3` (the buggy version with no `lazy`) calls the function and
 evaluates the second component of the pair it returns.

 So `u2` looks fine. I might try making that change and seeing if there are
 any regressions.  Unless you can think of any reason not to.

 Simon

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