This is the same as the version conditionally approved near the end of stage 1
https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01948.html, plus the
requested comment and a couple of testcases for platforms where the constants
are pushed into the constant pool. (I didn't commit this at the time because
without the rest of the series, benchmarking results didn't really show any
improvement, and the PR was not fixed.)

I've left the disqualified_constants check in, although I could be persuaded to
remove it if people felt strongly.

Also on AArch64, this still causes FAILs in guality pr54970.c:
FAIL: gcc.dg/guality/pr54970.c   -Os  line 15 a[0] == 1
FAIL: gcc.dg/guality/pr54970.c   -Os  line 20 a[0] == 1
FAIL: gcc.dg/guality/pr54970.c   -Os  line 25 a[0] == 1
and I think I must ask the AArch64 maintainers to take these for the time being.

Bootstrapped + check-gcc,g++,ada on x86_64, ARM, AArch64.

gcc/ChangeLog:

        PR target/63679
        * tree-sra.c (disqualified_constants): New.
        (sra_initialize): Allocate disqualified_constants.
        (sra_deinitialize): Free disqualified_constants.
        (disqualify_candidate): Update disqualified_constants when appropriate.
        (create_access): Scan for constant-pool entries as we go along.
        (scalarizable_type_p): Add check against type_contains_placeholder_p.
        (maybe_add_sra_candidate): Allow constant-pool entries.
        (initialize_constant_pool_replacements): New.
        (sra_modify_expr): Avoid mangling assignments created by previous.
        (sra_modify_function_body): Call initialize_constant_pool_replacements.

gcc/testsuite/ChangeLog:

        * gcc.dg/tree-ssa/sra-17.c: New.
        * gcc.dg/tree-ssa/sra-18.c: New.
---
 gcc/testsuite/gcc.dg/tree-ssa/sra-17.c | 19 +++++++
 gcc/testsuite/gcc.dg/tree-ssa/sra-18.c | 28 ++++++++++
 gcc/tree-sra.c                         | 99 ++++++++++++++++++++++++++++++++--
 3 files changed, 143 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/sra-17.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/sra-18.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/sra-17.c 
b/gcc/testsuite/gcc.dg/tree-ssa/sra-17.c
new file mode 100644
index 0000000..cff868a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-17.c
@@ -0,0 +1,19 @@
+/* { dg-do run { target { aarch64*-*-* alpha*-*-* arm*-*-* hppa*-*-* 
powerpc*-*-* s390*-*-* } } } */
+/* { dg-options "-O2 -fdump-tree-esra --param 
sra-max-scalarization-size-Ospeed=16" } */
+
+extern void abort (void);
+
+int
+main (int argc, char **argv)
+{
+  int a[4] = { 7, 19, 11, 255 };
+  int tot = 0;
+  for (int i = 0; i < 4; i++)
+    tot = (tot*256) + a[i];
+  if (tot == 0x07130bff)
+    return 0;
+  abort ();
+}
+
+/* { dg-final { scan-tree-dump-times "Removing load: a = \\\*.LC0;" 1 "esra" } 
} */
+/* { dg-final { scan-tree-dump-times "SR.\[0-9_\]+ = \\\*.LC0\\\[" 4 "esra" } 
} */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/sra-18.c 
b/gcc/testsuite/gcc.dg/tree-ssa/sra-18.c
new file mode 100644
index 0000000..6c0d52b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-18.c
@@ -0,0 +1,28 @@
+/* { dg-do run { target { aarch64*-*-* alpha*-*-* arm*-*-* hppa*-*-* 
powerpc*-*-* s390*-*-* } } } */
+/* { dg-options "-O2 -fdump-tree-esra --param 
sra-max-scalarization-size-Ospeed=16" } */
+
+extern void abort (void);
+struct foo { int x; };
+
+struct bar { struct foo f[2]; };
+
+struct baz { struct bar b[2]; };
+
+int
+main (int argc, char **argv)
+{
+  struct baz a = { { { { { 4 }, { 5 } } }, { { { 6 }, { 7 } } }  } };
+  int tot = 0;
+  for (int i = 0; i < 2; i++)
+    for (int j = 0; j < 2; j++)
+      tot = (tot*256) + a.b[i].f[j].x;
+  if (tot == 0x04050607)
+    return 0;
+  abort ();
+}
+
+/* { dg-final { scan-tree-dump-times "Removing load: a = \\\*.LC0;" 1 "esra" } 
} */
+/* { dg-final { scan-tree-dump-times "SR.\[0-9_\]+ = 
\\\*.LC0\\.b\\\[0\\\]\\.f\\\[0\\\]\\.x" 1 "esra" } } */
+/* { dg-final { scan-tree-dump-times "SR.\[0-9_\]+ = 
\\\*.LC0\\.b\\\[0\\\]\\.f\\\[1\\\]\\.x" 1 "esra" } } */
+/* { dg-final { scan-tree-dump-times "SR.\[0-9_\]+ = 
\\\*.LC0\\.b\\\[1\\\]\\.f\\\[0\\\]\\.x" 1 "esra" } } */
+/* { dg-final { scan-tree-dump-times "SR.\[0-9_\]+ = 
\\\*.LC0\\.b\\\[1\\\]\\.f\\\[1\\\]\\.x" 1 "esra" } } */
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index c4fea5b..4ea8550 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -328,6 +328,10 @@ candidate (unsigned uid)
    those which cannot be (because they are and need be used as a whole).  */
 static bitmap should_scalarize_away_bitmap, cannot_scalarize_away_bitmap;
 
