GitLab

Ben Gamari pushed to branch wip/ipe-return-prefer-current-mod at Glasgow Haskell Compiler / GHC

Commits:

ab46ad97

by Ben Gamari at 2026-06-19T18:08:40-04:00

compiler/ipe: Prefer source ticks in current module for return frames

Previously we would simply attribute return frames to the nearest
enclosing tick in the calling frame. However, this will very frequently
produce unhelpful results (e.g. pointing to `(>>)` rather than the
calling function).

2 changed files:

compiler/GHC/Driver/GenerateCgIPEStub.hs
compiler/GHC/Driver/Main/Compile.hs

Changes:

compiler/GHC/Driver/GenerateCgIPEStub.hs

  module GHC.Driver.GenerateCgIPEStub (generateCgIPEStub, lookupEstimatedTicks) where
 +import Control.Applicative ((<|>))
  import Data.Map.Strict (Map)
  import qualified Data.Map.Strict as Map
 +import Data.Maybe (listToMaybe)
  import Data.Semigroup ((<>))
  import GHC.Cmm
  import GHC.Cmm.CLabel (CLabel, mkAsmTempLabel)
@@ -10,6 +12,7 @@ import GHC.Cmm.Dataflow.Block (blockSplit, blockToList)
  import GHC.Cmm.Dataflow.Label
  import GHC.Cmm.Info.Build (emptySRT)
  import GHC.Cmm.Pipeline (cmmPipeline)
 +import GHC.Data.FastString (FastString, mkFastString)
  import GHC.Data.Stream (liftIO, liftEff)
  import qualified GHC.Data.Stream as Stream
  import GHC.Driver.Env (hsc_dflags, hsc_logger)
@@ -28,9 +31,10 @@ import GHC.StgToCmm.Utils
  import GHC.StgToCmm.CgUtils (CgStream)
  import GHC.Types.IPE (InfoTableProvMap (provInfoTables), IpeSourceLocation)
  import GHC.Types.Name.Set (NonCaffySet)
 +import GHC.Types.SrcLoc (srcSpanFile)
  import GHC.Types.Tickish (GenTickish (SourceNote))
  import GHC.Unit.Types (Module, moduleName)
 -import GHC.Unit.Module (moduleNameString)
 +import GHC.Unit.Module (moduleNameString, ModLocation, ml_hs_file)
  import qualified GHC.Utils.Logger as Logger
  import GHC.Utils.Outputable (ppr)
  import GHC.Types.Unique.DSM
@@ -186,6 +190,59 @@ Given a `CmmGraph`:
      location.
  See `labelsToSourcesSansTNTC` for the implementation of this algorithm.
