PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[H.J. Lu]

Hi,

RTL-based forward propagation pass shouldn't propagate hard register.
OK for trunk?

Thanks.


H.J.
---
2011-06-14  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/47449
	* fwprop.c (forward_propagate_subreg): Don't propagate hard
	register nor zero/sign extended hard register.

diff --git a/gcc/fwprop.c b/gcc/fwprop.c
index b2fd955..c8009d0 100644
--- a/gcc/fwprop.c
+++ b/gcc/fwprop.c
@@ -1101,6 +1101,7 @@ forward_propagate_subreg (df_ref use, rtx def_insn, rtx def_set)
       src = SET_SRC (def_set);
       if (GET_CODE (src) == SUBREG
 	  && REG_P (SUBREG_REG (src))
+	  && REGNO (SUBREG_REG (src)) >= FIRST_PSEUDO_REGISTER
 	  && GET_MODE (SUBREG_REG (src)) == use_mode
 	  && subreg_lowpart_p (src)
 	  && all_uses_available_at (def_insn, use_insn))
@@ -1119,6 +1120,7 @@ forward_propagate_subreg (df_ref use, rtx def_insn, rtx def_set)
       if ((GET_CODE (src) == ZERO_EXTEND
 	   || GET_CODE (src) == SIGN_EXTEND)
 	  && REG_P (XEXP (src, 0))
+	  && REGNO (XEXP (src, 0)) >= FIRST_PSEUDO_REGISTER
 	  && GET_MODE (XEXP (src, 0)) == use_mode
 	  && !free_load_extend (src, def_insn)
 	  && all_uses_available_at (def_insn, use_insn))

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[Richard Sandiford]

"H.J. Lu" <hongjiu.lu@intel.com> writes:
> RTL-based forward propagation pass shouldn't propagate hard register.

That's seems a bit draconian.  Many fixed hard registers ought to be OK.
E.g. there doesn't seem to be anything wrong with propagating uses of
the stack or frame pointers, subject to the usual availability checks.

To play devil's advocate, an alternative might be to

(a) make local_ref_killed_between_p return true for non-fixed hard
    registers when a call or asm comes between the two instructions

(b) make use_killed_between return true for non-fixed hard registers
    when the instructions are in different basic blocks

Thoughts?

Richard

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[H.J. Lu]

On Mon, Jul 4, 2011 at 12:57 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>> RTL-based forward propagation pass shouldn't propagate hard register.
>
> That's seems a bit draconian.  Many fixed hard registers ought to be OK.
> E.g. there doesn't seem to be anything wrong with propagating uses of
> the stack or frame pointers, subject to the usual availability checks.
>
> To play devil's advocate, an alternative might be to
>
> (a) make local_ref_killed_between_p return true for non-fixed hard
>    registers when a call or asm comes between the two instructions
>
> (b) make use_killed_between return true for non-fixed hard registers
>    when the instructions are in different basic blocks
>
> Thoughts?
>

There are a few problems with this suggestions:

1. The comments says:

/* If USE is a subreg, see if it can be replaced by a pseudo.  */

static bool
forward_propagate_subreg (df_ref use, rtx def_insn, rtx def_set)
{

It indicates this function is intended to work on pseudo registers.

2. propagate_rtx avoids hard registers:

static rtx
propagate_rtx (rtx x, enum machine_mode mode, rtx old_rtx, rtx new_rtx,
               bool speed)
{
  rtx tem;
  bool collapsed;
  int flags;

  if (REG_P (new_rtx) && REGNO (new_rtx) < FIRST_PSEUDO_REGISTER)
    return NULL_RTX;

It seems that fwprop is intended to deal with pseudo registers.  If we
want to extend it to hard registers, that should be a separate project.

Thanks.

-- 
H.J.

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[H.J. Lu]

On Mon, Jul 4, 2011 at 1:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Jul 4, 2011 at 12:57 PM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> "H.J. Lu" <hongjiu.lu@intel.com> writes:
>>> RTL-based forward propagation pass shouldn't propagate hard register.
>>
>> That's seems a bit draconian.  Many fixed hard registers ought to be OK.
>> E.g. there doesn't seem to be anything wrong with propagating uses of
>> the stack or frame pointers, subject to the usual availability checks.
>>
>> To play devil's advocate, an alternative might be to
>>
>> (a) make local_ref_killed_between_p return true for non-fixed hard
>>    registers when a call or asm comes between the two instructions
>>
>> (b) make use_killed_between return true for non-fixed hard registers
>>    when the instructions are in different basic blocks
>>
>> Thoughts?
>>
>
> There are a few problems with this suggestions:
>
> 1. The comments says:
>
> /* If USE is a subreg, see if it can be replaced by a pseudo.  */
>
> static bool
> forward_propagate_subreg (df_ref use, rtx def_insn, rtx def_set)
> {
>
> It indicates this function is intended to work on pseudo registers.
>
> 2. propagate_rtx avoids hard registers:
>
> static rtx
> propagate_rtx (rtx x, enum machine_mode mode, rtx old_rtx, rtx new_rtx,
>               bool speed)
> {
>  rtx tem;
>  bool collapsed;
>  int flags;
>
>  if (REG_P (new_rtx) && REGNO (new_rtx) < FIRST_PSEUDO_REGISTER)
>    return NULL_RTX;
>
> It seems that fwprop is intended to deal with pseudo registers.  If we
> want to extend it to hard registers, that should be a separate project.
>
> Thanks.

forward_propagate_subreg issue was introduced by

http://gcc.gnu.org/ml/gcc-patches/2009-08/msg01203.html

Before that,  fwprop never tries to work on hard registers.  Alan,
is your change to process hard registers intentional?

Thanks.


-- 
H.J.

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[Alan Modra]

On Mon, Jul 04, 2011 at 01:57:34PM -0700, H.J. Lu wrote:
> forward_propagate_subreg issue was introduced by
> 
> http://gcc.gnu.org/ml/gcc-patches/2009-08/msg01203.html
> 
> Before that,  fwprop never tries to work on hard registers.

I question this claim.  It seems to me that fwprop did look at
paradoxical subregs of hard regs before my change.

>  Alan,
> is your change to process hard registers intentional?

I didn't set out to do anything special with hard regs one way or the
other, just extended what was already done for paradoxical subregs to
sign and zero extended subregs.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[H.J. Lu]

On Mon, Jul 4, 2011 at 4:54 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Jul 04, 2011 at 01:57:34PM -0700, H.J. Lu wrote:
>> forward_propagate_subreg issue was introduced by
>>
>> http://gcc.gnu.org/ml/gcc-patches/2009-08/msg01203.html
>>
>> Before that,  fwprop never tries to work on hard registers.
>
> I question this claim.  It seems to me that fwprop did look at
> paradoxical subregs of hard regs before my change.

I should have said " fwprop never tries to work on zero/sign-extended
hard registers."

>>  Alan,
>> is your change to process hard registers intentional?
>
> I didn't set out to do anything special with hard regs one way or the
> other, just extended what was already done for paradoxical subregs to
> sign and zero extended subregs.
>

Does your change depend on processing zero/sign-extended
hard registers?

-- 
H.J.

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[Alan Modra]

On Mon, Jul 04, 2011 at 05:09:28PM -0700, H.J. Lu wrote:
> On Mon, Jul 4, 2011 at 4:54 PM, Alan Modra <amodra@gmail.com> wrote:
> > I didn't set out to do anything special with hard regs one way or the
> > other, just extended what was already done for paradoxical subregs to
> > sign and zero extended subregs.
> 
> Does your change depend on processing zero/sign-extended
> hard registers?

At the time I wrote the patch I was more interested in pseudos.  I
expect that powerpc64 won't be greatly affected if hard regs were
excluded from this fwprop optimization, but you need to discuss your
patch with maintainers of this code.  My opinion as a one-time
contributor to fwprop doesn't count for much.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[Paolo Bonzini]

On 07/05/2011 01:54 AM, Alan Modra wrote:
> >  Before that,  fwprop never tries to work on hard registers.
>
> I question this claim.  It seems to me that fwprop did look at
> paradoxical subregs of hard regs before my change.

That wasn't part of the design anyway.  The main purpose of fwprop's 
paradoxical subreg propagation is to get rid of back-to-back 
SI-to-QI-to-SI conversions, and working with pseudos is enough.

The patch is okay as far as I'm concerned, but I'm not a maintainer of 
fwprop.  Perhaps it could be conditionalized on SMALL_REGISTER_CLASSES 
(that's the source of the bug, I think: the single-register class "D" 
conflicts with an argument register) but I don't think it's worth it.

Paolo

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[Paolo Bonzini]

On 07/05/2011 01:54 AM, Alan Modra wrote:
> >  Before that,  fwprop never tries to work on hard registers.
>
> I question this claim.  It seems to me that fwprop did look at
> paradoxical subregs of hard regs before my change.

That wasn't part of the design anyway.  The main purpose of fwprop's 
paradoxical subreg propagation is to get rid of back-to-back 
SI-to-QI-to-SI conversions, and working with pseudos is enough.

The patch is okay as far as I'm concerned, but I'm not a maintainer of 
fwprop.  Perhaps it could be conditionalized on SMALL_REGISTER_CLASSES 
(that's the source of the bug, I think: the single-register class "D" 
conflicts with an argument register) but I don't think it's worth it.

Paolo

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[Richard Sandiford]

Paolo Bonzini <bonzini@gnu.org> writes:
> On 07/05/2011 01:54 AM, Alan Modra wrote:
>> >  Before that,  fwprop never tries to work on hard registers.
>>
>> I question this claim.  It seems to me that fwprop did look at
>> paradoxical subregs of hard regs before my change.
>
> That wasn't part of the design anyway.  The main purpose of fwprop's 
> paradoxical subreg propagation is to get rid of back-to-back 
> SI-to-QI-to-SI conversions, and working with pseudos is enough.

OK, in that case H.J., please go ahead.

> The patch is okay as far as I'm concerned, but I'm not a maintainer of 
> fwprop.

You probably should be :-)

Richard

Re: PATCH [6/n]: Prepare x32: PR rtl-optimization/47449: Don't propagate hard register non-local goto save area

[Paolo Bonzini]

On 07/05/2011 10:51 AM, Richard Sandiford wrote:
> >  The patch is okay as far as I'm concerned, but I'm not a maintainer of
> >  fwprop.
>
> You probably should be:-)

I'd have no problem with that!

Paolo