Opened 6 years ago

Closed 6 years ago

#7995 closed bug (fixed)

module Pretty's "text/str" rule doesn't fire

Reported by: parcs Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.7
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: simplCore/should_compile/T7995
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

In this trivial but characteristic example

import Pretty
import Data.IORef

main :: IO ()
main = do
    _ <- newIORef (text "test")
    return ()

the "text/str" rule, which optimizes calls to Pretty.text applied to a string literal argument, doesn't fire. Likewise, such calls to Outputable.text don't get rewritten either.

Change History (4)

comment:1 Changed 6 years ago by parcs

I suspect Pretty.text "blah" doesn't get rewritten because Pretty.text gets inlined before the rule has a chance to fire. Pretty.text is currently marked as NOINLINE [1]; if you change this to NOINLINE [0], then the rule does fire.

And it looks like Outputable.text "blah" doesn't get rewritten because Pretty.text gets inlined into the unfolding of Outputable.text.

comment:2 Changed 6 years ago by parcs

I can think of two solutions:

1) Mark Pretty.text as NOINLINE to ensure that the "text/str" rule has a chance to fire and that Pretty.text doesn't get inlined into the unfolding of Outputable.text. This solution has the caveat that Pretty.text can no longer get inlined.

2) Add a corresponding "text/str" rule on Outputable.text and mark both Pretty.text and Outputable.text as NOINLINE [0] (to ensure that the rules get a chance to fire).

comment:3 Changed 6 years ago by simonpj@…

commit d2c3630dedc577f7e6eb8e945b05a86992bd5e0a

Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Fri Jun 21 17:41:26 2013 +0100

    Make sure that Pretty.text is inlined in stage 0,
    so that RULE text/str gets a chance to fire (Trac #7995).
    
    And make sure that Outputable.text is inlined, so that the underlying
    Pretty.text rule can fire.
    
    The thing is that literal strings only turn into unpackCString#
    in phase 1.

 compiler/utils/Outputable.lhs |    3 +++
 compiler/utils/Pretty.lhs     |    4 +++-
 2 files changed, 6 insertions(+), 1 deletions(-)

comment:4 Changed 6 years ago by simonpj

difficulty: Unknown
Resolution: fixed
Status: newclosed
Test Case: simplCore/should_compile/T7995

OK, I've fixed this. I'm not sure it's really worth adding a test but I have done so anyway.

Good catch... knowing that this works should let us replace

  ptext (sLit "foo")

with the much tidier

  text "foo"

without increasing code size or execution time.

Note: See TracTickets for help on using tickets.