[Bug middle-end/37491] New: [4.4 Regression] Revision 140257 causes vectorizer tests failures

[Bug middle-end/37491] New: [4.4 Regression] Revision 140257 causes vectorizer tests failures

[hjl dot tools at gmail dot com]

On Linux/ia32, revision 140257 gave

FAIL: gcc.dg/vect/pr36493.c scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.target/i386/vectorize2.c scan-assembler cvtpd2dq
FAIL: gcc.target/i386/vectorize3.c scan-assembler cvtps2dq

Revision 140256 is OK.


-- 
           Summary: [4.4 Regression] Revision 140257 causes vectorizer tests
                    failures
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: hjl dot tools at gmail dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.4.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[irar at il dot ibm dot com]

------- Comment #1 from irar at il dot ibm dot com  2008-09-14 07:00 -------
I think those loops are not supposed to get vectorized. E.g., in
gcc.target/i386/vectorize2.c the store statement b[i_14] = D.1579_6 is not
vectorizable because vector long int (the vector type of the statement taken
from the type of D.1579_6) does not alias int (the type of b).
Since the use of the rhs type seems to be the best solution (for pr 37385) and
the alias sets have to be checked (pr 36493), I guess that the tests have to be
changed.  
In gcc.dg/vect/pr36493.c the vectorization check can be simply removed. In the
two other tests, changing the type of array b to long will help to vectorize
the loops. I think that does not matter for the tests, right?


-- 

irar at il dot ibm dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-09-14 07:00:50
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2008-09-14 09:32 -------
I don't follow.  For vectorize2.c we have

    b[i] = lrint (a[i]);

where we should be able to vectorize this using lrint vectorization and
a scalar long -> int conversion (which is a no-op on i686 and should be
vectorized on x86_64).  There is no aliasing issue involved here.

But yes, the testcase should be changed to actually check something more
meaningful than one or more cvtpd2dq instructions.

Same for vectorize3.c.

The loop in pr36493.c should also be vectorizable, there isn't any alias
problem there.

Oh well, I guess the alias check logic in the vectorizer is still bogus.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[irar at il dot ibm dot com]

------- Comment #3 from irar at il dot ibm dot com  2008-09-14 10:04 -------
(In reply to comment #2)
> I don't follow.  For vectorize2.c we have
> 
>     b[i] = lrint (a[i]);
> 
> where we should be able to vectorize this using lrint vectorization and
> a scalar long -> int conversion (which is a no-op on i686 and should be
> vectorized on x86_64).  There is no aliasing issue involved here.

I am working on x86_64 using -m32 (without -m32 I get
../../gcc.target/i386/vectorize2.c:15: note: function is not vectorizable.
../../gcc.target/i386/vectorize2.c:15: note: not vectorized: relevant stmt not
supported: D.1603_6 = lrint (D.1602_5); ).

I don't see any conversion:

foo ()
{
  unsigned int ivtmp.65;
  int pretmp.55;
  double pretmp.54;
  int i;
  long int D.1579;
  double D.1578;

<bb 2>:

<bb 3>:
  # i_14 = PHI <i_7(4), 0(2)>
  # ivtmp.65_13 = PHI <ivtmp.65_12(4), 256(2)>
  D.1578_5 = a[i_14];
  D.1579_6 = lrint (D.1578_5);
  b[i_14] = D.1579_6;
  i_7 = i_14 + 1;
  ivtmp.65_12 = ivtmp.65_13 - 1;
  if (ivtmp.65_12 != 0)
    goto <bb 4>;
  else
    goto <bb 5>;

<bb 4>:
  goto <bb 3>;

<bb 5>:
  return;

}

The vectorized version (if the alias check is removed) is:
 ...
  vector long int * ivtmp.120;
  vector long int vect_var_.113;
 ...

  vect_var_.111_20 = *ivtmp.110_18;
  ivtmp.110_21 = ivtmp.110_18 + 16;
  vect_var_.112_22 = *ivtmp.110_21;
  vect_var_.113_23 = __builtin_ia32_vec_pack_sfix (vect_var_.111_20,
vect_var_.112_22);
  *ivtmp.120_26 = vect_var_.113_23;

The alias check is for the store, checking *ivtmp.120_26 and b.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenther at suse dot de]

