public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends
@ 2021-11-13  9:42 Maxim Blinov
  2021-11-13  9:42 ` [PATCH 2/2] Implement TARGET_..._CA target hooks for AArch64 Darwin Maxim Blinov
  2021-11-15  7:11 ` [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends Richard Biener
  0 siblings, 2 replies; 8+ messages in thread
From: Maxim Blinov @ 2021-11-13  9:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: iain, maxim.blinov

The two target hooks responsible for informing GCC about stack
parameter alignment are `TARGET_FUNCTION_ARG_BOUNDARY` and
`TARGET_FUNCTION_ARG_ROUND_BOUNDARY`, which currently only consider
the tree and machine_mode of a specific given argument.

Create two new target hooks suffixed with '_CA', and pass in a third
`cumulative_args_t` parameter. This enables the backend to make
alignment decisions based on the context of the whole function rather
than individual parameters.

The orignal machine_mode/tree type macros are not removed - they are
called by the default implementations of `TARGET_...BOUNDARY_CA` and
`TARGET_...ROUND_BOUNDARY_CA`. This is done with the intetnion of
avoiding large mechanical modifications of nearly every backend in
GCC. There is also a new flag, -fstack-use-cumulative-args, which
provides a way to completely bypass the new `..._CA` macros. This
feature is intended for debugging GCC itself.

gcc/ChangeLog:

	* calls.c (initialize_argument_information): Pass `args_so_far`.
	* common.opt: New flag `-fstack-use-cumulative-args`.
	* config.gcc: No platforms currently use ..._CA-hooks: Set
	-fstack-use-cumulative-args to be off by default.
	* target.h (cumulative_args_t): Move declaration from here, to...
	* cumulative-args.h (cumulative_args_t): ...this new file. This is
	to permit backends to include the declaration of cumulative_args_t
	without dragging in circular dependencies.
	* function.c (assign_parm_find_entry_rtl): Provide
	cumulative_args_t to locate_and_pad_parm.
	(gimplify_parameters): Ditto.
	(locate_and_pad_parm): Conditionally call new hooks if we're
	invoked with -fstack-use-cumulative-args.
	* function.h: Include cumulative-args.h.
	(locate_and_pad_parm): Add cumulative_args_t parameter.
	* target.def (function_arg_boundary_ca): Add.
	(function_arg_round_boundary_ca): Ditto.
	* targhooks.c (default_function_arg_boundary_ca): Implement.
	(default_function_arg_round_boundary_ca): Ditto.
	* targhooks.h (default_function_arg_boundary_ca): Declare.
	(default_function_arg_round_boundary_ca): Ditto.
	* doc/invoke.texi (-fstack-use-cumulative-args): Document.
	* doc/tm.texi: Regenerate.
	* doc/tm.texi.in: Ditto.
---
 gcc/calls.c           |  3 +++
 gcc/common.opt        |  4 ++++
 gcc/config.gcc        |  7 +++++++
 gcc/cumulative-args.h | 20 ++++++++++++++++++++
 gcc/doc/invoke.texi   | 12 ++++++++++++
 gcc/doc/tm.texi       | 20 ++++++++++++++++++++
 gcc/doc/tm.texi.in    |  4 ++++
 gcc/function.c        | 25 +++++++++++++++++++++----
 gcc/function.h        |  2 ++
 gcc/target.def        | 24 ++++++++++++++++++++++++
 gcc/target.h          | 17 +----------------
 gcc/targhooks.c       | 16 ++++++++++++++++
 gcc/targhooks.h       |  6 ++++++
 13 files changed, 140 insertions(+), 20 deletions(-)
 create mode 100644 gcc/cumulative-args.h

diff --git a/gcc/calls.c b/gcc/calls.c
index 27b59f26ad3..cef612a6ef4 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1527,6 +1527,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
 #endif
 			     reg_parm_stack_space,
 			     args[i].pass_on_stack ? 0 : args[i].partial,
+			     args_so_far,
 			     fndecl, args_size, &args[i].locate);
 #ifdef BLOCK_REG_PADDING
       else
@@ -4205,6 +4206,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
 			   argvec[count].reg != 0,
 #endif
 			   reg_parm_stack_space, 0,
+			   args_so_far,
 			   NULL_TREE, &args_size, &argvec[count].locate);
 
       if (argvec[count].reg == 0 || argvec[count].partial != 0
@@ -4296,6 +4298,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
 			       argvec[count].reg != 0,
 #endif
 			       reg_parm_stack_space, argvec[count].partial,
+			       args_so_far,
 			       NULL_TREE, &args_size, &argvec[count].locate);
 	  args_size.constant += argvec[count].locate.size.constant;
 	  gcc_assert (!argvec[count].locate.size.var);
diff --git a/gcc/common.opt b/gcc/common.opt
index de9b848eda5..982417c1e39 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2705,6 +2705,10 @@ fstack-usage
 Common RejectNegative Var(flag_stack_usage)
 Output stack usage information on a per-function basis.
 
+fstack-use-cumulative-args
+Common RejectNegative Var(flag_stack_use_cumulative_args) Init(STACK_USE_CUMULATIVE_ARGS_INIT)
+Use cumulative args-based stack layout hooks.
+
 fstrength-reduce
 Common Ignore
 Does nothing.  Preserved for backward compatibility.
diff --git a/gcc/config.gcc b/gcc/config.gcc
index edd12655c4a..046d691af56 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1070,6 +1070,13 @@ case ${target} in
   ;;
 esac
 
+# Figure out if we need to enable -foff-stack-trampolines by default.
+case ${target} in
+*)
+  tm_defines="$tm_defines STACK_USE_CUMULATIVE_ARGS_INIT=0"
+  ;;
+esac
+
 case ${target} in
 aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-rtems*)
 	tm_file="${tm_file} dbxelf.h elfos.h newlib-stdint.h"
diff --git a/gcc/cumulative-args.h b/gcc/cumulative-args.h
new file mode 100644
index 00000000000..b60928e37f9
--- /dev/null
+++ b/gcc/cumulative-args.h
@@ -0,0 +1,20 @@
+#ifndef GCC_CUMULATIVE_ARGS_H
+#define GCC_CUMULATIVE_ARGS_H
+
+#if CHECKING_P
+
+struct cumulative_args_t { void *magic; void *p; };
+
+#else /* !CHECKING_P */
+
+/* When using a GCC build compiler, we could use
+   __attribute__((transparent_union)) to get cumulative_args_t function
+   arguments passed like scalars where the ABI would mandate a less
+   efficient way of argument passing otherwise.  However, that would come
+   at the cost of less type-safe !CHECKING_P compilation.  */
+
+union cumulative_args_t { void *p; };
+
+#endif /* !CHECKING_P */
+
+#endif /* GCC_CUMULATIVE_ARGS_H */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2aba4c70b44..7ffb2997c69 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -670,6 +670,7 @@ Objective-C and Objective-C++ Dialects}.
 -fverbose-asm  -fpack-struct[=@var{n}]  @gol
 -fleading-underscore  -ftls-model=@var{model} @gol
 -fstack-reuse=@var{reuse_level} @gol
+-fstack-use-cumulative-args @gol
 -ftrampolines  -ftrapv  -fwrapv @gol
 -fvisibility=@r{[}default@r{|}internal@r{|}hidden@r{|}protected@r{]} @gol
 -fstrict-volatile-bitfields  -fsync-libcalls}
@@ -16625,6 +16626,17 @@ the behavior of older compilers in which temporaries' stack space is
 not reused, the aggressive stack reuse can lead to runtime errors. This
 option is used to control the temporary stack reuse optimization.
 
+@item -fstack-use-cumulative-args
+@opindex fstack_use_cumulative_args
+This option instructs the compiler to use the
+@code{cumulative_args_t}-based stack layout target hooks,
+@code{TARGET_FUNCTION_ARG_BOUNDARY_CA} and
+@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA}. If a given target does
+not define these hooks, the default behaviour is to fallback to using
+the standard non-@code{_CA} variants instead. Certain targets (such as
+AArch64 Darwin) require using the more advanced @code{_CA}-based
+hooks: For these targets this option should be enabled by default.
+
 @item -ftrapv
 @opindex ftrapv
 This option generates traps for signed overflow on addition, subtraction,
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 78a1af1ad4d..261eaf46ef5 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -4322,6 +4322,16 @@ with the specified mode and type.  The default hook returns
 @code{PARM_BOUNDARY} for all arguments.
 @end deftypefn
 
+@deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_BOUNDARY_CA (machine_mode @var{mode}, const_tree @var{type}, cumulative_args_t @var{ca})
+This is the @code{cumulative_args_t}-based version of
+@code{TARGET_FUNCTION_ARG_BOUNDARY}. Define this hook if you need more
+fine-grained control over argument alignment, e.g. depending on whether
+it is a named argument or not, or any other criteria that you choose to
+place in the @var{ca} structure.
+
+The default hook will call @code{TARGET_FUNCTION_ARG_BOUNDARY}.
+@end deftypefn
+
 @deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_ROUND_BOUNDARY (machine_mode @var{mode}, const_tree @var{type})
 Normally, the size of an argument is rounded up to @code{PARM_BOUNDARY},
 which is the default value for this hook.  You can define this hook to
@@ -4329,6 +4339,16 @@ return a different value if an argument size must be rounded to a larger
 value.
 @end deftypefn
 
+@deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA (machine_mode @var{mode}, const_tree @var{type}, cumulative_args_t @var{ca})
+This is the @code{cumulative_args_t}-based version of
+@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}. Define this hook if you need more
+fine-grained control over argument size rounding, e.g. depending on whether
+it is a named argument or not, or any other criteria that you choose to
+place in the @var{ca} structure.
+
+The default hook will call @code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}.
+@end deftypefn
+
 @defmac FUNCTION_ARG_REGNO_P (@var{regno})
 A C expression that is nonzero if @var{regno} is the number of a hard
 register in which function arguments are sometimes passed.  This does
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 4401550989e..933644dfa86 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -3330,8 +3330,12 @@ required.
 
 @hook TARGET_FUNCTION_ARG_BOUNDARY
 
+@hook TARGET_FUNCTION_ARG_BOUNDARY_CA
+
 @hook TARGET_FUNCTION_ARG_ROUND_BOUNDARY
 
+@hook TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA
+
 @defmac FUNCTION_ARG_REGNO_P (@var{regno})
 A C expression that is nonzero if @var{regno} is the number of a hard
 register in which function arguments are sometimes passed.  This does
