Opened 5 years ago

Closed 5 years ago

#4495 closed bug (fixed)

GHC fails to inline methods of single-method classes

Reported by: diatchki Owned by: igloo
Priority: normal Milestone: 7.0.2
Component: Compiler Version: 7.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

Consider the following module:

module A where

class C a where
  f :: a

g :: C a => (a,a)
g = (f,f)

GHC has a nice optimization where the dictionaries for classes with just a single method (and no super-classes) are represented with a newtype. This means that, essentially, the "dictionary" is the method itself, and the code that "extracts" a method from a dictionary is just the identity function. For this reason, we can completely eliminate the extraction code by always inlining the method.

This works as expected in GHC 6.12.2 but something seems to have changed in the current HEAD, so that the "extraction code" (i.e., the method "f") does not seem to get eliminated:

[GHC 6.12.2]

f :: forall a. (C a) => a
GblId[ClassOp]

f = \ (@ a) (tpl [ALWAYS Once Nothing] :: C a) -> tpl `cast` ...

g :: forall a. (C a) => (a, a)
GblId

g = __inline_me (\ (@ a) (d :: C a) -> (d `cast` ..., d `cast` ...)

[HEAD as of Nov 14 2010]

f [InlPrag=[NEVER]] :: forall a. C a => a
[GblId[ClassOp],
 Arity=1,
 Unf=Unf{Src=<vanilla>, TopLvl=True, Arity=1, Value=True,
         ConLike=True, Cheap=True, Expandable=True,
         Guidance=ALWAYS_IF(unsat_ok=True,boring_ok=True)},
 RULES: Built in rule for f: "Class op f"]
f = \ (@ a) (tpl [Occ=Once] :: C a) -> tpl `cast` ...


g :: forall a. C a => (a, a)
[GblId,
 Arity=1,
 Unf=Unf{Src=InlineStable, TopLvl=True, Arity=1, Value=True,
         ConLike=True, Cheap=True, Expandable=True,
         Guidance=ALWAYS_IF(unsat_ok=True,boring_ok=False)
         Tmpl= \ (@ a) (d :: C a) -> (f @ a d, f @ a d)}]

g = \ (@ a) (d :: C a) -> (f @ a d, f @ a d)

Change History (3)

comment:1 Changed 5 years ago by igloo

  • Milestone set to 7.0.2

Thanks for the report.

comment:2 Changed 5 years ago by simonpj

  • Owner set to igloo

Yes, I've been meaning to get back to this. This patch fixes it

Tue Dec 21 16:19:11 GMT 2010  [email protected]
  * Single-method classes are implemented with a newtype
  
  This patch changes things so that such classes rely on the coercion
  mechanism for inlining (since the constructor is really just a cast)
  rather than on the dfun mechanism, therby removing some needless
  runtime indirections.

    M ./compiler/basicTypes/MkId.lhs -7 +10
    M ./compiler/typecheck/TcInstDcls.lhs -23 +24

It's not clear to me how to add a regression test. Ian, if you can, please do.

If this gets pushed into the stable branch then fine, but no special need to do so.

Simon

comment:3 Changed 5 years ago by igloo

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

I can't see anything better than some fragile matching on -ddump-simpl output, so I haven't added a test.

Note: See TracTickets for help on using tickets.