| Message ID | AM5PR0802MB2610A85D93128828E83A0C3C83B80@AM5PR0802MB2610.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On 10/11/16 17:19, Wilco Dijkstra wrote: > Improve the logic when setting max_insns_skipped. Limit the maximum size of IT > to MAX_INSN_PER_IT_BLOCK as otherwise multiple IT instructions are needed, > increasing codesize. > You don't provide any information about what benefits this brings. > Given 4 works well for Thumb-2, use the same limit for ARM > for consistency. Why? Logic might suggest that given thumb has to execute an IT instruction first, then allowing ARM to have one more conditional instruction is the best answer. R. > ChangeLog: > 2016-11-04 Wilco Dijkstra <wdijkstr@arm.com> > > * config/arm/arm.c (arm_option_params_internal): Improve setting of > max_insns_skipped. > -- > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index f046854e9665d54911616fc1c60fee407188f7d6..29e8d1d07d918fbb2a627a653510dfc8587ee01a 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -2901,20 +2901,12 @@ arm_option_params_internal (void) > targetm.max_anchor_offset = TARGET_MAX_ANCHOR_OFFSET; > } > > - if (optimize_size) > - { > - /* If optimizing for size, bump the number of instructions that we > - are prepared to conditionally execute (even on a StrongARM). */ > - max_insns_skipped = 6; > + /* Increase the number of conditional instructions with -Os. */ > + max_insns_skipped = optimize_size ? 4 : current_tune->max_insns_skipped; > > - /* For THUMB2, we limit the conditional sequence to one IT block. */ > - if (TARGET_THUMB2) > - max_insns_skipped = arm_restrict_it ? 1 : 4; > - } > - else > - /* When -mrestrict-it is in use tone down the if-conversion. */ > - max_insns_skipped = (TARGET_THUMB2 && arm_restrict_it) > - ? 1 : current_tune->max_insns_skipped; > + /* For THUMB2, we limit the conditional sequence to one IT block. */ > + if (TARGET_THUMB2) > + max_insns_skipped = MIN (max_insns_skipped, MAX_INSN_PER_IT_BLOCK); > } > > /* True if -mflip-thumb should next add an attribute for the default >
Richard Earnshaw wrote: > On 10/11/16 17:19, Wilco Dijkstra wrote: > > Improve the logic when setting max_insns_skipped. Limit the maximum size of IT > > to MAX_INSN_PER_IT_BLOCK as otherwise multiple IT instructions are needed, > > increasing codesize. > > You don't provide any information about what benefits this brings. It reduces codesize and improves performance as you avoid emitting a second IT instruction for sequences of more than 4 conditional instructions. > > Given 4 works well for Thumb-2, use the same limit for ARM > > for consistency. > > Why? Logic might suggest that given thumb has to execute an IT > instruction first, then allowing ARM to have one more conditional > instruction is the best answer. Long conditional sequences are slow on modern cores - the value 6 for max_insns_skipped is a few decades out of date as it was meant for ARM2! Even with -Os the performance loss for larger values is not worth the small codesize gain (there are many better options to reduce codesize that actually improve performance at the same time). So using the same code generation heuristics for ARM and Thumb-2 is a good idea. Wilco
Wilco Dijkstra wrote: > Richard Earnshaw wrote: > On 10/11/16 17:19, Wilco Dijkstra wrote: > Long conditional sequences are slow on modern cores - the value 6 for > max_insns_skipped is a few decades out of date as it was meant for ARM2! > Even with -Os the performance loss for larger values is not worth the > small codesize gain (there are many better options to reduce codesize > that actually improve performance at the same time). So using the same > code generation heuristics for ARM and Thumb-2 is a good idea. A simple codesize comparison on CSiBE shows using 4 rather than 6 for max_insns_skipped is just 0.07% larger on ARM with -Os. So it's not obvious that increasing max_insns_skipped in -Os is a useful codesize optimization... Wilco
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index f046854e9665d54911616fc1c60fee407188f7d6..29e8d1d07d918fbb2a627a653510dfc8587ee01a 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -2901,20 +2901,12 @@ arm_option_params_internal (void) targetm.max_anchor_offset = TARGET_MAX_ANCHOR_OFFSET; } - if (optimize_size) - { - /* If optimizing for size, bump the number of instructions that we - are prepared to conditionally execute (even on a StrongARM). */ - max_insns_skipped = 6; + /* Increase the number of conditional instructions with -Os. */ + max_insns_skipped = optimize_size ? 4 : current_tune->max_insns_skipped; - /* For THUMB2, we limit the conditional sequence to one IT block. */ - if (TARGET_THUMB2) - max_insns_skipped = arm_restrict_it ? 1 : 4; - } - else - /* When -mrestrict-it is in use tone down the if-conversion. */ - max_insns_skipped = (TARGET_THUMB2 && arm_restrict_it) - ? 1 : current_tune->max_insns_skipped; + /* For THUMB2, we limit the conditional sequence to one IT block. */ + if (TARGET_THUMB2) + max_insns_skipped = MIN (max_insns_skipped, MAX_INSN_PER_IT_BLOCK); } /* True if -mflip-thumb should next add an attribute for the default
Improve the logic when setting max_insns_skipped. Limit the maximum size of IT to MAX_INSN_PER_IT_BLOCK as otherwise multiple IT instructions are needed, increasing codesize. Given 4 works well for Thumb-2, use the same limit for ARM for consistency. ChangeLog: 2016-11-04 Wilco Dijkstra <wdijkstr@arm.com> * config/arm/arm.c (arm_option_params_internal): Improve setting of max_insns_skipped. --