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: PATCH: Rest of Intel Atom optimization


On Sun, Apr 19, 2009 at 10:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> 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! :) .
>

Here is the complete patch. I will wait for a day before checking it in.

Thanks.


-- 
H.J.
---
2009-04-17  Joey Ye  <joey.ye@intel.com>
	    Xuepeng Guo <xuepeng.guo@intel.com>
	    H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/atom.md: Add bypasses with ix86_dep_by_shift_count.

	* config/i386/i386.c (LEA_SEARCH_THRESHOLD): New macro.
	(IX86_LEA_PRIORITY): Likewise.
	(distance_non_agu_define): New function.
	(distance_agu_use): Likewise.
	(ix86_lea_for_add_ok): Likewise.
	(ix86_dep_by_shift_count): Likewise.

	* config/i386/i386.md: Call ix86_lea_for_add_ok to decide we
	should split for LEA.

	* config/i386/i386-protos.h (ix86_lea_for_add_ok): Declare new
	function.
	(ix86_dep_by_shift_count): Likewise.

Attachment: gcc-atom-4.patch
Description: Text document


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