PATCH: Rest of Intel Atom optimization

[Uros Bizjak]

H.J. Lu wrote:
> 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?
>   
>  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.

The updated patch is OK for mainline. Perhaps you should post a full 
patch and wait a day or two for possible comments on DF usage from a DF 
expert (Hello Steven! :) .

Thanks,
Uros.