Re: PATCH: Rest of Intel Atom optimization

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Sun, Apr 19, 2009 at 9:45 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> H.J. Lu wrote:
>
>>>> Here is the rest of Intel Atom optimization. OK for trunk?
>>>>
>>>> ? ? ?+static int
>>>> +distance_non_agu_define (rtx op1, rtx op2, rtx insn)
>>>> +{
>>>> + ?unsigned int regno1 = REGNO (op1);
>>>> + ?unsigned int regno2 = REG_P (op2) ? REGNO (op2) : (unsigned int) -1;
>>>>
>>>
>>> Hm, not true_regnum in the lines above?
>>>
>>
>> We know op1 is a register and op2 isn't a subreg. ?Do
>> we need to call ?true_regnum?
>>
>
> true_regnum also handles:
>
> ?if (REG_P (x))
> ? {
> ? ? if (REGNO (x) >= FIRST_PSEUDO_REGISTER && reg_renumber[REGNO (x)] >= 0)
> ? return reg_renumber[REGNO (x)];
> ? ? return REGNO (x);
> ? }
> ?...
>
> and _if_ op1 (or op2, FWIW) is a subreg, it won't crash... I think that
> calling true_regnum is the way to go, even if we are ATM sure that the
> operand won't be a SUBREG.
>
>>>> + ? ? ? ? ? ? ? ? {
>>>> + ? ? ? ? ? ? ? ? ? insn_type = get_attr_type (prev);
>>>> + ? ? ? ? ? ? ? ? ? if (insn_type != TYPE_LEA)
>>>> + ? ? ? ? ? ? ? ? ? ? goto done;
>>>> + ? ? ? ? ? ? ? ? }
>>>>
>>>
>>> ...
>>>
>>>
>>>>
>>>> +done:
>>>> + ?/* Restore recog_data which may be modified by get_attr_type. ?*/
>>>> + ?extract_insn_cached (insn);
>>>> + ?return distance;
>>>>
>>>
>>> This can't be right. You are restoring recog data on a different insn
>>> that
>>> was processed by get_attr_type. I think that extract_insn_cached should
>>> immediately follow get_attr_type in order to restore recog data.
>>>
>>>
>>
>> We want to restore recog data on the instruction, which is the
>> parameter, INSN, on which distance_non_agu_define is called.
>> INSN is unchanged here. Did I miss something?
>>
>
> No, ?I think I got the logic wrong. We want recog data to correspond to
> INSN, is this correct? If this is the case, please describe this in the
> comment, since this action on global variables is kind of non-intuitive and
> a bit tricky to understand.
>

Hi Uros,

I checked this patch into Atom branch. Is the updated Atom patch
OK for trunk?

Thanks.


-- 
H.J.
---
2009-04-19  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (distance_non_agu_define): Use register
	numbers as parameters.  Update comments.
	(distance_agu_use): Use register number parameter.
	(ix86_lea_for_add_ok): Updated.

Attachment: gcc-atom-reg-1.patch
Description: Text document