public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Martin Jambor <jamborm@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r12-1529] tree-sra: Do not refresh readonly decls (PR 100453)
Date: Wed, 16 Jun 2021 11:24:55 +0000 (GMT)	[thread overview]
Message-ID: <20210616112455.3E93F383B414@sourceware.org> (raw)

https://gcc.gnu.org/g:d7deee423f993bee8ee440f6fe0c9126c316c64b

commit r12-1529-gd7deee423f993bee8ee440f6fe0c9126c316c64b
Author: Martin Jambor <mjambor@suse.cz>
Date:   Wed Jun 16 13:18:46 2021 +0200

    tree-sra: Do not refresh readonly decls (PR 100453)
    
    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.
    
    gcc/ChangeLog:
    
    2021-06-11  Martin Jambor  <mjambor@suse.cz>
    
            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-10  Martin Jambor  <mjambor@suse.cz>
    
            PR tree-optimization/100453
            * gcc.dg/tree-ssa/pr100453.c: New test.

Diff:
---
 gcc/testsuite/gcc.dg/tree-ssa/pr100453.c | 18 ++++++++++++++++++
 gcc/tree-sra.c                           | 21 +++++++++++++++++----
 2 files changed, 35 insertions(+), 4 deletions(-)

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.


                 reply	other threads:[~2021-06-16 11:24 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210616112455.3E93F383B414@sourceware.org \
    --to=jamborm@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).