Rodrigo Mesquita pushed to branch wip/romes/step-out-9 at Glasgow Haskell Compiler / GHC WARNING: The push did not contain any new commits, but force pushed to delete the commits and changes below. Deleted commits: 5178fc25 by Rodrigo Mesquita at 2025-07-02T17:24:52+01:00 ghci: Allocate BreakArrays at link time only Previously, a BreakArray would be allocated with a slot for every tick in a module at `mkModBreaks`, in HsToCore. However, this approach has a few downsides: - It interleaves interpreter behaviour (allocating arrays for breakpoints) within the desugarer - It is inflexible in the sense it is impossible for the bytecode generator to add "internal" breakpoints that can be triggered at runtime, because those wouldn't have a source tick. (This is relevant for our intended implementation plan of step-out in #26042) - It ties the BreakArray indices to the *tick* indexes, while at runtime we would rather just have the *info* indexes (currently we have both because BreakArrays are indexed by the *tick* one). Paving the way for #26042 and #26064, this commit moves the allocation of BreakArrays to bytecode-loading time -- akin to what is done for CCS arrays. Since a BreakArray is allocated only when bytecode is linked, if a breakpoint is set (e.g. `:break 10`) before the bytecode is linked, there will exist no BreakArray to trigger the breakpoint in. Therefore, the function to allocate break arrays (`allocateBreakArrays`) is exposed and also used in GHC.Runtime.Eval to allocate a break array when a breakpoint is set, if it doesn't exist yet (in the linker env). - - - - - 8 changed files: - compiler/GHC/HsToCore.hs - compiler/GHC/HsToCore/Breakpoints.hs - compiler/GHC/Linker/Loader.hs - compiler/GHC/Runtime/Eval.hs - − compiler/GHC/Runtime/Interpreter.hs-boot - − compiler/GHC/Runtime/Interpreter/Types.hs-boot - testsuite/tests/count-deps/CountDepsAst.stdout - testsuite/tests/count-deps/CountDepsParser.stdout Changes: ===================================== compiler/GHC/HsToCore.hs ===================================== @@ -97,8 +97,8 @@ import GHC.Unit.Module.Deps import Data.List (partition) import Data.IORef -import Data.Traversable (for) import GHC.Iface.Make (mkRecompUsageInfo) +import GHC.Runtime.Interpreter (interpreterProfiled) {- ************************************************************************ @@ -162,13 +162,12 @@ deSugar hsc_env mod mod_loc export_set (typeEnvTyCons type_env) binds else return (binds, Nothing) - ; modBreaks <- for - [ (i, s) - | i <- hsc_interp hsc_env - , (_, s) <- m_tickInfo - , breakpointsAllowed dflags - ] - $ \(interp, specs) -> mkModBreaks interp mod specs + ; let modBreaks + | Just (_, specs) <- m_tickInfo + , breakpointsAllowed dflags + = Just $ mkModBreaks (interpreterProfiled $ hscInterp hsc_env) mod specs + | otherwise + = Nothing ; ds_hpc_info <- case m_tickInfo of Just (orig_file2, ticks) ===================================== compiler/GHC/HsToCore/Breakpoints.hs ===================================== @@ -33,14 +33,6 @@ import GHC.Unit.Module (Module) import GHC.Utils.Outputable import Data.List (intersperse) -import GHCi.BreakArray (BreakArray) -import GHCi.RemoteTypes (ForeignRef) - --- TODO: Break this cycle -import {-# SOURCE #-} GHC.Runtime.Interpreter.Types (Interp, interpreterProfiled) -import {-# SOURCE #-} qualified GHC.Runtime.Interpreter as GHCi (newBreakArray) -import Data.Array.Base (numElements) - -------------------------------------------------------------------------------- -- ModBreaks -------------------------------------------------------------------------------- @@ -58,10 +50,7 @@ import Data.Array.Base (numElements) -- and 'modBreaks_decls'. data ModBreaks = ModBreaks - { modBreaks_flags :: ForeignRef BreakArray - -- ^ The array of flags, one per breakpoint, - -- indicating which breakpoints are enabled. - , modBreaks_locs :: !(Array BreakTickIndex SrcSpan) + { modBreaks_locs :: !(Array BreakTickIndex SrcSpan) -- ^ An array giving the source span of each breakpoint. , modBreaks_vars :: !(Array BreakTickIndex [OccName]) -- ^ An array giving the names of the free variables at each breakpoint. @@ -83,40 +72,31 @@ data ModBreaks -- generator needs to encode this information for each expression, the data is -- allocated remotely in GHCi's address space and passed to the codegen as -- foreign pointers. -mkModBreaks :: Interp -> Module -> SizedSeq Tick -> IO ModBreaks -mkModBreaks interp mod extendedMixEntries - = do - let count = fromIntegral $ sizeSS extendedMixEntries +mkModBreaks :: Bool {-^ Whether the interpreter is profiled and thus if we should include store a CCS array -} + -> Module -> SizedSeq Tick -> ModBreaks +mkModBreaks interpreterProfiled modl extendedMixEntries + = let count = fromIntegral $ sizeSS extendedMixEntries entries = ssElts extendedMixEntries - let - locsTicks = listArray (0,count-1) [ tick_loc t | t <- entries ] - varsTicks = listArray (0,count-1) [ tick_ids t | t <- entries ] - declsTicks = listArray (0,count-1) [ tick_path t | t <- entries ] - ccs - | interpreterProfiled interp = - listArray - (0, count - 1) - [ ( concat $ intersperse "." $ tick_path t, - renderWithContext defaultSDocContext $ ppr $ tick_loc t - ) - | t <- entries - ] - | otherwise = listArray (0, -1) [] - hydrateModBreaks interp $ - ModBreaks - { modBreaks_flags = undefined, - modBreaks_locs = locsTicks, - modBreaks_vars = varsTicks, - modBreaks_decls = declsTicks, - modBreaks_ccs = ccs, - modBreaks_module = mod - } - -hydrateModBreaks :: Interp -> ModBreaks -> IO ModBreaks -hydrateModBreaks interp ModBreaks {..} = do - let count = numElements modBreaks_locs - modBreaks_flags <- GHCi.newBreakArray interp count - pure ModBreaks {..} + locsTicks = listArray (0,count-1) [ tick_loc t | t <- entries ] + varsTicks = listArray (0,count-1) [ tick_ids t | t <- entries ] + declsTicks = listArray (0,count-1) [ tick_path t | t <- entries ] + ccs + | interpreterProfiled = + listArray + (0, count - 1) + [ ( concat $ intersperse "." $ tick_path t, + renderWithContext defaultSDocContext $ ppr $ tick_loc t + ) + | t <- entries + ] + | otherwise = listArray (0, -1) [] + in ModBreaks + { modBreaks_locs = locsTicks + , modBreaks_vars = varsTicks + , modBreaks_decls = declsTicks + , modBreaks_ccs = ccs + , modBreaks_module = modl + } {- Note [Field modBreaks_decls] ===================================== compiler/GHC/Linker/Loader.hs ===================================== @@ -28,6 +28,7 @@ module GHC.Linker.Loader , extendLoadedEnv , deleteFromLoadedEnv -- * Internals + , allocateBreakArrays , rmDupLinkables , modifyLoaderState , initLinkDepsOpts @@ -122,8 +123,8 @@ import System.Win32.Info (getSystemDirectory) import GHC.Utils.Exception import GHC.Unit.Home.Graph (lookupHug, unitEnv_foldWithKey) import GHC.Driver.Downsweep - - +import qualified GHC.Runtime.Interpreter as GHCi +import Data.Array.Base (numElements) -- Note [Linkers and loaders] -- ~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -696,16 +697,8 @@ loadDecls interp hsc_env span linkable = do let le = linker_env pls le2_itbl_env <- linkITbls interp (itbl_env le) (concat $ map bc_itbls cbcs) le2_addr_env <- foldlM (\env cbc -> allocateTopStrings interp (bc_strs cbc) env) (addr_env le) cbcs - le2_breakarray_env <- - allocateBreakArrays - interp - (catMaybes $ map bc_breaks cbcs) - (breakarray_env le) - le2_ccs_env <- - allocateCCS - interp - (catMaybes $ map bc_breaks cbcs) - (ccs_env le) + le2_breakarray_env <- allocateBreakArrays interp (breakarray_env le) (catMaybes $ map bc_breaks cbcs) + le2_ccs_env <- allocateCCS interp (ccs_env le) (catMaybes $ map bc_breaks cbcs) let le2 = le { itbl_env = le2_itbl_env , addr_env = le2_addr_env , breakarray_env = le2_breakarray_env @@ -933,12 +926,8 @@ dynLinkBCOs interp pls bcos = do le1 = linker_env pls ie2 <- linkITbls interp (itbl_env le1) (concatMap bc_itbls cbcs) ae2 <- foldlM (\env cbc -> allocateTopStrings interp (bc_strs cbc) env) (addr_env le1) cbcs - be2 <- - allocateBreakArrays - interp - (catMaybes $ map bc_breaks cbcs) - (breakarray_env le1) - ce2 <- allocateCCS interp (catMaybes $ map bc_breaks cbcs) (ccs_env le1) + be2 <- allocateBreakArrays interp (breakarray_env le1) (catMaybes $ map bc_breaks cbcs) + ce2 <- allocateCCS interp (ccs_env le1) (catMaybes $ map bc_breaks cbcs) let le2 = le1 { itbl_env = ie2, addr_env = ae2, breakarray_env = be2, ccs_env = ce2 } names_and_refs <- linkSomeBCOs interp (pkgs_loaded pls) le2 cbcs @@ -1656,30 +1645,34 @@ allocateTopStrings interp topStrings prev_env = do where mk_entry nm ptr = (nm, (nm, AddrPtr ptr)) --- | Given a list of 'ModBreaks' collected from a list of --- 'CompiledByteCode', allocate the 'BreakArray'. +-- | Given a list of 'InternalModBreaks' collected from a list of +-- 'CompiledByteCode', allocate the 'BreakArray' used to trigger breakpoints. allocateBreakArrays :: Interp -> - [InternalModBreaks] -> ModuleEnv (ForeignRef BreakArray) -> + [InternalModBreaks] -> IO (ModuleEnv (ForeignRef BreakArray)) -allocateBreakArrays _interp mbs be = +allocateBreakArrays interp = foldlM - ( \be0 InternalModBreaks{imodBreaks_modBreaks=ModBreaks {..}} -> - evaluate $ extendModuleEnv be0 modBreaks_module modBreaks_flags + ( \be0 InternalModBreaks{imodBreaks_modBreaks=ModBreaks {..}} -> do + -- If no BreakArray is assigned to this module yet, create one + if not $ elemModuleEnv modBreaks_module be0 then do + let count = numElements modBreaks_locs + breakArray <- GHCi.newBreakArray interp count + evaluate $ extendModuleEnv be0 modBreaks_module breakArray + else + return be0 ) - be - mbs --- | Given a list of 'ModBreaks' collected from a list of --- 'CompiledByteCode', allocate the 'CostCentre' arrays when profiling --- is enabled. +-- | Given a list of 'InternalModBreaks' collected from a list +-- of 'CompiledByteCode', allocate the 'CostCentre' arrays when profiling is +-- enabled. allocateCCS :: Interp -> - [InternalModBreaks] -> ModuleEnv (Array BreakTickIndex (RemotePtr CostCentre)) -> + [InternalModBreaks] -> IO (ModuleEnv (Array BreakTickIndex (RemotePtr CostCentre))) -allocateCCS interp mbs ce +allocateCCS interp ce mbss | interpreterProfiled interp = foldlM ( \ce0 InternalModBreaks{imodBreaks_modBreaks=ModBreaks {..}} -> do @@ -1688,12 +1681,15 @@ allocateCCS interp mbs ce interp (moduleNameString $ moduleName modBreaks_module) (elems modBreaks_ccs) - evaluate $ - extendModuleEnv ce0 modBreaks_module $ - listArray - (0, length ccs - 1) - ccs + if not $ elemModuleEnv modBreaks_module ce0 then do + evaluate $ + extendModuleEnv ce0 modBreaks_module $ + listArray + (0, length ccs - 1) + ccs + else + return ce0 ) ce - mbs + mbss | otherwise = pure ce ===================================== compiler/GHC/Runtime/Eval.hs ===================================== @@ -64,6 +64,7 @@ import GHCi.RemoteTypes import GHC.ByteCode.Types import GHC.Linker.Loader as Loader +import GHC.Linker.Types (LinkerEnv(..)) import GHC.Hs @@ -126,6 +127,7 @@ import GHC.Tc.Utils.Instantiate (instDFunType) import GHC.Tc.Utils.Monad import GHC.IfaceToCore +import GHC.ByteCode.Breakpoints import Control.Monad import Data.Dynamic @@ -134,7 +136,7 @@ import Data.List (find,intercalate) import Data.List.NonEmpty (NonEmpty) import Unsafe.Coerce ( unsafeCoerce ) import qualified GHC.Unit.Home.Graph as HUG -import GHC.ByteCode.Breakpoints +import GHCi.BreakArray (BreakArray) -- ----------------------------------------------------------------------------- -- running a statement interactively @@ -348,13 +350,14 @@ handleRunStatus step expr bindings final_ids status history0 = do EvalBreak apStack_ref (Just eval_break) resume_ctxt ccs -> do let ibi = evalBreakpointToId eval_break let hug = hsc_HUG hsc_env - tick_brks <- liftIO $ readModBreaks hug (ibi_tick_mod ibi) + tick_brks <- liftIO $ readModBreaks hug (ibi_tick_mod ibi) + breakArray <- getBreakArray interp (toBreakpointId ibi) tick_brks let span = getBreakLoc ibi tick_brks decl = intercalate "." $ getBreakDecls ibi tick_brks -- Was this breakpoint explicitly enabled (ie. in @BreakArray@)? - bactive <- liftIO $ breakpointStatus interp (modBreaks_flags $ imodBreaks_modBreaks tick_brks) (ibi_tick_index ibi) + bactive <- liftIO $ breakpointStatus interp breakArray (ibi_tick_index ibi) apStack_fhv <- liftIO $ mkFinalizedHValue interp apStack_ref resume_ctxt_fhv <- liftIO $ mkFinalizedHValue interp resume_ctxt @@ -462,9 +465,24 @@ setupBreakpoint :: GhcMonad m => Interp -> BreakpointId -> Int -> m () -- #191 setupBreakpoint interp bi cnt = do hug <- hsc_HUG <$> getSession modBreaks <- liftIO $ readModBreaks hug (bi_tick_mod bi) - let breakarray = modBreaks_flags $ imodBreaks_modBreaks modBreaks - _ <- liftIO $ GHCi.storeBreakpoint interp breakarray (bi_tick_index bi) cnt - pure () + breakArray <- getBreakArray interp bi modBreaks + liftIO $ GHCi.storeBreakpoint interp breakArray (bi_tick_index bi) cnt + +getBreakArray :: GhcMonad m => Interp -> BreakpointId -> InternalModBreaks -> m (ForeignRef BreakArray) +getBreakArray interp BreakpointId{bi_tick_mod} imbs = do + + liftIO $ modifyLoaderState interp $ \ld_st -> do + let le = linker_env ld_st + + -- Recall that BreakArrays are allocated only at BCO link time, so if we + -- haven't linked the BCOs we intend to break at yet, we allocate the arrays here. + ba_env <- allocateBreakArrays interp (breakarray_env le) [imbs] + + return + ( ld_st { linker_env = le{breakarray_env = ba_env} } + , expectJust {- just computed -} $ + lookupModuleEnv ba_env bi_tick_mod + ) back :: GhcMonad m => Int -> m ([Name], Int, SrcSpan) back n = moveHist (+n) ===================================== compiler/GHC/Runtime/Interpreter.hs-boot deleted ===================================== @@ -1,10 +0,0 @@ -module GHC.Runtime.Interpreter where - -import {-# SOURCE #-} GHC.Runtime.Interpreter.Types -import Data.Int (Int) -import GHC.Base (IO) -import GHCi.BreakArray (BreakArray) -import GHCi.RemoteTypes (ForeignRef) - -newBreakArray :: Interp -> Int -> IO (ForeignRef BreakArray) - ===================================== compiler/GHC/Runtime/Interpreter/Types.hs-boot deleted ===================================== @@ -1,6 +0,0 @@ -module GHC.Runtime.Interpreter.Types where - -import Data.Bool - -data Interp -interpreterProfiled :: Interp -> Bool ===================================== testsuite/tests/count-deps/CountDepsAst.stdout ===================================== @@ -5,6 +5,7 @@ GHC.Builtin.Types GHC.Builtin.Types.Literals GHC.Builtin.Types.Prim GHC.Builtin.Uniques +GHC.ByteCode.Breakpoints GHC.ByteCode.Types GHC.Cmm.BlockId GHC.Cmm.CLabel @@ -110,6 +111,8 @@ GHC.Hs.Pat GHC.Hs.Specificity GHC.Hs.Type GHC.Hs.Utils +GHC.HsToCore.Breakpoints +GHC.HsToCore.Ticks GHC.Iface.Errors.Types GHC.Iface.Ext.Fields GHC.Iface.Flags @@ -150,7 +153,6 @@ GHC.Tc.Zonk.Monad GHC.Types.Annotations GHC.Types.Avail GHC.Types.Basic -GHC.Types.Breakpoint GHC.Types.CostCentre GHC.Types.CostCentre.State GHC.Types.Cpr ===================================== testsuite/tests/count-deps/CountDepsParser.stdout ===================================== @@ -5,6 +5,7 @@ GHC.Builtin.Types GHC.Builtin.Types.Literals GHC.Builtin.Types.Prim GHC.Builtin.Uniques +GHC.ByteCode.Breakpoints GHC.ByteCode.Types GHC.Cmm.BlockId GHC.Cmm.CLabel @@ -114,8 +115,10 @@ GHC.Hs.Pat GHC.Hs.Specificity GHC.Hs.Type GHC.Hs.Utils +GHC.HsToCore.Breakpoints GHC.HsToCore.Errors.Types GHC.HsToCore.Pmc.Solver.Types +GHC.HsToCore.Ticks GHC.Iface.Errors.Types GHC.Iface.Ext.Fields GHC.Iface.Flags @@ -171,7 +174,6 @@ GHC.Tc.Zonk.Monad GHC.Types.Annotations GHC.Types.Avail GHC.Types.Basic -GHC.Types.Breakpoint GHC.Types.CompleteMatch GHC.Types.CostCentre GHC.Types.CostCentre.State View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/5178fc25ee63483c67ce10f3b2e57a76... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/5178fc25ee63483c67ce10f3b2e57a76... You're receiving this email because of your account on gitlab.haskell.org.