fix c/17384

fix c/17384

[Richard Henderson]

[-- Attachment #1: Type: text/plain, Size: 913 bytes --]

The problem in this pr is that we create a empty RECORD_TYPE node,
attempt to apply the attributes, which stomps on the type node
replacing it with an INTEGER_TYPE node.  We return from applying
the attributes and then attempt to add the fields to the record
and lay out the type.  Needless to say, adding fields to an
INTEGER_TYPE node causes problems.

My fix is to deny application of a mode if it changes the base
type.  As a consequence, it is no longer legal to write

	int foo __attribute__((mode(SF)))

because INTEGER_TYPE and REAL_TYPE don't match.  You must use

	float foo __attribute__((mode(SF)))

I've always found the prior form kinda dodgy anyway.

This also caught a place in crtstuff where we were attempting
to apply a mode to an array type.  What was intended was to 
apply the mode to the array element type.

The first patch is for 3.4, the second for 4.0.  Both tested
on i686-linux.


r~

[-- Attachment #2: z-34 --]
[-- Type: text/plain, Size: 4069 bytes --]

        PR c/17384
        * c-common.c (handle_mode_attribute): Disallow mode changes that
        alter the CODE of the top-level type.
 
        * crtstuff.c (__FRAME_END__): Remove mode attribute.  Find 32-bit
        integer from internal limits macros.
        * config/i386/emmintrin.h (__v2df): Fix base type.
        * config/i386/xmmintrin.h (__m128, __v4sf): Likewise.

Index: gcc/c-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.c,v
retrieving revision 1.476.4.8
diff -u -p -r1.476.4.8 c-common.c
--- gcc/c-common.c	23 Aug 2004 18:02:39 -0000	1.476.4.8
+++ gcc/c-common.c	13 Oct 2004 23:15:47 -0000
@@ -4642,7 +4642,10 @@ handle_mode_attribute (tree *node, tree 
       else
 	for (j = 0; j < NUM_MACHINE_MODES; j++)
 	  if (!strcmp (p, GET_MODE_NAME (j)))
-	    mode = (enum machine_mode) j;
+	    {
+	      mode = (enum machine_mode) j;
+	      break;
+	    }
 
       if (mode == VOIDmode)
 	error ("unknown machine mode `%s'", p);
@@ -4675,8 +4678,17 @@ handle_mode_attribute (tree *node, tree 
 							mode);
 	      *node = ptr_type;
 	    }
+	  else if (VECTOR_MODE_P (mode)
+		   ? TREE_CODE (type) != TREE_CODE (TREE_TYPE (typefm))
+		   : TREE_CODE (type) != TREE_CODE (typefm))
+		   
+	    {
+	      error ("mode `%s' applied to inappropriate type", p);
+	      return NULL_TREE;
+	    }
 	  else
-	  *node = typefm;
+	    *node = typefm;
+
 	  /* No need to layout the type here.  The caller should do this.  */
 	}
     }
Index: gcc/crtstuff.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/crtstuff.c,v
retrieving revision 1.65
diff -u -p -r1.65 crtstuff.c
--- gcc/crtstuff.c	21 Nov 2003 19:48:24 -0000	1.65
+++ gcc/crtstuff.c	13 Oct 2004 23:15:47 -0000
@@ -445,9 +445,18 @@ STATIC func_ptr __DTOR_END__[1]
 #ifdef EH_FRAME_SECTION_NAME
 /* Terminate the frame unwind info section with a 4byte 0 as a sentinel;
    this would be the 'length' field in a real FDE.  */
-STATIC EH_FRAME_SECTION_CONST int __FRAME_END__[]
-     __attribute__ ((unused, mode(SI), section(EH_FRAME_SECTION_NAME),
-		     aligned(4)))
+# if __INT_MAX__ == 2147483647
+typedef int int32;
+# elif __LONG_MAX__ == 2147483647
+typedef long int32;
+# elif __SHRT_MAX__ == 2147483647
+typedef short int32;
+# else
+#  error "Missing a 4 byte integer"
+# endif
+STATIC EH_FRAME_SECTION_CONST int32 __FRAME_END__[]
+     __attribute__ ((unused, section(EH_FRAME_SECTION_NAME),
+		     aligned(sizeof(int32))))
      = { 0 };
 #endif /* EH_FRAME_SECTION_NAME */
 
Index: gcc/config/i386/emmintrin.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/emmintrin.h,v
retrieving revision 1.4.4.1
diff -u -p -r1.4.4.1 emmintrin.h
--- gcc/config/i386/emmintrin.h	31 Jan 2004 06:18:20 -0000	1.4.4.1
+++ gcc/config/i386/emmintrin.h	13 Oct 2004 23:15:47 -0000
@@ -34,7 +34,7 @@
 #include <xmmintrin.h>
 
 /* SSE2 */
