[GHC] #14626: No need to enter a scrutinised value

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
45 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
           Reporter:  heisenbug      |             Owner:  (none)
               Type:  bug            |            Status:  new
           Priority:  normal         |         Milestone:
          Component:  Compiler       |           Version:  8.2.2
           Keywords:  performance    |  Operating System:  Unknown/Multiple
       Architecture:                 |   Type of failure:  None/Unknown
  Unknown/Multiple                   |
          Test Case:                 |        Blocked By:
           Blocking:                 |   Related Tickets:  #13861
Differential Rev(s):                 |         Wiki Page:
-------------------------------------+-------------------------------------
 While analysing the output of #13861 I stumbled over an unnecessary
 pessimisation in handling of scrutinised values. With words of Simon (from
 https://phabricator.haskell.org/D4267 with minor edits added):

 Interesting. Yes, please make a ticket! (And transfer the info below into
 it.)

 I think the issue is this. Given (the STG-ish code)
 {{{#!hs
 data Colour = Red | Green | Blue
 f x = case x of y
            Red -> Green
            DEFAULT -> y
 }}}
 (here `y` is the case binder) we can just return `x` rather than entering
 it in DEFAULT branch, because `y` will be fully evaluated and its pointer
 will be correctly tagged.


 You absolutely can't check for an `OccName` of `"wild"`!! That is neither
 necessary nor sufficient :-).

 Instead, check `isEvaldUnfolding (idUnfolding y)`. See `Note [Preserve
 evaluatedness]` in `CoreTidy.hs`. And be sure to augment that note if you
 make the change.

 I would expect perf benefits to be small on average, but it's simple to
 implement.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by heisenbug):

 * owner:  (none) => heisenbug


Comment:

 @spj Yes the perf changes will be small, but I hope to get more by the
 time #13861 is ready. Anyway this will result in better (less branchy)
 code, so I think it is worth it.

 Thanks for the `isEvaldUnfolding` hint! I ''knew'' there must be a correct
 way to do it. I'll test, and come back with a phabricator.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:1>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by heisenbug):

 * cc: simonpj (added)


