Re: [V2/PATCH] Fix tree-optimization/102216: missed optimization causing Warray-bounds
On Mon, Nov 22, 2021 at 3:40 AM Richard Biener
wrote:
>
> On Mon, Nov 22, 2021 at 9:40 AM Andrew Pinski wrote:
> >
> > On Wed, Oct 27, 2021 at 3:42 AM Richard Biener via Gcc-patches
> > wrote:
> > >
> > > On Wed, Oct 27, 2021 at 12:00 PM apinski--- via Gcc-patches
> > > wrote:
> > > >
> > > > From: Andrew Pinski
> > > >
> > > > The problem here is tree-ssa-forwprop.c likes to produce
> > > > [(void *)_4 + 152B] which is the same as
> > > > _4 p+ 152 which the rest of GCC likes better.
> > > > This implements this transformation back to pointer plus to
> > > > improve better code generation later on.
> > >
> > > Why do you think so? Can you pin-point the transform that now
> > > fixes the new testcase?
> >
> > So we had originally:
> > language_names_p_9 = [(void *)_4 + 24B];
> > ...
> > _2 = _4 + 40;
>
> Of course if that would have been
>
> _2 = [_4 + 40B];
>
> the issue would be fixed as well. That said, I agree that _4 + 40
> is better but I think we should canonicalize all [SSA + CST]
> this way. There is a canonicalization phase in fold_stmt_1:
>
> /* First do required canonicalization of [TARGET_]MEM_REF addresses
> after propagation.
> ??? This shouldn't be done in generic folding but in the
> propagation helpers which also know whether an address was
> propagated.
> Also canonicalize operand order. */
> switch (gimple_code (stmt))
> {
> case GIMPLE_ASSIGN:
> if (gimple_assign_rhs_class (stmt) == GIMPLE_SINGLE_RHS)
> {
> tree *rhs = gimple_assign_rhs1_ptr (stmt);
> if ((REFERENCE_CLASS_P (*rhs)
>|| TREE_CODE (*rhs) == ADDR_EXPR)
> && maybe_canonicalize_mem_ref_addr (rhs))
> changed = true;
>
> where this could be done (apart from adding a match.pd pattern for this).
Yes that is a good idea, I now have a patch which I am testing to add
this canonicalization. It is actually simpler than the previous patch
too.
Thanks,
Andrew
>
> > if (_2 != language_names_p_9)
> >
> > Forwprop is able to figure out that the above if statement is now
> > always false as we have (_4 +p 40) != (_4 +p 24) which gets simplified
> > via a match-and-simplify pattern ().
> > Does that answer your question?
> >
> > I will look into the other comments in a new patch.
> >
> > Thanks,
> > Andrew
> >
> > >
> > > Comments below
> > >
> > > > OK? Bootstrapped and tested on aarch64-linux-gnu.
> > > >
> > > > Changes from v1:
> > > > * v2: Add comments.
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > > PR tree-optimization/102216
> > > > * tree-ssa-forwprop.c (rewrite_assign_addr): New function.
> > > > (forward_propagate_addr_expr_1): Use rewrite_assign_addr
> > > > when rewriting into the addr_expr into an assignment.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > PR tree-optimization/102216
> > > > * g++.dg/tree-ssa/pr102216.C: New test.
> > > > ---
> > > > gcc/testsuite/g++.dg/tree-ssa/pr102216.C | 22 +
> > > > gcc/tree-ssa-forwprop.c | 58 ++--
> > > > 2 files changed, 67 insertions(+), 13 deletions(-)
> > > > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > > >
> > > > diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > > > b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > > > new file mode 100644
> > > > index 000..b903e4eb57d
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > > > @@ -0,0 +1,22 @@
> > > > +/* { dg-options "-O2 -fdump-tree-optimized" } */
> > > > +void link_error ();
> > > > +void g ()
> > > > +{
> > > > + const char **language_names;
> > > > +
> > > > + language_names = new const char *[6];
> > > > +
> > > > + const char **language_names_p = language_names;
> > > > +
> > > > + language_names_p++;
> > > > + language_names_p++;
> > > > + language_names_p++;
> > > > +
> > > > + if ( (language_names_p) - (language_names+3) != 0)
> > > > +link_error();
> > > > + delete[] language_names;
> > > > +}
> > > > +/* We should have removed the link_error on the gimple level as GCC
> > > > should
> > > > + be able to tell that language_names_p is the same as
> > > > language_names+3. */
> > > > +/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
> > > > +
> > > > diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
> > > > index a830bab78ba..e4331c60525 100644
> > > > --- a/gcc/tree-ssa-forwprop.c
> > > > +++ b/gcc/tree-ssa-forwprop.c
> > > > @@ -637,6 +637,47 @@ forward_propagate_into_cond (gimple_stmt_iterator
> > > > *gsi_p)
> > > >return 0;
> > > > }
> > > >
> > > > +/* Rewrite the DEF_RHS as needed into the (plain) use statement. */
> > > > +
> > > > +static void
> > > > +rewrite_assign_addr (gimple_stmt_iterator *use_stmt_gsi, tree def_rhs)
> > > > +{
> > > > + tree def_rhs_base;
> > > > + poly_int64 def_rhs_offset;
> > > > +
> > > > + /* Get the base and offset. */
Re: [V2/PATCH] Fix tree-optimization/102216: missed optimization causing Warray-bounds
On Mon, Nov 22, 2021 at 9:40 AM Andrew Pinski wrote:
>
> On Wed, Oct 27, 2021 at 3:42 AM Richard Biener via Gcc-patches
> wrote:
> >
> > On Wed, Oct 27, 2021 at 12:00 PM apinski--- via Gcc-patches
> > wrote:
> > >
> > > From: Andrew Pinski
> > >
> > > The problem here is tree-ssa-forwprop.c likes to produce
> > > [(void *)_4 + 152B] which is the same as
> > > _4 p+ 152 which the rest of GCC likes better.
> > > This implements this transformation back to pointer plus to
> > > improve better code generation later on.
> >
> > Why do you think so? Can you pin-point the transform that now
> > fixes the new testcase?
>
> So we had originally:
> language_names_p_9 = [(void *)_4 + 24B];
> ...
> _2 = _4 + 40;
Of course if that would have been
_2 = [_4 + 40B];
the issue would be fixed as well. That said, I agree that _4 + 40
is better but I think we should canonicalize all [SSA + CST]
this way. There is a canonicalization phase in fold_stmt_1:
/* First do required canonicalization of [TARGET_]MEM_REF addresses
after propagation.
??? This shouldn't be done in generic folding but in the
propagation helpers which also know whether an address was
propagated.
Also canonicalize operand order. */
switch (gimple_code (stmt))
{
case GIMPLE_ASSIGN:
if (gimple_assign_rhs_class (stmt) == GIMPLE_SINGLE_RHS)
{
tree *rhs = gimple_assign_rhs1_ptr (stmt);
if ((REFERENCE_CLASS_P (*rhs)
|| TREE_CODE (*rhs) == ADDR_EXPR)
&& maybe_canonicalize_mem_ref_addr (rhs))
changed = true;
where this could be done (apart from adding a match.pd pattern for this).
> if (_2 != language_names_p_9)
>
> Forwprop is able to figure out that the above if statement is now
> always false as we have (_4 +p 40) != (_4 +p 24) which gets simplified
> via a match-and-simplify pattern ().
> Does that answer your question?
>
> I will look into the other comments in a new patch.
>
> Thanks,
> Andrew
>
> >
> > Comments below
> >
> > > OK? Bootstrapped and tested on aarch64-linux-gnu.
> > >
> > > Changes from v1:
> > > * v2: Add comments.
> > >
> > > gcc/ChangeLog:
> > >
> > > PR tree-optimization/102216
> > > * tree-ssa-forwprop.c (rewrite_assign_addr): New function.
> > > (forward_propagate_addr_expr_1): Use rewrite_assign_addr
> > > when rewriting into the addr_expr into an assignment.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > PR tree-optimization/102216
> > > * g++.dg/tree-ssa/pr102216.C: New test.
> > > ---
> > > gcc/testsuite/g++.dg/tree-ssa/pr102216.C | 22 +
> > > gcc/tree-ssa-forwprop.c | 58 ++--
> > > 2 files changed, 67 insertions(+), 13 deletions(-)
> > > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > >
> > > diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > > b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > > new file mode 100644
> > > index 000..b903e4eb57d
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > > @@ -0,0 +1,22 @@
> > > +/* { dg-options "-O2 -fdump-tree-optimized" } */
> > > +void link_error ();
> > > +void g ()
> > > +{
> > > + const char **language_names;
> > > +
> > > + language_names = new const char *[6];
> > > +
> > > + const char **language_names_p = language_names;
> > > +
> > > + language_names_p++;
> > > + language_names_p++;
> > > + language_names_p++;
> > > +
> > > + if ( (language_names_p) - (language_names+3) != 0)
> > > +link_error();
> > > + delete[] language_names;
> > > +}
> > > +/* We should have removed the link_error on the gimple level as GCC
> > > should
> > > + be able to tell that language_names_p is the same as
> > > language_names+3. */
> > > +/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
> > > +
> > > diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
> > > index a830bab78ba..e4331c60525 100644
> > > --- a/gcc/tree-ssa-forwprop.c
> > > +++ b/gcc/tree-ssa-forwprop.c
> > > @@ -637,6 +637,47 @@ forward_propagate_into_cond (gimple_stmt_iterator
> > > *gsi_p)
> > >return 0;
> > > }
> > >
> > > +/* Rewrite the DEF_RHS as needed into the (plain) use statement. */
> > > +
> > > +static void
> > > +rewrite_assign_addr (gimple_stmt_iterator *use_stmt_gsi, tree def_rhs)
> > > +{
> > > + tree def_rhs_base;
> > > + poly_int64 def_rhs_offset;
> > > +
> > > + /* Get the base and offset. */
> > > + if ((def_rhs_base = get_addr_base_and_unit_offset (TREE_OPERAND
> > > (def_rhs, 0),
> > > +_rhs_offset)))
> >
> > So this will cause us to rewrite [p_1].a.b.c; to a pointer-plus,
> > right? Don't
> > we want to preserve that for object-size stuff? So maybe directly pattern
> > match ADDR_EXPR > only?
> >
> > > +{
> > > + tree new_ptr;
> > > + poly_offset_int off = 0;
> > > +
> > > + /* If the base was
Re: [V2/PATCH] Fix tree-optimization/102216: missed optimization causing Warray-bounds
On Wed, Oct 27, 2021 at 3:42 AM Richard Biener via Gcc-patches
wrote:
>
> On Wed, Oct 27, 2021 at 12:00 PM apinski--- via Gcc-patches
> wrote:
> >
> > From: Andrew Pinski
> >
> > The problem here is tree-ssa-forwprop.c likes to produce
> > [(void *)_4 + 152B] which is the same as
> > _4 p+ 152 which the rest of GCC likes better.
> > This implements this transformation back to pointer plus to
> > improve better code generation later on.
>
> Why do you think so? Can you pin-point the transform that now
> fixes the new testcase?
So we had originally:
language_names_p_9 = [(void *)_4 + 24B];
...
_2 = _4 + 40;
if (_2 != language_names_p_9)
Forwprop is able to figure out that the above if statement is now
always false as we have (_4 +p 40) != (_4 +p 24) which gets simplified
via a match-and-simplify pattern ().
Does that answer your question?
I will look into the other comments in a new patch.
Thanks,
Andrew
>
> Comments below
>
> > OK? Bootstrapped and tested on aarch64-linux-gnu.
> >
> > Changes from v1:
> > * v2: Add comments.
> >
> > gcc/ChangeLog:
> >
> > PR tree-optimization/102216
> > * tree-ssa-forwprop.c (rewrite_assign_addr): New function.
> > (forward_propagate_addr_expr_1): Use rewrite_assign_addr
> > when rewriting into the addr_expr into an assignment.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR tree-optimization/102216
> > * g++.dg/tree-ssa/pr102216.C: New test.
> > ---
> > gcc/testsuite/g++.dg/tree-ssa/pr102216.C | 22 +
> > gcc/tree-ssa-forwprop.c | 58 ++--
> > 2 files changed, 67 insertions(+), 13 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> >
> > diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > new file mode 100644
> > index 000..b903e4eb57d
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> > @@ -0,0 +1,22 @@
> > +/* { dg-options "-O2 -fdump-tree-optimized" } */
> > +void link_error ();
> > +void g ()
> > +{
> > + const char **language_names;
> > +
> > + language_names = new const char *[6];
> > +
> > + const char **language_names_p = language_names;
> > +
> > + language_names_p++;
> > + language_names_p++;
> > + language_names_p++;
> > +
> > + if ( (language_names_p) - (language_names+3) != 0)
> > +link_error();
> > + delete[] language_names;
> > +}
> > +/* We should have removed the link_error on the gimple level as GCC should
> > + be able to tell that language_names_p is the same as language_names+3.
> > */
> > +/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
> > +
> > diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
> > index a830bab78ba..e4331c60525 100644
> > --- a/gcc/tree-ssa-forwprop.c
> > +++ b/gcc/tree-ssa-forwprop.c
> > @@ -637,6 +637,47 @@ forward_propagate_into_cond (gimple_stmt_iterator
> > *gsi_p)
> >return 0;
> > }
> >
> > +/* Rewrite the DEF_RHS as needed into the (plain) use statement. */
> > +
> > +static void
> > +rewrite_assign_addr (gimple_stmt_iterator *use_stmt_gsi, tree def_rhs)
> > +{
> > + tree def_rhs_base;
> > + poly_int64 def_rhs_offset;
> > +
> > + /* Get the base and offset. */
> > + if ((def_rhs_base = get_addr_base_and_unit_offset (TREE_OPERAND
> > (def_rhs, 0),
> > +_rhs_offset)))
>
> So this will cause us to rewrite [p_1].a.b.c; to a pointer-plus,
> right? Don't
> we want to preserve that for object-size stuff? So maybe directly pattern
> match ADDR_EXPR > only?
>
> > +{
> > + tree new_ptr;
> > + poly_offset_int off = 0;
> > +
> > + /* If the base was a MEM, then add the offset to the other
> > + offset and adjust the base. */
> > + if (TREE_CODE (def_rhs_base) == MEM_REF)
> > + {
> > + off += mem_ref_offset (def_rhs_base);
> > + new_ptr = TREE_OPERAND (def_rhs_base, 0);
> > + }
> > + else
> > + new_ptr = build_fold_addr_expr (def_rhs_base);
> > +
> > + /* If we have the new base is not an address express, then use a p+
> > expression
> > + as the new expression instead of [x, offset]. */
> > + if (TREE_CODE (new_ptr) != ADDR_EXPR)
> > + {
> > + tree offset = wide_int_to_tree (sizetype, off);
> > + def_rhs = build2 (POINTER_PLUS_EXPR, TREE_TYPE (def_rhs),
> > new_ptr, offset);
>
> Ick. You should be able to use gimple_assign_set_rhs_with_ops.
>
> > + }
> > +}
> > +
> > + /* Replace the rhs with the new expression. */
> > + def_rhs = unshare_expr (def_rhs);
>
> and definitely no need to unshare anything here?
>
> > + gimple_assign_set_rhs_from_tree (use_stmt_gsi, def_rhs);
> > + gimple *use_stmt = gsi_stmt (*use_stmt_gsi);
> > + update_stmt (use_stmt);
> > +}
> > +
> > /* We've just substituted an ADDR_EXPR into stmt. Update all the
> > relevant data structures to match. */
Re: [V2/PATCH] Fix tree-optimization/102216: missed optimization causing Warray-bounds
On 10/27/21 3:59 AM, apinski--- via Gcc-patches wrote:
From: Andrew Pinski
The problem here is tree-ssa-forwprop.c likes to produce
[(void *)_4 + 152B] which is the same as
_4 p+ 152 which the rest of GCC likes better.
This implements this transformation back to pointer plus to
improve better code generation later on.
Since the purpose of this transformation is to avoid a bogus
-Warray-bounds can you please include a test case showing
the difference it makes? (I.e., one that warns without
the patch and doesn't with it. The test in the patch doesn't
trigger a warning for me.)
Thanks
Martin
OK? Bootstrapped and tested on aarch64-linux-gnu.
Changes from v1:
* v2: Add comments.
gcc/ChangeLog:
PR tree-optimization/102216
* tree-ssa-forwprop.c (rewrite_assign_addr): New function.
(forward_propagate_addr_expr_1): Use rewrite_assign_addr
when rewriting into the addr_expr into an assignment.
gcc/testsuite/ChangeLog:
PR tree-optimization/102216
* g++.dg/tree-ssa/pr102216.C: New test.
---
gcc/testsuite/g++.dg/tree-ssa/pr102216.C | 22 +
gcc/tree-ssa-forwprop.c | 58 ++--
2 files changed, 67 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216.C
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
new file mode 100644
index 000..b903e4eb57d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
@@ -0,0 +1,22 @@
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+void link_error ();
+void g ()
+{
+ const char **language_names;
+
+ language_names = new const char *[6];
+
+ const char **language_names_p = language_names;
+
+ language_names_p++;
+ language_names_p++;
+ language_names_p++;
+
+ if ( (language_names_p) - (language_names+3) != 0)
+link_error();
+ delete[] language_names;
+}
+/* We should have removed the link_error on the gimple level as GCC should
+ be able to tell that language_names_p is the same as language_names+3. */
+/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
+
diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index a830bab78ba..e4331c60525 100644
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -637,6 +637,47 @@ forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)
return 0;
}
+/* Rewrite the DEF_RHS as needed into the (plain) use statement. */
+
+static void
+rewrite_assign_addr (gimple_stmt_iterator *use_stmt_gsi, tree def_rhs)
+{
+ tree def_rhs_base;
+ poly_int64 def_rhs_offset;
+
+ /* Get the base and offset. */
+ if ((def_rhs_base = get_addr_base_and_unit_offset (TREE_OPERAND (def_rhs, 0),
+_rhs_offset)))
+{
+ tree new_ptr;
+ poly_offset_int off = 0;
+
+ /* If the base was a MEM, then add the offset to the other
+ offset and adjust the base. */
+ if (TREE_CODE (def_rhs_base) == MEM_REF)
+ {
+ off += mem_ref_offset (def_rhs_base);
+ new_ptr = TREE_OPERAND (def_rhs_base, 0);
+ }
+ else
+ new_ptr = build_fold_addr_expr (def_rhs_base);
+
+ /* If we have the new base is not an address express, then use a p+
expression
+ as the new expression instead of [x, offset]. */
+ if (TREE_CODE (new_ptr) != ADDR_EXPR)
+ {
+ tree offset = wide_int_to_tree (sizetype, off);
+ def_rhs = build2 (POINTER_PLUS_EXPR, TREE_TYPE (def_rhs), new_ptr,
offset);
+ }
+}
+
+ /* Replace the rhs with the new expression. */
+ def_rhs = unshare_expr (def_rhs);
+ gimple_assign_set_rhs_from_tree (use_stmt_gsi, def_rhs);
+ gimple *use_stmt = gsi_stmt (*use_stmt_gsi);
+ update_stmt (use_stmt);
+}
+
/* We've just substituted an ADDR_EXPR into stmt. Update all the
relevant data structures to match. */
@@ -696,8 +737,8 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
if (single_use_p
&& useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (def_rhs)))
{
- gimple_assign_set_rhs1 (use_stmt, unshare_expr (def_rhs));
- gimple_assign_set_rhs_code (use_stmt, TREE_CODE (def_rhs));
+ rewrite_assign_addr (use_stmt_gsi, def_rhs);
+ gcc_assert (gsi_stmt (*use_stmt_gsi) == use_stmt);
return true;
}
@@ -741,14 +782,7 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
if (forward_propagate_addr_expr (lhs, new_def_rhs, single_use_p))
return true;
- if (useless_type_conversion_p (TREE_TYPE (lhs),
-TREE_TYPE (new_def_rhs)))
- gimple_assign_set_rhs_with_ops (use_stmt_gsi, TREE_CODE (new_def_rhs),
- new_def_rhs);
- else if (is_gimple_min_invariant (new_def_rhs))
- gimple_assign_set_rhs_with_ops (use_stmt_gsi, NOP_EXPR, new_def_rhs);
- else
- return
Re: [V2/PATCH] Fix tree-optimization/102216: missed optimization causing Warray-bounds
On Wed, Oct 27, 2021 at 12:00 PM apinski--- via Gcc-patches
wrote:
>
> From: Andrew Pinski
>
> The problem here is tree-ssa-forwprop.c likes to produce
> [(void *)_4 + 152B] which is the same as
> _4 p+ 152 which the rest of GCC likes better.
> This implements this transformation back to pointer plus to
> improve better code generation later on.
Why do you think so? Can you pin-point the transform that now
fixes the new testcase?
Comments below
> OK? Bootstrapped and tested on aarch64-linux-gnu.
>
> Changes from v1:
> * v2: Add comments.
>
> gcc/ChangeLog:
>
> PR tree-optimization/102216
> * tree-ssa-forwprop.c (rewrite_assign_addr): New function.
> (forward_propagate_addr_expr_1): Use rewrite_assign_addr
> when rewriting into the addr_expr into an assignment.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/102216
> * g++.dg/tree-ssa/pr102216.C: New test.
> ---
> gcc/testsuite/g++.dg/tree-ssa/pr102216.C | 22 +
> gcc/tree-ssa-forwprop.c | 58 ++--
> 2 files changed, 67 insertions(+), 13 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216.C
>
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> new file mode 100644
> index 000..b903e4eb57d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
> @@ -0,0 +1,22 @@
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +void link_error ();
> +void g ()
> +{
> + const char **language_names;
> +
> + language_names = new const char *[6];
> +
> + const char **language_names_p = language_names;
> +
> + language_names_p++;
> + language_names_p++;
> + language_names_p++;
> +
> + if ( (language_names_p) - (language_names+3) != 0)
> +link_error();
> + delete[] language_names;
> +}
> +/* We should have removed the link_error on the gimple level as GCC should
> + be able to tell that language_names_p is the same as language_names+3. */
> +/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
> +
> diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
> index a830bab78ba..e4331c60525 100644
> --- a/gcc/tree-ssa-forwprop.c
> +++ b/gcc/tree-ssa-forwprop.c
> @@ -637,6 +637,47 @@ forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)
>return 0;
> }
>
> +/* Rewrite the DEF_RHS as needed into the (plain) use statement. */
> +
> +static void
> +rewrite_assign_addr (gimple_stmt_iterator *use_stmt_gsi, tree def_rhs)
> +{
> + tree def_rhs_base;
> + poly_int64 def_rhs_offset;
> +
> + /* Get the base and offset. */
> + if ((def_rhs_base = get_addr_base_and_unit_offset (TREE_OPERAND (def_rhs,
> 0),
> +_rhs_offset)))
So this will cause us to rewrite [p_1].a.b.c; to a pointer-plus,
right? Don't
we want to preserve that for object-size stuff? So maybe directly pattern
match ADDR_EXPR > only?
> +{
> + tree new_ptr;
> + poly_offset_int off = 0;
> +
> + /* If the base was a MEM, then add the offset to the other
> + offset and adjust the base. */
> + if (TREE_CODE (def_rhs_base) == MEM_REF)
> + {
> + off += mem_ref_offset (def_rhs_base);
> + new_ptr = TREE_OPERAND (def_rhs_base, 0);
> + }
> + else
> + new_ptr = build_fold_addr_expr (def_rhs_base);
> +
> + /* If we have the new base is not an address express, then use a p+
> expression
> + as the new expression instead of [x, offset]. */
> + if (TREE_CODE (new_ptr) != ADDR_EXPR)
> + {
> + tree offset = wide_int_to_tree (sizetype, off);
> + def_rhs = build2 (POINTER_PLUS_EXPR, TREE_TYPE (def_rhs), new_ptr,
> offset);
Ick. You should be able to use gimple_assign_set_rhs_with_ops.
> + }
> +}
> +
> + /* Replace the rhs with the new expression. */
> + def_rhs = unshare_expr (def_rhs);
and definitely no need to unshare anything here?
> + gimple_assign_set_rhs_from_tree (use_stmt_gsi, def_rhs);
> + gimple *use_stmt = gsi_stmt (*use_stmt_gsi);
> + update_stmt (use_stmt);
> +}
> +
> /* We've just substituted an ADDR_EXPR into stmt. Update all the
> relevant data structures to match. */
>
> @@ -696,8 +737,8 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
>if (single_use_p
> && useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (def_rhs)))
> {
> - gimple_assign_set_rhs1 (use_stmt, unshare_expr (def_rhs));
> - gimple_assign_set_rhs_code (use_stmt, TREE_CODE (def_rhs));
> + rewrite_assign_addr (use_stmt_gsi, def_rhs);
> + gcc_assert (gsi_stmt (*use_stmt_gsi) == use_stmt);
> return true;
> }
>
> @@ -741,14 +782,7 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
>if (forward_propagate_addr_expr (lhs, new_def_rhs, single_use_p))
> return true;
>
> - if (useless_type_conversion_p (TREE_TYPE (lhs),
Re: [V2/PATCH] Fix tree-optimization/102216: missed optimization causing Warray-bounds
On 27 October 2021 11:59:58 CEST, apinski--- via Gcc-patches
wrote:
>From: Andrew Pinski
>
>The problem here is tree-ssa-forwprop.c likes to produce
> [(void *)_4 + 152B] which is the same as
>_4 p+ 152 which the rest of GCC likes better.
>This implements this transformation back to pointer plus to
>improve better code generation later on.
>
>OK? Bootstrapped and tested on aarch64-linux-gnu.
>
>Changes from v1:
>* v2: Add comments.
>
>gcc/ChangeLog:
>
> PR tree-optimization/102216
> * tree-ssa-forwprop.c (rewrite_assign_addr): New function.
> (forward_propagate_addr_expr_1): Use rewrite_assign_addr
> when rewriting into the addr_expr into an assignment.
>
>gcc/testsuite/ChangeLog:
>
> PR tree-optimization/102216
> * g++.dg/tree-ssa/pr102216.C: New test.
>---
> gcc/testsuite/g++.dg/tree-ssa/pr102216.C | 22 +
> gcc/tree-ssa-forwprop.c | 58 ++--
> 2 files changed, 67 insertions(+), 13 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216.C
>
>diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
>b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
>new file mode 100644
>index 000..b903e4eb57d
>--- /dev/null
>+++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
>@@ -0,0 +1,22 @@
>+/* { dg-options "-O2 -fdump-tree-optimized" } */
>+void link_error ();
>+void g ()
>+{
>+ const char **language_names;
>+
>+ language_names = new const char *[6];
>+
>+ const char **language_names_p = language_names;
>+
>+ language_names_p++;
>+ language_names_p++;
>+ language_names_p++;
>+
>+ if ( (language_names_p) - (language_names+3) != 0)
>+link_error();
>+ delete[] language_names;
>+}
>+/* We should have removed the link_error on the gimple level as GCC should
>+ be able to tell that language_names_p is the same as language_names+3. */
>+/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
>+
>diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
>index a830bab78ba..e4331c60525 100644
>--- a/gcc/tree-ssa-forwprop.c
>+++ b/gcc/tree-ssa-forwprop.c
>@@ -637,6 +637,47 @@ forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)
> return 0;
> }
>
>+/* Rewrite the DEF_RHS as needed into the (plain) use statement. */
>+
>+static void
>+rewrite_assign_addr (gimple_stmt_iterator *use_stmt_gsi, tree def_rhs)
>+{
>+ tree def_rhs_base;
>+ poly_int64 def_rhs_offset;
>+
>+ /* Get the base and offset. */
>+ if ((def_rhs_base = get_addr_base_and_unit_offset (TREE_OPERAND (def_rhs,
>0),
>+ _rhs_offset)))
>+{
>+ tree new_ptr;
>+ poly_offset_int off = 0;
>+
>+ /* If the base was a MEM, then add the offset to the other
>+ offset and adjust the base. */
>+ if (TREE_CODE (def_rhs_base) == MEM_REF)
>+ {
>+off += mem_ref_offset (def_rhs_base);
>+new_ptr = TREE_OPERAND (def_rhs_base, 0);
>+ }
>+ else
>+ new_ptr = build_fold_addr_expr (def_rhs_base);
>+
>+ /* If we have the new base is not an address express, then use a p+
>expression
>+ as the new expression instead of [x, offset]. */
>+ if (TREE_CODE (new_ptr) != ADDR_EXPR)
>+ {
>+tree offset = wide_int_to_tree (sizetype, off);
>+def_rhs = build2 (POINTER_PLUS_EXPR, TREE_TYPE (def_rhs), new_ptr,
>offset);
>+ }
>+}
>+
>+ /* Replace the rhs with the new expression. */
>+ def_rhs = unshare_expr (def_rhs);
>+ gimple_assign_set_rhs_from_tree (use_stmt_gsi, def_rhs);
>+ gimple *use_stmt = gsi_stmt (*use_stmt_gsi);
>+ update_stmt (use_stmt);
>+}
>+
> /* We've just substituted an ADDR_EXPR into stmt. Update all the
>relevant data structures to match. */
>
>@@ -696,8 +737,8 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
> if (single_use_p
> && useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (def_rhs)))
> {
>-gimple_assign_set_rhs1 (use_stmt, unshare_expr (def_rhs));
>-gimple_assign_set_rhs_code (use_stmt, TREE_CODE (def_rhs));
>+rewrite_assign_addr (use_stmt_gsi, def_rhs);
>+gcc_assert (gsi_stmt (*use_stmt_gsi) == use_stmt);
> return true;
> }
>
>@@ -741,14 +782,7 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
> if (forward_propagate_addr_expr (lhs, new_def_rhs, single_use_p))
> return true;
>
>- if (useless_type_conversion_p (TREE_TYPE (lhs),
>- TREE_TYPE (new_def_rhs)))
>- gimple_assign_set_rhs_with_ops (use_stmt_gsi, TREE_CODE (new_def_rhs),
>- new_def_rhs);
>- else if (is_gimple_min_invariant (new_def_rhs))
>- gimple_assign_set_rhs_with_ops (use_stmt_gsi, NOP_EXPR, new_def_rhs);
>- else
>- return false;
>+ rewrite_assign_addr (use_stmt_gsi, new_def_rhs);
> gcc_assert (gsi_stmt (*use_stmt_gsi) == use_stmt);
> update_stmt (use_stmt);
ISTM the above
[V2/PATCH] Fix tree-optimization/102216: missed optimization causing Warray-bounds
From: Andrew Pinski
The problem here is tree-ssa-forwprop.c likes to produce
[(void *)_4 + 152B] which is the same as
_4 p+ 152 which the rest of GCC likes better.
This implements this transformation back to pointer plus to
improve better code generation later on.
OK? Bootstrapped and tested on aarch64-linux-gnu.
Changes from v1:
* v2: Add comments.
gcc/ChangeLog:
PR tree-optimization/102216
* tree-ssa-forwprop.c (rewrite_assign_addr): New function.
(forward_propagate_addr_expr_1): Use rewrite_assign_addr
when rewriting into the addr_expr into an assignment.
gcc/testsuite/ChangeLog:
PR tree-optimization/102216
* g++.dg/tree-ssa/pr102216.C: New test.
---
gcc/testsuite/g++.dg/tree-ssa/pr102216.C | 22 +
gcc/tree-ssa-forwprop.c | 58 ++--
2 files changed, 67 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216.C
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
new file mode 100644
index 000..b903e4eb57d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
@@ -0,0 +1,22 @@
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+void link_error ();
+void g ()
+{
+ const char **language_names;
+
+ language_names = new const char *[6];
+
+ const char **language_names_p = language_names;
+
+ language_names_p++;
+ language_names_p++;
+ language_names_p++;
+
+ if ( (language_names_p) - (language_names+3) != 0)
+link_error();
+ delete[] language_names;
+}
+/* We should have removed the link_error on the gimple level as GCC should
+ be able to tell that language_names_p is the same as language_names+3. */
+/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
+
diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index a830bab78ba..e4331c60525 100644
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -637,6 +637,47 @@ forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)
return 0;
}
+/* Rewrite the DEF_RHS as needed into the (plain) use statement. */
+
+static void
+rewrite_assign_addr (gimple_stmt_iterator *use_stmt_gsi, tree def_rhs)
+{
+ tree def_rhs_base;
+ poly_int64 def_rhs_offset;
+
+ /* Get the base and offset. */
+ if ((def_rhs_base = get_addr_base_and_unit_offset (TREE_OPERAND (def_rhs, 0),
+_rhs_offset)))
+{
+ tree new_ptr;
+ poly_offset_int off = 0;
+
+ /* If the base was a MEM, then add the offset to the other
+ offset and adjust the base. */
+ if (TREE_CODE (def_rhs_base) == MEM_REF)
+ {
+ off += mem_ref_offset (def_rhs_base);
+ new_ptr = TREE_OPERAND (def_rhs_base, 0);
+ }
+ else
+ new_ptr = build_fold_addr_expr (def_rhs_base);
+
+ /* If we have the new base is not an address express, then use a p+
expression
+ as the new expression instead of [x, offset]. */
+ if (TREE_CODE (new_ptr) != ADDR_EXPR)
+ {
+ tree offset = wide_int_to_tree (sizetype, off);
+ def_rhs = build2 (POINTER_PLUS_EXPR, TREE_TYPE (def_rhs), new_ptr,
offset);
+ }
+}
+
+ /* Replace the rhs with the new expression. */
+ def_rhs = unshare_expr (def_rhs);
+ gimple_assign_set_rhs_from_tree (use_stmt_gsi, def_rhs);
+ gimple *use_stmt = gsi_stmt (*use_stmt_gsi);
+ update_stmt (use_stmt);
+}
+
/* We've just substituted an ADDR_EXPR into stmt. Update all the
relevant data structures to match. */
@@ -696,8 +737,8 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
if (single_use_p
&& useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (def_rhs)))
{
- gimple_assign_set_rhs1 (use_stmt, unshare_expr (def_rhs));
- gimple_assign_set_rhs_code (use_stmt, TREE_CODE (def_rhs));
+ rewrite_assign_addr (use_stmt_gsi, def_rhs);
+ gcc_assert (gsi_stmt (*use_stmt_gsi) == use_stmt);
return true;
}
@@ -741,14 +782,7 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
if (forward_propagate_addr_expr (lhs, new_def_rhs, single_use_p))
return true;
- if (useless_type_conversion_p (TREE_TYPE (lhs),
-TREE_TYPE (new_def_rhs)))
- gimple_assign_set_rhs_with_ops (use_stmt_gsi, TREE_CODE (new_def_rhs),
- new_def_rhs);
- else if (is_gimple_min_invariant (new_def_rhs))
- gimple_assign_set_rhs_with_ops (use_stmt_gsi, NOP_EXPR, new_def_rhs);
- else
- return false;
+ rewrite_assign_addr (use_stmt_gsi, new_def_rhs);
gcc_assert (gsi_stmt (*use_stmt_gsi) == use_stmt);
update_stmt (use_stmt);
return true;
@@ -951,9 +985,7 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
unshare_expr (def_rhs),
[PATCH] Fix tree-optimization/102216: missed optimization causing Warray-bounds
From: Andrew Pinski
The problem here is tree-ssa-forwprop.c likes to produce
[(void *)_4 + 152B] which is the same as
_4 p+ 152 which the rest of GCC likes better.
This implements this transformation back to pointer plus to
improve better code generation later on.
OK? Bootstrapped and tested on aarch64-linux-gnu.
gcc/ChangeLog:
PR tree-optimization/102216
* tree-ssa-forwprop.c (rewrite_assign_addr): New function.
(forward_propagate_addr_expr_1): Use rewrite_assign_addr
when rewriting into the addr_expr into an assignment.
gcc/testsuite/ChangeLog:
PR tree-optimization/102216
* g++.dg/tree-ssa/pr102216.C: New test.
---
gcc/testsuite/g++.dg/tree-ssa/pr102216.C | 22
gcc/tree-ssa-forwprop.c | 46 +---
2 files changed, 55 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr102216.C
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
new file mode 100644
index 000..b903e4eb57d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr102216.C
@@ -0,0 +1,22 @@
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+void link_error ();
+void g ()
+{
+ const char **language_names;
+
+ language_names = new const char *[6];
+
+ const char **language_names_p = language_names;
+
+ language_names_p++;
+ language_names_p++;
+ language_names_p++;
+
+ if ( (language_names_p) - (language_names+3) != 0)
+link_error();
+ delete[] language_names;
+}
+/* We should have removed the link_error on the gimple level as GCC should
+ be able to tell that language_names_p is the same as language_names+3. */
+/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" } } */
+
diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index a830bab78ba..ba06bccdf75 100644
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -637,6 +637,35 @@ forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)
return 0;
}
+static void
+rewrite_assign_addr (gimple_stmt_iterator *use_stmt_gsi, tree def_rhs)
+{
+ tree def_rhs_base;
+ poly_int64 def_rhs_offset;
+ if ((def_rhs_base = get_addr_base_and_unit_offset (TREE_OPERAND (def_rhs, 0),
+_rhs_offset)))
+{
+ tree new_ptr;
+ poly_offset_int off = 0;
+ if (TREE_CODE (def_rhs_base) == MEM_REF)
+ {
+ off += mem_ref_offset (def_rhs_base);
+ new_ptr = TREE_OPERAND (def_rhs_base, 0);
+ }
+ else
+ new_ptr = build_fold_addr_expr (def_rhs_base);
+ if (TREE_CODE (new_ptr) != ADDR_EXPR)
+ {
+ tree offset = wide_int_to_tree (sizetype, off);
+ def_rhs = build2 (POINTER_PLUS_EXPR, TREE_TYPE (def_rhs), new_ptr,
offset);
+ }
+}
+ def_rhs = unshare_expr (def_rhs);
+ gimple_assign_set_rhs_from_tree (use_stmt_gsi, def_rhs);
+ gimple *use_stmt = gsi_stmt (*use_stmt_gsi);
+ update_stmt (use_stmt);
+}
+
/* We've just substituted an ADDR_EXPR into stmt. Update all the
relevant data structures to match. */
@@ -696,8 +725,8 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
if (single_use_p
&& useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (def_rhs)))
{
- gimple_assign_set_rhs1 (use_stmt, unshare_expr (def_rhs));
- gimple_assign_set_rhs_code (use_stmt, TREE_CODE (def_rhs));
+ rewrite_assign_addr (use_stmt_gsi, def_rhs);
+ gcc_assert (gsi_stmt (*use_stmt_gsi) == use_stmt);
return true;
}
@@ -741,14 +770,7 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
if (forward_propagate_addr_expr (lhs, new_def_rhs, single_use_p))
return true;
- if (useless_type_conversion_p (TREE_TYPE (lhs),
-TREE_TYPE (new_def_rhs)))
- gimple_assign_set_rhs_with_ops (use_stmt_gsi, TREE_CODE (new_def_rhs),
- new_def_rhs);
- else if (is_gimple_min_invariant (new_def_rhs))
- gimple_assign_set_rhs_with_ops (use_stmt_gsi, NOP_EXPR, new_def_rhs);
- else
- return false;
+ rewrite_assign_addr (use_stmt_gsi, new_def_rhs);
gcc_assert (gsi_stmt (*use_stmt_gsi) == use_stmt);
update_stmt (use_stmt);
return true;
@@ -951,9 +973,7 @@ forward_propagate_addr_expr_1 (tree name, tree def_rhs,
unshare_expr (def_rhs),
fold_convert (ptr_type_node,
rhs2)));
- gimple_assign_set_rhs_from_tree (use_stmt_gsi, new_rhs);
- use_stmt = gsi_stmt (*use_stmt_gsi);
- update_stmt (use_stmt);
+ rewrite_assign_addr (use_stmt_gsi, new_rhs);
tidy_after_forward_propagate_addr (use_stmt);
return true;
}
--
2.17.1
