[Git][ghc/ghc][wip/orig-thunk-info] 3 commits: Fix up rules for ghcup-metadata-nightly-push

Ben Gamari (@bgamari) gitlab at gitlab.haskell.org
Tue May 9 17:47:42 UTC 2023



Ben Gamari pushed to branch wip/orig-thunk-info at Glasgow Haskell Compiler / GHC


Commits:
0e2df4c9 by Bryan Richter at 2023-05-09T12:03:35+03:00
Fix up rules for ghcup-metadata-nightly-push

- - - - -
4e59bbf9 by Ben Gamari at 2023-05-09T13:47:34-04:00
compiler: Fingerprint more code generation flags

Previously our recompilation check was quite inconsistent in its
coverage of non-optimisation code generation flags. Specifically, we
failed to account for most flags that would affect the behavior of
generated code in ways that might affect the result of a program's
execution (e.g. `-feager-blackholing`, `-fstrict-dicts`)

- - - - -
3d32cad7 by Ben Gamari at 2023-05-09T13:47:34-04:00
compiler: Record original thunk info tables on stack

- - - - -


13 changed files:

- .gitlab-ci.yml
- compiler/GHC/Cmm/CLabel.hs
- compiler/GHC/Driver/Config/StgToCmm.hs
- compiler/GHC/Driver/Flags.hs
- compiler/GHC/Driver/Session.hs
- compiler/GHC/Iface/Recomp/Flags.hs
- compiler/GHC/StgToCmm/Bind.hs
- compiler/GHC/StgToCmm/Config.hs
- docs/users_guide/debugging.rst
- rts/StgMiscClosures.cmm
- rts/include/rts/storage/Closures.h
- testsuite/tests/driver/T13914/T13914.stdout
- utils/deriveConstants/Main.hs


Changes:

=====================================
.gitlab-ci.yml
=====================================
@@ -1048,7 +1048,7 @@ ghcup-metadata-nightly:
   rules:
     - if: $NIGHTLY
 
-# Update the
+# Update the ghcup metadata with information about this nightly pipeline
 ghcup-metadata-nightly-push:
   stage: deploy
   image: "registry.gitlab.haskell.org/ghc/ci-images/x86_64-linux-fedora33:$DOCKER_REV"
@@ -1072,11 +1072,8 @@ ghcup-metadata-nightly-push:
     - git commit -m "Update metadata"
     - git push gitlab_origin HEAD:updates -o ci.skip
   rules:
-    - if: $NIGHTLY
     # Only run the update on scheduled nightly pipelines, ie once a day
-    - if: $CI_PIPELINE_SOURCE == "schedule"
-    # And only update the metadata for master branch
-    - if: '$CI_COMMIT_BRANCH == "master"'
+    - if: $NIGHTLY && $CI_PIPELINE_SOURCE == "schedule" && $CI_COMMIT_BRANCH == "master"
 
 
 ghcup-metadata-release:


