#4962: Dead code fed to CorePrep because RULEs keep it alive spuriously --------------------------------------+------------------------------------- Reporter: batterseapower | Owner: Type: bug | Status: closed Priority: normal | Milestone: 7.2.1 Component: Compiler | Version: 7.0.1 Resolution: fixed | Keywords: Os: Unknown/Multiple | Architecture: Unknown/Multiple Failure: Runtime performance bug | Difficulty: Unknown Testcase: | Blockedby: Blocking: | Related: --------------------------------------+------------------------------------- Changes (by simonpj): * difficulty: => Unknown Old description:
This ticket is split off from #4941.
I'm seeing output code that looks roughly like this in the final simplifier output:
{{{ let {-# RULES go (Just x) = $sgo_s1lg x #-} go = ... go ... $w$sgo ... $sgo_s1lg = ... $w$sgo_s1mv ... $w$sgo_s1mv = ... big ... in ... $w$sgo_s1mv ... }}}
This is bad because $sgo is will be dead for the purposes of code generation, but currently GHC will allocate a closure for it at runtime anyway.
What we should do is drop the dead binding once we have decided that it's not reachable via the exported RULES in the interface file.
I've confirmed that this patch achieves this effect (and eliminates the main source of increased allocations I was seeing in #4941):
{{{ hunk ./compiler/main/TidyPgm.lhs 22 +import OccurAnal ( occurAnalysePgm ) hunk ./compiler/main/TidyPgm.lhs 43 +import qualified ErrUtils as Err hunk ./compiler/main/TidyPgm.lhs 47 +import PprCore hunk ./compiler/main/TidyPgm.lhs 322 + + -- Occurrence analyse the input program, assuming all local rules are off, + -- but retaining any bindings referred to by external rules. + -- Occurrence information may have improved since the last run of the + -- simplifier because some bindings will become dead once RULES are dropped. + -- + -- The analysis itself will take care of dropping any newly- dead syntax. + ; Err.dumpIfSet_dyn dflags Opt_D_dump_occur_anal "BEFORE TidyPgm occurrence analysis" (pprCoreBindings binds) + ; binds <- return $ occurAnalysePgm Nothing ext_rules binds + ; Err.dumpIfSet_dyn dflags Opt_D_dump_occur_anal "TidyPgm occurrence analysis" (pprCoreBindings binds) + hunk ./compiler/simplCore/OccurAnal.lhs 393 - rule_fvs = idRuleVars bndr -- See Note [Rule dependency info] + rule_fvs = case occ_rule_act env of Nothing -> emptyVarSet; Just _ -> idRuleVars bndr -- See Note [Rule dependency info] + -- FIXME: this is a terrible hack to try to have OccAnal drop bindings that are kept alive only due to rules at the CoreTidy stage }}}
However the patch is a bit horrible: because attached RULES keep bindings alive in a LetRec even if the rules can never be activated (i.e. occ_rule_act env == Nothing) the bindings I want to be dropped never get dropped. The reason I consider my fix a hack is that just because the rules are inactive *now* doesn't mean that they will be inactive *later*.
A better approach would perhaps be to wait until the RULES are stripped from the binders entirely (e.g. OccAnal the output of CoreTidy). However, this is not straightforward because CoreTidy has globalised Ids, so OccAnaling the output says that all top level bindings have no FVs and hence turns all LetRecs into Lets!
New description: This ticket is split off from #4941. I'm seeing output code that looks roughly like this in the final simplifier output: {{{ let {-# RULES go (Just x) = $sgo_s1lg x #-} go = ... go ... $w$sgo ... $sgo_s1lg = ... $w$sgo_s1mv ... $w$sgo_s1mv = ... big ... in ... $w$sgo_s1mv ... }}} This is bad because $sgo is will be dead for the purposes of code generation, but currently GHC will allocate a closure for it at runtime anyway. What we should do is drop the dead binding once we have decided that it's not reachable via the exported RULES in the interface file. I've confirmed that this patch achieves this effect (and eliminates the main source of increased allocations I was seeing in #4941): {{{ hunk ./compiler/main/TidyPgm.lhs 22 +import OccurAnal ( occurAnalysePgm ) hunk ./compiler/main/TidyPgm.lhs 43 +import qualified ErrUtils as Err hunk ./compiler/main/TidyPgm.lhs 47 +import PprCore hunk ./compiler/main/TidyPgm.lhs 322 + + -- Occurrence analyse the input program, assuming all local rules are off, + -- but retaining any bindings referred to by external rules. + -- Occurrence information may have improved since the last run of the + -- simplifier because some bindings will become dead once RULES are dropped. + -- + -- The analysis itself will take care of dropping any newly- dead syntax. + ; Err.dumpIfSet_dyn dflags Opt_D_dump_occur_anal "BEFORE TidyPgm occurrence analysis" (pprCoreBindings binds) + ; binds <- return $ occurAnalysePgm Nothing ext_rules binds + ; Err.dumpIfSet_dyn dflags Opt_D_dump_occur_anal "TidyPgm occurrence analysis" (pprCoreBindings binds) + hunk ./compiler/simplCore/OccurAnal.lhs 393 - rule_fvs = idRuleVars bndr -- See Note [Rule dependency info] + rule_fvs = case occ_rule_act env of Nothing -> emptyVarSet; Just _ -> idRuleVars bndr -- See Note [Rule dependency info] + -- FIXME: this is a terrible hack to try to have OccAnal drop bindings that are kept alive only due to rules at the CoreTidy stage }}} However the patch is a bit horrible: because attached RULES keep bindings alive in a !LetRec even if the rules can never be activated (i.e. occ_rule_act env == Nothing) the bindings I want to be dropped never get dropped. The reason I consider my fix a hack is that just because the rules are inactive *now* doesn't mean that they will be inactive *later*. A better approach would perhaps be to wait until the RULES are stripped from the binders entirely (e.g. !OccAnal the output of !CoreTidy). However, this is not straightforward because !CoreTidy has globalised Ids, so OccAnaling the output says that all top level bindings have no FVs and hence turns all !LetRecs into Lets! -- -- Ticket URL: http://hackage.haskell.org/trac/ghc/ticket/4962#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler