From 9fddc2160c524d4fefb9fc8a42704f812aef7bf3 Mon Sep 17 00:00:00 2001 From: "simonpj@microsoft.com" Date: Fri, 7 Dec 2007 17:05:07 +0000 Subject: [PATCH] Remove debug warning, and explain why --- compiler/main/TidyPgm.lhs | 10 +++++++--- compiler/stranal/WorkWrap.lhs | 25 ++++++++++++++----------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/compiler/main/TidyPgm.lhs b/compiler/main/TidyPgm.lhs index 1f65d21..131b617 100644 --- a/compiler/main/TidyPgm.lhs +++ b/compiler/main/TidyPgm.lhs @@ -817,7 +817,7 @@ tidyWorker _tidy_env _show_unfold NoWorker = NoWorker tidyWorker tidy_env show_unfold (HasWorker work_id wrap_arity) | show_unfold = HasWorker (tidyVarOcc tidy_env work_id) wrap_arity - | otherwise = WARN( True, ppr work_id ) NoWorker + | otherwise = NoWorker -- NB: do *not* expose the worker if show_unfold is off, -- because that means this thing is a loop breaker or -- marked NOINLINE or something like that @@ -825,8 +825,12 @@ tidyWorker tidy_env show_unfold (HasWorker work_id wrap_arity) -- then you can make the simplifier go into an infinite loop, because -- in effect the unfolding is exposed. See Trac #1709 -- - -- Mind you, it probably should not be w/w'd in the first place; - -- hence the WARN + -- You might think that if show_unfold is False, then the thing should + -- not be w/w'd in the first place. But a legitimate reason is this: + -- the function returns bottom + -- In this case, show_unfold will be false (we don't expose unfoldings + -- for bottoming functions), but we might still have a worker/wrapper + -- split (see Note [Worker-wrapper for bottoming functions] in WorkWrap.lhs \end{code} %************************************************************************ diff --git a/compiler/stranal/WorkWrap.lhs b/compiler/stranal/WorkWrap.lhs index fbc0bf7..3667da8 100644 --- a/compiler/stranal/WorkWrap.lhs +++ b/compiler/stranal/WorkWrap.lhs @@ -376,17 +376,7 @@ worthSplittingFun ds res = any worth_it ds || returnsCPR res -- worthSplitting returns False for an empty list of demands, -- and hence do_strict_ww is False if arity is zero and there is no CPR - - -- We used not to split if the result is bottom. - -- [Justification: there's no efficiency to be gained.] - -- But it's sometimes bad not to make a wrapper. Consider - -- fw = \x# -> let x = I# x# in case e of - -- p1 -> error_fn x - -- p2 -> error_fn x - -- p3 -> the real stuff - -- The re-boxing code won't go away unless error_fn gets a wrapper too. - -- [We don't do reboxing now, but in general it's better to pass - -- an unboxed thing to f, and have it reboxed in the error cases....] + -- See Note [Worker-wrapper for bottoming functions] where worth_it Abs = True -- Absent arg worth_it (Eval (Prod ds)) = True -- Product arg to evaluate @@ -403,6 +393,19 @@ worthSplittingThunk maybe_dmd res worth_it other = False \end{code} +Note [Worker-wrapper for bottoming functions] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +We used not to split if the result is bottom. +[Justification: there's no efficiency to be gained.] + +But it's sometimes bad not to make a wrapper. Consider + fw = \x# -> let x = I# x# in case e of + p1 -> error_fn x + p2 -> error_fn x + p3 -> the real stuff +The re-boxing code won't go away unless error_fn gets a wrapper too. +[We don't do reboxing now, but in general it's better to pass an +unboxed thing to f, and have it reboxed in the error cases....] %************************************************************************ -- 1.7.10.4