From patchwork Tue Jun 15 15:09:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 1492285 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=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=suse.cz header.i=@suse.cz header.a=rsa-sha256 header.s=susede2_rsa header.b=qkNiuppU; dkim=pass header.d=suse.cz header.i=@suse.cz header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=RyvuE6bF; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (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 4G4BZD0Wtpz9sW7 for ; Wed, 16 Jun 2021 01:09:56 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A4CC439730C1 for ; Tue, 15 Jun 2021 15:09:53 +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 95C0338618FC for ; Tue, 15 Jun 2021 15:09:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 95C0338618FC 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 relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 44D5621A46 for ; Tue, 15 Jun 2021 15:09:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1623769780; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=A4WjRUCizOSLAmdrB46Vgs0qCU2iKwiX2FEJYmW9Cos=; b=qkNiuppUpIf4wWR88s6Z45vPEVDZMXeX10yZqv+PP6eWK+gtrLKaPTXjXWhWpLLQnPRpC8 ah4ChtvfRfa8cW2SIXNTeeJf8HLZaHsChmjTOz1Elk+6IGvrWlxsyvHXDS4An8WbRipHTQ 6gBD1e0V0AE+0K77+uFcsX3cDM18IEo= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1623769780; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=A4WjRUCizOSLAmdrB46Vgs0qCU2iKwiX2FEJYmW9Cos=; b=RyvuE6bFOrszLvVFzGxcJ4JIdEfT2j6uyegj5LnNGw5JJyW2daYQjNE64LSG7djpWWrUpz 0MV+uzhELH8OSYCQ== Received: from suse.cz (virgil.suse.cz [10.100.13.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 32B5CA3B89; Tue, 15 Jun 2021 15:09:40 +0000 (UTC) From: Martin Jambor To: GCC Patches Subject: [PATCH] tree-sra: Do not refresh readonly decls (PR 100453) User-Agent: Notmuch/0.32 (https://notmuchmail.org) Emacs/27.2 (x86_64-suse-linux-gnu) Date: Tue, 15 Jun 2021 17:09:40 +0200 Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-11.5 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.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: Richard Biener Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Hi, When SRA transforms an assignment where the RHS is an aggregate decl that it creates replacements for, the (least efficient) fallback method of dealing with them is to store all the replacements back into the original decl and then let the original assignment takes its course. That of course should not need to be done for TREE_READONLY bases which cannot change contents. The SRA code handled this situation only for DECL_IN_CONSTANT_POOL const decls, this patch modifies the check so that it tests for TREE_READONLY and I also looked at all other callers of generate_subtree_copies and added checks to another one dealing with the same exact situation and one which deals with it in a non-assignment context. This behavior also means that SRA has to disqualify any candidate decl that is read-only and written to. I plan to continue to hunt down at least some of such occurrences. Bootstrapped and tested on x86_64-linux, i686-linux and aarch64-linux (this time With Ada enabled on all three platforms). OK for trunk? Thanks, Martin gcc/ChangeLog: 2021-06-11 Martin Jambor PR tree-optimization/100453 * tree-sra.c (create_access): Disqualify any const candidates which are written to. (sra_modify_expr): Do not store sub-replacements back to a const base. (handle_unscalarized_data_in_subtree): Likewise. (sra_modify_assign): Likewise. Earlier, use TREE_READONLy test instead of constant_decl_p. gcc/testsuite/ChangeLog: 2021-06-11 Martin Jambor PR tree-optimization/100453 * gcc.dg/tree-ssa/pr100453.c: New test. --- gcc/testsuite/gcc.dg/tree-ssa/pr100453.c | 18 ++++++++++++++++++ gcc/tree-sra.c | 21 +++++++++++++++++---- 2 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr100453.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr100453.c b/gcc/testsuite/gcc.dg/tree-ssa/pr100453.c new file mode 100644 index 00000000000..0cf0ad23815 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr100453.c @@ -0,0 +1,18 @@ +/* { dg-do run } */ +/* { dg-options "-O1" } */ + +struct a { + int b : 4; +} d; +static int c, e; +static const struct a f; +static void g(const struct a h) { + for (; c < 1; c++) + d = h; + e = h.b; + c = h.b; +} +int main() { + g(f); + return 0; +} diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index 8dfc923ed7e..5e86d3fbb9d 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -915,6 +915,12 @@ create_access (tree expr, gimple *stmt, bool write) if (!DECL_P (base) || !bitmap_bit_p (candidate_bitmap, DECL_UID (base))) return NULL; + if (write && TREE_READONLY (base)) + { + disqualify_candidate (base, "Encountered a store to a read-only decl."); + return NULL; + } + HOST_WIDE_INT offset, size, max_size; if (!poffset.is_constant (&offset) || !psize.is_constant (&size) @@ -3826,7 +3832,7 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gsi, bool write) gsi_insert_after (gsi, ds, GSI_NEW_STMT); } - if (access->first_child) + if (access->first_child && !TREE_READONLY (access->base)) { HOST_WIDE_INT start_offset, chunk_size; if (bfr @@ -3890,6 +3896,13 @@ static void handle_unscalarized_data_in_subtree (struct subreplacement_assignment_data *sad) { tree src; + /* If the RHS is a load from a constant, we do not need to (and must not) + flush replacements to it and can use it directly as if we did. */ + if (TREE_READONLY (sad->top_racc->base)) + { + sad->refreshed = SRA_UDH_RIGHT; + return; + } if (sad->top_racc->grp_unscalarized_data) { src = sad->assignment_rhs; @@ -4243,8 +4256,8 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator *gsi) || contains_vce_or_bfcref_p (lhs) || stmt_ends_bb_p (stmt)) { - /* No need to copy into a constant-pool, it comes pre-initialized. */ - if (access_has_children_p (racc) && !constant_decl_p (racc->base)) + /* No need to copy into a constant, it comes pre-initialized. */ + if (access_has_children_p (racc) && !TREE_READONLY (racc->base)) generate_subtree_copies (racc->first_child, rhs, racc->offset, 0, 0, gsi, false, false, loc); if (access_has_children_p (lacc)) @@ -4333,7 +4346,7 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator *gsi) } /* Restore the aggregate RHS from its components so the prevailing aggregate copy does the right thing. */ - if (access_has_children_p (racc)) + if (access_has_children_p (racc) && !TREE_READONLY (racc->base)) generate_subtree_copies (racc->first_child, rhs, racc->offset, 0, 0, gsi, false, false, loc); /* Re-load the components of the aggregate copy destination.