[Git][ghc/ghc][wip/sjakobi/T27368-cbe-compress] Cmm: resolve substitution chains in common block elimination
Simon Jakobi pushed to branch wip/sjakobi/T27368-cbe-compress at Glasgow Haskell Compiler / GHC Commits: a9cb1e12 by Simon Jakobi at 2026-08-17T18:43:13+02:00 Cmm: resolve substitution chains in common block elimination The substitution built by elimCommonBlocks can map a label to another eliminated label, but replaceLabels looks up each label only once. An edge in a losing copy of a merged block could thus be rewritten to an eliminated label. When that label was a call continuation, callProcPoints turned it into a proc point, attachContInfoTables gave it an info table, but stack layout produced no stack map for it, and setInfoTableStackMap panicked. Resolve the substitution before rewriting, so that every label in the graph is mapped directly to its final representative. See Note [Resolving the CBE substitution] in GHC.Cmm.CommonBlockElim. Also: copyTicks now copies each loser's ticks into its final representative instead of into a dead intermediate block. The regression test distills the code shape that triggered the panic when compiling GHC.CmmToAsm.Dwarf.Types with -O2 on top of !16168. Fixes #27368 Assisted-by: Claude Fable 5 - - - - - 4 changed files: - + changelog.d/27368 - compiler/GHC/Cmm/CommonBlockElim.hs - + testsuite/tests/codeGen/should_compile/T27368.hs - testsuite/tests/codeGen/should_compile/all.T Changes: ===================================== changelog.d/27368 ===================================== @@ -0,0 +1,4 @@ +section: compiler +synopsis: Fix a ``setInfoTableStackMap`` panic caused by calls in unreachable Cmm blocks. +issues: #27368 +mrs: !16543 ===================================== compiler/GHC/Cmm/CommonBlockElim.hs ===================================== @@ -58,10 +58,44 @@ import qualified Data.List.NonEmpty as NE -- hashes, and at most once otherwise. Previously, we were slower, and people -- rightfully complained: #10397 +{- Note [Resolving the CBE substitution] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +The substitution built by `iterate` can contain chains: the winner of a +merge may itself lose a later merge, giving k1 :-> k2 and k2 :-> k3. +`lookupBid` resolves such chains transitively, but `replaceLabels` +looks up each label only once. So before rewriting the graph we +resolve the substitution, mapping every eliminated label directly to +its final representative. + +This matters because the losing blocks of the merges stay in the block +map, unreachable (removing them here would take an extra reachability +pass). With an unresolved +substitution, an edge in a losing block could be rewritten to k2 -- +itself an eliminated label with no live twin. In #27368 such a stranded +label was a call continuation: callProcPoints folds over the whole +block map, so the label became a proc point, attachContInfoTables gave +it an info table, but stack layout (which walks reachable blocks only) +produced no stack map for it, and setInfoTableStackMap panicked. + +With the substitution resolved, every edge -- including those in the +unreachable losing copies -- points at a surviving block, and every +surviving block is reachable: the input graph contains no unreachable +blocks (the control-flow optimiser runs first and drops them), and +merging only diverts paths from losers to their body-equal winners. +In particular the continuation of a call in a losing copy is also the +continuation of its reachable winner, so stack layout has a stack map +for it. + +Resolving also improves copyTicks: each loser's ticks are copied into +its final representative instead of into a dead intermediate block. +-} + -- TODO: Use optimization fuel elimCommonBlocks :: CmmGraph -> CmmGraph -elimCommonBlocks g = replaceLabels env $ copyTicks env g +elimCommonBlocks g = replaceLabels env' $ copyTicks env' g where + -- See Note [Resolving the CBE substitution] + env' = mapMap (lookupBid env) env env = iterate mapEmpty blocks_with_key -- The order of blocks doesn't matter here. While we could use -- revPostorder which drops unreachable blocks this is done in ===================================== testsuite/tests/codeGen/should_compile/T27368.hs ===================================== @@ -0,0 +1,20 @@ +-- Regression test for #27368: setInfoTableStackMap panicked because a +-- call in an unreachable block returned to an eliminated label. The two +-- branches below have identical suffixes from the inner case onwards, +-- so common block elimination merges the duplicated call blocks in +-- several rounds, building a substitution chain. See Note [Resolving +-- the CBE substitution] in GHC.Cmm.CommonBlockElim. +module T27368 (f) where + +{-# NOINLINE put #-} +put :: Int -> Int -> IO () +put h x = if h + x == 12345 then errorWithoutStackTrace "boom" else pure () + +data T = N | J Int | K + +f :: Int -> Bool -> T -> IO () +f h a t = do + if a + then do put h 1; case t of { N -> pure (); J _ -> put h 3; K -> put h 4 }; put h 0; put h 0 + else do put h 2; case t of { N -> pure (); J _ -> put h 3; K -> put h 4 }; put h 0; put h 0 + put h 0 ===================================== testsuite/tests/codeGen/should_compile/all.T ===================================== @@ -150,3 +150,5 @@ test('T16351', normal, compile, ['-O2 -ddump-simpl -dno-typeable-binds -dsuppres test('T20298a', normal, compile, ['-O2 -ddump-simpl -dno-typeable-binds -dsuppress-all -dsuppress-uniques']) test('T20298b', normal, compile, ['-O2 -dno-bignum-rules -ddump-simpl -dno-typeable-binds -dsuppress-all -dsuppress-uniques']) test('T20298c', normal, compile, ['-O2 -dno-builtin-rules -ddump-simpl -dno-typeable-binds -dsuppress-all -dsuppress-uniques']) + +test('T27368', normal, compile, ['-O2']) View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/a9cb1e12cfaee716f77c74e3e5bc81e7... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/a9cb1e12cfaee716f77c74e3e5bc81e7... 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
participants (1)
-
Simon Jakobi (@sjakobi)