Re: [PATCH] c++: implement C++17 hardware interference size
On 2021-07-17 06:32, Jonathan Wakely via Gcc-patches wrote:
On Sat, 17 Jul 2021, 09:15 Matthias Kretz, wrote:
On Friday, 16 July 2021 21:58:36 CEST Jonathan Wakely wrote: On Fri, 16
Jul 2021 at 20:26, Matthias Kretz wrote: On Friday, 16
July 2021 18:54:30 CEST Jonathan Wakely wrote: On Fri, 16 Jul 2021 at
16:33, Jason Merrill wrote: Adjusting them based on tuning would
certainly simplify a
significant
use
case, perhaps the only reasonable use. Cases more concerned with
ABI
stability probably shouldn't use them at all. And that would mean
not
needing to worry about the impossible task of finding the right
values
for
an entire architecture.
But it would be quite a significant change in behaviour if -mtune
started affecting ABI, wouldn't it?
For existing code -mtune still doesn't affect ABI.
True, because existing code isn't using the constants.
The users who write
struct keep_apart {
alignas(std::hardware_destructive_interference_size) std::atomic
cat;
alignas(std::hardware_destructive_interference_size) std::atomic
dog;
};
*want* to have different sizeof(keep_apart) depending on the CPU the
code
is compiled for. I.e. they *ask* for getting their ABI broken.
Right, but the person who wants that and the person who chooses the
-mtune option might be different people.
Yes. But it was the intent of the person who wrote the code that the
person
compiling the code can change the data layout of keep_apart via -mtune.
Of
course, if the one compiling doesn't want to choose because the binary
needs
to work on the widest range of systems, then there's a problem we might
want
to solve (direction of target_clones?). (Or the developer of the library
solves it by providing the ABI for all possible interference_size
values.)
A distro might add -mtune=core2 to all package builds by default, not
expecting it to cause ABI changes. Some header in a package in the
distro might start using the constants. Now everybody who includes
that header needs to use the same -mtune option as the distro default.
If somebody writes a library with `keep_apart` in the public API/ABI
then
you're right.
Yes, it's fine if those constants don't affect anything across module
boundaries.
That change in the behaviour and expected use of an existing option
seems scary to me. Even with a warning about using the constants
(because somebody's just going to use #pragma around their use of the
constants to disable the warning, and now the ABI impact of -mtune is
much less obvious).
There are people who say that linking TUs compiled with different
compiler
flags is UB. In general I think that's correct, but we can make
explicit
exceptions. Up to now -mtune wouldn't lead to UB, AFAIK, though -march
easily
does. So maybe, to keep the status quo, the constants should be tied to
-march
not -mtune?
It's much less scary in a world where the code is written and used by
the same group of people, but for something like a linux distro it
worries me.
The developer who wants his code to be included in a distro should care
about
binary distribution. If his code has an ABI issue, that's a bug he
needs
to
fix. It's not the fault of the packager.
Yes but in practice it's the packagers who have to deal with the bug
reports, analyze the problem, and often fix the bug too. It might not be
the packager's fault but it's often their problem :-(
Apropos of nothing, I can absolutely see the use of this creeping into
Boost at some point.
Re: [PATCH] c++: implement C++17 hardware interference size
On 7/19/21 5:41 AM, Richard Earnshaw wrote: On 17/07/2021 22:37, Jason Merrill via Gcc-patches wrote: On Sat, Jul 17, 2021 at 6:55 AM Matthias Kretz wrote: On Saturday, 17 July 2021 15:32:42 CEST Jonathan Wakely wrote: On Sat, 17 Jul 2021, 09:15 Matthias Kretz, wrote: If somebody writes a library with `keep_apart` in the public API/ABI then you're right. Yes, it's fine if those constants don't affect anything across module boundaries. I believe a significant fraction of hardware interference size usage will be internal. I would hope for this to be the vast majority of usage. I want the warning to discourage people from using the interference size variables in the public API of a library. The developer who wants his code to be included in a distro should care about binary distribution. If his code has an ABI issue, that's a bug he needs to fix. It's not the fault of the packager. Yes but in practice it's the packagers who have to deal with the bug reports, analyze the problem, and often fix the bug too. It might not be the packager's fault but it's often their problem I can imagine. But I don't think requiring users to specify the value according to what -mtune suggests will improve things. Users will write a configure/cmake/... macro to parse the value -mtune prints and pass that on the command line (we'll soon find this solution on SO ). I.e. things are likely to be even more broken. Simpler would be a flag to say "set them based on -mtune", e.g. -finterference-tuning or --param destructive-intereference-size=tuning. That would be just as easy to write as -Wno-interference-size. Please be very careful about an option name like that. The x86 meaning and interpretation of -mtune is subtly different to that of Arm and AArch64 and possibly other targets as well. Also, should the behaviour of a compiler configured with --with-cpu=foo be handled differently to a command-line option that sets foo explicitly? In the back-end I'm not sure we can really tell the difference. I don't see any reason to treat them differently. The meaning of this option would be "set the interference sizes to be optimal for the current target CPU, without regard for ABI stability". For x86 this wouldn't have any effect; for Arm/AArch64 it would set them to the tuning L1 cache line size, if set. Here's what I have currently: Jason >From b10bfd228f23ef2f7499802c8fd1c84798646039 Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Thu, 15 Jul 2021 15:30:17 -0400 Subject: [PATCH] c++: implement C++17 hardware interference size To: gcc-patches@gcc.gnu.org The last missing piece of the C++17 standard library is the hardware intereference size constants. Much of the delay in implementing these has been due to uncertainty about what the right values are, and even whether there is a single constant value that is suitable; the destructive interference size is intended to be used in structure layout, so program ABIs will depend on it. In principle, both of these values should be the same as the target's L1 cache line size. When compiling for a generic target that is intended to support a range of target CPUs with different cache line sizes, the constructive size should probably be the minimum size, and the destructive size the maximum, unless you are constrained by ABI compatibility with previous code. JF Bastien's implementation proposal is summarized at https://github.com/itanium-cxx-abi/cxx-abi/issues/74 I implement this by adding new --params for the two sizes. Targets need to override these values in targetm.target_option.override() to support the feature. 64 bytes still seems correct for the x86 family. I'm not sure why he said 64/64 for 32-bit ARM, since the Cortex A9 has a 32-byte cache line, and that seems to be the only ARM_PREFETCH_BENEFICIAL target, so I'd think 32/64 would make more sense. He proposed 64/128 for AArch64, but since the A64FX now has a 256B cache line, I've changed that to 64/256. Does that seem right? Currently the patch does not adjust the values based on -march, as in JF's proposal. I'll need more guidance from the ARM/AArch64 maintainers about how to go about that. --param l1-cache-line-size is set based on -mtune, but I don't think we want -mtune to change these ABI-affecting values. Are there -march values for which a smaller range than 64-256 makes sense? gcc/ChangeLog: * params.opt: Add destructive-interference-size and constructive-interference-size. * doc/invoke.texi: Document them. * config/aarch64/aarch64.c (aarch64_override_options_internal): Set them. * config/arm/arm.c (arm_option_override): Set them. * config/i386/i386-options.c (ix86_option_override_internal): Set them. gcc/c-family/ChangeLog: * c.opt: Add -Winterference-size. * c-cppbuiltin.c (cpp_atomic_builtins): Add __GCC_DESTRUCTIVE_SIZE and __GCC_CONSTRUCTIVE_SIZE. gcc/cp/ChangeLog: * decl.c (cxx_init_decl_processing): Check
Re: [PATCH] c++: implement C++17 hardware interference size
On 17/07/2021 22:37, Jason Merrill via Gcc-patches wrote: On Sat, Jul 17, 2021 at 6:55 AM Matthias Kretz wrote: On Saturday, 17 July 2021 15:32:42 CEST Jonathan Wakely wrote: On Sat, 17 Jul 2021, 09:15 Matthias Kretz, wrote: If somebody writes a library with `keep_apart` in the public API/ABI then you're right. Yes, it's fine if those constants don't affect anything across module boundaries. I believe a significant fraction of hardware interference size usage will be internal. I would hope for this to be the vast majority of usage. I want the warning to discourage people from using the interference size variables in the public API of a library. The developer who wants his code to be included in a distro should care about binary distribution. If his code has an ABI issue, that's a bug he needs to fix. It's not the fault of the packager. Yes but in practice it's the packagers who have to deal with the bug reports, analyze the problem, and often fix the bug too. It might not be the packager's fault but it's often their problem I can imagine. But I don't think requiring users to specify the value according to what -mtune suggests will improve things. Users will write a configure/cmake/... macro to parse the value -mtune prints and pass that on the command line (we'll soon find this solution on SO ). I.e. things are likely to be even more broken. Simpler would be a flag to say "set them based on -mtune", e.g. -finterference-tuning or --param destructive-intereference-size=tuning. That would be just as easy to write as -Wno-interference-size. Jason Please be very careful about an option name like that. The x86 meaning and interpretation of -mtune is subtly different to that of Arm and AArch64 and possibly other targets as well. Also, should the behaviour of a compiler configured with --with-cpu=foo be handled differently to a command-line option that sets foo explicitly? In the back-end I'm not sure we can really tell the difference. R.
Re: [PATCH] c++: implement C++17 hardware interference size
On Sat, Jul 17, 2021 at 6:55 AM Matthias Kretz wrote: > On Saturday, 17 July 2021 15:32:42 CEST Jonathan Wakely wrote: > > On Sat, 17 Jul 2021, 09:15 Matthias Kretz, wrote: > > > If somebody writes a library with `keep_apart` in the public API/ABI > then > > > you're right. > > > > Yes, it's fine if those constants don't affect anything across module > > boundaries. > > I believe a significant fraction of hardware interference size usage will > be > internal. > I would hope for this to be the vast majority of usage. I want the warning to discourage people from using the interference size variables in the public API of a library. > > > The developer who wants his code to be included in a distro should care > > > about > > > binary distribution. If his code has an ABI issue, that's a bug he > needs > > > to > > > fix. It's not the fault of the packager. > > > > Yes but in practice it's the packagers who have to deal with the bug > > reports, analyze the problem, and often fix the bug too. It might not be > > the packager's fault but it's often their problem > > I can imagine. But I don't think requiring users to specify the value > according to what -mtune suggests will improve things. Users will write a > configure/cmake/... macro to parse the value -mtune prints and pass that > on > the command line (we'll soon find this solution on SO ). I.e. things are > likely to be even more broken. Simpler would be a flag to say "set them based on -mtune", e.g. -finterference-tuning or --param destructive-intereference-size=tuning. That would be just as easy to write as -Wno-interference-size. Jason
Re: [PATCH] c++: implement C++17 hardware interference size
On Saturday, 17 July 2021 15:32:42 CEST Jonathan Wakely wrote: > On Sat, 17 Jul 2021, 09:15 Matthias Kretz, wrote: > > If somebody writes a library with `keep_apart` in the public API/ABI then > > you're right. > > Yes, it's fine if those constants don't affect anything across module > boundaries. I believe a significant fraction of hardware interference size usage will be internal. > > The developer who wants his code to be included in a distro should care > > about > > binary distribution. If his code has an ABI issue, that's a bug he needs > > to > > fix. It's not the fault of the packager. > > Yes but in practice it's the packagers who have to deal with the bug > reports, analyze the problem, and often fix the bug too. It might not be > the packager's fault but it's often their problem I can imagine. But I don't think requiring users to specify the value according to what -mtune suggests will improve things. Users will write a configure/cmake/... macro to parse the value -mtune prints and pass that on the command line (we'll soon find this solution on SO ). I.e. things are likely to be even more broken. -- ── Dr. Matthias Kretz https://mattkretz.github.io GSI Helmholtz Centre for Heavy Ion Research https://gsi.de std::experimental::simd https://github.com/VcDevel/std-simd ──
Re: [PATCH] c++: implement C++17 hardware interference size
On Sat, 17 Jul 2021, 09:15 Matthias Kretz, wrote:
> On Friday, 16 July 2021 21:58:36 CEST Jonathan Wakely wrote:
> > On Fri, 16 Jul 2021 at 20:26, Matthias Kretz wrote:
> > > On Friday, 16 July 2021 18:54:30 CEST Jonathan Wakely wrote:
> > > > On Fri, 16 Jul 2021 at 16:33, Jason Merrill wrote:
> > > > > Adjusting them based on tuning would certainly simplify a
> significant
> > > > > use
> > > > > case, perhaps the only reasonable use. Cases more concerned with
> ABI
> > > > > stability probably shouldn't use them at all. And that would mean
> not
> > > > > needing to worry about the impossible task of finding the right
> values
> > > > > for
> > > > > an entire architecture.
> > > >
> > > > But it would be quite a significant change in behaviour if -mtune
> > > > started affecting ABI, wouldn't it?
> > >
> > > For existing code -mtune still doesn't affect ABI.
> >
> > True, because existing code isn't using the constants.
> >
> > >The users who write
> > >
> > > struct keep_apart {
> > >
> > > alignas(std::hardware_destructive_interference_size) std::atomic
> > > cat;
> > > alignas(std::hardware_destructive_interference_size) std::atomic
> > > dog;
> > >
> > > };
> > >
> > > *want* to have different sizeof(keep_apart) depending on the CPU the
> code
> > > is compiled for. I.e. they *ask* for getting their ABI broken.
> >
> > Right, but the person who wants that and the person who chooses the
> > -mtune option might be different people.
>
> Yes. But it was the intent of the person who wrote the code that the
> person
> compiling the code can change the data layout of keep_apart via -mtune. Of
> course, if the one compiling doesn't want to choose because the binary
> needs
> to work on the widest range of systems, then there's a problem we might
> want
> to solve (direction of target_clones?). (Or the developer of the library
> solves it by providing the ABI for all possible interference_size values.)
>
> > A distro might add -mtune=core2 to all package builds by default, not
> > expecting it to cause ABI changes. Some header in a package in the
> > distro might start using the constants. Now everybody who includes
> > that header needs to use the same -mtune option as the distro default.
>
> If somebody writes a library with `keep_apart` in the public API/ABI then
> you're right.
>
Yes, it's fine if those constants don't affect anything across module
boundaries.
> > That change in the behaviour and expected use of an existing option
> > seems scary to me. Even with a warning about using the constants
> > (because somebody's just going to use #pragma around their use of the
> > constants to disable the warning, and now the ABI impact of -mtune is
> > much less obvious).
>
> There are people who say that linking TUs compiled with different compiler
> flags is UB. In general I think that's correct, but we can make explicit
> exceptions. Up to now -mtune wouldn't lead to UB, AFAIK, though -march
> easily
> does. So maybe, to keep the status quo, the constants should be tied to
> -march
> not -mtune?
>
> > It's much less scary in a world where the code is written and used by
> > the same group of people, but for something like a linux distro it
> > worries me.
>
> The developer who wants his code to be included in a distro should care
> about
> binary distribution. If his code has an ABI issue, that's a bug he needs
> to
> fix. It's not the fault of the packager.
>
Yes but in practice it's the packagers who have to deal with the bug
reports, analyze the problem, and often fix the bug too. It might not be
the packager's fault but it's often their problem :-(
Re: [PATCH] c++: implement C++17 hardware interference size
On Friday, 16 July 2021 21:58:36 CEST Jonathan Wakely wrote:
> On Fri, 16 Jul 2021 at 20:26, Matthias Kretz wrote:
> > On Friday, 16 July 2021 18:54:30 CEST Jonathan Wakely wrote:
> > > On Fri, 16 Jul 2021 at 16:33, Jason Merrill wrote:
> > > > Adjusting them based on tuning would certainly simplify a significant
> > > > use
> > > > case, perhaps the only reasonable use. Cases more concerned with ABI
> > > > stability probably shouldn't use them at all. And that would mean not
> > > > needing to worry about the impossible task of finding the right values
> > > > for
> > > > an entire architecture.
> > >
> > > But it would be quite a significant change in behaviour if -mtune
> > > started affecting ABI, wouldn't it?
> >
> > For existing code -mtune still doesn't affect ABI.
>
> True, because existing code isn't using the constants.
>
> >The users who write
> >
> > struct keep_apart {
> >
> > alignas(std::hardware_destructive_interference_size) std::atomic
> > cat;
> > alignas(std::hardware_destructive_interference_size) std::atomic
> > dog;
> >
> > };
> >
> > *want* to have different sizeof(keep_apart) depending on the CPU the code
> > is compiled for. I.e. they *ask* for getting their ABI broken.
>
> Right, but the person who wants that and the person who chooses the
> -mtune option might be different people.
Yes. But it was the intent of the person who wrote the code that the person
compiling the code can change the data layout of keep_apart via -mtune. Of
course, if the one compiling doesn't want to choose because the binary needs
to work on the widest range of systems, then there's a problem we might want
to solve (direction of target_clones?). (Or the developer of the library
solves it by providing the ABI for all possible interference_size values.)
> A distro might add -mtune=core2 to all package builds by default, not
> expecting it to cause ABI changes. Some header in a package in the
> distro might start using the constants. Now everybody who includes
> that header needs to use the same -mtune option as the distro default.
If somebody writes a library with `keep_apart` in the public API/ABI then
you're right.
> That change in the behaviour and expected use of an existing option
> seems scary to me. Even with a warning about using the constants
> (because somebody's just going to use #pragma around their use of the
> constants to disable the warning, and now the ABI impact of -mtune is
> much less obvious).
There are people who say that linking TUs compiled with different compiler
flags is UB. In general I think that's correct, but we can make explicit
exceptions. Up to now -mtune wouldn't lead to UB, AFAIK, though -march easily
does. So maybe, to keep the status quo, the constants should be tied to -march
not -mtune?
> It's much less scary in a world where the code is written and used by
> the same group of people, but for something like a linux distro it
> worries me.
The developer who wants his code to be included in a distro should care about
binary distribution. If his code has an ABI issue, that's a bug he needs to
fix. It's not the fault of the packager.
--
──
Dr. Matthias Kretz https://mattkretz.github.io
GSI Helmholtz Centre for Heavy Ion Research https://gsi.de
std::experimental::simd https://github.com/VcDevel/std-simd
──
Re: [PATCH] c++: implement C++17 hardware interference size
On Fri, Jul 16, 2021 at 3:37 PM Matthias Kretz wrote:
> On Friday, 16 July 2021 19:20:29 CEST Noah Goldstein wrote:
> > On Fri, Jul 16, 2021 at 11:12 AM Matthias Kretz wrote:
> > > I don't understand how this feature would lead to false sharing. But
> maybe
> > > I
> > > misunderstand the spatial prefetcher. The first access to one of the
> two
> > > cache
> > > lines pairs would bring both cache lines to LLC (and possibly L2). If a
> > > core
> > > with a different L2 reads the other cache line the cache line would be
> > > duplicated; if it writes to it, it would be exclusive to the other
> core's
> > > L2.
> > > The cache line pairs do not affect each other anymore. Maybe there's a
> > > minor
> > > inefficiency on initial transfer from memory, but isn't that all?
> >
> > If two cores that do not share an L2 cache need exclusive access to
> > a cache-line, the L2 spatial prefetcher could cause pingponging if those
> > two cache-lines were adjacent and shared the same 128 byte alignment.
> > Say core A requests line x1 in exclusive, it also get line x2 (not sure
> > if x2 would be in shared or exclusive), core B then requests x2 in
> > exclusive,
> > it also gets x1. Irrelevant of the state x1 comes into core B's private
> L2
> > cache
> > it invalidates the exclusive state on cache-line x1 in core A's private
> L2
> > cache. If this was done in a loop (say a simple `lock add` loop) it would
> > cause
> > pingponging on cache-lines x1/x2 between core A and B's private L2
> caches.
>
> Quoting the latest ORM: "The following two hardware prefetchers fetched
> data
> from memory to the L2 cache and last level cache:
> Spatial Prefetcher: This prefetcher strives to complete every cache line
> fetched to the L2 cache with the pair line that completes it to a 128-byte
> aligned chunk."
>
> 1. If the requested cache line is already present on some other core, the
> spatial prefetcher should not get used ("fetched data from memory").
>
I think this is correct and I'm incorrect that a request from LLC to L2
will invoke the spatial prefetcher. So not issues with 64 bytes. Sorry for
the added confusion!
>
> 2. The section is about data prefetching. It is unclear whether the
> spatial
> prefetcher applies at all for normal cache line fetches.
>
> 3. The ORM uses past tense ("The following two hardware prefetchers
> fetched
> data"), which indicates to me that Intel isn't doing this for newer
> generations anymore.
> 4. If I'm wrong on points 1 & 2 consider this: Core 1 requests a read of
> cache
> line A and the adjacent cache line B thus is also loaded to LLC. Core 2
> request a read of line B and thus loads line A into LLC. Now both cores
> have
> both cache lines in LLC. Core 1 writes to line A, which invalidates line A
> in
> LLC of Core 2 but does not affect line B. Core 2 writes to line B,
> invalidating line A for Core 1. => no false sharing. Where did I get my
> mental
> cache protocol wrong?
> --
> ──
> Dr. Matthias Kretz https://mattkretz.github.io
> GSI Helmholtz Centre for Heavy Ion Research https://gsi.de
> std::experimental::simd https://github.com/VcDevel/std-simd
> ──
>
>
>
>
Re: [PATCH] c++: implement C++17 hardware interference size
On Fri, 16 Jul 2021 at 20:26, Matthias Kretz wrote:
>
> On Friday, 16 July 2021 18:54:30 CEST Jonathan Wakely wrote:
> > On Fri, 16 Jul 2021 at 16:33, Jason Merrill wrote:
> > > Adjusting them based on tuning would certainly simplify a significant use
> > > case, perhaps the only reasonable use. Cases more concerned with ABI
> > > stability probably shouldn't use them at all. And that would mean not
> > > needing to worry about the impossible task of finding the right values for
> > > an entire architecture.
> >
> > But it would be quite a significant change in behaviour if -mtune
> > started affecting ABI, wouldn't it?
>
> For existing code -mtune still doesn't affect ABI.
True, because existing code isn't using the constants.
>The users who write
>
> struct keep_apart {
> alignas(std::hardware_destructive_interference_size) std::atomic cat;
> alignas(std::hardware_destructive_interference_size) std::atomic dog;
> };
>
> *want* to have different sizeof(keep_apart) depending on the CPU the code is
> compiled for. I.e. they *ask* for getting their ABI broken.
Right, but the person who wants that and the person who chooses the
-mtune option might be different people.
A distro might add -mtune=core2 to all package builds by default, not
expecting it to cause ABI changes. Some header in a package in the
distro might start using the constants. Now everybody who includes
that header needs to use the same -mtune option as the distro default.
That change in the behaviour and expected use of an existing option
seems scary to me. Even with a warning about using the constants
(because somebody's just going to use #pragma around their use of the
constants to disable the warning, and now the ABI impact of -mtune is
much less obvious).
It's much less scary in a world where the code is written and used by
the same group of people, but for something like a linux distro it
worries me.
Re: [PATCH] c++: implement C++17 hardware interference size
On Friday, 16 July 2021 19:20:29 CEST Noah Goldstein wrote:
> On Fri, Jul 16, 2021 at 11:12 AM Matthias Kretz wrote:
> > I don't understand how this feature would lead to false sharing. But maybe
> > I
> > misunderstand the spatial prefetcher. The first access to one of the two
> > cache
> > lines pairs would bring both cache lines to LLC (and possibly L2). If a
> > core
> > with a different L2 reads the other cache line the cache line would be
> > duplicated; if it writes to it, it would be exclusive to the other core's
> > L2.
> > The cache line pairs do not affect each other anymore. Maybe there's a
> > minor
> > inefficiency on initial transfer from memory, but isn't that all?
>
> If two cores that do not share an L2 cache need exclusive access to
> a cache-line, the L2 spatial prefetcher could cause pingponging if those
> two cache-lines were adjacent and shared the same 128 byte alignment.
> Say core A requests line x1 in exclusive, it also get line x2 (not sure
> if x2 would be in shared or exclusive), core B then requests x2 in
> exclusive,
> it also gets x1. Irrelevant of the state x1 comes into core B's private L2
> cache
> it invalidates the exclusive state on cache-line x1 in core A's private L2
> cache. If this was done in a loop (say a simple `lock add` loop) it would
> cause
> pingponging on cache-lines x1/x2 between core A and B's private L2 caches.
Quoting the latest ORM: "The following two hardware prefetchers fetched data
from memory to the L2 cache and last level cache:
Spatial Prefetcher: This prefetcher strives to complete every cache line
fetched to the L2 cache with the pair line that completes it to a 128-byte
aligned chunk."
1. If the requested cache line is already present on some other core, the
spatial prefetcher should not get used ("fetched data from memory").
2. The section is about data prefetching. It is unclear whether the spatial
prefetcher applies at all for normal cache line fetches.
3. The ORM uses past tense ("The following two hardware prefetchers fetched
data"), which indicates to me that Intel isn't doing this for newer
generations anymore.
4. If I'm wrong on points 1 & 2 consider this: Core 1 requests a read of cache
line A and the adjacent cache line B thus is also loaded to LLC. Core 2
request a read of line B and thus loads line A into LLC. Now both cores have
both cache lines in LLC. Core 1 writes to line A, which invalidates line A in
LLC of Core 2 but does not affect line B. Core 2 writes to line B,
invalidating line A for Core 1. => no false sharing. Where did I get my mental
cache protocol wrong?
--
──
Dr. Matthias Kretz https://mattkretz.github.io
GSI Helmholtz Centre for Heavy Ion Research https://gsi.de
std::experimental::simd https://github.com/VcDevel/std-simd
──
Re: [PATCH] c++: implement C++17 hardware interference size
On Friday, 16 July 2021 18:54:30 CEST Jonathan Wakely wrote:
> On Fri, 16 Jul 2021 at 16:33, Jason Merrill wrote:
> > Adjusting them based on tuning would certainly simplify a significant use
> > case, perhaps the only reasonable use. Cases more concerned with ABI
> > stability probably shouldn't use them at all. And that would mean not
> > needing to worry about the impossible task of finding the right values for
> > an entire architecture.
>
> But it would be quite a significant change in behaviour if -mtune
> started affecting ABI, wouldn't it?
For existing code -mtune still doesn't affect ABI. The users who write
struct keep_apart {
alignas(std::hardware_destructive_interference_size) std::atomic cat;
alignas(std::hardware_destructive_interference_size) std::atomic dog;
};
*want* to have different sizeof(keep_apart) depending on the CPU the code is
compiled for. I.e. they *ask* for getting their ABI broken. If they wanted to
specify the value themselves on the command line they'd written:
struct keep_apart {
alignas(SOME_MACRO) std::atomic cat;
alignas(SOME_MACRO) std::atomic dog;
};
I would be very disappointed if std::hardware_destructive_interference_size
and std::hardware_constructive_interference_size turn into a glorified macro.
--
──
Dr. Matthias Kretz https://mattkretz.github.io
GSI Helmholtz Centre for Heavy Ion Research https://gsi.de
std::experimental::simd https://github.com/VcDevel/std-simd
──
Re: [PATCH] c++: implement C++17 hardware interference size
On Fri, Jul 16, 2021, 12:54 PM Jonathan Wakely wrote: > On Fri, 16 Jul 2021 at 16:33, Jason Merrill wrote: > > Adjusting them based on tuning would certainly simplify a significant use > > case, perhaps the only reasonable use. Cases more concerned with ABI > > stability probably shouldn't use them at all. And that would mean not > > needing to worry about the impossible task of finding the right values > for > > an entire architecture. > > But it would be quite a significant change in behaviour if -mtune > started affecting ABI, wouldn't it? > Absolutely, though with the below warning, which could mention this issue, it would only affect the ABI of code that ignores the warning. Code that silences it by specifying values would not be affected. > I'm thinking about warning by default for any use of the variables without > > explicitly specifying their values on the command line. Users could > disable > > the warning if they're happy using whatever the defaults happen to be. > > I like that suggestion. > > Maybe the warning could suggest optimal values based on the current > -mtune flag. Sounds good. That way -mtune wouldn't need to alter ABI, but by > combining -mtune with explicit values for the variables you get the > best performance. And -mtune without overriding the default values > preserves ABI. > >
Re: [PATCH] c++: implement C++17 hardware interference size
On Fri, Jul 16, 2021 at 11:12 AM Matthias Kretz wrote: > On Friday, 16 July 2021 04:41:17 CEST Jason Merrill via Gcc-patches wrote: > > > Currently the patch does not adjust the values based on -march, as in > JF's > > > proposal. I'll need more guidance from the ARM/AArch64 maintainers > about > > > how to go about that. --param l1-cache-line-size is set based on > -mtune, > > > but I don't think we want -mtune to change these ABI-affecting values. > > > Are > > > there -march values for which a smaller range than 64-256 makes sense? > > As a user who cares about ABI but also cares about maximizing performance > of > builds for a specific HPC setup I'd expect the hardware interference size > values to be allowed to break ABIs. The point of these values is to give > me > better performance portability (but not necessarily binary portability) > than > my usual "pick 64 as a good average". > Wrt, -march / -mtune setting hardware interference size: IMO -mtune=X > should > be interpreted as "my binary is supposed to be optimized for X, I accept > inefficiencies on everything that's not X". > > On Friday, 16 July 2021 04:48:52 CEST Noah Goldstein wrote: > > On intel x86 systems with a private L2 cache the spatial prefetcher > > can cause destructive interference along 128 byte aligned boundaries. > > > https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-3 > > 2-architectures-optimization-manual.pdf#page=60 > > I don't understand how this feature would lead to false sharing. But maybe > I > misunderstand the spatial prefetcher. The first access to one of the two > cache > lines pairs would bring both cache lines to LLC (and possibly L2). If a > core > with a different L2 reads the other cache line the cache line would be > duplicated; if it writes to it, it would be exclusive to the other core's > L2. > The cache line pairs do not affect each other anymore. Maybe there's a > minor > inefficiency on initial transfer from memory, but isn't that all? > If two cores that do not share an L2 cache need exclusive access to a cache-line, the L2 spatial prefetcher could cause pingponging if those two cache-lines were adjacent and shared the same 128 byte alignment. Say core A requests line x1 in exclusive, it also get line x2 (not sure if x2 would be in shared or exclusive), core B then requests x2 in exclusive, it also gets x1. Irrelevant of the state x1 comes into core B's private L2 cache it invalidates the exclusive state on cache-line x1 in core A's private L2 cache. If this was done in a loop (say a simple `lock add` loop) it would cause pingponging on cache-lines x1/x2 between core A and B's private L2 caches. > > That said. Intel documents the spatial prefetcher exclusively for Sandy > Bridge. So if you still believe 128 is necessary, set the destructive > hardware > interference size to 64 for all of x86 except -mtune=sandybridge. > AFAIK the spatial prefetcher exists on newer x86_64 machines as well. > > -- > ── > Dr. Matthias Kretz https://mattkretz.github.io > GSI Helmholtz Centre for Heavy Ion Research https://gsi.de > std::experimental::simd https://github.com/VcDevel/std-simd > ── >
Re: [PATCH] c++: implement C++17 hardware interference size
On Fri, 16 Jul 2021 at 16:33, Jason Merrill wrote: > Adjusting them based on tuning would certainly simplify a significant use > case, perhaps the only reasonable use. Cases more concerned with ABI > stability probably shouldn't use them at all. And that would mean not > needing to worry about the impossible task of finding the right values for > an entire architecture. But it would be quite a significant change in behaviour if -mtune started affecting ABI, wouldn't it? > I'm thinking about warning by default for any use of the variables without > explicitly specifying their values on the command line. Users could disable > the warning if they're happy using whatever the defaults happen to be. I like that suggestion. Maybe the warning could suggest optimal values based on the current -mtune flag. That way -mtune wouldn't need to alter ABI, but by combining -mtune with explicit values for the variables you get the best performance. And -mtune without overriding the default values preserves ABI.
Re: [PATCH] c++: implement C++17 hardware interference size
On Fri, Jul 16, 2021, 11:12 AM Matthias Kretz wrote: > On Friday, 16 July 2021 04:41:17 CEST Jason Merrill via Gcc-patches wrote: > > > Currently the patch does not adjust the values based on -march, as in > JF's > > > proposal. I'll need more guidance from the ARM/AArch64 maintainers > about > > > how to go about that. --param l1-cache-line-size is set based on > -mtune, > > > but I don't think we want -mtune to change these ABI-affecting values. > > > Are > > > there -march values for which a smaller range than 64-256 makes sense? > > As a user who cares about ABI but also cares about maximizing performance > of > builds for a specific HPC setup I'd expect the hardware interference size > values to be allowed to break ABIs. The point of these values is to give > me > better performance portability (but not necessarily binary portability) > than > my usual "pick 64 as a good average". > > Wrt, -march / -mtune setting hardware interference size: IMO -mtune=X > should > be interpreted as "my binary is supposed to be optimized for X, I accept > inefficiencies on everything that's not X". > > On Friday, 16 July 2021 04:48:52 CEST Noah Goldstein wrote: > > On intel x86 systems with a private L2 cache the spatial prefetcher > > can cause destructive interference along 128 byte aligned boundaries. > > > https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-3 > > 2-architectures-optimization-manual.pdf#page=60 > > I don't understand how this feature would lead to false sharing. But maybe > I > misunderstand the spatial prefetcher. The first access to one of the two > cache > lines pairs would bring both cache lines to LLC (and possibly L2). If a > core > with a different L2 reads the other cache line the cache line would be > duplicated; if it writes to it, it would be exclusive to the other core's > L2. > The cache line pairs do not affect each other anymore. Maybe there's a > minor > inefficiency on initial transfer from memory, but isn't that all? > > That said. Intel documents the spatial prefetcher exclusively for Sandy > Bridge. So if you still believe 128 is necessary, set the destructive > hardware > interference size to 64 for all of x86 except -mtune=sandybridge. > Adjusting them based on tuning would certainly simplify a significant use case, perhaps the only reasonable use. Cases more concerned with ABI stability probably shouldn't use them at all. And that would mean not needing to worry about the impossible task of finding the right values for an entire architecture. I'm thinking about warning by default for any use of the variables without explicitly specifying their values on the command line. Users could disable the warning if they're happy using whatever the defaults happen to be. Jason >
Re: [PATCH] c++: implement C++17 hardware interference size
On Friday, 16 July 2021 04:41:17 CEST Jason Merrill via Gcc-patches wrote: > > Currently the patch does not adjust the values based on -march, as in JF's > > proposal. I'll need more guidance from the ARM/AArch64 maintainers about > > how to go about that. --param l1-cache-line-size is set based on -mtune, > > but I don't think we want -mtune to change these ABI-affecting values. > > Are > > there -march values for which a smaller range than 64-256 makes sense? As a user who cares about ABI but also cares about maximizing performance of builds for a specific HPC setup I'd expect the hardware interference size values to be allowed to break ABIs. The point of these values is to give me better performance portability (but not necessarily binary portability) than my usual "pick 64 as a good average". Wrt, -march / -mtune setting hardware interference size: IMO -mtune=X should be interpreted as "my binary is supposed to be optimized for X, I accept inefficiencies on everything that's not X". On Friday, 16 July 2021 04:48:52 CEST Noah Goldstein wrote: > On intel x86 systems with a private L2 cache the spatial prefetcher > can cause destructive interference along 128 byte aligned boundaries. > https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-3 > 2-architectures-optimization-manual.pdf#page=60 I don't understand how this feature would lead to false sharing. But maybe I misunderstand the spatial prefetcher. The first access to one of the two cache lines pairs would bring both cache lines to LLC (and possibly L2). If a core with a different L2 reads the other cache line the cache line would be duplicated; if it writes to it, it would be exclusive to the other core's L2. The cache line pairs do not affect each other anymore. Maybe there's a minor inefficiency on initial transfer from memory, but isn't that all? That said. Intel documents the spatial prefetcher exclusively for Sandy Bridge. So if you still believe 128 is necessary, set the destructive hardware interference size to 64 for all of x86 except -mtune=sandybridge. -- ── Dr. Matthias Kretz https://mattkretz.github.io GSI Helmholtz Centre for Heavy Ion Research https://gsi.de std::experimental::simd https://github.com/VcDevel/std-simd ──
Re: [PATCH] c++: implement C++17 hardware interference size
On 16/07/2021 12:17, Jonathan Wakely via Gcc-patches wrote: On Fri, 16 Jul 2021 at 03:51, Noah Goldstein wrote: On intel x86 systems with a private L2 cache the spatial prefetcher can cause destructive interference along 128 byte aligned boundaries. https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-optimization-manual.pdf#page=60 Which is a good example of why these "constants" should never have been standardized in the first place. Sigh. +1 for that. I'll have a chat with our architecture guys, but I've no idea if they'll commit to any (useful) values for either constant. R.
Re: [PATCH] c++: implement C++17 hardware interference size
On Fri, 16 Jul 2021 at 03:42, Jason Merrill via Libstdc++
wrote:
> > diff --git a/libstdc++-v3/include/std/version
> > b/libstdc++-v3/include/std/version
> > index 27bcd32cb60..d5e155db48b 100644
> > --- a/libstdc++-v3/include/std/version
> > +++ b/libstdc++-v3/include/std/version
> > @@ -140,6 +140,9 @@
> > #define __cpp_lib_filesystem 201703
> > #define __cpp_lib_gcd 201606
> > #define __cpp_lib_gcd_lcm 201606
> > +#ifdef __GCC_DESTRUCTIVE_SIZE
> > +# define __cpp_lib_hardware_interference_size 201703L
> > +#endif
> > #define __cpp_lib_hypot 201603
> > #define __cpp_lib_invoke 201411L
> > #define __cpp_lib_lcm 201606
> > diff --git a/libstdc++-v3/libsupc++/new b/libstdc++-v3/libsupc++/new
> > index 3349b13fd1b..7bc67a6cb02 100644
> > --- a/libstdc++-v3/libsupc++/new
> > +++ b/libstdc++-v3/libsupc++/new
> > @@ -183,9 +183,9 @@ inline void operator delete[](void*, void*)
> > _GLIBCXX_USE_NOEXCEPT { }
> > } // extern "C++"
> >
> > #if __cplusplus >= 201703L
> > -#ifdef _GLIBCXX_HAVE_BUILTIN_LAUNDER
> > namespace std
> > {
> > +#ifdef _GLIBCXX_HAVE_BUILTIN_LAUNDER
> > #define __cpp_lib_launder 201606
> >/// Pointer optimization barrier [ptr.launder]
> >template
> > @@ -205,8 +205,14 @@ namespace std
> >void launder(const void*) = delete;
> >void launder(volatile void*) = delete;
> >void launder(const volatile void*) = delete;
> > -}
> > #endif // _GLIBCXX_HAVE_BUILTIN_LAUNDER
> > +
> > +#ifdef __GCC_DESTRUCTIVE_SIZE
> > +# define __cpp_lib_hardware_interference_size 201703L
> > + inline constexpr size_t hardware_destructive_interference_size =
> > __GCC_DESTRUCTIVE_SIZE;
> > + inline constexpr size_t hardware_constructive_interference_size =
> > __GCC_CONSTRUCTIVE_SIZE;
> > +#endif // __GCC_DESTRUCTIVE_SIZE
> > +}
> > #endif // C++17
> >
> > #if __cplusplus > 201703L
Putting aside my dislike of the entire feature, the libstdc++ parts
are fine, thanks.
Re: [PATCH] c++: implement C++17 hardware interference size
On Fri, 16 Jul 2021 at 03:51, Noah Goldstein wrote: > On intel x86 systems with a private L2 cache the spatial prefetcher > can cause destructive interference along 128 byte aligned boundaries. > https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-optimization-manual.pdf#page=60 Which is a good example of why these "constants" should never have been standardized in the first place. Sigh.
Re: [PATCH] c++: implement C++17 hardware interference size
On Thu, Jul 15, 2021 at 10:41 PM Jason Merrill via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:
> Adding CCs that got lost in the initial mail.
>
> On Thu, Jul 15, 2021 at 10:36 PM Jason Merrill wrote:
>
> > The last missing piece of the C++17 standard library is the hardware
> > intereference size constants. Much of the delay in implementing these
> has
> > been due to uncertainty about what the right values are, and even whether
> > there is a single constant value that is suitable; the destructive
> > interference size is intended to be used in structure layout, so program
> > ABIs will depend on it.
> >
> > In principle, both of these values should be the same as the target's L1
> > cache line size. When compiling for a generic target that is intended to
> > support a range of target CPUs with different cache line sizes, the
> > constructive size should probably be the minimum size, and the
> destructive
> > size the maximum, unless you are constrained by ABI compatibility with
> > previous code.
> >
> > JF Bastien's implementation proposal is summarized at
> > https://github.com/itanium-cxx-abi/cxx-abi/issues/74
> >
> > I implement this by adding new --params for the two sizes. Targets need
> to
> > override these values in targetm.target_option.override() to support the
> > feature.
> >
> > 64 bytes still seems correct for the x86 family.
> >
> > I'm not sure why he said 64/64 for 32-bit ARM, since the Cortex A9 has a
> > 32-byte cache line, and that seems to be the only ARM_PREFETCH_BENEFICIAL
> > target, so I'd think 32/64 would make more sense.
> >
> > He proposed 64/128 for AArch64, but since the A64FX now has a 256B cache
> > line, I've changed that to 64/256. Does that seem right?
> >
> > Currently the patch does not adjust the values based on -march, as in
> JF's
> > proposal. I'll need more guidance from the ARM/AArch64 maintainers about
> > how to go about that. --param l1-cache-line-size is set based on -mtune,
> > but I don't think we want -mtune to change these ABI-affecting values.
> Are
> > there -march values for which a smaller range than 64-256 makes sense?
> >
> > gcc/ChangeLog:
> >
> > * params.opt: Add destructive-interference-size and
> > constructive-interference-size.
> > * doc/invoke.texi: Document them.
> > * config/aarch64/aarch64.c (aarch64_override_options_internal):
> > Set them.
> > * config/arm/arm.c (arm_option_override): Set them.
> > * config/i386/i386-options.c (ix86_option_override_internal):
> > Set them.
> >
> > gcc/c-family/ChangeLog:
> >
> > * c.opt: Add -Winterference-size.
> > * c-cppbuiltin.c (cpp_atomic_builtins): Add
> __GCC_DESTRUCTIVE_SIZE
> > and __GCC_CONSTRUCTIVE_SIZE.
> >
> > gcc/cp/ChangeLog:
> >
> > * decl.c (cxx_init_decl_processing): Check
> > --param *-interference-size values.
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/std/version: Define
> __cpp_lib_hardware_interference_size.
> > * libsupc++/new: Define hardware interference size variables.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.target/aarch64/interference.C: New test.
> > * g++.target/arm/interference.C: New test.
> > * g++.target/i386/interference.C: New test.
> > ---
> > gcc/doc/invoke.texi | 22 ++
> > gcc/c-family/c.opt| 5
> > gcc/params.opt| 15
> > gcc/c-family/c-cppbuiltin.c | 12 ++
> > gcc/config/aarch64/aarch64.c | 9
> > gcc/config/arm/arm.c | 6 +
> > gcc/config/i386/i386-options.c| 6 +
> > gcc/cp/decl.c | 23 +++
> > .../g++.target/aarch64/interference.C | 9
> > gcc/testsuite/g++.target/arm/interference.C | 9
> > gcc/testsuite/g++.target/i386/interference.C | 8 +++
> > libstdc++-v3/include/std/version | 3 +++
> > libstdc++-v3/libsupc++/new| 10 ++--
> > 13 files changed, 135 insertions(+), 2 deletions(-)
> > create mode 100644 gcc/testsuite/g++.target/aarch64/interference.C
> > create mode 100644 gcc/testsuite/g++.target/arm/interference.C
> > create mode 100644 gcc/testsuite/g++.target/i386/interference.C
> >
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index ea8812425e9..f93cb7a20f7 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -13857,6 +13857,28 @@ prefetch hints can be issued for any constant
> > stride.
> >
> > This setting is only useful for strides that are known and constant.
> >
> > +@item destructive_interference_size
> > +@item constructive_interference_size
> > +The values for the C++17 variables
> > +@code{std::hardware_destructive_interference_size} and
> >
Re: [PATCH] c++: implement C++17 hardware interference size
Adding CCs that got lost in the initial mail.
On Thu, Jul 15, 2021 at 10:36 PM Jason Merrill wrote:
> The last missing piece of the C++17 standard library is the hardware
> intereference size constants. Much of the delay in implementing these has
> been due to uncertainty about what the right values are, and even whether
> there is a single constant value that is suitable; the destructive
> interference size is intended to be used in structure layout, so program
> ABIs will depend on it.
>
> In principle, both of these values should be the same as the target's L1
> cache line size. When compiling for a generic target that is intended to
> support a range of target CPUs with different cache line sizes, the
> constructive size should probably be the minimum size, and the destructive
> size the maximum, unless you are constrained by ABI compatibility with
> previous code.
>
> JF Bastien's implementation proposal is summarized at
> https://github.com/itanium-cxx-abi/cxx-abi/issues/74
>
> I implement this by adding new --params for the two sizes. Targets need to
> override these values in targetm.target_option.override() to support the
> feature.
>
> 64 bytes still seems correct for the x86 family.
>
> I'm not sure why he said 64/64 for 32-bit ARM, since the Cortex A9 has a
> 32-byte cache line, and that seems to be the only ARM_PREFETCH_BENEFICIAL
> target, so I'd think 32/64 would make more sense.
>
> He proposed 64/128 for AArch64, but since the A64FX now has a 256B cache
> line, I've changed that to 64/256. Does that seem right?
>
> Currently the patch does not adjust the values based on -march, as in JF's
> proposal. I'll need more guidance from the ARM/AArch64 maintainers about
> how to go about that. --param l1-cache-line-size is set based on -mtune,
> but I don't think we want -mtune to change these ABI-affecting values. Are
> there -march values for which a smaller range than 64-256 makes sense?
>
> gcc/ChangeLog:
>
> * params.opt: Add destructive-interference-size and
> constructive-interference-size.
> * doc/invoke.texi: Document them.
> * config/aarch64/aarch64.c (aarch64_override_options_internal):
> Set them.
> * config/arm/arm.c (arm_option_override): Set them.
> * config/i386/i386-options.c (ix86_option_override_internal):
> Set them.
>
> gcc/c-family/ChangeLog:
>
> * c.opt: Add -Winterference-size.
> * c-cppbuiltin.c (cpp_atomic_builtins): Add __GCC_DESTRUCTIVE_SIZE
> and __GCC_CONSTRUCTIVE_SIZE.
>
> gcc/cp/ChangeLog:
>
> * decl.c (cxx_init_decl_processing): Check
> --param *-interference-size values.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/version: Define __cpp_lib_hardware_interference_size.
> * libsupc++/new: Define hardware interference size variables.
>
> gcc/testsuite/ChangeLog:
>
> * g++.target/aarch64/interference.C: New test.
> * g++.target/arm/interference.C: New test.
> * g++.target/i386/interference.C: New test.
> ---
> gcc/doc/invoke.texi | 22 ++
> gcc/c-family/c.opt| 5
> gcc/params.opt| 15
> gcc/c-family/c-cppbuiltin.c | 12 ++
> gcc/config/aarch64/aarch64.c | 9
> gcc/config/arm/arm.c | 6 +
> gcc/config/i386/i386-options.c| 6 +
> gcc/cp/decl.c | 23 +++
> .../g++.target/aarch64/interference.C | 9
> gcc/testsuite/g++.target/arm/interference.C | 9
> gcc/testsuite/g++.target/i386/interference.C | 8 +++
> libstdc++-v3/include/std/version | 3 +++
> libstdc++-v3/libsupc++/new| 10 ++--
> 13 files changed, 135 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.target/aarch64/interference.C
> create mode 100644 gcc/testsuite/g++.target/arm/interference.C
> create mode 100644 gcc/testsuite/g++.target/i386/interference.C
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index ea8812425e9..f93cb7a20f7 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -13857,6 +13857,28 @@ prefetch hints can be issued for any constant
> stride.
>
> This setting is only useful for strides that are known and constant.
>
> +@item destructive_interference_size
> +@item constructive_interference_size
> +The values for the C++17 variables
> +@code{std::hardware_destructive_interference_size} and
> +@code{std::hardware_constructive_interference_size}. The destructive
> +interference size is the minimum recommended offset between two
> +independent concurrently-accessed objects; the constructive
> +interference size is the maximum recommended size of contiguous memory
> +accessed together. Typically both will be the size of an L1 cache
> +line for the
[PATCH] c++: implement C++17 hardware interference size
The last missing piece of the C++17 standard library is the hardware
intereference size constants. Much of the delay in implementing these has
been due to uncertainty about what the right values are, and even whether
there is a single constant value that is suitable; the destructive
interference size is intended to be used in structure layout, so program
ABIs will depend on it.
In principle, both of these values should be the same as the target's L1
cache line size. When compiling for a generic target that is intended to
support a range of target CPUs with different cache line sizes, the
constructive size should probably be the minimum size, and the destructive
size the maximum, unless you are constrained by ABI compatibility with
previous code.
JF Bastien's implementation proposal is summarized at
https://github.com/itanium-cxx-abi/cxx-abi/issues/74
I implement this by adding new --params for the two sizes. Targets need to
override these values in targetm.target_option.override() to support the
feature.
64 bytes still seems correct for the x86 family.
I'm not sure why he said 64/64 for 32-bit ARM, since the Cortex A9 has a
32-byte cache line, and that seems to be the only ARM_PREFETCH_BENEFICIAL
target, so I'd think 32/64 would make more sense.
He proposed 64/128 for AArch64, but since the A64FX now has a 256B cache
line, I've changed that to 64/256. Does that seem right?
Currently the patch does not adjust the values based on -march, as in JF's
proposal. I'll need more guidance from the ARM/AArch64 maintainers about
how to go about that. --param l1-cache-line-size is set based on -mtune,
but I don't think we want -mtune to change these ABI-affecting values. Are
there -march values for which a smaller range than 64-256 makes sense?
gcc/ChangeLog:
* params.opt: Add destructive-interference-size and
constructive-interference-size.
* doc/invoke.texi: Document them.
* config/aarch64/aarch64.c (aarch64_override_options_internal):
Set them.
* config/arm/arm.c (arm_option_override): Set them.
* config/i386/i386-options.c (ix86_option_override_internal):
Set them.
gcc/c-family/ChangeLog:
* c.opt: Add -Winterference-size.
* c-cppbuiltin.c (cpp_atomic_builtins): Add __GCC_DESTRUCTIVE_SIZE
and __GCC_CONSTRUCTIVE_SIZE.
gcc/cp/ChangeLog:
* decl.c (cxx_init_decl_processing): Check
--param *-interference-size values.
libstdc++-v3/ChangeLog:
* include/std/version: Define __cpp_lib_hardware_interference_size.
* libsupc++/new: Define hardware interference size variables.
gcc/testsuite/ChangeLog:
* g++.target/aarch64/interference.C: New test.
* g++.target/arm/interference.C: New test.
* g++.target/i386/interference.C: New test.
---
gcc/doc/invoke.texi | 22 ++
gcc/c-family/c.opt| 5
gcc/params.opt| 15
gcc/c-family/c-cppbuiltin.c | 12 ++
gcc/config/aarch64/aarch64.c | 9
gcc/config/arm/arm.c | 6 +
gcc/config/i386/i386-options.c| 6 +
gcc/cp/decl.c | 23 +++
.../g++.target/aarch64/interference.C | 9
gcc/testsuite/g++.target/arm/interference.C | 9
gcc/testsuite/g++.target/i386/interference.C | 8 +++
libstdc++-v3/include/std/version | 3 +++
libstdc++-v3/libsupc++/new| 10 ++--
13 files changed, 135 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.target/aarch64/interference.C
create mode 100644 gcc/testsuite/g++.target/arm/interference.C
create mode 100644 gcc/testsuite/g++.target/i386/interference.C
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index ea8812425e9..f93cb7a20f7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -13857,6 +13857,28 @@ prefetch hints can be issued for any constant stride.
This setting is only useful for strides that are known and constant.
+@item destructive_interference_size
+@item constructive_interference_size
+The values for the C++17 variables
+@code{std::hardware_destructive_interference_size} and
+@code{std::hardware_constructive_interference_size}. The destructive
+interference size is the minimum recommended offset between two
+independent concurrently-accessed objects; the constructive
+interference size is the maximum recommended size of contiguous memory
+accessed together. Typically both will be the size of an L1 cache
+line for the target, in bytes. If the target can have a range of L1
+cache line sizes, typically the constructive interference size will be
+the small end of the range and the destructive size will be the large
+end.
+
+These values, particularly the destructive size, are intended to be
+used for layout, and
