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

Commits:

5 changed files:

Changes:

  • compiler/GHC/Driver/GenerateCgIPEStub.hs
    1 1 
     module GHC.Driver.GenerateCgIPEStub (generateCgIPEStub, lookupEstimatedTicks) where
    2 2
    3 
    +import Control.Applicative ((<|>))
    3 4 
     import Data.Map.Strict (Map)
    4 5 
     import qualified Data.Map.Strict as Map
    6 
    +import Data.Maybe (listToMaybe)
    5 7 
     import Data.Semigroup ((<>))
    6 8 
     import GHC.Cmm
    7 9 
     import GHC.Cmm.CLabel (CLabel, mkAsmTempLabel)
    ... ... @@ -10,6 +12,7 @@ import GHC.Cmm.Dataflow.Block (blockSplit, blockToList)
    10 12 
     import GHC.Cmm.Dataflow.Label
    11 13 
     import GHC.Cmm.Info.Build (emptySRT)
    12 14 
     import GHC.Cmm.Pipeline (cmmPipeline)
    15 
    +import GHC.Data.FastString (FastString, mkFastString)
    13 16 
     import GHC.Data.Stream (liftIO, liftEff)
    14 17 
     import qualified GHC.Data.Stream as Stream
    15 18 
     import GHC.Driver.Env (hsc_dflags, hsc_logger)
    ... ... @@ -28,9 +31,10 @@ import GHC.StgToCmm.Utils
    28 31 
     import GHC.StgToCmm.CgUtils (CgStream)
    29 32 
     import GHC.Types.IPE (InfoTableProvMap (provInfoTables), IpeSourceLocation)
    30 33 
     import GHC.Types.Name.Set (NonCaffySet)
    34 
    +import GHC.Types.SrcLoc (srcSpanFile)
    31 35 
     import GHC.Types.Tickish (GenTickish (SourceNote))
    32 36 
     import GHC.Unit.Types (Module, moduleName)
    33 
    -import GHC.Unit.Module (moduleNameString)
    37 
    +import GHC.Unit.Module (moduleNameString, ModLocation, ml_hs_file)
    34 38 
     import qualified GHC.Utils.Logger as Logger
    35 39 
     import GHC.Utils.Outputable (ppr)
    36 40 
     import GHC.Types.Unique.DSM
    ... ... @@ -257,11 +261,12 @@ generateCgIPEStub hsc_env this_mod denv (nonCaffySet, moduleLFInfos, infoTablesW
    257 261 
     -- performance suffered considerably as a result (see #23103).
    258 262 
     lookupEstimatedTicks
    259 263 
       :: HscEnv
    264 
    +  -> ModLocation -- ^ location of the module being compiled, for IPE provenance
    260 265 
       -> Map CmmInfoTable (Maybe IpeSourceLocation)
    261 266 
       -> IPEStats
    262 267 
       -> CmmGroupSRTs
    263 268 
       -> IO (Map CmmInfoTable (Maybe IpeSourceLocation), IPEStats)
    264 
    -lookupEstimatedTicks hsc_env ipes stats cmm_group_srts =
    269 
    +lookupEstimatedTicks hsc_env mod_location ipes stats cmm_group_srts =
    265 270 
         -- Pass 2: Create an entry in the IPE map for every info table listed in
    266 271 
         -- this CmmGroupSRTs. If the info table is a stack info table and
    267 272 
         -- -finfo-table-map-with-stack is enabled, look up its estimated source
    ... ... @@ -276,19 +281,24 @@ lookupEstimatedTicks hsc_env ipes stats cmm_group_srts =
    276 281 
         dflags = hsc_dflags hsc_env
    277 282 
         platform = targetPlatform dflags
    278 283
    279 
    -    -- Pass 1: Map every label meeting the conditions described in Note
    280 
    -    -- [Stacktraces from Info Table Provenance Entries (IPE based stack
    281 
    -    -- unwinding)] to the estimated source location (also as described in the
    282 
    -    -- aformentioned note)
    284 
    +    -- Source file of the module being compiled, used to prefer current-module
    285 
    +    -- source ticks for return frames. See Note [Prefer current-module source
    286 
    +    -- ticks for return frames].
    287 
    +    mb_src_file = mkFastString <$> ml_hs_file mod_location
    288 
    +
    289 
    +    -- Pass 1: Map every label meeting the conditions described in
    290 
    +    -- Note [Stacktraces from Info Table Provenance Entries (IPE based stack unwinding)]
    291 
    +    -- to the estimated source location (also as described in the aformentioned
    292 
    +    -- note).
    283 293 
         --
    284 294 
         -- Note: It's important that this remains a thunk so we do not compute this
    285 295 
         -- map if -fno-info-table-with-stack is given
    286 296 
         labelsToSources :: Map CLabel IpeSourceLocation
    287 297 
         labelsToSources =
    288 298 
           if platformTablesNextToCode platform then
    289 
    -        foldl' labelsToSourcesWithTNTC Map.empty cmm_group_srts
    299 
    +        foldl' (labelsToSourcesWithTNTC mb_src_file) Map.empty cmm_group_srts
    290 300 
           else
    291 
    -        foldl' labelsToSourcesSansTNTC Map.empty cmm_group_srts
    301 
    +        foldl' (labelsToSourcesSansTNTC mb_src_file) Map.empty cmm_group_srts
    292 302
    293 303 
         collectInfoTables
    294 304 
           :: (Map CmmInfoTable (Maybe IpeSourceLocation), IPEStats)
    ... ... @@ -331,15 +341,16 @@ lookupEstimatedTicks hsc_env ipes stats cmm_group_srts =
    331 341
    332 342 
     -- | See Note [Stacktraces from Info Table Provenance Entries (IPE based stack unwinding)]
    333 343 
     labelsToSourcesWithTNTC
    334 
    -  :: Map CLabel IpeSourceLocation
    344 
    +  :: Maybe FastString -- ^ source file of the module being compiled
    345 
    +  -> Map CLabel IpeSourceLocation
    335 346 
       -> GenCmmDecl RawCmmStatics CmmTopInfo CmmGraph
    336 347 
       -> Map CLabel IpeSourceLocation
    337 
    -labelsToSourcesWithTNTC acc (CmmProc _ _ _ cmm_graph) =
    348 
    +labelsToSourcesWithTNTC mb_src_file acc (CmmProc _ _ _ cmm_graph) =
    338 349 
         foldl' go acc (toBlockList cmm_graph)
    339 350 
       where
    340 351 
         go :: Map CLabel IpeSourceLocation -> CmmBlock -> Map CLabel IpeSourceLocation
    341 352 
         go acc block =
    342 
    -        case (,) <$> returnFrameLabel <*> lastTickInBlock of
    353 
    +        case (,) <$> returnFrameLabel <*> bestTickInBlock of
    343 354 
               Just (clabel, src_loc) -> Map.insert clabel src_loc acc
    344 355 
               Nothing -> acc
    345 356 
           where
    ... ... @@ -351,36 +362,135 @@ labelsToSourcesWithTNTC acc (CmmProc _ _ _ cmm_graph) =
    351 362 
                 (CmmCall _ (Just l) _ _ _ _) -> Just $ mkAsmTempLabel l
    352 363 
                 _ -> Nothing
    353 364
    354 
    -        lastTickInBlock = foldr maybeTick Nothing (blockToList middleBlock)
    365 
    +        -- All SourceNotes in the block, in block order.
    366 
    +        -- See Note [Prefer current-module source ticks for return frames].
    367 
    +        bestTickInBlock = preferThisFile mb_src_file procFallback (blockToList middleBlock)
    368 
    +
    369 
    +    -- Enclosing current-module note for the whole proc (its function's own
    370 
    +    -- span), used when a return frame's own block has no current-module tick.
    371 
    +    procFallback = enclosingThisFileTick mb_src_file (toBlockList cmm_graph)
    372 
    +labelsToSourcesWithTNTC _ acc _ = acc
    373 
    +
    374 
    +{-
    375 
    +Note [Prefer current-module source ticks for return frames]
    376 
    +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    377 
    +A return frame's source location is taken from the `SourceNote`s of the block
    378 
    +that *ends* in the frame's call (see
    379 
    +Note [Stacktraces from Info Table Provenance Entries (IPE based stack unwinding]
    380 
    +above and `labelsToSourcesWithTNTC`). At `-O`, inlining means such a
    381 
    +block frequently carries `SourceNote`s for inlined library glue (`>>`, `>>=`,
    382 
    +`threadDelay`, ...) and the *nearest* note — the one historically chosen — is
    383 
    +often a library note rather than the user's code. The resulting IPE entry then
    384 
    +points at the library (its file and label; see `toCgIPE` in
    385 
    +GHC.StgToCmm.InfoTableProv, which takes the file from the note's own span), so a
    386 
    +backtrace of a thread blocked in such a primop shows no user frames.
    387 
    +
    388 
    +For a concrete example, compile (at -O1)
    389 
    +
    390 
    +    -- Scan.hs
    391 
    +    f3 :: IO ()
    392 
    +    f3 = threadDelay 1000000 >> putStrLn "done"
    393 
    +
    394 
    +The body of `f3` reaches the inlined `threadDelay`'s internal `delay#` in a
    395 
    +block whose notes are *all* from `Conc.IO`/`Base` — the user's `Scan.hs` tick
    396 
    +for `f3` sits only in the proc's entry block:
    397 
    +
    398 
    +    entry:                                    -- the proc's entry block
    399 
    +        //tick src<.../Base.hs:2306:5-18>
    400 
    +        //tick src<Scan.hs:13:1-43>           -- the enclosing `f3` span
    401 
    +        //tick src<.../Conc/IO.hs:(223,1)-(235,10)>
    402 
    +        ...
    403 
    +    delayBlk:                                 -- no Scan.hs note here:
    404 
    +        //tick src<.../Conc/IO.hs:232:5-13>
    405 
    +        //tick src<.../Base.hs:2268:1-9>
    406 
    +        //tick src<.../Conc/IO.hs:(232,25)-(235,10)>   -- nearest note
    407 
    +        call stg_delay#(R1) returns to delayCont, args: 8, res: 8, upd: 8;
    408 
    +
    409 
    +Naively taking the nearest note attributes `delayCont` to `Conc/IO.hs:232`,
    410 
    +i.e. an internal of `threadDelay`, rather than `f3`.
    411 
    +
    412 
    +To fix this we attribute a return frame's source location in the following
    413 
    +preference order:
    414 
    +
    415 
    +  1. the nearest tick in the frame's block whose file is that of the module
    416 
    +     being compiled - the precise user call site. (When the user makes a
    417 
    +     blocking call directly, e.g. `f v = takeMVar v`, such a note is present in
    418 
    +     the call's block and this rule suffices; the `delayBlk` above has none.)
    419 
    +  2. failing that, the proc's *enclosing* current-module note (the outermost
    420 
    +     current-module `SourceNote` in the proc, i.e. its function's own span).
    421 
    +     For `f3` this is `src<Scan.hs:13:1-43>`, so `delayCont` is attributed to
    422 
    +     `f3` rather than to `threadDelay`'s internals.
    423 
    +  3. failing that, the nearest note of any module (the historical behaviour).
    424 
    +
    425 
    +This mirrors the same-file preference the DWARF path uses in
    426 
    +`GHC.Cmm.DebugBlock.bestSrcTick` and that `GHC.Stg.Debug.quickSourcePos` uses
    427 
    +for closures.
    428 
    +-}
    355 429
    356 
    -        maybeTick :: CmmNode O O -> Maybe IpeSourceLocation -> Maybe IpeSourceLocation
    357 
    -        maybeTick _ s@(Just _) = s
    358 
    -        maybeTick (CmmTick (SourceNote span name)) Nothing = Just (span, name)
    359 
    -        maybeTick _ _ = Nothing
    360 
    -labelsToSourcesWithTNTC acc _ = acc
    430 
    +-- | Pick the 'IpeSourceLocation' to attribute to a return frame from the
    431 
    +-- source-note-bearing nodes of its block (in block order).
    432 
    +--
    433 
    +-- See Note [Prefer current-module source ticks for return frames].
    434 
    +preferThisFile :: Maybe FastString -> Maybe IpeSourceLocation -> [CmmNode O O] -> Maybe IpeSourceLocation
    435 
    +preferThisFile mb_src_file procFallback nodes =
    436 
    +    nearest fromThisFile <|> procFallback <|> nearest sourceNotes
    437 
    +  where
    438 
    +    sourceNotes = [ (span, name) | CmmTick (SourceNote span name) <- nodes ]
    439 
    +    fromThisFile = case mb_src_file of
    440 
    +      Just f  -> filter ((== f) . srcSpanFile . fst) sourceNotes
    441 
    +      Nothing -> []
    442 
    +    nearest = listToMaybe . reverse
    443 
    +
    444 
    +-- | The outermost 'SourceNote' from the module being compiled across a proc's
    445 
    +-- blocks (in 'toBlockList' order, so the entry block's note — the function's own
    446 
    +-- span — comes first). Used as a fallback so inlined cross-module code is still
    447 
    +-- labelled with the enclosing user function. 'Nothing' when the proc has no
    448 
    +-- current-module note (e.g. when compiling the library itself).
    449 
    +enclosingThisFileTick :: Maybe FastString -> [CmmBlock] -> Maybe IpeSourceLocation
    450 
    +enclosingThisFileTick mb_src_file blocks =
    451 
    +    listToMaybe
    452 
    +      [ (span, name)
    453 
    +      | b <- blocks
    454 
    +      , let (_, mid, _) = blockSplit b
    455 
    +      , CmmTick (SourceNote span name) <- blockToList mid
    456 
    +      , Just (srcSpanFile span) == mb_src_file ]
    361 457
    362 458 
     -- | See Note [Stacktraces from Info Table Provenance Entries (IPE based stack unwinding)]
    363 459 
     labelsToSourcesSansTNTC
    364 
    -  :: Map CLabel IpeSourceLocation
    460 
    +  :: Maybe FastString -- ^ source file of the module being compiled
    461 
    +  -> Map CLabel IpeSourceLocation
    365 462 
       -> GenCmmDecl RawCmmStatics CmmTopInfo CmmGraph
    366 463 
       -> Map CLabel IpeSourceLocation
    367 
    -labelsToSourcesSansTNTC acc (CmmProc _ _ _ cmm_graph) =
    464 
    +labelsToSourcesSansTNTC mb_src_file acc (CmmProc _ _ _ cmm_graph) =
    368 465 
         foldl' go acc (toBlockList cmm_graph)
    369 466 
       where
    467 
    +    -- See 'enclosingThisFileTick'.
    468 
    +    procFallback = enclosingThisFileTick mb_src_file (toBlockList cmm_graph)
    469 
    +
    370 470 
         go :: Map CLabel IpeSourceLocation -> CmmBlock -> Map CLabel IpeSourceLocation
    371 
    -    go acc block = fst $ foldl' collectLabels (acc, Nothing) (blockToList middleBlock)
    471 
    +    go acc block = fst $ foldl' collectLabels (acc, (Nothing, Nothing)) (blockToList middleBlock)
    372 472 
           where
    373 473 
             (_, middleBlock, _) = blockSplit block
    374 474
    475 
    +        -- We track the nearest preceding SourceNote from the module being
    476 
    +        -- compiled and the nearest of any module, and prefer the former (then
    477 
    +        -- the proc's enclosing current-module note) when attributing a return
    478 
    +        -- frame. See Note [Prefer current-module source ticks for return frames].
    375 479 
             collectLabels
    376 
    -          :: (Map CLabel IpeSourceLocation, Maybe IpeSourceLocation)
    480 
    +          :: (Map CLabel IpeSourceLocation, (Maybe IpeSourceLocation, Maybe IpeSourceLocation))
    377 481 
               -> CmmNode O O
    378 
    -          -> (Map CLabel IpeSourceLocation, Maybe IpeSourceLocation)
    379 
    -        collectLabels (!acc, lastTick) b =
    380 
    -          case (b, lastTick) of
    381 
    -            (CmmStore _ (CmmLit (CmmLabel l)) _, Just src_loc) ->
    382 
    -              (Map.insert l src_loc acc, Nothing)
    383 
    -            (CmmTick (SourceNote span name), _) ->
    384 
    -              (acc, Just (span, name))
    385 
    -            _ -> (acc, lastTick)
    386 
    -labelsToSourcesSansTNTC acc _ = acc
    482 
    +          -> (Map CLabel IpeSourceLocation, (Maybe IpeSourceLocation, Maybe IpeSourceLocation))
    483 
    +        collectLabels (!acc, st@(lastThis, lastAny)) b =
    484 
    +          case b of
    485 
    +            CmmStore _ (CmmLit (CmmLabel l)) _ ->
    486 
    +              case lastThis <|> procFallback <|> lastAny of
    487 
    +                Just src_loc -> (Map.insert l src_loc acc, (Nothing, Nothing))
    488 
    +                Nothing      -> (acc, st)
    489 
    +            CmmTick (SourceNote span name) ->
    490 
    +              let tick = (span, name)
    491 
    +                  lastThis'
    492 
    +                    | Just (srcSpanFile span) == mb_src_file = Just tick
    493 
    +                    | otherwise                              = lastThis
    494 
    +              in (acc, (lastThis', Just tick))
    495 
    +            _ -> (acc, st)
    496 
    +labelsToSourcesSansTNTC _ acc _ = acc

  • compiler/GHC/Driver/Main/Compile.hs
    ... ... @@ -699,7 +699,7 @@ hscGenHardCode hsc_env cgguts mod_loc output_filename = do
    699 699 
                 _          ->
    700 700 
                   do
    701 701 
                   cmms <- {-# SCC "StgToCmm" #-}
    702 
    -                doCodeGen hsc_env this_mod denv tycons
    702 
    +                doCodeGen hsc_env this_mod mod_loc denv tycons
    703 703 
                     cost_centre_info
    704 704 
                     stg_binds
    705 705
    ... ... @@ -956,14 +956,17 @@ This reduces residency towards the end of the CodeGen phase significantly
    956 956 
     (5-10%).
    957 957 
     -}
    958 958
    959 
    -doCodeGen :: HscEnv -> Module -> InfoTableProvMap -> [TyCon]
    959 
    +doCodeGen :: HscEnv -> Module
    960 
    +          -> ModLocation -- ^ location of the module being compiled, used to
    961 
    +                         -- prefer current-module IPE source locations
    962 
    +          -> InfoTableProvMap -> [TyCon]
    960 963 
               -> CollectedCCs
    961 964 
               -> [CgStgTopBinding] -- ^ Bindings come already annotated with fvs
    962 965 
               -> IO (CgStream CmmGroupSRTs CmmCgInfos)
    963 966 
              -- Note we produce a 'Stream' of CmmGroups, so that the
    964 967 
              -- backend can be run incrementally.  Otherwise it generates all
    965 968 
              -- the C-- up front, which has a significant space cost.
    966 
    -doCodeGen hsc_env this_mod denv tycons
    969 
    +doCodeGen hsc_env this_mod mod_location denv tycons
    967 970 
                   cost_centre_info stg_binds_w_fvs = do
    968 971 
         let dflags     = hsc_dflags hsc_env
    969 972 
             logger     = hsc_logger hsc_env
    ... ... @@ -1032,7 +1035,7 @@ doCodeGen hsc_env this_mod denv tycons
    1032 1035 
               -- Positions] in GHC.Stg.Debug.
    1033 1036 
               (ipes', stats') <-
    1034 1037 
                 if (gopt Opt_InfoTableMap dflags) then
    1035 
    -              liftIO $ lookupEstimatedTicks hsc_env ipes stats cmm_srts
    1038 
    +              liftIO $ lookupEstimatedTicks hsc_env mod_location ipes stats cmm_srts
    1036 1039 
                 else
    1037 1040 
                   return (ipes, stats)
    1038 1041

  • testsuite/tests/rts/ipe/T27408.hs
    1 
    +module Main where
    2 
    +
    3 
    +import GHC.Stack.CloneStack (StackEntry(..), cloneMyStack, decode)
    4 
    +
    5 
    +userFunction :: IO [StackEntry]
    6 
    +userFunction = do
    7 
    +  putStr ""
    8 
    +  stk <- cloneMyStack
    9 
    +  putStr ""
    10 
    +  es <- decode stk
    11 
    +  putStr ""
    12 
    +  return es
    13 
    +
    14 
    +main :: IO ()
    15 
    +main = do
    16 
    +  entries <- userFunction
    17 
    +  let ours = filter ((== "Main") . moduleName) entries
    18 
    +  mapM_ (\e -> putStrLn (moduleName e ++ "\t" ++ functionName e)) ours

  • testsuite/tests/rts/ipe/T27408.stdout
    1 
    +Main
    2 
    +Main	main
    3 
    +Main	userFunction

  • testsuite/tests/rts/ipe/all.T
    ... ... @@ -4,6 +4,16 @@ def noCapabilityOutputFilter(s):
    4 4
    5 5 
     test('ipeMap', [extra_files(['ipe_lib.c', 'ipe_lib.h']), c_src, omit_ghci], compile_and_run, ['ipe_lib.c'])
    6 6
    7 
    +# Return frames whose Cmm block is dominated by inlined library ticks (e.g. the
    8 
    +# (>>)/(>>=) of a user do-block at -O) should still be attributed to the user's
    9 
    +# module. See Note [Prefer current-module source ticks for return frames] in
    10 
    +# GHC.Driver.GenerateCgIPEStub.
    11 
    +test('T27408',
    12 
    +     [ omit_ghci          # cloneMyStack# is not available in ghci
    13 
    +     , js_broken(22261)   # cloneMyStack# not yet implemented in the JS backend
    14 
    +     ],
    15 
    +     compile_and_run, ['-O1 -finfo-table-map -g3'])
    16 
    +
    7 17 
     # Manually create IPE entries and dump them to event log (stderr).
    8 18 
     test('ipeEventLog',
    9 19 
          [ c_src,

    • —
    View it on GitLab.
    You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications · Help