+/* Bitmap of candidates in the constant pool, which cannot be scalarized
+   because this would produce non-constant expressions (e.g. Ada).  */
+static bitmap disqualified_constants;
+
 /* Obstack for creation of fancy names.  */
 static struct obstack name_obstack;
 
@@ -652,6 +656,7 @@ sra_initialize (void)
     (vec_safe_length (cfun->local_decls) / 2);
   should_scalarize_away_bitmap = BITMAP_ALLOC (NULL);
   cannot_scalarize_away_bitmap = BITMAP_ALLOC (NULL);
+  disqualified_constants = BITMAP_ALLOC (NULL);
   gcc_obstack_init (&name_obstack);
   base_access_vec = new hash_map<tree, auto_vec<access_p> >;
   memset (&sra_stats, 0, sizeof (sra_stats));
@@ -670,6 +675,7 @@ sra_deinitialize (void)
   candidates = NULL;
   BITMAP_FREE (should_scalarize_away_bitmap);
   BITMAP_FREE (cannot_scalarize_away_bitmap);
+  BITMAP_FREE (disqualified_constants);
   access_pool.release ();
   assign_link_pool.release ();
   obstack_free (&name_obstack, NULL);
@@ -684,6 +690,8 @@ disqualify_candidate (tree decl, const char *reason)
 {
   if (bitmap_clear_bit (candidate_bitmap, DECL_UID (decl)))
     candidates->remove_elt_with_hash (decl, DECL_UID (decl));
+  if (TREE_CODE (decl) == VAR_DECL && DECL_IN_CONSTANT_POOL (decl))
+    bitmap_set_bit (disqualified_constants, DECL_UID (decl));
 
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
@@ -835,8 +843,11 @@ create_access_1 (tree base, HOST_WIDE_INT offset, 
HOST_WIDE_INT size)
   return access;
 }
 
+static bool maybe_add_sra_candidate (tree);
+
 /* Create and insert access for EXPR. Return created access, or NULL if it is
-   not possible.  */
+   not possible.  Also scan for uses of constant pool as we go along and add
+   to candidates.  */
 
 static struct access *
 create_access (tree expr, gimple *stmt, bool write)
@@ -859,6 +870,25 @@ create_access (tree expr, gimple *stmt, bool write)
   else
     ptr = false;
 
+  /* For constant-pool entries, check we can substitute the constant value.  */
+  if (TREE_CODE (base) == VAR_DECL && DECL_IN_CONSTANT_POOL (base)
+      && (sra_mode == SRA_MODE_EARLY_INTRA || sra_mode == SRA_MODE_INTRA))
+    {
+      gcc_assert (!bitmap_bit_p (disqualified_constants, DECL_UID (base)));
+      if (expr != base
+         && !is_gimple_reg_type (TREE_TYPE (expr))
+         && dump_file && (dump_flags & TDF_DETAILS))
+       {
+         /* This occurs in Ada with accesses to ARRAY_RANGE_REFs,
+            and elements of multidimensional arrays (which are
+            multi-element arrays in their own right).  */
+         fprintf (dump_file, "Allowing non-reg-type load of part"
+                             " of constant-pool entry: ");
+         print_generic_expr (dump_file, expr, 0);
+       }
+      maybe_add_sra_candidate (base);
+    }
+
   if (!DECL_P (base) || !bitmap_bit_p (candidate_bitmap, DECL_UID (base)))
     return NULL;
 
