[GHC] #14295: tagToEnum# leads to some silly closures
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Keywords: datacon-tags | Operating System: Unknown/Multiple Architecture: | Type of failure: Runtime Unknown/Multiple | performance bug Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- I don't know how important this is in practice, but it smells pretty silly. Suppose I write {{{#!hs foo :: (Bool -> a) -> Int# -> a foo f x = f (tagToEnum# x) }}} Since `tagToEnum#` can fail, GHC compiles this to {{{#!hs foo = \ (@ a_a10v) (f_s1by [Occ=Once!] :: GHC.Types.Bool -> a_a10v) (x_s1bz [Occ=Once] :: GHC.Prim.Int#) -> let { sat_s1bA [Occ=Once] :: GHC.Types.Bool [LclId] sat_s1bA = GHC.Prim.tagToEnum# @ GHC.Types.Bool x_s1bz } in f_s1by sat_s1bA }}} That seems pretty silly! We know that `tagToEnum#` is applied to `Bool`, so we can transform this to something like {{{#!hs foo f x = case x <=# 1# of 1# -> f $! tagToEnum# x _ -> f (error "tagToEnum# was used at Bool with tag ...") }}} which avoids an extra closure at the cost of a single `Int#` comparison. The same goes for arbitrary known enumeration types. I suspect the right place to fix this up is in CorePrep. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Description changed by dfeuer: Old description:
I don't know how important this is in practice, but it smells pretty silly.
Suppose I write
{{{#!hs foo :: (Bool -> a) -> Int# -> a foo f x = f (tagToEnum# x) }}}
Since `tagToEnum#` can fail, GHC compiles this to
{{{#!hs foo = \ (@ a_a10v) (f_s1by [Occ=Once!] :: GHC.Types.Bool -> a_a10v) (x_s1bz [Occ=Once] :: GHC.Prim.Int#) -> let { sat_s1bA [Occ=Once] :: GHC.Types.Bool [LclId] sat_s1bA = GHC.Prim.tagToEnum# @ GHC.Types.Bool x_s1bz } in f_s1by sat_s1bA }}}
That seems pretty silly! We know that `tagToEnum#` is applied to `Bool`, so we can transform this to something like
{{{#!hs foo f x = case x <=# 1# of 1# -> f $! tagToEnum# x _ -> f (error "tagToEnum# was used at Bool with tag ...") }}}
which avoids an extra closure at the cost of a single `Int#` comparison. The same goes for arbitrary known enumeration types. I suspect the right place to fix this up is in CorePrep.
New description: I don't know how important this is in practice, but it looks unfortunate. Suppose I write {{{#!hs foo :: (Bool -> a) -> Int# -> a foo f x = f (tagToEnum# x) }}} Since `tagToEnum#` can fail, GHC compiles this to {{{#!hs foo = \ (@ a_a10v) (f_s1by [Occ=Once!] :: GHC.Types.Bool -> a_a10v) (x_s1bz [Occ=Once] :: GHC.Prim.Int#) -> let { sat_s1bA [Occ=Once] :: GHC.Types.Bool [LclId] sat_s1bA = GHC.Prim.tagToEnum# @ GHC.Types.Bool x_s1bz } in f_s1by sat_s1bA }}} That seems pretty bad! We know that `tagToEnum#` is applied to `Bool`, so we can transform this to something like {{{#!hs foo f x = case x <=# 1# of 1# -> f $! tagToEnum# x _ -> f (error "tagToEnum# was used at Bool with tag ...") }}} which avoids an extra closure at the cost of a single `Int#` comparison. The same goes for arbitrary known enumeration types. I suspect the right place to fix this up is in CorePrep. -- -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Description changed by dfeuer: Old description:
I don't know how important this is in practice, but it looks unfortunate.
Suppose I write
{{{#!hs foo :: (Bool -> a) -> Int# -> a foo f x = f (tagToEnum# x) }}}
Since `tagToEnum#` can fail, GHC compiles this to
{{{#!hs foo = \ (@ a_a10v) (f_s1by [Occ=Once!] :: GHC.Types.Bool -> a_a10v) (x_s1bz [Occ=Once] :: GHC.Prim.Int#) -> let { sat_s1bA [Occ=Once] :: GHC.Types.Bool [LclId] sat_s1bA = GHC.Prim.tagToEnum# @ GHC.Types.Bool x_s1bz } in f_s1by sat_s1bA }}}
That seems pretty bad! We know that `tagToEnum#` is applied to `Bool`, so we can transform this to something like
{{{#!hs foo f x = case x <=# 1# of 1# -> f $! tagToEnum# x _ -> f (error "tagToEnum# was used at Bool with tag ...") }}}
which avoids an extra closure at the cost of a single `Int#` comparison. The same goes for arbitrary known enumeration types. I suspect the right place to fix this up is in CorePrep.
New description: I don't know how important this is in practice, but it looks unfortunate. Suppose I write {{{#!hs foo :: (Bool -> a) -> Int# -> a foo f x = f (tagToEnum# x) }}} Since `tagToEnum#` can fail, GHC compiles this to {{{#!hs foo = \ (@ a_a10v) (f_s1by [Occ=Once!] :: GHC.Types.Bool -> a_a10v) (x_s1bz [Occ=Once] :: GHC.Prim.Int#) -> let { sat_s1bA [Occ=Once] :: GHC.Types.Bool [LclId] sat_s1bA = GHC.Prim.tagToEnum# @ GHC.Types.Bool x_s1bz } in f_s1by sat_s1bA }}} That seems pretty bad! We know that `tagToEnum#` is applied to `Bool`, so we can transform this to something like {{{#!hs foo f x = case leWord# (intToWord# x) 1## of 1# -> f $! tagToEnum# x _ -> f (error "tagToEnum# was used at Bool with tag ...") }}} which avoids an extra closure at the cost of a single `Word#` comparison. The same goes for arbitrary known enumeration types. I suspect the right place to fix this up is in CorePrep. -- -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Specifically, any time CorePrep is tempted to produce {{{#!hs let x = tagToEnum# @KnownType y in e }}} it should pause a moment and instead produce {{{#!hs case leWord# (int2Word# y) limit of DEFAULT -> error "blah" 1# -> case tagToEnum# @KnownType y of x DEFAULT -> e }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Unsurprisingly, exactly the same thing is true for `quotInt#` and such. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): I think we can do better. Add a `BuiltInRule` for `tagToEnum#`. {{{ tagToEnum# Bool ===> \x -> case x of 0# -> False 1# -> True DEFAULT -> error "blah" }}} The rule can fire whenever `tagToEnum#` is applied to a known type, albeit perhaps one without too many constructors. It's a bit like inlining `tagToEnum#`. }}} The built-in rule mechanism works well; we can use it here too. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj):
Unsurprisingly, exactly the same thing is true for quotInt# and such.
I don't understand. Can you explain? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Replying to [comment:6 simonpj]:
Unsurprisingly, exactly the same thing is true for quotInt# and such.
I don't understand. Can you explain?
{{{#!hs foo :: (Int -> a) -> Int# -> Int# -> a foo f x y = f (I# (quotInt# x y)) }}} will suspend the division for fear of a zero denominator. We can fix that similarly by testing for 0 and forcing the division in the good branch. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Replying to [comment:5 simonpj]:
I think we can do better. Add a `BuiltInRule` for `tagToEnum#`. {{{ tagToEnum# Bool ===> \x -> case x of 0# -> False 1# -> True DEFAULT -> error "blah" }}} The rule can fire whenever `tagToEnum#` is applied to a known type, albeit perhaps one without too many constructors. It's a bit like inlining `tagToEnum#`. }}} The built-in rule mechanism works well; we can use it here too.
Yes, I thought about that too. But it's probably really only a good idea if there aren't many constructors; otherwise we're inlining too much. When we don't inline, we should still perform a bounds check to avoid the thunk, no? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Really? I get {{{ ==================== STG syntax: ==================== Foo.foo :: forall a. (GHC.Types.Int -> a) -> GHC.Prim.Int# -> GHC.Prim.Int# -> a [GblId, Arity=3, Caf=NoCafRefs, Unf=OtherCon []] = \r [f_s1av x_s1aw y_s1ax] let { sat_s1az [Occ=Once] :: GHC.Types.Int [LclId] = \u [] case quotInt# [x_s1aw y_s1ax] of wild_s1ay { __DEFAULT -> GHC.Types.I# [wild_s1ay]; }; } in f_s1av sat_s1az; }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Am I reading that wrong? I thought the outer `let` was suspending that whole thing. Compare it to the modified version. I'm going to sleep shortly, do I can't post the comparison right now. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
When we don't inline, we should still perform a bounds check to avoid
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): the thunk, no? Good point. I suppose so... but * If we did this we'd better give a different name to the primop that can't fail; otherwise the transformation is plain confusing. * I'm uncomfortable with transformations that are not readily expressed in Core. E.g. we could have {{{ case x of 0# -> blah DEFAULT -> ...(noFailQuotInt# y x)... }}} since we know that `x` is non-zero in the default branch. But then we might float the subexpression out, and the claim would no longer hold. We don't have a solid way to prevent this. So yes one could do it as a `CorePrep` tactic, but it's delicate; and we risk not getting the benefits because we don't implement the follow-on transformations. * I doubt it's worth doing in practice. i.e. the same engineering effort spent elsewhere might yield more benefit. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
So yes one could do it as a `CorePrep` tactic, but it's delicate; and we risk not getting the benefits because we don't implement the follow-on
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Replying to [comment:11 simonpj]: transformations. What makes it delicate? For `quotInt#` et al, I don't imagine that there ''are'' many interesting follow-on transformations in most cases, although I could be wrong. For `tagToEnum#`, we should decide whether to "inline" in core2core. If we go to the trouble of a `quotInt#` cleanup in `CorePrep`, then we can just tack the `tagToEnum#` cleanup on in the same way. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj):
What makes it delicate?
Just that `CorePrep` is trying to establish some tricky output invariants (about ANF) etc, and is ''not'' a simplification engine. If there are zero knock-on transformations then that's fine, but that's a delicate situation all by itself. Anyway, I'm not opposed, just doubtful about the cost/benefit tradeoff. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14295: tagToEnum# leads to some silly closures -------------------------------------+------------------------------------- Reporter: dfeuer | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: ⊥ Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.6.1 => ⊥ -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14295#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC