[GHC] #8091: retry# lacks strictness information

#8091: retry# lacks strictness information ------------------------------------+------------------------------------- Reporter: parcs | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Keywords: | Operating System: Unknown/Multiple Architecture: Unknown/Multiple | Type of failure: None/Unknown Difficulty: Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | ------------------------------------+------------------------------------- The `retry#` primop should be marked as returning bottom since it never actually returns anything as far as the simplifier is concerned. This change would help the simplifier eliminate unreachable code inside STM transactions. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information -------------------------------------+------------------------------------ Reporter: parcs | Owner: Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Changes (by parcs): * status: new => patch Comment: The patch (validated): {{{ #!diff diff --git a/compiler/prelude/primops.txt.pp b/compiler/prelude/primops.txt.pp index 3d3518b..e12c1a5 100644 --- a/compiler/prelude/primops.txt.pp +++ b/compiler/prelude/primops.txt.pp @@ -1614,6 +1614,7 @@ primop AtomicallyOp "atomically#" GenPrimOp primop RetryOp "retry#" GenPrimOp State# RealWorld -> (# State# RealWorld, a #) with + strictness = { \ _arity -> mkStrictSig (mkTopDmdType [topDmd] botRes) } out_of_line = True has_side_effects = True }}} The unfoldings in Control.Concurrent.STM.TSem and .TQueue are improved as a result of this change due to the aforementioned dead code elimination. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information -------------------------------------+------------------------------------ Reporter: parcs | Owner: Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by simonpj): I like the idea, but it does have strictness consequences too. For example: {{{ f :: TVar Int -> Int -> STM Int f r y = do { v <- readTVar r ; if v>0 then retry else if y>0 then return 0 else return y } }}} I have not checked but I think this will not be strict in y because the `then` branch doesn't evaluate y. With your change it'll become strict in y, just as it would if you replace `retry` with `throw exn`. I think that's probably fine, and no one will even notice; just saying. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information
-------------------------------------+------------------------------------
Reporter: parcs | Owner:
Type: bug | Status: patch
Priority: normal | Milestone:
Component: Compiler | Version: 7.6.3
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture: Unknown/Multiple
Type of failure: None/Unknown | Difficulty: Unknown
Test Case: | Blocked By:
Blocking: | Related Tickets:
-------------------------------------+------------------------------------
Comment (by parcs):
Ah, subtle..
But in this particular example , `f` seems to be lazy in `y` even when
`retry` is replaced with `throw Overflow`: its strictness signature is
`

#8091: retry# lacks strictness information -------------------------------------+------------------------------------ Reporter: parcs | Owner: Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by parcs): To be clear, the form of dead code elimination that this change enables is replacing {{{ #!haskell case retry# s1 of (# s2, a #) -> ...inaccessible.. }}} with simply {{{ #!haskell retry# s1 }}} Such Core can appear after performing case-of-case on code like {{{ #!haskell s <- readTVar v when (s > 0) retry ... }}} where everything after the `when` statement might get inlined under the scrutinization of `retry`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information
-------------------------------------+------------------------------------
Reporter: parcs | Owner:
Type: bug | Status: closed
Priority: normal | Milestone:
Component: Compiler | Version: 7.6.3
Resolution: fixed | Keywords:
Operating System: Unknown/Multiple | Architecture: Unknown/Multiple
Type of failure: None/Unknown | Difficulty: Unknown
Test Case: | Blocked By:
Blocking: | Related Tickets:
-------------------------------------+------------------------------------
Changes (by thoughtpolice):
* status: patch => closed
* resolution: => fixed
Comment:
Thanks Patrick. Committed in
{{{
commit e2b72cadce3ae799a74981f9957b40bb201f1ba1
Author: Austin Seipp

#8091: retry# lacks strictness information -------------------------------------+------------------------------------ Reporter: parcs | Owner: Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by simonpj): I wonder if we might have a regression test here... Perhaps a `-ddump- simpl` and check that a function in the `...inaccessible...` part no longer appears? Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information
-------------------------------------+-------------------------------------
Reporter: parcs | Owner: (none)
Type: bug | Status: closed
Priority: normal | Milestone:
Component: Compiler | Version: 7.6.3
Resolution: fixed | Keywords:
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s):
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by bgamari):
#13916 leads me to believe this is strictness signature is too strong.
Consider this program,
{{{#!hs
loop :: [TMVar a] -> STM a -> STM a
loop [] m = m
loop (x:xs) m = loop xs (m `orElse` takeTMVar x)
doIt :: [TMVar Int] -> STM Int
doIt xs = atomically $ loop xs retry
}}}
GHC will currently give `loop` a strictness signature of
`

#8091: retry# lacks strictness information -------------------------------------+------------------------------------- Reporter: parcs | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: closed => new * resolution: fixed => -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information -------------------------------------+------------------------------------- Reporter: parcs | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by bgamari): Actually, comment:7 isn't quite right: The fact that `loop` doesn't diverge should be accounted for by the `ExnStr` of the first argument of `orElse`. I would have thought that the `ExnStr` should propagate to the second argument of `loop` during demand analysis but it doesn't. That seems to be the issue. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information -------------------------------------+------------------------------------- Reporter: parcs | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: new => closed * resolution: => fixed Comment: It turns out that the root cause was #13977, which was fixed by Phab:D3756. Closing. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information -------------------------------------+------------------------------------- Reporter: parcs | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Do we have a regression test? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information -------------------------------------+------------------------------------- Reporter: parcs | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by bgamari): Do you mean for #13977? I intend on adding the case from #13916 to the testsuite. It's imperfect but better than nothing. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/8091#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#8091: retry# lacks strictness information
-------------------------------------+-------------------------------------
Reporter: parcs | Owner: (none)
Type: bug | Status: closed
Priority: normal | Milestone:
Component: Compiler | Version: 7.6.3
Resolution: fixed | Keywords:
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s):
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari
participants (1)
-
GHC