* [PATCH] Fix tree-loop-distribution.c ICE with -ftrapv (PR tree-optimization/89278)
@ 2019-02-14 22:52 Jakub Jelinek
2019-02-15 7:03 ` Richard Biener
2019-02-15 7:25 ` Bin.Cheng
0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2019-02-14 22:52 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
The following testcase ICEs, because we try to gimplify a complex expression
that with -ftrapv wants to emit multiple bbs. Fixed by using
rewrite_to_non_trapping_overflow. Bootstrapped/regtested on x86_64-linux
and i686-linux, ok for trunk and 8.3?
2019-02-14 Richard Biener <rguenther@suse.de>
Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/89278
* tree-loop-distribution.c: Include tree-eh.h.
(generate_memset_builtin, generate_memcpy_builtin): Call
rewrite_to_non_trapping_overflow on builtin->size before passing it
to force_gimple_operand_gsi.
* gcc.dg/pr89278.c: New test.
--- gcc/tree-loop-distribution.c.jj 2019-01-10 11:43:02.255576992 +0100
+++ gcc/tree-loop-distribution.c 2019-02-14 12:17:24.403403131 +0100
@@ -114,6 +114,7 @@ along with GCC; see the file COPYING3.
#include "tree-scalar-evolution.h"
#include "params.h"
#include "tree-vectorizer.h"
+#include "tree-eh.h"
#define MAX_DATAREFS_NUM \
@@ -996,7 +997,7 @@ generate_memset_builtin (struct loop *lo
/* The new statements will be placed before LOOP. */
gsi = gsi_last_bb (loop_preheader_edge (loop)->src);
- nb_bytes = builtin->size;
+ nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
nb_bytes = force_gimple_operand_gsi (&gsi, nb_bytes, true, NULL_TREE,
false, GSI_CONTINUE_LINKING);
mem = builtin->dst_base;
@@ -1048,7 +1049,7 @@ generate_memcpy_builtin (struct loop *lo
/* The new statements will be placed before LOOP. */
gsi = gsi_last_bb (loop_preheader_edge (loop)->src);
- nb_bytes = builtin->size;
+ nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
nb_bytes = force_gimple_operand_gsi (&gsi, nb_bytes, true, NULL_TREE,
false, GSI_CONTINUE_LINKING);
dest = builtin->dst_base;
--- gcc/testsuite/gcc.dg/pr89278.c.jj 2019-02-14 12:18:38.778173413 +0100
+++ gcc/testsuite/gcc.dg/pr89278.c 2019-02-14 12:18:19.065499344 +0100
@@ -0,0 +1,23 @@
+/* PR tree-optimization/89278 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -ftrapv -ftree-loop-distribute-patterns --param max-loop-header-insns=2" } */
+
+void
+foo (int *w, int x, int y, int z)
+{
+ while (x < y + z)
+ {
+ w[x] = 0;
+ ++x;
+ }
+}
+
+void
+bar (int *__restrict u, int *__restrict w, int x, int y, int z)
+{
+ while (x < y + z)
+ {
+ w[x] = u[x];
+ ++x;
+ }
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix tree-loop-distribution.c ICE with -ftrapv (PR tree-optimization/89278)
2019-02-14 22:52 [PATCH] Fix tree-loop-distribution.c ICE with -ftrapv (PR tree-optimization/89278) Jakub Jelinek
@ 2019-02-15 7:03 ` Richard Biener
2019-02-15 7:25 ` Bin.Cheng
1 sibling, 0 replies; 6+ messages in thread
From: Richard Biener @ 2019-02-15 7:03 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On February 14, 2019 11:52:17 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The following testcase ICEs, because we try to gimplify a complex
>expression
>that with -ftrapv wants to emit multiple bbs. Fixed by using
>rewrite_to_non_trapping_overflow. Bootstrapped/regtested on
>x86_64-linux
>and i686-linux, ok for trunk and 8.3?
OK.
Richard.
>2019-02-14 Richard Biener <rguenther@suse.de>
> Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/89278
> * tree-loop-distribution.c: Include tree-eh.h.
> (generate_memset_builtin, generate_memcpy_builtin): Call
> rewrite_to_non_trapping_overflow on builtin->size before passing it
> to force_gimple_operand_gsi.
>
> * gcc.dg/pr89278.c: New test.
>
>--- gcc/tree-loop-distribution.c.jj 2019-01-10 11:43:02.255576992 +0100
>+++ gcc/tree-loop-distribution.c 2019-02-14 12:17:24.403403131 +0100
>@@ -114,6 +114,7 @@ along with GCC; see the file COPYING3.
> #include "tree-scalar-evolution.h"
> #include "params.h"
> #include "tree-vectorizer.h"
>+#include "tree-eh.h"
>
>
> #define MAX_DATAREFS_NUM \
>@@ -996,7 +997,7 @@ generate_memset_builtin (struct loop *lo
> /* The new statements will be placed before LOOP. */
> gsi = gsi_last_bb (loop_preheader_edge (loop)->src);
>
>- nb_bytes = builtin->size;
>+ nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
> nb_bytes = force_gimple_operand_gsi (&gsi, nb_bytes, true, NULL_TREE,
> false, GSI_CONTINUE_LINKING);
> mem = builtin->dst_base;
>@@ -1048,7 +1049,7 @@ generate_memcpy_builtin (struct loop *lo
> /* The new statements will be placed before LOOP. */
> gsi = gsi_last_bb (loop_preheader_edge (loop)->src);
>
>- nb_bytes = builtin->size;
>+ nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
> nb_bytes = force_gimple_operand_gsi (&gsi, nb_bytes, true, NULL_TREE,
> false, GSI_CONTINUE_LINKING);
> dest = builtin->dst_base;
>--- gcc/testsuite/gcc.dg/pr89278.c.jj 2019-02-14 12:18:38.778173413
>+0100
>+++ gcc/testsuite/gcc.dg/pr89278.c 2019-02-14 12:18:19.065499344 +0100
>@@ -0,0 +1,23 @@
>+/* PR tree-optimization/89278 */
>+/* { dg-do compile } */
>+/* { dg-options "-O1 -ftrapv -ftree-loop-distribute-patterns --param
>max-loop-header-insns=2" } */
>+
>+void
>+foo (int *w, int x, int y, int z)
>+{
>+ while (x < y + z)
>+ {
>+ w[x] = 0;
>+ ++x;
>+ }
>+}
>+
>+void
>+bar (int *__restrict u, int *__restrict w, int x, int y, int z)
>+{
>+ while (x < y + z)
>+ {
>+ w[x] = u[x];
>+ ++x;
>+ }
>+}
>
> Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix tree-loop-distribution.c ICE with -ftrapv (PR tree-optimization/89278)
2019-02-14 22:52 [PATCH] Fix tree-loop-distribution.c ICE with -ftrapv (PR tree-optimization/89278) Jakub Jelinek
2019-02-15 7:03 ` Richard Biener
@ 2019-02-15 7:25 ` Bin.Cheng
2019-02-15 7:33 ` Jakub Jelinek
1 sibling, 1 reply; 6+ messages in thread
From: Bin.Cheng @ 2019-02-15 7:25 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches List
On Fri, Feb 15, 2019 at 6:52 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The following testcase ICEs, because we try to gimplify a complex expression
> that with -ftrapv wants to emit multiple bbs. Fixed by using
> rewrite_to_non_trapping_overflow. Bootstrapped/regtested on x86_64-linux
So with what condition we can safely rewrite trapping operations into
non trapping one? Does the rewrite nullify -ftrapv which requires
trap behavior?
Thanks,
bin
> and i686-linux, ok for trunk and 8.3?
>
> 2019-02-14 Richard Biener <rguenther@suse.de>
> Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/89278
> * tree-loop-distribution.c: Include tree-eh.h.
> (generate_memset_builtin, generate_memcpy_builtin): Call
> rewrite_to_non_trapping_overflow on builtin->size before passing it
> to force_gimple_operand_gsi.
>
> * gcc.dg/pr89278.c: New test.
>
> --- gcc/tree-loop-distribution.c.jj 2019-01-10 11:43:02.255576992 +0100
> +++ gcc/tree-loop-distribution.c 2019-02-14 12:17:24.403403131 +0100
> @@ -114,6 +114,7 @@ along with GCC; see the file COPYING3.
> #include "tree-scalar-evolution.h"
> #include "params.h"
> #include "tree-vectorizer.h"
> +#include "tree-eh.h"
>
>
> #define MAX_DATAREFS_NUM \
> @@ -996,7 +997,7 @@ generate_memset_builtin (struct loop *lo
> /* The new statements will be placed before LOOP. */
> gsi = gsi_last_bb (loop_preheader_edge (loop)->src);
>
> - nb_bytes = builtin->size;
> + nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
> nb_bytes = force_gimple_operand_gsi (&gsi, nb_bytes, true, NULL_TREE,
> false, GSI_CONTINUE_LINKING);
> mem = builtin->dst_base;
> @@ -1048,7 +1049,7 @@ generate_memcpy_builtin (struct loop *lo
> /* The new statements will be placed before LOOP. */
> gsi = gsi_last_bb (loop_preheader_edge (loop)->src);
>
> - nb_bytes = builtin->size;
> + nb_bytes = rewrite_to_non_trapping_overflow (builtin->size);
> nb_bytes = force_gimple_operand_gsi (&gsi, nb_bytes, true, NULL_TREE,
> false, GSI_CONTINUE_LINKING);
> dest = builtin->dst_base;
> --- gcc/testsuite/gcc.dg/pr89278.c.jj 2019-02-14 12:18:38.778173413 +0100
> +++ gcc/testsuite/gcc.dg/pr89278.c 2019-02-14 12:18:19.065499344 +0100
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/89278 */
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -ftrapv -ftree-loop-distribute-patterns --param max-loop-header-insns=2" } */
> +
> +void
> +foo (int *w, int x, int y, int z)
> +{
> + while (x < y + z)
> + {
> + w[x] = 0;
> + ++x;
> + }
> +}
> +
> +void
> +bar (int *__restrict u, int *__restrict w, int x, int y, int z)
> +{
> + while (x < y + z)
> + {
> + w[x] = u[x];
> + ++x;
> + }
> +}
>
> Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix tree-loop-distribution.c ICE with -ftrapv (PR tree-optimization/89278)
2019-02-15 7:25 ` Bin.Cheng
@ 2019-02-15 7:33 ` Jakub Jelinek
2019-02-15 7:49 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2019-02-15 7:33 UTC (permalink / raw)
To: Bin.Cheng; +Cc: Richard Biener, gcc-patches List
On Fri, Feb 15, 2019 at 03:25:33PM +0800, Bin.Cheng wrote:
> So with what condition we can safely rewrite trapping operations into
> non trapping one? Does the rewrite nullify -ftrapv which requires
> trap behavior?
For the particular expression? Yes, otherwise no.
-ftrapv should be either replaced with -fsanitize=signed-integer-overflow
-fsanitize-undefined-trap-on-error, or at least implemented that way in the
middle-end (perhaps with a separate ifn, so that we can pattern recognize it
during expansion and use library calls where the inline call is not small
enough). We haven't done that yet though.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix tree-loop-distribution.c ICE with -ftrapv (PR tree-optimization/89278)
2019-02-15 7:33 ` Jakub Jelinek
@ 2019-02-15 7:49 ` Jakub Jelinek
2019-02-15 8:16 ` Bin.Cheng
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2019-02-15 7:49 UTC (permalink / raw)
To: Bin.Cheng; +Cc: Richard Biener, gcc-patches List
On Fri, Feb 15, 2019 at 08:33:44AM +0100, Jakub Jelinek wrote:
> On Fri, Feb 15, 2019 at 03:25:33PM +0800, Bin.Cheng wrote:
> > So with what condition we can safely rewrite trapping operations into
> > non trapping one? Does the rewrite nullify -ftrapv which requires
> > trap behavior?
>
> For the particular expression? Yes, otherwise no.
>
> -ftrapv should be either replaced with -fsanitize=signed-integer-overflow
> -fsanitize-undefined-trap-on-error, or at least implemented that way in the
> middle-end (perhaps with a separate ifn, so that we can pattern recognize it
> during expansion and use library calls where the inline call is not small
> enough). We haven't done that yet though.
To clarify, the current -ftrapv implementation doesn't guarantee you get
traps on overflow, it will happily optimize computations away at any time
during GIMPLE optimizations, or turn stuff into unsigned computations etc.
(not just through this rewrite function, but many other ways).
For -fsanitize=signed-integer-overflow -fsanitize-undefined-trap-on-error
there are no guarantees either, but we try hard not to optimize those away,
we have TYPE_OVERFLOW_SANITIZED checks that punt certain optimizations in
fold-const.c/match.pd and early (right after going into ssa form) we turn
the arithmetics into ifns, which are optimized away only if we can prove
there will be no overflow. On the other side, it can hinder other
optimizations (a lot). And possibly overflowing computations introduced
during later optimizations are not sanitized.
The question is what -ftrapv users want, plus right now they have a choice,
catch perhaps less UB with more optimization opportunities (-ftrapv)
or catch more optimize less (UBSan).
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix tree-loop-distribution.c ICE with -ftrapv (PR tree-optimization/89278)
2019-02-15 7:49 ` Jakub Jelinek
@ 2019-02-15 8:16 ` Bin.Cheng
0 siblings, 0 replies; 6+ messages in thread
From: Bin.Cheng @ 2019-02-15 8:16 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches List
On Fri, Feb 15, 2019 at 3:48 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, Feb 15, 2019 at 08:33:44AM +0100, Jakub Jelinek wrote:
> > On Fri, Feb 15, 2019 at 03:25:33PM +0800, Bin.Cheng wrote:
> > > So with what condition we can safely rewrite trapping operations into
> > > non trapping one? Does the rewrite nullify -ftrapv which requires
> > > trap behavior?
> >
> > For the particular expression? Yes, otherwise no.
> >
> > -ftrapv should be either replaced with -fsanitize=signed-integer-overflow
> > -fsanitize-undefined-trap-on-error, or at least implemented that way in the
> > middle-end (perhaps with a separate ifn, so that we can pattern recognize it
> > during expansion and use library calls where the inline call is not small
> > enough). We haven't done that yet though.
>
> To clarify, the current -ftrapv implementation doesn't guarantee you get
> traps on overflow, it will happily optimize computations away at any time
> during GIMPLE optimizations, or turn stuff into unsigned computations etc.
> (not just through this rewrite function, but many other ways).
> For -fsanitize=signed-integer-overflow -fsanitize-undefined-trap-on-error
> there are no guarantees either, but we try hard not to optimize those away,
> we have TYPE_OVERFLOW_SANITIZED checks that punt certain optimizations in
> fold-const.c/match.pd and early (right after going into ssa form) we turn
> the arithmetics into ifns, which are optimized away only if we can prove
> there will be no overflow. On the other side, it can hinder other
> optimizations (a lot). And possibly overflowing computations introduced
> during later optimizations are not sanitized.
> The question is what -ftrapv users want, plus right now they have a choice,
> catch perhaps less UB with more optimization opportunities (-ftrapv)
> or catch more optimize less (UBSan).
Thanks very much for the explanation, that explains all questions I had.
Thanks,
bin
>
> Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-15 8:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 22:52 [PATCH] Fix tree-loop-distribution.c ICE with -ftrapv (PR tree-optimization/89278) Jakub Jelinek
2019-02-15 7:03 ` Richard Biener
2019-02-15 7:25 ` Bin.Cheng
2019-02-15 7:33 ` Jakub Jelinek
2019-02-15 7:49 ` Jakub Jelinek
2019-02-15 8:16 ` Bin.Cheng
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).