@@ -918,6 +948,8 @@ static bool
 scalarizable_type_p (tree type)
 {
   gcc_assert (!is_gimple_reg_type (type));
+  if (type_contains_placeholder_p (type))
+    return false;
 
   switch (TREE_CODE (type))
   {
@@ -1852,7 +1884,12 @@ maybe_add_sra_candidate (tree var)
       reject (var, "not aggregate");
       return false;
     }
-  if (needs_to_live_in_memory (var))
+  /* Allow constant-pool entries (that "need to live in memory")
+     unless we are doing IPA SRA.  */
+  if (needs_to_live_in_memory (var)
+      && (sra_mode == SRA_MODE_EARLY_IPA
+         || TREE_CODE (var) != VAR_DECL
+         || !DECL_IN_CONSTANT_POOL (var)))
     {
       reject (var, "needs to live in memory");
       return false;
@@ -3272,6 +3309,9 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator 
*gsi)
   racc = get_access_for_expr (rhs);
   if (!lacc && !racc)
     return SRA_AM_NONE;
+  /* Avoid modifying initializations of constant-pool replacements.  */
+  if (racc && (racc->replacement_decl == lhs))
+    return SRA_AM_NONE;
 
   loc = gimple_location (stmt);
   if (lacc && lacc->grp_to_be_replaced)
@@ -3388,7 +3428,10 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator 
*gsi)
       || contains_vce_or_bfcref_p (lhs)
       || stmt_ends_bb_p (stmt))
     {
-      if (access_has_children_p (racc))
+      /* No need to copy into a constant-pool, it comes pre-initialized.  */
+      if (access_has_children_p (racc)
+         && (TREE_CODE (racc->base) != VAR_DECL
+             || !DECL_IN_CONSTANT_POOL (racc->base)))
        generate_subtree_copies (racc->first_child, rhs, racc->offset, 0, 0,
                                 gsi, false, false, loc);
       if (access_has_children_p (lacc))
@@ -3491,6 +3534,54 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator 
*gsi)
     }
 }
 
+/* Set any scalar replacements of values in the constant pool to the initial
+   value of the constant.  (Constant-pool decls like *.LC0 have effectively
+   been initialized before the program starts, we must do the same for their
+   replacements.)  Thus, we output statements like 'SR.1 = *.LC0[0];' into
+   the function's entry block.  */
+
+static void
+initialize_constant_pool_replacements (void)
+{
+  gimple_seq seq = NULL;
+  gimple_stmt_iterator gsi = gsi_start (seq);
+  bitmap_iterator bi;
+  unsigned i;
+
+  EXECUTE_IF_SET_IN_BITMAP (candidate_bitmap, 0, i, bi)
+    if (bitmap_bit_p (should_scalarize_away_bitmap, i)
+       && !bitmap_bit_p (cannot_scalarize_away_bitmap, i))
+      {
+       tree var = candidate (i);
+       if (TREE_CODE (var) != VAR_DECL || !DECL_IN_CONSTANT_POOL (var))
+         continue;
+       vec<access_p> *access_vec = get_base_access_vector (var);
+       if (!access_vec)
+         continue;
+       for (unsigned i = 0; i < access_vec->length (); i++)
+         {
+           struct access *access = (*access_vec)[i];
+           if (!access->replacement_decl)
+             continue;
+           gassign *stmt = gimple_build_assign (
+             get_access_replacement (access), unshare_expr (access->expr));
+           if (dump_file && (dump_flags & TDF_DETAILS))
+             {
+               fprintf (dump_file, "Generating constant initializer: ");
+               print_gimple_stmt (dump_file, stmt, 0, 1);
+               fprintf (dump_file, "\n");
+             }
+           gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
+           update_stmt (stmt);
+         }
+      }
+
+  seq = gsi_seq (gsi);
+  if (seq)
+    gsi_insert_seq_on_edge_immediate (
+      single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)), seq);
+}
+
 /* Traverse the function body and all modifications as decided in
    analyze_all_variable_accesses.  Return true iff the CFG has been
    changed.  */
@@ -3501,6 +3592,8 @@ sra_modify_function_body (void)
   bool cfg_changed = false;
   basic_block bb;
 
+  initialize_constant_pool_replacements ();
+
   FOR_EACH_BB_FN (bb, cfun)
     {
       gimple_stmt_iterator gsi = gsi_start_bb (bb);
-- 
1.9.1

Reply via email to