Simon Jakobi pushed to branch wip/sjakobi/cmmsink-touch at Glasgow Haskell Compiler / GHC Commits: 5b1e1e19 by Simon Jakobi at 2026-09-08T18:52:28+02:00 CmmSink: fix the safety argument for sinking past touch# Note [Foreign calls clobber heap] claimed that sinking is intra-block, but the sinking pass also moves assignments into single-predecessor successor blocks. The correct argument is that no GC safe point can lie between a load's old and new position: after stack layout those are CmmCall nodes, which rule (C3)(d) still blocks, and suspendThread/resumeThread, which stay clobbering. Context: #27782. Assisted-by: Claude Fable 5.1 - - - - - ef34e1e3 by Simon Jakobi at 2026-09-08T19:02:41+02:00 CmmSink: classify every CallishMachOp's memory effect callishMachOpClobbersMemory now decides per constructor instead of allowing only touch# and prefetch. Most callish ops neither write memory nor imply ordering: the libm calls, the 64-bit fallbacks for 32-bit targets, the quotRem/carry/mul2 helpers, popcnt and friends, memcmp. Loads can now sink past them. Memcpy-likes, fences, atomics and suspend/resumeThread stay clobbering. See Note [Foreign calls clobber heap]. The common winning shape is a stack reload after a call return that is used after a quotRemInt#: case f x of I# y -> case quotRemInt# y d of (# q, r #) -> q +# n The reload of n stayed above the idiv, costing a load plus two moves because idiv clobbers rax and rdx. Now it is one add with a memory operand after the division. Context: #27782. Assisted-by: Claude Fable 5.1 - - - - - 6 changed files: - changelog.d/cmmsink-touch-27782 - compiler/GHC/Cmm/MachOp.hs - compiler/GHC/Cmm/Sink.hs - + testsuite/tests/codeGen/should_gen_asm/T27782b.asm - + testsuite/tests/codeGen/should_gen_asm/T27782b.cmm - testsuite/tests/codeGen/should_gen_asm/all.T Changes: ===================================== changelog.d/cmmsink-touch-27782 ===================================== @@ -1,6 +1,7 @@ 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. +synopsis: The Cmm sinking pass no longer treats ``touch#``, arithmetic + helpers such as ``quotRemInt#`` and ``timesWord2#``, bit operations, or + libm calls as clobbering memory, so a load preceding one of them can be + folded into the instruction that uses it, as in ``ByteString`` indexing. issues: #27782 mrs: !16650 ===================================== compiler/GHC/Cmm/MachOp.hs ===================================== @@ -885,9 +885,139 @@ callishMachOpHints op = case op of -- in GHC.Cmm.Sink. callishMachOpClobbersMemory :: CallishMachOp -> Bool callishMachOpClobbersMemory op = case op of + -- CQ[clobber-libm] + -- Q: these become C calls on most backends. Why is a C call to sin/pow + -- safe to sink a load past, given the stmCommitNestedTransaction + -- story in Note [Foreign calls clobber heap]? + -- A~ the counterexample is about RTS functions reached via ForeignTarget, + -- which stays clobbering. libm/ghc-prim helpers only read their + -- arguments and write errno, thread-local C memory no Cmm load reads. + -- Loads through caller-saved STG registers are stopped by rule (C1) + -- anyway (foreignTargetRegs in GHC.Cmm.Node). + MO_F64_Pwr -> False + MO_F64_Sin -> False + MO_F64_Cos -> False + MO_F64_Tan -> False + MO_F64_Sinh -> False + MO_F64_Cosh -> False + MO_F64_Tanh -> False + MO_F64_Asin -> False + MO_F64_Acos -> False + MO_F64_Atan -> False + MO_F64_Asinh -> False + MO_F64_Acosh -> False + MO_F64_Atanh -> False + MO_F64_Log -> False + MO_F64_Log1P -> False + MO_F64_Exp -> False + MO_F64_ExpM1 -> False + MO_F64_Fabs -> False + MO_F64_Sqrt -> False + MO_F32_Pwr -> False + MO_F32_Sin -> False + MO_F32_Cos -> False + MO_F32_Tan -> False + MO_F32_Sinh -> False + MO_F32_Cosh -> False + MO_F32_Tanh -> False + MO_F32_Asin -> False + MO_F32_Acos -> False + MO_F32_Atan -> False + MO_F32_Asinh -> False + MO_F32_Acosh -> False + MO_F32_Atanh -> False + MO_F32_Log -> False + MO_F32_Log1P -> False + MO_F32_Exp -> False + MO_F32_ExpM1 -> False + MO_F32_Fabs -> False + MO_F32_Sqrt -> False + + MO_I64_ToI -> False + MO_I64_FromI -> False + MO_W64_ToW -> False + MO_W64_FromW -> False + MO_x64_Neg -> False + MO_x64_Add -> False + MO_x64_Sub -> False + MO_x64_Mul -> False + MO_I64_Quot -> False + MO_I64_Rem -> False + MO_W64_Quot -> False + MO_W64_Rem -> False + MO_x64_And -> False + MO_x64_Or -> False + MO_x64_Xor -> False + MO_x64_Not -> False + MO_x64_Shl -> False + MO_I64_Shr -> False + MO_W64_Shr -> False + MO_x64_Eq -> False + MO_x64_Ne -> False + MO_I64_Ge -> False + MO_I64_Gt -> False + MO_I64_Le -> False + MO_I64_Lt -> False + MO_W64_Ge -> False + MO_W64_Gt -> False + MO_W64_Le -> False + MO_W64_Lt -> False + + MO_UF_Conv _ -> False + MO_S_Mul2 _ -> False + MO_S_QuotRem _ -> False + MO_U_QuotRem _ -> False + MO_U_QuotRem2 _ -> False + MO_Add2 _ -> False + MO_AddWordC _ -> False + MO_SubWordC _ -> False + MO_AddIntC _ -> False + MO_SubIntC _ -> False + MO_U_Mul2 _ -> False + + MO_VS_Quot _ _ -> False + MO_VS_Rem _ _ -> False + MO_VU_Quot _ _ -> False + MO_VU_Rem _ _ -> False + MO_I64X2_Min -> False + MO_I64X2_Max -> False + MO_W64X2_Min -> False + MO_W64X2_Max -> False + MO_Touch -> False -- pure liveness marker, no instructions MO_Prefetch_Data _ -> False -- reads memory, writes nothing - _ -> True + + MO_Memcpy _ -> True + MO_Memset _ -> True + MO_Memmove _ -> True + -- CQ[clobber-memcmp] + -- Q: memcmp reads both buffers and writes nothing, like prefetch. + -- Is there any RTS use where sinking a load past it matters? + MO_Memcmp _ -> False + + MO_PopCnt _ -> False + MO_Pdep _ -> False + MO_Pext _ -> False + MO_Clz _ -> False + MO_Ctz _ -> False + MO_BSwap _ -> False + MO_BRev _ -> False + + MO_AcquireFence -> True + MO_ReleaseFence -> True + MO_SeqCstFence -> True + MO_AtomicRMW {} -> True + -- CQ[clobber-atomic-read] + -- Q: an atomic read writes nothing. Why keep it clobbering? + -- A~ it carries an ordering: a plain load may not move past a seq-cst + -- read, and there is nothing to gain from allowing the relaxed case. + MO_AtomicRead {} -> True + MO_AtomicWrite {} -> True + MO_Cmpxchg _ -> True + MO_Xchg _ -> True + + MO_SuspendThread -> True + MO_ResumeThread -> True -- | The alignment of a 'memcpy'-ish operation. machOpMemcpyishAlign :: CallishMachOp -> Maybe Int ===================================== compiler/GHC/Cmm/Sink.hs ===================================== @@ -884,13 +884,23 @@ data AbsMem -- 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 +-- Conversely, most CallishMachOps neither write memory nor imply a -- barrier, and needn't block sinking ('callishMachOpClobbersMemory' in --- GHC.Cmm.MachOp classifies them): +-- GHC.Cmm.MachOp classifies them). Two deserve a word: +-- CQ[clobber-note-scope] +-- Q: the Note now says "most" but only explains touch# and prefetch. Where +-- is the argument for the arithmetic and libm ops? +-- A~ at callishMachOpClobbersMemory, see CQ[clobber-libm]. The +-- per-constructor rationale should live next to the classification, +-- the Note keeps the two cases that need a safety argument. +-- D~ move the touch#/prefetch bullets to the classification too, and have +-- the Note only point there. -- -- * 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. +-- The hazard of #18040, a read of the touched object delayed past a GC +-- safe point, cannot arise here: after stack layout the only safe points +-- are CmmCall nodes, which rule (C3)(d) still blocks, and +-- suspendThread/resumeThread, which stay clobbering. -- -- * MO_Prefetch_Data reads memory but writes nothing. ===================================== testsuite/tests/codeGen/should_gen_asm/T27782b.asm ===================================== @@ -0,0 +1,12 @@ +loadMul2: + movq + mulq + addq + movq + addq + jmp +loadBSwap: + bswapq + movq + addq + jmp ===================================== testsuite/tests/codeGen/should_gen_asm/T27782b.cmm ===================================== @@ -0,0 +1,19 @@ +#include "Cmm.h" + +// A load must sink past callish ops that don't clobber memory, so that it +// can be folded into the instruction that uses it. mul2u_64 also clobbers +// rax/rdx, so keeping the load live across it would cost extra moves. + +loadMul2 (W_ p, W_ x, W_ y) { + W_ a, hi, lo; + a = W_[p]; + (hi, lo) = prim %mul2u_64(x, y); + return (a + hi + lo); +} + +loadBSwap (W_ p, W_ x) { + W_ a, b; + a = W_[p]; + (b) = prim %bswap64(x); + return (a + b); +} ===================================== testsuite/tests/codeGen/should_gen_asm/all.T ===================================== @@ -28,6 +28,7 @@ test('T25233', [unless(arch('x86_64'), skip), test('T25233b', [unless(arch('x86_64'), skip), when(unregisterised(), skip)], compile_grep_asm, ['cmm', True, '']) test('T27782', is_amd64_codegen, compile_cmp_asm, ['cmm', '']) +test('T27782b', 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/-/compare/33f284d756ced7df18cd339911bdac1... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/33f284d756ced7df18cd339911bdac1... 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