* [PATCH 1/4] Add verification of SRA accesses
@ 2019-12-17 12:41 Martin Jambor
2019-12-18 11:15 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Martin Jambor @ 2019-12-17 12:41 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Biener, Jan Hubicka, Jan Hubicka
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).
Thanks,
Martin
2019-12-10 Martin Jambor <mjambor@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)
{
--
2.24.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 1/4] Add verification of SRA accesses
2019-12-17 12:41 [PATCH 1/4] Add verification of SRA accesses Martin Jambor
@ 2019-12-18 11:15 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2019-12-18 11:15 UTC (permalink / raw)
To: Martin Jambor, GCC Patches; +Cc: Jan Hubicka, Jan Hubicka
On December 17, 2019 1:40:47 PM GMT+01:00, Martin Jambor <mjambor@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 <mjambor@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)
> {
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-12-18 11:11 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-17 12:41 [PATCH 1/4] Add verification of SRA accesses Martin Jambor
2019-12-18 11:15 ` Richard Biener
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).