Include closure header size in StgLamLift's estimations
authorSebastian Graf <sebastian.graf@kit.edu>
Thu, 21 Feb 2019 15:02:38 +0000 (16:02 +0100)
committerMarge Bot <ben+marge-bot@smart-cactus.org>
Sun, 24 Feb 2019 03:02:10 +0000 (22:02 -0500)
While playing around with late lambda lifting, I realised that
StgLamLift.Analysis doesn't consider the removed closure header in its
allocation estimations.

That's because contrary to what I thought, the total word count returned
by `mkVirtHeapOffsets` doesn't include the size of the closure header.

We just add the header size manually now.

compiler/simplStg/StgLiftLams/Analysis.hs

index 7fb60df..3cdbfcb 100644 (file)
@@ -484,13 +484,12 @@ rhsLambdaBndrs (StgRhsClosure _ _ _ bndrs _) = map binderInfoBndr bndrs
 -- | The size in words of a function closure closing over the given 'Id's,
 -- including the header.
 closureSize :: DynFlags -> [Id] -> WordOff
-closureSize dflags ids = words
+closureSize dflags ids = words + sTD_HDR_SIZE dflags
+  -- We go through sTD_HDR_SIZE rather than fixedHdrSizeW so that we don't
+  -- optimise differently when profiling is enabled.
   where
     (words, _, _)
       -- Functions have a StdHeader (as opposed to ThunkHeader).
-      -- Note that mkVirtHeadOffsets will account for profiling headers, so
-      -- lifting decisions vary if we begin to profile stuff. Maybe we shouldn't
-      -- do this or deactivate profiling in @dflags@?
       = StgCmmLayout.mkVirtHeapOffsets dflags StgCmmLayout.StdHeader
       . StgCmmClosure.addIdReps
       . StgCmmClosure.nonVoidIds