diff mbox

[RFC,ARM] TARGET_ATOMIC_ASSIGN_EXPAND_FENV hook

Message ID 53635FBA.7060505@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah May 2, 2014, 9:04 a.m. UTC
On 02/05/14 10:15, Joseph S. Myers wrote:
> It doesn't seem a good idea to me for a host-side GCC file to use the FE_* 
> names for the target's FE_* values; you'd run into problems if that file 
> ever ends up including the host's <fenv.h>, directly or indirectly, on any 
> host.  The same comment applies to the AArch64 patch as well.
> 
> Instead I suggest names such as ARM_FE_* that won't conflict with the 
> host's system headers.
> 
Thanks for spotting it. Here is the updated patch that changes it to
ARM_FE_*.

Thanks,
Kugan


gcc/

+2014-05-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
+
+	* config/arm/arm.c (TARGET_ATOMIC_ASSIGN_EXPAND_FENV): New define.
+	(arm_builtins) : Add ARM_BUILTIN_GET_FPSCR and ARM_BUILTIN_SET_FPSCR.
+	(bdesc_2arg) : Add description for builtins __builtins_arm_set_fpscr
+	and __builtins_arm_get_fpscr.
+	(arm_init_builtins) : Initialize builtins __builtins_arm_set_fpscr and
+	__builtins_arm_get_fpscr.
+	(arm_expand_builtin) : Expand builtins __builtins_arm_set_fpscr and
+	__builtins_arm_ldfpscr.
+	(arm_atomic_assign_expand_fenv): New function.
+	* config/arm/vfp.md (set_fpscr): New pattern.
+	(get_fpscr) : Likewise.
+	* config/arm/unspecs.md (unspecv): Add VUNSPEC_GET_FPSCR and
+	VUNSPEC_SET_FPSCR.
+	* doc/extend.texi (AARCH64 Built-in Functions) : Document
+	__builtins_arm_set_fpscr, __builtins_arm_get_fpscr.
+

Comments

Kugan Vivekanandarajah May 11, 2014, 11:47 p.m. UTC | #1
Ping ?

Thanks,
Kugan

On 02/05/14 19:04, Kugan wrote:
> On 02/05/14 10:15, Joseph S. Myers wrote:
>> It doesn't seem a good idea to me for a host-side GCC file to use the FE_* 
>> names for the target's FE_* values; you'd run into problems if that file 
>> ever ends up including the host's <fenv.h>, directly or indirectly, on any 
>> host.  The same comment applies to the AArch64 patch as well.
>>
>> Instead I suggest names such as ARM_FE_* that won't conflict with the 
>> host's system headers.
>>
> Thanks for spotting it. Here is the updated patch that changes it to
> ARM_FE_*.
> 
> Thanks,
> Kugan
> 
> 
> gcc/
> 
> +2014-05-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
> +
> +	* config/arm/arm.c (TARGET_ATOMIC_ASSIGN_EXPAND_FENV): New define.
> +	(arm_builtins) : Add ARM_BUILTIN_GET_FPSCR and ARM_BUILTIN_SET_FPSCR.
> +	(bdesc_2arg) : Add description for builtins __builtins_arm_set_fpscr
> +	and __builtins_arm_get_fpscr.
> +	(arm_init_builtins) : Initialize builtins __builtins_arm_set_fpscr and
> +	__builtins_arm_get_fpscr.
> +	(arm_expand_builtin) : Expand builtins __builtins_arm_set_fpscr and
> +	__builtins_arm_ldfpscr.
> +	(arm_atomic_assign_expand_fenv): New function.
> +	* config/arm/vfp.md (set_fpscr): New pattern.
> +	(get_fpscr) : Likewise.
> +	* config/arm/unspecs.md (unspecv): Add VUNSPEC_GET_FPSCR and
> +	VUNSPEC_SET_FPSCR.
> +	* doc/extend.texi (AARCH64 Built-in Functions) : Document
> +	__builtins_arm_set_fpscr, __builtins_arm_get_fpscr.
> +
>
Kugan Vivekanandarajah May 26, 2014, 8:01 a.m. UTC | #2
Ping^2 ?

Thanks,
Kugan

