This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: revised varargs patch with references


> Date: Wed, 29 Mar 2000 01:11:31 -0800
> From: Richard Henderson <rth@cygnus.com>
> Cc: gcc-patches@gcc.gnu.org
> 
> On Tue, Mar 28, 2000 at 11:59:26AM -0800, Geoff Keating wrote:
> > This patch uses references to make it clear when the va_start and
> > va_copy macros change their first argument.  It works surprisingly
> > well, even in C (once you explain to convert_for_assignment what
> > references are).
> 
> Yes, this is much cleaner.
> 
> > Tested with no varargs failures on x86 linux...
> 
> Except that it crashes quite hard on alpha, right in
> the middle of the reference widgetry you added.  :-(

Ah. I'd used comp_target_types where I should have used comptypes.

Here's a revised patch.  Bootstraps & tests OK on x86 and alpha, and
tests OK on ppc.

OK to commit?

-- 
- Geoffrey Keating <geoffk@cygnus.com>

===File ~/patches/cygnus/egcs-vastart-fix-4.patch===========
2000-03-29  Geoff Keating  <geoffk@cygnus.com>

	* c-common.c (c_common_nodes_and_builtins): The first parameter to
	__builtin_va_start and __builtin_va_copy is now either a 'va_list'
	or a reference to a va_list.
	* builtins.c (stabilize_va_list): Simplify now we don't have to
	work around C array address decay.
	* c-typeck.c (convert_for_assignment): Handle assignment to
	a reference parameter by taking the address of the RHS.
	* ginclude/stdarg.h (va_start): Don't take address of first parameter.
	(va_copy): Likewise.
	(__va_copy): Likewise.
	* ginclude/varargs.h (va_start): Likewise.
	(__va_copy): Likewise.

Index: builtins.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/builtins.c,v
retrieving revision 1.41
diff -c -p -r1.41 builtins.c
*** builtins.c	2000/03/24 23:47:17	1.41
--- builtins.c	2000/03/29 15:33:50
*************** expand_builtin_next_arg (arglist)
*** 1901,1966 ****
     from multiple evaluations.  */
  
  static tree
! stabilize_va_list (valist, was_ptr)
       tree valist;
!      int was_ptr;
  {
    if (TREE_CODE (va_list_type_node) == ARRAY_TYPE)
      {
!       /* If stdarg.h took the address of an array-type valist that was passed
!          as a parameter, we'll have taken the address of the parameter itself
!          rather than the array as we'd intended.  Undo this mistake.  */
! 
!       if (was_ptr)
! 	{
! 	  STRIP_NOPS (valist);
! 
! 	  /* Two cases: either &array, which decomposed to 
! 	        <ptr <array <record> valist>>
! 	     or &ptr, which turned into
! 		<ptr <ptr <record>>>
! 	     In the first case we'll need to put the ADDR_EXPR back
! 	     after frobbing the types as if &array[0].  */
! 
! 	  if (TREE_CODE (valist) != ADDR_EXPR)
! 	    abort ();
! 	  valist = TREE_OPERAND (valist, 0);
! 	}
  
!       if (TYPE_MAIN_VARIANT (TREE_TYPE (valist))
! 	  == TYPE_MAIN_VARIANT (va_list_type_node))
! 	{
! 	  tree pt = build_pointer_type (TREE_TYPE (va_list_type_node));
! 	  valist = build1 (ADDR_EXPR, pt, valist);
! 	  TREE_SIDE_EFFECTS (valist)
! 	    = TREE_SIDE_EFFECTS (TREE_OPERAND (valist, 0));
! 	}
!       else
  	{
! 	  if (! POINTER_TYPE_P (TREE_TYPE (valist))
! 	      || (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (valist)))
! 		  != TYPE_MAIN_VARIANT (TREE_TYPE (va_list_type_node))))
! 	    abort ();
  	}
- 
-       if (TREE_SIDE_EFFECTS (valist))
- 	valist = save_expr (valist);
      }
    else
      {
!       if (! was_ptr)
! 	{
! 	  tree pt;
  
  	  if (! TREE_SIDE_EFFECTS (valist))
  	    return valist;
! 
  	  pt = build_pointer_type (va_list_type_node);
!           valist = fold (build1 (ADDR_EXPR, pt, valist));
  	  TREE_SIDE_EFFECTS (valist) = 1;
  	}
        if (TREE_SIDE_EFFECTS (valist))
!         valist = save_expr (valist);
        valist = fold (build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (valist)),
  			     valist));
      }