------- Comment #4 from rguenther at suse dot de  2008-09-14 10:36 -------
Subject: Re:  [4.4 Regression] Revision 140257 causes
 vectorizer tests failures

On Sun, 14 Sep 2008, irar at il dot ibm dot com wrote:

> 
> 
> ------- Comment #3 from irar at il dot ibm dot com  2008-09-14 10:04 -------
> (In reply to comment #2)
> > I don't follow.  For vectorize2.c we have
> > 
> >     b[i] = lrint (a[i]);
> > 
> > where we should be able to vectorize this using lrint vectorization and
> > a scalar long -> int conversion (which is a no-op on i686 and should be
> > vectorized on x86_64).  There is no aliasing issue involved here.
> 
> I am working on x86_64 using -m32 (without -m32 I get
> ../../gcc.target/i386/vectorize2.c:15: note: function is not vectorizable.
> ../../gcc.target/i386/vectorize2.c:15: note: not vectorized: relevant stmt not
> supported: D.1603_6 = lrint (D.1602_5); ).
>
> I don't see any conversion:

Indeed, the conversion is for the 64bit case:

 D.1614_6 = lrint (D.1613_5);
 D.1615_7 = (int) D.1614_6;

but yes, we cannot vectorize lrint if the result is 64bit long
(which it is on 64bit).

For the 32bit case the problem is really the choice of the vector
type for the store (where is this decided on?).  As the type of
b is int it should have chosen vector int instead of vector long.
Note that it is perfectly valid (on 32bit) to assign a vector long
to a vector int.  So with a change like

> The vectorized version (if the alias check is removed) is:
>  ...
>   vector long int * ivtmp.120;

vector int * ivtmp.120;

>   vector long int vect_var_.113;
>  ...
> 
>   vect_var_.111_20 = *ivtmp.110_18;
>   ivtmp.110_21 = ivtmp.110_18 + 16;
>   vect_var_.112_22 = *ivtmp.110_21;
>   vect_var_.113_23 = __builtin_ia32_vec_pack_sfix (vect_var_.111_20,
> vect_var_.112_22);
>   *ivtmp.120_26 = vect_var_.113_23;
> 
> The alias check is for the store, checking *ivtmp.120_26 and b.

the alias check would be fine.

So,

  /* The type of the vector store is determined by the rhs.  */
  vectype = get_vectype_for_scalar_type (TREE_TYPE (op));

the type should be determined by the lhs (after all we try to check
if the new vector lhs aliases the old scalar lhs).  But of course
this means the vector lhs type should be chosen to actually match
the scalar type of the lhs.

  if (!useless_type_conversion_p (TREE_TYPE (op), TREE_TYPE 
(scalar_dest)))
    {
      if (vect_print_dump_info (REPORT_DETAILS))
        fprintf (vect_dump, "operands of different types");
      return false;
    }

This test should then be adjusted to check

  if (!useless_type_conversion_p (TREE_TYPE (TREE_TYPE 
(vectype)), TREE_TYPE (op)))

first, it was the wrong way around, second we should check if the
conversion from the rhs (op) to the element type of the lhs
vector type (vectype) is useless.

Now the interesting part is of course where we select the type
for the induction variable for the store (I can't find this
atm).

Hope this helps,
Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[irar at il dot ibm dot com]

------- Comment #5 from irar at il dot ibm dot com  2008-09-14 12:05 -------
(In reply to comment #4)
> For the 32bit case the problem is really the choice of the vector
> type for the store (where is this decided on?).  As the type of
> b is int it should have chosen vector int instead of vector long.

Now I am completely confused. The decision to base the type of the store on the
rhs was made to fix pr 37385
(http://gcc.gnu.org/ml/gcc-patches/2008-09/msg00674.html).  Without it, there
is ICE in set_mem_alias_set...

Vector pointers are created in vect_create_data_ref_ptr. When called from
vectorizable_store, it gets the type of vectorized rhs and creates a vector
pointer of that type (this was the patch for pr 37385).

> Note that it is perfectly valid (on 32bit) to assign a vector long
> to a vector int.  So with a change like
> 
> > The vectorized version (if the alias check is removed) is:
> >  ...
> >   vector long int * ivtmp.120;
> 
> vector int * ivtmp.120;
> 
> >   vector long int vect_var_.113;
> >  ...
> > 
> >   vect_var_.111_20 = *ivtmp.110_18;
> >   ivtmp.110_21 = ivtmp.110_18 + 16;
> >   vect_var_.112_22 = *ivtmp.110_21;
> >   vect_var_.113_23 = __builtin_ia32_vec_pack_sfix (vect_var_.111_20,
> > vect_var_.112_22);
> >   *ivtmp.120_26 = vect_var_.113_23;
> > 
> > The alias check is for the store, checking *ivtmp.120_26 and b.
> 
> the alias check would be fine.

This part I do understand ;)

> 
> So,
> 
>   /* The type of the vector store is determined by the rhs.  */
>   vectype = get_vectype_for_scalar_type (TREE_TYPE (op));
> 
> the type should be determined by the lhs

not if we base the vectype on the scalar type of the rhs...

> (after all we try to check
> if the new vector lhs aliases the old scalar lhs).  But of course
> this means the vector lhs type should be chosen to actually match
> the scalar type of the lhs.
> 
>   if (!useless_type_conversion_p (TREE_TYPE (op), TREE_TYPE 
> (scalar_dest)))
>     {
>       if (vect_print_dump_info (REPORT_DETAILS))
>         fprintf (vect_dump, "operands of different types");
>       return false;
>     }
> 
> This test should then be adjusted to check
> 
>   if (!useless_type_conversion_p (TREE_TYPE (TREE_TYPE 
> (vectype)), TREE_TYPE (op)))
> 
> first, it was the wrong way around, second we should check if the
> conversion from the rhs (op) to the element type of the lhs
> vector type (vectype) is useless.

Do you mean
if (!useless_type_conversion_p (TREE_TYPE (scalar_dest), TREE_TYPE (op)))?
If vectype is built based on TREE_TYPE (scalar_dest) it should be the same, no?

> 
> Now the interesting part is of course where we select the type
> for the induction variable for the store (I can't find this
> atm).

Isn't it ivtmp.120_26 itself? (The updating of vector pointers is done in
vect_create_data_ref_ptr as well).

Thanks,
Ira

> 
> Hope this helps,
> Richard.
> 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenther at suse dot de]

