Magnus pushed to branch wip/mangoiv/9.12.5-rc4 at Glasgow Haskell Compiler / GHC Commits: 785781c5 by Zubin Duggal at 2026-09-04T16:24:41+02:00 CorePrep: don't speculate a call across an hs-boot edge We take care not to evaluate things that might be bottom, like a looping dictionary group, but our analysis is defeated by boot files. We only track recursion within a module, so two dictionaries that depend on each other across a module loop each look non-recursive, and we might speculate them. Any recursion we cannot see must cross an hs-boot edge, so refuse to speculate calls that cross one. Fixes #27717 (cherry picked from commit 664efc76286c475e0fcd1c1624e847831e7fc07a) - - - - - 9 changed files: - + changelog.d/27717 - compiler/GHC/CoreToStg/Prep.hs - + testsuite/tests/core-to-stg/T27717/Callee.hs - + testsuite/tests/core-to-stg/T27717/Callee.hs-boot - + testsuite/tests/core-to-stg/T27717/Main.hs - + testsuite/tests/core-to-stg/T27717/Mid.hs - + testsuite/tests/core-to-stg/T27717/T27717.stdout - + testsuite/tests/core-to-stg/T27717/Ty.hs - + testsuite/tests/core-to-stg/T27717/all.T Changes: ===================================== changelog.d/27717 ===================================== @@ -0,0 +1,6 @@ +section: compiler +synopsis: Fix a bug where -fspec-eval-dictfun could speculatively evaluate a + looping dictionary whose recursion went through an hs-boot import, hanging + the program. +mrs: !16519 +issues: #27717 ===================================== compiler/GHC/CoreToStg/Prep.hs ===================================== @@ -1892,87 +1892,131 @@ long as the callee might evaluate it. And if it is evaluated on most code paths anyway, we get to turn the unknown eval in the callee into a known call at the call site. -Very Nasty Wrinkle +(SEV1) There is a very nasty wrinkle: we must be very careful not to speculate + recursive calls! Doing so might well change termination behavior. + + That comes up in practice for DFuns, which are considered ok-for-spec, + because they always immediately return a constructor. + See Note [NON-BOTTOM-DICTS invariant] in GHC.Core. + + But not so if you speculate the recursive call, as #20836 shows: + + class Foo m => Foo m where + runFoo :: m a -> m a + newtype Trans m a = Trans { runTrans :: m a } + instance Monad m => Foo (Trans m) where + runFoo = id + + (NB: class Foo m => Foo m` looks weird and needs -XUndecidableSuperClasses. The + example in #20836 is more compelling, but boils down to the same thing.) + This program compiles to the following DFun for the `Trans` instance: + + Rec { + $fFooTrans + = \ @m $dMonad -> C:Foo ($fFooTrans $dMonad) (\ @a -> id) + end Rec } + + Note that the DFun immediately terminates and produces a dictionary, just + like DFuns ought to, but it calls itself recursively to produce the `Foo m` + dictionary. But alas, if we treat `$fFooTrans` as always-terminating, so + that we can speculate its calls, and hence use call-by-value, we get: + + $fFooTrans + = \ @m $dMonad -> case ($fFooTrans $dMonad) of sc -> + C:Foo sc (\ @a -> id) + + and that's an infinite loop! + Note that this bad-ness only happens in `$fFooTrans`'s own RHS. In the + *body* of the letrec, it's absolutely fine to use call-by-value on + `foo ($fFooTrans d)`. + + Our solution is this: we track in cpe_rec_ids the set of enclosing + recursively-bound Ids, the RHSs of which we are currently transforming and then + in 'exprOkForSpecEval' (a special entry point to 'exprOkForSpeculation', + basically) we'll say that any binder in this set is not ok-for-spec. + + Note if we have a letrec group `Rec { f1 = rhs1; ...; fn = rhsn }`, and we + prep up `rhs1`, we have to include not only `f1`, but all binders of the group + `f1..fn` in this set, otherwise our fix is not robust wrt. mutual recursive + DFuns. + + NB: If at some point we decide to have a termination analysis for general + functions (#8655, !1866), we need to take similar precautions for (guarded) + recursive functions: + + repeat x = x : repeat x + + Same problem here: As written, repeat evaluates rapidly to WHNF. So `repeat x` + is a cheap call that we are willing to speculate, but *not* in repeat's RHS. + Fortunately, pce_rec_ids already has all the information we need in that case. + + The problem is very similar to Note [Eta reduction in recursive RHSs]. + Here as well as there it is *unsound* to change the termination properties + of the very function whose termination properties we are exploiting. + + It is also similar to Note [Do not strictify a DFun's parameter dictionaries], + where marking recursive DFuns (of undecidable *instances*) strict in dictionary + *parameters* leads to quite the same change in termination as above. + +(SEV2) The situation in (SEV1) can cross modules. `cpe_rec_ids` is module-local, + so it does not catch a recursion that goes through an hs-boot import. + Suppose Mid and Callee form a module loop, and Mid sees Callee through its + hs-boot file: + + -- Mid.hs + $fCAT = \ @a $dCB -> C:CA ($fxCBT $dCB) ... + -- Callee.hs + $fCBT = \ @a $dCB -> C:CB ($fCAT $dCB) ... + + Neither module can see that these two call each other, so each looks + non-recursive, and we speculate the inner call in both: + + $fCAT = \ @a $dCB -> case $fxCBT $dCB of s { __DEFAULT -> C:CA s ... } + $fCBT = \ @a $dCB -> case $fCAT $dCB of s { __DEFAULT -> C:CB s ... } + + Now each forces the other and the program loops. Such a cycle must cross an + hs-boot edge, so we do not speculate a call whose callee has a BootUnfolding. + Note [Inlining and hs-boot files] in GHC.CoreToIface does the same for + infinite inlining. + +(SEV3) Belt and braces: do not speculate absent bindings. + + In 'decideFloatInfo' we decline to speculate a binding whose demand is + absent. There is no point in speculating an absent binding, since its + value is (presumably) not needed. + + This used to matter more. Worker/wrapper would bind an absent dictionary + to a rubbish literal filler, and speculation could force a superclass + selection out of that rubbish literal, causing a segfault (#25924). + Nowadays we never make a filler for a dictionary in the first place, so + this can no longer happen and the guard is merely belt and braces. + See Note [Don't make fillers for constraint types] + in GHC.Core.Opt.WorkWrap.Utils. -We must be very careful not to speculate recursive calls! Doing so -might well change termination behavior. +Note [Controlling Speculative Evaluation] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -That comes up in practice for DFuns, which are considered ok-for-spec, -because they always immediately return a constructor. -See Note [NON-BOTTOM-DICTS invariant] in GHC.Core. +Most of the time, speculative evaluation in the coreprep phase has a positive +effect on performance, however we have found that some forms of speculative +evaluation can lead to large performance regressions. See #25284. -But not so if you speculate the recursive call, as #20836 shows: +Therefore we have some flags to control which types of speculative evaluation +are done: - class Foo m => Foo m where - runFoo :: m a -> m a - newtype Trans m a = Trans { runTrans :: m a } - instance Monad m => Foo (Trans m) where - runFoo = id + -fspec-eval + Globally enable/disable speculative evaluation ( -fno-spec-eval also turns + off all other speculative evaluation). On by default for all + optimization levels. Turning on this flag by itself should never cause + a performance regression. Please open a ticket if you find any. -(NB: class Foo m => Foo m` looks weird and needs -XUndecidableSuperClasses. The -example in #20836 is more compelling, but boils down to the same thing.) -This program compiles to the following DFun for the `Trans` instance: + -fspec-eval-dictfun + Enable speculative evaluation for dictionary functions. Off by default + since it can cause an increase in allocations (#24284). We have no + examples that show a large performance improvement when turning on this + flag. Please open a ticket if you find any. - Rec { - $fFooTrans - = \ @m $dMonad -> C:Foo ($fFooTrans $dMonad) (\ @a -> id) - end Rec } - -Note that the DFun immediately terminates and produces a dictionary, just -like DFuns ought to, but it calls itself recursively to produce the `Foo m` -dictionary. But alas, if we treat `$fFooTrans` as always-terminating, so -that we can speculate its calls, and hence use call-by-value, we get: - - $fFooTrans - = \ @m $dMonad -> case ($fFooTrans $dMonad) of sc -> - C:Foo sc (\ @a -> id) - -and that's an infinite loop! -Note that this bad-ness only happens in `$fFooTrans`'s own RHS. In the -*body* of the letrec, it's absolutely fine to use call-by-value on -`foo ($fFooTrans d)`. - -Our solution is this: we track in cpe_rec_ids the set of enclosing -recursively-bound Ids, the RHSs of which we are currently transforming and then -in 'exprOkForSpecEval' (a special entry point to 'exprOkForSpeculation', -basically) we'll say that any binder in this set is not ok-for-spec. - -Note if we have a letrec group `Rec { f1 = rhs1; ...; fn = rhsn }`, and we -prep up `rhs1`, we have to include not only `f1`, but all binders of the group -`f1..fn` in this set, otherwise our fix is not robust wrt. mutual recursive -DFuns. - -NB: If at some point we decide to have a termination analysis for general -functions (#8655, !1866), we need to take similar precautions for (guarded) -recursive functions: - - repeat x = x : repeat x - -Same problem here: As written, repeat evaluates rapidly to WHNF. So `repeat x` -is a cheap call that we are willing to speculate, but *not* in repeat's RHS. -Fortunately, pce_rec_ids already has all the information we need in that case. - -The problem is very similar to Note [Eta reduction in recursive RHSs]. -Here as well as there it is *unsound* to change the termination properties -of the very function whose termination properties we are exploiting. - -It is also similar to Note [Do not strictify a DFun's parameter dictionaries], -where marking recursive DFuns (of undecidable *instances*) strict in dictionary -*parameters* leads to quite the same change in termination as above. - -Belt and braces: do not speculate absent bindings - -In 'decideFloatInfo' we decline to speculate a binding whose demand is absent. -There is no point in speculating an absent binding, since its value is -(presumably) not needed. - -This used to matter more. Worker/wrapper would bind an absent dictionary to a -rubbish literal filler, and speculation could force a superclass selection out -of that rubbish literal, causing a segfault (#25924). Nowadays we never make a -filler for a dictionary in the first place, so this can no longer happen and -the guard is merely belt and braces. -See Note [Don't make fillers for constraint types] -in GHC.Core.Opt.WorkWrap.Utils. +Also see the optimization section in the User's Guide for the description of +these flags and when to use them. Note [BindInfo and FloatInfo] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -2030,16 +2074,6 @@ conceptually. See also Note [Floats and FloatDecision] for how we maintain whole groups of floats and how far they go. -Note [Controlling Speculative Evaluation] -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Most of the time, speculative evaluation has a positive effect on performance, -but we have found a case where speculative evaluation of dictionary functions -leads to a performance regression #25284. - -Therefore we have some flags to control it. See the optimization section in -the User's Guide for the description of these flags and when to use them. - Note [Floats and FloatDecision] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ We have a special datatype `Floats` for modelling a telescope of `FloatingBind` @@ -2243,11 +2277,18 @@ mkNonRecFloat env is_unlifted bndr rhs ok_for_spec = exprOkForSpecEval call_ok_for_spec rhs -- See Note [Controlling Speculative Evaluation] call_ok_for_spec x - | is_rec_call x = False - | not (cp_specEval cfg) = False - | not (cp_specEvalDFun cfg) && isDFunId x = False + -- See Note [Speculative evaluation] + | is_rec_call x = False -- See (SEV1) + | is_boot_call x = False -- See (SEV2) + + -- See [Controlling speculative evaluation] + | not (cp_specEval cfg) = False -- Flag -fspec-eval + | not (cp_specEvalDFun cfg) && isDFunId x = False -- Flag -fspec-eval-dictfun + | otherwise = True - is_rec_call = (`elemUnVarSet` cpe_rec_ids env) + is_rec_call = (`elemUnVarSet` cpe_rec_ids env) + is_boot_call = isBootUnfolding . realIdUnfolding + -- See Note [Speculative evaluation], Very Nasty Wrinkle is_data_con = isJust . isDataConId_maybe -- | Wrap floats around an expression ===================================== testsuite/tests/core-to-stg/T27717/Callee.hs ===================================== @@ -0,0 +1,11 @@ +{-# LANGUAGE UndecidableInstances, FlexibleInstances, FlexibleContexts #-} +module Callee where + +import Ty +import Mid + +instance CB a => CB (T a) where + opB _ = 2 + +instance CB Int where + opB _ = 8 ===================================== testsuite/tests/core-to-stg/T27717/Callee.hs-boot ===================================== @@ -0,0 +1,7 @@ +{-# LANGUAGE FlexibleInstances, UndecidableInstances #-} +module Callee where + +import Ty + +instance CB a => CB (T a) +instance CB Int ===================================== testsuite/tests/core-to-stg/T27717/Main.hs ===================================== @@ -0,0 +1,6 @@ +module Main where +import Mid +import Callee () + +main :: IO () +main = print (g (1 :: Int)) ===================================== testsuite/tests/core-to-stg/T27717/Mid.hs ===================================== @@ -0,0 +1,26 @@ +{-# LANGUAGE UndecidableInstances, FlexibleInstances, FlexibleContexts #-} +{-# LANGUAGE GADTs, ConstraintKinds, ScopedTypeVariables #-} +module Mid where + +import Ty +import {-# SOURCE #-} Callee + +-- $fCATa calls Callee's $fxCBT, and that calls this one back. The cycle runs +-- through the hs-boot import, so both dfuns are NonRec in their own module and +-- cpe_rec_ids cannot see it. Speculating either one loops. +instance CB a => CA (T a) where + opA _ = 1 + +instance CA Int where + opA _ = 9 + +data Dict c where + Dict :: c => Dict c + +{-# NOINLINE keep #-} +keep :: [Dict c] -> Int +keep xs = length xs + 41 + +{-# NOINLINE g #-} +g :: forall a. CB a => a -> Int +g _ = keep [Dict :: Dict (CA (T a))] ===================================== testsuite/tests/core-to-stg/T27717/T27717.stdout ===================================== @@ -0,0 +1 @@ +42 ===================================== testsuite/tests/core-to-stg/T27717/Ty.hs ===================================== @@ -0,0 +1,10 @@ +{-# LANGUAGE UndecidableSuperClasses #-} +module Ty where + +data T a = MkT a + +class CB a => CA a where + opA :: a -> Int + +class CA a => CB a where + opB :: a -> Int ===================================== testsuite/tests/core-to-stg/T27717/all.T ===================================== @@ -0,0 +1,4 @@ +test('T27717', + [extra_files(['Main.hs', 'Mid.hs', 'Ty.hs', 'Callee.hs', 'Callee.hs-boot'])], + multimod_compile_and_run, + ['Main', '-O -fspec-eval-dictfun']) View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/785781c580f67e65790e40c5c0fb5c77... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/785781c580f67e65790e40c5c0fb5c77... 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