[v3,1/4] ree: Default ree pass for O2 and above for rs6000 target.

Hello All:

This is the patch-1 for improving ree pass for rs6000 target.
Bootstrapped and regtested on powerpc64-linux-gnu.

Thanks & Regards
Ajit

	ree: Improve ree pass for rs6000 target.

	Add ree pass as a default pass for rs6000 target.

	2023-04-19  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>

gcc/ChangeLog:

	* common/config/rs6000/rs6000-common.cc: Add REE pass as a
	default rs6000 target pass for O2 and above.
---
 gcc/common/config/rs6000/rs6000-common.cc | 2 ++
 1 file changed, 2 insertions(+)

Hi!

The subject should be something like

rs6000: Enable REE pass by default

(and no period at the end).

On Wed, Apr 19, 2023 at 11:23:07PM +0530, Ajit Agarwal wrote:
> This is the patch-1 for improving ree pass for rs6000 target.

It actually just enables it :-)

The mail body should be the proposed commit message.  Nothing more,
nothing less.  If you need (or want) to talk about more things, that is
what a "0/4" message is for (you create that with --cover).  Your patch
messages here do not thread properly, how did you create them?  Things
work fine if you use  git format-patch --thread  :-)

> 	ree: Improve ree pass for rs6000 target.
> 
> 	Add ree pass as a default pass for rs6000 target.
> 
> 	2023-04-19  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>

You aren't in MAINTAINERS yet, please fix that first!

> 
> gcc/ChangeLog:
> 
> 	* common/config/rs6000/rs6000-common.cc: Add REE pass as a
> 	default rs6000 target pass for O2 and above.

Why only for -O2?  Only when optimising at all makes sense, people use
-O0 only when they want to skip as many optimisations as possible, maybe
because of compilation time concerns, maybe to avoid an ICE or other
bug.  Isn't REE *always* a good thing, it never degrades code quality?
Or are there situations where it results in worse code?


Segher

Hello Segher:

On 20/04/23 1:30 am, Segher Boessenkool wrote:
> Hi!
> 
> The subject should be something like
> 
> rs6000: Enable REE pass by default
> 
> (and no period at the end).
> 
> On Wed, Apr 19, 2023 at 11:23:07PM +0530, Ajit Agarwal wrote:
>> This is the patch-1 for improving ree pass for rs6000 target.
> 
> It actually just enables it :-)
> he c
> The mail body should be the proposed commit message.  Nothing more,
> nothing less.  If you need (or want) to talk about more things, that is
> what a "0/4" message is for (you create that with --cover).  Your patch
> messages here do not thread properly, how did you create them?  Things
> work fine if you use  git format-patch --thread  :-)
> 
>> 	ree: Improve ree pass for rs6000 target.
>>
>> 	Add ree pass as a default pass for rs6000 target.
>>
>> 	2023-04-19  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>
> 
> You aren't in MAINTAINERS yet, please fix that first!
> 
>>

Done. Already added Write after approval in MAINTAINERS and pushed the changes.

>> gcc/ChangeLog:
>>
>> 	* common/config/rs6000/rs6000-common.cc: Add REE pass as a
>> 	default rs6000 target pass for O2 and above.
> 
> Why only for -O2?  Only when optimising at all makes sense, people use
> -O0 only when they want to skip as many optimisations as possible, maybe
> because of compilation time concerns, maybe to avoid an ICE or other
> bug.  Isn't REE *always* a good thing, it never degrades code quality?
> Or are there situations where it results in worse code?
> 

I think it should be O2 and above and am not sure how it behaves with O0.
According to me,  REE is always a good optimization to have and I don't think it degrades any performance or code quality. I don't see any situation where it results in worse code. It tries to remove extensions and combine them which will surely improves performance and code quality instead of worsening the code.

Thanks & Regards
Ajit

 
> Segher

On 4/19/23 3:00 PM, Segher Boessenkool wrote:
> On Wed, Apr 19, 2023 at 11:23:07PM +0530, Ajit Agarwal wrote:
>> 	* common/config/rs6000/rs6000-common.cc: Add REE pass as a
>> 	default rs6000 target pass for O2 and above.
> 
> Why only for -O2?  Only when optimising at all makes sense, people use
> -O0 only when they want to skip as many optimisations as possible, maybe
> because of compilation time concerns, maybe to avoid an ICE or other
> bug.  Isn't REE *always* a good thing, it never degrades code quality?
> Or are there situations where it results in worse code?