--- 1901,1943 ----
     from multiple evaluations.  */
  
  static tree
! stabilize_va_list (valist, needs_lvalue)
       tree valist;
!      int needs_lvalue;
  {
    if (TREE_CODE (va_list_type_node) == ARRAY_TYPE)
      {
!       if (TREE_SIDE_EFFECTS (valist))
! 	valist = save_expr (valist);
  
!       /* For this case, the backends will be expecting a pointer to
! 	 TREE_TYPE (va_list_type_node), but it's possible we've
! 	 actually been given an array (an actual va_list_type_node).
! 	 So fix it.  */
!       if (TREE_CODE (TREE_TYPE (valist)) == ARRAY_TYPE)
  	{
!  	  tree p1 = build_pointer_type (TREE_TYPE (va_list_type_node));
!  	  tree p2 = build_pointer_type (va_list_type_node);
!  	  valist = build1 (ADDR_EXPR, p2, valist);
! 	  valist = fold (build1 (NOP_EXPR, p1, valist));
  	}
      }
    else
      {
!       tree pt;
  
+       if (! needs_lvalue)
+ 	{
  	  if (! TREE_SIDE_EFFECTS (valist))
  	    return valist;
! 	  
  	  pt = build_pointer_type (va_list_type_node);
! 	  valist = fold (build1 (ADDR_EXPR, pt, valist));
  	  TREE_SIDE_EFFECTS (valist) = 1;
  	}
+ 
        if (TREE_SIDE_EFFECTS (valist))
! 	valist = save_expr (valist);
        valist = fold (build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (valist)),
  			     valist));
      }