=====================================
compiler/GHC/Cmm/CLabel.hs
=====================================
@@ -53,6 +53,7 @@ module GHC.Cmm.CLabel (
         mkDirty_MUT_VAR_Label,
         mkMUT_VAR_CLEAN_infoLabel,
         mkNonmovingWriteBarrierEnabledLabel,
+        mkOrigThunkInfoLabel,
         mkUpdInfoLabel,
         mkBHUpdInfoLabel,
         mkIndStaticInfoLabel,
@@ -641,7 +642,7 @@ mkBlockInfoTableLabel name c = IdLabel name c BlockInfoTable
 -- Constructing Cmm Labels
 mkDirty_MUT_VAR_Label,
     mkNonmovingWriteBarrierEnabledLabel,
-    mkUpdInfoLabel,
+    mkOrigThunkInfoLabel, mkUpdInfoLabel,
     mkBHUpdInfoLabel, mkIndStaticInfoLabel, mkMainCapabilityLabel,
     mkMAP_FROZEN_CLEAN_infoLabel, mkMAP_FROZEN_DIRTY_infoLabel,
     mkMAP_DIRTY_infoLabel,
@@ -655,6 +656,7 @@ mkDirty_MUT_VAR_Label,
 mkDirty_MUT_VAR_Label           = mkForeignLabel (fsLit "dirty_MUT_VAR") Nothing ForeignLabelInExternalPackage IsFunction
 mkNonmovingWriteBarrierEnabledLabel
                                 = CmmLabel rtsUnitId (NeedExternDecl False) (fsLit "nonmoving_write_barrier_enabled") CmmData
+mkOrigThunkInfoLabel            = CmmLabel rtsUnitId (NeedExternDecl False) (fsLit "stg_orig_thunk_info_frame") CmmInfo
 mkUpdInfoLabel                  = CmmLabel rtsUnitId (NeedExternDecl False) (fsLit "stg_upd_frame")         CmmInfo
 mkBHUpdInfoLabel                = CmmLabel rtsUnitId (NeedExternDecl False) (fsLit "stg_bh_upd_frame" )     CmmInfo
 mkIndStaticInfoLabel            = CmmLabel rtsUnitId (NeedExternDecl False) (fsLit "stg_IND_STATIC")        CmmInfo


=====================================
compiler/GHC/Driver/Config/StgToCmm.hs
=====================================
@@ -37,6 +37,7 @@ initStgToCmmConfig dflags mod = StgToCmmConfig
   , stgToCmmFastPAPCalls  = gopt Opt_FastPAPCalls          dflags
   , stgToCmmSCCProfiling  = sccProfilingEnabled            dflags
   , stgToCmmEagerBlackHole = gopt Opt_EagerBlackHoling     dflags
+  , stgToCmmOrigThunkInfo = gopt Opt_OrigThunkInfo         dflags
   , stgToCmmInfoTableMap  = gopt Opt_InfoTableMap          dflags
   , stgToCmmOmitYields    = gopt Opt_OmitYields            dflags
   , stgToCmmOmitIfPragmas = gopt Opt_OmitInterfacePragmas  dflags


=====================================
compiler/GHC/Driver/Flags.hs
=====================================
@@ -5,6 +5,7 @@ module GHC.Driver.Flags
    , GeneralFlag(..)
    , Language(..)
    , optimisationFlags
+   , codeGenFlags
 
    -- * Warnings
    , WarningGroup(..)
@@ -328,6 +329,7 @@ data GeneralFlag
    | Opt_IgnoreHpcChanges
    | Opt_ExcessPrecision
    | Opt_EagerBlackHoling
+   | Opt_OrigThunkInfo
    | Opt_NoHsMain
    | Opt_SplitSections
    | Opt_StgStats
@@ -473,15 +475,11 @@ data GeneralFlag
    | Opt_G_NoOptCoercion
    deriving (Eq, Show, Enum)
 
--- Check whether a flag should be considered an "optimisation flag"
--- for purposes of recompilation avoidance (see
--- Note [Ignoring some flag changes] in GHC.Iface.Recomp.Flags). Being listed here is
--- not a guarantee that the flag has no other effect. We could, and
--- perhaps should, separate out the flags that have some minor impact on
--- program semantics and/or error behavior (e.g., assertions), but
--- then we'd need to go to extra trouble (and an additional flag)
--- to allow users to ignore the optimisation level even though that
--- means ignoring some change.
+-- | The set of flags which affect optimisation for the purposes of
+-- recompilation avoidance. Specifically, these include flags which
+-- affect code generation but not the semantics of the program.
+--
+-- See Note [Ignoring some flag changes] in GHC.Iface.Recomp.Flags)
 optimisationFlags :: EnumSet GeneralFlag
 optimisationFlags = EnumSet.fromList
    [ Opt_CallArity
@@ -503,7 +501,6 @@ optimisationFlags = EnumSet.fromList
    , Opt_SpecConstr
    , Opt_SpecConstrKeen
    , Opt_DoLambdaEtaExpansion
-   , Opt_IgnoreAsserts
    , Opt_DoEtaReduction
    , Opt_CaseMerge
    , Opt_CaseFolding
@@ -513,16 +510,12 @@ optimisationFlags = EnumSet.fromList
    , Opt_EnableRewriteRules
    , Opt_RegsGraph
    , Opt_RegsIterative
-   , Opt_PedanticBottoms
    , Opt_LlvmTBAA
-   , Opt_LlvmFillUndefWithGarbage
    , Opt_IrrefutableTuples
    , Opt_CmmSink
    , Opt_CmmElimCommonBlocks
    , Opt_AsmShortcutting
-   , Opt_OmitYields
    , Opt_FunToThunk
-   , Opt_DictsStrict
    , Opt_DmdTxDictSel
    , Opt_Loopification
    , Opt_CfgBlocklayout
@@ -531,8 +524,47 @@ optimisationFlags = EnumSet.fromList
    , Opt_WorkerWrapper
    , Opt_WorkerWrapperUnlift
    , Opt_SolveConstantDicts
+   ]
+
+-- | The set of flags which affect code generation and can change a program's
+-- runtime behavior (other than performance). These include flags which affect:
+--
+--  * user visible debugging information (e.g. info table provenance)
+--  * the ability to catch runtime errors (e.g. -fignore-asserts)
+--  * the runtime result of the program (e.g. -fomit-yields)
+--  * which code or interface file declarations are emitted
+--
+-- We also considered placing flags which affect asympototic space behavior
+-- (e.g. -ffull-laziness) however this would mean that changing optimisation
+-- levels would trigger recompilation even with -fignore-optim-changes,
+-- regressing #13604.
+--
+-- Also, arguably Opt_IgnoreAsserts should be here as well; however, we place
+-- it instead in 'optimisationFlags' since it is implied by @-O[12]@ and
+-- therefore would also break #13604.
+--
+codeGenFlags :: EnumSet GeneralFlag
+codeGenFlags = EnumSet.fromList
+   [ -- Flags that affect runtime result
+     Opt_EagerBlackHoling
+   , Opt_ExcessPrecision
+   , Opt_DictsStrict
+   , Opt_PedanticBottoms
+   , Opt_OmitYields
+
+     -- Flags that affect generated code
+   , Opt_ExposeAllUnfoldings
+   , Opt_NoTypeableBinds
+
+     -- Flags that affect catching of runtime errors
    , Opt_CatchNonexhaustiveCases
-   , Opt_IgnoreAsserts
+   , Opt_LlvmFillUndefWithGarbage
+   , Opt_DoTagInferenceChecks
+
+     -- Flags that affect debugging information
+   , Opt_DistinctConstructorTables
+   , Opt_InfoTableMap
+   , Opt_OrigThunkInfo
    ]
 
 data WarningFlag =


