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

[Thomas Preudhomme]

Hi Kyrill,

On 11/04/18 10:02, Kyrill Tkachov wrote:
> 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.

Backports show no regression on a bootstrapped arm-none-linux-gnueabihf GCC 6 & 
7. Ok to commit those?

Best regards,

Thomas