I think this is a case of following what the other architectures are doing.
Namely, x86, aarch64, riscv, sparc, alpha and h8300 all enable -free at
-O2 and above, not -O1.  Not to say that is the best answer, but I think
that is why we did the same.  I agree I don't think -free can produce
worse code which makes using it with -O1 and above an option.  Maybe someone
was worried about compile time???  Doesn't seem like an optimization like
this would be too expensive though.

Ajit, one thing that is missing from this specific patch is a change to
gcc/doc/invoke.texi mentioning Power to the list of architectures that
are enabling -free with the -O* options, which currently only mentions
Alpha, AArch64 and x86.  Being good community participants, it'd be
good to add the missing riscv, sparc and h8300 when adding Power.

Peter

On Mon, Apr 24, 2023 at 10:23:06AM -0500, Peter Bergner wrote:
> On 4/19/23 3:00 PM, Segher Boessenkool wrote:
> > On Wed, Apr 19, 2023 at 11:23:07PM +0530, Ajit Agarwal wrote:
> >> 	* common/config/rs6000/rs6000-common.cc: Add REE pass as a
> >> 	default rs6000 target pass for O2 and above.
> > 
> > Why only for -O2?  Only when optimising at all makes sense, people use
> > -O0 only when they want to skip as many optimisations as possible, maybe
> > because of compilation time concerns, maybe to avoid an ICE or other
> > bug.  Isn't REE *always* a good thing, it never degrades code quality?
> > Or are there situations where it results in worse code?
> 
> I think this is a case of following what the other architectures are doing.
> Namely, x86, aarch64, riscv, sparc, alpha and h8300 all enable -free at
> -O2 and above, not -O1.  Not to say that is the best answer, but I think
> that is why we did the same.  I agree I don't think -free can produce
> worse code which makes using it with -O1 and above an option.  Maybe someone
> was worried about compile time???  Doesn't seem like an optimization like
> this would be too expensive though.

I thought that
  df_chain_add_problem (DF_UD_CHAIN + DF_DU_CHAIN);
is quite expensive (only other pass which does that is SMS pass) and
  df_mir_add_problem ();
as well (REE pass being the only user of that).  As -O1 is meant to scale
well on huge compiler generated functions, perhaps REE isn't appropriate
for those by default.

	Jakub

On 4/24/23 10:28 AM, Jakub Jelinek wrote:
> On Mon, Apr 24, 2023 at 10:23:06AM -0500, Peter Bergner wrote:
>> On 4/19/23 3:00 PM, Segher Boessenkool wrote:
>>> On Wed, Apr 19, 2023 at 11:23:07PM +0530, Ajit Agarwal wrote:
>>>> 	* common/config/rs6000/rs6000-common.cc: Add REE pass as a
>>>> 	default rs6000 target pass for O2 and above.
>>>
>>> Why only for -O2?  Only when optimising at all makes sense, people use
>>> -O0 only when they want to skip as many optimisations as possible, maybe
>>> because of compilation time concerns, maybe to avoid an ICE or other
>>> bug.  Isn't REE *always* a good thing, it never degrades code quality?
>>> Or are there situations where it results in worse code?
>>
>> I think this is a case of following what the other architectures are doing.
>> Namely, x86, aarch64, riscv, sparc, alpha and h8300 all enable -free at
>> -O2 and above, not -O1.  Not to say that is the best answer, but I think
>> that is why we did the same.  I agree I don't think -free can produce
>> worse code which makes using it with -O1 and above an option.  Maybe someone
>> was worried about compile time???  Doesn't seem like an optimization like
>> this would be too expensive though.
> 
> I thought that
>   df_chain_add_problem (DF_UD_CHAIN + DF_DU_CHAIN);
> is quite expensive (only other pass which does that is SMS pass) and
>   df_mir_add_problem ();
> as well (REE pass being the only user of that).  As -O1 is meant to scale
> well on huge compiler generated functions, perhaps REE isn't appropriate
> for those by default.

Ah, so it is an issue with compile time then.  If so, then sure, being -O2
and above makes sense then.  Thanks for pointing that out!

Peter