------- Comment #6 from rguenther at suse dot de  2008-09-14 12:53 -------
Subject: Re:  [4.4 Regression] Revision 140257 causes
 vectorizer tests failures

On Sun, 14 Sep 2008, irar at il dot ibm dot com wrote:

> ------- Comment #5 from irar at il dot ibm dot com  2008-09-14 12:05 -------
> (In reply to comment #4)
> > For the 32bit case the problem is really the choice of the vector
> > type for the store (where is this decided on?).  As the type of
> > b is int it should have chosen vector int instead of vector long.
> 
> Now I am completely confused. The decision to base the type of the store on the
> rhs was made to fix pr 37385
> (http://gcc.gnu.org/ml/gcc-patches/2008-09/msg00674.html).  Without it, there
> is ICE in set_mem_alias_set...

Sorry, I probably didn't look too closely when reviewing this patch.

> Vector pointers are created in vect_create_data_ref_ptr. When called from
> vectorizable_store, it gets the type of vectorized rhs and creates a vector
> pointer of that type (this was the patch for pr 37385).

The sanity checking should forgive any wrong decision we make there,
still basing it on the lhs looks more correct.  I will have a look
myself.  The important thing is to make both the creation and
the check consistent of course.

I see vect_create_data_ref_ptr is getting the type to use passed
in case of stores and this is TREE_TYPE (vec_oprnd) - is vec_oprnd
the lhs or the rhs?  It looks like it is the rhs which may explain
the ICE for PR37385, we should use the vector type according
to the lhs here (maybe passing NULL to vect_create_data_ref_ptr
will do this - dependend on how STMT_VINFO_VECTYPE is set for
stores).

> > Note that it is perfectly valid (on 32bit) to assign a vector long
> > to a vector int.  So with a change like
> > 
> > > The vectorized version (if the alias check is removed) is:
> > >  ...
> > >   vector long int * ivtmp.120;
> > 
> > vector int * ivtmp.120;
> > 
> > >   vector long int vect_var_.113;
> > >  ...
> > > 
> > >   vect_var_.111_20 = *ivtmp.110_18;
> > >   ivtmp.110_21 = ivtmp.110_18 + 16;
> > >   vect_var_.112_22 = *ivtmp.110_21;
> > >   vect_var_.113_23 = __builtin_ia32_vec_pack_sfix (vect_var_.111_20,
> > > vect_var_.112_22);
> > >   *ivtmp.120_26 = vect_var_.113_23;
> > > 
> > > The alias check is for the store, checking *ivtmp.120_26 and b.
> > 
> > the alias check would be fine.
> 
> This part I do understand ;)

Good ;)

> > 
> > So,
> > 
> >   /* The type of the vector store is determined by the rhs.  */
> >   vectype = get_vectype_for_scalar_type (TREE_TYPE (op));
> > 
> > the type should be determined by the lhs
> 
> not if we base the vectype on the scalar type of the rhs...
> 
> > (after all we try to check
> > if the new vector lhs aliases the old scalar lhs).  But of course
> > this means the vector lhs type should be chosen to actually match
> > the scalar type of the lhs.
> > 
> >   if (!useless_type_conversion_p (TREE_TYPE (op), TREE_TYPE 
> > (scalar_dest)))
> >     {
> >       if (vect_print_dump_info (REPORT_DETAILS))
> >         fprintf (vect_dump, "operands of different types");
> >       return false;
> >     }
> > 
> > This test should then be adjusted to check
> > 
> >   if (!useless_type_conversion_p (TREE_TYPE (TREE_TYPE 
> > (vectype)), TREE_TYPE (op)))
> > 
> > first, it was the wrong way around, second we should check if the
> > conversion from the rhs (op) to the element type of the lhs
> > vector type (vectype) is useless.
> 
> Do you mean
> if (!useless_type_conversion_p (TREE_TYPE (scalar_dest), TREE_TYPE (op)))?
> If vectype is built based on TREE_TYPE (scalar_dest) it should be the same, no?

It should be the same - ideally we'd check the conversion on
both vector types (the lhs and the rhs).

> > Now the interesting part is of course where we select the type
> > for the induction variable for the store (I can't find this
> > atm).
> 
> Isn't it ivtmp.120_26 itself? (The updating of vector pointers is done in
> vect_create_data_ref_ptr as well).

Yes, looks like vect_create_data_ref_ptr decides if it is not passed
the type (which it is for stores).

I can try to have a look here, but I'm not exactly familiar with
the code (thanks for the pointers ;)), so I'd appreciate if you
try to figure out what it takes to fix vect_create_data_ref_ptr or
its caller.

Thanks,
Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[irar at il dot ibm dot com]

------- Comment #7 from irar at il dot ibm dot com  2008-09-15 09:11 -------
(In reply to comment #6)

> 
> I see vect_create_data_ref_ptr is getting the type to use passed
> in case of stores and this is TREE_TYPE (vec_oprnd) - is vec_oprnd
> the lhs or the rhs?  It looks like it is the rhs which may explain
> the ICE for PR37385, 

It's the other way around in PR37385 - it ICEs if we use the type of lhs.

> I can try to have a look here, but I'm not exactly familiar with
> the code (thanks for the pointers ;)), so I'd appreciate if you
> try to figure out what it takes to fix vect_create_data_ref_ptr or
> its caller.

Sure, I never meant to ask you to look into the vectorizer code. I only gave
you the pointers because I thought you asked. Fixing the vectorizer code is not
a problem. The problem is I don't really understand what we should generate. 

For scalar store *x = y we create vector store *vect_x = vect_y. The type of
vect_x must be "vector type_of_x", right? This will cause the ICE in pr 37385,
so we must check something to prevent it. AFAIU, the ICE is because the lhs and
the element type of rhs do not alias. Testing "vector type_of_x" and
"type_of_y" for alias does the job. But this prevents vectorization of
vectorize2.c (vector int and long int do not alias)...

