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

[Terry Guo <flameroc at gmail dot com>]

On Thu, Feb 26, 2015 at 1:55 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Tue, Feb 17, 2015 at 11:39:34AM +0800, Terry Guo wrote:
>> On Sun, Feb 15, 2015 at 7:35 PM, Segher Boessenkool
>> <segher@kernel.crashing.org> wrote:
>> > Hi Terry,
>> >
>> > I still think this is stage1 material.
>> >
>> >> +  /* Don't combine if dest contains a user specified register and i3 contains
>> >> +     ASM_OPERANDS, 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.  */
>> >
>> > "Do not eliminate a register asm in an asm input" or similar?  Text
>> > explaining why REG_USERVAR_P && HARD_REGISTER_P works here would be
>> > good to have, too.
>
>> diff --git a/gcc/combine.c b/gcc/combine.c
>> index f779117..aeb2854 100644
>> --- a/gcc/combine.c
>> +++ b/gcc/combine.c
>> @@ -1779,7 +1779,7 @@ can_combine_p (rtx_insn *insn, rtx_insn *i3, rtx_insn *pred ATTRIBUTE_UNUSED,
>>  {
>>    int i;
>>    const_rtx set = 0;
>> -  rtx src, dest;
>> +  rtx src, dest, asm_op;
>>    rtx_insn *p;
>>  #ifdef AUTO_INC_DEC
>>    rtx link;
>> @@ -1914,6 +1914,14 @@ 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);
>>
>> +  /* Use REG_USERVAR_P and HARD_REGISTER_P to check whether DEST is a user
>> +     specified register, and do not eliminate such register if it is in an
>> +     asm input because we may end up with something different with user's
>> +     expectation.  */
>
> That doesn't explain why this will hit (almost) only on register asms.
> The user's expectation doesn't matter that much either: GCC would violate
> its own documentation / promises, that matters more ;-)
>
>> +  if (REG_P (dest) && REG_USERVAR_P (dest) && HARD_REGISTER_P (dest)
>> +      && ((asm_op = extract_asm_operands (PATTERN (i3))) != NULL))
>
> You do not need the temporary variable, nor the != 0 or the extra parens;
> just write
>
>      && extract_asm_operands (PATTERN (i3))
>
> Cheers,
>
>
> Segher

Thanks for comments. Patch is updated now. Please review again.

BR,
Terry

Attachment: pr64818-combine-user-specified-register.patch-5
Description: Binary data