Re: [PATCH][AArch64] Fix ICEs with +nofp/-mgeneral-regs-only and improve error messages; clarify docs.

Re: [PATCH][AArch64] Fix ICEs with +nofp/-mgeneral-regs-only and improve error messages; clarify docs.

[James Greenhalgh]

On Thu, Jun 11, 2015 at 12:28:08PM +0100, Alan Lawrence wrote:
> Hi,
> 
> This is a follow-up to Jim Wilson's patch fixing ICE's with
> -march=armv8-a+nofp, and the discussion here:
> https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00177.html
> 
> The first patch improves the error messages to describe what kind of code
> caused the problem, and to error rather than "sorry" (we should not be sorry,
> the user has asked the compiler to do something that makes no sense!).
> Moreover, to issue an error, rather than ICE, on some testcases (supplied!).
> 
> The error messages in aarch64_setup_incoming_varargs and 
> aarch64_expand_builtin_va_start are then never reached as error() has already 
> been called, so change them to asserts.
> 
> Compiling with -mgeneral-regs-only on functions taking vector arguments, and 
> simple arithmetic using float/double, all raise the error; on functions using 
> vectors only internally not via ABI, at least some are handled by the midend 
> using scalar code.
> 
> 
> The second patch cleans up the documentation in line with the previous
> discussion.

Submissions on this list should be one patch per mail, it makes
tracking review easier.

Comments inline below.

> gcc/ChangeLog:
> 
> 	* config/aarch64/aarch64-protos.h (aarch64_err_no_fpadvsimd): New.
> 
> 	* config/aarch64/aarch64.md (mov<mode>/GPF, movtf): Use
> 	aarch64_err_no_fpadvsimd.
> 
> 	* config/aarch64/aarch64.c (aarch64_err_no_fpadvsimd): New.
> 	(aarch64_layout_arg, aarch64_init_cumulative_args): Use
> 	aarch64_err_no_fpadvsimd if !TARGET_FLOAT and we need FP regs.
> 	(aarch64_expand_builtin_va_start, aarch64_setup_incoming_varargs):
> 	Turn error into assert, test TARGET_FLOAT.
> 	(aarch64_gimplify_va_arg_expr): Use aarch64_err_no_fpadvsimd, test
> 	TARGET_FLOAT.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/aarch64/mgeneral-regs_1.c: New file.
> 	* gcc.target/aarch64/mgeneral-regs_2.c: New file.
> 	* gcc.target/aarch64/nofp_1.c: New file.
>
> gcc/ChangeLog:
> 
> 	* doc/invoke.texi: Clarify AArch64 feature modifiers (no)fp, (no)simd
> 	and (no)crypto.

