[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

Fri Aug 26 18:26:36 PDT 2022

alexfh added a comment.

In D128113#3751477 <https://reviews.llvm.org/D128113#3751477>, @mizvekov wrote:

> In D128113#3747946 <https://reviews.llvm.org/D128113#3747946>, @alexfh wrote:
>
>> For now we've added a workaround for the most problematic use case and got us unblocked. Thus there is no need now in introducing additional flags. However, I'm rather concerned about the cumulative additional overhead our build infrastructure gets as a result of this patch. Measuring this would unfortunately be a project of its own, which I'm not ready to commit to. If reverting this and discussing with Richard Smith is an option for you, I'd suggest doing this now and if you end up not finding a good alternative, we would probably need to find a way to estimate the impact of this patch. WDYT?
>
> Thanks, and sorry for the long time to respond to this.
>
> I have been thinking about this problem, and I have an idea for a solution which is better on the performance side, but more complex change.
>
> We would need to implement some pattern matching during AST traversal, and keep track of the pack index there. When extracting a component of the larger type, such as when deducing non-pack arguments from a pack, we would wrap it over with a new sugar type node which encodes the current pack indexes.
>
> I reflected a bit on this, and yeah I think we should try to design something which does not potentially hinder large packs so much.
>
> I am not sure how this is going to play out in practice, and it's possible we may need to come back to this solution again, but I think it's prudent to revert it for now if we are not sure, as this is an API change, however obscure for most users.

I appreciate your effort on designing an alternative solution. Hopefully, it works out well.

> As an aside, it would be really helpful if you could track how the compilation of your codebase performs as clang evolves, regardless if we keep this change or not :)
> Maybe you can reuse something from http://llvm-compile-time-tracker.com/

Good point about tracking compile time. We do have compiler performance tests running in a controlled environment with a number of measures in place to reduce errors in results, but they are limited to a rather small sample of build targets. Tracking compiler performance (times, memory used) for all of our code and using this as a regression test for compiler performance is impractical due to the size of the code base, rate of changes, and the design of the build system (high level of parallelism, aggressive caching, shared build cluster consisting of machines with different performance characteristics).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128113