Simon Jakobi pushed to branch wip/sjakobi/T27688-bt-single-bit-tests at Glasgow Haskell Compiler / GHC

Commits:

9 changed files:

Changes:

  • changelog.d/ncg-x86-bt-single-bit-tests
    1
    +section: compiler
    
    2
    +synopsis: The x86 native code generator now uses the bit-test instruction
    
    3
    +  ``bt`` to test a single bit
    
    4
    +description:
    
    5
    +  Cmm patterns such as ``(x & (1 << i)) != 0`` and ``(x & (1 << i)) == 0``
    
    6
    +  now compile to a single ``bt`` instruction, whose carry flag feeds the
    
    7
    +  conditional branch directly, instead of a mov/shl/and/test sequence,
    
    8
    +  matching what C compilers produce. The same applies to a literal mask
    
    9
    +  that doesn't fit in an imm32 operand, such as ``x & (1 << 40)``.
    
    10
    +
    
    11
    +  As for ``btr``/``bts``/``btc``, a variable bit index benefits only when
    
    12
    +  the shift is unchecked. The bounds mask added by the checked ``shiftL``,
    
    13
    +  and hence by the default ``Data.Bits.testBit``, hides the pattern.
    
    14
    +mrs: !16639
    
    15
    +issues: #27688

  • compiler/GHC/CmmToAsm/X86/CodeGen.hs
    ... ... @@ -3945,6 +3945,24 @@ condIntCode' platform cond (CmmLoad x ty _) (CmmLit lit)
    3945 3945
         --
    
    3946 3946
         return (CondCode False cond code)
    
    3947 3947
     
    
    3948
    +-- single-bit test, e.g. (x & (1 << i)) != 0
    
    3949
    +-- see Note [Bit-test instructions]
    
    3950
    +condIntCode' platform cond (CmmMachOp (MO_And w) [x, y]) (CmmLit (CmmInt 0 _))
    
    3951
    +  | Just bt_cond <- bitTestCond cond
    
    3952
    +  , bitTestOpWidthOK (target32Bit platform) w
    
    3953
    +  , Just (opnd, ix) <- setBitArgs_maybe platform w x y
    
    3954
    +  , let fmt = intFormat w
    
    3955
    +  = case ix of
    
    3956
    +      BitIndexImm i -> do
    
    3957
    +        (x_reg, x_code) <- getSomeReg opnd
    
    3958
    +        return $ CondCode False bt_cond $
    
    3959
    +          x_code `snocOL` BT fmt (OpImm (ImmInt i)) (OpReg x_reg)
    
    3960
    +      BitIndexReg i -> do
    
    3961
    +        (i_reg, i_code) <- getNonClobberedReg i
    
    3962
    +        (x_reg, x_code) <- getSomeReg opnd
    
    3963
    +        return $ CondCode False bt_cond $
    
    3964
    +          i_code `appOL` x_code `snocOL` BT fmt (OpReg i_reg) (OpReg x_reg)
    
    3965
    +
    
    3948 3966
     -- anything vs zero, using a mask
    
    3949 3967
     -- TODO: Add some sanity checking!!!!
    
    3950 3968
     condIntCode' platform cond (CmmMachOp (MO_And _) [x,o2]) (CmmLit (CmmInt 0 ty))
    
    ... ... @@ -5901,17 +5919,18 @@ genTrivialCode rep instr a b = do
    5901 5919
     
    
    5902 5920
     {- Note [Bit-test instructions]
    
    5903 5921
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    
    5904
    -x86 has dedicated instructions for clearing (btr), setting (bts) and
    
    5905
    -complementing (btc) a single bit whose index is given in a register.  We use
    
    5906
    -them for Cmm patterns such as
    
    5922
    +x86 has dedicated instructions for testing (bt), clearing (btr), setting
    
    5923
    +(bts) and complementing (btc) a single bit whose index is given in a
    
    5924
    +register.  We use them for Cmm patterns such as
    
    5907 5925
     
    
    5908
    -  x & ~(1 << i)     ==>     btr i, x       (#25233)
    
    5926
    +  (x & (1 << i)) != 0      ==>     bt i, x; jc
    
    5927
    +  x & ~(1 << i)            ==>     btr i, x
    
    5909 5928
     
    
    5910
    -replacing a mov/shl/not/and sequence with a single instruction.  The
    
    5911
    -shift-count register operand of shl is masked modulo the operand width, and
    
    5912
    -the bit-offset register operand of btr/bts/btc is masked the same way, so
    
    5913
    -the replacement is faithful even for out-of-range i (where the Cmm shift is
    
    5914
    -in any case undefined).
    
    5929
    +replacing a mov/shl/and/test or mov/shl/not/and sequence with a single
    
    5930
    +instruction.  The shift-count register operand of shl is masked modulo the
    
    5931
    +operand width, and the bit-offset register operand of the bit-test
    
    5932
    +instructions is masked the same way, so the replacement is faithful even for
    
    5933
    +out-of-range i (where the Cmm shift is in any case undefined).
    
    5915 5934
     
    
    5916 5935
     The bit-offset operand of these instructions must be an immediate or a
    
    5917 5936
     register.  When the bit index is a literal, no shift reaches the NCG:
    
    ... ... @@ -5919,10 +5938,11 @@ constant folding has already turned the whole mask into a literal. If that
    5919 5938
     mask fits in an imm32, we keep the ordinary and/or/xor with an immediate:
    
    5920 5939
     it has the same latency and better throughput (more execution ports) than
    
    5921 5940
     the bit-test instructions,  and at worst two bytes of extra code size for bit
    
    5922
    -indices 7..30.
    
    5941
    +indices 7..30.  Likewise a single-bit test keeps 'test' with an immediate:
    
    5942
    +test+jcc macro-fuse into one uop, bt+jcc do not.
    
    5923 5943
     But a W64 mask touching the upper bits, e.g. ~(1 << 40), would have to be moved
    
    5924 5944
     into a register first.  For such masks we recognise the folded literal itself
    
    5925
    -(exactly one bit clear resp. set) and emit btr/bts/btc with an immediate
    
    5945
    +(exactly one bit clear resp. set) and emit bt/btr/bts/btc with an immediate
    
    5926 5946
     bit offset.
    
    5927 5947
     
    
    5928 5948
     We restrict the pattern to W32 and native-width W64: the instructions do not
    
    ... ... @@ -5959,14 +5979,25 @@ clearBitLit_maybe w m = setBitLit_maybe w (complement m)
    5959 5979
     bitTestOpWidthOK :: Bool -> Width -> Bool
    
    5960 5980
     bitTestOpWidthOK is32Bit w = w == W32 || (w == W64 && not is32Bit)
    
    5961 5981
     
    
    5962
    --- | The bit-offset operand of a bit-test instruction (btr/bts/btc).
    
    5982
    +-- | The condition to branch on after @bt@, for a @mask != 0@ ('NE') or
    
    5983
    +-- @mask == 0@ ('EQQ') comparison. 'Nothing' for any other condition.
    
    5984
    +--
    
    5985
    +-- @bt@ reports the tested bit in CF, so we branch on 'CARRY' or 'GEU'
    
    5986
    +-- rather than on ZF via 'NE' or 'EQQ'.
    
    5987
    +bitTestCond :: Cond -> Maybe Cond
    
    5988
    +bitTestCond NE  = Just CARRY
    
    5989
    +bitTestCond EQQ = Just GEU
    
    5990
    +bitTestCond _   = Nothing
    
    5991
    +
    
    5992
    +-- | The bit-offset operand of a bit-test instruction.
    
    5963 5993
     data BitIndex
    
    5964 5994
       = BitIndexReg CmmExpr  -- ^ variable index, computed into a register
    
    5965 5995
       | BitIndexImm Int      -- ^ literal index, emitted as an immediate
    
    5966 5996
     
    
    5967
    --- | Match the operands of a single-bit set or complement operation: one
    
    5968
    --- operand is a mask @1 << i@, or a literal with exactly one bit set that
    
    5969
    --- does not fit in an imm32. Returns the other operand and the bit index.
    
    5997
    +-- | Match the operands of a single-bit set, complement or test operation:
    
    5998
    +-- one operand is a mask @1 << i@, or a literal with exactly one bit set
    
    5999
    +-- that does not fit in an imm32. Returns the other operand and the bit
    
    6000
    +-- index.
    
    5970 6001
     --
    
    5971 6002
     -- Both operand orders are matched, e.g. @x | (1 << i)@ and @(1 << i) | x@.
    
    5972 6003
     --
    

  • compiler/GHC/CmmToAsm/X86/Instr.hs
    ... ... @@ -192,7 +192,8 @@ data Instr
    192 192
             | SHRD        Format Operand{-amount-} Operand Operand
    
    193 193
             | SHLD        Format Operand{-amount-} Operand Operand
    
    194 194
     
    
    195
    -        | BT          Format Imm Operand
    
    195
    +        -- | Bit test
    
    196
    +        | BT          Format Operand{- ^ bit offset (imm/reg) -} Operand
    
    196 197
             -- | Bit test-and-reset
    
    197 198
             | BTR         Format Operand{- ^ bit offset (imm/reg) -} Operand
    
    198 199
             -- | Bit set
    
    ... ... @@ -501,7 +502,7 @@ regUsageOfInstr platform instr
    501 502
         SHR    fmt imm dst    -> usageRM fmt imm dst
    
    502 503
         SHLD   fmt imm dst1 dst2 -> usageRMM fmt imm dst1 dst2
    
    503 504
         SHRD   fmt imm dst1 dst2 -> usageRMM fmt imm dst1 dst2
    
    504
    -    BT     fmt _   src    -> mkRUR (use_R fmt src [])
    
    505
    +    BT     fmt off src    -> mkRUR (use_R fmt off $! use_R fmt src [])
    
    505 506
         BTR    fmt off dst    -> usageRM fmt off dst
    
    506 507
         BTS    fmt off dst    -> usageRM fmt off dst
    
    507 508
         BTC    fmt off dst    -> usageRM fmt off dst
    
    ... ... @@ -838,7 +839,7 @@ patchRegsOfInstr platform instr env
    838 839
         SHR  fmt imm dst     -> patch1 (SHR fmt imm) dst
    
    839 840
         SHLD fmt imm dst1 dst2 -> patch2 (SHLD fmt imm) dst1 dst2
    
    840 841
         SHRD fmt imm dst1 dst2 -> patch2 (SHRD fmt imm) dst1 dst2
    
    841
    -    BT   fmt imm src     -> patch1 (BT  fmt imm) src
    
    842
    +    BT   fmt off src     -> patch2 (BT  fmt) off src
    
    842 843
         BTR  fmt off dst     -> patch2 (BTR fmt) off dst
    
    843 844
         BTS  fmt off dst     -> patch2 (BTS fmt) off dst
    
    844 845
         BTC  fmt off dst     -> patch2 (BTC fmt) off dst
    

  • compiler/GHC/CmmToAsm/X86/Ppr.hs
    ... ... @@ -859,8 +859,8 @@ pprInstr platform i = case i of
    859 859
        SHRD format src dst1 dst2
    
    860 860
           -> pprShift2 (text "shrd") format src dst1 dst2
    
    861 861
     
    
    862
    -   BT format imm src
    
    863
    -      -> pprFormatImmOp (text "bt") format imm src
    
    862
    +   BT format off src
    
    863
    +      -> pprFormatOpOp (text "bt") format off src
    
    864 864
     
    
    865 865
        BTR format off dst
    
    866 866
           -> pprFormatOpOp (text "btr") format off dst
    
    ... ... @@ -1250,16 +1250,6 @@ pprInstr platform i = case i of
    1250 1250
              FmtInt64  -> text "q"
    
    1251 1251
        pprBroadcastFormat _ = panic "Scalar Format invading vector operation"
    
    1252 1252
     
    
    1253
    -   pprFormatImmOp :: Line doc -> Format -> Imm -> Operand -> doc
    
    1254
    -   pprFormatImmOp name format imm op1
    
    1255
    -     = line $ hcat [
    
    1256
    -           pprMnemonic name format,
    
    1257
    -           char '$',
    
    1258
    -           pprImm platform imm,
    
    1259
    -           comma,
    
    1260
    -           pprOperand platform format op1
    
    1261
    -       ]
    
    1262
    -
    
    1263 1253
        pprFormatOp_ :: Line doc -> Format -> Operand -> doc
    
    1264 1254
        pprFormatOp_ name format op1
    
    1265 1255
          = line $ hcat [
    

  • testsuite/tests/codeGen/should_gen_asm/T27688.asm
    1
    +btq %
    
    2
    +btl %
    
    3
    +btq $40,

  • testsuite/tests/codeGen/should_gen_asm/T27688.hs
    1
    +{-# LANGUAGE MagicHash #-}
    
    2
    +
    
    3
    +-- Check that testing a single, variable bit uses the bt instruction
    
    4
    +-- (#27688).
    
    5
    +module T27688 where
    
    6
    +
    
    7
    +import GHC.Exts
    
    8
    +
    
    9
    +testBitNe :: Word# -> Int# -> Int#
    
    10
    +testBitNe x i =
    
    11
    +  if isTrue# ((x `and#` (1## `uncheckedShiftL#` i)) `neWord#` 0##)
    
    12
    +  then 1# else 2#
    
    13
    +
    
    14
    +testBitEq :: Word# -> Int# -> Int#
    
    15
    +testBitEq x i =
    
    16
    +  if isTrue# ((x `and#` (1## `uncheckedShiftL#` i)) `eqWord#` 0##)
    
    17
    +  then 1# else 2#
    
    18
    +
    
    19
    +testBit32 :: Word32# -> Int# -> Int#
    
    20
    +testBit32 x i =
    
    21
    +  if isTrue# ((x `andWord32#` (wordToWord32# 1## `uncheckedShiftLWord32#` i))
    
    22
    +                `neWord32#` wordToWord32# 0##)
    
    23
    +  then 1# else 2#
    
    24
    +
    
    25
    +-- With a constant bit index >= 32, the mask constant-folds to a literal
    
    26
    +-- that does not fit in an imm32, so bt with an immediate offset is used.
    
    27
    +testBit40 :: Word# -> Int#
    
    28
    +testBit40 x =
    
    29
    +  if isTrue# ((x `and#` (1## `uncheckedShiftL#` 40#)) `neWord#` 0##)
    
    30
    +  then 1# else 2#

  • testsuite/tests/codeGen/should_gen_asm/T27688b.asm
    1
    +btq %
    
    2
    +setc
    
    3
    +setae
    
    4
    +btq $40,

  • testsuite/tests/codeGen/should_gen_asm/T27688b.cmm
    1
    +#include "Cmm.h"
    
    2
    +
    
    3
    +// Single-bit tests whose result is used as a value rather than as a branch
    
    4
    +// condition, a literal mask that does not fit in an imm32, and the mask on
    
    5
    +// the left of the operator (#27688).
    
    6
    +
    
    7
    +testBitValue (W_ x, W_ i) {
    
    8
    +    return ((x & (1 << i)) != 0);
    
    9
    +}
    
    10
    +
    
    11
    +testBitValueEq (W_ x, W_ i) {
    
    12
    +    return ((x & (1 << i)) == 0);
    
    13
    +}
    
    14
    +
    
    15
    +testBit40 (W_ x) {
    
    16
    +    return (((0x10000000000 :: bits64) & x) != 0);
    
    17
    +}
    
    18
    +
    
    19
    +testBitLeft (W_ x, W_ i) {
    
    20
    +    return (((1 << i) & x) != 0);
    
    21
    +}

  • testsuite/tests/codeGen/should_gen_asm/all.T
    ... ... @@ -27,6 +27,10 @@ test('T25233', [unless(arch('x86_64'), skip),
    27 27
                     when(unregisterised(), skip)], compile_grep_asm, ['hs', True, '-O'])
    
    28 28
     test('T25233b', [unless(arch('x86_64'), skip),
    
    29 29
                      when(unregisterised(), skip)], compile_grep_asm, ['cmm', True, ''])
    
    30
    +test('T27688', [unless(arch('x86_64'), skip),
    
    31
    +                when(unregisterised(), skip)], compile_grep_asm, ['hs', True, '-O'])
    
    32
    +test('T27688b', [unless(arch('x86_64'), skip),
    
    33
    +                 when(unregisterised(), skip)], compile_grep_asm, ['cmm', True, ''])
    
    30 34
     is_aarch64_codegen = [
    
    31 35
         unless(arch('aarch64'), skip),
    
    32 36
         when(unregisterised(), skip),