-typedef int __v2df __attribute__ ((mode (V2DF)));
+typedef double __v2df __attribute__ ((mode (V2DF)));
 typedef int __v2di __attribute__ ((mode (V2DI)));
 typedef int __v4si __attribute__ ((mode (V4SI)));
 typedef int __v8hi __attribute__ ((mode (V8HI)));
Index: gcc/config/i386/xmmintrin.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/xmmintrin.h,v
retrieving revision 1.27.4.1
diff -u -p -r1.27.4.1 xmmintrin.h
--- gcc/config/i386/xmmintrin.h	31 Jan 2004 06:18:23 -0000	1.27.4.1
+++ gcc/config/i386/xmmintrin.h	13 Oct 2004 23:15:47 -0000
@@ -38,10 +38,10 @@
 #include <mmintrin.h>
 
 /* The data type intended for user use.  */
-typedef int __m128 __attribute__ ((__mode__(__V4SF__)));
+typedef float __m128 __attribute__ ((__mode__(__V4SF__)));
 
 /* Internal data types for implementing the intrinsics.  */
-typedef int __v4sf __attribute__ ((__mode__(__V4SF__)));
+typedef float __v4sf __attribute__ ((__mode__(__V4SF__)));
 
 /* Create a selector for use with the SHUFPS instruction.  */
 #define _MM_SHUFFLE(fp3,fp2,fp1,fp0) \

