Skip to content

Commit 655b5d6

Browse files
authored
Merge pull request #485 from haskell/indices-are-strict
Hint to GHC that indices are to be used strictly
2 parents 1ca259e + e13cb00 commit 655b5d6

File tree

2 files changed

+47
-24
lines changed

2 files changed

+47
-24
lines changed

vector/src/Data/Vector/Generic.hs

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -228,19 +228,35 @@ null = Bundle.null . stream
228228
-- Indexing
229229
-- --------
230230

231+
-- NOTE: [Strict indexing]
232+
-- ~~~~~~~~~~~~~~~~~~~~~~~
233+
--
234+
-- Why index parameters are strict in indexing ((!), (!?)) functions
235+
-- and functions for accessing elements in mutable arrays ('unsafeRead',
236+
-- 'unsafeWrite', 'unsafeModify'), and slice functions?
237+
--
238+
-- These function call class methods ('basicUnsafeIndexM',
239+
-- 'basicUnsafeRead', etc) and, unless (!) was already specialised to
240+
-- a specific v, GHC has no clue that i is most certainly to be used
241+
-- eagerly. Bang before i hints this vital for optimizer information.
242+
243+
231244
infixl 9 !
232245
-- | O(1) Indexing.
233246
(!) :: (HasCallStack, Vector v a) => v a -> Int -> a
234247
{-# INLINE_FUSED (!) #-}
235-
(!) v i = checkIndex Bounds i (length v) $ unBox (basicUnsafeIndexM v i)
248+
-- See NOTE: [Strict indexing]
249+
(!) v !i = checkIndex Bounds i (length v) $ unBox (basicUnsafeIndexM v i)
236250

237251
infixl 9 !?
238252
-- | O(1) Safe indexing.
239253
(!?) :: Vector v a => v a -> Int -> Maybe a
240254
{-# INLINE_FUSED (!?) #-}
255+
-- See NOTE: [Strict indexing]
241256
-- Use basicUnsafeIndexM @Box to perform the indexing eagerly.
242-
v !? i | i `inRange` length v = case basicUnsafeIndexM v i of Box a -> Just a
243-
| otherwise = Nothing
257+
v !? (!i)
258+
| i `inRange` length v = case basicUnsafeIndexM v i of Box a -> Just a
259+
| otherwise = Nothing
244260

245261

246262
-- | /O(1)/ First element.
@@ -256,7 +272,8 @@ last v = v ! (length v - 1)
256272
-- | /O(1)/ Unsafe indexing without bounds checking.
257273
unsafeIndex :: Vector v a => v a -> Int -> a
258274
{-# INLINE_FUSED unsafeIndex #-}
259-
unsafeIndex v i = checkIndex Unsafe i (length v) $ unBox (basicUnsafeIndexM v i)
275+
-- See NOTE: [Strict indexing]
276+
unsafeIndex v !i = checkIndex Unsafe i (length v) $ unBox (basicUnsafeIndexM v i)
260277

261278
-- | /O(1)/ First element, without checking if the vector is empty.
262279
unsafeHead :: Vector v a => v a -> a
@@ -316,7 +333,7 @@ unsafeLast v = unsafeIndex v (length v - 1)
316333
-- element) is evaluated eagerly.
317334
indexM :: (HasCallStack, Vector v a, Monad m) => v a -> Int -> m a
318335
{-# INLINE_FUSED indexM #-}
319-
indexM v i = checkIndex Bounds i (length v) $ liftBox $ basicUnsafeIndexM v i
336+
indexM v !i = checkIndex Bounds i (length v) $ liftBox $ basicUnsafeIndexM v i
320337

321338
-- | /O(1)/ First element of a vector in a monad. See 'indexM' for an
322339
-- explanation of why this is useful.
@@ -334,7 +351,7 @@ lastM v = indexM v (length v - 1)
334351
-- explanation of why this is useful.
335352
unsafeIndexM :: (Vector v a, Monad m) => v a -> Int -> m a
336353
{-# INLINE_FUSED unsafeIndexM #-}
337-
unsafeIndexM v i = checkIndex Unsafe i (length v)
354+
unsafeIndexM v !i = checkIndex Unsafe i (length v)
338355
$ liftBox
339356
$ basicUnsafeIndexM v i
340357

@@ -452,7 +469,8 @@ unsafeSlice :: Vector v a => Int -- ^ @i@ starting index
452469
-> v a
453470
-> v a
454471
{-# INLINE_FUSED unsafeSlice #-}
455-
unsafeSlice i n v = checkSlice Unsafe i n (length v) $ basicUnsafeSlice i n v
472+
-- See NOTE: [Strict indexing]
473+
unsafeSlice !i !n v = checkSlice Unsafe i n (length v) $ basicUnsafeSlice i n v
456474

457475
-- | /O(1)/ Yield all but the last element without copying. The vector may not
458476
-- be empty, but this is not checked.
@@ -993,7 +1011,7 @@ backpermute v is = seq v
9931011
-- NOTE: we do it this way to avoid triggering LiberateCase on n in
9941012
-- polymorphic code
9951013
index :: HasCallStack => Int -> Box a
996-
index i = checkIndex Bounds i n $ basicUnsafeIndexM v i
1014+
index !i = checkIndex Bounds i n $ basicUnsafeIndexM v i
9971015

9981016
-- | Same as 'backpermute', but without bounds checking.
9991017
unsafeBackpermute :: (Vector v a, Vector v Int) => v a -> v Int -> v a
@@ -1010,7 +1028,7 @@ unsafeBackpermute v is = seq v
10101028
{-# INLINE index #-}
10111029
-- NOTE: we do it this way to avoid triggering LiberateCase on n in
10121030
-- polymorphic code
1013-
index i = checkIndex Unsafe i n $ basicUnsafeIndexM v i
1031+
index !i = checkIndex Unsafe i n $ basicUnsafeIndexM v i
10141032

10151033
-- Safe destructive updates
10161034
-- ------------------------
@@ -2534,7 +2552,7 @@ streamR v = v `seq` n `seq` (Bundle.unfoldr get n `Bundle.sized` Exact n)
25342552

25352553
{-# INLINE get #-}
25362554
get 0 = Nothing
2537-
get i = let i' = i-1
2555+
get i = let !i' = i-1
25382556
in
25392557
case basicUnsafeIndexM v i' of Box x -> Just (x, i')
25402558

vector/src/Data/Vector/Generic/Mutable.hs

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -425,8 +425,9 @@ unsafeSlice :: MVector v a => Int -- ^ starting index
425425
-> v s a
426426
-> v s a
427427
{-# INLINE unsafeSlice #-}
428-
unsafeSlice i n v = checkSlice Unsafe i n (length v)
429-
$ basicUnsafeSlice i n v
428+
-- See NOTE: [Strict indexing] in D.V.Generic
429+
unsafeSlice !i !n v = checkSlice Unsafe i n (length v)
430+
$ basicUnsafeSlice i n v
430431

431432
-- | Same as 'init', but doesn't do range checks.
432433
unsafeInit :: MVector v a => v s a -> v s a
@@ -700,33 +701,37 @@ exchange v i x = checkIndex Bounds i (length v) $ unsafeExchange v i x
700701
-- | Yield the element at the given position. No bounds checks are performed.
701702
unsafeRead :: (PrimMonad m, MVector v a) => v (PrimState m) a -> Int -> m a
702703
{-# INLINE unsafeRead #-}
703-
unsafeRead v i = checkIndex Unsafe i (length v)
704-
$ stToPrim
705-
$ basicUnsafeRead v i
704+
-- See NOTE: [Strict indexing] in D.V.Generic
705+
unsafeRead v !i = checkIndex Unsafe i (length v)
706+
$ stToPrim
707+
$ basicUnsafeRead v i
706708

707709
-- | Replace the element at the given position. No bounds checks are performed.
708710
unsafeWrite :: (PrimMonad m, MVector v a) => v (PrimState m) a -> Int -> a -> m ()
709711
{-# INLINE unsafeWrite #-}
710-
unsafeWrite v i x = checkIndex Unsafe i (length v)
711-
$ stToPrim
712-
$ basicUnsafeWrite v i x
712+
-- See NOTE: [Strict indexing] in D.V.Generic
713+
unsafeWrite v !i x = checkIndex Unsafe i (length v)
714+
$ stToPrim
715+
$ basicUnsafeWrite v i x
713716

714717
-- | Modify the element at the given position. No bounds checks are performed.
715718
unsafeModify :: (PrimMonad m, MVector v a) => v (PrimState m) a -> (a -> a) -> Int -> m ()
716719
{-# INLINE unsafeModify #-}
717-
unsafeModify v f i = checkIndex Unsafe i (length v)
718-
$ stToPrim
719-
$ basicUnsafeRead v i >>= \x ->
720-
basicUnsafeWrite v i (f x)
720+
-- See NOTE: [Strict indexing] in D.V.Generic
721+
unsafeModify v f !i = checkIndex Unsafe i (length v)
722+
$ stToPrim
723+
$ basicUnsafeRead v i >>= \x ->
724+
basicUnsafeWrite v i (f x)
721725

722726
-- | Modify the element at the given position using a monadic
723727
-- function. No bounds checks are performed.
724728
--
725729
-- @since 0.12.3.0
726730
unsafeModifyM :: (PrimMonad m, MVector v a) => v (PrimState m) a -> (a -> m a) -> Int -> m ()
727731
{-# INLINE unsafeModifyM #-}
728-
unsafeModifyM v f i = checkIndex Unsafe i (length v)
729-
$ stToPrim . basicUnsafeWrite v i =<< f =<< stToPrim (basicUnsafeRead v i)
732+
-- See NOTE: [Strict indexing] in D.V.Generic
733+
unsafeModifyM v f !i = checkIndex Unsafe i (length v)
734+
$ stToPrim . basicUnsafeWrite v i =<< f =<< stToPrim (basicUnsafeRead v i)
730735

731736
-- | Swap the elements at the given positions. No bounds checks are performed.
732737
unsafeSwap :: (PrimMonad m, MVector v a) => v (PrimState m) a -> Int -> Int -> m ()

0 commit comments

Comments
 (0)