++
 +Note [Prefer current-module source ticks for return frames]
 +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 +A return frame's source location is taken from the `SourceNote`s of the block
 +that *ends* in the frame's call (see Note [Stacktraces from Info Table Provenance
 +Entries...] above and `labelsToSourcesWithTNTC`). At `-O`, inlining means such a
 +block frequently carries `SourceNote`s for inlined library glue (`>>`, `>>=`,
 +`threadDelay`, ...) and the *nearest* note — the one historically chosen — is
 +often a library note rather than the user's code. The resulting IPE entry then
 +points at the library (its file and label; see `toCgIPE` in
 +GHC.StgToCmm.InfoTableProv, which takes the file from the note's own span), so a
 +backtrace of a thread blocked in such a primop shows no user frames.
++
 +For a concrete example, compile (at -O1)
++
 +    -- Scan.hs
 +    f3 :: IO ()
 +    f3 = threadDelay 1000000 >> putStrLn "done"
++
 +The body of `f3` reaches the inlined `threadDelay`'s internal `delay#` in a
 +block whose notes are *all* from `Conc.IO`/`Base` — the user's `Scan.hs` tick
 +for `f3` sits only in the proc's entry block:
++
 +    entry:                                    -- the proc's entry block
 +        //tick src<.../Base.hs:2305:5-18>
 +        //tick src<Scan.hs:13:1-43>           -- the enclosing `f3` span
 +        //tick src<.../Conc/IO.hs:(223,1)-(235,10)>
 +        ...
 +    delayBlk:                                 -- no Scan.hs note here:
 +        //tick src<.../Conc/IO.hs:232:5-13>
 +        //tick src<.../Base.hs:2268:1-9>
 +        //tick src<.../Conc/IO.hs:(232,25)-(235,10)>   -- nearest note
 +        call stg_delay#(R1) returns to delayCont, args: 8, res: 8, upd: 8;
++
 +Naively taking the nearest note attributes `delayCont` to `Conc/IO.hs:232`,
 +i.e. an internal of `threadDelay`, rather than `f3`.
++
 +To fix this we attribute a return frame's source location in the following
 +preference order:
++
 +  1. the nearest tick in the frame's block whose file is that of the module
 +     being compiled - the precise user call site. (When the user makes a
 +     blocking call directly, e.g. `f v = takeMVar v`, such a note is present in
 +     the call's block and this rule suffices; the `delayBlk` above has none.)
 +  2. failing that, the proc's *enclosing* current-module note (the outermost
 +     current-module `SourceNote` in the proc, i.e. its function's own span).
 +     For `f3` this is `src<Scan.hs:13:1-43>`, so `delayCont` is attributed to
 +     `f3` rather than to `threadDelay`'s internals.
 +  3. failing that, the nearest note of any module (the historical behaviour).
++
 +This mirrors the same-file preference the DWARF path uses in
 +`GHC.Cmm.DebugBlock.bestSrcTick` and that `GHC.Stg.Debug.quickSourcePos` uses
 +for closures.
  -}
  generateCgIPEStub
@@ -257,11 +314,12 @@ generateCgIPEStub hsc_env this_mod denv (nonCaffySet, moduleLFInfos, infoTablesW
  -- performance suffered considerably as a result (see #23103).
  lookupEstimatedTicks
    :: HscEnv
 +  -> ModLocation -- ^ location of the module being compiled, for IPE provenance
    -> Map CmmInfoTable (Maybe IpeSourceLocation)
    -> IPEStats
    -> CmmGroupSRTs
    -> IO (Map CmmInfoTable (Maybe IpeSourceLocation), IPEStats)
 -lookupEstimatedTicks hsc_env ipes stats cmm_group_srts =
 +lookupEstimatedTicks hsc_env mod_location ipes stats cmm_group_srts =
      -- Pass 2: Create an entry in the IPE map for every info table listed in
      -- this CmmGroupSRTs. If the info table is a stack info table and
      -- -finfo-table-map-with-stack is enabled, look up its estimated source
@@ -276,6 +334,11 @@ lookupEstimatedTicks hsc_env ipes stats cmm_group_srts =
      dflags = hsc_dflags hsc_env
      platform = targetPlatform dflags
 +    -- Source file of the module being compiled, used to prefer current-module
 +    -- source ticks for return frames. See Note [Prefer current-module source
 +    -- ticks for return frames].
 +    mb_src_file = mkFastString <$> ml_hs_file mod_location
++
      -- Pass 1: Map every label meeting the conditions described in Note
      -- [Stacktraces from Info Table Provenance Entries (IPE based stack
      -- unwinding)] to the estimated source location (also as described in the
@@ -286,9 +349,9 @@ lookupEstimatedTicks hsc_env ipes stats cmm_group_srts =
      labelsToSources :: Map CLabel IpeSourceLocation
      labelsToSources =
        if platformTablesNextToCode platform then
 -        foldl' labelsToSourcesWithTNTC Map.empty cmm_group_srts
 +        foldl' (labelsToSourcesWithTNTC mb_src_file) Map.empty cmm_group_srts
        else
 -        foldl' labelsToSourcesSansTNTC Map.empty cmm_group_srts
 +        foldl' (labelsToSourcesSansTNTC mb_src_file) Map.empty cmm_group_srts
      collectInfoTables
        :: (Map CmmInfoTable (Maybe IpeSourceLocation), IPEStats)
@@ -331,15 +394,16 @@ lookupEstimatedTicks hsc_env ipes stats cmm_group_srts =
  -- | See Note [Stacktraces from Info Table Provenance Entries (IPE based stack unwinding)]
  labelsToSourcesWithTNTC
 -  :: Map CLabel IpeSourceLocation
 +  :: Maybe FastString -- ^ source file of the module being compiled
 +  -> Map CLabel IpeSourceLocation
    -> GenCmmDecl RawCmmStatics CmmTopInfo CmmGraph
    -> Map CLabel IpeSourceLocation
 -labelsToSourcesWithTNTC acc (CmmProc _ _ _ cmm_graph) =
 +labelsToSourcesWithTNTC mb_src_file acc (CmmProc _ _ _ cmm_graph) =
      foldl' go acc (toBlockList cmm_graph)
    where
      go :: Map CLabel IpeSourceLocation -> CmmBlock -> Map CLabel IpeSourceLocation
      go acc block =
 -        case (,) <$> returnFrameLabel <*> lastTickInBlock of
 +        case (,) <$> returnFrameLabel <*> bestTickInBlock of
            Just (clabel, src_loc) -> Map.insert clabel src_loc acc
            Nothing -> acc
        where
@@ -351,36 +415,80 @@ labelsToSourcesWithTNTC acc (CmmProc _ _ _ cmm_graph) =
              (CmmCall _ (Just l) _ _ _ _) -> Just $ mkAsmTempLabel l
              _ -> Nothing
 -        lastTickInBlock = foldr maybeTick Nothing (blockToList middleBlock)
 +        -- All SourceNotes in the block, in block order.
 +        -- See Note [Prefer current-module source ticks for return frames].
 +        bestTickInBlock = preferThisFile mb_src_file procFallback (blockToList middleBlock)
++
 +    -- Enclosing current-module note for the whole proc (its function's own
 +    -- span), used when a return frame's own block has no current-module tick.
 +    procFallback = enclosingThisFileTick mb_src_file (toBlockList cmm_graph)
 +labelsToSourcesWithTNTC _ acc _ = acc
 -        maybeTick :: CmmNode O O -> Maybe IpeSourceLocation -> Maybe IpeSourceLocation
 -        maybeTick _ s@(Just _) = s
 -        maybeTick (CmmTick (SourceNote span name)) Nothing = Just (span, name)
 -        maybeTick _ _ = Nothing
 -labelsToSourcesWithTNTC acc _ = acc
 +-- | Pick the 'IpeSourceLocation' to attribute to a return frame from the
 +-- source-note-bearing nodes of its block (in block order).
 +--
 +-- See Note [Prefer current-module source ticks for return frames].
 +preferThisFile :: Maybe FastString -> Maybe IpeSourceLocation -> [CmmNode O O] -> Maybe IpeSourceLocation
 +preferThisFile mb_src_file procFallback nodes =
 +    nearest fromThisFile <|> procFallback <|> nearest sourceNotes
 +  where
 +    sourceNotes = [ (span, name) | CmmTick (SourceNote span name) <- nodes ]
 +    fromThisFile = case mb_src_file of
 +      Just f  -> filter ((== f) . srcSpanFile . fst) sourceNotes
 +      Nothing -> []
 +    nearest = listToMaybe . reverse
++
 +-- | The outermost 'SourceNote' from the module being compiled across a proc's
 +-- blocks (in 'toBlockList' order, so the entry block's note — the function's own
 +-- span — comes first). Used as a fallback so inlined cross-module code is still
 +-- labelled with the enclosing user function. 'Nothing' when the proc has no
 +-- current-module note (e.g. when compiling the library itself).
 +enclosingThisFileTick :: Maybe FastString -> [CmmBlock] -> Maybe IpeSourceLocation
 +enclosingThisFileTick mb_src_file blocks =
 +    listToMaybe
 +      [ (span, name)
 +      | b <- blocks
 +      , let (_, mid, _) = blockSplit b
 +      , CmmTick (SourceNote span name) <- blockToList mid
 +      , Just (srcSpanFile span) == mb_src_file ]
  -- | See Note [Stacktraces from Info Table Provenance Entries (IPE based stack unwinding)]
  labelsToSourcesSansTNTC
 -  :: Map CLabel IpeSourceLocation
 +  :: Maybe FastString -- ^ source file of the module being compiled
 +  -> Map CLabel IpeSourceLocation
    -> GenCmmDecl RawCmmStatics CmmTopInfo CmmGraph
    -> Map CLabel IpeSourceLocation
 -labelsToSourcesSansTNTC acc (CmmProc _ _ _ cmm_graph) =
 +labelsToSourcesSansTNTC mb_src_file acc (CmmProc _ _ _ cmm_graph) =
      foldl' go acc (toBlockList cmm_graph)
    where
 +    -- See 'enclosingThisFileTick'.
 +    procFallback = enclosingThisFileTick mb_src_file (toBlockList cmm_graph)
++
      go :: Map CLabel IpeSourceLocation -> CmmBlock -> Map CLabel IpeSourceLocation
 -    go acc block = fst $ foldl' collectLabels (acc, Nothing) (blockToList middleBlock)
 +    go acc block = fst $ foldl' collectLabels (acc, (Nothing, Nothing)) (blockToList middleBlock)
        where
          (_, middleBlock, _) = blockSplit block
 +        -- We track the nearest preceding SourceNote from the module being
 +        -- compiled and the nearest of any module, and prefer the former (then
 +        -- the proc's enclosing current-module note) when attributing a return
 +        -- frame. See 'preferThisFile' and
 +        -- Note [Prefer current-module source ticks for return frames].
          collectLabels
 -          :: (Map CLabel IpeSourceLocation, Maybe IpeSourceLocation)
 +          :: (Map CLabel IpeSourceLocation, (Maybe IpeSourceLocation, Maybe IpeSourceLocation))
            -> CmmNode O O
 -          -> (Map CLabel IpeSourceLocation, Maybe IpeSourceLocation)
 -        collectLabels (!acc, lastTick) b =
 -          case (b, lastTick) of
 -            (CmmStore _ (CmmLit (CmmLabel l)) _, Just src_loc) ->
 -              (Map.insert l src_loc acc, Nothing)
 -            (CmmTick (SourceNote span name), _) ->
 -              (acc, Just (span, name))
 -            _ -> (acc, lastTick)
 -labelsToSourcesSansTNTC acc _ = acc
 +          -> (Map CLabel IpeSourceLocation, (Maybe IpeSourceLocation, Maybe IpeSourceLocation))
 +        collectLabels (!acc, st@(lastThis, lastAny)) b =
 +          case b of
 +            CmmStore _ (CmmLit (CmmLabel l)) _ ->
 +              case lastThis <|> procFallback <|> lastAny of
 +                Just src_loc -> (Map.insert l src_loc acc, (Nothing, Nothing))
 +                Nothing      -> (acc, st)
 +            CmmTick (SourceNote span name) ->
 +              let tick = (span, name)
 +                  lastThis'
 +                    | Just (srcSpanFile span) == mb_src_file = Just tick
 +                    | otherwise                              = lastThis
 +              in (acc, (lastThis', Just tick))
 +            _ -> (acc, st)
 +labelsToSourcesSansTNTC _ acc _ = acc

compiler/GHC/Driver/Main/Compile.hs

@@ -699,7 +699,7 @@ hscGenHardCode hsc_env cgguts mod_loc output_filename = do
              _          ->
                do
                cmms <- {-# SCC "StgToCmm" #-}
 -                doCodeGen hsc_env this_mod denv tycons
 +                doCodeGen hsc_env this_mod mod_loc denv tycons
                  cost_centre_info
                  stg_binds
@@ -956,14 +956,17 @@ This reduces residency towards the end of the CodeGen phase significantly
  (5-10%).
  -}
 -doCodeGen :: HscEnv -> Module -> InfoTableProvMap -> [TyCon]
 +doCodeGen :: HscEnv -> Module
 +          -> ModLocation -- ^ location of the module being compiled, used to
 +                         -- prefer current-module IPE source locations
 +          -> InfoTableProvMap -> [TyCon]
            -> CollectedCCs
            -> [CgStgTopBinding] -- ^ Bindings come already annotated with fvs
            -> IO (CgStream CmmGroupSRTs CmmCgInfos)
           -- Note we produce a 'Stream' of CmmGroups, so that the
           -- backend can be run incrementally.  Otherwise it generates all
           -- the C-- up front, which has a significant space cost.
 -doCodeGen hsc_env this_mod denv tycons
 +doCodeGen hsc_env this_mod mod_location denv tycons
                cost_centre_info stg_binds_w_fvs = do
      let dflags     = hsc_dflags hsc_env
          logger     = hsc_logger hsc_env
@@ -1032,7 +1035,7 @@ doCodeGen hsc_env this_mod denv tycons
            -- Positions] in GHC.Stg.Debug.
            (ipes', stats') <-
              if (gopt Opt_InfoTableMap dflags) then
 -              liftIO $ lookupEstimatedTicks hsc_env ipes stats cmm_srts
 +              liftIO $ lookupEstimatedTicks hsc_env mod_location ipes stats cmm_srts
              else
                return (ipes, stats)