hans added a comment.

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.


Given how long the new pass manager has been in progress, we definitely don't 
want to block on enabling it. So yes, porting this pass to the current pass 
manager is compelling to do right now. I also don't see why it should be a big 
deal.

As for splitting it into separate passes, this patch technically does that, 
although it extracts and changes the core code a bit so it can be shared 
between the passes. I think that's how most passes have been adapted to work 
with both pass managers, no?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83013/new/

https://reviews.llvm.org/D83013



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to