| Message ID | 20180711112506.GA16112@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [AArch64] Validate and set default parameters for stack-clash. [Patch (3/3)] | expand |
Hi All, I am sending an updated patch which takes into account a case where the set parameter value would not be safe to call. No change in the cover letter. Regards, Tamar > -----Original Message----- > From: Tamar Christina <tamar.christina@arm.com> > Sent: Wednesday, July 11, 2018 12:25 > To: gcc-patches@gcc.gnu.org > Cc: nd <nd@arm.com>; James Greenhalgh <James.Greenhalgh@arm.com>; > Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft > <Marcus.Shawcroft@arm.com> > Subject: [PATCH][GCC][AArch64] Validate and set default parameters for > stack-clash. [Patch (3/3)] > > Hi All, > > This patch defines the default parameters and validation for the aarch64 > stack clash probing interval and guard sizes. It cleans up the previous > implementation and insures that at no point the invalidate arguments are > present in the pipeline for AArch64. Currently they are only corrected once > cc1 initalizes the back-end. > > The default for AArch64 is 64 KB for both of these and we only support 4 KB > and 64 KB probes. We also enforce that any value you set here for the > parameters must be in sync. > > If an invalid value is specified an error will be generated and compilation > aborted. > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > Target was tested with stack clash on and off by default. > > Ok for trunk? > > Thanks, > Tamar > > gcc/ > 2018-07-11 Tamar Christina <tamar.christina@arm.com> > > * common/config/aarch64/aarch64-common.c > (TARGET_OPTION_DEFAULT_PARAM, > aarch64_option_default_param): New. > (params.h): Include. > (TARGET_OPTION_VALIDATE_PARAM, > aarch64_option_validate_param): New. > * config/aarch64/aarch64.c (aarch64_override_options_internal): > Simplify > stack-clash protection validation code. > > -- diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c index 292fb818705d4650113da59a6d88cf2aa7c9e57d..30bbec1380d6db60475f0d770944af98f566773d 100644 --- a/gcc/common/config/aarch64/aarch64-common.c +++ b/gcc/common/config/aarch64/aarch64-common.c @@ -30,6 +30,7 @@ #include "opts.h" #include "flags.h" #include "diagnostic.h" +#include "params.h" #ifdef TARGET_BIG_ENDIAN_DEFAULT #undef TARGET_DEFAULT_TARGET_FLAGS @@ -41,6 +42,10 @@ #undef TARGET_OPTION_OPTIMIZATION_TABLE #define TARGET_OPTION_OPTIMIZATION_TABLE aarch_option_optimization_table +#undef TARGET_OPTION_DEFAULT_PARAMS +#define TARGET_OPTION_DEFAULT_PARAMS aarch64_option_default_params +#undef TARGET_OPTION_VALIDATE_PARAM +#define TARGET_OPTION_VALIDATE_PARAM aarch64_option_validate_param /* Set default optimization options. */ static const struct default_options aarch_option_optimization_table[] = @@ -60,6 +65,48 @@ static const struct default_options aarch_option_optimization_table[] = { OPT_LEVELS_NONE, 0, NULL, 0 } }; +/* Implement target validation TARGET_OPTION_DEFAULT_PARAM. */ + +static bool +aarch64_option_validate_param (const int value, const int param) +{ + /* Check that both parameters are the same. */ + if (param == (int) PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE) + { + if (value != 12 && value != 16) + { + error ("only values 12 (4 KB) and 16 (64 KB) are supported for guard " + "size. Given value %d (%llu KB) is out of range.\n", + value, (1ULL << value) / 1024ULL); + return false; + } + } + + return true; +} + +/* Implement TARGET_OPTION_DEFAULT_PARAMS. */ + +static void +aarch64_option_default_params (void) +{ + /* We assume the guard page is 64k. */ + int index = (int) PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE; + if (!compiler_params[index].configure_value) + set_default_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE, 16); + + int guard_size + = default_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE); + + /* Set the interval parameter to be the same as the guard size. This way the + mid-end code does the right thing for us. */ + set_default_param_value (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL, + guard_size); + + /* Validate the options. */ + aarch64_option_validate_param (guard_size, index); +} + /* Implement TARGET_HANDLE_OPTION. This function handles the target specific options for CPU/target selection. diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index f2a4e0b3db62d9da87458bff98c16e8fb876f431..3fe0e47c561dfb7d1abf06a3322b4a6df63c7a21 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -10905,19 +10905,7 @@ aarch64_override_options_internal (struct gcc_options *opts) opts->x_param_values, global_options_set.x_param_values); - /* If the user hasn't change it via configure then set the default to 64 KB - for the backend. */ - if (DEFAULT_STK_CLASH_GUARD_SIZE == 0) - maybe_set_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE, 16, - opts->x_param_values, - global_options_set.x_param_values); - - /* Validate the guard size. */ int guard_size = PARAM_VALUE (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE); - if (guard_size != 12 && guard_size != 16) - error ("only values 12 (4 KB) and 16 (64 KB) are supported for guard " - "size. Given value %d (%llu KB) is out of range.\n", - guard_size, (1ULL << guard_size) / 1024ULL); /* Enforce that interval is the same size as size so the mid-end does the right thing. */
Hi All, This is an updated patch which applies the same style changes as requested in patch 5/6. Ok for trunk? Thanks, Tamar gcc/ 2018-08-07 Tamar Christina <tamar.christina@arm.com> * common/config/aarch64/aarch64-common.c (TARGET_OPTION_DEFAULT_PARAM, aarch64_option_default_param): New. (params.h): Include. (TARGET_OPTION_VALIDATE_PARAM, aarch64_option_validate_param): New. * config/aarch64/aarch64.c (aarch64_override_options_internal): Simplify stack-clash protection validation code. > -----Original Message----- > From: Tamar Christina > Sent: Friday, July 13, 2018 17:36 > To: Tamar Christina <Tamar.Christina@arm.com>; gcc-patches@gcc.gnu.org > Cc: nd <nd@arm.com>; James Greenhalgh <James.Greenhalgh@arm.com>; > Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft > <Marcus.Shawcroft@arm.com> > Subject: RE: [PATCH][GCC][AArch64] Validate and set default parameters for > stack-clash. [Patch (3/3)] > > Hi All, > > I am sending an updated patch which takes into account a case where the set > parameter value would not be safe to call. > > No change in the cover letter. > > Regards, > Tamar > > > -----Original Message----- > > From: Tamar Christina <tamar.christina@arm.com> > > Sent: Wednesday, July 11, 2018 12:25 > > To: gcc-patches@gcc.gnu.org > > Cc: nd <nd@arm.com>; James Greenhalgh <James.Greenhalgh@arm.com>; > > Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft > > <Marcus.Shawcroft@arm.com> > > Subject: [PATCH][GCC][AArch64] Validate and set default parameters for > > stack-clash. [Patch (3/3)] > > > > Hi All, > > > > This patch defines the default parameters and validation for the > > aarch64 stack clash probing interval and guard sizes. It cleans up > > the previous implementation and insures that at no point the > > invalidate arguments are present in the pipeline for AArch64. > > Currently they are only corrected once > > cc1 initalizes the back-end. > > > > The default for AArch64 is 64 KB for both of these and we only support > > 4 KB and 64 KB probes. We also enforce that any value you set here > > for the parameters must be in sync. > > > > If an invalid value is specified an error will be generated and > > compilation aborted. > > > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > Target was tested with stack clash on and off by default. > > > > Ok for trunk? > > > > Thanks, > > Tamar > > > > gcc/ > > 2018-07-11 Tamar Christina <tamar.christina@arm.com> > > > > * common/config/aarch64/aarch64-common.c > > (TARGET_OPTION_DEFAULT_PARAM, > > aarch64_option_default_param): New. > > (params.h): Include. > > (TARGET_OPTION_VALIDATE_PARAM, > > aarch64_option_validate_param): New. > > * config/aarch64/aarch64.c (aarch64_override_options_internal): > > Simplify > > stack-clash protection validation code. > > > > -- diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c index 292fb818705d4650113da59a6d88cf2aa7c9e57d..97d7770190b8388ba277db96c07609f7b6f46817 100644 --- a/gcc/common/config/aarch64/aarch64-common.c +++ b/gcc/common/config/aarch64/aarch64-common.c @@ -30,6 +30,7 @@ #include "opts.h" #include "flags.h" #include "diagnostic.h" +#include "params.h" #ifdef TARGET_BIG_ENDIAN_DEFAULT #undef TARGET_DEFAULT_TARGET_FLAGS @@ -41,6 +42,10 @@ #undef TARGET_OPTION_OPTIMIZATION_TABLE #define TARGET_OPTION_OPTIMIZATION_TABLE aarch_option_optimization_table +#undef TARGET_OPTION_DEFAULT_PARAMS +#define TARGET_OPTION_DEFAULT_PARAMS aarch64_option_default_params +#undef TARGET_OPTION_VALIDATE_PARAM +#define TARGET_OPTION_VALIDATE_PARAM aarch64_option_validate_param /* Set default optimization options. */ static const struct default_options aarch_option_optimization_table[] = @@ -60,6 +65,49 @@ static const struct default_options aarch_option_optimization_table[] = { OPT_LEVELS_NONE, 0, NULL, 0 } }; +/* Implement target validation TARGET_OPTION_DEFAULT_PARAM. */ + +static bool +aarch64_option_validate_param (const int value, const int param) +{ + /* Check that both parameters are the same. */ + if (param == (int) PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE) + { + if (value != 12 && value != 16) + { + error ("only values 12 (4 KB) and 16 (64 KB) are supported for guard " + "size. Given value %d (%llu KB) is out of range.", + value, (1ULL << value) / 1024ULL); + return false; + } + } + + return true; +} + +/* Implement TARGET_OPTION_DEFAULT_PARAMS. */ + +static void +aarch64_option_default_params (void) +{ + /* We assume the guard page is 64k. */ + int index = (int) PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE; + set_default_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE, + DEFAULT_STK_CLASH_GUARD_SIZE == 0 + ? 16 : DEFAULT_STK_CLASH_GUARD_SIZE); + + int guard_size + = default_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE); + + /* Set the interval parameter to be the same as the guard size. This way the + mid-end code does the right thing for us. */ + set_default_param_value (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL, + guard_size); + + /* Validate the options. */ + aarch64_option_validate_param (guard_size, index); +} + /* Implement TARGET_HANDLE_OPTION. This function handles the target specific options for CPU/target selection. diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 31948dc67c8ae1e879a3d3f1ca8cb96e975afc30..ed85093a1e5a9126f2498b2aecc369f054839e0c 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -10938,10 +10938,6 @@ aarch64_override_options_internal (struct gcc_options *opts) /* Validate the guard size. */ int guard_size = PARAM_VALUE (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE); - if (guard_size != 12 && guard_size != 16) - error ("only values 12 (4 KB) and 16 (64 KB) are supported for guard " - "size. Given value %d (%llu KB) is out of range.", - guard_size, (1ULL << guard_size) / 1024ULL); /* Enforce that interval is the same size as size so the mid-end does the right thing. */
On Tue, Aug 07, 2018 at 05:09:30AM -0500, Tamar Christina wrote: > Hi All, > > This is an updated patch which applies the same style changes as requested in patch 5/6. No full stop on an error message IIRC. Otherwise, OK. Thanks, James > gcc/ > 2018-08-07 Tamar Christina <tamar.christina@arm.com> > > * common/config/aarch64/aarch64-common.c (TARGET_OPTION_DEFAULT_PARAM, > aarch64_option_default_param): New. > (params.h): Include. > (TARGET_OPTION_VALIDATE_PARAM, aarch64_option_validate_param): New. > * config/aarch64/aarch64.c (aarch64_override_options_internal): Simplify > stack-clash protection validation code.
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c index 292fb818705d4650113da59a6d88cf2aa7c9e57d..73f2f95e8cb989f93e7a17bbf274f4364e660c0d 100644 --- a/gcc/common/config/aarch64/aarch64-common.c +++ b/gcc/common/config/aarch64/aarch64-common.c @@ -30,6 +30,7 @@ #include "opts.h" #include "flags.h" #include "diagnostic.h" +#include "params.h" #ifdef TARGET_BIG_ENDIAN_DEFAULT #undef TARGET_DEFAULT_TARGET_FLAGS @@ -41,6 +42,10 @@ #undef TARGET_OPTION_OPTIMIZATION_TABLE #define TARGET_OPTION_OPTIMIZATION_TABLE aarch_option_optimization_table +#undef TARGET_OPTION_DEFAULT_PARAMS +#define TARGET_OPTION_DEFAULT_PARAMS aarch64_option_default_params +#undef TARGET_OPTION_VALIDATE_PARAM +#define TARGET_OPTION_VALIDATE_PARAM aarch64_option_validate_param /* Set default optimization options. */ static const struct default_options aarch_option_optimization_table[] = @@ -60,6 +65,52 @@ static const struct default_options aarch_option_optimization_table[] = { OPT_LEVELS_NONE, 0, NULL, 0 } }; +/* Implement target validation TARGET_OPTION_DEFAULT_PARAM. */ + +static bool +aarch64_option_validate_param (const int value, const int param) +{ + /* Check that both parameters are the same. */ + if (param == (int) PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE) + { + if (value != 12 && value != 16) + { + error ("only values 12 (4 KB) and 16 (64 KB) are supported for guard " + "size. Given value %d (%llu KB) is out of range.\n", + value, (1ULL << value) / 1024ULL); + return false; + } + + /* Enforce that they are the same. */ + set_default_param_value (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL, + value); + } + + return true; +} + +/* Implement TARGET_OPTION_DEFAULT_PARAMS. */ + +static void +aarch64_option_default_params (void) +{ + /* We assume the guard page is 64k. */ + int index = (int) PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE; + if (!compiler_params[index].configure_value) + set_default_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE, 16); + + int guard_size + = default_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE); + + /* Set the interval parameter to be the same as the guard size. This way the + mid-end code does the right thing for us. */ + set_default_param_value (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL, + guard_size); + + /* Validate the options. */ + aarch64_option_validate_param (guard_size, index); +} + /* Implement TARGET_HANDLE_OPTION. This function handles the target specific options for CPU/target selection. diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index e62d8a92ff53128e5e10ffd3b52eb8898869b756..dfc0da6a27d6007c669db32abe5c7b0248ea28a5 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -10900,41 +10900,21 @@ aarch64_override_options_internal (struct gcc_options *opts) opts->x_param_values, global_options_set.x_param_values); - /* Use the alternative scheduling-pressure algorithm by default. */ - maybe_set_param_value (PARAM_SCHED_PRESSURE_ALGORITHM, SCHED_PRESSURE_MODEL, - opts->x_param_values, - global_options_set.x_param_values); - - /* If the user hasn't change it via configure then set the default to 64 KB - for the backend. */ - if (DEFAULT_STK_CLASH_GUARD_SIZE == 0) - maybe_set_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE, 16, - opts->x_param_values, - global_options_set.x_param_values); - - /* Validate the guard size. */ - int guard_size = PARAM_VALUE (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE); - if (guard_size != 12 && guard_size != 16) - error ("only values 12 (4 KB) and 16 (64 KB) are supported for guard " - "size. Given value %d (%llu KB) is out of range.\n", - guard_size, (1ULL << guard_size) / 1024ULL); - - /* Enforce that interval is the same size as size so the mid-end does the - right thing. */ - maybe_set_param_value (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL, - guard_size, - opts->x_param_values, - global_options_set.x_param_values); - /* The maybe_set calls won't update the value if the user has explicitly set one. Which means we need to validate that probing interval and guard size are equal. */ int probe_interval = PARAM_VALUE (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL); + int guard_size = PARAM_VALUE (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE); if (guard_size != probe_interval) error ("stack clash guard size '%d' must be equal to probing interval " "'%d'\n", guard_size, probe_interval); + /* Use the alternative scheduling-pressure algorithm by default. */ + maybe_set_param_value (PARAM_SCHED_PRESSURE_ALGORITHM, SCHED_PRESSURE_MODEL, + opts->x_param_values, + global_options_set.x_param_values); + /* Enable sw prefetching at specified optimization level for CPUS that have prefetch. Lower optimization level threshold by 1 when profiling is enabled. */