[PATCH] D66822: Hardware cache line size builtins

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

numbers for cacheline size.

In D66822#1647664 <https://reviews.llvm.org/D66822#1647664>, @zoecarver wrote:

> > Passing-by remark: i'm not sure it is possible to **guarantee** that this will be always correct and that no ABI break will happen.
>
> You're right. I should have said, "least-likely to cause an ABI break." And I completely agree that there is **no way** to gaurentee this is correct at compile time. `hardware_*_interference_size` certainly has the potential to do more harm than good but, I think that is another discussion.


I don't see why we'd bother to implement this as a builtin, if we're going to implement it like this. A much simpler implementation would be to have libc++ return 64 for constructive and 128 for destructive, across the board. That'd certainly be abi stable, and also correct, at the moment, for architectures people generally care about. (And we should tell people to never use these if they actually care about it.)

BTW, I note that facebook uses 128 bytes for x86, noting in the source <https://github.com/facebook/folly/blob/5ad17f0715288d08e0db580c90b7bf8716ddfd99/folly/lang/Align.h#L99>:

  Microbenchmarks indicate that pairs of cache lines also see destructive
  interference under heavy use of atomic operations, as observed for atomic
  increment on Sandy Bridge.
  
  We assume a cache line size of 64, so we use a cache line pair size of 128
  to avoid destructive interference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66822