From patchwork Tue Jan 11 10:45:00 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 49831 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 9A79C38A9400 for ; Tue, 11 Jan 2022 10:45:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9A79C38A9400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1641897947; bh=kRooyZXHQSNF8kXGSiZr+8NIrpsjSNU26P461cKH6pE=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=LaXedNXh87IhsWDEZT+LA9zEVG1gYcI4WyejQJLVmpg8VRkuGcHs4XStY7jXPVXj+ n2goT0cgIOssy1Pyp9N2srIZJIfX+Yj+e31qEa9F4PLJXiSkfmivg+xuvkMmCQdt20 3Xhpusx5aaQViUrfKfLfrHqr/68FDMZyNL6UYXf0= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by sourceware.org (Postfix) with ESMTPS id 4BEFF3858D39 for ; Tue, 11 Jan 2022 10:45:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4BEFF3858D39 Received: by mail-wm1-x335.google.com with SMTP id v123so10685305wme.2 for ; Tue, 11 Jan 2022 02:45:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=kRooyZXHQSNF8kXGSiZr+8NIrpsjSNU26P461cKH6pE=; b=DQnfZF+3kWavJLhB7AmO44NS+Wb6cp346rKCaC7HRa6vSNfxJGEacashgS9SOtx5MP tst5CmnynuVOQnhHHMeW1abIfH0yKX8u8nDJ2NXGNYbv157fKRIU0GgR13Uw7dobx/85 i9QZ0VgLaPBO5L95daOz+zl9q8IZnp6Mv7Jl8EbEO38z+hUCdqDC6exG1S+FsayV41Lb FOsJTYjnbjP6v/gj8fCe8a7LpkfXSmvlsvV04/YuSlOr5OsjP8Ew/VK1LffkePY8ofob 0C+lepnFBm9CC1/VmdD4tt4y7LF28ekVNFpJbshlgh8BrIMj5erP8lGreMPVQQznGbjL wKig== X-Gm-Message-State: AOAM533fpS5gPi1CrbL9thnFmLCNPmSpF1SBBFi7Vp1aXWTfIT5T7Kj3 PHYysqwkgYh4AmWOsCVZenyqK+iFcw3Ch8AR X-Google-Smtp-Source: ABdhPJwpEglQi412PM/9sItAoqBkyPlf/YsSigV+56q9UtWNX7eSOO0Iz9mxEn2h92bd2lMTirXy3Q== X-Received: by 2002:a05:600c:2299:: with SMTP id 25mr1919999wmf.52.1641897917055; Tue, 11 Jan 2022 02:45:17 -0800 (PST) Received: from fomalhaut.localnet ([2a01:e0a:41b:84f0:cf71:f5e0:b050:bede]) by smtp.gmail.com with ESMTPSA id bg19sm1539075wmb.47.2022.01.11.02.45.16 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Jan 2022 02:45:16 -0800 (PST) X-Google-Original-From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: [patch] Fix reverse SSO issues in IPA-SRA Date: Tue, 11 Jan 2022 11:45:00 +0100 Message-ID: <1805634.tdWV9SEqCh@fomalhaut> MIME-Version: 1.0 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Eric Botcazou via Gcc-patches From: Eric Botcazou Reply-To: Eric Botcazou Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi, we recently received the report that the IPA-SRA pass introduced in GCC 10 does not always play nice with the reverse scalar storage order that can be used in structures/records/unions. Reading the code, the pass apparently correctly detects it but fails to propagate the information to the rewriting phase in some cases and, in particular, does not stream it for LTO. The attached patch is a tentative fix for these issues spotted essentially by code reading. It also contains various minor tweaks left and right. Bootstrapped/regtested on x86-64/Linux, OK for mainline, 11 and 10 branches? 2022-01-11 Eric Botcazou * ipa-param-manipulation.c (ipa_dump_adjusted_parameters): Dump reverse flag as "reverse" for the sake of consistency. * ipa-sra.c: Fix copyright year. (ipa_sra_function_summaries::duplicate): Copy the reverse flag. (dump_isra_access): Remove confusing dump line. (isra_write_node_summary): Write the reverse flag. (isra_read_node_info): Read it. (pull_accesses_from_callee): Copy it. diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c index 4973bfb67dd..fa6815e0941 100644 --- a/gcc/ipa-param-manipulation.c +++ b/gcc/ipa-param-manipulation.c @@ -228,7 +228,7 @@ ipa_dump_adjusted_parameters (FILE *f, fprintf (f, " prefix: %s", ipa_param_prefixes[apm->param_prefix_index]); if (apm->reverse) - fprintf (f, ", reverse-sso"); + fprintf (f, ", reverse"); break; } fprintf (f, "\n"); diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c index 45030a17c07..c24812b971d 100644 --- a/gcc/ipa-sra.c +++ b/gcc/ipa-sra.c @@ -1,6 +1,5 @@ /* Interprocedural scalar replacement of aggregates - Copyright (C) 2008-2022 Free Software Foundation, Inc. - + Copyright (C) 2019-2022 Free Software Foundation, Inc. Contributed by Martin Jambor This file is part of GCC. @@ -21,7 +20,7 @@ along with GCC; see the file COPYING3. If not see /* IPA-SRA is an interprocedural pass that removes unused function return values (turning functions returning a value which is never used into void - functions), removes unused function parameters. It can also replace an + functions) and removes unused function parameters. It can also replace an aggregate parameter by a set of other parameters representing part of the original, turning those passed by reference into new ones which pass the value directly. @@ -57,7 +56,6 @@ along with GCC; see the file COPYING3. If not see ipa-param-manipulation.h for more details. */ - #include "config.h" #include "system.h" #include "coretypes.h" @@ -93,7 +91,7 @@ static void ipa_sra_summarize_function (cgraph_node *); #define ISRA_ARG_SIZE_LIMIT_BITS 16 #define ISRA_ARG_SIZE_LIMIT (1 << ISRA_ARG_SIZE_LIMIT_BITS) /* How many parameters can feed into a call actual argument and still be - tracked. */ + tracked. */ #define IPA_SRA_MAX_PARAM_FLOW_LEN 7 /* Structure describing accesses to a specific portion of an aggregate @@ -122,7 +120,7 @@ struct GTY(()) param_access transformed function - initially not set for portions of formal parameters that are only used as actual function arguments passed to callees. */ unsigned certain : 1; - /* Set if the access has a reversed scalar storage order. */ + /* Set if the access has reverse scalar storage order. */ unsigned reverse : 1; }; @@ -156,7 +154,7 @@ struct gensum_param_access arguments to a function call that can be tracked. */ bool nonarg; - /* Set if the access has a reversed scalar storage order. */ + /* Set if the access has reverse scalar storage order. */ bool reverse; }; @@ -219,8 +217,8 @@ struct gensum_param_desc }; /* Properly deallocate accesses of DESC. TODO: Since this data structure is - not in GC memory, this is not necessary and we can consider removing the - function. */ + allocated in GC memory, this is not necessary and we can consider removing + the function. */ static void free_param_decl_accesses (isra_param_desc *desc) @@ -275,9 +273,9 @@ public: unsigned m_queued : 1; }; -/* Clean up and deallocate isra_func_summary points to. TODO: Since this data - structure is not in GC memory, this is not necessary and we can consider - removing the destructor. */ +/* Deallocate the memory pointed to by isra_func_summary. TODO: Since this + data structure is allocated in GC memory, this is not necessary and we can + consider removing the destructor. */ isra_func_summary::~isra_func_summary () { @@ -287,7 +285,6 @@ isra_func_summary::~isra_func_summary () vec_free (m_parameters); } - /* Mark the function as not a candidate for any IPA-SRA transformation. Return true if it was a candidate until now. */ @@ -297,6 +294,7 @@ isra_func_summary::zap () bool ret = m_candidate; m_candidate = false; + /* TODO: see the destructor above. */ unsigned len = vec_safe_length (m_parameters); for (unsigned i = 0; i < len; ++i) free_param_decl_accesses (&(*m_parameters)[i]); @@ -306,7 +304,7 @@ isra_func_summary::zap () } /* Structure to describe which formal parameters feed into a particular actual - arguments. */ + argument. */ struct isra_param_flow { @@ -426,6 +424,7 @@ ipa_sra_function_summaries::duplicate (cgraph_node *, cgraph_node *, to->unit_offset = from->unit_offset; to->unit_size = from->unit_size; to->certain = from->certain; + to->reverse = from->reverse; d->accesses->quick_push (to); } } @@ -552,7 +551,7 @@ namespace { hash_map *decl2desc; -/* Countdown of allowed Alias analysis steps during summary building. */ +/* Countdown of allowed Alias Analysis steps during summary building. */ int aa_walking_limit; @@ -664,8 +663,6 @@ dump_isra_access (FILE *f, param_access *access) print_generic_expr (f, access->alias_ptr_type); if (access->certain) fprintf (f, ", certain"); - else - fprintf (f, ", not-certain"); if (access->reverse) fprintf (f, ", reverse"); fprintf (f, "\n"); @@ -927,8 +924,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, This function is similar to ptr_parm_has_nonarg_uses but its results are meant for unused parameter removal, as opposed to splitting of parameters - passed by reference or converting them to passed by value. - */ + passed by reference or converting them to passed by value. */ static bool isra_track_scalar_param_local_uses (function *fun, cgraph_node *node, tree parm, @@ -968,8 +964,7 @@ isra_track_scalar_param_local_uses (function *fun, cgraph_node *node, tree parm, This function is similar to isra_track_scalar_param_local_uses but its results are meant for splitting of parameters passed by reference or turning them into bits passed by value, as opposed to generic unused parameter - removal. - */ + removal. */ static bool ptr_parm_has_nonarg_uses (cgraph_node *node, function *fun, tree parm, @@ -1650,7 +1645,7 @@ record_nonregister_call_use (gensum_param_desc *desc, } /* Callback of walk_aliased_vdefs, just mark that there was a possible - modification. */ + modification. */ static bool mark_maybe_modified (ao_ref *, tree, void *data) @@ -2195,7 +2190,7 @@ static bool check_gensum_access (tree parm, gensum_param_desc *desc, gensum_param_access *access, HOST_WIDE_INT *nonarg_acc_size, bool *only_calls, - int entry_bb_index) + int entry_bb_index) { if (access->nonarg) { @@ -2363,8 +2358,8 @@ process_scan_results (cgraph_node *node, struct function *fun, offset in this function at IPA level. TODO: Measure the overhead and the effect of just being pessimistic. - Maybe this is only -O3 material? - */ + Maybe this is only -O3 material? */ + bool pdoms_calculated = false; if (check_pass_throughs) for (cgraph_edge *cs = node->callees; cs; cs = cs->next_callee) @@ -2584,6 +2579,7 @@ isra_write_node_summary (output_block *ob, cgraph_node *node) streamer_write_uhwi (ob, acc->unit_size); bitpack_d bp = bitpack_create (ob->main_stream); bp_pack_value (&bp, acc->certain, 1); + bp_pack_value (&bp, acc->reverse, 1); streamer_write_bitpack (&bp); } streamer_write_uhwi (ob, desc->param_size_limit); @@ -2702,6 +2698,7 @@ isra_read_node_info (struct lto_input_block *ib, cgraph_node *node, acc->unit_size = streamer_read_uhwi (ib); bitpack_d bp = streamer_read_bitpack (ib); acc->certain = bp_unpack_value (&bp, 1); + acc->reverse = bp_unpack_value (&bp, 1); vec_safe_push (desc->accesses, acc); } desc->param_size_limit = streamer_read_uhwi (ib); @@ -3161,7 +3158,7 @@ isra_mark_caller_param_used (isra_func_summary *from_ifs, int input_idx, /* Propagate information that any parameter is not used only locally within a SCC across CS to the caller, which must be in the same SCC as the - callee. Push any callers that need to be re-processed to STACK. */ + callee. Push any callers that need to be re-processed to STACK. */ static void propagate_used_across_scc_edge (cgraph_edge *cs, vec *stack) @@ -3199,7 +3196,7 @@ propagate_used_across_scc_edge (cgraph_edge *cs, vec *stack) /* Propagate information that any parameter is not used only locally within a SCC (i.e. is used also elsewhere) to all callers of NODE that are in the - same SCC. Push any callers that need to be re-processed to STACK. */ + same SCC. Push any callers that need to be re-processed to STACK. */ static bool propagate_used_to_scc_callers (cgraph_node *node, void *data) @@ -3349,6 +3346,7 @@ pull_accesses_from_callee (cgraph_node *caller, isra_param_desc *param_desc, copy->type = argacc->type; copy->alias_ptr_type = argacc->alias_ptr_type; copy->certain = true; + copy->reverse = argacc->reverse; vec_safe_push (param_desc->accesses, copy); } else if (prop_kinds[j] == ACC_PROP_CERTAIN) @@ -3610,7 +3608,6 @@ retval_used_p (cgraph_node *node, void *) PREV_ADJUSTMENT. If the parent clone is the original function, PREV_ADJUSTMENT is NULL and PREV_CLONE_INDEX is equal to BASE_INDEX. */ - static void push_param_adjustments_for_index (isra_func_summary *ifs, unsigned base_index, unsigned prev_clone_index,