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

Fri Feb 21 09:37:42 PST 2020

zoecarver marked an inline comment as done.
zoecarver added inline comments.

================
Comment at: clang/lib/Basic/Targets/X86.cpp:1834
+    case CK_Tigerlake:
+    case CK_Lakemont:
+
----------------
craig.topper wrote:
> zoecarver wrote:
> > craig.topper wrote:
> > > I think Lakemont is 16 bytes. Assuming I'm interpretting the CLFLUSH line size from this CPUID dump correctly https://github.com/InstLatx64/InstLatx64/blob/master/GenuineIntel/GenuineIntel0000590_Lakemont_CPUID2.txt 
> > I may very well be interpreting this incorrectly but I think that the cache line sizes are at `eax = 0x80000005` and `eax = 0x80000006`. However, all the registers at those codes are zero. If I instead look at `eax = 0x00000001` (which I think is incorrect), `ecx = 00010200` which would be 128 bytes (maybe this is where the 16 came from, if they were bits instead?). How were you interpreting it?
> Interpreted bits 15:8 of the ecx = 00010200 to be the clflush information. The spec for cpuid says to multiply by 8 to get cache line size. So 15:8 is 2, multiply by 8 is 16 bytes.
> 
> I think Lakemont is based on the 486 architecture so that seems possible.
I see. That looks correct. I'll update it.

Still, I do find it strange that all registers at `eax = 0x80000006` are zeroed out. Using the `cpuid` instruction on my cpu I get the following:
```
CPUID 0: 13 71 110 105
CPUID 1: 97 0 191 255
CPUID 2: 1 255 0 0
CPUID 80000000: 8 0 0 0
CPUID 80000001: 0 0 33 0
CPUID 80000002: 73 108 32 101
CPUID 80000003: 41 45 48 67
CPUID 80000004: 64 50 122 0
CPUID 80000005: 0 0 0 0
CPUID 80000006: 0 0 64 0
CPUID 80000007: 0 0 0 0
CPUID 80000008: 39 0 0 0
```
I know my L2 cache line size is 64 and the L2 cache line size should be `ecx` at `eax = 0x80000006` which would be `64`. But, when I read the dump you linked to, it doesn't look like there's any data at `0x80000006`.

Anyway, as I said above, I'll look past this and update it based on the clflush information. That seems valid. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74918