This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [PATCH, GCC, stage1] Fallback to copy-prop if constant-prop not possible
- From: "Thomas Preud'homme" <thomas dot preudhomme at arm dot com>
- To: "'Jeff Law'" <law at redhat dot com>, "Steven Bosscher" <stevenb dot gcc at gmail dot com>, <gcc-patches at gcc dot gnu dot org>, "'Richard Biener'" <rguenther at suse dot de>
- Date: Fri, 24 Apr 2015 11:10:14 +0800
- Subject: RE: [PATCH, GCC, stage1] Fallback to copy-prop if constant-prop not possible
- Authentication-results: sourceware.org; auth=none
- References: <000501d049d3$079385a0$16ba90e0$ at arm dot com> <552BBAF9 dot 2010504 at redhat dot com> <000001d07821$6fb82f60$4f288e20$ at arm dot com> <5539B177 dot 8020705 at redhat dot com>
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Friday, April 24, 2015 10:59 AM
>
Hi Jeff,
> > +
> > +static bool
> > +cprop_reg_p (const_rtx x)
> > +{
> > + return REG_P (x) && !HARD_REGISTER_P (x);
> > +}
> How about instead this move to a more visible location (perhaps a macro
> in regs.h or an inline function). Then as a followup, change the
> various places that have this sequence to use that common definition
> that exist outside of cprop.c.
According to Steven this was proposed in the past but was refused (see
end of [1]).
[1] https://gcc.gnu.org/ml/gcc-patches/2015-03/msg01066.html
>
> > @@ -1191,7 +1192,7 @@ do_local_cprop (rtx x, rtx_insn *insn)
> > /* Rule out USE instructions and ASM statements as we don't want
> to
> > change the hard registers mentioned. */
> > if (REG_P (x)
> > - && (REGNO (x) >= FIRST_PSEUDO_REGISTER
> > + && (cprop_reg_p (x)
> > || (GET_CODE (PATTERN (insn)) != USE
> > && asm_noperands (PATTERN (insn)) < 0)))
> Isn't the REG_P test now redundant?
I made the same mistake when reviewing that change and indeed it's not.
Note the opening parenthesis before cprop_reg_p that contains a bitwise
OR expression. So in the case where cprop_reg_p is false, REG_P still
needs to be true.
We could keep a check on FIRST_PSEUDO_REGISTER but the intent (checking
that the register is suitable for propagation) is clearer now, as pointed out by
Steven to me.
>
> OK for the trunk with those changes.
>
> jeff
Given the above I intent to keep the REG_P in the second excerpt and will
wait for your input about moving cprop_reg_p to rtl.h
Best regards,
Thomas