=====================================
compiler/GHC/Driver/Session.hs
=====================================
@@ -66,6 +66,7 @@ module GHC.Driver.Session (
         makeDynFlagsConsistent,
         positionIndependent,
         optimisationFlags,
+        codeGenFlags,
         setFlagsFromEnvFile,
         pprDynFlagsDiff,
         flagSpecOf,
@@ -3481,6 +3482,7 @@ fFlagsDeps = [
   flagSpec "do-eta-reduction"                 Opt_DoEtaReduction,
   flagSpec "do-lambda-eta-expansion"          Opt_DoLambdaEtaExpansion,
   flagSpec "eager-blackholing"                Opt_EagerBlackHoling,
+  flagSpec "orig-thunk-info"                  Opt_OrigThunkInfo,
   flagSpec "embed-manifest"                   Opt_EmbedManifest,
   flagSpec "enable-rewrite-rules"             Opt_EnableRewriteRules,
   flagSpec "enable-th-splice-warnings"        Opt_EnableThSpliceWarnings,


=====================================
compiler/GHC/Iface/Recomp/Flags.hs
=====================================
@@ -67,7 +67,10 @@ fingerprintDynFlags hsc_env this_mod nameio =
         ticky =
           map (`gopt` dflags) [Opt_Ticky, Opt_Ticky_Allocd, Opt_Ticky_LNE, Opt_Ticky_Dyn_Thunk, Opt_Ticky_Tag]
 
-        flags = ((mainis, safeHs, lang, cpp), (paths, prof, ticky, debugLevel, callerCcFilters))
+        -- Other flags which affect code generation
+        codegen = map (`gopt` dflags) (EnumSet.toList codeGenFlags)
+
+        flags = ((mainis, safeHs, lang, cpp), (paths, prof, ticky, codegen, debugLevel, callerCcFilters))
 
     in -- pprTrace "flags" (ppr flags) $
        computeFingerprint nameio flags


=====================================
compiler/GHC/StgToCmm/Bind.hs
=====================================
@@ -730,7 +730,8 @@ setupUpdate closure_info node body
               lbl | bh        = mkBHUpdInfoLabel
                   | otherwise = mkUpdInfoLabel
 
-          pushUpdateFrame lbl (CmmReg (CmmLocal node)) body
+          pushOrigThunkInfoFrame closure_info
+            $ pushUpdateFrame lbl (CmmReg (CmmLocal node)) body
 
   | otherwise   -- A static closure
   = do  { tickyUpdateBhCaf closure_info
@@ -738,7 +739,8 @@ setupUpdate closure_info node body
         ; if closureUpdReqd closure_info
           then do       -- Blackhole the (updatable) CAF:
                 { upd_closure <- link_caf node
-                ; pushUpdateFrame mkBHUpdInfoLabel upd_closure body }
+                ; pushOrigThunkInfoFrame closure_info
+                    $ pushUpdateFrame mkBHUpdInfoLabel upd_closure body }
           else do {tickyUpdateFrameOmitted; body}
     }
 
@@ -754,8 +756,7 @@ pushUpdateFrame lbl updatee body
   = do
        updfr  <- getUpdFrameOff
        profile <- getProfile
-       let
-           hdr         = fixedHdrSize profile
+       let hdr         = fixedHdrSize profile
            frame       = updfr + hdr + pc_SIZEOF_StgUpdateFrame_NoHdr (profileConstants profile)
        --
        emitUpdateFrame (CmmStackSlot Old frame) lbl updatee
@@ -773,6 +774,47 @@ emitUpdateFrame frame lbl updatee = do
   emitStore (cmmOffset platform frame off_updatee) updatee
   initUpdFrameProf frame
 
+-----------------------------------------------------------------------------
+-- Original thunk info table frames
+--
+-- Note [Original thunk info table frames]
+-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+-- In some debugging scenarios (e.g. when debugging cyclic thunks) it can be very
+-- useful to know which thunks the program is in the process of evaluating.
+-- However, in the case of updateable thunks this can be very difficult
+-- to determine since the process of blackholing overwrites the thunk's
+-- info table pointer.
+--
+-- To help in such situations we provide the -forig-thunk-info flag. This enables
+-- code generation logic which pushes a stg_orig_thunk_info_frame stack frame to
+-- accompany each update frame. As the name suggests, this frame captures the
+-- the original info table of the thunk being updated. The entry code for these
+-- frames has no operational effects; the frames merely exist as breadcrumbs
+-- for debugging.
+
+pushOrigThunkInfoFrame :: ClosureInfo -> FCode () -> FCode ()
+pushOrigThunkInfoFrame closure_info body = do
+  cfg <- getStgToCmmConfig
+  if stgToCmmOrigThunkInfo cfg
+     then do_it
+     else body
+  where
+    orig_itbl = mkLblExpr (closureInfoLabel closure_info)
+    do_it = do
+      updfr <- getUpdFrameOff
+      profile <- getProfile
+      let platform = profilePlatform profile
+          hdr = fixedHdrSize profile
+          orig_info_frame_sz =
+              hdr + pc_SIZEOF_StgOrigThunkInfoFrame_NoHdr (profileConstants profile)
+          off_orig_info = hdr + pc_OFFSET_StgOrigThunkInfoFrame_info_ptr (profileConstants profile)
+          frame_off = updfr + orig_info_frame_sz
+          frame = CmmStackSlot Old frame_off
+      --
+      emitStore frame (mkLblExpr mkOrigThunkInfoLabel)
+      emitStore (cmmOffset platform frame off_orig_info) orig_itbl
+      withUpdFrameOff frame_off body
+
 -----------------------------------------------------------------------------
 -- Entering a CAF
 --


=====================================
compiler/GHC/StgToCmm/Config.hs
=====================================
@@ -49,6 +49,7 @@ data StgToCmmConfig = StgToCmmConfig
   , stgToCmmFastPAPCalls   :: !Bool              -- ^
   , stgToCmmSCCProfiling   :: !Bool              -- ^ Check if cost-centre profiling is enabled
   , stgToCmmEagerBlackHole :: !Bool              -- ^
+  , stgToCmmOrigThunkInfo  :: !Bool              -- ^ Push @stg_orig_thunk_info@ frames during thunk update.
   , stgToCmmInfoTableMap   :: !Bool              -- ^ true means generate C Stub for IPE map, See note [Mapping
                                                  -- Info Tables to Source Positions]
   , stgToCmmOmitYields     :: !Bool              -- ^ true means omit heap checks when no allocation is performed


=====================================
docs/users_guide/debugging.rst
=====================================
@@ -1072,6 +1072,18 @@ Checking for consistency
     cases. This is helpful when debugging demand analysis or type checker bugs
     which can sometimes manifest as segmentation faults.
 
+.. ghc-flag:: -forig-thunk-info
+    :shortdesc: Generate ``stg_orig_thunk_info`` stack frames on thunk entry
+    :type: dynamic
+
+    When debugging cyclic thunks it can be helpful to know the original
+    info table of a thunk being evaluated. This flag enables code generation logic
+    to facilitate this, producing a ``stg_orig_thunk_info`` stack frame alongside
+    the usual update frame; such ``orig_thunk`` frames have no operational
+    effect but capture the original info table of the updated thunk for inspection
+    by debugging tools. See ``Note [Original thunk info table frames]`` in
+    ``GHC.StgToCmm.Bind`` for details.
+
 .. ghc-flag:: -fcheck-prim-bounds
     :shortdesc: Instrument array primops with bounds checks.
     :type: dynamic


=====================================
rts/StgMiscClosures.cmm
=====================================
@@ -45,6 +45,17 @@ import CLOSURE stg_ret_t_info;
 import CLOSURE stg_ret_v_info;
 #endif
 
+/* See Note [Original thunk info table frames] in GHC.StgToCmm.Bind. */
+INFO_TABLE_RET (stg_orig_thunk_info_frame, RET_SMALL,
+                W_ info_ptr,
+                W_ thunk_info_ptr)
+    /* no args => explicit stack */
+{
+    unwind Sp = W_[Sp + WDS(2)];
+    Sp_adj(2);
+    jump %ENTRY_CODE(Sp(0)) [*]; // NB. all registers live!
+}
+
 /* ----------------------------------------------------------------------------
    Stack underflow
    ------------------------------------------------------------------------- */


=====================================
rts/include/rts/storage/Closures.h
=====================================
@@ -261,6 +261,13 @@ typedef struct _StgUpdateFrame {
     StgClosure *updatee;
 } StgUpdateFrame;
 
+// Thunk update frame
+//
+// Closure types: RET_SMALL
+typedef struct _StgOrigThunkInfoFrame {
+    StgHeader  header;
+    StgInfoTable *info_ptr;
+} StgOrigThunkInfoFrame;
 
 // Closure types: RET_SMALL
 typedef struct {


=====================================
testsuite/tests/driver/T13914/T13914.stdout
=====================================
@@ -5,11 +5,11 @@ main: Assertion failed
 CallStack (from HasCallStack):
   assert, called at main.hs:3:8 in main:Main
 With -fignore-asserts
-[1 of 2] Compiling Main             ( main.hs, main.o ) [Optimisation flags changed]
+[1 of 2] Compiling Main             ( main.hs, main.o ) [Flags changed]
 [2 of 2] Linking main [Objects changed]
 OK
 Without -fignore-asserts
-[1 of 2] Compiling Main             ( main.hs, main.o ) [Optimisation flags changed]
+[1 of 2] Compiling Main             ( main.hs, main.o ) [Flags changed]
 [2 of 2] Linking main [Objects changed]
 main: Assertion failed
 CallStack (from HasCallStack):


=====================================
utils/deriveConstants/Main.hs
=====================================
@@ -437,6 +437,7 @@ wanteds os = concat
           ,structField  Both "StgEntCounter" "entry_count"
 
           ,closureSize  Both "StgUpdateFrame"
+          ,closureSize  Both "StgOrigThunkInfoFrame"
           ,closureSize  C    "StgCatchFrame"
           ,closureSize  C    "StgStopFrame"
           ,closureSize  C    "StgDeadThreadFrame"
@@ -479,6 +480,7 @@ wanteds os = concat
           ,structSize C "StgTSOProfInfo"
 
           ,closureField Both "StgUpdateFrame" "updatee"
+          ,closureField Both "StgOrigThunkInfoFrame" "info_ptr"
 
           ,closureField C "StgCatchFrame" "handler"
           ,closureField C "StgCatchFrame" "exceptions_blocked"



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/09fb21d748fbc5d6805dcf8e260c16b2ef686fcf...3d32cad7947b22dd753200dbce82039ea86db794

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/09fb21d748fbc5d6805dcf8e260c16b2ef686fcf...3d32cad7947b22dd753200dbce82039ea86db794
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20230509/0a8b1dc3/attachment-0001.html>


More information about the ghc-commits mailing list