diff mbox series

[PATCH7/8] vect: Add TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM

Message ID a65c1f78-1159-d6df-c355-d6f92032ccd2@arm.com
State New
Headers show
Series [1/8] parloops: Copy target and optimizations when creating a function clone | expand

Commit Message

Andre Vieira (lists) Aug. 30, 2023, 9:17 a.m. UTC 
This patch adds a new target hook to enable us to adapt the types of 
return and parameters of simd clones.  We use this in two ways, the 
first one is to make sure we can create valid SVE types, including the 
SVE type attribute, when creating a SVE simd clone, even when the target 
options do not support SVE.  We are following the same behaviour seen 
with x86 that creates simd clones according to the ABI rules when no 
simdlen is provided, even if that simdlen is not supported by the 
current target options.  Note that this doesn't mean the simd clone will 
be used in auto-vectorization.

gcc/ChangeLog:

	(TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM): Define.
	* doc/tm.texi (TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM): Document.
	* doc/tm.texi.in (TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM): New.
	* omp-simd-clone.cc (simd_adjust_return_type): Call new hook.
	(simd_clone_adjust_argument_types): Likewise.
	* target.def (adjust_ret_or_param): New hook.
	* targhooks.cc (default_simd_clone_adjust_ret_or_param): New.
	* targhooks.h (default_simd_clone_adjust_ret_or_param): New.

Comments

Richard Biener Aug. 30, 2023, 1:04 p.m. UTC | #1 
On Wed, 30 Aug 2023, Andre Vieira (lists) wrote:

> This patch adds a new target hook to enable us to adapt the types of return
> and parameters of simd clones.  We use this in two ways, the first one is to
> make sure we can create valid SVE types, including the SVE type attribute,
> when creating a SVE simd clone, even when the target options do not support
> SVE.  We are following the same behaviour seen with x86 that creates simd
> clones according to the ABI rules when no simdlen is provided, even if that
> simdlen is not supported by the current target options.  Note that this
> doesn't mean the simd clone will be used in auto-vectorization.

You are not documenting the bool parameter of the new hook.

What's wrong with doing the adjustment in TARGET_SIMD_CLONE_ADJUST?

> gcc/ChangeLog:
> 
> 	(TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM): Define.
> 	* doc/tm.texi (TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM): Document.
> 	* doc/tm.texi.in (TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM): New.
> 	* omp-simd-clone.cc (simd_adjust_return_type): Call new hook.
> 	(simd_clone_adjust_argument_types): Likewise.
> 	* target.def (adjust_ret_or_param): New hook.
> 	* targhooks.cc (default_simd_clone_adjust_ret_or_param): New.
> 	* targhooks.h (default_simd_clone_adjust_ret_or_param): New.
>
Andre Vieira (lists) Oct. 4, 2023, 10:32 a.m. UTC | #2 
On 30/08/2023 14:04, Richard Biener wrote:
> On Wed, 30 Aug 2023, Andre Vieira (lists) wrote:
> 
>> This patch adds a new target hook to enable us to adapt the types of return
>> and parameters of simd clones.  We use this in two ways, the first one is to
>> make sure we can create valid SVE types, including the SVE type attribute,
>> when creating a SVE simd clone, even when the target options do not support
>> SVE.  We are following the same behaviour seen with x86 that creates simd
>> clones according to the ABI rules when no simdlen is provided, even if that
>> simdlen is not supported by the current target options.  Note that this
>> doesn't mean the simd clone will be used in auto-vectorization.
> 
> You are not documenting the bool parameter of the new hook.
> 
> What's wrong with doing the adjustment in TARGET_SIMD_CLONE_ADJUST?

simd_clone_adjust_argument_types is called after that hook, so by the 
time we call TARGET_SIMD_CLONE_ADJUST the types are still in scalar, not 
vector.  The same is true for the return type one.

Also the changes to the types need to be taken into consideration in 
'adjustments' I think.

PS: I hope the subject line survived, my email client is having a bit of 
a wobble this morning... it's what you get for updating software :(
Richard Biener Oct. 4, 2023, 10:41 a.m. UTC | #3 
On Wed, 4 Oct 2023, Andre Vieira (lists) wrote:

> 
> 
> On 30/08/2023 14:04, Richard Biener wrote:
> > On Wed, 30 Aug 2023, Andre Vieira (lists) wrote:
> > 
> >> This patch adds a new target hook to enable us to adapt the types of return
> >> and parameters of simd clones.  We use this in two ways, the first one is
> >> to
> >> make sure we can create valid SVE types, including the SVE type attribute,
> >> when creating a SVE simd clone, even when the target options do not support
> >> SVE.  We are following the same behaviour seen with x86 that creates simd
> >> clones according to the ABI rules when no simdlen is provided, even if that
> >> simdlen is not supported by the current target options.  Note that this
> >> doesn't mean the simd clone will be used in auto-vectorization.
> > 
> > You are not documenting the bool parameter of the new hook.
> > 
> > What's wrong with doing the adjustment in TARGET_SIMD_CLONE_ADJUST?
> 
> simd_clone_adjust_argument_types is called after that hook, so by the time we
> call TARGET_SIMD_CLONE_ADJUST the types are still in scalar, not vector.  The
> same is true for the return type one.
> 
> Also the changes to the types need to be taken into consideration in
> 'adjustments' I think.

Nothing in the three existing implementations of TARGET_SIMD_CLONE_ADJUST
relies on this ordering I think, how about moving the hook invocation 
after simd_clone_adjust_argument_types?

Richard.

> PS: I hope the subject line survived, my email client is having a bit of a
> wobble this morning... it's what you get for updating software :(
Andre Vieira (lists) Oct. 4, 2023, 12:40 p.m. UTC | #4 
On 04/10/2023 11:41, Richard Biener wrote:
> On Wed, 4 Oct 2023, Andre Vieira (lists) wrote:
> 
>>
>>
>> On 30/08/2023 14:04, Richard Biener wrote:
>>> On Wed, 30 Aug 2023, Andre Vieira (lists) wrote:
>>>
>>>> This patch adds a new target hook to enable us to adapt the types of return
>>>> and parameters of simd clones.  We use this in two ways, the first one is
>>>> to
>>>> make sure we can create valid SVE types, including the SVE type attribute,
>>>> when creating a SVE simd clone, even when the target options do not support
>>>> SVE.  We are following the same behaviour seen with x86 that creates simd
>>>> clones according to the ABI rules when no simdlen is provided, even if that
>>>> simdlen is not supported by the current target options.  Note that this
>>>> doesn't mean the simd clone will be used in auto-vectorization.
>>>
>>> You are not documenting the bool parameter of the new hook.
>>>
>>> What's wrong with doing the adjustment in TARGET_SIMD_CLONE_ADJUST?
>>
>> simd_clone_adjust_argument_types is called after that hook, so by the time we
>> call TARGET_SIMD_CLONE_ADJUST the types are still in scalar, not vector.  The
>> same is true for the return type one.
>>
>> Also the changes to the types need to be taken into consideration in
>> 'adjustments' I think.
> 
> Nothing in the three existing implementations of TARGET_SIMD_CLONE_ADJUST
> relies on this ordering I think, how about moving the hook invocation
> after simd_clone_adjust_argument_types?
> 

But that wouldn't change the 'ipa_param_body_adjustments' for when we 
have a function definition and we need to redo the body.
> Richard.
> 
>> PS: I hope the subject line survived, my email client is having a bit of a
>> wobble this morning... it's what you get for updating software :(
diff mbox series

Patch

diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index bde22e562ebb9069122eb3b142ab8f4a4ae56a3a..b80c09ec36d51f1bb55b14229f46207fb4457223 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -6343,6 +6343,9 @@  non-negative number if it is usable.  In that case, the smaller the number is,
 the more desirable it is to use it.
 @end deftypefn
 
+@deftypefn {Target Hook} tree TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM (struct cgraph_node *@var{}, @var{tree}, @var{bool})
+If defined, this hook should adjust the type of the return or parameter
+@var{type} to be used by the simd clone @var{node}.
 @end deftypefn
 
 @deftypefn {Target Hook} int TARGET_SIMT_VF (void)
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 4ac96dc357d35e0e57bb43a41d1b1a4f66d05946..7496a32d84f7c422fe7ea88215ee72f3c354a3f4 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -4211,6 +4211,8 @@  address;  but often a machine-dependent strategy can generate better code.
 
 @hook TARGET_SIMD_CLONE_USABLE
 
+@hook TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM
+
 @hook TARGET_SIMT_VF
 
 @hook TARGET_OMP_DEVICE_KIND_ARCH_ISA
diff --git a/gcc/omp-simd-clone.cc b/gcc/omp-simd-clone.cc
index ef0b9b48c7212900023bc0eaebca5e1f9389db77..c2fd4d3be878e56b6394e34097d2de826a0ba1ff 100644
--- a/gcc/omp-simd-clone.cc
+++ b/gcc/omp-simd-clone.cc
@@ -736,6 +736,7 @@  simd_clone_adjust_return_type (struct cgraph_node *node)
       t = build_array_type_nelts (t, exact_div (node->simdclone->simdlen,
 						veclen));
     }