diff --git a/gcc/function.c b/gcc/function.c
index 61b3bd036b8..1ebf3e0ffe5 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -2610,7 +2610,9 @@ assign_parm_find_entry_rtl (struct assign_parm_data_all *all,
 
   locate_and_pad_parm (data->arg.mode, data->arg.type, in_regs,
 		       all->reg_parm_stack_space,
-		       entry_parm ? data->partial : 0, current_function_decl,
+		       entry_parm ? data->partial : 0,
+		       all->args_so_far,
+		       current_function_decl,
 		       &all->stack_args_size, &data->locate);
 
   /* Update parm_stack_boundary if this parameter is passed in the
@@ -4027,6 +4029,7 @@ gimplify_parameters (gimple_seq *cleanup)
 void
 locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs,
 		     int reg_parm_stack_space, int partial,
+		     cumulative_args_t ca,
 		     tree fndecl ATTRIBUTE_UNUSED,
 		     struct args_size *initial_offset_ptr,
 		     struct locate_and_pad_arg_data *locate)
@@ -4064,9 +4067,23 @@ locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs,
 	      ? arg_size_in_bytes (type)
 	      : size_int (GET_MODE_SIZE (passed_mode)));
   where_pad = targetm.calls.function_arg_padding (passed_mode, type);
-  boundary = targetm.calls.function_arg_boundary (passed_mode, type);
-  round_boundary = targetm.calls.function_arg_round_boundary (passed_mode,
-							      type);
+
+  if (flag_stack_use_cumulative_args)
+    {
+      boundary = targetm.calls.function_arg_boundary_ca (passed_mode,
+							 type,
+							 ca);
+      round_boundary = targetm.calls.function_arg_round_boundary_ca
+	(passed_mode, type, ca);
+    }
+  else
+    {
+      boundary = targetm.calls.function_arg_boundary (passed_mode,
+						      type);
+      round_boundary = targetm.calls.function_arg_round_boundary
+	(passed_mode, type);
+    }
+
   locate->where_pad = where_pad;
 
   /* Alignment can't exceed MAX_SUPPORTED_STACK_ALIGNMENT.  */
diff --git a/gcc/function.h b/gcc/function.h
index 899430833ce..26f342caba3 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -20,6 +20,7 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_FUNCTION_H
 #define GCC_FUNCTION_H
 
+#include "cumulative-args.h"
 
 /* Stack of pending (incomplete) sequences saved by `start_sequence'.
    Each element describes one pending sequence.
@@ -661,6 +662,7 @@ extern int aggregate_value_p (const_tree, const_tree);
 extern bool use_register_for_decl (const_tree);
 extern gimple_seq gimplify_parameters (gimple_seq *);
 extern void locate_and_pad_parm (machine_mode, tree, int, int, int,
+				 cumulative_args_t,
 				 tree, struct args_size *,
 				 struct locate_and_pad_arg_data *);
 extern void generate_setjmp_warnings (void);
diff --git a/gcc/target.def b/gcc/target.def
index 51ea167172b..bb56afab539 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -4959,6 +4959,18 @@ with the specified mode and type.  The default hook returns\n\
  unsigned int, (machine_mode mode, const_tree type),
  default_function_arg_boundary)
 
+DEFHOOK
+(function_arg_boundary_ca,
+ "This is the @code{cumulative_args_t}-based version of\n\
+@code{TARGET_FUNCTION_ARG_BOUNDARY}. Define this hook if you need more\n\
+fine-grained control over argument alignment, e.g. depending on whether\n\
+it is a named argument or not, or any other criteria that you choose to\n\
+place in the @var{ca} structure.\n\
+\n\
+The default hook will call @code{TARGET_FUNCTION_ARG_BOUNDARY}.",
+ unsigned int, (machine_mode mode, const_tree type, cumulative_args_t ca),
+ default_function_arg_boundary_ca)
+
 DEFHOOK
 (function_arg_round_boundary,
  "Normally, the size of an argument is rounded up to @code{PARM_BOUNDARY},\n\
@@ -4968,6 +4980,18 @@ value.",
  unsigned int, (machine_mode mode, const_tree type),
  default_function_arg_round_boundary)
 
+DEFHOOK
+(function_arg_round_boundary_ca,
+ "This is the @code{cumulative_args_t}-based version of\n\
+@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}. Define this hook if you need more\n\
+fine-grained control over argument size rounding, e.g. depending on whether\n\
+it is a named argument or not, or any other criteria that you choose to\n\
+place in the @var{ca} structure.\n\
+\n\
+The default hook will call @code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}.",
+ unsigned int, (machine_mode mode, const_tree type, cumulative_args_t ca),
+ default_function_arg_round_boundary_ca)
+
 /* Return the diagnostic message string if function without a prototype
    is not allowed for this 'val' argument; NULL otherwise. */
 DEFHOOK
diff --git a/gcc/target.h b/gcc/target.h
index d8f45fb99c3..5b28ece7e1c 100644
--- a/gcc/target.h
+++ b/gcc/target.h
@@ -51,22 +51,7 @@
 #include "insn-codes.h"
 #include "tm.h"
 #include "hard-reg-set.h"
-
-#if CHECKING_P
-
-struct cumulative_args_t { void *magic; void *p; };
-
-#else /* !CHECKING_P */
-
-/* When using a GCC build compiler, we could use
-   __attribute__((transparent_union)) to get cumulative_args_t function
-   arguments passed like scalars where the ABI would mandate a less
-   efficient way of argument passing otherwise.  However, that would come
-   at the cost of less type-safe !CHECKING_P compilation.  */
-
-union cumulative_args_t { void *p; };
-
-#endif /* !CHECKING_P */
+#include "cumulative-args.h"
 
 /* Types of memory operation understood by the "by_pieces" infrastructure.
    Used by the TARGET_USE_BY_PIECES_INFRASTRUCTURE_P target hook and
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 6f071f80231..d83f362a5ae 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -850,6 +850,14 @@ default_function_arg_boundary (machine_mode mode ATTRIBUTE_UNUSED,
   return PARM_BOUNDARY;
 }
 
+unsigned int
+default_function_arg_boundary_ca (machine_mode mode ATTRIBUTE_UNUSED,
+				  const_tree type ATTRIBUTE_UNUSED,
+				  cumulative_args_t ca ATTRIBUTE_UNUSED)
+{
+  return default_function_arg_boundary (mode, type);
+}
+
 unsigned int
 default_function_arg_round_boundary (machine_mode mode ATTRIBUTE_UNUSED,
 				     const_tree type ATTRIBUTE_UNUSED)
@@ -857,6 +865,14 @@ default_function_arg_round_boundary (machine_mode mode ATTRIBUTE_UNUSED,
   return PARM_BOUNDARY;
 }
 
+unsigned int
+default_function_arg_round_boundary_ca (machine_mode mode ATTRIBUTE_UNUSED,
+					const_tree type ATTRIBUTE_UNUSED,
+					cumulative_args_t ca ATTRIBUTE_UNUSED)
+{
+  return default_function_arg_round_boundary (mode, type);
+}
+
 void
 hook_void_bitmap (bitmap regs ATTRIBUTE_UNUSED)
 {
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 11e9d7dd1a8..f33374ef073 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -154,6 +154,12 @@ extern unsigned int default_function_arg_boundary (machine_mode,
 						   const_tree);
 extern unsigned int default_function_arg_round_boundary (machine_mode,
 							 const_tree);
+extern unsigned int default_function_arg_boundary_ca (machine_mode,
+						      const_tree,
+						      cumulative_args_t ca);
+extern unsigned int default_function_arg_round_boundary_ca (machine_mode,
+							    const_tree,
+							    cumulative_args_t ca);
 extern bool hook_bool_const_rtx_commutative_p (const_rtx, int);
 extern rtx default_function_value (const_tree, const_tree, bool);
 extern HARD_REG_SET default_zero_call_used_regs (HARD_REG_SET);
-- 
2.30.1 (Apple Git-130)


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

* [PATCH 2/2] Implement TARGET_..._CA target hooks for AArch64 Darwin
  2021-11-13  9:42 [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends Maxim Blinov
@ 2021-11-13  9:42 ` Maxim Blinov
  2021-11-15  7:11 ` [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends Richard Biener
  1 sibling, 0 replies; 8+ messages in thread
From: Maxim Blinov @ 2021-11-13  9:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: iain, maxim.blinov

Note: This patch is not yet ready for trunk as its dependent on some
patches that are not-yet-upstream, however it serves as motivation for
the previous patch(es) which are independent.

----

The AArch64 Darwin platform requires that named stack arguments are
passed naturally-aligned, while variadic stack arguments are passed on
word boundaries. Use the TARGET_FUNCTION_ARG_BOUNDARY_CA and
TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA target hooks to let the backend
correctly layout stack parameters.

gcc/ChangeLog:

	* config.gcc: Enable -fstack-use-cumulative-args by default if the
	host platform is MacOS 11.x or 12.x and we're on AArch64.

gcc/config/aarch64/ChangeLog:

	* aarch64-protos.h (aarch64_init_cumulative_incoming_args):
	Declare.
	* aarch64.c (aarch64_init_cumulative_args): Initialize
	`darwinpcs_n_named` (Total number of named parameters) and
	`darwinpcs_n_args_processed` (Total number of parameters we
	have processed, including variadic if any.)
	(aarch64_init_cumulative_incoming_args): Implement the
	INIT_CUMULATIVE_INCOMING_ARGS macro in order to capture
	information on the number of named parameters for the current
	function.
	(aarch64_function_arg_advance): Increment
	`darwinpcs_n_args_processed` each time we layout a function
	parameter.
	(aarch64_function_arg_boundary_ca): Implement
	TARGET_FUNCTION_ARG_BOUNDARY_CA and
	TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA to layout args based on
	whether we're a named parameter or not.
	(aarch64_function_arg_round_boundary_ca): Ditto.
	(TARGET_FUNCTION_ARG_BOUNDARY_CA): Define.
	(TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA): Ditto.
	* aarch64.h (CUMULATIVE_ARGS): Add `darwinpcs_n_named` and
	`darwinpcs_n_args_processed`.
	(INIT_CUMULATIVE_INCOMING_ARGS): Define.
---
 gcc/config.gcc                      |  7 ++++
 gcc/config/aarch64/aarch64-protos.h |  1 +
 gcc/config/aarch64/aarch64.c        | 56 +++++++++++++++++++++++++++++
 gcc/config/aarch64/aarch64.h        |  5 +++
 4 files changed, 69 insertions(+)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index e12a9f042d0..626ba089c07 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1072,6 +1072,13 @@ esac
 
 # Figure out if we need to enable -foff-stack-trampolines by default.
 case ${target} in
+aarch64*-*darwin* | arm64*-*darwin*)
+  if test ${macos_maj} = 11 || test ${macos_maj} = 12; then
+    tm_defines="$tm_defines STACK_USE_CUMULATIVE_ARGS_INIT=1"
+  else
+    tm_defines="$tm_defines STACK_USE_CUMULATIVE_ARGS_INIT=0"
+  fi
+  ;;
 *)
   tm_defines="$tm_defines STACK_USE_CUMULATIVE_ARGS_INIT=0"
   ;;
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index a204647241e..cdc51fce906 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -896,6 +896,7 @@ void aarch64_expand_vector_init (rtx, rtx);
 void aarch64_sve_expand_vector_init (rtx, rtx);
 void aarch64_init_cumulative_args (CUMULATIVE_ARGS *, const_tree, rtx,
 				   const_tree, unsigned, bool = false);
+void aarch64_init_cumulative_incoming_args (CUMULATIVE_ARGS *, const_tree, rtx);
 void aarch64_init_expanders (void);
 void aarch64_init_simd_builtins (void);
 void aarch64_emit_call_insn (rtx);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 38b3f1eab89..70c2336ab3a 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -7042,6 +7042,8 @@ aarch64_init_cumulative_args (CUMULATIVE_ARGS *pcum,
   pcum->darwinpcs_stack_bytes = 0;
   pcum->darwinpcs_sub_word_offset = 0;
   pcum->darwinpcs_sub_word_pos = 0;
+  pcum->darwinpcs_n_named = n_named;
+  pcum->darwinpcs_n_args_processed = 0;
   pcum->silent_p = silent_p;
   pcum->aapcs_vfp_rmode = VOIDmode;
 
@@ -7072,6 +7074,20 @@ aarch64_init_cumulative_args (CUMULATIVE_ARGS *pcum,
     }
 }
 
+void
+aarch64_init_cumulative_incoming_args (CUMULATIVE_ARGS *pcum,
+				       const_tree fntype,
+				       rtx libname ATTRIBUTE_UNUSED)
+{
+#if !TARGET_MACHO
+  INIT_CUMULATIVE_ARGS (*pcum, fntype, libname, current_function_decl, -1);
+#else
+  int n_named_args = (list_length (TYPE_ARG_TYPES (fntype)));
+
+  aarch64_init_cumulative_args (pcum, fntype, libname, current_function_decl, n_named_args);
+#endif
+}
+
 static void
 aarch64_function_arg_advance (cumulative_args_t pcum_v,
 			      const function_arg_info &arg)
@@ -7092,6 +7108,7 @@ aarch64_function_arg_advance (cumulative_args_t pcum_v,
       pcum->aapcs_stack_size += pcum->aapcs_stack_words;
       pcum->aapcs_stack_words = 0;
       pcum->aapcs_reg = NULL_RTX;
+      pcum->darwinpcs_n_args_processed++;
     }
 }
 
@@ -7147,6 +7164,19 @@ aarch64_function_arg_boundary (machine_mode mode, const_tree type)
 #endif
 }
 
+static unsigned int
+aarch64_function_arg_boundary_ca (machine_mode mode, const_tree type,
+				  cumulative_args_t ca)
+{
+  CUMULATIVE_ARGS *pcum = get_cumulative_args (ca);
+  bool named_p = pcum->darwinpcs_n_args_processed < pcum->darwinpcs_n_named;
+
+  if (named_p)
+    return aarch64_function_arg_boundary (mode, type);
+  else
+    return MAX (aarch64_function_arg_boundary (mode, type), PARM_BOUNDARY);
+}
+
 #if TARGET_MACHO
 /* Implement TARGET_FUNCTION_ARG_ROUND_BOUNDARY for darwinpcs which allows
    non-standard passing of byte-aligned items [D.2].
@@ -7157,6 +7187,26 @@ aarch64_function_arg_round_boundary (machine_mode, const_tree)
 {
   return BITS_PER_UNIT;
 }
+
+static unsigned int
+aarch64_function_arg_round_boundary_ca (machine_mode mode, const_tree type,
+					cumulative_args_t ca)
+{
+  CUMULATIVE_ARGS *pcum = get_cumulative_args (ca);
+  bool named_p = pcum->darwinpcs_n_args_processed < pcum->darwinpcs_n_named;
+  bool last_named_p = pcum->darwinpcs_n_args_processed + 1 == pcum->darwinpcs_n_named;
+
+  if (named_p)
+    {
+      if (last_named_p)
+	return PARM_BOUNDARY;
+      else
+	return aarch64_function_arg_round_boundary (mode, type);
+    }
+  else
+    return PARM_BOUNDARY;
+}
+
 #endif
 
 /* Implement TARGET_GET_RAW_RESULT_MODE and TARGET_GET_RAW_ARG_MODE.  */
@@ -26692,9 +26742,15 @@ aarch64_run_selftests (void)
 #undef TARGET_FUNCTION_ARG_BOUNDARY
 #define TARGET_FUNCTION_ARG_BOUNDARY aarch64_function_arg_boundary
 
+#undef TARGET_FUNCTION_ARG_BOUNDARY_CA
+#define TARGET_FUNCTION_ARG_BOUNDARY_CA aarch64_function_arg_boundary_ca
+
 #if TARGET_MACHO
 #undef  TARGET_FUNCTION_ARG_ROUND_BOUNDARY
 #define TARGET_FUNCTION_ARG_ROUND_BOUNDARY aarch64_function_arg_round_boundary
+
+#undef  TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA
+#define TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA aarch64_function_arg_round_boundary_ca
 #endif
 
 #undef TARGET_FUNCTION_ARG_PADDING
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 82d8803149b..2ea45fd829b 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -997,6 +997,8 @@ typedef struct
 				   when placing smaller items for darwinpcs.  */
   int darwinpcs_sub_word_pos;	/* The next byte available within the word for
 				   darwinpcs.  */
+  int darwinpcs_n_named;        /* Number of named arguments.  */
+  int darwinpcs_n_args_processed; /* Number of arguments processed so far.  */
   bool silent_p;		/* True if we should act silently, rather than
 				   raise an error for invalid calls.  */
 } CUMULATIVE_ARGS;
