[Bug target/14899] New: wrong code due to change in compatibility rules for vector types

[Bug target/14899] New: wrong code due to change in compatibility rules for vector types

[bonzini at gnu dot org]

> #include <altivec.h>
> int main()
> {
>        __vector signed short  res;
>        __vector signed short t8;
>        __vector signed short t9;
>        res = vec_subs(t8, t9);
> }

is incorrectly using vsubuh instead of vsubsh.

This is because __builtin_types_compatible_p (__vector bool short, __vector 
signed short) is returning 1. 

This test case says that it said 1:

int f()
{
return __builtin_types_compatible_p (__vector bool short,
                                     __vector signed short);
}

That's my fault (my vector patch's fault).  It is easy to fix by modifying 
comptypes (I had it in my tree for a while before submitting the patch, but 
caused other regressions on i686, IIRC), but overall it seems quite fragile... 

Paolo

-- 
           Summary: wrong code due to change in compatibility rules for
                    vector types
           Product: gcc
           Version: 3.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: bonzini at gnu dot org
                CC: apinski at apple dot com,bonzini at gnu dot
                    org,fjahanian at apple dot com,gcc-bugs at gcc dot gnu
                    dot org,zlaski at apple dot com
GCC target triplet: powerpc-*-*


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-04-09 11:22 -------
Already confirmed by me on the mailing list.  Also note my Apple email account no longer works for 
me, either use my @gcc email address or my physics.uc.edu.edu one.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|apinski at apple dot com    |pinskia at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
           Keywords|                            |wrong-code
      Known to fail|                            |3.5.0
      Known to work|                            |3.4.0
   Last reconfirmed|0000-00-00 00:00:00         |2004-04-09 11:22:11
               date|                            |
            Summary|wrong code due to change in |[3.5 Regression] wrong code
                   |compatibility rules for     |due to change in
                   |vector types                |compatibility rules for
                   |                            |vector types
   Target Milestone|---                         |3.5.0


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[fjahanian at apple dot com]

------- Additional Comments From fjahanian at apple dot com  2004-04-09 17:09 -------
But in this case, code should come out correct. This is because of what I see in altivec.h:

