[Git][ghc/ghc][wip/sjakobi/cmmsink-touch] CmmSink: don't treat MO_Touch as clobbering all memory

7 Sep 2026


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


Commits:
33f284d7 by Simon Jakobi at 2026-09-07T14:30:59+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

- - - - -


7 changed files:

- + changelog.d/cmmsink-touch-27782
- 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:

=====================================
changelog.d/cmmsink-touch-27782
=====================================
@@ -0,0 +1,6 @@
+section: codegen
+synopsis: The Cmm sinking pass no longer treats ``touch#`` as clobbering
+  memory, so a load preceding a ``touch#`` can be folded into the
+  instruction that uses it, as in ``ByteString`` indexing.
+issues: #27782
+mrs: !16650


=====================================
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
=====================================
@@ -0,0 +1,7 @@
+loadTouch:
+        movzbl
+        jmp
+loadPrefetch:
+        prefetchnta
+        movzbl
+        jmp


=====================================
testsuite/tests/codeGen/should_gen_asm/T27782.cmm
=====================================
@@ -0,0 +1,18 @@
+#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),



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/33f284d756ced7df18cd339911bdac18...

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/33f284d756ced7df18cd339911bdac18...
You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications: https://gitlab.haskell.org/-/profile/notifications | Help: https://gitlab.haskell.org/help

    

Simon Jakobi (＠sjakobi)

tags

participants (1)