Thanks a lot for your help,
Ira


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenther at suse dot de]

------- Comment #8 from rguenther at suse dot de  2008-09-15 09:23 -------
Subject: Re:  [4.4 Regression] Revision 140257 causes
 vectorizer tests failures

On Mon, 15 Sep 2008, irar at il dot ibm dot com wrote:

> 
> 
> ------- Comment #7 from irar at il dot ibm dot com  2008-09-15 09:11 -------
> (In reply to comment #6)
> 
> > 
> > I see vect_create_data_ref_ptr is getting the type to use passed
> > in case of stores and this is TREE_TYPE (vec_oprnd) - is vec_oprnd
> > the lhs or the rhs?  It looks like it is the rhs which may explain
> > the ICE for PR37385, 
> 
> It's the other way around in PR37385 - it ICEs if we use the type of lhs.
> 
> > I can try to have a look here, but I'm not exactly familiar with
> > the code (thanks for the pointers ;)), so I'd appreciate if you
> > try to figure out what it takes to fix vect_create_data_ref_ptr or
> > its caller.
> 
> Sure, I never meant to ask you to look into the vectorizer code. I only gave
> you the pointers because I thought you asked. Fixing the vectorizer code is not
> a problem. The problem is I don't really understand what we should generate. 
> 
> For scalar store *x = y we create vector store *vect_x = vect_y. The type of
> vect_x must be "vector type_of_x", right? This will cause the ICE in pr 37385,
> so we must check something to prevent it. AFAIU, the ICE is because the lhs and
> the element type of rhs do not alias. Testing "vector type_of_x" and
> "type_of_y" for alias does the job. But this prevents vectorization of
> vectorize2.c (vector int and long int do not alias)...

Well, I think the ICE happens because for stores we call 
vect_create_data_ref_ptr with an explicit vector type which is based on
the rhs, not the lhs.  The ICE should never occur if we based the
vector type on the lhs.

Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[irar at il dot ibm dot com]

------- Comment #9 from irar at il dot ibm dot com  2008-09-15 09:35 -------
(In reply to comment #8)
> Well, I think the ICE happens because for stores we call 
> vect_create_data_ref_ptr with an explicit vector type which is based on
> the rhs, not the lhs.  The ICE should never occur if we based the
> vector type on the lhs.

No, no. The ICE occurs when we base the vector type on the lhs! 
  vector void (*<T48f>) (int) * ivtmp.30;
  vector void (*<T49a>) (int_t) vect_cst_.23;

  *ivtmp.30_13 = vect_cst_.23_9;
The rhs and the lhs are not the same (and do not alias).

If the vector type is based on the rhs, then everything is ok here, but not in 
vectorize2.c.

Ira   


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenther at suse dot de]

------- Comment #10 from rguenther at suse dot de  2008-09-15 09:39 -------
Subject: Re:  [4.4 Regression] Revision 140257 causes
 vectorizer tests failures

On Mon, 15 Sep 2008, irar at il dot ibm dot com wrote:

> 
> 
> ------- Comment #9 from irar at il dot ibm dot com  2008-09-15 09:35 -------
> (In reply to comment #8)
> > Well, I think the ICE happens because for stores we call 
> > vect_create_data_ref_ptr with an explicit vector type which is based on
> > the rhs, not the lhs.  The ICE should never occur if we based the
> > vector type on the lhs.
> 
> No, no. The ICE occurs when we base the vector type on the lhs! 
>   vector void (*<T48f>) (int) * ivtmp.30;
>   vector void (*<T49a>) (int_t) vect_cst_.23;
> 
>   *ivtmp.30_13 = vect_cst_.23_9;
> The rhs and the lhs are not the same (and do not alias).

The rhs ans the lhs do not need to alias.  The important thing is that
the *ivtmp.30_13 aliases the original scalar store - if *ivtmp.30_13
is based on the scalar lhs type this will be always the case.

I am going to have a look myself (not right now, but somewhen this
week) - feel free to assign this bug to me for now.

Thanks,
Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenth at gcc dot gnu dot org]

