[Fortran] OG11 backports
I've backported several patches having to do with Fortran/C interoperability from mainline to the OG11 branch. See attached log for details. -Sandra commit d554155c07771935778f557e9ef649cc3624d1ce Author: Sandra Loosemore Date: Wed Aug 11 19:24:17 2021 -0700 Fortran: Fix c_float128 and c_float128_complex definitions. gfc_float128_type_node is only non-NULL on targets that support a 128-bit type that is not long double. Use float128_type_node instead when computing the value of the kind constants c_float128 and c_float128_complex from the ISO_C_BINDING intrinsic module; this also ensures it actually corresponds to __float128 (the IEEE encoding) and not some other 128-bit floating-point type. 2021-08-11 Sandra Loosemore gcc/fortran/ * iso-c-binding.def (c_float128, c_float128_complex): Check float128_type_node instead of gfc_float128_type_node. * trans-types.c (gfc_init_kinds, gfc_build_real_type): Update comments re supported 128-bit floating-point types. (cherry picked from commit 58340a7cd3670024bafdbbc6ca63a9af841df98a) commit 0a8af79817f5c633543d2aa32f4f0385af4cf22c Author: Tobias Burnus Date: Wed Aug 11 19:18:42 2021 -0700 gfortran: Fix in-build-tree testing [PR101305, PR101660] ISO_Fortran_binding.h is written in the build dir - hence, a previous commit added it as include directory for in-build-tree testing. However, it turned out that -I$specdir/libgfortran interferes with reading .mod files as they are then no longer regareded as intrinsic modules. Solution: Create an extra include/ directory in the libgfortran build dir and copy ISO_Fortran_binding.h to that directory. As -B$specdir/libgfortran already causes gfortran to read that include subdirectory, the -I flag is no longer needed. PR libfortran/101305 PR fortran/101660 PR testsuite/101847 libgfortran/ChangeLog: * Makefile.am (ISO_Fortran_binding.h): Create include/ in the build dir and copy the include file to it. (clean-local): Add for removing the 'include' directory. * Makefile.in: Regenerate. gcc/testsuite/ChangeLog: * lib/gfortran.exp (gfortran_init): Remove -I$specpath/libgfortran from the string used to set GFORTRAN_UNDER_TEST. (cherry picked from commit 2ba0376ac40447ce7ee09fcef00511c18db25dfa) commit d2b1fbc8a159a465ce6114723301721808972a6e Author: Tobias Burnus Date: Mon Aug 9 12:35:23 2021 +0200 testsuite/lib/gfortran.exp: Add -I for ISO*.h [PR101305, PR101660] This patch adds -I$specdir/libgfortran to GFORTRAN_UNDER_TEST, when set by proc gfortran_init. As the $specdir depends on the multilib setting, it has to be re-set for a different multilib; hence, we track whether a previous call to gfortran_init set that var or whether it was set differently. gcc/testsuite/ PR libfortran/101305 PR fortran/101660 * lib/gfortran.exp (gfortran_init): Add -I $specdir/libgfortran to GFORTRAN_UNDER_TEST; update it when set by previous gfortran_init call. * gfortran.dg/ISO_Fortran_binding_1.c: Use <...> not "..." for ISO_Fortran_binding.h's #include. * gfortran.dg/ISO_Fortran_binding_10.c: Likewise. * gfortran.dg/ISO_Fortran_binding_11.c: Likewise. * gfortran.dg/ISO_Fortran_binding_12.c: Likewise. * gfortran.dg/ISO_Fortran_binding_15.c: Likewise. * gfortran.dg/ISO_Fortran_binding_16.c: Likewise. * gfortran.dg/ISO_Fortran_binding_17.c: Likewise. * gfortran.dg/ISO_Fortran_binding_18.c: Likewise. * gfortran.dg/ISO_Fortran_binding_3.c: Likewise. * gfortran.dg/ISO_Fortran_binding_5.c: Likewise. * gfortran.dg/ISO_Fortran_binding_6.c: Likewise. * gfortran.dg/ISO_Fortran_binding_7.c: Likewise. * gfortran.dg/ISO_Fortran_binding_8.c: Likewise. * gfortran.dg/ISO_Fortran_binding_9.c: Likewise. * gfortran.dg/PR94327.c: Likewise. * gfortran.dg/PR94331.c: Likewise. * gfortran.dg/bind_c_array_params_3_aux.c: Likewise. * gfortran.dg/iso_fortran_binding_uint8_array_driver.c: Likewise. * gfortran.dg/pr93524.c: Likewise. (cherry picked from commit 527a1cf32c27a3fbeaf6be7596241570d864cc4c) commit 5084c7d199d149cb58a3c41aae4ed9e97ef9ad31 Author: Sandra Loosemore Date: Wed Aug 11 18:57:34 2021 -0700 Bind(c): Improve error checking in CFI_* functions This patch adds additional run-time checking for invalid arguments to CFI_establish and CFI_setpointer. It also changes existing messages throughout the CFI_* functions to use PRIiPTR to format CFI_index_t values instead of casting them to int and using %d (which may not work on targets where int is a smaller type), simplifies wording of some messages, and fixes issues with capitalization, typos, and the like. Additionally some coding standards problems such as >80 character lines
Re: [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514
On Thu, Aug 19, 2021 at 12:12 PM Harald Anlauf via Gcc-patches wrote: > > Hi Tobias, > > > I am inclined to say that the Intel compiler has a bug by not > > accepting it – but as written before, I regard sub-string length > > (esp. with const expr) inquiries as an odd corner case which > > is unlikely to occur in real-world code. > > ok. > > > Still does not work – or rather: ...%t(:)(3:4) [i.e. substring with array > > section] > > and ...%str(3:4) [i.e. substring of deferred-length scalar] both do work > > but if one combines the two (→ ...%str2(:)(3:4), i.e. substring of > > deferred-length > > array section), it does not: > > > > Array ‘r’ at (1) is a variable, which does not reduce to a constant > > expression > > > > for: > > > > --- a/gcc/testsuite/gfortran.dg/pr100950.f90 > > +++ b/gcc/testsuite/gfortran.dg/pr100950.f90 > > @@ -15,2 +15,3 @@ program p > >character(len=:), allocatable :: str > > + character(len=:), allocatable :: str2(:) > > end type t_ > > @@ -24,2 +25,4 @@ program p > > integer, parameter :: l6 = len (r(1)%str (3:4)) > > + integer, parameter :: l7 = len (r(1)%str2(1)(3:4)) > > + integer, parameter :: l8 = len (r(1)%str2(:)(3:4)) > > > > > > which feels odd. > > I agree. I have revised the code slightly to accept substrings > of deferred-length. Your suggested variants now work correctly. > > > In principle, LGTM – except I wonder what we do about the > > len(r(1)%str(1)(3:4)); > > I think we really do handle most code available and I would like to > > close this > > topic – but still it feels a bit odd to leave this bit out. > > That is handle now as discussed, see attached final patch. > Regtested again. > > > I was also wondering whether we should check that the > > compile-time simplification works – i.e. use -fdump-tree-original for this; > > I attached a patch for this. > > I added this to the final patch and taken the liberty to push the result > to master as d881460deb1f0bdfc3e8fa2d391a03a9763cbff4. > > Thanks for your patience, given the rather extensive review... > > Harald This may have broken bootstrap on 32-bit hosts: https://gcc.gnu.org/pipermail/gcc-regression/2021-August/075209.html -- H.J.
[PATCH] libgfortran : Use the libtool macro to determine libm availability.
Hi, A while ago had a report of build failure against a Darwin branch on the latest OS release. This was because (temporarily) the symlink from libm.dylib => libSystem.dylib had been removed/omitted. libm is not needed on Darwin, and should not be added unconditionally even if that is (mostly) harmless since it is a symlink to libc. There could be cases where the addition was not completely harmless because the presentation of the symlink would cause the symbols exposed in libSystem to be considered ahead of ones presented in convenience libraries. tested on x86_64, i686-darwin, x86_64-linux, OK for master? thanks Iain libgfortran/ChangeLog: * Makefile.am: Use configured libm availability. * Makefile.in: Regenerate. * configure: Regenerate. * configure.ac: Use libtool macro to find libm availability. * libgfortran.spec.in: Use configured libm availability. --- libgfortran/Makefile.am | 2 +- libgfortran/Makefile.in | 3 +- libgfortran/configure | 142 libgfortran/configure.ac| 1 + libgfortran/libgfortran.spec.in | 2 +- 5 files changed, 147 insertions(+), 3 deletions(-) diff --git a/libgfortran/Makefile.am b/libgfortran/Makefile.am index 8d104321567..6fc4b465c6e 100644 --- a/libgfortran/Makefile.am +++ b/libgfortran/Makefile.am @@ -42,7 +42,7 @@ libgfortran_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS) libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` \ $(LTLDFLAGS) $(LIBQUADLIB) ../libbacktrace/libbacktrace.la \ $(HWCAP_LDFLAGS) \ - -lm $(extra_ldflags_libgfortran) \ + $(LIBM) $(extra_ldflags_libgfortran) \ $(version_arg) -Wc,-shared-libgcc libgfortran_la_DEPENDENCIES = $(version_dep) libgfortran.spec $(LIBQUADLIB_DEP) diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index 523eb24bca1..513fd80b936 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -260,6 +260,7 @@ AC_PROG_INSTALL #AC_MSG_NOTICE([== Starting libtool configuration]) AC_LIBTOOL_DLOPEN AM_PROG_LIBTOOL +AC_CHECK_LIBM ACX_LT_HOST_FLAGS AC_SUBST(enable_shared) AC_SUBST(enable_static) diff --git a/libgfortran/libgfortran.spec.in b/libgfortran/libgfortran.spec.in index 95aa3f842a3..b870e78c151 100644 --- a/libgfortran/libgfortran.spec.in +++ b/libgfortran/libgfortran.spec.in @@ -5,4 +5,4 @@ # %rename lib liborig -*lib: @LIBQUADSPEC@ -lm %(libgcc) %(liborig) +*lib: @LIBQUADSPEC@ @LIBM@ %(libgcc) %(liborig) -- 2.24.3 (Apple Git-128)
Re: [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514
Hi Tobias,
> I am inclined to say that the Intel compiler has a bug by not
> accepting it – but as written before, I regard sub-string length
> (esp. with const expr) inquiries as an odd corner case which
> is unlikely to occur in real-world code.
ok.
> Still does not work – or rather: ...%t(:)(3:4) [i.e. substring with array
> section]
> and ...%str(3:4) [i.e. substring of deferred-length scalar] both do work
> but if one combines the two (→ ...%str2(:)(3:4), i.e. substring of
> deferred-length
> array section), it does not:
>
> Array ‘r’ at (1) is a variable, which does not reduce to a constant expression
>
> for:
>
> --- a/gcc/testsuite/gfortran.dg/pr100950.f90
> +++ b/gcc/testsuite/gfortran.dg/pr100950.f90
> @@ -15,2 +15,3 @@ program p
>character(len=:), allocatable :: str
> + character(len=:), allocatable :: str2(:)
> end type t_
> @@ -24,2 +25,4 @@ program p
> integer, parameter :: l6 = len (r(1)%str (3:4))
> + integer, parameter :: l7 = len (r(1)%str2(1)(3:4))
> + integer, parameter :: l8 = len (r(1)%str2(:)(3:4))
>
>
> which feels odd.
I agree. I have revised the code slightly to accept substrings
of deferred-length. Your suggested variants now work correctly.
> In principle, LGTM – except I wonder what we do about the
> len(r(1)%str(1)(3:4));
> I think we really do handle most code available and I would like to
> close this
> topic – but still it feels a bit odd to leave this bit out.
That is handle now as discussed, see attached final patch.
Regtested again.
> I was also wondering whether we should check that the
> compile-time simplification works – i.e. use -fdump-tree-original for this;
> I attached a patch for this.
I added this to the final patch and taken the liberty to push the result
to master as d881460deb1f0bdfc3e8fa2d391a03a9763cbff4.
Thanks for your patience, given the rather extensive review...
Harald
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index c27b47aa98f..492867e12cb 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4512,6 +4512,78 @@ gfc_simplify_leadz (gfc_expr *e)
}
+/* Check for constant length of a substring. */
+
+static bool
+substring_has_constant_len (gfc_expr *e)
+{
+ gfc_ref *ref;
+ HOST_WIDE_INT istart, iend, length;
+ bool equal_length = false;
+
+ if (e->ts.type != BT_CHARACTER)
+return false;
+
+ for (ref = e->ref; ref; ref = ref->next)
+if (ref->type != REF_COMPONENT && ref->type != REF_ARRAY)
+ break;
+
+ if (!ref
+ || ref->type != REF_SUBSTRING
+ || !ref->u.ss.start
+ || ref->u.ss.start->expr_type != EXPR_CONSTANT
+ || !ref->u.ss.end
+ || ref->u.ss.end->expr_type != EXPR_CONSTANT
+ || !ref->u.ss.length)
+return false;
+
+ /* For non-deferred strings the given length shall be constant. */
+ if (!e->ts.deferred
+ && (!ref->u.ss.length->length
+ || ref->u.ss.length->length->expr_type != EXPR_CONSTANT))
+return false;
+
+ /* Basic checks on substring starting and ending indices. */
+ if (!gfc_resolve_substring (ref, _length))
+return false;
+
+ istart = gfc_mpz_get_hwi (ref->u.ss.start->value.integer);
+ iend = gfc_mpz_get_hwi (ref->u.ss.end->value.integer);
+
+ if (istart <= iend)
+{
+ if (istart < 1)
+ {
+ gfc_error ("Substring start index (" HOST_WIDE_INT_PRINT_DEC
+ ") at %L below 1",
+ istart, >u.ss.start->where);
+ return false;
+ }
+
+ /* For deferred strings use end index as proxy for length. */
+ if (e->ts.deferred)
+ length = iend;
+ else
+ length = gfc_mpz_get_hwi (ref->u.ss.length->length->value.integer);
+ if (iend > length)
+ {
+ gfc_error ("Substring end index (" HOST_WIDE_INT_PRINT_DEC
+ ") at %L exceeds string length",
+ iend, >u.ss.end->where);
+ return false;
+ }
+ length = iend - istart + 1;
+}
+ else
+length = 0;
+
+ /* Fix substring length. */
+ e->value.character.length = length;
+
+ return true;
+}
+
+
gfc_expr *
gfc_simplify_len (gfc_expr *e, gfc_expr *kind)
{
@@ -4521,7 +4593,8 @@ gfc_simplify_len (gfc_expr *e, gfc_expr *kind)
if (k == -1)
return _bad_expr;
- if (e->expr_type == EXPR_CONSTANT)
+ if (e->expr_type == EXPR_CONSTANT
+ || substring_has_constant_len (e))
{
result = gfc_get_constant_expr (BT_INTEGER, k, >where);
mpz_set_si (result->value.integer, e->value.character.length);
diff --git a/gcc/testsuite/gfortran.dg/pr100950.f90 b/gcc/testsuite/gfortran.dg/pr100950.f90
new file mode 100644
index 000..cb9d126bc18
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr100950.f90
@@ -0,0 +1,53 @@
+! { dg-do run }
+! { dg-additional-options "-fdump-tree-original" }
+! PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514
+
+program p
+ character(8), parameter :: u = "123"
+ character(8):: x = "", s
+ character(2):: w(2) = [character(len(x(3:4))) :: 'a','b' ]
+ character(*), parameter :: y(*) =
Re: [PATCH v3, Fortran] TS 29113 testsuite
On 7/27/21 5:07 AM, Tobias Burnus wrote: Hi Sandra, hi Thomas, hi all, @Thomas K: Comments about the following - and of course to the testsuite itself - are highly welcome. In my opinion, the testsuite LGTM and can be committed. @Sandra: - Thoughts on the directory name? (cf. below) - Give others/Thomas a chance to comment on this, before committing it. (And remove the now passing xfails.) Thanks for the testsuite! Regarding: * XFAILS - as discussed before, I think having some XFAILS is not ideal but fine, especially if the XFAIL/PASS ratio is low and there are plans to fix the known fails, some posted patches for those, and open PRs for the issues. (I think there is one patch pending review and two patches pending committal with some modifications by Sandra - plus several patches by José which still need to be reviewed.) * Naming of the directory + .exp file: ts29113/ts29113.exp is okay. Given that 'select rank' (in F2018 but not in TS29113) is also tested, there was some controversy regarding the name and the coverage; additionally, TS29113 is a name which is not immediately clear. Thus, we could use some other name like: c-interop/c-interop.exp or (suggestions?). In any case, I do not feel strong about either name. * I had a closer look at earlier versions of the testsuite, I did browse through the current one + looked at the diff to previous version, but it is big enough and the spec is complex enough that I have likely missed something. Thus: Additional reviews are highly welcome! Here is the current version of the testsuite. Changes since the last version include: * Renaming the directory and .exp file from ts29113 -> c-interop per the request above. There have been no additional review comments. * I also made it explicit that section and constraint numbers mentioned in comments in the test cases refer to TS 29113. I considered using the numbering from 2018 standard, but given that the standard already renumbered things twice since the time TS 29113 was published I didn't really see the point, as long as it is unambiguous what document is being cited. * I flattened the subdirectory structure after realizing that dg-additional-sources can't cope with relative pathnames in remote-host testing. * I split up the typecodes tests (for testing that descriptors constructed by the front end match ISO_Fortran_binding.h) to allow for finer-grained control over xfails and dg-require-effective-target, and added a new effective target for Fortran C_FLOAT128 support. There are also some additional things being tested now in this group. The current xfails in the tests reflect the two patches I posted last night that are still waiting for review: https://gcc.gnu.org/pipermail/fortran/2021-August/056382.html https://gcc.gnu.org/pipermail/fortran/2021-August/056383.html I've been testing on x86 (both 32- and 64-bit) and powerpc64le-linux-gnu. Given that Tobias already said the last version of the patch was OK, I'd like to commit this soon, either at the same time I push the patches above, or next week if there is some hold-up on them. If anybody wants more time to review this first, let me know. -Sandra ts29113-aug19.patch.gz Description: application/gzip
Re: F2018 C937
Arjen, If this is your first go around with patching gfortran, I'll suggest running the testsuite (if you haven't, see step 5. below; if you have see step 6.). I suspect you already know much of what I enumerate below, but it may help others. With a bug report, the workflow for me is/was 1. Check versions of the standard to determine if it is a bug, and what is the expected result. 2. Identify where the problem can be addressed in source. For you (or anyone else interested in gfortran development), I suspect there a lot of question about how to do this step and the structure of the gfortran source code. I can answer some of those questions in follow-up emails. Send them to fortran@ and CC me. 3. Develop patch. I've done the first 3 steps. You are now at testing the patch. 4. Build gfortran with the patch. Assuming a Unix-like system, I have gcc/gccx with the source and gcc/objx is the build directory. So, for a first time build on an N cpu system do % cd gcc/objx % ../gcc/gccx/configure --prefix=$HOME/work/x \ --enable-languages=c,c++,fortran,lto \ --enable-bootstrap --disable-nls --enable-checking % make -j N-1 bootstrap && make install Otherwise, % cd gcc/objx % make -j N-1 && make install This installs everything in $HOME/work/x. 5. Run the testsuite to check for regressions. If any occur, fix regressions or fix the patch. % make -j N-1 check-fortran % tail gcc/testsuite/gfortran/gfortran.sum === gfortran Summary === # of expected passes58647 # of expected failures 253 # of unsupported tests 92 objx/gcc/gfortran version 12.0.0 20210816 (experimental) (GCC) With the C937 and C949 patches, I changed "typespec" to "type-spec" in nearby unrelated error messages. Both are being used and type-spec matches the standard. This may cause a regression, so one or more testcases may need a change. The C937 patch did not cause a regression. The C949 one did. You'll see a line like # of unexpected failures 7 The file gcc/testsuite/gfortran/gfortran.log contains the buildlog, which is huge. You can find the failures with a search for lines containing ^FAIL. 6. Prepare ChangeLog. This has changed with git so you'll need to ask Tobias, Thomas, Harald, or on the gcc@ list for guidance. 7. Submit patch to fortran@gcc and gcc-patches@gcc asking for review. 8. Wait a few days. Ping fortran@gcc and gcc-patches@gcc. 9. Wait a few days. Ping fortran@gcc and gcc-patches@gcc. As I developed, the original patch and presumably you reviewed it for correctness, you can probably skip step 9. 10. Wait a few days. Commit patch with or without a review. I do not know if you need any explicit access/permission to commit a patch. I iknow very little about git and how it works. At this point, you can either backport the patch to release branches or close the PR. For me, I always did a backport if it was a trivial task. At some point, HEAD will diverge sufficiently from a branch, I would then stop backporting. -- steve On Thu, Aug 19, 2021 at 05:10:47PM +0200, Arjen Markus wrote: > I have applied the patches locally (take care to restore the tabs ;)). > Should I now commit these changes or is there a more formal procedure > involved? > > Regards, > > Arjen > > Op do 19 aug. 2021 om 08:59 schreef Arjen Markus >: > > > Hi Steve, > > > > I am willing to take up this challenge ;), as well as the patch for C949. > > It would be my next attempt to get acquainted with the source code (a first > > step hopefully to actively contribute). > > > > Regards, > > > > Arjen > > > > Op di 17 aug. 2021 om 21:02 schreef Steve Kargl via Fortran < > > fortran@gcc.gnu.org>: > > > >> For those that might care, I draw your attention to > >> > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101951 > >> > >> Good opportunity for a lurker to step forward and > >> become a gfortran committer. Otherwise, this patch > >> will fester in bugzilla the dozen or so other patches > >> I've attached to PRs. > >> > >> -- > >> Steve > >> > > -- Steve
Re: F2018 C937
I have applied the patches locally (take care to restore the tabs ;)). Should I now commit these changes or is there a more formal procedure involved? Regards, Arjen Op do 19 aug. 2021 om 08:59 schreef Arjen Markus : > Hi Steve, > > I am willing to take up this challenge ;), as well as the patch for C949. > It would be my next attempt to get acquainted with the source code (a first > step hopefully to actively contribute). > > Regards, > > Arjen > > Op di 17 aug. 2021 om 21:02 schreef Steve Kargl via Fortran < > fortran@gcc.gnu.org>: > >> For those that might care, I draw your attention to >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101951 >> >> Good opportunity for a lurker to step forward and >> become a gfortran committer. Otherwise, this patch >> will fester in bugzilla the dozen or so other patches >> I've attached to PRs. >> >> -- >> Steve >> >
Re: [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514
Hi Harald,
On 18.08.21 23:01, Harald Anlauf wrote:
Von: "Tobias Burnus"
Note, however, that gfc_simplify_len still won't handle neither
deferred strings nor their substrings.
Obviously, nonsubstrings cannot be simplified but I do not
see why len(str(1:2)) cannot or should not be simplified.
well, here's an example that Intel rejects:
...
character(:), allocatable :: str
end type u
type(u) :: q
...
integer, parameter :: k3 = len (q% str (3:4)) ! Rejected by Intel
pr100950-ww.f90(7): error #6814: When using this inquiry function, the length
of this object cannot be evaluated to a constant. [LEN]
I think the question is really how to interpret "10.1.12 Constant expression"
"(4) a specification inquiry where each designator or argument is
...
(b) a variable whose properties inquired about are not
(i) assumed,
(ii) deferred, or
(iii) defined by an expression that is not a constant expression,"
And as the substring bounds are constant expressions,
one can argue that (4)(b) is fulfilled as (i)–(iii) do not apply.
I am inclined to say that the Intel compiler has a bug by not
accepting it – but as written before, I regard sub-string length
(esp. with const expr) inquiries as an odd corner case which
is unlikely to occur in real-world code.
However, there is no reason why the user cannot do [...]
Maybe you can enlighten me here. [...]
I can't as I did not understand your question. However ...
But, IMHO, the latter remark does_not_ imply that we
shall/must/have to accept code like:
if (allocated(str)) then
block
integer, parameter :: n = len(str(:5))
end block
endif
So shall we not simplify here (and thus reject it)?
This is important! Or silently simplify and accept it?
I tried to draw the line between simplification – to generate better code –
and 'constant expression' handling (accept where permitted, diagnose
non-standard-conforming code). — However, nearly but not quite always:
if it can be simplified to a constant the standard also regards it as
constant expression.
I think in for the purpose of the examples in this email thread,
we do not need to distinguish the two. — And can always simplify
deferred-length substrings where the substring bounds are const
expressions (or the lower-bound is absent and, hence, 1).
With the caveat from above that len() is rather special,
there is no real reason why: str_array(:)(4:5) cannot be handled.
(→ len = 2).
Good point. This is fixed in the revised patch and tested for.
Still does not work – or rather: ...%t(:)(3:4) [i.e. substring with array
section]
and ...%str(3:4) [i.e. substring of deferred-length scalar] both do work
but if one combines the two (→ ...%str2(:)(3:4), i.e. substring of
deferred-length
array section), it does not:
Array ‘r’ at (1) is a variable, which does not reduce to a constant expression
for:
--- a/gcc/testsuite/gfortran.dg/pr100950.f90
+++ b/gcc/testsuite/gfortran.dg/pr100950.f90
@@ -15,2 +15,3 @@ program p
character(len=:), allocatable :: str
+ character(len=:), allocatable :: str2(:)
end type t_
@@ -24,2 +25,4 @@ program p
integer, parameter :: l6 = len (r(1)%str (3:4))
+ integer, parameter :: l7 = len (r(1)%str2(1)(3:4))
+ integer, parameter :: l8 = len (r(1)%str2(:)(3:4))
which feels odd.
The updated patch regtests fine. OK?
Looks good to me except for the caveats.
Regtested again.
[...]
Well, there's already
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101735
I have added the example to the PR.
For deferred length, I have no strong opinion; [...]
Actually, this is now an important point. If we really want
to allow to handle substrings of deferred length strings
in constant expressions, the new patch would be fine,
I think handling len=: substrings is fine.
In principle, LGTM – except I wonder what we do about the
len(r(1)%str(1)(3:4));
I think we really do handle most code available and I would like to
close this
topic – but still it feels a bit odd to leave this bit out.
I was also wondering whether we should check that the
compile-time simplification works – i.e. use -fdump-tree-original for this;
I attached a patch for this.
Thanks,
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
diff --git a/gcc/testsuite/gfortran.dg/pr100950.f90 b/gcc/testsuite/gfortran.dg/pr100950.f90
index 7de589fe882..b9dcef0a7af 100644
--- a/gcc/testsuite/gfortran.dg/pr100950.f90
+++ b/gcc/testsuite/gfortran.dg/pr100950.f90
@@ -1,0 +2 @@
+! { dg-additional-options "-fdump-tree-original" }
@@ -15,0 +17 @@ program p
+ character(len=:), allocatable :: str2(:)
@@ -24,0 +27,2 @@ program p
+! integer, parameter :: l7 = len (r(1)%str2(1)(3:4))
+! integer, parameter :: l8 = len (r(1)%str2(:)(3:4))
@@
Re: F2018 C937
Hi Steve, I am willing to take up this challenge ;), as well as the patch for C949. It would be my next attempt to get acquainted with the source code (a first step hopefully to actively contribute). Regards, Arjen Op di 17 aug. 2021 om 21:02 schreef Steve Kargl via Fortran < fortran@gcc.gnu.org>: > For those that might care, I draw your attention to > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101951 > > Good opportunity for a lurker to step forward and > become a gfortran committer. Otherwise, this patch > will fester in bugzilla the dozen or so other patches > I've attached to PRs. > > -- > Steve >
