Re: [1/4] aarch64: Refactor check_required_extensions

[Richard Sandiford <richard.sandiford@arm.com>]

Andrew Carlotti <andrew.carlotti@arm.com> writes:
> Move SVE extension checking functionality to aarch64-builtins.cc, so
> that it can be shared by non-SVE intrinsics.
>
> gcc/ChangeLog:
>
> 	* config/aarch64/aarch64-sve-builtins.cc (check_builtin_call)
> 	(expand_builtin): Update calls to the below.
> 	(report_missing_extension, check_required_registers)
> 	(check_required_extensions): Move out of aarch64_sve namespace,
> 	rename, and move into...
> 	* config/aarch64/aarch64-builtins.cc (aarch64_report_missing_extension)
> 	(aarch64_check_non_general_registers)
> 	(aarch64_check_required_extensions) ...here.
> 	* config/aarch64/aarch64-protos.h (aarch64_check_required_extensions):
> 	Add prototype.

OK for trunk and backports, but...

> diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc
> index 04f59fd9a54306d6422b03e32dce79bc00aed4f8..11a9ba2256f105d8cb9cdc4d6decb5b2be3d69af 100644
> --- a/gcc/config/aarch64/aarch64-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-builtins.cc
> @@ -2054,6 +2054,90 @@ aarch64_general_builtin_decl (unsigned code, bool)
>    return aarch64_builtin_decls[code];
>  }
>  
> +/* True if we've already complained about attempts to use functions
> +   when the required extension is disabled.  */
> +static bool reported_missing_extension_p;
> +
> +/* True if we've already complained about attempts to use functions
> +   which require registers that are missing.  */
> +static bool reported_missing_registers_p;
> +
> +/* Report an error against LOCATION that the user has tried to use
> +   function FNDECL when extension EXTENSION is disabled.  */
> +static void
> +aarch64_report_missing_extension (location_t location, tree fndecl,
> +			  const char *extension)

...this needs reindenting for the longer name.  Same for
aarch64_check_required_extensions.

Thanks,
Richard