@@ -1010,6 +1012,9 @@ typedef struct
 #define INIT_CUMULATIVE_ARGS(CUM, FNTYPE, LIBNAME, FNDECL, N_NAMED_ARGS) \
   aarch64_init_cumulative_args (&(CUM), FNTYPE, LIBNAME, FNDECL, N_NAMED_ARGS)
 
+#define INIT_CUMULATIVE_INCOMING_ARGS(CUM, FNTYPE, LIBNAME) \
+  aarch64_init_cumulative_incoming_args (&(CUM), FNTYPE, LIBNAME)
+
 #define FUNCTION_ARG_REGNO_P(REGNO) \
   aarch64_function_arg_regno_p(REGNO)
 \f
-- 
2.30.1 (Apple Git-130)


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

* Re: [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends
  2021-11-13  9:42 [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends Maxim Blinov
  2021-11-13  9:42 ` [PATCH 2/2] Implement TARGET_..._CA target hooks for AArch64 Darwin Maxim Blinov
@ 2021-11-15  7:11 ` Richard Biener
  2021-11-22 14:40   ` Maxim Blinov
  1 sibling, 1 reply; 8+ messages in thread
From: Richard Biener @ 2021-11-15  7:11 UTC (permalink / raw)
  To: Maxim Blinov; +Cc: GCC Patches, Iain Sandoe

On Sat, Nov 13, 2021 at 10:43 AM Maxim Blinov <maxim.blinov@embecosm.com> wrote:
>
> The two target hooks responsible for informing GCC about stack
> parameter alignment are `TARGET_FUNCTION_ARG_BOUNDARY` and
> `TARGET_FUNCTION_ARG_ROUND_BOUNDARY`, which currently only consider
> the tree and machine_mode of a specific given argument.
>
> Create two new target hooks suffixed with '_CA', and pass in a third
> `cumulative_args_t` parameter. This enables the backend to make
> alignment decisions based on the context of the whole function rather
> than individual parameters.
>
> The orignal machine_mode/tree type macros are not removed - they are
> called by the default implementations of `TARGET_...BOUNDARY_CA` and
> `TARGET_...ROUND_BOUNDARY_CA`. This is done with the intetnion of
> avoiding large mechanical modifications of nearly every backend in
> GCC. There is also a new flag, -fstack-use-cumulative-args, which
> provides a way to completely bypass the new `..._CA` macros. This
> feature is intended for debugging GCC itself.

Just two quick comments without looking at the patch.

Please do not introduce options in the user namespace -f... which are
for debugging only.  I think you should go without this part instead.

Second, you fail to motivate the change.  I cannot make sense of
"This enables the backend to make alignment decisions based on the
context of the whole function rather than individual parameters."

Richard.

>
> gcc/ChangeLog:
>
>         * calls.c (initialize_argument_information): Pass `args_so_far`.
>         * common.opt: New flag `-fstack-use-cumulative-args`.
>         * config.gcc: No platforms currently use ..._CA-hooks: Set
>         -fstack-use-cumulative-args to be off by default.
>         * target.h (cumulative_args_t): Move declaration from here, to...
>         * cumulative-args.h (cumulative_args_t): ...this new file. This is
>         to permit backends to include the declaration of cumulative_args_t
>         without dragging in circular dependencies.
>         * function.c (assign_parm_find_entry_rtl): Provide
>         cumulative_args_t to locate_and_pad_parm.
>         (gimplify_parameters): Ditto.
>         (locate_and_pad_parm): Conditionally call new hooks if we're
>         invoked with -fstack-use-cumulative-args.
>         * function.h: Include cumulative-args.h.
>         (locate_and_pad_parm): Add cumulative_args_t parameter.
>         * target.def (function_arg_boundary_ca): Add.
>         (function_arg_round_boundary_ca): Ditto.
>         * targhooks.c (default_function_arg_boundary_ca): Implement.
>         (default_function_arg_round_boundary_ca): Ditto.
>         * targhooks.h (default_function_arg_boundary_ca): Declare.
>         (default_function_arg_round_boundary_ca): Ditto.
>         * doc/invoke.texi (-fstack-use-cumulative-args): Document.
>         * doc/tm.texi: Regenerate.
>         * doc/tm.texi.in: Ditto.
> ---
>  gcc/calls.c           |  3 +++
>  gcc/common.opt        |  4 ++++
>  gcc/config.gcc        |  7 +++++++
>  gcc/cumulative-args.h | 20 ++++++++++++++++++++
>  gcc/doc/invoke.texi   | 12 ++++++++++++
>  gcc/doc/tm.texi       | 20 ++++++++++++++++++++
>  gcc/doc/tm.texi.in    |  4 ++++
>  gcc/function.c        | 25 +++++++++++++++++++++----
>  gcc/function.h        |  2 ++
>  gcc/target.def        | 24 ++++++++++++++++++++++++
>  gcc/target.h          | 17 +----------------
>  gcc/targhooks.c       | 16 ++++++++++++++++
>  gcc/targhooks.h       |  6 ++++++
>  13 files changed, 140 insertions(+), 20 deletions(-)
>  create mode 100644 gcc/cumulative-args.h
>
> diff --git a/gcc/calls.c b/gcc/calls.c
> index 27b59f26ad3..cef612a6ef4 100644
> --- a/gcc/calls.c
> +++ b/gcc/calls.c
> @@ -1527,6 +1527,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
>  #endif
>                              reg_parm_stack_space,
>                              args[i].pass_on_stack ? 0 : args[i].partial,
> +                            args_so_far,
>                              fndecl, args_size, &args[i].locate);
>  #ifdef BLOCK_REG_PADDING
>        else
> @@ -4205,6 +4206,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
>                            argvec[count].reg != 0,
>  #endif
>                            reg_parm_stack_space, 0,
> +                          args_so_far,
>                            NULL_TREE, &args_size, &argvec[count].locate);
>
>        if (argvec[count].reg == 0 || argvec[count].partial != 0
> @@ -4296,6 +4298,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
>                                argvec[count].reg != 0,
>  #endif
>                                reg_parm_stack_space, argvec[count].partial,
> +                              args_so_far,
>                                NULL_TREE, &args_size, &argvec[count].locate);
>           args_size.constant += argvec[count].locate.size.constant;
>           gcc_assert (!argvec[count].locate.size.var);
> diff --git a/gcc/common.opt b/gcc/common.opt
> index de9b848eda5..982417c1e39 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2705,6 +2705,10 @@ fstack-usage
>  Common RejectNegative Var(flag_stack_usage)
>  Output stack usage information on a per-function basis.
>
> +fstack-use-cumulative-args
> +Common RejectNegative Var(flag_stack_use_cumulative_args) Init(STACK_USE_CUMULATIVE_ARGS_INIT)
> +Use cumulative args-based stack layout hooks.
> +
>  fstrength-reduce
>  Common Ignore
>  Does nothing.  Preserved for backward compatibility.
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index edd12655c4a..046d691af56 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -1070,6 +1070,13 @@ case ${target} in
>    ;;
>  esac
>
> +# Figure out if we need to enable -foff-stack-trampolines by default.
> +case ${target} in
> +*)
> +  tm_defines="$tm_defines STACK_USE_CUMULATIVE_ARGS_INIT=0"
> +  ;;
> +esac
> +
>  case ${target} in
>  aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-rtems*)
>         tm_file="${tm_file} dbxelf.h elfos.h newlib-stdint.h"
> diff --git a/gcc/cumulative-args.h b/gcc/cumulative-args.h
> new file mode 100644
> index 00000000000..b60928e37f9
> --- /dev/null
> +++ b/gcc/cumulative-args.h
> @@ -0,0 +1,20 @@
> +#ifndef GCC_CUMULATIVE_ARGS_H
> +#define GCC_CUMULATIVE_ARGS_H
> +
> +#if CHECKING_P
> +
> +struct cumulative_args_t { void *magic; void *p; };
> +
> +#else /* !CHECKING_P */
> +
> +/* When using a GCC build compiler, we could use
> +   __attribute__((transparent_union)) to get cumulative_args_t function
> +   arguments passed like scalars where the ABI would mandate a less
> +   efficient way of argument passing otherwise.  However, that would come
> +   at the cost of less type-safe !CHECKING_P compilation.  */
> +
> +union cumulative_args_t { void *p; };
> +
> +#endif /* !CHECKING_P */
> +
> +#endif /* GCC_CUMULATIVE_ARGS_H */
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 2aba4c70b44..7ffb2997c69 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -670,6 +670,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fverbose-asm  -fpack-struct[=@var{n}]  @gol
>  -fleading-underscore  -ftls-model=@var{model} @gol
>  -fstack-reuse=@var{reuse_level} @gol
> +-fstack-use-cumulative-args @gol
>  -ftrampolines  -ftrapv  -fwrapv @gol
>  -fvisibility=@r{[}default@r{|}internal@r{|}hidden@r{|}protected@r{]} @gol
>  -fstrict-volatile-bitfields  -fsync-libcalls}
> @@ -16625,6 +16626,17 @@ the behavior of older compilers in which temporaries' stack space is
>  not reused, the aggressive stack reuse can lead to runtime errors. This
>  option is used to control the temporary stack reuse optimization.
>
> +@item -fstack-use-cumulative-args
> +@opindex fstack_use_cumulative_args
> +This option instructs the compiler to use the
> +@code{cumulative_args_t}-based stack layout target hooks,
> +@code{TARGET_FUNCTION_ARG_BOUNDARY_CA} and
> +@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA}. If a given target does
> +not define these hooks, the default behaviour is to fallback to using
> +the standard non-@code{_CA} variants instead. Certain targets (such as
> +AArch64 Darwin) require using the more advanced @code{_CA}-based
> +hooks: For these targets this option should be enabled by default.
> +
>  @item -ftrapv
>  @opindex ftrapv
>  This option generates traps for signed overflow on addition, subtraction,
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 78a1af1ad4d..261eaf46ef5 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -4322,6 +4322,16 @@ with the specified mode and type.  The default hook returns
>  @code{PARM_BOUNDARY} for all arguments.
>  @end deftypefn
>
> +@deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_BOUNDARY_CA (machine_mode @var{mode}, const_tree @var{type}, cumulative_args_t @var{ca})
> +This is the @code{cumulative_args_t}-based version of
> +@code{TARGET_FUNCTION_ARG_BOUNDARY}. Define this hook if you need more
> +fine-grained control over argument alignment, e.g. depending on whether
> +it is a named argument or not, or any other criteria that you choose to
> +place in the @var{ca} structure.
> +
> +The default hook will call @code{TARGET_FUNCTION_ARG_BOUNDARY}.
> +@end deftypefn
> +
>  @deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_ROUND_BOUNDARY (machine_mode @var{mode}, const_tree @var{type})
>  Normally, the size of an argument is rounded up to @code{PARM_BOUNDARY},
>  which is the default value for this hook.  You can define this hook to
> @@ -4329,6 +4339,16 @@ return a different value if an argument size must be rounded to a larger
>  value.
>  @end deftypefn
>
> +@deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA (machine_mode @var{mode}, const_tree @var{type}, cumulative_args_t @var{ca})
> +This is the @code{cumulative_args_t}-based version of
> +@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}. Define this hook if you need more
> +fine-grained control over argument size rounding, e.g. depending on whether
> +it is a named argument or not, or any other criteria that you choose to
> +place in the @var{ca} structure.
> +
> +The default hook will call @code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}.
> +@end deftypefn
> +
>  @defmac FUNCTION_ARG_REGNO_P (@var{regno})
>  A C expression that is nonzero if @var{regno} is the number of a hard
>  register in which function arguments are sometimes passed.  This does
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 4401550989e..933644dfa86 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -3330,8 +3330,12 @@ required.
>
>  @hook TARGET_FUNCTION_ARG_BOUNDARY
>
> +@hook TARGET_FUNCTION_ARG_BOUNDARY_CA
> +
>  @hook TARGET_FUNCTION_ARG_ROUND_BOUNDARY
>
> +@hook TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA
> +
>  @defmac FUNCTION_ARG_REGNO_P (@var{regno})
>  A C expression that is nonzero if @var{regno} is the number of a hard
>  register in which function arguments are sometimes passed.  This does
> diff --git a/gcc/function.c b/gcc/function.c
> index 61b3bd036b8..1ebf3e0ffe5 100644
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -2610,7 +2610,9 @@ assign_parm_find_entry_rtl (struct assign_parm_data_all *all,
>
>    locate_and_pad_parm (data->arg.mode, data->arg.type, in_regs,
>                        all->reg_parm_stack_space,
> -                      entry_parm ? data->partial : 0, current_function_decl,
> +                      entry_parm ? data->partial : 0,
> +                      all->args_so_far,
> +                      current_function_decl,
>                        &all->stack_args_size, &data->locate);
>
>    /* Update parm_stack_boundary if this parameter is passed in the
> @@ -4027,6 +4029,7 @@ gimplify_parameters (gimple_seq *cleanup)
>  void
>  locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs,
>                      int reg_parm_stack_space, int partial,
> +                    cumulative_args_t ca,
>                      tree fndecl ATTRIBUTE_UNUSED,
>                      struct args_size *initial_offset_ptr,
>                      struct locate_and_pad_arg_data *locate)
> @@ -4064,9 +4067,23 @@ locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs,
>               ? arg_size_in_bytes (type)
>               : size_int (GET_MODE_SIZE (passed_mode)));
>    where_pad = targetm.calls.function_arg_padding (passed_mode, type);
> -  boundary = targetm.calls.function_arg_boundary (passed_mode, type);
> -  round_boundary = targetm.calls.function_arg_round_boundary (passed_mode,
> -                                                             type);
> +
> +  if (flag_stack_use_cumulative_args)
> +    {
> +      boundary = targetm.calls.function_arg_boundary_ca (passed_mode,
> +                                                        type,
> +                                                        ca);
> +      round_boundary = targetm.calls.function_arg_round_boundary_ca
> +       (passed_mode, type, ca);
> +    }
> +  else
> +    {
> +      boundary = targetm.calls.function_arg_boundary (passed_mode,
> +                                                     type);
> +      round_boundary = targetm.calls.function_arg_round_boundary
> +       (passed_mode, type);
> +    }
> +
>    locate->where_pad = where_pad;
>
>    /* Alignment can't exceed MAX_SUPPORTED_STACK_ALIGNMENT.  */
> diff --git a/gcc/function.h b/gcc/function.h
> index 899430833ce..26f342caba3 100644
> --- a/gcc/function.h
> +++ b/gcc/function.h
> @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3.  If not see
>  #ifndef GCC_FUNCTION_H
>  #define GCC_FUNCTION_H
>
> +#include "cumulative-args.h"
>
>  /* Stack of pending (incomplete) sequences saved by `start_sequence'.
>     Each element describes one pending sequence.
> @@ -661,6 +662,7 @@ extern int aggregate_value_p (const_tree, const_tree);
>  extern bool use_register_for_decl (const_tree);
>  extern gimple_seq gimplify_parameters (gimple_seq *);
>  extern void locate_and_pad_parm (machine_mode, tree, int, int, int,
> +                                cumulative_args_t,
>                                  tree, struct args_size *,
>                                  struct locate_and_pad_arg_data *);
>  extern void generate_setjmp_warnings (void);
> diff --git a/gcc/target.def b/gcc/target.def
> index 51ea167172b..bb56afab539 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -4959,6 +4959,18 @@ with the specified mode and type.  The default hook returns\n\
>   unsigned int, (machine_mode mode, const_tree type),
>   default_function_arg_boundary)
>
> +DEFHOOK
> +(function_arg_boundary_ca,
> + "This is the @code{cumulative_args_t}-based version of\n\
> +@code{TARGET_FUNCTION_ARG_BOUNDARY}. Define this hook if you need more\n\
> +fine-grained control over argument alignment, e.g. depending on whether\n\
> +it is a named argument or not, or any other criteria that you choose to\n\
> +place in the @var{ca} structure.\n\
> +\n\
> +The default hook will call @code{TARGET_FUNCTION_ARG_BOUNDARY}.",
> + unsigned int, (machine_mode mode, const_tree type, cumulative_args_t ca),
> + default_function_arg_boundary_ca)
> +
>  DEFHOOK
>  (function_arg_round_boundary,
>   "Normally, the size of an argument is rounded up to @code{PARM_BOUNDARY},\n\
> @@ -4968,6 +4980,18 @@ value.",
>   unsigned int, (machine_mode mode, const_tree type),
>   default_function_arg_round_boundary)
>
> +DEFHOOK
> +(function_arg_round_boundary_ca,
> + "This is the @code{cumulative_args_t}-based version of\n\
> +@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}. Define this hook if you need more\n\
> +fine-grained control over argument size rounding, e.g. depending on whether\n\
> +it is a named argument or not, or any other criteria that you choose to\n\
> +place in the @var{ca} structure.\n\
> +\n\
> +The default hook will call @code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}.",
> + unsigned int, (machine_mode mode, const_tree type, cumulative_args_t ca),
> + default_function_arg_round_boundary_ca)
> +
>  /* Return the diagnostic message string if function without a prototype
>     is not allowed for this 'val' argument; NULL otherwise. */
>  DEFHOOK
> diff --git a/gcc/target.h b/gcc/target.h
> index d8f45fb99c3..5b28ece7e1c 100644
> --- a/gcc/target.h
> +++ b/gcc/target.h
> @@ -51,22 +51,7 @@
>  #include "insn-codes.h"
>  #include "tm.h"
>  #include "hard-reg-set.h"
> -
> -#if CHECKING_P
> -
> -struct cumulative_args_t { void *magic; void *p; };
> -
> -#else /* !CHECKING_P */
> -
> -/* When using a GCC build compiler, we could use
> -   __attribute__((transparent_union)) to get cumulative_args_t function
> -   arguments passed like scalars where the ABI would mandate a less
> -   efficient way of argument passing otherwise.  However, that would come
> -   at the cost of less type-safe !CHECKING_P compilation.  */
> -
> -union cumulative_args_t { void *p; };
> -
> -#endif /* !CHECKING_P */
> +#include "cumulative-args.h"
>
>  /* Types of memory operation understood by the "by_pieces" infrastructure.
>     Used by the TARGET_USE_BY_PIECES_INFRASTRUCTURE_P target hook and
> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index 6f071f80231..d83f362a5ae 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -850,6 +850,14 @@ default_function_arg_boundary (machine_mode mode ATTRIBUTE_UNUSED,
>    return PARM_BOUNDARY;
>  }
>
> +unsigned int
> +default_function_arg_boundary_ca (machine_mode mode ATTRIBUTE_UNUSED,
> +                                 const_tree type ATTRIBUTE_UNUSED,
> +                                 cumulative_args_t ca ATTRIBUTE_UNUSED)
> +{
> +  return default_function_arg_boundary (mode, type);
> +}
> +
>  unsigned int
>  default_function_arg_round_boundary (machine_mode mode ATTRIBUTE_UNUSED,
>                                      const_tree type ATTRIBUTE_UNUSED)
> @@ -857,6 +865,14 @@ default_function_arg_round_boundary (machine_mode mode ATTRIBUTE_UNUSED,
>    return PARM_BOUNDARY;
>  }
>
> +unsigned int
> +default_function_arg_round_boundary_ca (machine_mode mode ATTRIBUTE_UNUSED,
> +                                       const_tree type ATTRIBUTE_UNUSED,
> +                                       cumulative_args_t ca ATTRIBUTE_UNUSED)
> +{
> +  return default_function_arg_round_boundary (mode, type);
> +}
> +
>  void
>  hook_void_bitmap (bitmap regs ATTRIBUTE_UNUSED)
>  {
> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> index 11e9d7dd1a8..f33374ef073 100644
> --- a/gcc/targhooks.h
> +++ b/gcc/targhooks.h
> @@ -154,6 +154,12 @@ extern unsigned int default_function_arg_boundary (machine_mode,
>                                                    const_tree);
>  extern unsigned int default_function_arg_round_boundary (machine_mode,
>                                                          const_tree);
> +extern unsigned int default_function_arg_boundary_ca (machine_mode,
> +                                                     const_tree,
> +                                                     cumulative_args_t ca);
> +extern unsigned int default_function_arg_round_boundary_ca (machine_mode,
> +                                                           const_tree,
> +                                                           cumulative_args_t ca);
>  extern bool hook_bool_const_rtx_commutative_p (const_rtx, int);
>  extern rtx default_function_value (const_tree, const_tree, bool);
>  extern HARD_REG_SET default_zero_call_used_regs (HARD_REG_SET);
> --
> 2.30.1 (Apple Git-130)
>

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

* Re: [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends
  2021-11-15  7:11 ` [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends Richard Biener
@ 2021-11-22 14:40   ` Maxim Blinov
  2021-11-22 15:15     ` Richard Biener
  0 siblings, 1 reply; 8+ messages in thread
From: Maxim Blinov @ 2021-11-22 14:40 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches, Iain Sandoe

Hi Richard,

The purpose of this patch is to give more of the target code access to
the cumulative_args_t structure in order to enable certain (otherwise
currently impossible) stack layout constraints, specifically for
parameters that are passed over the stack. For example, there are some
targets (not yet in GCC trunk) which explicitly require the
distinction between named and variadic parameters in order to enforce
different alignment rules (when passing over the stack.) Such a
constraint would be accommodated by this patch.

The patch itself is very straightforward and simply adds the parameter
to the two functions which we'd like to extend. The motivation of
defining new target hooks was to minimize the patch size.

A concrete user of this change that we have in mind is the AArch64
Darwin parameter passing abi, which mandates that when passing on the
stack, named parameters are to be naturally-aligned, however variadic
ones are to be word-aligned. However this patch is completely generic
in nature and should enable all targets to have more control over
their parameter layout process.

Best Regards,
Maxim

On Mon, 15 Nov 2021 at 07:11, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Sat, Nov 13, 2021 at 10:43 AM Maxim Blinov <maxim.blinov@embecosm.com> wrote:
> >
> > The two target hooks responsible for informing GCC about stack
> > parameter alignment are `TARGET_FUNCTION_ARG_BOUNDARY` and
> > `TARGET_FUNCTION_ARG_ROUND_BOUNDARY`, which currently only consider
> > the tree and machine_mode of a specific given argument.
> >
> > Create two new target hooks suffixed with '_CA', and pass in a third
> > `cumulative_args_t` parameter. This enables the backend to make
> > alignment decisions based on the context of the whole function rather
> > than individual parameters.
> >
> > The orignal machine_mode/tree type macros are not removed - they are
> > called by the default implementations of `TARGET_...BOUNDARY_CA` and
> > `TARGET_...ROUND_BOUNDARY_CA`. This is done with the intetnion of
> > avoiding large mechanical modifications of nearly every backend in
> > GCC. There is also a new flag, -fstack-use-cumulative-args, which
> > provides a way to completely bypass the new `..._CA` macros. This
> > feature is intended for debugging GCC itself.
>
> Just two quick comments without looking at the patch.
>
> Please do not introduce options in the user namespace -f... which are
> for debugging only.  I think you should go without this part instead.
>
> Second, you fail to motivate the change.  I cannot make sense of
> "This enables the backend to make alignment decisions based on the
> context of the whole function rather than individual parameters."
>
> Richard.
>
> >
> > gcc/ChangeLog:
> >
> >         * calls.c (initialize_argument_information): Pass `args_so_far`.
> >         * common.opt: New flag `-fstack-use-cumulative-args`.
> >         * config.gcc: No platforms currently use ..._CA-hooks: Set
> >         -fstack-use-cumulative-args to be off by default.
> >         * target.h (cumulative_args_t): Move declaration from here, to...
> >         * cumulative-args.h (cumulative_args_t): ...this new file. This is
> >         to permit backends to include the declaration of cumulative_args_t
> >         without dragging in circular dependencies.
> >         * function.c (assign_parm_find_entry_rtl): Provide
> >         cumulative_args_t to locate_and_pad_parm.
> >         (gimplify_parameters): Ditto.
> >         (locate_and_pad_parm): Conditionally call new hooks if we're
> >         invoked with -fstack-use-cumulative-args.
> >         * function.h: Include cumulative-args.h.
> >         (locate_and_pad_parm): Add cumulative_args_t parameter.
> >         * target.def (function_arg_boundary_ca): Add.
> >         (function_arg_round_boundary_ca): Ditto.
> >         * targhooks.c (default_function_arg_boundary_ca): Implement.
> >         (default_function_arg_round_boundary_ca): Ditto.
> >         * targhooks.h (default_function_arg_boundary_ca): Declare.
> >         (default_function_arg_round_boundary_ca): Ditto.
> >         * doc/invoke.texi (-fstack-use-cumulative-args): Document.
> >         * doc/tm.texi: Regenerate.
> >         * doc/tm.texi.in: Ditto.
> > ---
> >  gcc/calls.c           |  3 +++
> >  gcc/common.opt        |  4 ++++
> >  gcc/config.gcc        |  7 +++++++
> >  gcc/cumulative-args.h | 20 ++++++++++++++++++++
> >  gcc/doc/invoke.texi   | 12 ++++++++++++
> >  gcc/doc/tm.texi       | 20 ++++++++++++++++++++
> >  gcc/doc/tm.texi.in    |  4 ++++
> >  gcc/function.c        | 25 +++++++++++++++++++++----
> >  gcc/function.h        |  2 ++
> >  gcc/target.def        | 24 ++++++++++++++++++++++++
> >  gcc/target.h          | 17 +----------------
> >  gcc/targhooks.c       | 16 ++++++++++++++++
> >  gcc/targhooks.h       |  6 ++++++
> >  13 files changed, 140 insertions(+), 20 deletions(-)
> >  create mode 100644 gcc/cumulative-args.h
> >
> > diff --git a/gcc/calls.c b/gcc/calls.c
> > index 27b59f26ad3..cef612a6ef4 100644
> > --- a/gcc/calls.c
> > +++ b/gcc/calls.c
> > @@ -1527,6 +1527,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
> >  #endif
> >                              reg_parm_stack_space,
> >                              args[i].pass_on_stack ? 0 : args[i].partial,
> > +                            args_so_far,
> >                              fndecl, args_size, &args[i].locate);
> >  #ifdef BLOCK_REG_PADDING
> >        else
> > @@ -4205,6 +4206,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
> >                            argvec[count].reg != 0,
> >  #endif
> >                            reg_parm_stack_space, 0,
> > +                          args_so_far,
> >                            NULL_TREE, &args_size, &argvec[count].locate);
> >
> >        if (argvec[count].reg == 0 || argvec[count].partial != 0
> > @@ -4296,6 +4298,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
> >                                argvec[count].reg != 0,
> >  #endif
> >                                reg_parm_stack_space, argvec[count].partial,
> > +                              args_so_far,
> >                                NULL_TREE, &args_size, &argvec[count].locate);
> >           args_size.constant += argvec[count].locate.size.constant;
> >           gcc_assert (!argvec[count].locate.size.var);
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index de9b848eda5..982417c1e39 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -2705,6 +2705,10 @@ fstack-usage
> >  Common RejectNegative Var(flag_stack_usage)
> >  Output stack usage information on a per-function basis.
> >
> > +fstack-use-cumulative-args
> > +Common RejectNegative Var(flag_stack_use_cumulative_args) Init(STACK_USE_CUMULATIVE_ARGS_INIT)
> > +Use cumulative args-based stack layout hooks.
> > +
> >  fstrength-reduce
> >  Common Ignore
> >  Does nothing.  Preserved for backward compatibility.
> > diff --git a/gcc/config.gcc b/gcc/config.gcc
> > index edd12655c4a..046d691af56 100644
> > --- a/gcc/config.gcc
> > +++ b/gcc/config.gcc
> > @@ -1070,6 +1070,13 @@ case ${target} in
> >    ;;
> >  esac
> >
> > +# Figure out if we need to enable -foff-stack-trampolines by default.
> > +case ${target} in
> > +*)
> > +  tm_defines="$tm_defines STACK_USE_CUMULATIVE_ARGS_INIT=0"
> > +  ;;
> > +esac
> > +
> >  case ${target} in
> >  aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-rtems*)
> >         tm_file="${tm_file} dbxelf.h elfos.h newlib-stdint.h"
> > diff --git a/gcc/cumulative-args.h b/gcc/cumulative-args.h
> > new file mode 100644
> > index 00000000000..b60928e37f9
> > --- /dev/null
> > +++ b/gcc/cumulative-args.h
> > @@ -0,0 +1,20 @@
> > +#ifndef GCC_CUMULATIVE_ARGS_H
> > +#define GCC_CUMULATIVE_ARGS_H
> > +
> > +#if CHECKING_P
> > +
> > +struct cumulative_args_t { void *magic; void *p; };
> > +
> > +#else /* !CHECKING_P */
> > +
> > +/* When using a GCC build compiler, we could use
> > +   __attribute__((transparent_union)) to get cumulative_args_t function
> > +   arguments passed like scalars where the ABI would mandate a less
> > +   efficient way of argument passing otherwise.  However, that would come
> > +   at the cost of less type-safe !CHECKING_P compilation.  */
> > +
> > +union cumulative_args_t { void *p; };
> > +
> > +#endif /* !CHECKING_P */
> > +
> > +#endif /* GCC_CUMULATIVE_ARGS_H */
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index 2aba4c70b44..7ffb2997c69 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -670,6 +670,7 @@ Objective-C and Objective-C++ Dialects}.
> >  -fverbose-asm  -fpack-struct[=@var{n}]  @gol
> >  -fleading-underscore  -ftls-model=@var{model} @gol
> >  -fstack-reuse=@var{reuse_level} @gol
> > +-fstack-use-cumulative-args @gol
> >  -ftrampolines  -ftrapv  -fwrapv @gol
> >  -fvisibility=@r{[}default@r{|}internal@r{|}hidden@r{|}protected@r{]} @gol
> >  -fstrict-volatile-bitfields  -fsync-libcalls}
> > @@ -16625,6 +16626,17 @@ the behavior of older compilers in which temporaries' stack space is
> >  not reused, the aggressive stack reuse can lead to runtime errors. This
> >  option is used to control the temporary stack reuse optimization.
> >
> > +@item -fstack-use-cumulative-args
> > +@opindex fstack_use_cumulative_args
> > +This option instructs the compiler to use the
> > +@code{cumulative_args_t}-based stack layout target hooks,
> > +@code{TARGET_FUNCTION_ARG_BOUNDARY_CA} and
> > +@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA}. If a given target does
> > +not define these hooks, the default behaviour is to fallback to using
> > +the standard non-@code{_CA} variants instead. Certain targets (such as
> > +AArch64 Darwin) require using the more advanced @code{_CA}-based
> > +hooks: For these targets this option should be enabled by default.
> > +
> >  @item -ftrapv
> >  @opindex ftrapv
> >  This option generates traps for signed overflow on addition, subtraction,
> > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> > index 78a1af1ad4d..261eaf46ef5 100644
> > --- a/gcc/doc/tm.texi
> > +++ b/gcc/doc/tm.texi
> > @@ -4322,6 +4322,16 @@ with the specified mode and type.  The default hook returns
> >  @code{PARM_BOUNDARY} for all arguments.
> >  @end deftypefn
> >
> > +@deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_BOUNDARY_CA (machine_mode @var{mode}, const_tree @var{type}, cumulative_args_t @var{ca})
> > +This is the @code{cumulative_args_t}-based version of
> > +@code{TARGET_FUNCTION_ARG_BOUNDARY}. Define this hook if you need more
> > +fine-grained control over argument alignment, e.g. depending on whether
> > +it is a named argument or not, or any other criteria that you choose to
> > +place in the @var{ca} structure.
> > +
> > +The default hook will call @code{TARGET_FUNCTION_ARG_BOUNDARY}.
> > +@end deftypefn
> > +
> >  @deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_ROUND_BOUNDARY (machine_mode @var{mode}, const_tree @var{type})
> >  Normally, the size of an argument is rounded up to @code{PARM_BOUNDARY},
> >  which is the default value for this hook.  You can define this hook to
> > @@ -4329,6 +4339,16 @@ return a different value if an argument size must be rounded to a larger
> >  value.
> >  @end deftypefn
> >
> > +@deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA (machine_mode @var{mode}, const_tree @var{type}, cumulative_args_t @var{ca})
> > +This is the @code{cumulative_args_t}-based version of
> > +@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}. Define this hook if you need more
> > +fine-grained control over argument size rounding, e.g. depending on whether
> > +it is a named argument or not, or any other criteria that you choose to
> > +place in the @var{ca} structure.
> > +
> > +The default hook will call @code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}.
> > +@end deftypefn
> > +
> >  @defmac FUNCTION_ARG_REGNO_P (@var{regno})
> >  A C expression that is nonzero if @var{regno} is the number of a hard
> >  register in which function arguments are sometimes passed.  This does
> > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> > index 4401550989e..933644dfa86 100644
> > --- a/gcc/doc/tm.texi.in
> > +++ b/gcc/doc/tm.texi.in
> > @@ -3330,8 +3330,12 @@ required.
> >
> >  @hook TARGET_FUNCTION_ARG_BOUNDARY
> >
> > +@hook TARGET_FUNCTION_ARG_BOUNDARY_CA
> > +
> >  @hook TARGET_FUNCTION_ARG_ROUND_BOUNDARY
> >
> > +@hook TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA
> > +
> >  @defmac FUNCTION_ARG_REGNO_P (@var{regno})
> >  A C expression that is nonzero if @var{regno} is the number of a hard
> >  register in which function arguments are sometimes passed.  This does
> > diff --git a/gcc/function.c b/gcc/function.c
> > index 61b3bd036b8..1ebf3e0ffe5 100644
> > --- a/gcc/function.c
> > +++ b/gcc/function.c
> > @@ -2610,7 +2610,9 @@ assign_parm_find_entry_rtl (struct assign_parm_data_all *all,
> >
> >    locate_and_pad_parm (data->arg.mode, data->arg.type, in_regs,
> >                        all->reg_parm_stack_space,
> > -                      entry_parm ? data->partial : 0, current_function_decl,
> > +                      entry_parm ? data->partial : 0,
> > +                      all->args_so_far,
> > +                      current_function_decl,
> >                        &all->stack_args_size, &data->locate);
> >
> >    /* Update parm_stack_boundary if this parameter is passed in the
> > @@ -4027,6 +4029,7 @@ gimplify_parameters (gimple_seq *cleanup)
> >  void
> >  locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs,
> >                      int reg_parm_stack_space, int partial,
> > +                    cumulative_args_t ca,
> >                      tree fndecl ATTRIBUTE_UNUSED,
> >                      struct args_size *initial_offset_ptr,
> >                      struct locate_and_pad_arg_data *locate)
> > @@ -4064,9 +4067,23 @@ locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs,
> >               ? arg_size_in_bytes (type)
> >               : size_int (GET_MODE_SIZE (passed_mode)));
> >    where_pad = targetm.calls.function_arg_padding (passed_mode, type);
> > -  boundary = targetm.calls.function_arg_boundary (passed_mode, type);
> > -  round_boundary = targetm.calls.function_arg_round_boundary (passed_mode,
> > -                                                             type);
> > +
> > +  if (flag_stack_use_cumulative_args)
> > +    {
> > +      boundary = targetm.calls.function_arg_boundary_ca (passed_mode,
> > +                                                        type,
> > +                                                        ca);
> > +      round_boundary = targetm.calls.function_arg_round_boundary_ca
> > +       (passed_mode, type, ca);
> > +    }
> > +  else
> > +    {
> > +      boundary = targetm.calls.function_arg_boundary (passed_mode,
> > +                                                     type);
> > +      round_boundary = targetm.calls.function_arg_round_boundary
> > +       (passed_mode, type);
> > +    }
> > +
> >    locate->where_pad = where_pad;
> >
> >    /* Alignment can't exceed MAX_SUPPORTED_STACK_ALIGNMENT.  */
> > diff --git a/gcc/function.h b/gcc/function.h
> > index 899430833ce..26f342caba3 100644
> > --- a/gcc/function.h
> > +++ b/gcc/function.h
> > @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3.  If not see
> >  #ifndef GCC_FUNCTION_H
> >  #define GCC_FUNCTION_H
> >
> > +#include "cumulative-args.h"
> >
> >  /* Stack of pending (incomplete) sequences saved by `start_sequence'.
> >     Each element describes one pending sequence.
> > @@ -661,6 +662,7 @@ extern int aggregate_value_p (const_tree, const_tree);
> >  extern bool use_register_for_decl (const_tree);
> >  extern gimple_seq gimplify_parameters (gimple_seq *);
> >  extern void locate_and_pad_parm (machine_mode, tree, int, int, int,
> > +                                cumulative_args_t,
> >                                  tree, struct args_size *,
> >                                  struct locate_and_pad_arg_data *);
> >  extern void generate_setjmp_warnings (void);
> > diff --git a/gcc/target.def b/gcc/target.def
> > index 51ea167172b..bb56afab539 100644
> > --- a/gcc/target.def
> > +++ b/gcc/target.def
> > @@ -4959,6 +4959,18 @@ with the specified mode and type.  The default hook returns\n\
> >   unsigned int, (machine_mode mode, const_tree type),
> >   default_function_arg_boundary)
> >
> > +DEFHOOK
> > +(function_arg_boundary_ca,
> > + "This is the @code{cumulative_args_t}-based version of\n\
> > +@code{TARGET_FUNCTION_ARG_BOUNDARY}. Define this hook if you need more\n\
> > +fine-grained control over argument alignment, e.g. depending on whether\n\
> > +it is a named argument or not, or any other criteria that you choose to\n\
> > +place in the @var{ca} structure.\n\
> > +\n\
> > +The default hook will call @code{TARGET_FUNCTION_ARG_BOUNDARY}.",
> > + unsigned int, (machine_mode mode, const_tree type, cumulative_args_t ca),
> > + default_function_arg_boundary_ca)
> > +
> >  DEFHOOK
> >  (function_arg_round_boundary,
> >   "Normally, the size of an argument is rounded up to @code{PARM_BOUNDARY},\n\
> > @@ -4968,6 +4980,18 @@ value.",
> >   unsigned int, (machine_mode mode, const_tree type),
> >   default_function_arg_round_boundary)
> >
> > +DEFHOOK
> > +(function_arg_round_boundary_ca,
> > + "This is the @code{cumulative_args_t}-based version of\n\
> > +@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}. Define this hook if you need more\n\
> > +fine-grained control over argument size rounding, e.g. depending on whether\n\
> > +it is a named argument or not, or any other criteria that you choose to\n\
> > +place in the @var{ca} structure.\n\
> > +\n\
> > +The default hook will call @code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}.",
> > + unsigned int, (machine_mode mode, const_tree type, cumulative_args_t ca),
> > + default_function_arg_round_boundary_ca)
> > +
> >  /* Return the diagnostic message string if function without a prototype
> >     is not allowed for this 'val' argument; NULL otherwise. */
> >  DEFHOOK
> > diff --git a/gcc/target.h b/gcc/target.h
> > index d8f45fb99c3..5b28ece7e1c 100644
> > --- a/gcc/target.h
> > +++ b/gcc/target.h
> > @@ -51,22 +51,7 @@
> >  #include "insn-codes.h"
> >  #include "tm.h"
> >  #include "hard-reg-set.h"
> > -
> > -#if CHECKING_P
> > -
> > -struct cumulative_args_t { void *magic; void *p; };
> > -
> > -#else /* !CHECKING_P */
> > -
> > -/* When using a GCC build compiler, we could use
> > -   __attribute__((transparent_union)) to get cumulative_args_t function
> > -   arguments passed like scalars where the ABI would mandate a less
> > -   efficient way of argument passing otherwise.  However, that would come
> > -   at the cost of less type-safe !CHECKING_P compilation.  */
> > -
> > -union cumulative_args_t { void *p; };
> > -
> > -#endif /* !CHECKING_P */
> > +#include "cumulative-args.h"
> >
> >  /* Types of memory operation understood by the "by_pieces" infrastructure.
> >     Used by the TARGET_USE_BY_PIECES_INFRASTRUCTURE_P target hook and
> > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> > index 6f071f80231..d83f362a5ae 100644
> > --- a/gcc/targhooks.c
> > +++ b/gcc/targhooks.c
> > @@ -850,6 +850,14 @@ default_function_arg_boundary (machine_mode mode ATTRIBUTE_UNUSED,
> >    return PARM_BOUNDARY;
> >  }
> >
> > +unsigned int
> > +default_function_arg_boundary_ca (machine_mode mode ATTRIBUTE_UNUSED,
> > +                                 const_tree type ATTRIBUTE_UNUSED,
> > +                                 cumulative_args_t ca ATTRIBUTE_UNUSED)
> > +{
> > +  return default_function_arg_boundary (mode, type);
> > +}
> > +
> >  unsigned int
> >  default_function_arg_round_boundary (machine_mode mode ATTRIBUTE_UNUSED,
> >                                      const_tree type ATTRIBUTE_UNUSED)
> > @@ -857,6 +865,14 @@ default_function_arg_round_boundary (machine_mode mode ATTRIBUTE_UNUSED,
> >    return PARM_BOUNDARY;
> >  }
> >
> > +unsigned int
> > +default_function_arg_round_boundary_ca (machine_mode mode ATTRIBUTE_UNUSED,
> > +                                       const_tree type ATTRIBUTE_UNUSED,
> > +                                       cumulative_args_t ca ATTRIBUTE_UNUSED)
> > +{
> > +  return default_function_arg_round_boundary (mode, type);
> > +}
> > +
> >  void
> >  hook_void_bitmap (bitmap regs ATTRIBUTE_UNUSED)
> >  {
> > diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> > index 11e9d7dd1a8..f33374ef073 100644
> > --- a/gcc/targhooks.h
> > +++ b/gcc/targhooks.h
> > @@ -154,6 +154,12 @@ extern unsigned int default_function_arg_boundary (machine_mode,
> >                                                    const_tree);
> >  extern unsigned int default_function_arg_round_boundary (machine_mode,
> >                                                          const_tree);
> > +extern unsigned int default_function_arg_boundary_ca (machine_mode,
> > +                                                     const_tree,
> > +                                                     cumulative_args_t ca);
> > +extern unsigned int default_function_arg_round_boundary_ca (machine_mode,
> > +                                                           const_tree,
> > +                                                           cumulative_args_t ca);
> >  extern bool hook_bool_const_rtx_commutative_p (const_rtx, int);
> >  extern rtx default_function_value (const_tree, const_tree, bool);
> >  extern HARD_REG_SET default_zero_call_used_regs (HARD_REG_SET);
> > --
> > 2.30.1 (Apple Git-130)
> >

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

* Re: [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends
  2021-11-22 14:40   ` Maxim Blinov
@ 2021-11-22 15:15     ` Richard Biener
  2021-12-02 21:08       ` Jeff Law
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Biener @ 2021-11-22 15:15 UTC (permalink / raw)
  To: Maxim Blinov; +Cc: GCC Patches, Iain Sandoe

On Mon, Nov 22, 2021 at 3:40 PM Maxim Blinov <maxim.blinov@embecosm.com> wrote:
>
> Hi Richard,
>
> The purpose of this patch is to give more of the target code access to
> the cumulative_args_t structure in order to enable certain (otherwise
> currently impossible) stack layout constraints, specifically for
> parameters that are passed over the stack. For example, there are some
> targets (not yet in GCC trunk) which explicitly require the
> distinction between named and variadic parameters in order to enforce
> different alignment rules (when passing over the stack.) Such a
> constraint would be accommodated by this patch.
>
> The patch itself is very straightforward and simply adds the parameter
> to the two functions which we'd like to extend. The motivation of
> defining new target hooks was to minimize the patch size.

I would prefer to adjust the existing hooks if there's currently no way to
implement the aarch64 darwin ABI instead of optimizing for patch size.

I have no opinion on whether passing down cummulative args is the
proper thing to do design-wise.  It might be that TARGET_FUNCTION_ARG_BOUNDARY
is only one part that cummulative args using code should look at
(given you don't show the other users of function_arg_boundary that do not
conveniently have a CA struct around).

Richard.

> A concrete user of this change that we have in mind is the AArch64
> Darwin parameter passing abi, which mandates that when passing on the
> stack, named parameters are to be naturally-aligned, however variadic
> ones are to be word-aligned. However this patch is completely generic
> in nature and should enable all targets to have more control over
> their parameter layout process.
>
> Best Regards,
> Maxim
>
> On Mon, 15 Nov 2021 at 07:11, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Sat, Nov 13, 2021 at 10:43 AM Maxim Blinov <maxim.blinov@embecosm.com> wrote:
> > >
> > > The two target hooks responsible for informing GCC about stack
> > > parameter alignment are `TARGET_FUNCTION_ARG_BOUNDARY` and
> > > `TARGET_FUNCTION_ARG_ROUND_BOUNDARY`, which currently only consider
> > > the tree and machine_mode of a specific given argument.
> > >
> > > Create two new target hooks suffixed with '_CA', and pass in a third
> > > `cumulative_args_t` parameter. This enables the backend to make
> > > alignment decisions based on the context of the whole function rather
> > > than individual parameters.
> > >
> > > The orignal machine_mode/tree type macros are not removed - they are
> > > called by the default implementations of `TARGET_...BOUNDARY_CA` and
> > > `TARGET_...ROUND_BOUNDARY_CA`. This is done with the intetnion of
> > > avoiding large mechanical modifications of nearly every backend in
> > > GCC. There is also a new flag, -fstack-use-cumulative-args, which
> > > provides a way to completely bypass the new `..._CA` macros. This
> > > feature is intended for debugging GCC itself.
> >
> > Just two quick comments without looking at the patch.
> >
> > Please do not introduce options in the user namespace -f... which are
> > for debugging only.  I think you should go without this part instead.
> >
> > Second, you fail to motivate the change.  I cannot make sense of
> > "This enables the backend to make alignment decisions based on the
> > context of the whole function rather than individual parameters."
> >
> > Richard.
> >
> > >
> > > gcc/ChangeLog:
> > >
> > >         * calls.c (initialize_argument_information): Pass `args_so_far`.
> > >         * common.opt: New flag `-fstack-use-cumulative-args`.
> > >         * config.gcc: No platforms currently use ..._CA-hooks: Set
> > >         -fstack-use-cumulative-args to be off by default.
> > >         * target.h (cumulative_args_t): Move declaration from here, to...
> > >         * cumulative-args.h (cumulative_args_t): ...this new file. This is
> > >         to permit backends to include the declaration of cumulative_args_t
> > >         without dragging in circular dependencies.
> > >         * function.c (assign_parm_find_entry_rtl): Provide
> > >         cumulative_args_t to locate_and_pad_parm.
> > >         (gimplify_parameters): Ditto.
> > >         (locate_and_pad_parm): Conditionally call new hooks if we're
> > >         invoked with -fstack-use-cumulative-args.
> > >         * function.h: Include cumulative-args.h.
> > >         (locate_and_pad_parm): Add cumulative_args_t parameter.
> > >         * target.def (function_arg_boundary_ca): Add.
> > >         (function_arg_round_boundary_ca): Ditto.
> > >         * targhooks.c (default_function_arg_boundary_ca): Implement.
> > >         (default_function_arg_round_boundary_ca): Ditto.
> > >         * targhooks.h (default_function_arg_boundary_ca): Declare.
> > >         (default_function_arg_round_boundary_ca): Ditto.
> > >         * doc/invoke.texi (-fstack-use-cumulative-args): Document.
> > >         * doc/tm.texi: Regenerate.
> > >         * doc/tm.texi.in: Ditto.
> > > ---
> > >  gcc/calls.c           |  3 +++
> > >  gcc/common.opt        |  4 ++++
> > >  gcc/config.gcc        |  7 +++++++
> > >  gcc/cumulative-args.h | 20 ++++++++++++++++++++
> > >  gcc/doc/invoke.texi   | 12 ++++++++++++
> > >  gcc/doc/tm.texi       | 20 ++++++++++++++++++++
> > >  gcc/doc/tm.texi.in    |  4 ++++
> > >  gcc/function.c        | 25 +++++++++++++++++++++----
> > >  gcc/function.h        |  2 ++
> > >  gcc/target.def        | 24 ++++++++++++++++++++++++
> > >  gcc/target.h          | 17 +----------------
> > >  gcc/targhooks.c       | 16 ++++++++++++++++
> > >  gcc/targhooks.h       |  6 ++++++
> > >  13 files changed, 140 insertions(+), 20 deletions(-)
> > >  create mode 100644 gcc/cumulative-args.h
> > >
> > > diff --git a/gcc/calls.c b/gcc/calls.c
> > > index 27b59f26ad3..cef612a6ef4 100644
> > > --- a/gcc/calls.c
> > > +++ b/gcc/calls.c
> > > @@ -1527,6 +1527,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
> > >  #endif
> > >                              reg_parm_stack_space,
> > >                              args[i].pass_on_stack ? 0 : args[i].partial,
> > > +                            args_so_far,
> > >                              fndecl, args_size, &args[i].locate);
> > >  #ifdef BLOCK_REG_PADDING
> > >        else
> > > @@ -4205,6 +4206,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
> > >                            argvec[count].reg != 0,
> > >  #endif
> > >                            reg_parm_stack_space, 0,
> > > +                          args_so_far,
> > >                            NULL_TREE, &args_size, &argvec[count].locate);
> > >
> > >        if (argvec[count].reg == 0 || argvec[count].partial != 0
> > > @@ -4296,6 +4298,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
> > >                                argvec[count].reg != 0,
> > >  #endif
> > >                                reg_parm_stack_space, argvec[count].partial,
> > > +                              args_so_far,
> > >                                NULL_TREE, &args_size, &argvec[count].locate);
> > >           args_size.constant += argvec[count].locate.size.constant;
> > >           gcc_assert (!argvec[count].locate.size.var);
> > > diff --git a/gcc/common.opt b/gcc/common.opt
> > > index de9b848eda5..982417c1e39 100644
> > > --- a/gcc/common.opt
> > > +++ b/gcc/common.opt
> > > @@ -2705,6 +2705,10 @@ fstack-usage
> > >  Common RejectNegative Var(flag_stack_usage)
> > >  Output stack usage information on a per-function basis.
> > >
> > > +fstack-use-cumulative-args
> > > +Common RejectNegative Var(flag_stack_use_cumulative_args) Init(STACK_USE_CUMULATIVE_ARGS_INIT)
> > > +Use cumulative args-based stack layout hooks.
> > > +
> > >  fstrength-reduce
> > >  Common Ignore
> > >  Does nothing.  Preserved for backward compatibility.
> > > diff --git a/gcc/config.gcc b/gcc/config.gcc
> > > index edd12655c4a..046d691af56 100644
> > > --- a/gcc/config.gcc
> > > +++ b/gcc/config.gcc
> > > @@ -1070,6 +1070,13 @@ case ${target} in
> > >    ;;
> > >  esac
> > >
> > > +# Figure out if we need to enable -foff-stack-trampolines by default.
> > > +case ${target} in
> > > +*)
> > > +  tm_defines="$tm_defines STACK_USE_CUMULATIVE_ARGS_INIT=0"
> > > +  ;;
> > > +esac
> > > +
> > >  case ${target} in
> > >  aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-rtems*)
> > >         tm_file="${tm_file} dbxelf.h elfos.h newlib-stdint.h"
> > > diff --git a/gcc/cumulative-args.h b/gcc/cumulative-args.h
> > > new file mode 100644
> > > index 00000000000..b60928e37f9
> > > --- /dev/null
> > > +++ b/gcc/cumulative-args.h
> > > @@ -0,0 +1,20 @@
> > > +#ifndef GCC_CUMULATIVE_ARGS_H
> > > +#define GCC_CUMULATIVE_ARGS_H
> > > +
> > > +#if CHECKING_P
> > > +
> > > +struct cumulative_args_t { void *magic; void *p; };
> > > +
> > > +#else /* !CHECKING_P */
> > > +
> > > +/* When using a GCC build compiler, we could use
> > > +   __attribute__((transparent_union)) to get cumulative_args_t function
> > > +   arguments passed like scalars where the ABI would mandate a less
> > > +   efficient way of argument passing otherwise.  However, that would come
> > > +   at the cost of less type-safe !CHECKING_P compilation.  */
> > > +
> > > +union cumulative_args_t { void *p; };
> > > +
> > > +#endif /* !CHECKING_P */
> > > +
> > > +#endif /* GCC_CUMULATIVE_ARGS_H */
> > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > index 2aba4c70b44..7ffb2997c69 100644
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -670,6 +670,7 @@ Objective-C and Objective-C++ Dialects}.
> > >  -fverbose-asm  -fpack-struct[=@var{n}]  @gol
> > >  -fleading-underscore  -ftls-model=@var{model} @gol
> > >  -fstack-reuse=@var{reuse_level} @gol
> > > +-fstack-use-cumulative-args @gol
> > >  -ftrampolines  -ftrapv  -fwrapv @gol
> > >  -fvisibility=@r{[}default@r{|}internal@r{|}hidden@r{|}protected@r{]} @gol
> > >  -fstrict-volatile-bitfields  -fsync-libcalls}
> > > @@ -16625,6 +16626,17 @@ the behavior of older compilers in which temporaries' stack space is
> > >  not reused, the aggressive stack reuse can lead to runtime errors. This
> > >  option is used to control the temporary stack reuse optimization.
> > >
> > > +@item -fstack-use-cumulative-args
> > > +@opindex fstack_use_cumulative_args
> > > +This option instructs the compiler to use the
> > > +@code{cumulative_args_t}-based stack layout target hooks,
> > > +@code{TARGET_FUNCTION_ARG_BOUNDARY_CA} and
> > > +@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA}. If a given target does
> > > +not define these hooks, the default behaviour is to fallback to using
> > > +the standard non-@code{_CA} variants instead. Certain targets (such as
> > > +AArch64 Darwin) require using the more advanced @code{_CA}-based
> > > +hooks: For these targets this option should be enabled by default.
> > > +
> > >  @item -ftrapv
> > >  @opindex ftrapv
> > >  This option generates traps for signed overflow on addition, subtraction,
> > > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> > > index 78a1af1ad4d..261eaf46ef5 100644
> > > --- a/gcc/doc/tm.texi
> > > +++ b/gcc/doc/tm.texi
> > > @@ -4322,6 +4322,16 @@ with the specified mode and type.  The default hook returns
> > >  @code{PARM_BOUNDARY} for all arguments.
> > >  @end deftypefn
> > >
> > > +@deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_BOUNDARY_CA (machine_mode @var{mode}, const_tree @var{type}, cumulative_args_t @var{ca})
> > > +This is the @code{cumulative_args_t}-based version of
> > > +@code{TARGET_FUNCTION_ARG_BOUNDARY}. Define this hook if you need more
> > > +fine-grained control over argument alignment, e.g. depending on whether
> > > +it is a named argument or not, or any other criteria that you choose to
> > > +place in the @var{ca} structure.
> > > +
> > > +The default hook will call @code{TARGET_FUNCTION_ARG_BOUNDARY}.
> > > +@end deftypefn
> > > +
> > >  @deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_ROUND_BOUNDARY (machine_mode @var{mode}, const_tree @var{type})
> > >  Normally, the size of an argument is rounded up to @code{PARM_BOUNDARY},
> > >  which is the default value for this hook.  You can define this hook to
> > > @@ -4329,6 +4339,16 @@ return a different value if an argument size must be rounded to a larger
> > >  value.
> > >  @end deftypefn
> > >
> > > +@deftypefn {Target Hook} {unsigned int} TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA (machine_mode @var{mode}, const_tree @var{type}, cumulative_args_t @var{ca})
> > > +This is the @code{cumulative_args_t}-based version of
> > > +@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}. Define this hook if you need more
> > > +fine-grained control over argument size rounding, e.g. depending on whether
> > > +it is a named argument or not, or any other criteria that you choose to
> > > +place in the @var{ca} structure.
> > > +
> > > +The default hook will call @code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}.
> > > +@end deftypefn
> > > +
> > >  @defmac FUNCTION_ARG_REGNO_P (@var{regno})
> > >  A C expression that is nonzero if @var{regno} is the number of a hard
> > >  register in which function arguments are sometimes passed.  This does
> > > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> > > index 4401550989e..933644dfa86 100644
> > > --- a/gcc/doc/tm.texi.in
> > > +++ b/gcc/doc/tm.texi.in
> > > @@ -3330,8 +3330,12 @@ required.
> > >
> > >  @hook TARGET_FUNCTION_ARG_BOUNDARY
> > >
> > > +@hook TARGET_FUNCTION_ARG_BOUNDARY_CA
> > > +
> > >  @hook TARGET_FUNCTION_ARG_ROUND_BOUNDARY
> > >
> > > +@hook TARGET_FUNCTION_ARG_ROUND_BOUNDARY_CA
> > > +
> > >  @defmac FUNCTION_ARG_REGNO_P (@var{regno})
> > >  A C expression that is nonzero if @var{regno} is the number of a hard
> > >  register in which function arguments are sometimes passed.  This does
> > > diff --git a/gcc/function.c b/gcc/function.c
> > > index 61b3bd036b8..1ebf3e0ffe5 100644
> > > --- a/gcc/function.c
> > > +++ b/gcc/function.c
> > > @@ -2610,7 +2610,9 @@ assign_parm_find_entry_rtl (struct assign_parm_data_all *all,
> > >
> > >    locate_and_pad_parm (data->arg.mode, data->arg.type, in_regs,
> > >                        all->reg_parm_stack_space,
> > > -                      entry_parm ? data->partial : 0, current_function_decl,
> > > +                      entry_parm ? data->partial : 0,
> > > +                      all->args_so_far,
> > > +                      current_function_decl,
> > >                        &all->stack_args_size, &data->locate);
> > >
> > >    /* Update parm_stack_boundary if this parameter is passed in the
> > > @@ -4027,6 +4029,7 @@ gimplify_parameters (gimple_seq *cleanup)
> > >  void
> > >  locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs,
> > >                      int reg_parm_stack_space, int partial,
> > > +                    cumulative_args_t ca,
> > >                      tree fndecl ATTRIBUTE_UNUSED,
> > >                      struct args_size *initial_offset_ptr,
> > >                      struct locate_and_pad_arg_data *locate)
> > > @@ -4064,9 +4067,23 @@ locate_and_pad_parm (machine_mode passed_mode, tree type, int in_regs,
> > >               ? arg_size_in_bytes (type)
> > >               : size_int (GET_MODE_SIZE (passed_mode)));
> > >    where_pad = targetm.calls.function_arg_padding (passed_mode, type);
> > > -  boundary = targetm.calls.function_arg_boundary (passed_mode, type);
> > > -  round_boundary = targetm.calls.function_arg_round_boundary (passed_mode,
> > > -                                                             type);
> > > +
> > > +  if (flag_stack_use_cumulative_args)
> > > +    {
> > > +      boundary = targetm.calls.function_arg_boundary_ca (passed_mode,
> > > +                                                        type,
> > > +                                                        ca);
> > > +      round_boundary = targetm.calls.function_arg_round_boundary_ca
> > > +       (passed_mode, type, ca);
> > > +    }
> > > +  else
> > > +    {
> > > +      boundary = targetm.calls.function_arg_boundary (passed_mode,
> > > +                                                     type);
> > > +      round_boundary = targetm.calls.function_arg_round_boundary
> > > +       (passed_mode, type);
> > > +    }
> > > +
> > >    locate->where_pad = where_pad;
> > >
> > >    /* Alignment can't exceed MAX_SUPPORTED_STACK_ALIGNMENT.  */
> > > diff --git a/gcc/function.h b/gcc/function.h
> > > index 899430833ce..26f342caba3 100644
> > > --- a/gcc/function.h
> > > +++ b/gcc/function.h
> > > @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3.  If not see
> > >  #ifndef GCC_FUNCTION_H
> > >  #define GCC_FUNCTION_H
> > >
> > > +#include "cumulative-args.h"
> > >
> > >  /* Stack of pending (incomplete) sequences saved by `start_sequence'.
> > >     Each element describes one pending sequence.
> > > @@ -661,6 +662,7 @@ extern int aggregate_value_p (const_tree, const_tree);
> > >  extern bool use_register_for_decl (const_tree);
> > >  extern gimple_seq gimplify_parameters (gimple_seq *);
> > >  extern void locate_and_pad_parm (machine_mode, tree, int, int, int,
> > > +                                cumulative_args_t,
> > >                                  tree, struct args_size *,
> > >                                  struct locate_and_pad_arg_data *);
> > >  extern void generate_setjmp_warnings (void);
> > > diff --git a/gcc/target.def b/gcc/target.def
> > > index 51ea167172b..bb56afab539 100644
> > > --- a/gcc/target.def
> > > +++ b/gcc/target.def
> > > @@ -4959,6 +4959,18 @@ with the specified mode and type.  The default hook returns\n\
> > >   unsigned int, (machine_mode mode, const_tree type),
> > >   default_function_arg_boundary)
> > >
> > > +DEFHOOK
> > > +(function_arg_boundary_ca,
> > > + "This is the @code{cumulative_args_t}-based version of\n\
> > > +@code{TARGET_FUNCTION_ARG_BOUNDARY}. Define this hook if you need more\n\
> > > +fine-grained control over argument alignment, e.g. depending on whether\n\
> > > +it is a named argument or not, or any other criteria that you choose to\n\
> > > +place in the @var{ca} structure.\n\
> > > +\n\
> > > +The default hook will call @code{TARGET_FUNCTION_ARG_BOUNDARY}.",
> > > + unsigned int, (machine_mode mode, const_tree type, cumulative_args_t ca),
> > > + default_function_arg_boundary_ca)
> > > +
> > >  DEFHOOK
> > >  (function_arg_round_boundary,
> > >   "Normally, the size of an argument is rounded up to @code{PARM_BOUNDARY},\n\
> > > @@ -4968,6 +4980,18 @@ value.",
> > >   unsigned int, (machine_mode mode, const_tree type),
> > >   default_function_arg_round_boundary)
> > >
> > > +DEFHOOK
> > > +(function_arg_round_boundary_ca,
> > > + "This is the @code{cumulative_args_t}-based version of\n\
> > > +@code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}. Define this hook if you need more\n\
> > > +fine-grained control over argument size rounding, e.g. depending on whether\n\
> > > +it is a named argument or not, or any other criteria that you choose to\n\
> > > +place in the @var{ca} structure.\n\
> > > +\n\
> > > +The default hook will call @code{TARGET_FUNCTION_ARG_ROUND_BOUNDARY}.",
> > > + unsigned int, (machine_mode mode, const_tree type, cumulative_args_t ca),
> > > + default_function_arg_round_boundary_ca)
> > > +
> > >  /* Return the diagnostic message string if function without a prototype
> > >     is not allowed for this 'val' argument; NULL otherwise. */
> > >  DEFHOOK
> > > diff --git a/gcc/target.h b/gcc/target.h
> > > index d8f45fb99c3..5b28ece7e1c 100644
> > > --- a/gcc/target.h
> > > +++ b/gcc/target.h
> > > @@ -51,22 +51,7 @@
> > >  #include "insn-codes.h"
> > >  #include "tm.h"
> > >  #include "hard-reg-set.h"
> > > -
> > > -#if CHECKING_P
> > > -
> > > -struct cumulative_args_t { void *magic; void *p; };
> > > -
> > > -#else /* !CHECKING_P */
> > > -
> > > -/* When using a GCC build compiler, we could use
> > > -   __attribute__((transparent_union)) to get cumulative_args_t function
> > > -   arguments passed like scalars where the ABI would mandate a less
> > > -   efficient way of argument passing otherwise.  However, that would come
> > > -   at the cost of less type-safe !CHECKING_P compilation.  */
> > > -
> > > -union cumulative_args_t { void *p; };
> > > -
> > > -#endif /* !CHECKING_P */
> > > +#include "cumulative-args.h"
> > >
> > >  /* Types of memory operation understood by the "by_pieces" infrastructure.
> > >     Used by the TARGET_USE_BY_PIECES_INFRASTRUCTURE_P target hook and
> > > diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> > > index 6f071f80231..d83f362a5ae 100644
> > > --- a/gcc/targhooks.c
> > > +++ b/gcc/targhooks.c
> > > @@ -850,6 +850,14 @@ default_function_arg_boundary (machine_mode mode ATTRIBUTE_UNUSED,
> > >    return PARM_BOUNDARY;
> > >  }
> > >
> > > +unsigned int
> > > +default_function_arg_boundary_ca (machine_mode mode ATTRIBUTE_UNUSED,
> > > +                                 const_tree type ATTRIBUTE_UNUSED,
> > > +                                 cumulative_args_t ca ATTRIBUTE_UNUSED)
> > > +{
> > > +  return default_function_arg_boundary (mode, type);
> > > +}
> > > +
> > >  unsigned int
> > >  default_function_arg_round_boundary (machine_mode mode ATTRIBUTE_UNUSED,
> > >                                      const_tree type ATTRIBUTE_UNUSED)
> > > @@ -857,6 +865,14 @@ default_function_arg_round_boundary (machine_mode mode ATTRIBUTE_UNUSED,
> > >    return PARM_BOUNDARY;
> > >  }
> > >
> > > +unsigned int
> > > +default_function_arg_round_boundary_ca (machine_mode mode ATTRIBUTE_UNUSED,
> > > +                                       const_tree type ATTRIBUTE_UNUSED,
> > > +                                       cumulative_args_t ca ATTRIBUTE_UNUSED)
> > > +{
> > > +  return default_function_arg_round_boundary (mode, type);
> > > +}
> > > +
> > >  void
> > >  hook_void_bitmap (bitmap regs ATTRIBUTE_UNUSED)
> > >  {
> > > diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> > > index 11e9d7dd1a8..f33374ef073 100644
> > > --- a/gcc/targhooks.h
> > > +++ b/gcc/targhooks.h
> > > @@ -154,6 +154,12 @@ extern unsigned int default_function_arg_boundary (machine_mode,
> > >                                                    const_tree);
> > >  extern unsigned int default_function_arg_round_boundary (machine_mode,
> > >                                                          const_tree);
> > > +extern unsigned int default_function_arg_boundary_ca (machine_mode,
> > > +                                                     const_tree,
> > > +                                                     cumulative_args_t ca);
> > > +extern unsigned int default_function_arg_round_boundary_ca (machine_mode,
> > > +                                                           const_tree,
> > > +                                                           cumulative_args_t ca);
> > >  extern bool hook_bool_const_rtx_commutative_p (const_rtx, int);
> > >  extern rtx default_function_value (const_tree, const_tree, bool);
> > >  extern HARD_REG_SET default_zero_call_used_regs (HARD_REG_SET);
> > > --
> > > 2.30.1 (Apple Git-130)
> > >

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

* Re: [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends
  2021-11-22 15:15     ` Richard Biener
@ 2021-12-02 21:08       ` Jeff Law
  2021-12-02 21:24         ` Iain Sandoe
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2021-12-02 21:08 UTC (permalink / raw)
  To: Richard Biener, Maxim Blinov; +Cc: Iain Sandoe, GCC Patches



On 11/22/2021 8:15 AM, Richard Biener via Gcc-patches wrote:
> On Mon, Nov 22, 2021 at 3:40 PM Maxim Blinov <maxim.blinov@embecosm.com> wrote:
>> Hi Richard,
>>
>> The purpose of this patch is to give more of the target code access to
>> the cumulative_args_t structure in order to enable certain (otherwise
>> currently impossible) stack layout constraints, specifically for
>> parameters that are passed over the stack. For example, there are some
>> targets (not yet in GCC trunk) which explicitly require the
>> distinction between named and variadic parameters in order to enforce
>> different alignment rules (when passing over the stack.) Such a
>> constraint would be accommodated by this patch.
>>
>> The patch itself is very straightforward and simply adds the parameter
>> to the two functions which we'd like to extend. The motivation of
>> defining new target hooks was to minimize the patch size.
> I would prefer to adjust the existing hooks if there's currently no way to
> implement the aarch64 darwin ABI instead of optimizing for patch size.
Agreed.  Additionally I don't think we want any -f options controlling 
this behavior.


>
> I have no opinion on whether passing down cummulative args is the
> proper thing to do design-wise.  It might be that TARGET_FUNCTION_ARG_BOUNDARY
> is only one part that cummulative args using code should look at
> (given you don't show the other users of function_arg_boundary that do not
> conveniently have a CA struct around).
In the past I think we passed some additional parameters indicated where 
the last named parameter was so that it could be used to set up the CA 
struct.  But if the aarch64 darwin ABI needs that info somewhere we 
didn't before, then we'd likely need to add the CA structure.


Jeff

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

* Re: [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends
  2021-12-02 21:08       ` Jeff Law
@ 2021-12-02 21:24         ` Iain Sandoe
  2021-12-02 21:37           ` Jeff Law
  0 siblings, 1 reply; 8+ messages in thread
From: Iain Sandoe @ 2021-12-02 21:24 UTC (permalink / raw)
  To: Jeff Law; +Cc: Richard Biener, Maxim Blinov, GCC Patches



> On 2 Dec 2021, at 21:08, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> On 11/22/2021 8:15 AM, Richard Biener via Gcc-patches wrote:
>> On Mon, Nov 22, 2021 at 3:40 PM Maxim Blinov <maxim.blinov@embecosm.com> wrote:
>>> Hi Richard,
>>> 
>>> The purpose of this patch is to give more of the target code access to
>>> the cumulative_args_t structure in order to enable certain (otherwise
>>> currently impossible) stack layout constraints, specifically for
>>> parameters that are passed over the stack. For example, there are some
>>> targets (not yet in GCC trunk) which explicitly require the
>>> distinction between named and variadic parameters in order to enforce
>>> different alignment rules (when passing over the stack.) Such a
>>> constraint would be accommodated by this patch.
>>> 
>>> The patch itself is very straightforward and simply adds the parameter
>>> to the two functions which we'd like to extend. The motivation of
>>> defining new target hooks was to minimize the patch size.
>> I would prefer to adjust the existing hooks if there's currently no way to
>> implement the aarch64 darwin ABI instead of optimizing for patch size.
> Agreed.  Additionally I don't think we want any -f options controlling this behavior.

Perhaps the choice of expression was not ideal - we were trying to minimize the
invasiveness of the change (my steer to Maxim, so mea culpa there).

The concern was that re-using existing hooks would touch every target, including
all those we have no way to test.

I wonder if C++ will allow us to have a default NULL argument to the hook
so that it’s a NOP change unless a target chooses to populate that arg.
(I guess an overload will not work since we are populating a fn pointer table).

>> I have no opinion on whether passing down cummulative args is the
>> proper thing to do design-wise.  It might be that TARGET_FUNCTION_ARG_BOUNDARY
>> is only one part that cummulative args using code should look at
>> (given you don't show the other users of function_arg_boundary that do not
>> conveniently have a CA struct around).
> In the past I think we passed some additional parameters indicated where the last named parameter was so that it could be used to set up the CA struct.  But if the aarch64 darwin ABI needs that info somewhere we didn't before, then we'd likely need to add the CA structure.

yeah, the problem is in knowing that we need to switch from natural alignment of
parms (so that parm N+1 is only padded sufficiently to be naturally aligned)
 to word boundary.

We’ll figure out a revised patch as soon as time permits.

Iain

> 
> 
> Jeff


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

* Re: [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends
  2021-12-02 21:24         ` Iain Sandoe
@ 2021-12-02 21:37           ` Jeff Law
  0 siblings, 0 replies; 8+ messages in thread
From: Jeff Law @ 2021-12-02 21:37 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: Richard Biener, Maxim Blinov, GCC Patches



On 12/2/2021 2:24 PM, Iain Sandoe wrote:
>
>> On 2 Dec 2021, at 21:08, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> On 11/22/2021 8:15 AM, Richard Biener via Gcc-patches wrote:
>>> On Mon, Nov 22, 2021 at 3:40 PM Maxim Blinov <maxim.blinov@embecosm.com> wrote:
>>>> Hi Richard,
>>>>
>>>> The purpose of this patch is to give more of the target code access to
>>>> the cumulative_args_t structure in order to enable certain (otherwise
>>>> currently impossible) stack layout constraints, specifically for
>>>> parameters that are passed over the stack. For example, there are some
>>>> targets (not yet in GCC trunk) which explicitly require the
>>>> distinction between named and variadic parameters in order to enforce
>>>> different alignment rules (when passing over the stack.) Such a
>>>> constraint would be accommodated by this patch.
>>>>
>>>> The patch itself is very straightforward and simply adds the parameter
>>>> to the two functions which we'd like to extend. The motivation of
>>>> defining new target hooks was to minimize the patch size.
>>> I would prefer to adjust the existing hooks if there's currently no way to
>>> implement the aarch64 darwin ABI instead of optimizing for patch size.
>> Agreed.  Additionally I don't think we want any -f options controlling this behavior.
> Perhaps the choice of expression was not ideal - we were trying to minimize the
> invasiveness of the change (my steer to Maxim, so mea culpa there).
No worries.  I was on your side of this problem eons ago, ironically in 
a closely related chunk of code.    I don't remember all the details 
anymore.  But we needed to pass around an additional argument to either 
FUNCTION_ARG or INIT_CUMULATIVE_ARGS for the PA to fix an obscure issue 
with passing FP values to static varargs/stdarg functions.



>
> The concern was that re-using existing hooks would touch every target, including
> all those we have no way to test.
True, but we actually do test most targets these days :-)  And if we're 
just adding an argument to the API for those functions, if we get it 
wrong, it should be fairly obvious.

>
> I wonder if C++ will allow us to have a default NULL argument to the hook
> so that it’s a NOP change unless a target chooses to populate that arg.
> (I guess an overload will not work since we are populating a fn pointer table).
>
>>> I have no opinion on whether passing down cummulative args is the
>>> proper thing to do design-wise.  It might be that TARGET_FUNCTION_ARG_BOUNDARY
>>> is only one part that cummulative args using code should look at
>>> (given you don't show the other users of function_arg_boundary that do not
>>> conveniently have a CA struct around).
>> In the past I think we passed some additional parameters indicated where the last named parameter was so that it could be used to set up the CA struct.  But if the aarch64 darwin ABI needs that info somewhere we didn't before, then we'd likely need to add the CA structure.
> yeah, the problem is in knowing that we need to switch from natural alignment of
> parms (so that parm N+1 is only padded sufficiently to be naturally aligned)
>   to word boundary.
>
> We’ll figure out a revised patch as soon as time permits.
Sounds good.

jeff


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

end of thread, other threads:[~2021-12-02 21:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-13  9:42 [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends Maxim Blinov
2021-11-13  9:42 ` [PATCH 2/2] Implement TARGET_..._CA target hooks for AArch64 Darwin Maxim Blinov
2021-11-15  7:11 ` [PATCH 1/2] Add cumulative_args_t variants of TARGET_FUNCTION_ROUND_BOUNDARY and friends Richard Biener
2021-11-22 14:40   ` Maxim Blinov
2021-11-22 15:15     ` Richard Biener
2021-12-02 21:08       ` Jeff Law
2021-12-02 21:24         ` Iain Sandoe
2021-12-02 21:37           ` Jeff Law

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).