Opened 5 years ago

Last modified 4 months ago

#3990 new bug

UNPACK doesn't unbox data families

Reported by: rl Owned by:
Priority: low Milestone: 7.12.1
Component: Compiler Version: 7.0.3
Keywords: Cc:…, reiner.pope@…, tomberek@…, tkn.akio@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:


Here is an example:

data family Complex a
data instance Complex Double = CD {-# UNPACK #-} !Double
                                  {-# UNPACK #-} !Double

data T = T {-# UNPACK #-} !(Complex Double)

We would like T to just store two Double# but that doesn't happen. Contrast this with

data Complex_Double = CD {-# UNPACK #-} !Double
                         {-# UNPACK #-} !Double

data T = T {-# UNPACK #-} !Complex_Double

Change History (17)

comment:1 Changed 5 years ago by rl

FWIW, this isn't getting unpacked, either:

data Complex a where
  CD :: {-# UNPACK #-} !Double -> {-# UNPACK #-} !Double -> Complex Double

data T = T {-# UNPACK #-} !(Complex Double)

comment:2 Changed 5 years ago by igloo

  • Milestone set to 6.14.1

comment:3 Changed 4 years ago by igloo

  • Milestone changed from 7.0.1 to 7.0.2

comment:4 Changed 4 years ago by igloo

  • Milestone changed from 7.0.2 to 7.2.1

comment:5 Changed 4 years ago by liyang

  • Cc… added
  • Version changed from 6.13 to 7.0.3

In 7.0.3, at least there's a warning:

    Warning: Ignoring unusable UNPACK pragma on the
             first argument of `T'

comment:6 Changed 4 years ago by reinerp

  • Cc reiner.pope@… added

comment:7 Changed 4 years ago by tomberek

  • Cc tomberek@… added

comment:8 Changed 4 years ago by igloo

  • Milestone changed from 7.2.1 to 7.4.1

comment:9 Changed 3 years ago by simonpj

I don't think I have anything on the go that'll fix this. There's no fundamental difficulty; we just need to normalise types when dealing with UNPACK pragmas.

The really tricky bit about UNPACK, that always given me a brain haemorrhage is avoiding falling into an infinite loop

	data T = MkT Int {-# UNPACK #-} T

Clearly we can't UNPACK here. This is easy enough for self-recursive types. But we can UNPACK for *some* recursive types:

	data T = MkT {-# UNPACK #-} S
	data S = MkS Int (Foo S)
	data Foo a = MkFoo Bool a

See? S is recursive but it can be unpacked.

If we are doing normalisation of type families as well, the question of what depends on what (and hence what is recursive) becomes much more obscure. So its more than a matter of attaching a flag to each type constructor saying "I'm recursive" or otherwise.

Maybe we could do something more dynamic, such as "have I seen this TyCon before when expanding this UNPACK?".
Currently this is not easy, because UNPACK processing deals with just one "layer" at a time. For example:

  data T = MkT {-# UNPACK #-} S
  data S = MkS {-# UNPACK #-} R
  data R = MkR {-# UNPACK #} Int

When dealing the the data declaration for T we look at MkS, but no deeper... we simply delegate to the MkS constructor to do its own upacking. But now I think about, if the UNPACK processing looked deeply rather than shallowly,

  • we wouldn't be in danger of accidentally depending recursively on T (leading to a black hole in the compiler)
  • we might be able handle polymorphic unpack

What I mean by "polymorphic unpack" is this:

  data Poly a = MkP Bool {-# UNPACK #-} a 
  data Mango = MkMango {-# UNPACK #-} (Poly Int)

Now a value of type Poly t would be represented using two pointer fields, as usual (ie the UNPACK would have no direct effect on Poly). But a Mango value would be represented thus:

  data Mango = MkMangoRep Bool Int#

  MkMango :: Poly Int -> MangoRep
  MkMango (MkP b (I# i)) = MkMangoRep b i

  -- Pattern match      (MkMango p -> rhs)
  -- is transformed to  (MkMangoRep b i -> let p = MkP b (I# i) in rhs

comment:10 Changed 3 years ago by igloo

  • Milestone changed from 7.4.1 to 7.6.1
  • Priority changed from normal to low

comment:11 Changed 3 years ago by akio

  • Cc tkn.akio@… added

comment:12 Changed 3 years ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:13 Changed 2 years ago by simonpj@…

commit 1ee1cd4194555e498d05bfc391b7b0e635d11e29

Author: Simon Peyton Jones <[email protected]>
Date:   Sun Dec 23 15:38:48 2012 +0000

    Make {-# UNPACK #-} work for type/data family invocations
    This fixes most of Trac #3990.  Consider
      data family D a
      data instance D Double = CD Int Int
      data T = T {-# UNPACK #-} !(D Double)
    Then we want the (D Double unpacked).
    To do this we need to construct a suitable coercion, and it's much
    safer to record that coercion in the interface file, lest the in-scope
    instances differ somehow.  That in turn means elaborating the HsBang
    type to include a coercion.
    To do that I moved HsBang from BasicTypes to DataCon, which caused
    quite a few minor knock-on changes.
    Interface-file format has changed!
    Still to do: need to do knot-tying to allow instances to take effect
    within the same module.

 compiler/basicTypes/BasicTypes.lhs             |   51 ------
 compiler/basicTypes/DataCon.lhs                |   52 ++++++-
 compiler/basicTypes/MkId.lhs                   |  230 ++++++++++++++----------
 compiler/hsSyn/HsTypes.lhs                     |    1 +
 compiler/iface/BinIface.hs                     |   20 +--
 compiler/iface/BuildTyCl.lhs                   |   12 +-
 compiler/iface/IfaceSyn.lhs                    |   13 +-
 compiler/iface/MkIface.lhs                     |    8 +-
 compiler/iface/TcIface.lhs                     |   13 ++-
 compiler/main/PprTyThing.hs                    |    3 +-
 compiler/prelude/TysWiredIn.lhs                |    2 +-
 compiler/simplCore/Simplify.lhs                |    4 +-
 compiler/stranal/DmdAnal.lhs                   |    4 +-
 compiler/typecheck/TcRnDriver.lhs              |    2 +-
 compiler/typecheck/TcSplice.lhs                |   10 +-
 compiler/typecheck/TcTyClsDecls.lhs            |    6 +-
 compiler/vectorise/Vectorise/Generic/PData.hs  |    9 +-
 compiler/vectorise/Vectorise/Type/TyConDecl.hs |    4 +-
 18 files changed, 253 insertions(+), 191 deletions(-)

comment:14 Changed 2 years ago by simonpj

  • difficulty set to Unknown

See #7647 re polymorphic unpacking.

comment:15 Changed 16 months ago by sjoerd_visscher

What about closed data families? GHC could check that every instance is unpackable. Then this would work:

data Nat = Z | S Nat

data family Vec (size :: Nat) where
  Vec Z = Nil
  Vec (S n) = Cons {-# UNPACK #-} !Double {-# UNPACK #-} !(Vec n)

comment:16 Changed 10 months ago by thoughtpolice

  • Milestone changed from 7.6.2 to 7.10.1

Moving to 7.10.1.

comment:17 Changed 4 months ago by thoughtpolice

  • Milestone changed from 7.10.1 to 7.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.

Note: See TracTickets for help on using tickets.