[clang] [libcxx] [C++17] Support __GCC_[CON|DE]STRUCTIVE_SIZE (PR #89446)

James Y Knight via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 23 12:00:16 PDT 2024


https://github.com/jyknight commented:

I  think having the on-by-default diagnostic before we release this functionality is critically important -- but the primarily-useful part of that is going to be for the public libc++ APIs which expose these values (and, that already landed in mainline). This PR is almost an internal implementation detail, so doesn't really make things much worse than they are, so I don't feel like it needs to be held up by that.

I do really hope that diagnostic does get added soon.

I concur with previous comments that getting the values exactly right for each target in the first commit isn't super-important: each target maintainer may adjust as desired in the future, since these are abi-unstable values.

However, I do note that we already _almost_ have the required data in the LLVM backend for many targets, via TargetTransformInfo::getCacheLineSize. That is only a single value, so it's only correct if requesting tuning for a single CPU-model, vs a generic tuning which must express the full range of possibilities. But, when collecting the values to use here, that existing data has been ignored, which I think is unfortunate. Ideally, we would actually _share_ the data, rather than duplicating it in two different places. Though, due to architectural decisions made way-back-in-time, we often have trouble achieving actual sharing of backend-specific data...

But at the least, if we cannot actually share the same code, maybe we should attempt to be internally-consistent?

https://github.com/llvm/llvm-project/pull/89446


More information about the cfe-commits mailing list