
#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