> +{
> +  /* Avoid reporting a slew of messages for a single oversight.  */
> +  if (reported_missing_extension_p)
> +    return;
> +
> +  error_at (location, "ACLE function %qD requires ISA extension %qs",
> +	    fndecl, extension);
> +  inform (location, "you can enable %qs using the command-line"
> +	  " option %<-march%>, or by using the %<target%>"
> +	  " attribute or pragma", extension);
> +  reported_missing_extension_p = true;
> +}
> +
> +/* Check whether non-general registers required by ACLE function fndecl are
> + * available.  Report an error against LOCATION and return false if not.  */
> +static bool
> +aarch64_check_non_general_registers (location_t location, tree fndecl)
> +{
> +  /* Avoid reporting a slew of messages for a single oversight.  */
> +  if (reported_missing_registers_p)
> +    return false;
> +
> +  if (TARGET_GENERAL_REGS_ONLY)
> +    {
> +      /* FP/SIMD/SVE registers are not usable when -mgeneral-regs-only option
> +	 is specified.  */
> +      error_at (location,
> +		"ACLE function %qD is incompatible with the use of %qs",
> +		fndecl, "-mgeneral-regs-only");
> +      reported_missing_registers_p = true;
> +      return false;
> +    }
> +
> +  return true;
> +}
> +
> +/* Check whether all the AARCH64_FL_* values in REQUIRED_EXTENSIONS are
> +   enabled, given that those extensions are required for function FNDECL.
> +   Report an error against LOCATION if not.
> +   If REQUIRES_NON_GENERAL_REGISTERS is true, then also check whether
> +   non-general registers are available.  */
> +bool
> +aarch64_check_required_extensions (location_t location, tree fndecl,
> +			   aarch64_feature_flags required_extensions,
> +			   bool requires_non_general_registers)
> +{
> +  auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
> +  if (missing_extensions == 0)
> +    return requires_non_general_registers
> +	   ? aarch64_check_non_general_registers (location, fndecl)
> +	   : true;
> +
> +  static const struct {
> +    aarch64_feature_flags flag;
> +    const char *name;
> +  } extensions[] = {
> +#define AARCH64_OPT_EXTENSION(EXT_NAME, IDENT, C, D, E, F) \
> +    { AARCH64_FL_##IDENT, EXT_NAME },
> +#include "aarch64-option-extensions.def"
> +  };
> +
> +  for (unsigned int i = 0; i < ARRAY_SIZE (extensions); ++i)
> +    if (missing_extensions & extensions[i].flag)
> +      {
> +	aarch64_report_missing_extension (location, fndecl, extensions[i].name);
> +	return false;
> +      }
> +  gcc_unreachable ();
> +}
> +
> +
>  typedef enum
>  {
>    SIMD_ARG_COPY_TO_REG,
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 60a55f4bc1956786ea687fc7cad7ec9e4a84e1f0..30726140a6945dcb86b787f8f47952810d99379f 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -988,6 +988,9 @@ tree aarch64_general_builtin_rsqrt (unsigned int);
>  void handle_arm_acle_h (void);
>  void handle_arm_neon_h (void);
>  
> +bool aarch64_check_required_extensions (location_t, tree,
> +					aarch64_feature_flags, bool = true);
> +
>  namespace aarch64_sve {
>    void init_builtins ();
>    void handle_arm_sve_h ();
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
> index 161a14edde7c9fb1b13b146cf50463e2d78db264..c72baed08759e143e6b9bc4fabf115a8374af7c8 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
> @@ -558,14 +558,6 @@ static GTY(()) vec<registered_function *, va_gc> *registered_functions;
>     overloaded functions.  */
>  static hash_table<registered_function_hasher> *function_table;
>  
> -/* True if we've already complained about attempts to use functions
> -   when the required extension is disabled.  */
> -static bool reported_missing_extension_p;
> -
> -/* True if we've already complained about attempts to use functions
> -   which require registers that are missing.  */
> -static bool reported_missing_registers_p;
> -
>  /* Record that TYPE is an ABI-defined SVE type that contains NUM_ZR SVE vectors
>     and NUM_PR SVE predicates.  MANGLED_NAME, if nonnull, is the ABI-defined
>     mangling of the type.  ACLE_NAME is the <arm_sve.h> name of the type.  */
> @@ -648,75 +640,6 @@ find_type_suffix_for_scalar_type (const_tree type)
>    return NUM_TYPE_SUFFIXES;
>  }
>  
> -/* Report an error against LOCATION that the user has tried to use
> -   function FNDECL when extension EXTENSION is disabled.  */
> -static void
> -report_missing_extension (location_t location, tree fndecl,
> -			  const char *extension)
> -{
> -  /* Avoid reporting a slew of messages for a single oversight.  */
> -  if (reported_missing_extension_p)
> -    return;
> -
> -  error_at (location, "ACLE function %qD requires ISA extension %qs",
> -	    fndecl, extension);
> -  inform (location, "you can enable %qs using the command-line"
> -	  " option %<-march%>, or by using the %<target%>"
> -	  " attribute or pragma", extension);
> -  reported_missing_extension_p = true;
> -}
> -
> -/* Check whether the registers required by SVE function fndecl are available.
> -   Report an error against LOCATION and return false if not.  */
> -static bool
> -check_required_registers (location_t location, tree fndecl)
> -{
> -  /* Avoid reporting a slew of messages for a single oversight.  */
> -  if (reported_missing_registers_p)
> -    return false;
> -
> -  if (TARGET_GENERAL_REGS_ONLY)
> -    {
> -      /* SVE registers are not usable when -mgeneral-regs-only option
> -	 is specified.  */
> -      error_at (location,
> -		"ACLE function %qD is incompatible with the use of %qs",
> -		fndecl, "-mgeneral-regs-only");
> -      reported_missing_registers_p = true;
> -      return false;
> -    }
> -
> -  return true;
> -}
> -
> -/* Check whether all the AARCH64_FL_* values in REQUIRED_EXTENSIONS are
> -   enabled, given that those extensions are required for function FNDECL.
> -   Report an error against LOCATION if not.  */
> -static bool
> -check_required_extensions (location_t location, tree fndecl,
> -			   aarch64_feature_flags required_extensions)
> -{
> -  auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
> -  if (missing_extensions == 0)
> -    return check_required_registers (location, fndecl);
> -
> -  static const struct {
> -    aarch64_feature_flags flag;
> -    const char *name;
> -  } extensions[] = {
> -#define AARCH64_OPT_EXTENSION(EXT_NAME, IDENT, C, D, E, F) \
> -    { AARCH64_FL_##IDENT, EXT_NAME },
> -#include "aarch64-option-extensions.def"
> -  };
> -
> -  for (unsigned int i = 0; i < ARRAY_SIZE (extensions); ++i)
> -    if (missing_extensions & extensions[i].flag)
> -      {
> -	report_missing_extension (location, fndecl, extensions[i].name);
> -	return false;
> -      }
> -  gcc_unreachable ();
> -}
>  
>  /* Report that LOCATION has a call to FNDECL in which argument ARGNO
>     was not an integer constant expression.  ARGNO counts from zero.  */
> @@ -3605,7 +3528,8 @@ check_builtin_call (location_t location, vec<location_t>, unsigned int code,
>  		    tree fndecl, unsigned int nargs, tree *args)
>  {
>    const registered_function &rfn = *(*registered_functions)[code];
> -  if (!check_required_extensions (location, rfn.decl, rfn.required_extensions))
> +  if (!aarch64_check_required_extensions (location, rfn.decl,
> +					  rfn.required_extensions))
>      return false;
>    return function_checker (location, rfn.instance, fndecl,
>  			   TREE_TYPE (rfn.decl), nargs, args).check ();
> @@ -3628,8 +3552,8 @@ rtx
>  expand_builtin (unsigned int code, tree exp, rtx target)
>  {
>    registered_function &rfn = *(*registered_functions)[code];
> -  if (!check_required_extensions (EXPR_LOCATION (exp), rfn.decl,
> -				  rfn.required_extensions))
> +  if (!aarch64_check_required_extensions (EXPR_LOCATION (exp), rfn.decl,
> +					  rfn.required_extensions))
>      return target;
>    return function_expander (rfn.instance, rfn.decl, exp, target).expand ();
>  }