Re: PATCH: Update ix86_agi_dependent

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

On Sat, Mar 28, 2009 at 12:14 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> H.J. Lu wrote:
>>
>> Here is a patch to update ix86_agi_dependent. It will be used by
>> upcoming Intel Atom optimization patches. This patch changes
>> ix86_agi_dependent to take SET_INSN and DEP_INSN. The special
>> case for TARGET_PENTIUM is moved to ix86_adjust_cost. OK for trunk?
>>
>> Thanks.
>>
>>
>> H.J.
>> -----
>> 2009-03-28 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>
>> ? ? ? ?* config/i386/i386.c (ix86_agi_dependent): Rewrite.
>> ? ? ? ?(ix86_adjust_cost): Updated.
>>
>> --- gcc/config/i386/i386.c.foo ?2009-03-28 08:39:48.000000000 -0700
>> +++ gcc/config/i386/i386.c ? ? ?2009-03-28 08:48:57.000000000 -0700
>> @@ -19070,41 +19070,21 @@ ix86_flags_dependent (rtx insn, rtx dep_
>> ? return 1;
>> ?}
>> ?-/* A subroutine of ix86_adjust_cost -- return true iff INSN has a memory
>> - ? address with operands set by DEP_INSN. ?*/
>> +/* Return true iff USE_INSN has a memory address with operands set by
>> + ? SET_INSN. ?*/
>> ?-static int
>> -ix86_agi_dependent (rtx insn, rtx dep_insn, enum attr_type insn_type)
>> +bool
>> +ix86_agi_dependent (rtx set_insn, rtx use_insn)
>>
>
> Is there a reason that the operand order is now switched? I would prefer to
> leave operand order as is, to avoid obvious mistakes, like:

atom.md has

(define_bypass 0 "atom_lea"
                 "atom_alu_mem, atom_alu_carry_mem, atom_alu1_mem,
                  atom_imovx_mem, atom_imovx_2_mem,
                  atom_imov_mem, atom_icmov_mem, atom_fmov_mem"
                 "!ix86_agi_dependent")

 The following construction is used to describe exceptions in the
latency time for given instruction pair.  This is so called bypasses.

     (define_bypass NUMBER OUT_INSN_NAMES IN_INSN_NAMES
                    [GUARD])

 NUMBER defines when the result generated by the instructions given in
string OUT_INSN_NAMES will be ready for the instructions given in
string IN_INSN_NAMES.  The instructions in the string are separated by
commas.

 GUARD is an optional string giving the name of a C function which
defines an additional guard for the bypass.  The function will get the
two insns as parameters.  If the function returns zero the bypass will
be ignored for this case.  The additional guard is necessary to
recognize complicated bypasses, e.g. when the consumer is only an
address of insn `store' (not a stored value).

We are matching the order of GUARD function.  Otherwise, we
need another function to return the same thing with the reversed
parameter order.

>> - ? ? ? ? && !ix86_agi_dependent (insn, dep_insn, insn_type))
>> + ? ? ? ? && !ix86_agi_dependent (dep_insn, insn))
>> ? ? ? ?cost += 1;
>> ? ? ? break;
>>
>
> vs.:
>
>> - ? ? ? ? && !ix86_agi_dependent (insn, dep_insn, insn_type))
>> + ? ? ? ? && !ix86_agi_dependent (insn, dep_insn))
>>
>
> Otherwise, the patch looks more like a cleanup - am I right that there is no
> functional change?
>

Yes.

Thanks.


-- 
H.J.