From patchwork Thu Apr 9 17:57:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 1268736 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.cz Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48yplJ4sS4z9sPF for ; Fri, 10 Apr 2020 03:57:47 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4FA9F385DC12; Thu, 9 Apr 2020 17:57:43 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 9FDB4385DC12 for ; Thu, 9 Apr 2020 17:57:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9FDB4385DC12 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mjambor@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 88556AC19; Thu, 9 Apr 2020 17:57:38 +0000 (UTC) From: Martin Jambor To: bule Subject: [PATCH] ipa-sra: Fix treatment of internal functions (PR 94434) In-Reply-To: <8FB10B4CA53A144E918874949D50CEB1022B336A@dggeml528-mbx.china.huawei.com> References: <8FB10B4CA53A144E918874949D50CEB1022B24E3@dggeml528-mbx.china.huawei.com> <8FB10B4CA53A144E918874949D50CEB1022B336A@dggeml528-mbx.china.huawei.com> User-Agent: Notmuch/0.29.3 (https://notmuchmail.org) Emacs/26.3 (x86_64-suse-linux-gnu) Date: Thu, 09 Apr 2020 19:57:37 +0200 Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-3054.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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: , Cc: "gcc-patches@gcc.gnu.org" , Jan Hubicka Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi, On Tue, Apr 07 2020, bule wrote: > Hi, > > The patch is tested and works fine. It is more appropriate to handle > the context by considering it as a section of assemble code. > > A minor question is that I think svst functions are for store >operations. Why pass ISRA_CTX_LOAD to scan_expr_access rather than >ISRA_CTX_STORE? > That's a good point, I did not immediately realize that these were writes. IPA-SRA really distinguishes between reads and writes only when it comes to parameters passed by reference but I agree that we should not lie to the bit of the compiler (if we can avoid it :-). Therefore I'd like to fix this issue with the patch below. I'd encourage anyone with experience with adding SVE testcases to add one specifically for this. Thanks, Martin ipa-sra: Fix treatment of internal functions (PR 94434) IPA-SRA can segfault when processing a coll to an internal function because such calls do not have corresponding call graphs and should be treated like memory accesses anyway, which what the patch below does. The patch makes an attempt to differentiate between reads and writes, although for things passed by value it does not make any difference. It treats all arguments of functions denoted with internal_store_fn_p as written to, even though in practice only some are, but for IPA-SRA purposes, actions needed to be taken when processing a read are also always performed when analyzing a write, so the code is just slightly pessimistic. But not as pessimistic as bailing out on any internal call immediately. I have LTO bootstrapped and tested the patch on x86_64-linux and normally bootstrapped and tested it on aarch64-linux, although one which is not SVE capable. I would appreciate testing on such machine too - as well as a small testcase that would follow all relevant conventions in gcc.target/aarch64/sve. OK for trunk? 2020-04-09 Martin Jambor PR ipa/94434 * ipa-sra.c: Include internal-fn.h. (enum isra_scan_context): Update comment. (scan_function): Treat calls to internal_functions like loads or stores. --- gcc/ChangeLog | 7 +++++++ gcc/ipa-sra.c | 26 ++++++++++++++++++++------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index e10fb251c16..a838634b707 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2020-04-09 Martin Jambor + + PR ipa/94434 + * ipa-sra.c: Include internal-fn.h. + (enum isra_scan_context): Update comment. + (scan_function): Treat calls to internal_functions like loads or stores. + 2020-04-05 Zachary Spytz * extend.texi: Add free to list of ISO C90 functions that diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c index f0ebaec708d..7c922e40a4e 100644 --- a/gcc/ipa-sra.c +++ b/gcc/ipa-sra.c @@ -83,7 +83,7 @@ along with GCC; see the file COPYING3. If not see #include "builtins.h" #include "cfganal.h" #include "tree-streamer.h" - +#include "internal-fn.h" /* Bits used to track size of an aggregate in bytes interprocedurally. */ #define ISRA_ARG_SIZE_LIMIT_BITS 16 @@ -1281,7 +1281,9 @@ allocate_access (gensum_param_desc *desc, } /* In what context scan_expr_access has been called, whether it deals with a - load, a function argument, or a store. */ + load, a function argument, or a store. Please note that in rare + circumstances when it is not clear if the access is a load or store, + ISRA_CTX_STORE is used too. */ enum isra_scan_context {ISRA_CTX_LOAD, ISRA_CTX_ARG, ISRA_CTX_STORE}; @@ -1870,15 +1872,27 @@ scan_function (cgraph_node *node, struct function *fun) case GIMPLE_CALL: { unsigned argument_count = gimple_call_num_args (stmt); - scan_call_info call_info; - call_info.cs = node->get_edge (stmt); - call_info.argument_count = argument_count; + isra_scan_context ctx = ISRA_CTX_ARG; + scan_call_info call_info, *call_info_p = &call_info; + if (gimple_call_internal_p (stmt)) + { + call_info_p = NULL; + ctx = ISRA_CTX_LOAD; + internal_fn ifn = gimple_call_internal_fn (stmt); + if (internal_store_fn_p (ifn)) + ctx = ISRA_CTX_STORE; + } + else + { + call_info.cs = node->get_edge (stmt); + call_info.argument_count = argument_count; + } for (unsigned i = 0; i < argument_count; i++) { call_info.arg_idx = i; scan_expr_access (gimple_call_arg (stmt, i), stmt, - ISRA_CTX_ARG, bb, &call_info); + ctx, bb, call_info_p); } tree lhs = gimple_call_lhs (stmt);