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.


Segher Boessenkool <segher@kernel.crashing.org> writes:
> 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?

Yeah, meant hard REG_USERVAR_P, sorry, as for the patch.

>> 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. ]
>
> 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?

Whether it does not, making the test unnecessarily wide is at best only
going to paper over problems elsewhere.  I really think we should test
for i3 being an asm.

Thanks,
Richard


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