Hi all, Janne!
On Wed, 19 Sep 2018 16:40:01 +0200
Bernhard Reutner-Fischer wrote:
> On Fri, 7 Sep 2018 at 10:07, Bernhard Reutner-Fischer
> wrote:
> >
> > On Wed, 5 Sep 2018 at 20:57, Janne Blomqvist
> > wrote:
> > >
> > > On Wed, Sep 5, 2018 at 5:58 PM Bernhard Reutner-Fischer
> > > 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@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@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 attr
