#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.3 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: | -------------------------------------+------------------------------------- Changes (by alexbiehl): * Attachment "tywhitehead_rewrite.hs" added. Test program from Tywhitedheads bug report -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.3 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: | -------------------------------------+------------------------------------- Changes (by alexbiehl): * Attachment "11688_smaller_testcase.hs" added. Smaller testcase which failes to rewrite break and failes to unbox result of findIndexOrEnd -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

I suspect the issue here is that `(==)` is being inlined too early, hiding the rule fire opportunity. By contrast, if you give `(==)` another name with a later inline pragma the `break -> breakByte` rule fires as expected.

{{{#!hs main = do chunk <- DB.hGetSome stdin 32768 let newline = c2w '\n' let (prefix,suffix) = DB.break (eq newline) chunk DB.hPut stdout prefix DB.hPut stdout suffix

eq :: Eq a => a -> a -> Bool eq = (==) {-# INLINE [1] eq #-}

{-# RULES "ByteString specialise break (eq x)" forall x. DB.break (eq x) = DB.breakByte x #-} }}}

Actually, even if one doesn't specify an explicit phase on `eq` the rule still fires! This is likely accidental, since GHC just happens to look at

#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.3 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): I briefly looked at this; I left a comment on the `bytestring` ticket which I'll reproduce here, the `breakByte` rule before the `eq` inlining. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1980 Wiki Page: | -------------------------------------+------------------------------------- Comment (by bgamari): Oh dear, Phab:D1980 actually won't work at all; the problem isn't the inlining of the class methods themselves, it is the class op rule generated by GHC. Namely, {{{ Rule fired Rule: Class op == Before: GHC.Classes.== TyArg GHC.Word.Word8 ValArg GHC.Word.$fEqWord8 ValArg ds_d2p3 ValArg ds_d2p4 After: GHC.Word.$fEqWord8_$c== Cont: Stop[BoringCtxt] GHC.Types.Bool }}} This will require further pondering. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1980 Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): This comes up regularly; e.g. #10595, #7141 (maybe), #5973 (maybe), #4397 (I bet the fix is delicate), #2271, #1434. And #10418 is similar but for constructors not class ops. In general, I it's dangerous to match on an overloaded class op. After all, did you really mean to match on `(==)` at every type? I doubt it? In this case we mean `==` at type `Char`. So the advice in comment:4 of #10595, is to write {{{ instance Eq Char where (==) = eqChar eqChar :: Char -> Char -> Bool {-# INLINE [0] eqChar #-} -- Delay inlining }}} and then make the rule be {{{ {-# RULES "bsbreak" forall x chunk. ByteString.break (eqChar x) chunk = ... #-} }}} This is simple and robust. But it does mean chaging the base library to use named functions for instance methods. Probably good practice anyway. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1980 Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Yes, we should! Also can you make the base-lib changes; and check that with the right RULE the original bug in `bytestring` is fixed? And then tell the `bytestring` devs. Incidentally, doesn't the rule elicit a warning? It should! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1980 Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): See also #10528, esp comment:13, which explains why rewriting the LHS of a rule is not so cool. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: upstream Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1980 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: patch => upstream * milestone: => 8.0.1 Comment: Merged to `ghc-8.0` as ed3398d176fc777dff8f5ae5315425e40c705c2d. I have a `bytestring` branch updating the rewrite rules which I now need to upstream. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: upstream Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1980 Wiki Page: | -------------------------------------+------------------------------------- Comment (by Ben Gamari ): In [changeset:"26f86f3d397159f9c0db0b59766138f553ba5a86/ghc" 26f86f3/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="26f86f3d397159f9c0db0b59766138f553ba5a86" base: Fix GHC.Word and GHC.Int on 32-bit platforms Due to a cut-and-paste error D1980 (#11688) broke 32-bit platforms. This should fix it. See #11750. }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11688: Bytestring break failing rewrite to breakByte and failing to eliminate boxing/unboxing -------------------------------------+------------------------------------- Reporter: alexbiehl | Owner: Type: bug | Status: closed Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.3 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Runtime | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1980 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: upstream => closed * resolution: => fixed Comment: Merged to `ghc-8.0` as well. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11688#comment:17 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler