Saurabh Jha wrote:
> +/* Returns true if a function has variadic arguments.
> +   Only works for Windows ABI.  */
> +
> +static bool
> +aarch64_ms_is_variadic_function_type (const_tree fntype)
> +{
> +  if (!TARGET_AARCH64_MS_ABI)
> +    return false;
> +
> +  if (TYPE_NO_NAMED_ARGS_STDARG_P (fntype))
> +    return true;
> +
> +  auto arg_count = 0;
> +  for (tree arg = TYPE_ARG_TYPES (fntype); arg; arg = TREE_CHAIN (arg))
> +    {
> +      if (TREE_VALUE (arg) == void_type_node)
> + return false;
> +      arg_count++;
> +    }
> +
> +  return arg_count > 0;
> +}
> +
> +/* Return the descriptor of the Windows Arm64 variadic function call ABI.  */
> +
> +static const predefined_function_abi &
> +aarch64_ms_variadic_abi (void)
> +{
> +  predefined_function_abi &ms_variadic_abi = 
> function_abis[ARM_PCS_MS_VARIADIC];
> +  if (!ms_variadic_abi.initialized_p ())
> +    {
> +      HARD_REG_SET full_reg_clobbers
> +       = default_function_abi.full_reg_clobbers ();
> +      ms_variadic_abi.initialize (ARM_PCS_MS_VARIADIC, full_reg_clobbers);
> +    }
> +  return ms_variadic_abi;
> +}
> +
>  /* Implement TARGET_FNTYPE_ABI.  */
>
>  static const predefined_function_abi &
>  aarch64_fntype_abi (const_tree fntype)
>  {
> +  if (aarch64_ms_is_variadic_function_type (fntype))
> +    return aarch64_ms_variadic_abi ();

It looks like aarch64_ms_is_variadic_function_type is a duplicate of
stdarg_p, and it might be replaced with something like this...

   if (TARGET_AARCH64_MS_ABI && stdarg_p (fntype))
     return aarch64_ms_variadic_abi ();

> +  if (lookup_attribute ("ms_abi", TYPE_ATTRIBUTES (fntype)))
> +    return aarch64_ms_variadic_abi ();
> +

Why should aarch64_ms_variadic_abi be returned for the ms_abi attribute
if the function is not varidiac?

>        case ARM_PCS_SIMD:
>  /* The vector PCS saves the low 128 bits (which is the full
>     register on non-SVE targets).  */
> @@ -7345,6 +7399,7 @@ num_pcs_arg_regs (enum arm_pcs pcs)
>      case ARM_PCS_SIMD:
>      case ARM_PCS_SVE:
>      case ARM_PCS_TLSDESC:
> +    case ARM_PCS_MS_VARIADIC:
>      case ARM_PCS_UNKNOWN:
>        return NUM_ARG_REGS;

There is an order to the naming.

>      }
> @@ -7369,12 +7424,84 @@ get_pcs_arg_reg (enum arm_pcs pcs, int num)
>      case ARM_PCS_SIMD:
>      case ARM_PCS_SVE:
>      case ARM_PCS_TLSDESC:
> +    case ARM_PCS_MS_VARIADIC:
>      case ARM_PCS_UNKNOWN:
>        return R0_REGNUM + num;
>      }
>    gcc_unreachable ();
>  }

The same here, and more similar changes below.

> @@ -7767,7 +7892,8 @@ aarch64_function_arg (cumulative_args_t pcum_v, const 
> function_arg_info &arg)
>    gcc_assert (pcum->pcs_variant == ARM_PCS_AAPCS64
>        || pcum->pcs_variant == ARM_PCS_SIMD
>        || pcum->pcs_variant == ARM_PCS_SVE
> -       || pcum->pcs_variant == ARM_PCS_PRESERVE_NONE);
> +       || pcum->pcs_variant == ARM_PCS_PRESERVE_NONE
> +       || pcum->pcs_variant == ARM_PCS_MS_VARIADIC);

... and here.

> +/* Windows Arm64 variadic function call ABI specific va_list type node.  */
> +tree ms_va_list_type_node;

What about initializing it with NULL_TREE?

> +#if TARGET_AARCH64_MS_ABI == 1
> +
> +/* Iterate through the target-specific builtin types for va_list.
> +   IDX denotes the iterator, *PTREE is set to the result type of
> +   the va_list builtin, and *PNAME to its internal type.
> +   Returns zero if there is no element for this index, otherwise
> +   IDX should be increased upon the next call.
> +   Note, do not iterate a base builtin's name like __builtin_va_list.
> +   Used from c_common_nodes_and_builtins.  */
> +
> +static int
> +aarch64_ms_variadic_abi_enum_va_list (int idx, const char **pname, tree 
> *ptree)
> +{
> +  switch (idx)
> +    {

ms_abi specific code was moved under #if TARGET_AARCH64_MS_ABI, for
better structure
and readability it might be nice to move it to separate file,
something like aarch64-abi-ms.cc.

> +#if TARGET_AARCH64_MS_ABI == 0
>  static tree
>  aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p,
>        gimple_seq *post_p ATTRIBUTE_UNUSED)
> @@ -22262,6 +22510,7 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, 
> gimple_seq *pre_p,

the same for this abi.

> +/* aarch64-w64-mingw32 handles variadic ABI differently.  */
> +#undef  SUBTARGET_INIT_BUILTINS
> +#define SUBTARGET_INIT_BUILTINS \
> + do \
> + { \
> + aarch64_ms_variadic_abi_init_builtins (); \
> + } while (0)

it might make sense to move this check directly to aarch64_init_builtins
and make it conditional on TARGET_AARCH64_MS_ABI.

>  extern tree mingw_handle_selectany_attribute (tree *, tree, tree, int, bool 
> *);
> +extern tree aarch64_handle_ms_abi_attribute (tree *, tree, tree, int, bool 
> *);

the order in the subgroup, aarch64_handle_ms_abi_attribute should be
placed first.

Regards,
Evgeny

Reply via email to