[GHC] #13589: Possible inconsistency in CSE's treatment of NOINLINE
#13589: Possible inconsistency in CSE's treatment of NOINLINE -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.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: -------------------------------------+------------------------------------- While debugging #13535 I noticed the following inconsistency in CSE: `Note [CSE for INLINE and NOINLINE]` states that we need to take care when adding expressions bound to binders with inline pragmas to the `CSEnv`. To see why, consider the following, {{{#!hs {-# NOINLINE bar #-} bar = <rhs> -- Same rhs as foo foo = <rhs> }}} Given this program, we need to avoid producing `foo = bar` since doing so would mean that we would lose the ability to inline `foo`'s original RHS. The note then goes on to give the following rule,
We should not add
{{{<rhs> :-> bar}}}
to the CSEnv if `bar` has any constraints on when it can inline; that is, if its 'activation' not always active. Otherwise we might replace `<rhs>` by `bar`, and then later be unable to see that it really was `<rhs>`. This rule is implemented in `noCSE` with, {{{#!hs not (isAlwaysActive (idInlineActivation id)) }}}
However, it's quite unclear to me that this rule avoids the issue we set out to solve. Afterall, `NOINLINE bar` is always active, but it still means that rewriting `foo` to `foo=bar` would lose us the ability to see `foo`'s original RHS. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13589 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13589: Possible inconsistency in CSE's treatment of NOINLINE -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 Resolution: | Keywords: 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 simonpj): I think the rule does address the issue. If CSE fires we'll get {{{ bar = rhs foo = bar }}} Before CSE, if `foo` is inlined, then the context into which it inlines will see `<rhs>`. After CSE, if we inline `foo` we'll see `bar` not `<rhs>` so we want `bar` to be able to inline too.
Afterall, NOINLINE bar is always active,
No: a NOINLINE pragma cause a `NeverActive` activation. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13589#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13589: Possible inconsistency in CSE's treatment of NOINLINE -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 Resolution: | Keywords: 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 simonpj): That said, it is silly that we can't CSE {{{ {-# INLINE [0] f #-} f x = blah {-# INLINE [0] g #-} g x = blah }}} How could we solve this? Something like this: * Have `cs_map :: CoreMap (OutExpr, Activation)` * Add `blah :-> (f, ActiveAfter 0)` when extending the `cs_map` in `cse_bind`. * In `tryForCSE`, when called from `cse_bind`, pass the `Activation` of the binding (`g` in this example), and check that the `Activation` of the new Id is the same. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13589#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13589: Possible inconsistency in CSE's treatment of NOINLINE -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 Resolution: | Keywords: 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 bgamari):
No: a NOINLINE pragma cause a NeverActive activation.
Ahh, there's my misunderstanding.
How could we solve this? Something like this:
Sounds reasonable to me. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13589#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13589: Possible inconsistency in CSE's treatment of NOINLINE -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 Resolution: | Keywords: CSE 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: => CSE -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13589#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC