Skip to content

Commit 171b2e6

Browse files
authored
Remove unnecessary Int-Word conversions (#1058)
This simplifies the code and the GHC Core. It is not expected to affect performance since Int-Word conversions are free at runtime. Additionally, * Remove the Nat synonym * Document some preconditions
1 parent 5b3da8f commit 171b2e6

File tree

5 files changed

+58
-76
lines changed

5 files changed

+58
-76
lines changed

containers/src/Data/IntMap/Internal.hs

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -263,12 +263,7 @@ module Data.IntMap.Internal (
263263
, showTree
264264
, showTreeWith
265265

266-
-- * Internal types
267-
, Nat
268-
269266
-- * Utility
270-
, natFromInt
271-
, intFromNat
272267
, link
273268
, linkKey
274269
, linkWithMask
@@ -313,8 +308,9 @@ import Data.IntSet.Internal.IntTreeCommons
313308
, branchMask
314309
, TreeTreeBranch(..)
315310
, treeTreeBranch
311+
, i2w
316312
)
317-
import Utils.Containers.Internal.BitUtil
313+
import Utils.Containers.Internal.BitUtil (shiftLL, shiftRL, iShiftRL)
318314
import Utils.Containers.Internal.StrictPair
319315

320316
#ifdef __GLASGOW_HASKELL__
@@ -334,17 +330,6 @@ import Text.Read
334330
import qualified Control.Category as Category
335331

336332

337-
-- A "Nat" is a natural machine word (an unsigned Int)
338-
type Nat = Word
339-
340-
natFromInt :: Key -> Nat
341-
natFromInt = fromIntegral
342-
{-# INLINE natFromInt #-}
343-
344-
intFromNat :: Nat -> Key
345-
intFromNat = fromIntegral
346-
{-# INLINE intFromNat #-}
347-
348333
{--------------------------------------------------------------------
349334
Types
350335
--------------------------------------------------------------------}
@@ -2146,7 +2131,7 @@ mergeA
21462131
-> Int -> f (IntMap a)
21472132
-> f (IntMap a)
21482133
linkA k1 t1 k2 t2
2149-
| natFromInt k1 < natFromInt k2 = binA p t1 t2
2134+
| i2w k1 < i2w k2 = binA p t1 t2
21502135
| otherwise = binA p t2 t1
21512136
where
21522137
m = branchMask k1 k2
@@ -3178,7 +3163,7 @@ fromSet f (IntSet.Tip kx bm) = buildTree f kx bm (IntSet.suffixBitMask + 1)
31783163
-- and we construct the IntMap from that half.
31793164
buildTree g !prefix !bmask bits = case bits of
31803165
0 -> Tip prefix (g prefix)
3181-
_ -> case intFromNat ((natFromInt bits) `shiftRL` 1) of
3166+
_ -> case bits `iShiftRL` 1 of
31823167
bits2
31833168
| bmask .&. ((1 `shiftLL` bits2) - 1) == 0 ->
31843169
buildTree g (prefix + bits2) (bmask `shiftRL` bits2) bits2
@@ -3552,7 +3537,7 @@ link k1 t1 k2 t2 = linkWithMask (branchMask k1 k2) k1 t1 k2 t2
35523537
-- `linkWithMask` is useful when the `branchMask` has already been computed
35533538
linkWithMask :: Int -> Key -> IntMap a -> Key -> IntMap a -> IntMap a
35543539
linkWithMask m k1 t1 k2 t2
3555-
| natFromInt k1 < natFromInt k2 = Bin p t1 t2
3540+
| i2w k1 < i2w k2 = Bin p t1 t2
35563541
| otherwise = Bin p t2 t1
35573542
where
35583543
p = Prefix (mask k1 m .|. m)

containers/src/Data/IntMap/Strict/Internal.hs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,6 @@ import Data.IntSet.Internal.IntTreeCommons
266266
(Key, Prefix(..), nomatch, left, signBranch, mask, branchMask)
267267
import Data.IntMap.Internal
268268
( IntMap (..)
269-
, natFromInt
270-
, intFromNat
271269
, bin
272270
, binCheckLeft
273271
, binCheckRight
@@ -346,7 +344,7 @@ import Data.IntMap.Internal
346344
, withoutKeys
347345
)
348346
import qualified Data.IntSet.Internal as IntSet
349-
import Utils.Containers.Internal.BitUtil
347+
import Utils.Containers.Internal.BitUtil (iShiftRL, shiftLL, shiftRL)
350348
import Utils.Containers.Internal.StrictPair
351349
import qualified Data.Foldable as Foldable
352350

@@ -1056,7 +1054,7 @@ fromSet f (IntSet.Tip kx bm) = buildTree f kx bm (IntSet.suffixBitMask + 1)
10561054
-- one of them is nonempty and we construct the IntMap from that half.
10571055
buildTree g !prefix !bmask bits = case bits of
10581056
0 -> Tip prefix $! g prefix
1059-
_ -> case intFromNat ((natFromInt bits) `shiftRL` 1) of
1057+
_ -> case bits `iShiftRL` 1 of
10601058
bits2 | bmask .&. ((1 `shiftLL` bits2) - 1) == 0 ->
10611059
buildTree g (prefix + bits2) (bmask `shiftRL` bits2) bits2
10621060
| (bmask `shiftRL` bits2) .&. ((1 `shiftLL` bits2) - 1) == 0 ->

containers/src/Data/IntSet/Internal.hs

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ import Utils.Containers.Internal.Prelude hiding
205205
(filter, foldr, foldl, foldl', foldMap, null, map)
206206
import Prelude ()
207207

208-
import Utils.Containers.Internal.BitUtil
208+
import Utils.Containers.Internal.BitUtil (iShiftRL, shiftLL, shiftRL)
209209
import Utils.Containers.Internal.StrictPair
210210
import Data.IntSet.Internal.IntTreeCommons
211211
( Key
@@ -217,6 +217,7 @@ import Data.IntSet.Internal.IntTreeCommons
217217
, branchMask
218218
, TreeTreeBranch(..)
219219
, treeTreeBranch
220+
, i2w
220221
)
221222

222223
#if __GLASGOW_HASKELL__
@@ -240,17 +241,6 @@ import Data.Functor.Identity (Identity(..))
240241

241242
infixl 9 \\{-This comment teaches CPP correct behaviour -}
242243

243-
-- A "Nat" is a natural machine word (an unsigned Int)
244-
type Nat = Word
245-
246-
natFromInt :: Int -> Nat
247-
natFromInt i = fromIntegral i
248-
{-# INLINE natFromInt #-}
249-
250-
intFromNat :: Nat -> Int
251-
intFromNat w = fromIntegral w
252-
{-# INLINE intFromNat #-}
253-
254244
{--------------------------------------------------------------------
255245
Operators
256246
--------------------------------------------------------------------}
@@ -1388,10 +1378,10 @@ fromRange (lx,rx)
13881378
| m < suffixBitMask = Tip p (complement 0)
13891379
| otherwise = Bin (Prefix (p .|. m)) (goFull p (shr1 m)) (goFull (p .|. m) (shr1 m))
13901380
lbm :: Int -> Int
1391-
lbm p = intFromNat (lowestBitMask (natFromInt p))
1381+
lbm p = p .&. negate p -- lowest bit mask
13921382
{-# INLINE lbm #-}
13931383
shr1 :: Int -> Int
1394-
shr1 m = intFromNat (natFromInt m `shiftRL` 1)
1384+
shr1 m = m `iShiftRL` 1
13951385
{-# INLINE shr1 #-}
13961386

13971387
-- | \(O(n)\). Build a set from an ascending list of elements.
@@ -1621,7 +1611,7 @@ link k1 t1 k2 t2 = linkWithMask (branchMask k1 k2) k1 t1 k2 t2
16211611
-- `linkWithMask` is useful when the `branchMask` has already been computed
16221612
linkWithMask :: Int -> Key -> IntSet -> Key -> IntSet -> IntSet
16231613
linkWithMask m k1 t1 k2 t2
1624-
| natFromInt k1 < natFromInt k2 = Bin p t1 t2
1614+
| i2w k1 < i2w k2 = Bin p t1 t2
16251615
| otherwise = Bin p t2 t1
16261616
where
16271617
p = Prefix (mask k1 m .|. m)
@@ -1685,18 +1675,18 @@ bitmapOf x = bitmapOfSuffix (suffixOf x)
16851675
The signatures of methods in question are placed after this comment.
16861676
----------------------------------------------------------------------}
16871677

1688-
lowestBitSet :: Nat -> Int
1689-
highestBitSet :: Nat -> Int
1690-
foldlBits :: Int -> (a -> Int -> a) -> a -> Nat -> a
1691-
foldl'Bits :: Int -> (a -> Int -> a) -> a -> Nat -> a
1692-
foldrBits :: Int -> (Int -> a -> a) -> a -> Nat -> a
1693-
foldr'Bits :: Int -> (Int -> a -> a) -> a -> Nat -> a
1678+
lowestBitSet :: Word -> Int
1679+
highestBitSet :: Word -> Int
1680+
foldlBits :: Int -> (a -> Int -> a) -> a -> Word -> a
1681+
foldl'Bits :: Int -> (a -> Int -> a) -> a -> Word -> a
1682+
foldrBits :: Int -> (Int -> a -> a) -> a -> Word -> a
1683+
foldr'Bits :: Int -> (Int -> a -> a) -> a -> Word -> a
16941684
#if MIN_VERSION_base(4,11,0)
1695-
foldMapBits :: Semigroup a => Int -> (Int -> a) -> Nat -> a
1685+
foldMapBits :: Semigroup a => Int -> (Int -> a) -> Word -> a
16961686
#else
1697-
foldMapBits :: Monoid a => Int -> (Int -> a) -> Nat -> a
1687+
foldMapBits :: Monoid a => Int -> (Int -> a) -> Word -> a
16981688
#endif
1699-
takeWhileAntitoneBits :: Int -> (Int -> Bool) -> Nat -> Nat
1689+
takeWhileAntitoneBits :: Int -> (Int -> Bool) -> Word -> Word
17001690

17011691
{-# INLINE lowestBitSet #-}
17021692
{-# INLINE highestBitSet #-}
@@ -1707,26 +1697,26 @@ takeWhileAntitoneBits :: Int -> (Int -> Bool) -> Nat -> Nat
17071697
{-# INLINE foldMapBits #-}
17081698
{-# INLINE takeWhileAntitoneBits #-}
17091699

1710-
lowestBitMask :: Nat -> Nat
1700+
#if defined(__GLASGOW_HASKELL__)
1701+
1702+
lowestBitMask :: Word -> Word
17111703
lowestBitMask x = x .&. negate x
17121704
{-# INLINE lowestBitMask #-}
17131705

1714-
#if defined(__GLASGOW_HASKELL__)
1715-
17161706
lowestBitSet x = countTrailingZeros x
17171707

17181708
highestBitSet x = WORD_SIZE_IN_BITS - 1 - countLeadingZeros x
17191709

1720-
-- Reverse the order of bits in the Nat.
1721-
revNat :: Nat -> Nat
1710+
-- Reverse the order of bits in the Word.
1711+
revWord :: Word -> Word
17221712
#if WORD_SIZE_IN_BITS==32
1723-
revNat x1 = case ((x1 `shiftRL` 1) .&. 0x55555555) .|. ((x1 .&. 0x55555555) `shiftLL` 1) of
1713+
revWord x1 = case ((x1 `shiftRL` 1) .&. 0x55555555) .|. ((x1 .&. 0x55555555) `shiftLL` 1) of
17241714
x2 -> case ((x2 `shiftRL` 2) .&. 0x33333333) .|. ((x2 .&. 0x33333333) `shiftLL` 2) of
17251715
x3 -> case ((x3 `shiftRL` 4) .&. 0x0F0F0F0F) .|. ((x3 .&. 0x0F0F0F0F) `shiftLL` 4) of
17261716
x4 -> case ((x4 `shiftRL` 8) .&. 0x00FF00FF) .|. ((x4 .&. 0x00FF00FF) `shiftLL` 8) of
17271717
x5 -> ( x5 `shiftRL` 16 ) .|. ( x5 `shiftLL` 16);
17281718
#else
1729-
revNat x1 = case ((x1 `shiftRL` 1) .&. 0x5555555555555555) .|. ((x1 .&. 0x5555555555555555) `shiftLL` 1) of
1719+
revWord x1 = case ((x1 `shiftRL` 1) .&. 0x5555555555555555) .|. ((x1 .&. 0x5555555555555555) `shiftLL` 1) of
17301720
x2 -> case ((x2 `shiftRL` 2) .&. 0x3333333333333333) .|. ((x2 .&. 0x3333333333333333) `shiftLL` 2) of
17311721
x3 -> case ((x3 `shiftRL` 4) .&. 0x0F0F0F0F0F0F0F0F) .|. ((x3 .&. 0x0F0F0F0F0F0F0F0F) `shiftLL` 4) of
17321722
x4 -> case ((x4 `shiftRL` 8) .&. 0x00FF00FF00FF00FF) .|. ((x4 .&. 0x00FF00FF00FF00FF) `shiftLL` 8) of
@@ -1747,14 +1737,14 @@ foldl'Bits prefix f z bitmap = go bitmap z
17471737
where !bitmask = lowestBitMask bm
17481738
!bi = countTrailingZeros bitmask
17491739

1750-
foldrBits prefix f z bitmap = go (revNat bitmap) z
1740+
foldrBits prefix f z bitmap = go (revWord bitmap) z
17511741
where go 0 acc = acc
17521742
go bm acc = go (bm `xor` bitmask) ((f $! (prefix+(WORD_SIZE_IN_BITS-1)-bi)) acc)
17531743
where !bitmask = lowestBitMask bm
17541744
!bi = countTrailingZeros bitmask
17551745

17561746

1757-
foldr'Bits prefix f z bitmap = go (revNat bitmap) z
1747+
foldr'Bits prefix f z bitmap = go (revWord bitmap) z
17581748
where go 0 acc = acc
17591749
go bm !acc = go (bm `xor` bitmask) ((f $! (prefix+(WORD_SIZE_IN_BITS-1)-bi)) acc)
17601750
where !bitmask = lowestBitMask bm

containers/src/Data/IntSet/Internal/IntTreeCommons.hs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,11 @@ module Data.IntSet.Internal.IntTreeCommons
3535
, treeTreeBranch
3636
, mask
3737
, branchMask
38+
, i2w
3839
) where
3940

40-
import Data.Bits (Bits(..))
41-
import Utils.Containers.Internal.BitUtil (highestBitMask)
41+
import Data.Bits (Bits(..), countLeadingZeros)
42+
import Utils.Containers.Internal.BitUtil (wordSize)
4243

4344
#ifdef __GLASGOW_HASKELL__
4445
import Language.Haskell.TH.Syntax (Lift)
@@ -149,18 +150,17 @@ mask i m = i .&. ((-m) `xor` m)
149150
{-# INLINE mask #-}
150151

151152
-- | The first switching bit where the two prefixes disagree.
153+
--
154+
-- Precondition for defined behavior: p1 /= p2
152155
branchMask :: Int -> Int -> Int
153-
branchMask p1 p2 = w2i (highestBitMask (i2w (p1 `xor` p2)))
156+
branchMask p1 p2 =
157+
unsafeShiftL 1 (wordSize - 1 - countLeadingZeros (p1 `xor` p2))
154158
{-# INLINE branchMask #-}
155159

156160
i2w :: Int -> Word
157161
i2w = fromIntegral
158162
{-# INLINE i2w #-}
159163

160-
w2i :: Word -> Int
161-
w2i = fromIntegral
162-
{-# INLINE w2i #-}
163-
164164
{--------------------------------------------------------------------
165165
Notes
166166
--------------------------------------------------------------------}

containers/src/Utils/Containers/Internal/BitUtil.hs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{-# LANGUAGE CPP #-}
2-
#if !defined(TESTING) && defined(__GLASGOW_HASKELL__)
3-
{-# LANGUAGE Safe #-}
2+
#ifdef __GLASGOW_HASKELL__
3+
{-# LANGUAGE MagicHash #-}
4+
{-# LANGUAGE Trustworthy #-}
45
#endif
56

67
#include "containers.h"
@@ -28,26 +29,34 @@
2829
-- closely.
2930

3031
module Utils.Containers.Internal.BitUtil
31-
( highestBitMask
32-
, shiftLL
32+
( shiftLL
3333
, shiftRL
3434
, wordSize
35+
, iShiftRL
3536
) where
3637

37-
import Data.Bits (unsafeShiftL, unsafeShiftR
38-
, countLeadingZeros, finiteBitSize
39-
)
40-
41-
-- | Return a word where only the highest bit is set.
42-
highestBitMask :: Word -> Word
43-
highestBitMask w = shiftLL 1 (wordSize - 1 - countLeadingZeros w)
44-
{-# INLINE highestBitMask #-}
38+
import Data.Bits (unsafeShiftL, unsafeShiftR, finiteBitSize)
39+
#ifdef __GLASGOW_HASKELL__
40+
import GHC.Exts (Int(..), uncheckedIShiftRL#)
41+
#endif
4542

4643
-- Right and left logical shifts.
44+
--
45+
-- Precondition for defined behavior: 0 <= shift amount < wordSize
4746
shiftRL, shiftLL :: Word -> Int -> Word
4847
shiftRL = unsafeShiftR
4948
shiftLL = unsafeShiftL
5049

5150
{-# INLINE wordSize #-}
5251
wordSize :: Int
5352
wordSize = finiteBitSize (0 :: Word)
53+
54+
-- Right logical shift.
55+
--
56+
-- Precondition for defined behavior: 0 <= shift amount < wordSize
57+
iShiftRL :: Int -> Int -> Int
58+
#ifdef __GLASGOW_HASKELL__
59+
iShiftRL (I# x#) (I# sh#) = I# (uncheckedIShiftRL# x# sh#)
60+
#else
61+
iShiftRL x sh = fromIntegral (unsafeShiftR (fromIntegral x :: Word) sh)
62+
#endif

0 commit comments

Comments
 (0)