diff options
Diffstat (limited to 'compiler/codeGen')
| -rw-r--r-- | compiler/codeGen/StgCmmCon.hs | 14 | ||||
| -rw-r--r-- | 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 2ddeceb825..258896ff1a 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 30603ee904..7a2340ed5f 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 |