[3/4,Aarch64] v2: Implement Aarch64 SIMD ABI

This is a patch 3 to support the Aarch64 SIMD ABI [1] in GCC.

It defines a new target hook targetm.remove_extra_call_preserved_regs
that takes a rtx_insn and will remove registers from the register
set passed in if we know that this call preserves those registers.
Aarch64 SIMD functions preserve some registers that normal functions
do not.  The default version of this function will do nothing.

Steve Ellcey
sellcey@cavium.com


2018-11-08  Steve Ellcey  <sellcey@cavium.com>

	* config/aarch64/aarch64.c (aarch64_simd_call_p): New function.
	(aarch64_remove_extra_call_preserved_regs): New function.
	(TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): New macro.
	* doc/tm.texi.in (TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): New hook.
	* final.c (get_call_reg_set_usage): Call new hook.
	* target.def (remove_extra_call_preserved_regs): New hook.
	* targhooks.c (default_remove_extra_call_preserved_regs): New function.

LGTM, just minor stuff.

Steve Ellcey <sellcey@cavium.com> writes:
> +/* Return true if the instruction is a call to a SIMD function, false
> +   if it is not a SIMD function or if we do not know anything about
> +   the function.  */
> +
> +static bool
> +aarch64_simd_call_p (rtx_insn *insn)
> +{
> +  rtx symbol;
> +  rtx call;
> +  tree fndecl;
> +
> +  if (!insn)
> +    return false;

Better to arrange it so that the hook never sees null insns, since there's
nothing the hook can do in that case.  The global sets should be correct
when no other information is available.

> +/* Possibly remove some registers from register set if we know they
> +   are preserved by this call, even though they are marked as not
> +   being callee saved in CALL_USED_REGISTERS.  */
> +
> +void
> +aarch64_remove_extra_call_preserved_regs (rtx_insn *insn,
> +					  HARD_REG_SET *return_set)

s/from register set/from RETURN_SET/.  But it would be better to avoid
duplicating the description of the hook so much.  Maybe:

/* Implement TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS.  If INSN calls
   a function that uses the SIMD ABI, take advantage of the extra
   call-preserved registers that the ABI provides.  */

> +{
> +  int regno;
> +
> +  if (aarch64_simd_call_p (insn))
> +    {
> +      for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
> +	if (FP_SIMD_SAVED_REGNUM_P (regno))
> +	  CLEAR_HARD_REG_BIT (*return_set, regno);
> +    }

Might as well use:

    for (int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)

> diff --git a/gcc/target.def b/gcc/target.def
> index 4b166d1..25be927 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -5757,6 +5757,12 @@ for targets that don't have partly call-clobbered registers.",
>   bool, (unsigned int regno, machine_mode mode),
>   hook_bool_uint_mode_false)
>  
> +DEFHOOK
> +(remove_extra_call_preserved_regs,
> + "This hook removes some registers from the callee used register set.",

Think a bit more detail would be useful here.  E.g.:

 "This hook removes registers from the set of call-clobbered registers\n\
in @var{used_regs} if, contrary to the default rules, something guarantees\n\
that @samp{insn} preserves those registers.  For example, some targets\n\
support variant ABIs in which functions preserve more registers than\n\
normal functions would.  Removing those extra registers from @var{used_regs}\n\
can lead to better register allocation.\n\
\n\
The default implementation does nothing, which is always safe.\n\
Defining the hook is purely an optimization."

> + void, (rtx_insn *insn, HARD_REG_SET *used_regs),
> + default_remove_extra_call_preserved_regs)

You need to declare this in targhooks.h.  Please sanity-test on
something like x86 that doesn't override the hook.

> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index 3d8b3b9..a9fb101 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -2372,4 +2372,11 @@ default_speculation_safe_value (machine_mode mode ATTRIBUTE_UNUSED,
>    return result;
>  }
>  
> +void
> +default_remove_extra_call_preserved_regs (rtx_insn *insn ATTRIBUTE_UNUSED,
> +					  HARD_REG_SET *used_regs
> +						ATTRIBUTE_UNUSED)
> +{
> +}

Seems easier to leave out the parameter names and drop the ATTRIBUTE_UNUSED.
The formatting wouldn't be as awkward that way.

Thanks,
Richard

Here is an update of this patch.  I believe I addressed all of the
issues you raised.  Thanks for the new target.def description, it
seems much clearer than mine.  I did a full build and test on x86 as
well as aarch64 to make sure that architectures that do not define
TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS build correctly.

Steve Ellcey
sellcey@marvell.com


2019-01-02  Steve Ellcey  <sellcey@marvell.com>

	* config/aarch64/aarch64.c (aarch64_simd_call_p): New function.
	(aarch64_remove_extra_call_preserved_regs): New function.
	(TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): New macro.
	* doc/tm.texi.in (TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): New hook.
	* final.c (get_call_reg_set_usage): Call new hook.
	* target.def (remove_extra_call_preserved_regs): New hook.
	* targhooks.c (default_remove_extra_call_preserved_regs): New function.
	* targhooks.h (default_remove_extra_call_preserved_regs): New function.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index c5036c8..c916689 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1565,6 +1565,45 @@ aarch64_reg_save_mode (tree fndecl, unsigned regno)
 	   : (aarch64_simd_decl_p (fndecl) ? E_TFmode : E_DFmode);
 }
 
