Hi Thomas,

On 18/04/18 12:31, Thomas Preudhomme wrote:
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.preudho...@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.preudho...@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?


Yes, thanks.
Kyrill

Best regards,

Thomas

Reply via email to