ilya-biryukov wrote: > Because this isn't for correctness but is for performance, I think there > isn't a pressing need to land something ASAP from the community perspective > (or is the performance truly bad enough that you think the feature is not > really usable without doing something?). So could you keep the temporary > changes in your downstream until the full changes are ready upstream?
This is not something we would do because we do not want to have a dialect of C++ that we have to maintain the compiler for. Our integrates and internal compiler releases are already quite complicated, and would very quickly become unbearable if we start carrying load-bearing patches like this. When I mention that performance is a concern I don't mean to say that our problem is that we are leaving 25% of compile time on the table. Our problems are a little more severe, we are hitting compile-time limits in our remote build infrastructure that we cannot easily lift. And while the long-term solution involves doing something with the code rather than the compiler, having this builtin would allow to keep the pressure off for months to figure this out on our end. > It sounds like the other review may not be as far away from landing as > originally thought: > https://github.com/llvm/llvm-project/pull/106730#issuecomment-2876979768 There are still quite a few things to figure out there and while I'll be trying to work them out promptly, I am not so certain it can't stall. There are certainly much more details to figure out than in this PR, I can hardly imagine anything going wrong with this approach and we've been using it for a very long time for things like `__make_integer_seq`. I still believe that this change is orders of magnitude simpler and blocking it on the premise of it being implementable via pack expansions that need to be implemented in addition to it is quite significant scope creep. And again, while I'm happy to contribute that and to help move pack expansions forward with this, I am merely asking for the ability to have the ability I've originally tried to put into the compiler in the meantime, with whatever mechanism for marking it as experimental and subject to change you can imagine. > In some regards that helps, in other regards it expands the problem because > now we may end up with folks relying on the -cc1 argument existing. I thought that `-cc1` arguments are supposed to be changing over Clang releases and not something people can rely on? If this is not the right mechanism to advertise "please don't use this feature, it's experimental and subject to change", what is? Would saying "_experimental_" somewhere in the name help a little? Any option that enables to have this timely would be greatly appreciated. > (I'm pushing back because we have a historical pattern of temporary olutions > becoming permanent solutions. If the solution didn't involve introducing new > user-facing functionality, I'd be less concerned but because it is > user-facing, the concerns are higher. Imagine a world where GCC or EDG then > adds the same [temporary] builtin for Clang compatibility, for an example of > another kind of problem we have to worry about.) I understand that concern, but mimicing something marked as "experimental, we are working on the long-term solution <here>; and it's under -cc1 -fenable_this_experimental_builtin; please don't copy it just yet" seems rather adversarial. I believe that if we communicate the experimental nature of the builtin clearly, EDG and GCC developers won't just blindly force us to support it long-term. And I hope users would also do the due dilligence to use `__has_builtin` and provide alternative versions of their code if it's not supported. https://github.com/llvm/llvm-project/pull/139730 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
