From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by sourceware.org (Postfix) with ESMTPS id B07263858D20; Thu, 13 Apr 2023 21:04:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B07263858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-x429.google.com with SMTP id o29so4430536wro.0; Thu, 13 Apr 2023 14:04:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1681419885; x=1684011885; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=EjU9S6si/Jbl13L12tXAxZbUGXbUef27gbBv/mnBqs8=; b=pZWeRkI6w0VPH7IsMJ0fGP3St/eYDQs4hagN1CphdbhOfq50ZaLKUUB42LNLFoncP/ Myxw7oOfj6g7yRCShB28lXPBpGd5BuAyzleSEmyg/OClBO1s7sCKGNBLavThGATpbwAU gOeWDgL8KU+RdUzagxMZF7TceO3lDdOFMgqhookaKwvtMqHrgVx/TV+/enOqEVc9gQms ZcEd2yJU8jQPNq/scSnrem/lSrDzXaJk2wpO7iptDSsUIOOIErxSR6v5d7hgszVbSUXR GK1Q/D6dBDHH/CnygxcNXZ+MFOxlo2crIvAUHu/zxCyvfF1O2j6T+ujYtSsRfnR6yXkK X9Cw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681419885; x=1684011885; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=EjU9S6si/Jbl13L12tXAxZbUGXbUef27gbBv/mnBqs8=; b=jntwEM9Y2dN/CqH8ML3ZVZOa/JkQKPt3yHlxrZ2yrM+Br/ujn9WMyNC96iXYEihMyX r6e7GwfHcB/9s1eywcfUhNnzIOW/Fc2ZswWC9YaI55SVneEY03PZV/VcuNgaQqgBVMIV u/LF4EZEktZ3YG66Ai5u3vzD1f4gOfDZ4WcK+HPZKK8ayDxnivbPx7/UYhrXqo699wE4 Kv+6Xz75BprsmAldAavvO+atfBAirz9WF2PollRS5FjRjWpIEFNUHqo3nXKy9ck9UIuF BqYXnR+mWyIK5BmzOjg6eQf9ciUGkU+SuQCzNrk+wABh8iN9nQkugR1m7XYWAjX9JXO4 O7dQ== X-Gm-Message-State: AAQBX9cWzG1AdfgIgC59FVVflaIQc9i3PisWIb8bsG7Asj456OTy0bQz G/063n1kB+00tz8nB9ImC8Q= X-Google-Smtp-Source: AKy350YwukDYvj3WMLrdXz9w1F6ysjpBzRBxDraLMmxO/OAiXr7d54WTxM2q7kFWDTdyEVzgSSPCXA== X-Received: by 2002:a5d:6a87:0:b0:2ce:a8e9:bb3a with SMTP id s7-20020a5d6a87000000b002cea8e9bb3amr2473644wru.4.1681419885257; Thu, 13 Apr 2023 14:04:45 -0700 (PDT) Received: from nbbrfq (80-110-214-113.static.upcbusiness.at. [80.110.214.113]) by smtp.gmail.com with ESMTPSA id q17-20020adfdfd1000000b002e4cd2ec5c7sm2027158wrn.86.2023.04.13.14.04.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Apr 2023 14:04:44 -0700 (PDT) Date: Thu, 13 Apr 2023 23:04:40 +0200 From: Bernhard Reutner-Fischer To: Janne Blomqvist Cc: rep.dot.nop@gmail.com, gfortran , GCC Patches Subject: Re: [PATCH,FORTRAN 00/29] Move towards stringpool, part 1 Message-ID: <20230413230440.229ebc2f@nbbrfq> In-Reply-To: References: <20180905145732.404-1-rep.dot.nop@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_NUMSUBJECT,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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.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,