------- Comment #11 from rguenth at gcc dot gnu dot org  2008-09-17 10:10 -------
The problem in PR37385 is that we do not look at the canonical type in
get_alias_set, so we end up with a different alias set for the typedef
(which makes the behavior of get_alias_set inconsistent with the middle-end
type-system).  I have a patch.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-09-14 07:00:50         |2008-09-17 10:10:33
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenth at gcc dot gnu dot org]

------- Comment #12 from rguenth at gcc dot gnu dot org  2008-09-17 11:43 -------
Fixed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenth at gcc dot gnu dot org]

------- Comment #13 from rguenth at gcc dot gnu dot org  2008-09-17 11:43 -------
Subject: Bug 37491

Author: rguenth
Date: Wed Sep 17 11:42:11 2008
New Revision: 140412

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=140412
Log:
2008-09-17  Richard Guenther  <rguenther@suse.de>

        PR middle-end/37385
        PR tree-optimization/37491
        * alias.c (get_alias_set): Use the canonical type.
        * tree-vect-transform.c (vectorizable_store): Use the type of
        the lhs for the vector type.  Adjust checking.
        (vectorizable_load): Adjust checking.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/alias.c
    trunk/gcc/tree-vect-transform.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[irar at il dot ibm dot com]

------- Comment #14 from irar at il dot ibm dot com  2008-09-22 09:24 -------
This patch causes the following failures on ppc:

FAIL: gcc.dg/vect/pr35821-altivec.c scan-tree-dump-times vect "vectorized 1
loops" 1
FAIL: gcc.dg/vect/slp-perm-1.c scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/slp-perm-1.c scan-tree-dump-times vect "vectorizing stmts
using SLP" 1
FAIL: gcc.dg/vect/slp-perm-2.c scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/slp-perm-2.c scan-tree-dump-times vect "vectorizing stmts
using SLP" 1
FAIL: gcc.dg/vect/slp-perm-3.c scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/slp-perm-3.c scan-tree-dump-times vect "vectorizing stmts
using SLP" 1
FAIL: gcc.dg/vect/slp-perm-5.c scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/slp-perm-5.c scan-tree-dump-times vect "vectorizing stmts
using SLP" 2
FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump-times vect "vectorizing stmts
using SLP" 2
FAIL: gcc.dg/vect/slp-perm-7.c scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/slp-perm-7.c scan-tree-dump-times vect "vectorizing stmts
using SLP" 1

For example, for slp-perm-1.c:

slp-perm-1.c:23: note: ==> examining statement: a_7 = *pInput_40;
slp-perm-1.c:23: note: get vectype with 4 units of type unsigned int
slp-perm-1.c:23: note: vectype: __vector unsigned int
...
slp-perm-1.c:23: note: ???  vector type does not alias scalar type
...
slp-perm-1.c:23: note: not vectorized: relevant stmt not supported: a_7 =
*pInput_40;

pInput_40 is unsigned int*
(a_7 is unsigned int)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenth at gcc dot gnu dot org]

------- Comment #15 from rguenth at gcc dot gnu dot org  2008-09-22 10:07 -------
This is because the original access is through a restricted pointer, so the
check is conservatively correct at this point.  We can move it to the
point where the vector pointer is created (but somehow I cannot make a
cross-compiler go past the support check for strided loads - what are the
commandline options required?).

So, can you check the following?  A similar change to vectorizable_store
would be needed.

Index: tree-vect-transform.c
===================================================================
*** tree-vect-transform.c       (revision 140544)
--- tree-vect-transform.c       (working copy)
*************** vectorizable_load (gimple stmt, gimple_s
*** 6388,6404 ****
        return false;
      }

