Re: [PATCH] libstdc++-v3: Implement {,b}float16_t nextafter and some fixes [PR106652]

[Jonathan Wakely <jwakely@redhat.com>]

On Wed, 19 Oct 2022 at 09:23, Jakub Jelinek wrote:
>
> Hi!
>
> The following patch implements nextafter for std::{,b}float16_t,
> though right now only without constexpr support, and adds a testcase for it.
> The testcase unfortunately relevealed I've screwed up testing of my last
> patch.  I've tested earlier version of the patch with
> --target_board=unix/-std=c++23
> but didn't test the final version with that RUNTESTFLAGS, so missed
> an invalid call to std::sph_neumann (too many arguments) in the test.
> And, I've made a typo in the guard for numeric_limits (the reason
> for the guard is I wanted to avoid defining a large macro that nothing
> will then use because the std::{,b}float*_t types are C++23 only) and
> so numeric_limits wasn't specialized for the types at all but
> testsuite/18_support/headers/limits/synopsis_cxx23.cc test didn't
> detect that.
> In the nextafter implementation I'm calling __builtin_nextafterf
> to get various required side-effects for nextafter from 0/-0, or from max
> to inf or from min to largest subnormal to avoid needing to set errno
> inline, or use inline asm specific for each processor to force math
> evaluation barriers.  Dunno if
> #ifdef __INT16_TYPE__
>     using __float16_int_type = __INT16_TYPE__;
> #else
>     using __float16_int_type = short int;
> #endif
> isn't too ugly, perhaps we could just blindly use short int and hope
> or even assert it has the same size as _Float16 or __gnu_cxx::__bfloat16_t?

I'm happy with the preprocessor conditions above, but using short int
and a static assert would be OK too.


> Only aarch64, arm, csky, gcn, x86, nvptx and riscv support these types
> and all of them have 16-bit short (I think the only target with some
> other short size is avr with certain command line switches where both
> short and int are 8-bit, but such mode isn't compatible with C and C++
> requirements).

It's not, but we do try to support avr, especially for something like
<limits> which requires no OS support and has always been required for
freestanding. But will it have float16 anyway? I suppose it might do,
so maybe the preprocessor dance for __INT16_TYPE__ is better. It's
only needed in two places, so isn't too bad.



> --- libstdc++-v3/testsuite/26_numerics/headers/cmath/nextafter_c++23.cc.jj      2022-10-18 17:18:32.237061706 +0200
> +++ libstdc++-v3/testsuite/26_numerics/headers/cmath/nextafter_c++23.cc 2022-10-18 18:51:21.465258133 +0200
> @@ -0,0 +1,124 @@
> +// Copyright (C) 2022 Free Software Foundation, Inc.
> +//
> +// This file is part of the GNU ISO C++ Library.  This library is free
> +// software; you can redistribute it and/or modify it under the
> +// terms of the GNU General Public License as published by the
> +// Free Software Foundation; either version 3, or (at your option)
> +// any later version.
> +
> +// This library is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +
> +// You should have received a copy of the GNU General Public License along
> +// with this library; see the file COPYING3.  If not see
> +// <http://www.gnu.org/licenses/>.
> +
> +// { dg-do run { target c++23 } }

This should have { dg-options "-std=gnu++2b" } before the dg-do.

OK with that change.