> commit efbf0f4699ac963472834c912b46b1a3a076fa64
> Author: Alan Lawrence <alan.lawrence@arm.com>
> Date:   Mon Jan 12 15:04:06 2015 +0000
> 
>     Approved r/3008, rebased
> 
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 965a11b..ac92c59 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -259,6 +259,7 @@ unsigned aarch64_dbx_register_number (unsigned);
>  unsigned aarch64_trampoline_size (void);
>  void aarch64_asm_output_labelref (FILE *, const char *);
>  void aarch64_elf_asm_named_section (const char *, unsigned, tree);
> +void aarch64_err_no_fpadvsimd (machine_mode, const char *);
>  void aarch64_expand_epilogue (bool);
>  void aarch64_expand_mov_immediate (rtx, rtx);
>  void aarch64_expand_prologue (void);
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 968a6b6..5636e7b 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -534,6 +534,16 @@ static const char * const aarch64_condition_codes[] =
>    "hi", "ls", "ge", "lt", "gt", "le", "al", "nv"
>  };
>  
> +void
> +aarch64_err_no_fpadvsimd (machine_mode mode, const char *msg)
> +{
> +  const char *mc = FLOAT_MODE_P (mode) ? "floating point" : "vector";

GCC coding conventions, this should be
floating-point (https://gcc.gnu.org/codingconventions.html).

> +  if (TARGET_GENERAL_REGS_ONLY)
> +    error ("%qs is incompatible with %s %s", "-mgeneral-regs-only", mc, msg);
> +  else
> +    error ("%qs feature modifier is incompatible with %s %s", "+nofp", mc, msg);
> +}
> +
>  static unsigned int
>  aarch64_min_divisions_for_recip_mul (enum machine_mode mode)
>  {
> @@ -1784,6 +1794,9 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
>       and homogenous short-vector aggregates (HVA).  */
>    if (allocate_nvrn)
>      {
> +      if (!TARGET_FLOAT)
> +	aarch64_err_no_fpadvsimd (mode, "argument");
> +
>        if (nvrn + nregs <= NUM_FP_ARG_REGS)
>  	{
>  	  pcum->aapcs_nextnvrn = nvrn + nregs;
> @@ -1910,6 +1923,17 @@ aarch64_init_cumulative_args (CUMULATIVE_ARGS *pcum,
>    pcum->aapcs_stack_words = 0;
>    pcum->aapcs_stack_size = 0;
>  
> +  if (!TARGET_FLOAT
> +      && fndecl && TREE_PUBLIC (fndecl)
> +      && fntype && fntype != error_mark_node)
> +    {
> +      const_tree args, type = TREE_TYPE (fntype);
> +      machine_mode mode; /* To pass pointer as argument; never used.  */
> +      int nregs; /* Likewise.  */

Do these need annotations to avoid errors in a Werror build? I don't see
any mention of what testing this patch has been through?

> +      if (aarch64_vfp_is_call_or_return_candidate (TYPE_MODE (type), type,
> +						   &mode, &nregs, NULL))
> +	aarch64_err_no_fpadvsimd (TYPE_MODE (type), "return type");
> +    }
>    return;
>  }
>  
> @@ -7573,9 +7597,7 @@ aarch64_expand_builtin_va_start (tree valist, rtx nextarg ATTRIBUTE_UNUSED)
>  
>    if (!TARGET_FLOAT)
>      {
> -      if (cum->aapcs_nvrn > 0)
> -	sorry ("%qs and floating point or vector arguments",
> -	       "-mgeneral-regs-only");
> +      gcc_assert (cum->aapcs_nvrn == 0);

This promotes an error to an ICE? Can we really never get to this
point through the error control flow I would have expected that to
report an error then return control-flow to here.

>        vr_save_area_size = 0;
>      }
>  
> @@ -7682,8 +7704,7 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p,
>      {
>        /* TYPE passed in fp/simd registers.  */
>        if (!TARGET_FLOAT)
> -	sorry ("%qs and floating point or vector arguments",
> -	       "-mgeneral-regs-only");
> +	aarch64_err_no_fpadvsimd (mode, "varargs");
>  
>        f_top = build3 (COMPONENT_REF, TREE_TYPE (f_vrtop),
>  		      unshare_expr (valist), f_vrtop, NULL_TREE);
> @@ -7920,9 +7941,7 @@ aarch64_setup_incoming_varargs (cumulative_args_t cum_v, machine_mode mode,
>  
>    if (!TARGET_FLOAT)
>      {
> -      if (local_cum.aapcs_nvrn > 0)
> -	sorry ("%qs and floating point or vector arguments",
> -	       "-mgeneral-regs-only");
> +      gcc_assert (local_cum.aapcs_nvrn == 0);

As above?

>        vr_saved = 0;
>      }
>  
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 11123d6..99cefec 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -981,10 +981,7 @@
>    ""
>    "
>      if (!TARGET_FLOAT)
> -     {
> -	sorry (\"%qs and floating point code\", \"-mgeneral-regs-only\");
> -	FAIL;
> -     }
> +      aarch64_err_no_fpadvsimd (<MODE>mode, \"code\");

You've dropped the FAIL?

>  
>      if (GET_CODE (operands[0]) == MEM)
>        operands[1] = force_reg (<MODE>mode, operands[1]);
> @@ -1035,10 +1032,7 @@
>    ""
>    "
>      if (!TARGET_FLOAT)
> -     {
> -	sorry (\"%qs and floating point code\", \"-mgeneral-regs-only\");
> -	FAIL;
> -     }
> +      aarch64_err_no_fpadvsimd (TFmode, \"code\");

Likewise.

>  
>      if (GET_CODE (operands[0]) == MEM)
>        operands[1] = force_reg (TFmode, operands[1]);
> diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c
> new file mode 100644
> index 0000000..b5192a6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c
> @@ -0,0 +1,10 @@
> +/* { dg-options "-mgeneral-regs-only" } */
> +
> +typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
> +
> +/* { dg-error "'-mgeneral-regs-only' is incompatible with vector return type" "" {target "aarch64*-*-*"} 7 } */
> +/* { dg-error "'-mgeneral-regs-only' is incompatible with vector argument" "" {target "aarch64*-*-*"} 7 } */
> +int32x2_t test (int32x2_t a, int32x2_t b)
> +{
> +  return a + b;
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c
> new file mode 100644
> index 0000000..8590199
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c
> @@ -0,0 +1,15 @@
> +/* { dg-options "-mgeneral-regs-only" } */
> +
> +#include <stdarg.h>
> +
> +typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
> +
> +int
> +test (int i, ...)
> +{
> +  va_list argp;
> +  va_start (argp, i);
> +  int32x2_t x = (int32x2_t) {0, 1};
> +  x += va_arg (argp, int32x2_t); /* { dg-error "'-mgeneral-regs-only' is incompatible with vector varargs" } */
> +  return x[0] + x[1];
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/nofp_1.c b/gcc/testsuite/gcc.target/aarch64/nofp_1.c
> new file mode 100644
> index 0000000..3fc0036
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/nofp_1.c
> @@ -0,0 +1,19 @@
> +/* { dg-skip-if "conflicting -march" { *-*-* } { "-march=*" } { "-march=*+nofp" } } */
> +/* If there are multiple -march's, the latest wins; skip the test either way.
> +   -march overrides -mcpu, so there is no possibility of conflict.  */
> +
> +/* { dg-options "-march=armv8-a+nofp" } */
> +
> +#include <stdarg.h>
> +
> +typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
> +
> +int test (int i, ...);
> +
> +int
> +main (int argc, char **argv)
> +{
> +  int32x2_t a = (int32x2_t) {0, 1};
> +  int32x2_t b = (int32x2_t) {2, 3};
> +  return test (2, a, b); /* { dg-error "'\\+nofp' feature modifier is incompatible with vector argument" } */
> +}

<---->

> commit 298595e5254183de5b4c1cd2acaed43949b4dd30
> Author: Alan Lawrence <alan.lawrence@arm.com>
> Date:   Mon Jan 19 12:18:02 2015 +0000
> 
>     doc/invoke.texi, as approved, with whitespace.
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index e25bd62..0d62edf 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -12359,7 +12359,10 @@ Generate big-endian code.  This is the default when GCC is configured for an
>  
>  @item -mgeneral-regs-only
>  @opindex mgeneral-regs-only
> -Generate code which uses only the general registers.
> +Generate code which uses only the general registers.  Equivalent to feature

The ARMARM uses "general-purpose registers" to refer to these registers,
we should match that style.

s/Equivalent to feature/This is equivalent to the feature/

> +modifier @option{nofp} of @option{-march} or @option{-mcpu}, except that
> +@option{-mgeneral-regs-only} takes precedence over any conflicting feature
> +modifier regardless of sequence.
>  
>  @item -mlittle-endian
>  @opindex mlittle-endian
> @@ -12498,22 +12501,28 @@ over the appropriate part of this option.
>  @subsubsection @option{-march} and @option{-mcpu} Feature Modifiers
>  @cindex @option{-march} feature modifiers
>  @cindex @option{-mcpu} feature modifiers
> -Feature modifiers used with @option{-march} and @option{-mcpu} can be one
> -the following:
> +Feature modifiers used with @option{-march} and @option{-mcpu} can be any of
> +the following, or their inverses @option{no@var{feature}}:

s/inverses/inverse/

>  @table @samp
>  @item crc
>  Enable CRC extension.
>  @item crypto
> -Enable Crypto extension.  This implies Advanced SIMD is enabled.
> +Enable Crypto extension.  This also enables Advanced SIMD and floating-point
> +instructions.
>  @item fp
> -Enable floating-point instructions.
> +Enable floating-point instructions.  This is on by default for all possible
> +values for options @option{-march} and @option{-mcpu}.
>  @item simd
> -Enable Advanced SIMD instructions.  This implies floating-point instructions
> -are enabled.  This is the default for all current possible values for options
> -@option{-march} and @option{-mcpu=}.
> +Enable Advanced SIMD instructions.  This also enables floating-point
> +instructions.  This is on by default for all possible values for options
> +@option{-march} and @option{-mcpu}.
>  @end table
>  
> +As stated above, @option{crypto} implies @option{simd} implies @option{fp}.

Drop the "As stated above".

> +Conversely, @option{nofp} (or equivalently, @option{-mgeneral-regs-only})
> +implies @option{nosimd} implies @option{nocrypto}.
> +
>  @node Adapteva Epiphany Options
>  @subsection Adapteva Epiphany Options
>  

Thanks,
James

[PATCH 1/3][AArch64 nofp] Fix ICEs with +nofp/-mgeneral-regs-only and improve error messages

[Alan Lawrence]

[-- Attachment #1: Type: text/plain, Size: 3847 bytes --]

James Greenhalgh wrote:
> Submissions on this list should be one patch per mail, it makes
> tracking review easier.

OK here's a respin of the first, I've added a third patch after I found another 
route to get to an ICE.

>> +void
>> +aarch64_err_no_fpadvsimd (machine_mode mode, const char *msg)
>> +{
>> +  const char *mc = FLOAT_MODE_P (mode) ? "floating point" : "vector";
> 
> GCC coding conventions, this should be
> floating-point (https://gcc.gnu.org/codingconventions.html).

Done.

>> +  if (!TARGET_FLOAT
>> +      && fndecl && TREE_PUBLIC (fndecl)
>> +      && fntype && fntype != error_mark_node)
>> +    {
>> +      const_tree args, type = TREE_TYPE (fntype);
>> +      machine_mode mode; /* To pass pointer as argument; never used.  */
>> +      int nregs; /* Likewise.  */
> 
> Do these need annotations to avoid errors in a Werror build? I don't see
> any mention of what testing this patch has been through?

Dropped the args, added ATTRIBUTE_UNUSED to the others - the attribute isn't 
necessary at the moment but might become so if inlining became more aggressive.

This version has been bootstrapped on aarch64 linux.

>> -      if (cum->aapcs_nvrn > 0)
>> -	sorry ("%qs and floating point or vector arguments",
>> -	       "-mgeneral-regs-only");
>> +      gcc_assert (cum->aapcs_nvrn == 0);
> 
> This promotes an error to an ICE? Can we really never get to this
> point through the error control flow

Indeed - the new checks in init_cumulative_args and aarch64_layout_arg mean we 
never get here. (If said new checks were sorry(), we would still get here, but 
since they are error() we do not.)

>> @@ -7920,9 +7941,7 @@ aarch64_setup_incoming_varargs (cumulative_args_t cum_v, machine_mode mode,
>>  
>>    if (!TARGET_FLOAT)
>>      {
>> -      if (local_cum.aapcs_nvrn > 0)
>> -	sorry ("%qs and floating point or vector arguments",
>> -	       "-mgeneral-regs-only");
>> +      gcc_assert (local_cum.aapcs_nvrn == 0);
> 
> As above?

Similarly because of change from sorry() -> error().

>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index 11123d6..99cefec 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -981,10 +981,7 @@
>>    ""
>>    "
>>      if (!TARGET_FLOAT)
>> -     {
>> -	sorry (\"%qs and floating point code\", \"-mgeneral-regs-only\");
>> -	FAIL;
>> -     }
>> +      aarch64_err_no_fpadvsimd (<MODE>mode, \"code\");
> 
> You've dropped the FAIL?

(*2)

This usually gets called from emit_move_insn_1, via a call to emit_insn 
(GEN_FCN...). If we FAIL, we return NULL, and emit_insn then returns whatever 
insn was last in the BB; if we don't FAIL, we return a move to a general 
register (which is still a valid bit of RTL!). So either seems valid, but 
keeping the FAIL generates fewer instances of the error message, which can 
already get quite numerous. So reinstated FAIL. (Also changed "" quotes to {} 
braces.)

Bootstrap + check-gcc on aarch64-none-linux-gnu.

(ChangeLog's identical to v1)

gcc/ChangeLog:

	* config/aarch64/aarch64-protos.h (aarch64_err_no_fpadvsimd): New.

	* config/aarch64/aarch64.md (mov<mode>/GPF, movtf): Use
	aarch64_err_no_fpadvsimd.

	* config/aarch64/aarch64.c (aarch64_err_no_fpadvsimd): New.
	(aarch64_layout_arg, aarch64_init_cumulative_args): Use
	aarch64_err_no_fpadvsimd if !TARGET_FLOAT and we need FP regs.
	(aarch64_expand_builtin_va_start, aarch64_setup_incoming_varargs):
	Turn error into assert, test TARGET_FLOAT.
	(aarch64_gimplify_va_arg_expr): Use aarch64_err_no_fpadvsimd, test
	TARGET_FLOAT.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/mgeneral-regs_1.c: New file.
	* gcc.target/aarch64/mgeneral-regs_2.c: New file.
	* gcc.target/aarch64/nofp_1.c: New file.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: genregs-1-v2.patch --]
[-- Type: text/x-patch; name=genregs-1-v2.patch, Size: 7359 bytes --]

diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 965a11b7beeaaaa188819796e2b17017a87dca80..ac92c5924a4cfc5941fe8eeb31281e18bd21a5a0 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -259,6 +259,7 @@ unsigned aarch64_dbx_register_number (unsigned);
 unsigned aarch64_trampoline_size (void);
 void aarch64_asm_output_labelref (FILE *, const char *);
 void aarch64_elf_asm_named_section (const char *, unsigned, tree);
+void aarch64_err_no_fpadvsimd (machine_mode, const char *);
 void aarch64_expand_epilogue (bool);
 void aarch64_expand_mov_immediate (rtx, rtx);
 void aarch64_expand_prologue (void);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index a79bb6a96572799181a5bff3c3818e294f87cb7a..3193a15970e5524e0f3a8a5505baea5582e55731 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -522,6 +522,16 @@ static const char * const aarch64_condition_codes[] =
   "hi", "ls", "ge", "lt", "gt", "le", "al", "nv"
 };
 
+void
+aarch64_err_no_fpadvsimd (machine_mode mode, const char *msg)
+{
+  const char *mc = FLOAT_MODE_P (mode) ? "floating-point" : "vector";
+  if (TARGET_GENERAL_REGS_ONLY)
+    error ("%qs is incompatible with %s %s", "-mgeneral-regs-only", mc, msg);
+  else
+    error ("%qs feature modifier is incompatible with %s %s", "+nofp", mc, msg);
+}
+
 static unsigned int
 aarch64_min_divisions_for_recip_mul (enum machine_mode mode)
 {
@@ -1772,6 +1782,9 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
      and homogenous short-vector aggregates (HVA).  */
   if (allocate_nvrn)
     {
+      if (!TARGET_FLOAT)
+	aarch64_err_no_fpadvsimd (mode, "argument");
+
       if (nvrn + nregs <= NUM_FP_ARG_REGS)
 	{
 	  pcum->aapcs_nextnvrn = nvrn + nregs;
@@ -1898,6 +1911,17 @@ aarch64_init_cumulative_args (CUMULATIVE_ARGS *pcum,
   pcum->aapcs_stack_words = 0;
   pcum->aapcs_stack_size = 0;
 
+  if (!TARGET_FLOAT
+      && fndecl && TREE_PUBLIC (fndecl)
+      && fntype && fntype != error_mark_node)
+    {
+      const_tree type = TREE_TYPE (fntype);
+      machine_mode mode ATTRIBUTE_UNUSED; /* To pass pointer as argument.  */
+      int nregs ATTRIBUTE_UNUSED; /* Likewise.  */
+      if (aarch64_vfp_is_call_or_return_candidate (TYPE_MODE (type), type,
+						   &mode, &nregs, NULL))
+	aarch64_err_no_fpadvsimd (TYPE_MODE (type), "return type");
+    }
   return;
 }
 
@@ -7557,9 +7581,7 @@ aarch64_expand_builtin_va_start (tree valist, rtx nextarg ATTRIBUTE_UNUSED)
 
   if (!TARGET_FLOAT)
     {
-      if (cum->aapcs_nvrn > 0)
-	sorry ("%qs and floating point or vector arguments",
-	       "-mgeneral-regs-only");
+      gcc_assert (cum->aapcs_nvrn == 0);
       vr_save_area_size = 0;
     }
 
@@ -7666,8 +7688,7 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p,
     {
       /* TYPE passed in fp/simd registers.  */
       if (!TARGET_FLOAT)
-	sorry ("%qs and floating point or vector arguments",
-	       "-mgeneral-regs-only");
+	aarch64_err_no_fpadvsimd (mode, "varargs");
 
       f_top = build3 (COMPONENT_REF, TREE_TYPE (f_vrtop),
 		      unshare_expr (valist), f_vrtop, NULL_TREE);
@@ -7904,9 +7925,7 @@ aarch64_setup_incoming_varargs (cumulative_args_t cum_v, machine_mode mode,
 
   if (!TARGET_FLOAT)
     {
-      if (local_cum.aapcs_nvrn > 0)
-	sorry ("%qs and floating point or vector arguments",
-	       "-mgeneral-regs-only");
+      gcc_assert (local_cum.aapcs_nvrn == 0);
       vr_saved = 0;
     }
 
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 1efe57c91b10e47ab7511d089f7b4bb53f18f06e..a9c41fdfee64784591a4a7360652d7da3e7f90d1 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -979,16 +979,16 @@
   [(set (match_operand:GPF 0 "nonimmediate_operand" "")
 	(match_operand:GPF 1 "general_operand" ""))]
   ""
-  "
+  {
     if (!TARGET_FLOAT)
-     {
-	sorry (\"%qs and floating point code\", \"-mgeneral-regs-only\");
+      {
+	aarch64_err_no_fpadvsimd (<MODE>mode, "code");
 	FAIL;
-     }
+      }
 
     if (GET_CODE (operands[0]) == MEM)
       operands[1] = force_reg (<MODE>mode, operands[1]);
-  "
+  }
 )
 
 (define_insn "*movsf_aarch64"
@@ -1033,18 +1033,18 @@
   [(set (match_operand:TF 0 "nonimmediate_operand" "")
 	(match_operand:TF 1 "general_operand" ""))]
   ""
-  "
+  {
     if (!TARGET_FLOAT)
-     {
-	sorry (\"%qs and floating point code\", \"-mgeneral-regs-only\");
+      {
+	aarch64_err_no_fpadvsimd (TFmode, "code");
 	FAIL;
-     }
+      }
 
     if (GET_CODE (operands[0]) == MEM
         && ! (GET_CODE (operands[1]) == CONST_DOUBLE
 	      && aarch64_float_const_zero_rtx_p (operands[1])))
       operands[1] = force_reg (TFmode, operands[1]);
-  "
+  }
 )
 
 (define_insn "*movtf_aarch64"
diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..b5192a6a4838c14fcf0d00cd4d33b7c7255abd80
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c
@@ -0,0 +1,10 @@
+/* { dg-options "-mgeneral-regs-only" } */
+
+typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
+
+/* { dg-error "'-mgeneral-regs-only' is incompatible with vector return type" "" {target "aarch64*-*-*"} 7 } */
+/* { dg-error "'-mgeneral-regs-only' is incompatible with vector argument" "" {target "aarch64*-*-*"} 7 } */
+int32x2_t test (int32x2_t a, int32x2_t b)
+{
+  return a + b;
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c
new file mode 100644
index 0000000000000000000000000000000000000000..859019970ae044d8b36a095329c9e47a2826a399
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c
@@ -0,0 +1,15 @@
+/* { dg-options "-mgeneral-regs-only" } */
+
+#include <stdarg.h>
+
+typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
+
+int
+test (int i, ...)
+{
+  va_list argp;
+  va_start (argp, i);
+  int32x2_t x = (int32x2_t) {0, 1};
+  x += va_arg (argp, int32x2_t); /* { dg-error "'-mgeneral-regs-only' is incompatible with vector varargs" } */
+  return x[0] + x[1];
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/nofp_1.c b/gcc/testsuite/gcc.target/aarch64/nofp_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3fc00368668e2d9cfbf3b6c83d72cb9ebbb84821
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/nofp_1.c
@@ -0,0 +1,19 @@
+/* { dg-skip-if "conflicting -march" { *-*-* } { "-march=*" } { "-march=*+nofp" } } */
+/* If there are multiple -march's, the latest wins; skip the test either way.
+   -march overrides -mcpu, so there is no possibility of conflict.  */
+
+/* { dg-options "-march=armv8-a+nofp" } */
+
+#include <stdarg.h>
+
+typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
+
+int test (int i, ...);
+
+int
+main (int argc, char **argv)
+{
+  int32x2_t a = (int32x2_t) {0, 1};
+  int32x2_t b = (int32x2_t) {2, 3};
+  return test (2, a, b); /* { dg-error "'\\+nofp' feature modifier is incompatible with vector argument" } */
+}

Re: [PATCH 1/3][AArch64 nofp] Fix ICEs with +nofp/-mgeneral-regs-only and improve error messages

[James Greenhalgh]

On Tue, Jun 23, 2015 at 05:02:46PM +0100, Alan Lawrence wrote:
> James Greenhalgh wrote:
 
> Bootstrap + check-gcc on aarch64-none-linux-gnu.
> 
> (ChangeLog's identical to v1)
> 
> gcc/ChangeLog:
> 
> 	* config/aarch64/aarch64-protos.h (aarch64_err_no_fpadvsimd): New.
> 
> 	* config/aarch64/aarch64.md (mov<mode>/GPF, movtf): Use
> 	aarch64_err_no_fpadvsimd.
> 
> 	* config/aarch64/aarch64.c (aarch64_err_no_fpadvsimd): New.
> 	(aarch64_layout_arg, aarch64_init_cumulative_args): Use
> 	aarch64_err_no_fpadvsimd if !TARGET_FLOAT and we need FP regs.
> 	(aarch64_expand_builtin_va_start, aarch64_setup_incoming_varargs):
> 	Turn error into assert, test TARGET_FLOAT.
> 	(aarch64_gimplify_va_arg_expr): Use aarch64_err_no_fpadvsimd, test
> 	TARGET_FLOAT.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/aarch64/mgeneral-regs_1.c: New file.
> 	* gcc.target/aarch64/mgeneral-regs_2.c: New file.
> 	* gcc.target/aarch64/nofp_1.c: New file.


OK.

Thanks,
James

> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 965a11b7beeaaaa188819796e2b17017a87dca80..ac92c5924a4cfc5941fe8eeb31281e18bd21a5a0 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -259,6 +259,7 @@ unsigned aarch64_dbx_register_number (unsigned);
>  unsigned aarch64_trampoline_size (void);
>  void aarch64_asm_output_labelref (FILE *, const char *);
>  void aarch64_elf_asm_named_section (const char *, unsigned, tree);
> +void aarch64_err_no_fpadvsimd (machine_mode, const char *);
>  void aarch64_expand_epilogue (bool);
>  void aarch64_expand_mov_immediate (rtx, rtx);
>  void aarch64_expand_prologue (void);
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index a79bb6a96572799181a5bff3c3818e294f87cb7a..3193a15970e5524e0f3a8a5505baea5582e55731 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -522,6 +522,16 @@ static const char * const aarch64_condition_codes[] =
>    "hi", "ls", "ge", "lt", "gt", "le", "al", "nv"
>  };
>  
> +void
> +aarch64_err_no_fpadvsimd (machine_mode mode, const char *msg)
> +{
> +  const char *mc = FLOAT_MODE_P (mode) ? "floating-point" : "vector";
> +  if (TARGET_GENERAL_REGS_ONLY)
> +    error ("%qs is incompatible with %s %s", "-mgeneral-regs-only", mc, msg);
> +  else
> +    error ("%qs feature modifier is incompatible with %s %s", "+nofp", mc, msg);
> +}
> +
>  static unsigned int
>  aarch64_min_divisions_for_recip_mul (enum machine_mode mode)
>  {
> @@ -1772,6 +1782,9 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
>       and homogenous short-vector aggregates (HVA).  */
>    if (allocate_nvrn)
>      {
> +      if (!TARGET_FLOAT)
> +	aarch64_err_no_fpadvsimd (mode, "argument");
> +
>        if (nvrn + nregs <= NUM_FP_ARG_REGS)
>  	{
>  	  pcum->aapcs_nextnvrn = nvrn + nregs;
> @@ -1898,6 +1911,17 @@ aarch64_init_cumulative_args (CUMULATIVE_ARGS *pcum,
>    pcum->aapcs_stack_words = 0;
>    pcum->aapcs_stack_size = 0;
>  
> +  if (!TARGET_FLOAT
> +      && fndecl && TREE_PUBLIC (fndecl)
> +      && fntype && fntype != error_mark_node)
> +    {
> +      const_tree type = TREE_TYPE (fntype);
> +      machine_mode mode ATTRIBUTE_UNUSED; /* To pass pointer as argument.  */
> +      int nregs ATTRIBUTE_UNUSED; /* Likewise.  */
> +      if (aarch64_vfp_is_call_or_return_candidate (TYPE_MODE (type), type,
> +						   &mode, &nregs, NULL))
> +	aarch64_err_no_fpadvsimd (TYPE_MODE (type), "return type");
> +    }
>    return;
>  }
>  
> @@ -7557,9 +7581,7 @@ aarch64_expand_builtin_va_start (tree valist, rtx nextarg ATTRIBUTE_UNUSED)
>  
>    if (!TARGET_FLOAT)
>      {
> -      if (cum->aapcs_nvrn > 0)
> -	sorry ("%qs and floating point or vector arguments",
> -	       "-mgeneral-regs-only");
> +      gcc_assert (cum->aapcs_nvrn == 0);
>        vr_save_area_size = 0;
>      }
>  
> @@ -7666,8 +7688,7 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p,
>      {
>        /* TYPE passed in fp/simd registers.  */
>        if (!TARGET_FLOAT)
> -	sorry ("%qs and floating point or vector arguments",
> -	       "-mgeneral-regs-only");
> +	aarch64_err_no_fpadvsimd (mode, "varargs");
>  
>        f_top = build3 (COMPONENT_REF, TREE_TYPE (f_vrtop),
>  		      unshare_expr (valist), f_vrtop, NULL_TREE);
> @@ -7904,9 +7925,7 @@ aarch64_setup_incoming_varargs (cumulative_args_t cum_v, machine_mode mode,
>  
>    if (!TARGET_FLOAT)
>      {
> -      if (local_cum.aapcs_nvrn > 0)
> -	sorry ("%qs and floating point or vector arguments",
> -	       "-mgeneral-regs-only");
> +      gcc_assert (local_cum.aapcs_nvrn == 0);
>        vr_saved = 0;
>      }
>  
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 1efe57c91b10e47ab7511d089f7b4bb53f18f06e..a9c41fdfee64784591a4a7360652d7da3e7f90d1 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -979,16 +979,16 @@
>    [(set (match_operand:GPF 0 "nonimmediate_operand" "")
>  	(match_operand:GPF 1 "general_operand" ""))]
>    ""
> -  "
> +  {
>      if (!TARGET_FLOAT)
> -     {
> -	sorry (\"%qs and floating point code\", \"-mgeneral-regs-only\");
> +      {
> +	aarch64_err_no_fpadvsimd (<MODE>mode, "code");
>  	FAIL;
> -     }
> +      }
>  
>      if (GET_CODE (operands[0]) == MEM)
>        operands[1] = force_reg (<MODE>mode, operands[1]);
> -  "
> +  }
>  )
>  
>  (define_insn "*movsf_aarch64"
> @@ -1033,18 +1033,18 @@
>    [(set (match_operand:TF 0 "nonimmediate_operand" "")
>  	(match_operand:TF 1 "general_operand" ""))]
>    ""
> -  "
> +  {
>      if (!TARGET_FLOAT)
> -     {
> -	sorry (\"%qs and floating point code\", \"-mgeneral-regs-only\");
> +      {
> +	aarch64_err_no_fpadvsimd (TFmode, "code");
>  	FAIL;
> -     }
> +      }
>  
>      if (GET_CODE (operands[0]) == MEM
>          && ! (GET_CODE (operands[1]) == CONST_DOUBLE
>  	      && aarch64_float_const_zero_rtx_p (operands[1])))
>        operands[1] = force_reg (TFmode, operands[1]);
> -  "
> +  }
>  )
>  
>  (define_insn "*movtf_aarch64"
> diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b5192a6a4838c14fcf0d00cd4d33b7c7255abd80
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c
> @@ -0,0 +1,10 @@
> +/* { dg-options "-mgeneral-regs-only" } */
> +
> +typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
> +
> +/* { dg-error "'-mgeneral-regs-only' is incompatible with vector return type" "" {target "aarch64*-*-*"} 7 } */
> +/* { dg-error "'-mgeneral-regs-only' is incompatible with vector argument" "" {target "aarch64*-*-*"} 7 } */
> +int32x2_t test (int32x2_t a, int32x2_t b)
> +{
> +  return a + b;
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..859019970ae044d8b36a095329c9e47a2826a399
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c
> @@ -0,0 +1,15 @@
> +/* { dg-options "-mgeneral-regs-only" } */
> +
> +#include <stdarg.h>
> +
> +typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
> +
> +int
> +test (int i, ...)
> +{
> +  va_list argp;
> +  va_start (argp, i);
> +  int32x2_t x = (int32x2_t) {0, 1};
> +  x += va_arg (argp, int32x2_t); /* { dg-error "'-mgeneral-regs-only' is incompatible with vector varargs" } */
> +  return x[0] + x[1];
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/nofp_1.c b/gcc/testsuite/gcc.target/aarch64/nofp_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..3fc00368668e2d9cfbf3b6c83d72cb9ebbb84821
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/nofp_1.c
> @@ -0,0 +1,19 @@
> +/* { dg-skip-if "conflicting -march" { *-*-* } { "-march=*" } { "-march=*+nofp" } } */
> +/* If there are multiple -march's, the latest wins; skip the test either way.
> +   -march overrides -mcpu, so there is no possibility of conflict.  */
> +
> +/* { dg-options "-march=armv8-a+nofp" } */
> +
> +#include <stdarg.h>
> +
> +typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
> +
> +int test (int i, ...);
> +
> +int
> +main (int argc, char **argv)
> +{
> +  int32x2_t a = (int32x2_t) {0, 1};
> +  int32x2_t b = (int32x2_t) {2, 3};
> +  return test (2, a, b); /* { dg-error "'\\+nofp' feature modifier is incompatible with vector argument" } */
> +}

[PATCH 2/3][AArch64 nofp] Clarify docs for +nofp/-mgeneral-regs-only

[Alan Lawrence]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]

James Greenhalgh wrote:
> 
>> -Generate code which uses only the general registers.
>> +Generate code which uses only the general registers.  Equivalent to feature
> 
> The ARMARM uses "general-purpose registers" to refer to these registers,
> we should match that style.
> 
> s/Equivalent to feature/This is equivalent to the feature/

Done.

>> -Feature modifiers used with @option{-march} and @option{-mcpu} can be one
>> -the following:
>> +Feature modifiers used with @option{-march} and @option{-mcpu} can be any of
>> +the following, or their inverses @option{no@var{feature}}:
> 
> s/inverses/inverse/

The grammar is quite difficult here, so have gone for "and their inverses" as 
the set of possibilities definitely includes 3 inverses.

>>  
>> +As stated above, @option{crypto} implies @option{simd} implies @option{fp}.
> 
> Drop the "As stated above".

To my eye, beginning a sentence in lowercase looks very odd in pdf, and still a 
bit odd in html. Have changed to "That is"...?

Tested with make pdf & make html.

gcc/ChangeLog (unchanged):

	* doc/invoke.texi: Clarify AArch64 feature modifiers (no)fp, (no)simd
	and (no)crypto.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: genregs-2-v2.patch --]
[-- Type: text/x-patch; name=genregs-2-v2.patch, Size: 2506 bytes --]

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d8e982c3aa338819df3785696c493a66c1f5b674..0579bf2ecf993bb56987e0bb9686925537ab61e3 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12359,7 +12359,10 @@ Generate big-endian code.  This is the default when GCC is configured for an
 
 @item -mgeneral-regs-only
 @opindex mgeneral-regs-only
-Generate code which uses only the general registers.
+Generate code which uses only the general-purpose registers.  This is equivalent
+to feature modifier @option{nofp} of @option{-march} or @option{-mcpu}, except
+that @option{-mgeneral-regs-only} takes precedence over any conflicting feature
+modifier regardless of sequence.
 
 @item -mlittle-endian
 @opindex mlittle-endian
@@ -12498,20 +12501,22 @@ over the appropriate part of this option.
 @subsubsection @option{-march} and @option{-mcpu} Feature Modifiers
 @cindex @option{-march} feature modifiers
 @cindex @option{-mcpu} feature modifiers
-Feature modifiers used with @option{-march} and @option{-mcpu} can be one
-the following:
+Feature modifiers used with @option{-march} and @option{-mcpu} can be any of
+the following and their inverses @option{no@var{feature}}:
 
 @table @samp
 @item crc
 Enable CRC extension.
 @item crypto
-Enable Crypto extension.  This implies Advanced SIMD is enabled.
+Enable Crypto extension.  This also enables Advanced SIMD and floating-point
+instructions.
 @item fp
-Enable floating-point instructions.
+Enable floating-point instructions.  This is on by default for all possible
+values for options @option{-march} and @option{-mcpu}.
 @item simd
-Enable Advanced SIMD instructions.  This implies floating-point instructions
-are enabled.  This is the default for all current possible values for options
-@option{-march} and @option{-mcpu=}.
+Enable Advanced SIMD instructions.  This also enables floating-point
+instructions.  This is on by default for all possible values for options
+@option{-march} and @option{-mcpu}.
 @item lse
 Enable Large System Extension instructions.
 @item pan
@@ -12522,6 +12527,10 @@ Enable Limited Ordering Regions support.
 Enable ARMv8.1 Advanced SIMD instructions.
 @end table
 
+That is, @option{crypto} implies @option{simd} implies @option{fp}.
+Conversely, @option{nofp} (or equivalently, @option{-mgeneral-regs-only})
+implies @option{nosimd} implies @option{nocrypto}.
+
 @node Adapteva Epiphany Options
 @subsection Adapteva Epiphany Options
 

Re: [PATCH 2/3][AArch64 nofp] Clarify docs for +nofp/-mgeneral-regs-only

[James Greenhalgh]

On Tue, Jun 23, 2015 at 05:03:13PM +0100, Alan Lawrence wrote:
> James Greenhalgh wrote:
<<snip>>

> To my eye, beginning a sentence in lowercase looks very odd in pdf, and still a 
> bit odd in html. Have changed to "That is"...?
> 
> Tested with make pdf & make html.
> 
> gcc/ChangeLog (unchanged):
> 
> 	* doc/invoke.texi: Clarify AArch64 feature modifiers (no)fp, (no)simd
> 	and (no)crypto.

OK.

Thanks,
James

> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index d8e982c3aa338819df3785696c493a66c1f5b674..0579bf2ecf993bb56987e0bb9686925537ab61e3 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -12359,7 +12359,10 @@ Generate big-endian code.  This is the default when GCC is configured for an
>  
>  @item -mgeneral-regs-only
>  @opindex mgeneral-regs-only
> -Generate code which uses only the general registers.
> +Generate code which uses only the general-purpose registers.  This is equivalent
> +to feature modifier @option{nofp} of @option{-march} or @option{-mcpu}, except
> +that @option{-mgeneral-regs-only} takes precedence over any conflicting feature
> +modifier regardless of sequence.
>  
>  @item -mlittle-endian
>  @opindex mlittle-endian
> @@ -12498,20 +12501,22 @@ over the appropriate part of this option.
>  @subsubsection @option{-march} and @option{-mcpu} Feature Modifiers
>  @cindex @option{-march} feature modifiers
>  @cindex @option{-mcpu} feature modifiers
> -Feature modifiers used with @option{-march} and @option{-mcpu} can be one
> -the following:
> +Feature modifiers used with @option{-march} and @option{-mcpu} can be any of
> +the following and their inverses @option{no@var{feature}}:
>  
>  @table @samp
>  @item crc
>  Enable CRC extension.
>  @item crypto
> -Enable Crypto extension.  This implies Advanced SIMD is enabled.
> +Enable Crypto extension.  This also enables Advanced SIMD and floating-point
> +instructions.
>  @item fp
> -Enable floating-point instructions.
> +Enable floating-point instructions.  This is on by default for all possible
> +values for options @option{-march} and @option{-mcpu}.
>  @item simd
> -Enable Advanced SIMD instructions.  This implies floating-point instructions
> -are enabled.  This is the default for all current possible values for options
> -@option{-march} and @option{-mcpu=}.
> +Enable Advanced SIMD instructions.  This also enables floating-point
> +instructions.  This is on by default for all possible values for options
> +@option{-march} and @option{-mcpu}.
>  @item lse
>  Enable Large System Extension instructions.
>  @item pan
> @@ -12522,6 +12527,10 @@ Enable Limited Ordering Regions support.
>  Enable ARMv8.1 Advanced SIMD instructions.
>  @end table
>  
> +That is, @option{crypto} implies @option{simd} implies @option{fp}.
> +Conversely, @option{nofp} (or equivalently, @option{-mgeneral-regs-only})
> +implies @option{nosimd} implies @option{nocrypto}.
> +
>  @node Adapteva Epiphany Options
>  @subsection Adapteva Epiphany Options
>  

[PATCH 3/3][AArch64 nofp] Fix another ICE with +nofp/-mgeneral-regs-only

[Alan Lawrence]

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

This fixes another ICE, obtained with the attached testcase - yes, there was a 
way to get hold of a float, without passing an argument or going through 
movsf/movdf!

Bootstrapped + check-gcc on aarch64-none-linux-gnu.

gcc/ChangeLog:

	* config/aarch64/aarch64.md (<optab><fcvt_target><GPF:mode>2):
	Condition on TARGET_FLOAT.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/mgeneral-regs_3.c: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: genregs-3.patch --]
[-- Type: text/x-patch; name=genregs-3.patch, Size: 1160 bytes --]

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 99cefece8093791ccf17cb071a4e9997bda8fd89..bcaafda5ea46f136dc90f34aa8f2dfaddabd09f5 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4106,7 +4106,7 @@
 (define_insn "<optab><fcvt_target><GPF:mode>2"
   [(set (match_operand:GPF 0 "register_operand" "=w,w")
         (FLOATUORS:GPF (match_operand:<FCVT_TARGET> 1 "register_operand" "w,r")))]
-  ""
+  "TARGET_FLOAT"
   "@
    <su_optab>cvtf\t%<GPF:s>0, %<s>1
    <su_optab>cvtf\t%<GPF:s>0, %<w1>1"
diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c
new file mode 100644
index 0000000000000000000000000000000000000000..225d9eaa45530d88315a146f3fae72d86fe66373
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c
@@ -0,0 +1,11 @@
+/* { dg-options "-mgeneral-regs-only -O2" } */
+
+extern void abort (void);
+
+int
+test (int i, ...)
+{
+  float f = (float) i; /* { dg-error "'-mgeneral-regs-only' is incompatible with floating point code" } */
+  if (f != f) abort ();
+  return 2;
+}

Re: [PATCH 3/3][AArch64 nofp] Fix another ICE with +nofp/-mgeneral-regs-only

[James Greenhalgh]

On Tue, Jun 23, 2015 at 05:03:28PM +0100, Alan Lawrence wrote:
> This fixes another ICE, obtained with the attached testcase - yes, there was a 
> way to get hold of a float, without passing an argument or going through 
> movsf/movdf!
> 
> Bootstrapped + check-gcc on aarch64-none-linux-gnu.
> 
> gcc/ChangeLog:
> 
> 	* config/aarch64/aarch64.md (<optab><fcvt_target><GPF:mode>2):
> 	Condition on TARGET_FLOAT.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/aarch64/mgeneral-regs_3.c: New.

OK.

Thanks,
James

> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 99cefece8093791ccf17cb071a4e9997bda8fd89..bcaafda5ea46f136dc90f34aa8f2dfaddabd09f5 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -4106,7 +4106,7 @@
>  (define_insn "<optab><fcvt_target><GPF:mode>2"
>    [(set (match_operand:GPF 0 "register_operand" "=w,w")
>          (FLOATUORS:GPF (match_operand:<FCVT_TARGET> 1 "register_operand" "w,r")))]
> -  ""
> +  "TARGET_FLOAT"
>    "@
>     <su_optab>cvtf\t%<GPF:s>0, %<s>1
>     <su_optab>cvtf\t%<GPF:s>0, %<w1>1"
> diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..225d9eaa45530d88315a146f3fae72d86fe66373
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c
> @@ -0,0 +1,11 @@
> +/* { dg-options "-mgeneral-regs-only -O2" } */
> +
> +extern void abort (void);
> +
> +int
> +test (int i, ...)
> +{
> +  float f = (float) i; /* { dg-error "'-mgeneral-regs-only' is incompatible with floating point code" } */
> +  if (f != f) abort ();
> +  return 2;
> +}