On 12/05/14 09:47, Kugan wrote:
> Ping ?
> 
> Thanks,
> Kugan
> 
> On 02/05/14 19:04, Kugan wrote:
>> On 02/05/14 10:15, Joseph S. Myers wrote:
>>> It doesn't seem a good idea to me for a host-side GCC file to use the FE_* 
>>> names for the target's FE_* values; you'd run into problems if that file 
>>> ever ends up including the host's <fenv.h>, directly or indirectly, on any 
>>> host.  The same comment applies to the AArch64 patch as well.
>>>
>>> Instead I suggest names such as ARM_FE_* that won't conflict with the 
>>> host's system headers.
>>>
>> Thanks for spotting it. Here is the updated patch that changes it to
>> ARM_FE_*.
>>
>> Thanks,
>> Kugan
>>
>>
>> gcc/
>>
>> +2014-05-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
>> +
>> +	* config/arm/arm.c (TARGET_ATOMIC_ASSIGN_EXPAND_FENV): New define.
>> +	(arm_builtins) : Add ARM_BUILTIN_GET_FPSCR and ARM_BUILTIN_SET_FPSCR.
>> +	(bdesc_2arg) : Add description for builtins __builtins_arm_set_fpscr
>> +	and __builtins_arm_get_fpscr.
>> +	(arm_init_builtins) : Initialize builtins __builtins_arm_set_fpscr and
>> +	__builtins_arm_get_fpscr.
>> +	(arm_expand_builtin) : Expand builtins __builtins_arm_set_fpscr and
>> +	__builtins_arm_ldfpscr.
>> +	(arm_atomic_assign_expand_fenv): New function.
>> +	* config/arm/vfp.md (set_fpscr): New pattern.
>> +	(get_fpscr) : Likewise.
>> +	* config/arm/unspecs.md (unspecv): Add VUNSPEC_GET_FPSCR and
>> +	VUNSPEC_SET_FPSCR.
>> +	* doc/extend.texi (AARCH64 Built-in Functions) : Document
>> +	__builtins_arm_set_fpscr, __builtins_arm_get_fpscr.
>> +
>>
>
Ramana Radhakrishnan May 30, 2014, 8:35 a.m. UTC | #3
>+  if (!TARGET_VFP)
>+    return;
>+
>+  /* Generate the equivalence of :

s/equivalence/equivalent.

Ok with that change and if no regressions.

Ramana

On Mon, May 26, 2014 at 9:01 AM, Kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Ping^2 ?
>
> Thanks,
> Kugan
>
> On 12/05/14 09:47, Kugan wrote:
>> Ping ?
>>
>> Thanks,
>> Kugan
>>
>> On 02/05/14 19:04, Kugan wrote:
>>> On 02/05/14 10:15, Joseph S. Myers wrote:
>>>> It doesn't seem a good idea to me for a host-side GCC file to use the FE_*
>>>> names for the target's FE_* values; you'd run into problems if that file
>>>> ever ends up including the host's <fenv.h>, directly or indirectly, on any
>>>> host.  The same comment applies to the AArch64 patch as well.
>>>>
>>>> Instead I suggest names such as ARM_FE_* that won't conflict with the
>>>> host's system headers.
>>>>
>>> Thanks for spotting it. Here is the updated patch that changes it to
>>> ARM_FE_*.
>>>
>>> Thanks,
>>> Kugan
>>>
>>>
>>> gcc/
>>>
>>> +2014-05-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>> +
>>> +    * config/arm/arm.c (TARGET_ATOMIC_ASSIGN_EXPAND_FENV): New define.
>>> +    (arm_builtins) : Add ARM_BUILTIN_GET_FPSCR and ARM_BUILTIN_SET_FPSCR.
>>> +    (bdesc_2arg) : Add description for builtins __builtins_arm_set_fpscr
>>> +    and __builtins_arm_get_fpscr.
>>> +    (arm_init_builtins) : Initialize builtins __builtins_arm_set_fpscr and
>>> +    __builtins_arm_get_fpscr.
>>> +    (arm_expand_builtin) : Expand builtins __builtins_arm_set_fpscr and
>>> +    __builtins_arm_ldfpscr.
>>> +    (arm_atomic_assign_expand_fenv): New function.
>>> +    * config/arm/vfp.md (set_fpscr): New pattern.
>>> +    (get_fpscr) : Likewise.
>>> +    * config/arm/unspecs.md (unspecv): Add VUNSPEC_GET_FPSCR and
>>> +    VUNSPEC_SET_FPSCR.
>>> +    * doc/extend.texi (AARCH64 Built-in Functions) : Document
>>> +    __builtins_arm_set_fpscr, __builtins_arm_get_fpscr.
>>> +
>>>
>>
Jay Foad June 23, 2014, 12:32 p.m. UTC | #4
On 2 May 2014 10:04, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
> Thanks for spotting it. Here is the updated patch that changes it to
> ARM_FE_*.

> +2014-05-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
> +
> +       * config/arm/arm.c (TARGET_ATOMIC_ASSIGN_EXPAND_FENV): New define.
> +       (arm_builtins) : Add ARM_BUILTIN_GET_FPSCR and ARM_BUILTIN_SET_FPSCR.
> +       (bdesc_2arg) : Add description for builtins __builtins_arm_set_fpscr
> +       and __builtins_arm_get_fpscr.

s/__builtins/__builtin/g

> +       (arm_init_builtins) : Initialize builtins __builtins_arm_set_fpscr and
> +       __builtins_arm_get_fpscr.

s/__builtins/__builtin/g

This doesn't match the code, which initializes builtins "...ldfscr"
and "...stfscr" (with no "p" in "fscr").

> +       (arm_expand_builtin) : Expand builtins __builtins_arm_set_fpscr and
> +       __builtins_arm_ldfpscr.

s/__builtins/__builtin/g

Did you mean "and __builtin_arm_get_fpscr"?

> +#define FP_BUILTIN(L, U) \
> +  {0, CODE_FOR_##L, "__builtin_arm_"#L, ARM_BUILTIN_##U, \
> +   UNKNOWN, 0},
> +
> +  FP_BUILTIN (set_fpscr, GET_FPSCR)
> +  FP_BUILTIN (get_fpscr, SET_FPSCR)
> +#undef FP_BUILTIN

This looks like a typo: you have mapped set->GET and get->SET.

Jay.
diff mbox

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 0240cc7..b2d50f6 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -59,6 +59,7 @@ 
 #include "params.h"
 #include "opts.h"
 #include "dumpfile.h"
+#include "gimple-expr.h"
 
 /* Forward definitions of types.  */
 typedef struct minipool_node    Mnode;
