From 3bb324543f4cc92cf6e543919efa74a9ac90b8c6 Mon Sep 17 00:00:00 2001 From: "simonpj@microsoft.com" Date: Fri, 14 Jan 2011 16:29:27 +0000 Subject: [PATCH] Fix a buglet in postInlineUnconditionally Under obscure circumstances (actually only shown up when fixing something else) it was possible for a variable binding to be discarded although it was still used. See Note [Top level and postInlineUnconditionally] --- compiler/simplCore/SimplUtils.lhs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/compiler/simplCore/SimplUtils.lhs b/compiler/simplCore/SimplUtils.lhs index 76ce1f9..99a63e4 100644 --- a/compiler/simplCore/SimplUtils.lhs +++ b/compiler/simplCore/SimplUtils.lhs @@ -857,7 +857,7 @@ a thing based on the form of its RHS; in particular if it has a trivial RHS. If so, we can inline and discard the binding altogether. NB: a loop breaker has must_keep_binding = True and non-loop-breakers -only have *forward* references Hence, it's safe to discard the binding +only have *forward* references. Hence, it's safe to discard the binding NOTE: This isn't our last opportunity to inline. We're at the binding site right now, and we'll get another opportunity when we get to the @@ -892,8 +892,8 @@ postInlineUnconditionally env top_lvl bndr occ_info rhs unfolding -- because it might be referred to "earlier" | isExportedId bndr = False | isStableUnfolding unfolding = False -- Note [InlineRule and postInlineUnconditionally] - | exprIsTrivial rhs = True | isTopLevel top_lvl = False -- Note [Top level and postInlineUnconditionally] + | exprIsTrivial rhs = True | otherwise = case occ_info of -- The point of examining occ_info here is that for *non-values* @@ -960,14 +960,29 @@ postInlineUnconditionally env top_lvl bndr occ_info rhs unfolding Note [Top level and postInlineUnconditionally] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -We don't do postInlineUnconditionally for top-level things (exept ones that -are trivial): - * There is no point, because the main goal is to get rid of local - bindings used in multiple case branches. +We don't do postInlineUnconditionally for top-level things (even for +ones that are trivial): + * Doing so will inline top-level error expressions that have been carefully floated out by FloatOut. More generally, it might replace static allocation with dynamic. + * Even for trivial expressions there's a problem. Consider + {-# RULE "foo" forall (xs::[T]). reverse xs = ruggle xs #-} + blah xs = reverse xs + ruggle = sort + In one simplifier pass we might fire the rule, getting + blah xs = ruggle xs + but in *that* simplifier pass we must not do postInlineUnconditionally + on 'ruggle' because then we'll have an unbound occurrence of 'ruggle' + + If the rhs is trivial it'll be inlined by callSiteInline, and then + the binding will be dead and discarded by the next use of OccurAnal + + * There is less point, because the main goal is to get rid of local + bindings used in multiple case branches. + + Note [InlineRule and postInlineUnconditionally] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Do not do postInlineUnconditionally if the Id has an InlineRule, otherwise -- 1.7.10.4