Re: [RFC v2][2/2] Target FP: Make use of MPFR if available

[John Baldwin <jhb@freebsd.org>]

On Thursday, November 16, 2017 08:00:08 PM Ulrich Weigand wrote:
> [RFC v2][2/2] Target FP: Make use of MPFR if available
> 
> This second patch introduces mfpr_float_ops, an new implementation
> of target_float_ops.  This implements precise emulation of target
> floating-point formats using the MPFR library.  This is then used
> to perform operations on types that do not match any host type.
> 
> Note that use of MPFR is still not required.  The patch adds
> a configure option --with-mpfr similar to --with-expat.  If use of
> MPFR is disabled via the option or MPFR is not available, code will
> fall back to current behavior.  This means that operations on types
> that do not match any host type will be implemented on the host
> long double type instead.
> 
> A new test case verifies that we can correctly print the largest
> __float128 value now.

This might need a stricter configure check perhaps?  I'm getting the
following build failure compiling GDB on FreeBSD 11.x with mpfr-3.1.5
installed:

c++ -x c++ -std=gnu++11 -pipe -DRL_NO_COMPAT -Wno-unused-function -Wno-unused-variable -Wno-absolute-value -Wno-parentheses-equality -Wno-unknown-warning-option -g -DLIBICONV_PLUG -g -fno-strict-aliasing   -I. -I../../gdb -I../../gdb/common -I../../gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../gdb/../include/opcode -I../../gdb/../opcodes/.. -I../../gdb/../readline/.. -I../../gdb/../zlib -I../bfd -I../../gdb/../bfd -I../../gdb/../include -I../libdecnumber -I../../gdb/../libdecnumber  -I../../gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1  -I/usr/local/include  -I/usr/local/include/python2.7 -I/usr/local/include/python2.7 -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-mismatched-tags -Wformat-nonliteral  `echo " -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wno-mismatched-tags -Wformat-nonliteral " | sed "s/ -Wformat-nonliteral / -Wno-format-nonliteral /g"` \
        -c -o target-float.o -MT target-float.o -MMD -MP -MF ./.deps/target-float.Tpo ../../gdb/target-float.c
../../gdb/target-float.c:1567:10: error: use of undeclared identifier
      'mpfr_get_sj'; did you mean 'mpfr_get_si'?
  return mpfr_get_sj (tmp.val, MPFR_RNDZ);
         ^~~~~~~~~~~
         mpfr_get_si                                                            
/usr/local/include/mpfr.h:413:22: note: 'mpfr_get_si' declared here
__MPFR_DECLSPEC long mpfr_get_si _MPFR_PROTO ((mpfr_srcptr, mpfr_rnd_t));
                     ^
../../gdb/target-float.c:1578:3: error: use of undeclared identifier
      'mpfr_set_sj'; did you mean 'mpfr_set_si'?
  mpfr_set_sj (tmp.val, val, MPFR_RNDN);
  ^~~~~~~~~~~
  mpfr_set_si
/usr/local/include/mpfr.h:370:21: note: 'mpfr_set_si' declared here
__MPFR_DECLSPEC int mpfr_set_si _MPFR_PROTO ((mpfr_ptr, long, mpfr_rnd_t));
                    ^
../../gdb/target-float.c:1590:3: error: use of undeclared identifier
      'mpfr_set_uj'; did you mean 'mpfr_set_ui'?
  mpfr_set_uj (tmp.val, val, MPFR_RNDN);
  ^~~~~~~~~~~
  mpfr_set_ui
/usr/local/include/mpfr.h:372:3: note: 'mpfr_set_ui' declared here
  mpfr_set_ui _MPFR_PROTO ((mpfr_ptr, unsigned long, mpfr_rnd_t));
  ^
3 errors generated.
gmake[2]: *** [Makefile:2402: target-float.o] Error 1
gmake[2]: Leaving directory '/usr/home/john/work/git/gdb/obj/gdb'
gmake[1]: *** [Makefile:10100: all-gdb] Error 2
gmake[1]: Leaving directory '/usr/home/john/work/git/gdb/obj'
gmake: *** [Makefile:849: all] Error 2

Hmmm, it seems that mpfr.h depends on this conditional to decide if
intmax_t is available:

/* Check if <stdint.h> / <inttypes.h> is included or if the user
   explicitly wants intmax_t. Automatical detection is done by
   checking:
     - INTMAX_C and UINTMAX_C, but not if the compiler is a C++ one
       (as suggested by Patrick Pelissier) because the test does not
       work well in this case. See:
         https://sympa.inria.fr/sympa/arc/mpfr/2010-02/msg00025.html
       We do not check INTMAX_MAX and UINTMAX_MAX because under Solaris,
       these macros are always defined by <limits.h> (i.e. even when
       <stdint.h> and <inttypes.h> are not included).
     - _STDINT_H (defined by the glibc), _STDINT_H_ (defined under
       Mac OS X) and _STDINT (defined under MS Visual Studio), but
       this test may not work with all implementations.
       Portable software should not rely on these tests.
*/
#if (defined (INTMAX_C) && defined (UINTMAX_C) && !defined(__cplusplus)) || \
  defined (MPFR_USE_INTMAX_T) || \
  defined (_STDINT_H) || defined (_STDINT_H_) || defined (_STDINT)
# ifndef _MPFR_H_HAVE_INTMAX_T
# define _MPFR_H_HAVE_INTMAX_T 1

FreeBSD's stdint.h uses _SYS_STDINT_H_ as it's include guard for <stdint.h>,
so this check doesn't work.  Perhaps GDB's sources should just define
MPFR_USE_INTMAX_T explicitly after ensuring <stdint.h> is included?  That
would seem to be the most portable approach and I think is what the "Portable
software should not rely on these tests" implies.

target-float.c always includes <stdint.h> via "defs.h" ->
"common/common-defs.h", so just adding the #define should be sufficient.

Indeed, this does fix the build on FreeBSD:

diff --git a/gdb/target-float.c b/gdb/target-float.c
index 32237ec9d9..b40b6416c1 100644
--- a/gdb/target-float.c
+++ b/gdb/target-float.c
@@ -1147,6 +1147,8 @@ host_float_ops<T>::compare (const gdb_byte *x, const struct type *type_x,
 
 #ifdef HAVE_LIBMPFR
 
+#define MPFR_USE_INTMAX_T
+
 #include <mpfr.h>
 
 class mpfr_float_ops : public target_float_ops

-- 
John Baldwin