From patchwork Sat Nov 12 01:46:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 60469 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 458BC3898C6F for ; Sat, 12 Nov 2022 01:47:43 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 3AC2638A9087 for ; Sat, 12 Nov 2022 01:46:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3AC2638A9087 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 7425022329; Sat, 12 Nov 2022 01:46:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1668217596; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=kp8hmCMaU8KYpnQ5mPDEqjal+ruxPJc8BhCn53TXyEE=; b=ONhD6sLdhzn5EsfsNzTUcEdvomeZ6aa/UJ/p2lBaB1d6rJL5tp7plk3dvSEHQAvl1byyZp sFi7BlRIyjZsxuQrnYbV0mVI4u6Fa1iZM7TIFGRQLqu3RL0YlT5yLqQPFMXZ5agpZFvs0N vXn/yoC0pEoxfv8aeksT28OqhEFJYIc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1668217596; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=kp8hmCMaU8KYpnQ5mPDEqjal+ruxPJc8BhCn53TXyEE=; b=FmWGd92NMGrllLo5HirOT4U1zf7BbSup/NY3hAzRcd2uyJWGw5KiL0FaUAGRhTSL7T2JTm j4sTlzQCkLzfyhBg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 642AD13A08; Sat, 12 Nov 2022 01:46:36 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id RwoAGPz6bmM8ZAAAMHmgww (envelope-from ); Sat, 12 Nov 2022 01:46:36 +0000 From: Martin Jambor To: GCC Patches Cc: Jan Hubicka , Jan Hubicka Subject: [PATCH 07/12] ipa-sra: Treat REFERENCE_TYPES as always dereferencable User-Agent: Notmuch/0.37 (https://notmuchmail.org) Emacs/28.1 (x86_64-suse-linux-gnu) Date: Sat, 12 Nov 2022 02:46:35 +0100 Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi, C++ and especially Fortran pass data by references which are not pointers potentially pointing anywhere and so can be assumed to be safely dereferencable. This patch teaches IPA-SRA to treat them as such and avoid the dance we do to prove that we can move loads from them to the caller. When we do not know that a dereference will happen all the time, we need a heuristics so that we do not force memory accesses that normally happen only rarely. The patch simply uses the (possibly guessed) profile and checks whether the (expected) number of loads is at least half of function invocations invocations. Bootstrapped and tested on x86_64-linux. OK for master? Thanks, Martin gcc/ChangeLog: 2022-11-11 Martin Jambor PR ipa/103585 * ipa-sra.c (struct gensum_param_access): New field load_count. (struct gensum_param_desc): New field safe_ref, adjusted comments. (by_ref_count): Renamed to unsafe_by_ref_count, adjusted all uses. (dump_gensum_access): Dump the new field. (dump_gensum_param_descriptor): Likewise. (create_parameter_descriptors): Set safe_ref field, move setting by_ref forward. Only increment unsafe_by_ref_count for unsafe by_ref parameters. (allocate_access): Initialize new field. (mark_param_dereference): Adjust indentation. Only add data to bb_dereferences for unsafe by_ref parameters. (scan_expr_access): For loads, accumulate BB counts. (dereference_probable_p): New function. (check_gensum_access): Fix leading comment, add parameter FUN. Check cumulative counts of loads for safe by_ref accesses instead of dereferences. (process_scan_results): Do not propagate dereference distances for safe by_ref parameters. Pass fun to check_gensum_access. Safe by_ref params do not need the postdominance check. gcc/testsuite/ChangeLog: 2022-11-11 Martin Jambor * g++.dg/ipa/ipa-sra-5.C: New test --- gcc/ipa-sra.cc | 100 +++++++++++++++++++-------- gcc/testsuite/g++.dg/ipa/ipa-sra-5.C | 23 ++++++ 2 files changed, 95 insertions(+), 28 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ipa/ipa-sra-5.C diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc index 0f137e810fe..e8a4cd47429 100644 --- a/gcc/ipa-sra.cc +++ b/gcc/ipa-sra.cc @@ -151,6 +151,8 @@ struct gensum_param_access arguments. */ tree alias_ptr_type; + /* Cumulative count of all loads. */ + profile_count load_count; /* Have there been writes to or reads from this exact location except for as arguments to a function call that can be tracked. */ bool nonarg; @@ -207,8 +209,13 @@ struct gensum_param_desc by reference that is a candidate for being converted to a set of parameters passing those data by value. */ bool split_candidate; - /* Is this a parameter passing stuff by reference? */ + /* Is this a parameter passing stuff by reference (either a pointer or a + source language reference type)? */ bool by_ref; + /* If this parameter passes stuff by reference, can it be safely dereferenced + without performing further checks (for example because it is a + REFERENCE_TYPE)? */ + bool safe_ref; /* The number of this parameter as they are ordered in function decl. */ int param_number; @@ -561,7 +568,7 @@ int aa_walking_limit; accessed in that BB. */ HOST_WIDE_INT *bb_dereferences = NULL; /* How many by-reference parameters there are in the current function. */ -int by_ref_count; +int unsafe_by_ref_count; /* Bitmap of BBs that can cause the function to "stop" progressing by returning, throwing externally, looping infinitely or calling a function @@ -643,6 +650,8 @@ dump_gensum_access (FILE *f, gensum_param_access *access, unsigned indent) print_generic_expr (f, access->type); fprintf (f, ", alias_ptr_type: "); print_generic_expr (f, access->alias_ptr_type); + fprintf (f, ", load_count: "); + access->load_count.dump (f); fprintf (f, ", nonarg: %u, reverse: %u\n", access->nonarg, access->reverse); for (gensum_param_access *ch = access->first_child; ch; @@ -692,7 +701,8 @@ dump_gensum_param_descriptor (FILE *f, gensum_param_desc *desc) return; } if (desc->by_ref) - fprintf (f, " by_ref with %u pass throughs\n", desc->ptr_pt_count); + fprintf (f, " %s by_ref with %u pass throughs\n", + desc->safe_ref ? "safe" : "unsafe", desc->ptr_pt_count); for (gensum_param_access *acc = desc->accesses; acc; acc = acc->next_sibling) dump_gensum_access (f, acc, 2); @@ -1140,6 +1150,8 @@ create_parameter_descriptors (cgraph_node *node, if (POINTER_TYPE_P (type)) { + desc->by_ref = true; + desc->safe_ref = (TREE_CODE (type) == REFERENCE_TYPE); type = TREE_TYPE (type); if (TREE_CODE (type) == FUNCTION_TYPE @@ -1187,7 +1199,6 @@ create_parameter_descriptors (cgraph_node *node, "nonarg uses\n"); continue; } - desc->by_ref = true; } else if (!AGGREGATE_TYPE_P (type)) { @@ -1231,8 +1242,8 @@ create_parameter_descriptors (cgraph_node *node, ret = true; desc->split_candidate = true; - if (desc->by_ref) - desc->deref_index = by_ref_count++; + if (desc->by_ref && !desc->safe_ref) + desc->deref_index = unsafe_by_ref_count++; } return ret; } @@ -1301,6 +1312,7 @@ allocate_access (gensum_param_desc *desc, memset (access, 0, sizeof (*access)); access->offset = offset; access->size = size; + access->load_count = profile_count::zero (); return access; } @@ -1555,15 +1567,16 @@ asm_visit_addr (gimple *, tree op, tree, void *) static void mark_param_dereference (gensum_param_desc *desc, HOST_WIDE_INT dist, - basic_block bb) + basic_block bb) { gcc_assert (desc->by_ref); gcc_checking_assert (desc->split_candidate); - if (bitmap_bit_p (final_bbs, bb->index)) + if (desc->safe_ref + || bitmap_bit_p (final_bbs, bb->index)) return; - int idx = bb->index * by_ref_count + desc->deref_index; + int idx = bb->index * unsafe_by_ref_count + desc->deref_index; if (bb_dereferences[idx] < dist) bb_dereferences[idx] = dist; } @@ -1811,6 +1824,8 @@ scan_expr_access (tree expr, gimple *stmt, isra_scan_context ctx, other. */ access->nonarg = true; } + else if (ctx == ISRA_CTX_LOAD && bb->count.initialized_p ()) + access->load_count += bb->count; if (!access->type) { @@ -2092,9 +2107,9 @@ dump_dereferences_table (FILE *f, struct function *fun, const char *str) if (bb != EXIT_BLOCK_PTR_FOR_FN (fun)) { int i; - for (i = 0; i < by_ref_count; i++) + for (i = 0; i < unsafe_by_ref_count; i++) { - int idx = bb->index * by_ref_count + i; + int idx = bb->index * unsafe_by_ref_count + i; fprintf (f, " %4" HOST_WIDE_INT_PRINT "d", bb_dereferences[idx]); } } @@ -2139,15 +2154,15 @@ propagate_dereference_distances (struct function *fun) if (bitmap_bit_p (final_bbs, bb->index)) continue; - for (i = 0; i < by_ref_count; i++) + for (i = 0; i < unsafe_by_ref_count; i++) { - int idx = bb->index * by_ref_count + i; + int idx = bb->index * unsafe_by_ref_count + i; bool first = true; HOST_WIDE_INT inh = 0; FOR_EACH_EDGE (e, ei, bb->succs) { - int succ_idx = e->dest->index * by_ref_count + i; + int succ_idx = e->dest->index * unsafe_by_ref_count + i; if (e->dest == EXIT_BLOCK_PTR_FOR_FN (fun)) continue; @@ -2184,13 +2199,23 @@ propagate_dereference_distances (struct function *fun) "Dereference table after propagation:\n"); } -/* Perform basic checks on ACCESS to PARM described by DESC and all its - children, return true if the parameter cannot be split, otherwise return - true and update *TOTAL_SIZE and *ONLY_CALLS. ENTRY_BB_INDEX must be the - index of the entry BB in the function of PARM. */ +/* Return true if the ACCESS loads happen frequently enough in FUN to risk + moving them to the caller and only pass the result. */ static bool -check_gensum_access (tree parm, gensum_param_desc *desc, +dereference_probable_p (struct function *fun, gensum_param_access *access) +{ + return access->load_count + >= ENTRY_BLOCK_PTR_FOR_FN (fun)->count.apply_scale (1, 2); +} + +/* Perform basic checks on ACCESS to PARM (of FUN) described by DESC and all + its children, return true if the parameter cannot be split, otherwise return + false and update *NONARG_ACC_SIZE and *ONLY_CALLS. ENTRY_BB_INDEX must be + the index of the entry BB in the function of PARM. */ + +static bool +check_gensum_access (struct function *fun, tree parm, gensum_param_desc *desc, gensum_param_access *access, HOST_WIDE_INT *nonarg_acc_size, bool *only_calls, int entry_bb_index) @@ -2218,19 +2243,31 @@ check_gensum_access (tree parm, gensum_param_desc *desc, if (desc->by_ref) { - int idx = (entry_bb_index * by_ref_count + desc->deref_index); - if ((access->offset + access->size) > bb_dereferences[idx]) + if (desc->safe_ref) { - disqualify_split_candidate (desc, "Would create a possibly " - "illegal dereference in a caller."); - return true; + if (!dereference_probable_p (fun, access)) + { + disqualify_split_candidate (desc, "Dereferences in callers " + "would happen much more frequently."); + return true; + } + } + else + { + int idx = (entry_bb_index * unsafe_by_ref_count + desc->deref_index); + if ((access->offset + access->size) > bb_dereferences[idx]) + { + disqualify_split_candidate (desc, "Would create a possibly " + "illegal dereference in a caller."); + return true; + } } } for (gensum_param_access *ch = access->first_child; ch; ch = ch->next_sibling) - if (check_gensum_access (parm, desc, ch, nonarg_acc_size, only_calls, + if (check_gensum_access (fun, parm, desc, ch, nonarg_acc_size, only_calls, entry_bb_index)) return true; @@ -2288,6 +2325,7 @@ process_scan_results (cgraph_node *node, struct function *fun, if (!dereferences_propagated && desc->by_ref + && !desc->safe_ref && desc->accesses) { propagate_dereference_distances (fun); @@ -2302,8 +2340,8 @@ process_scan_results (cgraph_node *node, struct function *fun, for (gensum_param_access *acc = desc->accesses; acc; acc = acc->next_sibling) - if (check_gensum_access (parm, desc, acc, &nonarg_acc_size, &only_calls, - entry_bb_index)) + if (check_gensum_access (fun, parm, desc, acc, &nonarg_acc_size, + &only_calls, entry_bb_index)) { check_failed = true; break; @@ -2394,6 +2432,12 @@ process_scan_results (cgraph_node *node, struct function *fun, if (!uses_memory_as_obtained) continue; + if (desc->safe_ref) + { + csum->m_arg_flow[argidx].safe_to_import_accesses = true; + continue; + } + /* Post-dominator check placed last, hoping that it usually won't be needed. */ if (!pdoms_calculated) @@ -4124,7 +4168,7 @@ ipa_sra_summarize_function (cgraph_node *node) cfun_pushed = true; final_bbs = BITMAP_ALLOC (NULL); bb_dereferences = XCNEWVEC (HOST_WIDE_INT, - by_ref_count + unsafe_by_ref_count * last_basic_block_for_fn (fun)); aa_walking_limit = opt_for_fn (node->decl, param_ipa_max_aa_steps); scan_function (node, fun); diff --git a/gcc/testsuite/g++.dg/ipa/ipa-sra-5.C b/gcc/testsuite/g++.dg/ipa/ipa-sra-5.C new file mode 100644 index 00000000000..26a221e1734 --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/ipa-sra-5.C @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-sra" } */ + +volatile int vi; + +static void __attribute__((noinline)) +foo (int c, int &r) +{ + int i; + if (c) + i = r; + else + i = 0; + vi = i; +} + +void +bar (int c, int j) +{ + foo (c, j); +} + +/* { dg-final { scan-ipa-dump "Will split parameter" "sra" } } */