fedor.sergeev added a comment. In D57265#1393453 <https://reviews.llvm.org/D57265#1393453>, @vsk wrote:
> > I'd prefer not adding this kind of state to PassBuilder. SplitColdCode is > > soemthing that refers to the construction of one particular pipeline, not > > to pipeline-building in general. It should be an argument passed down > > through the build*Pipeline calls. > > I'm not sure I understand the distinction being made here -- could you > elaborate? Isn't enabling a pass related to pipeline-building in general? If > not, I don't see how arguments to build*Pipeline //do// become related to > pipeline-building in general. > > For context, I've modeled the addition of the SplitColdCode member to > PassBuilder on PGOOpt (c.f. PGOOptions::RunProfileGen). One thing I like > about this approach is that it doesn't require changing IR, or changing very > many different APIs. But if it's really not reasonable, I'm happy to take > another shot at it. I cant say for PGOOpt in particular, but overall idea of PassBuilder is that, well, it is not "builder" as per "builder pattern". You do not fill it with parts of a complex pipeline object and then press a magical "build" button. Instead you ask it to build various "named" pipelines, or just parse it from text. If you compare with PassManagerBuilder, which contains a dozen of various data members that seem to drive the pipeline construction, PassBuilder contains only a few. And the purpose of PassBuilder members is to be used during individual *pass*es creation. Say, you wont find OptLevel there. Instead it is being passed as an argument to build*Pipeline functions. As for PGOOpts - it seems to be the only member that stands out. And to me it looks like we should remove it from PassBuilder either, and start passing it to build*Pipeline functions as well. But honestly, this is the first time I really looked through the PGOOpts code, so I might be well mistaken. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57265/new/ https://reviews.llvm.org/D57265 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
