From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x536.google.com (mail-ed1-x536.google.com [IPv6:2a00:1450:4864:20::536]) by sourceware.org (Postfix) with ESMTPS id 023E4385803D for ; Wed, 5 May 2021 13:03:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 023E4385803D Received: by mail-ed1-x536.google.com with SMTP id i24so1907844edy.8 for ; Wed, 05 May 2021 06:03:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=tlk3SUBKeWf7jWqoe/gQr1ddx+gmbJGkWZqW4vWUIf0=; b=jkwb7TG3WyE+ZKPjIyS4ixJdIf3QsI99rzIFqBpFVHU3nu4smlW2AGPHSkKxliisHf y5T9Sp+3bK6JTvTe/RFteef5tQbGF+R1bselJ8Tbwn4U/JqosxLBszlTK1EOvVOAJZ1F O60fjnckzzYxdJtcLDeo9ZoMpNk/B7tpBLaGQXnqKfxDz963LFtuU1k1aex8SV1N8n1n nsagXIdh7myis+B5Qd/OVqKe/bIgtZ8Gmq89hKaWiSVfPg+lRiMJtyQCJAZCW+u+5sGm frl7BUCVM2ViEqRO2vs3quROBBg9e4+C3UE0n3jB0rDjywtGwrQ1/8mvlPhKO9DruyJq ZAzw== X-Gm-Message-State: AOAM532ylC5TU1upw0BiGznON62zheTmo9h80FRa0HJHw2nT0hhAvjOd +p8uefpQQNB3PgV1+vrEc58SrtpeODCBKX8Jqxk= X-Google-Smtp-Source: ABdhPJxGeDp1OKDIpE7wPMP3gPNY/fI1A2y9+Sg6i8qJOta78Q4+EMBGfi1QvOfFVdoqShXOIK/BPD8o2Qy6ovJO8pI= X-Received: by 2002:aa7:dd41:: with SMTP id o1mr16421260edw.361.1620219784959; Wed, 05 May 2021 06:03:04 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Wed, 5 May 2021 15:02:53 +0200 Message-ID: Subject: Re: [PATCH] ipa-sra: Do not bail out when callers cannot be cloned To: Martin Jambor Cc: GCC Patches , Jan Hubicka Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 May 2021 13:03:07 -0000 On Tue, Apr 27, 2021 at 5:29 PM Martin Jambor wrote: > > Hi, > > IPA-SRA fails to produce (very simple) edge summaries when a caller > cannot be cloned or its signature cannot be changed which makes it > less powerful for no good reason. This patch fixes that problem. > > Bootstrapped, LTO-bootstrapped and tested on x86_64-linux. OK for trunk? OK. > A common reason why we think that a function cannot change its > signature is presence of function type attributes. I dumped those > that caused this in our testsuite on x86_64 and got: > > - access > - alloc_align > - alloc_size > - externally_visible > - fn spec > - force_align_arg_pointer > - format > - interrupt > - ms_abi > - no_caller_saved_registers > - nocf_check > - nonnull > - regparm > - returns_nonnull > - sysv_abi > - transaction_callable > - transaction_may_cancel_outer > - transaction_pure > - transaction_safe > - transaction_unsafe > - type generic > - warn_unused_result > > and on an Aarch64 I have also seen aarch64_vector_pcs. > > Allowing to clone and modify functions with some of these parameters > like returns_nonull should be easy (it still should probably be > dropped if the clone does not return anything at all) while doing so > for attributes like fnspec would need massaging their value. I am not > sure what to think of the transaction related ones. I guess one can classify attributes in those that can survive regardless of signature changes, those that can be safely dropped (safely as in no wrong-code, only missed optimizations) and those that can be transformed (fixing missed optimizations). I suppose some global attribute_handler[] indexed by some new attribute key enum providing a class interface so an attribute can implement its own IPA signature transform method (defaulting to dropping) and predicate on whether it wants to be exempted from IPA transforms (default to true) would be nice to have. Unfortunately attribute handling is scattered amongst frontends at the moment... It would also finally allow the attributes to be stored with their ID, not requiring string compares and eventually allowing a better data structure for searching. > (I have seen IPA-SRA remove unused return values of functions marked > as malloc, so the alloc_align could also be dealt with but I am not > sure how important it is.) > > Thanks, > > Martin > > > gcc/ChangeLog: > > 2021-04-12 Martin Jambor > > * ipa-sra.c (ipa_sra_dump_all_summaries): Dump edge summaries even > when there is no function summary. > (ipa_sra_summarize_function): produce edge summaries even when > bailing out early. > > gcc/testsuite/ChangeLog: > > 2021-04-12 Martin Jambor > > * gcc.dg/ipa/ipa-sra-1.c (main): Revert change done by > 05193687dde, make the argv again pointer to an array. > --- > gcc/ipa-sra.c | 45 +++++++++++++++------------- > gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c | 2 +- > 2 files changed, 25 insertions(+), 22 deletions(-) > > diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c > index 7a89906cee6..3f90d4d81b6 100644 > --- a/gcc/ipa-sra.c > +++ b/gcc/ipa-sra.c > @@ -2795,27 +2795,27 @@ ipa_sra_dump_all_summaries (FILE *f) > > isra_func_summary *ifs = func_sums->get (node); > if (!ifs) > - { > - fprintf (f, " Function does not have any associated IPA-SRA " > - "summary\n"); > - continue; > - } > - if (!ifs->m_candidate) > - { > - fprintf (f, " Not a candidate function\n"); > - continue; > - } > - if (ifs->m_returns_value) > - fprintf (f, " Returns value\n"); > - if (vec_safe_is_empty (ifs->m_parameters)) > - fprintf (f, " No parameter information. \n"); > + fprintf (f, " Function does not have any associated IPA-SRA " > + "summary\n"); > else > - for (unsigned i = 0; i < ifs->m_parameters->length (); ++i) > - { > - fprintf (f, " Descriptor for parameter %i:\n", i); > - dump_isra_param_descriptor (f, &(*ifs->m_parameters)[i]); > - } > - fprintf (f, "\n"); > + { > + if (!ifs->m_candidate) > + { > + fprintf (f, " Not a candidate function\n"); > + continue; > + } > + if (ifs->m_returns_value) > + fprintf (f, " Returns value\n"); > + if (vec_safe_is_empty (ifs->m_parameters)) > + fprintf (f, " No parameter information. \n"); > + else > + for (unsigned i = 0; i < ifs->m_parameters->length (); ++i) > + { > + fprintf (f, " Descriptor for parameter %i:\n", i); > + dump_isra_param_descriptor (f, &(*ifs->m_parameters)[i]); > + } > + fprintf (f, "\n"); > + } > > struct cgraph_edge *cs; > for (cs = node->callees; cs; cs = cs->next_callee) > @@ -4063,7 +4063,10 @@ ipa_sra_summarize_function (cgraph_node *node) > fprintf (dump_file, "Creating summary for %s/%i:\n", node->name (), > node->order); > if (!ipa_sra_preliminary_function_checks (node)) > - return; > + { > + isra_analyze_all_outgoing_calls (node); > + return; > + } > gcc_obstack_init (&gensum_obstack); > isra_func_summary *ifs = func_sums->get_create (node); > ifs->m_candidate = true; > diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c b/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c > index df7e356daf3..4a22e3978f9 100644 > --- a/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c > +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c > @@ -24,7 +24,7 @@ ox (struct bovid cow) > } > > int > -main (int argc, char **argv) > +main (int argc, char *argv[]) > { > struct bovid cow; > > -- > 2.31.1 >