Re: [PATCH] tree-optimization/102659 - avoid undefined overflow after if-conversion
Richard Biener writes:
> On Wed, 13 Oct 2021, Richard Sandiford wrote:
>
>> Richard Biener via Gcc-patches writes:
>> > The following makes sure to rewrite arithmetic with undefined behavior
>> > on overflow to a well-defined variant when moving them to be always
>> > executed as part of doing if-conversion for loop vectorization.
>> >
>> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
>> >
>> > Any comments?
>>
>> LGTM FWIW, although…
>>
>> > Thanks,
>> > Richard.
>> >
>> > 2021-10-11 Richard Biener
>> >
>> >PR tree-optimization/102659
>> >* tree-if-conv.c (need_to_rewrite_undefined): New flag.
>> >(if_convertible_gimple_assign_stmt_p): Mark the loop for
>> >rewrite when stmts with undefined behavior on integer
>> >overflow appear.
>> >(combine_blocks): Predicate also when we need to rewrite stmts.
>> >(predicate_statements): Rewrite affected stmts to something
>> >with well-defined behavior on overflow.
>> >(tree_if_conversion): Initialize need_to_rewrite_undefined.
>> >
>> >* gcc.dg/torture/pr69760.c: Adjust the testcase.
>> >* gcc.target/i386/avx2-vect-mask-store-move1.c: Expect to move
>> >the conversions to unsigned as well.
>> > ---
>> > gcc/testsuite/gcc.dg/torture/pr69760.c| 3 +-
>> > .../i386/avx2-vect-mask-store-move1.c | 2 +-
>> > gcc/tree-if-conv.c| 28 ++-
>> > 3 files changed, 29 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/gcc/testsuite/gcc.dg/torture/pr69760.c
>> > b/gcc/testsuite/gcc.dg/torture/pr69760.c
>> > index 53733c7c6a4..47e01ae59bd 100644
>> > --- a/gcc/testsuite/gcc.dg/torture/pr69760.c
>> > +++ b/gcc/testsuite/gcc.dg/torture/pr69760.c
>> > @@ -1,11 +1,10 @@
>> > /* PR tree-optimization/69760 */
>> > /* { dg-do run { target { { *-*-linux* *-*-gnu* *-*-uclinux* } && mmap }
>> > } } */
>> > -/* { dg-options "-O2" } */
>> >
>> > #include
>> > #include
>> >
>> > -__attribute__((noinline, noclone)) void
>> > +__attribute__((noinline, noclone)) static void
>> > test_func (double *a, int L, int m, int n, int N)
>> > {
>> >int i, k;
>> > diff --git a/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
>> > b/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
>> > index 989ba402e0e..6a47a09c835 100644
>> > --- a/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
>> > +++ b/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
>> > @@ -78,4 +78,4 @@ avx2_test (void)
>> >abort ();
>> > }
>> >
>> > -/* { dg-final { scan-tree-dump-times "Move stmt to created bb" 6 "vect" }
>> > } */
>> > +/* { dg-final { scan-tree-dump-times "Move stmt to created bb" 10 "vect"
>> > } } */
>> > diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
>> > index d7b7b309309..6a67acfeaae 100644
>> > --- a/gcc/tree-if-conv.c
>> > +++ b/gcc/tree-if-conv.c
>> > @@ -132,6 +132,11 @@ along with GCC; see the file COPYING3. If not see
>> > predicate_statements for the kinds of predication we support. */
>> > static bool need_to_predicate;
>> >
>> > +/* True if we have to rewrite stmts that may invoke undefined behavior
>> > + when a condition C was false so it doesn't if it is always executed.
>> > + See predicate_statements for the kinds of predication we support. */
>> > +static bool need_to_rewrite_undefined;
>> > +
>> > /* Indicate if there are any complicated PHIs that need to be handled in
>> > if-conversion. Complicated PHI has more than two arguments and can't
>> > be degenerated to two arguments PHI. See more information in comment
>> > @@ -1042,6 +1047,12 @@ if_convertible_gimple_assign_stmt_p (gimple *stmt,
>> >fprintf (dump_file, "tree could trap...\n");
>> >return false;
>> > }
>> > + else if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
>> > + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (lhs))
>> > + && arith_code_with_undefined_signed_overflow
>> > + (gimple_assign_rhs_code (stmt)))
>> > +/* We have to rewrite stmts with undefined overflow. */
>> > +need_to_rewrite_undefined = true;
>> >
>> >/* When if-converting stores force versioning, likewise if we
>> > ended up generating store data races. */
>> > @@ -2563,6 +2574,20 @@ predicate_statements (loop_p loop)
>> >
>> > gsi_replace (, new_stmt, true);
>> >}
>> > +else if (INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
>> > + && TYPE_OVERFLOW_UNDEFINED
>> > + (TREE_TYPE (gimple_assign_lhs (stmt)))
>> > + && arith_code_with_undefined_signed_overflow
>> > + (gimple_assign_rhs_code (stmt)))
>> > + {
>> > +gsi_remove (, true);
>> > +gsi_insert_seq_before (, rewrite_to_defined_overflow (stmt),
>> > + GSI_SAME_STMT);
>> > +if (gsi_end_p (gsi))
>> > + gsi = gsi_last_bb (gimple_bb (stmt));
>> > +else
>> > +
Re: [PATCH] tree-optimization/102659 - avoid undefined overflow after if-conversion
On Wed, 13 Oct 2021, Richard Sandiford wrote:
> Richard Biener via Gcc-patches writes:
> > The following makes sure to rewrite arithmetic with undefined behavior
> > on overflow to a well-defined variant when moving them to be always
> > executed as part of doing if-conversion for loop vectorization.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > Any comments?
>
> LGTM FWIW, although…
>
> > Thanks,
> > Richard.
> >
> > 2021-10-11 Richard Biener
> >
> > PR tree-optimization/102659
> > * tree-if-conv.c (need_to_rewrite_undefined): New flag.
> > (if_convertible_gimple_assign_stmt_p): Mark the loop for
> > rewrite when stmts with undefined behavior on integer
> > overflow appear.
> > (combine_blocks): Predicate also when we need to rewrite stmts.
> > (predicate_statements): Rewrite affected stmts to something
> > with well-defined behavior on overflow.
> > (tree_if_conversion): Initialize need_to_rewrite_undefined.
> >
> > * gcc.dg/torture/pr69760.c: Adjust the testcase.
> > * gcc.target/i386/avx2-vect-mask-store-move1.c: Expect to move
> > the conversions to unsigned as well.
> > ---
> > gcc/testsuite/gcc.dg/torture/pr69760.c| 3 +-
> > .../i386/avx2-vect-mask-store-move1.c | 2 +-
> > gcc/tree-if-conv.c| 28 ++-
> > 3 files changed, 29 insertions(+), 4 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.dg/torture/pr69760.c
> > b/gcc/testsuite/gcc.dg/torture/pr69760.c
> > index 53733c7c6a4..47e01ae59bd 100644
> > --- a/gcc/testsuite/gcc.dg/torture/pr69760.c
> > +++ b/gcc/testsuite/gcc.dg/torture/pr69760.c
> > @@ -1,11 +1,10 @@
> > /* PR tree-optimization/69760 */
> > /* { dg-do run { target { { *-*-linux* *-*-gnu* *-*-uclinux* } && mmap } }
> > } */
> > -/* { dg-options "-O2" } */
> >
> > #include
> > #include
> >
> > -__attribute__((noinline, noclone)) void
> > +__attribute__((noinline, noclone)) static void
> > test_func (double *a, int L, int m, int n, int N)
> > {
> >int i, k;
> > diff --git a/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
> > b/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
> > index 989ba402e0e..6a47a09c835 100644
> > --- a/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
> > +++ b/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
> > @@ -78,4 +78,4 @@ avx2_test (void)
> >abort ();
> > }
> >
> > -/* { dg-final { scan-tree-dump-times "Move stmt to created bb" 6 "vect" }
> > } */
> > +/* { dg-final { scan-tree-dump-times "Move stmt to created bb" 10 "vect" }
> > } */
> > diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
> > index d7b7b309309..6a67acfeaae 100644
> > --- a/gcc/tree-if-conv.c
> > +++ b/gcc/tree-if-conv.c
> > @@ -132,6 +132,11 @@ along with GCC; see the file COPYING3. If not see
> > predicate_statements for the kinds of predication we support. */
> > static bool need_to_predicate;
> >
> > +/* True if we have to rewrite stmts that may invoke undefined behavior
> > + when a condition C was false so it doesn't if it is always executed.
> > + See predicate_statements for the kinds of predication we support. */
> > +static bool need_to_rewrite_undefined;
> > +
> > /* Indicate if there are any complicated PHIs that need to be handled in
> > if-conversion. Complicated PHI has more than two arguments and can't
> > be degenerated to two arguments PHI. See more information in comment
> > @@ -1042,6 +1047,12 @@ if_convertible_gimple_assign_stmt_p (gimple *stmt,
> > fprintf (dump_file, "tree could trap...\n");
> >return false;
> > }
> > + else if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> > + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (lhs))
> > + && arith_code_with_undefined_signed_overflow
> > + (gimple_assign_rhs_code (stmt)))
> > +/* We have to rewrite stmts with undefined overflow. */
> > +need_to_rewrite_undefined = true;
> >
> >/* When if-converting stores force versioning, likewise if we
> > ended up generating store data races. */
> > @@ -2563,6 +2574,20 @@ predicate_statements (loop_p loop)
> >
> > gsi_replace (, new_stmt, true);
> > }
> > + else if (INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
> > + && TYPE_OVERFLOW_UNDEFINED
> > + (TREE_TYPE (gimple_assign_lhs (stmt)))
> > + && arith_code_with_undefined_signed_overflow
> > + (gimple_assign_rhs_code (stmt)))
> > + {
> > + gsi_remove (, true);
> > + gsi_insert_seq_before (, rewrite_to_defined_overflow (stmt),
> > +GSI_SAME_STMT);
> > + if (gsi_end_p (gsi))
> > + gsi = gsi_last_bb (gimple_bb (stmt));
> > + else
> > + gsi_prev ();
>
> …perhaps there should be a GSI_* for this idiom.
Yeah, the issue is that gsi_remove
Re: [PATCH] tree-optimization/102659 - avoid undefined overflow after if-conversion
Richard Biener via Gcc-patches writes:
> The following makes sure to rewrite arithmetic with undefined behavior
> on overflow to a well-defined variant when moving them to be always
> executed as part of doing if-conversion for loop vectorization.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> Any comments?
LGTM FWIW, although…
> Thanks,
> Richard.
>
> 2021-10-11 Richard Biener
>
> PR tree-optimization/102659
> * tree-if-conv.c (need_to_rewrite_undefined): New flag.
> (if_convertible_gimple_assign_stmt_p): Mark the loop for
> rewrite when stmts with undefined behavior on integer
> overflow appear.
> (combine_blocks): Predicate also when we need to rewrite stmts.
> (predicate_statements): Rewrite affected stmts to something
> with well-defined behavior on overflow.
> (tree_if_conversion): Initialize need_to_rewrite_undefined.
>
> * gcc.dg/torture/pr69760.c: Adjust the testcase.
> * gcc.target/i386/avx2-vect-mask-store-move1.c: Expect to move
> the conversions to unsigned as well.
> ---
> gcc/testsuite/gcc.dg/torture/pr69760.c| 3 +-
> .../i386/avx2-vect-mask-store-move1.c | 2 +-
> gcc/tree-if-conv.c| 28 ++-
> 3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/torture/pr69760.c
> b/gcc/testsuite/gcc.dg/torture/pr69760.c
> index 53733c7c6a4..47e01ae59bd 100644
> --- a/gcc/testsuite/gcc.dg/torture/pr69760.c
> +++ b/gcc/testsuite/gcc.dg/torture/pr69760.c
> @@ -1,11 +1,10 @@
> /* PR tree-optimization/69760 */
> /* { dg-do run { target { { *-*-linux* *-*-gnu* *-*-uclinux* } && mmap } } }
> */
> -/* { dg-options "-O2" } */
>
> #include
> #include
>
> -__attribute__((noinline, noclone)) void
> +__attribute__((noinline, noclone)) static void
> test_func (double *a, int L, int m, int n, int N)
> {
>int i, k;
> diff --git a/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
> b/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
> index 989ba402e0e..6a47a09c835 100644
> --- a/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
> +++ b/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
> @@ -78,4 +78,4 @@ avx2_test (void)
>abort ();
> }
>
> -/* { dg-final { scan-tree-dump-times "Move stmt to created bb" 6 "vect" } }
> */
> +/* { dg-final { scan-tree-dump-times "Move stmt to created bb" 10 "vect" } }
> */
> diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
> index d7b7b309309..6a67acfeaae 100644
> --- a/gcc/tree-if-conv.c
> +++ b/gcc/tree-if-conv.c
> @@ -132,6 +132,11 @@ along with GCC; see the file COPYING3. If not see
> predicate_statements for the kinds of predication we support. */
> static bool need_to_predicate;
>
> +/* True if we have to rewrite stmts that may invoke undefined behavior
> + when a condition C was false so it doesn't if it is always executed.
> + See predicate_statements for the kinds of predication we support. */
> +static bool need_to_rewrite_undefined;
> +
> /* Indicate if there are any complicated PHIs that need to be handled in
> if-conversion. Complicated PHI has more than two arguments and can't
> be degenerated to two arguments PHI. See more information in comment
> @@ -1042,6 +1047,12 @@ if_convertible_gimple_assign_stmt_p (gimple *stmt,
> fprintf (dump_file, "tree could trap...\n");
>return false;
> }
> + else if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
> +&& TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (lhs))
> +&& arith_code_with_undefined_signed_overflow
> + (gimple_assign_rhs_code (stmt)))
> +/* We have to rewrite stmts with undefined overflow. */
> +need_to_rewrite_undefined = true;
>
>/* When if-converting stores force versioning, likewise if we
> ended up generating store data races. */
> @@ -2563,6 +2574,20 @@ predicate_statements (loop_p loop)
>
> gsi_replace (, new_stmt, true);
> }
> + else if (INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
> +&& TYPE_OVERFLOW_UNDEFINED
> + (TREE_TYPE (gimple_assign_lhs (stmt)))
> +&& arith_code_with_undefined_signed_overflow
> + (gimple_assign_rhs_code (stmt)))
> + {
> + gsi_remove (, true);
> + gsi_insert_seq_before (, rewrite_to_defined_overflow (stmt),
> + GSI_SAME_STMT);
> + if (gsi_end_p (gsi))
> + gsi = gsi_last_bb (gimple_bb (stmt));
> + else
> + gsi_prev ();
…perhaps there should be a GSI_* for this idiom.
Thanks,
Richard
> + }
> else if (gimple_vdef (stmt))
> {
> tree lhs = gimple_assign_lhs (stmt);
> @@ -2647,7 +2672,7 @@ combine_blocks (class loop *loop)
>insert_gimplified_predicates (loop);
>
[PATCH] tree-optimization/102659 - avoid undefined overflow after if-conversion
The following makes sure to rewrite arithmetic with undefined behavior
on overflow to a well-defined variant when moving them to be always
executed as part of doing if-conversion for loop vectorization.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Any comments?
Thanks,
Richard.
2021-10-11 Richard Biener
PR tree-optimization/102659
* tree-if-conv.c (need_to_rewrite_undefined): New flag.
(if_convertible_gimple_assign_stmt_p): Mark the loop for
rewrite when stmts with undefined behavior on integer
overflow appear.
(combine_blocks): Predicate also when we need to rewrite stmts.
(predicate_statements): Rewrite affected stmts to something
with well-defined behavior on overflow.
(tree_if_conversion): Initialize need_to_rewrite_undefined.
* gcc.dg/torture/pr69760.c: Adjust the testcase.
* gcc.target/i386/avx2-vect-mask-store-move1.c: Expect to move
the conversions to unsigned as well.
---
gcc/testsuite/gcc.dg/torture/pr69760.c| 3 +-
.../i386/avx2-vect-mask-store-move1.c | 2 +-
gcc/tree-if-conv.c| 28 ++-
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/torture/pr69760.c
b/gcc/testsuite/gcc.dg/torture/pr69760.c
index 53733c7c6a4..47e01ae59bd 100644
--- a/gcc/testsuite/gcc.dg/torture/pr69760.c
+++ b/gcc/testsuite/gcc.dg/torture/pr69760.c
@@ -1,11 +1,10 @@
/* PR tree-optimization/69760 */
/* { dg-do run { target { { *-*-linux* *-*-gnu* *-*-uclinux* } && mmap } } } */
-/* { dg-options "-O2" } */
#include
#include
-__attribute__((noinline, noclone)) void
+__attribute__((noinline, noclone)) static void
test_func (double *a, int L, int m, int n, int N)
{
int i, k;
diff --git a/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
b/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
index 989ba402e0e..6a47a09c835 100644
--- a/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
+++ b/gcc/testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c
@@ -78,4 +78,4 @@ avx2_test (void)
abort ();
}
-/* { dg-final { scan-tree-dump-times "Move stmt to created bb" 6 "vect" } } */
+/* { dg-final { scan-tree-dump-times "Move stmt to created bb" 10 "vect" } } */
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index d7b7b309309..6a67acfeaae 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -132,6 +132,11 @@ along with GCC; see the file COPYING3. If not see
predicate_statements for the kinds of predication we support. */
static bool need_to_predicate;
+/* True if we have to rewrite stmts that may invoke undefined behavior
+ when a condition C was false so it doesn't if it is always executed.
+ See predicate_statements for the kinds of predication we support. */
+static bool need_to_rewrite_undefined;
+
/* Indicate if there are any complicated PHIs that need to be handled in
if-conversion. Complicated PHI has more than two arguments and can't
be degenerated to two arguments PHI. See more information in comment
@@ -1042,6 +1047,12 @@ if_convertible_gimple_assign_stmt_p (gimple *stmt,
fprintf (dump_file, "tree could trap...\n");
return false;
}
+ else if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+ && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (lhs))
+ && arith_code_with_undefined_signed_overflow
+ (gimple_assign_rhs_code (stmt)))
+/* We have to rewrite stmts with undefined overflow. */
+need_to_rewrite_undefined = true;
/* When if-converting stores force versioning, likewise if we
ended up generating store data races. */
@@ -2563,6 +2574,20 @@ predicate_statements (loop_p loop)
gsi_replace (, new_stmt, true);
}
+ else if (INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
+ && TYPE_OVERFLOW_UNDEFINED
+ (TREE_TYPE (gimple_assign_lhs (stmt)))
+ && arith_code_with_undefined_signed_overflow
+ (gimple_assign_rhs_code (stmt)))
+ {
+ gsi_remove (, true);
+ gsi_insert_seq_before (, rewrite_to_defined_overflow (stmt),
+GSI_SAME_STMT);
+ if (gsi_end_p (gsi))
+ gsi = gsi_last_bb (gimple_bb (stmt));
+ else
+ gsi_prev ();
+ }
else if (gimple_vdef (stmt))
{
tree lhs = gimple_assign_lhs (stmt);
@@ -2647,7 +2672,7 @@ combine_blocks (class loop *loop)
insert_gimplified_predicates (loop);
predicate_all_scalar_phis (loop);
- if (need_to_predicate)
+ if (need_to_predicate || need_to_rewrite_undefined)
predicate_statements (loop);
/* Merge basic blocks. */
@@ -3148,6 +3173,7 @@ tree_if_conversion (class loop *loop, vec
*preds)
rloop = NULL;
ifc_bbs = NULL;
