On December 17, 2019 1:40:47 PM GMT+01:00, Martin Jambor <mjam...@suse.cz>
wrote:
>Hi,
>
>because the follow-up patches perform some non-trivial operations on
>SRA patches, I wrote myself a verifier. And sure enough, it has
>spotted two issues, one of which is fixed in this patch too - we did
>not correctly set the parent link when creating artificial accesses
>for propagation across assignments. The second one is the (not)
>setting of reverse flag when creating accesses for total scalarization
>but since the following patch removes the offending function, this
>patch does not fix it.
>
>Bootstrapped and tested on x86_64, I consider this a pre-requisite for
>the followup patches (and the parent link fix really is).
OK.
Thanks
Richard.
>Thanks,
>
>Martin
>
>2019-12-10 Martin Jambor <mjam...@suse.cz>
>
> * tree-sra.c (verify_sra_access_forest): New function.
> (verify_all_sra_access_forests): Likewise.
> (create_artificial_child_access): Set parent.
> (analyze_all_variable_accesses): Call the verifier.
>---
> gcc/tree-sra.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
>
>diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
>index 87c156f2f54..e077a811da9 100644
>--- a/gcc/tree-sra.c
>+++ b/gcc/tree-sra.c
>@@ -2321,6 +2321,88 @@ build_access_trees (struct access *access)
> return true;
> }
>
>+/* Traverse the access forest where ROOT is the first root and verify
>that
>+ various important invariants hold true. */
>+
>+DEBUG_FUNCTION void
>+verify_sra_access_forest (struct access *root)
>+{
>+ struct access *access = root;
>+ tree first_base = root->base;
>+ gcc_assert (DECL_P (first_base));
>+ do
>+ {
>+ gcc_assert (access->base == first_base);
>+ if (access->parent)
>+ gcc_assert (access->offset >= access->parent->offset
>+ && access->size <= access->parent->size);
>+ if (access->next_sibling)
>+ gcc_assert (access->next_sibling->offset
>+ >= access->offset + access->size);
>+
>+ poly_int64 poffset, psize, pmax_size;
>+ bool reverse;
>+ tree base = get_ref_base_and_extent (access->expr, &poffset,
>&psize,
>+ &pmax_size, &reverse);
>+ HOST_WIDE_INT offset, size, max_size;
>+ if (!poffset.is_constant (&offset)
>+ || !psize.is_constant (&size)
>+ || !pmax_size.is_constant (&max_size))
>+ gcc_unreachable ();
>+ gcc_assert (base == first_base);
>+ gcc_assert (offset == access->offset);
>+ gcc_assert (access->grp_unscalarizable_region
>+ || size == max_size);
>+ gcc_assert (max_size == access->size);
>+ gcc_assert (reverse == access->reverse);
>+
>+ if (access->first_child)
>+ {
>+ gcc_assert (access->first_child->parent == access);
>+ access = access->first_child;
>+ }
>+ else if (access->next_sibling)
>+ {
>+ gcc_assert (access->next_sibling->parent == access->parent);
>+ access = access->next_sibling;
>+ }
>+ else
>+ {
>+ while (access->parent && !access->next_sibling)
>+ access = access->parent;
>+ if (access->next_sibling)
>+ access = access->next_sibling;
>+ else
>+ {
>+ gcc_assert (access == root);
>+ root = root->next_grp;
>+ access = root;
>+ }
>+ }
>+ }
>+ while (access);
>+}
>+
>+/* Verify access forests of all candidates with accesses by calling
>+ verify_access_forest on each on them. */
>+
>+DEBUG_FUNCTION void
>+verify_all_sra_access_forests (void)
>+{
>+ bitmap_iterator bi;
>+ unsigned i;
>+ EXECUTE_IF_SET_IN_BITMAP (candidate_bitmap, 0, i, bi)
>+ {
>+ tree var = candidate (i);
>+ struct access *access = get_first_repr_for_decl (var);
>+ if (access)
>+ {
>+ gcc_assert (access->base == var);
>+ verify_sra_access_forest (access);
>+ }
>+ }
>+}
>+
>/* Return true if expr contains some ARRAY_REFs into a variable bounded
> array. */
>
>@@ -2566,6 +2648,7 @@ create_artificial_child_access (struct access
>*parent, struct access *model,
> access->offset = new_offset;
> access->size = model->size;
> access->type = model->type;
>+ access->parent = parent;
> access->grp_write = set_grp_write;
> access->grp_read = false;
> access->reverse = model->reverse;
>@@ -2850,6 +2933,9 @@ analyze_all_variable_accesses (void)
>
> propagate_all_subaccesses ();
>
>+ if (flag_checking)
>+ verify_all_sra_access_forests ();
>+
> bitmap_copy (tmp, candidate_bitmap);
> EXECUTE_IF_SET_IN_BITMAP (tmp, 0, i, bi)
> {
