Opened 15 months ago

Closed 3 months ago

#7619 closed feature request (fixed)

Make worker-wrapper unbox data families

Reported by: akio Owned by: nomeata
Priority: normal Milestone: 7.8.1
Component: Compiler Version: 7.7
Keywords: type family Cc: jwlato@…, hackage.haskell.org@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty: Unknown
Test Case: perf/should_run/T7619 Blocked By:
Blocking: Related Tickets:

Description

I noticed that the worker-wrapper optimization doesn't unbox arguments whose type is a data family instance. For example in this module:

{-# LANGUAGE TypeFamilies #-}
{-# LANGUAGE BangPatterns #-}
module Foo where
data family Foo a

data instance Foo Int = FooInt Int Int

foo :: Foo Int -> Int
foo (FooInt a b) = loop a b
    where
        loop 0 y = length $ replicate y b
        loop x !y = loop (mod y x) x

foo1 :: (Int, Int) -> Int
foo1 (a, b) = loop a b
    where
        loop 0 y = length $ replicate y b
        loop x !y = loop (mod y x) x

foo and foo1 both get worker-wrapper applied, with worker functions of the following types:

$wfoo :: Foo Int -> Int#
$wfoo1 :: Int# -> Int# -> Int#

It would be nice if $wfoo could get the same type as $wfoo1.

This issue happened in real life with unboxed vectors from the vector package, resulting in a lot of boxing with unboxed vector constructors immediately followed by unboxing.

Change History (9)

comment:1 Changed 15 months ago by simonpj

  • Difficulty set to Unknown

Yes absolutely. The trick here is that

  • To get from (Foo Int) to the underlying data type involves generating a coercion.
  • So if foo wants to unbox its argument, the worker/wrapper code must include coercions.
  • Where do those coercions come from? You need the type-family instance environment, and even then it's a bit dodgy to be looking them up in the optimiser.
  • Or maybe we can gather them from the code itself during analysis, and keep them in the demand signature.

Same thing happens in CPR I think

comment:2 Changed 14 months ago by jwlato

  • Cc jwlato@… added

comment:3 Changed 13 months ago by liyang

  • Cc hackage.haskell.org@… added

comment:4 Changed 12 months ago by igloo

  • Milestone set to 7.8.1
  • Owner set to simonpj

comment:5 Changed 3 months ago by simonpj

  • Owner changed from simonpj to nomeata

comment:6 Changed 3 months ago by nomeata

This is actually simple to implement (see [a1ddb71/ghc] on branch wip/T7619), but rather ugly due to the additional passing of the FamInstEnv? everywhere. That’s why I did not push to master yet.

There are types FamInstEnvs and FamInstEnv, but it is not well-explained, only

type FamInstEnvs = (FamInstEnv, FamInstEnv)
     -- External package inst-env, Home-package inst-env

Note that topNormaliseType_maybe takes a FamInstEnvs, and I can get a FamInstEnv from the ModGuts and from ExternalPackageState.

If I combine these as follows, am I doing the right thing?

doPassDFU :: (DynFlags -> FamInstEnvs -> UniqSupply -> CoreProgram -> CoreProgram) -> ModGuts -> CoreM ModGuts
doPassDFU do_pass guts = do
    dflags <- getDynFlags
    us     <- getUniqueSupplyM
    hsc_env <- getHscEnv
    eps <- liftIO $ hscEPS hsc_env
    let fam_envs = (eps_fam_inst_env eps, mg_fam_inst_env guts)
    doPass (do_pass dflags fam_envs us) guts

And why do we need to make that distinction in the first place – it seems that topNormaliseType_maybe does not treat them differently.

comment:7 Changed 3 months ago by simonpj

The package inst-env get extended by a kind of side effect when we lad new modules. The home-package inst env doesn't. You could combine them into one and pass one instead of a pair, but not much would be gained, and there'd be some extra computation to build the combined env that might only be interrogated once.

Nothing deep

comment:8 Changed 3 months ago by Joachim Breitner <mail@…>

In 2bb19fad1d809dda37011f442b0fd561aea045b6/ghc:

Make worker-wrapper unbox data families

by passing the FamInstEnvs all the way down. This closes #7619.

comment:9 Changed 3 months ago by nomeata

  • Resolution set to fixed
  • Status changed from new to closed
  • Test Case set to perf/should_run/T7619
Note: See TracTickets for help on using tickets.