Patch submissions and discussion of these patches

Igor Zamyatin | 27 Jun 2012 18:07

Picon

Re: [PATCH 1/3] Add rtx costs for sse integer ops

May I ask about the purpose of the following piece of change? Doesn't
it affect non-sse cases either?

 <at>  <at>  -32038,7 +32042,15  <at>  <at>  ix86_rtx_costs (rtx x, int code, int
outer_code_i, int opno, int *total,
    case ASHIFTRT:
    case LSHIFTRT:
    case ROTATERT:
-      if (!TARGET_64BIT && GET_MODE (XEXP (x, 0)) == DImode)
+      if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
+       {
+         /* ??? Should be SSE vector operation cost.  */
+         /* At least for published AMD latencies, this really is the same
+            as the latency for a simple fpu operation like fabs.  */
+         *total = cost->fabs;
+         return false;
+       }
+      if (GET_MODE_SIZE (mode) < UNITS_PER_WORD)
       {
         if (CONST_INT_P (XEXP (x, 1)))
           {

It also seems that we reversed the condition for the code that is now
under if (GET_MODE_SIZE (mode) < UNITS_PER_WORD). Why do we need this?

Thanks,
Igor

-----Original Message-----
From: gcc-patches-owner <at> gcc.gnu.org

(Continue reading)

Richard Henderson | 27 Jun 2012 19:00

Picon

Re: [PATCH 1/3] Add rtx costs for sse integer ops

On 06/27/2012 09:07 AM, Igor Zamyatin wrote:
> May I ask about the purpose of the following piece of change? Doesn't
> it affect non-sse cases either?

Err, no, it doesn't affect non-sse cases.  All MODE_VECTOR_INT
cases will be implemented in the xmm registers (ignoring the
deprecated and largely ignored mmx case).

> 
>  <at>  <at>  -32038,7 +32042,15  <at>  <at>  ix86_rtx_costs (rtx x, int code, int
> outer_code_i, int opno, int *total,
>     case ASHIFTRT:
>     case LSHIFTRT:
>     case ROTATERT:
> -      if (!TARGET_64BIT && GET_MODE (XEXP (x, 0)) == DImode)
> +      if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
> +       {
> +         /* ??? Should be SSE vector operation cost.  */
> +         /* At least for published AMD latencies, this really is the same
> +            as the latency for a simple fpu operation like fabs.  */
> +         *total = cost->fabs;
> +         return false;
> +       }
> +      if (GET_MODE_SIZE (mode) < UNITS_PER_WORD)
>        {
>          if (CONST_INT_P (XEXP (x, 1)))
>            {
> 
> It also seems that we reversed the condition for the code that is now
> under if (GET_MODE_SIZE (mode) < UNITS_PER_WORD). Why do we need this?

(Continue reading)

Igor Zamyatin | 27 Jun 2012 19:08

Picon

Re: [PATCH 1/3] Add rtx costs for sse integer ops

On Wed, Jun 27, 2012 at 9:00 PM, Richard Henderson <rth <at> redhat.com> wrote:
> On 06/27/2012 09:07 AM, Igor Zamyatin wrote:
>> May I ask about the purpose of the following piece of change? Doesn't
>> it affect non-sse cases either?
>
> Err, no, it doesn't affect non-sse cases.  All MODE_VECTOR_INT
> cases will be implemented in the xmm registers (ignoring the
> deprecated and largely ignored mmx case).

Probably I misunderstand something... This condition - GET_MODE_SIZE
(mode) < UNITS_PER_WORD - is outside the check for MODE_VECTOR_INT.

>
>
>>
>>  <at>  <at>  -32038,7 +32042,15  <at>  <at>  ix86_rtx_costs (rtx x, int code, int
>> outer_code_i, int opno, int *total,
>>     case ASHIFTRT:
>>     case LSHIFTRT:
>>     case ROTATERT:
>> -      if (!TARGET_64BIT && GET_MODE (XEXP (x, 0)) == DImode)
>> +      if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
>> +       {
>> +         /* ??? Should be SSE vector operation cost.  */
>> +         /* At least for published AMD latencies, this really is the same
>> +            as the latency for a simple fpu operation like fabs.  */
>> +         *total = cost->fabs;
>> +         return false;
>> +       }
>> +      if (GET_MODE_SIZE (mode) < UNITS_PER_WORD)

(Continue reading)

Richard Henderson | 27 Jun 2012 19:14

Picon

Re: [PATCH 1/3] Add rtx costs for sse integer ops

On 06/27/2012 10:08 AM, Igor Zamyatin wrote:
> On Wed, Jun 27, 2012 at 9:00 PM, Richard Henderson <rth <at> redhat.com> wrote:
>> > On 06/27/2012 09:07 AM, Igor Zamyatin wrote:
>>> >> May I ask about the purpose of the following piece of change? Doesn't
>>> >> it affect non-sse cases either?
>> >
>> > Err, no, it doesn't affect non-sse cases.  All MODE_VECTOR_INT
>> > cases will be implemented in the xmm registers (ignoring the
>> > deprecated and largely ignored mmx case).
> Probably I misunderstand something... This condition - GET_MODE_SIZE
> (mode) < UNITS_PER_WORD - is outside the check for MODE_VECTOR_INT.

Of course.

We currently have

	if (vector mode)
	else if (mode <= word size)
	else /* scalar mode > word size */

We could no doubt legitimately rearrange this to

	if (mode < word size)
	else if (vector mode)
	else /* scalar mode > word size */

but I don't see how that's any clearer.  We certainly
can't eliminate any tests, since there are in fact
three different possibilities.

(Continue reading)

Igor Zamyatin | 27 Jun 2012 19:24

Picon

Re: [PATCH 1/3] Add rtx costs for sse integer ops

On Wed, Jun 27, 2012 at 9:14 PM, Richard Henderson <rth <at> redhat.com> wrote:
> On 06/27/2012 10:08 AM, Igor Zamyatin wrote:
>> On Wed, Jun 27, 2012 at 9:00 PM, Richard Henderson <rth <at> redhat.com> wrote:
>>> > On 06/27/2012 09:07 AM, Igor Zamyatin wrote:
>>>> >> May I ask about the purpose of the following piece of change? Doesn't
>>>> >> it affect non-sse cases either?
>>> >
>>> > Err, no, it doesn't affect non-sse cases.  All MODE_VECTOR_INT
>>> > cases will be implemented in the xmm registers (ignoring the
>>> > deprecated and largely ignored mmx case).
>> Probably I misunderstand something... This condition - GET_MODE_SIZE
>> (mode) < UNITS_PER_WORD - is outside the check for MODE_VECTOR_INT.
>
> Of course.
>
> We currently have
>
>        if (vector mode)
>        else if (mode <= word size)
>        else /* scalar mode > word size */

Sure, it's clear. So am I correct that in this case now for second
possibility we run the code which was executed for third possibility
before the patch (it was under TARGET_64BIT && GET_MODE (XEXP (x, 0))
== DImode)? If yes, why?

>
> We could no doubt legitimately rearrange this to
>
>        if (mode < word size)

(Continue reading)

Richard Henderson | 27 Jun 2012 23:36

Picon

[PATCH] i386: Fix logic error in r188785

As noticed by Igor Zamyatin.  Committed.

r~
---

PR target/53749
        * config/i386/i386.c (ix86_rtx_costs): Fix typo vs UNITS_PER_WORD
        in 2012-06-23 change.  Adjust two other DImode tests as well.
---
 gcc/ChangeLog          |    6 ++++++
 gcc/config/i386/i386.c |    8 +++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index edfc649..aae8a4d 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
 <at>  <at>  -32210,9 +32210,8  <at>  <at>  ix86_rtx_costs (rtx x, int code_i, int outer_code_i, int opno, int *total,
 	    }
 	  else
 	    *total = cost->fabs;
-	  return false;
 	}
-      if (GET_MODE_SIZE (mode) < UNITS_PER_WORD)
+      else if (GET_MODE_SIZE (mode) > UNITS_PER_WORD)
 	{
 	  if (CONST_INT_P (XEXP (x, 1)))
 	    {
 <at>  <at>  -32441,7 +32440,7  <at>  <at>  ix86_rtx_costs (rtx x, int code_i, int outer_code_i, int opno, int *total,
     case AND:

(Continue reading)

Jakub Jelinek | 18 Sep 2012 14:10

Picon

[PATCH] Fix i386 costs (was: i386: Fix logic error in r188785, PR target/54592)

On Wed, Jun 27, 2012 at 02:36:14PM -0700, Richard Henderson wrote:
> As noticed by Igor Zamyatin.  Committed.

> PR target/53749
>         * config/i386/i386.c (ix86_rtx_costs): Fix typo vs UNITS_PER_WORD
>         in 2012-06-23 change.  Adjust two other DImode tests as well.

This change broke cost computation for vector PLUS/MINUS/AND/IOR/XOR,
the vector modes are all wider than word, but they are now all handled as
double word integer arithmetics.

> ---
>  gcc/ChangeLog          |    6 ++++++
>  gcc/config/i386/i386.c |    8 +++-----
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index edfc649..aae8a4d 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
...
>  <at>  <at>  -32441,7 +32440,7  <at>  <at>  ix86_rtx_costs (rtx x, int code_i, int outer_code_i, int opno, int *total,
>      case AND:
>      case IOR:
>      case XOR:
> -      if (!TARGET_64BIT && mode == DImode)
> +      if (GET_MODE_SIZE (mode) > UNITS_PER_WORD)
>  	{
>  	  *total = (cost->add * 2
>  		    + (rtx_cost (XEXP (x, 0), outer_code, opno, speed)

(Continue reading)

Richard Henderson | 18 Sep 2012 17:05

Picon

Re: [PATCH] Fix i386 costs

On 09/18/2012 05:10 AM, Jakub Jelinek wrote:
> 2012-09-18  Jakub Jelinek  <jakub <at> redhat.com>
> 
> 	PR target/54592
> 	* config/i386/i386.c (ix86_rtx_costs): Limit > UNITS_PER_WORD
> 	AND/IOR/XOR cost calculation to MODE_INT class modes.
> 
> 	* gcc.target/i386/pr54592.c: New test.

Ok.

r~

Return

Return to gmane.comp.gcc.patches.

Advertisement

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane