Re: [PATCH 5/5] Fortran: Re-enable 128-bit integers for AMD GCN
On 18.06.21 16:20, Julian Brown wrote:
This patch reverts the part of Tobias's patch for PR target/96306 that
disables 128-bit integer support for AMD GCN.
OK for mainline (assuming the previous patches are in first)?
Well, as the only reason for that patch was to avoid tons of fails/ICE due
to incomplete TI support, I think it is fine (obvious) that this band aid
can be removed when complete/mostly complete TI mode
(int128_t/integer kind=16) is now available.
Besides, as remarked on IRC, as this is target specific, I think you
can also approve it yourself as GCN maintainer.
But for completeness: OK from my/Fortran's side.
And thanks for the patches!
Tobias
2021-06-18 Julian Brown
libgfortran/
PR target/96306
* configure.ac: Remove stanza that removes KIND=16 integers for AMD GCN.
* configure: Regenerate.
---
libgfortran/configure| 22 --
libgfortran/configure.ac | 4
2 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/libgfortran/configure b/libgfortran/configure
index f3634389cf8..886216f69d4 100755
--- a/libgfortran/configure
+++ b/libgfortran/configure
@@ -6017,7 +6017,7 @@ case "$host" in
case "$enable_cet" in
auto)
# Check if target supports multi-byte NOPs
- # and if assembler supports CET insn.
+ # and if compiler and assembler support CET insn.
cet_save_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS -fcf-protection"
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -6216,10 +6216,6 @@ fi
LIBGOMP_CHECKED_INT_KINDS="1 2 4 8 16"
LIBGOMP_CHECKED_REAL_KINDS="4 8 10 16"
-if test "x${target_cpu}" = xamdgcn; then
- # amdgcn only has limited support for __int128.
- LIBGOMP_CHECKED_INT_KINDS="1 2 4 8"
-fi
@@ -12731,7 +12727,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 12744 "configure"
+#line 12730 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -12837,7 +12833,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 12850 "configure"
+#line 12836 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -15532,16 +15528,6 @@ freebsd* | dragonfly*)
esac
;;
-gnu*)
- version_type=linux
- need_lib_prefix=no
- need_version=no
- library_names_spec='${libname}${release}${shared_ext}$versuffix
${libname}${release}${shared_ext}${major} ${libname}${shared_ext}'
- soname_spec='${libname}${release}${shared_ext}$major'
- shlibpath_var=LD_LIBRARY_PATH
- hardcode_into_libs=yes
- ;;
-
haiku*)
version_type=linux
need_lib_prefix=no
@@ -15663,7 +15649,7 @@ linux*oldld* | linux*aout* | linux*coff*)
# project, but have not yet been accepted: they are GCC-local changes
# for the time being. (See
# https://lists.gnu.org/archive/html/libtool-patches/2018-05/msg0.html)
-linux* | k*bsd*-gnu | kopensolaris*-gnu | uclinuxfdpiceabi)
+linux* | k*bsd*-gnu | kopensolaris*-gnu | gnu* | uclinuxfdpiceabi)
version_type=linux
need_lib_prefix=no
need_version=no
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index 8961e314d82..523eb24bca1 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -222,10 +222,6 @@ AM_CONDITIONAL(LIBGFOR_MINIMAL, [test "x${target_cpu}" =
xnvptx])
LIBGOMP_CHECKED_INT_KINDS="1 2 4 8 16"
LIBGOMP_CHECKED_REAL_KINDS="4 8 10 16"
-if test "x${target_cpu}" = xamdgcn; then
- # amdgcn only has limited support for __int128.
- LIBGOMP_CHECKED_INT_KINDS="1 2 4 8"
-fi
AC_SUBST(LIBGOMP_CHECKED_INT_KINDS)
AC_SUBST(LIBGOMP_CHECKED_REAL_KINDS)
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
Re: [patch] Fortran: fix sm computation in CFI_allocate [PR93524]
On 21.06.21 08:05, Sandra Loosemore wrote: I ran into this bug in CFI_allocate while testing something else and then realized there was already a PR open for it. It seems like an easy fix, and I've used Tobias's test case from the issue more or less verbatim. There were some other bugs added on to this issue but I think they have all been fixed already except for this one. OK to check in? OK – but see some comments below. libgfortran/ PR fortran/93524 * runtime/ISO_Fortran_binding.c (CFI_allocate): Fix sm computation. gcc/testsuite/ PR fortran/93524 * gfortran.dg/pr93524.c, gfortran.dg/pr93524.f90: New. It is new to me that we use this syntax. I think you want to have one line per file, each starting with "*" +++ b/gcc/testsuite/gfortran.dg/pr93524.c @@ -0,0 +1,33 @@ +/* Test the fix for PR93524, in which CFI_allocate was computing + sm incorrectly for dimensions > 2. */ + +#include // For size_t +#include I keep making this mistake myself: The last line works if you use the installed compiler for testing; if you run the testsuite via the build directory, it will either fail or take the wrong version of the file (the one under /usr/include). Solution: Use #include "../../../libgfortran/ISO_Fortran_binding.h" as we do in the other tests which use that file. +++ b/gcc/testsuite/gfortran.dg/pr93524.f90 ... +! Test the fix for PR93524. The main program is in pr93524.c. + +subroutine my_fortran_sub_1 (A) bind(C) + real :: A(:, :,:) + print *, 'Lower bounds: ', lbound(A) ! Lower bounds:111 + print *, 'Upper bounds: ', ubound(A) ! Upper bounds: 2168 +end +subroutine my_fortran_sub_2 (A) bind(C) + real, ALLOCATABLE :: A(:, :,:) + print *, 'Lower bounds: ', lbound(A) + print *, 'Upper bounds: ', ubound(A) I think the 'print' should be replaced (or commented + augmented) by 'if (any (lbound(A) /= 1) stop 1'; 'if (any (ubound(A) /= [21,6,8])) stop 2' etc. Actually, it probably does not work for the second function due to PR92189 (lbounds are wrong). If so, you could use 'if (any (shape(A) /= [21,6,8])) stop 4' instead. Can you also add 'if (.not. is_contiguous (A)) stop 3' to both functions? That issue was mentioned in the PR and is probably fixed by your change. Otherwise, it looks fine :-) Thanks for the patch. Tobias - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
Re: [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514
Hi Harald,
sorry for being way behind my review duties :-(
On 10.06.21 20:52, Harald Anlauf via Fortran wrote:
+static bool
+substring_has_constant_len (gfc_expr *e)
+{
+ ptrdiff_t istart, iend;
+ size_t length;
+ bool equal_length = false;
+
+ if (e->ts.type != BT_CHARACTER
+ || !e->ref
+ || e->ref->type != REF_SUBSTRING
Is there a reason why you do not handle:
type t
character(len=5) :: str1
character(len=:), allocatable :: str2
end type
type(t) :: x
allocate(x%str2, source="abd")
if (len (x%str)) /= 1) ...
if (len (x%str2(1:2) /= 2) ...
etc.
Namely: Search the last_ref = expr->ref->next->next ...?
and then check that lastref?
* * *
Slightly unrelated: I think the following does not violate
F2018's R916 / C923 – but is rejected, namely:
R916 type-param-inquiry is designator % type-param-name
the latter is 'len' or 'kind' for intrinsic types. And:
R901 designator is ...
or substring
But
character(len=5) :: str
print *, str(1:3)%len
end
fails with
2 | print *, str(1:3)%len
| 1
Error: Syntax error in PRINT statement at (1)
Assuming you don't want to handle it, can you open a new PR?
Thanks!
* * *
That's in so far related to your patch as last_ref would
then be the last ref before ref->next == NULL or
before ref->next->type == REF_INQUIRY
+ istart = gfc_mpz_get_hwi (e->ref->u.ss.start->value.integer);
+ iend = gfc_mpz_get_hwi (e->ref->u.ss.end->value.integer);
+ length = gfc_mpz_get_hwi (e->ref->u.ss.length->length->value.integer);
+
+ if (istart <= iend)
+{
+ if (istart < 1)
+ {
+ gfc_error ("Substring start index (%ld) at %L below 1",
+ (long) istart, &e->ref->u.ss.start->where);
As mentioned by Bernhard, you could use HOST_WIDE_INT_PRINT_DEC.
(It probably only matters on Windows which uses long == int = 32bit for
strings longer than INT_MAX.)
Thanks,
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
Re: [Ping^2, Patch, Fortran] PR100337 Should be able to pass non-present optional arguments to CO_BROADCAST
Any reason that you did not put it under gfortran.dg/coarray/ such that it is also run with -fcoarray=lib (-lcaf_single)? I know that the issue only exists for single, but it also makes sense to check that libcaf_single works In that sense, I wonder whether also the other CO_* should be checked in the testsuite as they are handled differently in libcaf_... (but identical with -fcoarray=single). Except for those two nits, it LGTM. Thanks! Tobias PS: The function is used by case GFC_ISYM_CO_BROADCAST: case GFC_ISYM_CO_MIN: case GFC_ISYM_CO_MAX: case GFC_ISYM_CO_REDUCE: case GFC_ISYM_CO_SUM: and, with -fcoarray=single, errmsg is not touched as stat is (unconditionally) 0 (success).. On 19.06.21 13:23, Andre Vehreschild via Fortran wrote: PING! On Fri, 4 Jun 2021 18:05:18 +0200 Andre Vehreschild wrote: Ping! On Fri, 21 May 2021 15:33:11 +0200 Andre Vehreschild wrote: Hi, the attached patch fixes an issue when calling CO_BROADCAST in -fcoarray=single mode, where the optional but non-present (in the calling scope) stat variable was assigned to before checking for it being not present. Regtests fine on x86-64-linux/f33. Ok for trunk? Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
Re: [Patch, fortran V3] PR fortran/100683 - Array initialization refuses valid
Hi José, On 17.06.21 21:34, José Rui Faustino de Sousa via Gcc-patches wrote: Update to a proposed patch to: PR100683 - Array initialization refuses valid due to more errors being found... Patch tested only on x86_64-pc-linux-gnu. LGTM – sorry for the very belated review. Add call to simplify expression before parsing Nit: I think you mean resolving/processing/expanding/checking – as gfc_resolve_expr comes after the actual parsing. *and* check *appropriately* if the expression is still an array after simplification. * * * I have to admit that I got a bit lost with your patches. Are there still outstanding patches? I also recall approving a patch quite some time ago which was then not committed for a long time. (I have not checked whether it was committed by now.) Thus: Do you have a list of patches pending review? Secondly, I assume you can commit or do you have commit issues? Tobias Fortran: Fix bogus error gcc/fortran/ChangeLog: PR fortran/100683 * resolve.c (gfc_resolve_expr): Add call to gfc_simplify_expr. gcc/testsuite/ChangeLog: PR fortran/100683 * gfortran.dg/pr87993.f90: increased test coverage. * gfortran.dg/PR100683.f90: New test. - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
Re: [Patch, fortran V2] PR fortran/93308/93963/94327/94331/97046 problems raised by descriptor handling
Hi José, (in principle, I'd like to have the libgfortran function moved to the compiler proper to avoid some issues, but that's admittedly a task independent of your work.) On 15.06.21 01:09, José Rui Faustino de Sousa via Fortran wrote: Update to a proposed patch to: Bug 93308 - bind(c) subroutine changes lower bound of array argument in caller Bug 93963 - Select rank mishandling allocatable and pointer arguments with bind(c) Bug 94327 - Bind(c) argument attributes are incorrectly set Bug 94331 - Bind(C) corrupts array descriptors Bug 97046 - Bad interaction between lbound/ubound, allocatable arrays and bind(C) subroutine with dimension(..) parameter ... Patch tested only on x86_64-pc-linux-gnu. Fix attribute handling, which reflect a prior intermediate version of the Fortran standard. LGTM – except for one minor nit. In trans-expr.c's gfc_conv_gfc_desc_to_cfi_desc: /* Transfer values back to gfc descriptor. */ + if (cfi_attribute != 2 + && !fsym->attr.value + && fsym->attr.intent != INTENT_IN) Can you add after the '2' the string ' /* CFI_attribute_other. */' to make the number less magic. Thanks, Tobias CFI descriptors, in most cases, should not be copied out has they can corrupt the Fortran descriptor. Bounds will vary and the original Fortran bounds are definitively lost on conversion. Thank you very much. Best regards, José Rui Fortran: Fix attributtes and bounds in ISO_Fortran_binding. gcc/fortran/ChangeLog: PR fortran/93308 PR fortran/93963 PR fortran/94327 PR fortran/94331 PR fortran/97046 * trans-decl.c (convert_CFI_desc): Only copy out the descriptor if necessary. * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Updated attribute handling which reflect a previous intermediate version of the standard. Only copy out the descriptor if necessary. libgfortran/ChangeLog: PR fortran/93308 PR fortran/93963 PR fortran/94327 PR fortran/94331 PR fortran/97046 * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Add code to verify the descriptor. Correct bounds calculation. (gfc_desc_to_cfi_desc): Add code to verify the descriptor. gcc/testsuite/ChangeLog: PR fortran/93308 PR fortran/93963 PR fortran/94327 PR fortran/94331 PR fortran/97046 * gfortran.dg/ISO_Fortran_binding_1.f90: Add pointer attribute, this test is still erroneous but now it compiles. * gfortran.dg/bind_c_array_params_2.f90: Update regex to match code changes. * gfortran.dg/PR93308.f90: New test. * gfortran.dg/PR93963.f90: New test. * gfortran.dg/PR94327.c: New test. * gfortran.dg/PR94327.f90: New test. * gfortran.dg/PR94331.c: New test. * gfortran.dg/PR94331.f90: New test. * gfortran.dg/PR97046.f90: New test. - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
Re: [Patch, fortran V3] PR fortran/100683 - Array initialization refuses valid
Hi Tobias, On 21/06/21 12:37, Tobias Burnus wrote: Thus: Do you have a list of patches pending review? > https://gcc.gnu.org/pipermail/fortran/2021-April/055924.html https://gcc.gnu.org/pipermail/fortran/2021-April/055933.html https://gcc.gnu.org/pipermail/fortran/2021-June/056168.html https://gcc.gnu.org/pipermail/fortran/2021-June/056167.html https://gcc.gnu.org/pipermail/fortran/2021-June/056163.html https://gcc.gnu.org/pipermail/fortran/2021-June/056162.html https://gcc.gnu.org/pipermail/fortran/2021-June/056155.html https://gcc.gnu.org/pipermail/fortran/2021-June/056154.html https://gcc.gnu.org/pipermail/fortran/2021-June/056152.html https://gcc.gnu.org/pipermail/fortran/2021-June/056159.html https://gcc.gnu.org/pipermail/fortran/2021-April/055982.html https://gcc.gnu.org/pipermail/fortran/2021-April/055949.html https://gcc.gnu.org/pipermail/fortran/2021-April/055946.html https://gcc.gnu.org/pipermail/fortran/2021-April/055934.html https://gcc.gnu.org/pipermail/fortran/2021-June/056169.html https://gcc.gnu.org/pipermail/fortran/2021-April/055921.html I am not 100% sure this is all of them but it should be most. Secondly, I assume you can commit or do you have commit issues? Up to now there were no problems. Best regards, José Rui
Re: [Patch, fortran V2] PR fortran/93308/93963/94327/94331/97046 problems raised by descriptor handling
On 21/06/21 13:46, Tobias Burnus wrote: Hi José, (in principle, I'd like to have the libgfortran function moved to the compiler proper to avoid some issues, but that's admittedly a task independent of your work.) cfi_desc_to_gfc_desc and gfc_desc_to_cfi_desc from ISO_c_binding.c, right? Since fixing: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100917 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100910 would very likely require passing an additional "kind" parameter (and future descriptor unification) that would be a good idea. I had a patch to do this, passing the kind value, but AFAIR there were issues with kind values for C_PTR and C_FUNPTR (and I didn't want to mess with the ABI also in one go)... But I might have fixed that somewhere else afterwards... So, I could look further into that. Were would you like them placed? LGTM – except for one minor nit. In trans-expr.c's gfc_conv_gfc_desc_to_cfi_desc: /* Transfer values back to gfc descriptor. */ + if (cfi_attribute != 2 + && !fsym->attr.value + && fsym->attr.intent != INTENT_IN) Can you add after the '2' the string ' /* CFI_attribute_other. */' to make the number less magic. Yes... I had the same idea... :-) But all those constants are defined in "ISO_Fortran_binding.h"... And moving all those definitions would be a major change... So I left it as it was... What do you suggest I do? Best regards, José Rui
Re: [Patch, fortran V2] PR fortran/93308/93963/94327/94331/97046 problems raised by descriptor handling
Hi José,
On 21.06.21 17:51, José Rui Faustino de Sousa via Fortran wrote:
On 21/06/21 13:46, Tobias Burnus wrote:
(in principle, I'd like to have the libgfortran function moved to the
compiler proper to avoid some issues, but that's admittedly a task
independent of your work.)
cfi_desc_to_gfc_desc and gfc_desc_to_cfi_desc from ISO_c_binding.c,
right?
Yes.
So, I could look further into that. Were would you like them placed?
Well, as said: directly into the compiler where currently the call to
libgomp is.
LGTM – except for one minor nit.
Found a second tiny nit:
+ if (GFC_DESCRIPTOR_DATA (d))
+for (n = 0; n < GFC_DESCRIPTOR_RANK (d); n++)
+ {
+ CFI_index_t lb = 1;
+
+ if (s->attribute != CFI_attribute_other)
There is tailing whitespace in the otherwise empty line.
In trans-expr.c's gfc_conv_gfc_desc_to_cfi_desc:
/* Transfer values back to gfc descriptor. */
+ if (cfi_attribute != 2
+ && !fsym->attr.value
+ && fsym->attr.intent != INTENT_IN)
Can you add after the '2' the string ' /* CFI_attribute_other. */'
to make the number less magic.
Yes... I had the same idea... :-) But all those constants are defined
in "ISO_Fortran_binding.h"... And moving all those definitions would
be a major change... So I left it as it was...
Well, I am currently only asking to add a comment after the "2;".
This fixing those two nits (removing tailing whitespace + adding a
comment) and is be trivial.
* * *
However, in the long run, I think we should put it into either a
separate file, which is included into ISO_Fortran_binding.h and the
proper compiler (and installed alongside ISO_Fortran_binding.h) - or
just in libgfortran.h and adding some check/(static)assert that it
matches to the value in ISO_Fortran_binding.h.
Or, possibly, we could also include ISO_Fortran_binding.h when building
the compiler itself, possibly adding some '#ifdef' code to disable parts
we do not want when we do #include. it.
(We already have '#include "libgfortran.h"' in gcc/fortran/gfortran.h.)
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank
Thürauf
Re: [Patch, fortran V2] PR fortran/93308/93963/94327/94331/97046 problems raised by descriptor handling
Hi Tobias, On 21/06/21 16:46, Tobias Burnus wrote: Well, as said: directly into the compiler where currently the call to libgomp is. > I don't think I understand were you mean. You don't mean the includes in "f95-lang.c" do you? Best regards, José Rui
Re: [Patch, fortran V3] PR fortran/100683 - Array initialization refuses valid
Hi Jose and Tobias, I am glad that you produced the list of patches waiting for approval. I have been out of action following a house move and will likely not be doing any reviewing or contributing for another month or so. As soon as I am ready, I will make use of this list to check out what has not been reviewed and will get on with helping out. Thanks for your patience. Paul On Mon, 21 Jun 2021 at 16:23, José Rui Faustino de Sousa via Fortran < fortran@gcc.gnu.org> wrote: > Hi Tobias, > > On 21/06/21 12:37, Tobias Burnus wrote: > > Thus: Do you have a list of patches pending review? > > > > https://gcc.gnu.org/pipermail/fortran/2021-April/055924.html > > https://gcc.gnu.org/pipermail/fortran/2021-April/055933.html > > https://gcc.gnu.org/pipermail/fortran/2021-June/056168.html > > https://gcc.gnu.org/pipermail/fortran/2021-June/056167.html > > https://gcc.gnu.org/pipermail/fortran/2021-June/056163.html > > https://gcc.gnu.org/pipermail/fortran/2021-June/056162.html > > https://gcc.gnu.org/pipermail/fortran/2021-June/056155.html > > https://gcc.gnu.org/pipermail/fortran/2021-June/056154.html > > https://gcc.gnu.org/pipermail/fortran/2021-June/056152.html > > https://gcc.gnu.org/pipermail/fortran/2021-June/056159.html > > https://gcc.gnu.org/pipermail/fortran/2021-April/055982.html > > https://gcc.gnu.org/pipermail/fortran/2021-April/055949.html > > https://gcc.gnu.org/pipermail/fortran/2021-April/055946.html > > https://gcc.gnu.org/pipermail/fortran/2021-April/055934.html > > https://gcc.gnu.org/pipermail/fortran/2021-June/056169.html > > https://gcc.gnu.org/pipermail/fortran/2021-April/055921.html > > I am not 100% sure this is all of them but it should be most. > > > Secondly, I assume > > you can commit or do you have commit issues? > > > > Up to now there were no problems. > > Best regards, > José Rui > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran V2] PR fortran/93308/93963/94327/94331/97046 problems raised by descriptor handling
Hi José, On 21.06.21 19:52, José Rui Faustino de Sousa wrote: On 21/06/21 16:46, Tobias Burnus wrote: Well, as said: directly into the compiler where currently the call to libgomp is. (should be libgfortran) I meant converting the operation done by the libgfortran/runtime/ISO_Fortran_binding.c functions * cfi_desc_to_gfc_desc and *gfc_desc_to_cfi_desc into tree code, generated in place by the current callers * gfor_fndecl_gfc_to_cfi (in trans-decl.c) * gfc_conv_gfc_desc_to_cfi_desc (in trans-expr.c) And then effectively retiring those functions (except for old code which still calls them). * * * However, that's independent from the patch you had submitted and which is fine except for the two tiny nits. Tobias - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
Re: [patch v2] Fortran: fix sm computation in CFI_allocate [PR93524]
On 6/21/21 5:42 AM, Tobias Burnus wrote:
On 21.06.21 08:05, Sandra Loosemore wrote:
I ran into this bug in CFI_allocate while testing something else and
then realized there was already a PR open for it. It seems like an
easy fix, and I've used Tobias's test case from the issue more or less
verbatim.
There were some other bugs added on to this issue but I think they
have all been fixed already except for this one.
OK to check in?
OK – but see some comments below.
Revised patch attached. How's this one?
-Sandra
commit 323fda07729fa0b0f2d1f8b4269db874280ac318
Author: Sandra Loosemore
Date: Mon Jun 21 13:25:55 2021 -0700
Fortran: fix sm computation in CFI_allocate [PR93524]
This patch fixes a bug in setting the step multiplier field in the
C descriptor for array dimensions > 2.
2021-06-21 Sandra Loosemore
Tobias Burnus
libgfortran/
PR fortran/93524
* runtime/ISO_Fortran_binding.c (CFI_allocate): Fix
sm computation.
gcc/testsuite/
PR fortran/93524
* gfortran.dg/pr93524.c: New.
* gfortran.dg/pr93524.f90: New.
diff --git a/gcc/testsuite/gfortran.dg/pr93524.c b/gcc/testsuite/gfortran.dg/pr93524.c
new file mode 100644
index 000..24e5e09
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr93524.c
@@ -0,0 +1,33 @@
+/* Test the fix for PR93524, in which CFI_allocate was computing
+ sm incorrectly for dimensions > 2. */
+
+#include // For size_t
+#include "../../../libgfortran/ISO_Fortran_binding.h"
+
+void my_fortran_sub_1 (CFI_cdesc_t *dv);
+void my_fortran_sub_2 (CFI_cdesc_t *dv);
+
+int main ()
+{
+ CFI_CDESC_T (3) a;
+ CFI_cdesc_t *dv = (CFI_cdesc_t *) &a;
+ // dv, base_addr, attribute,type, elem_len, rank, extents
+ CFI_establish (dv, NULL, CFI_attribute_allocatable, CFI_type_float, 0, 3, NULL);
+
+ if (dv->base_addr != NULL)
+return 1; // shall not be allocated
+
+ CFI_index_t lower_bounds[] = {-10, 0, 3};
+ CFI_index_t upper_bounds[] = {10, 5, 10};
+ size_t elem_len = 0; // only needed for strings
+ if (CFI_SUCCESS != CFI_allocate (dv, lower_bounds, upper_bounds, elem_len))
+return 2;
+
+ if (!CFI_is_contiguous (dv))
+return 2; // allocatables shall be contiguous,unless a strided section is used
+
+ my_fortran_sub_1 (dv);
+ my_fortran_sub_2 (dv);
+ CFI_deallocate (dv);
+ return 0;
+}
diff --git a/gcc/testsuite/gfortran.dg/pr93524.f90 b/gcc/testsuite/gfortran.dg/pr93524.f90
new file mode 100644
index 000..0cebc8f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr93524.f90
@@ -0,0 +1,17 @@
+! { dg-additional-sources pr93524.c }
+! { dg-do run }
+!
+! Test the fix for PR93524. The main program is in pr93524.c.
+
+subroutine my_fortran_sub_1 (A) bind(C)
+ real :: A(:, :, :)
+ if (any (lbound(A) /= 1)) stop 1
+ if (any (ubound(A) /= [21,6,8])) stop 2
+ if (.not. is_contiguous (A)) stop 3
+end
+subroutine my_fortran_sub_2 (A) bind(C)
+ real, ALLOCATABLE :: A(:, :, :)
+ if (any (lbound(A) /= [-10,0,3])) stop 1
+ if (any (ubound(A) /= [10,5,10])) stop 2
+ if (.not. is_contiguous (A)) stop 3
+end subroutine my_fortran_sub_2
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c b/libgfortran/runtime/ISO_Fortran_binding.c
index 20833ad..0978832 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -254,10 +254,7 @@ CFI_allocate (CFI_cdesc_t *dv, const CFI_index_t lower_bounds[],
{
dv->dim[i].lower_bound = lower_bounds[i];
dv->dim[i].extent = upper_bounds[i] - dv->dim[i].lower_bound + 1;
- if (i == 0)
- dv->dim[i].sm = dv->elem_len;
- else
- dv->dim[i].sm = dv->elem_len * dv->dim[i - 1].extent;
+ dv->dim[i].sm = dv->elem_len * arr_len;
arr_len *= dv->dim[i].extent;
}
}
Re: [patch v2] Fortran: fix sm computation in CFI_allocate [PR93524]
On 21.06.21 22:31, Sandra Loosemore wrote: On 6/21/21 5:42 AM, Tobias Burnus wrote: OK – but see some comments below. Revised patch attached. How's this one? LGTM - thanks! Tobias - Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
