On 29 April 2014 03:37, Kugan <kugan.vivekanandara...@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. /Marcus
