From 228f6361f1477c7e6d809e18ab28ae097fd4b840 Mon Sep 17 00:00:00 2001 From: Simon Peyton Jones Date: Tue, 15 Jun 2021 23:04:15 +0100 Subject: Add comments explaining why #19833 is wrong I realised that the suggestion in #19833 doesn't work, and documented why in Note [Zapping Used Once info in WorkWrap] --- compiler/GHC/Core/Opt/WorkWrap.hs | 81 ++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 36 deletions(-) diff --git a/compiler/GHC/Core/Opt/WorkWrap.hs b/compiler/GHC/Core/Opt/WorkWrap.hs index 87dcd92d1e..046b7705b1 100644 --- a/compiler/GHC/Core/Opt/WorkWrap.hs +++ b/compiler/GHC/Core/Opt/WorkWrap.hs @@ -539,7 +539,7 @@ tryWW ww_opts is_rec fn_id rhs new_fn_id = zapIdUsedOnceInfo (zapIdUsageEnvInfo fn_id) -- See Note [Zapping DmdEnv after Demand Analyzer] and - -- See Note [Zapping Used Once info WorkWrap] + -- See Note [Zapping Used Once info in WorkWrap] -- is_eta_exp: see Note [Don't eta expand in w/w] is_eta_exp = length wrap_dmds == manifestArity rhs @@ -570,47 +570,56 @@ Note [Final Demand Analyser run] in GHC.Core.Opt.DmdAnal). Note [Zapping Used Once info in WorkWrap] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -In the worker-wrapper pass we zap the used once info in demands and in -strictness signatures. +During the work/wrap pass, using zapIdUsedOnceInfo, we zap the "used once" info +* on every binder (let binders, case binders, lambda binders) +* in both demands and in strictness signatures +* recursively Why? * The simplifier may happen to transform code in a way that invalidates the data (see #11731 for an example). * It is not used in later passes, up to code generation. -So as the data is useless and possibly wrong, we want to remove it. The most -convenient place to do that is the worker wrapper phase, as it runs after every -run of the demand analyser besides the very last one (which is the one where we -want to _keep_ the info for the code generator). - -We do not do it in the demand analyser for the same reasons outlined in -Note [Zapping DmdEnv after Demand Analyzer] above. - -For example, consider - let y = factorial v in - let x = y in - x + x - -Demand analysis will conclude, correctly, that `y` is demanded once. But if we inline `x` we get - let y = factorial v in - y + y - -Similarly for - f y = let x = y in x+x -where we will put a used-once demand on y, and hence also in f's demand signature. - -And recursively - f y = case y of (p,q) -> let p2 = p in p2+p2 - Here we'll get a used-once demand on p; but that is not robust to inlining p2. - -Conclusion: "demanded once" info is fragile. -* We want it after the final immediately-before-code-gen demand analysis, so we can identify single-entry thunks. -* But we don't want it otherwise because it is not robust. - -Conclusion: kill it during worker/wrapper, using `zapUsedOnceInfo`. Both the *demand signature* of -the binder, and the *demand-info* of the binder. Moreover, do so recursively. - -(NB THE pre-code-gen demand analysis is not followed by worker/wrapper.) +At first it's hard to see how the simplifier might invalidate it (and +indeed for a while I thought it couldn't: #19482), but it's not quite +as simple as I thought. Consider this: + {-# STRICTNESS SIG #-} + f p = let v = case p of (a,b) -> a + in p `seq` (v,v) + +I think we'll give `f` the strictness signature ``, where the +`M` sayd that we'll evaluate the first component of the pair at most +once. Why? Because the RHS of the thunk `v` is evaluated at most +once. + +But now let's worker/wrapper f: + {-# STRICTNESS SIG #-} + $wf p1 = let p2 = absentError "urk" in + let p = (p1,p2) in + let v = case p of (a,b) -> a + in p `seq` (v,v) + +where I've gotten the demand on `p1` by decomposing the P(M,A) argument demand. +This rapidly simplifies to + {-# STRICTNESS SIG #-} + $wf p1 = let v = p1 in + (v,v) + +and thence to `(p1,p1)` by inlining the trivial let. Now the demand on `p1` should +not be at most once!! + +Conclusion: used-once info is fragile to simplification, because of +the non-monotonic behaviour of let's, which turn used-many into +used-once. So indeed we should zap this info in worker/wrapper. + +Conclusion: kill it during worker/wrapper, using `zapUsedOnceInfo`. +Both the *demand signature* of the binder, and the *demand-info* of +the binder. Moreover, do so recursively. + +You might wonder: why do we generate used-once info if we then throw +it away. The main reason is that we do a final run of the demand analyser, +immediately before CoreTidy, which is /not/ followed by worker/wrapper; it +is there only to generate used-once info for single-entry thunks. Note [Don't eta expand in w/w] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -- cgit v1.2.1