A interesting wrinkle!
I agree with matt that having class methods show up in the profile can
be very helpful at times.
Constructors and their wrappers on the other hand should be of no concern.
We explicitly try to avoid wrapping them in cost centres as they
generally add no useful information to profiles.
I've briefly looked over the code and adding class methods seems to be
the *first* thing tidy does, not at it's end. (in tidyProgram)
That would suggest to me that there is a need to tidy these implicit
bindings so moving them might have further unintended consequences.
But then Note [Injecting implicit bindings] claims we do it at the end
of tidy, but seems outdated.This seems like a proper mess.
If you are sure there is no need to tidy the class method selectors then
it would be clearest to have a explicit "materializeImplicitBinds" step
in the pipeline after tidy and before lateCC runs.
That seems like it would be clearer and more coherent overall. And it
would sidestep this issue completely.
Cheers
Andreas
Am 07/04/2025 um 11:08 schrieb Matthew Pickering:
Hi Simon,
It does seem strange (at first glance) that these implicit things
happen in two different places.
However, I think you should do this refactoring in a separate MR to
!10479, it seems unrelated to the purpose of that patch and affects
other things like you have mentioned.
Why do you think the change in callstack is fine? It seems the
difference is `callstack001` is whether there an entry for `>>=` or
not, which seems like quite an important thing to place a cost centre
on? Andreas has a
better intuition for this than me. I would quite like to know if there
is an unoptimised call to >>= in my program. You have probably thought
about this a little, so interested to hear your opinion.
Cheers,
Matt
On Fri, Apr 4, 2025 at 10:44 PM Simon Peyton Jones
<simon.peytonjo...@gmail.com> wrote:
Hi Matthew, Andreas, Ben
In GHC today there is a strange inconsistency:
* Bindings for data constructor wrappers and class method
selectors are added at the end of CoreTidy.
* Bindings for data constructor workers are added at the start
of CorePrep
This is deeply strange. In !10479 (unary class patch)
<https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10479> I had
to make some changes in this area and after being baffled I moved
all these implicit bindings to one place at the start of
CorePrep. That is simpler and neater. There is no reason for
them to be separate.
BUT I find that one thing happens between Tidy and Prep, namely
late-cost-centre injection. That means we will no longer get
late-cost-centres for data constructor wrappers and class method
selectors. I think that this is what is causing diffs in the
output for callstack001 and friends in !10479.
I think that this is perfectly fine, and I propose to accept the
changes.
Another alternative would be to inject all of these bindings at
the end of Tidy (instead of the start of Prep), But it's much
nicer at the start of Prep -- we don't want any of these bindings
to appear in interface files.
Can you confirm that the change is ok? Thanks
Simon
_______________________________________________
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
_______________________________________________
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
