[PATCH, GCC/ARM] Fix PR85261: ICE with FPSCR setter builtin

[Kyrill Tkachov]

Hi Thomas,

On 09/04/18 15:29, Thomas Preudhomme wrote:
> Hi Ramana,
>
> On 06/04/18 17:17, Thomas Preudhomme wrote:
> >
> >
> > On 06/04/18 17:08, Ramana Radhakrishnan wrote:
> >> On 06/04/2018 16:54, Thomas Preudhomme wrote:
> >>> Instruction pattern for setting the FPSCR expects the input value to be
> >>> in a register. However, __builtin_arm_set_fpscr expander does not ensure
> >>> that this is the case and as a result GCC ICEs when the builtin is
> >>> called with a constant literal.
> >>>
> >>> This commit fixes the builtin to force the input value into a register.
> >>> It also remove the unneeded volatile in the existing fpscr test and
> >>> fixes the function prototype.
> >>>
> >>> ChangeLog entries are as follows:
> >>>
> >>> *** gcc/ChangeLog ***
> >>>
> >>> 2018-04-06  Thomas Preud'homme <thomas.preudhomme@arm.com>
> >>>
> >>>     PR target/85261
> >>>     * config/arm/arm-builtins.c (arm_expand_builtin): Force input operand
> >>>     into register.
> >>>
> >>> *** gcc/testsuite/ChangeLog ***
> >>>
> >>> 2018-04-06  Thomas Preud'homme <thomas.preudhomme@arm.com>
> >>>
> >>>     PR target/85261
> >>>     * gcc.target/arm/fpscr.c: Add call to __builtin_arm_set_fpscr with
> >>>     literal value.  Expect 2 MCR instruction. Fix function prototype.
> >>>     Remove volatile keyword.
> >>>
> >>> Testing: Built an arm-none-eabi GCC cross-compiler and testsuite shows
> >>> no regression.
> >>>
> >>> Is this ok for stage4?
> >>>
> >>> Best regards,
> >>>
> >>> Thomas
> >>>
> >>
> >> (sorry about the duplicate for those who get it)
> >>
> >>
> >> LGTM, though in this case I would prefer a bootstrap and regression run
> >> as this is automatically exercised most with gcc.dg/atomic_*.c and you
> >> really need this tested on linux than just bare-metal as I'm not sure
> >> how this gets tested on arm-none-eabi.
> >
> > Oh it is indeed. Didn't realized it was used anywhere. Will start bootstrap
> > right away.
>
> Done with --with-arch=armv8-a --with-mode=thumb --with-fpu=neon-vfpv4
> --with-float=hard --enable-languages=c,c++,fortran --with-system-zlib
> --enable-plugins --enable-bootstrap. Testsuite for that GCC does not show any
> regression either.
>
> Ok to commit?
>

Thanks for doing this.
This is ok for trunk.

> >
> >>
> >> What about earlier branches, have you looked ? This is a silly target
> >> bug and fixes should go back to older branches in this particular case
> >> after baking this on trunk for some time.
> >
> > GCC 6 and 7 are affected as well and a backport will be done once it has baked
> > long enough of course.
>
> Will now bootstrap and regtest against GCC 6 and 7. Will let you know once that
> is finished.

Thanks,
Kyrill

>
> Best regards,
>
> Thomas