This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: Ping : [PATCH] [gcc, combine] PR46164: Don't combine the insns if a volatile register is contained.


> -----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





Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]