[GHC] #16254: INLINABLE pragma and newtype wrappers prevent inlining
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: #5327 Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- In #5327, we allowed case-of-known constructor to see through bindings that were marked as INLINABLE and used newtypes. However, this works only if the newtype does not have a wrapper. The following code, which is the same as T5237 except for the extra type parameters in Size, does not cause the optimization to fire. {{{ {-# LANGUAGE GADTs, ExplicitForAll #-} module T16254 where newtype Size a b where Size :: forall b a. Int -> Size a b {-# INLINABLE val2 #-} val2 = Size 17 f n = case val2 of Size s -> s + s > n }}} The reason is that `exprIsConApp_maybe` sees `$WSize (I# 17#)`, which is an application with a nontrivial argument. In general, this could cause duplication of work, but in this case we're dealing with a newtype wrapper, so this is safe. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by monoidal): * owner: (none) => monoidal -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:1> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/247 -------------------------------------+------------------------------------- Changes (by monoidal): * differential: => https://gitlab.haskell.org/ghc/ghc/merge_requests/247 -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:2> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/247 -------------------------------------+------------------------------------- Changes (by monoidal): * status: new => patch -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:3> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/247 -------------------------------------+------------------------------------- Comment (by simonpj):
The reason is that exprIsConApp_maybe sees $WSize (I# 17#)
This is true only if data constructor wrappers are inlined late, isn't it? Or is it some true even of (say) GHC 8.6? -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:4> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/247 -------------------------------------+------------------------------------- Comment (by monoidal): The is true in GHC 8.6 or master - it doesn't require the inlining late patch. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:5> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/247 -------------------------------------+------------------------------------- Comment (by simonpj): Hmm. I wonder why? I'd expect Size's wrapper to be inlined even inside the INLINEABLE unfolding. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:6> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/247 -------------------------------------+------------------------------------- Changes (by aspiwack): * cc: aspiwack (added) -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:7> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/325/ -------------------------------------+------------------------------------- Changes (by monoidal): * status: patch => closed * differential: https://gitlab.haskell.org/ghc/ghc/merge_requests/247 => https://gitlab.haskell.org/ghc/ghc/merge_requests/325/ * resolution: => fixed -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:8> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/325/ -------------------------------------+------------------------------------- Comment (by simonpj): What about a regression test for this ticket? -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:9> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | simplCore/should_compile/T16254 Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/325/ -------------------------------------+------------------------------------- Changes (by monoidal): * testcase: => simplCore/should_compile/T16254 Comment: It was pushed, I've set it here now. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:10> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | simplCore/should_compile/T16254 Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/325/ -------------------------------------+------------------------------------- Comment (by Marge Bot <ben+marge-bot@…>): In [changeset:"7833cf407d1f608bebb1d38bb99d3035d8d735e6/ghc" 7833cf40/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="7833cf407d1f608bebb1d38bb99d3035d8d735e6" Look through newtype wrappers (Trac #16254) exprIsConApp_maybe could detect that I# 10 is a constructor application, but not that Size (I# 10) is, because it was an application with a nontrivial argument. }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:11> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#16254: INLINABLE pragma and newtype wrappers prevent inlining -------------------------------------+------------------------------------- Reporter: monoidal | Owner: monoidal Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | simplCore/should_compile/T16254 Blocked By: | Blocking: Related Tickets: #5327 | Differential Rev(s): Wiki Page: | https://gitlab.haskell.org/ghc/ghc/merge_requests/325/ -------------------------------------+------------------------------------- Comment (by Matthew Pickering <matthewtpickering@…>): In [changeset:"c25b135ff5b9c69a90df0ccf51b04952c2dc6ee1/ghc" c25b135f/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="c25b135ff5b9c69a90df0ccf51b04952c2dc6ee1" Fix exprIsConApp_maybe In this commit commit 7833cf407d1f608bebb1d38bb99d3035d8d735e6 Date: Thu Jan 24 17:58:50 2019 +0100 Look through newtype wrappers (Trac #16254) we made exprIsConApp_maybe quite a bit cleverer. But I had not paid enough attention to keeping exactly the correct substitution and in-scope set, which led to Trac #16348. There were several buglets (like applying the substitution twice in exprIsConApp_maybe, but the proximate source of the bug was that we were calling addNewInScopeIds, which deleted things from the substitution as well as adding them to the in-scope set. That's usually right, but not here! This was quite tricky to track down. But it is nicer now. }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/16254#comment:12> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
participants (1)
-
GHC