Index: c-common.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/c-common.c,v
retrieving revision 1.102
diff -c -p -r1.102 c-common.c
*** c-common.c	2000/03/24 20:20:56	1.102
--- c-common.c	2000/03/29 15:33:51
*************** c_common_nodes_and_builtins (cplus_mode,
*** 3478,3484 ****
    tree traditional_cptr_type_node;
    tree traditional_len_type_node;
    tree traditional_len_endlink;
!   tree va_list_ptr_type_node;
    tree va_list_arg_type_node;
  
    pushdecl (build_decl (TYPE_DECL, get_identifier ("__builtin_va_list"),
--- 3478,3484 ----
    tree traditional_cptr_type_node;
    tree traditional_len_type_node;
    tree traditional_len_endlink;
!   tree va_list_ref_type_node;
    tree va_list_arg_type_node;
  
    pushdecl (build_decl (TYPE_DECL, get_identifier ("__builtin_va_list"),
*************** c_common_nodes_and_builtins (cplus_mode,
*** 3490,3502 ****
    pushdecl (build_decl (TYPE_DECL, get_identifier ("__builtin_size_t"),
  			sizetype));
  
-   va_list_ptr_type_node = build_pointer_type (va_list_type_node);
- 
    if (TREE_CODE (va_list_type_node) == ARRAY_TYPE)
!     va_list_arg_type_node = build_pointer_type (TREE_TYPE (va_list_type_node));
    else
!     va_list_arg_type_node = va_list_type_node;
! 
    endlink = void_list_node;
    int_endlink = tree_cons (NULL_TREE, integer_type_node, endlink);
    double_endlink = tree_cons (NULL_TREE, double_type_node, endlink);
--- 3490,3506 ----
    pushdecl (build_decl (TYPE_DECL, get_identifier ("__builtin_size_t"),
  			sizetype));
  
    if (TREE_CODE (va_list_type_node) == ARRAY_TYPE)
!     {
!       va_list_arg_type_node = va_list_ref_type_node =
! 	build_pointer_type (TREE_TYPE (va_list_type_node));
!     }
    else
!     {
!       va_list_arg_type_node = va_list_type_node;
!       va_list_ref_type_node = build_reference_type (va_list_type_node);
!     }
!  
    endlink = void_list_node;
    int_endlink = tree_cons (NULL_TREE, integer_type_node, endlink);
    double_endlink = tree_cons (NULL_TREE, double_type_node, endlink);
*************** c_common_nodes_and_builtins (cplus_mode,
*** 3725,3752 ****
    builtin_function ("__builtin_varargs_start",
  		    build_function_type (void_type_node,
  					 tree_cons (NULL_TREE,
! 						    va_list_ptr_type_node,
  						    endlink)),
  		    BUILT_IN_VARARGS_START, BUILT_IN_NORMAL, NULL_PTR);
  
    builtin_function ("__builtin_stdarg_start",
  		    build_function_type (void_type_node,
  					 tree_cons (NULL_TREE,
! 						    va_list_ptr_type_node,
  						    NULL_TREE)),
  		    BUILT_IN_STDARG_START, BUILT_IN_NORMAL, NULL_PTR);
  
    builtin_function ("__builtin_va_end",
  		    build_function_type (void_type_node,
  					 tree_cons (NULL_TREE,
! 						    va_list_arg_type_node,
  						    endlink)),
  		    BUILT_IN_VA_END, BUILT_IN_NORMAL, NULL_PTR);
  
    builtin_function ("__builtin_va_copy",
  		    build_function_type (void_type_node,
  					 tree_cons (NULL_TREE,
! 						    va_list_ptr_type_node,
  						    tree_cons (NULL_TREE,
  						      va_list_arg_type_node,
  						      endlink))),
--- 3729,3756 ----
    builtin_function ("__builtin_varargs_start",
  		    build_function_type (void_type_node,
  					 tree_cons (NULL_TREE,
! 						    va_list_ref_type_node,
  						    endlink)),
  		    BUILT_IN_VARARGS_START, BUILT_IN_NORMAL, NULL_PTR);
  
    builtin_function ("__builtin_stdarg_start",
  		    build_function_type (void_type_node,
  					 tree_cons (NULL_TREE,
! 						    va_list_ref_type_node,
  						    NULL_TREE)),
  		    BUILT_IN_STDARG_START, BUILT_IN_NORMAL, NULL_PTR);
  
    builtin_function ("__builtin_va_end",
  		    build_function_type (void_type_node,
  					 tree_cons (NULL_TREE,
! 						    va_list_ref_type_node,
  						    endlink)),
  		    BUILT_IN_VA_END, BUILT_IN_NORMAL, NULL_PTR);
  
    builtin_function ("__builtin_va_copy",
  		    build_function_type (void_type_node,
  					 tree_cons (NULL_TREE,
! 						    va_list_ref_type_node,
  						    tree_cons (NULL_TREE,
  						      va_list_arg_type_node,
  						      endlink))),
Index: c-typeck.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/c-typeck.c,v
retrieving revision 1.60
diff -c -p -r1.60 c-typeck.c
*** c-typeck.c	2000/03/25 18:34:00	1.60
--- c-typeck.c	2000/03/29 15:33:51
*************** convert_for_assignment (type, rhs, errty
*** 3960,3970 ****
        error ("void value not ignored as it ought to be");
        return error_mark_node;
      }
    /* Arithmetic types all interconvert, and enum is treated like int.  */
!   if ((codel == INTEGER_TYPE || codel == REAL_TYPE || codel == ENUMERAL_TYPE
!        || codel == COMPLEX_TYPE)
!       && (coder == INTEGER_TYPE || coder == REAL_TYPE || coder == ENUMERAL_TYPE
! 	  || coder == COMPLEX_TYPE))
      return convert_and_check (type, rhs);
  
    /* Conversion to a transparent union from its member types.
--- 3960,3991 ----
        error ("void value not ignored as it ought to be");
        return error_mark_node;
      }
+   /* A type converts to a reference to it.  
+      This code doesn't fully support references, it's just for the
+      special case of va_start and va_copy.  */
+   if (codel == REFERENCE_TYPE
+       && comptypes (TREE_TYPE (type), TREE_TYPE (rhs)) == 1)
+     {
+       if (mark_addressable (rhs) == 0)
+ 	return error_mark_node;
+       rhs = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (rhs)), rhs);
+ 
+       /* We already know that these two types are compatible, but they
+ 	 may not be exactly identical.  In fact, `TREE_TYPE (type)' is
+ 	 likely to be __builtin_va_list and `TREE_TYPE (rhs)' is
+ 	 likely to be va_list, a typedef to __builtin_va_list, which
+ 	 is different enough that it will cause problems later.  */
+       if (TREE_TYPE (TREE_TYPE (rhs)) != TREE_TYPE (type))
+ 	rhs = build1 (NOP_EXPR, build_pointer_type (TREE_TYPE (type)), rhs);
+ 
+       rhs = build1 (NOP_EXPR, type, rhs);
+       return rhs;
+     }
    /* Arithmetic types all interconvert, and enum is treated like int.  */
!   else if ((codel == INTEGER_TYPE || codel == REAL_TYPE 
! 	    || codel == ENUMERAL_TYPE || codel == COMPLEX_TYPE)
! 	   && (coder == INTEGER_TYPE || coder == REAL_TYPE 
! 	       || coder == ENUMERAL_TYPE || coder == COMPLEX_TYPE))
      return convert_and_check (type, rhs);
  
    /* Conversion to a transparent union from its member types.
Index: ginclude/stdarg.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/ginclude/stdarg.h,v
retrieving revision 1.14
diff -c -p -r1.14 stdarg.h
*** stdarg.h	2000/01/13 00:37:06	1.14
--- stdarg.h	2000/03/29 15:33:51
***************
*** 1,4 ****
! /* Copyright (C) 1989, 1997, 1998, 1999 Free Software Foundation, Inc.
  
  This file is part of GNU CC.
  
--- 1,4 ----
! /* Copyright (C) 1989, 1997, 1998, 1999, 2000 Free Software Foundation, Inc.
  
  This file is part of GNU CC.
  
*************** typedef __builtin_va_list __gnuc_va_list
*** 51,63 ****
     actual type **after default promotions**.
     Thus, va_arg (..., short) is not valid.  */
  
! #define va_start(v,l)	__builtin_stdarg_start(&(v),l)
  #define va_end		__builtin_va_end
  #define va_arg		__builtin_va_arg
  #if !defined(__STRICT_ANSI__) || __STDC_VERSION__ + 0 >= 199900L
! #define va_copy(d,s)	__builtin_va_copy(&(d),(s))
  #endif
! #define __va_copy(d,s)	__builtin_va_copy(&(d),(s))
  
  
  /* Define va_list, if desired, from __gnuc_va_list. */
--- 51,63 ----
     actual type **after default promotions**.
     Thus, va_arg (..., short) is not valid.  */
  
! #define va_start(v,l)	__builtin_stdarg_start((v),l)
  #define va_end		__builtin_va_end
  #define va_arg		__builtin_va_arg
  #if !defined(__STRICT_ANSI__) || __STDC_VERSION__ + 0 >= 199900L
! #define va_copy(d,s)	__builtin_va_copy((d),(s))
  #endif
! #define __va_copy(d,s)	__builtin_va_copy((d),(s))
  
  
  /* Define va_list, if desired, from __gnuc_va_list. */
Index: ginclude/varargs.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/ginclude/varargs.h,v
retrieving revision 1.14
diff -c -p -r1.14 varargs.h
*** varargs.h	2000/01/13 00:37:06	1.14
--- varargs.h	2000/03/29 15:33:51
***************
*** 1,4 ****
! /* Copyright (C) 1989, 1997, 1998, 1999 Free Software Foundation, Inc.
  
  This file is part of GNU CC.
  
--- 1,4 ----
! /* Copyright (C) 1989, 1997, 1998, 1999, 2000 Free Software Foundation, Inc.
  
  This file is part of GNU CC.
  
*************** typedef int __builtin_va_alist_t __attri
*** 63,72 ****
  typedef __builtin_va_list __gnuc_va_list;
  #endif
  
! #define va_start(v)	__builtin_varargs_start(&(v))
  #define va_end		__builtin_va_end
  #define va_arg		__builtin_va_arg
! #define __va_copy(d,s)	__builtin_va_copy(&(d),(s))
  
  /* Define va_list from __gnuc_va_list.  */
  
--- 63,72 ----
  typedef __builtin_va_list __gnuc_va_list;
  #endif
  
! #define va_start(v)	__builtin_varargs_start((v))
  #define va_end		__builtin_va_end
  #define va_arg		__builtin_va_arg
! #define __va_copy(d,s)	__builtin_va_copy((d),(s))
  
  /* Define va_list from __gnuc_va_list.  */
  
============================================================

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]