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.
Yes, I want to put all the materialisation in one place. I think you are suggesting Plan A - Tidy - Materialise implicit bindings (class op selectors, data constructor workers, data constructor wrappers) - Add late cost centres - Run late plugins - Prep - CoreToStg But then we'll add cost centres to data constructor workers and wrappers. Or Plan B - Tidy - Add late cost centres - Materialise implicit bindings (class op selectors, data constructor workers, data constructor wrappers - Run late plugins - Prep - CoreToStg Now we won't get cost centres on class-op selectors, which Matthew says he wants. I suppose we could have do Plan A, but have the "add cost centre" pass skip over data constructor workers and wrappers. Do you have any views? I'm a bit inclined to combine the late-cc/materialise/prep combo into the main function of GHC.CoreToStg.Prep. But I wonder if the "late plugins" need to see (a) the materialised implicit bindings and (b) late cost centres. I have no idea how late plugins are used. Matthew asks 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. I was thinking that the class-op selector is a small, cheap function, so I don't mind not profiling it. But perhaps your point is that calling it symptomatic of running overloaded code, and we might want to know that. Simon On Mon, 7 Apr 2025 at 10:37, Andreas Klebinger <klebinger.andr...@gmx.at> wrote: > 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 > listghc-devs@haskell.orghttp://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