-   /* If accesses through a pointer to vectype do not alias the original
-      memory reference we have a problem.  This should never happen.  */
-   if (get_alias_set (vectype) != get_alias_set (gimple_assign_rhs1 (stmt))
-       && !alias_set_subset_of (get_alias_set (vectype),
-                                get_alias_set (gimple_assign_rhs1 (stmt))))
-     {
-       if (vect_print_dump_info (REPORT_DETAILS))
-         fprintf (vect_dump, "???  vector type does not alias scalar type");
-       return false;
-     }
- 
    /* The vector component type needs to be trivially convertible to the
       scalar lhs.  This should always be the case.  */
    if (!useless_type_conversion_p (TREE_TYPE (scalar_dest), TREE_TYPE
(vectype)))
--- 6388,6393 ----
*************** vectorizable_load (gimple stmt, gimple_s
*** 6662,6667 ****
--- 6651,6663 ----
            default:
              gcc_unreachable ();
            }
+ 
+         /* If accesses through a pointer to vectype do not alias the original
+            memory reference we have a problem.  This should never happen.  */
+         gcc_assert (get_alias_set (data_ref) != get_alias_set
(gimple_assign_rhs1 (stmt))
+                     && !alias_set_subset_of (get_alias_set (data_ref),
+                                              get_alias_set
(gimple_assign_rhs1 (stmt))));
+ 
          vec_dest = vect_create_destination_var (scalar_dest, vectype);
          new_stmt = gimple_build_assign (vec_dest, data_ref);
          new_temp = make_ssa_name (vec_dest, new_stmt);


Note that this _does_ show that we create wrong alias information as the
pointer type vect_create_data_ref_ptr creates is _not_ marked restrict
which makes us emit wrong code (wrong code in the sense that our handling
of restrict pointers is not exactly correct).

So in the end we probably shouldn't open this can of worms and go back
to check the alias compatibility based on the types...  I will prepare
a patch for this.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[irar at il dot ibm dot com]

------- Comment #16 from irar at il dot ibm dot com  2008-09-22 10:33 -------
(In reply to comment #15)
> This is because the original access is through a restricted pointer, so the
> check is conservatively correct at this point.  We can move it to the
> point where the vector pointer is created 

But this way we move the check to the transformation and we don't have a
mechanism to stop in the middle of transformation.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenther at suse dot de]

------- Comment #17 from rguenther at suse dot de  2008-09-22 11:14 -------
Subject: Re:  [4.4 Regression] Revision 140257 causes
 vectorizer tests failures

On Mon, 22 Sep 2008, irar at il dot ibm dot com wrote:

