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 ===================================== @@ -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/1394f1919a3c3d732ce81e40ae3a5084... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/1394f1919a3c3d732ce81e40ae3a5084... 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