Opened 2 years ago

Last modified 9 months ago

#13692 new bug

Constructors and such should be able to move around seq# sometimes

Reported by: dfeuer Owned by:
Priority: normal Milestone: 8.10.1
Component: Compiler Version: 8.2.1-rc2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

This code

module SeqCon where

import Control.Exception (evaluate)

blah :: Int -> IO Int
blah x = (+2) <$> evaluate (x + 3)

compiled with -O2 -ddump-prep -dsuppress-coercions produces the following (and -ddump-stg doesn't change much of note):

SeqCon.blah1
  :: GHC.Types.Int
     -> GHC.Prim.State# GHC.Prim.RealWorld
     -> (# GHC.Prim.State# GHC.Prim.RealWorld, GHC.Types.Int #)
[GblId,
 Arity=2,
 Caf=NoCafRefs,
 Str=<L,1*U(U)><S,U>,
 Unf=OtherCon []]
SeqCon.blah1
  = \ (x_s1cz [Occ=Once!] :: GHC.Types.Int)
      (s_s1cA [Occ=Once] :: GHC.Prim.State# GHC.Prim.RealWorld) ->
      let {
        sat_s1cE [Occ=Once] :: GHC.Types.Int
        [LclId]
        sat_s1cE
          = case x_s1cz of { GHC.Types.I# x1_s1cC [Occ=Once] ->
            case GHC.Prim.+# x1_s1cC 3# of sat_s1cD { __DEFAULT ->
            GHC.Types.I# sat_s1cD
            }
            } } in
      case GHC.Prim.seq#
             @ GHC.Types.Int @ GHC.Prim.RealWorld sat_s1cE s_s1cA
      of
      { (# ipv_s1cG [Occ=Once], ipv1_s1cH [Occ=Once!] #) ->
      let {
        sat_s1cL [Occ=Once] :: GHC.Types.Int
        [LclId]
        sat_s1cL
          = case ipv1_s1cH of { GHC.Types.I# x1_s1cJ [Occ=Once] ->
            case GHC.Prim.+# x1_s1cJ 2# of sat_s1cK { __DEFAULT ->
            GHC.Types.I# sat_s1cK
            }
            } } in
      (# ipv_s1cG, sat_s1cL #)
      }

-- RHS size: {terms: 5, types: 3, coercions: 5, joins: 0/0}
SeqCon.blah :: GHC.Types.Int -> GHC.Types.IO GHC.Types.Int
[GblId,
 Arity=2,
 Caf=NoCafRefs,
 Str=<L,1*U(U)><S,U>,
 Unf=OtherCon []]
SeqCon.blah
  = (\ (eta_B2 [Occ=Once] :: GHC.Types.Int)
       (eta_B1 [Occ=Once] :: GHC.Prim.State# GHC.Prim.RealWorld) ->
       SeqCon.blah1 eta_B2 eta_B1)
    `cast` <Co:5>

This builds a closure to build an Int and passes it to seq#. That seems a bit wasteful, since we don't actually need the Int box. I think what we'd really like to end up with is something like

blah1 = \ (x :: Int) (s :: State# RealWorld) ->
  case seq# x s of { (# s', x' #) ->
  case x' of { I# x# -> (# s', I# (x# +# 5#) #) }}

Here's one vague idea: when we analyse x+3 (the argument to seq#) under a strict demand, we see it is strict in x. So we can transform seq# (x + 3) s into

case seq# x s of
  (# s', x' #) -> seq# (x' + 3) s'

We know that x' is in WHNF, so we should (I think) be able to see that x' + 3 isn't bottom, so we can use

case seq# x s of {(# s', x' #) ->
case x' + 3 of {!res -> s', res}}

Side note: the redundant eta-expansion in blah is a bit surprising. blah1 already has arity 2, so I'd have expected blah to just coerce it directly.

Change History (4)

comment:1 Changed 2 years ago by simonpj

Good point. Is it a performance-critical point? Or just an observation.

seq# seems poorly documented in GHC's source code. It should really be called evaluate# for a start. I think it's important that it is not strict because I think the idea is that no earlier code should evaluate its argument.

I guess this transformation would be sound

   seq (case e of p -> r) s
-->
   seq e s of (# s', v #) ->
   case v of p ->
   seq r s'

which I think is more or less what you are suggesting. I have no idea if it'd be worth it; but I suppose it could be if it removed allocation from some inner loop.

comment:2 Changed 20 months ago by bgamari

Milestone: 8.4.18.6.1

This ticket won't be resolved in 8.4; remilestoning for 8.6. Do holler if you are affected by this or would otherwise like to work on it.

comment:3 Changed 15 months ago by bgamari

Milestone: 8.6.18.8.1

These will not be addressed in GHC 8.6.

comment:4 Changed 9 months ago by osa1

Milestone: 8.8.18.10.1

Bumping milestones of low-priority tickets.

Note: See TracTickets for help on using tickets.