[power-ieee128] libgfortran: Small progress on the library side

[Jakub Jelinek via Fortran]

Hi!

The following patch quiets
../../../libgfortran/generated/in_pack_r17.c:35:1: warning: no previous 
prototype for ‘internal_pack_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/in_pack_c17.c:35:1: warning: no previous 
prototype for ‘internal_pack_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/in_unpack_r17.c:33:1: warning: no previous 
prototype for ‘internal_unpack_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/in_unpack_c17.c:33:1: warning: no previous 
prototype for ‘internal_unpack_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/pack_r17.c:73:1: warning: no previous prototype 
for ‘pack_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/pack_c17.c:73:1: warning: no previous prototype 
for ‘pack_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/unpack_r17.c:34:1: warning: no previous 
prototype for ‘unpack0_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/unpack_r17.c:178:1: warning: no previous 
prototype for ‘unpack1_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/unpack_c17.c:34:1: warning: no previous 
prototype for ‘unpack0_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/unpack_c17.c:178:1: warning: no previous 
prototype for ‘unpack1_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/spread_r17.c:34:1: warning: no previous 
prototype for ‘spread_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/spread_r17.c:230:1: warning: no previous 
prototype for ‘spread_scalar_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/spread_c17.c:34:1: warning: no previous 
prototype for ‘spread_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/spread_c17.c:230:1: warning: no previous 
prototype for ‘spread_scalar_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/cshift0_r17.c:33:1: warning: no previous 
prototype for ‘cshift0_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/cshift0_c17.c:33:1: warning: no previous 
prototype for ‘cshift0_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/cshift1_4_r17.c:32:1: warning: no previous 
prototype for ‘cshift1_4_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/cshift1_4_c17.c:32:1: warning: no previous 
prototype for ‘cshift1_4_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/cshift1_8_r17.c:32:1: warning: no previous 
prototype for ‘cshift1_8_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/cshift1_8_c17.c:32:1: warning: no previous 
prototype for ‘cshift1_8_c17’ [-Wmissing-prototypes]
../../../libgfortran/generated/cshift1_16_r17.c:32:1: warning: no previous 
prototype for ‘cshift1_16_r17’ [-Wmissing-prototypes]
../../../libgfortran/generated/cshift1_16_c17.c:32:1: warning: no previous 
prototype for ‘cshift1_16_c17’ [-Wmissing-prototypes]
warnings during libgfortran build and exports the new entrypoints.
Note, not all of them, clearly e.g. there are fewer *_r17* entrypoints than
*_r16* entrypoints, so more work is needed.

Ok for power-ieee128 branch?

2021-12-31  Jakub Jelinek  

* libgfortran.h (internal_pack_r17, internal_pack_c17,
internal_unpack_r17, internal_unpack_c17, pack_r17, pack_c17,
unpack0_r17, unpack0_c17, unpack1_r17, unpack1_c17, spread_r17,
spread_c17, spread_scalar_r17, spread_scalar_c17, cshift0_r17,
cshift0_c17, cshift1_4_r17, cshift1_8_r17, cshift1_16_r17,
cshift1_4_c17, cshift1_8_c17, cshift1_16_c17): Declare.
* gfortran.map (GFORTRAN_12): Export *_r17 and *_c17.

--- libgfortran/libgfortran.h
+++ libgfortran/libgfortran.h
@@ -968,6 +968,11 @@ GFC_REAL_16 *internal_pack_r16 (gfc_array_r16 *);
 internal_proto(internal_pack_r16);
 #endif
 
+#if defined HAVE_GFC_REAL_17
+GFC_REAL_17 *internal_pack_r17 (gfc_array_r17 *);
+internal_proto(internal_pack_r17);
+#endif
+
 GFC_COMPLEX_4 *internal_pack_c4 (gfc_array_c4 *);
 internal_proto(internal_pack_c4);
 
@@ -984,6 +989,11 @@ GFC_COMPLEX_16 *internal_pack_c16 (gfc_array_c16 *);
 internal_proto(internal_pack_c16);
 #endif
 
+#if defined HAVE_GFC_COMPLEX_17
+GFC_COMPLEX_17 *internal_pack_c17 (gfc_array_c17 *);
+internal_proto(internal_pack_c17);
+#endif
+
 extern void internal_unpack_1 (gfc_array_i1 *, const GFC_INTEGER_1 *);
 internal_proto(internal_unpack_1);
 
@@ -1017,6 +1027,11 @@ extern void internal_unpack_r16 (gfc_array_r16 *, const 
GFC_REAL_16 *);
 internal_proto(internal_unpack_r16);
 #endif
 
+#if defined HAVE_GFC_REAL_17
+extern void internal_unpack_r17 (gfc_array_r17 *, const GFC_REAL_17 *);
+internal_proto(internal_unpack_r17);
+#endif
+
 extern void internal_unpack_c4 (gfc_array_c4 *, const GFC_COMPLEX_4 *);
 internal_proto(internal_unpack_c4);
 
@@ -1033,6 +1048,11 @@ extern void internal_unpack_c16 (gfc_array_c16 *, const 
GFC_COMPLEX_16 *);
 internal_proto(internal_unpack_c16);
 #endif
 
+#if defined HAVE_GFC_COMPLEX_17
+extern void internal_unpack_c17 (gfc_array_c17 *, const GFC_COMPLEX_17 *);
+internal_proto(internal_unpack_c17);
+#endif
+
 /

[power-ieee128] gfortran: Introduce gfc_type_abi_kind

[Jakub Jelinek via Fortran]

Hi!

The following patch detects the powerpc64le-linux kind == 16 cases
and for the -mabi=ieeelongdouble case (no matter whether it is the
configured in default or just option used on the command line) uses
_r17 or _c17 instead of _r16 or _c17 in the library API names.

>From what I can see, e.g. calls to sin on real(kind = 16) works fine
with or without this patch (we call __builtin_sinl and the backend
uses rs6000_mangle_decl_assembler_name which ensures __sinieee128
is called).
Haven't played enough with it to see if the various *_r17 or *_c17
API entrypoints are called (but verified abi_kind is right in the
debugger), in all my attempts so far everything was emitted inline.

What is clearly still broken is IO, where for
  real(kind=16) a
  a = 1.0
  print *, a
end
we call
  _gfortran_transfer_real_write (&dt_parm.0, &a, 16);
for both -mabi=ibmlongdouble and -mabi=ieeelongdouble
I don't remember what was the agreement, do we want
  _gfortran_transfer_real_write (&dt_parm.0, &a, 17);
for the ieeelongdouble case, or some new entrypoint for
the abi_kind == 17 real/complex IO?
Also, what about kind stored in array descriptors?  Shall we use
there the abi_kind or kind?

I guess at least before the IO case is solved there is no point
in checking the testsuite, too many things will be majorly broken...

2021-12-31  Jakub Jelinek  

* gfortran.h (gfc_real_info): Add abi_kind member.
(gfc_type_abi_kind): Declare.
* trans-types.c (gfc_init_kinds): Initialize abi_kind.
* intrinsic.c (gfc_type_abi_kind): New function.
(conv_name): Use it.
* iresolve.c (resolve_transformational, gfc_resolve_abs,
gfc_resolve_char_achar, gfc_resolve_acos, gfc_resolve_acosh,
gfc_resolve_aimag, gfc_resolve_and, gfc_resolve_aint, gfc_resolve_all,
gfc_resolve_anint, gfc_resolve_any, gfc_resolve_asin,
gfc_resolve_asinh, gfc_resolve_atan, gfc_resolve_atanh,
gfc_resolve_atan2, gfc_resolve_bessel_n2, gfc_resolve_ceiling,
gfc_resolve_cmplx, gfc_resolve_complex, gfc_resolve_cos,
gfc_resolve_cosh, gfc_resolve_count, gfc_resolve_dble,
gfc_resolve_dim, gfc_resolve_dot_product, gfc_resolve_dprod,
gfc_resolve_exp, gfc_resolve_floor, gfc_resolve_hypot,
gfc_resolve_int, gfc_resolve_int2, gfc_resolve_int8, gfc_resolve_long,
gfc_resolve_log, gfc_resolve_log10, gfc_resolve_logical,
gfc_resolve_matmul, gfc_resolve_minmax, gfc_resolve_maxloc,
gfc_resolve_findloc, gfc_resolve_maxval, gfc_resolve_merge,
gfc_resolve_minloc, gfc_resolve_minval, gfc_resolve_mod,
gfc_resolve_modulo, gfc_resolve_nearest, gfc_resolve_or,
gfc_resolve_real, gfc_resolve_realpart, gfc_resolve_reshape,
gfc_resolve_sign, gfc_resolve_sin, gfc_resolve_sinh, gfc_resolve_sqrt,
gfc_resolve_tan, gfc_resolve_tanh, gfc_resolve_transpose,
gfc_resolve_trigd, gfc_resolve_xor, gfc_resolve_random_number):
Likewise.
* trans-decl.c (gfc_build_intrinsic_function_decls): Use 
gfc_real_kinds[rkinds[rkind]].abi_kind instead of rkinds[rkind].

--- gcc/fortran/gfortran.h
+++ gcc/fortran/gfortran.h
@@ -2643,7 +2643,7 @@ extern gfc_logical_info gfc_logical_kinds[];
 typedef struct
 {
   mpfr_t epsilon, huge, tiny, subnormal;
-  int kind, radix, digits, min_exponent, max_exponent;
+  int kind, abi_kind, radix, digits, min_exponent, max_exponent;
   int range, precision;
 
   /* The precision of the type as reported by GET_MODE_PRECISION.  */
@@ -3499,6 +3499,7 @@ void gfc_intrinsic_init_1 (void);
 void gfc_intrinsic_done_1 (void);
 
 char gfc_type_letter (bt, bool logical_equals_int = false);
+int gfc_type_abi_kind (gfc_typespec *);
 gfc_symbol * gfc_get_intrinsic_sub_symbol (const char *);
 gfc_symbol *gfc_get_intrinsic_function_symbol (gfc_expr *);
 gfc_symbol *gfc_find_intrinsic_symbol (gfc_expr *);
--- gcc/fortran/trans-types.c
+++ gcc/fortran/trans-types.c
@@ -363,6 +363,8 @@ gfc_init_kinds (void)
   int i_index, r_index, kind;
   bool saw_i4 = false, saw_i8 = false;
   bool saw_r4 = false, saw_r8 = false, saw_r10 = false, saw_r16 = false;
+  scalar_mode r16_mode = QImode;
+  scalar_mode composite_mode = QImode;
 
   i_index = 0;
   FOR_EACH_MODE_IN_CLASS (int_mode_iter, MODE_INT)
@@ -428,6 +430,10 @@ gfc_init_kinds (void)
   if (!targetm.scalar_mode_supported_p (mode))
continue;
 
+  if (MODE_COMPOSITE_P (mode)
+ && (GET_MODE_PRECISION (mode) + 7) / 8 == 16)
+   composite_mode = mode;
+
   /* Only let float, double, long double and TFmode go through.
 Runtime support for others is not provided, so they would be
 useless.  */
@@ -471,7 +477,10 @@ gfc_init_kinds (void)
   if (kind == 10)
saw_r10 = true;
   if (kind == 16)
-   saw_r16 = true;
+   {
+ saw_r16 = true;
+ r16_mode = mode;
+   }
 
   /* Careful we don't stumble a weird internal mode.  */
   gcc_assert (r_index <= 0 || gfc_real_

[power-ieee128] gfortran, v2: Introduce gfc_type_abi_kind

[Jakub Jelinek via Fortran]

On Fri, Dec 31, 2021 at 03:16:47PM +0100, Jakub Jelinek via Gcc-patches wrote:
> Haven't played enough with it to see if the various *_r17 or *_c17
> API entrypoints are called (but verified abi_kind is right in the
> debugger), in all my attempts so far everything was emitted inline.

Actually playing with that (e.g. for matmul) revealed a brown paper bag
bug in the previous patch, fixed thusly:

2021-12-31  Jakub Jelinek  

* gfortran.h (gfc_real_info): Add abi_kind member.
(gfc_type_abi_kind): Declare.
* trans-types.c (gfc_init_kinds): Initialize abi_kind.
* intrinsic.c (gfc_type_abi_kind): New function.
(conv_name): Use it.
* iresolve.c (resolve_transformational, gfc_resolve_abs,
gfc_resolve_char_achar, gfc_resolve_acos, gfc_resolve_acosh,
gfc_resolve_aimag, gfc_resolve_and, gfc_resolve_aint, gfc_resolve_all,
gfc_resolve_anint, gfc_resolve_any, gfc_resolve_asin,
gfc_resolve_asinh, gfc_resolve_atan, gfc_resolve_atanh,
gfc_resolve_atan2, gfc_resolve_bessel_n2, gfc_resolve_ceiling,
gfc_resolve_cmplx, gfc_resolve_complex, gfc_resolve_cos,
gfc_resolve_cosh, gfc_resolve_count, gfc_resolve_dble,
gfc_resolve_dim, gfc_resolve_dot_product, gfc_resolve_dprod,
gfc_resolve_exp, gfc_resolve_floor, gfc_resolve_hypot,
gfc_resolve_int, gfc_resolve_int2, gfc_resolve_int8, gfc_resolve_long,
gfc_resolve_log, gfc_resolve_log10, gfc_resolve_logical,
gfc_resolve_matmul, gfc_resolve_minmax, gfc_resolve_maxloc,
gfc_resolve_findloc, gfc_resolve_maxval, gfc_resolve_merge,
gfc_resolve_minloc, gfc_resolve_minval, gfc_resolve_mod,
gfc_resolve_modulo, gfc_resolve_nearest, gfc_resolve_or,
gfc_resolve_real, gfc_resolve_realpart, gfc_resolve_reshape,
gfc_resolve_sign, gfc_resolve_sin, gfc_resolve_sinh, gfc_resolve_sqrt,
gfc_resolve_tan, gfc_resolve_tanh, gfc_resolve_transpose,
gfc_resolve_trigd, gfc_resolve_xor, gfc_resolve_random_number):
Likewise.
* trans-decl.c (gfc_build_intrinsic_function_decls): Likewise.

--- gcc/fortran/gfortran.h
+++ gcc/fortran/gfortran.h
@@ -2643,7 +2643,7 @@ extern gfc_logical_info gfc_logical_kinds[];
 typedef struct
 {
   mpfr_t epsilon, huge, tiny, subnormal;
-  int kind, radix, digits, min_exponent, max_exponent;
+  int kind, abi_kind, radix, digits, min_exponent, max_exponent;
   int range, precision;
 
   /* The precision of the type as reported by GET_MODE_PRECISION.  */
@@ -3499,6 +3499,12 @@ void gfc_intrinsic_init_1 (void);
 void gfc_intrinsic_done_1 (void);
 
 char gfc_type_letter (bt, bool logical_equals_int = false);
+int gfc_type_abi_kind (bt, int);
+static inline int
+gfc_type_abi_kind (gfc_typespec *ts)
+{
+  return gfc_type_abi_kind (ts->type, ts->kind);
+}
 gfc_symbol * gfc_get_intrinsic_sub_symbol (const char *);
 gfc_symbol *gfc_get_intrinsic_function_symbol (gfc_expr *);
 gfc_symbol *gfc_find_intrinsic_symbol (gfc_expr *);
--- gcc/fortran/trans-types.c
+++ gcc/fortran/trans-types.c
@@ -363,6 +363,8 @@ gfc_init_kinds (void)
   int i_index, r_index, kind;
   bool saw_i4 = false, saw_i8 = false;
   bool saw_r4 = false, saw_r8 = false, saw_r10 = false, saw_r16 = false;
+  scalar_mode r16_mode = QImode;
+  scalar_mode composite_mode = QImode;
 
   i_index = 0;
   FOR_EACH_MODE_IN_CLASS (int_mode_iter, MODE_INT)
@@ -428,6 +430,10 @@ gfc_init_kinds (void)
   if (!targetm.scalar_mode_supported_p (mode))
continue;
 
+  if (MODE_COMPOSITE_P (mode)
+ && (GET_MODE_PRECISION (mode) + 7) / 8 == 16)
+   composite_mode = mode;
+
   /* Only let float, double, long double and TFmode go through.
 Runtime support for others is not provided, so they would be
 useless.  */
@@ -471,7 +477,10 @@ gfc_init_kinds (void)
   if (kind == 10)
saw_r10 = true;
   if (kind == 16)
-   saw_r16 = true;
+   {
+ saw_r16 = true;
+ r16_mode = mode;
+   }
 
   /* Careful we don't stumble a weird internal mode.  */
   gcc_assert (r_index <= 0 || gfc_real_kinds[r_index-1].kind != kind);
@@ -479,6 +488,7 @@ gfc_init_kinds (void)
   gcc_assert (r_index != MAX_REAL_KINDS);
 
   gfc_real_kinds[r_index].kind = kind;
+  gfc_real_kinds[r_index].abi_kind = kind;
   gfc_real_kinds[r_index].radix = fmt->b;
   gfc_real_kinds[r_index].digits = fmt->p;
   gfc_real_kinds[r_index].min_exponent = fmt->emin;
@@ -496,6 +506,19 @@ gfc_init_kinds (void)
   r_index += 1;
 }
 
+  /* Detect the powerpc64le-linux case with -mabi=ieeelongdouble, where
+ the long double type is non-MODE_COMPOSITE_P TFmode but one can use
+ -mabi=ibmlongdouble too and get MODE_COMPOSITE_P TFmode with the same
+ precision.  For libgfortran calls pretend the IEEE 754 quad TFmode has
+ kind 17 rather than 16 and use kind 16 for the IBM extended format
+ TFmode.  */
+  if (composite_mode != QImode && saw_r16 && !MOD

Re: [power-iee128] How to specify linker flags

[Jakub Jelinek via Fortran]

On Mon, Jan 03, 2022 at 11:19:21AM +0100, Thomas Koenig wrote:
> > > If you are building libraries that contain modules with multiple
> > > long double
> > > types, you must use the '-mno-gnu-attribute'.  We also use the
> > > '-Wno-psabi'
> > > option, which silences the warning that you are switching long
> > > double types (if
> > > glibc is not 2.34 or newer).  We may need to tweak -Wno-psabi for
> > > use with
> > > Fortran.
> > 
> > I am now at the point where the object files are also compiled correctly
> > for the gfortran specifics:
> > 
> >  <_gfortran_specific__abs_r17>:
> >     0:   09 00 43 f4 lxv vs34,0(r3)
> >     4:   48 16 40 fc xsabsqp v2,v2
> >     8:   20 00 80 4e blr
> > 
> > However, the linker complains, as you said it would, about the different
> > formats:
> > 
> > /opt/at15.0/bin/ld: .libs/maxloc0_4_r16.o uses IBM long double,
> > .libs/_abs_r17.o uses IEEE long double
> > /opt/at15.0/bin/ld: failed to merge target specific data of file
> > .libs/_abs_r17.o
> > 
> > I know next to nothing about libtool, so I do not know how to
> > add the flags so the linker can find them.
> > 
> > Any pointers?
> 
> One additional point.  The linker does not understand
> -mno-gnu-attribute:
> 
> $ /opt/at15.0/bin/ld -mno-gnu-attribute
> /opt/at15.0/bin/ld: unrecognised emulation mode: no-gnu-attribute
> Supported emulations: elf64lppc elf32lppc elf32lppclinux elf32lppcsim
> elf64ppc elf32ppc elf32ppclinux elf32ppcsim
> 
> So, waiting for info to proceed.

-mno-gnu-attribute isn't a linker flag, but a compiler flag.
And e.g. libstdc++ configure uses it while compiling itself on
powerpc*linux:
LONG_DOUBLE_COMPAT_FLAGS="$LONG_DOUBLE_COMPAT_FLAGS -mno-gnu-attribute"
# Check for IEEE128 support in libm:
AC_CHECK_LIB(m, __frexpieee128,
 [ac_ldbl_ieee128_in_libc=yes],
 [ac_ldbl_ieee128_in_libc=no])
if test $ac_ldbl_ieee128_in_libc = yes; then
  # Determine which long double format is the compiler's default:
  AC_TRY_COMPILE(, [
#ifndef __LONG_DOUBLE_IEEE128__
#error compiler defaults to ibm128
#endif
  ], [ac_ldbl_ieee128_default=yes], [ac_ldbl_ieee128_default=no])
  # Library objects should use default long double format.
  if test "$ac_ldbl_ieee128_default" = yes; then
LONG_DOUBLE_128_FLAGS="-mno-gnu-attribute"
# Except for the ones that explicitly use these flags:
LONG_DOUBLE_ALT128_COMPAT_FLAGS="-mabi=ibmlongdouble 
-mno-gnu-attribute -Wno-psabi"
  else
LONG_DOUBLE_128_FLAGS="-mno-gnu-attribute"
LONG_DOUBLE_ALT128_COMPAT_FLAGS="-mabi=ieeelongdouble 
-mno-gnu-attribute -Wno-psabi"
  fi
  AC_DEFINE([_GLIBCXX_LONG_DOUBLE_ALT128_COMPAT],1,
[Define if compatibility should be provided for alternative 
128-bit long double formats.])
  port_specific_symbol_files="$port_specific_symbol_files 
\$(top_srcdir)/config/os/gnu-linux/ldbl-ieee128-extra.ver"
  ac_ldbl_alt128_compat=yes
else
  ac_ldbl_alt128_compat=no
fi
;;
The idea behind this is that libstdc++ is written such that it can handle
both IBM extended and IEEE quad long double, so its object files are
compatible with both.

So I think we want:
2022-01-03  Jakub Jelinek  

* configure.ac (Use -mno-gnu-attribute together with
-mabi=ibmlongdouble or -mabi=ieeelongdouble.

--- libgfortran/configure.ac2021-12-31 11:08:19.032835533 +
+++ libgfortran/configure.ac2022-01-03 10:32:16.927834682 +
@@ -163,9 +163,9 @@ if test "x$GCC" = "xyes"; then
   #error long double is double
   #endif]],
  [[(void) 0;]])],
-[AM_FCFLAGS="$AM_FCFLAGS -mabi=ibmlongdouble";
-AM_CFLAGS="$AM_CFLAGS -mabi=ibmlongdouble";
-CFLAGS="$CFLAGS -mabi=ibmlongdouble";
+[AM_FCFLAGS="$AM_FCFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
+AM_CFLAGS="$AM_CFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
+CFLAGS="$CFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
 have_real_17=yes])
   ;;
 *)


Jakub

[power-iee128] libgfortran: Use -mno-gnu-attribute in libgfortran

[Jakub Jelinek via Fortran]

On Mon, Jan 03, 2022 at 11:33:32AM +0100, Jakub Jelinek wrote:
> The idea behind this is that libstdc++ is written such that it can handle
> both IBM extended and IEEE quad long double, so its object files are
> compatible with both.

Now tested on powerpc64le-linux (and together with the regenerated file),
ok for power-ieee128?

2022-01-03  Jakub Jelinek  

* configure.ac (Use -mno-gnu-attribute together with
-mabi=ibmlongdouble or -mabi=ieeelongdouble.
* configure: Regenerated.

--- libgfortran/configure.ac.jj 2021-12-31 11:08:19.032835533 +
+++ libgfortran/configure.ac2022-01-03 10:32:16.927834682 +
@@ -163,9 +163,9 @@ if test "x$GCC" = "xyes"; then
   #error long double is double
   #endif]],
  [[(void) 0;]])],
-[AM_FCFLAGS="$AM_FCFLAGS -mabi=ibmlongdouble";
-AM_CFLAGS="$AM_CFLAGS -mabi=ibmlongdouble";
-CFLAGS="$CFLAGS -mabi=ibmlongdouble";
+[AM_FCFLAGS="$AM_FCFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
+AM_CFLAGS="$AM_CFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
+CFLAGS="$CFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
 have_real_17=yes])
   ;;
 *)
--- libgfortran/configure.jj2021-12-31 11:08:19.032835533 +
+++ libgfortran/configure   2022-01-03 13:55:38.684926082 +
@@ -6025,9 +6025,9 @@ main ()
 }
 _ACEOF
 if ac_fn_c_try_cpp "$LINENO"; then :
-  AM_FCFLAGS="$AM_FCFLAGS -mabi=ibmlongdouble";
-AM_CFLAGS="$AM_CFLAGS -mabi=ibmlongdouble";
-CFLAGS="$CFLAGS -mabi=ibmlongdouble";
+  AM_FCFLAGS="$AM_FCFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
+AM_CFLAGS="$AM_CFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
+CFLAGS="$CFLAGS -mabi=ibmlongdouble -mno-gnu-attribute";
 have_real_17=yes
 fi
 rm -f conftest.err conftest.i conftest.$ac_ext


Jakub

[power-ieee128] libquadmath: Use -mno-gnu-attribute in libquadmath

[Jakub Jelinek via Fortran]

Hi!

Testing found that we also need libquadmath to be built with
-mno-gnu-attribute, otherwise -mabi=ieeelongdouble programs don't link.

Ok for power-ieee128?

2022-01-03  Jakub Jelinek  

* configure.ac: Set XCFLAGS to -mno-gnu-attribute on
powerpc64le*-linux*.
* configure: Regenerated.

--- libquadmath/configure.ac
+++ libquadmath/configure.ac
@@ -352,6 +352,19 @@ fi
 # Add CET specific flags if CET is enabled
 GCC_CET_FLAGS(CET_FLAGS)
 XCFLAGS="$XCFLAGS $CET_FLAGS"
+
+case x$target in
+  xpowerpc64le*-linux*)
+AC_PREPROC_IFELSE(
+  [AC_LANG_PROGRAM([[#if __SIZEOF_LONG_DOUBLE__ != 16
+ #error long double is double
+ #endif]],
+   [[(void) 0;]])],
+  [XCFLAGS="$XCFLAGS -mno-gnu-attribute"])
+;;
+  *)
+;;
+esac
 AC_SUBST(XCFLAGS)
 
 AC_CACHE_SAVE
--- libquadmath/configure
+++ libquadmath/configure
@@ -13096,6 +13096,30 @@ fi
 
 XCFLAGS="$XCFLAGS $CET_FLAGS"
 
+case x$target in
+  xpowerpc64le*-linux*)
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#if __SIZEOF_LONG_DOUBLE__ != 16
+ #error long double is double
+ #endif
+int
+main ()
+{
+(void) 0;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_cpp "$LINENO"; then :
+  XCFLAGS="$XCFLAGS -mno-gnu-attribute"
+fi
+rm -f conftest.err conftest.i conftest.$ac_ext
+;;
+  *)
+;;
+esac
+
 
 cat >confcache <<\_ACEOF
 # This file is a shell script that caches the results of configure

Jakub

[power-ieee128] libgfortran: -mabi=ieeelongdouble I/O

[Jakub Jelinek via Fortran]

Hi!

The following patch adds the library side of -mabi=ieeelongdouble
I/O support.

There is one issue to be resolved though, for the sake of libgfortran.a
built on an older powerpc64le-linux system (glibc older than 2.32) and
then deployed on glibc 2.32 or later, I believe we want to use
_gfortran_transfer_real128_write etc. APIs so that we force in libquadmath
in that case.  The following patch does that, but unfortunately it means
that right now those
   512: 001a31d056 FUNCGLOBAL DEFAULT   11 
_gfortran_transfer_real128@@GFORTRAN_8   [: 8]
   920: 001a321056 FUNCGLOBAL DEFAULT   11 
_gfortran_transfer_real128_write@@GFORTRAN_8 [: 8]
   487: 001a329056 FUNCGLOBAL DEFAULT   11 
_gfortran_transfer_complex128_write@@GFORTRAN_8  [: 8]
   574: 001a325056 FUNCGLOBAL DEFAULT   11 
_gfortran_transfer_complex128@@GFORTRAN_8[: 8]
symbols.  But those symbols weren't exported on powerpc64le-linux in
GCC 8, 9, 10 or 11, so they shouldn't be exported @@GFORTRAN_8, but 
@@GFORTRAN_12.

So, either we'd need to add e.g. preprocessing support for gfortran.map
or some other way how to make certain symbols appear conditionally at
different symbol versions, or another option would be to choose different
symbol names for those for the powerpc64le-linux cases
(e.g. _gfortran_transfer_{real,complex}ieee128{,_write}).

Any preferences?

2022-01-03  Jakub Jelinek  

* libgfortran.h (__acoshieee128, __acosieee128, __asinhieee128,
__asinieee128, __atan2ieee128, __atanhieee128, __atanieee128,
__coshieee128, __cosieee128, __erfieee128, __expieee128,
__fabsieee128, __jnieee128, __log10ieee128, __logieee128,
__powieee128, __sinhieee128, __sinieee128, __sqrtieee128,
__tanhieee128, __tanieee128, __ynieee128): Formatting fixes.
(__strtoieee128, __snprintfieee128): Declare.
* io/io.h (default_width_for_float, default_precision_for_float):
Handle kind == 17.
* io/size_from_kind.c (size_from_real_kind, size_from_complex_kind):
Likewise.
* io/read.c (set_integer, si_max, convert_real, convert_infnan,
read_f): Likewise.
* io/write.c (extract_uint, size_from_kind, set_fnode_default):
Likewise.
* io/write_float.def (DTOA2Q, FDTOA2Q): Define for HAVE_GFC_REAL_17.
(determine_en_precision, get_float_string): Handle kind == 17.
* io/transfer128.c: Use also for HAVE_GFC_REAL_17, but don't drag in
libquadmath if POWER_IEEE128.

--- libgfortran/libgfortran.h.jj2021-12-31 11:45:06.121158716 +
+++ libgfortran/libgfortran.h   2022-01-03 14:32:45.063730903 +
@@ -1936,28 +1936,54 @@ internal_proto(cshift1_16_c17);
 
 /* Prototypes for the POWER __ieee128 functions.  */
 #ifdef POWER_IEEE128
