Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#8091 closed bug (fixed)

retry# lacks strictness information

Reported by: parcs Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.6.3
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):


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.

Change History (6)

comment:1 Changed 2 years ago by parcs

  • Status changed from new to patch

The patch (validated):

  • compiler/prelude/primops.txt.pp

    diff --git a/compiler/prelude/primops.txt.pp b/compiler/prelude/primops.txt.pp
    index 3d3518b..e12c1a5 100644
    a b primop AtomicallyOp "atomically#" GenPrimOp 
    16141614primop  RetryOp "retry#" GenPrimOp
    16151615   State# RealWorld -> (# State# RealWorld, a #)
    16161616   with
     1617   strictness  = { \ _arity -> mkStrictSig (mkTopDmdType [topDmd] botRes) }
    16171618   out_of_line = True
    16181619   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.

comment:2 follow-up: Changed 2 years ago 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.


comment:3 in reply to: ↑ 2 Changed 2 years ago 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 <S,1*U(U)><L,1*U(U)><L,U> either way..

(It would be incorrect, I think, to have f strict in y since that could potentially elide the side-effect-having call to readTVar.)

I found an example of a function whose strictness does change due to this patch (courtesy of the comments for RaiseIOOp in primops.txt.pp):

g :: Int -> Int -> STM Int
g x y | x>0       = retry
      | y>0       = return 1
      | otherwise = return 2

This function is now strict in y, where it originally wasn't. It looks like this change will only have strictness implications in fairly contrived functions where retry is used in such a way that the transaction could not possibly make any progress; and in that case behaving as if retry returns bottom makes a good amount of sense.

Version 0, edited 2 years ago by parcs (next)

comment:4 Changed 2 years ago by parcs

To be clear, the form of dead code elimination that this change enables is


case retry# s1 of
    (# s2, a #) -> ...inaccessible..

with simply

retry# s1

Such Core can appear after performing case-of-case on code like

s <- readTVar v
when (s > 0) retry

where everything after the when statement might get inlined under the scrutinization of retry.

comment:5 Changed 2 years ago by thoughtpolice

  • Resolution set to fixed
  • Status changed from patch to closed

Thanks Patrick. Committed in

commit e2b72cadce3ae799a74981f9957b40bb201f1ba1
Author: Austin Seipp <[email protected]>
Date:   Sat Aug 10 22:39:15 2013 -0500

    Mark retry# as returning bottom.
    This change helps the simplifier eliminate unreachable code, since
    retry# technically doesn't return.
    This closes ticket #8091.
    Authored-by: Patrick Palka <[email protected]>
    Signed-off-by: Austin Seipp <[email protected]>

comment:6 Changed 2 years ago 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

Note: See TracTickets for help on using tickets.