zoecarver marked 3 inline comments as done.
zoecarver added a comment.
@lebedev.ri LLVM may be better, I'm not sure. If you feel strongly I can move
it.
@jyknight I'm planning on adding a builtin that uses this method. I can put the
patch up for that first if you would like.
================
Comment at: clang/include/clang/Basic/TargetInfo.h:1192
+ // Get the cache line size of a given cpu. This method switches over
+ // the given cpu and returns `0` if the CPU is not found.
+ virtual Optional<unsigned> getCPUCacheLineSize() const { return None; }
----------------
lebedev.ri wrote:
> Comment is no longer valid - returns `None` instead.
> Also, might it be worth explicitly calling out that there is zero guarantees
> of stability of the returned values?
Good catch. I think we should try to have as much stability as possible but I
can add a note that these values may change.
================
Comment at: clang/lib/Basic/Targets/X86.cpp:1786
+ // i386
+ case CK_i386:
+ // Netburst
----------------
craig.topper wrote:
> I found the documentation for the 82385 cache controller chip for the 386.
> It's a bit weird. The tags for the cache are based on 16 byte lines, but
> there are valid bits for every 2 bytes within that line. So its possible for
> only a portion of a line to be valid.
Interesting. Does this mean that we should be returning a different value?
================
Comment at: clang/lib/Basic/Targets/X86.cpp:1840
+ // Core
+ case CK_Core2:
+ case CK_Generic:
----------------
craig.topper wrote:
> If Yonah and Penryn are 64, then Core2 should be as well. It's the generation
> between them.
I'll update it. Thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74918/new/
https://reviews.llvm.org/D74918
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