@@ -93,6 +94,7 @@  static int thumb_far_jump_used_p (void);
 static bool thumb_force_lr_save (void);
 static unsigned arm_size_return_regs (void);
 static bool arm_assemble_integer (rtx, unsigned int, int);
+static void arm_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update);
 static void arm_print_operand (FILE *, rtx, int);
 static void arm_print_operand_address (FILE *, rtx);
 static bool arm_print_operand_punct_valid_p (unsigned char code);
@@ -584,6 +586,9 @@  static const struct attribute_spec arm_attribute_table[] =
 #undef TARGET_MANGLE_TYPE
 #define TARGET_MANGLE_TYPE arm_mangle_type
 
+#undef TARGET_ATOMIC_ASSIGN_EXPAND_FENV
+#define TARGET_ATOMIC_ASSIGN_EXPAND_FENV arm_atomic_assign_expand_fenv
+
 #undef TARGET_BUILD_BUILTIN_VA_LIST
 #define TARGET_BUILD_BUILTIN_VA_LIST arm_build_builtin_va_list
 #undef TARGET_EXPAND_BUILTIN_VA_START
@@ -23212,6 +23217,9 @@  enum arm_builtins
   ARM_BUILTIN_CRC32CH,
   ARM_BUILTIN_CRC32CW,
 
+  ARM_BUILTIN_GET_FPSCR,
+  ARM_BUILTIN_SET_FPSCR,
+
 #undef CRYPTO1
 #undef CRYPTO2
 #undef CRYPTO3
@@ -24010,6 +24018,15 @@  static const struct builtin_description bdesc_2arg[] =
   IWMMXT_BUILTIN2 (iwmmxt_wmacuz, WMACUZ)
   IWMMXT_BUILTIN2 (iwmmxt_wmacsz, WMACSZ)
 
+
+#define FP_BUILTIN(L, U) \
+  {0, CODE_FOR_##L, "__builtin_arm_"#L, ARM_BUILTIN_##U, \
+   UNKNOWN, 0},
+
+  FP_BUILTIN (set_fpscr, GET_FPSCR)
+  FP_BUILTIN (get_fpscr, SET_FPSCR)
+#undef FP_BUILTIN
+
 #define CRC32_BUILTIN(L, U) \
   {0, CODE_FOR_##L, "__builtin_arm_"#L, ARM_BUILTIN_##U, \
    UNKNOWN, 0},
