[PATCH] fortran, debug: Fix up DW_AT_rank [PR103315]
Hi! For DW_AT_rank we were emitting .uleb128 0x4# DW_AT_rank .byte 0x97# DW_OP_push_object_address .byte 0x23# DW_OP_plus_uconst .uleb128 0x1c .byte 0x6 # DW_OP_deref on 64-bit and .uleb128 0x4# DW_AT_rank .byte 0x97# DW_OP_push_object_address .byte 0x23# DW_OP_plus_uconst .uleb128 0x10 .byte 0x6 # DW_OP_deref on 32-bit. I think this is wrong, as dtype.rank field in the descriptor has unsigned char type, not pointer type nor pointer sized integral. E.g. if we have a REAL :: a(..) dummy argument, which is passed as a reference to the function descriptor, we want to evaluate a->dtype.rank. The above DWARF expressions perform *(uintptr_t *)(a + 0x1c) and *(uintptr_t *)(a + 0x10) respectively. The following patch changes those to: .uleb128 0x5# DW_AT_rank .byte 0x97# DW_OP_push_object_address .byte 0x23# DW_OP_plus_uconst .uleb128 0x1c .byte 0x94# DW_OP_deref_size .byte 0x1 and .uleb128 0x5# DW_AT_rank .byte 0x97# DW_OP_push_object_address .byte 0x23# DW_OP_plus_uconst .uleb128 0x10 .byte 0x94# DW_OP_deref_size .byte 0x1 which perform *(unsigned char *)(a + 0x1c) and *(unsigned char *)(a + 0x10) respectively. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2021-11-19 Jakub Jelinek PR debug/103315 * trans-types.c (gfc_get_array_descr_info): Use DW_OP_deref_size 1 instead of DW_OP_deref for DW_AT_rank. --- gcc/fortran/trans-types.c.jj2021-11-12 15:54:21.0 +0100 +++ gcc/fortran/trans-types.c 2021-11-18 15:13:45.131281198 +0100 @@ -3459,8 +3459,8 @@ gfc_get_array_descr_info (const_tree typ if (!integer_zerop (dtype_off)) t = fold_build_pointer_plus (t, rank_off); - t = build1 (NOP_EXPR, build_pointer_type (gfc_array_index_type), t); - t = build1 (INDIRECT_REF, gfc_array_index_type, t); + t = build1 (NOP_EXPR, build_pointer_type (TREE_TYPE (field)), t); + t = build1 (INDIRECT_REF, TREE_TYPE (field), t); info->rank = t; t = build0 (PLACEHOLDER_EXPR, TREE_TYPE (dim_off)); t = size_binop (MULT_EXPR, t, dim_size); Jakub
[PATCH, v2, OpenMP 5.0] Remove array section base-pointer mapping semantics, and other front-end adjustments (mainline trunk)
Hi Jakub, attached is a rebased version of this "OpenMP fixes/adjustments" patch. This version removes some of the (ort == C_ORT_OMP || ort == C_ORT_ACC) stuff that's not needed in handle_omp_array_sections_1 and [c_]finish_omp_clauses. Note that this is meant to be patched atop of the recent also posted C++ PR92120 v5 patch: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584602.html Again, tested without regressions (together with the PR92120 patch), awaiting review. Thanks, Chung-Lin (ChangeLog updated below) On 2021/5/25 9:36 PM, Chung-Lin Tang wrote: This patch largely implements three pieces of functionality: (1) Per discussion and clarification on the omp-lang mailing list, standards conforming behavior for mapping array sections should *NOT* also map the base-pointer, i.e for this code: struct S { int *ptr; ... }; struct S s; #pragma omp target enter data map(to: s.ptr[:100]) Currently we generate after gimplify: #pragma omp target enter data map(struct:s [len: 1]) map(alloc:s.ptr [len: 8]) \ map(to:*_1 [len: 400]) map(attach:s.ptr [bias: 0]) which is deemed incorrect. After this patch, the gimplify results are now adjusted to: #pragma omp target enter data map(to:*_1 [len: 400]) map(attach:s.ptr [bias: 0]) (the attach operation is still generated, and if s.ptr is already mapped prior, attachment will happen) The correct way of achieving the base-pointer-also-mapped behavior would be to use: #pragma omp target enter data map(to: s.ptr, s.ptr[:100]) This adjustment in behavior required a number of small adjustments here and there in gimplify, including to accomodate map sequences for C++ references. There is also a small Fortran front-end patch involved (hence CCing Tobias and fortran@). The new gimplify processing changed behavior in handling GOMP_MAP_ALWAYS_POINTER maps such that the libgomp.fortran/struct-elem-map-1.f90 regressed. It appeared that the Fortran FE was generating a GOMP_MAP_ALWAYS_POINTER for array types, which didn't seem quite correct, and the pre-patch behavior was removing this map anyways. I have a small change in trans-openmp.c:gfc_trans_omp_array_section to not generate the map in this case, and so far no bad test results. (2) The second part (though kind of related to the first above) are fixes in libgomp/target.c to not overwrite attached pointers when handling device<->host copies, mainly for the "always" case. This behavior is also noted in the 5.0 spec, but not yet properly coded before. (3) The third is a set of changes to the C/C++ front-ends to extend the allowed component access syntax in map clauses. This is actually mainly an effort to allow SPEC HPC to compile, so despite in the long term the entire map clause syntax parsing is probably going to be revamped, we're still adding this in for now. These changes are enabled for both OpenACC and OpenMP. 2021-11-19 Chung-Lin Tang gcc/c/ChangeLog: * c-parser.c (struct omp_dim): New struct type for use inside c_parser_omp_variable_list. (c_parser_omp_variable_list): Allow multiple levels of array and component accesses in array section base-pointer expression. (c_parser_omp_clause_to): Set 'allow_deref' to true in call to c_parser_omp_var_list_parens. (c_parser_omp_clause_from): Likewise. * c-typeck.c (handle_omp_array_sections_1): Extend allowed range of base-pointer expressions involving INDIRECT/MEM/ARRAY_REF and POINTER_PLUS_EXPR. (c_finish_omp_clauses): Extend allowed ranged of expressions involving INDIRECT/MEM/ARRAY_REF and POINTER_PLUS_EXPR. gcc/cp/ChangeLog: * parser.c (struct omp_dim): New struct type for use inside cp_parser_omp_var_list_no_open. (cp_parser_omp_var_list_no_open): Allow multiple levels of array and component accesses in array section base-pointer expression. (cp_parser_omp_all_clauses): Set 'allow_deref' to true in call to cp_parser_omp_var_list for to/from clauses. * semantics.c (handle_omp_array_sections_1): Extend allowed range of base-pointer expressions involving INDIRECT/MEM/ARRAY_REF and POINTER_PLUS_EXPR. (handle_omp_array_sections): Adjust pointer map generation of references. (finish_omp_clauses): Extend allowed ranged of expressions involving INDIRECT/MEM/ARRAY_REF and POINTER_PLUS_EXPR. gcc/fortran/ChangeLog: * trans-openmp.c (gfc_trans_omp_array_section): Do not generate GOMP_MAP_ALWAYS_POINTER map for main array maps of ARRAY_TYPE type. gcc/ChangeLog: * gimplify.c (extract_base_bit_offset): Add 'tree *offsetp' parameter, accomodate case where 'offset' return of get_inner_reference is non-NULL. (is_or_contains_p): Further robustify conditions. (omp_target_reorder_clauses): In alloc/to/from sorting phase, also move follow
Re: [PATCH, PR90030] Fortran OpenMP/OpenACC array mapping alignment fix
Ping. On 2021/11/4 4:23 PM, Chung-Lin Tang wrote: Hi Jakub, As Thomas reported and submitted a patch a while ago: https://gcc.gnu.org/pipermail/gcc-patches/2019-April/519932.html https://gcc.gnu.org/pipermail/gcc-patches/2019-May/522738.html There's an issue with the Fortran front-end when mapping arrays: when creating the data MEM_REF for the map clause, there's a convention of casting the referencing pointer to 'c_char *' by fold_convert (build_pointer_type (char_type_node), ptr). This causes the alignment passed to the libgomp runtime for array data hardwared to '1', and causes alignment errors on the offload target (not always showing up, but can trigger due to slight change of clause ordering) This patch is not exactly Thomas' patch from 2019, but does the same thing. The new libgomp tests are directly reused though. A lot of scan test adjustment is also included in this patch. Patch has been tested for no regressions for gfortran and libgomp, is this okay for trunk? Thanks, Chung-Lin Fortran: fix array alignment for OpenMP/OpenACC target mapping clauses [PR90030] The Fortran front-end is creating maps of array data with a type of pointer to char_type_node, which when eventually passed to libgomp during runtime, marks the passed array with an alignment of 1, which can cause mapping alignment errors on the offload target. This patch removes the related fold_convert(build_pointer_type (char_type_node)) calls in fortran/trans-openmp.c, and adds gcc_asserts to ensure pointer type. 2021-11-04 Chung-Lin Tang Thomas Schwinge PR fortran/90030 gcc/fortran/ChangeLog: * trans-openmp.c (gfc_omp_finish_clause): Remove fold_convert to pointer to char_type_node, add gcc_assert of POINTER_TYPE_P. (gfc_trans_omp_array_section): Likewise. (gfc_trans_omp_clauses): Likewise. gcc/testsuite/ChangeLog: * gfortran.dg/goacc/finalize-1.f: Adjust scan test. * gfortran.dg/gomp/affinity-clause-1.f90: Likewise. * gfortran.dg/gomp/affinity-clause-5.f90: Likewise. * gfortran.dg/gomp/defaultmap-4.f90: Likewise. * gfortran.dg/gomp/defaultmap-5.f90: Likewise. * gfortran.dg/gomp/defaultmap-6.f90: Likewise. * gfortran.dg/gomp/map-3.f90: Likewise. * gfortran.dg/gomp/pr78260-2.f90: Likewise. * gfortran.dg/gomp/pr78260-3.f90: Likewise. libgomp/ChangeLog: * testsuite/libgomp.oacc-fortran/pr90030.f90: New test. * testsuite/libgomp.fortran/pr90030.f90: New test.
Re: [RFC] User-visible changes for powerpc64-le-linux ABI changes
On Tue, Nov 16, 2021 at 08:51:03AM +0100, Thomas Koenig wrote: > If you could start working on the points above, that would be great. Just small completely untested step, which IMHO should ensure that on powerpc64le-*linux* (unless --with-long-double-64 configured) we build libgfortran by default with -mabi=ibmlongdouble (so that code using GFC_REAL_16 can use long double etc. and the kind 16 in filenames stands for IBM long double), but the *_r17.F90 and *_c17.F90 files are built with -mabi=ieeelongdouble (for Fortran we need to choose either one or another one but not both). *_r17*.c and *_c17*.c can be compiled with -mabi=ibmlongdouble because in C we can use __float128 (or __ieee128) or KF/KC mode to make it work with both kind 16 and "17" in the same TU. I bet the next step will be try to generate the generated m4 files in maintainer mode and start fixing what needs to be fixed. E.g. make sure that for the REAL_17/COMPLEX_17 cases we actually emit the libquadmath functions or __*ieee128 functions depending on whether glibc has support or not when using libm functions, I think there are some sources that read kind from function descriptors and that would be probably 16 rather than 17 even for the IEEE long double, etc. But that is as far as I can spend time on this right now. --- libgfortran/configure.ac.jj 2021-09-23 10:07:16.011181551 +0200 +++ libgfortran/configure.ac2021-11-19 15:07:29.505962930 +0100 @@ -145,6 +145,7 @@ AC_SUBST(CFLAGS) AM_PROG_CC_C_O # Add -Wall -fno-repack-arrays -fno-underscoring if we are using GCC. +have_real_17=no if test "x$GCC" = "xyes"; then AM_FCFLAGS="-I . -Wall -Werror -fimplicit-none -fno-repack-arrays -fno-underscoring" ## We like to use C11 and C99 routines when available. This makes @@ -154,8 +155,24 @@ if test "x$GCC" = "xyes"; then ## Compile the following tests with the same system header contents ## that we'll encounter when compiling our own source files. CFLAGS="-std=gnu11 $CFLAGS" -fi + case x$target in +powerpc64le*-linux*) + AC_PREPROC_IFELSE( +[AC_LANG_PROGRAM([[#if __SIZEOF_LONG_DOUBLE__ != 16 + #error long double is double + #endif]], + [[(void) 0;]])], +[AM_FCFLAGS="$AM_FCFLAGS -mabi=ibmlongdouble"; +AM_CFLAGS="$AM_CFLAGS -mabi=ibmlongdouble"; +CFLAGS="$CFLAGS -mabi=ibmlongdouble"; +have_real_17=yes]) + ;; +*) + ;; + esac +fi +AM_CONDITIONAL([HAVE_REAL_17], [test "x$have_real_17" != xno]) # Add CET specific flags if CET is enabled GCC_CET_FLAGS(CET_FLAGS) AM_FCFLAGS="$AM_FCFLAGS $CET_FLAGS" @@ -665,10 +682,10 @@ LIBGFOR_CHECK_CRLF # Check whether we support AVX extensions LIBGFOR_CHECK_AVX -# Check wether we support AVX2 extensions +# Check whether we support AVX2 extensions LIBGFOR_CHECK_AVX2 -# Check wether we support AVX512f extensions +# Check whether we support AVX512f extensions LIBGFOR_CHECK_AVX512F # Check for FMA3 extensions --- libgfortran/kinds-override.h.jj 2021-01-04 10:25:54.764053433 +0100 +++ libgfortran/kinds-override.h2021-11-19 14:28:05.799401414 +0100 @@ -44,3 +44,17 @@ see the files COPYING3 and COPYING.RUNTI # endif #endif +#if defined(__powerpc64__) \ +&& __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ \ +&& __SIZEOF_LONG_DOUBLE__ == 16 \ +&& defined(GFC_REAL_16_IS_LONG_DOUBLE) +typedef __float128 GFC_REAL_17; +typedef _Complex float __attribute__((mode(KC))) GFC_COMPLEX_17; +#define HAVE_GFC_REAL_17 +#define HAVE_GFC_COMPLEX_17 +#define GFC_REAL_17_HUGE 1.18973149535723176508575932662800702e4932q +#define GFC_REAL_17_LITERAL_SUFFIX q +#define GFC_REAL_17_LITERAL(X) (X ## q) +#define GFC_REAL_17_DIGITS 113 +#define GFC_REAL_17_RADIX 2 +#endif --- libgfortran/Makefile.am.jj 2021-09-08 09:55:29.002718817 +0200 +++ libgfortran/Makefile.am 2021-11-19 15:08:01.634506758 +0100 @@ -241,7 +241,8 @@ i_bessel_c= \ $(srcdir)/generated/bessel_r4.c \ $(srcdir)/generated/bessel_r8.c \ $(srcdir)/generated/bessel_r10.c \ -$(srcdir)/generated/bessel_r16.c +$(srcdir)/generated/bessel_r16.c \ +$(srcdir)/generated/bessel_r17.c i_count_c= \ $(srcdir)/generated/count_1_l.c \ @@ -281,10 +282,12 @@ $(srcdir)/generated/findloc0_r4.c \ $(srcdir)/generated/findloc0_r8.c \ $(srcdir)/generated/findloc0_r10.c \ $(srcdir)/generated/findloc0_r16.c \ +$(srcdir)/generated/findloc0_r17.c \ $(srcdir)/generated/findloc0_c4.c \ $(srcdir)/generated/findloc0_c8.c \ $(srcdir)/generated/findloc0_c10.c \ -$(srcdir)/generated/findloc0_c16.c +$(srcdir)/generated/findloc0_c16.c \ +$(srcdir)/generated/findloc0_c17.c i_findloc0s_c= \ $(srcdir)/generated/findloc0_s1.c \ @@ -300,10 +303,12 @@ $(srcdir)/generated/findloc1_r4.c \ $(srcdir)/generated/findloc1_r8.c \ $(srcdir)/generated/findloc1_r10.c \ $(srcdir)/generated/findloc1_r16.c \ +$(srcdir)/generated/findloc1_r17.c \ $(srcdir)/generated/findloc1_c4.c \ $(srcdir)/generated/findloc1_c8.c \ $(src
Re: [RFC] User-visible changes for powerpc64-le-linux ABI changes
On Mon, Nov 15, 2021 at 06:42:18PM -0500, Michael Meissner wrote: > > I assume we would to the development on a branch. My git fu > > is extremely weak, so I would appreciate if somebody did that > > for me. > > Sure, we can create an IBM vendor branch. It should not be an IBM branch, we should not mix that with community stuff. Instead it should be in devel/. I'll create it, but someone needs to come up with a good name. Something better than the "kind16" I would do ;-) Segher
Re: [RFC] User-visible changes for powerpc64-le-linux ABI changes
Hi Segher, On Mon, Nov 15, 2021 at 06:42:18PM -0500, Michael Meissner wrote: I assume we would to the development on a branch. My git fu is extremely weak, so I would appreciate if somebody did that for me. Sure, we can create an IBM vendor branch. It should not be an IBM branch, we should not mix that with community stuff. Instead it should be in devel/. I'll create it, but someone needs to come up with a good name. Something better than the "kind16" I would do ;-) power-ieee128, maybe? I do not think we need to adhere to the somewhat outdated "rs6000" name :-) Best regards Thomas
Re: [RFC] User-visible changes for powerpc64-le-linux ABI changes
On 11/19/21 1:09 PM, Thomas Koenig wrote: >> On Mon, Nov 15, 2021 at 06:42:18PM -0500, Michael Meissner wrote: >>> Sure, we can create an IBM vendor branch. >> >> It should not be an IBM branch, we should not mix that with community >> stuff. Instead it should be in devel/. Agreed, this would be better as a normal devel branch. >> I'll create it, but someone needs to come up with a good name. >> Something better than the "kind16" I would do ;-) > > power-ieee128, maybe? I do not think we need to adhere to the > somewhat outdated "rs6000" name :-) Or how about gfortran-ieee128? Peter
[PATCH] PR fortran/87851 - [9/10/11/12 Regression] Wrong return type for len_trim
Dear Fortranners, scalariziation of the elemental intrinsic LEN_TRIM was ICEing when the optional KIND argument was present. The cleanest solution is to use the infrastructure added by Mikael's fix for PR97896. In that case it is a 1-liner. :-) That fix is available on mainline and on 11-branch only, though. My suggestion is to fix the current PR only for the same branches, leaving the regression unfixed for older ones. Regtested on x86_64-pc-linux-gnu. OK for mainline and 11-branch? Thanks, Harald From f700c43fef4a239af25dd30dc22930b1bb1dbe95 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 19 Nov 2021 20:20:44 +0100 Subject: [PATCH] Fortran: fix scalarization for intrinsic LEN_TRIM with present KIND argument gcc/fortran/ChangeLog: PR fortran/87851 * trans-array.c (arg_evaluated_for_scalarization): Add LEN_TRIM to list of intrinsics for which an optional KIND argument needs to be removed before scalarization. gcc/testsuite/ChangeLog: PR fortran/87851 * gfortran.dg/len_trim.f90: New test. --- gcc/fortran/trans-array.c | 1 + gcc/testsuite/gfortran.dg/len_trim.f90 | 26 ++ 2 files changed, 27 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/len_trim.f90 diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c index 2090adf01e7..238b1b72385 100644 --- a/gcc/fortran/trans-array.c +++ b/gcc/fortran/trans-array.c @@ -11499,6 +11499,7 @@ arg_evaluated_for_scalarization (gfc_intrinsic_sym *function, switch (function->id) { case GFC_ISYM_INDEX: + case GFC_ISYM_LEN_TRIM: if (strcmp ("kind", gfc_dummy_arg_get_name (*dummy_arg)) == 0) return false; /* Fallthrough. */ diff --git a/gcc/testsuite/gfortran.dg/len_trim.f90 b/gcc/testsuite/gfortran.dg/len_trim.f90 new file mode 100644 index 000..63f803960d5 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/len_trim.f90 @@ -0,0 +1,26 @@ +! { dg-do compile } +! { dg-options "-O -Wall -Wconversion-extra -fdump-tree-original" } +! { dg-final { scan-tree-dump-not "_gfortran_stop_numeric" "original" } } +! PR fortran/87851 - return type for len_trim + +program main + implicit none + character(3), parameter :: a(1) = 'aa' + character(3):: b= "bb" + character(3):: c(1) = 'cc' + integer(4), parameter :: l4(1) = len_trim (a, kind=4) + integer(8), parameter :: l8(1) = len_trim (a, kind=8) + integer :: kk(1) = len_trim (a) + integer(4) :: mm(1) = len_trim (a, kind=4) + integer(8) :: nn(1) = len_trim (a, kind=8) + kk = len_trim (a) + mm = len_trim (a, kind=4) + nn = len_trim (a, kind=8) + kk = len_trim ([b]) + mm = len_trim ([b],kind=4) + nn = len_trim ([b],kind=8) + kk = len_trim (c) + mm = len_trim (c, kind=4) + nn = len_trim (c, kind=8) + if (any (l4 /= 2_4) .or. any (l8 /= 2_8)) stop 1 +end program main -- 2.26.2
Re: [RFC] User-visible changes for powerpc64-le-linux ABI changes
On Fri, Nov 19, 2021 at 01:36:33PM -0600, Peter Bergner wrote: > On 11/19/21 1:09 PM, Thomas Koenig wrote: > >> On Mon, Nov 15, 2021 at 06:42:18PM -0500, Michael Meissner wrote: > >>> Sure, we can create an IBM vendor branch. > >> > >> It should not be an IBM branch, we should not mix that with community > >> stuff. Instead it should be in devel/. > > Agreed, this would be better as a normal devel branch. > > > >> I'll create it, but someone needs to come up with a good name. > >> Something better than the "kind16" I would do ;-) > > > > power-ieee128, maybe? I do not think we need to adhere to the > > somewhat outdated "rs6000" name :-) > > Or how about gfortran-ieee128? I have created devel/power-ieee128. Descriptive names are good, but so are shorter names. Segher