[GHC] #13208: Do two-phase inlining in simpleOptPgm

#13208: Do two-phase inlining in simpleOptPgm -------------------------------------+------------------------------------- Reporter: lukemaurer | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.1 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- As of #12988, having `simpleOptPgm` leave β-redexes lying around is painful because it requires a special loophole in Core Lint to allow a jump inside a β-redex. We'd rather use a two-phase approach, mirroring the simplifier's `preInlineUnconditionally` and `postInlineUnconditionally`, so we can aggressively eliminate β-redexes without fear of exponential blowup. Consider: {{{ joinrec j1 x = e1 in join j2 x y = jump j1 e2 in jump j2 e3 e4 }}} Since `j2` is only used once, it gets inlined. But `simpleOptPgm` only performs inlining //after// a binding is simplified, so we end up here: {{{ joinrec j1 x = e1' in (\x y -> jump j1 e2') e3' e4' }}} Since `e2'` was already simplified, performing the β-reduction here risks exponential blowup for the same reason it can happen in the simplifier (see the Secrets paper; `perf/compiler/T783` is a live example, increasing in allocs by over 80% if we re-simplify here). We //could// just `let`-bind `e3'` and `e4'` (carefully avoiding capturing free occurrences of `x` in `e4'`), but not if one them is actually a type argument. Well, okay, we //could// introduce a type `let`, but doing that here means the desugarer can now return a type `let` and we're not prepared for that. (Specifically, under certain circumstances, the simplifier never runs in between the desugarer and Float Out, and the latter breaks in the presence of type `let`.) So for the moment, we leave the β-redex there, but this needs a special rule just for β-redexes because normally you can't have a jump under a lambda (or an application, for that matter). In the long term, we'd prefer to take the simplifier's approach instead: If we want to inline something because it only occurs once (even though it may be big), we add it to the substitution //before// simplifying it so that we only simplify it once. This means the substitution has to carry lexical closures around, though, just like the simplifier does (see `SimplSR`'s `ContEx` disjunct), so it's not trivial. The `simpleOptPgm` β-redexes are the only reason for the special rule in Core Lint (see Note [Beta redexes] in `CoreLint.hs`), so once this is done we can be rid of that. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13208 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13208: Do two-phase inlining in simpleOptPgm
-------------------------------------+-------------------------------------
Reporter: lukemaurer | Owner:
Type: bug | Status: new
Priority: normal | Milestone:
Component: Compiler | Version: 8.1
Resolution: | Keywords: JoinPoints
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s):
Wiki Page: |
-------------------------------------+-------------------------------------
Changes (by simonpj):
* keywords: => JoinPoints
Comment:
Done!
{{{
commit 8e9593fb2147252ecb8b685ef6bf9c0237a71219
Author: Simon Peyton Jones
The simpleOptPgm β-redexes are the only reason for the special rule in Core Lint (see Note [Beta redexes] in CoreLint.hs), so once this is done we can be rid of that.
but don't we sometimes generate some beta-redexes in worker/wrapper after strictness analysis? I have not yet changed this. Adding keyword `JoinPoints` so we don't lose track of this. It'd be cool to finish it off. Ideally getting rid of type-lets too. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13208#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13208: Do two-phase inlining in simpleOptPgm -------------------------------------+------------------------------------- Reporter: lukemaurer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: JoinPoints Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by bjmprice): What's the status of this ticket? I ask because in `Note [The simple optimiser]`, it is claimed that "It thereby guarantees to leave no un- reduced beta-redexes.", but in 8.6.3 the Haskell `beta = let f = \x -> x in f True` gives rise to the Core `beta = (\ (x_aVG :: Bool) -> x_aVG) GHC.Types.True`, which seems to contradict that claim. (This core is with `-ddump-ds`, which I think runs the simple optimiser. With `-ddump-ds-preopt` it is a let-binding, not a beta-redex.) I guess that the Note could mean that "all beta-redexes in the input are reduced", rather than "there are no beta-redexes in the output", but this isn't how I understood the text. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13208#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13208: Do two-phase inlining in simpleOptPgm -------------------------------------+------------------------------------- Reporter: lukemaurer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: JoinPoints Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bjmprice): * cc: bjmprice (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13208#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13208: Do two-phase inlining in simpleOptPgm -------------------------------------+------------------------------------- Reporter: lukemaurer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: JoinPoints Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | deSugar/should_compile/T13208 Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/394 -------------------------------------+------------------------------------- Changes (by RyanGlScott): * testcase: => deSugar/should_compile/T13208 * differential: => https://gitlab.haskell.org/ghc/ghc/merge_requests/394 Comment: simonpj, what is the status of this ticket post-[https://gitlab.haskell.org/ghc/ghc/commit/5eeefe4c1e007ea2098f241634b48a4dad... 5eeefe4c1e007ea2098f241634b48a4dada785a5]? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13208#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13208: Do two-phase inlining in simpleOptPgm -------------------------------------+------------------------------------- Reporter: lukemaurer | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: JoinPoints Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | deSugar/should_compile/T13208 Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/394 -------------------------------------+------------------------------------- Changes (by simonpj): * status: new => closed * resolution: => fixed Comment: It's fixed! Thanks for reporting the infelicity in comment:2, @bjmprice. The commit mentioned in comment:4 fixes your problem. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13208#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13208: Do two-phase inlining in simpleOptPgm -------------------------------------+------------------------------------- Reporter: lukemaurer | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: JoinPoints Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | deSugar/should_compile/T13208 Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/394 -------------------------------------+------------------------------------- Comment (by monoidal): I agree that [comment:2 comment:2] was addressed, but what about [comment:1 comment:1]? It seems to me that this was not addressed and was the original reason for leaving the ticket open. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13208#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13208: Do two-phase inlining in simpleOptPgm
-------------------------------------+-------------------------------------
Reporter: lukemaurer | Owner: (none)
Type: bug | Status: closed
Priority: normal | Milestone:
Component: Compiler | Version: 8.1
Resolution: fixed | Keywords: JoinPoints
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
| deSugar/should_compile/T13208
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s):
Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/394
-------------------------------------+-------------------------------------
Comment (by Marge Bot

I agree that comment:2 was addressed, but what about comment:1? It seems to me that this was not addressed and was the original reason for leaving
#13208: Do two-phase inlining in simpleOptPgm -------------------------------------+------------------------------------- Reporter: lukemaurer | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: JoinPoints Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | deSugar/should_compile/T13208 Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/394 -------------------------------------+------------------------------------- Comment (by simonpj): the ticket open. Yes, you are right. * I'd like to get rid of `Note [Beta redexes]` in Core Lint. * The change to `simpleOptPgm` makes that more feasible. * But we do worker/wrapper join points, and currently w/w generates exactly these kind of beta redexes (contrary, I think, to the claim that it was only `simpleOptPgm`) I'm not sure it matters all that much, but yes we should leave the ticket open -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13208#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#13208: Do two-phase inlining in simpleOptPgm -------------------------------------+------------------------------------- Reporter: lukemaurer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.1 Resolution: | Keywords: JoinPoints Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | deSugar/should_compile/T13208 Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/394 -------------------------------------+------------------------------------- Changes (by monoidal): * status: closed => new * resolution: fixed => -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13208#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC