| Message ID | HE1PR0801MB2121DBD899AC0F5B4FA5A41083450@HE1PR0801MB2121.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,ARM] Disable code hoisting with -O3 (PR80155) | expand |
On Tue, 26 Nov 2019 at 16:18, Wilco Dijkstra <Wilco.Dijkstra@arm.com> 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 <wdijkstr@arm.com> > > 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 } > }; >
On November 26, 2019 4:39:09 PM GMT+01:00, Christophe Lyon <christophe.lyon@linaro.org> wrote: >On Tue, 26 Nov 2019 at 16:18, Wilco Dijkstra <Wilco.Dijkstra@arm.com> >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 <wdijkstr@arm.com> >> >> 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 } >> }; >>
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
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 <wdijkstr@arm.com>
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 }
};
Hi! On Tue, Jan 21, 2020 at 02:10:21PM +0000, 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
On Mon, Jan 27, 2020 at 10:47 PM Segher Boessenkool <segher@kernel.crashing.org> wrote: > > Hi! > > On Tue, Jan 21, 2020 at 02:10:21PM +0000, 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
On Tue, Nov 26, 2019 at 3:18 PM Wilco Dijkstra <Wilco.Dijkstra@arm.com> 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 <wdijkstr@arm.com> > > 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
On Tue, Jan 28, 2020 at 10:42:16AM +0100, Richard Biener wrote: > On Mon, Jan 27, 2020 at 10:47 PM Segher Boessenkool > <segher@kernel.crashing.org> wrote: > > On Tue, Jan 21, 2020 at 02:10:21PM +0000, 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
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 } };