GitLab

Simon Jakobi pushed to branch wip/sjakobi/cmmsink-touch at Glasgow Haskell Compiler / GHC

Commits:

1394f191

by Simon Jakobi at 2026-09-07T14:25:58+02:00

CmmSink: don't treat MO_Touch as clobbering all memory

Rule (C3)(c) of the sinking pass blocked loads from sinking past any
CmmUnsafeForeignCall, including touch#, which emits no code. In the
read-then-touch pattern of bytestring's unsafe indexing this left a
movb + movzbl where one movzbl from memory suffices.

Add callishMachOpClobbersMemory to classify callish ops. For now only
MO_Touch and MO_Prefetch_Data count as non-clobbering.

Sinking a load past touch# is safe. The hazard in #18040 is a
Core-level read delayed past a GC safe point. The sinking pass works
within a block, and any call that might contain a safe point still
blocks it. See Note [Foreign calls clobber heap].

Also let the Cmm parser accept prim %touch(...), needed by the test.

Fixes #27782.

Assisted-by: Claude Fable 5.1

6 changed files:

compiler/GHC/Cmm/MachOp.hs
compiler/GHC/Cmm/Parser.y
compiler/GHC/Cmm/Sink.hs
+ testsuite/tests/codeGen/should_gen_asm/T27782.asm
+ testsuite/tests/codeGen/should_gen_asm/T27782.cmm
testsuite/tests/codeGen/should_gen_asm/all.T

Changes:

compiler/GHC/Cmm/MachOp.hs

@@ -18,6 +18,7 @@ module GHC.Cmm.MachOp
      -- CallishMachOp
      , CallishMachOp(..), callishMachOpHints
 +    , callishMachOpClobbersMemory
      , pprCallishMachOp
      , machOpMemcpyishAlign
      , callishMachOpArgTys
@@ -876,6 +877,18 @@ callishMachOpHints op = case op of
    _                -> ([],[])
    -- empty lists indicate NoHint
 +-- | May this operation write to memory, or otherwise act as a memory
 +-- barrier?  When in doubt, answer 'True'.
 +--
 +-- Used by the sinking pass to decide whether a memory read may be moved
 +-- past a call of this operation; see Note [Foreign calls clobber heap]
 +-- in GHC.Cmm.Sink.
 +callishMachOpClobbersMemory :: CallishMachOp -> Bool
 +callishMachOpClobbersMemory op = case op of
 +  MO_Touch           -> False  -- pure liveness marker, no instructions
 +  MO_Prefetch_Data _ -> False  -- reads memory, writes nothing
 +  _                  -> True
++
  -- | The alignment of a 'memcpy'-ish operation.
  machOpMemcpyishAlign :: CallishMachOp -> Maybe Int
  machOpMemcpyishAlign op = case op of

compiler/GHC/Cmm/Parser.y

@@ -1203,6 +1203,8 @@ callishMachOps platform = listToUFM $
          ( "prefetch2", (MO_Prefetch_Data 2,)),
          ( "prefetch3", (MO_Prefetch_Data 3,)),
 +        ( "touch", (MO_Touch,) ),
++
          ( "bswap16", (MO_BSwap W16,) ),
          ( "bswap32", (MO_BSwap W32,) ),
          ( "bswap64", (MO_BSwap W64,) )

compiler/GHC/Cmm/Sink.hs

@@ -719,7 +719,10 @@ conflicts platform assig@(r, rhs, addr) node
    = True
    -- (c) foreign calls clobber heap: see Note [Foreign calls clobber heap]
 -  | CmmUnsafeForeignCall{} <- node, memConflicts addr AnyMem      = True
 +  | CmmUnsafeForeignCall target _ _ <- node
 +  , foreignTargetClobbersMemory target
 +  , memConflicts addr AnyMem
 +  = True
    -- (d) native calls clobber any memory
    | CmmCall{} <- node, memConflicts addr AnyMem                   = True
@@ -732,6 +735,12 @@ conflicts platform assig@(r, rhs, addr) node
    | otherwise = False
 +-- | May this foreign-call target write to memory, or otherwise act as a
 +-- memory barrier?  See Note [Foreign calls clobber heap].
 +foreignTargetClobbersMemory :: ForeignTarget -> Bool
 +foreignTargetClobbersMemory (PrimTarget op)    = callishMachOpClobbersMemory op
 +foreignTargetClobbersMemory (ForeignTarget {}) = True
++
  {- Note [Inlining foldRegsDefd]
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  foldRegsDefd is, after optimization, *not* a small function so
@@ -874,6 +883,16 @@ data AbsMem
  -- `suspendThread` releases the capability used by the thread, hence we mustn't
  -- float accesses to heap, stack or virtual global registers stored in the
  -- capability (e.g. with unregisterised build, see #19237).
 +--
 +-- Conversely, a few CallishMachOps neither write memory nor imply a
 +-- barrier, and needn't block sinking ('callishMachOpClobbersMemory' in
 +-- GHC.Cmm.MachOp classifies them):
 +--
 +--  * MO_Touch is a pure liveness marker: no backend emits any code for it.
 +--    Sinking is intra-block, so the Core-level touch# reordering hazard
 +--    of #18040 does not arise.
 +--
 +--  * MO_Prefetch_Data reads memory but writes nothing.
  bothMems :: AbsMem -> AbsMem -> AbsMem

testsuite/tests/codeGen/should_gen_asm/T27782.asm

 +loadTouch:
 +        movzbl
 +        jmp
 +loadPrefetch:
 +        prefetchnta
 +        movzbl
 +        jmp

testsuite/tests/codeGen/should_gen_asm/T27782.cmm

 +#include "Cmm.h"
++
 +// A load must sink past touch# and prefetch, which don't clobber memory,
 +// so that it can be folded into the instruction that uses it.
++
 +loadTouch (W_ p, P_ o) {
 +    bits8 b;
 +    b = bits8[p];
 +    prim %touch(o);
 +    return (%zx64(b));
 +}
++
 +loadPrefetch (W_ p, W_ q) {
 +    bits8 b;
 +    b = bits8[p];
 +    prim %prefetch0(q);
 +    return (%zx64(b));
 +}

testsuite/tests/codeGen/should_gen_asm/all.T

@@ -27,6 +27,7 @@ test('T25233', [unless(arch('x86_64'), skip),
                  when(unregisterised(), skip)], compile_grep_asm, ['hs', True, '-O'])
  test('T25233b', [unless(arch('x86_64'), skip),
                   when(unregisterised(), skip)], compile_grep_asm, ['cmm', True, ''])
 +test('T27782', is_amd64_codegen, compile_cmp_asm, ['cmm', ''])
  is_aarch64_codegen = [
      unless(arch('aarch64'), skip),
      when(unregisterised(), skip),