@@ -24524,6 +24541,21 @@  arm_init_builtins (void)
 
   if (TARGET_CRC32)
     arm_init_crc32_builtins ();
+
+  if (TARGET_VFP)
+    {
+      tree ftype_set_fpscr
+	= build_function_type_list (void_type_node, unsigned_type_node, NULL);
+      tree ftype_get_fpscr
+	= build_function_type_list (unsigned_type_node, NULL);
+
+      arm_builtin_decls[ARM_BUILTIN_GET_FPSCR]
+	= add_builtin_function ("__builtin_arm_ldfscr", ftype_get_fpscr,
+				ARM_BUILTIN_GET_FPSCR, BUILT_IN_MD, NULL, NULL_TREE);
+      arm_builtin_decls[ARM_BUILTIN_SET_FPSCR]
+	= add_builtin_function ("__builtin_arm_stfscr", ftype_set_fpscr,
+				ARM_BUILTIN_SET_FPSCR, BUILT_IN_MD, NULL, NULL_TREE);
+    }
 }
 
 /* Return the ARM builtin for CODE.  */
@@ -25251,6 +25283,25 @@  arm_expand_builtin (tree exp,
 
   switch (fcode)
     {
+    case ARM_BUILTIN_GET_FPSCR:
+    case ARM_BUILTIN_SET_FPSCR:
+      if (fcode == ARM_BUILTIN_GET_FPSCR)
+	{
+	  icode = CODE_FOR_get_fpscr;
+	  target = gen_reg_rtx (SImode);
+	  pat = GEN_FCN (icode) (target);
+	}
+      else
+	{
+	  target = NULL_RTX;
+	  icode = CODE_FOR_set_fpscr;
+	  arg0 = CALL_EXPR_ARG (exp, 0);
+	  op0 = expand_normal (arg0);
+	  pat = GEN_FCN (icode) (op0);
+	}
+      emit_insn (pat);
+      return target;
+
     case ARM_BUILTIN_TEXTRMSB:
     case ARM_BUILTIN_TEXTRMUB:
     case ARM_BUILTIN_TEXTRMSH:
@@ -31116,4 +31167,73 @@  arm_asan_shadow_offset (void)
   return (unsigned HOST_WIDE_INT) 1 << 29;
 }
 
+static void
+arm_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
+{
+  const unsigned ARM_FE_INVALID = 1;
+  const unsigned ARM_FE_DIVBYZERO = 2;
+  const unsigned ARM_FE_OVERFLOW = 4;
+  const unsigned ARM_FE_UNDERFLOW = 8;
+  const unsigned ARM_FE_INEXACT = 16;
+  const unsigned HOST_WIDE_INT ARM_FE_ALL_EXCEPT = (ARM_FE_INVALID
+						    | ARM_FE_DIVBYZERO
+						    | ARM_FE_OVERFLOW
+						    | ARM_FE_UNDERFLOW
+						    | ARM_FE_INEXACT);
+  const unsigned HOST_WIDE_INT ARM_FE_EXCEPT_SHIFT = 8;
+  tree fenv_var, get_fpscr, set_fpscr, mask, ld_fenv, masked_fenv;
+  tree new_fenv_var, reload_fenv, restore_fnenv;
+  tree update_call, atomic_feraiseexcept, hold_fnclex;
+
+  if (!TARGET_VFP)
+    return;
+
+  /* Generate the equivalence of :
+       unsigned int fenv_var;
+       fenv_var = __builtin_arm_get_fpscr ();
+
+       unsigned int masked_fenv;
+       masked_fenv = fenv_var & mask;
+
+       __builtin_arm_set_fpscr (masked_fenv);  */
+
+  fenv_var = create_tmp_var (unsigned_type_node, NULL);
+  get_fpscr = arm_builtin_decls[ARM_BUILTIN_GET_FPSCR];
+  set_fpscr = arm_builtin_decls[ARM_BUILTIN_SET_FPSCR];
+  mask = build_int_cst (unsigned_type_node,
+			~((ARM_FE_ALL_EXCEPT << ARM_FE_EXCEPT_SHIFT)
+			  | ARM_FE_ALL_EXCEPT));
+  ld_fenv = build2 (MODIFY_EXPR, unsigned_type_node,
+		    fenv_var, build_call_expr (get_fpscr, 0));
+  masked_fenv = build2 (BIT_AND_EXPR, unsigned_type_node, fenv_var, mask);
+  hold_fnclex = build_call_expr (set_fpscr, 1, masked_fenv);
+  *hold = build2 (COMPOUND_EXPR, void_type_node,
+		  build2 (COMPOUND_EXPR, void_type_node, masked_fenv, ld_fenv),
+		  hold_fnclex);
+
+  /* Store the value of masked_fenv to clear the exceptions:
+     __builtin_arm_set_fpscr (masked_fenv);  */
+
+  *clear = build_call_expr (set_fpscr, 1, masked_fenv);
+
+  /* Generate the equivalent of :
+       unsigned int new_fenv_var;
+       new_fenv_var = __builtin_arm_get_fpscr ();
+
+       __builtin_arm_set_fpscr (fenv_var);
+
+       __atomic_feraiseexcept (new_fenv_var);  */
+
+  new_fenv_var = create_tmp_var (unsigned_type_node, NULL);
+  reload_fenv = build2 (MODIFY_EXPR, unsigned_type_node, new_fenv_var,
+			build_call_expr (get_fpscr, 0));
+  restore_fnenv = build_call_expr (set_fpscr, 1, fenv_var);
+  atomic_feraiseexcept = builtin_decl_implicit (BUILT_IN_ATOMIC_FERAISEEXCEPT);
+  update_call = build_call_expr (atomic_feraiseexcept, 1,
+				 fold_convert (integer_type_node, new_fenv_var));
+  *update = build2 (COMPOUND_EXPR, void_type_node,
+		    build2 (COMPOUND_EXPR, void_type_node,
+			    reload_fenv, restore_fnenv), update_call);
+}
+
 #include "gt-arm.h"
diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md
index 8caa953..147cb80 100644
--- a/gcc/config/arm/unspecs.md
+++ b/gcc/config/arm/unspecs.md
@@ -143,6 +143,8 @@ 
   VUNSPEC_SLX		; Represent a store-register-release-exclusive.
   VUNSPEC_LDA		; Represent a store-register-acquire.
   VUNSPEC_STL		; Represent a store-register-release.
+  VUNSPEC_GET_FPSCR	; Represent fetch of FPSCR content.
+  VUNSPEC_SET_FPSCR	; Represent assign of FPSCR content.
 ])
 
 ;; Enumerators for NEON unspecs.
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index e1a48ee..3c7744f 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -1322,6 +1322,22 @@ 
    (set_attr "conds" "unconditional")]
 )
 
