public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RFA: Fix middle-end/46500
@ 2010-11-17  7:25 Joern Rennecke
  2010-11-17 15:50 ` Paul Koning
  2010-11-19  5:24 ` Updated: RFA: Fix middle-end/46500 (void * encapsulated) Joern Rennecke
  0 siblings, 2 replies; 27+ messages in thread
From: Joern Rennecke @ 2010-11-17  7:25 UTC (permalink / raw)
  To: gcc-patches

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

target.h includes tm.h in order to get the CUMULATIVE_ARGS type.
That means that target.h has become completely ineffective as a means
of hiding target specific implementation details while allowing access
to the functionality.
In fact, I found three front end files that included target.h so
that they could use macros from tm.h

The attached patch rectifies the situation by replacing CUMULATIVE_ARGS *
with cumulative_args_t, which is typedefed as void *, in the target hook
interface.

bootstrapped on i686-pc-linux-gnu .

cross-tested on i686-pc-linux-gnu configured with --enable-werror-always
using gcc (GCC) 4.6.0 20101111 (experimental) for the following targets:

alpha-linux-gnu hppa-linux-gnu mips-elf sh-elf arc-elf ia64-elf
mmix-knuth-mmixware sparc-elf arm-eabi iq2000-elf mn10300-elf spu-elf
avr-elf lm32-elf moxie-elf v850-elf bfin-elf m32c-elf pdp11-aout
vax-linux-gnu cris-elf m32r-elf xstormy16-elf ppc-elf fr30-elf m68k-elf
rx-elf frv-elf mcore-elf s390-linux-gnu h8300-elf mep-elf score-elf

The following three targets required manual intervention to get beyond
pre-existing warning issues:
crx-elf m68hc11-elf xtensa-elf

And finally, picochip-elf couldn't be built because of some per-existing
errors.

