From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 116734 invoked by alias); 19 Sep 2018 14:40:17 -0000 Mailing-List: contact fortran-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: fortran-owner@gcc.gnu.org Received: (qmail 116715 invoked by uid 89); 19 Sep 2018 14:40:17 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM,KAM_NUMSUBJECT,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no version=3.3.2 spammy=appreciation, EXTERNAL, upe X-HELO: mail-ot1-f68.google.com Received: from mail-ot1-f68.google.com (HELO mail-ot1-f68.google.com) (209.85.210.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Sep 2018 14:40:15 +0000 Received: by mail-ot1-f68.google.com with SMTP id w17-v6so6013364otk.3; Wed, 19 Sep 2018 07:40:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=TstoZWH9pSMXSS9N8KpExA6c+zdOqj7xlUvn9ZM31K8=; b=FtlxWaoPISNRB8lnHQshZjEsLNisK/vhts9kU2DJVDsejhi/lPT+LmH6ywBSn/IAIY UV10wXqEWDFm3eHaSb/Q6Xe0VvgO3h6n6fmHkBQqX6W+P0BJI4vbkLpHmbvtsH4+3TBB zqvxNI6CG4r/XZrQReLnGAln2inQI6CsZVD+k/x5bYLhEhz+vBS5aujlNoHrTeST6FyG BinsLc8hnDjIZgWGbrx6hYPodvD2mA3pwUrU6I3tQebWj3sweRL7TdpOpRspAXbMYHK1 kwxCpAkWPDwu16tDPq0EajiFPuBmPooOGXXszaGCfXRjxfhMu1k+drY66mcUS6MamYPl ZkYA== MIME-Version: 1.0 References: <20180905145732.404-1-rep.dot.nop@gmail.com> In-Reply-To: From: Bernhard Reutner-Fischer Date: Wed, 19 Sep 2018 14:40:00 -0000 Message-ID: Subject: Re: [PATCH,FORTRAN 00/29] Move towards stringpool, part 1 To: Janne Blomqvist Cc: gfortran , GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00129.txt.bz2 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. > > 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,