+;; Write Floating-point Status and Control Register.
+(define_insn "set_fpscr"
+  [(unspec_volatile [(match_operand:SI 0 "register_operand" "r")] VUNSPEC_SET_FPSCR)]
+  "TARGET_VFP"
+  "mcr\\tp10, 7, %0, cr1, cr0, 0\\t @SET_FPSCR"
+  [(set_attr "type" "mrs")])
+
+;; Read Floating-point Status and Control Register.
+(define_insn "get_fpscr"
+  [(set (match_operand:SI 0 "register_operand" "=r")
+        (unspec_volatile:SI [(const_int 0)] VUNSPEC_GET_FPSCR))]
+  "TARGET_VFP"
+  "mrc\\tp10, 7, %0, cr1, cr0, 0\\t @GET_FPSCR"
+  [(set_attr "type" "mrs")])
+
+
 ;; Unimplemented insns:
 ;; fldm*
 ;; fstm*
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 347a94a..9104331 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9114,6 +9114,7 @@  instructions, but allow the compiler to schedule those calls.
 * ARM iWMMXt Built-in Functions::
 * ARM NEON Intrinsics::
 * ARM ACLE Intrinsics::
+* ARM Floating Point Status and Control Intrinsics::
 * AVR Built-in Functions::
 * Blackfin Built-in Functions::
 * FR-V Built-in Functions::
@@ -9918,6 +9919,17 @@  the @option{-march=armv8-a+crc} switch is used:
 
 @include arm-acle-intrinsics.texi
 
+@node ARM Floating Point Status and Control Intrinsics
+@subsection ARM Floating Point Status and Control Intrinsics
+
+These built-in functions are available for the ARM family of
+processors with floating-point unit.
+
+@smallexample
+unsigned int __builtin_arm_get_fpscr ()
+void __builtin_arm_set_fpscr (unsigned int)
+@end smallexample
+
 @node AVR Built-in Functions
 @subsection AVR Built-in Functions