[commit: ghc] master: ghc-prim: Mark unpackCStringUtf8# and unpackNBytes# as NOINLINE (58bbb40)

git at git.haskell.org git at git.haskell.org
Wed Mar 30 19:20:27 UTC 2016


Repository : ssh://git@git.haskell.org/ghc

On branch  : master
Link       : http://ghc.haskell.org/trac/ghc/changeset/58bbb40ba23860df2ede1275493ef32ba69a2083/ghc

>---------------------------------------------------------------

commit 58bbb40ba23860df2ede1275493ef32ba69a2083
Author: Ben Gamari <bgamari.foss at gmail.com>
Date:   Wed Mar 30 10:29:02 2016 +0200

    ghc-prim: Mark unpackCStringUtf8# and unpackNBytes# as NOINLINE
    
    There is no benefit to be had from inlining this function and it may
    defeat rewrite rules if inlined early. See #11772..
    
    Test Plan: Validate, nofib
    
    Reviewers: simonpj, austin
    
    Subscribers: thomie
    
    Differential Revision: https://phabricator.haskell.org/D2057
    
    GHC Trac Issues: #11772


>---------------------------------------------------------------

58bbb40ba23860df2ede1275493ef32ba69a2083
 libraries/ghc-prim/GHC/CString.hs | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/libraries/ghc-prim/GHC/CString.hs b/libraries/ghc-prim/GHC/CString.hs
index 53d1ddd..19e6f75 100644
--- a/libraries/ghc-prim/GHC/CString.hs
+++ b/libraries/ghc-prim/GHC/CString.hs
@@ -24,7 +24,7 @@ import GHC.Types
 import GHC.Prim
 
 -----------------------------------------------------------------------------
--- Unpacking C strings}
+-- Unpacking C strings
 -----------------------------------------------------------------------------
 
 -- This code is needed for virtually all programs, since it's used for
@@ -34,11 +34,33 @@ import GHC.Prim
 -- stuff uses Strings in the representation, so to give representations for
 -- ghc-prim types we need unpackCString#
 
+{-
+Note [Inlining unpackCString#]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+There's really no point in ever inlining things like unpackCString# as the loop
+doesn't specialise in an interesting way and we can't deforest the list
+constructors (we'd want to use unpackFoldrCString# for this). Moreover, it's
+pretty small, so there's a danger that it'll be inlined at every literal, which
+is a waste.
+
+Moreover, inlining early may interfere with a variety of rules that are supposed
+to match unpackCString#,
+
+ * BuiltInRules in PrelRules.hs; e.g.
+       eqString (unpackCString# (Lit s1)) (unpackCString# (Lit s2)
+          = s1 == s2
+
+ * unpacking rules; e.g. in GHC.Base,
+       unpackCString# a
+          = build (unpackFoldrCString# a)
+
+ * stream fusion rules; e.g. in the `text` library,
+       unstream (S.map safe (S.streamList (GHC.unpackCString# a)))
+          = unpackCString# a
+-}
+
 unpackCString# :: Addr# -> [Char]
 {-# NOINLINE unpackCString# #-}
-    -- There's really no point in inlining this, ever, as the loop doesn't
-    -- specialise in an interesting But it's pretty small, so there's a danger
-    -- that it'll be inlined at every literal, which is a waste
 unpackCString# addr
   = unpack 0#
   where
@@ -64,7 +86,7 @@ unpackFoldrCString# :: Addr# -> (Char  -> a -> a) -> a -> a
 
 -- Usually the unpack-list rule turns unpackFoldrCString# into unpackCString#
 
--- It also has a BuiltInRule in PrelRules.lhs:
+-- It also has a BuiltInRule in PrelRules.hs:
 --      unpackFoldrCString# "foo" c (unpackFoldrCString# "baz" c n)
 --        =  unpackFoldrCString# "foobaz" c n
 
@@ -85,7 +107,10 @@ unpackFoldrCString# addr f z
       where
         !ch = indexCharOffAddr# addr nh
 
+-- There's really no point in inlining this for the same reasons as
+-- unpackCString. See Note [Inlining unpackCString#] above for details.
 unpackCStringUtf8# :: Addr# -> [Char]
+{-# NOINLINE unpackCStringUtf8# #-}
 unpackCStringUtf8# addr
   = unpack 0#
   where
@@ -110,7 +135,10 @@ unpackCStringUtf8# addr
       where
         !ch = indexCharOffAddr# addr nh
 
+-- There's really no point in inlining this for the same reasons as
+-- unpackCString. See Note [Inlining unpackCString#] above for details.
 unpackNBytes# :: Addr# -> Int# -> [Char]
+{-# NOINLINE unpackNBytes# #-}
 unpackNBytes# _addr 0#   = []
 unpackNBytes#  addr len# = unpack [] (len# -# 1#)
     where



More information about the ghc-commits mailing list