[commit: ghc] master: Comments only (e08d34b)

[git at git.haskell.org]

Repository : ssh://git@git.haskell.org/ghc

On branch  : master
Link       : http://ghc.haskell.org/trac/ghc/changeset/e08d34bbb4e68cd2c14e8e94ca3933ce5407d65b/ghc

>---------------------------------------------------------------

commit e08d34bbb4e68cd2c14e8e94ca3933ce5407d65b
Author: Simon Peyton Jones <simonpj at microsoft.com>
Date:   Wed Nov 28 15:30:10 2018 +0000

    Comments only


>---------------------------------------------------------------

e08d34bbb4e68cd2c14e8e94ca3933ce5407d65b
 compiler/codeGen/StgCmmCon.hs  | 14 ++++++--------
 compiler/codeGen/StgCmmExpr.hs | 28 +++++++++++++++++-----------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/compiler/codeGen/StgCmmCon.hs b/compiler/codeGen/StgCmmCon.hs
index 2ddeceb..258896f 100644
--- a/compiler/codeGen/StgCmmCon.hs
+++ b/compiler/codeGen/StgCmmCon.hs
@@ -272,14 +272,12 @@ bindConArgs (DataAlt con) base args
            -- when accessing the constructor field.
            bind_arg :: (NonVoid Id, ByteOff) -> FCode (Maybe LocalReg)
            bind_arg (arg@(NonVoid b), offset)
-             | isDeadBinder b =
-                 -- Do not load unused fields from objects to local variables.
-                 -- (CmmSink can optimize this, but it's cheap and common enough
-                 -- to handle here)
-                 return Nothing
-             | otherwise      = do
-                 emit $ mkTaggedObjectLoad dflags (idToReg dflags arg) base offset tag
-                 Just <$> bindArgToReg arg
+             | isDeadBinder b  -- See Note [Dead-binder optimisation] in StgCmmExpr
+             = return Nothing
+             | otherwise
+             = do { emit $ mkTaggedObjectLoad dflags (idToReg dflags arg)
+                                              base offset tag
+                  ; Just <$> bindArgToReg arg }
 
        mapMaybeM bind_arg args_w_offsets
 
diff --git a/compiler/codeGen/StgCmmExpr.hs b/compiler/codeGen/StgCmmExpr.hs
index 30603ee..7a2340e 100644
--- a/compiler/codeGen/StgCmmExpr.hs
+++ b/compiler/codeGen/StgCmmExpr.hs
@@ -305,7 +305,7 @@ cgCase (StgOpApp (StgPrimOp op) args _) bndr (AlgAlt tycon) alts
   = do { tag_expr <- do_enum_primop op args
 
        -- If the binder is not dead, convert the tag to a constructor
-       -- and assign it. See Note [Dead case binders in -O0]
+       -- and assign it. See Note [Dead-binder optimisation]
        ; unless (isDeadBinder bndr) $ do
             { dflags <- getDynFlags
             ; tmp_reg <- bindArgToReg (NonVoid bndr)
@@ -386,17 +386,23 @@ arguments in the environment; they don't live anywhere.  See the
 calls to nonVoidIds in various places.  So we must not look up
 's' in the environment.  Instead, just evaluate the RHS!  Simple.
 
-Note [Dead case binders in -O0]
+Note [Dead-binder optimisation]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-Before CmmSink came to eliminate dead assignments, omitting assignment of dead
-case binders was a cheap and worthwhile optimisation. This probably also was the
-reason for occurrence hacks such as in Phab:D5339 to exist, because the
-occurrence information preserved by 'CoreTidy.tidyIdBndr' was insufficient.
-
-Nowadays, with CmmSink there's little reason to complicate the code by checking
-for dead case binders, except that CmmSink won't run with -O0. Since the
-majority of case binders are dead, this optimisation probably still has a great
-benefit-cost ratio and we want to keep it for -O0. See also Phab:D5358.
+A case-binder, or data-constructor argument, may be marked as dead,
+because we preserve occurrence-info on binders in CoreTidy (see
+CoreTidy.tidyIdBndr).
+
+If the binder is dead, we can sometimes eliminate a load.  While
+CmmSink will eliminate that load, it's very easy to kill it at source
+(giving CmmSink less work to do), and in any case CmmSink only runs
+with -O. Since the majority of case binders are dead, this
+optimisation probably still has a great benefit-cost ratio and we want
+to keep it for -O0. See also Phab:D5358.
+
+This probably also was the reason for occurrence hack in Phab:D5339 to
+exist, perhaps because the occurrence information preserved by
+'CoreTidy.tidyIdBndr' was insufficient.  But now that CmmSink does the
+job we deleted the hacks.
 -}
 
 cgCase (StgApp v []) _ (PrimAlt _) alts