__ch (__bin_args_eq (vector bool short, (a1), vector signed short, (a2)), \
      ((vector signed short) __builtin_altivec_vsubshs ((vector signed short) (a1), (vector signed short) 
(a2))), \

Can you elaborate as to why __builtin_types_compatible_p (vector bool short, vector signed short);
returning 1 results in bad codegen in my test case, thanks.(In reply to comment #1)
> Already confirmed by me on the mailing list.  Also note my Apple email account no longer works for 
> me, either use my @gcc email address or my physics.uc.edu.edu one.

-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-04-09 17:17 -------
Actually I made the mistake of putting testing the wrong thing.
Here is the test which should return 0 but does not but did in 3.3.3:
int f()
{
return __builtin_types_compatible_p (__vector unsigned short,
                                     __vector signed short);
}

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|3.4.0                       |3.4.0 3.3.3


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[fjahanian at apple dot com]

------- Additional Comments From fjahanian at apple dot com  2004-04-09 17:20 -------
Yes, this makes sense now (the bug I mean).
(In reply to comment #3)
> Actually I made the mistake of putting testing the wrong thing.
> Here is the test which should return 0 but does not but did in 3.3.3:
> int f()
> {
> return __builtin_types_compatible_p (__vector unsigned short,
>                                      __vector signed short);
> }



-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[fjahanian at apple dot com]

------- Additional Comments From fjahanian at apple dot com  2004-04-09 18:01 -------
I have tried following patch and it seems to work in my very limited testing. 

Index: c-typeck.c
============================================================
=======
RCS file: /cvs/gcc/gcc/gcc/c-typeck.c,v
retrieving revision 1.196.2.43.2.3
diff -c -p -r1.196.2.43.2.3 c-typeck.c
*** c-typeck.c  7 Apr 2004 22:09:14 -0000       1.196.2.43.2.3
--- c-typeck.c  9 Apr 2004 18:00:03 -0000
*************** comptypes (tree type1, tree type2, int f
*** 577,583 ****
        /* The target might allow certain vector types to be compatible.  */
        val = targetm.vector_opaque_p (t1)
        || targetm.vector_opaque_p (t2)
!       || TYPE_MODE (t1) == TYPE_MODE (t2);
        break;
  
      default:
--- 577,584 ----
        /* The target might allow certain vector types to be compatible.  */
        val = targetm.vector_opaque_p (t1)
        || targetm.vector_opaque_p (t2)
!       || (TYPE_MODE (t1) == TYPE_MODE (t2) 
!           && TREE_UNSIGNED(t1) == TREE_UNSIGNED(t2));
        break;
  
      default:


-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[bonzini at gnu dot org]

------- Additional Comments From bonzini at gnu dot org  2004-04-10 09:46 -------
making unsigned and signed types
incompatible caused regressions: see this excerpt from
http://gcc.gnu.org/ml/gcc-patches/2004-03/msg00843.html

    "The same is for gcc.dg/simd-1.c and gcc.dg/20020531-1.c, where one was
    checking for incompatibility between vector types with different
    signedness but the same mode, and the other was happily passing an
    unsigned V8QI to __builtin_ia32_paddusb which expects a signed V8QI.
    I fixed gcc.dg/simd-1.c to not expect an error on vector types with
    the same mode but different signedness."


-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[bonzini at gnu dot org]

------- Additional Comments From bonzini at gnu dot org  2004-04-11 07:41 -------
The bug is __builtin_types_compatible_p being ill-named because it is actually 
__builtin_same_type_p (it just uses comptypes).  __builtin_types_compatible_p 
(int, unsigned int) is zero, so the problem is the name of the builtin.  And 
this name used in Altivec code caused me to think that comptypes actually meant 
*compatible* types for vectors, instead of *compare* types.  However, the C++ 
front-end's shortcut name for comptypes(..., ..., COMPARE_STRICT), which is 
same_type_p, makes it clear that comptypes is not checking compatibility.

I prepared a patch which has yet another redefinition of comptypes for vector 
types, and posted it to gcc-patches, but it did not pass regtesting (developing 
gcc is expensive for me right now, so I try to limit online time as much as 
possible).  Making two vector types *equivalent* if *one of them* is opaque 
looks wrong, so here is the definition adopted in this patch:

1) if two vector types are of different sizes, they are not equivalent
2) if only one of two vector types is opaque, they are not equivalent
3) else, if both are opaque, they are equivalent
4) or, if they are not opaque but have the same underlying type, they are
equivalent.

The *compatibility* (as in, you can assign even without an explicit cast) is
now implemented in a separate vector_types_compatible_p, which matches more or
less what current mainline does in comptypes for vector types.

Paolo



-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[jsm at polyomino dot org dot uk]

------- Additional Comments From jsm at polyomino dot org dot uk  2004-04-11 11:53 -------
Subject: Re:  [3.5 Regression] wrong code due to change in
 compatibility rules for vector types

On Sun, 11 Apr 2004, bonzini at gnu dot org wrote:

> The bug is __builtin_types_compatible_p being ill-named because it is actually 
> __builtin_same_type_p (it just uses comptypes).  __builtin_types_compatible_p 
> (int, unsigned int) is zero, so the problem is the name of the builtin.  And 

It *does* test compatibility.  int and unsigned int are not compatible.  
int[10] and int[] are compatible but are not the same type.  If vector int
and vector unsigned int are meant to be compatible (for example, you can
freely assign between vector int ** and vector unsigned int **), then it's
a matter for the language specifications for vector extensions to specify
clearly what the compatibility rules are, and for GCC then to implement
that.  The documentation of __builtin_types_compatible_p gives examples.



