Re: [PATCH v2][ARM] Disable code hoisting with -O3 (PR80155)
On Tue, Jan 28, 2020 at 10:42:16AM +0100, Richard Biener wrote: > On Mon, Jan 27, 2020 at 10:47 PM Segher Boessenkool > wrote: > > On Tue, Jan 21, 2020 at 02:10:21PM +, Wilco Dijkstra wrote: > > > While code hoisting generally improves codesize, it can affect performance > > > negatively. Benchmarking shows it doesn't help SPEC and negatively affects > > > embedded benchmarks. Since the impact is relatively small with -O2 and > > > mainly > > > affects -O3, the simplest option is to disable code hoisting for -O3 and > > > higher. > > > > Should this be a generic thing, not target-specific? > > The change doesn't make sense - I'm sure if you'd look at the actual cases > it's not code hoisting in itself but "optimizations" enabled by it that cause > the issues. It's probably the same thing as PRE causing issues downstream > for some benchmarks but do you disable PRE then by default just because of > that? Well, that depends on how bad it is. And of course not if it is just because of benchmark peculiarities, we're not a banchmark compiler after all, we're meant to compile real code. But if PRE (just to continue your example) would mess that up more often than not, then yes, it should not be enabled by default. Segher
Re: [PATCH v2][ARM] Disable code hoisting with -O3 (PR80155)
On Tue, Nov 26, 2019 at 3:18 PM Wilco Dijkstra wrote:
>
> Hi,
>
> While code hoisting generally improves codesize, it can affect performance
> negatively. Benchmarking shows it doesn't help SPEC and negatively affects
> embedded benchmarks. Since the impact is relatively small with -O2 and mainly
> affects -O3, the simplest option is to disable code hoisting for -O3 and
> higher.
>
> OK for commit?
>
> ChangeLog:
> 2019-11-26 Wilco Dijkstra
>
> PR tree-optimization/80155
> * common/config/arm/arm-common.c (arm_option_optimization_table):
> Disable -fcode-hoisting with -O3.
> --
>
> diff --git a/gcc/common/config/arm/arm-common.c
> b/gcc/common/config/arm/arm-common.c
> index
> b761d3abd670a144a593c4b410b1e7fbdcb52f56..3e11f21b7dd76cc071b645c32a6fdb4a92511279
> 100644
> --- a/gcc/common/config/arm/arm-common.c
> +++ b/gcc/common/config/arm/arm-common.c
> @@ -39,6 +39,8 @@ static const struct default_options
> arm_option_optimization_table[] =
> /* Enable section anchors by default at -O1 or higher. */
> { OPT_LEVELS_1_PLUS, OPT_fsection_anchors, NULL, 1 },
> { OPT_LEVELS_FAST, OPT_fsched_pressure, NULL, 1 },
> +/* Disable code hoisting with -O3 or higher. */
> +{ OPT_LEVELS_3_PLUS, OPT_fcode_hoisting, NULL, 0 },
> { OPT_LEVELS_NONE, 0, NULL, 0 }
>};
>
What are the cases in O3 where this is slower with embedded benchmarks
and how can we fix it ? Keeping the target "different" in this manner
doesn't augur well for the long term.
Ramana
Re: [PATCH v2][ARM] Disable code hoisting with -O3 (PR80155)
On Mon, Jan 27, 2020 at 10:47 PM Segher Boessenkool wrote: > > Hi! > > On Tue, Jan 21, 2020 at 02:10:21PM +, Wilco Dijkstra wrote: > > While code hoisting generally improves codesize, it can affect performance > > negatively. Benchmarking shows it doesn't help SPEC and negatively affects > > embedded benchmarks. Since the impact is relatively small with -O2 and > > mainly > > affects -O3, the simplest option is to disable code hoisting for -O3 and > > higher. > > Should this be a generic thing, not target-specific? The change doesn't make sense - I'm sure if you'd look at the actual cases it's not code hoisting in itself but "optimizations" enabled by it that cause the issues. It's probably the same thing as PRE causing issues downstream for some benchmarks but do you disable PRE then by default just because of that? Richard. > > Segher
Re: [PATCH v2][ARM] Disable code hoisting with -O3 (PR80155)
Hi! On Tue, Jan 21, 2020 at 02:10:21PM +, Wilco Dijkstra wrote: > While code hoisting generally improves codesize, it can affect performance > negatively. Benchmarking shows it doesn't help SPEC and negatively affects > embedded benchmarks. Since the impact is relatively small with -O2 and mainly > affects -O3, the simplest option is to disable code hoisting for -O3 and > higher. Should this be a generic thing, not target-specific? Segher
Re: [PATCH v2][ARM] Disable code hoisting with -O3 (PR80155)
ping
Hi,
While code hoisting generally improves codesize, it can affect performance
negatively. Benchmarking shows it doesn't help SPEC and negatively affects
embedded benchmarks. Since the impact is relatively small with -O2 and mainly
affects -O3, the simplest option is to disable code hoisting for -O3 and higher.
OK for commit?
ChangeLog:
2019-11-26 Wilco Dijkstra
PR tree-optimization/80155
* common/config/arm/arm-common.c (arm_option_optimization_table):
Disable -fcode-hoisting with -O3.
--
diff --git a/gcc/common/config/arm/arm-common.c
b/gcc/common/config/arm/arm-common.c
index
b761d3abd670a144a593c4b410b1e7fbdcb52f56..3e11f21b7dd76cc071b645c32a6fdb4a92511279
100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -39,6 +39,8 @@ static const struct default_options
arm_option_optimization_table[] =
/* Enable section anchors by default at -O1 or higher. */
{ OPT_LEVELS_1_PLUS, OPT_fsection_anchors, NULL, 1 },
{ OPT_LEVELS_FAST, OPT_fsched_pressure, NULL, 1 },
+/* Disable code hoisting with -O3 or higher. */
+{ OPT_LEVELS_3_PLUS, OPT_fcode_hoisting, NULL, 0 },
{ OPT_LEVELS_NONE, 0, NULL, 0 }
};
Re: [PATCH v2][ARM] Disable code hoisting with -O3 (PR80155)
Hi Christophe, > Some time ago, you proposed to enable code hoisting for -Os instead, > and this is the approach that was chosen > in arm-9-branch. Why are you proposing a different setting for trunk? Like I said in my message, I've now done more detailed benchmarking which shows it affects -O3 performance very significantly but -O2 by not much. Since it's a good idea to keep the O1/O2 configs as standard as possible (as Richi suggests), just disabling with -O3 seems better than the previous version. Cheers, Wilco
Re: [PATCH v2][ARM] Disable code hoisting with -O3 (PR80155)
On November 26, 2019 4:39:09 PM GMT+01:00, Christophe Lyon
wrote:
>On Tue, 26 Nov 2019 at 16:18, Wilco Dijkstra
>wrote:
>>
>> Hi,
>>
>> While code hoisting generally improves codesize, it can affect
>performance
>> negatively. Benchmarking shows it doesn't help SPEC and negatively
>affects
>> embedded benchmarks. Since the impact is relatively small with -O2
>and mainly
>> affects -O3, the simplest option is to disable code hoisting for -O3
>and higher.
>>
>> OK for commit?
>>
>
>Hi,
>
>Some time ago, you proposed to enable code hoisting for -Os instead,
>and this is the approach that was chosen
>in arm-9-branch. Why are you proposing a different setting for trunk?
These kind of tweaks also single out targets in intransparent ways for the user
and for debugging code generation issues.
But it's your target...
Richard
>Thanks,
>
>Christophe
>
>> ChangeLog:
>> 2019-11-26 Wilco Dijkstra
>>
>> PR tree-optimization/80155
>> * common/config/arm/arm-common.c
>(arm_option_optimization_table):
>> Disable -fcode-hoisting with -O3.
>> --
>>
>> diff --git a/gcc/common/config/arm/arm-common.c
>b/gcc/common/config/arm/arm-common.c
>> index
>b761d3abd670a144a593c4b410b1e7fbdcb52f56..3e11f21b7dd76cc071b645c32a6fdb4a92511279
>100644
>> --- a/gcc/common/config/arm/arm-common.c
>> +++ b/gcc/common/config/arm/arm-common.c
>> @@ -39,6 +39,8 @@ static const struct default_options
>arm_option_optimization_table[] =
>> /* Enable section anchors by default at -O1 or higher. */
>> { OPT_LEVELS_1_PLUS, OPT_fsection_anchors, NULL, 1 },
>> { OPT_LEVELS_FAST, OPT_fsched_pressure, NULL, 1 },
>> +/* Disable code hoisting with -O3 or higher. */
>> +{ OPT_LEVELS_3_PLUS, OPT_fcode_hoisting, NULL, 0 },
>> { OPT_LEVELS_NONE, 0, NULL, 0 }
>>};
>>
Re: [PATCH v2][ARM] Disable code hoisting with -O3 (PR80155)
On Tue, 26 Nov 2019 at 16:18, Wilco Dijkstra wrote:
>
> Hi,
>
> While code hoisting generally improves codesize, it can affect performance
> negatively. Benchmarking shows it doesn't help SPEC and negatively affects
> embedded benchmarks. Since the impact is relatively small with -O2 and mainly
> affects -O3, the simplest option is to disable code hoisting for -O3 and
> higher.
>
> OK for commit?
>
Hi,
Some time ago, you proposed to enable code hoisting for -Os instead,
and this is the approach that was chosen
in arm-9-branch. Why are you proposing a different setting for trunk?
Thanks,
Christophe
> ChangeLog:
> 2019-11-26 Wilco Dijkstra
>
> PR tree-optimization/80155
> * common/config/arm/arm-common.c (arm_option_optimization_table):
> Disable -fcode-hoisting with -O3.
> --
>
> diff --git a/gcc/common/config/arm/arm-common.c
> b/gcc/common/config/arm/arm-common.c
> index
> b761d3abd670a144a593c4b410b1e7fbdcb52f56..3e11f21b7dd76cc071b645c32a6fdb4a92511279
> 100644
> --- a/gcc/common/config/arm/arm-common.c
> +++ b/gcc/common/config/arm/arm-common.c
> @@ -39,6 +39,8 @@ static const struct default_options
> arm_option_optimization_table[] =
> /* Enable section anchors by default at -O1 or higher. */
> { OPT_LEVELS_1_PLUS, OPT_fsection_anchors, NULL, 1 },
> { OPT_LEVELS_FAST, OPT_fsched_pressure, NULL, 1 },
> +/* Disable code hoisting with -O3 or higher. */
> +{ OPT_LEVELS_3_PLUS, OPT_fcode_hoisting, NULL, 0 },
> { OPT_LEVELS_NONE, 0, NULL, 0 }
>};
>
[PATCH v2][ARM] Disable code hoisting with -O3 (PR80155)
Hi,
While code hoisting generally improves codesize, it can affect performance
negatively. Benchmarking shows it doesn't help SPEC and negatively affects
embedded benchmarks. Since the impact is relatively small with -O2 and mainly
affects -O3, the simplest option is to disable code hoisting for -O3 and higher.
OK for commit?
ChangeLog:
2019-11-26 Wilco Dijkstra
PR tree-optimization/80155
* common/config/arm/arm-common.c (arm_option_optimization_table):
Disable -fcode-hoisting with -O3.
--
diff --git a/gcc/common/config/arm/arm-common.c
b/gcc/common/config/arm/arm-common.c
index
b761d3abd670a144a593c4b410b1e7fbdcb52f56..3e11f21b7dd76cc071b645c32a6fdb4a92511279
100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -39,6 +39,8 @@ static const struct default_options
arm_option_optimization_table[] =
/* Enable section anchors by default at -O1 or higher. */
{ OPT_LEVELS_1_PLUS, OPT_fsection_anchors, NULL, 1 },
{ OPT_LEVELS_FAST, OPT_fsched_pressure, NULL, 1 },
+/* Disable code hoisting with -O3 or higher. */
+{ OPT_LEVELS_3_PLUS, OPT_fcode_hoisting, NULL, 0 },
{ OPT_LEVELS_NONE, 0, NULL, 0 }
};
