From 80965661715b664b4926fc525d0c89306b92e832 Mon Sep 17 00:00:00 2001
From: "simonpj@microsoft.com" <unknown>
Date: Thu, 10 Aug 2006 14:11:45 +0000
Subject: [PATCH] Make postInlineUnconditaionally more conservative

I'm being more paranoid about repeatedly simplifying things (to avoid
exponential behaviour.)  postInlineUnconditionally looks as if it
could repeated simplify the same expression; this patch stops it doing
so.

The extra lines are all comments!
---
 compiler/simplCore/SimplUtils.lhs |   63 +++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 23 deletions(-)

diff --git a/compiler/simplCore/SimplUtils.lhs b/compiler/simplCore/SimplUtils.lhs
index 2fd3870..202a617 100644
--- a/compiler/simplCore/SimplUtils.lhs
+++ b/compiler/simplCore/SimplUtils.lhs
@@ -695,7 +695,13 @@ our new view that inlining is like a RULE, so I'm sticking to the 'active'
 story for now.
 
 \begin{code}
-postInlineUnconditionally :: SimplEnv -> TopLevelFlag -> OutId -> OccInfo -> OutExpr -> Unfolding -> Bool
+postInlineUnconditionally 
+    :: SimplEnv -> TopLevelFlag
+    -> InId		-- The binder (an OutId would be fine too)
+    -> OccInfo 		-- From the InId
+    -> OutExpr
+    -> Unfolding
+    -> Bool
 postInlineUnconditionally env top_lvl bndr occ_info rhs unfolding
   | not active		   = False
   | isLoopBreaker occ_info = False
@@ -703,19 +709,28 @@ postInlineUnconditionally env top_lvl bndr occ_info rhs unfolding
   | exprIsTrivial rhs 	   = True
   | otherwise
   = case occ_info of
-      OneOcc in_lam one_br int_cxt
-	->     (one_br || smallEnoughToInline unfolding)	-- Small enough to dup
+	-- The point of examining occ_info here is that for *non-values* 
+	-- that occur outside a lambda, the call-site inliner won't have
+	-- a chance (becuase it doesn't know that the thing
+	-- only occurs once).   The pre-inliner won't have gotten
+	-- it either, if the thing occurs in more than one branch
+	-- So the main target is things like
+	--	let x = f y in
+	--	case v of
+	--	   True  -> case x of ...
+	--	   False -> case x of ...
+	-- I'm not sure how important this is in practice
+      OneOcc in_lam one_br int_cxt	-- OneOcc => no work-duplication issue
+	->     smallEnoughToInline unfolding	-- Small enough to dup
 			-- ToDo: consider discount on smallEnoughToInline if int_cxt is true
 			--
-		 	-- NB: Do we want to inline arbitrarily big things becuase
-			-- one_br is True? that can lead to inline cascades.  But
-			-- preInlineUnconditionlly has dealt with all the common cases
-			-- so perhaps it's worth the risk. Here's an example
-			--	let f = if b then Left (\x.BIG) else Right (\y.BIG)
-			--	in \y. ....f....
-			-- We can't preInlineUnconditionally because that woud invalidate
-			-- the occ info for b.  Yet f is used just once, and duplicating
-			-- the case work is fine (exprIsCheap).
+		 	-- NB: Do NOT inline arbitrarily big things, even if one_br is True
+			-- Reason: doing so risks exponential behaviour.  We simplify a big
+			--	   expression, inline it, and simplify it again.  But if the
+			--	   very same thing happens in the big expression, we get 
+			--	   exponential cost!
+			-- PRINCIPLE: when we've already simplified an expression once, 
+			-- make sure that we only inline it if it's reasonably small.
 
 	   &&  ((isNotTopLevel top_lvl && not in_lam) || 
 			-- But outside a lambda, we want to be reasonably aggressive
@@ -732,17 +747,19 @@ postInlineUnconditionally env top_lvl bndr occ_info rhs unfolding
 			-- good reason.  See the notes on int_cxt in preInlineUnconditionally
 
       other -> False
-	-- The point here is that for *non-values* that occur
-	-- outside a lambda, the call-site inliner won't have
-	-- a chance (becuase it doesn't know that the thing
-	-- only occurs once).   The pre-inliner won't have gotten
-	-- it either, if the thing occurs in more than one branch
-	-- So the main target is things like
-	--	let x = f y in
-	--	case v of
-	--	   True  -> case x of ...
-	--	   False -> case x of ...
-	-- I'm not sure how important this is in practice
+
+-- Here's an example that we don't handle well:
+--	let f = if b then Left (\x.BIG) else Right (\y.BIG)
+--	in \y. ....case f of {...} ....
+-- Here f is used just once, and duplicating the case work is fine (exprIsCheap).
+-- But
+-- * We can't preInlineUnconditionally because that woud invalidate
+--   the occ info for b.  
+-- * We can't postInlineUnconditionally because the RHS is big, and
+--   that risks exponential behaviour
+-- * We can't call-site inline, because the rhs is big
+-- Alas!
+
   where
     active = case getMode env of
 		   SimplGently  -> isAlwaysActive prag
-- 
1.7.10.4