[GHC] #14140: Better treatment for dataToTag
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.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: -------------------------------------+------------------------------------- In `libraries/Cabal/Cabal/Distribution/System.hs` I found {{{ eqOS :: OS -> OS -> Bool eqOS (OtherOS s1) (OtherOS s2) = s1 == (s2 :: String) eqOs a b = dataToTag a == dataTag b }}} (actually it's not called `eqOS`; it's the derived equality for `OS`). By the time this gets to Core it looks something like this {{{ eqOS a b = case a of OtherOS s1 -> case b of OtherOS s2 -> eqString s1 s2 _ -> case dataToTag b of 16# -> True r -> False _ -> dataToTag a == dataToTag b }}} The `dataToTag` code has been duplicated; and in the `OtherOS s1` branch GHC can constant-fold the `dataToTag` on `a` to `16#`, the tag of `OtherOS`. But GHC is no so clever for the `dataToTag b`. We know that `b` is not `OtherOS`, so we know its tag is not `16#`, so the innermost case is entirely redundant. But we don't spot that. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags 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 bgamari): * keywords: => datacon-tags -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags 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 dfeuer): For derived instances, might it make sense to write this? {{{#!hs eqOS :: OS -> OS -> Bool eqOS a b | dataToTag a /= dataToTag b = False eqOS (OtherOS s1) (OtherOS s2) = s1 == (s2 :: String) eqOS _ _ = False }}} Sometimes this will be better; sometimes it will be worse. But I don't think it's likely to do anything silly. Of course, even if we do that, we probably want to try to make the compiler cleverer too. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags 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 dfeuer): Simon, could you point me to the code responsible for the constant folding here? I'd like to see if I can guess how to make it do what you want. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags 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 dfeuer): * cc: dfeuer (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags 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 hsyl20): https://git.haskell.org/ghc.git/blob/HEAD:/compiler/prelude/PrelRules.hs#l90... ("b" case) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags 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): hysl20 is right. Doing stuff in `PrelRules` works if the rewrite we want is something like `dataToTag True ---> 1`. But for this ticket we don't want that. What we know is negative information: `dataToTag x` cannot be `16#`. This is carried by the `OtherCon` uunfolding for `x`. So the place to implement this ticket (if that is behind your question) is `SimplUtils.prepareAlts`. We want to say "if the scrutinee is `dataToTag x` and `x` has an unfolding of `OtherCon [A,B,C]`, then eliminate any alternatives that match those constructors". Something like this in `prepareAlts` {{{ imposs_cons = case scrut of Var v -> otherCons (idUnfolding v) -- as now App dataToTag (Var v) -> map (LitCon . conToTag) (otherCons (idUnfolding v)) _ -> [] }}} That is, * If the scrutinee is `dataToTag v` * And v cannot be `[A,C,F]` * Then we know that `dataToTag v` cannot be `[1#, 3#, 6#]` * And so we can make a suitable `imposs_cons` Does that help? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags 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 dfeuer): Yes, that's exactly what I was looking for. I'm trying it now. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3932 Wiki Page: | -------------------------------------+------------------------------------- Changes (by dfeuer): * status: new => patch * differential: => Phab:D3932 Comment: I think I probably did that right... -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3932 Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): Argh! I should have checked this against HEAD. It seems this was already fixed, in 193664d42dbceadaa1e4689dfa17ff1cf5a405a0. We now rewrite {{{#!hs case dataToTag b of 16# -> True _ -> False }}} to {{{#!hs case b of OtherOS _ -> True _ -> False }}} which has always been good. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3932 Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Huh, you are right. And that is a better place to do it. So, why isn't it working? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3934 Wiki Page: | -------------------------------------+------------------------------------- Changes (by dfeuer): * differential: Phab:D3932 => Phab:D3934 Comment: I think it ''is'' working, unless it broke very very recently. I've just changed the linked differential to a test case. When I compiled various versions of your code under GHC HEAD (September 1 edition or so), I got exactly what we want. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: datacon-tags Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3934 Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj):
I think it is working, unless it broke very very recently
You are right. How mysterious. I can't account for why I ever saw the bad code. But indeed it seems fixed. I recompiled `libraries/Cabal/Cabal/Distribution/System.hs` and it looks fine. Close when you commit the test case. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14140: Better treatment for dataToTag
-------------------------------------+-------------------------------------
Reporter: simonpj | Owner: (none)
Type: bug | Status: patch
Priority: normal | Milestone:
Component: Compiler | Version: 8.2.1
Resolution: | Keywords: datacon-tags
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D3934
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by David Feuer
#14140: Better treatment for dataToTag -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.2.1 Resolution: fixed | 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): Phab:D3934 Wiki Page: | -------------------------------------+------------------------------------- Changes (by dfeuer): * status: patch => closed * failure: None/Unknown => Runtime performance bug * resolution: => fixed * milestone: => 8.4.1 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14140#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC