This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: Ping : [PATCH] [gcc, combine] PR46164: Don't combine the insns if a volatile register is contained.
- From: "Hale Wang" <hale dot wang at arm dot com>
- To: "'GCC Patches'" <gcc-patches at gcc dot gnu dot org>, "Richard Sandiford" <Richard dot Sandiford at arm dot com>
- Cc: "'Segher Boessenkool'" <segher at kernel dot crashing dot org>
- Date: Fri, 13 Feb 2015 11:17:36 +0800
- Subject: RE: Ping : [PATCH] [gcc, combine] PR46164: Don't combine the insns if a volatile register is contained.
- Authentication-results: sourceware.org; auth=none
- References: <002701d04409$a20efcb0$e62cf610$ at arm dot com> <87pp9fcehe dot fsf at e105548-lin dot cambridge dot arm dot com> <20150212221552 dot GA25883 at gate dot crashing dot org>
> -----Original Message-----
> From: Segher Boessenkool [mailto:segher@kernel.crashing.org]
> Sent: Friday, February 13, 2015 6:16 AM
> To: Hale Wang; 'GCC Patches'; Richard Sandiford
> Subject: Re: Ping : [PATCH] [gcc, combine] PR46164: Don't combine the
insns
> if a volatile register is contained.
>
> On Thu, Feb 12, 2015 at 03:54:21PM +0000, Richard Sandiford wrote:
> > "Hale Wang" <hale.wang@arm.com> writes:
> > > Ping?
>
> It's not a regression (or is it?), so it is not appropriate for stage4.
>
>
> > >> diff --git a/gcc/combine.c b/gcc/combine.c index 5c763b4..6901ac2
> > >> 100644
> > >> --- a/gcc/combine.c
> > >> +++ b/gcc/combine.c
> > >> @@ -1904,6 +1904,12 @@ can_combine_p (rtx_insn *insn, rtx_insn *i3,
> > >> rtx_insn *pred ATTRIBUTE_UNUSED,
> > >> set = expand_field_assignment (set);
> > >> src = SET_SRC (set), dest = SET_DEST (set);
> > >>
> > >> + /* Don't combine if dest contains a user specified register,
> > >> + because
> > > the
> > >> + user specified register (same with dest) in i3 would be
> > >> + replaced by
> > > the
> > >> + src of insn which might be different with the user's
expectation.
> > >> + */ if (REG_P (dest) && REG_USERVAR_P (dest) &&
> HARD_REGISTER_P
> > >> (dest))
> > >> + return 0;
> >
> > I suppose this is similar to Andrew's comment, but I think the rule is
> > that it's invalid to replace a REG_USERVAR_P operand in an inline asm.
>
> Why not? You probably mean register asm, not all user variables?
>
> > Outside of an inline asm we make no guarantee about whether something
> > is stored in a particular register or not.
> >
> > So IMO we should be checking whether either INSN or I3 is an asm as
> > well as the above.
>
> [ INSN can never be an asm, that is already refused by can_combine_p. ]
>
Indeed. If INSN is an asm operand, it's already refused by can_combine_p.
Hale.
> We do not guarantee things will end up in the specified reg (except for
asm),
> but will it hurt to leave things in the reg the user said it should be in,
even if
> we do not guarantee this behaviour?
>
>
> Segher