Opened 5 years ago

Closed 3 years ago

#3526 closed bug (fixed)

Inliner behaviour with instances is confusing

Reported by: bos Owned by: simonpj
Priority: low Milestone: 7.2.1
Component: Compiler Version: 6.10.4
Keywords: Cc: michal.terepeta@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description

I have a fairly simple typeclass:

class Variate a where
    uniform :: Gen s -> ST s a

With instances like this:

instance Variate Int8 where
    uniform = f where f = uniform1 fromIntegral
                      {-# INLINE f #-}

uniform1 :: (Word32 -> a) -> Gen s -> ST s a
uniform1 f gen = do
  i <- uniformWord32 gen
  return $! f i
{-# INLINE uniform1 #-}

Notice the peculiar form of the instance definition. I had to write the above instead of the more intuitive form:

instance Variate Int8 where
    uniform = uniform1 fromIntegral
    {-# INLINE uniform #-}

With the more obvious form, GHC's inliner didn't fire at all for this, and I was unable to tell why. It was Duncan who suggested the more convoluted instance above. The result is about a 3x performance difference sometimes, so this has a noticeable effect.

I'm not completely sure that this is a bug, but I don't know how to describe why one form works and the other doesn't, so the compiler's behaviour is surprising to me.

Change History (10)

comment:1 Changed 5 years ago by igloo

  • Difficulty set to Unknown
  • Milestone set to 6.12 branch

Thanks for the report.

comment:2 Changed 5 years ago by simonpj

  • Owner set to simonpj

I'll look at this when I've got the new INLINE patch done.

Simon

comment:3 Changed 4 years ago by simonpj

Bryan

You don't actually give a reproducible test case, so I can't test this. Nevertheless I believe I have fixed the problem. You should be able to write the straightforward instance, and it should work. Can you try, and let me know?

Simon

comment:4 Changed 4 years ago by bos

Thanks, Simon; I'll take a look this weekend and close this bug out if all is now well.

comment:5 Changed 4 years ago by igloo

  • Milestone changed from 6.12 branch to 6.12.3

comment:6 Changed 4 years ago by igloo

  • Milestone changed from 6.12.3 to 6.14.1
  • Priority changed from normal to low

comment:7 Changed 3 years ago by igloo

  • Milestone changed from 7.0.1 to 7.0.2

comment:8 Changed 3 years ago by igloo

  • Milestone changed from 7.0.2 to 7.2.1

comment:9 Changed 3 years ago by michalt

  • Cc michal.terepeta@… added
  • Status changed from new to infoneeded
  • Type of failure set to Runtime performance bug

comment:10 Changed 3 years ago by bos

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

As far as I can tell, this is happy in the world of GHC 7.

Note: See TracTickets for help on using tickets.