+  t = targetm.simd_clone.adjust_ret_or_param (node, t, false);
   TREE_TYPE (TREE_TYPE (fndecl)) = t;
   if (!node->definition)
     return NULL_TREE;
@@ -748,6 +749,7 @@  simd_clone_adjust_return_type (struct cgraph_node *node)
 
   tree atype = build_array_type_nelts (orig_rettype,
 				       node->simdclone->simdlen);
+  atype = targetm.simd_clone.adjust_ret_or_param (node, atype, false);
   if (maybe_ne (veclen, node->simdclone->simdlen))
     return build1 (VIEW_CONVERT_EXPR, atype, t);
 
@@ -880,6 +882,8 @@  simd_clone_adjust_argument_types (struct cgraph_node *node)
 				       ? IDENTIFIER_POINTER (DECL_NAME (parm))
 				       : NULL, parm_type, sc->simdlen);
 	}
+      adj.type = targetm.simd_clone.adjust_ret_or_param (node, adj.type,
+							 false);
       vec_safe_push (new_params, adj);
     }
 
@@ -912,6 +916,8 @@  simd_clone_adjust_argument_types (struct cgraph_node *node)
 	adj.type = build_vector_type (pointer_sized_int_node, veclen);
       else
 	adj.type = build_vector_type (base_type, veclen);
+      adj.type = targetm.simd_clone.adjust_ret_or_param (node, adj.type,
+							 true);
       vec_safe_push (new_params, adj);
 
       k = vector_unroll_factor (sc->simdlen, veclen);
@@ -937,6 +943,7 @@  simd_clone_adjust_argument_types (struct cgraph_node *node)
 	    sc->args[i].simd_array = NULL_TREE;
 	}
       sc->args[i].orig_type = base_type;
+      sc->args[i].vector_type = adj.type;
       sc->args[i].arg_type = SIMD_CLONE_ARG_TYPE_MASK;
       sc->args[i].vector_type = adj.type;
     }
diff --git a/gcc/target.def b/gcc/target.def
index 6a0cbc454526ee29011451b570354bf234a4eabd..665083ce035da03b40b15f23684ccdacce33c9d3 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -1650,6 +1650,13 @@  non-negative number if it is usable.  In that case, the smaller the number is,\n
 the more desirable it is to use it.",
 int, (struct cgraph_node *, machine_mode), NULL)
 
+DEFHOOK
+(adjust_ret_or_param,
+"If defined, this hook should adjust the type of the return or parameter\n\
+@var{type} to be used by the simd clone @var{node}.",
+tree, (struct cgraph_node *, tree, bool),
+default_simd_clone_adjust_ret_or_param)
+
 
 HOOK_VECTOR_END (simd_clone)
 
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 1a0db8dddd594d9b1fb04ae0d9a66ad6b7a396dc..558157514814228ef2ed41ae0861e1c088eea9ef 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -75,6 +75,9 @@  extern void default_print_operand (FILE *, rtx, int);
 extern void default_print_operand_address (FILE *, machine_mode, rtx);
 extern bool default_print_operand_punct_valid_p (unsigned char);
 extern tree default_mangle_assembler_name (const char *);
+extern tree default_simd_clone_adjust_ret_or_param
+  (struct cgraph_node *,tree , bool);
+
 
 extern machine_mode default_translate_mode_attribute (machine_mode);
 extern bool default_scalar_mode_supported_p (scalar_mode);
diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
index e190369f87a92e6a92372dc348d9374c3a965c0a..6b6f6132c6dc62b92ad8d448d63ca6041386788f 100644
--- a/gcc/targhooks.cc
+++ b/gcc/targhooks.cc
@@ -399,6 +399,16 @@  default_mangle_assembler_name (const char *name ATTRIBUTE_UNUSED)
   return get_identifier (stripped);
 }
 
+/* The default implementation of TARGET_SIMD_CLONE_ADJUST_RET_OR_PARAM.  */
+
+tree
+default_simd_clone_adjust_ret_or_param (struct cgraph_node *node ATTRIBUTE_UNUSED,
+					tree type,
+					bool is_return ATTRIBUTE_UNUSED)
+{
+  return type;
+}
+
 /* The default implementation of TARGET_TRANSLATE_MODE_ATTRIBUTE.  */
 
 machine_mode