Opened 8 years ago

Last modified 9 months ago

#5302 new bug

Unused arguments in join points

Reported by: reinerp Owned by: simonpj
Priority: low Milestone:
Component: Compiler Version: 7.0.3
Keywords: JoinPoints, DemandAnalysis 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

Sometimes GHC produces join points with unused parameters. In the example attached, we get join points like the following (when compiled with -O2):

...
    $j1_XHI
      :: GHC.Prim.Int#
         -> GHC.Types.Int
         -> (# Unboxed.FingerTree
                 Unboxed.Size (Unboxed.Node Unboxed.Size b_ahY),
               Unboxed.Node Unboxed.Size b_ahY,
               Unboxed.FingerTree
                 Unboxed.Size (Unboxed.Node Unboxed.Size b_ahY) #)
    [LclId, Arity=2, Str=DmdType LL]
    $j1_XHI =
      \ (x2_XE8 :: GHC.Prim.Int#) _ ->
...

which is always called as follows:

...
    $j1_XHI x2_XE2 (GHC.Types.I# x2_XE2)
...

i.e. where the second argument is a boxed version of the first. GHC should remove the dead parameter from the join point, to avoid unnecessary boxing.

I get this Core with 7.0.3 and with 7.1.20110629.

I've attached a self-contained example, as small as I can make it. (Making it smaller lets GHC do more unfolding and the problem disappears.) These join points occur inside the 'Deep' case of '$wsplitTree'.

Attachments (5)

Unboxed.hs (3.7 KB) - added by reinerp 8 years ago.
T5302.hs (4.7 KB) - added by reinerp 8 years ago.
T5302_full.hs (12.8 KB) - added by reinerp 8 years ago.
Unboxed_late_dmd.dump-stg (170.0 KB) - added by osa1 20 months ago.
Unboxed.hs, late demand analysis
Unboxed_stock.dump-stg (182.7 KB) - added by osa1 20 months ago.
Unboxed.hs, default

Download all attachments as: .zip

Change History (23)

Changed 8 years ago by reinerp

Attachment: Unboxed.hs added

comment:1 Changed 8 years ago by igloo

Milestone: 7.4.1
Owner: set to simonpj

Thanks for the report.

comment:2 Changed 8 years ago by simonpj

I've just tried this, and can't reproduce it. I used

ghc-7.0.3 -c -O2 -ddump-simpl T5302.hs

Can you try that and check what's happening?

But yes, I can just about see how this might happen. It would be dealt with by the fix we're investigating for #4941, namely running the strictness analyser a second time near the end.

Simon

Changed 8 years ago by reinerp

Attachment: T5302.hs added

comment:3 Changed 8 years ago by reinerp

I can't reproduce either. I must have uploaded the wrong version of the file by mistake -- sorry.

The new file I just uploaded (T5302.hs) should work. With exactly the command you gave, look for the first join point inside the Deep case of wsplitTree. I get

                    $w$j_sIo
                      :: GHC.Prim.Int#
                         -> GHC.Prim.Int#
                         -> (# Unboxed.FingerTree
                                 Unboxed.Size (Unboxed.Node Unboxed.Size b_ak6),
                               Unboxed.Node Unboxed.Size b_ak6,
                               Unboxed.FingerTree
                                 Unboxed.Size (Unboxed.Node Unboxed.Size b_ak6) #)
                    [LclId, Arity=2, Str=DmdType LL]
                    $w$j_sIo =
                      \ (w2_sHj :: GHC.Prim.Int#) _ ->

So in this case the second argument is an Int# rather than an Int.

With

ghc-7.1.20110629 -c -O2 -ddump-simpl T5302.hs

I get

                $j_sJ4
                  :: GHC.Prim.Int#
                     -> GHC.Types.Int
                     -> (# Unboxed.FingerTree
                             Unboxed.Size (Unboxed.Node Unboxed.Size b_aiB),
                           Unboxed.Node Unboxed.Size b_aiB,
                           Unboxed.FingerTree
                             Unboxed.Size (Unboxed.Node Unboxed.Size b_aiB) #)
                [LclId, Arity=2, Str=DmdType LL]
                $j_sJ4 =
                  \ (x_aEm :: GHC.Prim.Int#) _ ->

in the same place. So in this case the second argument is an Int, as in my original report.

Changed 8 years ago by reinerp

Attachment: T5302_full.hs added

comment:4 Changed 8 years ago by reinerp

As it can be somewhat subtle exactly when the unused argument arises (in particular, removing too much code causes the unused argument to disappear), I also attached a "less minimal" example. So the file T5302_full.hs is a less cut-down version of the T5302.hs, in case that is helpful. Run

ghc-7.0.3 -c -ddump-simpl -O2 T5302_full.hs

and as before, look for the first join point inside the Deep case of $wsplitTree.

comment:5 Changed 8 years ago by igloo

Milestone: 7.4.17.6.1
Priority: normallow

comment:6 Changed 7 years ago by igloo

Milestone: 7.6.17.6.2

comment:7 Changed 7 years ago by nfrisby

As of 321941a, this test case (at less the _full.hs one) still generates the unused arguments. In fact, the join point under discussion also contains a join point with the same flaw. This happens with and without a SpecConstr.

In both cases, a late demand analysis removes both unnecessary arguments.

comment:8 Changed 5 years ago by thoughtpolice

Milestone: 7.6.27.10.1

Moving to 7.10.1.

comment:9 Changed 5 years ago by thoughtpolice

Milestone: 7.10.17.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:10 Changed 4 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:11 Changed 4 years ago by thomie

Milestone: 8.0.1

comment:12 Changed 3 years ago by simonpj

Keywords: JoinPoints added

comment:13 Changed 3 years ago by dfeuer

Milestone: 8.4.1

This does not appear to be fixed by the join point update. The second argument is still boxed.

comment:14 Changed 3 years ago by simonpj

The second argument is still boxed

Even with -O2 (which does a second demand/absence analysis pass? I think, though I have not dug into it, that the absence may only show up late.

Even if that's true, we should just check to ensure that there's no way it could have been caught in the first pass.

comment:15 Changed 3 years ago by simonpj

Still reproducible in HEAD.

  • The reason the late demand-analysis doesn't catch it is because we don't do w/w. (Reason: see Note [Final Demand Analyser run] in DmdAnal.)
  • The absent argument is actually a case binder, passed as a result of Note [Case binders and join points] in Simplify. But in this case it turns out that not only is the case-binder argument eventually unused, but (even worse) every juump to that join point looks like jump j x (I# x). So we box it, and then immediately discard it. Urk.

Just recording breadcrumbs for now. My thought: never pass the case binder to a join point; instead re-box it. It's much the same trade-off as in other places where we worry about re-boxing. And it would be a lot simpler; in particular, we can stop having unfoldings in lambda-binders, which is pretty dodgy anyway.

comment:16 Changed 21 months ago by bgamari

Milestone: 8.4.1

These tasks won't happen for 8.4 and have been sitting at low priority for quite some time. Consequently I am un-milestoning them. Don't let this stop you, the interested reader, from picking one up, however.

comment:17 Changed 20 months ago by osa1

Just tried with GHC HEAD. Late demand analysis removes unused join point argument.

let-no-escape {
  $j_s4wc [Occ=Once*!T[2], Dmd=<C(C(S)),1*C1(C1(U(U,U)))>]
    :: GHC.Prim.Int#
       -> GHC.Types.Int
       -> (# Unboxed.FingerTree
               Unboxed.Size (Unboxed.Node Unboxed.Size c_s44z),
             Unboxed.FingerTree
               Unboxed.Size (Unboxed.Node Unboxed.Size c_s44z) #)
  [LclId[JoinId(2)], Arity=2, Str=<S,U><L,A>, Unf=OtherCon []] = ...

After:

let-no-escape {
  $w$j_s4BK [InlPrag=NOUSERINLINE[0],
             Occ=Once*!T[1],
             Dmd=<C(S),1*C1(U(U,U))>]
    :: GHC.Prim.Int#
       -> (# Unboxed.FingerTree
               Unboxed.Size (Unboxed.Node Unboxed.Size c_s45o),
             Unboxed.FingerTree
               Unboxed.Size (Unboxed.Node Unboxed.Size c_s45o) #)
  [LclId[JoinId(1)], Arity=1, Str=<S,U>, Unf=OtherCon []] = ...

Changed 20 months ago by osa1

Attachment: Unboxed_late_dmd.dump-stg added

Unboxed.hs, late demand analysis

Changed 20 months ago by osa1

Attachment: Unboxed_stock.dump-stg added

Unboxed.hs, default

comment:18 Changed 9 months ago by simonpj

Keywords: DemandAnalysis added
Note: See TracTickets for help on using tickets.