Re: [RFC][AARCH64] TARGET_ATOMIC_ASSIGN_EXPAND_FENV hook

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

Ping ?

Thanks,
Kugan
On 02/05/14 22:27, Kugan wrote:
> 
> 
> 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.
> +
>