[-- Attachment #2: pr46500-20101117-0413.bz2 --]
[-- Type: application/x-bzip-compressed-tar, Size: 18498 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: RFA: Fix middle-end/46500
  2010-11-17  7:25 RFA: Fix middle-end/46500 Joern Rennecke
@ 2010-11-17 15:50 ` Paul Koning
  2010-11-19  5:24 ` Updated: RFA: Fix middle-end/46500 (void * encapsulated) Joern Rennecke
  1 sibling, 0 replies; 27+ messages in thread
From: Paul Koning @ 2010-11-17 15:50 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: gcc-patches


On Nov 16, 2010, at 11:46 PM, Joern Rennecke wrote:

> target.h includes tm.h in order to get the CUMULATIVE_ARGS type.
> That means that target.h has become completely ineffective as a means
> of hiding target specific implementation details while allowing access
> to the functionality.
> In fact, I found three front end files that included target.h so
> that they could use macros from tm.h
> 
> The attached patch rectifies the situation by replacing CUMULATIVE_ARGS *
> with cumulative_args_t, which is typedefed as void *, in the target hook
> interface.
> 
> bootstrapped on i686-pc-linux-gnu .
> 
> cross-tested on i686-pc-linux-gnu configured with --enable-werror-always
> using gcc (GCC) 4.6.0 20101111 (experimental) for the following targets:
> 
> alpha-linux-gnu hppa-linux-gnu mips-elf sh-elf arc-elf ia64-elf
> mmix-knuth-mmixware sparc-elf arm-eabi iq2000-elf mn10300-elf spu-elf
> avr-elf lm32-elf moxie-elf v850-elf bfin-elf m32c-elf pdp11-aout
> vax-linux-gnu cris-elf m32r-elf xstormy16-elf ppc-elf fr30-elf m68k-elf
> rx-elf frv-elf mcore-elf s390-linux-gnu h8300-elf mep-elf score-elf
> 
> The following three targets required manual intervention to get beyond
> pre-existing warning issues:
> crx-elf m68hc11-elf xtensa-elf
> 
> And finally, picochip-elf couldn't be built because of some per-existing
> errors.
> <pr46500-20101117-0413.bz2>

pdp11 is ok.

	paul

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Updated: RFA: Fix middle-end/46500 (void * encapsulated)
  2010-11-17  7:25 RFA: Fix middle-end/46500 Joern Rennecke
  2010-11-17 15:50 ` Paul Koning
@ 2010-11-19  5:24 ` Joern Rennecke
  2010-11-19 17:04   ` Paul Koning
                     ` (3 more replies)
  1 sibling, 4 replies; 27+ messages in thread
From: Joern Rennecke @ 2010-11-19  5:24 UTC (permalink / raw)
  To: gcc-patches

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

This is an update to this patch:
http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01769.html

The change from the previous version of this patch is that the void pointers
are encapsulated into a struct or union to provide static type checking,
as recently discussed on the gcc mailing list:

http://gcc.gnu.org/ml/gcc/2010-11/msg00471.html
http://gcc.gnu.org/ml/gcc/2010-11/msg00476.html
http://gcc.gnu.org/ml/gcc/2010-11/msg00474.html

With ENABLE_CHECKING, also I do a runtime check of a magic value that
identifies the targetm.calls structure this type pertains to (in this  
implementation,  I actually use the address of that struct).

This patch remains an alternative to this patch:
http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01810.html

I don't personally favour one approach over the other, I just want to
get the CUMULATIVE_ARGS dependency out of the taget hook interface,
so that I can get on with getting tm.h dependencies out of the frontends
and tree optimizers. (Next milestone would be function.h with rtl_data.)

Bootstrapped on i686-pc-linux-gnu.

I also did a simple all-gcc build on i686-pc-linux-gnu
with --enable-werror-always --disable-checking (to test the transparent
union mechanism)

Cross-built with --enable-werror-always for:
alpha-linux-gnu hppa-linux-gnu mips-elf sh-elf arc-elf ia64-elf
mmix-knuth-mmixware sparc-elf arm-eabi iq2000-elf mn10300-elf spu-elf avr-elf
lm32-elf moxie-elf v850-elf bfin-elf m32c-elf pdp11-aout vax-linux-gnu
cris-elf m32r-elf picochip-elf xstormy16-elf ppc-elf xtensa-elf fr30-elf
m68k-elf rx-elf frv-elf mcore-elf s390-linux-gnu h8300-elf mep-elf score-elf


crx-elf and m68hc11-elf required manual intervention to get past issues
with pre-existing warnings.

[-- Attachment #2: pr46500-struct-201019-0236.bz2 --]
[-- Type: application/x-bzip-compressed-tar, Size: 21854 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Updated: RFA: Fix middle-end/46500 (void * encapsulated)
  2010-11-19  5:24 ` Updated: RFA: Fix middle-end/46500 (void * encapsulated) Joern Rennecke
@ 2010-11-19 17:04   ` Paul Koning
  2010-11-19 18:33   ` P.S.: " Joern Rennecke
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 27+ messages in thread
From: Paul Koning @ 2010-11-19 17:04 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: gcc-patches


On Nov 18, 2010, at 10:32 PM, Joern Rennecke wrote:

> This is an update to this patch:
> http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01769.html
...

pdp11 is ok.  Thanks!

	paul

^ permalink raw reply	[flat|nested] 27+ messages in thread

* P.S.: Updated: RFA: Fix middle-end/46500 (void * encapsulated)
  2010-11-19  5:24 ` Updated: RFA: Fix middle-end/46500 (void * encapsulated) Joern Rennecke
  2010-11-19 17:04   ` Paul Koning
@ 2010-11-19 18:33   ` Joern Rennecke
  2010-11-22 21:49     ` P.P.S.: " Joern Rennecke
  2010-11-26  0:44   ` Ping: " Joern Rennecke
  2011-05-07 15:53   ` Updated^2: " Joern Rennecke
  3 siblings, 1 reply; 27+ messages in thread
From: Joern Rennecke @ 2010-11-19 18:33 UTC (permalink / raw)
  To: gcc-patches, gcc

Quoting Joern Rennecke <amylaar@spamcop.net>:

> This is an update to this patch:
> http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01769.html

At 160 KB, this patch has become somewhat largish.

The breakdown is approximately 30 KB target independent code, 120 KB
target port code in gcc/config, and 10 KB ChangeLog, most of which is for
the target port code, too.

If it helps the review process, I could decouple the middle-end end the
target part, by checking config/${cpu_type}/${cpu_type}.c if it mentions
cumulative_args_t, and set a macro if that is the case.

Then for unconverted ports, target.h can continue to include "tm.h" and do:
typedef CUMULATIVE_ARGS *cumulative_args_t;

to provide backward compatibility.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* P.P.S.: Updated: RFA: Fix middle-end/46500 (void * encapsulated)
  2010-11-19 18:33   ` P.S.: " Joern Rennecke
@ 2010-11-22 21:49     ` Joern Rennecke
  0 siblings, 0 replies; 27+ messages in thread
From: Joern Rennecke @ 2010-11-22 21:49 UTC (permalink / raw)
  To: gcc-patches

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

Quoting Joern Rennecke <amylaar@spamcop.net>:

> Quoting Joern Rennecke <amylaar@spamcop.net>:
> If it helps the review process, I could decouple the middle-end end the
> target part, by checking config/${cpu_type}/${cpu_type}.c if it mentions
> cumulative_args_t, and set a macro if that is the case.
>
> Then for unconverted ports, target.h can continue to include "tm.h" and do:
> typedef CUMULATIVE_ARGS *cumulative_args_t;
>
> to provide backward compatibility.

This patch set shows how this can be implemented.  I originally though about
putting the test for cumulative_args_t being used by the target into
configure, but we can't put the result into tm_defines - we want to get
away from including tm.h, messing with the compiler flags is ... messy ...
because that would interfere with flag overrides, and there are way to many
Makefile rules affected to change them all for a temporary phasing-in hack.
Therefore, I use a generated file, insn-cumulative_args_t, to convey the
result of the test.  As this file needs new Makefile rules, I put the check
straight in the Makefile rule that generates this file.

Unconverted ports will miss a dependency on $(TM_H) in $(TARGET_H), but that
should only matter if CUMULATIVE_ARGS is changed in
config/${cpu_type}/${cpu_type}.h - and when a port maintainer does that, (s)he
might as well update the argument types for the CUMULATIVE_ARGS taking hook
functions; the patches for all the official ports are in the second  
attachment,
and any new / unoffical ports should also be easy to adjust.

N.B. this patch makes PR target/46608 more likely to be visible.

Bootstrapped without target specific patches on i686-pc-linux-gnu

Bootstrapped & regression tested with target specific patches on
i686-pc-linux-gnu .

build-gcc cross-test succeeded with target-specific patches
on i686-pc-linux-gnu for the following targets:

alpha-linux-gnu hppa-linux-gnu mips-elf sh-elf arc-elf ia64-elf
mmix-knuth-mmixware sparc-elf arm-eabi iq2000-elf mn10300-elf spu-elf
avr-elf lm32-elf moxie-elf v850-elf bfin-elf pdp11-aout vax-linux-gnu
cris-elf m32r-elf xstormy16-elf m68hc11-elf ppc-elf xtensa-elf fr30-elf
m68k-elf rx-elf frv-elf mcore-elf s390-linux-gnu h8300-elf mep-elf
score-elf

picochip-elf is affected by PR46608 - build succeeds with manual make of
generated files or with the proposed patch for PR46608 applied.

m32c-elf does not build since r167020.

crx-elf is still affected by PR46434 .

[-- Attachment #2: pr46500-struct-notgt-20101122 --]
[-- Type: text/plain, Size: 34917 bytes --]

2010-11-19  Joern Rennecke  <amylaar@spamcop.net>

	PR middle-end/46500
gcc:
	* doc/tm.texi: Regenerate.
	* targhooks.c (default_setup_incoming_varargs): Replace
	CUMULATIVE_ARGS* argument type with cumulative_args_t.
	(default_pretend_outgoing_varargs_named): Likewise.
	(hook_pass_by_reference_must_pass_in_stack): Likewise.
	(hook_callee_copies_named): Likewise.
	(default_function_arg_advance): Likewise.
	(default_function_arg): Likewise.
	(default_function_incoming_arg): Likewise.
	(hook_bool_CUMULATIVE_ARGS_false): Likewise.
	(hook_bool_CUMULATIVE_ARGS_true): Likewise.
	(hook_bool_CUMULATIVE_ARGS_mode_tree_bool_false): Likewise.
	(hook_bool_CUMULATIVE_ARGS_mode_tree_bool_true): Likewise.
	(hook_int_CUMULATIVE_ARGS_mode_tree_bool_0): Likewise.
	* targhooks.h (default_setup_incoming_varargs): Likewise.
	(default_pretend_outgoing_varargs_named): Likewise.
	(hook_pass_by_reference_must_pass_in_stack): Likewise.
	(hook_callee_copies_named): Likewise.
	(default_function_arg_advance): Likewise.
	(default_function_arg): Likewise.
	(default_function_incoming_arg): Likewise.
	(hook_bool_CUMULATIVE_ARGS_false): Likewise.
	(hook_bool_CUMULATIVE_ARGS_true): Likewise.
	(hook_bool_CUMULATIVE_ARGS_mode_tree_bool_false): Likewise.
	(hook_bool_CUMULATIVE_ARGS_mode_tree_bool_true): Likewise.
	(hook_int_CUMULATIVE_ARGS_mode_tree_bool_0): Likewise.
	* target.def (pass_by_reference): Likewise.
	(setup_incoming_varargs, strict_argument_naming): Likewise.
	(pretend_outgoing_varargs_named, callee_copies): Likewise.
	(arg_partial_bytes, function_arg_advance, function_arg): Likewise.
	(function_incoming_arg): Likewise.
	* target.h: Don't include "tm.h" .
	Include "insn-cumulative_args_t.h" .
	(cumulative_args_t): New typedef.
	[GCC_TM_H] (get_cumulative_args): New static inline function.
	[GCC_TM_H] (pack_cumulative_args): Likewise.
	* calls.c (initialize_argument_information): Use pack_cumulative_args.
	(expand_call, emit_library_call_value_1): Likewise.
	* dse.c (get_call_args): Likewise.
	* expr.c (block_move_libcall_safe_for_call_parm): Likewise.
	* function.c (pass_by_reference, reference_callee_copied): Likewise.
	(assign_parm_find_data_types, assign_parms_setup_varargs): Likewise.
	(assign_parm_find_entry_rtl, assign_parms): Likewise.
	(gimplify_parameters): Likewise.
	* Makefile.in (TARGET_H): Replace $(TM_H) with insn-cumulative_args_t.h.
	(insn-cumulative_args_t.h, s-cumulative_args_t): New rules.
gcc/c-family:
	* c-opts.c: Include "tm.h" .
gcc/java:
	* expr.c: Include "tm.h" .
gcc/fortran:
	* trans-types.c: Include "tm.h" .

Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 167032)
+++ gcc/doc/tm.texi	(working copy)
@@ -4051,7 +4051,7 @@ @defmac FUNCTION_INCOMING_ARG (@var{cum}
 serves both purposes.
 @end defmac
 
-@deftypefn {Target Hook} int TARGET_ARG_PARTIAL_BYTES (CUMULATIVE_ARGS *@var{cum}, enum machine_mode @var{mode}, tree @var{type}, bool @var{named})
+@deftypefn {Target Hook} int TARGET_ARG_PARTIAL_BYTES (cumulative_args_t @var{cum}, enum machine_mode @var{mode}, tree @var{type}, bool @var{named})
 This target hook returns the number of bytes at the beginning of an
 argument that must be put in registers.  The value must be zero for
 arguments that are passed entirely in registers or that are entirely
@@ -4070,7 +4070,7 @@ structure) crosses that boundary, its fi
 @code{FUNCTION_INCOMING_ARG}, for the called function.
 @end deftypefn
 
-@deftypefn {Target Hook} bool TARGET_PASS_BY_REFERENCE (CUMULATIVE_ARGS *@var{cum}, enum machine_mode @var{mode}, const_tree @var{type}, bool @var{named})
+@deftypefn {Target Hook} bool TARGET_PASS_BY_REFERENCE (cumulative_args_t @var{cum}, enum machine_mode @var{mode}, const_tree @var{type}, bool @var{named})
 This target hook should return @code{true} if an argument at the
 position indicated by @var{cum} should be passed by reference.  This
 predicate is queried after target independent reasons for being
@@ -4082,7 +4082,7 @@ passed by reference, such as @code{TREE_
 to that type.
 @end deftypefn
 
-@deftypefn {Target Hook} bool TARGET_CALLEE_COPIES (CUMULATIVE_ARGS *@var{cum}, enum machine_mode @var{mode}, const_tree @var{type}, bool @var{named})
+@deftypefn {Target Hook} bool TARGET_CALLEE_COPIES (cumulative_args_t @var{cum}, enum machine_mode @var{mode}, const_tree @var{type}, bool @var{named})
 The function argument described by the parameters to this hook is
 known to be passed by reference.  The hook should return true if the
 function argument should be copied by the callee instead of copied
@@ -5016,7 +5016,7 @@ @deftypefn {Target Hook} rtx TARGET_EXPA
 to use as the return of @code{__builtin_saveregs}.
 @end deftypefn
 
-@deftypefn {Target Hook} void TARGET_SETUP_INCOMING_VARARGS (CUMULATIVE_ARGS *@var{args_so_far}, enum machine_mode @var{mode}, tree @var{type}, int *@var{pretend_args_size}, int @var{second_time})
+@deftypefn {Target Hook} void TARGET_SETUP_INCOMING_VARARGS (cumulative_args_t @var{args_so_far}, enum machine_mode @var{mode}, tree @var{type}, int *@var{pretend_args_size}, int @var{second_time})
 This target hook offers an alternative to using
 @code{__builtin_saveregs} and defining the hook
 @code{TARGET_EXPAND_BUILTIN_SAVEREGS}.  Use it to store the anonymous
@@ -5050,7 +5050,7 @@ structure, containing the values that ar
 not generate any instructions in this case.
 @end deftypefn
 
-@deftypefn {Target Hook} bool TARGET_STRICT_ARGUMENT_NAMING (CUMULATIVE_ARGS *@var{ca})
+@deftypefn {Target Hook} bool TARGET_STRICT_ARGUMENT_NAMING (cumulative_args_t @var{ca})
 Define this hook to return @code{true} if the location where a function
 argument is passed depends on whether or not it is a named argument.
 
@@ -5065,7 +5065,7 @@ @deftypefn {Target Hook} bool TARGET_STR
 You need not define this hook if it always returns @code{false}.
 @end deftypefn
 
-@deftypefn {Target Hook} bool TARGET_PRETEND_OUTGOING_VARARGS_NAMED (CUMULATIVE_ARGS *@var{ca})
+@deftypefn {Target Hook} bool TARGET_PRETEND_OUTGOING_VARARGS_NAMED (cumulative_args_t @var{ca})
 If you need to conditionally change ABIs so that one works with
 @code{TARGET_SETUP_INCOMING_VARARGS}, but the other works like neither
 @code{TARGET_SETUP_INCOMING_VARARGS} nor @code{TARGET_STRICT_ARGUMENT_NAMING} was
Index: gcc/targhooks.c
===================================================================
--- gcc/targhooks.c	(revision 167032)
+++ gcc/targhooks.c	(working copy)
@@ -167,7 +167,7 @@ default_expand_builtin_saveregs (void)
 }
 
 void
-default_setup_incoming_varargs (CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED,
+default_setup_incoming_varargs (cumulative_args_t ca ATTRIBUTE_UNUSED,
 				enum machine_mode mode ATTRIBUTE_UNUSED,
 				tree type ATTRIBUTE_UNUSED,
 				int *pretend_arg_size ATTRIBUTE_UNUSED,
@@ -186,13 +186,13 @@ default_builtin_setjmp_frame_value (void
 /* Generic hook that takes a CUMULATIVE_ARGS pointer and returns false.  */
 
 bool
-hook_bool_CUMULATIVE_ARGS_false (CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED)
+hook_bool_CUMULATIVE_ARGS_false (cumulative_args_t ca ATTRIBUTE_UNUSED)
 {
   return false;
 }
 
 bool
-default_pretend_outgoing_varargs_named (CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED)
+default_pretend_outgoing_varargs_named (cumulative_args_t ca ATTRIBUTE_UNUSED)
 {
   return (targetm.calls.setup_incoming_varargs
 	  != default_setup_incoming_varargs);
@@ -250,7 +250,7 @@ default_mode_rep_extended (enum machine_
 /* Generic hook that takes a CUMULATIVE_ARGS pointer and returns true.  */
 
 bool
-hook_bool_CUMULATIVE_ARGS_true (CUMULATIVE_ARGS * a ATTRIBUTE_UNUSED)
+hook_bool_CUMULATIVE_ARGS_true (cumulative_args_t a ATTRIBUTE_UNUSED)
 {
   return true;
 }
@@ -299,7 +299,7 @@ default_cxx_get_cookie_size (tree type)
    of the TARGET_PASS_BY_REFERENCE hook uses just MUST_PASS_IN_STACK.  */
 
 bool
-hook_pass_by_reference_must_pass_in_stack (CUMULATIVE_ARGS *c ATTRIBUTE_UNUSED,
+hook_pass_by_reference_must_pass_in_stack (cumulative_args_t c ATTRIBUTE_UNUSED,
 	enum machine_mode mode ATTRIBUTE_UNUSED, const_tree type ATTRIBUTE_UNUSED,
 	bool named_arg ATTRIBUTE_UNUSED)
 {
@@ -310,7 +310,7 @@ hook_pass_by_reference_must_pass_in_stac
    version of the hook is true for all named arguments.  */
 
 bool
-hook_callee_copies_named (CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED,
+hook_callee_copies_named (cumulative_args_t ca ATTRIBUTE_UNUSED,
 			  enum machine_mode mode ATTRIBUTE_UNUSED,
 			  const_tree type ATTRIBUTE_UNUSED, bool named)
 {
@@ -541,7 +541,7 @@ default_builtin_reciprocal (unsigned int
 
 bool
 hook_bool_CUMULATIVE_ARGS_mode_tree_bool_false (
-	CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED,
+	cumulative_args_t ca ATTRIBUTE_UNUSED,
 	enum machine_mode mode ATTRIBUTE_UNUSED,
 	const_tree type ATTRIBUTE_UNUSED, bool named ATTRIBUTE_UNUSED)
 {
@@ -550,7 +550,7 @@ hook_bool_CUMULATIVE_ARGS_mode_tree_bool
 
 bool
 hook_bool_CUMULATIVE_ARGS_mode_tree_bool_true (
-	CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED,
+	cumulative_args_t ca ATTRIBUTE_UNUSED,
 	enum machine_mode mode ATTRIBUTE_UNUSED,
 	const_tree type ATTRIBUTE_UNUSED, bool named ATTRIBUTE_UNUSED)
 {
@@ -559,7 +559,7 @@ hook_bool_CUMULATIVE_ARGS_mode_tree_bool
 
 int
 hook_int_CUMULATIVE_ARGS_mode_tree_bool_0 (
-	CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED,
+	cumulative_args_t ca ATTRIBUTE_UNUSED,
 	enum machine_mode mode ATTRIBUTE_UNUSED,
 	tree type ATTRIBUTE_UNUSED, bool named ATTRIBUTE_UNUSED)
 {
@@ -567,41 +567,43 @@ hook_int_CUMULATIVE_ARGS_mode_tree_bool_
 }
 
 void
-default_function_arg_advance (CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED,
+default_function_arg_advance (cumulative_args_t ca ATTRIBUTE_UNUSED,
 			      enum machine_mode mode ATTRIBUTE_UNUSED,
 			      const_tree type ATTRIBUTE_UNUSED,
 			      bool named ATTRIBUTE_UNUSED)
 {
 #ifdef FUNCTION_ARG_ADVANCE
-  CUMULATIVE_ARGS args = *ca;
+  CUMULATIVE_ARGS args = *get_cumulative_args (ca);
   FUNCTION_ARG_ADVANCE (args, mode, CONST_CAST_TREE (type), named);
-  *ca = args;
+  *ca = pack_cumulative_args (args);
 #else
   gcc_unreachable ();
 #endif
 }
 
 rtx
-default_function_arg (CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED,
+default_function_arg (cumulative_args_t ca ATTRIBUTE_UNUSED,
 		      enum machine_mode mode ATTRIBUTE_UNUSED,
 		      const_tree type ATTRIBUTE_UNUSED,
 		      bool named ATTRIBUTE_UNUSED)
 {
 #ifdef FUNCTION_ARG
-  return FUNCTION_ARG (*ca, mode, CONST_CAST_TREE (type), named);
+  return FUNCTION_ARG (*get_cumulative_args (ca), mode, CONST_CAST_TREE (type),
+		       named);
 #else
   gcc_unreachable ();
 #endif
 }
 
 rtx
-default_function_incoming_arg (CUMULATIVE_ARGS *ca ATTRIBUTE_UNUSED,
+default_function_incoming_arg (cumulative_args_t ca ATTRIBUTE_UNUSED,
 			       enum machine_mode mode ATTRIBUTE_UNUSED,
 			       const_tree type ATTRIBUTE_UNUSED,
 			       bool named ATTRIBUTE_UNUSED)
 {
 #ifdef FUNCTION_INCOMING_ARG
-  return FUNCTION_INCOMING_ARG (*ca, mode, CONST_CAST_TREE (type), named);
+  return FUNCTION_INCOMING_ARG (*get_cumulative_args (ca), mode,
+				CONST_CAST_TREE (type), named);
 #else
   gcc_unreachable ();
 #endif
Index: gcc/targhooks.h
===================================================================
--- gcc/targhooks.h	(revision 167032)
+++ gcc/targhooks.h	(working copy)
@@ -35,9 +35,9 @@ extern enum machine_mode default_cc_mode
 extern bool default_return_in_memory (const_tree, const_tree);
 
 extern rtx default_expand_builtin_saveregs (void);
-extern void default_setup_incoming_varargs (CUMULATIVE_ARGS *, enum machine_mode, tree, int *, int);
+extern void default_setup_incoming_varargs (cumulative_args_t, enum machine_mode, tree, int *, int);
 extern rtx default_builtin_setjmp_frame_value (void);
-extern bool default_pretend_outgoing_varargs_named (CUMULATIVE_ARGS *);
+extern bool default_pretend_outgoing_varargs_named (cumulative_args_t);
 
 extern enum machine_mode default_eh_return_filter_mode (void);
 extern enum machine_mode default_libgcc_cmp_return_mode (void);
@@ -58,9 +58,9 @@ extern tree default_cxx_guard_type (void
 extern tree default_cxx_get_cookie_size (tree);
 
 extern bool hook_pass_by_reference_must_pass_in_stack
-  (CUMULATIVE_ARGS *, enum machine_mode mode, const_tree, bool);
+  (cumulative_args_t, enum machine_mode mode, const_tree, bool);
 extern bool hook_callee_copies_named
-  (CUMULATIVE_ARGS *ca, enum machine_mode, const_tree, bool);
+  (cumulative_args_t ca, enum machine_mode, const_tree, bool);
 
 extern void default_print_operand (FILE *, rtx, int);
 extern void default_print_operand_address (FILE *, rtx);
@@ -94,23 +94,23 @@ extern unsigned int default_autovectoriz
 /* These are here, and not in hooks.[ch], because not all users of
    hooks.h include tm.h, and thus we don't have CUMULATIVE_ARGS.  */
 
-extern bool hook_bool_CUMULATIVE_ARGS_false (CUMULATIVE_ARGS *);
-extern bool hook_bool_CUMULATIVE_ARGS_true (CUMULATIVE_ARGS *);
+extern bool hook_bool_CUMULATIVE_ARGS_false (cumulative_args_t);
+extern bool hook_bool_CUMULATIVE_ARGS_true (cumulative_args_t);
 
 extern bool hook_bool_CUMULATIVE_ARGS_mode_tree_bool_false
-  (CUMULATIVE_ARGS *, enum machine_mode, const_tree, bool);
+  (cumulative_args_t, enum machine_mode, const_tree, bool);
 extern bool hook_bool_CUMULATIVE_ARGS_mode_tree_bool_true
-  (CUMULATIVE_ARGS *, enum machine_mode, const_tree, bool);
+  (cumulative_args_t, enum machine_mode, const_tree, bool);
 extern int hook_int_CUMULATIVE_ARGS_mode_tree_bool_0
-  (CUMULATIVE_ARGS *, enum machine_mode, tree, bool);
+  (cumulative_args_t, enum machine_mode, tree, bool);
 extern const char *hook_invalid_arg_for_unprototyped_fn
   (const_tree, const_tree, const_tree);
 extern void default_function_arg_advance
-  (CUMULATIVE_ARGS *, enum machine_mode, const_tree, bool);
+  (cumulative_args_t, enum machine_mode, const_tree, bool);
 extern rtx default_function_arg
-  (CUMULATIVE_ARGS *, enum machine_mode, const_tree, bool);
+  (cumulative_args_t, enum machine_mode, const_tree, bool);
 extern rtx default_function_incoming_arg
-  (CUMULATIVE_ARGS *, enum machine_mode, const_tree, bool);
+  (cumulative_args_t, enum machine_mode, const_tree, bool);
 extern unsigned int default_function_arg_boundary (enum machine_mode,
 						   const_tree);
 extern bool hook_bool_const_rtx_commutative_p (const_rtx, int);
Index: gcc/c-family/c-opts.c
===================================================================
--- gcc/c-family/c-opts.c	(revision 167032)
+++ gcc/c-family/c-opts.c	(working copy)
@@ -22,6 +22,7 @@ Software Foundation; either version 3, o
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "tm.h"
 #include "tree.h"
 #include "c-common.h"
 #include "c-pragma.h"
Index: gcc/java/expr.c
===================================================================
--- gcc/java/expr.c	(revision 167032)
+++ gcc/java/expr.c	(working copy)
@@ -27,6 +27,7 @@ the Free Software Foundation; either ver
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "tm.h"
 #include "tree.h"
 #include "flags.h"
 #include "java-tree.h"
Index: gcc/target.def
===================================================================
--- gcc/target.def	(revision 167032)
+++ gcc/target.def	(working copy)
@@ -1890,7 +1890,7 @@ HOOK_VECTOR (TARGET_CALLS, calls)
 (pass_by_reference,
  "",
  bool,
- (CUMULATIVE_ARGS *cum, enum machine_mode mode, const_tree type, bool named),
+ (cumulative_args_t cum, enum machine_mode mode, const_tree type, bool named),
  hook_bool_CUMULATIVE_ARGS_mode_tree_bool_false)
 
 DEFHOOK
@@ -1903,14 +1903,14 @@ HOOK_VECTOR (TARGET_CALLS, calls)
 DEFHOOK
 (setup_incoming_varargs,
  "",
- void, (CUMULATIVE_ARGS *args_so_far, enum machine_mode mode, tree type,
+ void, (cumulative_args_t args_so_far, enum machine_mode mode, tree type,
 	int *pretend_args_size, int second_time),
  default_setup_incoming_varargs)
 
 DEFHOOK
 (strict_argument_naming,
  "",
- bool, (CUMULATIVE_ARGS *ca),
+ bool, (cumulative_args_t ca),
  hook_bool_CUMULATIVE_ARGS_false)
 
 /* Returns true if we should use
@@ -1919,7 +1919,7 @@ HOOK_VECTOR (TARGET_CALLS, calls)
 DEFHOOK
 (pretend_outgoing_varargs_named,
  "",
- bool, (CUMULATIVE_ARGS *ca),
+ bool, (cumulative_args_t ca),
  default_pretend_outgoing_varargs_named)
 
 /* Given a complex type T, return true if a parameter of type T
@@ -1946,7 +1946,7 @@ HOOK_VECTOR (TARGET_CALLS, calls)
 (callee_copies,
  "",
  bool,
- (CUMULATIVE_ARGS *cum, enum machine_mode mode, const_tree type, bool named),
+ (cumulative_args_t cum, enum machine_mode mode, const_tree type, bool named),
  hook_bool_CUMULATIVE_ARGS_mode_tree_bool_false)
 
 /* Return zero for arguments passed entirely on the stack or entirely
@@ -1955,7 +1955,7 @@ HOOK_VECTOR (TARGET_CALLS, calls)
 DEFHOOK
 (arg_partial_bytes,
  "",
- int, (CUMULATIVE_ARGS *cum, enum machine_mode mode, tree type, bool named),
+ int, (cumulative_args_t cum, enum machine_mode mode, tree type, bool named),
  hook_int_CUMULATIVE_ARGS_mode_tree_bool_0)
 
 /* Update the state in CA to advance past an argument in the
@@ -1966,7 +1966,7 @@ HOOK_VECTOR (TARGET_CALLS, calls)
 (function_arg_advance,
  "",
  void,
- (CUMULATIVE_ARGS *ca, enum machine_mode mode, const_tree type, bool named),
+ (cumulative_args_t ca, enum machine_mode mode, const_tree type, bool named),
  default_function_arg_advance)
 
 /* Return zero if the argument described by the state of CA should
@@ -1977,7 +1977,7 @@ HOOK_VECTOR (TARGET_CALLS, calls)
 DEFHOOK_UNDOC
 (function_arg,
  "",
- rtx, (CUMULATIVE_ARGS *ca, enum machine_mode mode, const_tree type,
+ rtx, (cumulative_args_t ca, enum machine_mode mode, const_tree type,
        bool named),
  default_function_arg)
 
@@ -1987,7 +1987,7 @@ HOOK_VECTOR (TARGET_CALLS, calls)
 DEFHOOK_UNDOC
 (function_incoming_arg,
  "",
- rtx, (CUMULATIVE_ARGS *ca, enum machine_mode mode, const_tree type,
+ rtx, (cumulative_args_t ca, enum machine_mode mode, const_tree type,
        bool named),
  default_function_incoming_arg)
 
Index: gcc/target.h
===================================================================
--- gcc/target.h	(revision 167032)
+++ gcc/target.h	(working copy)
@@ -49,9 +49,31 @@
 #ifndef GCC_TARGET_H
 #define GCC_TARGET_H
 
-#include "tm.h"
 #include "insn-modes.h"
 
+#include "insn-cumulative_args_t.h"
+
+#ifndef TARGET_USES_CUMULATIVE_ARGS_T
+
+#include "tm.h"
+typedef CUMULATIVE_ARGS *cumulative_args_t;
+
+#elif defined (ENABLE_CHECKING)
+
+typedef struct { void *magic; void *p; } cumulative_args_t;
+
+#else /* !ENABLE_CHECKING */
+
+#ifdef __GNUC__
+#define ATTRIBUTE_TRANSPARENT_UNION __attribute__((transparent_union))
+#else
+#define ATTRIBUTE_TRANSPARENT_UNION
+#endif
+
+typedef union ATTRIBUTE_TRANSPARENT_UNION { void *p; } cumulative_args_t;
+
+#endif /* !ENABLE_CHECKING */
+
 /* Types used by the record_gcc_switches() target function.  */
 typedef enum
 {
@@ -176,4 +198,50 @@ struct default_options
 /* Each target can provide their own.  */
 extern struct gcc_targetcm targetcm;
 
+#ifdef GCC_TM_H
+
+#ifdef TARGET_USES_CUMULATIVE_ARGS_T
+
+#ifndef CUMULATIVE_ARGS_MAGIC
+#define CUMULATIVE_ARGS_MAGIC ((void *) &targetm.calls)
+#endif
+
+static inline CUMULATIVE_ARGS *
+get_cumulative_args (cumulative_args_t arg)
+{
+#ifdef ENABLE_CHECKING
+  gcc_assert (arg.magic == CUMULATIVE_ARGS_MAGIC);
+#endif /* ENABLE_CHECKING */
+  return (CUMULATIVE_ARGS *) arg.p;
+}
+
+static inline cumulative_args_t
+pack_cumulative_args (CUMULATIVE_ARGS *arg)
+{
+  cumulative_args_t ret;
+
+#ifdef ENABLE_CHECKING
+  ret.magic = CUMULATIVE_ARGS_MAGIC;
+#endif /* ENABLE_CHECKING */
+  ret.p = (void *) arg;
+  return ret;
+}
+
+#else /* !TARGET_USES_CUMULATIVE_ARGS_T */
+
+static inline CUMULATIVE_ARGS *
+get_cumulative_args (cumulative_args_t arg)
+{
+  return arg;
+}
+
+static inline cumulative_args_t
+pack_cumulative_args (CUMULATIVE_ARGS *arg)
+{
+  return arg;
+}
+
+#endif /* !TARGET_USES_CUMULATIVE_ARGS_T */
+#endif /* GCC_TM_H */
+
 #endif /* GCC_TARGET_H */
Index: gcc/expr.c
===================================================================
--- gcc/expr.c	(revision 167032)
+++ gcc/expr.c	(working copy)
@@ -1238,14 +1238,16 @@ block_move_libcall_safe_for_call_parm (v
     for ( ; arg != void_list_node ; arg = TREE_CHAIN (arg))
       {
 	enum machine_mode mode = TYPE_MODE (TREE_VALUE (arg));
-	rtx tmp = targetm.calls.function_arg (&args_so_far, mode,
-					      NULL_TREE, true);
+	rtx tmp
+	  = targetm.calls.function_arg (pack_cumulative_args (&args_so_far),
+					mode, NULL_TREE, true);
 	if (!tmp || !REG_P (tmp))
 	  return false;
-	if (targetm.calls.arg_partial_bytes (&args_so_far, mode, NULL, 1))
+	if (targetm.calls.arg_partial_bytes
+	     (pack_cumulative_args (&args_so_far), mode, NULL, 1))
 	  return false;
-	targetm.calls.function_arg_advance (&args_so_far, mode,
-					    NULL_TREE, true);
+	targetm.calls.function_arg_advance (pack_cumulative_args (&args_so_far),
+					    mode, NULL_TREE, true);
       }
   }
   return true;
Index: gcc/dse.c
===================================================================
--- gcc/dse.c	(revision 167032)
+++ gcc/dse.c	(working copy)
@@ -2322,7 +2322,8 @@ get_call_args (rtx call_insn, tree fn, r
     {
       enum machine_mode mode = TYPE_MODE (TREE_VALUE (arg));
       rtx reg, link, tmp;
-      reg = targetm.calls.function_arg (&args_so_far, mode, NULL_TREE, true);
+      reg = targetm.calls.function_arg (pack_cumulative_args (&args_so_far),
+					mode, NULL_TREE, true);
       if (!reg || !REG_P (reg) || GET_MODE (reg) != mode
 	  || GET_MODE_CLASS (mode) != MODE_INT)
 	return false;
@@ -2356,7 +2357,8 @@ get_call_args (rtx call_insn, tree fn, r
       if (tmp)
 	args[idx] = tmp;
 
-      targetm.calls.function_arg_advance (&args_so_far, mode, NULL_TREE, true);
+      targetm.calls.function_arg_advance (pack_cumulative_args (&args_so_far),
+					  mode, NULL_TREE, true);
     }
   if (arg != void_list_node || idx != nargs)
     return false;
Index: gcc/fortran/trans-types.c
===================================================================
--- gcc/fortran/trans-types.c	(revision 167032)
+++ gcc/fortran/trans-types.c	(working copy)
@@ -26,6 +26,7 @@ Software Foundation; either version 3, o
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "tm.h"
 #include "tree.h"
 #include "langhooks.h"	/* For iso-c-bindings.def.  */
 #include "target.h"
Index: gcc/function.c
===================================================================
--- gcc/function.c	(revision 167032)
+++ gcc/function.c	(working copy)
@@ -2106,7 +2106,8 @@ pass_by_reference (CUMULATIVE_ARGS *ca, 
 	}
     }
 
-  return targetm.calls.pass_by_reference (ca, mode, type, named_arg);
+  return targetm.calls.pass_by_reference (pack_cumulative_args (ca), mode,
+					  type, named_arg);
 }
 
 /* Return true if TYPE, which is passed by reference, should be callee
@@ -2118,7 +2119,7 @@ reference_callee_copied (CUMULATIVE_ARGS
 {
   if (type && TREE_ADDRESSABLE (type))
     return false;
-  return targetm.calls.callee_copies (ca, mode, type, named_arg);
+  return targetm.calls.callee_copies (pack_cumulative_args (ca), mode, type, named_arg);
 }
 
 /* Structures to communicate between the subroutines of assign_parms.
@@ -2291,7 +2292,8 @@ assign_parm_find_data_types (struct assi
     data->named_arg = 1;  /* No variadic parms.  */
   else if (DECL_CHAIN (parm))
     data->named_arg = 1;  /* Not the last non-variadic parm. */
-  else if (targetm.calls.strict_argument_naming (&all->args_so_far))
+  else if (targetm.calls.strict_argument_naming
+	    (pack_cumulative_args (&all->args_so_far)))
     data->named_arg = 1;  /* Only variadic ones are unnamed.  */
   else
     data->named_arg = 0;  /* Treat as variadic.  */
@@ -2356,10 +2358,9 @@ assign_parms_setup_varargs (struct assig
 {
   int varargs_pretend_bytes = 0;
 
-  targetm.calls.setup_incoming_varargs (&all->args_so_far,
-					data->promoted_mode,
-					data->passed_type,
-					&varargs_pretend_bytes, no_rtl);
+  (targetm.calls.setup_incoming_varargs
+    (pack_cumulative_args (&all->args_so_far), data->promoted_mode,
+			   data->passed_type, &varargs_pretend_bytes, no_rtl));
 
   /* If the back-end has requested extra stack space, record how much is
      needed.  Do not change pretend_args_size otherwise since it may be
@@ -2385,10 +2386,9 @@ assign_parm_find_entry_rtl (struct assig
       return;
     }
 
-  entry_parm = targetm.calls.function_incoming_arg (&all->args_so_far,
-						    data->promoted_mode,
-						    data->passed_type,
-						    data->named_arg);
+  entry_parm = (targetm.calls.function_incoming_arg
+		 (pack_cumulative_args (&all->args_so_far),
+		  data->promoted_mode, data->passed_type, data->named_arg));
 
   if (entry_parm == 0)
     data->promoted_mode = data->passed_mode;
@@ -2409,12 +2409,13 @@ assign_parm_find_entry_rtl (struct assig
 #endif
   if (!in_regs && !data->named_arg)
     {
-      if (targetm.calls.pretend_outgoing_varargs_named (&all->args_so_far))
+      if (targetm.calls.pretend_outgoing_varargs_named
+	   (pack_cumulative_args (&all->args_so_far)))
 	{
 	  rtx tem;
-	  tem = targetm.calls.function_incoming_arg (&all->args_so_far,
-						     data->promoted_mode,
-						     data->passed_type, true);
+	  tem = (targetm.calls.function_incoming_arg
+		  (pack_cumulative_args (&all->args_so_far),
+		   data->promoted_mode, data->passed_type, true));
 	  in_regs = tem != NULL;
 	}
     }
@@ -2429,10 +2430,9 @@ assign_parm_find_entry_rtl (struct assig
     {
       int partial;
 
-      partial = targetm.calls.arg_partial_bytes (&all->args_so_far,
-						 data->promoted_mode,
-						 data->passed_type,
-						 data->named_arg);
+      partial = (targetm.calls.arg_partial_bytes
+		  (pack_cumulative_args (&all->args_so_far),
+		   data->promoted_mode, data->passed_type, data->named_arg));
       data->partial = partial;
 
       /* The caller might already have allocated stack space for the
@@ -3363,8 +3363,9 @@ assign_parms (tree fndecl)
       set_decl_incoming_rtl (parm, data.entry_parm, data.passed_pointer);
 
       /* Update info on where next arg arrives in registers.  */
-      targetm.calls.function_arg_advance (&all.args_so_far, data.promoted_mode,
-					  data.passed_type, data.named_arg);
+      (targetm.calls.function_arg_advance
+	(pack_cumulative_args (&all.args_so_far), data.promoted_mode,
+			       data.passed_type, data.named_arg));
 
       assign_parm_adjust_stack_rtl (&data);
 
@@ -3553,8 +3554,9 @@ gimplify_parameters (void)
 	continue;
 
       /* Update info on where next arg arrives in registers.  */
-      targetm.calls.function_arg_advance (&all.args_so_far, data.promoted_mode,
-					  data.passed_type, data.named_arg);
+      (targetm.calls.function_arg_advance
+	(pack_cumulative_args (&all.args_so_far), data.promoted_mode,
+			       data.passed_type, data.named_arg));
 
       /* ??? Once upon a time variable_size stuffed parameter list
 	 SAVE_EXPRs (amongst others) onto a pending sizes list.  This
Index: gcc/calls.c
===================================================================
--- gcc/calls.c	(revision 167032)
+++ gcc/calls.c	(working copy)
@@ -1138,23 +1138,25 @@ initialize_argument_information (int num
       args[i].unsignedp = unsignedp;
       args[i].mode = mode;
 
-      args[i].reg = targetm.calls.function_arg (args_so_far, mode, type,
-						argpos < n_named_args);
+      args[i].reg
+	= targetm.calls.function_arg (pack_cumulative_args (args_so_far), mode,
+				      type, argpos < n_named_args);
 
       /* If this is a sibling call and the machine has register windows, the
 	 register window has to be unwinded before calling the routine, so
 	 arguments have to go into the incoming registers.  */
       if (targetm.calls.function_incoming_arg != targetm.calls.function_arg)
 	args[i].tail_call_reg
-	  = targetm.calls.function_incoming_arg (args_so_far, mode, type,
-						 argpos < n_named_args);
+	  = (targetm.calls.function_incoming_arg
+	      (pack_cumulative_args (args_so_far), mode, type,
+	       argpos < n_named_args));
       else
 	args[i].tail_call_reg = args[i].reg;
 
       if (args[i].reg)
 	args[i].partial
-	  = targetm.calls.arg_partial_bytes (args_so_far, mode, type,
-					     argpos < n_named_args);
+	  = targetm.calls.arg_partial_bytes (pack_cumulative_args (args_so_far),
+					     mode, type, argpos < n_named_args);
 
       args[i].pass_on_stack = targetm.calls.must_pass_in_stack (mode, type);
 
@@ -1204,7 +1206,8 @@ initialize_argument_information (int num
       /* Increment ARGS_SO_FAR, which has info about which arg-registers
 	 have been used, etc.  */
 
-      targetm.calls.function_arg_advance (args_so_far, TYPE_MODE (type),
+      targetm.calls.function_arg_advance (pack_cumulative_args (args_so_far),
+					  TYPE_MODE (type),
 					  type, argpos < n_named_args);
     }
 }
@@ -2247,10 +2250,12 @@ expand_call (tree exp, rtx target, int i
      registers, so we must force them into memory.  */
 
   if (type_arg_types != 0
-      && targetm.calls.strict_argument_naming (&args_so_far))
+      && (targetm.calls.strict_argument_naming
+	   (pack_cumulative_args (&args_so_far))))
     ;
   else if (type_arg_types != 0
-	   && ! targetm.calls.pretend_outgoing_varargs_named (&args_so_far))
+	   && ! (targetm.calls.pretend_outgoing_varargs_named
+		  (pack_cumulative_args (&args_so_far))))
     /* Don't include the last named arg.  */
     --n_named_args;
   else
@@ -2861,14 +2866,13 @@ expand_call (tree exp, rtx target, int i
       /* Set up next argument register.  For sibling calls on machines
 	 with register windows this should be the incoming register.  */
       if (pass == 0)
-	next_arg_reg = targetm.calls.function_incoming_arg (&args_so_far,
-							    VOIDmode,
-							    void_type_node,
-							    true);
+	next_arg_reg = (targetm.calls.function_incoming_arg
+			 (pack_cumulative_args (&args_so_far), VOIDmode,
+			  void_type_node, true));
       else
-	next_arg_reg = targetm.calls.function_arg (&args_so_far,
-						   VOIDmode, void_type_node,
-						   true);
+	next_arg_reg
+	  = targetm.calls.function_arg (pack_cumulative_args (&args_so_far),
+					VOIDmode, void_type_node, true);
 
       /* All arguments and registers used for the call must be set up by
 	 now!  */
@@ -3455,10 +3459,12 @@ emit_library_call_value_1 (int retval, r
       argvec[count].mode = Pmode;
       argvec[count].partial = 0;
 
-      argvec[count].reg = targetm.calls.function_arg (&args_so_far,
-						      Pmode, NULL_TREE, true);
-      gcc_assert (targetm.calls.arg_partial_bytes (&args_so_far, Pmode,
-						   NULL_TREE, 1) == 0);
+      argvec[count].reg
+	= targetm.calls.function_arg (pack_cumulative_args (&args_so_far),
+				      Pmode, NULL_TREE, true);
+      gcc_assert ((targetm.calls.arg_partial_bytes
+		    (pack_cumulative_args (&args_so_far), Pmode, NULL_TREE, 1))
+		  == 0);
 
       locate_and_pad_parm (Pmode, NULL_TREE,
 #ifdef STACK_PARMS_IN_REG_PARM_AREA
@@ -3472,7 +3478,8 @@ emit_library_call_value_1 (int retval, r
 	  || reg_parm_stack_space > 0)
 	args_size.constant += argvec[count].locate.size.constant;
 
-      targetm.calls.function_arg_advance (&args_so_far, Pmode, (tree) 0, true);
+      targetm.calls.function_arg_advance (pack_cumulative_args (&args_so_far),
+					  Pmode, (tree) 0, true);
 
       count++;
     }
@@ -3531,11 +3538,13 @@ emit_library_call_value_1 (int retval, r
       argvec[count].value = val;
       argvec[count].mode = mode;
 
-      argvec[count].reg = targetm.calls.function_arg (&args_so_far, mode,
-						      NULL_TREE, true);
+      argvec[count].reg
+	= targetm.calls.function_arg (pack_cumulative_args (&args_so_far),
+				      mode, NULL_TREE, true);
 
       argvec[count].partial
-	= targetm.calls.arg_partial_bytes (&args_so_far, mode, NULL_TREE, 1);
+	= targetm.calls.arg_partial_bytes (pack_cumulative_args (&args_so_far),
+					   mode, NULL_TREE, 1);
 
       locate_and_pad_parm (mode, NULL_TREE,
 #ifdef STACK_PARMS_IN_REG_PARM_AREA
@@ -3552,7 +3561,8 @@ emit_library_call_value_1 (int retval, r
 	  || reg_parm_stack_space > 0)
 	args_size.constant += argvec[count].locate.size.constant;
 
-      targetm.calls.function_arg_advance (&args_so_far, mode, (tree) 0, true);
+      targetm.calls.function_arg_advance (pack_cumulative_args (&args_so_far),
+					  mode, (tree) 0, true);
     }
 
   /* If this machine requires an external definition for library
@@ -3868,7 +3878,7 @@ emit_library_call_value_1 (int retval, r
 	       build_function_type (tfom, NULL_TREE),
 	       original_args_size.constant, args_size.constant,
 	       struct_value_size,
-	       targetm.calls.function_arg (&args_so_far,
+	       targetm.calls.function_arg (pack_cumulative_args (&args_so_far),
 					   VOIDmode, void_type_node, true),
 	       valreg,
 	       old_inhibit_defer_pop + 1, call_fusage, flags, & args_so_far);
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	(revision 167032)
+++ gcc/Makefile.in	(working copy)
@@ -874,7 +874,7 @@ REVISION_s  := "\"$(if $(DEVPHASE_c), $(
 VEC_H = vec.h statistics.h
 EXCEPT_H = except.h $(HASHTAB_H) vecprim.h vecir.h
 TOPLEV_H = toplev.h $(INPUT_H) bversion.h $(DIAGNOSTIC_CORE_H)
-TARGET_H = $(TM_H) target.h target.def insn-modes.h
+TARGET_H = insn-cumulative_args_t.h target.h target.def insn-modes.h
 MACHMODE_H = machmode.h mode-classes.def insn-modes.h
 HOOKS_H = hooks.h $(MACHMODE_H)
 HOSTHOOKS_DEF_H = hosthooks-def.h $(HOOKS_H)
@@ -3708,6 +3708,13 @@ s-constrs-h: $(MD_DEPS) build/genpreds$(
 	$(SHELL) $(srcdir)/../move-if-change tmp-constrs.h tm-constrs.h
 	$(STAMP) s-constrs-h
 
+insn-cumulative_args_t.h : s-cumulative_args_t; @true
+s-cumulative_args_t: $(out_file:.o=.c)
+	if test -n "`grep cumulative_args_t $^`"; then \
+	  echo '#define TARGET_USES_CUMULATIVE_ARGS_T 1'; \
+	fi > tmp-cumulative_args_t.h
+	$(SHELL) $(srcdir)/../move-if-change tmp-cumulative_args_t.h insn-cumulative_args_t.h
+
 target-hooks-def.h: s-target-hooks-def-h; @true
 # make sure that when we build info files, the used tm.texi is up to date.
 $(srcdir)/doc/tm.texi: s-tm-texi; @true

[-- Attachment #3: pr46500-struct-tgt-20101122.bz2 --]
[-- Type: application/x-bzip2, Size: 16862 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Ping: Updated: RFA: Fix middle-end/46500 (void * encapsulated)
  2010-11-19  5:24 ` Updated: RFA: Fix middle-end/46500 (void * encapsulated) Joern Rennecke
  2010-11-19 17:04   ` Paul Koning
  2010-11-19 18:33   ` P.S.: " Joern Rennecke
@ 2010-11-26  0:44   ` Joern Rennecke
  2011-05-07 15:53   ` Updated^2: " Joern Rennecke
  3 siblings, 0 replies; 27+ messages in thread
From: Joern Rennecke @ 2010-11-26  0:44 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: gcc-patches

This patch hasn't been reviewed for a week:
http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01994.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2010-11-19  5:24 ` Updated: RFA: Fix middle-end/46500 (void * encapsulated) Joern Rennecke
                     ` (2 preceding siblings ...)
  2010-11-26  0:44   ` Ping: " Joern Rennecke
@ 2011-05-07 15:53   ` Joern Rennecke
  2011-05-07 16:01     ` Joseph S. Myers
  3 siblings, 1 reply; 27+ messages in thread
From: Joern Rennecke @ 2011-05-07 15:53 UTC (permalink / raw)
  To: gcc-patches

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

This is an update to this patch:
http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01994.html

Tested with the contrib/config-list.mk
in svn mainline revision 173498 on gcc20.

No new failures; there are pre-existing failures in r173498 for the
following targets:

alpha-dec-osf5.1                  lm32-uclinux
am33_2.0-linux                    microblaze-elf
arm-freebsd6                      microblaze-linux
arm-wince-pe                      mips-openbsd
avr-elf                           mn10300-elf
avr-rtems                         powerpc-wrs-vxworksae
i686-interix3 --enable-obsolete   rs6000-ibm-aix5.2.0
i686-openbsd3.0                   rs6000-ibm-aix5.3.0
i686-pc-msdosdjgpp                rs6000-ibm-aix6.0
i686-wrs-vxworksae                score-elf --enable-obsolete
iq2000-elf                        vax-openbsd
lm32-elf                          x86_64-knetbsd-gnu
lm32-rtems

For details of those failures, see PR target/47093.

[-- Attachment #2: pr46500-patch-20110507.gz --]
[-- Type: application/x-gzip, Size: 27789 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-05-07 15:53   ` Updated^2: " Joern Rennecke
@ 2011-05-07 16:01     ` Joseph S. Myers
  2011-05-07 16:35       ` Joseph S. Myers
  0 siblings, 1 reply; 27+ messages in thread
From: Joseph S. Myers @ 2011-05-07 16:01 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: gcc-patches

The c-opts.c change to include tm.h seems independent of the rest of the 
patch, and is incorrect since the file already has an explicit tm.h 
include with a comment saying why it's included.  I think it would be a 
good idea for the java/expr.c include (which I can't approve, Java patches 
should best be CC:ed to java-patches) to have a comment saying what target 
macros are used (BITS_PER_UNIT INT_TYPE_SIZE MODIFY_JNI_METHOD_CALL 
PARM_BOUNDARY TARGET_VTABLE_USES_DESCRIPTORS) - in general that applies to 
any tm.h includes in files that "shouldn't" include tm.h.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-05-07 16:01     ` Joseph S. Myers
@ 2011-05-07 16:35       ` Joseph S. Myers
  2011-05-07 19:42         ` Joern Rennecke
  0 siblings, 1 reply; 27+ messages in thread
From: Joseph S. Myers @ 2011-05-07 16:35 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: gcc-patches

Similar comments also apply to the fortran/trans-types.c change: send to 
the fortran list as well as gcc-patches, include a comment listing the 
target macros involved (BOOL_TYPE_SIZE CHAR_TYPE_SIZE DOUBLE_TYPE_SIZE 
FLOAT_TYPE_SIZE INT_TYPE_SIZE LIBGCC2_HAS_TF_MODE LONG_DOUBLE_TYPE_SIZE 
LONG_LONG_TYPE_SIZE LONG_TYPE_SIZE POINTER_SIZE SHORT_TYPE_SIZE 
SIZE_TYPE).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-05-07 16:35       ` Joseph S. Myers
@ 2011-05-07 19:42         ` Joern Rennecke
  2011-05-14 16:41           ` Ping: " Joern Rennecke
  0 siblings, 1 reply; 27+ messages in thread
From: Joern Rennecke @ 2011-05-07 19:42 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: gcc-patches, java-patches, fortran

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

Quoting "Joseph S. Myers" <joseph@codesourcery.com>:

> The c-opts.c change to include tm.h seems independent of the rest of the
> patch, and is incorrect since the file already has an explicit tm.h
> include with a comment saying why it's included.

Yes, I've left this bit out now.  It used to be necessary, but it has been
obsoleted by your c-opts.c patch last month.

> I think it would be a
> good idea for the java/expr.c include (which I can't approve, Java patches
> should best be CC:ed to java-patches) to have a comment saying what target
> macros are used

Added.

> Similar comments also apply to the fortran/trans-types.c change: send to
> the fortran list as well as gcc-patches, include a comment listing the
> target macros involved (BOOL_TYPE_SIZE CHAR_TYPE_SIZE DOUBLE_TYPE_SIZE
> FLOAT_TYPE_SIZE INT_TYPE_SIZE LIBGCC2_HAS_TF_MODE LONG_DOUBLE_TYPE_SIZE
> LONG_LONG_TYPE_SIZE LONG_TYPE_SIZE POINTER_SIZE SHORT_TYPE_SIZE
> SIZE_TYPE).

Instead of including SIZE_TYPE in this list, I've removed the #if 0 code
that did (not) use it.
I've also added 2011 to the Copyright years in touched files that  
didn't already
have it.

[-- Attachment #2: pr46500-patch-20110507-2.gz --]
[-- Type: application/x-gzip, Size: 29333 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Ping: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-05-07 19:42         ` Joern Rennecke
@ 2011-05-14 16:41           ` Joern Rennecke
  2011-05-14 16:44             ` Tobias Burnus
  2011-05-22  8:12             ` Ping^2: " Joern Rennecke
  0 siblings, 2 replies; 27+ messages in thread
From: Joern Rennecke @ 2011-05-14 16:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: Joseph S. Myers, java-patches, fortran

This patch hasn't been reviewed for a week:
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ping: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-05-14 16:41           ` Ping: " Joern Rennecke
@ 2011-05-14 16:44             ` Tobias Burnus
  2011-05-22  8:12             ` Ping^2: " Joern Rennecke
  1 sibling, 0 replies; 27+ messages in thread
From: Tobias Burnus @ 2011-05-14 16:44 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: gcc-patches, Joseph S. Myers, java-patches, fortran

Joern Rennecke wrote:
> This patch hasn't been reviewed for a week:
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html
>

The Fortran bits are OK.

Tobias

PS: For those, who have not looked at the patch, the Fortran-relevant 
part is '#include "tm.h"' and the removal of "#if 0" code.


2010-05-07  Joern Rennecke <joern.rennecke@embecosm.com>

         PR middle-end/46500
gcc:
         * target.h: Don't include "tm.h" .
[...]
gcc/fortran:
         * trans-types.c: Include "tm.h" .
         [0] (c_size_t_size): Remove.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Ping^2: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-05-14 16:41           ` Ping: " Joern Rennecke
  2011-05-14 16:44             ` Tobias Burnus
@ 2011-05-22  8:12             ` Joern Rennecke
  2011-05-30  8:28               ` Ping^3: " Joern Rennecke
  1 sibling, 1 reply; 27+ messages in thread
From: Joern Rennecke @ 2011-05-22  8:12 UTC (permalink / raw)
  To: gcc-patches; +Cc: java-patches

Except or the fortran bits (committed), this patch hasn't been reviewed for
two weeks:
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Ping^3: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-05-22  8:12             ` Ping^2: " Joern Rennecke
@ 2011-05-30  8:28               ` Joern Rennecke
  2011-05-30 10:16                 ` Andrew Haley
  2011-06-04 13:22                 ` Ping^4: " Joern Rennecke
  0 siblings, 2 replies; 27+ messages in thread
From: Joern Rennecke @ 2011-05-30  8:28 UTC (permalink / raw)
  To: gcc-patches; +Cc: java-patches

Except or the fortran bits (committed), this patch hasn't been reviewed for
three weeks:
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ping^3: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-05-30  8:28               ` Ping^3: " Joern Rennecke
@ 2011-05-30 10:16                 ` Andrew Haley
  2011-06-04 13:22                 ` Ping^4: " Joern Rennecke
  1 sibling, 0 replies; 27+ messages in thread
From: Andrew Haley @ 2011-05-30 10:16 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: gcc-patches, java-patches

On 30/05/11 04:26, Joern Rennecke wrote:
> Except or the fortran bits (committed), this patch hasn't been reviewed for
> three weeks:
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html

This is OK.  I said so already.

Andrew.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Ping^4: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-05-30  8:28               ` Ping^3: " Joern Rennecke
  2011-05-30 10:16                 ` Andrew Haley
@ 2011-06-04 13:22                 ` Joern Rennecke
  2011-06-14 10:03                   ` Ping^5: " Joern Rennecke
  1 sibling, 1 reply; 27+ messages in thread
From: Joern Rennecke @ 2011-06-04 13:22 UTC (permalink / raw)
  To: gcc-patches

Quoting Joern Rennecke <amylaar@spamcop.net>:

Except or the fortran/java bits (committed), this patch hasn't been  
reviewed for
four weeks:
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Ping^5: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-04 13:22                 ` Ping^4: " Joern Rennecke
@ 2011-06-14 10:03                   ` Joern Rennecke
  2011-06-14 10:40                     ` Richard Guenther
  0 siblings, 1 reply; 27+ messages in thread
From: Joern Rennecke @ 2011-06-14 10:03 UTC (permalink / raw)
  To: gcc-patches

Except or the fortran/java bits (committed), this patch hasn't been
reviewed for five weeks:
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ping^5: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-14 10:03                   ` Ping^5: " Joern Rennecke
@ 2011-06-14 10:40                     ` Richard Guenther
  2011-06-14 11:28                       ` Joern Rennecke
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Guenther @ 2011-06-14 10:40 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: gcc-patches

On Tue, Jun 14, 2011 at 11:40 AM, Joern Rennecke <amylaar@spamcop.net> wrote:
> Except or the fortran/java bits (committed), this patch hasn't been
> reviewed for five weeks:
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html

A patch doing s/CUMULATIVE_ARGS*/cumulative_args_t/ only
is ok.  Posting compressed attached patches makes it too easy
to not review things btw ...

After that patch the "meat" of the patch should be much much smaller
and easier to review (if there is anything left besides the renaming?).

Thanks,
Richard.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ping^5: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-14 10:40                     ` Richard Guenther
@ 2011-06-14 11:28                       ` Joern Rennecke
  2011-06-14 11:34                         ` Richard Guenther
  0 siblings, 1 reply; 27+ messages in thread
From: Joern Rennecke @ 2011-06-14 11:28 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

Quoting Richard Guenther <richard.guenther@gmail.com>:

> On Tue, Jun 14, 2011 at 11:40 AM, Joern Rennecke <amylaar@spamcop.net> wrote:
>> Except or the fortran/java bits (committed), this patch hasn't been
>> reviewed for five weeks:
>> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html
>
> A patch doing s/CUMULATIVE_ARGS*/cumulative_args_t/ only
> is ok.

It's not quite that simple.  The patch makes a distinction between pointers
to the target specific types CUMULATIVE_ARGS, and the target-independent
cumulative_args_t.

Is it still OK if I selectively do the replacement where the
target-independent type is meant, and add a provisional
typedef CUMULATIVE_ARGS *cumulative_args_t to tie it together?

> Posting compressed attached patches makes it too easy
> to not review things btw ...

The mailing list size limits did't allow this patch to be posted
without compression.

> After that patch the "meat" of the patch should be much much smaller
> and easier to review (if there is anything left besides the renaming?).

It should be somewhat smaller, but there are lots of places where we have
to convert between cumulative_args_t and CUMULATIVE_ARGS *.
Were a target-independent interface is required, we need cumulative_args_t .
Where a target accesses struct components, it needs CUMULATIVE_ARGS *.
There are some places that just pass CUMULATIVE_ARGS * around, both in  
rtl-centric middle-end/ rtl-optimizer code and in target code, which
could be electively converted.  In general, I haven't done such optional
conversions.  They could be added according to taste once the interface
has been straightened out.  There is also a judgement call in each place
how closely the code is tied to the cumulative_args_t side or the
CUMULATIVE_ARGS * side.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ping^5: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-14 11:28                       ` Joern Rennecke
@ 2011-06-14 11:34                         ` Richard Guenther
  2011-06-14 12:53                           ` Bernd Schmidt
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Guenther @ 2011-06-14 11:34 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: GCC Patches, Ian Lance Taylor

On Tue, Jun 14, 2011 at 1:16 PM, Joern Rennecke <amylaar@spamcop.net> wrote:
> Quoting Richard Guenther <richard.guenther@gmail.com>:
>
>> On Tue, Jun 14, 2011 at 11:40 AM, Joern Rennecke <amylaar@spamcop.net>
>> wrote:
>>>
>>> Except or the fortran/java bits (committed), this patch hasn't been
>>> reviewed for five weeks:
>>> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html
>>
>> A patch doing s/CUMULATIVE_ARGS*/cumulative_args_t/ only
>> is ok.
>
> It's not quite that simple.  The patch makes a distinction between pointers
> to the target specific types CUMULATIVE_ARGS, and the target-independent
> cumulative_args_t.
>
> Is it still OK if I selectively do the replacement where the
> target-independent type is meant, and add a provisional
> typedef CUMULATIVE_ARGS *cumulative_args_t to tie it together?
>
>> Posting compressed attached patches makes it too easy
>> to not review things btw ...
>
> The mailing list size limits did't allow this patch to be posted
> without compression.
>
>> After that patch the "meat" of the patch should be much much smaller
>> and easier to review (if there is anything left besides the renaming?).
>
> It should be somewhat smaller, but there are lots of places where we have
> to convert between cumulative_args_t and CUMULATIVE_ARGS *.
> Were a target-independent interface is required, we need cumulative_args_t .
> Where a target accesses struct components, it needs CUMULATIVE_ARGS *.
> There are some places that just pass CUMULATIVE_ARGS * around, both in
> rtl-centric middle-end/ rtl-optimizer code and in target code, which
> could be electively converted.  In general, I haven't done such optional
> conversions.  They could be added according to taste once the interface
> has been straightened out.  There is also a judgement call in each place
> how closely the code is tied to the cumulative_args_t side or the
> CUMULATIVE_ARGS * side.

Hmm, I see.  Maybe a GWP wants to ack your patch in whole then.

Ian?

Thanks,
Richard.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ping^5: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-14 11:34                         ` Richard Guenther
@ 2011-06-14 12:53                           ` Bernd Schmidt
  2011-06-14 13:05                             ` Joern Rennecke
  2011-06-16  6:22                             ` Updated^3: " Joern Rennecke
  0 siblings, 2 replies; 27+ messages in thread
From: Bernd Schmidt @ 2011-06-14 12:53 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Joern Rennecke, GCC Patches, Ian Lance Taylor

On 06/14/2011 01:29 PM, Richard Guenther wrote:
> On Tue, Jun 14, 2011 at 1:16 PM, Joern Rennecke <amylaar@spamcop.net> wrote:
>> Quoting Richard Guenther <richard.guenther@gmail.com>:
>>
>>> On Tue, Jun 14, 2011 at 11:40 AM, Joern Rennecke <amylaar@spamcop.net>
>>> wrote:
>>>>
>>>> Except or the fortran/java bits (committed), this patch hasn't been
>>>> reviewed for five weeks:
>>>> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00582.html
>>>
>>> A patch doing s/CUMULATIVE_ARGS*/cumulative_args_t/ only
>>> is ok.
>>
>> It's not quite that simple.  The patch makes a distinction between pointers
>> to the target specific types CUMULATIVE_ARGS, and the target-independent
>> cumulative_args_t.
>>
>> Is it still OK if I selectively do the replacement where the
>> target-independent type is meant, and add a provisional
>> typedef CUMULATIVE_ARGS *cumulative_args_t to tie it together?
>>
>>> Posting compressed attached patches makes it too easy
>>> to not review things btw ...
>>
>> The mailing list size limits did't allow this patch to be posted
>> without compression.
>>
>>> After that patch the "meat" of the patch should be much much smaller
>>> and easier to review (if there is anything left besides the renaming?).
>>
>> It should be somewhat smaller, but there are lots of places where we have
>> to convert between cumulative_args_t and CUMULATIVE_ARGS *.
>> Were a target-independent interface is required, we need cumulative_args_t .
>> Where a target accesses struct components, it needs CUMULATIVE_ARGS *.
>> There are some places that just pass CUMULATIVE_ARGS * around, both in
>> rtl-centric middle-end/ rtl-optimizer code and in target code, which
>> could be electively converted.  In general, I haven't done such optional
>> conversions.  They could be added according to taste once the interface
>> has been straightened out.  There is also a judgement call in each place
>> how closely the code is tied to the cumulative_args_t side or the
>> CUMULATIVE_ARGS * side.
> 
> Hmm, I see.  Maybe a GWP wants to ack your patch in whole then.

I'm not getting the point of the use of attribute((transparent_union)).
That should be removed to eliminate potential differences when compiling
other compilers, and to eliminate a potential source of bugs when
passing cumulative_args_t arguments.

Some of the formatting changes to avoid long lines are unfortunate (and
it's not done consistently); I think I'd prefer to add temporary
variables to hold the return value of pack_cumulative_args and
get_cumulative_args.

-  targetm.calls.setup_incoming_varargs (&all->args_so_far,
-                                       data->promoted_mode,
-                                       data->passed_type,
-                                       &varargs_pretend_bytes, no_rtl);
+  (targetm.calls.setup_incoming_varargs
+    (pack_cumulative_args (&all->args_so_far), data->promoted_mode,
+                          data->passed_type, &varargs_pretend_bytes,
no_rtl));

No need for parentheses around the expression. Occurs in three places.
See previous comment about using temporary variables to avoid ugly
formatting.

I think it would be best just to minimize changes in backends as much as
possible by using the following pattern everywhere:

 static void
-ix86_function_arg_advance (CUMULATIVE_ARGS *cum, enum machine_mode mode,
+ix86_function_arg_advance (cumulative_args_t cum_v, enum machine_mode mode,
                           const_tree type, bool named)
 {
+  CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);

I.e., avoid changes such as the one in mn10300_function_arg_advance.

Also,

-       if (iq2000_function_arg (&temp, mode, type, named) != 0)
+       if (iq2000_function_arg (pack_cumulative_args (&temp), mode,
type, named)          != 0)

Extra tab character before !=.

-  if (targetm.calls.strict_argument_naming (arg_regs_used_so_far))
+  /* ??? the code inside is a pointer increment.  */
+  if (targetm.calls.strict_argument_naming (arg_regs_used_so_far_v))

What does this comment mean?

Finally, I could do without the comments squished to the right-hand side
like this:

+#include "tm.h"                /* For INTMAX_TYPE, INT8_TYPE,
INT16_TYPE, INT32_TYPE,
+                          INT64_TYPE, INT_LEAST8_TYPE, INT_LEAST16_TYPE,
+                          INT_LEAST32_TYPE, INT_LEAST64_TYPE,
INT_FAST8_TYPE,
+                          INT_FAST16_TYPE, INT_FAST32_TYPE,
INT_FAST64_TYPE,
+                          BOOL_TYPE_SIZE, BITS_PER_UNIT, POINTER_SIZE,
+                          INT_TYPE_SIZE, CHAR_TYPE_SIZE, SHORT_TYPE_SIZE,
+                          LONG_TYPE_SIZE, LONG_LONG_TYPE_SIZE,
+                          FLOAT_TYPE_SIZE, DOUBLE_TYPE_SIZE,
+                          LONG_DOUBLE_TYPE_SIZE and
LIBGCC2_HAS_TF_MODE.  */

(I could do without these comments entirely but I see from the archives
that Joseph requested it.)

With these changes I think it'll be OK, but I'd like to see a new patch
version first.


Bernd

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ping^5: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-14 12:53                           ` Bernd Schmidt
@ 2011-06-14 13:05                             ` Joern Rennecke
  2011-06-14 13:43                               ` Bernd Schmidt
  2011-06-16  6:22                             ` Updated^3: " Joern Rennecke
  1 sibling, 1 reply; 27+ messages in thread
From: Joern Rennecke @ 2011-06-14 13:05 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: Richard Guenther, GCC Patches, Ian Lance Taylor

Quoting Bernd Schmidt <bernds@codesourcery.com>:

> I'm not getting the point of the use of attribute((transparent_union)).

Without that attribute, lots of ABIs add a lot of overhead for function
argument and return value passing.  E.g. instead of putting the argument
in a register, put it on the stack, and place a pointer to that stack
location in a register; instead of returning the result in a register,
have the caller pass a pointer to a location in the stack, then have the
callee write the result via that pointer into the stack, and return the
pointer.
With the transparent union attribute, you get the same straigtforward
compiled code as before with CUMULATIVE_ARGS used throughout.

> That should be removed to eliminate potential differences when compiling
> other compilers,

I'm not sure what you mean here.  Do you want to have compilation units
of ENABLE_CHECKING compilers be compatible with !ENABLE_CHECKING ones?
In that case, we'd have to revamp vec.h, among others.

> and to eliminate a potential source of bugs when
> passing cumulative_args_t arguments.

Is that about not trusting the bootstrap gcc to implement that attribute
correctly?
Or do you want the integrity check from the ENABLE_CHECKING case to be
always present?


[fr30.c:fr30_setup_incoming_varargs]
> -  if (targetm.calls.strict_argument_naming (arg_regs_used_so_far))
> +  /* ??? the code inside is a pointer increment.  */
> +  if (targetm.calls.strict_argument_naming (arg_regs_used_so_far_v))
>
> What does this comment mean?

It means that the code inside the if clause. i.e.:

     arg_regs_used_so_far += fr30_num_arg_regs (mode, type);

is nonsense.  arg_regs_used_so_far is a pointer to int.  The pointed-to int
is supposed to record the number of words used for argument passing.
The statement increments the pointer.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ping^5: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-14 13:05                             ` Joern Rennecke
@ 2011-06-14 13:43                               ` Bernd Schmidt
  2011-06-14 14:27                                 ` Joern Rennecke
  0 siblings, 1 reply; 27+ messages in thread
From: Bernd Schmidt @ 2011-06-14 13:43 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: Richard Guenther, GCC Patches, Ian Lance Taylor

On 06/14/2011 02:53 PM, Joern Rennecke wrote:
> Quoting Bernd Schmidt <bernds@codesourcery.com>:
> 
>> I'm not getting the point of the use of attribute((transparent_union)).
> 
> Without that attribute, lots of ABIs add a lot of overhead for function
> argument and return value passing.

* These functions are not hotspots.
* Most sane ABIs pass single-word structs in registers
* For the most part, gcc runs on i686 and there it doesn't make a
  difference. If ARM takes over the world, it still does not make a
  difference.

This is an unnecessary and premature microoptimization. Please remove it.

>> and to eliminate a potential source of bugs when
>> passing cumulative_args_t arguments.
> 
> Is that about not trusting the bootstrap gcc to implement that attribute
> correctly?
> Or do you want the integrity check from the ENABLE_CHECKING case to be
> always present?

According to the transparent union documentation, the compiler will
accept void * rather than cumulative_args_t if the latter is declared as
a transparent union.

If the point of your ENABLE_CHECKING machinery (which I also don't
really understand) is to avoid exactly that kind of bug, then the
ENABLE_CHECKING code should go away along with the use of transparent_union.

> [fr30.c:fr30_setup_incoming_varargs]
>> -  if (targetm.calls.strict_argument_naming (arg_regs_used_so_far))
>> +  /* ??? the code inside is a pointer increment.  */
>> +  if (targetm.calls.strict_argument_naming (arg_regs_used_so_far_v))
>>
>> What does this comment mean?
> 
> It means that the code inside the if clause. i.e.:
> 
>     arg_regs_used_so_far += fr30_num_arg_regs (mode, type);
> 
> is nonsense.  arg_regs_used_so_far is a pointer to int.  The pointed-to int
> is supposed to record the number of words used for argument passing.
> The statement increments the pointer.

Ok, so move the comment before that statement then and adjust it to say
this.


Bernd

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Ping^5: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-14 13:43                               ` Bernd Schmidt
@ 2011-06-14 14:27                                 ` Joern Rennecke
  0 siblings, 0 replies; 27+ messages in thread
From: Joern Rennecke @ 2011-06-14 14:27 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: Richard Guenther, GCC Patches, Ian Lance Taylor

Quoting Bernd Schmidt <bernds@codesourcery.com>:

> If the point of your ENABLE_CHECKING machinery (which I also don't
> really understand) is to avoid exactly that kind of bug, then the
> ENABLE_CHECKING code should go away along with the use of transparent_union.

No, it does a lot more than that.  It gives a sanity check that the contents
of a cumulative_args_t have actually been packed with pack_cumulative_args,
and in case more than one target is supported, the check will also
verify that the target on which get_cumulative_args is used is the same as
the one on which pack_cumulative_args was called before to pack the  
cumlative_args_t.
Bugs where a target hook for the wrong target is called, or a data structure
from the wrong target is used, are hard to track down when all the optimizers
are operating in garbage-in-garbage-out mode.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Updated^3: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-14 12:53                           ` Bernd Schmidt
  2011-06-14 13:05                             ` Joern Rennecke
@ 2011-06-16  6:22                             ` Joern Rennecke
  2011-06-16 10:29                               ` Bernd Schmidt
  1 sibling, 1 reply; 27+ messages in thread
From: Joern Rennecke @ 2011-06-16  6:22 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: Richard Guenther, GCC Patches, Ian Lance Taylor

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

Quoting Bernd Schmidt <bernds@codesourcery.com>:

> Some of the formatting changes to avoid long lines are unfortunate (and
> it's not done consistently); I think I'd prefer to add temporary
> variables to hold the return value of pack_cumulative_args and
> get_cumulative_args.

With the cumulative_args_t values available, it was more natural to also
convert emit_call_1 and initialize_argument_information, so I did that.

However, pass_by_reference and reference_callee_copied are called in a number
of places from target ports, so I've left this for a later round of
patches and/or discussion.

Likewise, it becomes a bit clearer what we need from a hook  
replaceemnt of INIT_CUMULATIVE_ARGS, which we can discuss later.

> I think it would be best just to minimize changes in backends as much as
> possible by using the following pattern everywhere:
>
>  static void
> -ix86_function_arg_advance (CUMULATIVE_ARGS *cum, enum machine_mode mode,
> +ix86_function_arg_advance (cumulative_args_t cum_v, enum machine_mode mode,
>                            const_tree type, bool named)
>  {
> +  CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
>
> I.e., avoid changes such as the one in mn10300_function_arg_advance.

I've employed this pattern now in all the target hook implementation heads
except where there was a only a trivial single substitution that caused
little or no reformatting.

> Also,
>
> -       if (iq2000_function_arg (&temp, mode, type, named) != 0)
> +       if (iq2000_function_arg (pack_cumulative_args (&temp), mode,
> type, named)          != 0)
>
> Extra tab character before !=.

Actually, it was a missing carriage return.

[-- Attachment #2: 46500-patch-20110615.gz --]
[-- Type: application/x-gzip, Size: 29997 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Updated^3: RFA: Fix middle-end/46500 (void * encapsulated)
  2011-06-16  6:22                             ` Updated^3: " Joern Rennecke
@ 2011-06-16 10:29                               ` Bernd Schmidt
  0 siblings, 0 replies; 27+ messages in thread
From: Bernd Schmidt @ 2011-06-16 10:29 UTC (permalink / raw)
  To: Joern Rennecke; +Cc: Richard Guenther, GCC Patches, Ian Lance Taylor

Ok, much better.

+  cumulative_args_t dummy;
+
+  return m32r_pass_by_reference (dummy, TYPE_MODE (type), type, false);

Please initialize the variable.

Otherwise I think it's OK. Thanks!


Bernd

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2011-06-16 10:00 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-17  7:25 RFA: Fix middle-end/46500 Joern Rennecke
2010-11-17 15:50 ` Paul Koning
2010-11-19  5:24 ` Updated: RFA: Fix middle-end/46500 (void * encapsulated) Joern Rennecke
2010-11-19 17:04   ` Paul Koning
2010-11-19 18:33   ` P.S.: " Joern Rennecke
2010-11-22 21:49     ` P.P.S.: " Joern Rennecke
2010-11-26  0:44   ` Ping: " Joern Rennecke
2011-05-07 15:53   ` Updated^2: " Joern Rennecke
2011-05-07 16:01     ` Joseph S. Myers
2011-05-07 16:35       ` Joseph S. Myers
2011-05-07 19:42         ` Joern Rennecke
2011-05-14 16:41           ` Ping: " Joern Rennecke
2011-05-14 16:44             ` Tobias Burnus
2011-05-22  8:12             ` Ping^2: " Joern Rennecke
2011-05-30  8:28               ` Ping^3: " Joern Rennecke
2011-05-30 10:16                 ` Andrew Haley
2011-06-04 13:22                 ` Ping^4: " Joern Rennecke
2011-06-14 10:03                   ` Ping^5: " Joern Rennecke
2011-06-14 10:40                     ` Richard Guenther
2011-06-14 11:28                       ` Joern Rennecke
2011-06-14 11:34                         ` Richard Guenther
2011-06-14 12:53                           ` Bernd Schmidt
2011-06-14 13:05                             ` Joern Rennecke
2011-06-14 13:43                               ` Bernd Schmidt
2011-06-14 14:27                                 ` Joern Rennecke
2011-06-16  6:22                             ` Updated^3: " Joern Rennecke
2011-06-16 10:29                               ` Bernd Schmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).