[GHC] #5129: "evaluate" optimized away

[GHC]

#5129: "evaluate" optimized away
-------------------------------------+-------------------------------------
        Reporter:  dons              |                Owner:  (none)
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:  8.4.2
       Component:  Compiler          |              Version:  7.0.3
      Resolution:                    |             Keywords:  seq, evaluate
Operating System:  Unknown/Multiple  |         Architecture:
 Type of failure:  Incorrect result  |  Unknown/Multiple
  at runtime                         |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13930            |  Differential Rev(s):  Phab:D615,
       Wiki Page:                    |  Phab:D4514
-------------------------------------+-------------------------------------

Comment (by simonpj):

 > One way to fix this if we want to keep seq# as effect-free is avoiding
 inlining evaluate with {-# NOINLINE evaluate #-}.

 I'm very unhappy with this.  It just sweeps the problem under the rug.
 What if the ''user'' wrote
 {{{
 ..(case (seq# (throwIfNegative (I# -1#)) s) of <alts>)...
 }}}
 We don't want to discard the case.

 Moreover we shouldn't.  The "plain-seq" transformation in `Simplify.hs`
 looks like
 {{{
   | is_plain_seq
   , exprOkForSideEffects scrut
   = ...
 }}}
 Well, should `exprOkForSideEffects` return False (as it does) for `seq#
 (throwIfNegative (I# -1#)) s`?  The comments in `CoreUtils` say
 {{{
 -- Precisely, exprOkForSpeculation returns @True@ iff:
 --  a) The expression guarantees to terminate,
 --  b) soon,
 --  c) without causing a write side effect (e.g. writing a mutable
 variable)
 --  d) without throwing a Haskell exception
 --  e) without risking an unchecked runtime exception (array out of
 bounds,
 --     divide by zero)
 --
 -- For @exprOkForSideEffects@ the list is the same, but omitting (e).
 }}}
 So clearly `exprOkForSideEffects` should return False!  That's the real
 bug!

 Why does it return True?  Because in `arg_ok` in `CoreUtils.app_ok` we see
 {{{
     arg_ok (Anon ty) arg        -- A term argument
        | isUnliftedType ty = expr_ok primop_ok arg
        | otherwise         = True  -- See Note [Primops with lifted
 arguments]
 }}}
 Uh oh!  It never even looks at the argument `throwIfNegative minus_one`!

 I think that `seq#` is exception to the reasoning in `Note [Primops with
 lifted arguments]`, which says that a primop doesn't evaluate a lifted
 argument.  In effect `seq#` does.

 So in the `PrimOpId` case of `app_ok` I think we want to add
 {{{
         | SeqOp <- op
         -> all (expr_ok primop_ok) args
 }}}
 And sure enough that fixes it.

 I'll make a patch.

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