| Message ID | 20210910131625.159525-1-jason@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] c++: implement C++17 hardware interference size | expand |
On 10/09/2021 15:16, Jason Merrill via Gcc-patches wrote: > OK, time to finish this up. The main change relative to the last patch I sent > to the list is dropping the -finterference-tune flag and making that behavior > the default. Any more comments? > > ==== > > 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. > > From discussion on gcc-patches, I've come to the conclusion that the > solution to the difficulty of choosing stable values is to give up on it, > and instead encourage only uses where ABI stability is unimportant: in > particular, uses where the ABI is shared at most between translation units > built at the same time with the same flags. > > To that end, I've added a warning for any use of the constant value of > std::hardware_destructive_interference_size in a header or module export. > Appropriate uses within a project can disable the warning. > > A previous iteration of this patch included an -finterference-tune flag to > make the value vary with -mtune; this iteration makes that the default > behavior, which should be appropriate for all reasonable uses of the > variable. The previous default of "stable-ish" seems to me likely to have > been more of an attractive nuisance; since we can't promise actual > stability, we should instead make proper uses more convenient. > > 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 can > override these values in targetm.target_option.override() to support a range > of values for the generic target; otherwise, both will default to the L1 > cache line size. > > 64 bytes still seems correct for all x86. > > I'm not sure why he proposed 64/64 for generic 32-bit ARM, since the Cortex > A9 has a 32-byte cache line, so I'd think 32/64 would make more sense. While this works for an arm-linux-gnueabihf toolchain configured --with-mode=arm, it fails --with-mode=thumb (also using --with-cpu=cortex-a9): <built-in>: error: '--param constructive-interference-size=64' is greater than '--param l1-cache-line-size=32' [-Werror=interference-size] cc1plus: all warnings being treated as errors make[4]: *** [Makefile:678: alloc_c.lo] Error 1 > > He proposed 64/128 for generic AArch64, but since the A64FX now has a 256B > cache line, I've changed that to 64/256. Similarly, for aarch64 I'm seeing: <built-in>: error: '--param constructive-interference-size=64' is greater than '--param l1-cache-line-size=32' [-Werror=interference-size] cc1plus: all warnings being treated as errors make[4]: *** [Makefile:678: alloc_c.lo] Error 1 So adjustment is needed for both arm and aarch64 targets Christophe > > With the above choice to reject stability as a goal, getting these values > "right" is now just a matter of what we want the default optimization to be, > and we can feel free to adjust them as CPUs with different cache lines > become more and less common. > > 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: > > * constexpr.c (maybe_warn_about_constant_value): > Complain about std::hardware_destructive_interference_size. > (cxx_eval_constant_expression): Call it. > * 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++.dg/warn/Winterference.h: New file. > * g++.dg/warn/Winterference.C: New test. > * 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 | 65 +++++++++++++++++++ > gcc/c-family/c.opt | 5 ++ > gcc/params.opt | 16 +++++ > gcc/c-family/c-cppbuiltin.c | 12 ++++ > gcc/config/aarch64/aarch64.c | 22 +++++++ > gcc/config/arm/arm.c | 22 +++++++ > gcc/config/i386/i386-options.c | 6 ++ > gcc/cp/constexpr.c | 33 ++++++++++ > gcc/cp/decl.c | 32 +++++++++ > gcc/testsuite/g++.dg/warn/Winterference-2.C | 14 ++++ > gcc/testsuite/g++.dg/warn/Winterference.C | 6 ++ > .../g++.target/aarch64/interference.C | 9 +++ > gcc/testsuite/g++.target/arm/interference.C | 9 +++ > gcc/testsuite/g++.target/i386/interference.C | 8 +++ > gcc/testsuite/g++.dg/warn/Winterference.H | 7 ++ > libstdc++-v3/include/std/version | 3 + > libstdc++-v3/libsupc++/new | 10 ++- > 17 files changed, 277 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/warn/Winterference-2.C > create mode 100644 gcc/testsuite/g++.dg/warn/Winterference.C > 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 > create mode 100644 gcc/testsuite/g++.dg/warn/Winterference.H > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index d4b3a66ee4f..f49d82aa508 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -9017,6 +9017,43 @@ that has already been done in the current function. Therefore, > seemingly insignificant changes in the source program can cause the > warnings produced by @option{-Winline} to appear or disappear. > > +@item -Winterference-size > +@opindex Winterference-size > +Warn about use of C++17 @code{std::hardware_destructive_interference_size} > +without specifying its value with @option{--param destructive-interference-size}. > +Also warn about questionable values for that option. > + > +This variable is intended to be used for controlling class layout, to > +avoid false sharing in concurrent code: > + > +@smallexample > +struct independent_fields @{ > + alignas(std::hardware_destructive_interference_size) std::atomic<int> one; > + alignas(std::hardware_destructive_interference_size) std::atomic<int> two; > +@}; > +@end smallexample > + > +Here @samp{one} and @samp{two} are intended to be far enough apart > +that stores to one won't require accesses to the other to reload the > +cache line. > + > +By default, @option{--param destructive-interference-size} and > +@option{--param constructive-interference-size} are set based on the > +current @option{-mtune} option, typically to the L1 cache line size > +for the particular target CPU, sometimes to a range if tuning for a > +generic target. So all translation units that depend on ABI > +compatibility for the use of these variables must be compiled with > +the same @option{-mtune} (or @option{-mcpu}). > + > +If ABI stability is important, such as if the use is in a header for a > +library, you should probably not use the hardware interference size > +variables at all. Alternatively, you can force a particular value > +with @option{--param}. > + > +If you are confident that your use of the variable does not affect ABI > +outside a single build of your project, you can turn off the warning > +with @option{-Wno-interference-size}. > + > @item -Wint-in-bool-context > @opindex Wint-in-bool-context > @opindex Wno-int-in-bool-context > @@ -13902,6 +13939,34 @@ 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. For a generic target covering 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. > + > +The destructive interference size is intended to be used for layout, > +and thus has ABI impact. The default value is not expected to be > +stable, and on some targets varies with @option{-mtune}, so use of > +this variable in a context where ABI stability is important, such as > +the public interface of a library, is strongly discouraged; if it is > +used in that context, users can stabilize the value using this > +option. > + > +The constructive interference size is less sensitive, as it is > +typically only used in a @samp{static_assert} to make sure that a type > +fits within a cache line. > + > +See also @option{-Winterference-size}. > + > @item loop-interchange-max-num-stmts > The maximum number of stmts in a loop to be interchanged. > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > index c5fe90003f2..9c151d19870 100644 > --- a/gcc/c-family/c.opt > +++ b/gcc/c-family/c.opt > @@ -722,6 +722,11 @@ Winit-list-lifetime > C++ ObjC++ Var(warn_init_list) Warning Init(1) > Warn about uses of std::initializer_list that can result in dangling pointers. > > +Winterference-size > +C++ ObjC++ Var(warn_interference_size) Warning Init(1) > +Warn about nonsensical values of --param destructive-interference-size or > +constructive-interference-size. > + > Wimplicit > C ObjC Var(warn_implicit) Warning LangEnabledBy(C ObjC,Wall) > Warn about implicit declarations. > diff --git a/gcc/params.opt b/gcc/params.opt > index 3a701e22c46..658ca028851 100644 > --- a/gcc/params.opt > +++ b/gcc/params.opt > @@ -361,6 +361,22 @@ The maximum code size growth ratio when expanding into a jump table (in percent) > Common Joined UInteger Var(param_l1_cache_line_size) Init(32) Param Optimization > The size of L1 cache line. > > +-param=destructive-interference-size= > +Common Joined UInteger Var(param_destruct_interfere_size) Init(0) Param Optimization > +The minimum recommended offset between two concurrently-accessed objects to > +avoid additional performance degradation due to contention introduced by the > +implementation. Typically the L1 cache line size, but can be larger to > +accommodate a variety of target processors with different cache line sizes. > +C++17 code might use this value in structure layout, but is strongly > +discouraged from doing so in public ABIs. > + > +-param=constructive-interference-size= > +Common Joined UInteger Var(param_construct_interfere_size) Init(0) Param Optimization > +The maximum recommended size of contiguous memory occupied by two objects > +accessed with temporal locality by concurrent threads. Typically the L1 cache > +line size, but can be smaller to accommodate a variety of target processors with > +different cache line sizes. > + > -param=l1-cache-size= > Common Joined UInteger Var(param_l1_cache_size) Init(64) Param Optimization > The size of L1 cache. > diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c > index 48cbefd8bf8..020e2ea4f25 100644 > --- a/gcc/c-family/c-cppbuiltin.c > +++ b/gcc/c-family/c-cppbuiltin.c > @@ -741,6 +741,18 @@ cpp_atomic_builtins (cpp_reader *pfile) > builtin_define_with_int_value ("__GCC_ATOMIC_TEST_AND_SET_TRUEVAL", > targetm.atomic_test_and_set_trueval); > > + /* Macros for C++17 hardware interference size constants. Either both or > + neither should be set. */ > + gcc_assert (!param_destruct_interfere_size > + == !param_construct_interfere_size); > + if (param_destruct_interfere_size) > + { > + builtin_define_with_int_value ("__GCC_DESTRUCTIVE_SIZE", > + param_destruct_interfere_size); > + builtin_define_with_int_value ("__GCC_CONSTRUCTIVE_SIZE", > + param_construct_interfere_size); > + } > + > /* ptr_type_node can't be used here since ptr_mode is only set when > toplev calls backend_init which is not done with -E or pch. */ > psize = POINTER_SIZE_UNITS; > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 1fbe9e0daa0..05d42a66c7e 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -16539,6 +16539,28 @@ aarch64_override_options_internal (struct gcc_options *opts) > SET_OPTION_IF_UNSET (opts, &global_options_set, > param_l1_cache_line_size, > aarch64_tune_params.prefetch->l1_cache_line_size); > + > + if (aarch64_tune_params.prefetch->l1_cache_line_size >= 0) > + { > + SET_OPTION_IF_UNSET (opts, &global_options_set, > + param_destruct_interfere_size, > + aarch64_tune_params.prefetch->l1_cache_line_size); > + SET_OPTION_IF_UNSET (opts, &global_options_set, > + param_construct_interfere_size, > + aarch64_tune_params.prefetch->l1_cache_line_size); > + } > + else > + { > + /* For a generic AArch64 target, cover the current range of cache line > + sizes. */ > + SET_OPTION_IF_UNSET (opts, &global_options_set, > + param_destruct_interfere_size, > + 256); > + SET_OPTION_IF_UNSET (opts, &global_options_set, > + param_construct_interfere_size, > + 64); > + } > + > if (aarch64_tune_params.prefetch->l2_cache_size >= 0) > SET_OPTION_IF_UNSET (opts, &global_options_set, > param_l2_cache_size, > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index f1e628253d0..6c6e77fab66 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -3669,6 +3669,28 @@ arm_option_override (void) > SET_OPTION_IF_UNSET (&global_options, &global_options_set, > param_l1_cache_line_size, > current_tune->prefetch.l1_cache_line_size); > + if (current_tune->prefetch.l1_cache_line_size >= 0) > + { > + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > + param_destruct_interfere_size, > + current_tune->prefetch.l1_cache_line_size); > + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > + param_construct_interfere_size, > + current_tune->prefetch.l1_cache_line_size); > + } > + else > + { > + /* For a generic ARM target, JF Bastien proposed using 64 for both. */ > + /* ??? Cortex A9 has a 32-byte cache line, so why not 32 for > + constructive? */ > + /* More recent Cortex chips have a 64-byte cache line, but are marked > + ARM_PREFETCH_NOT_BENEFICIAL, so they get these defaults. */ > + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > + param_destruct_interfere_size, 64); > + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > + param_construct_interfere_size, 64); > + } > + > if (current_tune->prefetch.l1_cache_size >= 0) > SET_OPTION_IF_UNSET (&global_options, &global_options_set, > param_l1_cache_size, > diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c > index 2cb87cedec0..c0006b3674b 100644 > --- a/gcc/config/i386/i386-options.c > +++ b/gcc/config/i386/i386-options.c > @@ -2579,6 +2579,12 @@ ix86_option_override_internal (bool main_args_p, > SET_OPTION_IF_UNSET (opts, opts_set, param_l2_cache_size, > ix86_tune_cost->l2_cache_size); > > + /* 64B is the accepted value for these for all x86. */ > + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > + param_destruct_interfere_size, 64); > + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > + param_construct_interfere_size, 64); > + > /* Enable sw prefetching at -O3 for CPUS that prefetching is helpful. */ > if (opts->x_flag_prefetch_loop_arrays < 0 > && HAVE_prefetch > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index 7772fe62d95..0c2498aee22 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -6075,6 +6075,37 @@ inline_asm_in_constexpr_error (location_t loc) > "%<constexpr%> function in C++20"); > } > > +/* We're getting the constant value of DECL in a manifestly constant-evaluated > + context; maybe complain about that. */ > + > +static void > +maybe_warn_about_constant_value (location_t loc, tree decl) > +{ > + static bool explained = false; > + if (cxx_dialect >= cxx17 > + && warn_interference_size > + && !global_options_set.x_param_destruct_interfere_size > + && DECL_CONTEXT (decl) == std_node > + && id_equal (DECL_NAME (decl), "hardware_destructive_interference_size") > + && (LOCATION_FILE (input_location) != main_input_filename > + || module_exporting_p ()) > + && warning_at (loc, OPT_Winterference_size, "use of %qD", decl) > + && !explained) > + { > + explained = true; > + inform (loc, "its value can vary between compiler versions or " > + "with different %<-mtune%> or %<-mcpu%> flags"); > + inform (loc, "if this use is part of a public ABI, change it to " > + "instead use a constant variable you define"); > + inform (loc, "the default value for the current CPU tuning " > + "is %d bytes", param_destruct_interfere_size); > + inform (loc, "you can stabilize this value with %<--param " > + "hardware_destructive_interference_size=%d%>, or disable " > + "this warning with %<-Wno-interference-size%>", > + param_destruct_interfere_size); > + } > +} > + > /* Attempt to reduce the expression T to a constant value. > On failure, issue diagnostic and return error_mark_node. */ > /* FIXME unify with c_fully_fold */ > @@ -6219,6 +6250,8 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, > r = *p; > break; > } > + if (ctx->manifestly_const_eval) > + maybe_warn_about_constant_value (loc, t); > if (COMPLETE_TYPE_P (TREE_TYPE (t)) > && is_really_empty_class (TREE_TYPE (t), /*ignore_vptr*/false)) > { > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > index bce62ad202a..c2065027369 100644 > --- a/gcc/cp/decl.c > +++ b/gcc/cp/decl.c > @@ -4752,6 +4752,38 @@ cxx_init_decl_processing (void) > /* Show we use EH for cleanups. */ > if (flag_exceptions) > using_eh_for_cleanups (); > + > + /* Check that the hardware interference sizes are at least > + alignof(max_align_t), as required by the standard. */ > + const int max_align = max_align_t_align () / BITS_PER_UNIT; > + if (param_destruct_interfere_size) > + { > + if (param_destruct_interfere_size < max_align) > + error ("%<--param destructive-interference-size=%d%> is less than " > + "%d", param_destruct_interfere_size, max_align); > + else if (param_destruct_interfere_size < param_l1_cache_line_size) > + warning (OPT_Winterference_size, > + "%<--param destructive-interference-size=%d%> " > + "is less than %<--param l1-cache-line-size=%d%>", > + param_destruct_interfere_size, param_l1_cache_line_size); > + } > + else if (param_l1_cache_line_size >= max_align) > + param_destruct_interfere_size = param_l1_cache_line_size; > + /* else leave it unset. */ > + > + if (param_construct_interfere_size) > + { > + if (param_construct_interfere_size < max_align) > + error ("%<--param constructive-interference-size=%d%> is less than " > + "%d", param_construct_interfere_size, max_align); > + else if (param_construct_interfere_size > param_l1_cache_line_size) > + warning (OPT_Winterference_size, > + "%<--param constructive-interference-size=%d%> " > + "is greater than %<--param l1-cache-line-size=%d%>", > + param_construct_interfere_size, param_l1_cache_line_size); > + } > + else if (param_l1_cache_line_size >= max_align) > + param_construct_interfere_size = param_l1_cache_line_size; > } > > /* Enter an abi node in global-module context. returns a cookie to > diff --git a/gcc/testsuite/g++.dg/warn/Winterference-2.C b/gcc/testsuite/g++.dg/warn/Winterference-2.C > new file mode 100644 > index 00000000000..2af75c63f83 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Winterference-2.C > @@ -0,0 +1,14 @@ > +// { dg-do compile { target c++20 } } > +// { dg-additional-options -fmodules-ts } > + > +module ; > + > +#include <new> > + > +export module foo; > + > +export { > + struct A { > + alignas(std::hardware_destructive_interference_size) int x; // { dg-warning Winterference-size } > + }; > +} > diff --git a/gcc/testsuite/g++.dg/warn/Winterference.C b/gcc/testsuite/g++.dg/warn/Winterference.C > new file mode 100644 > index 00000000000..57c001bc032 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Winterference.C > @@ -0,0 +1,6 @@ > +// Test that we warn about use of std::hardware_destructive_interference_size > +// in a header. > +// { dg-do compile { target c++17 } } > + > +// { dg-warning Winterference-size "" { target *-*-* } 0 } > +#include "Winterference.H" > diff --git a/gcc/testsuite/g++.target/aarch64/interference.C b/gcc/testsuite/g++.target/aarch64/interference.C > new file mode 100644 > index 00000000000..0fc01655223 > --- /dev/null > +++ b/gcc/testsuite/g++.target/aarch64/interference.C > @@ -0,0 +1,9 @@ > +// Test C++17 hardware interference size constants > +// { dg-do compile { target c++17 } } > + > +#include <new> > + > +// Most AArch64 CPUs have an L1 cache line size of 64, but some recent ones use > +// 128 or even 256. > +static_assert(std::hardware_destructive_interference_size == 256); > +static_assert(std::hardware_constructive_interference_size == 64); > diff --git a/gcc/testsuite/g++.target/arm/interference.C b/gcc/testsuite/g++.target/arm/interference.C > new file mode 100644 > index 00000000000..34fe8a52bff > --- /dev/null > +++ b/gcc/testsuite/g++.target/arm/interference.C > @@ -0,0 +1,9 @@ > +// Test C++17 hardware interference size constants > +// { dg-do compile { target c++17 } } > + > +#include <new> > + > +// Recent ARM CPUs have a cache line size of 64. Older ones have > +// a size of 32, but I guess they're old enough that we don't care? > +static_assert(std::hardware_destructive_interference_size == 64); > +static_assert(std::hardware_constructive_interference_size == 64); > diff --git a/gcc/testsuite/g++.target/i386/interference.C b/gcc/testsuite/g++.target/i386/interference.C > new file mode 100644 > index 00000000000..c7b910e3ada > --- /dev/null > +++ b/gcc/testsuite/g++.target/i386/interference.C > @@ -0,0 +1,8 @@ > +// Test C++17 hardware interference size constants > +// { dg-do compile { target c++17 } } > + > +#include <new> > + > +// It is generally agreed that these are the right values for all x86. > +static_assert(std::hardware_destructive_interference_size == 64); > +static_assert(std::hardware_constructive_interference_size == 64); > diff --git a/gcc/testsuite/g++.dg/warn/Winterference.H b/gcc/testsuite/g++.dg/warn/Winterference.H > new file mode 100644 > index 00000000000..36f0ad5f6d1 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Winterference.H > @@ -0,0 +1,7 @@ > +#include <new> > + > +struct A > +{ > + alignas(std::hardware_destructive_interference_size) int i; > + alignas(std::hardware_destructive_interference_size) int j; > +}; > diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version > index f950bf0f0db..f41004b5911 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<typename _Tp> > @@ -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 > > base-commit: de515ce0b209cc7e5a780d9846e5154d380a763e
On 14/09/2021 08:56, Christophe LYON via Gcc-patches wrote: > > On 10/09/2021 15:16, Jason Merrill via Gcc-patches wrote: >> OK, time to finish this up. The main change relative to the last >> patch I sent >> to the list is dropping the -finterference-tune flag and making that >> behavior >> the default. Any more comments? >> >> ==== >> >> 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. >> >> From discussion on gcc-patches, I've come to the conclusion that the >> solution to the difficulty of choosing stable values is to give up on it, >> and instead encourage only uses where ABI stability is unimportant: in >> particular, uses where the ABI is shared at most between translation >> units >> built at the same time with the same flags. >> >> To that end, I've added a warning for any use of the constant value of >> std::hardware_destructive_interference_size in a header or module export. >> Appropriate uses within a project can disable the warning. >> >> A previous iteration of this patch included an -finterference-tune >> flag to >> make the value vary with -mtune; this iteration makes that the default >> behavior, which should be appropriate for all reasonable uses of the >> variable. The previous default of "stable-ish" seems to me likely to >> have >> been more of an attractive nuisance; since we can't promise actual >> stability, we should instead make proper uses more convenient. >> >> 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 can >> override these values in targetm.target_option.override() to support a >> range >> of values for the generic target; otherwise, both will default to the L1 >> cache line size. >> >> 64 bytes still seems correct for all x86. >> >> I'm not sure why he proposed 64/64 for generic 32-bit ARM, since the >> Cortex >> A9 has a 32-byte cache line, so I'd think 32/64 would make more sense. > > While this works for an arm-linux-gnueabihf toolchain configured > --with-mode=arm, it fails --with-mode=thumb (also using > --with-cpu=cortex-a9): > > <built-in>: error: '--param constructive-interference-size=64' is > greater than '--param l1-cache-line-size=32' [-Werror=interference-size] > cc1plus: all warnings being treated as errors > make[4]: *** [Makefile:678: alloc_c.lo] Error 1 > > > >> >> He proposed 64/128 for generic AArch64, but since the A64FX now has a >> 256B >> cache line, I've changed that to 64/256. > > > Similarly, for aarch64 I'm seeing: > > <built-in>: error: '--param constructive-interference-size=64' is > greater than '--param l1-cache-line-size=32' [-Werror=interference-size] > cc1plus: all warnings being treated as errors > make[4]: *** [Makefile:678: alloc_c.lo] Error 1 > > > So adjustment is needed for both arm and aarch64 targets > > FWIW, I'm still in discussion with our architects about the best values to use here, but certainly this needs fixing quickly as it seems to be breaking hundreds of tests in the C++ testsuite. R. > Christophe > > >> >> With the above choice to reject stability as a goal, getting these values >> "right" is now just a matter of what we want the default optimization >> to be, >> and we can feel free to adjust them as CPUs with different cache lines >> become more and less common. >> >> 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: >> >> * constexpr.c (maybe_warn_about_constant_value): >> Complain about std::hardware_destructive_interference_size. >> (cxx_eval_constant_expression): Call it. >> * 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++.dg/warn/Winterference.h: New file. >> * g++.dg/warn/Winterference.C: New test. >> * 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 | 65 +++++++++++++++++++ >> gcc/c-family/c.opt | 5 ++ >> gcc/params.opt | 16 +++++ >> gcc/c-family/c-cppbuiltin.c | 12 ++++ >> gcc/config/aarch64/aarch64.c | 22 +++++++ >> gcc/config/arm/arm.c | 22 +++++++ >> gcc/config/i386/i386-options.c | 6 ++ >> gcc/cp/constexpr.c | 33 ++++++++++ >> gcc/cp/decl.c | 32 +++++++++ >> gcc/testsuite/g++.dg/warn/Winterference-2.C | 14 ++++ >> gcc/testsuite/g++.dg/warn/Winterference.C | 6 ++ >> .../g++.target/aarch64/interference.C | 9 +++ >> gcc/testsuite/g++.target/arm/interference.C | 9 +++ >> gcc/testsuite/g++.target/i386/interference.C | 8 +++ >> gcc/testsuite/g++.dg/warn/Winterference.H | 7 ++ >> libstdc++-v3/include/std/version | 3 + >> libstdc++-v3/libsupc++/new | 10 ++- >> 17 files changed, 277 insertions(+), 2 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/warn/Winterference-2.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Winterference.C >> 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 >> create mode 100644 gcc/testsuite/g++.dg/warn/Winterference.H >> >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >> index d4b3a66ee4f..f49d82aa508 100644 >> --- a/gcc/doc/invoke.texi >> +++ b/gcc/doc/invoke.texi >> @@ -9017,6 +9017,43 @@ that has already been done in the current >> function. Therefore, >> seemingly insignificant changes in the source program can cause the >> warnings produced by @option{-Winline} to appear or disappear. >> +@item -Winterference-size >> +@opindex Winterference-size >> +Warn about use of C++17 >> @code{std::hardware_destructive_interference_size} >> +without specifying its value with @option{--param >> destructive-interference-size}. >> +Also warn about questionable values for that option. >> + >> +This variable is intended to be used for controlling class layout, to >> +avoid false sharing in concurrent code: >> + >> +@smallexample >> +struct independent_fields @{ >> + alignas(std::hardware_destructive_interference_size) >> std::atomic<int> one; >> + alignas(std::hardware_destructive_interference_size) >> std::atomic<int> two; >> +@}; >> +@end smallexample >> + >> +Here @samp{one} and @samp{two} are intended to be far enough apart >> +that stores to one won't require accesses to the other to reload the >> +cache line. >> + >> +By default, @option{--param destructive-interference-size} and >> +@option{--param constructive-interference-size} are set based on the >> +current @option{-mtune} option, typically to the L1 cache line size >> +for the particular target CPU, sometimes to a range if tuning for a >> +generic target. So all translation units that depend on ABI >> +compatibility for the use of these variables must be compiled with >> +the same @option{-mtune} (or @option{-mcpu}). >> + >> +If ABI stability is important, such as if the use is in a header for a >> +library, you should probably not use the hardware interference size >> +variables at all. Alternatively, you can force a particular value >> +with @option{--param}. >> + >> +If you are confident that your use of the variable does not affect ABI >> +outside a single build of your project, you can turn off the warning >> +with @option{-Wno-interference-size}. >> + >> @item -Wint-in-bool-context >> @opindex Wint-in-bool-context >> @opindex Wno-int-in-bool-context >> @@ -13902,6 +13939,34 @@ 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. For a generic target covering 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. >> + >> +The destructive interference size is intended to be used for layout, >> +and thus has ABI impact. The default value is not expected to be >> +stable, and on some targets varies with @option{-mtune}, so use of >> +this variable in a context where ABI stability is important, such as >> +the public interface of a library, is strongly discouraged; if it is >> +used in that context, users can stabilize the value using this >> +option. >> + >> +The constructive interference size is less sensitive, as it is >> +typically only used in a @samp{static_assert} to make sure that a type >> +fits within a cache line. >> + >> +See also @option{-Winterference-size}. >> + >> @item loop-interchange-max-num-stmts >> The maximum number of stmts in a loop to be interchanged. >> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt >> index c5fe90003f2..9c151d19870 100644 >> --- a/gcc/c-family/c.opt >> +++ b/gcc/c-family/c.opt >> @@ -722,6 +722,11 @@ Winit-list-lifetime >> C++ ObjC++ Var(warn_init_list) Warning Init(1) >> Warn about uses of std::initializer_list that can result in dangling >> pointers. >> +Winterference-size >> +C++ ObjC++ Var(warn_interference_size) Warning Init(1) >> +Warn about nonsensical values of --param >> destructive-interference-size or >> +constructive-interference-size. >> + >> Wimplicit >> C ObjC Var(warn_implicit) Warning LangEnabledBy(C ObjC,Wall) >> Warn about implicit declarations. >> diff --git a/gcc/params.opt b/gcc/params.opt >> index 3a701e22c46..658ca028851 100644 >> --- a/gcc/params.opt >> +++ b/gcc/params.opt >> @@ -361,6 +361,22 @@ The maximum code size growth ratio when expanding >> into a jump table (in percent) >> Common Joined UInteger Var(param_l1_cache_line_size) Init(32) Param >> Optimization >> The size of L1 cache line. >> +-param=destructive-interference-size= >> +Common Joined UInteger Var(param_destruct_interfere_size) Init(0) >> Param Optimization >> +The minimum recommended offset between two concurrently-accessed >> objects to >> +avoid additional performance degradation due to contention introduced >> by the >> +implementation. Typically the L1 cache line size, but can be larger to >> +accommodate a variety of target processors with different cache line >> sizes. >> +C++17 code might use this value in structure layout, but is strongly >> +discouraged from doing so in public ABIs. >> + >> +-param=constructive-interference-size= >> +Common Joined UInteger Var(param_construct_interfere_size) Init(0) >> Param Optimization >> +The maximum recommended size of contiguous memory occupied by two >> objects >> +accessed with temporal locality by concurrent threads. Typically the >> L1 cache >> +line size, but can be smaller to accommodate a variety of target >> processors with >> +different cache line sizes. >> + >> -param=l1-cache-size= >> Common Joined UInteger Var(param_l1_cache_size) Init(64) Param >> Optimization >> The size of L1 cache. >> diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c >> index 48cbefd8bf8..020e2ea4f25 100644 >> --- a/gcc/c-family/c-cppbuiltin.c >> +++ b/gcc/c-family/c-cppbuiltin.c >> @@ -741,6 +741,18 @@ cpp_atomic_builtins (cpp_reader *pfile) >> builtin_define_with_int_value ("__GCC_ATOMIC_TEST_AND_SET_TRUEVAL", >> targetm.atomic_test_and_set_trueval); >> + /* Macros for C++17 hardware interference size constants. Either >> both or >> + neither should be set. */ >> + gcc_assert (!param_destruct_interfere_size >> + == !param_construct_interfere_size); >> + if (param_destruct_interfere_size) >> + { >> + builtin_define_with_int_value ("__GCC_DESTRUCTIVE_SIZE", >> + param_destruct_interfere_size); >> + builtin_define_with_int_value ("__GCC_CONSTRUCTIVE_SIZE", >> + param_construct_interfere_size); >> + } >> + >> /* ptr_type_node can't be used here since ptr_mode is only set when >> toplev calls backend_init which is not done with -E or pch. */ >> psize = POINTER_SIZE_UNITS; >> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >> index 1fbe9e0daa0..05d42a66c7e 100644 >> --- a/gcc/config/aarch64/aarch64.c >> +++ b/gcc/config/aarch64/aarch64.c >> @@ -16539,6 +16539,28 @@ aarch64_override_options_internal (struct >> gcc_options *opts) >> SET_OPTION_IF_UNSET (opts, &global_options_set, >> param_l1_cache_line_size, >> aarch64_tune_params.prefetch->l1_cache_line_size); >> + >> + if (aarch64_tune_params.prefetch->l1_cache_line_size >= 0) >> + { >> + SET_OPTION_IF_UNSET (opts, &global_options_set, >> + param_destruct_interfere_size, >> + aarch64_tune_params.prefetch->l1_cache_line_size); >> + SET_OPTION_IF_UNSET (opts, &global_options_set, >> + param_construct_interfere_size, >> + aarch64_tune_params.prefetch->l1_cache_line_size); >> + } >> + else >> + { >> + /* For a generic AArch64 target, cover the current range of >> cache line >> + sizes. */ >> + SET_OPTION_IF_UNSET (opts, &global_options_set, >> + param_destruct_interfere_size, >> + 256); >> + SET_OPTION_IF_UNSET (opts, &global_options_set, >> + param_construct_interfere_size, >> + 64); >> + } >> + >> if (aarch64_tune_params.prefetch->l2_cache_size >= 0) >> SET_OPTION_IF_UNSET (opts, &global_options_set, >> param_l2_cache_size, >> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >> index f1e628253d0..6c6e77fab66 100644 >> --- a/gcc/config/arm/arm.c >> +++ b/gcc/config/arm/arm.c >> @@ -3669,6 +3669,28 @@ arm_option_override (void) >> SET_OPTION_IF_UNSET (&global_options, &global_options_set, >> param_l1_cache_line_size, >> current_tune->prefetch.l1_cache_line_size); >> + if (current_tune->prefetch.l1_cache_line_size >= 0) >> + { >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, >> + param_destruct_interfere_size, >> + current_tune->prefetch.l1_cache_line_size); >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, >> + param_construct_interfere_size, >> + current_tune->prefetch.l1_cache_line_size); >> + } >> + else >> + { >> + /* For a generic ARM target, JF Bastien proposed using 64 for >> both. */ >> + /* ??? Cortex A9 has a 32-byte cache line, so why not 32 for >> + constructive? */ >> + /* More recent Cortex chips have a 64-byte cache line, but are >> marked >> + ARM_PREFETCH_NOT_BENEFICIAL, so they get these defaults. */ >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, >> + param_destruct_interfere_size, 64); >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, >> + param_construct_interfere_size, 64); >> + } >> + >> if (current_tune->prefetch.l1_cache_size >= 0) >> SET_OPTION_IF_UNSET (&global_options, &global_options_set, >> param_l1_cache_size, >> diff --git a/gcc/config/i386/i386-options.c >> b/gcc/config/i386/i386-options.c >> index 2cb87cedec0..c0006b3674b 100644 >> --- a/gcc/config/i386/i386-options.c >> +++ b/gcc/config/i386/i386-options.c >> @@ -2579,6 +2579,12 @@ ix86_option_override_internal (bool main_args_p, >> SET_OPTION_IF_UNSET (opts, opts_set, param_l2_cache_size, >> ix86_tune_cost->l2_cache_size); >> + /* 64B is the accepted value for these for all x86. */ >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, >> + param_destruct_interfere_size, 64); >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, >> + param_construct_interfere_size, 64); >> + >> /* Enable sw prefetching at -O3 for CPUS that prefetching is >> helpful. */ >> if (opts->x_flag_prefetch_loop_arrays < 0 >> && HAVE_prefetch >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >> index 7772fe62d95..0c2498aee22 100644 >> --- a/gcc/cp/constexpr.c >> +++ b/gcc/cp/constexpr.c >> @@ -6075,6 +6075,37 @@ inline_asm_in_constexpr_error (location_t loc) >> "%<constexpr%> function in C++20"); >> } >> +/* We're getting the constant value of DECL in a manifestly >> constant-evaluated >> + context; maybe complain about that. */ >> + >> +static void >> +maybe_warn_about_constant_value (location_t loc, tree decl) >> +{ >> + static bool explained = false; >> + if (cxx_dialect >= cxx17 >> + && warn_interference_size >> + && !global_options_set.x_param_destruct_interfere_size >> + && DECL_CONTEXT (decl) == std_node >> + && id_equal (DECL_NAME (decl), >> "hardware_destructive_interference_size") >> + && (LOCATION_FILE (input_location) != main_input_filename >> + || module_exporting_p ()) >> + && warning_at (loc, OPT_Winterference_size, "use of %qD", decl) >> + && !explained) >> + { >> + explained = true; >> + inform (loc, "its value can vary between compiler versions or " >> + "with different %<-mtune%> or %<-mcpu%> flags"); >> + inform (loc, "if this use is part of a public ABI, change it to " >> + "instead use a constant variable you define"); >> + inform (loc, "the default value for the current CPU tuning " >> + "is %d bytes", param_destruct_interfere_size); >> + inform (loc, "you can stabilize this value with %<--param " >> + "hardware_destructive_interference_size=%d%>, or disable " >> + "this warning with %<-Wno-interference-size%>", >> + param_destruct_interfere_size); >> + } >> +} >> + >> /* Attempt to reduce the expression T to a constant value. >> On failure, issue diagnostic and return error_mark_node. */ >> /* FIXME unify with c_fully_fold */ >> @@ -6219,6 +6250,8 @@ cxx_eval_constant_expression (const >> constexpr_ctx *ctx, tree t, >> r = *p; >> break; >> } >> + if (ctx->manifestly_const_eval) >> + maybe_warn_about_constant_value (loc, t); >> if (COMPLETE_TYPE_P (TREE_TYPE (t)) >> && is_really_empty_class (TREE_TYPE (t), /*ignore_vptr*/false)) >> { >> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c >> index bce62ad202a..c2065027369 100644 >> --- a/gcc/cp/decl.c >> +++ b/gcc/cp/decl.c >> @@ -4752,6 +4752,38 @@ cxx_init_decl_processing (void) >> /* Show we use EH for cleanups. */ >> if (flag_exceptions) >> using_eh_for_cleanups (); >> + >> + /* Check that the hardware interference sizes are at least >> + alignof(max_align_t), as required by the standard. */ >> + const int max_align = max_align_t_align () / BITS_PER_UNIT; >> + if (param_destruct_interfere_size) >> + { >> + if (param_destruct_interfere_size < max_align) >> + error ("%<--param destructive-interference-size=%d%> is less than " >> + "%d", param_destruct_interfere_size, max_align); >> + else if (param_destruct_interfere_size < param_l1_cache_line_size) >> + warning (OPT_Winterference_size, >> + "%<--param destructive-interference-size=%d%> " >> + "is less than %<--param l1-cache-line-size=%d%>", >> + param_destruct_interfere_size, param_l1_cache_line_size); >> + } >> + else if (param_l1_cache_line_size >= max_align) >> + param_destruct_interfere_size = param_l1_cache_line_size; >> + /* else leave it unset. */ >> + >> + if (param_construct_interfere_size) >> + { >> + if (param_construct_interfere_size < max_align) >> + error ("%<--param constructive-interference-size=%d%> is less than " >> + "%d", param_construct_interfere_size, max_align); >> + else if (param_construct_interfere_size > >> param_l1_cache_line_size) >> + warning (OPT_Winterference_size, >> + "%<--param constructive-interference-size=%d%> " >> + "is greater than %<--param l1-cache-line-size=%d%>", >> + param_construct_interfere_size, param_l1_cache_line_size); >> + } >> + else if (param_l1_cache_line_size >= max_align) >> + param_construct_interfere_size = param_l1_cache_line_size; >> } >> /* Enter an abi node in global-module context. returns a cookie to >> diff --git a/gcc/testsuite/g++.dg/warn/Winterference-2.C >> b/gcc/testsuite/g++.dg/warn/Winterference-2.C >> new file mode 100644 >> index 00000000000..2af75c63f83 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Winterference-2.C >> @@ -0,0 +1,14 @@ >> +// { dg-do compile { target c++20 } } >> +// { dg-additional-options -fmodules-ts } >> + >> +module ; >> + >> +#include <new> >> + >> +export module foo; >> + >> +export { >> + struct A { >> + alignas(std::hardware_destructive_interference_size) int x; // { >> dg-warning Winterference-size } >> + }; >> +} >> diff --git a/gcc/testsuite/g++.dg/warn/Winterference.C >> b/gcc/testsuite/g++.dg/warn/Winterference.C >> new file mode 100644 >> index 00000000000..57c001bc032 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Winterference.C >> @@ -0,0 +1,6 @@ >> +// Test that we warn about use of >> std::hardware_destructive_interference_size >> +// in a header. >> +// { dg-do compile { target c++17 } } >> + >> +// { dg-warning Winterference-size "" { target *-*-* } 0 } >> +#include "Winterference.H" >> diff --git a/gcc/testsuite/g++.target/aarch64/interference.C >> b/gcc/testsuite/g++.target/aarch64/interference.C >> new file mode 100644 >> index 00000000000..0fc01655223 >> --- /dev/null >> +++ b/gcc/testsuite/g++.target/aarch64/interference.C >> @@ -0,0 +1,9 @@ >> +// Test C++17 hardware interference size constants >> +// { dg-do compile { target c++17 } } >> + >> +#include <new> >> + >> +// Most AArch64 CPUs have an L1 cache line size of 64, but some >> recent ones use >> +// 128 or even 256. >> +static_assert(std::hardware_destructive_interference_size == 256); >> +static_assert(std::hardware_constructive_interference_size == 64); >> diff --git a/gcc/testsuite/g++.target/arm/interference.C >> b/gcc/testsuite/g++.target/arm/interference.C >> new file mode 100644 >> index 00000000000..34fe8a52bff >> --- /dev/null >> +++ b/gcc/testsuite/g++.target/arm/interference.C >> @@ -0,0 +1,9 @@ >> +// Test C++17 hardware interference size constants >> +// { dg-do compile { target c++17 } } >> + >> +#include <new> >> + >> +// Recent ARM CPUs have a cache line size of 64. Older ones have >> +// a size of 32, but I guess they're old enough that we don't care? >> +static_assert(std::hardware_destructive_interference_size == 64); >> +static_assert(std::hardware_constructive_interference_size == 64); >> diff --git a/gcc/testsuite/g++.target/i386/interference.C >> b/gcc/testsuite/g++.target/i386/interference.C >> new file mode 100644 >> index 00000000000..c7b910e3ada >> --- /dev/null >> +++ b/gcc/testsuite/g++.target/i386/interference.C >> @@ -0,0 +1,8 @@ >> +// Test C++17 hardware interference size constants >> +// { dg-do compile { target c++17 } } >> + >> +#include <new> >> + >> +// It is generally agreed that these are the right values for all x86. >> +static_assert(std::hardware_destructive_interference_size == 64); >> +static_assert(std::hardware_constructive_interference_size == 64); >> diff --git a/gcc/testsuite/g++.dg/warn/Winterference.H >> b/gcc/testsuite/g++.dg/warn/Winterference.H >> new file mode 100644 >> index 00000000000..36f0ad5f6d1 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Winterference.H >> @@ -0,0 +1,7 @@ >> +#include <new> >> + >> +struct A >> +{ >> + alignas(std::hardware_destructive_interference_size) int i; >> + alignas(std::hardware_destructive_interference_size) int j; >> +}; >> diff --git a/libstdc++-v3/include/std/version >> b/libstdc++-v3/include/std/version >> index f950bf0f0db..f41004b5911 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<typename _Tp> >> @@ -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 >> >> base-commit: de515ce0b209cc7e5a780d9846e5154d380a763e
On Wed, Sep 15, 2021 at 12:25 PM Richard Earnshaw via Gcc-patches < gcc-patches@gcc.gnu.org> wrote: > > > On 14/09/2021 08:56, Christophe LYON via Gcc-patches wrote: > > > > On 10/09/2021 15:16, Jason Merrill via Gcc-patches wrote: > >> OK, time to finish this up. The main change relative to the last > >> patch I sent > >> to the list is dropping the -finterference-tune flag and making that > >> behavior > >> the default. Any more comments? > >> > >> ==== > >> > >> 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. > >> > >> From discussion on gcc-patches, I've come to the conclusion that the > >> solution to the difficulty of choosing stable values is to give up on > it, > >> and instead encourage only uses where ABI stability is unimportant: in > >> particular, uses where the ABI is shared at most between translation > >> units > >> built at the same time with the same flags. > >> > >> To that end, I've added a warning for any use of the constant value of > >> std::hardware_destructive_interference_size in a header or module > export. > >> Appropriate uses within a project can disable the warning. > >> > >> A previous iteration of this patch included an -finterference-tune > >> flag to > >> make the value vary with -mtune; this iteration makes that the default > >> behavior, which should be appropriate for all reasonable uses of the > >> variable. The previous default of "stable-ish" seems to me likely to > >> have > >> been more of an attractive nuisance; since we can't promise actual > >> stability, we should instead make proper uses more convenient. > >> > >> 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 can > >> override these values in targetm.target_option.override() to support a > >> range > >> of values for the generic target; otherwise, both will default to the L1 > >> cache line size. > >> > >> 64 bytes still seems correct for all x86. > >> > >> I'm not sure why he proposed 64/64 for generic 32-bit ARM, since the > >> Cortex > >> A9 has a 32-byte cache line, so I'd think 32/64 would make more sense. > > > > While this works for an arm-linux-gnueabihf toolchain configured > > --with-mode=arm, it fails --with-mode=thumb (also using > > --with-cpu=cortex-a9): > > > > <built-in>: error: '--param constructive-interference-size=64' is > > greater than '--param l1-cache-line-size=32' [-Werror=interference-size] > > cc1plus: all warnings being treated as errors > > make[4]: *** [Makefile:678: alloc_c.lo] Error 1 > > > > > > > >> > >> He proposed 64/128 for generic AArch64, but since the A64FX now has a > >> 256B > >> cache line, I've changed that to 64/256. > > > > > > Similarly, for aarch64 I'm seeing: > > > > <built-in>: error: '--param constructive-interference-size=64' is > > greater than '--param l1-cache-line-size=32' [-Werror=interference-size] > > cc1plus: all warnings being treated as errors > > make[4]: *** [Makefile:678: alloc_c.lo] Error 1 > > > > > > So adjustment is needed for both arm and aarch64 targets > > > > > > FWIW, I'm still in discussion with our architects about the best values > to use here, but certainly this needs fixing quickly as it seems to be > breaking hundreds of tests in the C++ testsuite. > Indeed. A lot of messages to gcc-testresults are blocked because they are too large. Christophe > > R. > > > Christophe > > > > > >> > >> With the above choice to reject stability as a goal, getting these > values > >> "right" is now just a matter of what we want the default optimization > >> to be, > >> and we can feel free to adjust them as CPUs with different cache lines > >> become more and less common. > >> > >> 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: > >> > >> * constexpr.c (maybe_warn_about_constant_value): > >> Complain about std::hardware_destructive_interference_size. > >> (cxx_eval_constant_expression): Call it. > >> * 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++.dg/warn/Winterference.h: New file. > >> * g++.dg/warn/Winterference.C: New test. > >> * 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 | 65 +++++++++++++++++++ > >> gcc/c-family/c.opt | 5 ++ > >> gcc/params.opt | 16 +++++ > >> gcc/c-family/c-cppbuiltin.c | 12 ++++ > >> gcc/config/aarch64/aarch64.c | 22 +++++++ > >> gcc/config/arm/arm.c | 22 +++++++ > >> gcc/config/i386/i386-options.c | 6 ++ > >> gcc/cp/constexpr.c | 33 ++++++++++ > >> gcc/cp/decl.c | 32 +++++++++ > >> gcc/testsuite/g++.dg/warn/Winterference-2.C | 14 ++++ > >> gcc/testsuite/g++.dg/warn/Winterference.C | 6 ++ > >> .../g++.target/aarch64/interference.C | 9 +++ > >> gcc/testsuite/g++.target/arm/interference.C | 9 +++ > >> gcc/testsuite/g++.target/i386/interference.C | 8 +++ > >> gcc/testsuite/g++.dg/warn/Winterference.H | 7 ++ > >> libstdc++-v3/include/std/version | 3 + > >> libstdc++-v3/libsupc++/new | 10 ++- > >> 17 files changed, 277 insertions(+), 2 deletions(-) > >> create mode 100644 gcc/testsuite/g++.dg/warn/Winterference-2.C > >> create mode 100644 gcc/testsuite/g++.dg/warn/Winterference.C > >> 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 > >> create mode 100644 gcc/testsuite/g++.dg/warn/Winterference.H > >> > >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > >> index d4b3a66ee4f..f49d82aa508 100644 > >> --- a/gcc/doc/invoke.texi > >> +++ b/gcc/doc/invoke.texi > >> @@ -9017,6 +9017,43 @@ that has already been done in the current > >> function. Therefore, > >> seemingly insignificant changes in the source program can cause the > >> warnings produced by @option{-Winline} to appear or disappear. > >> +@item -Winterference-size > >> +@opindex Winterference-size > >> +Warn about use of C++17 > >> @code{std::hardware_destructive_interference_size} > >> +without specifying its value with @option{--param > >> destructive-interference-size}. > >> +Also warn about questionable values for that option. > >> + > >> +This variable is intended to be used for controlling class layout, to > >> +avoid false sharing in concurrent code: > >> + > >> +@smallexample > >> +struct independent_fields @{ > >> + alignas(std::hardware_destructive_interference_size) > >> std::atomic<int> one; > >> + alignas(std::hardware_destructive_interference_size) > >> std::atomic<int> two; > >> +@}; > >> +@end smallexample > >> + > >> +Here @samp{one} and @samp{two} are intended to be far enough apart > >> +that stores to one won't require accesses to the other to reload the > >> +cache line. > >> + > >> +By default, @option{--param destructive-interference-size} and > >> +@option{--param constructive-interference-size} are set based on the > >> +current @option{-mtune} option, typically to the L1 cache line size > >> +for the particular target CPU, sometimes to a range if tuning for a > >> +generic target. So all translation units that depend on ABI > >> +compatibility for the use of these variables must be compiled with > >> +the same @option{-mtune} (or @option{-mcpu}). > >> + > >> +If ABI stability is important, such as if the use is in a header for a > >> +library, you should probably not use the hardware interference size > >> +variables at all. Alternatively, you can force a particular value > >> +with @option{--param}. > >> + > >> +If you are confident that your use of the variable does not affect ABI > >> +outside a single build of your project, you can turn off the warning > >> +with @option{-Wno-interference-size}. > >> + > >> @item -Wint-in-bool-context > >> @opindex Wint-in-bool-context > >> @opindex Wno-int-in-bool-context > >> @@ -13902,6 +13939,34 @@ 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. For a generic target covering 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. > >> + > >> +The destructive interference size is intended to be used for layout, > >> +and thus has ABI impact. The default value is not expected to be > >> +stable, and on some targets varies with @option{-mtune}, so use of > >> +this variable in a context where ABI stability is important, such as > >> +the public interface of a library, is strongly discouraged; if it is > >> +used in that context, users can stabilize the value using this > >> +option. > >> + > >> +The constructive interference size is less sensitive, as it is > >> +typically only used in a @samp{static_assert} to make sure that a type > >> +fits within a cache line. > >> + > >> +See also @option{-Winterference-size}. > >> + > >> @item loop-interchange-max-num-stmts > >> The maximum number of stmts in a loop to be interchanged. > >> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt > >> index c5fe90003f2..9c151d19870 100644 > >> --- a/gcc/c-family/c.opt > >> +++ b/gcc/c-family/c.opt > >> @@ -722,6 +722,11 @@ Winit-list-lifetime > >> C++ ObjC++ Var(warn_init_list) Warning Init(1) > >> Warn about uses of std::initializer_list that can result in dangling > >> pointers. > >> +Winterference-size > >> +C++ ObjC++ Var(warn_interference_size) Warning Init(1) > >> +Warn about nonsensical values of --param > >> destructive-interference-size or > >> +constructive-interference-size. > >> + > >> Wimplicit > >> C ObjC Var(warn_implicit) Warning LangEnabledBy(C ObjC,Wall) > >> Warn about implicit declarations. > >> diff --git a/gcc/params.opt b/gcc/params.opt > >> index 3a701e22c46..658ca028851 100644 > >> --- a/gcc/params.opt > >> +++ b/gcc/params.opt > >> @@ -361,6 +361,22 @@ The maximum code size growth ratio when expanding > >> into a jump table (in percent) > >> Common Joined UInteger Var(param_l1_cache_line_size) Init(32) Param > >> Optimization > >> The size of L1 cache line. > >> +-param=destructive-interference-size= > >> +Common Joined UInteger Var(param_destruct_interfere_size) Init(0) > >> Param Optimization > >> +The minimum recommended offset between two concurrently-accessed > >> objects to > >> +avoid additional performance degradation due to contention introduced > >> by the > >> +implementation. Typically the L1 cache line size, but can be larger to > >> +accommodate a variety of target processors with different cache line > >> sizes. > >> +C++17 code might use this value in structure layout, but is strongly > >> +discouraged from doing so in public ABIs. > >> + > >> +-param=constructive-interference-size= > >> +Common Joined UInteger Var(param_construct_interfere_size) Init(0) > >> Param Optimization > >> +The maximum recommended size of contiguous memory occupied by two > >> objects > >> +accessed with temporal locality by concurrent threads. Typically the > >> L1 cache > >> +line size, but can be smaller to accommodate a variety of target > >> processors with > >> +different cache line sizes. > >> + > >> -param=l1-cache-size= > >> Common Joined UInteger Var(param_l1_cache_size) Init(64) Param > >> Optimization > >> The size of L1 cache. > >> diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c > >> index 48cbefd8bf8..020e2ea4f25 100644 > >> --- a/gcc/c-family/c-cppbuiltin.c > >> +++ b/gcc/c-family/c-cppbuiltin.c > >> @@ -741,6 +741,18 @@ cpp_atomic_builtins (cpp_reader *pfile) > >> builtin_define_with_int_value ("__GCC_ATOMIC_TEST_AND_SET_TRUEVAL", > >> targetm.atomic_test_and_set_trueval); > >> + /* Macros for C++17 hardware interference size constants. Either > >> both or > >> + neither should be set. */ > >> + gcc_assert (!param_destruct_interfere_size > >> + == !param_construct_interfere_size); > >> + if (param_destruct_interfere_size) > >> + { > >> + builtin_define_with_int_value ("__GCC_DESTRUCTIVE_SIZE", > >> + param_destruct_interfere_size); > >> + builtin_define_with_int_value ("__GCC_CONSTRUCTIVE_SIZE", > >> + param_construct_interfere_size); > >> + } > >> + > >> /* ptr_type_node can't be used here since ptr_mode is only set when > >> toplev calls backend_init which is not done with -E or pch. */ > >> psize = POINTER_SIZE_UNITS; > >> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > >> index 1fbe9e0daa0..05d42a66c7e 100644 > >> --- a/gcc/config/aarch64/aarch64.c > >> +++ b/gcc/config/aarch64/aarch64.c > >> @@ -16539,6 +16539,28 @@ aarch64_override_options_internal (struct > >> gcc_options *opts) > >> SET_OPTION_IF_UNSET (opts, &global_options_set, > >> param_l1_cache_line_size, > >> aarch64_tune_params.prefetch->l1_cache_line_size); > >> + > >> + if (aarch64_tune_params.prefetch->l1_cache_line_size >= 0) > >> + { > >> + SET_OPTION_IF_UNSET (opts, &global_options_set, > >> + param_destruct_interfere_size, > >> + aarch64_tune_params.prefetch->l1_cache_line_size); > >> + SET_OPTION_IF_UNSET (opts, &global_options_set, > >> + param_construct_interfere_size, > >> + aarch64_tune_params.prefetch->l1_cache_line_size); > >> + } > >> + else > >> + { > >> + /* For a generic AArch64 target, cover the current range of > >> cache line > >> + sizes. */ > >> + SET_OPTION_IF_UNSET (opts, &global_options_set, > >> + param_destruct_interfere_size, > >> + 256); > >> + SET_OPTION_IF_UNSET (opts, &global_options_set, > >> + param_construct_interfere_size, > >> + 64); > >> + } > >> + > >> if (aarch64_tune_params.prefetch->l2_cache_size >= 0) > >> SET_OPTION_IF_UNSET (opts, &global_options_set, > >> param_l2_cache_size, > >> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > >> index f1e628253d0..6c6e77fab66 100644 > >> --- a/gcc/config/arm/arm.c > >> +++ b/gcc/config/arm/arm.c > >> @@ -3669,6 +3669,28 @@ arm_option_override (void) > >> SET_OPTION_IF_UNSET (&global_options, &global_options_set, > >> param_l1_cache_line_size, > >> current_tune->prefetch.l1_cache_line_size); > >> + if (current_tune->prefetch.l1_cache_line_size >= 0) > >> + { > >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > >> + param_destruct_interfere_size, > >> + current_tune->prefetch.l1_cache_line_size); > >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > >> + param_construct_interfere_size, > >> + current_tune->prefetch.l1_cache_line_size); > >> + } > >> + else > >> + { > >> + /* For a generic ARM target, JF Bastien proposed using 64 for > >> both. */ > >> + /* ??? Cortex A9 has a 32-byte cache line, so why not 32 for > >> + constructive? */ > >> + /* More recent Cortex chips have a 64-byte cache line, but are > >> marked > >> + ARM_PREFETCH_NOT_BENEFICIAL, so they get these defaults. */ > >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > >> + param_destruct_interfere_size, 64); > >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > >> + param_construct_interfere_size, 64); > >> + } > >> + > >> if (current_tune->prefetch.l1_cache_size >= 0) > >> SET_OPTION_IF_UNSET (&global_options, &global_options_set, > >> param_l1_cache_size, > >> diff --git a/gcc/config/i386/i386-options.c > >> b/gcc/config/i386/i386-options.c > >> index 2cb87cedec0..c0006b3674b 100644 > >> --- a/gcc/config/i386/i386-options.c > >> +++ b/gcc/config/i386/i386-options.c > >> @@ -2579,6 +2579,12 @@ ix86_option_override_internal (bool main_args_p, > >> SET_OPTION_IF_UNSET (opts, opts_set, param_l2_cache_size, > >> ix86_tune_cost->l2_cache_size); > >> + /* 64B is the accepted value for these for all x86. */ > >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > >> + param_destruct_interfere_size, 64); > >> + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > >> + param_construct_interfere_size, 64); > >> + > >> /* Enable sw prefetching at -O3 for CPUS that prefetching is > >> helpful. */ > >> if (opts->x_flag_prefetch_loop_arrays < 0 > >> && HAVE_prefetch > >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > >> index 7772fe62d95..0c2498aee22 100644 > >> --- a/gcc/cp/constexpr.c > >> +++ b/gcc/cp/constexpr.c > >> @@ -6075,6 +6075,37 @@ inline_asm_in_constexpr_error (location_t loc) > >> "%<constexpr%> function in C++20"); > >> } > >> +/* We're getting the constant value of DECL in a manifestly > >> constant-evaluated > >> + context; maybe complain about that. */ > >> + > >> +static void > >> +maybe_warn_about_constant_value (location_t loc, tree decl) > >> +{ > >> + static bool explained = false; > >> + if (cxx_dialect >= cxx17 > >> + && warn_interference_size > >> + && !global_options_set.x_param_destruct_interfere_size > >> + && DECL_CONTEXT (decl) == std_node > >> + && id_equal (DECL_NAME (decl), > >> "hardware_destructive_interference_size") > >> + && (LOCATION_FILE (input_location) != main_input_filename > >> + || module_exporting_p ()) > >> + && warning_at (loc, OPT_Winterference_size, "use of %qD", decl) > >> + && !explained) > >> + { > >> + explained = true; > >> + inform (loc, "its value can vary between compiler versions or " > >> + "with different %<-mtune%> or %<-mcpu%> flags"); > >> + inform (loc, "if this use is part of a public ABI, change it to " > >> + "instead use a constant variable you define"); > >> + inform (loc, "the default value for the current CPU tuning " > >> + "is %d bytes", param_destruct_interfere_size); > >> + inform (loc, "you can stabilize this value with %<--param " > >> + "hardware_destructive_interference_size=%d%>, or disable " > >> + "this warning with %<-Wno-interference-size%>", > >> + param_destruct_interfere_size); > >> + } > >> +} > >> + > >> /* Attempt to reduce the expression T to a constant value. > >> On failure, issue diagnostic and return error_mark_node. */ > >> /* FIXME unify with c_fully_fold */ > >> @@ -6219,6 +6250,8 @@ cxx_eval_constant_expression (const > >> constexpr_ctx *ctx, tree t, > >> r = *p; > >> break; > >> } > >> + if (ctx->manifestly_const_eval) > >> + maybe_warn_about_constant_value (loc, t); > >> if (COMPLETE_TYPE_P (TREE_TYPE (t)) > >> && is_really_empty_class (TREE_TYPE (t), /*ignore_vptr*/false)) > >> { > >> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > >> index bce62ad202a..c2065027369 100644 > >> --- a/gcc/cp/decl.c > >> +++ b/gcc/cp/decl.c > >> @@ -4752,6 +4752,38 @@ cxx_init_decl_processing (void) > >> /* Show we use EH for cleanups. */ > >> if (flag_exceptions) > >> using_eh_for_cleanups (); > >> + > >> + /* Check that the hardware interference sizes are at least > >> + alignof(max_align_t), as required by the standard. */ > >> + const int max_align = max_align_t_align () / BITS_PER_UNIT; > >> + if (param_destruct_interfere_size) > >> + { > >> + if (param_destruct_interfere_size < max_align) > >> + error ("%<--param destructive-interference-size=%d%> is less than " > >> + "%d", param_destruct_interfere_size, max_align); > >> + else if (param_destruct_interfere_size < > param_l1_cache_line_size) > >> + warning (OPT_Winterference_size, > >> + "%<--param destructive-interference-size=%d%> " > >> + "is less than %<--param l1-cache-line-size=%d%>", > >> + param_destruct_interfere_size, param_l1_cache_line_size); > >> + } > >> + else if (param_l1_cache_line_size >= max_align) > >> + param_destruct_interfere_size = param_l1_cache_line_size; > >> + /* else leave it unset. */ > >> + > >> + if (param_construct_interfere_size) > >> + { > >> + if (param_construct_interfere_size < max_align) > >> + error ("%<--param constructive-interference-size=%d%> is less than > " > >> + "%d", param_construct_interfere_size, max_align); > >> + else if (param_construct_interfere_size > > >> param_l1_cache_line_size) > >> + warning (OPT_Winterference_size, > >> + "%<--param constructive-interference-size=%d%> " > >> + "is greater than %<--param l1-cache-line-size=%d%>", > >> + param_construct_interfere_size, param_l1_cache_line_size); > >> + } > >> + else if (param_l1_cache_line_size >= max_align) > >> + param_construct_interfere_size = param_l1_cache_line_size; > >> } > >> /* Enter an abi node in global-module context. returns a cookie to > >> diff --git a/gcc/testsuite/g++.dg/warn/Winterference-2.C > >> b/gcc/testsuite/g++.dg/warn/Winterference-2.C > >> new file mode 100644 > >> index 00000000000..2af75c63f83 > >> --- /dev/null > >> +++ b/gcc/testsuite/g++.dg/warn/Winterference-2.C > >> @@ -0,0 +1,14 @@ > >> +// { dg-do compile { target c++20 } } > >> +// { dg-additional-options -fmodules-ts } > >> + > >> +module ; > >> + > >> +#include <new> > >> + > >> +export module foo; > >> + > >> +export { > >> + struct A { > >> + alignas(std::hardware_destructive_interference_size) int x; // { > >> dg-warning Winterference-size } > >> + }; > >> +} > >> diff --git a/gcc/testsuite/g++.dg/warn/Winterference.C > >> b/gcc/testsuite/g++.dg/warn/Winterference.C > >> new file mode 100644 > >> index 00000000000..57c001bc032 > >> --- /dev/null > >> +++ b/gcc/testsuite/g++.dg/warn/Winterference.C > >> @@ -0,0 +1,6 @@ > >> +// Test that we warn about use of > >> std::hardware_destructive_interference_size > >> +// in a header. > >> +// { dg-do compile { target c++17 } } > >> + > >> +// { dg-warning Winterference-size "" { target *-*-* } 0 } > >> +#include "Winterference.H" > >> diff --git a/gcc/testsuite/g++.target/aarch64/interference.C > >> b/gcc/testsuite/g++.target/aarch64/interference.C > >> new file mode 100644 > >> index 00000000000..0fc01655223 > >> --- /dev/null > >> +++ b/gcc/testsuite/g++.target/aarch64/interference.C > >> @@ -0,0 +1,9 @@ > >> +// Test C++17 hardware interference size constants > >> +// { dg-do compile { target c++17 } } > >> + > >> +#include <new> > >> + > >> +// Most AArch64 CPUs have an L1 cache line size of 64, but some > >> recent ones use > >> +// 128 or even 256. > >> +static_assert(std::hardware_destructive_interference_size == 256); > >> +static_assert(std::hardware_constructive_interference_size == 64); > >> diff --git a/gcc/testsuite/g++.target/arm/interference.C > >> b/gcc/testsuite/g++.target/arm/interference.C > >> new file mode 100644 > >> index 00000000000..34fe8a52bff > >> --- /dev/null > >> +++ b/gcc/testsuite/g++.target/arm/interference.C > >> @@ -0,0 +1,9 @@ > >> +// Test C++17 hardware interference size constants > >> +// { dg-do compile { target c++17 } } > >> + > >> +#include <new> > >> + > >> +// Recent ARM CPUs have a cache line size of 64. Older ones have > >> +// a size of 32, but I guess they're old enough that we don't care? > >> +static_assert(std::hardware_destructive_interference_size == 64); > >> +static_assert(std::hardware_constructive_interference_size == 64); > >> diff --git a/gcc/testsuite/g++.target/i386/interference.C > >> b/gcc/testsuite/g++.target/i386/interference.C > >> new file mode 100644 > >> index 00000000000..c7b910e3ada > >> --- /dev/null > >> +++ b/gcc/testsuite/g++.target/i386/interference.C > >> @@ -0,0 +1,8 @@ > >> +// Test C++17 hardware interference size constants > >> +// { dg-do compile { target c++17 } } > >> + > >> +#include <new> > >> + > >> +// It is generally agreed that these are the right values for all x86. > >> +static_assert(std::hardware_destructive_interference_size == 64); > >> +static_assert(std::hardware_constructive_interference_size == 64); > >> diff --git a/gcc/testsuite/g++.dg/warn/Winterference.H > >> b/gcc/testsuite/g++.dg/warn/Winterference.H > >> new file mode 100644 > >> index 00000000000..36f0ad5f6d1 > >> --- /dev/null > >> +++ b/gcc/testsuite/g++.dg/warn/Winterference.H > >> @@ -0,0 +1,7 @@ > >> +#include <new> > >> + > >> +struct A > >> +{ > >> + alignas(std::hardware_destructive_interference_size) int i; > >> + alignas(std::hardware_destructive_interference_size) int j; > >> +}; > >> diff --git a/libstdc++-v3/include/std/version > >> b/libstdc++-v3/include/std/version > >> index f950bf0f0db..f41004b5911 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<typename _Tp> > >> @@ -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 > >> > >> base-commit: de515ce0b209cc7e5a780d9846e5154d380a763e >
On 9/14/21 09:56, Christophe LYON via Gcc-patches wrote:
> So adjustment is needed for both arm and aarch64 targets
Hello.
I noticed the same problem and I've got a patch candidate for it.
What do you think about it?
Martin
On 9/15/21 8:31 AM, Martin Liška wrote: > On 9/14/21 09:56, Christophe LYON via Gcc-patches wrote: >> So adjustment is needed for both arm and aarch64 targets > > Hello. > > I noticed the same problem and I've got a patch candidate for it. > > What do you think about it? I've now silenced the warning for internal default values, but they're still worth discussion. For arm, I think it would make sense to use 32 for constructive for the generic target, but perhaps we think the older chips aren't worth constraining new code for? In which case, perhaps param_l1_cache_line_size should also default to 64 for the generic target. For aarch64, it seems even more questionable that param_l1_cache_line_size is 32 for the generic target; are there any aarch64 CPUs with a 32B L1 cache line? Jason
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index d4b3a66ee4f..f49d82aa508 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -9017,6 +9017,43 @@ that has already been done in the current function. Therefore, seemingly insignificant changes in the source program can cause the warnings produced by @option{-Winline} to appear or disappear. +@item -Winterference-size +@opindex Winterference-size +Warn about use of C++17 @code{std::hardware_destructive_interference_size} +without specifying its value with @option{--param destructive-interference-size}. +Also warn about questionable values for that option. + +This variable is intended to be used for controlling class layout, to +avoid false sharing in concurrent code: + +@smallexample +struct independent_fields @{ + alignas(std::hardware_destructive_interference_size) std::atomic<int> one; + alignas(std::hardware_destructive_interference_size) std::atomic<int> two; +@}; +@end smallexample + +Here @samp{one} and @samp{two} are intended to be far enough apart +that stores to one won't require accesses to the other to reload the +cache line. + +By default, @option{--param destructive-interference-size} and +@option{--param constructive-interference-size} are set based on the +current @option{-mtune} option, typically to the L1 cache line size +for the particular target CPU, sometimes to a range if tuning for a +generic target. So all translation units that depend on ABI +compatibility for the use of these variables must be compiled with +the same @option{-mtune} (or @option{-mcpu}). + +If ABI stability is important, such as if the use is in a header for a +library, you should probably not use the hardware interference size +variables at all. Alternatively, you can force a particular value +with @option{--param}. + +If you are confident that your use of the variable does not affect ABI +outside a single build of your project, you can turn off the warning +with @option{-Wno-interference-size}. + @item -Wint-in-bool-context @opindex Wint-in-bool-context @opindex Wno-int-in-bool-context @@ -13902,6 +13939,34 @@ 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. For a generic target covering 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. + +The destructive interference size is intended to be used for layout, +and thus has ABI impact. The default value is not expected to be +stable, and on some targets varies with @option{-mtune}, so use of +this variable in a context where ABI stability is important, such as +the public interface of a library, is strongly discouraged; if it is +used in that context, users can stabilize the value using this +option. + +The constructive interference size is less sensitive, as it is +typically only used in a @samp{static_assert} to make sure that a type +fits within a cache line. + +See also @option{-Winterference-size}. + @item loop-interchange-max-num-stmts The maximum number of stmts in a loop to be interchanged. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index c5fe90003f2..9c151d19870 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -722,6 +722,11 @@ Winit-list-lifetime C++ ObjC++ Var(warn_init_list) Warning Init(1) Warn about uses of std::initializer_list that can result in dangling pointers. +Winterference-size +C++ ObjC++ Var(warn_interference_size) Warning Init(1) +Warn about nonsensical values of --param destructive-interference-size or +constructive-interference-size. + Wimplicit C ObjC Var(warn_implicit) Warning LangEnabledBy(C ObjC,Wall) Warn about implicit declarations. diff --git a/gcc/params.opt b/gcc/params.opt index 3a701e22c46..658ca028851 100644 --- a/gcc/params.opt +++ b/gcc/params.opt @@ -361,6 +361,22 @@ The maximum code size growth ratio when expanding into a jump table (in percent) Common Joined UInteger Var(param_l1_cache_line_size) Init(32) Param Optimization The size of L1 cache line. +-param=destructive-interference-size= +Common Joined UInteger Var(param_destruct_interfere_size) Init(0) Param Optimization +The minimum recommended offset between two concurrently-accessed objects to +avoid additional performance degradation due to contention introduced by the +implementation. Typically the L1 cache line size, but can be larger to +accommodate a variety of target processors with different cache line sizes. +C++17 code might use this value in structure layout, but is strongly +discouraged from doing so in public ABIs. + +-param=constructive-interference-size= +Common Joined UInteger Var(param_construct_interfere_size) Init(0) Param Optimization +The maximum recommended size of contiguous memory occupied by two objects +accessed with temporal locality by concurrent threads. Typically the L1 cache +line size, but can be smaller to accommodate a variety of target processors with +different cache line sizes. + -param=l1-cache-size= Common Joined UInteger Var(param_l1_cache_size) Init(64) Param Optimization The size of L1 cache. diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c index 48cbefd8bf8..020e2ea4f25 100644 --- a/gcc/c-family/c-cppbuiltin.c +++ b/gcc/c-family/c-cppbuiltin.c @@ -741,6 +741,18 @@ cpp_atomic_builtins (cpp_reader *pfile) builtin_define_with_int_value ("__GCC_ATOMIC_TEST_AND_SET_TRUEVAL", targetm.atomic_test_and_set_trueval); + /* Macros for C++17 hardware interference size constants. Either both or + neither should be set. */ + gcc_assert (!param_destruct_interfere_size + == !param_construct_interfere_size); + if (param_destruct_interfere_size) + { + builtin_define_with_int_value ("__GCC_DESTRUCTIVE_SIZE", + param_destruct_interfere_size); + builtin_define_with_int_value ("__GCC_CONSTRUCTIVE_SIZE", + param_construct_interfere_size); + } + /* ptr_type_node can't be used here since ptr_mode is only set when toplev calls backend_init which is not done with -E or pch. */ psize = POINTER_SIZE_UNITS; diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 1fbe9e0daa0..05d42a66c7e 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -16539,6 +16539,28 @@ aarch64_override_options_internal (struct gcc_options *opts) SET_OPTION_IF_UNSET (opts, &global_options_set, param_l1_cache_line_size, aarch64_tune_params.prefetch->l1_cache_line_size); + + if (aarch64_tune_params.prefetch->l1_cache_line_size >= 0) + { + SET_OPTION_IF_UNSET (opts, &global_options_set, + param_destruct_interfere_size, + aarch64_tune_params.prefetch->l1_cache_line_size); + SET_OPTION_IF_UNSET (opts, &global_options_set, + param_construct_interfere_size, + aarch64_tune_params.prefetch->l1_cache_line_size); + } + else + { + /* For a generic AArch64 target, cover the current range of cache line + sizes. */ + SET_OPTION_IF_UNSET (opts, &global_options_set, + param_destruct_interfere_size, + 256); + SET_OPTION_IF_UNSET (opts, &global_options_set, + param_construct_interfere_size, + 64); + } + if (aarch64_tune_params.prefetch->l2_cache_size >= 0) SET_OPTION_IF_UNSET (opts, &global_options_set, param_l2_cache_size, diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index f1e628253d0..6c6e77fab66 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -3669,6 +3669,28 @@ arm_option_override (void) SET_OPTION_IF_UNSET (&global_options, &global_options_set, param_l1_cache_line_size, current_tune->prefetch.l1_cache_line_size); + if (current_tune->prefetch.l1_cache_line_size >= 0) + { + SET_OPTION_IF_UNSET (&global_options, &global_options_set, + param_destruct_interfere_size, + current_tune->prefetch.l1_cache_line_size); + SET_OPTION_IF_UNSET (&global_options, &global_options_set, + param_construct_interfere_size, + current_tune->prefetch.l1_cache_line_size); + } + else + { + /* For a generic ARM target, JF Bastien proposed using 64 for both. */ + /* ??? Cortex A9 has a 32-byte cache line, so why not 32 for + constructive? */ + /* More recent Cortex chips have a 64-byte cache line, but are marked + ARM_PREFETCH_NOT_BENEFICIAL, so they get these defaults. */ + SET_OPTION_IF_UNSET (&global_options, &global_options_set, + param_destruct_interfere_size, 64); + SET_OPTION_IF_UNSET (&global_options, &global_options_set, + param_construct_interfere_size, 64); + } + if (current_tune->prefetch.l1_cache_size >= 0) SET_OPTION_IF_UNSET (&global_options, &global_options_set, param_l1_cache_size, diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c index 2cb87cedec0..c0006b3674b 100644 --- a/gcc/config/i386/i386-options.c +++ b/gcc/config/i386/i386-options.c @@ -2579,6 +2579,12 @@ ix86_option_override_internal (bool main_args_p, SET_OPTION_IF_UNSET (opts, opts_set, param_l2_cache_size, ix86_tune_cost->l2_cache_size); + /* 64B is the accepted value for these for all x86. */ + SET_OPTION_IF_UNSET (&global_options, &global_options_set, + param_destruct_interfere_size, 64); + SET_OPTION_IF_UNSET (&global_options, &global_options_set, + param_construct_interfere_size, 64); + /* Enable sw prefetching at -O3 for CPUS that prefetching is helpful. */ if (opts->x_flag_prefetch_loop_arrays < 0 && HAVE_prefetch diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 7772fe62d95..0c2498aee22 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -6075,6 +6075,37 @@ inline_asm_in_constexpr_error (location_t loc) "%<constexpr%> function in C++20"); } +/* We're getting the constant value of DECL in a manifestly constant-evaluated + context; maybe complain about that. */ + +static void +maybe_warn_about_constant_value (location_t loc, tree decl) +{ + static bool explained = false; + if (cxx_dialect >= cxx17 + && warn_interference_size + && !global_options_set.x_param_destruct_interfere_size + && DECL_CONTEXT (decl) == std_node + && id_equal (DECL_NAME (decl), "hardware_destructive_interference_size") + && (LOCATION_FILE (input_location) != main_input_filename + || module_exporting_p ()) + && warning_at (loc, OPT_Winterference_size, "use of %qD", decl) + && !explained) + { + explained = true; + inform (loc, "its value can vary between compiler versions or " + "with different %<-mtune%> or %<-mcpu%> flags"); + inform (loc, "if this use is part of a public ABI, change it to " + "instead use a constant variable you define"); + inform (loc, "the default value for the current CPU tuning " + "is %d bytes", param_destruct_interfere_size); + inform (loc, "you can stabilize this value with %<--param " + "hardware_destructive_interference_size=%d%>, or disable " + "this warning with %<-Wno-interference-size%>", + param_destruct_interfere_size); + } +} + /* Attempt to reduce the expression T to a constant value. On failure, issue diagnostic and return error_mark_node. */ /* FIXME unify with c_fully_fold */ @@ -6219,6 +6250,8 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, r = *p; break; } + if (ctx->manifestly_const_eval) + maybe_warn_about_constant_value (loc, t); if (COMPLETE_TYPE_P (TREE_TYPE (t)) && is_really_empty_class (TREE_TYPE (t), /*ignore_vptr*/false)) { diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index bce62ad202a..c2065027369 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -4752,6 +4752,38 @@ cxx_init_decl_processing (void) /* Show we use EH for cleanups. */ if (flag_exceptions) using_eh_for_cleanups (); + + /* Check that the hardware interference sizes are at least + alignof(max_align_t), as required by the standard. */ + const int max_align = max_align_t_align () / BITS_PER_UNIT; + if (param_destruct_interfere_size) + { + if (param_destruct_interfere_size < max_align) + error ("%<--param destructive-interference-size=%d%> is less than " + "%d", param_destruct_interfere_size, max_align); + else if (param_destruct_interfere_size < param_l1_cache_line_size) + warning (OPT_Winterference_size, + "%<--param destructive-interference-size=%d%> " + "is less than %<--param l1-cache-line-size=%d%>", + param_destruct_interfere_size, param_l1_cache_line_size); + } + else if (param_l1_cache_line_size >= max_align) + param_destruct_interfere_size = param_l1_cache_line_size; + /* else leave it unset. */ + + if (param_construct_interfere_size) + { + if (param_construct_interfere_size < max_align) + error ("%<--param constructive-interference-size=%d%> is less than " + "%d", param_construct_interfere_size, max_align); + else if (param_construct_interfere_size > param_l1_cache_line_size) + warning (OPT_Winterference_size, + "%<--param constructive-interference-size=%d%> " + "is greater than %<--param l1-cache-line-size=%d%>", + param_construct_interfere_size, param_l1_cache_line_size); + } + else if (param_l1_cache_line_size >= max_align) + param_construct_interfere_size = param_l1_cache_line_size; } /* Enter an abi node in global-module context. returns a cookie to diff --git a/gcc/testsuite/g++.dg/warn/Winterference-2.C b/gcc/testsuite/g++.dg/warn/Winterference-2.C new file mode 100644 index 00000000000..2af75c63f83 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Winterference-2.C @@ -0,0 +1,14 @@ +// { dg-do compile { target c++20 } } +// { dg-additional-options -fmodules-ts } + +module ; + +#include <new> + +export module foo; + +export { + struct A { + alignas(std::hardware_destructive_interference_size) int x; // { dg-warning Winterference-size } + }; +} diff --git a/gcc/testsuite/g++.dg/warn/Winterference.C b/gcc/testsuite/g++.dg/warn/Winterference.C new file mode 100644 index 00000000000..57c001bc032 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Winterference.C @@ -0,0 +1,6 @@ +// Test that we warn about use of std::hardware_destructive_interference_size +// in a header. +// { dg-do compile { target c++17 } } + +// { dg-warning Winterference-size "" { target *-*-* } 0 } +#include "Winterference.H" diff --git a/gcc/testsuite/g++.target/aarch64/interference.C b/gcc/testsuite/g++.target/aarch64/interference.C new file mode 100644 index 00000000000..0fc01655223 --- /dev/null +++ b/gcc/testsuite/g++.target/aarch64/interference.C @@ -0,0 +1,9 @@ +// Test C++17 hardware interference size constants +// { dg-do compile { target c++17 } } + +#include <new> + +// Most AArch64 CPUs have an L1 cache line size of 64, but some recent ones use +// 128 or even 256. +static_assert(std::hardware_destructive_interference_size == 256); +static_assert(std::hardware_constructive_interference_size == 64); diff --git a/gcc/testsuite/g++.target/arm/interference.C b/gcc/testsuite/g++.target/arm/interference.C new file mode 100644 index 00000000000..34fe8a52bff --- /dev/null +++ b/gcc/testsuite/g++.target/arm/interference.C @@ -0,0 +1,9 @@ +// Test C++17 hardware interference size constants +// { dg-do compile { target c++17 } } + +#include <new> + +// Recent ARM CPUs have a cache line size of 64. Older ones have +// a size of 32, but I guess they're old enough that we don't care? +static_assert(std::hardware_destructive_interference_size == 64); +static_assert(std::hardware_constructive_interference_size == 64); diff --git a/gcc/testsuite/g++.target/i386/interference.C b/gcc/testsuite/g++.target/i386/interference.C new file mode 100644 index 00000000000..c7b910e3ada --- /dev/null +++ b/gcc/testsuite/g++.target/i386/interference.C @@ -0,0 +1,8 @@ +// Test C++17 hardware interference size constants +// { dg-do compile { target c++17 } } + +#include <new> + +// It is generally agreed that these are the right values for all x86. +static_assert(std::hardware_destructive_interference_size == 64); +static_assert(std::hardware_constructive_interference_size == 64); diff --git a/gcc/testsuite/g++.dg/warn/Winterference.H b/gcc/testsuite/g++.dg/warn/Winterference.H new file mode 100644 index 00000000000..36f0ad5f6d1 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Winterference.H @@ -0,0 +1,7 @@ +#include <new> + +struct A +{ + alignas(std::hardware_destructive_interference_size) int i; + alignas(std::hardware_destructive_interference_size) int j; +}; diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version index f950bf0f0db..f41004b5911 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<typename _Tp> @@ -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