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  <mjam...@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-11  Martin Jambor  <mjam...@suse.cz>

        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.
-- 
2.31.1

Reply via email to