From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89368 invoked by alias); 25 Sep 2018 12:59:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 89347 invoked by uid 89); 25 Sep 2018 12:59:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy=body's, streaming, bodys X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 25 Sep 2018 12:59:20 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id BE29FAF55; Tue, 25 Sep 2018 12:59:18 +0000 (UTC) From: Martin Jambor To: Cesar Philippidis , "gcc-patches\@gcc.gnu.org" Cc: Subject: Re: [patch,openacc] Fix PR71959: lto dump of callee counts In-Reply-To: <319b3ebd-c601-449b-718c-963b68414224@codesourcery.com> References: <319b3ebd-c601-449b-718c-963b68414224@codesourcery.com> User-Agent: Notmuch/0.26 (https://notmuchmail.org) Emacs/26.1 (x86_64-suse-linux-gnu) Date: Tue, 25 Sep 2018 13:01:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg01422.txt.bz2 Hi, I have noticed a few things... On Thu, Sep 20 2018, Cesar Philippidis wrote: > This is another old gomp4 patch that demotes an ICE in PR71959 to a > linker warning. One problem here is that it is not clear if OpenACC > allows individual member functions in C++ classes to be marked as acc > routines. There's another issue accessing member data inside offloaded > regions. We'll add some support for member data OpenACC 2.6, but some of > the OpenACC C++ semantics are still unclear. > > Is this OK for trunk? I bootstrapped and regtested it for x86_64 Linux > with nvptx offloading. > > Thanks, > Cesar > [PR71959] lto dump of callee counts > > 2018-XX-YY Nathan Sidwell > Cesar Philippidis > > gcc/ > * ipa-inline-analysis.c (inline_write_summary): Only dump callee > counts when dumping the function's body. ...the changelog needs updating and.... > > libgomp/ > * testsuite/libgomp.oacc-c++/pr71959.C: New. > * testsuite/libgomp.oacc-c++/pr71959-a.C: New. > > (cherry picked from gomp-4_0-branch r239788) > --- > gcc/ipa-fnsummary.c | 18 ++++++++--- > .../testsuite/libgomp.oacc-c++/pr71959-a.C | 31 +++++++++++++++++++ > libgomp/testsuite/libgomp.oacc-c++/pr71959.C | 31 +++++++++++++++++++ > 3 files changed, 75 insertions(+), 5 deletions(-) > create mode 100644 libgomp/testsuite/libgomp.oacc-c++/pr71959-a.C > create mode 100644 libgomp/testsuite/libgomp.oacc-c++/pr71959.C > > diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c > index 62095c6cf6f..e796b085e14 100644 > --- a/gcc/ipa-fnsummary.c > +++ b/gcc/ipa-fnsummary.c > @@ -3409,8 +3409,10 @@ ipa_fn_summary_write (void) > int i; > size_time_entry *e; > struct condition *c; > + int index = lto_symtab_encoder_encode (encoder, cnode); > + bool body = encoder->nodes[index].body; > > - streamer_write_uhwi (ob, lto_symtab_encoder_encode (encoder, cnode)); > + streamer_write_uhwi (ob, index); > streamer_write_hwi (ob, info->estimated_self_stack_size); > streamer_write_hwi (ob, info->self_size); > info->time.stream_out (ob); > @@ -3453,10 +3455,16 @@ ipa_fn_summary_write (void) > info->array_index->stream_out (ob); > else > streamer_write_uhwi (ob, 0); > - for (edge = cnode->callees; edge; edge = edge->next_callee) > - write_ipa_call_summary (ob, edge); > - for (edge = cnode->indirect_calls; edge; edge = edge->next_callee) > - write_ipa_call_summary (ob, edge); > + if (body) > + { > + /* Only write callee counts when we're emitting the > + body, as the reader only knows about the callees when > + the body's emitted. */ this comment is wrong because write_ipa_call_summary does not only stream counts but also sizes, predicates and other stuff. I also wonder 1) whether the cnode->definition test that guards this streaming should not be replaced by your body check (and why the definition is not enough, really) and 2) why you don't have the same problem with ipa_prop_write_jump_functions and the function it calls. Martin > + for (edge = cnode->callees; edge; edge = edge->next_callee) > + write_ipa_call_summary (ob, edge); > + for (edge = cnode->indirect_calls; edge; edge = edge->next_callee) > + write_ipa_call_summary (ob, edge); > + } > } > } > streamer_write_char_stream (ob->main_stream, 0);