#7797 closed bug (fixed)

re-enable the defun RULE from a SPECIALISE instance pragma

Reported by: nfrisby Owned by:
Priority: normal Milestone: 7.8.1
Component: Compiler Version: 7.6.2
Keywords: Cc: nfrisby, johan.tibell@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty: Unknown
Test Case: perf/should_run/T7797 Blocked By:
Blocking: Related Tickets:

Description

As of commit 51d89a55c3, SPECIALISE instance pragmas do not result in a RULE for the dictionary function.

For example, consider this Eq instance for List.

module M where

data List a = Nil | Cons a (List a)

instance (Eq a) => Eq (List a) where
    {-# SPECIALISE instance Eq (List Char) #-}
    Nil     == Nil     = True
    (Cons x xs) == (Cons y ys) = x == y && xs == ys
    _xs    == _ys    = False

With ghc-7.4.2, we get:

==================== Tidy Core rules ====================
"SPEC $c/=" [ALWAYS]
    forall ($dEq :: GHC.Classes.Eq GHC.Types.Char).
      M.$fEqList_$c/=1 @ GHC.Types.Char $dEq
      = M.$fEqList_$c/=
"SPEC $c==" [ALWAYS]
    forall ($dEq :: GHC.Classes.Eq GHC.Types.Char).
      M.$fEqList_$c==1 @ GHC.Types.Char $dEq
      = M.$fEqList_$c==
"SPEC M.$fEqList" [ALWAYS]
    forall ($dEq :: GHC.Classes.Eq GHC.Types.Char).
      M.$fEqList @ GHC.Types.Char $dEq
      = M.$fEqList_$fEqList

Note the last rule: it specializes the normal defun M.$fEqList when applied at type Char.

With anything after 7.4.2 -- eg if you download the binary sources for 7.4.2 and make the change at line 779 from that commit --- you instead get this:

==================== Tidy Core rules ====================
"SPEC $c/=" [ALWAYS]
    forall ($dEq :: GHC.Classes.Eq GHC.Types.Char).
      M.$fEqList_$c/=1 @ GHC.Types.Char $dEq
      = M.$fEqList_$c/=
"SPEC $c==" [ALWAYS]
    forall ($dEq :: GHC.Classes.Eq GHC.Types.Char).
      M.$fEqList_$c==1 @ GHC.Types.Char $dEq
      = M.$fEqList_$c==

(Actually, after some patch the /= RULE disappears too, but I don't know which/why.)

If the dictionary is used at the relevant type in the same module, the specializer will automatically create the omitted rule. That will not, however, currently happen across module boundaries.

In my contrived example, omitting this defun specialization increases runtime by a factor of 2 at -O1.

{-# LANGUAGE ExistentialQuantification #-}

module Main where

import M

data Box = forall a. Eq a => Box a a

box = Box (go 10000000) (go 10000000) where
  go :: Int -> List Char
  go 0 = Nil
  go n = Cons 'c' $ go (n - 1)
{-# NOINLINE box #-}

main = print $ case box of
  Box l r -> l == r

(-O2 squashes the runtime difference; I haven't investigated in detail.)

Change History (5)

comment:1 Changed 13 months ago by nfrisby

I'm talked to SPJ about this and the concern is that the defun specialisation may interfere with the specialization of method selections. We think it'll just require some careful thinking and a bit of refactoring of defun inlinings.

comment:2 Changed 13 months ago by nfrisby

  • Cc nfrisby added

comment:3 Changed 12 months ago by igloo

  • Difficulty set to Unknown
  • Milestone set to 7.8.1

comment:4 Changed 11 months ago by simonpj@…

commit 1ed0409010afeaa318676e351b833aea659bf93a

Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Thu May 30 12:08:39 2013 +0100

    Make 'SPECIALISE instance' work again
    
    This is a long-standing regression (Trac #7797), which meant that in
    particular the Eq [Char] instance does not get specialised.
    (The *methods* do, but the dictionary itself doesn't.)  So when you
    call a function
         f :: Eq a => blah
    on a string type (ie a=[Char]), 7.6 passes a dictionary of un-specialised
    methods.
    
    This only matters when calling an overloaded function from a
    specialised context, but that does matter in some programs.  I
    remember (though I cannot find the details) that Nick Frisby discovered
    this to be the source of some pretty solid performanc regresisons.
    
    Anyway it works now. The key change is that a DFunUnfolding now takes
    a form that is both simpler than before (the DFunArg type is eliminated)
    and more general:
    
    data Unfolding
      = ...
      | DFunUnfolding {     -- The Unfolding of a DFunId
        			-- See Note [DFun unfoldings]
          		  	--     df = /\a1..am. \d1..dn. MkD t1 .. tk
                            --                                 (op1 a1..am d1..dn)
         		      	--     	    	      	       	   (op2 a1..am d1..dn)
            df_bndrs :: [Var],      -- The bound variables [a1..m],[d1..dn]
            df_con   :: DataCon,    -- The dictionary data constructor (never a newtype datacon)
            df_args  :: [CoreExpr]  -- Args of the data con: types, superclasses and methods,
        }                           -- in positional order
    
    That in turn allowed me to re-enable the DFunUnfolding specialisation in
    DsBinds.  Lots of details here in TcInstDcls:
    	  Note [SPECIALISE instance pragmas]
    
    I also did some refactoring, in particular to pass the InScopeSet to
    exprIsConApp_maybe (which in turn means it has to go to a RuleFun).
    
    NB: Interface file format has changed!

 compiler/basicTypes/MkId.lhs                   |    8 +-
 compiler/coreSyn/CoreFVs.lhs                   |   33 +++--
 compiler/coreSyn/CoreSubst.lhs                 |   65 ++++-----
 compiler/coreSyn/CoreSyn.lhs                   |   39 ++----
 compiler/coreSyn/CoreTidy.lhs                  |    7 +-
 compiler/coreSyn/CoreUnfold.lhs                |   11 +-
 compiler/coreSyn/PprCore.lhs                   |   10 +-
 compiler/deSugar/DsBinds.lhs                   |   39 +++---
 compiler/iface/BinIface.hs                     |   15 +--
 compiler/iface/IfaceSyn.lhs                    |    9 +-
 compiler/iface/MkIface.lhs                     |    4 +-
 compiler/iface/TcIface.lhs                     |   10 +-
 compiler/main/TidyPgm.lhs                      |    3 +-
 compiler/prelude/PrelRules.lhs                 |  180 +++++++-----------------
 compiler/simplCore/OccurAnal.lhs               |    2 +-
 compiler/simplCore/SimplUtils.lhs              |   12 +-
 compiler/simplCore/Simplify.lhs                |   12 +-
 compiler/specialise/Rules.lhs                  |   76 +++++-----
 compiler/specialise/Specialise.lhs             |   20 +++-
 compiler/typecheck/TcInstDcls.lhs              |  120 +++++++++-------
 compiler/vectorise/Vectorise/Generic/PADict.hs |   24 ++--
 21 files changed, 316 insertions(+), 383 deletions(-)

comment:5 Changed 11 months ago by simonpj

  • Cc johan.tibell@… added
  • Resolution set to fixed
  • Status changed from new to closed
  • Test Case set to perf/should_run/T7797

Finally done! I hope this will improve some old regressions.

Simon

Note: See TracTickets for help on using tickets.