[PATCH] Named address support for GCC 4.5

Michael Meissner meissner@linux.vnet.ibm.com
Wed Dec 3 00:01:00 GMT 2008


On Tue, Dec 02, 2008 at 11:31:27PM +0000, Joseph S. Myers wrote:
> On Mon, 1 Dec 2008, Michael Meissner wrote:
> 
> > +/* Map an address space number into a name.  */
> > +static const char *
> > +spu_addr_space_name (addr_space_t addrspace)
> > +{
> > +  switch (addrspace)
> > +    {
> > +    case ADDR_SPACE_GENERIC:
> > +      return "generic";
> > +
> > +    case ADDR_SPACE_EA:
> > +      return "__ea";
> 
> Is generic meant to be a literal used in any language, as in this patch, 
> or should it be translated, in which case as I read the code this should 
> return _("generic")?

Yes, it should be translated.
 
> > @@ -4521,6 +4575,15 @@ grokdeclarator (const struct c_declarato
> >  	    if (really_funcdef)
> >  	      put_pending_sizes (arg_info->pending_sizes);
> >  
> > +	    /* Warn about functions declared in another address space.  */
> > +	    address_space = DECODE_QUAL_ADDR_SPACE (type_quals);
> > +	    if (address_space)
> > +	      {
> > +		type_quals = CLEAR_QUAL_ADDR_SPACE (type_quals);
> > +		error ("%qs specified for function %qs",
> > +		       targetm.addr_space.name (address_space), name);
> > +	      }
> 
> This case is for qualifiers on the return type of a function, not on the 
> function itself.  Is there anything to disallow such qualifiers, or should 
> they receive warnings as with normal qualifiers there?

After the patch is applied, line 4579 has this check for declaring functions
with named address spaces.

	    /* Warn about functions declared in another address space.  */
	    address_space = DECODE_QUAL_ADDR_SPACE (type_quals);
	    if (address_space)
	      {
		type_quals = CLEAR_QUAL_ADDR_SPACE (type_quals);
		error ("%qs specified for function %qs",
		       targetm.addr_space.name (address_space), name);
	      }


This is tested in the test suite:

extern __ea void f1 ();	 /* { dg-error "'__ea' specified for function 'f1'" } */

The above checks to strip the address space are to prevent duplicate warnings.

> >  	    if (pedantic && TREE_CODE (type) == FUNCTION_TYPE
> > -		&& type_quals)
> > +		&& CLEAR_QUAL_ADDR_SPACE (type_quals))
> >  	      pedwarn (input_location, OPT_pedantic,
> >  		       "ISO C forbids qualified function types");
> 
> In this and subsequent places with this diagnostic or the "ISO C forbids 
> const or volatile function types" one, however, such a prohibition applies 
> to address space qualifiers as well, so their use should be diagnosed.

Because an error is given previously, I didn't see the point of adding a
warning for this case.

> > Index: gcc/c-typeck.c
> > ===================================================================
> > --- gcc/c-typeck.c	(.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk)	(revision 142333)
> > +++ gcc/c-typeck.c	(working copy)
> > @@ -120,7 +120,8 @@ null_pointer_constant_p (const_tree expr
> >  	  && (INTEGRAL_TYPE_P (type)
> >  	      || (TREE_CODE (type) == POINTER_TYPE
> >  		  && VOID_TYPE_P (TREE_TYPE (type))
> > -		  && TYPE_QUALS (TREE_TYPE (type)) == TYPE_UNQUALIFIED)));
> > +		  && (TYPE_QUALS_NO_ADDR_SPACE (TREE_TYPE (type))
> > +		      == TYPE_UNQUALIFIED))));
> 
> "void" with address space qualifiers is not "void", so I do not believe 
> such a pointer is a null pointer constant.

Let me think on this.

> > @@ -4201,7 +4240,8 @@ convert_for_assignment (tree type, tree 
> 
> My previous comments on this function in 
> <http://gcc.gnu.org/ml/gcc-patches/2008-08/msg02424.html> still apply.
> 
> > @@ -8274,6 +8319,17 @@ build_binary_op (location_t location, en
> >  		  && TREE_CODE (tt1) == FUNCTION_TYPE)
> >  		pedwarn (location, OPT_pedantic, "ISO C forbids "
> >  			 "comparison of %<void *%> with function pointer");
> > +
> > +	      /* If this operand is a pointer into another address
> > +		 space, make the result of the comparison such a
> > +		 pointer also.  */
> > +	      if (POINTER_TYPE_P (type0)
> > +		  && (as = TYPE_ADDR_SPACE (TREE_TYPE (type0))))
> > +		{
> > +		  int qual = ENCODE_QUAL_ADDR_SPACE (as);
> > +		  result_type = build_pointer_type
> > +		    (build_qualified_type (void_type_node, qual));
> > +		}
> >  	    }
> >  	  else if (VOID_TYPE_P (tt1))
> >  	    {
> > @@ -8281,6 +8337,17 @@ build_binary_op (location_t location, en
> >  		  && TREE_CODE (tt0) == FUNCTION_TYPE)
> >  		pedwarn (location, OPT_pedantic, "ISO C forbids "
> >  			 "comparison of %<void *%> with function pointer");
> > +
> > +	      /* If this operand is a pointer into another address
> > +		 space, make the result of the comparison such a
> > +		 pointer also.  */
> > +	      if (POINTER_TYPE_P (type1)
> > +		  && (as = TYPE_ADDR_SPACE (TREE_TYPE (type1))))
> > +		{
> > +		  int qual = ENCODE_QUAL_ADDR_SPACE (as);
> > +		  result_type = build_pointer_type
> > +		    (build_qualified_type (void_type_node, qual));
> > +		}
> 
> I don't see the checks that the address spaces overlap, as noted in my 
> previous message.

I assume such checks would occur in the backend target hooks
TARGET_ADDR_SPACE_CAN_CONVERT_P and TARGET_ADDR_SPACE_NOP_CONVERT_P would be
the place to put such checks.

> I don't see any changes to the handling of conditional expressions, as 
> noted in my previous message.

I assumed the checks in common_pointer_type would catch the case of conditional
expressions as well as subtracting two different pointers.

-- 
Michael Meissner, IBM
4 Technology Place Drive, MS 2203A, Westford, MA, 01886, USA
meissner@linux.vnet.ibm.com



More information about the Gcc-patches mailing list