+/* Return true if the instruction is a call to a SIMD function, false
+   if it is not a SIMD function or if we do not know anything about
+   the function.  */
+
+static bool
+aarch64_simd_call_p (rtx_insn *insn)
+{
+  rtx symbol;
+  rtx call;
+  tree fndecl;
+
+  gcc_assert (CALL_P (insn));
+  call = get_call_rtx_from (insn);
+  symbol = XEXP (XEXP (call, 0), 0);
+  if (GET_CODE (symbol) != SYMBOL_REF)
+    return false;
+  fndecl = SYMBOL_REF_DECL (symbol);
+  if (!fndecl)
+    return false;
+
+  return aarch64_simd_decl_p (fndecl);
+}
+
+/* Implement TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS.  If INSN calls
+   a function that uses the SIMD ABI, take advantage of the extra
+   call-preserved registers that the ABI provides.  */
+
+void
+aarch64_remove_extra_call_preserved_regs (rtx_insn *insn,
+					  HARD_REG_SET *return_set)
+{
+  if (aarch64_simd_call_p (insn))
+    {
+      for (int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+	if (FP_SIMD_SAVED_REGNUM_P (regno))
+	  CLEAR_HARD_REG_BIT (*return_set, regno);
+    }
+}
+
 /* Implement TARGET_HARD_REGNO_CALL_PART_CLOBBERED.  The callee only saves
    the lower 64 bits of a 128-bit register.  Tell the compiler the callee
    clobbers the top 64 bits when restoring the bottom 64 bits.  */
@@ -18524,6 +18563,10 @@ aarch64_libgcc_floating_mode_supported_p
 #define TARGET_HARD_REGNO_CALL_PART_CLOBBERED \
   aarch64_hard_regno_call_part_clobbered
 
+#undef TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS
+#define TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS \
+  aarch64_remove_extra_call_preserved_regs
+
 #undef TARGET_CONSTANT_ALIGNMENT
 #define TARGET_CONSTANT_ALIGNMENT aarch64_constant_alignment
 
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 976a700..d9f40a1 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -1707,6 +1707,8 @@ of @code{CALL_USED_REGISTERS}.
 @cindex call-saved register
 @hook TARGET_HARD_REGNO_CALL_PART_CLOBBERED
 
+@hook TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS
+
 @findex fixed_regs
 @findex call_used_regs
 @findex global_regs
diff --git a/gcc/final.c b/gcc/final.c
index 6dc1cd1..f6edd6a 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -5095,7 +5095,7 @@ get_call_reg_set_usage (rtx_insn *insn, HARD_REG_SET *reg_set,
 	  return true;
 	}
     }
-
   COPY_HARD_REG_SET (*reg_set, default_set);
+  targetm.remove_extra_call_preserved_regs (insn, reg_set);
   return false;
 }
diff --git a/gcc/target.def b/gcc/target.def
index e8f0f70..908f9b9 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -5775,6 +5775,20 @@ for targets that don't have partly call-clobbered registers.",
  bool, (unsigned int regno, machine_mode mode),
  hook_bool_uint_mode_false)
 
+DEFHOOK
+(remove_extra_call_preserved_regs,
+ "This hook removes registers from the set of call-clobbered registers\n\
+ in @var{used_regs} if, contrary to the default rules, something guarantees\n\
+ that @samp{insn} preserves those registers.  For example, some targets\n\
+ support variant ABIs in which functions preserve more registers than\n\
+ normal functions would.  Removing those extra registers from @var{used_regs}\n\
+ can lead to better register allocation.\n\
+ \n\
+ The default implementation does nothing, which is always safe.\n\
+ Defining the hook is purely an optimization.",
+ void, (rtx_insn *insn, HARD_REG_SET *used_regs),
+ default_remove_extra_call_preserved_regs)
+
 /* Return the smallest number of different values for which it is best to
    use a jump-table instead of a tree of conditional branches.  */
 DEFHOOK
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 898848f..6bd9767 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -2374,4 +2374,9 @@ default_speculation_safe_value (machine_mode mode ATTRIBUTE_UNUSED,
   return result;
 }
 
+void
+default_remove_extra_call_preserved_regs (rtx_insn *, HARD_REG_SET *)
+{
+}
+
 #include "gt-targhooks.h"
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 3b6e404..01ee0be 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -284,5 +284,7 @@ extern tree default_preferred_else_value (unsigned, tree, unsigned, tree *);
 extern bool default_have_speculation_safe_value (bool);
 extern bool speculation_safe_value_not_needed (bool);
 extern rtx default_speculation_safe_value (machine_mode, rtx, rtx, rtx);
+extern void default_remove_extra_call_preserved_regs (rtx_insn *,
+						      HARD_REG_SET *);
 
 #endif /* GCC_TARGHOOKS_H */

Steve Ellcey <sellcey@marvell.com> writes:
> Here is an update of this patch.  I believe I addressed all of the
> issues you raised.  Thanks for the new target.def description, it
> seems much clearer than mine.  I did a full build and test on x86 as
> well as aarch64 to make sure that architectures that do not define
> TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS build correctly.
>
> Steve Ellcey
> sellcey@marvell.com
>
>
> 2019-01-02  Steve Ellcey  <sellcey@marvell.com>
>
> 	* config/aarch64/aarch64.c (aarch64_simd_call_p): New function.
> 	(aarch64_remove_extra_call_preserved_regs): New function.
> 	(TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): New macro.
> 	* doc/tm.texi.in (TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): New hook.
> 	* final.c (get_call_reg_set_usage): Call new hook.
> 	* target.def (remove_extra_call_preserved_regs): New hook.
> 	* targhooks.c (default_remove_extra_call_preserved_regs): New function.
> 	* targhooks.h (default_remove_extra_call_preserved_regs): New function.

OK, thanks.

Richard