[-- Attachment #3: z-40 --]
[-- Type: text/plain, Size: 4248 bytes --]

        PR c/17384
        * c-common.c (handle_mode_attribute): Disallow mode changes that
        alter the CODE of the top-level type.
  
        * crtstuff.c (__FRAME_END__): Remove mode attribute.  Find 32-bit
        integer from internal limits macros.
        * config/i386/mm3dnow.h (__v2sf): Fix base type.

Index: gcc/c-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.c,v
retrieving revision 1.576
diff -u -p -r1.576 c-common.c
--- gcc/c-common.c	3 Oct 2004 20:52:59 -0000	1.576
+++ gcc/c-common.c	13 Oct 2004 23:18:41 -0000
@@ -4303,7 +4303,10 @@ handle_mode_attribute (tree *node, tree 
       else
 	for (j = 0; j < NUM_MACHINE_MODES; j++)
 	  if (!strcmp (p, GET_MODE_NAME (j)))
-	    mode = (enum machine_mode) j;
+	    {
+	      mode = (enum machine_mode) j;
+	      break;
+	    }
 
       if (mode == VOIDmode)
 	{
@@ -4363,7 +4366,7 @@ handle_mode_attribute (tree *node, tree 
 
       if (typefm == NULL_TREE)
 	{
-	  error ("no data type for mode %<%s%>", p);
+	  error ("no data type for mode %qs", p);
 	  return NULL_TREE;
 	}
       else if (TREE_CODE (type) == ENUMERAL_TYPE)
@@ -4373,8 +4376,7 @@ handle_mode_attribute (tree *node, tree 
 	     this mode for this type.  */
 	  if (TREE_CODE (typefm) != INTEGER_TYPE)
 	    {
-	      error ("cannot use mode %qs for enumeral types",
-		     GET_MODE_NAME (mode));
+	      error ("cannot use mode %qs for enumeral types", p);
 	      return NULL_TREE;
 	    }
 
@@ -4383,6 +4385,12 @@ handle_mode_attribute (tree *node, tree 
 	  TYPE_PRECISION (type) = TYPE_PRECISION (typefm);
 	  typefm = type;
 	}
+      else if (TREE_CODE (type) != TREE_CODE (typefm))
+	{
+	  error ("mode %qs applied to inappropriate type", p);
+	  return NULL_TREE;
+	}
+
       *node = typefm;
 
       /* No need to layout the type here.  The caller should do this.  */
Index: gcc/crtstuff.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/crtstuff.c,v
retrieving revision 1.68
diff -u -p -r1.68 crtstuff.c
--- gcc/crtstuff.c	18 Sep 2004 19:47:09 -0000	1.68
+++ gcc/crtstuff.c	13 Oct 2004 23:18:41 -0000
@@ -455,9 +455,18 @@ STATIC func_ptr __DTOR_END__[1]
 #ifdef EH_FRAME_SECTION_NAME
 /* Terminate the frame unwind info section with a 4byte 0 as a sentinel;
    this would be the 'length' field in a real FDE.  */
-STATIC EH_FRAME_SECTION_CONST int __FRAME_END__[]
-     __attribute__ ((unused, mode(SI), section(EH_FRAME_SECTION_NAME),
-		     aligned(4)))
+# if __INT_MAX__ == 2147483647
+typedef int int32;
+# elif __LONG_MAX__ == 2147483647
+typedef long int32;
+# elif __SHRT_MAX__ == 2147483647
+typedef short int32;
+# else
+#  error "Missing a 4 byte integer"
+# endif
+STATIC EH_FRAME_SECTION_CONST int32 __FRAME_END__[]
+     __attribute__ ((unused, section(EH_FRAME_SECTION_NAME),
+		     aligned(sizeof(int32))))
      = { 0 };
 #endif /* EH_FRAME_SECTION_NAME */
 
Index: gcc/config/i386/mm3dnow.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/mm3dnow.h,v
retrieving revision 1.1
diff -u -p -r1.1 mm3dnow.h
--- gcc/config/i386/mm3dnow.h	10 Jul 2004 00:27:58 -0000	1.1
+++ gcc/config/i386/mm3dnow.h	13 Oct 2004 23:18:41 -0000
@@ -35,7 +35,7 @@
 #include <mmintrin.h>
 
 /* Internal data types for implementing the intrinsics.  */
-typedef int __v2sf __attribute__ ((__mode__ (__SF__), __vector_size__ (8)));
+typedef float __v2sf __attribute__ ((__vector_size__ (8)));
 
 static __inline void
 _m_femms (void)
Index: gcc/testsuite/gcc.dg/attr-mode-1.c
===================================================================
RCS file: gcc/testsuite/gcc.dg/attr-mode-1.c
diff -N gcc/testsuite/gcc.dg/attr-mode-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/gcc.dg/attr-mode-1.c	13 Oct 2004 23:18:41 -0000
@@ -0,0 +1,13 @@
+/* PR c/17384 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+typedef struct __attribute__((mode(SI))) { 
+    unsigned    INT0    :1, 
+                RES0    :1, 
+                        :6, 
+                INT1    :1, 
+                RES1    :1, 
+                        :6, 
+                        :16; 
+} MCR;  /* { dg-error "inappropriate type" } */

Re: fix c/17384

[Richard Guenther]

On Wed, 13 Oct 2004 16:39:28 -0700, Richard Henderson <rth@redhat.com> wrote:
> The problem in this pr is that we create a empty RECORD_TYPE node,
> attempt to apply the attributes, which stomps on the type node
> replacing it with an INTEGER_TYPE node.  We return from applying
> the attributes and then attempt to add the fields to the record
> and lay out the type.  Needless to say, adding fields to an
> INTEGER_TYPE node causes problems.
> 
> My fix is to deny application of a mode if it changes the base
> type.  As a consequence, it is no longer legal to write
> 
>         int foo __attribute__((mode(SF)))
> 
> because INTEGER_TYPE and REAL_TYPE don't match.  You must use
> 
>         float foo __attribute__((mode(SF)))

I'm now getting
glsimd.h: warning: specifying vector types with __attribute__ ((mode))
is deprecated
glsimd.h: warning: use __attribute__ ((vector_size)) instead
glsimd.h: error: mode 'V4SF' applied to inappropriate type

for

typedef float v4sf __attribute__((mode(V4SF)));

and I have no idea what basetype to use.

Richard.

Re: fix c/17384

[Richard Henderson]

On Sun, Oct 17, 2004 at 11:18:03PM +0200, Richard Guenther wrote:
> I'm now getting
> glsimd.h: warning: specifying vector types with __attribute__ ((mode))
> is deprecated
> glsimd.h: warning: use __attribute__ ((vector_size)) instead
> glsimd.h: error: mode 'V4SF' applied to inappropriate type
> 
> for
> 
> typedef float v4sf __attribute__((mode(V4SF)));

For mainline?

Have you considered following the directions and using vector_size?


r~

Re: fix c/17384

[Richard Guenther]

On Sun, 17 Oct 2004 16:06:13 -0700, Richard Henderson <rth@redhat.com> wrote:
> On Sun, Oct 17, 2004 at 11:18:03PM +0200, Richard Guenther wrote:
> > I'm now getting
> > glsimd.h: warning: specifying vector types with __attribute__ ((mode))
> > is deprecated
> > glsimd.h: warning: use __attribute__ ((vector_size)) instead
> > glsimd.h: error: mode 'V4SF' applied to inappropriate type
> >
> > for
> >
> > typedef float v4sf __attribute__((mode(V4SF)));
> 
> For mainline?

Yes, for mainline as of yesterday.

> Have you considered following the directions and using vector_size?

But then it won't compile with 3.3 anymore (but I didn't try).  I
guess you maybe broke support for __attribute__((mode()))) with your
patch?

Richard.

Re: fix c/17384

[Andrew Pinski]

On Oct 18, 2004, at 4:36 AM, Richard Guenther wrote:

> On Sun, 17 Oct 2004 16:06:13 -0700, Richard Henderson <rth@redhat.com> 
> wrote:
>> On Sun, Oct 17, 2004 at 11:18:03PM +0200, Richard Guenther wrote:
>>> I'm now getting
>>> glsimd.h: warning: specifying vector types with __attribute__ 
>>> ((mode))
>>> is deprecated
>>> glsimd.h: warning: use __attribute__ ((vector_size)) instead
>>> glsimd.h: error: mode 'V4SF' applied to inappropriate type
>>>
>>> for
>>>
>>> typedef float v4sf __attribute__((mode(V4SF)));
>>
>> For mainline?
>
> Yes, for mainline as of yesterday.
>
>> Have you considered following the directions and using vector_size?
>
> But then it won't compile with 3.3 anymore (but I didn't try).  I
> guess you maybe broke support for __attribute__((mode()))) with your
> patch?


vector_size has been supported since 3.1 IIRC, it was added for altivec
support.

Thanks,
Andrew Pinski

Re: fix c/17384

[Richard Guenther]

On Mon, 18 Oct 2004 07:29:15 -0400, Andrew Pinski
<pinskia@physics.uc.edu> wrote:
> 
> On Oct 18, 2004, at 4:36 AM, Richard Guenther wrote:
> 
> > On Sun, 17 Oct 2004 16:06:13 -0700, Richard Henderson <rth@redhat.com>
> > wrote:
> >> On Sun, Oct 17, 2004 at 11:18:03PM +0200, Richard Guenther wrote:
> >>> I'm now getting
> >>> glsimd.h: warning: specifying vector types with __attribute__
> >>> ((mode))
> >>> is deprecated
> >>> glsimd.h: warning: use __attribute__ ((vector_size)) instead
> >>> glsimd.h: error: mode 'V4SF' applied to inappropriate type
> >>>
> >>> for
> >>>
> >>> typedef float v4sf __attribute__((mode(V4SF)));
> >>
> >> For mainline?
> >
> > Yes, for mainline as of yesterday.
> >
> >> Have you considered following the directions and using vector_size?
> >
> > But then it won't compile with 3.3 anymore (but I didn't try).  I
> > guess you maybe broke support for __attribute__((mode()))) with your
> > patch?
> 
> vector_size has been supported since 3.1 IIRC, it was added for altivec
> support.