Comment:

 @simonpj: unfortunately the `isEvaldUnfolding (idUnfolding y)` criterion
 does not hold for case scrutinees. Any idea why? I `partake`-d them and
 while some traces show up, none is `wild_*`. What s going on here?

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:2>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by heisenbug):

 Hmm, I made a run with traces about which `Id`s the condition holds for:
 https://circleci.com/api/v1.1/project/github/ghc/ghc/951/output/108/0?file=true

 you have to `grep getCallMethod` to find them. If I treat those values as
 tagged, and simply return them, the `ghc-stage2` crashes. :-(

 So something is definitely fishy here. For completeness, this is what I
 changed: https://github.com/ghc/ghc/compare/92f6a671a6a8...8f627c9f25b6

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:3>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 Puzzled by comment:2 I tried it myself.  Sure enough, the evaluted-ness
 flags, so carefully attached by `CoreTidy` were being lost in `CorePrep`.

 This turned out to date back at least 7 years.

 * See `Note [dataToTag magic]` in `CorePrep`.  It relies evaluated-ness
 flags.

 * But those flags were being killed off in `cpCloneBndr`, for reasons
 described in `Note [Dead code in CorePrep]`.

 * This was just a bug: it means that the `dataToTag magic` doesn't work
 properly.  Sure enough we get a redundant case (after `CorePrep`) with
 this program
 {{{
 data T = MkT !Bool

 f v = case v of
          MkT y -> dataToTag# y
 }}}
   After `CorePrep` we end up with
 {{{
 f v = case v of
          MkT y -> case y of z -> dataToTag# z
 }}}
   which is silly.

 I'll fix all this and add suitable comments

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:4>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 > So something is definitely fishy here.

 I think that there's something different about '''top-level''' binders.
 Consider
 {{{
 data T = MkT !Bool
 top_x = True

 f True  (MkT local_y) = local_y
 f False _             = top_x
 }}}
 Here I think that `local_y` gets bound to a correctly-tagged pointer,
 fetched out of the `MkT` constructor.

 But, in contrast, I think that `top_x` is bound to the label for the top-
 level closure for `top_x`, which is 8-byte aligned.  So the label isn't
 tagged; instead, the code generator has to tag it.  Is that happening in
 your "return it instead of eantering it" path?

 Other than that I have no idea why it crashes.  Things you might try:

 * Switch off the new optimisation when building stage2 and the libraries.
 Use it only for the test suite: these are small programs and easier to
 debug.

 * Maybe add an assertion in the Cmm: the claim is that on the "return it"
 path, the thing beign returned is a correctly-tagged pointer.  So, the
 assertion can follow the pointer and check that the thing pointed to is a
 value, with the right tag, etc.  There must be some code in the RTS (or
 somewhere) that checks that.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:5>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by heisenbug):

 Replying to [comment:5 simonpj]:
 > > So something is definitely fishy here.
 >
 > I think that there's something different about '''top-level''' binders.
 Consider
 > {{{#!hs
 > data T = MkT !Bool
 > top_x = True
 >
 > f True  (MkT local_y) = local_y
 > f False _             = top_x
 > }}}
 > Here I think that `local_y` gets bound to a correctly-tagged pointer,
 fetched out of the `MkT` constructor.
 >
 > But, in contrast, I think that `top_x` is bound to the label for the
 top-level closure for `top_x`, which is 8-byte aligned.  So the label
 isn't tagged; instead, the code generator has to tag it.  Is that
 happening in your "return it instead of eantering it" path?


 No, I am not returning top-level bindings, I would be okay with entering
 those. What I want is to avoid entering local bindings from the `StgCase`:
 {{{#!hs
 f = \inp -> case inp of wildBind { True -> ...;  False -> wildBind }
 }}}
 `wildBind` is known to be evaled and properly tagged. I want  to
 `ReturnIt`.

 >
 > Other than that I have no idea why it crashes.  Things you might try:
 >
 > * Switch off the new optimisation when building stage2 and the
 libraries.  Use it only for the test suite: these are small programs and
 easier to debug.
 >
 > * Maybe add an assertion in the Cmm: the claim is that on the "return
 it" path, the thing beign returned is a correctly-tagged pointer.  So, the
 assertion can follow the pointer and check that the thing pointed to is a
 value, with the right tag, etc.  There must be some code in the RTS (or
 somewhere) that checks that.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:6>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by Simon Peyton Jones <simonpj@…>):

 In [changeset:"bd438b2d67ec8f5d8ac8472f13b3175b569951b9/ghc" bd438b2/ghc]:
 {{{
 #!CommitTicketReference repository="ghc"
 revision="bd438b2d67ec8f5d8ac8472f13b3175b569951b9"
 Get evaluated-ness right in the back end

 See Trac #14626, comment:4.  We want to maintain evaluted-ness
 info on Ids into the code generateor for two reasons
 (see Note [Preserve evaluated-ness in CorePrep] in CorePrep)

 - DataToTag magic
 - Potentially using it in the codegen (this is Gabor's
   current work)

 But it was all being done very inconsistently, and actually
 outright wrong -- the DataToTag magic hasn't been working for
 years.

 This patch tidies it all up, with Notes to match.
 }}}

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:7>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 OK I've fixed the stuff in comment:4

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:8>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by heisenbug):

 @simonpj, thanks this is a good start. When restricting myself to `wild*`
 vars, I already get a nice performance diff on dynamic instruction counts:
 https://perf.haskell.org/ghc/#compare/7a25659efc4d22086a9e75dc90e3701c1706c625/9ad6982e4074c1fdeff967cafa51e436023d69bb

 Then I tried to add `ds*` variables too, and (some of) those made the
 stage-2 crash. It is checked in on the branch if you want to have a try...
 https://github.com/ghc/ghc/commit/b30d61f64772d744e06e2acbab21895cb20d9bf7

 Any hints appreciated!

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:9>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 The names of the variables should not make any difference!  That's
 bizarre.

 Guessing is usually fruitless; you need data.

 Did you add that assertionn I suggested?

 As I say, a stage2 compiler is a huge program.  I urge you to first build
 the libraries and compiler without the change; then switch the change on
 and run the testsuite. Any bugs must be in those little programs.  Then
 nofib.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:10>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by heisenbug):

 Replying to [comment:10 simonpj]:
 > The names of the variables should not make any difference!  That's
 bizarre.

 Of course it is not the names, ''per se''! Rather, one of the names
 starting with `ds` is misbehaving in a specific way, that trips over
 `stage2`.

 I built a `quick` compiler with the given revision, that is `stage2`
 compiled with `-O0`, and it behaves well. Testsuite passes without any
 strangeness.

 Then I bisected the sources in `compiler/*` and isolated these two
 (mutually dependent) sources, which when built with `-O1` (and all others
 with `-O0`) break stage 2. Here is what I did (for the record):


 {{{#!sh
 nice make V=1 GhcStage2HcOpts="-O0 -g" -j8
 rm compiler/stage2/build/StgCmmBind.*
 nice make V=1 GhcStage2HcOpts="-O1 -g" -j8
 ./compiler/stage2/build/StgCmmBind.hi
 nice make V=1 GhcStage2HcOpts="-O0 -g" -j8
 }}}

 I'll have a look which differences occur here between `-O0` and `-O1`.
 Both interesting sources are >500 LOC, so it won't be trivial :-)


 >
 > Guessing is usually fruitless; you need data.
 >
 > Did you add that assertionn I suggested?
 >
 > As I say, a stage2 compiler is a huge program.  I urge you to first
 build the libraries and compiler without the change; then switch the
 change on and run the testsuite. Any bugs must be in those little
 programs.  Then nofib.
 >

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:11>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 Once more, I urge you ''not'' to try debugging a stage2 compiler, unless
 you have a great deal of time on your hands. Instead, compile the
 libraries and stage2 compiler without the optimisation; and then try the
 testsuite and nofib. These are small programs and much easier to debug.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:12>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by heisenbug):

 Replying to [comment:12 simonpj]:
 > Once more, I urge you ''not'' to try debugging a stage2 compiler, unless
 you have a great deal of time on your hands. Instead, compile the
 libraries and stage2 compiler without the optimisation; and then try the
 testsuite and nofib. These are small programs and much easier to debug.

 Yup, I entertained this idea of an assertion for some time, but it looked
 like too much of a hassle compared to some quick debugging session. Turns
 out you are right. So I now plant in taggedness assertions (pushed to
 `wip/T14626` for your reviewing pleasure) for suspicious constellations,
 and they fire indeed!
 {{{
   * frame #0: 0x000000010a0fcbc8 libHSrts_thr-
 ghc8.5.20180103.dylib`checkTagged
     frame #1: 0x000000010073e2c1
 libHSghc-8.5-ghc8.5.20180103.dylib`ghc_Name_nzuocc_info [inlined] _cpOv +
 17 at Name.hs:111
     frame #2: 0x000000010073e2b0
 libHSghc-8.5-ghc8.5.20180103.dylib`ghc_Name_nzuocc_info + 72
     frame #3: 0x000000010a104fb0 libHSrts_thr-
 ghc8.5.20180103.dylib`stg_upd_frame_info_dsp + 16
 }}}

 `Name.hs:111` is a strict record field BTW. Does this ring a bell? Why is
 it `OtherCon _ <- idUnfolding id` but not tagged? Is it possibly
 implicitly unpacked?

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:13>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 > Name.hs:111 is a strict record field BTW. Does this ring a bell? Why is
 it OtherCon _ <- idUnfolding id but not tagged? Is it possibly implicitly
 unpacked?

 Can you explain more?  I can't make sense of this paragraph.  What is "it"
 that might be implicitly unpacked?  What does it mean to be "implicitly
 unpacked" ?

 One good thing would be to distill a tiny example, and it sounds as if you
 have enough insight to do that now.  E.g. perhaps you are saying that
 {{{
 data T = MkT ![Int]
 f (MkT xs) = xs
 }}}
 returns a badly-tagged pointer?  If so, just compile that tiny program and
 see.  etc.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:14>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by alexbiehl):

 Indeed GHC seems to unnecessarily enter `MkT`s argument!

 {{{
     ...
 c1ab: // global
    R1 = P64[R1 + 7] & (-8); -- load constructor arg
                             -- and untag(!) it
    Sp = Sp + 8;
    call (I64[R1])(R1) args: 8, res: 0, upd: 8; -- and enter it!
 }}}

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:15>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by heisenbug):

 Replying to [comment:15 alexbiehl]:
 > Indeed GHC seems to unnecessarily enter `MkT`s argument!
 >
 > {{{
 >     ...
 > c1ab: // global
 >    R1 = P64[R1 + 7] & (-8); -- load xs and untag(!) it
 >    Sp = Sp + 8;
 >    call (I64[R1])(R1) args: 8, res: 0, upd: 8; -- and enter it!
 > }}}

 Yes, my branch is supposed to fix (soon) many (if not all) of these cases.
 Simon fixed the lost tracking of ''evaled-ness'' in core-prep already, now
 I am building on that patch. Which GHC do you use to check? My branch
 currently won't bootstrap, so you can only do your experiments with
 `stage1` (or roll your own modifications).

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:16>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 Yes, the whole point heisenbug's patch is to return `xs` without entering
 it.  But we we are returning a badly-tagged pointer.  If in the above code
 we return R1, that will be bad.  Really we should un-tag it only if/when
 we enter it.  For example if we had
 {{{
 data MkS = MkS ![Int]
 f (MkT xs) = MkS xs
 }}}
 then when we build `MkS xs` we need a correctly tagged xs, not a de-tagged
 one.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:17>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by heisenbug):

 Replying to [comment:14 simonpj]:
 > > Name.hs:111 is a strict record field BTW. Does this ring a bell? Why
 is it OtherCon _ <- idUnfolding id but not tagged? Is it possibly
 implicitly unpacked?
 >
 > Can you explain more?  I can't make sense of this paragraph.  What is
 "it" that might be implicitly unpacked?  What does it mean to be
 "implicitly unpacked" ?

 No, this is a red herring. Just me, desperately looking for hints. The
 `n_occ` is not unpacked.

 But I have a theory now.

 I set a few breakpoints:


 {{{
 (gdb) info breakpoints
 Num     Type           Disp Enb Address            What
 8       breakpoint     keep n   0x00007ffff52e39e0 in ghc_Name_nzuocC_info
                                                    at
 compiler/basicTypes/Name.hs:111
         breakpoint already hit 1 time
 11      breakpoint     keep y   0x00007ffff52e3a18 in ghc_Name_nzuocC_info
                                                    at
 compiler/basicTypes/Name.hs:111
         breakpoint already hit 46 times
 12      breakpoint     keep y   0x00007ffff2332a58 <checkTagged>
         breakpoint already hit 1 time
 }}}

 ''bp11'' is set as `breakpoint *ghc_Name_nzuocC_info+56`

 ''bp11'' is the continuation of ''bp8'' which is jumped at when the `Name`
 is in WHNF. Then the `n_occ` field is being extracted.

 It turns out that ''bp11'' needs to be hit '''46 times''' in order to find
 it untagged!

 Then I looked into why it is untagged at all. Here is a transcript from
 `gdb`
 `0x420006f4a9` is the (tagged) `Name`:
 {{{
 (gdb) x/x 0x420006f4a9-1
 0x420006f4a8:   0xf52ece08
 (gdb) x/x 0x420006f4a9+7
 0x420006f4b0:   0x0006f451
 (gdb) x/x 0x420006f4a9+15
 0x420006f4b8:   0x000662a0
 (gdb) x/x 0x420006f4a9+19
 0x420006f4bc:   0x00000042
 ### the `n_occ` field is 0x42000662a0

 (gdb) x/x 0x420006f4a9+23
 0x420006f4c0:   0xf74a17e8
 (gdb) x/x 0x420006f4a9+31
 0x420006f4c8:   0x00001818
 }}}

 Then I follow the closure pointer of the `OccName`:

 {{{
 (gdb) x/2x 0x42000662a0
 0x42000662a0:   0xf2335878      0x00007fff
 (gdb) x/x 0x00007ffff2335878
 0x7ffff2335878 <stg_BLACKHOLE_info>:    0x08438b48
 }}}

 Ooops! How can a strict field point to a '''BLACKHOLE'''?

 So this is my findings. Are strict fields able to hold blackholes?
 And if so, why do they carry an ''isEvaldUnfolding` when pattern matched
 against?

 Any hints appreciated!

 >
 > One good thing would be to distill a tiny example, and it sounds as if
 you have enough insight to do that now.  E.g. perhaps you are saying that
 > {{{
 > data T = MkT ![Int]
 > f (MkT xs) = xs
 > }}}
 > returns a badly-tagged pointer?  If so, just compile that tiny program
 and see.  etc.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:18>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
Reply | Threaded
Open this post in threaded view
|

Re: [GHC] #14626: No need to enter a scrutinised value

GHC - devs mailing list
In reply to this post by GHC - devs mailing list
#14626: No need to enter a scrutinised value
-------------------------------------+-------------------------------------
        Reporter:  heisenbug         |                Owner:  heisenbug
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.2.2
      Resolution:                    |             Keywords:  performance
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #13861            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by heisenbug):

 Replying to [comment:18 heisenbug]:
 > [snippety]
 >
 > So this is my findings. Are strict fields able to hold blackholes?
 > And if so, why do they carry an `isEvaldUnfolding` when pattern matched
 against?
 >
 > Any hints appreciated! Do I have a (black)hole in my reasoning?
 >

 Following up my own question...

 I had luck and could set a few watchpoints in `lldb`, with the fourth one
 capturing the history of the `OccName`'s closure. I'll leave it here for
 the reference...
 {{{
 Watchpoint 4 hit:
 new value: 4313987056
 Process 2009 stopped
 * thread #1: tid = 0xa7e8d1, 0x0000000101223ddd
 libHSghc-8.5-ghc8.5.20180103.dylib`sxWx_info [inlined] _cA5r + 12 at
 BinIface.hs:149, queue = 'com.apple.main-thread', stop reason = watchpoint
 4
     frame #0: 0x0000000101223ddd
 libHSghc-8.5-ghc8.5.20180103.dylib`sxWx_info [inlined] _cA5r + 12 at
 BinIface.hs:149
    146
    147      -- Initialise the user-data field of bh
    148      bh <- do
 -> 149          bh <- return $ setUserData bh $ newReadState (error
 "getSymtabName")
    150
 (getDictFastString dict)
    151          symtab_p <- Binary.get bh     -- Get the symtab ptr
    152          data_p <- tellBin bh          -- Remember where we are now
 }}}
 This is presumably when the `OccName` got allocated on the heap.

 The next change happened here:
 {{{
 Watchpoint 4 hit:
 old value: 4313987056
 new value: 4463800616
 Process 2009 stopped
 * thread #1: tid = 0xa7e8d1, 0x000000010a104fc2 libHSrts_thr-
 ghc8.5.20180103.dylib`stg_upd_frame_info + 18, queue = 'com.apple.main-
 thread', stop reason = watchpoint 4
     frame #0: 0x000000010a104fc2 libHSrts_thr-
 ghc8.5.20180103.dylib`stg_upd_frame_info + 18
 libHSrts_thr-ghc8.5.20180103.dylib`stg_upd_frame_info:
 ->  0x10a104fc2 <+18>: movq   %rax, %rcx
     0x10a104fc5 <+21>: andq   $-0x100000, %rcx
     0x10a104fcc <+28>: movq   %rax, %rdx
     0x10a104fcf <+31>: andl   $0xff000, %edx
 }}}

 Let's disassemble the change:
 {{{
 (lldb) disassemble -s 4313987056
 libHSghc-8.5-ghc8.5.20180103.dylib`sxXT_info:
     0x1012237f0 <+0>:  leaq   -0x18(%rbp), %rax
     0x1012237f4 <+4>:  cmpq   %r15, %rax
     0x1012237f7 <+7>:  jb     0x10122388c               ; <+156> [inlined]
 _cA3D
     0x1012237fd <+13>: movq   0x1d6b6fc(%rip), %rax     ; (void
 *)0x000000010a104fb0: stg_upd_frame_info
     0x101223804 <+20>: movq   %rax, -0x10(%rbp)
     0x101223808 <+24>: movq   %rbx, -0x8(%rbp)
 }}}

 This gets overwritten with a BLACKHOLE:
 {{{
 (lldb) disassemble -s 4463800616
 libHSrts_thr-ghc8.5.20180103.dylib`stg_BLACKHOLE_info:
     0x10a103128 <+0>:  movq   0x8(%rbx), %rax
     0x10a10312c <+4>:  testb  $0x7, %al
     0x10a10312e <+6>:  jne    0x10a103230               ; <+264>
     0x10a103134 <+12>: movq   (%rax), %rcx
     0x10a103137 <+15>: cmpq   0x190c2(%rip), %rcx       ; (void
 *)0x000000010a1030c8: stg_IND_info
     0x10a10313e <+22>: je     0x10a103128               ; <+0>
     0x10a103140 <+24>: cmpq   0x19121(%rip), %rcx       ; (void
 *)0x000000010a103390: stg_TSO_info
 }}}

 Now, maybe the `Binary` instance breaks the invariant that the `n_occ`
 field is strict?

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14626#comment:19>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler

_______________________________________________
ghc-tickets mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-tickets
123