Re: [RFC][AARCH64] TARGET_ATOMIC_ASSIGN_EXPAND_FENV hook

[Kugan <kugan dot vivekanandarajah at linaro dot org>]

On 02/05/14 20:06, Marcus Shawcroft wrote:
> On 29 April 2014 03:37, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>
>> On 28/04/14 21:01, Ramana Radhakrishnan wrote:
>>> On 04/26/14 11:57, Kugan wrote:
>>>> Attached patch implements TARGET_ATOMIC_ASSIGN_EXPAND_FENV for AARCH64.
>>>> With this, atomic test-case gcc.dg/atomic/c11-atomic-exec-5.c now PASS.
>>>>
>>>> This implementation is based on SPARC and i386 implementations.
>>>>
>>>> Regression tested on qemu-aarch64 for aarch64-none-linux-gnu with no new
>>>> regression. Is this OK for trunk?
>>>
>>> Again like A32 please test on hardware to make sure this behaves
>>> correctly with c11-atomic-exec-5.c .
>>>
>>> If you don't have access to hardware, let us know : we'll take it for a
>>> spin once you update the patch according to Marcus's comments.
>>>
>>
>> Thanks for the review. I have updated the patch. I also have updated
>> hold, clear and update to be exactly as in feholdexcpt.c, fclrexcpt.c
>> and feupdateenv.c of glibc/ports/sysdeps/aarch64/fpu.
>>
> 
> Kugan, I've not looked at the respin in detail yet, but it has just
> occurred to me that the sequence used here to set FPCR is
> insufficient.  The architecture reference manual requires that any
> write to FPCR must be syncrhronized by a context synchronization
> operation so we need to plant an ISB after the write.   Both the write
> and ISB are likely to be expensive on some implementations so it would
> be good to ensure that both the write and the isb are scheduled
> independently.  IIRC there si
> 
>> I have limited real hardware access and just did a bootstrap and tested
>> c11-atomic-exec-5.c alone to make sure that it PASS. I have also
>> regression tested again on qemu-aarch64 for aarch64-none-linux-gnu with
>> no new regressions. I will appreciate if you could do the regression
>> testing on real hw.
> 
> Once the ISB issue is resolved I'll give the patch a spin on HW here.

Here is the modified patch which also includes changes Yufeng has
suggested. Regression tested on qemu-aarch64 for aarch64-none-linux-gnu
with no new regressions.

Thanks,
Kugan


gcc/
+2014-05-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
+
+	* config/aarch64/aarch64.c (TARGET_ATOMIC_ASSIGN_EXPAND_FENV): New
+	define.
+	* config/aarch64/aarch64-protos.h (aarch64_atomic_assign_expand_fenv):
+	New function declaration.
+	* config/aarch64/aarch64-builtins.c (aarch64_builtins) : Add
+	AARCH64_BUILTIN_GET_FPCR, AARCH64_BUILTIN_SET_FPCR.
+	AARCH64_BUILTIN_GET_FPSR and AARCH64_BUILTIN_SET_FPSR.
+	(aarch64_init_builtins) : Initialize builtins
+	__builtins_aarch64_set_fpcr, __builtins_aarch64_get_fpcr.
+	__builtins_aarch64_set_fpsr and __builtins_aarch64_get_fpsr.
+	(aarch64_expand_builtin) : Expand builtins __builtins_aarch64_set_fpcr
+	__builtins_aarch64_get_fpcr, __builtins_aarch64_get_fpsr,
+	and __builtins_aarch64_set_fpsr.
+	(aarch64_atomic_assign_expand_fenv): New function.
+	* config/aarch64/aarch64.md (set_fpcr): New pattern.
+	(get_fpcr) : Likewise.
+	(set_fpsr) : Likewise.
+	(get_fpsr) : Likewise.
+	(unspecv): Add UNSPECV_GET_FPCR and UNSPECV_SET_FPCR, UNSPECV_GET_FPSR
+	 and UNSPECV_SET_FPSR.
+	* doc/extend.texi (AARCH64 Built-in Functions) : Document
+	__builtins_aarch64_set_fpcr, __builtins_aarch64_get_fpcr.
+	__builtins_aarch64_set_fpsr and __builtins_aarch64_get_fpsr.
+

Attachment: aarch64_v4.txt
Description: Text document