[GHC] #13380: raiseIO# result looks wrong
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: Other Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- `primops.txt.pp` includes the following note:
`raiseIO#` needs to be a primop, because exceptions in the `IO` monad must be ''precise'' - we don't want the strictness analyser turning one kind of bottom into another, as it is allowed to do in pure code.
But we ''do'' want to know that it returns bottom after being applied to two arguments, so that this function is strict in `y` {{{#!hs f x y | x>0 = raiseIO blah | y>0 = return 1 | otherwise = return 2 }}}
I believe the "But we do" portion is, unfortunately, entirely wrong. From a user perspective, it is extremely strange to replace a precise exception with an imprecise one. That is, I strongly believe the above code should be considered ''lazy'' in `y`. The function `f` calculates an `IO` action; the value of `y` is only necessary to calculate that action if `x` is non- positive. We can, conservatively, say for certain that the second component of the result of `raiseIO#` (i.e., the "return value") is bottom. We can also say, given {{{#!hs case raiseIO# e s of (# s', r #) -> q }}} that `q` is dead (we could replace it by `(# s', undefined #)`). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): I've set an optimistic milestone for this, but we will probably need to wait for 8.4 to dig a bit deeper into how demand analysis interacts with `raise#`, `raiseIO#`, `retry#`, `catch#`, `catchSTM#`, and `catchRetry#`. There seem to be subtle problems in multiple places. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by rwbarton): I think it's just a bit of imprecise wording. It's right that `raiseIO#` returns bottom after being applied to ''two'' arguments, as: {{{#!hs raiseIO# :: a -> State# RealWorld -> (# State# RealWorld, b #) }}} But the function in the example is strict in `y` only after being applied to a third argument, i.e., executed. You can try this: {{{#!hs import Control.Exception f :: Int -> Int -> IO Int f x y | x > 0 = throwIO StackOverflow | y > 0 = return 1 | otherwise = return 2 u :: Int u = -1 {-# NOINLINE u #-} main :: IO () main = f u undefined `seq` return () }}} and it's not optimized to bottom. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): @rwbarton, no, I understood that just fine. The trouble is that I don't think it's valid to analyze that as bottom when applied to three arguments, because then we lose the perfectly well-defined ''action'' of throwing a precise exception. Instead of performing that action, we can end up throwing an imprecise exception evaluating something we have no business evaluating. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Full test case: {{{#!hs import Control.Exception {-# NOINLINE f #-} f :: Int -> Int -> IO Int f x y | x>0 = throwIO (userError "What") | y>0 = return 1 | otherwise = return 2 main = f 2 undefined >>= print }}} I would expect this to throw the "What" exception, but when it's compiled with optimization I get the "undefined" exception. If I had written `throw (userError "What")`, that would be acceptable, because we don't make any guarantees about which imprecise exception in the imprecise exception set we actually throw. But here we are re-ordering evaluation in such a fashion as to throw an imprecise exception instead of a precise one, and I don't think that's legit. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by dfeuer): * keywords: => Exceptions -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong
-------------------------------------+-------------------------------------
Reporter: dfeuer | Owner: (none)
Type: bug | Status: new
Priority: normal | Milestone: 8.2.1
Component: Compiler | Version: 8.1
Resolution: | Keywords: Exceptions
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: Other | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s):
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: new => closed * resolution: => fixed Comment: This is now fixed, although as acknowledged in the commit message we may be able to do better in the future. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Wow. Did we measure the perf impact, if any? What's the nofib change? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Replying to [comment:8 simonpj]:
Wow. We said "let's see how bad it is". Did we measure the perf impact, if any? What's the nofib change?
It wasn't absolutely horrifying. /da4687f63ffe5a6162e3d7856aa53de048dd0f42 I'm benchmarking a change right now to manually strictify a few library functions that were likely affected; we'll see how that goes. BTW, did you see my note on the insufficiency of the note on the I/O hack in the demand analyzer? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): OK: please put the nfib results here. And check that we still get the improvements reported in #11222 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by simonpj): * priority: normal => high * status: closed => new * resolution: fixed => Comment: Re-opening to ensure that we do check those perf numbers. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * owner: (none) => dfeuer Comment: David, could you provide some more quantitative performance evaluation? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Oh, it seems I pasted the link above wrong. I meant to paste [https://perf.haskell.org/ghc/#revision/7b087aeba45a7a70a5553ef4c116ee6766042... a link to the perf dashboard for that commit]. There were two significant changes in nofib runtimes. {{{ test old change new nofib/time/k-nucleotide 5.338 + 4.95% 5.602 seconds nofib/time/lambda 1.095 - 3.2% 1.06 seconds }}} There was also an unfortunate increase in nofib sizes that I didn't notice before (gipeda did not flag it for some reason). The sizes went up by a little over 1% across the board. Ouch. I'm guessing this is because we lost the dead code elimination we previously had for {{{#!hs case raiseIO# e s of ... }}} Could we recover that by using some sort of special rule somewhere that rewrites {{{#!hs case raiseIO# e s of p -> ... }}} to `raiseIO# e s`? I could give it a shot if someone tells me where to put it. There were a few test suite allocation regressions, the largest of which were {{{ tests/alloc/T10547 33561264 + 1.36% 34019072 bytes tests/alloc/T5837 51419384 + 1.27% 52074608 bytes tests/alloc/T12425 134735944 + 0.52% 135441336 bytes tests/alloc/T10858 267909584 + 0.49% 269215800 bytes tests/alloc/T13035 93847840 + 0.44% 94264440 bytes tests/alloc/T6048 109461504 + 0.38% 109873560 bytes tests/alloc/T13056 420744984 + 0.37% 422320424 bytes }}} I believe the right way to fix these is to go through any code in `base` that uses `throwIO` and make sure we manually force everything we want to. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.4.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.2.1 => 8.4.1 Comment: Given the adverse performance effects of comment:6 we have reverted it on `ghc-8.2`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.4.1 => Comment: It's unlikely that this will be fixed for 8.4. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13380: raiseIO# result looks wrong -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: Exceptions Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Other | Test Case: Blocked By: | Blocking: Related Tickets: #14998 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by sgraf): * related: => #14998 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13380#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC