[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

Thu Feb 27 13:59:59 PST 2020

jyknight requested changes to this revision.
jyknight added a comment.
This revision now requires changes to proceed.

In D74918#1885869 <https://reviews.llvm.org/D74918#1885869>, @zoecarver wrote:

> @jyknight It would probably be best if we could account for CPUs who like to use aligned pairs when getting a cache line. It's probably also important that we don't change the value `getCPUCacheLineSize` returns, so, if we are going to account for that, we should probably update `getCPUCacheLineSize ` before this patch lands.

It would be extremely unfortunate to go to all the trouble of attempting to return accurate results from the P0154 interfaces, and then to return an answer insufficient to actually achieve the performance benefit it's intended for, and then be unable to fix it due to ABI concerns. So, yes, I do believe that this issue must be decided.

Additionally, my opinion here has really not changed from a couple of years ago. I continue to believe it was a mistake to standardize these constexpr values, and that absolutely the best course of action would be to continue to decline to implement this part of the standard, forever. (And that GCC should similarly also continue to decline to implement it).

That said, the list of cacheline sizes collected in this review is still useful in any case, since it needs to be copied into LLVM's X86Subtarget to implement the backend getCacheLineSize function.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74918