This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PATCH: Update ix86_agi_dependent
On Sun, Mar 29, 2009 at 11:34:41AM +0200, Uros Bizjak wrote:
> H.J. Lu wrote:
>> 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.
>>
>
> The patch is OK, but please fix operand order at all callsites (see above).
>
This is what I checked in. I also added ix86_agi_dependent to
PROCESSOR_PENTIUM for insn_type != TYPE_LEA.
H.J.
----
2009-03-29 H.J. Lu <hongjiu.lu@intel.com>
* config/i386/i386-protos.h (ix86_agi_dependent): New.
* config/i386/i386.c (ix86_agi_dependent): Rewrite.
(ix86_adjust_cost): Updated.
--- config/i386/i386-protos.h.agi 2009-03-28 08:39:48.000000000 -0700
+++ config/i386/i386-protos.h 2009-03-29 07:41:28.000000000 -0700
@@ -85,6 +85,7 @@ extern void ix86_fixup_binary_operands_n
extern void ix86_expand_binary_operator (enum rtx_code,
enum machine_mode, rtx[]);
extern int ix86_binary_operator_ok (enum rtx_code, enum machine_mode, rtx[]);
+extern bool ix86_agi_dependent (rtx set_insn, rtx use_insn);
extern void ix86_expand_unary_operator (enum rtx_code, enum machine_mode,
rtx[]);
extern rtx ix86_build_const_vector (enum machine_mode, bool, rtx);
--- config/i386/i386.c.agi 2009-03-28 08:39:48.000000000 -0700
+++ config/i386/i386.c 2009-03-29 07:46:45.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)
{
- rtx addr;
-
- if (insn_type == TYPE_LEA
- && TARGET_PENTIUM)
- {
- addr = PATTERN (insn);
-
- if (GET_CODE (addr) == PARALLEL)
- addr = XVECEXP (addr, 0, 0);
-
- gcc_assert (GET_CODE (addr) == SET);
-
- addr = SET_SRC (addr);
- }
- else
- {
- int i;
- extract_insn_cached (insn);
- for (i = recog_data.n_operands - 1; i >= 0; --i)
- if (MEM_P (recog_data.operand[i]))
- {
- addr = XEXP (recog_data.operand[i], 0);
- goto found;
- }
- return 0;
- found:;
- }
-
- return modified_in_p (addr, dep_insn);
+ int i;
+ extract_insn_cached (use_insn);
+ for (i = recog_data.n_operands - 1; i >= 0; --i)
+ if (MEM_P (recog_data.operand[i]))
+ {
+ rtx addr = XEXP (recog_data.operand[i], 0);
+ return modified_in_p (addr, set_insn) != 0;
+ }
+ return false;
}
static int
@@ -19132,7 +19112,20 @@ ix86_adjust_cost (rtx insn, rtx link, rt
{
case PROCESSOR_PENTIUM:
/* Address Generation Interlock adds a cycle of latency. */
- if (ix86_agi_dependent (insn, dep_insn, insn_type))
+ if (insn_type == TYPE_LEA)
+ {
+ rtx addr = PATTERN (insn);
+
+ if (GET_CODE (addr) == PARALLEL)
+ addr = XVECEXP (addr, 0, 0);
+
+ gcc_assert (GET_CODE (addr) == SET);
+
+ addr = SET_SRC (addr);
+ if (modified_in_p (addr, dep_insn))
+ cost += 1;
+ }
+ else if (ix86_agi_dependent (dep_insn, insn))
cost += 1;
/* ??? Compares pair with jump/setcc. */
@@ -19142,7 +19135,7 @@ ix86_adjust_cost (rtx insn, rtx link, rt
/* Floating point stores require value to be ready one cycle earlier. */
if (insn_type == TYPE_FMOV
&& get_attr_memory (insn) == MEMORY_STORE
- && !ix86_agi_dependent (insn, dep_insn, insn_type))
+ && !ix86_agi_dependent (dep_insn, insn))
cost += 1;
break;
@@ -19165,7 +19158,7 @@ ix86_adjust_cost (rtx insn, rtx link, rt
in parallel with previous instruction in case
previous instruction is not needed to compute the address. */
if ((memory == MEMORY_LOAD || memory == MEMORY_BOTH)
- && !ix86_agi_dependent (insn, dep_insn, insn_type))
+ && !ix86_agi_dependent (dep_insn, insn))
{
/* Claim moves to take one cycle, as core can issue one load
at time and the next load can start cycle later. */
@@ -19194,7 +19187,7 @@ ix86_adjust_cost (rtx insn, rtx link, rt
in parallel with previous instruction in case
previous instruction is not needed to compute the address. */
if ((memory == MEMORY_LOAD || memory == MEMORY_BOTH)
- && !ix86_agi_dependent (insn, dep_insn, insn_type))
+ && !ix86_agi_dependent (dep_insn, insn))
{
/* Claim moves to take one cycle, as core can issue one load
at time and the next load can start cycle later. */
@@ -19219,7 +19212,7 @@ ix86_adjust_cost (rtx insn, rtx link, rt
in parallel with previous instruction in case
previous instruction is not needed to compute the address. */
if ((memory == MEMORY_LOAD || memory == MEMORY_BOTH)
- && !ix86_agi_dependent (insn, dep_insn, insn_type))
+ && !ix86_agi_dependent (dep_insn, insn))
{
enum attr_unit unit = get_attr_unit (insn);
int loadcost = 3;