But even the 3.4.2 manual still talks about __attribute__((mode)) in sec. 5.43.
I guess a patch from mainline needs to be backported, if the desired attribute
is vector_size rather than mode for vector types.  To 3.3, too, I presume.

Richard.

Re: fix c/17384

[Richard Henderson]

On Mon, Oct 18, 2004 at 10:36:59AM +0200, Richard Guenther wrote:
> > > typedef float v4sf __attribute__((mode(V4SF)));
> > 
> > For mainline?
> 
> Yes, for mainline as of yesterday.
> 
> > Have you considered following the directions and using vector_size?
> 
> But then it won't compile with 3.3 anymore (but I didn't try).  I
> guess you maybe broke support for __attribute__((mode()))) with your
> patch?

Yes.  I've applied the following.  It can be removed when we
removed the deprecation warning.  It'll then be a hard error.


r~


        * c-common.c (handle_mode_attribute): Allow scalar->vector
        type changes yet.

Index: c-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.c,v
retrieving revision 1.578
diff -u -p -r1.578 c-common.c
--- c-common.c	16 Oct 2004 22:58:45 -0000	1.578
+++ c-common.c	18 Oct 2004 21:59:17 -0000
@@ -4386,7 +4386,9 @@ handle_mode_attribute (tree *node, tree 
 	  TYPE_PRECISION (type) = TYPE_PRECISION (typefm);
 	  typefm = type;
 	}
-      else if (TREE_CODE (type) != TREE_CODE (typefm))
+      else if (VECTOR_MODE_P (mode)
+	       ? TREE_CODE (type) != TREE_CODE (TREE_TYPE (typefm))
+	       : TREE_CODE (type) != TREE_CODE (typefm))
 	{
 	  error ("mode %qs applied to inappropriate type", p);
 	  return NULL_TREE;