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

[Richard Sandiford]

"Hale Wang" <hale.wang@arm.com> writes:
> Ping?
>
>> -----Original Message-----
>> From: Hale Wang [mailto:hale.wang@arm.com]
>> Sent: Thursday, January 29, 2015 9:58 AM
>> To: Hale Wang; 'Segher Boessenkool'
>> Cc: GCC Patches
>> Subject: RE: [PATCH] [gcc, combine] PR46164: Don't combine the insns if a
>> volatile register is contained.
>> 
>> Hi Segher,
>> 
>> I have updated the patch as you suggested. Both the patch and the
>> changelog are attached.
>> 
>> By the way, the test case provided by Tim Pambor in PR46164 was a
> different
>> bug with PR46164. So I resubmitted the bug in
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64818.
>> And this patch is just used to fix this bug. Is it OK for you?
>> 
>> Thanks,
>> Hale
>> 
>> gcc/ChangeLog:
>> 2015-01-27  Segher Boessenkool  <segher@kernel.crashing.org>
>> 	    Hale Wang  <hale.wang@arm.com>
>> 
>> 	PR rtl-optimization/64818
>> 	* combine.c (can_combine_p): Don't combine the insn if
>> 	the dest of insn is a user specified register.
>> 
>> gcc/testsuit/ChangeLog:
>> 2015-01-27  Segher Boessenkool  <segher@kernel.crashing.org>
>> 	    Hale Wang  <hale.wang@arm.com>
>> 
>> 	PR rtl-optimization/64818
>> 	* gcc.target/arm/pr64818.c: New test.
>> 
>> 
>> 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.
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.

Thanks,
Richard