Commit 592269df authored by simonpj@microsoft.com's avatar simonpj@microsoft.com
Browse files

Fix #1709: do not expose the worker for a loop-breaker

The massive 'Uni' program produced a situation in which a function that
had a worker/wrapper split was chosen as a loop breaker.  If the worker
is exposed in the interface file, then an importing module may go into
an inlining loop: see comments on TidyPgm.tidyWorker.

This patch fixes the inlining bug.  The code that gives rise to this
bizarre case is still not good (it's a bunch of implication constraints
and we are choosing a bad loop breaker) but the first thing is to fix the
bug.

It's rather hard to produce a test case!

Please merge to the 6.8 branch.

parent 56a39804
...@@ -717,9 +717,10 @@ tidyTopPair ext_ids rhs_tidy_env caf_info name' (bndr, rhs) ...@@ -717,9 +717,10 @@ tidyTopPair ext_ids rhs_tidy_env caf_info name' (bndr, rhs)
bndr' = mkVanillaGlobal name' ty' idinfo' bndr' = mkVanillaGlobal name' ty' idinfo'
ty' = tidyTopType (idType bndr) ty' = tidyTopType (idType bndr)
rhs' = tidyExpr rhs_tidy_env rhs rhs' = tidyExpr rhs_tidy_env rhs
idinfo = idInfo bndr
idinfo' = tidyTopIdInfo rhs_tidy_env (isJust maybe_external) idinfo' = tidyTopIdInfo rhs_tidy_env (isJust maybe_external)
(idInfo bndr) unfold_info arity idinfo unfold_info worker_info
caf_info arity caf_info
-- Expose an unfolding if ext_ids tells us to -- Expose an unfolding if ext_ids tells us to
-- Remember that ext_ids maps an Id to a Bool: -- Remember that ext_ids maps an Id to a Bool:
...@@ -728,6 +729,7 @@ tidyTopPair ext_ids rhs_tidy_env caf_info name' (bndr, rhs) ...@@ -728,6 +729,7 @@ tidyTopPair ext_ids rhs_tidy_env caf_info name' (bndr, rhs)
show_unfold = maybe_external `orElse` False show_unfold = maybe_external `orElse` False
unfold_info | show_unfold = mkTopUnfolding rhs' unfold_info | show_unfold = mkTopUnfolding rhs'
| otherwise = noUnfolding | otherwise = noUnfolding
worker_info = tidyWorker rhs_tidy_env show_unfold (workerInfo idinfo)
-- Usually the Id will have an accurate arity on it, because -- Usually the Id will have an accurate arity on it, because
-- the simplifier has just run, but not always. -- the simplifier has just run, but not always.
...@@ -751,7 +753,7 @@ tidyTopPair ext_ids rhs_tidy_env caf_info name' (bndr, rhs) ...@@ -751,7 +753,7 @@ tidyTopPair ext_ids rhs_tidy_env caf_info name' (bndr, rhs)
-- unfoldings, which are inside Ids imported by GHCi. Ditto RULES. -- unfoldings, which are inside Ids imported by GHCi. Ditto RULES.
-- CoreToStg makes use of this when constructing SRTs. -- CoreToStg makes use of this when constructing SRTs.
tidyTopIdInfo tidy_env is_external idinfo unfold_info arity caf_info tidyTopIdInfo tidy_env is_external idinfo unfold_info worker_info arity caf_info
| not is_external -- For internal Ids (not externally visible) | not is_external -- For internal Ids (not externally visible)
= vanillaIdInfo -- we only need enough info for code generation = vanillaIdInfo -- we only need enough info for code generation
-- Arity and strictness info are enough; -- Arity and strictness info are enough;
...@@ -767,17 +769,27 @@ tidyTopIdInfo tidy_env is_external idinfo unfold_info arity caf_info ...@@ -767,17 +769,27 @@ tidyTopIdInfo tidy_env is_external idinfo unfold_info arity caf_info
`setAllStrictnessInfo` newStrictnessInfo idinfo `setAllStrictnessInfo` newStrictnessInfo idinfo
`setInlinePragInfo` inlinePragInfo idinfo `setInlinePragInfo` inlinePragInfo idinfo
`setUnfoldingInfo` unfold_info `setUnfoldingInfo` unfold_info
`setWorkerInfo` tidyWorker tidy_env (workerInfo idinfo) `setWorkerInfo` worker_info
-- NB: we throw away the Rules -- NB: we throw away the Rules
-- They have already been extracted by findExternalRules -- They have already been extracted by findExternalRules
------------ Worker -------------- ------------ Worker --------------
tidyWorker tidy_env (HasWorker work_id wrap_arity) tidyWorker tidy_env show_unfold NoWorker
= HasWorker (tidyVarOcc tidy_env work_id) wrap_arity
tidyWorker tidy_env other
= 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
-- 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
-- This is important: if you expose the worker for a loop-breaker
-- 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
\end{code} \end{code}
%************************************************************************ %************************************************************************
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment