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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