-extern __float128 __acoshieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __acosieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __asinhieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __asinieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __atan2ieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __atanhieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __atanieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __coshieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __cosieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __erfieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __expieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __fabsieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __jnieee128 (int, __float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __log10ieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __logieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __powieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __sinhieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __sinieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __sqrtieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __tanhieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __tanieee128 (__float128) __attribute__ ((__nothrow__, 
__leaf__));
-extern __float128 __ynieee128 (int , __float128) __attribute__ ((__nothrow__, 
__leaf__));
+extern __float128 __acoshieee128 (__float128)
+  __attribute__ ((__nothrow__, __leaf__));
+extern __float128 __acosieee128 (__float128)
+  __attribute__ ((__nothrow__, __leaf__));
+extern __float128 __asinhieee128 (__

[power-ieee128] fortran: trans-io.c side of -mabi=ieeelongdouble I/O support

[Jakub Jelinek via Fortran]

Hi!

Here is the compiler side of those changes, but depends of course
on the decision what to do with those *real128* and *complex128* symbols.

With all the 4 patches e.g. print *, var for real(kind=16) var; var = 1.0;
works both with -mabi=ibmlongdouble and -mabi=ieeelongdouble.

2022-01-03  Jakub Jelinek  

* trans-io.c (transfer_namelist_element): Use gfc_type_abi_kind,
formatting fixes.
(transfer_expr): Use gfc_type_abi_kind, use *REAL128* APIs even
for abi_kind == 17.

--- gcc/fortran/trans-io.c.jj   2021-12-31 11:00:15.052190585 +
+++ gcc/fortran/trans-io.c  2022-01-03 14:20:55.238159269 +
@@ -1765,18 +1765,17 @@ transfer_namelist_element (stmtblock_t *
   else
 tmp = build_int_cst (gfc_charlen_type_node, 0);
 
+  int abi_kind = gfc_type_abi_kind (ts);
   if (dtio_proc == null_pointer_node)
-tmp = build_call_expr_loc (input_location,
-  iocall[IOCALL_SET_NML_VAL], 6,
-  dt_parm_addr, addr_expr, string,
-  build_int_cst (gfc_int4_type_node, ts->kind),
-  tmp, dtype);
+tmp = build_call_expr_loc (input_location, iocall[IOCALL_SET_NML_VAL], 6,
+  dt_parm_addr, addr_expr, string,
+  build_int_cst (gfc_int4_type_node, abi_kind),
+  tmp, dtype);
   else
-tmp = build_call_expr_loc (input_location,
-  iocall[IOCALL_SET_NML_DTIO_VAL], 8,
-  dt_parm_addr, addr_expr, string,
-  build_int_cst (gfc_int4_type_node, ts->kind),
-  tmp, dtype, dtio_proc, vtable);
+tmp = build_call_expr_loc (input_location, iocall[IOCALL_SET_NML_DTIO_VAL],
+  8, dt_parm_addr, addr_expr, string,
+  build_int_cst (gfc_int4_type_node, abi_kind),
+  tmp, dtype, dtio_proc, vtable);
   gfc_add_expr_to_block (block, tmp);
 
   /* If the object is an array, transfer rank times:
@@ -2298,7 +2297,7 @@ transfer_expr (gfc_se * se, gfc_typespec
   ts->kind = gfc_index_integer_kind;
 }
 
-  kind = ts->kind;
+  kind = gfc_type_abi_kind (ts);
   function = NULL;
   arg2 = NULL;
   arg3 = NULL;
@@ -2318,14 +2317,14 @@ transfer_expr (gfc_se * se, gfc_typespec
   arg2 = build_int_cst (integer_type_node, kind);
   if (last_dt == READ)
{
- if (gfc_real16_is_float128 && ts->kind == 16)
+ if ((gfc_real16_is_float128 && kind == 16) || kind == 17)
function = iocall[IOCALL_X_REAL128];
  else
function = iocall[IOCALL_X_REAL];
}
   else
{
- if (gfc_real16_is_float128 && ts->kind == 16)
+ if ((gfc_real16_is_float128 && kind == 16) || kind == 17)
function = iocall[IOCALL_X_REAL128_WRITE];
  else
function = iocall[IOCALL_X_REAL_WRITE];
@@ -2337,14 +2336,14 @@ transfer_expr (gfc_se * se, gfc_typespec
   arg2 = build_int_cst (integer_type_node, kind);
   if (last_dt == READ)
{
- if (gfc_real16_is_float128 && ts->kind == 16)
+ if ((gfc_real16_is_float128 && kind == 16) || kind == 17)
function = iocall[IOCALL_X_COMPLEX128];
  else
function = iocall[IOCALL_X_COMPLEX];
}
   else
{
- if (gfc_real16_is_float128 && ts->kind == 16)
+ if ((gfc_real16_is_float128 && kind == 16) || kind == 17)
function = iocall[IOCALL_X_COMPLEX128_WRITE];
  else
function = iocall[IOCALL_X_COMPLEX_WRITE];

Jakub

Re: [power-ieee128] libgfortran: -mabi=ieeelongdouble I/O

[Jakub Jelinek via Fortran]

On Mon, Jan 03, 2022 at 04:36:21PM +0100, Jakub Jelinek via Gcc-patches wrote:
> The following patch adds the library side of -mabi=ieeelongdouble
> I/O support.
> 
> There is one issue to be resolved though, for the sake of libgfortran.a
> built on an older powerpc64le-linux system (glibc older than 2.32) and
> then deployed on glibc 2.32 or later, I believe we want to use
> _gfortran_transfer_real128_write etc. APIs so that we force in libquadmath
> in that case.  The following patch does that, but unfortunately it means
> that right now those
>512: 001a31d056 FUNCGLOBAL DEFAULT   11 
> _gfortran_transfer_real128@@GFORTRAN_8 [: 8]
>920: 001a321056 FUNCGLOBAL DEFAULT   11 
> _gfortran_transfer_real128_write@@GFORTRAN_8   [: 8]
>487: 001a329056 FUNCGLOBAL DEFAULT   11 
> _gfortran_transfer_complex128_write@@GFORTRAN_8[: 8]
>574: 001a325056 FUNCGLOBAL DEFAULT   11 
> _gfortran_transfer_complex128@@GFORTRAN_8  [: 8]
> symbols.  But those symbols weren't exported on powerpc64le-linux in
> GCC 8, 9, 10 or 11, so they shouldn't be exported @@GFORTRAN_8, but 
> @@GFORTRAN_12.
> 
> So, either we'd need to add e.g. preprocessing support for gfortran.map

Note, an example of preprocessed version file is e.g. libgomp, in the
Makefile it does:
# -Wc is only a libtool option.
comma = ,
PREPROCESS = $(subst -Wc$(comma), , $(COMPILE)) -E

libgomp.ver: $(top_srcdir)/libgomp.map
$(EGREP) -v '#(#| |$$)' $< | \
  $(PREPROCESS) -P -include config.h - > $@ || (rm -f $@ ; exit 1)
and in libgomp.map it has both:
#ifdef HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT
# If the assembler used lacks the .symver directive or the linker
# doesn't support GNU symbol versioning, we have the same symbol in
# two versions, which Sun ld chokes on.
omp_init_lock;
...
#endif
so we could similarly have something like:
#if !(defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ && 
__SIZEOF_LONG_DOUBLE__ == 16)
_gfortran_transfer_complex128;
_gfortran_transfer_complex128_write;
#endif
...
#if !(defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ && 
__SIZEOF_LONG_DOUBLE__ == 16)
_gfortran_transfer_real128;
_gfortran_transfer_real128_write;
#endif
...
#if defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ && 
__SIZEOF_LONG_DOUBLE__ == 16
  _gfortran_transfer_complex128;
  _gfortran_transfer_complex128_write;
  _gfortran_transfer_real128;
  _gfortran_transfer_real128_write;
#endif

or make that dependent on HAVE_GFC_REAL_17 or whatever else (with suitable
includes that only define macros and not actual C code).

Jakub

[power-ieee128] libgfortran, fortran: -mabi=ieeelongdouble I/O

[Jakub Jelinek via Fortran]

On Mon, Jan 03, 2022 at 06:03:41PM +0100, Thomas Koenig wrote:
> On 03.01.22 17:26, Jakub Jelinek wrote:
> 
> > so we could similarly have something like:
> > #if !(defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ 
> > && __SIZEOF_LONG_DOUBLE__ == 16)
> >  _gfortran_transfer_complex128;
> >  _gfortran_transfer_complex128_write;
> > #endif
> > ...
> > #if !(defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ 
> > && __SIZEOF_LONG_DOUBLE__ == 16)
> >  _gfortran_transfer_real128;
> >  _gfortran_transfer_real128_write;
> > #endif
> > ...
> > #if defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ && 
> > __SIZEOF_LONG_DOUBLE__ == 16
> >_gfortran_transfer_complex128;
> >_gfortran_transfer_complex128_write;
> >_gfortran_transfer_real128;
> >_gfortran_transfer_real128_write;
> > #endif
> 
> That would also work for me.
> 
> > or make that dependent on HAVE_GFC_REAL_17 or whatever else (with suitable
> > includes that only define macros and not actual C code).
> 
> With my most recent commit, HAVE_GFC_REAL_17 can now be found in
> kinds.inc if all of the macros above are defined, that should
> be suitable.
> 
> I found that __powerpc64__ is not defined when compiling *.F90 files
> (which is why I added them by hand). Not sure how the preprocessor is
> invoked on gfortran.map, but if things don't work, this could be
> related :-)
> 
> So, it's OK either way with me.  What do others think?

Here is an updated patch that does that (and now includes also the
gcc/fortran part, since it makes no sense to split the two).

I've run make check-fortran both normally and with
RUNTESTFLAGS='--target_board=unix\{-mabi=ieeelongdouble\}'
The former has:
FAIL: gfortran.dg/reshape_shape_2.f90   -O  (internal compiler error)
FAIL: gfortran.dg/reshape_shape_2.f90   -O   (test for errors, line 6)
FAIL: gfortran.dg/reshape_shape_2.f90   -O  (test for excess errors)
FAIL: gfortran.dg/vector_subscript_1.f90   -O1  execution test
FAIL: gfortran.dg/vector_subscript_1.f90   -O2  execution test
FAIL: gfortran.dg/vector_subscript_1.f90   -O3 -fomit-frame-pointer 
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/vector_subscript_1.f90   -O3 -g  execution test
FAIL: gfortran.dg/vector_subscript_1.f90   -Os  execution test
FAIL: gfortran.dg/ieee/large_2.f90   -O0  execution test
FAIL: gfortran.dg/ieee/large_2.f90   -O1  execution test
FAIL: gfortran.dg/ieee/large_2.f90   -O2  execution test
FAIL: gfortran.dg/ieee/large_2.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/ieee/large_2.f90   -O3 -g  execution test
FAIL: gfortran.dg/ieee/large_2.f90   -Os  execution test
and the latter has:
FAIL: gfortran.dg/dec_math.f90   -O0  execution test
FAIL: gfortran.dg/dec_math.f90   -O1  execution test
FAIL: gfortran.dg/dec_math.f90   -O2  execution test
FAIL: gfortran.dg/dec_math.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/dec_math.f90   -O3 -g  execution test
FAIL: gfortran.dg/dec_math.f90   -Os  execution test
FAIL: gfortran.dg/fmt_en.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -O2  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -O3 -g  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -Os  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -O2  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -O2  output pattern test
FAIL: gfortran.dg/fmt_g0_7.f08   -O0  execution test
FAIL: gfortran.dg/fmt_g0_7.f08   -O1  execution test
FAIL: gfortran.dg/fmt_g0_7.f08   -O2  execution test
FAIL: gfortran.dg/fmt_en_rd.f90   -O3 -g  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gfortran.dg/fmt_g0_7.f08   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/fmt_g0_7.f08   -O3 -g  execution test
FAIL: gfortran.dg/fmt_en_rd.f90   -Os  output pattern test
FAIL: gfortran.dg/fmt_g0_7.f08   -Os  execution test
FAIL: gfortran.dg/fmt_en_rn.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -O3 -g  output pattern test
FAIL: gfortran.dg/fmt_en_rn.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -Os  output pattern test
FAIL: gfortran.dg/fmt_e

[power-ieee128] RFH: LTO broken

[Jakub Jelinek via Fortran]

On Mon, Jan 03, 2022 at 11:43:57PM +0100, Thomas Koenig wrote:
> > clearly there is still work to fix (but seems e.g. most of the lto tests
> > are related to the gnu attributes stuff:(  ).
> 
> This is looking better than what I expected.  Apart from LTO, I expect

I've just verified that LTO is broken even in C/C++, it isn't just gfortran.
Just do
make check-gcc RUNTESTFLAGS='--target_board=unix\{-mabi=ieeelongdouble\} 
lto.exp'
on a system where gcc is configured to default to -mabi=ibmlongdouble
with glibc 2.32 or later and watch all the FAILs.
All the failures look like:
/home/jakub/gcc/obj/gcc/xgcc -B/home/jakub/gcc/obj/gcc/ c_lto_20081024_0.o 
-mabi=ieeelongdouble -fdiagnostics-plain-output -O0 -flto -flto-partition=none 
-o gcc-
dg-lto-20081024-01.exe
lto1: warning: Using IEEE extended precision 'long double' [-Wpsabi]
FAIL: gcc.dg/lto/20081024 c_lto_20081024_0.o-c_lto_20081024_0.o link, -O0 -flto 
-flto-partition=none 

Michael, do you think you could have a look?  Either it is the ELF object
created for debug info or the one created by lto1.

Jakub

[power-ieee128] libgfortran: -mabi=ieeelongdouble I/O fix

[Jakub Jelinek via Fortran]

Hi!

The following patch fixes:
FAIL: gfortran.dg/fmt_en.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -O2  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -O3 -g  output pattern test
FAIL: gfortran.dg/fmt_en.f90   -Os  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -O2  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -O3 -g  output pattern test
FAIL: gfortran.dg/fmt_en_rd.f90   -Os  output pattern test
FAIL: gfortran.dg/fmt_en_rn.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_en_rn.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_en_rn.f90   -O2  output pattern test
FAIL: gfortran.dg/fmt_en_rn.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gfortran.dg/fmt_en_rn.f90   -O3 -g  output pattern test
FAIL: gfortran.dg/fmt_en_rn.f90   -Os  output pattern test
FAIL: gfortran.dg/fmt_en_ru.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_en_ru.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_en_ru.f90   -O2  output pattern test
FAIL: gfortran.dg/fmt_en_ru.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gfortran.dg/fmt_en_ru.f90   -O3 -g  output pattern test
FAIL: gfortran.dg/fmt_en_ru.f90   -Os  output pattern test
FAIL: gfortran.dg/fmt_en_rz.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_en_rz.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_en_rz.f90   -O2  output pattern test
FAIL: gfortran.dg/fmt_en_rz.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gfortran.dg/fmt_en_rz.f90   -O3 -g  output pattern test
FAIL: gfortran.dg/fmt_en_rz.f90   -Os  output pattern test
FAIL: gfortran.dg/fmt_g0_7.f08   -O0  execution test
FAIL: gfortran.dg/fmt_g0_7.f08   -O1  execution test
FAIL: gfortran.dg/fmt_g0_7.f08   -O2  execution test
FAIL: gfortran.dg/fmt_g0_7.f08   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/fmt_g0_7.f08   -O3 -g  execution test
FAIL: gfortran.dg/fmt_g0_7.f08   -Os  execution test
FAIL: gfortran.dg/fmt_pf.f90   -O0  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -O1  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -O2  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -O3 -g  output pattern test
FAIL: gfortran.dg/fmt_pf.f90   -Os  output pattern test
FAIL: gfortran.dg/large_real_kind_1.f90   -O0  execution test
FAIL: gfortran.dg/large_real_kind_1.f90   -O1  execution test
FAIL: gfortran.dg/large_real_kind_1.f90   -O2  execution test
FAIL: gfortran.dg/large_real_kind_1.f90   -O3 -fomit-frame-pointer 
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/large_real_kind_1.f90   -O3 -g  execution test
FAIL: gfortran.dg/large_real_kind_1.f90   -Os  execution test

Ok for power-ieee128?

2022-01-04  Jakub Jelinek  

* io/write_float.def (CALCULATE_EXP): If HAVE_GFC_REAL_17, also use
CALCULATE_EXP(17).
(determine_en_precision): Use 17 instead of 16 as first EN_PREC
argument for kind 17.
(get_float_string): Use 17 instead of 16 as first FORMAT_FLOAT
argument for kind 17.

--- libgfortran/io/write_float.def.jj   2022-01-04 10:27:56.528323600 +
+++ libgfortran/io/write_float.def  2022-01-04 13:09:51.751534884 +
@@ -799,6 +799,10 @@ CALCULATE_EXP(10)
 #ifdef HAVE_GFC_REAL_16
 CALCULATE_EXP(16)
 #endif
+
+#ifdef HAVE_GFC_REAL_17
+CALCULATE_EXP(17)
+#endif
 #undef CALCULATE_EXP
 
 
@@ -942,7 +946,7 @@ determine_en_precision (st_parameter_dt
 #endif
 #ifdef HAVE_GFC_REAL_17
 case 17:
-  EN_PREC(16,Q)
+  EN_PREC(17,Q)
 #endif
   break;
 default:
@@ -1150,7 +1154,7 @@ get_float_string (st_parameter_dt *dtp,
 #endif
 #ifdef HAVE_GFC_REAL_17
 case 17:
-  FORMAT_FLOAT(16,Q)
+  FORMAT_FLOAT(17,Q)
   break;
 #endif
 default:

Jakub

[power-ieee128] fortran, libgfortran: Assorted -mabi=ieeelongdouble I/O fixes

[Jakub Jelinek via Fortran]

Hi!

Another patch, this fixes:
FAIL: gfortran.dg/intrinsic_spread_2.f90   -O0  execution test
FAIL: gfortran.dg/intrinsic_spread_2.f90   -O1  execution test
FAIL: gfortran.dg/intrinsic_spread_2.f90   -O2  execution test
FAIL: gfortran.dg/intrinsic_spread_2.f90   -O3 -fomit-frame-pointer 
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/intrinsic_spread_2.f90   -O3 -g  execution test
FAIL: gfortran.dg/intrinsic_spread_2.f90   -Os  execution test
FAIL: gfortran.dg/intrinsic_unpack_2.f90   -O0  execution test
FAIL: gfortran.dg/intrinsic_unpack_2.f90   -O1  execution test
FAIL: gfortran.dg/intrinsic_unpack_2.f90   -O2  execution test
FAIL: gfortran.dg/intrinsic_unpack_2.f90   -O3 -fomit-frame-pointer 
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/intrinsic_unpack_2.f90   -O3 -g  execution test
FAIL: gfortran.dg/intrinsic_unpack_2.f90   -Os  execution test
FAIL: gfortran.dg/large_real_kind_form_io_1.f90   -O0  execution test
FAIL: gfortran.dg/large_real_kind_form_io_1.f90   -O1  execution test
FAIL: gfortran.dg/large_real_kind_form_io_1.f90   -O2  execution test
FAIL: gfortran.dg/large_real_kind_form_io_1.f90   -O3 -fomit-frame-pointer 
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/large_real_kind_form_io_1.f90   -O3 -g  execution test
FAIL: gfortran.dg/large_real_kind_form_io_1.f90   -Os  execution test
FAIL: gfortran.dg/quad_2.f90   -O0  execution test
FAIL: gfortran.dg/quad_2.f90   -O1  execution test
FAIL: gfortran.dg/quad_2.f90   -O2  execution test
FAIL: gfortran.dg/quad_2.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/quad_2.f90   -O3 -g  execution test
FAIL: gfortran.dg/quad_2.f90   -Os  execution test

Ok for power-ieee128?

2022-01-04  Jakub Jelinek  

gcc/fortran/
* trans-io.c (transfer_array_desc): Pass abi kind instead of kind
to libgfortran.
libgfortran/
* io/read.c (convert_real): Add missing break; for the
HAVE_GFC_REAL_17 case.

--- gcc/fortran/trans-io.c.jj   2022-01-04 10:27:56.498322942 +
+++ gcc/fortran/trans-io.c  2022-01-04 13:51:50.336998696 +
@@ -2528,7 +2528,7 @@ transfer_array_desc (gfc_se * se, gfc_ty
   else
 charlen_arg = build_int_cst (gfc_charlen_type_node, 0);
 
-  kind_arg = build_int_cst (integer_type_node, ts->kind);
+  kind_arg = build_int_cst (integer_type_node, gfc_type_abi_kind (ts));
 
   tmp = gfc_build_addr_expr (NULL_TREE, dt_parm);
   if (last_dt == READ)
--- libgfortran/io/read.c.jj2022-01-04 10:27:56.518323381 +
+++ libgfortran/io/read.c   2022-01-04 13:58:51.676285518 +
@@ -203,6 +203,7 @@ convert_real (st_parameter_dt *dtp, void
 # else
   *((GFC_REAL_17*) dest) = __qmath_(strtoflt128) (buffer, &endptr);
 # endif
+  break;
 #endif
 
 default:

Jakub

[power-ieee128] fortran, libgfortran: Add remaining missing *_r17 symbols

[Jakub Jelinek via Fortran]

Hi!

Following patch adds remaining missing *_r17 entrypoints, so that
we have 91 *_r16 and 91 *_r17 entrypoints (and 24 *_c16 and 24 *_c17).

This fixes:
FAIL: gfortran.dg/dec_math.f90   -O0  execution test
FAIL: gfortran.dg/dec_math.f90   -O1  execution test
FAIL: gfortran.dg/dec_math.f90   -O2  execution test
FAIL: gfortran.dg/dec_math.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/dec_math.f90   -O3 -g  execution test
FAIL: gfortran.dg/dec_math.f90   -Os  execution test
FAIL: gfortran.dg/ieee/dec_math_1.f90   -O0  execution test
FAIL: gfortran.dg/ieee/dec_math_1.f90   -O1  execution test
FAIL: gfortran.dg/ieee/dec_math_1.f90   -O2  execution test
FAIL: gfortran.dg/ieee/dec_math_1.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/ieee/dec_math_1.f90   -O3 -g  execution test
FAIL: gfortran.dg/ieee/dec_math_1.f90   -Os  execution test

Ok for power-ieee128?

2022-01-04  Jakub Jelinek  

gcc/fortran/
* trans-intrinsic.c (gfc_get_intrinsic_lib_fndecl): Use
gfc_type_abi_kind.
libgfortran/
* libgfortran.h (GFC_REAL_17_INFINITY, GFC_REAL_17_QUIET_NAN): Define.
(__erfcieee128): Declare.
* intrinsics/trigd.c (_gfortran_sind_r17, _gfortran_cosd_r17,
_gfortran_tand_r17): Define for HAVE_GFC_REAL_17.
* intrinsics/random.c (random_r17, arandom_r17, rnumber_17): Define.
* intrinsics/erfc_scaled.c (ERFC_SCALED): Define.
(erfc_scaled_r16): Use ERFC_SCALED macro.
(erfc_scaled_r17): Define.

--- gcc/fortran/trans-intrinsic.c.jj2021-12-31 11:08:18.642826955 +
+++ gcc/fortran/trans-intrinsic.c   2022-01-04 15:32:29.789881496 +
@@ -881,7 +881,7 @@ gfc_get_intrinsic_lib_fndecl (gfc_intrin
 {
   snprintf (name, sizeof (name), PREFIX ("%s_%c%d"), m->name,
ts->type == BT_COMPLEX ? 'c' : 'r',
-   ts->kind);
+   gfc_type_abi_kind (ts));
 }
 
   argtypes = NULL;
--- libgfortran/libgfortran.h.jj2022-01-04 10:27:56.528323600 +
+++ libgfortran/libgfortran.h   2022-01-04 16:44:54.075203222 +
@@ -309,6 +309,9 @@ typedef GFC_UINTEGER_4 gfc_char4_t;
 #   define GFC_REAL_16_INFINITY __builtin_infq ()
 #  endif
 # endif
+# ifdef HAVE_GFC_REAL_17
+#  define GFC_REAL_17_INFINITY __builtin_inff128 ()
+# endif
 #endif
 #if __FLT_HAS_QUIET_NAN__
 # define GFC_REAL_4_QUIET_NAN __builtin_nanf ("")
@@ -327,6 +330,9 @@ typedef GFC_UINTEGER_4 gfc_char4_t;
 #   define GFC_REAL_16_QUIET_NAN nanq ("")
 #  endif
 # endif
+# ifdef HAVE_GFC_REAL_17
+#  define GFC_REAL_17_QUIET_NAN __builtin_nanf128 ("")
+# endif
 #endif
 
 typedef struct descriptor_dimension
@@ -1954,6 +1960,8 @@ extern __float128 __coshieee128 (__float
   __attribute__ ((__nothrow__, __leaf__));
 extern __float128 __cosieee128 (__float128)
   __attribute__ ((__nothrow__, __leaf__));
+extern __float128 __erfcieee128 (__float128)
+  __attribute__ ((__nothrow__, __leaf__));
 extern __float128 __erfieee128 (__float128)
   __attribute__ ((__nothrow__, __leaf__));
 extern __float128 __expieee128 (__float128)
--- libgfortran/intrinsics/trigd.c.jj   2021-12-31 11:00:58.083137032 +
+++ libgfortran/intrinsics/trigd.c  2022-01-04 16:29:56.585599529 +
@@ -289,3 +289,42 @@ see the files COPYING3 and COPYING.RUNTI
 #undef HAVE_INFINITY_KIND
 
 #endif /* HAVE_GFC_REAL_16 */
+
+#ifdef HAVE_GFC_REAL_17
+
+/* Build _gfortran_sind_r17, _gfortran_cosd_r17, and _gfortran_tand_r17  */
+
+#define KIND   17
+#define TINY   0x1.p-16400 /* ~= 1.28e-4937 */
+#undef  SIND_SMALL /* not precise */
+
+/* Proper float128 precision.  */
+#define COSD_SMALL  0x1.p-51   /* ~= 4.441e-16 */
+#define COSD30  8.66025403784438646763723170752936183e-01
+#define PIO180H 1.74532925199433197605003442731685936e-02
+#define PIO180L -2.39912634365882824665106671063098954e-17
+
+/* libquadmath or glibc 2.32+: HAVE_*Q are never defined.  They must be 
available.  */
+#define ENABLE_SIND
+#define ENABLE_COSD
+#define ENABLE_TAND
+
+#ifdef GFC_REAL_17_INFINITY
+#define HAVE_INFINITY_KIND
+#endif
+
+#include "trigd_lib.inc"
+
+#undef KIND
+#undef TINY
+#undef COSD_SMALL
+#undef SIND_SMALL
+#undef COSD30
+#undef PIO180H
+#undef PIO180L
+#undef ENABLE_SIND
+#undef ENABLE_COSD
+#undef ENABLE_TAND
+#undef HAVE_INFINITY_KIND
+
+#endif /* HAVE_GFC_REAL_17 */
--- libgfortran/intrinsics/random.c.jj  2021-12-31 11:00:58.083137032 +
+++ libgfortran/intrinsics/random.c 2022-01-04 16:40:37.819575318 +
@@ -79,6 +79,16 @@ export_proto(arandom_r16);
 
 #endif
 
+#ifdef HAVE_GFC_REAL_17
+
+extern void random_r17 (GFC_REAL_17 *);
+iexport_proto(random_r17);
+
+extern void arandom_r17 (gfc_array_r17 *);
+export_proto(arandom_r17);
+
+#endif
+
 #ifdef __GTHREAD_MUTEX_INIT
 static __gthread_mutex_t random_lock = __GTHREAD_MUTEX_INIT;
 #else
@@ -161,6 +171,27 @@ rnumber_16 (GFC_REAL_16 *f, GFC_UINTEGER

Re: [power-ieee128] fortran, libgfortran: Assorted -mabi=ieeelongdouble I/O fixes

[Jakub Jelinek via Fortran]

Hi!

This test FAILs because
f951: Error: '-mabi=ieeelongdouble' requires full ISA 2.06 support
compiler exited with status 1
FAIL: gfortran.dg/pr47614.f   -O0  (test for excess errors)
As powerpc64le* only supports -mcpu=power8 and newer, I think we shouldn't
be testing with that option.

Ok for power-ieee128?

All the remaining FAILs I get are due to the -flto -mgnu-attribute issues
or FAIL also with -mabi=ibmlongdouble.

Though, I'm still unsure on what we should do with the array descriptors.
typedef struct dtype_type
{
  size_t elem_len;
  int version;
  signed char rank;
  signed char type;
  signed short attribute;
}
dtype_type;

Is elem_len really element length, or kind, or both?

It seems a lot of code uses that interchangeably, is there anything where
we'd rely on whether it is the IBM extended real(kind=16) or IEEE quad
real(kind=16) (either in libgfortran or elsewhere)?
At least in libgfortran/generates/*, GFC_DESCRIPTOR_SIZE is mostly used
as mask_kind (I think the mask arrays are always logical not real/complex,
right?), or for logical stuff like matmul_l*.

2022-01-04  Jakub Jelinek  

* gfortran.dg/pr47614.f: Don't use -mcpu=power4 for
powerpc64le*-*-linux*.

--- gcc/testsuite/gfortran.dg/pr47614.f.jj  2021-12-31 11:00:53.733041354 
+
+++ gcc/testsuite/gfortran.dg/pr47614.f 2022-01-04 17:51:05.422663254 +
@@ -1,6 +1,7 @@
 ! { dg-do run { target { powerpc*-*-* } } }
 ! { dg-skip-if "" { powerpc*-*-darwin* } }
 ! { dg-options "-O3 -funroll-loops -ffast-math -mcpu=power4" }
+! { dg-options "-O3 -funroll-loops -ffast-math" { target powerpc64le*-*-linux* 
} }
 
 
   SUBROUTINE SFCPAR(ZET,NZ,ZMH,TSL,TMES)

Jakub

Re: [power-ieee128] RFH: LTO broken

[Jakub Jelinek via Fortran]

On Thu, Jan 06, 2022 at 09:01:54PM +0100, Thomas Koenig wrote:
> On 06.01.22 06:00, Michael Meissner via Fortran wrote:
> > I pushed the patch to the branch.
> 
> Test results are looking quite good right now.
> 
> What is still missing is the conversion for unformatted I/O, both
> ways.  I'll start doing some stuff on it. Just one question:
> What are functions that I can use to convert from IBM long double
> to IEEE and long double and vice versa?  It was in an e-mail somewhere,
> but I cannot find it at the moment.

Under the hood __extendkftf2 and __trunctfkf2, as can be seen on:
__ibm128 foo (__float128 x) { return x; }
__float128 bar (__ibm128 x) { return x; }

But, I really don't think libgfortran should call those by hand, just
use C casts, (GFC_REAL_17) var_with_GFC_REAL_16_type or
(GFC_REAL_16) var_with_GFC_REAL_17_type.

Jakub

[power-ieee128] OPEN CONV

[Jakub Jelinek via Fortran]

On Thu, Jan 06, 2022 at 09:01:54PM +0100, Thomas Koenig wrote:
> 
> On 06.01.22 06:00, Michael Meissner via Fortran wrote:
> What is still missing is the conversion for unformatted I/O, both
> ways.  I'll start doing some stuff on it. Just one question:
> What are functions that I can use to convert from IBM long double
> to IEEE and long double and vice versa?  It was in an e-mail somewhere,
> but I cannot find it at the moment.

So, what's the plan for that?
Can't find CONVERT= in Fortran 2018, so I assume it is a non-standard
extension,
https://www.intel.com/content/www/us/en/develop/documentation/fortran-compiler-oneapi-dev-guide-and-reference/top/language-reference/file-operation-i-o-statements/open-statement-specifiers/open-convert-specifier.html#open-convert-specifier
documents the Intel one and we accept
CONVERT='native'
CONVERT='swap'
CONVERT='big_endian'
CONVERT='little_endian'
Now, I suppose for powerpc64le we want to add
some more, but the question is how they play together
with the byteswapping and how to name them, so that
it is clear they talk about REAL/COMPLEX KIND=16 format
and nothing else.  Can we (or do we) want to allow
multiple comma separated strings from the orthogonal
choices, like
CONVERT='big_endian,ibm_extended'
CONVERT='swap,ieee_extended'
or add ibm_extended, ieee_extended and strings that
combine those with swap, big_endian and little_endian
ibm_extended_swap, ieee_extended_little etc.?

Jakub

Re: [power-ieee128] RFH: LTO broken

[Jakub Jelinek via Fortran]

On Thu, Jan 06, 2022 at 09:01:54PM +0100, Thomas Koenig wrote:
> 
> On 06.01.22 06:00, Michael Meissner via Fortran wrote:
> > I pushed the patch to the branch.
> 
> Test results are looking quite good right now.

I've just tried to build libgfortran on an old glibc system
(gcc112.fsffrance.org) and unfortunately we still have work to do:

[jakub@gcc2-power8 obj38]$ 
LD_PRELOAD=/home/jakub/gcc/obj38/powerpc64le-unknown-linux-gnu/libgfortran/.libs/libgfortran.so.5.0.0
 /bin/true
[jakub@gcc2-power8 obj38]$ LD_BIND_NOW=1 
LD_PRELOAD=/home/jakub/gcc/obj38/powerpc64le-unknown-linux-gnu/libgfortran/.libs/libgfortran.so.5.0.0
 /bin/true
/bin/true: symbol lookup error: 
/home/jakub/gcc/obj38/powerpc64le-unknown-linux-gnu/libgfortran/.libs/libgfortran.so.5.0.0:
 undefined symbol: __atan2ieee128

While we do use some libquadmath APIs:
readelf -Wr 
/home/jakub/gcc/obj38/powerpc64le-unknown-linux-gnu/libgfortran/.libs/libgfortran.so.5.0.0
 | grep QUADMATH
00251268  05e40026 R_PPC64_ADDR64  
quadmath_snprintf@QUADMATH_1.0 + 0
00251270  03060026 R_PPC64_ADDR64  
strtoflt128@QUADMATH_1.0 + 0
002502e0  01160015 R_PPC64_JMP_SLOT    
ynq@QUADMATH_1.0 + 0
00250390  01600015 R_PPC64_JMP_SLOT    
sqrtq@QUADMATH_1.0 + 0
00250508  01fa0015 R_PPC64_JMP_SLOT    
fmaq@QUADMATH_1.0 + 0
00250530  02120015 R_PPC64_JMP_SLOT    
fabsq@QUADMATH_1.0 + 0
00250760  03060015 R_PPC64_JMP_SLOT    
strtoflt128@QUADMATH_1.0 + 0
00250990  03df0015 R_PPC64_JMP_SLOT    
cosq@QUADMATH_1.0 + 0
002509f0  040a0015 R_PPC64_JMP_SLOT    
expq@QUADMATH_1.0 + 0
00250a88  04510015 R_PPC64_JMP_SLOT    
erfcq@QUADMATH_1.0 + 0
00250a98  045e0015 R_PPC64_JMP_SLOT    
jnq@QUADMATH_1.0 + 0
00250ac8  047e0015 R_PPC64_JMP_SLOT    
sinq@QUADMATH_1.0 + 0
00250e38  05db0015 R_PPC64_JMP_SLOT    
fmodq@QUADMATH_1.0 + 0
00250e48  05e00015 R_PPC64_JMP_SLOT    
tanq@QUADMATH_1.0 + 0
00250e58  05e40015 R_PPC64_JMP_SLOT    
quadmath_snprintf@QUADMATH_1.0 + 0
00250f20  06290015 R_PPC64_JMP_SLOT    
copysignq@QUADMATH_1.0 + 0
we don't do it consistently:
readelf -Wr 
/home/jakub/gcc/obj38/powerpc64le-unknown-linux-gnu/libgfortran/.libs/libgfortran.so.5.0.0
 | grep ieee128
00250310  01280015 R_PPC64_JMP_SLOT    
__atan2ieee128 + 0
00250340  01420015 R_PPC64_JMP_SLOT    
__clogieee128 + 0
00250438  01a30015 R_PPC64_JMP_SLOT    
__acoshieee128 + 0
002504b8  01cc0015 R_PPC64_JMP_SLOT    
__csinieee128 + 0
00250500  01f30015 R_PPC64_JMP_SLOT    
__sinhieee128 + 0
00250570  022a0015 R_PPC64_JMP_SLOT    
__asinieee128 + 0
00250580  022d0015 R_PPC64_JMP_SLOT    
__roundieee128 + 0
002505a0  023e0015 R_PPC64_JMP_SLOT    
__logieee128 + 0
002505c8  02490015 R_PPC64_JMP_SLOT    
__tanieee128 + 0
00250630  02750015 R_PPC64_JMP_SLOT    
__ccosieee128 + 0
00250670  028a0015 R_PPC64_JMP_SLOT    
__log10ieee128 + 0
002506c8  02bd0015 R_PPC64_JMP_SLOT    
__cexpieee128 + 0
002506d8  02c80015 R_PPC64_JMP_SLOT    
__coshieee128 + 0
002509b0  03ef0015 R_PPC64_JMP_SLOT    
__truncieee128 + 0
00250af8  04a60015 R_PPC64_JMP_SLOT    
__expieee128 + 0
00250b50  04c60015 R_PPC64_JMP_SLOT    
__fmodieee128 + 0
00250bb0  04e70015 R_PPC64_JMP_SLOT    
__tanhieee128 + 0
00250c38  05130015 R_PPC64_JMP_SLOT    
__acosieee128 + 0
00250ce0  05540015 R_PPC64_JMP_SLOT    
__sinieee128 + 0
00250d60  057e0015 R_PPC64_JMP_SLOT    
__atanieee128 + 0
00250dd8  05b10015 R_PPC64_JMP_SLOT    
__sqrtieee128 + 0
00250e98  06020015 R_PPC64_JMP_SLOT    
__cosieee128 + 0
00250eb0  060a0015 R_PPC64_JMP_SLOT    
__atanhieee128 + 0
00250ef0  06200015 R_PPC64_JMP_SLOT    
__asinhieee128 + 0
00250fd8  0

Re: [power-ieee128] RFH: LTO broken

[Jakub Jelinek via Fortran]

On Fri, Jan 07, 2022 at 12:29:25PM +0100, Jakub Jelinek wrote:
> we don't do it consistently:
> readelf -Wr 
> /home/jakub/gcc/obj38/powerpc64le-unknown-linux-gnu/libgfortran/.libs/libgfortran.so.5.0.0
>  | grep ieee128
> 00250310  01280015 R_PPC64_JMP_SLOT    
> __atan2ieee128 + 0
> 00250340  01420015 R_PPC64_JMP_SLOT    
> __clogieee128 + 0
> 00250438  01a30015 R_PPC64_JMP_SLOT    
> __acoshieee128 + 0
> 002504b8  01cc0015 R_PPC64_JMP_SLOT    
> __csinieee128 + 0
> 00250500  01f30015 R_PPC64_JMP_SLOT    
> __sinhieee128 + 0
> 00250570  022a0015 R_PPC64_JMP_SLOT    
> __asinieee128 + 0
> 00250580  022d0015 R_PPC64_JMP_SLOT    
> __roundieee128 + 0
> 002505a0  023e0015 R_PPC64_JMP_SLOT    
> __logieee128 + 0
> 002505c8  02490015 R_PPC64_JMP_SLOT    
> __tanieee128 + 0
> 00250630  02750015 R_PPC64_JMP_SLOT    
> __ccosieee128 + 0
> 00250670  028a0015 R_PPC64_JMP_SLOT    
> __log10ieee128 + 0
> 002506c8  02bd0015 R_PPC64_JMP_SLOT    
> __cexpieee128 + 0
> 002506d8  02c80015 R_PPC64_JMP_SLOT    
> __coshieee128 + 0
> 002509b0  03ef0015 R_PPC64_JMP_SLOT    
> __truncieee128 + 0
> 00250af8  04a60015 R_PPC64_JMP_SLOT    
> __expieee128 + 0
> 00250b50  04c60015 R_PPC64_JMP_SLOT    
> __fmodieee128 + 0
> 00250bb0  04e70015 R_PPC64_JMP_SLOT    
> __tanhieee128 + 0
> 00250c38  05130015 R_PPC64_JMP_SLOT    
> __acosieee128 + 0
> 00250ce0  05540015 R_PPC64_JMP_SLOT    
> __sinieee128 + 0
> 00250d60  057e0015 R_PPC64_JMP_SLOT    
> __atanieee128 + 0
> 00250dd8  05b10015 R_PPC64_JMP_SLOT    
> __sqrtieee128 + 0
> 00250e98  06020015 R_PPC64_JMP_SLOT    
> __cosieee128 + 0
> 00250eb0  060a0015 R_PPC64_JMP_SLOT    
> __atanhieee128 + 0
> 00250ef0  06200015 R_PPC64_JMP_SLOT    
> __asinhieee128 + 0
> 00250fd8  067f0015 R_PPC64_JMP_SLOT    
> __csqrtieee128 + 0
> 00251038  06ad0015 R_PPC64_JMP_SLOT    
> __cabsieee128 + 0
> All these should for POWER_IEEE128 use atan2q@QUADMATH_1.0 etc.

So, seems all these come from f951 compiled sources.
For user code, I think the agreement was if you want to use successfully
-mabi=ieeelongdouble, you need glibc 2.32 or later, which is why the Fortran
FE doesn't conditionalize on whether glibc 2.32 is available or not and just
emits __WHATEVERieee128 entrypoints.
But for Fortran compiled sources in libgfortran, we need to use
__WHATEVERieee128 only if glibc 2.32 or later and WHATEVERq (from
libquadmath) otherwise.
I guess easiest would be to do this always in the FE, but we need to
determine in the FE if the target is glibc 2.32 or later.

> On the other side, on glibc 2.32+ build, we still use some libquadmath APIs
> when we shouldn't:
> readelf -Wr 
> /home/jakub/gcc/obj/powerpc64le-unknown-linux-gnu/libgfortran/.libs/libgfortran.so.5
>  | grep QUADMATH
> 002502c8  00260015 R_PPC64_JMP_SLOT    
> fmaq@QUADMATH_1.0 + 0
> 002505f8  00670015 R_PPC64_JMP_SLOT    
> tanq@QUADMATH_1.0 + 0
> 00250930  009b0015 R_PPC64_JMP_SLOT    
> fabsq@QUADMATH_1.0 + 0
> 00250940  009d0015 R_PPC64_JMP_SLOT    
> sinq@QUADMATH_1.0 + 0
> 00250c98  00cf0015 R_PPC64_JMP_SLOT    
> copysignq@QUADMATH_1.0 + 0
> 00251038  01070015 R_PPC64_JMP_SLOT    
> cosq@QUADMATH_1.0 + 0
> 00251068  010a0015 R_PPC64_JMP_SLOT    
> fmodq@QUADMATH_1.0 + 0
> These should use __fmaieee128, __tanieee128 etc. instead.

This one seems easily fixed by the following patch, ok for power-ieee128?

2022-01-07  Jakub Jelinek  

* libgfortran.h (__copysignieee128, __fmaieee128, __fmodieee128):
Declare.
* intrinsics/trigd.c (COPYSIGN, FMOD, FABS, FMA, SIN, COS, TAN): If
POWER_IEEE128 is defined, define these for kind 17 include.
* intrinsics/trigd_lib.inc (COPYSIGN, FMOD, FABS, FMA, SIN, COS, TAN):
Don't define if COPYSIGN is already defined.

--- libgfortran/libgfortran.h.jj2022-01-07 09:39:10.222157644 +
+++ libgfort

Re: [power-ieee128] RFH: LTO broken

[Jakub Jelinek via Fortran]

On Fri, Jan 07, 2022 at 03:25:57PM +0100, Thomas Koenig wrote:
> > > 00251038  06ad0015 R_PPC64_JMP_SLOT   
> > >  __cabsieee128 + 0
> > > All these should for POWER_IEEE128 use atan2q@QUADMATH_1.0 etc.
> > 
> > So, seems all these come from f951 compiled sources.
> > For user code, I think the agreement was if you want to use successfully
> > -mabi=ieeelongdouble, you need glibc 2.32 or later, which is why the Fortran
> > FE doesn't conditionalize on whether glibc 2.32 is available or not and just
> > emits __WHATEVERieee128 entrypoints.
> 
> That was the idea, I think.
> 
> > But for Fortran compiled sources in libgfortran, we need to use
> > __WHATEVERieee128 only if glibc 2.32 or later and WHATEVERq (from
> > libquadmath) otherwise.
> > I guess easiest would be to do this always in the FE, but we need to
> > determine in the FE if the target is glibc 2.32 or later.
> 
> Instead of determining this in the front end, maybe we can add
> an option (documented, but marked as useful as only for internal
> use and with no guarantee that it will remain) and use that option
> when compiling libgfortran.

We apparently have already TARGET_GLIBC_MAJOR and TARGET_GLIBC_MINOR target
macros, and e.g. libgcc or D testsuite uses internal options like
-fbuilding-libgcc.

So, the following patch adds -fbuilding-libgfortran option and uses
it together with TARGET_GLIBC_M* checks to decide whether to use
libquadmath APIs (for the IEEE quad kind=16 if -fbuilding-libgfortran
and not glibc or glibc is older than 2.32) or glibc 2.32 APIs
(otherwise).  This way, libgfortran uses solely the libquadmath APIs
on glibc < 2.32 and __*ieee128 APIs on glibc 2.32, while user code
always uses the latter.

Ok for power-ieee128?

2022-01-07  Jakub Jelinek  

gcc/fortran/
* trans-types.c (gfc_init_kinds): When setting abi_kind to 17, if not
targetting glibc 2.32 or later and -fbuilding-libgfortran, set
gfc_real16_is_float128 and c_float128 in gfc_real_kinds.
(gfc_build_real_type): Don't set c_long_double if c_float128 is
already set.
* trans-intrinsic.c (builtin_decl_for_precision): Don't use
long_double_built_in if gfc_real16_is_float128 and
long_double_type_node == gfc_float128_type_node.
* lang.opt (fbuilding-libgfortran): New undocumented option.
libgfortran/
* Makefile.am (AM_FCFLAGS): Add -fbuilding-libgfortran after
-fallow-leading-underscore.
* Makefile.in: Regenerated.

--- gcc/fortran/trans-types.c.jj2022-01-04 10:27:56.498322942 +
+++ gcc/fortran/trans-types.c   2022-01-07 16:19:06.737066905 +
@@ -516,7 +516,16 @@ gfc_init_kinds (void)
 {
   for (int i = 0; i < r_index; ++i)
if (gfc_real_kinds[i].kind == 16)
- gfc_real_kinds[i].abi_kind = 17;
+ {
+   gfc_real_kinds[i].abi_kind = 17;
+   if (flag_building_libgfortran
+   && (TARGET_GLIBC_MAJOR < 2
+   || (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR < 32)))
+ {
+   gfc_real16_is_float128 = true;
+   gfc_real_kinds[i].c_float128 = 1;
+ }
+ }
 }
 
   /* Choose the default integer kind.  We choose 4 unless the user directs us
@@ -859,7 +868,7 @@ gfc_build_real_type (gfc_real_info *info
 info->c_float = 1;
   if (mode_precision == DOUBLE_TYPE_SIZE)
 info->c_double = 1;
-  if (mode_precision == LONG_DOUBLE_TYPE_SIZE)
+  if (mode_precision == LONG_DOUBLE_TYPE_SIZE && !info->c_float128)
 info->c_long_double = 1;
   if (mode_precision != LONG_DOUBLE_TYPE_SIZE && mode_precision == 128)
 {
--- gcc/fortran/trans-intrinsic.c.jj2022-01-07 09:39:10.222157644 +
+++ gcc/fortran/trans-intrinsic.c   2022-01-07 13:57:35.451495059 +
@@ -154,7 +154,9 @@ builtin_decl_for_precision (enum built_i
 i = m->float_built_in;
   else if (precision == TYPE_PRECISION (double_type_node))
 i = m->double_built_in;
-  else if (precision == TYPE_PRECISION (long_double_type_node))
+  else if (precision == TYPE_PRECISION (long_double_type_node)
+  && (!gfc_real16_is_float128
+  || long_double_type_node != gfc_float128_type_node))
 i = m->long_double_built_in;
   else if (precision == TYPE_PRECISION (gfc_float128_type_node))
 {
--- gcc/fortran/lang.opt.jj 2021-12-31 11:00:15.042190365 +
+++ gcc/fortran/lang.opt2022-01-07 16:18:17.685995005 +
@@ -413,6 +413,9 @@ fblas-matmul-limit=
 Fortran RejectNegative Joined UInteger Var(flag_blas_matmul_limit) Init(30)
 -fblas-matmul-limit=Size of the smallest matrix for which matmul 
will use BLAS.
 
+fbuilding-libgfortran
+Fortran Undocumented Var(flag_building_libgfortran)
+
 fcheck-array-temporaries
 Fortran
 Produce a warning at runtime if a array temporary has been created for a 
procedure argument.
--- libgfortran/Makefile.am.jj  2022-01-04 10:27:56.498322942 +
+++ libgfortran/Makefile.am 202

Re: [power-ieee128] OPEN CONV

[Jakub Jelinek via Fortran]

On Fri, Jan 07, 2022 at 11:26:15AM +0100, Thomas Koenig wrote:
> In
> 
> https://gcc.gnu.org/pipermail/fortran/2021-October/056895.html
> 
> I made a suggestion how how the format could look like.  I used
> a plus sign instead of a comma because I thought the environment
> variable should follow the same syntax as the CONVERT specifier,
> and I did not want to think about having commas in there :-)
> 
> Thinking about this again after some time, I think the syntax of
> the environment variable would be clearer if the keywords for
> the two conversions were separate, so somethig like
> 
> big_endian;r16_ieee;r16_ibm:10-20;
> 
> for the environment variable and
> 
> CONVERT="big_endian,r16_ibm"
> 
> would probably be better.

Here is completely untested patch that implements something,
but doesn't implement the gcc option stuff, nor the CONVERT=
syntax to supply multiple conversion options nor done anything
about env var nor any testcases.

But it tries to have the native/swap/big/little choice orthogonal from
native/r16_ieee/r16_ibm with r16_ieee and r16_ibm only supported on
ppc64le-linux.

For INQUIRE it has the so far perhaps manageable set of possibilities
handled so that it uses string literals and doesn't have to construct
those strings at runtime (haven't studied how it would need to be done).

I'm afraid I don't know that stuff enough to move forward from this.

--- gcc/fortran/libgfortran.h.jj2022-01-07 18:41:55.473722388 +0100
+++ gcc/fortran/libgfortran.h   2022-01-07 19:14:23.881784305 +0100
@@ -86,14 +86,16 @@ along with GCC; see the file COPYING3.
 #define GFC_INVALID_UNIT   -3
 
 /* Possible values for the CONVERT I/O specifier.  */
-/* Keep in sync with GFC_FLAG_CONVERT_* in gcc/flags.h.  */
+/* Keep in sync with GFC_FLAG_CONVERT_* in gcc/flag-types.h.  */
 typedef enum
 {
   GFC_CONVERT_NONE = -1,
   GFC_CONVERT_NATIVE = 0,
   GFC_CONVERT_SWAP,
   GFC_CONVERT_BIG,
-  GFC_CONVERT_LITTLE
+  GFC_CONVERT_LITTLE,
+  GFC_CONVERT_R16_IEEE = 4,
+  GFC_CONVERT_R16_IBM = 8
 }
 unit_convert;
 
--- gcc/flag-types.h.jj 2022-01-07 18:41:55.452722678 +0100
+++ gcc/flag-types.h2022-01-07 19:13:55.953170776 +0100
@@ -424,7 +424,9 @@ enum gfc_convert
   GFC_FLAG_CONVERT_NATIVE = 0,
   GFC_FLAG_CONVERT_SWAP,
   GFC_FLAG_CONVERT_BIG,
-  GFC_FLAG_CONVERT_LITTLE
+  GFC_FLAG_CONVERT_LITTLE,
+  GFC_FLAG_CONVERT_R16_IEEE = 4,
+  GFC_FLAG_CONVERT_R16_IBM = 8
 };
 
 
--- libgfortran/io/open.c.jj2022-01-07 18:41:56.078714031 +0100
+++ libgfortran/io/open.c   2022-01-07 19:19:11.582780100 +0100
@@ -153,6 +153,10 @@ static const st_option convert_opt[] =
   { "swap", GFC_CONVERT_SWAP},
   { "big_endian", GFC_CONVERT_BIG},
   { "little_endian", GFC_CONVERT_LITTLE},
+#ifdef HAVE_GFC_REAL_17
+  { "r16_ieee", GFC_CONVERT_R16_IEEE},
+  { "r16_ibm", GFC_CONVERT_R16_IBM},
+#endif
   { NULL, 0}
 };
 
@@ -820,7 +824,14 @@ st_open (st_parameter_open *opp)
   else
conv = compile_options.convert;
 }
-  
+
+  flags.convert = 0;
+
+#ifdef HAVE_GFC_REAL_17
+  flags.convert = conv & (GFC_CONVERT_R16_IEEE | GFC_CONVERT_R16_IBM);
+  conv &= ~(GFC_CONVERT_R16_IEEE | GFC_CONVERT_R16_IBM);
+#endif
+
   switch (conv)
 {
 case GFC_CONVERT_NATIVE:
@@ -840,7 +851,7 @@ st_open (st_parameter_open *opp)
   break;
 }
 
-  flags.convert = conv;
+  flags.convert |= conv;
 
   if (flags.position != POSITION_UNSPECIFIED
   && flags.access == ACCESS_DIRECT)
--- libgfortran/io/transfer.c.jj2022-01-07 18:41:56.080714003 +0100
+++ libgfortran/io/transfer.c   2022-01-07 20:43:36.146920392 +0100
@@ -1126,7 +1126,11 @@ unformatted_read (st_parameter_dt *dtp,
 size *= GFC_SIZE_OF_CHAR_KIND(kind);
   read_block_direct (dtp, dest, size * nelems);
 
-  if (unlikely (dtp->u.p.current_unit->flags.convert == GFC_CONVERT_SWAP)
+  int convert = dtp->u.p.current_unit->flags.convert;
+#ifdef HAVE_GFC_REAL_17
+  convert &= ~(GFC_CONVERT_R16_IEEE | GFC_CONVERT_R16_IBM);
+#endif
+  if (unlikely (convert == GFC_CONVERT_SWAP)
   && kind != 1)
 {
   /* Handle wide chracters.  */
@@ -1144,6 +1148,48 @@ unformatted_read (st_parameter_dt *dtp,
}
   bswap_array (dest, dest, size, nelems);
 }
+#ifdef HAVE_GFC_REAL_17
+  if ((dtp->u.p.current_unit->flags.convert & GFC_CONVERT_R16_IEEE)
+  && kind == 16
+  && (type == BT_REAL || type == BT_COMPLEX))
+{
+  if (type == BT_COMPLEX && size == 32)
+   {
+ nelems *= 2;
+ size /= 2;
+   }
+  char *pd = dest;
+  for (size_t i = 0; i < nelems; i++)
+   {
+ GFC_REAL_16 r16;
+ GFC_REAL_17 r17;
+ memcpy (&r17, pd, 16);
+ r16 = r17;
+ memcpy (pd, &r16, 16);
+ pd += size;
+   }
+}
+  else if ((dtp->u.p.current_unit->flags.convert & GFC_CONVERT_R16_IBM)
+  && kind == 17
+  && (type == BT_REAL || type == BT_COMPLEX))
+{
+  if (type == BT_COMPLEX && size == 32)
+   {
+ nelems *= 2;
+ size /= 2;
+   }
+

Re: [power-ieee128] OPEN CONV

[Jakub Jelinek via Fortran]

On Fri, Jan 07, 2022 at 10:40:50PM +0100, Thomas Koenig wrote:
> One thing that one has to watch out for is a big-endian IBM long double
> file, so the byte swapping will have to be done before assigning
> the value.

I've tried to handle that right, i.e. on unformatted read with
byte-swapping and r16 <-> r17 conversions first do byte-swapping
and then r16 <-> r17 conversions, while for unformatted writes
first r16 <-> r17 conversions and then byte-swapping.

Jakub

Re: [power-ieee128] OPEN CONV

[Jakub Jelinek via Fortran]

On Sat, Jan 08, 2022 at 11:07:24AM +0100, Thomas Koenig wrote:
> I have tried to unravel the different cases here, I count six
> (lumping together the environment variables, the CONVERT specifier
> and -fconvert, and leaving out the byte swapping)
> 
> CompilerConvert   Read action Write action
> 
> IEEENone  NoneNone
> IEEEIEEE  NoneNone
> IEEEIBM   IBM->IEEE   IEEE->IBM
> 
> IBM None  NoneNone
> IBM IEEE  IEEE->IBM   IBM->IEEE
> IBM IBM   NoneNone
> 
> From this table, it is clear that the compiler has to inform
> the library about the option it is using, I think it is best
> encoded in the number passed to _gfortran_set_convert.

Whether the compiler is using IEEE or IBM real(kind=16) or
complex(kind=16) for a particular spot (which doesn't have to be
the same in the whole program) is known to the library by the
kind argument it provides to the I/O routines, if it is kind=16,
it is IBM, if it is kind=17, it is IEEE.
See the patch I've posted, which does one thing when the runtime
kind (i.e. abi_kind on the compiler side) is 17 and convert
says r16_ibm, and another thing when runtime kind is 16 and
convert says r16_ieee.  Other cases shouldn't need conversion.
And IMHO the default like for byte-swapping should be the native
format, i.e. the one the program actually used.
The only thing that should be encoded in _gfortran_set_convert
is -fconvertWHATEVER command line option IMO.

Jakub

Re: [power-ieee128] OPEN CONV

[Jakub Jelinek via Fortran]

On Sat, Jan 08, 2022 at 12:00:38PM +0100, Jakub Jelinek via Gcc-patches wrote:
> And IMHO the default like for byte-swapping should be the native
> format, i.e. the one the program actually used.

One reason for that is that neither conversion is lossless, neither format
is a subset or superset of the other.  Yes, IEEE quad has both much bigger
exponent range (-16382..16383 vs. -1022..1023) and slightly bigger fixed
precision (113 vs. 106 bits).
But IBM extended has that weirdo numerically awful flexible precision where
certain numbers can have much bigger precision than those 106 bits, up to
2048+52 or so.  So there is rounding in both directions.
So, after distros switch to -mabi=ieeelongdouble by default or when people
use -mabi=ieeelongdouble on their programs, they'd better store that format
into data files by default, without the need of some magic CONVERT= options,
env vars or command line options.  Only in the case where they need to
interact with -mabi=ibmlongdouble environments, they need to take some
action.

Jakub

Re: [power-ieee128] OPEN CONV

[Jakub Jelinek via Fortran]

On Sat, Jan 08, 2022 at 12:10:56PM +0100, Jakub Jelinek via Gcc-patches wrote:
> One reason for that is that neither conversion is lossless, neither format
> is a subset or superset of the other.  Yes, IEEE quad has both much bigger
> exponent range (-16382..16383 vs. -1022..1023) and slightly bigger fixed
> precision (113 vs. 106 bits).
> But IBM extended has that weirdo numerically awful flexible precision where
> certain numbers can have much bigger precision than those 106 bits, up to
> 2048+52 or so.  So there is rounding in both directions.
> So, after distros switch to -mabi=ieeelongdouble by default or when people
> use -mabi=ieeelongdouble on their programs, they'd better store that format
> into data files by default, without the need of some magic CONVERT= options,
> env vars or command line options.  Only in the case where they need to
> interact with -mabi=ibmlongdouble environments, they need to take some
> action.

Note, as for byteswapping, apparently it wasn't ever working right fox
the IBM extended real(kind=16) and complex(kind=16).
Because unlike IEEE extended or integral types, it seems powerpc*-*-*
doesn't actually fully byteswap those between little and big endian.
Proof:
long double a = 
0.L;
compiled little endian IBM long double:
.size   a, 16
a:
.long   1431655765
.long   1070945621
.long   1431655766
.long   1014322517
compiled big endian IBM long double:
.size   a, 16
a:
.long   1070945621
.long   1431655765
.long   1014322517
.long   1431655766
compiled little endian IEEE long double:
.size   a, 16
a:
.long   1431655765
.long   1431655765
.long   1431655765
.long   1073567061
compiled big endian IEEE long double:
.size   a, 16
a:
.long   1073567061
.long   1431655765
.long   1431655765
.long   1431655765
where the numbers in .long arguments are 32-bit numbers stored in the
selected endianity.  Compiled with -mlong-double-64 little endian:
.size   a, 8
a:
.long   1431655765
.long   1070945621
and big endian:
.size   a, 8
a:
.long   1070945621
.long   1431655765
Unless I'm misreading this, for IEEE long double, or double (and I bet float
too) byteswapping the whole numbers is what is needed for interoperability
between powerpc64{,le}-linux, for IBM long double we'd actually want to
byteswap it as 2 real(kind=8) numbers and not one real(kind=16) one, i.e.
the numbers are always stored as the more significant double followed by
less significant double in memory.

Jakub

Re: [power-ieee128] OPEN CONV

[Jakub Jelinek via Fortran]

On Sat, Jan 08, 2022 at 03:13:10PM +0100, Thomas Koenig wrote:
> 
> On 08.01.22 15:02, Jakub Jelinek via Fortran wrote:
> > Note, as for byteswapping, apparently it wasn't ever working right fox
> > the IBM extended real(kind=16) and complex(kind=16).
> 
> The lack of bug reports since the conversion feature was introduced in
> 2006, more than 15 years ago, tells us something, I guess...

powerpc64le was only introduced in GCC 4.8 in 2013, so slightly less
than that, but still.
Either nobody interchanges/shares fortran unformatted data between
powerpc big and little endian, or if they do, they don't use real(kind=16)
or complex(kind=16) in there...

Jakub

Re: [PATCH] C, C++, Fortran, OpenMP: Add 'has_device_addr' clause to 'target' construct

[Jakub Jelinek via Fortran]

On Wed, Nov 24, 2021 at 06:08:02PM +0100, Marcel Vollweiler wrote:
> + case OMP_CLAUSE_HAS_DEVICE_ADDR:
> +   t = OMP_CLAUSE_DECL (c);
> +   if (TREE_CODE (t) == TREE_LIST)
> + {
> +   if (handle_omp_array_sections (c, ort))
> + remove = true;
> +   else
> + {
> +   t = OMP_CLAUSE_DECL (c);
> +   while (TREE_CODE (t) == ARRAY_REF)
> + t = TREE_OPERAND (t, 0);
> + }
> + }
> +   if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR)
> + bitmap_set_bit (&is_on_device_head, DECL_UID (t));

Why the OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR check?
There is no goto into this block nor fallthru into it, and
handle_omp_array_sections better shouldn't change OMP_CLAUSE_CODE.

> goto check_dup_generic;
>  
> + case OMP_CLAUSE_HAS_DEVICE_ADDR:
> +   t = OMP_CLAUSE_DECL (c);
> +   if (TREE_CODE (t) == TREE_LIST)
> + if (handle_omp_array_sections (c, ort))
> +   remove = true;
> + else
> +   {
> + t = OMP_CLAUSE_DECL (c);
> + while (TREE_CODE (t) == ARRAY_REF)
> +   t = TREE_OPERAND (t, 0);
> +   }
> +   if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_HAS_DEVICE_ADDR)
> + bitmap_set_bit (&is_on_device_head, DECL_UID (t));

Likewise.

> +   if (VAR_P (t) || TREE_CODE (t) == PARM_DECL)
> + cxx_mark_addressable (t);
> +   goto check_dup_generic_t;
> +
>   case OMP_CLAUSE_USE_DEVICE_ADDR:
> field_ok = true;
> t = OMP_CLAUSE_DECL (c);

> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
> @@ -1391,7 +1391,8 @@ enum
>OMP_LIST_USE_DEVICE_PTR,
>OMP_LIST_USE_DEVICE_ADDR,
>OMP_LIST_NONTEMPORAL,
> -  OMP_LIST_NUM
> +  OMP_LIST_HAS_DEVICE_ADDR,
> +  OMP_LIST_NUM  /* must be the last  */

Capital M and . at the end.

> @@ -2077,6 +2078,12 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const 
> omp_mask mask,
>   }
> break;
>   case 'h':
> +   if ((mask & OMP_CLAUSE_HAS_DEVICE_ADDR)
> +   && gfc_match_omp_variable_list
> +("has_device_addr (",
> + &c->lists[OMP_LIST_HAS_DEVICE_ADDR], false, NULL, NULL,
> +  true) == MATCH_YES)

