From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
To: Janne Blomqvist <blomqvist.janne@gmail.com>
Cc: rep.dot.nop@gmail.com, gfortran <fortran@gcc.gnu.org>,
GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH,FORTRAN 00/29] Move towards stringpool, part 1
Date: Thu, 13 Apr 2023 23:04:40 +0200 [thread overview]
Message-ID: <20230413230440.229ebc2f@nbbrfq> (raw)
In-Reply-To: <CAC1BbcSqDGHi9=RGzHmS1fo7G+zNMivxiAv69j_1+xYu15JAJQ@mail.gmail.com>
Hi all, Janne!
On Wed, 19 Sep 2018 16:40:01 +0200
Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> On Fri, 7 Sep 2018 at 10:07, Bernhard Reutner-Fischer
> <rep.dot.nop@gmail.com> wrote:
> >
> > On Wed, 5 Sep 2018 at 20:57, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
> > >
> > > On Wed, Sep 5, 2018 at 5:58 PM Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> >
> > >> Bootstrapped and regtested on x86_64-foo-linux.
> > >>
> > >> I'd appreciate if someone could double check for regressions on other
> > >> setups. Git branch:
> > >> https://gcc.gnu.org/git/?p=gcc.git;a=log;h=refs/heads/aldot/fortran-fe-stringpool
> > >>
> > >> Ok for trunk?
> > >
> > >
> > > Hi,
> > >
> > > this is quite an impressive patch set. I have looked through all the patches, and on the surface they all look ok.
> >
> > Thanks alot for your appreciation!
> > >
> > > Unfortunately I don't have any exotic target to test on either, so I think you just have to commit it and check for regression reports. Though I don't see this set doing anything which would work differently on other targets, but you never know..
> > >
> > > I'd say wait a few days in case anybody else wants to comment on it, then commit it to trunk.
Unfortunately I never got to commit it.
Are you still OK with this series?
Iff so, i will refresh it for gcc-14 stage1. I would formally resubmit
the series for approval then, of course, for good measure.
It will need some small adjustments since coarrays were added
afterwards and a few other spots were changed since then.
Note that after your OK i fixed the problem described below with this
patch
https://inbox.sourceware.org/gcc-patches/20180919225533.20009-1-rep.dot.nop@gmail.com/
and so i think this "[PATCH,FORTRAN v2] Use stringpool on loading
module symbols" was not formally OKed yet, FWIW. I believe that this v2 worked flawlessly.
Note, however, that by adding/changing module names of symbols in the
module files, this will (i think / fear) require a bump of the module
version if we are honest. Ultimately that was the reason why i did not
push the series back then.
Link to the old series:
https://inbox.sourceware.org/gcc-patches/20180905145732.404-1-rep.dot.nop@gmail.com/
thanks and cheers,
> >
> > Upon further testing i encountered a regression in module writing,
> > manifesting itself in a failure to compile ieee_8.f90 (and only this).
>
> > Sorry for that, I'll have another look during the weekend.
>
> so in free_pi_tree we should not free true_name nor module:
>
> @@ -239,12 +239,6 @@ free_pi_tree (pointer_info *p)
> free_pi_tree (p->left);
> free_pi_tree (p->right);
>
> - if (iomode == IO_INPUT)
> - {
> - XDELETEVEC (p->u.rsym.true_name);
> - XDELETEVEC (p->u.rsym.module);
> - }
> -
> free (p);
> }
>
> This fixes the module writing but leaks, obviously.
> Now the reason why i initially did not use mio_pool_string for both
> rsym.module and rsym.true_name was that mio_pool_string sets the name
> to NULL if the string is empty.
> Currently these are read by read_string() [which we should get rid of
> entirely, the 2 mentioned fields are the last two who use
> read_string()] which does not nullify the empty string but returns
> just the pointer. For e.g. ieee_8.f90 using mio_pool_string gives us a
> NULL module which leads to gfc_use_module -> load_needed ->
> gfc_find_symbol -> gfc_find_sym_tree -> gfc_find_symtree which tries
> to c = strcmp (name, st->name); where name is NULL.
>
> The main culprits seem to be class finalization wrapper variables so
> i'm adding modules to those now.
> Which leaves me with regressions like allocate_with_source_14.f03.
> "Fixing" these by falling back to gfc_current_ns->proc_name->name in
> load_needed for !ns->proc_name if the rsym->module is NULL seems to
> work.
>
> Now there are a number of issues with names of fixups. Like the 9 in e.g.:
>
> $ zcat /tmp/n/m.mod | egrep -v "^(\(\)|\(\(\)|$)"
> GFORTRAN module version '15' created from generic_27.f90
> (('testif' 'm' 2 3))
> (4 'm' 'm' '' 1 ((MODULE UNKNOWN-INTENT UNKNOWN-PROC UNKNOWN UNKNOWN 0 0)
> 3 'test1' 'm' '' 1 ((PROCEDURE UNKNOWN-INTENT MODULE-PROC DECL UNKNOWN 0
> 0 FUNCTION) () (REAL 4 0 0 0 REAL ()) 5 0 (6) () 3 () () () 0 0)
> 2 'test2' 'm' '' 1 ((PROCEDURE UNKNOWN-INTENT MODULE-PROC DECL UNKNOWN 0
> 0 FUNCTION ARRAY_OUTER_DEPENDENCY) () (REAL 4 0 0 0 REAL ()) 7 0 (8) ()
> 2 () () () 0 0)
> 6 'obj' '' '' 5 ((VARIABLE UNKNOWN-INTENT UNKNOWN-PROC UNKNOWN UNKNOWN 0
> 0 DUMMY) () (REAL 4 0 0 0 REAL ()) 0 0 () () 0 () () () 0 0)
> 8 'pr' '' '' 7 ((PROCEDURE UNKNOWN-INTENT DUMMY-PROC UNKNOWN UNKNOWN 0 0
> EXTERNAL DUMMY FUNCTION PROCEDURE ARRAY_OUTER_DEPENDENCY) () (REAL 4 9 0
> 0 REAL ()) 0 0 () () 8 () () () 0 0)
> 9 '' '' '' 7 ((PROCEDURE UNKNOWN-INTENT UNKNOWN-PROC UNKNOWN UNKNOWN 0 0
> FUNCTION) () (REAL 4 0 0 0 REAL ()) 0 0 () () 0 () () () 0 0)
> )
> ('m' 0 4 'test1' 0 3 'test2' 0 2)
>
> which is a bit of a complication since we need them to verify proper
> interface types and attributes etc, etc.
> generic_27.f90 would then warn in check_proc_interface() that
> "Interface %qs at %L must be explicit".
> To bypass this warning i suggest to flag these as artificial like so:
> @@ -6679,10 +6683,12 @@ match_procedure_decl (void)
> return MATCH_ERROR;
> sym->ts.interface = gfc_new_symbol ("", gfc_current_ns);
> sym->ts.interface->ts = current_ts;
> sym->ts.interface->attr.flavor = FL_PROCEDURE;
> sym->ts.interface->attr.function = 1;
> + /* Suppress warnings about explicit interface */
> + sym->ts.interface->attr.artificial = 1;
> sym->attr.function = 1;
> sym->attr.if_source = IFSRC_UNKNOWN;
> }
>
> if (gfc_match (" =>") == MATCH_YES)
> @@ -6818,10 +6824,12 @@ match_ppc_decl (void)
> c->ts.interface = gfc_new_symbol ("", gfc_current_ns);
> c->ts.interface->result = c->ts.interface;
> c->ts.interface->ts = ts;
> c->ts.interface->attr.flavor = FL_PROCEDURE;
> c->ts.interface->attr.function = 1;
> + /* Suppress warnings about explicit interface */
> + c->ts.interface->attr.artificial = 1;
> c->attr.function = 1;
> c->attr.if_source = IFSRC_UNKNOWN;
> }
>
> if (gfc_match (" =>") == MATCH_YES)
>
> and then not exclude ""-names but attr.artificial for the "must be
> explicit" warning. This works fine.
>
> Another spot where we encounter trouble with NULL module in the sym is
> generic_1.f90 where we would be unable to distinguish interface sub
> arguments during true_name lookup.
> We find x in true names and consequently use this one sym for both the
> REAL as well as the INTEGER variable which of course doesn't work when
> resolving.
> As it turns out we get away with punting true_name lookup if the module is NULL.
>
> The next hintch are unlimited polymorphic component class containers
> as in select_type_36.f03 when used in module context.
> gfc_find_gsymbol() around the "upe" really wants a module that is
> non-NULL which we luckily have at hand. This just extends the
> proc_name-hack.
>
> @@ -4061,6 +4061,10 @@ gfc_match_decl_type_spec (gfc_typespec *ts, int
> implicit_flag)
> upe->refs++;
> upe->ts.type = BT_VOID;
> upe->attr.unlimited_polymorphic = 1;
> + /* Make sure gfc_find_gsymbol sees a (non-NULL) name to
> + * search for by plugging in some module name. */
> + if (gfc_current_ns->proc_name != NULL)
> + upe->module = gfc_current_ns->proc_name->name;
> /* This is essential to force the construction of
> unlimited polymorphic component class containers. */
> upe->attr.zero_comp = 1;
>
> The area of true_name and pi_root handling is a bit unpleasant to work
> with, i must admit. But then i do not volunteer to rip it all out ;)
> I think we will be able to remove some of these proc_name-hacks as
> soon as we switch the symbol finding to pointer comparison, at least.
>
> I'm cleaning up the above for a final test and will send it as
> alternative, extended approach intended to replace the
> "[PATCH,FORTRAN 25/29] Use stringpool on loading module symbols" (
> https://gcc.gnu.org/ml/fortran/2018-09/msg00039.html )
>
> patch, fwiw.
>
> thanks,
next prev parent reply other threads:[~2023-04-13 21:04 UTC|newest]
Thread overview: 94+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-01 12:55 [PATCH] Use gfc_add_*_component defines where appropriate Bernhard Reutner-Fischer
2015-12-01 12:55 ` [PATCH] Derive interface buffers from max name length Bernhard Reutner-Fischer
2015-12-01 14:52 ` Janne Blomqvist
2015-12-01 16:51 ` Bernhard Reutner-Fischer
2015-12-03 9:46 ` Janne Blomqvist
2016-06-18 19:46 ` Bernhard Reutner-Fischer
2017-10-19 8:03 ` Bernhard Reutner-Fischer
2017-10-20 22:46 ` Bernhard Reutner-Fischer
2017-10-21 15:18 ` Thomas Koenig
2017-10-21 18:11 ` Bernhard Reutner-Fischer
2017-10-31 20:35 ` Bernhard Reutner-Fischer
2018-09-03 16:05 ` Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 13/29] Use stringpool for intrinsics and common Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 07/29] Use stringpool for some gfc_code2string return values Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 03/29] Use stringpool for gfc_get_name Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 08/29] Add uop/name helpers Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 00/29] Move towards stringpool, part 1 Bernhard Reutner-Fischer
2018-09-05 18:57 ` Janne Blomqvist
2018-09-07 8:09 ` Bernhard Reutner-Fischer
2018-09-19 14:40 ` Bernhard Reutner-Fischer
2023-04-13 21:04 ` Bernhard Reutner-Fischer [this message]
[not found] ` <cba81495-832c-2b95-3c30-d2ef819ea9fb@charter.net>
[not found] ` <CAC1BbcThL4Cj=mVRuGg2p8jUipwLOeosB7kwoVD27myRnKcgZA@mail.gmail.com>
2021-04-18 21:30 ` Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 01/29] gdbinit: break on gfc_internal_error Bernhard Reutner-Fischer
2021-10-29 18:58 ` Bernhard Reutner-Fischer
2021-10-29 22:13 ` Jerry D
2021-10-30 18:25 ` Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 09/29] Use stringpool for modules Bernhard Reutner-Fischer
2018-09-05 18:44 ` Janne Blomqvist
2018-09-05 20:59 ` Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 06/29] Use stringpool for association_list Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 02/29] Use stringpool for gfc_match_defined_op_name() Bernhard Reutner-Fischer
2018-09-05 14:57 ` [PATCH,FORTRAN 04/29] Use stringpool for gfc_match_generic_spec Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 12/29] Use stringpool for remaining names Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 26/29] Use stringpool for mangled common names Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 24/29] Use stringpool for intrinsic functions Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 14/29] Fix write_omp_udr for user-operator REDUCTIONs Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 22/29] Use stringpool in class and procedure-pointer result Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 11/29] Do pointer comparison instead of strcmp Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 25/29] Use stringpool on loading module symbols Bernhard Reutner-Fischer
2018-09-19 22:55 ` [PATCH,FORTRAN v2] " Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 21/29] Use stringpool for module tbp Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 05/29] Use stringpool for gfc_match("%n") Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 29/29] PR87103: Remove max symbol length check from gfc_new_symbol Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 23/29] Use stringpool for module binding_label Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 10/29] Do not copy name for check_function_name Bernhard Reutner-Fischer
2018-09-05 14:58 ` [PATCH,FORTRAN 27/29] Use stringpool for OMP clause reduction code Bernhard Reutner-Fischer
2018-09-05 15:02 ` [PATCH,FORTRAN 18/29] Use stringpool for charkind Bernhard Reutner-Fischer
2018-09-05 15:02 ` [PATCH,FORTRAN 19/29] Use stringpool and unified uppercase handling for types Bernhard Reutner-Fischer
2018-09-05 15:02 ` [PATCH,FORTRAN 17/29] Use stringpool for iso_fortran_env Bernhard Reutner-Fischer
2018-09-05 15:02 ` [PATCH,FORTRAN 15/29] Use stringpool for iso_c_binding module names Bernhard Reutner-Fischer
2018-09-05 15:02 ` [PATCH,FORTRAN 16/29] Do pointer comparison in iso_c_binding_module Bernhard Reutner-Fischer
2018-09-05 15:02 ` [PATCH,FORTRAN 28/29] Free type-bound procedure structs Bernhard Reutner-Fischer
2021-10-29 0:05 ` Bernhard Reutner-Fischer
2021-10-29 14:54 ` Jerry D
2021-10-29 16:42 ` Bernhard Reutner-Fischer
[not found] ` <slhifq$rlb$1@ciao.gmane.io>
2021-10-29 20:09 ` Bernhard Reutner-Fischer
2021-10-31 22:35 ` Bernhard Reutner-Fischer
2018-09-05 15:02 ` [PATCH,FORTRAN 20/29] Use stringpool in class et al Bernhard Reutner-Fischer
2015-12-01 12:55 ` [PATCH] Commentary typo fix for gfc_typenode_for_spec() Bernhard Reutner-Fischer
2015-12-01 16:00 ` Steve Kargl
2016-06-18 20:07 ` Bernhard Reutner-Fischer
2015-12-01 12:56 ` [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE Bernhard Reutner-Fischer
2015-12-01 15:02 ` Steve Kargl
2015-12-01 16:13 ` Bernhard Reutner-Fischer
2015-12-01 16:41 ` Steve Kargl
2015-12-01 17:35 ` Bernhard Reutner-Fischer
2015-12-01 19:49 ` Steve Kargl
2015-12-01 17:28 ` David Malcolm
2015-12-01 17:51 ` Bernhard Reutner-Fischer
2015-12-01 17:58 ` David Malcolm
2015-12-01 20:00 ` Steve Kargl
2015-12-03 9:29 ` Janne Blomqvist
2015-12-03 13:53 ` Mikael Morin
2015-12-04 0:08 ` Steve Kargl
2015-12-05 19:53 ` Mikael Morin
2015-12-09 1:07 ` [PATCH] v2 " David Malcolm
2015-12-10 16:15 ` Tobias Burnus
2015-12-22 13:57 ` Fortran release notes (was: [PATCH] v2 ...) Gerald Pfeifer
2015-12-12 17:02 ` [PATCH] v2 Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE Bernhard Reutner-Fischer
2015-12-27 21:43 ` [PATCH, RFC, v2] " Bernhard Reutner-Fischer
2016-03-05 22:46 ` [PATCH, fortran, v3] " Bernhard Reutner-Fischer
2016-03-07 14:57 ` David Malcolm
2016-04-23 18:22 ` Bernhard Reutner-Fischer
2016-04-25 17:07 ` David Malcolm
2016-06-18 19:59 ` [PATCH, fortran, v4] " Bernhard Reutner-Fischer
2016-06-20 10:26 ` VandeVondele Joost
2016-07-03 22:46 ` Ping: [Re: [PATCH, fortran, v4] Use Levenshtein spelling suggestions in Fortran FE] Bernhard Reutner-Fischer
2016-07-04 3:31 ` Jerry DeLisle
2016-07-04 5:03 ` Janne Blomqvist
2017-10-19 7:26 ` Bernhard Reutner-Fischer
2017-10-19 7:51 ` [PATCH, fortran, v4] Use Levenshtein spelling suggestions in Fortran FE Bernhard Reutner-Fischer
2016-06-18 19:47 ` [PATCH] Use gfc_add_*_component defines where appropriate Bernhard Reutner-Fischer
2016-06-19 9:18 ` Paul Richard Thomas
2016-06-19 10:39 ` Bernhard Reutner-Fischer
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=20230413230440.229ebc2f@nbbrfq \
--to=rep.dot.nop@gmail.com \
--cc=blomqvist.janne@gmail.com \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
/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).