ChuanqiXu9 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.
Thank you! > > > 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. Yeah, understood. I think it is based on the risky level. For this patch, although it has some lines, it is relatively trivial to me. So in my mind, its risky is more or less controllable. For other patches which has higher risky level, I generally choose to land them in the early of the release circle and give its longer baking time. e.g, the Reduced BMI. 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
