On June 15, 2021 5:09:40 PM GMT+02:00, Martin Jambor <mjam...@suse.cz> wrote:
>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?
Ok.
Thanks,
Richard.
>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.