> ------- Comment #16 from irar at il dot ibm dot com  2008-09-22 10:33 -------
> (In reply to comment #15)
> > This is because the original access is through a restricted pointer, so the
> > check is conservatively correct at this point.  We can move it to the
> > point where the vector pointer is created 
> 
> But this way we move the check to the transformation and we don't have a
> mechanism to stop in the middle of transformation.

Right, but if we correctly base the vectorized access on the scalar
one we never should fail this test (so I made it an assert - though
logically inverted :().

WRT vect_create_data_ref_ptr not only we have to build a correctly
restrict qualified type but also the decl (vect_get_new_vect_var)
needs to have its DECL_POINTER_ALIAS_SET set correctly.

So, like the following (which seems to work on x86_64, but it looks
like all the testcases with restricted pointers are not supported on 
x86_64).

Index: gcc/tree-vect-transform.c
===================================================================
*** gcc/tree-vect-transform.c   (revision 140544)
--- gcc/tree-vect-transform.c   (working copy)
*************** vect_create_data_ref_ptr (gimple stmt, s
*** 1077,1084 ****
--- 1077,1095 ----
    else
      vect_ptr_type = build_pointer_type (vectype);

+   if (TREE_CODE (DR_BASE_ADDRESS (dr)) == SSA_NAME
+       && TYPE_RESTRICT (TREE_TYPE (DR_BASE_ADDRESS (dr))))
+     vect_ptr_type = build_qualified_type (vect_ptr_type, TYPE_QUAL_RESTRICT);
    vect_ptr = vect_get_new_vect_var (vect_ptr_type, vect_pointer_var,
                                      get_name (base_name));
+   if (TREE_CODE (DR_BASE_ADDRESS (dr)) == SSA_NAME
+       && TYPE_RESTRICT (TREE_TYPE (DR_BASE_ADDRESS (dr))))
+     {
+       get_alias_set (base_name);
+       DECL_POINTER_ALIAS_SET (vect_ptr)
+       = DECL_POINTER_ALIAS_SET (SSA_NAME_VAR (DR_BASE_ADDRESS (dr)));
+     }
+ 
    add_referenced_var (vect_ptr);

    /** (2) Add aliasing information to the new vector-pointer:
*************** vectorizable_load (gimple stmt, gimple_s
*** 6388,6404 ****
        return false;
      }

-   /* If accesses through a pointer to vectype do not alias the original
-      memory reference we have a problem.  This should never happen.  */
-   if (get_alias_set (vectype) != get_alias_set (gimple_assign_rhs1 (stmt))
-       && !alias_set_subset_of (get_alias_set (vectype),
-                                get_alias_set (gimple_assign_rhs1 (stmt))))
-     {
-       if (vect_print_dump_info (REPORT_DETAILS))
-         fprintf (vect_dump, "???  vector type does not alias scalar type");
-       return false;
-     }
- 
    /* The vector component type needs to be trivially convertible to the
       scalar lhs.  This should always be the case.  */
    if (!useless_type_conversion_p (TREE_TYPE (scalar_dest), TREE_TYPE
(vectype)))
--- 6399,6404 ----
*************** vectorizable_load (gimple stmt, gimple_s
*** 6662,6667 ****
--- 6662,6672 ----
            default:
              gcc_unreachable ();
            }
+         /* If accesses through a pointer to vectype do not alias the original
+            memory reference we have a problem.  This should never happen.  */
+         gcc_assert (get_alias_set (data_ref) == get_alias_set
(gimple_assign_rhs1 (stmt))
+                     || alias_set_subset_of (get_alias_set (data_ref),
+                                             get_alias_set (gimple_assign_rhs1
(stmt))));
          vec_dest = vect_create_destination_var (scalar_dest, vectype);
          new_stmt = gimple_build_assign (vec_dest, data_ref);
          new_temp = make_ssa_name (vec_dest, new_stmt);


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenth at gcc dot gnu dot org]

------- Comment #18 from rguenth at gcc dot gnu dot org  2008-09-22 11:48 -------
Created an attachment (id=16377)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16377&action=view)
patch

Actually this one vectorizes slp-perm-1.c for me on a ppc64 cross.  It also
passes the x86_64 vect testsuite.

I will give it full testing on x86_64, can you test it on ppc?  Thanks.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[irar at il dot ibm dot com]

------- Comment #19 from irar at il dot ibm dot com  2008-09-22 12:32 -------
(In reply to comment #18)
> Created an attachment (id=16377)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16377&action=view) [edit]
> patch
> 
> Actually this one vectorizes slp-perm-1.c for me on a ppc64 cross.  It also
> passes the x86_64 vect testsuite.
> 
> I will give it full testing on x86_64, can you test it on ppc?  Thanks.
> 

It passes the vectorizer testsuite on ppc. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[irar at il dot ibm dot com]

------- Comment #20 from irar at il dot ibm dot com  2008-09-22 12:41 -------
(In reply to comment #18)
> Created an attachment (id=16377)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16377&action=view) [edit]
> patch
> 
> Actually this one vectorizes slp-perm-1.c for me on a ppc64 cross.  It also
> passes the x86_64 vect testsuite.
> 
> I will give it full testing on x86_64, can you test it on ppc?  Thanks.
> 
I am fully testing it now, but I'll be able to update you about the results
only tomorrow.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491

[Bug middle-end/37491] [4.4 Regression] Revision 140257 causes vectorizer tests failures

[rguenth at gcc dot gnu dot org]

------- Comment #21 from rguenth at gcc dot gnu dot org  2008-09-30 10:37 -------
Subject: Bug 37491

Author: rguenth
Date: Tue Sep 30 10:36:22 2008
New Revision: 140781

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=140781
Log:
2008-09-30  Richard Guenther  <rguenther@suse.de>

        PR middle-end/37491
        * tree-vect-transform.c (vect_create_data_ref_ptr): Properly
        build restrict-qualified pointers.
        (vectorizable_store): Move alias check later.
        (vectorizable_load): Likewise.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-vect-transform.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491