Formatting, true should be IMO below &c->lists.

> + continue;
> if ((mask & OMP_CLAUSE_HINT)
> && (m = gfc_match_dupl_check (!c->hint, "hint", true, &c->hint))
>!= MATCH_NO)
> @@ -2850,7 +2857,8 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, const 
> omp_mask mask,
> if ((mask & OMP_CLAUSE_USE_DEVICE_ADDR)
> && gfc_match_omp_variable_list
>  ("use_device_addr (",
> - &c->lists[OMP_LIST_USE_DEVICE_ADDR], false) == MATCH_YES)
> + &c->lists[OMP_LIST_USE_DEVICE_ADDR], false, NULL, NULL,
> +  true) == MATCH_YES)

Likewise.

> --- a/gcc/fortran/trans-openmp.c
> +++ b/gcc/fortran/trans-openmp.c
> @@ -1910,7 +1910,17 @@ gfc_trans_omp_variable_list (enum omp_clause_code code,
>   tree t = gfc_trans_omp_variable (namelist->sym, declare_simd);
>   if (t != error_mark_node)
> {
> - tree node = build_omp_clause (input_location, code);
> + tree node;
> + /* For HAS_DEVICE_ADDR of an array descriptor, firstprivatize the
> +descriptor such that the bounds are available; its data component
> +is unmodified; it is handled as device address inside target. */
> + if (code == OMP_CLAUSE_HAS_DEVICE_ADDR
> + && (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (t))
> + || (POINTER_TYPE_P (TREE_TYPE (t))
> + && GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (TREE_TYPE (t))
> +   node = build_omp_clause (input_location, OMP_CLAUSE_FIRSTPRIVATE);

Not sure about the above,

> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -10024,6 +10024,15 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq 
> *pre_p,
> flags = GOVD_EXPLICIT;
> goto do_add;
>  
> + case OMP_CLAUSE_HAS_DEVICE_ADDR:
> +   decl = OMP_CLAUSE_DECL (c);
> +   if (TREE_CODE (decl) == ARRAY_REF)
> + {
> +   flags = GOVD_FIRSTPRIVATE | GOVD_EXPLICIT;
> +   while (TREE_CODE (decl) == ARRAY_REF)
> + decl = TREE_OPERAND (decl, 0);
> +   goto do_add_decl;

but this looks weird.
If decl after stripping the ARRAY_REFs is a var with pointer type, sure,
firstprivatizing it is the way to go.
But it can be also a variable with ARRAY_TYPE, can't it?  Something like:
  int a[64];
  #pragma omp target data map(a) use_device_addr(a)
  {
#pragma omp target has_device_addr(a[3:16])
a[3] = 1;
  }
and in this case firstprivatization of a looks wrong.  use_device_addr
should replace (but only at omp-low.c time I think) a used in the block
with 

Re: [power-ieee128, committed] Enable conversion selection via environment variable

[Jakub Jelinek via Fortran]

On Mon, Jan 10, 2022 at 11:44:13PM +0100, Thomas Koenig wrote:
> Hello world,
> 
> I have just pushed the attched patch to the branch.

Thanks.

Here is a patch to fix up the ppc64be vs. ppc64le byteswapping
of IBM extended real(kind=16) and complex(kind=16).
Similarly to the BT_COMPLEX case it halves size and doubles nelems
for the bswap_array calls.  Of course for r16_ibm and r16_ieee conversions
one needs to make sure it is only done when the on file data is in that
format and not in IEEE quad.

> So... time to merge the branch into trunk before stage 4
> kicks in?

IMHO yes.  We need to git merge master; git rebase of course
before trying to cherry-pick those commits into trunk and pushing there.

2022-01-11  Jakub Jelinek  

* io/transfer.c (unformatted_read, unformatted_write): When
byteswapping IBM extended real(kind=16), handle it as byteswapping
two real(kind=8) values.

--- libgfortran/io/transfer.c.jj2022-01-11 13:31:14.881806323 +0100
+++ libgfortran/io/transfer.c   2022-01-11 13:46:00.584288005 +0100
@@ -1145,11 +1145,28 @@ unformatted_read (st_parameter_dt *dtp,
  size /= 2;
}
 #ifndef HAVE_GFC_REAL_17
+#if defined(HAVE_GFC_REAL_16) && GFC_REAL_16_DIGITS == 106
+  /* IBM extended format is stored as a pair of IEEE754
+double values, with the more significant value first
+in both big and little endian.  */
+  if (kind == 16 && (type == BT_REAL || type == BT_COMPLEX))
+   {
+ nelems *= 2;
+ size /= 2;
+   }
+#endif
   bswap_array (dest, dest, size, nelems);
 #else
   unit_convert bswap = convert & ~(GFC_CONVERT_R16_IEEE | 
GFC_CONVERT_R16_IBM);
   if (bswap == GFC_CONVERT_SWAP)
-   bswap_array (dest, dest, size, nelems);
+   {
+ if ((type == BT_REAL || type == BT_COMPLEX)
+ && ((kind == 16 && (convert & GFC_CONVERT_R16_IEEE) == 0)
+ || (kind == 17 && (convert & GFC_CONVERT_R16_IBM
+   bswap_array (dest, dest, size / 2, nelems * 2);
+ else
+   bswap_array (dest, dest, size, nelems);
+   }
 
   if ((convert & GFC_CONVERT_R16_IEEE)
  && kind == 16
@@ -1274,6 +1291,18 @@ unformatted_write (st_parameter_dt *dtp,
  size /= 2;
}
 
+#if !defined(HAVE_GFC_REAL_17) && defined(HAVE_GFC_REAL_16) \
+&& GFC_REAL_16_DIGITS == 106
+  /* IBM extended format is stored as a pair of IEEE754
+double values, with the more significant value first
+in both big and little endian.  */
+  if (kind == 16 && (type == BT_REAL || type == BT_COMPLEX))
+   {
+ nelems *= 2;
+ size /= 2;
+   }
+#endif
+
   /* By now, all complex variables have been split into their
 constituent reals.  */
 
@@ -1321,7 +1350,12 @@ unformatted_write (st_parameter_dt *dtp,
  if ((dtp->u.p.current_unit->flags.convert
   & ~(GFC_CONVERT_R16_IEEE | GFC_CONVERT_R16_IBM))
  == GFC_CONVERT_SWAP)
-   bswap_array (buffer, buffer, size, nc);
+   bswap_array (buffer, buffer, size / 2, nc * 2);
+   }
+ else if (kind == 16 && (type == BT_REAL || type == BT_COMPLEX))
+   {
+ bswap_array (buffer, p, size / 2, nc * 2);
+ p += size * nc;
}
  else
 #endif


Jakub

Re: [PATCH] C, C++, Fortran, OpenMP: Add 'has_device_addr' clause to 'target' construct

[Jakub Jelinek via Fortran]

On Tue, Jan 11, 2022 at 02:27:57PM +0100, Tobias Burnus wrote:
> Hi Jakub, hi all,
> 
> let me quickly comment on 'has_device_addr' with Fortran arrays
> and with an array section (i.e. regarding your comment quoted
> at the very bottom of this email).
> 
> Unfortunately, the OpenMP specification is rather unclear
> what has_device_addr means for C/C++ array sections and in general
> for Fortran, especially when arrays, allocatables/pointers, and
> type parameters like nonconst string lengths are involved. Thus,
> I opened a spec issue – after some discussions (lang-spec meeting,
> C++/affinity (→ Fortran) meeting), it starts to converge:
> https://github.com/OpenMP/spec/issues/3180
> 
> If I understood it correctly, for C/C++, using has_device_addr with
> an array section implies firstprivate, while it does not without
> array section.

That seems just wrong for arrays, that will just crash, see below.

Cases like:
  struct S { whatever; } s;
  #pragma omp target data map (s) use_device_addr (s)
  {
// At this point it is invalid to use s.field etc. because
// &s is a device address
#pragma omp target has_device_addr (s)
{
  access (&s);
}
  }
and s/struct S { whatever; } s/int s[16];/
are similar, in all the cases use_device_addr will replace
s in the body with *device_addr_of_s and has_device_addr
needs to firstprivatize the artificial address of s and ensure
that even in the target body s is *some_addr_of_s.
And IMHO array sections should be treated the same, just the
target data could map only parts of the array and not the whole
array, but still device_addr_of_s will be something pointing to the
start of the array, perhaps before the actual object allocated on the
device.
So, I think the gimplifier should strip the ARRAY_REFs and if that yields
something with ARRAY_TYPE, should just treat that var as what appeared
in the has_device_addr clause.  Only if it the array section has
a base pointer that base pointer needs to be copied to the device as is
and so the artificial firstprivate on that pointer that copies the pointer
to the device code.

> Side remark: I note that use_device_addr permits array sections,
> but GCC does not support them yet. (Useful when doing a partial
> map of an array + 'omp data use_device_addr()' on the partially
> mapped array.)

Yes, we should implement that.  But even without that supported, one can
have:
  int a[32];
  #pragma omp target data map (a) use_device_addr (a)
  {
// So, &a[0] is now a device pointer, whole a is mapped
#pragma omp target has_device_addr (a[3:17])
{
  ++a[3];
}
  }
When whole a[0:32] is mapped, obviously a[3:17] is mapped too
and for has_device-addr it IMHO should act like has_device_addr (a)
under the hood, except the user doesn't guarantee that the whole
array is mapped, just that a has a device address.

Now, if we treat the has_device_addr (a[3:17]) in the above testcase
as firstprivate (a), that will mean we try to copy the whole array from
host to the device.  But &a[0] etc. aren't host addresses, they are device
addresses, so unless the host can access the device addresses, that will
segfault.

Jakub

Re: [PATCH] Mass rename of C++ .c files to .cc suffix

[Jakub Jelinek via Fortran]

On Tue, Jan 11, 2022 at 04:50:02PM +0100, Martin Liška wrote:
> On 1/11/22 16:48, Toon Moene wrote:
> > On 1/11/22 13:56, Martin Liška wrote:
> > 
> > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > > Plus it survives build of all FEs (--enable-languages=all) on 
> > > x86_64-linux-gnu
> > > and I've built all cross compilers.
> > 
> > Does this also rename .c files in the fortran and libgfortran directories ?
> 
> Hello.
> 
> Yes, it does the first one.

And it shouldn't in libgfortran - libgfortran, libgcc, libgomp, libquadmath are 
all
written in C, not C++.
While e.g. libcpp or gcc are in C++.

Jakub

Re: [PATCH] Mass rename of C++ .c files to .cc suffix

[Jakub Jelinek via Fortran]

On Tue, Jan 11, 2022 at 05:03:34PM +0100, Martin Liška wrote:
> On 1/11/22 16:56, Jakub Jelinek wrote:
> > While e.g. libcpp or gcc are in C++.
> 
> Which means I should rename .c files under libcpp, right?
> Is there any other folder from gcc/ and libcpp/ that would need that as well?

>From the directories that use .c files for C++, I think that is all.
c++tools/, libcody/, libitm/, libsanitizer/, libstdc++-v3/ already
use .cc or .cpp.

Jakub

Re: [PATCH] Mass rename of C++ .c files to .cc suffix

[Jakub Jelinek via Fortran]

On Tue, Jan 11, 2022 at 06:23:51PM +, Jonathan Wakely via Gcc-patches wrote:
> > Regarding fortran: can we have a vote on this one?
> >
> > Some developers (including myself) are not really familiar with C++,
> > and in the past preference has been expressed on the fortran ML in
> > favor of not using too much C++.
> >
> > I would also not really be in a position to review real C++ code.
> 
> The discussion is purely about renaming files that are *already* C++
> source files but have the misleading .c file extension.
> 
> Nobody is suggesting using C++ where it isn't already being used.

And even gcc/fortran/ is written in C++, the gcc/ headers it uses are C++
only, they use templates etc., so gcc/fortran/ that uses those headers
has to be C++ too.  That doesn't mean you need to use C++ idioms everywhere
in your code, those files can stay to be mostly C-like with C++ headers and
use C++-only constructs only where it brings sufficient advantages.
Many of the gcc/*.c sources that Martin wants to rename are also written
like that.
The renaming will just match the reality, clang++ will stop warning that
support for .c extension for C++ is deprecated when building gcc, sites like
openhub.net (if they twice a year manage to build stats for gcc, dunno what
they are doing wrong or if it is because of the limiting of anonymous git
on sourceware) will not claim most of GCC is written in C etc.

Jakub

Re: [power-ieee128, committed] Enable conversion selection via environment variable

[Jakub Jelinek via Fortran]

On Tue, Jan 11, 2022 at 10:44:56PM +0100, Thomas Koenig wrote:
> > > So... time to merge the branch into trunk before stage 4
> > > kicks in?
> > 
> > IMHO yes.  We need to git merge master; git rebase of course
> > before trying to cherry-pick those commits into trunk and pushing there.
> 
> I would prefer if somebody else did this - my lack of git-fu would
> forseeably lead to some unforseen problems :-)

My git-fu is limited too, but I've pushed it to trunk now.

Jakub

Re: libgfortran bootstrap failure

[Jakub Jelinek via Fortran]

On Tue, Jan 11, 2022 at 10:30:26PM -0500, David Edelsohn wrote:
> The recent patch to support Power IEEE128 causes a bootstrap failure
> on AIX and possibly all non-GLIBC systems.
> 
> +#if defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ \
> +&& defined __GLIBC_PREREQ && __GLIBC_PREREQ (2, 32)
> +#define POWER_IEEE128 1
> +#endif
> 
> __GLIBC_PREREQ is tested on all systems.
> 
> /nasfarm/edelsohn/src/src/libgfortran/libgfortran.h:107:49: error:
> missing binary operator before token "("
>   107 | && defined __GLIBC_PREREQ && __GLIBC_PREREQ (2, 32)
>   | ^

Guess we need:
#if defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ \
&& defined __GLIBC_PREREQ
# if __GLIBC_PREREQ (2, 32)
#  define POWER_IEEE128 1
# endif
#endif
instead.

Jakub

Re: libgfortran bootstrap failure

[Jakub Jelinek via Fortran]

On Tue, Jan 11, 2022 at 10:30:26PM -0500, David Edelsohn wrote:
> The recent patch to support Power IEEE128 causes a bootstrap failure
> on AIX and possibly all non-GLIBC systems.
> 
> +#if defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ \
> +&& defined __GLIBC_PREREQ && __GLIBC_PREREQ (2, 32)
> +#define POWER_IEEE128 1
> +#endif
> 
> __GLIBC_PREREQ is tested on all systems.
> 
> /nasfarm/edelsohn/src/src/libgfortran/libgfortran.h:107:49: error:
> missing binary operator before token "("
>   107 | && defined __GLIBC_PREREQ && __GLIBC_PREREQ (2, 32)
>   | ^

This is what I've committed as obvious:

2022-01-12  Jakub Jelinek  

* libgfortran.h (POWER_IEEE128): Use __GLIBC_PREREQ in a separate
#if directive inside of #if ... && defined __GLIBC_PREREQ.

--- libgfortran/libgfortran.h.jj2022-01-11 23:49:51.759830402 +0100
+++ libgfortran/libgfortran.h   2022-01-12 09:41:48.779107854 +0100
@@ -104,9 +104,11 @@ typedef off_t gfc_offset;
 #endif
 
 #if defined(__powerpc64__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ \
-&& defined __GLIBC_PREREQ && __GLIBC_PREREQ (2, 32)
+&& defined __GLIBC_PREREQ
+#if __GLIBC_PREREQ (2, 32)
 #define POWER_IEEE128 1
 #endif
+#endif
 
 /* These functions from  should only be used on values that can be
represented as unsigned char, otherwise the behavior is undefined.


Jakub

Re: [PATCH] Fortran: make IEEE_CLASS recognize signaling NaNs

[Jakub Jelinek via Fortran]

On Wed, Jan 12, 2022 at 11:23:43AM +0100, FX via Gcc-patches wrote:
> I can confirm that I don’t see this failure on a Debian bullseye/sid (Linux 
> 5.11.0-46, glibc 2.31-0ubuntu9.2) with a fresh bootstrap of master:
> 
> $ grep signaling testsuite/gfortran/gfortran.sum
> PASS: gfortran.dg/ieee/signaling_1.f90   -O0  (test for excess errors)
> PASS: gfortran.dg/ieee/signaling_1.f90   -O0  execution test
> PASS: gfortran.dg/ieee/signaling_1.f90   -O1  (test for excess errors)
> PASS: gfortran.dg/ieee/signaling_1.f90   -O1  execution test
> PASS: gfortran.dg/ieee/signaling_1.f90   -O2  (test for excess errors)
> PASS: gfortran.dg/ieee/signaling_1.f90   -O2  execution test
> PASS: gfortran.dg/ieee/signaling_1.f90   -O3 -fomit-frame-pointer 
> -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for excess 
> errors)
> PASS: gfortran.dg/ieee/signaling_1.f90   -O3 -fomit-frame-pointer 
> -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> PASS: gfortran.dg/ieee/signaling_1.f90   -O3 -g  (test for excess errors)
> PASS: gfortran.dg/ieee/signaling_1.f90   -O3 -g  execution test
> PASS: gfortran.dg/ieee/signaling_1.f90   -Os  (test for excess errors)
> PASS: gfortran.dg/ieee/signaling_1.f90   -Os  execution test

The error I was getting was:
/home/jakub/src/gcc/obj46/gcc/testsuite/gfortran2/../../gfortran 
-B/home/jakub/src/gcc/obj46/gcc/testsuite/gfortran2/../../ 
-B/home/jakub/src/gcc/obj46/x86_64-pc
-linux-gnu/./libgfortran/ 
/home/jakub/src/gcc/gcc/testsuite/gfortran.dg/ieee/signaling_1.f90 
-fdiagnostics-plain-output -fdiagnostics-plain-output -O1 -fsignaling-nans 
/home/jakub/sr
c/gcc/gcc/testsuite/gfortran.dg/ieee/signaling_1_c.c -dumpbase  
-B/home/jakub/src/gcc/obj46/x86_64-pc-linux-gnu/./libgfortran/.libs 
-L/home/jakub/src/gcc/obj46/x86_64-pc-linux-gnu/./
libgfortran/.libs 
-L/home/jakub/src/gcc/obj46/x86_64-pc-linux-gnu/./libgfortran/.libs 
-L/home/jakub/src/gcc/obj46/x86_64-pc-linux-gnu/./libatomic/.libs 
-B/home/jakub/src/gcc/obj46/x8
6_64-pc-linux-gnu/./libquadmath/.libs 
-L/home/jakub/src/gcc/obj46/x86_64-pc-linux-gnu/./libquadmath/.libs 
-L/home/jakub/src/gcc/obj46/x86_64-pc-linux-gnu/./libquadmath/.libs -lm -o .
/signaling_1.exe
/home/jakub/src/gcc/gcc/testsuite/gfortran.dg/ieee/signaling_1.f90:8:20: Fatal 
Error: Cannot find an intrinsic module named 'ieee_arithmetic' at (1)
compilation terminated.
compiler exited with status 1
FAIL: gfortran.dg/ieee/signaling_1.f90   -O1  (test for excess errors)
Excess errors:
/home/jakub/src/gcc/gcc/testsuite/gfortran.dg/ieee/signaling_1.f90:8:20: Fatal 
Error: Cannot find an intrinsic module named 'ieee_arithmetic' at (1)
compilation terminated.

UNRESOLVED: gfortran.dg/ieee/signaling_1.f90   -O1  compilation failed to 
produce executable
And
-! { dg-options "-fsignaling-nans" }
+! { dg-additional-options "-fsignaling-nans" }
doesn't fix it, it changes the FAIL into:
cc1: warning: command-line option '-fintrinsic-modules-path 
/home/jakub/src/gcc/obj46/x86_64-pc-linux-gnu/./libgfortran/' is valid for 
Fortran but not for C
FAIL: gfortran.dg/ieee/signaling_1.f90   -O1  (test for excess errors)
Excess errors:
cc1: warning: command-line option '-fintrinsic-modules-path 
/home/jakub/src/gcc/obj46/x86_64-pc-linux-gnu/./libgfortran/' is valid for 
Fortran but not for C

We need -fintrinsic-modules-path option for the signalling_1.f90 compilation
but need to make sure it isn't used when the *.c file is compiled, so they
need to be compiled by separate compiler invocations probably.

Jakub

Re: [PATCH] Fortran: make IEEE_CLASS recognize signaling NaNs

[Jakub Jelinek via Fortran]

On Wed, Jan 12, 2022 at 12:03:40PM +0100, FX wrote:
> > We need -fintrinsic-modules-path option for the signalling_1.f90 compilation
> > but need to make sure it isn't used when the *.c file is compiled, so they
> > need to be compiled by separate compiler invocations probably.
> 
> Thanks for posting the errors!  So I wasn’t seeing it because I had run
> “make install” before the testsuite.  The patch I pushed at
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=6b14100b9504800768da726dcb81f1857db3b493
> should fix the failure, then.

Thanks.  Just a nit, it is cc1 that reports the warning, not f951.

Jakub

[committed] libgfortran: Fix Solaris version file creation [PR104006]

[Jakub Jelinek via Fortran]

Hi!

I forgot to change the gfortran.map-sun goal to gfortran.ver-sun
when changing other spots for the preprocessed version file.

Fixed thusly, committed to trunk as obvious.

2022-01-13  Jakub Jelinek  

PR libfortran/104006
* Makefile.am (gfortran.map-sun): Rename target to ...
(gfortran.ver-sun): ... this.
* Makefile.in: Regenerated.

--- libgfortran/Makefile.am.jj  2022-01-13 17:43:58.685553296 +0100
+++ libgfortran/Makefile.am 2022-01-13 17:44:40.503962317 +0100
@@ -23,7 +23,7 @@ endif
 if LIBGFOR_USE_SYMVER_SUN
 version_arg = -Wl,-M,gfortran.ver-sun
 version_dep = gfortran.ver-sun gfortran.ver
-gfortran.map-sun : gfortran.ver \
+gfortran.ver-sun : gfortran.ver \
$(top_srcdir)/../contrib/make_sunver.pl \
$(libgfortran_la_OBJECTS) $(libgfortran_la_LIBADD)
perl $(top_srcdir)/../contrib/make_sunver.pl \
--- libgfortran/Makefile.in.jj  2022-01-13 17:43:58.687553267 +0100
+++ libgfortran/Makefile.in 2022-01-13 17:44:51.468807363 +0100
@@ -719,7 +719,6 @@ pdfdir = @pdfdir@
 prefix = @prefix@
 program_transform_name = @program_transform_name@
 psdir = @psdir@
-runstatedir = @runstatedir@
 sbindir = @sbindir@
 sharedstatedir = @sharedstatedir@
 srcdir = @srcdir@
@@ -7616,7 +7615,7 @@ uninstall-am: uninstall-cafexeclibLTLIBR
 @libgfor_use_symver_t...@gfortran.ver: $(srcdir)/gfortran.map kinds.inc
 @LIBGFOR_USE_SYMVER_TRUE@  $(EGREP) -v '#(#| |$$)' $< | \
 @LIBGFOR_USE_SYMVER_TRUE@$(PREPROCESS) -P -include config.h -include 
kinds.inc - > $@ || (rm -f $@ ; exit 1)
-@LIBGFOR_USE_SYMVER_SUN_TRUE@@libgfor_use_symver_t...@gfortran.map-sun : 
gfortran.ver \
+@LIBGFOR_USE_SYMVER_SUN_TRUE@@libgfor_use_symver_t...@gfortran.ver-sun : 
gfortran.ver \
 @LIBGFOR_USE_SYMVER_SUN_TRUE@@LIBGFOR_USE_SYMVER_TRUE@ 
$(top_srcdir)/../contrib/make_sunver.pl \
 @LIBGFOR_USE_SYMVER_SUN_TRUE@@LIBGFOR_USE_SYMVER_TRUE@ 
$(libgfortran_la_OBJECTS) $(libgfortran_la_LIBADD)
 @LIBGFOR_USE_SYMVER_SUN_TRUE@@LIBGFOR_USE_SYMVER_TRUE@ perl 
$(top_srcdir)/../contrib/make_sunver.pl \

Jakub

[committed] libgfortran: Partly revert my r12-6498 change to fix Solaris build [PR104006]

[Jakub Jelinek via Fortran]

Hi!

In r12-6498 I've added $(version_dep) to BUILT_SOURCES, previously version_dep
on Linux used to be a file in $(srcdir), but with my changes it is a generated
file in the object directory (preprocessed version of the $(srcdir) file)
and I thought generated files belong to BUILT_SOURCES so that they are
cleaned up etc.
For Linux that is fine, but it broke parallel builds on Solaris.
BUILT_SOURCES is a special variable for automake where automake ensures
that for make all, make check and make install all those $(BUILT_SOURCES)
are generated before actually starting building in parallel the various
object files.  That way we can avoid hacks like:
$(patsubst %.F90,%.lo,$(notdir $(filter %.F90,$(prereq_SRC: kinds.inc 
c99_protos.inc
$(patsubst %.c,%.lo,$(notdir $(filter %.c,$(prereq_SRC: kinds.h
$(patsubst %.f90,%.lo,selected_real_kind.f90): selected_real_kind.inc
$(patsubst %.f90,%.lo,selected_int_kind.f90): selected_int_kind.inc
$(patsubst %.F90,%.lo,ieee_exceptions.F90): fpu-target.inc
$(patsubst %.F90,%.lo,ieee_arithmetic.F90): fpu-target.inc ieee_exceptions.lo
$(patsubst %.c,%.lo,fpu.c): fpu-target.h
which makes those dependencies explicit but hides it from automake, so that it
doesn't throw away its rules for those object files.
On Solaris, $(version_dep) contains gfortran.ver and gfortran.ver-sun.
gfortran.ver is like on Linux, it can be in $(BUILT_SOURCES), but unfortunately
gfortran.ver-sun depends on all the object files being compiled already,
so if gfortran.ver-sun appears in $(BUILT_SOURCES), the BUILT_SOURCES function
of ensuring the generated files are generated before building object files
is gone, almost everything is built before all-am is entered and there
are no explicit dependencies that e.g. *.F90 files depend on
kinds.inc etc.

So, this change reverts that mistake and instead adds $(version_dep) to
what is removed during make clean (clean-local in particular).

The reversion committed under the reversion of our own patches policy,
the clean-local change considered obvious, committed to trunk.

2022-01-14  Jakub Jelinek  

PR libfortran/104006
* Makefile.am (BUILT_SOURCES): Don't include $(version_dep).
(clean-local): Remove $(version_dep).
* Makefile.in: Regenerated.

--- libgfortran/Makefile.am.jj  2022-01-13 17:44:40.503962317 +0100
+++ libgfortran/Makefile.am 2022-01-13 23:37:52.876004924 +0100
@@ -1118,7 +1118,7 @@ ieee_arithmetic.mod: ieee_arithmetic.lo
:
 
 BUILT_SOURCES=$(gfor_built_src) $(gfor_built_specific_src) \
-   $(gfor_built_specific2_src) $(gfor_misc_specifics) $(version_dep)
+   $(gfor_built_specific2_src) $(gfor_misc_specifics)
 
 prereq_SRC = $(gfor_src) $(gfor_built_src) $(gfor_io_src) \
$(gfor_helper_src) $(gfor_ieee_src) $(gfor_io_headers) 
$(gfor_specific_src)
@@ -1356,7 +1356,7 @@ $(gfor_misc_specifics): m4/misc_specific
 endif
 
 clean-local:
-   -rm -rf include
+   -rm -rf include $(version_dep)
 
 EXTRA_DIST = $(m4_files)
 
--- libgfortran/Makefile.in.jj  2022-01-13 17:44:51.468807363 +0100
+++ libgfortran/Makefile.in 2022-01-13 23:37:58.729923341 +0100
@@ -1652,7 +1652,7 @@ intrinsics/random_init.f90
 
 BUILT_SOURCES = $(gfor_built_src) $(gfor_built_specific_src) \
$(gfor_built_specific2_src) $(gfor_misc_specifics) \
-   $(version_dep) $(am__append_7)
+   $(am__append_7)
 prereq_SRC = $(gfor_src) $(gfor_built_src) $(gfor_io_src) \
$(gfor_helper_src) $(gfor_ieee_src) $(gfor_io_headers) 
$(gfor_specific_src)
 
@@ -7857,7 +7857,7 @@ include/ISO_Fortran_binding.h: $(srcdir)
 @MAINTAINER_MODE_TRUE@ $(M4) -Dfile=$@ -I$(srcdir)/m4 misc_specifics.m4 > $@
 
 clean-local:
-   -rm -rf include
+   -rm -rf include $(version_dep)
 
 # target overrides
 -include $(tmake_file)

Jakub

Re: [PATCH] [gfortran] Add support for allocate clause (OpenMP 5.0).

[Jakub Jelinek via Fortran]

On Fri, Jan 14, 2022 at 12:45:54PM +0100, Tobias Burnus wrote:
> On 14.01.22 10:10, Thomas Schwinge wrote:
> > > +  integer  :: x
> > > ...
> > > +  !$omp parallel allocate (0: x) private(x) ! { dg-error "Expected 
> > > integer expression of the 'omp_allocator_handle_kind' kind at .1." }
> > We do for x86_64 default '-m64', but for '-m32' and '-mx32' compilation,
> > we're not seeing this latter diagnostic:
> >  FAIL: gfortran.dg/gomp/allocate-2.f90   -O   (test for errors, line 36)
> > 
> > I suppose the reason is unintended congruence of data types?  Would it
> > work to make 'x' a floating-point data type, for example -- or is this
> > meant to explicitly check certain integer data type characteristics?
> 
> Alternatively, you could use 'integer(kind=1)' (which is a 1-byte/8-bits
> type.) I assume we do not have any platform which still uses 8-bit
> pointers and supports libgomp :-)

If we want to check intptr_t, we should guard the dg-error with
"" { target { lp64 || llp64 } }
or so.
Otherwise yes, we can add some other kind and hope it is not the
same as omp_allocator_handle_kind.  Or we can do both,
keep the current one with the target lp64 || llp64 and
add another one with some integer(kind=1).

Jakub

Re: [PATCH] OpenMP front-end: allow requires dynamic_allocators

[Jakub Jelinek via Fortran]

On Mon, Dec 20, 2021 at 03:16:23PM +, Andrew Stubbs wrote:
> This patch removes the "sorry" message for the OpenMP "requires
> dynamic_allocators" feature in C, C++ and Fortran.
> 
> The clause is supposed to state that the user code will not work without the
> omp_alloc/omp_free and omp_init_allocator/omp_destroy_allocator and these
> things *are* present, so there should be no problem allowing it.
> 
> I can't see any reason for our implementation to *do* anything with this
> statement -- it's fine for the allocators to work the same with or without
> it.

Well, we should do a lot with it, but that can wait for later.
In particular, if OMP_REQUIRES_DYNAMIC_ALLOCATORS is not present,
we should be rejecting omp_init_allocator and omp_destroy_allocator
in target regions (maybe just a warning though), and for allocate clauses,
directives, and the omp_alloc etc. APIs we should in the target region
enforce they don't use omp_null_allocator and the allocators they use
is mentioned in uses_allocators (and implement uses_allocators).
But, right now it is unclear to me how exactly is that supposed to work
with declare target to routines, those aren't nested inside of the target
and so can't know what uses_allocator is used, so they can't make any
allocations?  Or is it just that we can't diagnose this in such routines
and only can diagnose omp_null_allocator isn't used...
> 
> I think this patch ought to be fine for GCC 12, but if not it can wait until
> stage 1 opens. I shall backport this to OG11 shortly.

Anyway, your patch is a step forward, as we don't support uses_allocators,
the non-requires dynamic_allocators way is unusable for any target region
allocations due to that and so requires dynamic_allocators is the only thing
we actually support, but we were rejecting that directive...

Ok and sorry for the delay.

> gcc/c/ChangeLog:
> 
>   * c-parser.c (c_parser_omp_requires): Don't "sorry" dynamic_allocators.
> 
> gcc/cp/ChangeLog:
> 
>   * parser.c (cp_parser_omp_requires): Don't "sorry" dynamic_allocators.
> 
> gcc/fortran/ChangeLog:
> 
>   * openmp.c (gfc_match_omp_requires): Don't "sorry" dynamic_allocators.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/requires-8.f90: Reinstate dynamic allocators
>   requirement.

Jakub

[PATCH] fortran: Extend -fconvert= option for ppc64le r16_ieee and r16_ibm

[Jakub Jelinek via Fortran]

Hi!

This patch on top of the previously posted option handling changes patch
allows specifying -fconvert=swap,r16_ieee etc. (but will error on it
when not on powerpc64le because in the library such swapping is only
implemented for HAVE_REAL_17).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2022-01-21  Jakub Jelinek  

* lang.opt (fconvert=): Add EnumSet property and mention also
r16_ieee and r16_ibm arguments.
(big-endian, little-endian, native, swap): Add Set(1) property.
(r16_ieee, r16_ibm): New EnumValue entries with Set(2) property.
* trans-types.cc (gfc_init_kinds): Emit gfc_fatal_error for
-fconvert=r16_ieee or -fconvert=r16_ibm when R16_IEEE <=> R16_IBM
conversions aren't supported.

--- gcc/fortran/lang.opt.jj 2022-01-11 23:49:52.167824673 +0100
+++ gcc/fortran/lang.opt2022-01-21 15:15:15.494099716 +0100
@@ -421,23 +421,29 @@ Fortran
 Produce a warning at runtime if a array temporary has been created for a 
procedure argument.
 
 fconvert=
-Fortran RejectNegative Joined Enum(gfc_convert) Var(flag_convert) 
Init(GFC_FLAG_CONVERT_NATIVE)
--fconvert=   The endianness used for 
unformatted files.
+Fortran RejectNegative Joined Enum(gfc_convert) EnumSet Var(flag_convert) 
Init(GFC_FLAG_CONVERT_NATIVE)
+-fconvert=  The 
endianness used for unformatted files.
 
 Enum
 Name(gfc_convert) Type(enum gfc_convert) UnknownError(Unrecognized option to 
endianness value: %qs)
 
 EnumValue
-Enum(gfc_convert) String(big-endian) Value(GFC_FLAG_CONVERT_BIG)
+Enum(gfc_convert) String(big-endian) Value(GFC_FLAG_CONVERT_BIG) Set(1)
 
 EnumValue
-Enum(gfc_convert) String(little-endian) Value(GFC_FLAG_CONVERT_LITTLE)
+Enum(gfc_convert) String(little-endian) Value(GFC_FLAG_CONVERT_LITTLE) Set(1)
 
 EnumValue
-Enum(gfc_convert) String(native) Value(GFC_FLAG_CONVERT_NATIVE)
+Enum(gfc_convert) String(native) Value(GFC_FLAG_CONVERT_NATIVE) Set(1)
 
 EnumValue
-Enum(gfc_convert) String(swap) Value(GFC_FLAG_CONVERT_SWAP)
+Enum(gfc_convert) String(swap) Value(GFC_FLAG_CONVERT_SWAP) Set(1)
+
+EnumValue
+Enum(gfc_convert) String(r16_ieee) Value(GFC_FLAG_CONVERT_R16_IEEE) Set(2)
+
+EnumValue
+Enum(gfc_convert) String(r16_ibm) Value(GFC_FLAG_CONVERT_R16_IBM) Set(2)
 
 fcray-pointer
 Fortran Var(flag_cray_pointer)
--- gcc/fortran/trans-types.cc.jj   2022-01-18 11:58:59.579982099 +0100
+++ gcc/fortran/trans-types.cc  2022-01-21 20:26:29.438558960 +0100
@@ -527,6 +527,9 @@ gfc_init_kinds (void)
  }
  }
 }
+  else if ((flag_convert & (GFC_CONVERT_R16_IEEE | GFC_CONVERT_R16_IBM)) != 0)
+gfc_fatal_error ("%<-fconvert=r16_ieee%> or %<-fconvert=r16_ibm%> not "
+"supported on this architecture");
 
   /* Choose the default integer kind.  We choose 4 unless the user directs us
  otherwise.  Even if the user specified that the default integer kind is 8,

Jakub

Re: [PATCH] Fortran: detect signaling NaNs on targets without issignaling macro in libc

[Jakub Jelinek via Fortran]

On Mon, Jan 17, 2022 at 12:11:59AM +0100, FX via Gcc-patches wrote:
> This patch is the third in my “signaling NaN” series. For targets with IEEE 
> support but without the issignaling macro in libc (i.e., everywhere except 
> glibc), this allows us to provide a fallback implementation. In order to keep 
> the code in ieee_helper.c relatively readable, I’ve put that new 
> implementation in a separate file, issignaling_fallback.h.
> 
> The logic is borrowed from different routines in glibc, but gathered into a 
> single file and much simpler than the glibc implementation, because we do not 
> need to cover all the cases they have (comments with details are available in 
> issignaling_fallback.h).
> 
> I can’t test this on all the targets I’d like to, obviously. But it was 
> tested on x86_64-pc-linux-gnu (where it doesn’t do anything), on 
> x86_64-pc-linux-gnu by mimicking the lack of a issignaling macro, and on 
> x86_64-apple-darwin (which does not have issignaling).

This doesn't seem to handle the powerpc* IBM double double long double.

__LDBL_IS_IEC_60559__ isn't defined for this type, because it is far from
an IEEE754 type, but it has signaling NaNs - as can be seen in glibc
libc/sysdeps/ieee754/ldbl-128ibm/s_issignalingl.c
the type is a pair of doubles and whether it is a sNaN or qNaN is determined
by whether the first double is a sNaN or qNaN.

Ok for trunk?

2022-01-25  Jakub Jelinek  

* ieee/issignaling_fallback.h (__issignalingl): Define for
IBM extended long double are returning __issignaling on the
first double.

--- libgfortran/ieee/issignaling_fallback.h.jj  2022-01-25 12:14:45.404232320 
+0100
+++ libgfortran/ieee/issignaling_fallback.h 2022-01-25 12:14:52.504131720 
+0100
@@ -137,6 +137,19 @@ __issignalingl (long double x)
   return ret || (((exi & 0x7fff) == 0x7fff) && (hxi > 0xc000));
 }
 
+#elif (__LDBL_DIG__ == 31)
+
+/* Long double is 128-bit IBM extended type.  */
+
+static inline int
+__issignalingl (long double x)
+{
+  union { long double value; double parts[2]; } u;
+
+  u.value = x;
+  return __issignaling (u.parts[0]);
+}
+
 #elif (__LDBL_DIG__ == 33) && __LDBL_IS_IEC_60559__
 
 /* Long double is 128-bit type.  */


Jakub

powerpc64le real(kind=16) and IEEE_{ARITHMETIC,EXCEPTIONS} modules

[Jakub Jelinek via Fortran]

Hi!

Apparently something we (at least I) have totally missed, we clearly have a
problem with the IEEE modules for the dual -mabi={ibm,ieee}longdouble.
We have:
__ieee_arithmetic_MOD_ieee_class_16;
__ieee_arithmetic_MOD_ieee_support_datatype_16;
__ieee_arithmetic_MOD_ieee_support_denormal_16;
__ieee_arithmetic_MOD_ieee_support_divide_16;
__ieee_arithmetic_MOD_ieee_support_inf_16;
__ieee_arithmetic_MOD_ieee_support_io_16;
__ieee_arithmetic_MOD_ieee_support_nan_16;
__ieee_arithmetic_MOD_ieee_support_rounding_16;
__ieee_arithmetic_MOD_ieee_support_sqrt_16;
__ieee_arithmetic_MOD_ieee_support_standard_16;
__ieee_arithmetic_MOD_ieee_support_underflow_control_16;
__ieee_arithmetic_MOD_ieee_value_16;
__ieee_exceptions_MOD_ieee_support_flag_16;
  __ieee_arithmetic_MOD_ieee_support_subnormal_16;
exported from the library, but no corresponding _17 entrypoints.
1) is there any way how to define them in a different source file
   so that they can be compiled with -mabi=ieeelongdouble?
2) I'm afraid interfaces like:
  interface IEEE_IS_NEGATIVE
procedure &
#ifdef HAVE_GFC_REAL_16
  _gfortran_ieee_is_negative_16, &
#endif
#ifdef HAVE_GFC_REAL_10
  _gfortran_ieee_is_negative_10, &
#endif
  _gfortran_ieee_is_negative_8, _gfortran_ieee_is_negative_4
  end interface
  public :: IEEE_IS_NEGATIVE
  just aren't going to work for the real(kind=16) case
  when it has abi_kind 17, the FE will still resolve it to the
  _16 case or error out that we have two real(kind=16) entries.
Can everything these modules do be resolved at compile time inline
such that for the abi_kind 17 nothing is really called, or
any other thoughts on this?

Jakub

Re: [PATCH] C, C++, Fortran, OpenMP: Add 'has_device_addr' clause to 'target' construct

[Jakub Jelinek via Fortran]

On Wed, Feb 02, 2022 at 09:19:03AM +0100, Marcel Vollweiler wrote:
> gcc/c-family/ChangeLog:
> 
>   * c-omp.cc (c_omp_split_clauses): Added OMP_CLAUSE_HAS_DEVICE_ADDR case.
>   * c-pragma.h (enum pragma_kind): Added 5.1 in comment.
>   (enum pragma_omp_clause): Added PRAGMA_OMP_CLAUSE_HAS_DEVICE_ADDR.
> 
> gcc/c/ChangeLog:
> 
>   * c-parser.cc (c_parser_omp_clause_name): Parse 'has_device_addr'
>   clause.
>   (c_parser_omp_variable_list): Handle array sections.
>   (c_parser_omp_clause_has_device_addr): Added.
>   (c_parser_omp_all_clauses): Added PRAGMA_OMP_CLAUSE_HAS_DEVICE_ADDR
>   case.
>   (c_parser_omp_target_exit_data): Added HAS_DEVICE_ADDR to 
>   OMP_CLAUSE_MASK.
>   * c-typeck.cc (handle_omp_array_sections): Handle clause restrictions.
>   (c_finish_omp_clauses): Handle array sections.
> 
> gcc/cp/ChangeLog:
> 
>   * parser.cc (cp_parser_omp_clause_name): Parse 'has_device_addr' clause.
>   (cp_parser_omp_var_list_no_open): Handle array sections.
>   (cp_parser_omp_all_clauses): Added PRAGMA_OMP_CLAUSE_HAS_DEVICE_ADDR
>   case.
>   (cp_parser_omp_target_update): Added HAS_DEVICE_ADDR to OMP_CLAUSE_MASK.
>   * pt.c (tsubst_omp_clauses): Added cases for OMP_CLAUSE_HAS_DEVICE_ADDR.
>   * semantics.cc (handle_omp_array_sections): Handle clause restrictions.
>   (finish_omp_clauses): Handle array sections.
> 
> gcc/fortran/ChangeLog:
> 
>   * dump-parse-tree.cc (show_omp_clauses): Added OMP_LIST_HAS_DEVICE_ADDR
>   case.
>   * gfortran.h: Added OMP_LIST_HAS_DEVICE_ADDR.
>   * openmp.cc (enum omp_mask2): Added OMP_CLAUSE_HAS_DEVICE_ADDR.
>   (gfc_match_omp_clauses): Parse HAS_DEVICE_ADDR clause.
>   (resolve_omp_clauses): Same.
>   * trans-openmp.cc (gfc_trans_omp_variable_list): Added 
>   OMP_LIST_HAS_DEVICE_ADDR case.
>   (gfc_trans_omp_clauses): Firstprivatize of array descriptors.
> 
> gcc/ChangeLog:
> 
>   * gimplify.cc (gimplify_scan_omp_clauses): Added cases for
>   OMP_CLAUSE_HAS_DEVICE_ADDR
>   and handle array sections.
>   (gimplify_adjust_omp_clauses): Added OMP_CLAUSE_HAS_DEVICE_ADDR case.
>   * omp-low.cc (scan_sharing_clauses): Handle OMP_CLAUSE_HAS_DEVICE_ADDR.
>   (lower_omp_target): Same.
>   * tree-core.h (enum omp_clause_code): Same.
>   * tree-nested.cc (convert_nonlocal_omp_clauses): Same.
>   (convert_local_omp_clauses): Same.
>   * tree-pretty-print.cc (dump_omp_clause): Same.
>   * tree.cc: Same.
> 
> libgomp/ChangeLog:
> 
>   * libgomp.texi: Updated entry for HAS_DEVICE_ADDR.
>   * target.c (copy_firstprivate_data): Copy only if host address is not
>   NULL.
>   * testsuite/libgomp.c++/target-has-device-addr-2.C: New test.
>   * testsuite/libgomp.c++/target-has-device-addr-4.C: New test.
>   * testsuite/libgomp.c++/target-has-device-addr-5.C: New test.
>   * testsuite/libgomp.c++/target-has-device-addr-6.C: New test.
>   * testsuite/libgomp.c-c++-common/target-has-device-addr-1.c: New test.
>   * testsuite/libgomp.c/target-has-device-addr-3.c: New test.
>   * testsuite/libgomp.fortran/target-has-device-addr-1.f90: New test.
>   * testsuite/libgomp.fortran/target-has-device-addr-2.f90: New test.
>   * testsuite/libgomp.fortran/target-has-device-addr-3.f90: New test.
>   * testsuite/libgomp.fortran/target-has-device-addr-4.f90: New test.
> 
> gcc/testsuite/ChangeLog:
> 
>   * c-c++-common/gomp/clauses-1.c: Added has_device_addr to test cases.
>   * g++.dg/gomp/attrs-1.C: Added has_device_addr to test cases.
>   * g++.dg/gomp/attrs-2.C: Added has_device_addr to test cases.
>   * c-c++-common/gomp/target-has-device-addr-1.c: New test.
>   * c-c++-common/gomp/target-has-device-addr-2.c: New test.
>   * c-c++-common/gomp/target-is-device-ptr-1.c: New test.
>   * c-c++-common/gomp/target-is-device-ptr-2.c: New test.
>   * gfortran.dg/gomp/is_device_ptr-3.f90: New test.
>   * gfortran.dg/gomp/target-has-device-addr-1.f90: New test.
>   * gfortran.dg/gomp/target-has-device-addr-2.f90: New test.

Ok, thanks.

Jakub

[committed] openmp, fortran: Improve !$omp atomic checks [PR104328]

[Jakub Jelinek via Fortran]

Hi!

The testcase shows some cases that weren't verified and we ICE on
invalid because of that.
One problem is that unlike before, we weren't checking if some expression
is EXPR_VARIABLE with non-NULL symtree in the case where there was
a conversion around it.
The other two issues is that we check that in an IF ->block is non-NULL
and then immediately dereference ->block->next->op, but on invalid
code with no statements in the then clause ->block->next might be NULL.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to
trunk.

2022-02-02  Jakub Jelinek  

PR fortran/104328
* openmp.cc (is_scalar_intrinsic_expr): If must_be_var && conv_ok
and expr is conversion, verify it is a conversion from EXPR_VARIABLE
with non-NULL symtree.  Check ->block->next before dereferencing it.

* gfortran.dg/gomp/atomic-27.f90: New test.

--- gcc/fortran/openmp.cc.jj2022-01-21 11:01:12.458449420 +0100
+++ gcc/fortran/openmp.cc   2022-02-02 20:20:22.02000 +0100
@@ -7660,9 +7660,16 @@ static bool
 is_scalar_intrinsic_expr (gfc_expr *expr, bool must_be_var, bool conv_ok)
 {
   if (must_be_var
-  && (expr->expr_type != EXPR_VARIABLE || !expr->symtree)
-  && (!conv_ok || !is_conversion (expr, true, true)))
-return false;
+  && (expr->expr_type != EXPR_VARIABLE || !expr->symtree))
+{
+  if (!conv_ok)
+   return false;
+  gfc_expr *conv = is_conversion (expr, true, true);
+  if (!conv)
+   return false;
+  if (conv->expr_type != EXPR_VARIABLE || !conv->symtree)
+   return false;
+}
   return (expr->rank == 0
  && !gfc_is_coindexed (expr)
  && (expr->ts.type == BT_INTEGER
@@ -7705,6 +7712,7 @@ resolve_omp_atomic (gfc_code *code)
   if (next->op == EXEC_IF
  && next->block
  && next->block->op == EXEC_IF
+ && next->block->next
  && next->block->next->op == EXEC_ASSIGN)
{
  comp_cond = next->block->expr1;
@@ -7757,6 +7765,7 @@ resolve_omp_atomic (gfc_code *code)
   if (code->op == EXEC_IF
  && code->block
  && code->block->op == EXEC_IF
+ && code->block->next
  && code->block->next->op == EXEC_ASSIGN)
{
  comp_cond = code->block->expr1;
--- gcc/testsuite/gfortran.dg/gomp/atomic-27.f90.jj 2022-02-02 
20:30:44.101437198 +0100
+++ gcc/testsuite/gfortran.dg/gomp/atomic-27.f902022-02-02 
20:29:50.651217307 +0100
@@ -0,0 +1,34 @@
+! PR fortran/104328
+! { dg-do compile }
+
+subroutine foo
+  integer :: k = 1
+  !$omp atomic compare
+  if ( k == 2 ) then   ! { dg-error "unexpected !.OMP ATOMIC expression" }
+  end if
+end
+subroutine bar
+  real :: x = 1
+  !$omp atomic compare
+  if ( x == 2 ) then   ! { dg-error "unexpected !.OMP ATOMIC expression" }
+  end if
+end
+subroutine baz
+  integer :: i
+  !$omp atomic capture
+  i = 1
+  i = i + 1.   ! { dg-error "!.OMP ATOMIC capture-statement requires a 
scalar variable of intrinsic type" }
+end
+subroutine qux
+  integer :: i = 0
+  !$omp atomic capture
+  i = i + 1.0
+  i = i + 1.0  ! { dg-error "!.OMP ATOMIC capture-statement requires a 
scalar variable of intrinsic type" }
+end
+subroutine garply
+  logical :: k = .true.
+  !$omp atomic capture compare
+  if ( k ) then! { dg-error "unexpected !.OMP ATOMIC 
expression" }
+  else
+  end if
+end


Jakub

Re: [Patch] Fortran/OpenMP: Avoid ICE for invalid char array in omp atomic [PR104329]

[Jakub Jelinek via Fortran]

On Fri, Feb 04, 2022 at 12:39:53PM +0100, Tobias Burnus wrote:
> Already during parsing, the allocatable character array assignment
>x = (x)
> 
> is converted to two gfc_codes with EXEC_ASSIGN, namely:
> 
>   ASSIGN z1:_F.DA0(FULL) (parens z1:x(FULL))
>   ASSIGN z1:x(FULL) z1:_F.DA0(FULL)
> 
> But the current code expects only one gfc_code - as parse.c does some
> checks, that's unexpected for resolution and currently is checked with
> an gcc_assert.
> 
> Solution: I now defer the gfc_assert until after diagnosing the arguments.
> 
> OK for mainline (only affected version)?
> 
> Tobias
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> Fortran/OpenMP: Avoid ICE for invalid char array in omp atomic [PR104329]
> 
>   PR fortran/104329
> gcc/fortran/ChangeLog:
> 
>   * openmp.cc (resolve_omp_atomic): Defer extra-code assert after
>   other diagnostics.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/atomic-28.f90: New test.
> 
>  gcc/fortran/openmp.cc| 11 ---
>  gcc/testsuite/gfortran.dg/gomp/atomic-28.f90 | 28 
> 
>  2 files changed, 36 insertions(+), 3 deletions(-)

Ok, thanks.

Jakub

Re: [Patch] Fortran/OpenMP: Fix depend-clause handling

[Jakub Jelinek via Fortran]

On Tue, Feb 15, 2022 at 11:26:12AM +0100, Tobias Burnus wrote:
> As found by Marcel, the 'depend' clause was differently handled in
> 'omp depobj(...) depend(...)' and in 'omp task depend(...)'.
> 
> The problem was that for a scalar pointer, depobj depended
> on the pointer address - while task depended on the pointer-target address.
> 
> If one now combines depobj and direct var dependency, the dependency
> is on different addresses, such that the dependency is not honored.
> Marcel's example is testsuite/libgomp.fortran/depend-4.f90.
> (Thanks for the report!)
> 
> 
> I think in the real world, the problem is not that big as most
> code either uses depobj or a variable and does not mix them. Likewise,
> using the address of a temporary variable (cf. below) will also usually
> work in terms of dependency.
> 
> 
> The attached patch does:
> - depend clause (as used by task etc):
>   For scalar allocatable/pointer, add another dereference
> 
> - For depobj (which handles the depend clause by itself)
>   - Fix array(element) handling by coping the depend-clause.
> Before the result was 'D.123 = var; depobj = &D.123;'
> which is really not intended.
>   - Several issues related to optional and allocatable/pointer.
> 
> OK for mainline? Does backporting to GCC 11 make sense?

Ok.  Dunno about backporting, perhaps later.

Jakub

Re: [Patch] Fortran/OpenMP: Fix depend-clause handling for c_ptr (was: [Patch] Fortran/OpenMP: Fix depend-clause handling)

[Jakub Jelinek via Fortran]

On Tue, Feb 15, 2022 at 06:01:09PM +0100, Tobias Burnus wrote:
> OK?

Ok.

> Fortran/OpenMP: Fix depend-clause handling for c_ptr
> 
> gcc/fortran/ChangeLog:
> 
>   * trans-openmp.cc (gfc_trans_omp_depobj): Fix to alloc/ptr dummy
>   and for c_ptr.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/depend-4.f90: Add VALUE test, update scan test.
>   * gfortran.dg/gomp/depend-5.f90: Fix scan tree for -m32.
>   * gfortran.dg/gomp/depend-6.f90: New test.
> 
>  gcc/fortran/trans-openmp.cc |   7 +-
>  gcc/testsuite/gfortran.dg/gomp/depend-4.f90 |  29 +++-
>  gcc/testsuite/gfortran.dg/gomp/depend-5.f90 |  12 +-
>  gcc/testsuite/gfortran.dg/gomp/depend-6.f90 | 259 
> 
>  4 files changed, 295 insertions(+), 12 deletions(-)

Jakub

Re: [PATCH] [gfortran] Set omp_requires_mask for dynamic_allocators.

[Jakub Jelinek via Fortran]

On Mon, Feb 21, 2022 at 02:24:40PM +, Hafiz Abid Qadeer wrote:
> This patch fixes an issue that although gfortran accepts
> 'requires dynamic_allocators', it does not set the omp_requires_mask
> accordingly.
> 
> gcc/fortran/ChangeLog:
> 
>   * parse.cc (gfc_parse_file): Set OMP_REQUIRES_DYNAMIC_ALLOCATORS
>   bit in omp_requires_mask.
> ---
>  gcc/fortran/parse.cc | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc
> index db918291b10..821555bd85f 100644
> --- a/gcc/fortran/parse.cc
> +++ b/gcc/fortran/parse.cc
> @@ -6890,6 +6890,9 @@ done:
>break;
>  }
>  
> +  if (omp_requires & OMP_REQ_DYNAMIC_ALLOCATORS)
> +omp_requires_mask
> + = (enum omp_requires) (omp_requires_mask | 
> OMP_REQUIRES_DYNAMIC_ALLOCATORS);
>/* Do the parse tree dump.  */
>gfc_current_ns = flag_dump_fortran_original ? gfc_global_ns_list : NULL;

I see we do that for !$omp requires atomic_default_mem_order(...)
but it doesn't look correct to me.

The thing is, omp_requires_mask was added for C/C++ from the C/C++ notion of
translation units (and a question is how does that cope with C++20 modules),
with the assumption that once certain #pragma omp requires is seen, it
applies for the rest of the translation unit and there are some restrictions
that require it to appear before certain constructs in the source.
But, Fortran I think doesn't really have a concept of the translation unit,
the OpenMP term compilation unit is in Fortran program unit, so each
function/subroutine should have its own.  So, instead of what gfc_parse_file
does currently where it computes omp_requires as or of requires from each
function/subroutine (I think especially for atomic_default_mem_order that
can do really weird things, nothing requires that e.g. in different
functions those can't be different in Fortran, while in C/C++ it needs to be
the same), we need to make sure that omp_requires_mask omp-generic.cc sees
or uses is for Fortran the value from the current function/subroutine.

For the yet unimplemented requires unified_address etc., the plan was that
we'd emit the requirement e.g. into the offloading data such that we could
tell the runtime library all the requirements together from whole program or
shared library.  In that case using an or from the various
functions/subroutines is desirable, if at least one function requires
unified_address, the runtime should filter out any devices that don't
satisfy it, etc.

Jakub

Re: [PATCH] [gfortran] Set omp_requires_mask for dynamic_allocators.

[Jakub Jelinek via Fortran]

On Mon, Feb 21, 2022 at 06:02:06PM +0100, Tobias Burnus wrote:
> I wonder whether there is a real problem in terms of the ME, but maybe
> there is.
> 
> For atomic_default_mem_order: That's purely handle by the FEs by
> setting the default – and just using it when parsing the 'atomic'
> directive, if there is no explicit mem_order.

Well, for !$omp requires atomic_default_mem_order(whatever)
vs. !$omp atomic that is handled purely in the FE and I hope we do it right.
Where ME is involved is
!$omp requires atomic_default_mem_order(whatever) vs.
!$omp declare variant ...atomic_default_mem_order(whatever).
That is handled in omp-generic.cc and right now that is done during
gimplification of a function.
My reading of what gfc_parse_file does is that if I have:
subroutine foo
  !$omp requires atomic_default_mem_order(relaxed)
end subroutine foo
subroutine bar
  !$omp requires atomic_default_mem_order(acq_rel)
end subroutine bar
subroutine baz
  interface
subroutine foo
end subroutine
  end interface
  interface
subroutine bar
end subroutine
!$omp declare variant (foo) &
!$omp & match (implementation={atomic_default_mem_order(seq_cst)})
  end interface
  call bar
end subroutine baz
then it will call foo because omp_requires in one function is
OMP_MEMORY_ORDER_RELAXED aka 1 and in another one
OMP_MEMORY_ORDER_ACQ_REL aka 4, and (1 | 4) == OMP_MEMORY_ORDER_SEQ_CST
whenb no !$omp requires is in the baz program unit visible and so
it should just call bar.

And similarly with dynamic_allocators, if I have:
subroutine qux
  !$omp requires dynamic_allocators
end subroutine qux
subroutine corge
  interface
subroutine garply
end subroutine
!$omp declare variant (quux) &
!$omp & match (implementation={dynamic_allocators})
  end interface
  call garply
end subroutine corge
I think with the posted patch it would call quux which it should not.

Jakub

Re: [PATCH] openmp, fortran: Check that event handles passed to detach clauses are not arrays [PR104131]

[Jakub Jelinek via Fortran]

On Mon, Feb 28, 2022 at 02:01:03PM +, Kwok Cheung Yeung wrote:
> gcc/fortran/
> 
>   PR fortran/104131
>   * openmp.cc (gfc_match_omp_detach): Check that the event handle is not
>   an array type.
> 
> gcc/testsuite/
> 
>   PR fortran/104131
>   * gfortran.dg/gomp/pr104131.f90: New.
>   * gfortran.dg/gomp/pr104131-2.f90: New.
>   * gfortran.dg/gomp/task-detach-1.f90: Update expected error message.
> ---
>  gcc/fortran/openmp.cc|  5 +++--
>  gcc/testsuite/gfortran.dg/gomp/pr104131-2.f90| 10 ++
>  gcc/testsuite/gfortran.dg/gomp/pr104131.f90  | 10 ++
>  gcc/testsuite/gfortran.dg/gomp/task-detach-1.f90 |  2 +-
>  4 files changed, 24 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gfortran.dg/gomp/pr104131-2.f90
>  create mode 100644 gcc/testsuite/gfortran.dg/gomp/pr104131.f90
> 
> diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
> index 19142c4d8d0..50a1c476009 100644
> --- a/gcc/fortran/openmp.cc
> +++ b/gcc/fortran/openmp.cc
> @@ -531,9 +531,10 @@ gfc_match_omp_detach (gfc_expr **expr)
>if (gfc_match_variable (expr, 0) != MATCH_YES)
>  goto syntax_error;
>  
> -  if ((*expr)->ts.type != BT_INTEGER || (*expr)->ts.kind != 
> gfc_c_intptr_kind)
> +  if ((*expr)->ts.type != BT_INTEGER || (*expr)->ts.kind != gfc_c_intptr_kind
> +  || (*expr)->symtree->n.sym->as)

Don't we usually test instead || (*expr)->rank != 0 when testing for
scalars?

Jakub

Re: [PATCH] openmp, fortran: Check that event handles passed to detach clauses are not arrays [PR104131]

[Jakub Jelinek via Fortran]

On Mon, Feb 28, 2022 at 04:54:24PM +0100, Mikael Morin wrote:
> Le 28/02/2022 à 15:27, Kwok Cheung Yeung a écrit :
> > On 28/02/2022 2:07 pm, Jakub Jelinek wrote:
> (...)
> > > Don't we usually test instead || (*expr)->rank != 0 when testing for
> > > scalars?
> > > 
> (...)
> > 
> > So (*expr)->rank is 0 here even with an array. I'm not sure why - is
> > rank updated later, or did we forget to call something on the event
> > handle expression?
> > 
> > Testing against n->sym->as for an array check has been used elsewhere in
> > openmp.cc, to prevent reductions against arrays in OpenACC in
> > resolve_omp_clauses.
> > 
> I can’t tell what openmp requires; it depends on your needs.
> 
> Checking sym->as captures array variables which may include scalar
> expressions (arr(10) is a scalar expression even if arr is an array
> variable), while checking expr->rank only capture array expression,
> including scalar variable with array subcomponent (scal%array_comp(:) is an
> array expression, even if scal is a scalar variable).
> 
> gfc_resolve_expr, through gfc_expression_rank takes care of properly setting
> expr->rank.
> If the check is done at resolution stage (somewhere in resolve_omp_clauses I
> guess?), the rank should be set.
> 
> I hope it helps.

It is true that the spots I saw in fortran/openmp.cc that test rank look
like:
if (!gfc_resolve_expr (el->expr)
|| el->expr->ts.type != BT_INTEGER || el->expr->rank != 0)
etc., so probably !gfc_resolve_expr call is missing.

Jakub

Re: [PATCH] openmp, fortran: Check that event handles passed to detach clauses are not arrays [PR104131]

[Jakub Jelinek via Fortran]

On Mon, Feb 28, 2022 at 06:33:15PM +0100, Mikael Morin wrote:
> > It is true that the spots I saw in fortran/openmp.cc that test rank look
> > like:
> >  if (!gfc_resolve_expr (el->expr)
> >  || el->expr->ts.type != BT_INTEGER || el->expr->rank != 0)
> > etc., so probably !gfc_resolve_expr call is missing.
> > 
> As long as the expression is expected to not be a (contained) function call,
> I think it should work.
> 
> In the general case non-syntaxic errors are preferably checked and reported
> later at resolution stage, where contained functions are known.

Oh, I've missed that it is done during parsing and not during resolution.
That !gfc_resolve_expr call and the checking if it is BT_INTEGER etc.
should be certainly moved to resolve_omp_clauses.

Jakub

Re: [PATCH] openmp, fortran: Check that event handles passed to detach clauses are not arrays [PR104131]

[Jakub Jelinek via Fortran]

On Mon, Feb 28, 2022 at 09:45:10PM +0100, Mikael Morin wrote:
> Le 28/02/2022 à 21:37, Mikael Morin a écrit :
> > Maybe corank should be checked together with rank?
> 
> Lesson learned today: expressions don’t have a corank.
> Does pr104131-2.f90 really need to be rejected?

OpenMP 5.2 says that detach clause should be treated as if it appears on a
firstprivate clause and for the privatization clauses says:
"A private variable must not be coindexed or appear as an actual argument to a 
procedure where
the corresponding dummy argument is a coarray."
5.1 had the same restriction.

Jakub

Re: [PATCH] openmp, fortran: Check that event handles passed to detach clauses are not arrays [PR104131]

[Jakub Jelinek via Fortran]

On Tue, Mar 01, 2022 at 08:58:54AM +0100, Tobias Burnus wrote:
> The wording actually also permits array sections. But contrary to coarrays,
> (which are odd but otherwise fine), I think it does not really make sense
> to have arrays and array sections here.
> 
> (Do we need/want to get this clarified/changed in spec?)

In 5.2 we have:
"A variable that is part of another variable (as an array element or a 
structure element) cannot
appear in a detach clause."
and
"An array section can appear only in clauses for which it is explicitly
allowed."
and
C/C++
"A variable of  OpenMP type is a variable of type 
omp__t."
and
Fortran
"A variable of  OpenMP type is a scalar integer variable of kind
omp__kind."
and
"event-handle   variable of event_handle type   default"

As there is no explicit allowing of array sections, array sections aren't
allowed in detach, and it must be scalar integer.
The question is if a non-coindexed coarray is a scalar integer variable or
not.

Jakub

Re: OpenACC 'kernels' decomposition: Mark variables used in synthesized data clauses as addressable [PR100280]

[Jakub Jelinek via Fortran]

On Tue, Mar 01, 2022 at 05:46:20PM +0100, Thomas Schwinge wrote:
> OK to proceed in this way?

With a suitable ChangeLog entry and one nit fixed yes.

> --- gcc/omp-low.cc
> +++ gcc/omp-low.cc
> @@ -188,7 +188,7 @@ struct omp_context
>  static splay_tree all_contexts;
>  static int taskreg_nesting_level;
>  static int target_nesting_level;
> -static bitmap task_shared_vars;
> +static bitmap make_addressable_vars;
>  static bitmap global_nonaddressable_vars;
>  static vec taskreg_contexts;
>  static vec task_cpyfns;
> @@ -572,9 +572,9 @@ use_pointer_for_field (tree decl, omp_context *shared_ctx)
>   /* Taking address of OUTER in lower_send_shared_vars
>  might need regimplification of everything that uses the
>  variable.  */
> - if (!task_shared_vars)
> -   task_shared_vars = BITMAP_ALLOC (NULL);
> - bitmap_set_bit (task_shared_vars, DECL_UID (outer));
> + if (!make_addressable_vars)
> +   make_addressable_vars = BITMAP_ALLOC (NULL);
> + bitmap_set_bit (make_addressable_vars, DECL_UID (outer));

Has the MUA replaced tabs with spaces?

> --- gcc/omp-oacc-kernels-decompose.cc
> +++ gcc/omp-oacc-kernels-decompose.cc
> @@ -845,7 +845,11 @@ maybe_build_inner_data_region (location_t loc, gimple 
> *body,
>   prev_mapped_var = v;
> 
>   /* See .  */
> - TREE_ADDRESSABLE (v) = 1;
> + if (!TREE_ADDRESSABLE (v))
> +   {
> + /* Request that OMP lowering make 'v' addressable.  */
> + OMP_CLAUSE_MAP_DECL_MAKE_ADDRESSABLE (new_clause) = 1;
> +   }

That is a single statement body, so shouldn't have {}s around it.

Jakub

Re: [PATCH][v2] openmp, fortran: Check that the type of an event handle in a detach clause is suitable [PR104131]

[Jakub Jelinek via Fortran]

On Wed, Mar 02, 2022 at 05:22:21PM +, Kwok Cheung Yeung wrote:
> I have updated the patch to catch array elements and structure components as
> additional checks, in addition to checking that the variable is a scalar.
> 
> The check has been moved to the end of resolve_omp_clauses as it is more
> appropriate there. This gets rid of the additional 'Unexpected !$OMP END
> TASK statement' error, since the type error is now caught after the matching
> phase.
> 
> Coarrays (with the testcases in pr104131-2.f90) can be dealt with in a
> separate patch. Is this part okay for trunk?

LGTM, thanks.
Yes, coarrays is something that we need to deal with in all the clauses,
not just this one, so before we claim OpenMP 5.0 support we need to go
through it fully.

Jakub

Re: [PATCH] OpenMP, libgomp: Add new runtime routine omp_get_mapped_ptr.

[Jakub Jelinek via Fortran]

On Fri, Mar 04, 2022 at 03:47:31PM +0100, Marcel Vollweiler wrote:
> libgomp/ChangeLog:
> 
>   * libgomp.map: Added omp_get_mapped_ptr.
>   * libgomp.texi: Tagged omp_get_mapped_ptr as supported.
>   * omp.h.in: Added omp_get_mapped_ptr.
>   * omp_lib.f90.in: Added interface for omp_get_mapped_ptr.
>   * omp_lib.h.in: Likewise.
>   * target.c (omp_get_mapped_ptr): Added implementation of
>   omp_get_mapped_ptr.
>   * testsuite/libgomp.c-c++-common/get-mapped-ptr-1.c: New test.
>   * testsuite/libgomp.c-c++-common/get-mapped-ptr-2.c: New test.
>   * testsuite/libgomp.c-c++-common/get-mapped-ptr-3.c: New test.
>   * testsuite/libgomp.c-c++-common/get-mapped-ptr-4.c: New test.
>   * testsuite/libgomp.fortran/get-mapped-ptr-1.f90: New test.
>   * testsuite/libgomp.fortran/get-mapped-ptr-2.f90: New test.
>   * testsuite/libgomp.fortran/get-mapped-ptr-3.f90: New test.
>   * testsuite/libgomp.fortran/get-mapped-ptr-4.f90: New test.
> 
> diff --git a/libgomp/libgomp.map b/libgomp/libgomp.map
> index 2ac5809..00a4858 100644
> --- a/libgomp/libgomp.map
> +++ b/libgomp/libgomp.map
> @@ -224,6 +224,7 @@ OMP_5.1 {
>   omp_set_teams_thread_limit_8_;
>   omp_get_teams_thread_limit;
>   omp_get_teams_thread_limit_;
> + omp_get_mapped_ptr;
>  } OMP_5.0.2;

I think it is too late for this to be targetted for GCC 12, and
for GCC 13 it will need to go into OMP_5.1.1 symver.

>  GOMP_1.0 {
> diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
> index 161a423..c163b56 100644
> --- a/libgomp/libgomp.texi
> +++ b/libgomp/libgomp.texi
> @@ -314,7 +314,7 @@ The OpenMP 4.5 specification is fully supported.
>  @item @code{omp_target_is_accessible} runtime routine @tab N @tab
>  @item @code{omp_target_memcpy_async} and @code{omp_target_memcpy_rect_async}
>runtime routines @tab N @tab
> -@item @code{omp_get_mapped_ptr} runtime routine @tab N @tab
> +@item @code{omp_get_mapped_ptr} runtime routine @tab Y @tab
>  @item @code{omp_calloc}, @code{omp_realloc}, @code{omp_aligned_alloc} and
>@code{omp_aligned_calloc} runtime routines @tab Y @tab
>  @item @code{omp_alloctrait_key_t} enum: @code{omp_atv_serialized} added,
> diff --git a/libgomp/omp.h.in b/libgomp/omp.h.in
> index 89c5d65..18d0152 100644
> --- a/libgomp/omp.h.in
> +++ b/libgomp/omp.h.in
> @@ -282,6 +282,7 @@ extern int omp_target_memcpy_rect (void *, const void *, 
> __SIZE_TYPE__, int,
>  extern int omp_target_associate_ptr (const void *, const void *, 
> __SIZE_TYPE__,
>__SIZE_TYPE__, int) __GOMP_NOTHROW;
>  extern int omp_target_disassociate_ptr (const void *, int) __GOMP_NOTHROW;
> +extern void *omp_get_mapped_ptr (const void *, int) __GOMP_NOTHROW;
>  
>  extern void omp_set_affinity_format (const char *) __GOMP_NOTHROW;
>  extern __SIZE_TYPE__ omp_get_affinity_format (char *, __SIZE_TYPE__)
> diff --git a/libgomp/omp_lib.f90.in b/libgomp/omp_lib.f90.in
> index daf40dc..506f15c 100644
> --- a/libgomp/omp_lib.f90.in
> +++ b/libgomp/omp_lib.f90.in
> @@ -835,6 +835,15 @@
>end function omp_target_disassociate_ptr
>  end interface
>  
> +interface
> +  function omp_get_mapped_ptr (ptr, device_num) bind(c)
> +use, intrinsic :: iso_c_binding, only : c_ptr, c_int
> +type(c_ptr) :: omp_get_mapped_ptr
> +type(c_ptr), value :: ptr
> +integer(c_int), value :: device_num
> +  end function omp_get_mapped_ptr
> +end interface
> +
>  #if _OPENMP >= 201811
>  !GCC$ ATTRIBUTES DEPRECATED :: omp_get_nested, omp_set_nested
>  #endif
> diff --git a/libgomp/omp_lib.h.in b/libgomp/omp_lib.h.in
> index ff857a4..0f48510 100644
> --- a/libgomp/omp_lib.h.in
> +++ b/libgomp/omp_lib.h.in
> @@ -416,3 +416,12 @@
>integer(c_int), value :: device_num
>  end function omp_target_disassociate_ptr
>end interface
> +
> +  interface
> +function omp_get_mapped_ptr (ptr, device_num) bind(c)
> +  use, intrinsic :: iso_c_binding, only : c_ptr, c_int
> +  type(c_ptr) :: omp_get_mapped_ptr
> +  type(c_ptr), value :: ptr
> +  integer(c_int), value :: device_num
> +end function omp_get_mapped_ptr
> +  end interface
> diff --git a/libgomp/target.c b/libgomp/target.c
> index 9017458..735d70b 100644
> --- a/libgomp/target.c
> +++ b/libgomp/target.c
> @@ -3665,6 +3665,49 @@ omp_target_disassociate_ptr (const void *ptr, int 
> device_num)
>return ret;
>  }
>  
> +void *
> +omp_get_mapped_ptr (const void *ptr, int device_num)
> +{
> +  if (device_num < 0 || device_num > omp_get_num_devices ())
> +return NULL;
> +
> +  if (device_num == omp_get_initial_device ())
> +return (void*)ptr;

Space before * and space after )

> +  struct gomp_device_descr *devicep = resolve_device (device_num);
> +  if (devicep == NULL)
> +return NULL;
> +
> +  if (!(devicep->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)
> + 

Re: [PATCH] OpenMP, libgomp: Add new runtime routine omp_get_mapped_ptr.

[Jakub Jelinek via Fortran]

On Thu, Mar 10, 2022 at 05:01:35PM +0100, Marcel Vollweiler wrote:
> --- a/gcc/omp-low.cc
> +++ b/gcc/omp-low.cc
> @@ -3962,6 +3962,7 @@ omp_runtime_api_call (const_tree fndecl)
>"target_is_present",
>"target_memcpy",
>"target_memcpy_rect",
> +  "get_mapped_ptr",
>NULL,
>/* Now omp_* calls that are available as omp_* and omp_*_; however, the
>DECL_NAME is always omp_* without tailing underscore.  */

The entries in each NULL separated subsection are supposed to be sorted
alphabetically.
Other than that LGTM, but stage1 is still far...

Jakub

Re: [PATCH] OpenMP, Fortran: Bugfix for omp_set_num_teams.

[Jakub Jelinek via Fortran]

On Tue, Mar 15, 2022 at 06:05:48PM +0100, Marcel Vollweiler wrote:
> Hi,
> 
> This patch fixes a small bug for omp_set_num_teams in fortran.c.
> 
> Marcel
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> OpenMP, Fortran: Bugfix for omp_set_num_teams.
> 
> This patch fixes a small bug in the omp_set_num_teams implementation.
> 
> libgomp/ChangeLog:
> 
>   * fortran.c (omp_set_num_teams_8_): Fix bug.

Thanks for spotting this, but would be nice to cover it with
a testcase.

! { dg-do run }
! { dg-additional-options "-fdefault-integer-8" }

program set_num_teams_8
  use omp_lib
  omp_set_num_teams (42)
  if (omp_get_num_teams () .ne. 42) stop 1
end program

or so would IMHO do it, please test that it FAILs without your fortran.c
fix and succeeds with it.

Ok for trunk with that change.

> diff --git a/libgomp/fortran.c b/libgomp/fortran.c
> index 8c1cfd1..d984ce5 100644
> --- a/libgomp/fortran.c
> +++ b/libgomp/fortran.c
> @@ -491,7 +491,7 @@ omp_set_num_teams_ (const int32_t *num_teams)
>  void
>  omp_set_num_teams_8_ (const int64_t *num_teams)
>  {
> -  omp_set_max_active_levels (TO_INT (*num_teams));
> +  omp_set_num_teams (TO_INT (*num_teams));
>  }
>  
>  int32_t


Jakub

Re: [PATCH] OpenMP, Fortran: Bugfix for omp_set_num_teams.

[Jakub Jelinek via Fortran]

On Wed, Mar 16, 2022 at 02:06:16PM +0100, Marcel Vollweiler wrote:
> libgomp/ChangeLog:
> 
>   * fortran.c (omp_set_num_teams_8_): Fix bug.
>   * testsuite/libgomp.fortran/icv-8.f90: New test.

Ok, with a minor nit.  Please use
Call omp_set_num_teams instead of omp_set_max_active_levels.
instead of
Fix bug.
in the ChangeLog.

> diff --git a/libgomp/fortran.c b/libgomp/fortran.c
> index 8c1cfd1..d984ce5 100644
> --- a/libgomp/fortran.c
> +++ b/libgomp/fortran.c
> @@ -491,7 +491,7 @@ omp_set_num_teams_ (const int32_t *num_teams)
>  void
>  omp_set_num_teams_8_ (const int64_t *num_teams)
>  {
> -  omp_set_max_active_levels (TO_INT (*num_teams));
> +  omp_set_num_teams (TO_INT (*num_teams));
>  }
>  
>  int32_t
> diff --git a/libgomp/testsuite/libgomp.fortran/icv-8.f90 
> b/libgomp/testsuite/libgomp.fortran/icv-8.f90
> new file mode 100644
> index 000..9478c15
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/icv-8.f90
> @@ -0,0 +1,10 @@
> +! This tests 'set_num_teams_8' function.
> +
> +program set_num_teams_8
> +  use omp_lib
> +  use, intrinsic :: iso_fortran_env
> +  integer(int64) :: x
> +  x = 42
> +  call omp_set_num_teams (x)
> +  if (omp_get_max_teams () .ne. 42) stop 1
> +end program


Jakub

Re: [Patch] Fortran/OpenMP: Improve associate-name diagnostic [PR103039]

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 02:15:11PM +0100, Tobias Burnus wrote:
> This patch addresses a side issue found when looking at PR103039.
> 
> Namely instead of printing:
> 
>55 |   !$omp parallel firstprivate(tt)
>   |  1
> Error: ASSOCIATE name ‘__tmp_INTEGER_4’ in FIRSTPRIVATE clause at (1)
> 
> With the patch, the error is:
> 
> Error: Associate name ‘tt’ in FIRSTPRIVATE clause at (1)
> 
> That is: It prints the proper name and it uses 'associate name'
> matching the Fortran standard – and takes into account that an
> associate name not only used with ASSOCIATE but also with
> SELECT TYPE, SELECT RANK, and (untested) CHANGE TEAMS.
> 
> OK for mainline?

LGTM, thanks.

> Fortran/OpenMP: Improve associate-name diagnostic [PR103039]
> 
> gcc/fortran/ChangeLog:
> 
>   PR fortran/103039
>   * openmp.cc (resolve_omp_clauses): Improve associate-name diagnostic
>   for select type/rank.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR fortran/103039
>   * gfortran.dg/gomp/associate1.f90: Update dg-error.
>   * gfortran.dg/gomp/associate2.f90: New test.
> 
>  gcc/fortran/openmp.cc | 12 +++--
>  gcc/testsuite/gfortran.dg/gomp/associate1.f90 | 40 +++---
>  gcc/testsuite/gfortran.dg/gomp/associate2.f90 | 76 
> +++
>  3 files changed, 104 insertions(+), 24 deletions(-)

Jakub

Re: [Patch] Fortran/OpenMP: Improve associate-name diagnostic [PR103039]

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 05:27:02PM +0100, Tobias Burnus wrote:
> SELECT TYPE, SELECT RANK and ASSOCIATE have
>   associate-name => selector
> and create a pointer to the selector.
> 
> GCC was fixed to handle CLASS properly in
>   class(t) :: var
>   !$omp ... firstprivate(var)
> As a side effect, firstprivate(assoc_name) now also gets
> handled that way, effectively trying to firstprivate(selector)
> which should be shared...
> 
> While firstprivate(var) does not appear explicitly, it gets
> added via gfc_omp_predetermined_sharing.
> 
> I went for the simple solution and handle it only
> in gfortran's ctor/dtor.
> 
> An alternative would be to set OMP_CLAUSE_FIRSTPRIVATE_NO_REFERENCE,
> which is currently only set for C++'s __for_end / __for_range
> and then later process it in ctor/dtor. I am not sure whether that's
> really best and what's the best way to propagate it. One way would
> be to create and use OMP_CLAUSE_DEFAULT_FIRSTPRIVATE_NO_REFERENCE.
> 
> OK as is (simple version) – or is a fuller version better. If so,
> suggestion how to do this best?

LGTM.

> Fortran/OpenMP: Fix privatization of associated names
> 
> gfc_omp_predetermined_sharing cases the associate-name pointer variable
> to be OMP_CLAUSE_DEFAULT_FIRSTPRIVATE, which is fine. However, the associated
> selector is shared. Thus, the target of associate-name pointer should not get
> copied. (It was before but because of gfc_omp_privatize_by_reference returning
> false, the selector was not only wrongly copied but this was also not done
> properly.)
> 
> gcc/fortran/ChangeLog:
> 
>   PR fortran/103039
>   * trans-openmp.cc (gfc_omp_clause_copy_ctor, gfc_omp_clause_dtor):
>   Only privatize pointer for associate names.
> 
> libgomp/ChangeLog:
> 
>   PR fortran/103039
>   * testsuite/libgomp.fortran/associate4.f90: New test.
> 
>  gcc/fortran/trans-openmp.cc  | 10 +++
>  libgomp/testsuite/libgomp.fortran/associate4.f90 | 92 
> 
>  2 files changed, 102 insertions(+)
> 

Jakub

[PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]

[Jakub Jelinek via Fortran]

Hi!

On the gfortran.dg/pr103691.f90 testcase the Fortran ICE emits
  static real(kind=4) a[0] = {[0 ... -1]=2.0e+0};
That is an invalid RANGE_EXPR where the maximum is smaller than the minimum.

The following patch fixes that.  If TYPE_MAX_VALUE is smaller than
TYPE_MIN_VALUE, the array is empty and so doesn't need any initializer,
if the two are equal, we don't need to bother with a RANGE_EXPR and
can just use that INTEGER_CST as the index and finally for the 2+ values
in the range it uses a RANGE_EXPR as before.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2022-03-25  Jakub Jelinek  

PR fortran/103691
* trans-array.cc (gfc_conv_array_initializer): If TYPE_MAX_VALUE is
smaller than TYPE_MIN_VALUE (i.e. empty array), throw the initializer
on the floor, if TYPE_MIN_VALUE is equal to TYPE_MAX_VALUE, use just
the TYPE_MIN_VALUE as index instead of RANGE_EXPR.

--- gcc/fortran/trans-array.cc.jj   2022-02-04 14:36:55.113603791 +0100
+++ gcc/fortran/trans-array.cc  2022-03-24 16:14:58.334498775 +0100
@@ -6267,10 +6267,17 @@ gfc_conv_array_initializer (tree type, g
   else
gfc_conv_structure (&se, expr, 1);
 
-  CONSTRUCTOR_APPEND_ELT (v, build2 (RANGE_EXPR, gfc_array_index_type,
-TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
-TYPE_MAX_VALUE (TYPE_DOMAIN (type))),
- se.expr);
+  if (tree_int_cst_lt (TYPE_MAX_VALUE (TYPE_DOMAIN (type)),
+  TYPE_MIN_VALUE (TYPE_DOMAIN (type
+   break;
+  else if (tree_int_cst_equal (TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
+  TYPE_MAX_VALUE (TYPE_DOMAIN (type
+   range = TYPE_MIN_VALUE (TYPE_DOMAIN (type));
+  else
+   range = build2 (RANGE_EXPR, gfc_array_index_type,
+   TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
+   TYPE_MAX_VALUE (TYPE_DOMAIN (type)));
+  CONSTRUCTOR_APPEND_ELT (v, range, se.expr);
   break;
 
 case EXPR_ARRAY:

Jakub

Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]

[Jakub Jelinek via Fortran]

On Fri, Mar 25, 2022 at 12:16:40PM +0100, Richard Biener wrote:
> On Fri, Mar 25, 2022 at 11:13 AM Tobias Burnus  
> wrote:
> >
> > On 25.03.22 09:57, Jakub Jelinek via Fortran wrote:
> > > On the gfortran.dg/pr103691.f90 testcase the Fortran ICE emits
> > >static real(kind=4) a[0] = {[0 ... -1]=2.0e+0};
> > > That is an invalid RANGE_EXPR where the maximum is smaller than the 
> > > minimum.
> > >
> > > The following patch fixes that.  If TYPE_MAX_VALUE is smaller than
> > > TYPE_MIN_VALUE, the array is empty and so doesn't need any initializer,
> > > if the two are equal, we don't need to bother with a RANGE_EXPR and
> > > can just use that INTEGER_CST as the index and finally for the 2+ values
> > > in the range it uses a RANGE_EXPR as before.
> > >
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > LGTM – thanks for taking care of Fortran patches and regressions.
> >
> > > 2022-03-25  Jakub Jelinek  
> > >
> > >   PR fortran/103691
> > >   * trans-array.cc (gfc_conv_array_initializer): If TYPE_MAX_VALUE is
> > >   smaller than TYPE_MIN_VALUE (i.e. empty array), throw the 
> > > initializer
> > >   on the floor, if TYPE_MIN_VALUE is equal to TYPE_MAX_VALUE, use just
> > >   the TYPE_MIN_VALUE as index instead of RANGE_EXPR.
> >
> > I am not sure whether "throw the initializer on the floor" is the best 
> > wording
> > for a changelog. I think I prefer a wording like "ignore the initializer" or
> > another less idiomatic expression. And I think a ';' before the second 'if'
> > also increases readability.
> 
> Can there be side-effects in those initializer elements in Fortran?

For PARAMETERs certainly not, those need to be constant.
Even otherwise, this is in a routine that does
  /* Create a constructor from the list of elements.  */
  tmp = build_constructor (type, v);
  TREE_CONSTANT (tmp) = 1;
  return tmp;
at the end so I wouldn't expect side-effects anywhere.

Also, I think typically in the Fortran FE side-effects would go into
se.pre and se.post sequences, not into se.expr, and this routine
doesn't emit those se.pre/se.post sequences anywhere, so presumably it
assumes they don't exist.

What is the behavior with a RANGE_EXPR when one has { [0..10] = ++i; },
is that applying the side-effects 11 times or once ?


Jakub

Re: [PATCH] fortran: Fix up initializers of param(0) PARAMETERs [PR103691]

[Jakub Jelinek via Fortran]

On Fri, Mar 25, 2022 at 01:13:06PM +0100, Richard Biener wrote:
> > Also, I think typically in the Fortran FE side-effects would go into
> > se.pre and se.post sequences, not into se.expr, and this routine
> > doesn't emit those se.pre/se.post sequences anywhere, so presumably it
> > assumes they don't exist.
> >
> > What is the behavior with a RANGE_EXPR when one has { [0..10] = ++i; },
> > is that applying the side-effects 11 times or once ?
> 
> 11 times is what is documented.

Then [0..-1] = ++i should be 0 times the side-effect.

Jakub

Re: [PATCH] Fortran: Fix clause splitting for OMP masked taskloop directive

[Jakub Jelinek via Fortran]

On Fri, Mar 25, 2022 at 08:02:04PM -0600, Sandra Loosemore wrote:
> I ran into this bug in the handling of clauses on the combined "masked
> taskloop" OMP directive when I was working on something else.  The fix
> turned out to be a 1-liner.  OK for trunk?
> 
> -Sandra

> commit 17c4fa0bd97c070945004095a06fb7d9e91869e3
> Author: Sandra Loosemore 
> Date:   Wed Mar 23 18:45:25 2022 -0700
> 
> Fortran: Fix clause splitting for OMP masked taskloop directive
> 
> This patch fixes an obvious coding goof that caused all clauses for
> the combined OMP masked taskloop directive to be discarded.
> 
>   gcc/fortran/
>   * trans-openmp.cc (gfc_split_omp_clauses): Fix mask for
>   EXEC_OMP_MASKED_TASKLOOP.
> 
>   gcc/testsuite/
>   * gfortran.dg/gomp/masked-taskloop.f90: New.

Ok, thanks.

> +! { dg-final { scan-tree-dump "omp taskloop collapse\\(2\\) 
> grainsize\\(4\\)" "original" } }

Though perhaps the test should be more flexible and allow both orderings of
the clauses and extra clauses too?  So:
! { dg-final { scan-tree-dump "omp taskloop \[^\n\r]*grainsize\\(4\\)" 
"original" } }
! { dg-final { scan-tree-dump "omp taskloop \[^\n\r]*collapse\\(2\\)" 
"original" } }
?

Jakub

Re: [PATCH] Fortran: Add location info to OpenMP tree nodes

[Jakub Jelinek via Fortran]

On Fri, Mar 25, 2022 at 08:03:09PM -0600, Sandra Loosemore wrote:
> I've got another patch forthcoming (stage 1 material) that adds some new
> diagnostics for non-rectangular loops during gimplification of OMP nodes.
> When I was working on that, I discovered that the Fortran front end wasn't
> attaching location information to the tree nodes corresponding to the
> various OMP directives, so the new errors weren't coming out with location
> info either.  I went through trans-openmp.cc and fixed all the places where
> make_node was being called to explicitly set the location.
> 
> I don't have a test case specifically for this change, but my test cases for
> the new diagnostics in the non-rectangular loops patch do exercise it.  Is
> this OK for trunk now, or for stage 1 when we get there?

Ok for GCC 13.

> commit 4c745003d0b39d0e92032b62421df4920753783a
> Author: Sandra Loosemore 
> Date:   Thu Mar 24 21:02:34 2022 -0700
> 
> Fortran: Add location info to OpenMP tree nodes
> 
>gcc/fortran/
>* trans-openmp.cc (gfc_trans_omp_critical): Set location on OMP
>tree node.
>(gfc_trans_omp_do): Likewise.
>(gfc_trans_omp_masked): Likewise.
>(gfc_trans_omp_do_simd): Likewise.
>(gfc_trans_omp_scope): Likewise.
>(gfc_trans_omp_taskgroup): Likewise.
>(gfc_trans_omp_taskwait): Likewise.
>(gfc_trans_omp_distribute): Likewise.
>(gfc_trans_omp_taskloop): Likewise.
>(gfc_trans_omp_master_masked_taskloop): Likewise.

Jakub

Re: [Patch] OpenMP/Fortran: Fix EXIT in loop diagnostic [PR105242]

[Jakub Jelinek via Fortran]

On Wed, Apr 13, 2022 at 05:16:48PM +0200, Tobias Burnus wrote:
> Trivial fix – after finding the issue. The LOOP directive and
> several LOOP/DO/SIMD combined directives were missing in the
> check. As the PR shows, this leads to an ICE on invalid code.
> 
> OK?
> 
> Tobias
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> OpenMP/Fortran: Fix EXIT in loop diagnostic [PR105242]
> 
> gcc/fortran/ChangeLog:
> 
>   PR fortran/105242
>   * match.cc (match_exit_cycle): Handle missing OMP LOOP, DO and SIMD
>   directives in the EXIT/CYCLE diagnostic.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR fortran/105242
>   * gfortran.dg/gomp/loop-exit.f90: New test.

Ok, thanks.

Jakub

Re: [pushed] testsuite: add additional option to force DSE execution [PR103662]

[Jakub Jelinek via Fortran]

On Mon, Apr 25, 2022 at 01:38:25PM +0200, Mikael Morin wrote:
> I have just pushed the attached fix for two UNRESOLVED checks at -O0 that I
> hadn’t seen.

I don't like forcing of DSE in -O0 compilation, wouldn't it be better
to just not check the dse dump at -O0 like in the following patch?

Even better would be to check that the z._data = stores are both present
in *.optimized dump, but that doesn't really work at -O2 or above because
we inline the functions and optimize it completely away (both the stores
and corresponding reads).

The first hunk is needed so that __OPTIMIZE__ effective target works in
Fortran testsuite, otherwise one gets a pedantic error and __OPTIMIZE__
is considered not to match at all.

2022-04-25  Jakub Jelinek  

PR fortran/103662
* lib/target-supports.exp (check_effective_target___OPTIMIZE__): Add
a var definition to avoid pedwarn about empty translation unit.
* gfortran.dg/unlimited_polymorphic_3.f03: Remove -ftree-dse from
dg-additional-options, guard scan-tree-dump-not directives on
__OPTIMIZE__ target.

--- gcc/testsuite/lib/target-supports.exp.jj2022-04-22 13:36:56.150960826 
+0200
+++ gcc/testsuite/lib/target-supports.exp   2022-04-25 14:06:21.620537483 
+0200
@@ -11770,6 +11770,7 @@ proc check_effective_target___OPTIMIZE__
#ifndef __OPTIMIZE__
# error nein
#endif
+   int optimize;
 } [current_compiler_flags]]
 }
 
--- gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03.jj2022-04-25 
13:54:38.320309119 +0200
+++ gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03   2022-04-25 
14:04:01.346486431 +0200
@@ -1,5 +1,5 @@
 ! { dg-do run }
-! { dg-additional-options "-ftree-dse -fdump-tree-dse-details" }
+! { dg-additional-options "-fdump-tree-dse-details" }
 !
 ! Check that pointer assignments allowed by F2003:C717
 ! work and check null initialization of CLASS(*) pointers.
@@ -71,5 +71,5 @@ end subroutine foo_sq
 ! We used to produce multiple independant types for the unlimited polymorphic
 ! descriptors (types for class(*)) which caused stores to them to be seen as
 ! useless.
-! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &w" "dse1" } }
-! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &x" "dse1" } }
+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &w" "dse1" { 
target __OPTIMIZE__ } } }
+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = &x" "dse1" { 
target __OPTIMIZE__ } } }


Jakub

Re: [PATCH] fortran: Avoid infinite self-recursion [PR105381]

[Jakub Jelinek via Fortran]

On Tue, Apr 26, 2022 at 03:22:08PM +0200, Tobias Burnus wrote:
> LGTM - however:
> 
> On 26.04.22 14:38, Mikael Morin wrote:
> > --- a/gcc/fortran/trans-array.cc
> > +++ b/gcc/fortran/trans-array.cc
> > @@ -3698,7 +3698,8 @@ non_negative_strides_array_p (tree expr)
> > if (DECL_P (expr)
> > && DECL_LANG_SPECIFIC (expr))
> >   if (tree orig_decl = GFC_DECL_SAVED_DESCRIPTOR (expr))
> > -  return non_negative_strides_array_p (orig_decl);
> > +  if (orig_decl != expr)
> > + return non_negative_strides_array_p (orig_decl);
> 
> Is the if()if()if() cascade really needed? I can see a reason that an
> extra 'if' is preferred for the variable declaration of orig_decl, but
> can't we at least put the new 'orig_decl != expr' with an '&&' into the
> same if as the decl/in the second if? Besides clearer, it also avoids
> further identing the return line.

I think we can't in C++11/C++14.  The options can be if orig_decl would be 
declared
earlier, then it can be
tree orig_decl;
if (DECL_P (expr)
&& DECL_LANG_SPECIFIC (expr)
&& (orig_decl = GFC_DECL_SAVED_DESCRIPTOR (expr))
&& orig_decl != expr)
  return non_negative_strides_array_p (orig_decl);
but I think this is generally frowned upon,
or one can repeat it like:
if (DECL_P (expr)
&& DECL_LANG_SPECIFIC (expr)
&& GFC_DECL_SAVED_DESCRIPTOR (expr)
&& GFC_DECL_SAVED_DESCRIPTOR (expr) != expr)
  return non_negative_strides_array_p (GFC_DECL_SAVED_DESCRIPTOR (expr));
or what Mikael wrote, perhaps with the && on one line:
if (DECL_P (expr) && DECL_LANG_SPECIFIC (expr))
  if (tree orig_decl = GFC_DECL_SAVED_DESCRIPTOR (expr))
if (orig_decl != expr)
  return non_negative_strides_array_p (orig_decl);
In C++17 and later one can write:
if (DECL_P (expr) && DECL_LANG_SPECIFIC (expr))
  if (tree orig_decl = GFC_DECL_SAVED_DESCRIPTOR (expr);
  orig_decl && orig_decl != expr)
return non_negative_strides_array_p (orig_decl);

Jakub

Re: [PATCH] fortran: Avoid infinite self-recursion [PR105381]

[Jakub Jelinek via Fortran]

On Tue, Apr 26, 2022 at 07:12:13PM +0200, Mikael Morin wrote:
> > I think we can't in C++11/C++14.  The options can be if orig_decl would be 
> > declared
> > earlier, then it can be
> >  tree orig_decl;
> >  if (DECL_P (expr)
> > && DECL_LANG_SPECIFIC (expr)
> > && (orig_decl = GFC_DECL_SAVED_DESCRIPTOR (expr))
> > && orig_decl != expr)
> >return non_negative_strides_array_p (orig_decl);
> > but I think this is generally frowned upon,
> > or one can repeat it like:
> >  if (DECL_P (expr)
> > && DECL_LANG_SPECIFIC (expr)
> > && GFC_DECL_SAVED_DESCRIPTOR (expr)
> > && GFC_DECL_SAVED_DESCRIPTOR (expr) != expr)
> >return non_negative_strides_array_p (GFC_DECL_SAVED_DESCRIPTOR 
> > (expr));
> 
> I think I’ll use that.  There are numerous places where macros are repeated
> like this already and everybody seems to be pleased with it.
> Thanks for the feedback, and for the suggestions.

Agreed in this case, GFC_DECL_SAVED_DESCRIPTOR is really just a dereference
at least in release compiler.  Doing that when the macro actually calls some
functions is worse.

Jakub

Re: [Patch] OpenMP: Fix use_device_{addr,ptr} with in-data-sharing arg

[Jakub Jelinek via Fortran]

On Wed, Apr 20, 2022 at 03:19:38PM +0200, Tobias Burnus wrote:
> For array-descriptor vars, the descriptor is assigned to a temporary. However,
> this failed when the clause's argument was in turn in a data-sharing clause
> as the outer context's VALUE_EXPR wasn't used.
> 
> gcc/ChangeLog:
> 
>   * omp-low.cc (lower_omp_target): Fix use_device_{addr,ptr} with list
>   item that is in an outer data-sharing clause.
> 
> libgomp/ChangeLog:
> 
>   * testsuite/libgomp.fortran/use_device_addr-5.f90: New test.
> 
>  gcc/omp-low.cc |  22 ++--
>  .../libgomp.fortran/use_device_addr-5.f90  | 143 
> +
>  2 files changed, 156 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
> index bf5779b6543..6e387fd9a61 100644
> --- a/gcc/omp-low.cc
> +++ b/gcc/omp-low.cc
> @@ -13656,26 +13656,30 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, 
> omp_context *ctx)
>   new_var = lookup_decl (var, ctx);
>   new_var = DECL_VALUE_EXPR (new_var);
>   tree v = new_var;
> + tree v2 = var;
> + if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_USE_DEVICE_PTR
> + || OMP_CLAUSE_CODE (c) == OMP_CLAUSE_USE_DEVICE_ADDR)
> +   {
> + v2 = maybe_lookup_decl_in_outer_ctx (var, ctx);
> + if (DECL_HAS_VALUE_EXPR_P (v2))
> +   v2 = DECL_VALUE_EXPR (v2);

I don't understand the above 2 lines, why do you need that?
Regardless whether v2 has DECL_VALUE_EXPR or not, the type of the
DECL_VALUE_EXPR (v2) and v2 should be the same, build_fold_indirect_ref
should work on both and then v2 is only used as second operand of
gimplify_assign, where the gimplifier makes sure to handle DECL_VALUE_EXPR
correctly.  I certainly don't see any difference in the *.omplower dump
if I comment out the above 2 lines.

Otherwise LGTM, so if the 2 lines aren't needed, please also drop the
{}s around v2 = maybe_lookup_decl_in_outer_ctx (var, ctx); and reindent.

> +   }
>  
>   if (is_ref)
> {
> - var = build_fold_indirect_ref (var);
> - gimplify_expr (&var, &assign_body, NULL, is_gimple_val,
> -fb_rvalue);
> - v = create_tmp_var_raw (TREE_TYPE (var), get_name (var));
> + v2 = build_fold_indirect_ref (v2);
> + v = create_tmp_var_raw (TREE_TYPE (v2), get_name (var));
>   gimple_add_tmp_var (v);
>   TREE_ADDRESSABLE (v) = 1;
> - gimple_seq_add_stmt (&assign_body,
> -  gimple_build_assign (v, var));
> + gimplify_assign (v, v2, &assign_body);
>   tree rhs = build_fold_addr_expr (v);
>   gimple_seq_add_stmt (&assign_body,
>gimple_build_assign (new_var, rhs));
> }
>   else
> -   gimple_seq_add_stmt (&assign_body,
> -gimple_build_assign (new_var, var));
> +   gimplify_assign (new_var, v2, &assign_body);
>  
> - tree v2 = lang_hooks.decls.omp_array_data (unshare_expr (v), 
> false);
> + v2 = lang_hooks.decls.omp_array_data (unshare_expr (v), false);
>   gcc_assert (v2);
>   gimplify_expr (&x, &assign_body, NULL, is_gimple_val, 
> fb_rvalue);
>   gimple_seq_add_stmt (&assign_body,

Jakub

Re: [PATCH, stage 1] Fortran: Add support for OMP non-rectangular loops

[Jakub Jelinek via Fortran]

On Fri, Mar 25, 2022 at 08:03:38PM -0600, Sandra Loosemore wrote:
> This patch adds support for OMP 5.1 "canonical loop nest form" to the
> Fortran front end, marks non-rectangular loops for processing
> by the middle end, and implements missing checks in the gimplifier
> for additional prohibitions on non-rectangular loops.
> 
> Note that the OMP spec also prohibits non-rectangular loops with the TILE
> construct; that construct hasn't been implemented yet, so that error will
> need to be filled in later.
> 
>   gcc/fortran/
>   * gfortran.h (struct gfc_omp_clauses): Add non_rectangular bit.
>   * openmp.cc (is_outer_iteration_variable): New function.
>   (expr_is_invariant): New function.
>   (bound_expr_is_canonical): New function.
>   (resolve_omp_do): Replace existing non-rectangularity error with
>   check for canonical form and setting non_rectangular bit.
>   * trans-openmp.cc (gfc_trans_omp_do): Transfer non_rectangular
>   flag to generated tree structure.
> 
>   gcc/
>   * gimplify.cc (gimplify_omp_for): Update messages for SCHEDULED
>   and ORDERED clause conflict errors.  Add check for GRAINSIZE and
>   NUM_TASKS on TASKLOOP.
> 
>   gcc/testsuite/
>   * c-c++-common/gomp/loop-6.c (f3): New function to test TASKLOOP
>   diagnostics.
>   * gfortran.dg/gomp/collapse1.f90: Update expected messages.
>   * gfortran.dg/gomp/pr85313.f90: Remove dg-error on non-rectangular
>   loops that are now accepted.
>   * gfortran.dg/gomp/non-rectangular-loop.f90: New file.
>   * gfortran.dg/gomp/canonical-loop-1.f90: New file.
>   * gfortran.dg/gomp/canonical-loop-2.f90: New file.
> 
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -12468,11 +12468,11 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)
>  OMP_CLAUSE_SCHEDULE))
>   error_at (EXPR_LOCATION (for_stmt),
> "%qs clause may not appear on non-rectangular %qs",
> -   "schedule", "for");
> +   "schedule", (lang_GNU_Fortran () ? "do" : "for"));
> if (omp_find_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_ORDERED))
>   error_at (EXPR_LOCATION (for_stmt),
> "%qs clause may not appear on non-rectangular %qs",
> -   "ordered", "for");
> +   "ordered", (lang_GNU_Fortran () ? "do" : "for"));

Please drop the superfluous ()s around the argument, just use
  "...", lang_GNU_Fortran () ? "do" : "for");

Ok for trunk with that nit fixed.  And thanks for discovering the missing
grainsize/num_tasks checks.

Jakub

Re: [PATCH] Add a restriction on allocate clause (OpenMP 5.0)

[Jakub Jelinek via Fortran]

On Fri, Feb 18, 2022 at 11:13:16PM +, Hafiz Abid Qadeer wrote:
> An allocate clause in target region must specify an allocator
> unless the compilation unit has requires construct with
> dynamic_allocators clause.  Current implementation of the allocate
> clause did not check for this restriction. This patch fills that
> gap.
> 
> gcc/ChangeLog:
> 
>   * omp-low.cc (omp_maybe_offloaded_ctx): New prototype.
>   (scan_sharing_clauses):  Check a restriction on allocate clause.
> 
> gcc/testsuite/ChangeLog:
> 
>   * c-c++-common/gomp/allocate-2.c: Add tests.
>   * c-c++-common/gomp/allocate-8.c: New test.
>   * gfortran.dg/gomp/allocate-3.f90: Add tests.
>   * gcc.dg/gomp/pr104517.c: Update.

I think it has even stronger requirement, that the allocator is present
and is present in uses_allocators clause.

But we don't parse uses_allocators, so this is a good step forward.

Ok.

Jakub

Re: [PATCH] OpenMP, libgomp: Add new runtime routines omp_target_memcpy_async and omp_target_memcpy_rect_async

[Jakub Jelinek via Fortran]

On Mon, Feb 21, 2022 at 12:19:20PM +0100, Marcel Vollweiler wrote:
> gcc/ChangeLog:
> 
>   * omp-low.cc (omp_runtime_api_call): Added target_memcpy_async and
>   target_memcpy_rect_async to omp_runtime_apis array.
> 
> libgomp/ChangeLog:
> 
>   * libgomp.map: Added omp_target_memcpy_async and
>   omp_target_memcpy_rect_async.
>   * libgomp.texi: Both functions are now supported.
>   * omp.h.in: Added omp_target_memcpy_async and
>   omp_target_memcpy_rect_async.
>   * omp_lib.f90.in: Added interfaces for both new functions.
>   * omp_lib.h.in: Likewise.
>   * target.c (omp_target_memcpy): Restructured into check and copy part.
>   (omp_target_memcpy_check): New helper function for omp_target_memcpy and
>   omp_target_memcpy_async that checks requirements.
>   (omp_target_memcpy_copy): New helper function for omp_target_memcpy and
>   omp_target_memcpy_async that performs the memcpy.
>   (omp_target_memcpy_async_helper): New helper function that is used in
>   omp_target_memcpy_async for the asynchronous task.
>   (omp_target_memcpy_async): Added.
>   (omp_target_memcpy_rect): Restructured into check and copy part.
>   (omp_target_memcpy_rect_check): New helper function for
>   omp_target_memcpy_rect and omp_target_memcpy_rect_async that checks
>   requirements.
>   (omp_target_memcpy_rect_copy): New helper function for
>   omp_target_memcpy_rect and omp_target_memcpy_rect_async that performs
>   the memcpy.
>   (omp_target_memcpy_rect_async_helper): New helper function that is used
>   in omp_target_memcpy_rect_async for the asynchronous task.
>   (omp_target_memcpy_rect_async): Added.
>   * testsuite/libgomp.c-c++-common/target-memcpy-async-1.c: New test.
>   * testsuite/libgomp.c-c++-common/target-memcpy-async-2.c: New test.
>   * testsuite/libgomp.c-c++-common/target-memcpy-rect-async-1.c: New test.
>   * testsuite/libgomp.c-c++-common/target-memcpy-rect-async-2.c: New test.
>   * testsuite/libgomp.fortran/target-memcpy-async-1.f90: New test.
>   * testsuite/libgomp.fortran/target-memcpy-async-2.f90: New test.
>   * testsuite/libgomp.fortran/target-memcpy-rect-async-1.f90: New test.
>   * testsuite/libgomp.fortran/target-memcpy-rect-async-2.f90: New test.
> 
> --- a/libgomp/libgomp.map
> +++ b/libgomp/libgomp.map
> @@ -224,6 +224,8 @@ OMP_5.1 {
>   omp_set_teams_thread_limit_8_;
>   omp_get_teams_thread_limit;
>   omp_get_teams_thread_limit_;
> + omp_target_memcpy_async;
> + omp_target_memcpy_rect_async;
>  } OMP_5.0.2;

These should be added to OMP_5.1.1, not here.

> --- a/libgomp/omp.h.in
> +++ b/libgomp/omp.h.in
> @@ -272,6 +272,10 @@ extern int omp_target_is_present (const void *, int) 
> __GOMP_NOTHROW;
>  extern int omp_target_memcpy (void *, const void *, __SIZE_TYPE__,
> __SIZE_TYPE__, __SIZE_TYPE__, int, int)
>__GOMP_NOTHROW;
> +extern int omp_target_memcpy_async (void *, const void *, __SIZE_TYPE__,
> + __SIZE_TYPE__, __SIZE_TYPE__, int, int,
> + int, omp_depend_t*)

Formatting, space before *.

> +  __GOMP_NOTHROW;
>  extern int omp_target_memcpy_rect (void *, const void *, __SIZE_TYPE__, int,
>  const __SIZE_TYPE__ *,
>  const __SIZE_TYPE__ *,
> @@ -279,6 +283,14 @@ extern int omp_target_memcpy_rect (void *, const void *, 
> __SIZE_TYPE__, int,
>  const __SIZE_TYPE__ *,
>  const __SIZE_TYPE__ *, int, int)
>__GOMP_NOTHROW;
> +extern int omp_target_memcpy_rect_async (void *, const void *, __SIZE_TYPE__,
> +  int, const __SIZE_TYPE__ *,
> +  const __SIZE_TYPE__ *,
> +  const __SIZE_TYPE__ *,
> +  const __SIZE_TYPE__ *,
> +  const __SIZE_TYPE__ *, int, int, int,
> +  omp_depend_t*)

Likewise.

> -int
> -omp_target_memcpy (void *dst, const void *src, size_t length,
> -size_t dst_offset, size_t src_offset, int dst_device_num,
> -int src_device_num)
> +static int
> +omp_target_memcpy_check (void *dst, const void *src, int dst_device_num,
> +  int src_device_num,
> +  struct gomp_device_descr **dst_devicep,
> +  struct gomp_device_descr **src_devicep)
>  {

Why does omp_target_memcpy_check need the dst and src arguments?  From what
I can see, they aren't used by it.

> +typedef struct
> +{
> +  void *dst;
> +  const void *src;
> +  size_t length;
> +  size_t dst_offset;
> +  size_t src_offset;
> +  struct gomp_device_descr *dst_devicep;
> +  struct gomp_device_descr *src_devicep;
> +} memcpy_t;

Please come up with s

Re: [Patch] OpenMP, libgomp: Add new runtime routine omp_target_is_accessible.

[Jakub Jelinek via Fortran]

On Mon, Mar 14, 2022 at 04:42:14PM +0100, Marcel Vollweiler wrote:
> --- a/libgomp/libgomp.map
> +++ b/libgomp/libgomp.map
> @@ -226,6 +226,11 @@ OMP_5.1 {
>   omp_get_teams_thread_limit_;
>  } OMP_5.0.2;
>  
> +OMP_5.1.1 {
> +  global:
> + omp_target_is_accessible;
> +} OMP_5.1;
> +

You've already added another OMP_5.1.1 symbol, so this hunk will need to be
adjusted.  Keep the names in there alphabetically sorted.

> --- a/libgomp/omp_lib.f90.in
> +++ b/libgomp/omp_lib.f90.in
> @@ -835,6 +835,16 @@
>end function omp_target_disassociate_ptr
>  end interface
>  
> +interface
> +  function omp_target_is_accessible (ptr, size, device_num) bind(c)
> +use, intrinsic :: iso_c_binding, only : c_ptr, c_size_t, c_int
> +integer(c_int) :: omp_target_is_accessible

The function returning integer(c_int) rather than logical seems like
a screw up in the standard, but too late to fix that :(.

> --- a/libgomp/target.c
> +++ b/libgomp/target.c
> @@ -3666,6 +3666,24 @@ omp_target_disassociate_ptr (const void *ptr, int 
> device_num)
>  }
>  
>  int
> +omp_target_is_accessible (const void *ptr, size_t size, int device_num)
> +{
> +  if (device_num < 0 || device_num > gomp_get_num_devices ())
> +return false;
> +
> +  if (device_num == gomp_get_num_devices ())
> +return true;
> +
> +  struct gomp_device_descr *devicep = resolve_device (device_num);
> +  if (devicep == NULL)
> +return false;
> +
> +  /* TODO: Unified shared memory must be handled when available.  */
> +
> +  return devicep->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM;

I guess for now it is reasonable, but I wonder if even without
GOMP_OFFLOAD_CAP_SHARED_MEM one can't for CUDA or GCN allocate host
memory (not all, but just some subset) that will be accessible on the
device (I bet that means accessible through the same address on the host and
device, aka partial shared mem).

So, ok for trunk.

OT, tried to look how libomptarget implements it and they don't at least
on llvm-project trunk, but while looking at that, noticed that for
omp_target_is_present they do return false from omp_target_is_present
while we return true.  It is unclear if NULL has corresponding storage
on the device (NULL always corresponds to NULL on the device) or not.

Jakub

Re: [Patch] OpenMP, libgomp: Add new runtime routine omp_target_is_accessible.

[Jakub Jelinek via Fortran]

On Thu, May 05, 2022 at 11:45:19AM +0200, Tobias Burnus wrote:
> > On Mon, Mar 14, 2022 at 04:42:14PM +0100, Marcel Vollweiler wrote:
> > > +interface
> > > +  function omp_target_is_accessible (ptr, size, device_num) 
> > > bind(c)
> > > +use, intrinsic :: iso_c_binding, only : c_ptr, c_size_t, 
> > > c_int
> > > +integer(c_int) :: omp_target_is_accessible
> > The function returning integer(c_int) rather than logical seems like
> > a screw up in the standard, but too late to fix that :(.
> 
> I think the idea is that it can directly call the C function without
> needing a wrapper. And as default-kind 'logical' != 'integer(c_int)' in
> general, it cannot return logical. (In case of GCC, just claiming that
> it is logical would work. But some Fortran compilers use -1 for .true.
> and only flip a single bit for .not. For those,
> "if(.not.omp_target_is_accessible(..)) will not work properly, if the C
> function returns 1.
> 
> But I concur that requiring "/= 0" is ugly!

Yeah, but for the APIs that don't have any iso_c_binding arguments
we just use wrappers rather than bind(c) and it allows for more Fortran-like
callers.  So, if omp_target_is_accessible had the *_ wrapper (or alias if
we determine logical ir the same as c_int in the ABI passing), people could
avoid the /= 0 stuff.
Anyway, that is just a thought for future APIs that if they return
false/true only bind(c) isn't always a good idea.

Jakub

Re: [Patch] OpenMP/Fortran: Use firstprivat not alloc for ptr attach for arrays

[Jakub Jelinek via Fortran]

On Fri, May 13, 2022 at 07:21:02PM +0200, Tobias Burnus wrote:
> gcc/fortran/ChangeLog:
> 
>   * trans-openmp.cc (gfc_trans_omp_clauses): When mapping nondescriptor
>   array sections, use GOMP_MAP_FIRSTPRIVATE_POINTER instead of
>   GOMP_MAP_POINTER for the pointer attachment.
> 
> libgomp/ChangeLog:
> 
>   * testsuite/libgomp.fortran/target-nowait-array-section.f90: New test.

Not 100% sure if we want to add such a testcase into the testsuite given
that it is not valid OpenMP, but perhaps it is ok as we are testing a QoI.

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/target-nowait-array-section.f90
> @@ -0,0 +1,56 @@
> +! Runs the the target region asynchrolously and checks for it
> +!
> +! Note that  map(alloc: work(:, i)) + nowait  should be save

s/save/safe/

Otherwise LGTM.

Jakub

Re: [Patch] OpenMP: Add omp_all_memory support to Fortran

[Jakub Jelinek via Fortran]

On Fri, May 13, 2022 at 10:51:56PM +0200, Tobias Burnus wrote:
> This adds omp_all_memory handling to Fortran, following C/C++ and shamelessly 
> coping
> the C testcases and adapting them to Fortran.
> 
> OK?
> 
> Tobias
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> OpenMP: Add omp_all_memory support to Fortran
> 
> Fortran part to the C/C++/backend implementation
> r13-337-g7f78783dbedca0183d193e475262ca3c489fd365
> 
> gcc/fortran/ChangeLog:
> 
>   * dump-parse-tree.cc (show_omp_namelist): Handle omp_all_memory.
>   * openmp.cc (gfc_match_omp_variable_list, gfc_match_omp_depend_sink,
>   gfc_match_omp_clauses, resolve_omp_clauses): Likewise.
>   * trans-openmp.cc (gfc_trans_omp_clauses, gfc_trans_omp_depobj):
>   Likewise.
>   * resolve.cc (resolve_symbol): Reject it as symbol.
> 
> libgomp/ChangeLog:
> 
>   * libgomp.texi (OpenMP 5.1): Set omp_all_memory to 'Y'.
>   * testsuite/libgomp.fortran/depend-5.f90: New test.
>   * testsuite/libgomp.fortran/depend-6.f90: New test.
>   * testsuite/libgomp.fortran/depend-7.f90: New test.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/all-memory-1.f90: New test.
>   * gfortran.dg/gomp/all-memory-2.f90: New test.
>   * gfortran.dg/gomp/all-memory-3.f90: New test.

LGTM, thanks.

Jakub

Re: [Patch] OpenMP: Add Fortran support for inoutset depend-kind (was: openmp: Add support for inoutset depend-kind)

[Jakub Jelinek via Fortran]

On Wed, May 18, 2022 at 11:02:27AM +0200, Tobias Burnus wrote:
> OpenMP: Add Fortran support for inoutset depend-kind
> 
> Fortran additions to the C/C++ + ME/libgomp commit
> r13-556-g2c16eb3157f86ae561468c540caf8eb326106b5f
> 
> gcc/fortran/ChangeLog:
> 
>   * gfortran.h (enum gfc_omp_depend_op): Add OMP_DEPEND_INOUTSET.
>   (gfc_omp_clauses): Enlarge ENUM_BITFIELD.
>   * dump-parse-tree.cc (show_omp_namelist, show_omp_clauses): Handle
>   'inoutset' depend modifier.
>   * openmp.cc (gfc_match_omp_clauses, gfc_match_omp_depobj): Likewise.
>   * trans-openmp.cc (gfc_trans_omp_clauses, gfc_trans_omp_depobj):
>   Likewise.
> 
> libgomp/ChangeLog:
> 
>   * libgomp.texi (OpenMP 5.1): Set 'inoutset' to Y.
>   (OpenMP Context Selectors): Add missing comma.
>   * testsuite/libgomp.fortran/depend-5.f90: Add inoutset test.
>   * testsuite/libgomp.fortran/depend-6.f90: Likewise.
>   * testsuite/libgomp.fortran/depend-7.f90: Likewise.
>   * testsuite/libgomp.fortran/depend-inoutset-1.f90: New test.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/all-memory-1.f90: Add inoutset test.
>   * gfortran.dg/gomp/all-memory-2.f90: Likewise.
>   * gfortran.dg/gomp/depobj-1.f90: Likewise.
>   * gfortran.dg/gomp/depobj-2.f90: Likewise.

Ok, thanks.

Jakub

Re: [PATCH] OpenMP, libgomp: Add new runtime routines omp_target_memcpy_async and omp_target_memcpy_rect_async

[Jakub Jelinek via Fortran]

On Thu, May 19, 2022 at 10:39:05AM +0200, Marcel Vollweiler wrote:
> > add here
> >else
> >  {
> >depend[0] = 0;
> > ...
> >  }
> 
> Added the "depend" definition to the "if" branch (instead the "else" branch).

Thanks for correcting my thinko.

> gcc/ChangeLog:
> 
>   * omp-low.cc (omp_runtime_api_call): Added target_memcpy_async and
>   target_memcpy_rect_async to omp_runtime_apis array.
> 
> libgomp/ChangeLog:
> 
>   * libgomp.map: Added omp_target_memcpy_async and
>   omp_target_memcpy_rect_async.
>   * libgomp.texi: Both functions are now supported.
>   * omp.h.in: Added omp_target_memcpy_async and
>   omp_target_memcpy_rect_async.
>   * omp_lib.f90.in: Added interfaces for both new functions.
>   * omp_lib.h.in: Likewise.
>   * target.c (ialias_redirect): Added for GOMP_task.
>   (omp_target_memcpy): Restructured into check and copy part.
>   (omp_target_memcpy_check): New helper function for omp_target_memcpy and
>   omp_target_memcpy_async that checks requirements.
>   (omp_target_memcpy_copy): New helper function for omp_target_memcpy and
>   omp_target_memcpy_async that performs the memcpy.
>   (omp_target_memcpy_async_helper): New helper function that is used in
>   omp_target_memcpy_async for the asynchronous task.
>   (omp_target_memcpy_async): Added.
>   (omp_target_memcpy_rect): Restructured into check and copy part.
>   (omp_target_memcpy_rect_check): New helper function for
>   omp_target_memcpy_rect and omp_target_memcpy_rect_async that checks
>   requirements.
>   (omp_target_memcpy_rect_copy): New helper function for
>   omp_target_memcpy_rect and omp_target_memcpy_rect_async that performs
>   the memcpy.
>   (omp_target_memcpy_rect_async_helper): New helper function that is used
>   in omp_target_memcpy_rect_async for the asynchronous task.
>   (omp_target_memcpy_rect_async): Added.
>   * task.c (ialias): Added for GOMP_task.
>   * testsuite/libgomp.c-c++-common/target-memcpy-async-1.c: New test.
>   * testsuite/libgomp.c-c++-common/target-memcpy-async-2.c: New test.
>   * testsuite/libgomp.c-c++-common/target-memcpy-rect-async-1.c: New test.
>   * testsuite/libgomp.c-c++-common/target-memcpy-rect-async-2.c: New test.
>   * testsuite/libgomp.fortran/target-memcpy-async-1.f90: New test.
>   * testsuite/libgomp.fortran/target-memcpy-async-2.f90: New test.
>   * testsuite/libgomp.fortran/target-memcpy-rect-async-1.f90: New test.
>   * testsuite/libgomp.fortran/target-memcpy-rect-async-2.f90: New test.

Ok, thanks.

Jakub

Re: [Patch] OpenMP: Handle descriptors in target's firstprivate [PR104949]

[Jakub Jelinek via Fortran]

On Wed, May 11, 2022 at 07:33:00PM +0200, Tobias Burnus wrote:
> gcc/fortran/ChangeLog:
> 
>   PR fortran/104949
>   * f95-lang.cc (LANG_HOOKS_OMP_ARRAY_SIZE): Redefine.
>   * trans-openmp.cc (gfc_omp_array_size): New.
>   (gfc_trans_omp_variable_list): Never turn has_device_addr
>   to firstprivate.
>   * trans.h (gfc_omp_array_size): New.
> 
> gcc/ChangeLog:
> 
>   PR fortran/104949
>   * langhooks-def.h (lhd_omp_array_size): New.
>   (LANG_HOOKS_OMP_ARRAY_SIZE): Define

Missing full stop above.

>   (LANG_HOOKS_DECLS): Add it.
>   * langhooks.cc (lhd_omp_array_size): New.
>   * langhooks.h (struct lang_hooks_for_decls): Add hook.
>   * omp-low.cc (scan_sharing_clauses, lower_omp_target):
>   Handle GOMP_MAP_FIRSTPRIVATE for array descriptors.
> 
> libgomp/ChangeLog:
> 
>   PR fortran/104949
>   * target.c (gomp_map_vars_internal, copy_firstprivate_data):
>   Support attach for GOMP_MAP_FIRSTPRIVATE.
>   * testsuite/libgomp.fortran/target-firstprivate-1.f90: New test.
>   * testsuite/libgomp.fortran/target-firstprivate-2.f90: New test.
>   * testsuite/libgomp.fortran/target-firstprivate-3.f90: New test.

I guess ok like this for now, but handling the further deep copy cases
(allocatable members of derived types) wouldn't be very nice, I think
generally we need a target hook to handle the stuff that is target specific
and express it say in further clauses or their modified copies (perhaps some
flags on them, or new clause types) which will allow the pointer attachments
to be done.

Generally, for firstprivate on constructs other than target we invoke
a copy constructor or its language equivalent (memcpy for C, perhaps some
deep copying for Fortran), which takes care of stuff like in C++ embedded
reference type members etc.
But for target we don't have such a luxury, we don't have copy ctors between
different devices and so we need to do something different, for now it has
been mainly just bitwise copying the aggregate between devices.
But eventually it would be nice to have a target hook that emulates the
cross-device copy construction.  And we probably need also something to
emulate destruction...

Jakub

Re: [PATCH v2 01/11] OpenMP 5.0: Clause ordering for OpenMP 5.0 (topological sorting by base pointer)

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 09:24:51AM -0700, Julian Brown wrote:
> 2021-11-23  Julian Brown  
> 
> gcc/
>   * gimplify.c (is_or_contains_p, omp_target_reorder_clauses): Delete
>   functions.
>   (omp_tsort_mark): Add enum.
>   (omp_mapping_group): Add struct.
>   (debug_mapping_group, omp_get_base_pointer, omp_get_attachment,
>   omp_group_last, omp_gather_mapping_groups, omp_group_base,
>   omp_index_mapping_groups, omp_containing_struct,
>   omp_tsort_mapping_groups_1, omp_tsort_mapping_groups,
>   omp_segregate_mapping_groups, omp_reorder_mapping_groups): New
>   functions.
>   (gimplify_scan_omp_clauses): Call above functions instead of
>   omp_target_reorder_clauses, unless we've seen an error.
>   * omp-low.c (scan_sharing_clauses): Avoid strict test if we haven't
>   sorted mapping groups.
> 
> gcc/testsuite/
>   * g++.dg/gomp/target-lambda-1.C: Adjust expected output.
>   * g++.dg/gomp/target-this-3.C: Likewise.
>   * g++.dg/gomp/target-this-4.C: Likewise.
> +

Wouldn't hurt to add a comment on the meanings of the enumerators.

> +enum omp_tsort_mark {
> +  UNVISITED,
> +  TEMPORARY,
> +  PERMANENT
> +};
> +
> +struct omp_mapping_group {
> +  tree *grp_start;
> +  tree grp_end;
> +  omp_tsort_mark mark;
> +  struct omp_mapping_group *sibling;
> +  struct omp_mapping_group *next;
> +};
> +
> +__attribute__((used)) static void

I'd use what is used elsewhere,
DEBUG_FUNCTION void
without static.

> +debug_mapping_group (omp_mapping_group *grp)
> +{
> +  tree tmp = OMP_CLAUSE_CHAIN (grp->grp_end);
> +  OMP_CLAUSE_CHAIN (grp->grp_end) = NULL;
> +  debug_generic_expr (*grp->grp_start);
> +  OMP_CLAUSE_CHAIN (grp->grp_end) = tmp;
> +}
> +
> +/* Return the OpenMP "base pointer" of an expression EXPR, or NULL if there
> +   isn't one.  This needs improvement.  */
> +
> +static tree
> +omp_get_base_pointer (tree expr)
> +{
> +  while (TREE_CODE (expr) == ARRAY_REF)
> +expr = TREE_OPERAND (expr, 0);
> +
> +  while (TREE_CODE (expr) == COMPONENT_REF
> +  && (DECL_P (TREE_OPERAND (expr, 0))
> +  || (TREE_CODE (TREE_OPERAND (expr, 0)) == COMPONENT_REF)
> +  || TREE_CODE (TREE_OPERAND (expr, 0)) == INDIRECT_REF
> +  || (TREE_CODE (TREE_OPERAND (expr, 0)) == MEM_REF
> +  && integer_zerop (TREE_OPERAND (TREE_OPERAND (expr, 0), 1)))
> +  || TREE_CODE (TREE_OPERAND (expr, 0)) == ARRAY_REF))
> +{
> +  expr = TREE_OPERAND (expr, 0);
> +
> +  while (TREE_CODE (expr) == ARRAY_REF)
> + expr = TREE_OPERAND (expr, 0);
> +
> +  if (TREE_CODE (expr) == INDIRECT_REF || TREE_CODE (expr) == MEM_REF)
> + break;
> +}

I must say I don't see advantages of just a single loop that
looks through all ARRAY_REFs and all COMPONENT_REFs and then just
checks if the expr it got in the end is a decl or INDIRECT_REF
or MEM_REF with offset 0.

> +  if (DECL_P (expr))
> +return NULL_TREE;
> +
> +  if (TREE_CODE (expr) == INDIRECT_REF
> +  || TREE_CODE (expr) == MEM_REF)
> +{
> +  expr = TREE_OPERAND (expr, 0);
> +  while (TREE_CODE (expr) == COMPOUND_EXPR)
> + expr = TREE_OPERAND (expr, 1);
> +  if (TREE_CODE (expr) == POINTER_PLUS_EXPR)
> + expr = TREE_OPERAND (expr, 0);
> +  if (TREE_CODE (expr) == SAVE_EXPR)
> + expr = TREE_OPERAND (expr, 0);
> +  STRIP_NOPS (expr);
> +  return expr;
> +}
> +
> +  return NULL_TREE;
> +}
> +

> +static tree
> +omp_containing_struct (tree expr)
> +{
> +  tree expr0 = expr;
> +
> +  STRIP_NOPS (expr);
> +
> +  tree expr1 = expr;
> +
> +  /* FIXME: other types of accessors?  */
> +  while (TREE_CODE (expr) == ARRAY_REF)
> +expr = TREE_OPERAND (expr, 0);
> +
> +  if (TREE_CODE (expr) == COMPONENT_REF)
> +{
> +  if (DECL_P (TREE_OPERAND (expr, 0))
> +   || TREE_CODE (TREE_OPERAND (expr, 0)) == COMPONENT_REF
> +   || TREE_CODE (TREE_OPERAND (expr, 0)) == INDIRECT_REF
> +   || (TREE_CODE (TREE_OPERAND (expr, 0)) == MEM_REF
> +   && integer_zerop (TREE_OPERAND (TREE_OPERAND (expr, 0), 1)))
> +   || TREE_CODE (TREE_OPERAND (expr, 0)) == ARRAY_REF)
> + expr = TREE_OPERAND (expr, 0);
> +  else
> + internal_error ("unhandled component");
> +}

Again?

> @@ -9063,11 +9820,29 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq 
> *pre_p,
>   break;
>}
>  
> -  if (code == OMP_TARGET
> -  || code == OMP_TARGET_DATA
> -  || code == OMP_TARGET_ENTER_DATA
> -  || code == OMP_TARGET_EXIT_DATA)
> -omp_target_reorder_clauses (list_p);
> +  /* Topological sorting may fail if we have duplicate nodes, which
> + we should have detected and shown an error for already.  Skip
> + sorting in that case.  */
> +  if (!seen_error ()
> +  && (code == OMP_TARGET
> +   || code == OMP_TARGET_DATA
> +   || code == OMP_TARGET_ENTER_DATA
> +   || code == OMP_TARGET_EXIT_DATA))
> +{
> +  vec *groups;
> +  groups = omp_gather_mapping_group

Re: [PATCH v2 02/11] Remove omp_target_reorder_clauses

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 09:24:52AM -0700, Julian Brown wrote:
> This patch has been split out from the previous one to avoid a
> confusingly-interleaved diff.  The two patches should probably be
> committed squashed together.

Agreed, LGTM.
> 
> 2021-10-01  Julian Brown  
> 
> gcc/
>   * gimplify.c (omp_target_reorder_clauses): Delete.

Jakub

Re: [PATCH v2 03/11] OpenMP/OpenACC struct sibling list gimplification extension and rework

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 09:24:53AM -0700, Julian Brown wrote:
> 2022-03-17  Julian Brown  
> 
> gcc/fortran/
> * trans-openmp.cc (gfc_trans_omp_clauses): Don't create
> GOMP_MAP_TO_PSET mappings for class metadata, nor GOMP_MAP_POINTER
> mappings for POINTER_TYPE_P decls.
> 
> gcc/
> * gimplify.c (gimplify_omp_var_data): Remove GOVD_MAP_HAS_ATTACHMENTS.
> (insert_struct_comp_map): Refactor function into...
> (build_struct_comp_nodes): This new function.  Remove list handling
> and improve self-documentation.
> (extract_base_bit_offset): Remove BASE_REF, OFFSETP parameters.  Move
> code to strip outer parts of address out of function, but strip no-op
> conversions.
> (omp_mapping_group): Add DELETED field for use during reindexing.
> (strip_components_and_deref, strip_indirections): New functions.
> (omp_group_last, omp_group_base): Add GOMP_MAP_STRUCT handling.
> (omp_gather_mapping_groups): Initialise DELETED field for new groups.
> (omp_index_mapping_groups): Notice DELETED groups when (re)indexing.
> (insert_node_after, move_node_after, move_nodes_after,
> move_concat_nodes_after): New helper functions.
> (accumulate_sibling_list): New function to build up GOMP_MAP_STRUCT
> node groups for sibling lists. Outlined from 
> gimplify_scan_omp_clauses.
> (omp_build_struct_sibling_lists): New function.
> (gimplify_scan_omp_clauses): Remove struct_map_to_clause,
> struct_seen_clause, struct_deref_set.  Call
> omp_build_struct_sibling_lists as pre-pass instead of handling sibling
> lists in the function's main processing loop.
> (gimplify_adjust_omp_clauses_1): Remove GOVD_MAP_HAS_ATTACHMENTS
> handling, unused now.
> * omp-low.cc (scan_sharing_clauses): Handle pointer-type indirect
> struct references, and references to pointers to structs also.
> 
> gcc/testsuite/
> * g++.dg/goacc/member-array-acc.C: New test.
> * g++.dg/gomp/member-array-omp.C: New test.
> * g++.dg/gomp/target-3.C: Update expected output.
> * g++.dg/gomp/target-lambda-1.C: Likewise.
> * g++.dg/gomp/target-this-2.C: Likewise.
> * c-c++-common/goacc/deep-copy-arrayofstruct.c: Move test from here.
> 
> libgomp/
> * testsuite/libgomp.oacc-c-c++-common/deep-copy-15.c: New test.
> * testsuite/libgomp.oacc-c-c++-common/deep-copy-16.c: New test.
> * testsuite/libgomp.oacc-c++/deep-copy-17.C: New test.
> * testsuite/libgomp.oacc-c-c++-common/deep-copy-arrayofstruct.c: Move
> test to here, make "run" test.

> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -125,10 +125,6 @@ enum gimplify_omp_var_data
>/* Flag for GOVD_REDUCTION: inscan seen in {in,ex}clusive clause.  */
>GOVD_REDUCTION_INSCAN = 0x200,
>  
> -  /* Flag for GOVD_MAP: (struct) vars that have pointer attachments for
> - fields.  */
> -  GOVD_MAP_HAS_ATTACHMENTS = 0x400,
> -
>/* Flag for GOVD_FIRSTPRIVATE: OMP_CLAUSE_FIRSTPRIVATE_IMPLICIT.  */
>GOVD_FIRSTPRIVATE_IMPLICIT = 0x800,

I'd renumber the GOVD_* constants after this, otherwise we won't remember
we've left a gap.

> +   (or derived type, etc.) component, create an "alloc" or "release" node to
> +   insert into a list following a GOMP_MAP_STRUCT node.  For some types of
> +   mapping (e.g. Fortran arrays with descriptors), an additional mapping may
> +   be created that is inserted into the list of mapping nodes attached to the
> +   directive being processed -- not part of the sorted list of nodes after
> +   GOMP_MAP_STRUCT.
> +
> +   CODE is the code of the directive being processed.  GRP_START and GRP_END
> +   are the first and last of two or three nodes representing this array 
> section
> +   mapping (e.g. a data movement node like GOMP_MAP_{TO,FROM}, optionally a
> +   GOMP_MAP_TO_PSET, and finally a GOMP_MAP_ALWAYS_POINTER).  EXTRA_NODE is
> +   filled with the additional node described above, if needed.
> +
> +   This function does not add the new nodes to any lists itself.  It is the
> +   responsibility of the caller to do that.  */
>  
>  static tree
> -insert_struct_comp_map (enum tree_code code, tree c, tree struct_node,
> - tree prev_node, tree *scp)
> +build_struct_comp_nodes (enum tree_code code, tree grp_start, tree grp_end,
> +  tree *extra_node)

I think it would be nice to use omp_ prefixes even for these static
functions, this is all in the gimplifier, so it should be clear that it
isn't some generic code but OpenMP specific gimplification code.

Another variant would be to introduce omp-gimplify.cc and move lots of stuff
there, but if we do that, best time might be during stage3 so that it
doesn't collide with too many patches.
>  
> +/* Link node NEWNODE so it is pointed to by chain INSERT_AT.  NEWNODE's chain
> +   is linked to

Re: [PATCH v2 04/11] OpenMP/OpenACC: Add inspector class to unify mapped address analysis

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 09:24:54AM -0700, Julian Brown wrote:
> 2022-03-17  Julian Brown  
> 
> gcc/c-family/
> * c-common.h (c_omp_address_inspector): New class.
> * c-omp.c (c_omp_address_inspector::get_deref_origin,
> c_omp_address_inspector::component_access_p,
> c_omp_address_inspector::check_clause,
> c_omp_address_inspector::get_root_term,

Spaces instead of tabs.

>   c_omp_address_inspector::map_supported_p,
>   c_omp_address_inspector::mappable_type,
>   c_omp_address_inspector::get_origin,
>   c_omp_address_inspector::peel_components,
>   c_omp_address_inspector::maybe_peel_ref,
>   c_omp_address_inspector::maybe_zero_length_array_section,
>   c_omp_address_inspector::get_base_pointer,
>   c_omp_address_inspector::get_base_pointer_tgt,
>   c_omp_address_inspector::get_attachment_point): New methods.

> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1253,6 +1253,61 @@ extern void c_omp_mark_declare_variant (location_t, 
> tree, tree);
>  extern const char *c_omp_map_clause_name (tree, bool);
>  extern void c_omp_adjust_map_clauses (tree, bool);
>  
> +class c_omp_address_inspector
> +{
> +  location_t loc;
> +  tree root_term;
> +  bool indirections;
> +  int map_supported;
> +
> +protected:
> +  tree orig;
> +
> +public:
> +  c_omp_address_inspector (location_t loc, tree t)
> +: loc (loc), root_term (NULL_TREE), indirections (false),
> +  map_supported (-1), orig (t)
> +  { }
> +
> +  ~c_omp_address_inspector () {}
> +
> +  virtual bool processing_template_decl_p () { return false; }
> +  virtual bool mappable_type (tree t);
> +  virtual void emit_unmappable_type_notes (tree) { }
> +
> +  bool check_clause (tree);
> +  tree get_root_term (bool);
> +
> +  tree get_address () { return orig; }
> +  tree get_deref_origin ();
> +  bool component_access_p ();
> +
> +  bool has_indirections_p ()
> +{
> +  if (!root_term)
> + get_root_term (false);
> +  return indirections;
> +}
> +
> +  bool indir_component_ref_p ()
> +{
> +  return component_access_p () && has_indirections_p ();
> +}

I think https://gcc.gnu.org/codingconventions.html#Cxx_Conventions
just says that no member functions should be defined inside of the
class, which is something that almost nobody actually honors.
But, when they are inline, there should be one style, not many,
so either
  type method (args)
  {
  }
(guess my preference) or
  type method (args)
{
}
but not those mixed up, which you have in the patch.

> --- a/gcc/c-family/c-omp.cc
> +++ b/gcc/c-family/c-omp.cc
> @@ -3113,6 +3113,274 @@ c_omp_adjust_map_clauses (tree clauses, bool 
> is_target)
>  }
>  }
>  

There should be function comment for all the out of line definitions.
> +tree
> +c_omp_address_inspector::get_deref_origin ()

>  {
>if (error_operand_p (t))
>   return error_mark_node;
> +  c_omp_address_inspector t_insp (OMP_CLAUSE_LOCATION (c), t);

Wouldn't ai (address inspector) be better than t_insp?

> +/* C++ specialisation of the c_omp_address_inspector class.  */
> +
> +class cp_omp_address_inspector : public c_omp_address_inspector
> +{
> +public:
> +  cp_omp_address_inspector (location_t loc, tree t)
> +: c_omp_address_inspector (loc, t)
> +  { }
> +
> +  ~cp_omp_address_inspector ()
> +  { }
> +
> +  bool processing_template_decl_p ()
> +  {
> +return processing_template_decl;
> +  }
> +
> +  bool mappable_type (tree t)
> +  {
> +return cp_omp_mappable_type (t);
> +  }
> +
> +  void emit_unmappable_type_notes (tree t)
> +  {
> +cp_omp_emit_unmappable_type_notes (t);
> +  }
> +
> +  static bool ref_p (tree t)
> +{
> +  return (TYPE_REF_P (TREE_TYPE (t))
> +   || REFERENCE_REF_P (t));
> +}

See above the mixing of styles...
I know, some headers are really bad examples, e.g. hash-map.h
even has 3 different styles,
  {
  }
and
{
}
and
  {
  }
for the type method (args) indented by 2 spaces.

> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/gomp/unmappable-component-1.C
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +
> +struct A {
> +  static int x[10];
> +};
> +
> +struct B {
> +  A a;
> +};
> +
> +int
> +main (int argc, char *argv[])
> +{
> +  B *b = new B;
> +#pragma omp target map(b->a) // { dg-error "'b->B::a' does not have a 
> mappable type in 'map' clause" }
> +  ;
> +  B bb;
> +#pragma omp target map(bb.a) // { dg-error "'bb\.B::a' does not have a 
> mappable type in 'map' clause" }

We don't diagnose static data members as non-mappable anymore.
So I don't see how this test can work.

> +int
> +main (int argc, char *argv[])

Why "int argc, char *argv[]" when you don't use it?

> +  p0 = (p0type *) malloc (sizeof *p0);
> +  p0->x0[0].p1 = (p1type *) malloc (sizeof *p0->x0[0].p1);
> +  p0->x0[0].p1->p2 = (p2type *) malloc (sizeof *p0->x0[0].p1->p2);
> +  memset (p0->x0[0].p1->p2, 0, sizeof *p0->x0[0].p1->p2);
> +
> +#pragma omp target map(

Re: [PATCH v2 05/11] OpenMP: Handle reference-typed struct members

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 09:26:46AM -0700, Julian Brown wrote:
> This patch relates to OpenMP mapping clauses containing struct members of
> reference type, e.g. "mystruct.myref.myptr[:N]".  To be able to access
> the array slice through the reference in the middle, we need to perform
> an attach action for that reference, since it is represented internally
> as a pointer.
> 
> I don't think the spec allows for this case explicitly.  The closest
> clause is (OpenMP 5.0, "2.19.7.1 map Clause"):
> 
>   "If the type of a list item is a reference to a type T then the
>reference in the device data environment is initialized to refer to
>the object in the device data environment that corresponds to the
>object referenced by the list item. If mapping occurs, it occurs as
>though the object were mapped through a pointer with an array section
>of type T and length one."

Plus the general rule that aggregates are mapped as mapping of all its
individual members/elements.

> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -9813,7 +9813,10 @@ accumulate_sibling_list (enum omp_region_type 
> region_type, enum tree_code code,
>/* FIXME: If we're not mapping the base pointer in some other clause on 
> this
>   directive, I think we want to create ALLOC/RELEASE here -- i.e. not
>   early-exit.  */
> -  if (openmp && attach_detach)
> +  if (openmp
> +  && attach_detach
> +  && !(TREE_CODE (TREE_TYPE (ocd)) == REFERENCE_TYPE
> +&& TREE_CODE (TREE_TYPE (TREE_TYPE (ocd))) != POINTER_TYPE))
>  return NULL;

Why isn't a reference to pointer handled that way too?

Jakub

Re: [PATCH v2 06/11] OpenMP: lvalue parsing for map clauses (C++)

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 09:26:47AM -0700, Julian Brown wrote:
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -4266,6 +4266,9 @@ cp_parser_new (cp_lexer *lexer)
>parser->omp_declare_simd = NULL;
>parser->oacc_routine = NULL;
>  
> +  /* Allow array slice in expression.  */

Better /* Disallow OpenMP array sections in expressions.  */

> +  parser->omp_array_section_p = false;
> +
>/* Not declaring an implicit function template.  */
>parser->auto_is_implicit_function_template_parm_p = false;
>parser->fully_implicit_function_template_p = false;

I think we should figure out when we should temporarily disable
  parser->omp_array_section_p = false;
and restore it afterwards to a saved value.  E.g.
cp_parser_lambda_expression seems like a good candidate, the fact that
OpenMP array sections are allowed say in map clause doesn't mean they are
allowed inside of lambdas and it would be especially hard when the lambda
is defining a separate function and the search for OMP_ARRAY_SECTION
probably wouldn't be able to discover those.
Other spots to consider might be statement expressions, perhaps type
definitions etc.

> @@ -8021,6 +8024,7 @@ cp_parser_postfix_open_square_expression (cp_parser 
> *parser,
>releasing_vec expression_list = NULL;
>location_t loc = cp_lexer_peek_token (parser->lexer)->location;
>bool saved_greater_than_is_operator_p;
> +  bool saved_colon_corrects_to_scope_p;
>  
>/* Consume the `[' token.  */
>cp_lexer_consume_token (parser->lexer);
> @@ -8028,6 +8032,9 @@ cp_parser_postfix_open_square_expression (cp_parser 
> *parser,
>saved_greater_than_is_operator_p = parser->greater_than_is_operator_p;
>parser->greater_than_is_operator_p = true;
>  
> +  saved_colon_corrects_to_scope_p = parser->colon_corrects_to_scope_p;
> +  parser->colon_corrects_to_scope_p = false;

I think the last above line should be guarded on
  if (parser->omp_array_section_p)
There is no reason to get worse diagnostics in non-OpenMP code or even in
OpenMP code where array sections aren't allowed.

> +
> +  /* NOTE: We are reusing using the type of the whole array as the type 
> of
> +  the array section here, which isn't necessarily entirely correct.
> +  Might need revisiting.  */

"reusing using" looks weird.
As for the type of OMP_ARRAY_SECTION trees, perhaps we could initially use
an incomplete array (so array element would be meaningful)
and when we figure out the details and the array section is contiguous
change its type to array type covering it.

> +  return build3_loc (input_location, OMP_ARRAY_SECTION,
> +  TREE_TYPE (postfix_expression),
> +  postfix_expression, index, length);
> +}
> +
> +  parser->colon_corrects_to_scope_p = saved_colon_corrects_to_scope_p;
> +
>/* Look for the closing `]'.  */
>cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
>  
> @@ -36536,7 +36570,7 @@ struct omp_dim
>  static tree
>  cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind,
>   tree list, bool *colon,
> - bool allow_deref = false)
> + bool map_lvalue = false)
>  {
>auto_vec dims;
>bool array_section_p;
> @@ -36547,12 +36581,95 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, 
> enum omp_clause_code kind,
>parser->colon_corrects_to_scope_p = false;
>*colon = false;
>  }
> +  begin_scope (sk_omp, NULL);

Why?  Base-language-wise, clauses don't introduce a new scope
for name-lookup.
And if it is really needed, I'd strongly prefer to either do it solely
for the clauses that might need it, or do begin_scope before first
such clause and finish at the end if it has been introduced.

>while (1)
>  {
>tree name, decl;
>  
>if (kind == OMP_CLAUSE_DEPEND || kind == OMP_CLAUSE_AFFINITY)
>   cp_parser_parse_tentatively (parser);
> +  else if (map_lvalue && kind == OMP_CLAUSE_MAP)
> + {

This shouldn't be done just for OMP_CLAUSE_MAP, but for all the
other clauses that accept array sections, including
OMP_CLAUSE_DEPEND, OMP_CLAUSE_AFFINITY, OMP_CLAUSE_MAP, OMP_CLAUSE_TO,
OMP_CLAUSE_FROM, OMP_CLAUSE_INCLUSIVE, OMP_CLAUSE_EXCLUSIVE,
OMP_CLAUSE_USE_DEVICE_ADDR, OMP_CLAUSE_HAS_DEVICE_ADDR,
OMP_CLAUSE_*REDUCTION.
And preferrably, they should be kept in the IL until *finish_omp_clauses,
which should handle those instead of TREE_LIST that represented them before.
Additionally, something should diagnose incorrect uses of OMP_ARRAY_SECTION,
which is everywhere in the expressions but as the outermost node(s),
i.e. for clauses that do allow array sections scan OMP_CLAUSE_DECL after
handling handleable array sections and complain about embedded
OMP_ARRAY_SECTION, including OMP_ARRAY_SECTION say in the lower-bound,
length and/or stride expressions of the valid OMP_ARRAY_SECTION.

For C++ that also means handling OMP_ARRAY_SECTION code in pt.c.

 

Re: [PATCH v2 08/11] Use OMP_ARRAY_SECTION instead of TREE_LIST in C++ FE

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 09:26:49AM -0700, Julian Brown wrote:
> This patch changes the representation of OMP array sections in the
> C++ front end to use the new OMP_ARRAY_SECTION tree code instead of a
> TREE_LIST.  This is important for "declare mapper" support, because the
> array section representation may stick around longer (in "declare mapper"
> definitions), and special-case handling TREE_LIST becomes necessary in
> more places, which starts to become unwieldy.
> 
> 2022-02-18  Julian Brown  
> 
> gcc/c-family/
>   * c-omp.cc (c_omp_split_clauses): Support OMP_ARRAY_SECTION.
> 
> gcc/cp/
>   * parser.cc (cp_parser_omp_var_list_no_open): Use OMP_ARRAY_SECTION
>   code instead of TREE_LIST to represent OpenMP array sections.
>   * pt.cc (tsubst_copy, tsubst_omp_clause_decl, tsubst_copy_and_build):
>   Add OMP_ARRAY_SECTION support.
>   * semantics.cc (handle_omp_array_sections_1, handle_omp_array_sections,
>   cp_oacc_check_attachments, finish_omp_clauses): Use OMP_ARRAY_SECTION
>   instead of TREE_LIST where appropriate.
>   * gimplify.cc (gimplify_expr): Ensure OMP_ARRAY_SECTION has been
>   processed out before gimplification.

THis is all a step towards the right direction, but we really do want to
transition from uses of TREE_LIST to represent array sections to
OMP_ARRAY_SECTION.  For some clauses that do allow lvalue expressions that
is a must, for the rest just a good cleanup even when the OMP_ARRAY_SECTION
are created instead of TREE_LIST during the explicit array section parsing
in OpenMP var list parsing.

Jakub

Re: [PATCH v2 09/11] OpenMP 5.0 "declare mapper" support for C++

[Jakub Jelinek via Fortran]

On Fri, Mar 18, 2022 at 09:26:50AM -0700, Julian Brown wrote:
> This patch implements OpenMP 5.0 "declare mapper" support for C++ --
> except for arrays of structs with mappers, which are TBD. I've taken cues
> from the existing "declare reduction" support where appropriate, though
> obviously the details of implementation differ somewhat (in particular,
> "declare mappers" must survive longer, until gimplification time).
> 
> Both named and unnamed (default) mappers are supported, and both
> explicitly-mapped structures and implicitly-mapped struct-typed variables
> used within an offload region are supported. For the latter, we need a
> way to communicate to the middle end which mapper (visible, in-scope) is
> the right one to use -- for that, we scan the target body in the front
> end for uses of structure (etc.) types, and create artificial "mapper
> binding" clauses to associate types with visible mappers. (It doesn't
> matter too much if we create these mapper bindings a bit over-eagerly,
> since they will only be used if needed later during gimplification.)
> 
> Another difficulty concerns the order in which an OpenMP offload region
> body's clauses are processed relative to its body: in order to add
> clauses for instantiated mappers, we need to have processed the body
> already in order to find which variables have been used, but at present
> the region's clauses are processed strictly before the body. So, this
> patch adds a second clause-processing step after gimplification of the
> body in order to add clauses resulting from instantiated mappers.
> 
> This version of the patch improves detection of explicitly-mapped struct
> accesses which inhibit implicitly-triggered user-defined mappers for a
> target region.

Will start with a general comment, from looking at the dumps it seems
handling the mappers in the FE right away for explicit mapping clauses
and attaching mapper binding clauses for types that are (or could
conservatively be, including from the recursive mappers themselves) be
used in the target body and letting gimplification find those var in detail
and use mapper binding clauses to actually expand it looks like the right
approach to me.  As I raised in an earlier patch, a big question is if we
should do map clause sorting on gimplify_scan_omp_clauses or
gimplify_adjust_omp_clauses or both...
The conservative discovery of what types we might need to create mapper
binding clauses for should be probably done only if
!processing_template_decl.

One question is though if DECL_OMP_DECLARE_MAPPER_P should be a magic
FUNCTION_DECL or a magic TREE_STATIC VAR_DECL or say CONST_DECLs.
The reason for the choice of FUNCTION_DECLs for UDRs is that they actually
contain code, but for UDMs we don't need any code, all we need is some
decl to which we can somehow attach list of clauses and a placeholder
decl used in them.  Perhaps a magic VAR_DECL or CONST_DECL would be
cheaper than a FUNCTION_DECL...

> @@ -32193,6 +32197,16 @@ cp_parser_late_parsing_for_member (cp_parser* 
> parser, tree member_function)
> finish_function (/*inline_p=*/true);
> cp_check_omp_declare_reduction (member_function);
>   }
> +  else if (DECL_OMP_DECLARE_MAPPER_P (member_function))
> + {
> +   parser->lexer->in_pragma = true;
> +   cp_parser_omp_declare_mapper_maplist (member_function, parser);
> +   finish_function (/*inline_p=*/true);
> +   cp_check_omp_declare_mapper (member_function);
> +   /* If this is a template class, this forces the body of the mapper
> +  to be instantiated.  */
> +   DECL_PRESERVE_P (member_function) = 1;

UDRs don't do this.  Why aren't the clauses instantiated when we actually
need such a template?

> @@ -39509,11 +39522,27 @@ cp_parser_omp_clause_map (cp_parser *parser, tree 
> list)
>  
>if (cp_lexer_peek_nth_token (parser->lexer, pos + 1)->type == 
> CPP_COMMA)
>   pos++;
> +  else if ((cp_lexer_peek_nth_token (parser->lexer, pos + 1)->type
> + == CPP_OPEN_PAREN)
> +&& ((cp_lexer_peek_nth_token (parser->lexer, pos + 2)->type
> + == CPP_NAME)
> +|| ((cp_lexer_peek_nth_token (parser->lexer, pos + 2)->type
> + == CPP_KEYWORD)
> +&& (cp_lexer_peek_nth_token (parser->lexer,
> + pos + 2)->keyword
> +== RID_DEFAULT)))
> +&& (cp_lexer_peek_nth_token (parser->lexer, pos + 3)->type
> +== CPP_CLOSE_PAREN)
> +&& (cp_lexer_peek_nth_token (parser->lexer, pos + 4)->type
> +== CPP_COMMA))

In this loop we don't need to be exact, all we want is find out
if the mapper-mdifier candidates are followed by : or not, the
actual parsing is done only later.  So, can't we just use
for CPP_OPEN_PAREN cp_parser_skip_balanced_tokens to move over
all the modifier's arguments?

Jakub

Re: [PATCH v2 09/11] OpenMP 5.0 "declare mapper" support for C++

[Jakub Jelinek via Fortran]

On Tue, May 24, 2022 at 04:48:13PM +0200, Jakub Jelinek wrote:
> > This version of the patch improves detection of explicitly-mapped struct
> > accesses which inhibit implicitly-triggered user-defined mappers for a
> > target region.
> 
> Will start with a general comment, from looking at the dumps it seems
> handling the mappers in the FE right away for explicit mapping clauses
> and attaching mapper binding clauses for types that are (or could
> conservatively be, including from the recursive mappers themselves) be
> used in the target body and letting gimplification find those var in detail
> and use mapper binding clauses to actually expand it looks like the right
> approach to me.  As I raised in an earlier patch, a big question is if we
> should do map clause sorting on gimplify_scan_omp_clauses or
> gimplify_adjust_omp_clauses or both...
> The conservative discovery of what types we might need to create mapper
> binding clauses for should be probably done only if
> !processing_template_decl.

Oh, and one very important thing I forgot to say yesterday.
With declare mapper but even the general mapping of aggregate is mapping
of all its members/elements individually, we are going to end up with huge
mapping lists.  We need to undo that at compile time whenever possible,
so if we after the declare mapper handling (from explicit or implicit
mappings) and sorting the mapping clauses see that we have say
struct S { int x, y, z[2], w; } s;
and we see map (tofrom: s.x, s.y, s.z[0], s.z[1], s.w) we should turn
that back into map (tofrom: s).  Basically optimize consecutive mappings
of the same kind to one that covers them together.
Then there is the question of padding bits, if there is reasonably small
padding in between mapped fields we could be mapping the padding too.

Jakub

Re: [Patch] OpenMP/Fortran: Add support for enter clause on declare target (was: [committed] openmp: Add support for enter clause on declare target)

[Jakub Jelinek via Fortran]

On Fri, May 27, 2022 at 04:52:17PM +0200, Tobias Burnus wrote:
> This patch adds the Fortran support to the just-committed C/C++ support for 
> the 'enter' clause.
> 
> The 'TO'/'ENTER' usage is first stored in a linked list – and
> then as attribute to the symbol. I am not sure how to handle it best.
> I did went for adding an ENTER_LIST but kept only using the single attribute.
> 
> Result: In one diagnostic, I use 'TO or ENTER clause'  in the other,
> I can properly use 'TO' or 'ENTER' clause.
> 
> This could be kept as is, but to save memory it would be possible
> to drop the ENTER_LIST – or, to improve diagnostic, we could try harder
> (e.g. by re-walking the list or by another gfc_attribute). Preferences?
> If not, I would go for the attached middle way.
> 
> Tobias
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> OpenMP/Fortran: Add support for enter clause on declare target
> 
> Fortran version to C/C++ commit r13-797-g0ccba4ed8571c18c7015413441e971
> 
> gcc/fortran/ChangeLog:
> 
>   * dump-parse-tree.cc (show_omp_clauses): Handle OMP_LIST_ENTER.
>   * gfortran.h: Add OMP_LIST_ENTER.
>   * openmp.cc (enum omp_mask2, OMP_DECLARE_TARGET_CLAUSES): Add
>   OMP_CLAUSE_ENTER.
>   (gfc_match_omp_clauses, gfc_match_omp_declare_target,
>   resolve_omp_clauses): Handle 'enter' clause.
> 
> libgomp/ChangeLog:
> 
>   * libgomp.texi (OpenMP 5.2): Mark 'enter' clause as supported.
>   * testsuite/libgomp.fortran/declare-target-1.f90: Extend to test
>   explicit 'to' and 'enter' clause.
>   * testsuite/libgomp.fortran/declare-target-2.f90: Update accordingly.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/declare-target-2.f90: Add 'enter' clause test.
>   * gfortran.dg/gomp/declare-target-4.f90: Likewise.

Mostly good, but see below.

> -  for (list = OMP_LIST_TO; list != OMP_LIST_NUM;
> -   list = (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM))
> +  for (list = OMP_LIST_ENTER; list != OMP_LIST_NUM;
> +   list = (list == OMP_LIST_ENTER
> +? OMP_LIST_TO
> +: (list == OMP_LIST_TO ? OMP_LIST_LINK : OMP_LIST_NUM)))
>  for (n = c->lists[list]; n; n = n->next)
>if (n->sym)
>   {
> @@ -4564,14 +4584,14 @@ gfc_match_omp_declare_target (void)
>  && n->sym->attr.omp_declare_target_link
>  && list != OMP_LIST_LINK)
>   gfc_error_now ("OMP DECLARE TARGET variable at %L previously "
> -"mentioned in LINK clause and later in TO clause",
> -&n->where);
> +"mentioned in LINK clause and later in %s clause",
> +&n->where, list == OMP_LIST_TO ? "TO" : "ENTER");
> else if (n->sym->attr.omp_declare_target
>  && !n->sym->attr.omp_declare_target_link
>  && list == OMP_LIST_LINK)
>   gfc_error_now ("OMP DECLARE TARGET variable at %L previously "
> -"mentioned in TO clause and later in LINK clause",
> -&n->where);
> +"mentioned in TO or ENTER clause and later in "
> +"LINK clause", &n->where);

The wording of the messages "previously mentioned in FOO and later in BAR 
clause"
makes not much sense to me, because we in the Fortran FE don't remember which
clause was specified earlier and which one was later.
The loop is walking first all enter clauses, then all to clauses, then all
link clauses.

Now, if we change the wording so that it complains that a variable is
"mentioned in both FOO and BAR clauses", we could avoid the TO or ENTER
stuff even without some O(n^2) complexity or extra bit somewhere simply
by walking the clauses in the order of TO, LINK, ENTER (or ENTER, LINK, TO)
clauses, wer could be exact.

Though, further thinking about it, this isn't just about the case
where it is on the same declare target, but also on multiple and in that
case previous/later makes sense.

As we don't remember if it was previously TO or ENTER, I'd just suggest:
1) simplify the 2 for cycles, with 3 lists it is too unreadable, so use
   something like:
  static const int to_enter_link_lists[]
= { OMP_LIST_TO, OMP_LIST_ENTER, OMP_LIST_LINK };
  for (int listn = 0; listn < ARRAY_SIZE (to_enter_link_lists)
  && (list = to_enter_link_lists[listn], true); ++listn)
2) move the
  else if (n->sym->mark)
gfc_error_now ("Variable at %L mentioned multiple times in "
   "clauses of the same OMP DECLARE TARGET directive",
   &n->where);
  diagnostics earlier, above
  else if (n->sym->attr.omp_declare_target
   && n->sym->attr.omp_declare_target_

Re: [Patch] OpenMP/Fortran: Add support for enter clause on declare target (was: [committed] openmp: Add support for enter clause on declare target)

[Jakub Jelinek via Fortran]

On Fri, May 27, 2022 at 05:20:08PM +0200, Jakub Jelinek wrote:
> 2) move the
>   else if (n->sym->mark)
> gfc_error_now ("Variable at %L mentioned multiple times in "
>"clauses of the same OMP DECLARE TARGET directive",
>&n->where);
>   diagnostics earlier, above
>   else if (n->sym->attr.omp_declare_target
>&& n->sym->attr.omp_declare_target_link
>&& list != OMP_LIST_LINK)
>   and adjust testsuite if needed

Note, that matches what the C/C++ FEs do, they also first check for clauses
on the same construct and only afterwards do the link vs. to/enter on
separate directives.

Here is an incremental patch I plan to commit after full testing for the
C/C++ FE wording.

2022-05-27  Jakub Jelinek  

gcc/c/
* c-parser.cc (c_parser_omp_declare_target): If OMP_CLAUSE_LINK was
seen first, use "%" or "%" depending on
OMP_CLAUSE_ENTER_TO of the current clause, otherwise use
"% or %" wording.
gcc/cp/
* parser.c (handle_omp_declare_target_clause): If OMP_CLAUSE_LINK was
seen first, use "%" or "%" depending on
OMP_CLAUSE_ENTER_TO of the current clause, otherwise use
"% or %" wording.
gcc/testsuite/
* c-c++-common/gomp/declare-target-2.c: Add further tests for mixing of
link and to/enter clauses on separate directives.

--- gcc/c/c-parser.cc.jj2022-05-26 23:22:30.312850583 +0200
+++ gcc/c/c-parser.cc   2022-05-27 17:28:59.120420300 +0200
@@ -22067,9 +22067,14 @@ c_parser_omp_declare_target (c_parser *p
id = get_identifier ("omp declare target");
   if (at2)
{
- error_at (OMP_CLAUSE_LOCATION (c),
-   "%qD specified both in declare target % and %"
-   " clauses", t);
+ if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_ENTER)
+   error_at (OMP_CLAUSE_LOCATION (c),
+ "%qD specified both in declare target % and %qs"
+ " clauses", t, OMP_CLAUSE_ENTER_TO (c) ? "to" : "enter");
+ else
+   error_at (OMP_CLAUSE_LOCATION (c),
+ "%qD specified both in declare target % and "
+ "% or % clauses", t);
  continue;
}
   if (!at1)
--- gcc/cp/parser.cc.jj 2022-05-26 23:22:30.306850645 +0200
+++ gcc/cp/parser.cc2022-05-27 17:31:19.086980582 +0200
@@ -45991,9 +45991,14 @@ handle_omp_declare_target_clause (tree c
 id = get_identifier ("omp declare target");
   if (at2)
 {
-  error_at (OMP_CLAUSE_LOCATION (c),
-   "%qD specified both in declare target % and %"
-   " clauses", t);
+  if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_ENTER)
+   error_at (OMP_CLAUSE_LOCATION (c),
+ "%qD specified both in declare target % and %qs"
+ " clauses", t, OMP_CLAUSE_ENTER_TO (c) ? "to" : "enter");
+  else
+   error_at (OMP_CLAUSE_LOCATION (c),
+ "%qD specified both in declare target % and "
+ "% or % clauses", t);
   return false;
 }
   if (!at1)
--- gcc/testsuite/c-c++-common/gomp/declare-target-2.c.jj   2022-05-27 
12:31:36.114087022 +0200
+++ gcc/testsuite/c-c++-common/gomp/declare-target-2.c  2022-05-27 
17:38:12.173731493 +0200
@@ -10,7 +10,22 @@ int b;
 #pragma omp declare target enter (b) link (b)  /* { dg-error "appears more 
than once on the same .declare target. directive" } */
 int c;
 #pragma omp declare target (c)
-#pragma omp declare target link (c)/* { dg-error "specified both 
in declare target" } */
+#pragma omp declare target link (c)/* { dg-error "specified both 
in declare target 'link' and 'to' or 'enter' clauses" } */
+int c2;
+#pragma omp declare target to (c2)
+#pragma omp declare target link (c2)   /* { dg-error "specified both 
in declare target 'link' and 'to' or 'enter' clauses" } */
+int c3;
+#pragma omp declare target enter (c3)
+#pragma omp declare target link (c3)   /* { dg-error "specified both 
in declare target 'link' and 'to' or 'enter' clauses" } */
+int c4;
+#pragma omp declare target link (c4)
+#pragma omp declare target (c4)/* { dg-error 
"specified both in declare target 'link' and 'enter' clauses" } */
+int c5;
+#pragma omp declare target link (c5)
+#pragma omp declare target to (c5) /* { dg-error "specified both 
in declare target 'link' and 'to' clauses" } */
+int c6;
+#pragma omp declare target link (c6)
+#pragma omp declare target enter (c6)  /* { dg-error "specified both 
in declare target 'link' and 'enter' clauses" } */
 int foo (void);
 #pragma omp declare target link (foo)  /* { dg-error "is not a 
variable in clause" } */
 struct S;

Jakub

Re: [Patch] OpenMP/Fortran: Add support for firstprivate and allocate clauses on scope construct

[Jakub Jelinek via Fortran]

On Fri, Jun 03, 2022 at 03:37:56PM +0200, Tobias Burnus wrote:
> Simple patch. Testcases based on the C/C++ commit.
> For allocate, I found an unrelated bug which prevented me from adding
> the associated testcase: https://gcc.gnu.org/PR105836
> 
> Tested on x86-64 (w/o offloading).
> OK for mainline?
> 
> Tobias
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> OpenMP/Fortran: Add support for firstprivate and allocate clauses on scope 
> construct
> 
> Fortran commit to C/C++/backend commit
> r13-862-gf38b20d68fade5a922b9f68c4c3841e653d1b83c
> 
> gcc/fortran/ChangeLog:
> 
>   * openmp.cc (OMP_SCOPE_CLAUSES): Add firstprivate and allocate.
> 
> libgomp/ChangeLog:
> 
>   * libgomp.texi (OpenMP 5.2): Mark scope w/ firstprivate/allocate as Y.
>   * testsuite/libgomp.fortran/scope-2.f90: New test.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/scope-5.f90: New test.
>   * gfortran.dg/gomp/scope-6.f90: New test.

Ok, thanks.

Jakub

Re: [Patch] OpenMP: Fortran - fix ancestor's requires reverse_offload check

[Jakub Jelinek via Fortran]

On Wed, Jun 08, 2022 at 09:54:07AM +0200, Tobias Burnus wrote:
> The OpenMP requires directive may only be placed in the specification part of
> a program unit (except it happens via the USE of a module).
> 
> But the target directive ancestor-requires-'reverse_offload' only checked
> the current namespace.
> 
> OK for mainline?
> 
> Tobias
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> OpenMP: Fortran - fix ancestor's requires reverse_offload check
> 
> gcc/fortran/
> 
>   * openmp.cc (gfc_match_omp_clauses): Check also parent namespace
>   for 'requires reverse_offload'.
> 
> gcc/testsuite/
> 
>   * gfortran.dg/gomp/target-device-ancestor-5.f90: New test.

LGTM, thanks.

Jakub

Re: [PATCH v2 01/11] OpenMP 5.0: Clause ordering for OpenMP 5.0 (topological sorting by base pointer)

[Jakub Jelinek via Fortran]

On Wed, Jun 08, 2022 at 04:00:39PM +0100, Julian Brown wrote:
> > I think big question is if we do want to do this map clause reordering
> > before processing the  omp target etc. clauses, or after (during
> > gimplify_adjust_omp_clauses, when clauses from the implicit mappings
> > are added too and especially with the declare mapper expansions),
> > or both before and after.
> 
> The existing code constrains us a bit here, unless we want to
> completely rewrite it!
> 
> We can only do sorting on clauses before gimplification, otherwise the
> "structural" matching of the parsed syntax of base pointers inside other
> clauses on the directive, etc. will certainly fail.
> 
> (Semi-relatedly, I asked this on the omp-lang mailing list:
> 
>   "When we have mappings that represent base pointers, and other
>   mappings that use those base pointers, the former must be ordered to
>   take place before the latter -- but should we determine that relation
>   purely syntactically? How about if we write e.g. "p->" on one vs.
>   "(*p)." on the other?"
> 
> but no reply...)
> 
> So, this is fine for sorting explicit mapping clauses. When planning
> the approach I've used for "declare mapper" support, I wrote this (in
> an internal email):
> 
> "At the moment, gimplifying OMP workshare regions proceeds in three
> phases:
> 
>  1. Clauses are processed (gimplify_scan_omp_clauses), creating
> records of mapped variables in a splay tree, with associated flags.
> 
>  2. The body of the workshare region is processed (gimplified),
> augmenting the same splay tree with information about variables
> which are used implicitly (and maybe also modifying the "explicit"
> mappings from the first step).
> 
>  3. The clauses are modified based on the results of the second stage
> (gimplify_adjust_omp_clauses). E.g. clauses are removed that refer
> to variables that aren't actually used in the region, or new
> clauses created for implicitly-referenced variables without mapping
> clauses on the construct.
> 
> The problem with this with regards to mappers is that the "expanded"
> mappers should undergo some of the processing we currently perform
> during phase 1 (struct sibling list handling, and so on), but we don't
> know which variables are implicitly referenced until phase 2.
> 
> [description of a plan that didn't work removed]
> 
> So the new plan is to do:
> 
> phase 1  (scan original clauses)
> phase 2  (scan workshare body)
> phase 1  (use variables from "2" to instantiate mappers, and process
>   new clauses only. Prepend new list to original clauses)
> phase 3  (as before)
> 
> I was concerned that this would upset the sorting code -- but I think
> actually, as long as implicitly-created clauses are inserted at the
> front of the clause list, there can't be a case where a pointer base is
> mapped after a use of that base. If that assumption turns out to be
> wrong, then things might get a little more complicated."
> 
> ...and so far, the plan seems to be working out. The assumption, to
> state it in other words, is that an implicitly-added map clause *cannot*
> have a dependency on an explicit map clause, in terms of relying on a
> base pointer in that explicit clause, by construction.

I don't think there is any need to add extra phases, but we can move
some code from gimplify_scan_omp_clauses to gimplify_adjust_omp_clauses.
What must be done in gimplify_scan_omp_clauses is stuff that will or
could affect the gimplification of the region's body, in that phase 2
we want to know say that some variable was privatized explicitly or
explicitly mapped or none of that, so we can based on that decide if we
should note implicit data sharing or implicit mapping etc.
But e.g. the sorting of the OMP_CLAUSE_MAP clauses is something that can
IMHO be deferred until we have all those clauses, probably it is done
in gimplify_scan_omp_clauses right now was just that the sorting at least
initially was only needed for struct mapping (map (tofrom: a.b, a.c, a.d.e, 
a.d.f))
and that could appear only explicitly, not implicitly, implicit mapping
would only map the whole var.
But declare mapper changes this substantially, declare mapper can add
similar mappings even from the implicit maps.
So, I think we should keep in phase 1 for OMP_CLAUSE_MAP only the stuff that
perhaps gimplifies some expressions used in those and puts records about
them into splay trees and sorting and ideally some kind of merging of
adjacent mappings can be done only when we have even the implicit
mappings all collected (so that would be after
  splay_tree_foreach (ctx->variables, gimplify_adjust_omp_clauses_1, &data);
finishes).

Jakub

Re: GSoC Blog Post 0 - GCCprefab build system

[Jakub Jelinek via Fortran]

On Fri, Jun 17, 2022 at 08:45:04PM +0200, Bernhard Reutner-Fischer via Gcc 
wrote:
> PS: we should rm https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=contrib/gcc_build

No.  gcc_build is used by maintainer-scripts/gcc_release, so by killing it
you'd make gcc unreleasable.

> It was not updated since the switch to git so, apparently, nobody uses it 
> anymore (sadly, seeing who authored it).

Jakub

[PATCH] fortran, libgfortran: Avoid using libquadmath for glibc 2.26+

[Jakub Jelinek via Fortran]

Hi!

As mentioned by Joseph in PR105101, glibc 2.26 or later has on x86
(both -m32/-m64), powerpc64le, ia64 and mips support for
*f128 math/complex APIs plus strtof128 and strfromf128, and these APIs allow
us to avoid libquadmath for Fortran purposes on these architectures,
replace *q math/complex APIs, strtof128 instead of strtoflt128 and,
while strfromf128 unfortunately isn't a perfect replacement to
quadmath_snprintf, it can be made to work.

The advantage of this is that when configured against such glibcs
(2.26 is now almost 5 years old), we can avoid linking against an extra shared
library and the math support in glibc is maintained better than libquadmath.

We need both a compiler change (so that for glibc 2.26+ it uses *f128 APIs
instead of *q) and library change.

The above mentioned problem with strfromf128 is that the strfrom* functions
are severely restricted versions of snprintf.  In libgfortran, we handle
!isfinite differently and just use snprintf/quadmath_snprintf for
%+-#.*{L,Q}{f,e} printing.
strfrom* doesn't allow +, -, # modifiers and it only supports .34 or
similar precision, not .* .  The L/Q etc. letters are omitted.
The + is there to force + sign at the start if it is positive.
Workaround in the patch is to add the + at the start manually for
!signbit (val).
The - (left alignment instead of right) I don't understand why we need it,
when minimum field width isn't specified (for strfrom* can't be specified),
no padding is ever added anywhere I believe.
The # is to force adding . - workaround is to search for first . or e or '\0'
character, if it is '\0', just append ., if it is e, insert . before e and
memmove the rest (which is just a few bytes, e, +/- and at most a few digits)
one byte later.
The .* case is handled by creating the format string for strfrom* by
snprintf into a temporary buffer.

So far lightly tested on x86_64-linux with glibc 2.35 (removed libgfortran
dirs, rebuilt stage3 f951 and make all-target-libgfortran + make
check-gfortran), ok for trunk if it passes full testing?

On powerpc64le-linux, guess we'll need to test 3 configurations, glibc < 2.26,
glibc 2.26 to 2.31 and glibc 2.32 and later.

2022-06-23  Jakub Jelinek  

gcc/fortran/
* gfortran.h (gfc_real_inf0: Add c__Float128 bitfield.
* trans-types.h (gfc_real16_is_float128): Update comment.
(gfc_real16_is__Float128): Declare.
* trans-types.cc (gfc_real16_is__Float128): Define.
(gfc_init_kinds): When building powerpc64le-linux libgfortran
on glibc 2.26 to 2.31, set gfc_real16_is__Float128 and
c__Float128 instead of gfc_real16_is_float128 and c_float128.
(gfc_build_real_type): Don't set c_long_double if c__Float128.
Set gfc_real16_is__Float128 and c__Float128 instead of
gfc_real16_is_float128 and c_float128 on glibc 2.26 or later.
(gfc_init_types): Handle c__Float128 like c_float128.
* trans-intrinsic.cc (builtin_decl_for_precision): Handle
gfc_real16_is__Float128 like gfc_real16_is_float128.
(gfc_builtin_decl_for_float_kind): Handle c__Float128 like c_float128.
(gfc_build_intrinsic_lib_fndecls): For gfc_real16_is__Float128
use *f128 APIs rather than *q APIs used for gfc_real16_is_float128.
(gfc_get_intrinsic_lib_fndecl): Likewise.
* trans-expr.cc (gfc_conv_power_op): Handle gfc_real16_is__Float128
like gfc_real16_is_float128.
libgfortran/
* configure.ac: Check for strtof128 and strfromf128.
Check for math and complex *f128 functions.  Set
have__Float128_libc_support to yes if *f128 support is around, for
--enable-libquadmath-support default to no rather than yes if
have__Float128_libc_support is yes.
* acinclude.m4 (LIBGFOR_CHECK_FLOAT128): If libquadmath support
isn't enabled and have__Float128_libc_support is yes, test
_Float128/_Complex _Float128 support and define and subst
HAVE__FLOAT128.
* Makefile.am (kinds.h): Pass @HAVE__FLOAT128@ as an extra
mk-kinds-h.sh argument.
* mk-kinds-h.sh: Accept 4th have__float128 argument, if it is yes,
use _Float128/_Complex _Float128 types insted of __float128 and
_Complex float __attribute__((mode(TC))), use f128 suffix instead
of q and define GFC_REAL_16_IS__FLOAT128 instead of
GFC_REAL_16_IS_FLOAT128.
* kinds-override.h: Add consistency check for GFC_REAL_16_IS__FLOAT128.
* libgfortran.h (GFC_REAL_16_INFINITY, GFC_REAL_16_QUIET_NAN): Define
for GFC_REAL_16_IS__FLOAT128.
* caf/single.c (convert_type): Use _Float128/_Complex _Float128 for
GFC_REAL_16_IS__FLOAT128.  For HAVE_GFC_REAL_10 when HAVE_GFC_REAL_16
isn't defined use _Complex long double instead of long double.
* ieee/issignaling_fallback.h (__issignalingf128): Handle
GFC_REAL_16_IS__FLOAT128.
(issignaling): Likewise.
* ieee/ieee_helper.c: Handle GFC_REAL_16_I