AaronBallman wrote: > Looks like this change has some compile-time impact even if modules are not > used: > https://llvm-compile-time-tracker.com/compare.php?from=edc02351dd11cc4a39b7c541b26b71c6f36c8e55&to=7201cae106260aeb3e9bbbb7d5291ff30f05076a&stat=instructions:u > It seems to add about 0.5% to C++ compilations. > > cc @AaronBallman on the process here. I find it a bit concerning that big > changes get landed without approval to make it into the LLVM 20 release. This > is not how things are supposed to work.
We generally trust the maintainer's discretion but prefer there to be review before committing (https://llvm.org/docs/CodeReview.html#must-code-be-reviewed-prior-to-being-committed). In this case, @ChuanqiXu9 is the expert in the area and is the one who primarily reviews all modules patches anyway, so I'm inclined to trust his judgement. > I want to land this in 20.x and give it some baking times so that we can find > issues in it if any. FWIW, if we feel the need for it to have baking time, we usually would do that *after* the branch. (We get significant testing from community stakeholders like Google, Intel, etc because they pull from top of tree and test a bunch of their massive internal code bases on it.) If there are problems with the patch, pulling it out of the branch is more work and impacts folks like the release managers. If it were me, I probably would have waited to land this one, but I also don't think we need a process-related revert either. https://github.com/llvm/llvm-project/pull/122887 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
