Opened 10 years ago

Closed 10 years ago

#3772 closed bug (fixed)

Methods not inlined

Reported by: rl Owned by:
Priority: normal Milestone: 6.12 branch
Component: Compiler Version: 6.12.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case: simplCore/should_compile/T3772
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

This affects both 6.12 and the HEAD (albeit differently) but works fine with 6.10. It also works if everything is in one module. We are interested in the function U.foo; in particular, we want the call to deepSeq (resulting from inlining T.apply) to be inlined. This is what we get with 6.10 (compiling with -O2):

go_riA :: [GHC.Types.Double] -> ()
go_riA =
  \ (ds_agP :: [GHC.Types.Double]) ->
    case ds_agP of wild_agQ {
      [] -> GHC.Unit.();
      : y_agU ys_agV ->
        case y_agU of tpl2_ah1 { GHC.Types.D# ipv_ah3 -> go_riA ys_agV }
    }

U.foo :: GHC.Types.Int -> ()
U.foo =
  \ (n_afR :: GHC.Types.Int) ->
    case n_afR of w_Xhn { GHC.Types.I# ww_ahi ->
    go_riA (T.$wgen @ GHC.Types.Double T.$f2 ww_ahi)
    }

Everything has been nicely inlined. With 6.12, we get this:

U.foo [NEVER Nothing] :: GHC.Types.Int -> ()
U.foo =
  \ (n_adD :: GHC.Types.Int) ->
    case n_adD of _ { GHC.Types.I# ww_afv ->
    let {
      eta_sgc :: [GHC.Types.Double]
      eta_sgc = T.$wgen @ GHC.Types.Double T.$fCDouble ww_afv } in
    __inline_me (GHC.Base.foldr
                   @ GHC.Types.Double @ () (T.$fCDouble1 @ ()) GHC.Unit.() eta_sgc)
    }

The deepSeq call has been inlined but hasn't been optimised, I guess because GHC retained the __inline_me for some reason. Things are even worse with the HEAD:

U.foo :: GHC.Types.Int -> ()
U.foo =
  \ (n_aaO :: GHC.Types.Int) ->
    case n_aaO of _ { GHC.Types.I# ww_abR ->
    T.$fC[]1
      @ GHC.Types.Double
      T.$fCDouble
      @ ()
      (T.$w$cgen @ GHC.Types.Double T.$fCDouble ww_abR)
      GHC.Unit.()
    }

Here, deepSeq hasn't even been inlined. But let's add a dummy method to DeepSeq:

class DeepSeq a where
  deepSeq :: a -> b -> b

  dummy :: a
  dummy = undefined

Now, the HEAD actually inlines the call:

go_rcM :: [GHC.Types.Double] -> ()
go_rcM =
  \ (ds_acl :: [GHC.Types.Double]) ->
    case ds_acl of _ {
      [] -> GHC.Unit.();
      : y_acq ys_acr ->
        case y_acq of _ { GHC.Types.D# _ -> go_rcM ys_acr }
    }

U.foo :: GHC.Types.Int -> ()
U.foo =
  \ (n_aaQ :: GHC.Types.Int) ->
    case n_aaQ of _ { GHC.Types.I# ww_abI ->
    go_rcM (T.$w$cgen @ GHC.Types.Double T.$fCDouble ww_abI)
    }

Unfortunately, 6.12 still doesn't.

Attachments (3)

Tst.hs (196 bytes) - added by rl 10 years ago.
T.hs (501 bytes) - added by rl 10 years ago.
U.hs (110 bytes) - added by rl 10 years ago.

Download all attachments as: .zip

Change History (6)

Changed 10 years ago by rl

Attachment: Tst.hs added

Changed 10 years ago by rl

Attachment: T.hs added

Changed 10 years ago by rl

Attachment: U.hs added

comment:1 Changed 10 years ago by rl

Argh, please ignore Tst.hs, uploading files is too hard for me...

comment:2 Changed 10 years ago by igloo

Milestone: 6.12 branch

comment:3 Changed 10 years ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: simplCore/should_compile/T3772

Actually I fixed this one but forgot to update Trac:

Tue Jan  5 10:16:00 GMT 2010  simonpj@microsoft.com
  * Undo the fix for Trac #3772 and do it a new way
  
  The main idea is that I'm now treating a single-method dictionary very
  much like a multi-method dictionary.  In particular, it respond to
  exprIsConApp_maybe, even though newtypes aren't *really* proper
  constructors.
  
  See long comments with Note [Single-method classes] for why
  this slight hack is justified.

    M ./compiler/basicTypes/MkId.lhs -8 +4
    M ./compiler/typecheck/TcInstDcls.lhs -25 +67

Mon Dec 21 16:04:31 GMT 2009  simonpj@microsoft.com
  * Fix Trac #3772: dict funs for single-field classes
  
  This patch fixes a bug that meant that INLINE pragamas on
  a method of a single-field class didn't work properly.
  
  See Note [Single-method classes] in TcInstDcls, and Trac #3772

    M ./compiler/typecheck/TcInstDcls.lhs -34 +61

There's a test in the regression suite. Do not attempt to merge to 6.12; the code base is very different in this respect.

Simon

Note: See TracTickets for help on using tickets.