-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[bonzini at gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |bonzini at gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-05-05 07:21 -------
Subject: Bug 14899

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	bonzini@gcc.gnu.org	2004-05-05 07:21:49

Modified files:
	gcc            : c-common.c c-common.h c-typeck.c tree.c 
	gcc/cp         : call.c typeck.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/ext: spe1.C 

Log message:
	gcc/ChangeLog:
	2004-05-04  Paolo Bonzini  <bonzini@gnu.org>
	Richard Henderson  <rth@redhat.com>
	
	PR target/14899
	
	* c-common.c (vector_types_convertible_p): New function.
	* c-typeck.c (comptypes): Recurse on vector types.
	(convert_for_assignment): Use vector_types_convertible_p.
	(digest_init): Use vector_types_convertible_p to check
	validness of constant vector initializers; otherwise treat
	them as scalars.
	* tree.c (make_or_reuse_type): New.
	(build_common_tree_nodes): Use it.
	* cp/call.c (standard_conversion): Likewise.
	* cp/typeck.c (comptypes): Recurse on vector types.
	(convert_for_assignment): Use vector_types_convertible_p.
	* config/rs6000/rs6000.c (build_opaque_vector_type):
	New function.
	(rs6000_init_builtins): Use it.
	
	gcc/testsuite/ChangeLog:
	2004-05-04  Paolo Bonzini  <bonzini@gnu.org>
	
	* g++.dg/ext/spe1.C: New testcase.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-common.c.diff?cvsroot=gcc&r1=1.496&r2=1.497
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-common.h.diff?cvsroot=gcc&r1=1.224&r2=1.225
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-typeck.c.diff?cvsroot=gcc&r1=1.301&r2=1.302
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree.c.diff?cvsroot=gcc&r1=1.368&r2=1.369
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&r1=1.473&r2=1.474
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/typeck.c.diff?cvsroot=gcc&r1=1.539&r2=1.540
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3719&r2=1.3720
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/ext/spe1.C.diff?cvsroot=gcc&r1=NONE&r2=1.1



-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-05-05 07:23 -------
Subject: Bug 14899

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	bonzini@gcc.gnu.org	2004-05-05 07:23:14

Modified files:
	gcc            : ChangeLog 

Log message:
	gcc/ChangeLog:
	2004-05-04  Paolo Bonzini  <bonzini@gnu.org>
	Richard Henderson  <rth@redhat.com>
	
	PR target/14899
	
	* c-common.c (vector_types_convertible_p): New function.
	* c-typeck.c (comptypes): Recurse on vector types.
	(convert_for_assignment): Use vector_types_convertible_p.
	(digest_init): Use vector_types_convertible_p to check
	validness of constant vector initializers; otherwise treat
	them as scalars.
	* tree.c (make_or_reuse_type): New.
	(build_common_tree_nodes): Use it.
	* cp/call.c (standard_conversion): Likewise.
	* cp/typeck.c (comptypes): Recurse on vector types.
	(convert_for_assignment): Use vector_types_convertible_p.
	* config/rs6000/rs6000.c (build_opaque_vector_type):
	New function.
	(rs6000_init_builtins): Use it.
	
	gcc/testsuite/ChangeLog:
	2004-05-04  Paolo Bonzini  <bonzini@gnu.org>
	
	* g++.dg/ext/spe1.C: New testcase.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3573&r2=2.3574



-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-05-05 13:32 -------
This is fixed, right? 

-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[bonzini at gnu dot org]

------- Additional Comments From bonzini at gnu dot org  2004-05-05 15:33 -------
Subject: Re:  [3.5 Regression] wrong code due to change
 in compatibility rules for vector types

> This is fixed, right? 

Yes, it is.

Paolo



-- 


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

[Bug target/14899] [3.5 Regression] wrong code due to change in compatibility rules for vector types

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-05-05 15:37 -------
Fixed.

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


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