public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tobias Burnus <tobias@codesourcery.com>
To: Harald Anlauf <anlauf@gmx.de>, Tobias Burnus <tobias@codesourcery.com>
Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>,
	Harald Anlauf via Gcc-patches <gcc-patches@gcc.gnu.org>,
	fortran <fortran@gcc.gnu.org>
Subject: Re: [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514
Date: Thu, 19 Aug 2021 09:52:24 +0200	[thread overview]
Message-ID: <8d25c317-74fa-d8a2-724f-de6944fa602e@codesourcery.com> (raw)
In-Reply-To: <trinity-e2132663-0637-437e-87a2-c1053b872f7a-1629320485192@3c-app-gmx-bap48>

[-- Attachment #1: Type: text/plain, Size: 4756 bytes --]

Hi Harald,

On 18.08.21 23:01, Harald Anlauf wrote:
>> Von: "Tobias Burnus"<tobias@codesourcery.com>
>>> 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(<substring>) 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

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 867 bytes --]

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))
@@ -37,0 +42 @@ program p
+!  if (l3 /= 2 .or. l6 /= 2 .or. l7 /= 2 .or. l8 /= 2) stop 12
@@ -39,0 +45,4 @@ end
+
+! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 2 "original" } }
+! { dg-final { scan-tree-dump "_gfortran_stop_numeric \\(3, 0\\);" "original" } }
+! { dg-final { scan-tree-dump "_gfortran_stop_numeric \\(7, 0\\);" "original" } }

  reply	other threads:[~2021-08-19  7:52 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 21:39 Harald Anlauf
2021-06-10 10:24 ` Bernhard Reutner-Fischer
2021-06-10 12:47   ` Bernhard Reutner-Fischer
2021-06-10 18:52   ` Harald Anlauf
2021-06-11  6:02     ` Bernhard Reutner-Fischer
2021-06-21 12:12     ` Tobias Burnus
2021-07-12 21:23       ` Harald Anlauf
2021-08-03 21:17         ` Harald Anlauf
2021-08-11 19:28           ` *PING* " Harald Anlauf
2021-08-18 10:22           ` Tobias Burnus
2021-08-18 21:01             ` Harald Anlauf
2021-08-19  7:52               ` Tobias Burnus [this message]
2021-08-19 19:11                 ` Harald Anlauf
2021-08-20  0:21                   ` H.J. Lu
2021-08-20  6:48                     ` Harald Anlauf
2021-08-20  9:16                       ` Jakub Jelinek
2021-08-20  9:45                         ` Aw: " Harald Anlauf
2021-08-20 10:12                           ` Jakub Jelinek
2021-08-20 10:53                             ` Aw: " Harald Anlauf
2021-08-20 10:59                               ` Jakub Jelinek
2021-08-20 11:43                                 ` [PATCH, committed] Fix bootstrap breakage caused by r12-3033 (PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514) Harald Anlauf
2021-08-20 12:01                               ` Aw: Re: Re: [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514 Tobias Burnus
2021-08-20 12:17                                 ` Aw: " Harald Anlauf
2021-08-20 14:19                                   ` Tobias Burnus
2021-08-20 19:43                                     ` Harald Anlauf
2021-08-20 11:50                         ` [Patch] c-format.c/Fortran: Support %wd / host-wide integer in gfc_error (Re: [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514) Tobias Burnus
2021-08-20 11:56                           ` Jakub Jelinek
2021-08-20 13:47                             ` Tobias Burnus
2021-08-20 13:53                               ` Jakub Jelinek
2021-08-20  9:29                     ` [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514 Tobias Burnus
2021-06-18 20:47 ` PING " Harald Anlauf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8d25c317-74fa-d8a2-724f-de6944fa602e@codesourcery.com \
    --to=tobias@codesourcery.com \
    --cc=anlauf@gmx.de \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rep.dot.nop@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).