[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM
Alina Sbirlea via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 6 11:09:00 PDT 2020
asbirlea added a comment.
In D83013#2132088 <https://reviews.llvm.org/D83013#2132088>, @aeubanks wrote:
> In D83013#2132070 <https://reviews.llvm.org/D83013#2132070>, @echristo wrote:
>
> > Adding Chandler and Alina here as well.
> >
> > In general, I don't think that this is such a great idea. Being able to have this sort of thing work more reliably is one of the reasons for the new pass manager. I think I'd like to see this split out into an old versus new pass manager pass to avoid the difficulty of cleaning this up after we finish migrating llvm to the new pass manager. This also seems to add some technical debt around options and other enablement which is also less than ideal. Is this compelling to add right now versus finishing work migrating llvm completely to the new pass manager and removing the old one? From speaking with Alina I think that work should be done in a short while.
> >
> > Thanks.
> >
> > -eric
>
>
> I don't think we're that close yet, probably at least a couple months out, there are lots of loose ends to be tied up. I'll make a post soon in llvm-dev (maybe first we can sync up again) about what I think needs to be done before the NPM switch.
+1 to sync up again and make progress towards the NPM switch.
I don't want to block this patch, but I do agree with Eric's point. We *really* want to focus more on the switch then invest into more LPM infra. Short term resolutions to unblock folks, with the best split possible, sure, keeping in mind they'll need to be cleaned up. But I don't want us to lose focus on tying up the remaining loose ends for the switch.
I think it's critical for LLVM's codebase health to focus on the NPM switch in the next couple of months.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83013/new/
https://reviews.llvm.org/D83013
More information about the cfe-commits
mailing list