* [PATCH] Fix reassoc ICE (PR tree-optimization/69802)
@ 2016-02-15 20:54 Jakub Jelinek
2016-02-15 21:27 ` Michael Matz
2016-02-16 9:09 ` Richard Biener
0 siblings, 2 replies; 4+ messages in thread
From: Jakub Jelinek @ 2016-02-15 20:54 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
The following patch fixes an ICE where one of the range tests
is SSA_NAME_DEF_STMT of a bool/_Bool or unsigned : 1 bitfield.
In that case, we don't know where to put the adjusted range test.
The patch for this uncommon case gives up, unless the range test
can be the SSA_NAME_DEF_STMT itself, and in that case makes sure we
DTRT.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-02-15 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/69802
* tree-ssa-reassoc.c (update_range_test): If op is
SSA_NAME_IS_DEFAULT_DEF, give up unless tem is a positive
op == 1 test of precision 1 integral op, otherwise handle
that case as op itself. Fix up formatting.
(optimize_range_tests_to_bit_test, optimize_range_tests): Fix
up formatting.
* gcc.dg/pr69802.c: New test.
--- gcc/tree-ssa-reassoc.c.jj 2016-02-12 10:23:51.000000000 +0100
+++ gcc/tree-ssa-reassoc.c 2016-02-15 14:25:54.996572238 +0100
@@ -2046,19 +2046,41 @@ update_range_test (struct range_entry *r
{
operand_entry *oe = (*ops)[range->idx];
tree op = oe->op;
- gimple *stmt = op ? SSA_NAME_DEF_STMT (op) :
- last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
+ gimple *stmt = op ? SSA_NAME_DEF_STMT (op)
+ : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
location_t loc = gimple_location (stmt);
tree optype = op ? TREE_TYPE (op) : boolean_type_node;
tree tem = build_range_check (loc, optype, unshare_expr (exp),
in_p, low, high);
enum warn_strict_overflow_code wc = WARN_STRICT_OVERFLOW_COMPARISON;
gimple_stmt_iterator gsi;
- unsigned int i;
+ unsigned int i, uid;
if (tem == NULL_TREE)
return false;
+ /* If op is default def SSA_NAME, there is no place to insert the
+ new comparison. Give up, unless we can use OP itself as the
+ range test. */
+ if (op && SSA_NAME_IS_DEFAULT_DEF (op))
+ {
+ if (op == range->exp
+ && ((TYPE_PRECISION (optype) == 1 && TYPE_UNSIGNED (optype))
+ || TREE_CODE (optype) == BOOLEAN_TYPE)
+ && (op == tem
+ || (TREE_CODE (tem) == EQ_EXPR
+ && TREE_OPERAND (tem, 0) == op
+ && integer_onep (TREE_OPERAND (tem, 1))))
+ && opcode != BIT_IOR_EXPR
+ && (opcode != ERROR_MARK || oe->rank != BIT_IOR_EXPR))
+ {
+ stmt = NULL;
+ tem = op;
+ }
+ else
+ return false;
+ }
+
if (strict_overflow_p && issue_strict_overflow_warning (wc))
warning_at (loc, OPT_Wstrict_overflow,
"assuming signed overflow does not occur "
@@ -2096,12 +2118,22 @@ update_range_test (struct range_entry *r
tem = invert_truthvalue_loc (loc, tem);
tem = fold_convert_loc (loc, optype, tem);
- gsi = gsi_for_stmt (stmt);
- unsigned int uid = gimple_uid (stmt);
+ if (stmt)
+ {
+ gsi = gsi_for_stmt (stmt);
+ uid = gimple_uid (stmt);
+ }
+ else
+ {
+ gsi = gsi_none ();
+ uid = 0;
+ }
+ if (stmt == NULL)
+ gcc_checking_assert (tem == op);
/* In rare cases range->exp can be equal to lhs of stmt.
In that case we have to insert after the stmt rather then before
it. If stmt is a PHI, insert it at the start of the basic block. */
- if (op != range->exp)
+ else if (op != range->exp)
{
gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
@@ -2489,7 +2521,7 @@ optimize_range_tests_to_bit_test (enum t
operand_entry *oe = (*ops)[ranges[i].idx];
tree op = oe->op;
gimple *stmt = op ? SSA_NAME_DEF_STMT (op)
- : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
+ : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
location_t loc = gimple_location (stmt);
tree optype = op ? TREE_TYPE (op) : boolean_type_node;
@@ -2553,7 +2585,7 @@ optimize_range_tests_to_bit_test (enum t
gcc_assert (TREE_CODE (exp) == SSA_NAME);
gimple_set_visited (SSA_NAME_DEF_STMT (exp), true);
gimple *g = gimple_build_assign (make_ssa_name (optype),
- BIT_IOR_EXPR, tem, exp);
+ BIT_IOR_EXPR, tem, exp);
gimple_set_location (g, loc);
gimple_seq_add_stmt_without_update (&seq, g);
exp = gimple_assign_lhs (g);
@@ -2599,8 +2631,9 @@ optimize_range_tests (enum tree_code opc
oe = (*ops)[i];
ranges[i].idx = i;
init_range_entry (ranges + i, oe->op,
- oe->op ? NULL :
- last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id)));
+ oe->op
+ ? NULL
+ : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id)));
/* For | invert it now, we will invert it again before emitting
the optimized expression. */
if (opcode == BIT_IOR_EXPR
--- gcc/testsuite/gcc.dg/pr69802.c.jj 2016-02-15 14:06:22.716491146 +0100
+++ gcc/testsuite/gcc.dg/pr69802.c 2016-02-15 14:05:58.000000000 +0100
@@ -0,0 +1,23 @@
+/* PR tree-optimization/69802 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+struct S { unsigned f : 1; };
+int a, d;
+
+int
+foo (void)
+{
+ unsigned b = 0;
+ struct S c;
+ d = ((1 && b) < c.f) & c.f; /* { dg-warning "is used uninitialized" } */
+ return a;
+}
+
+int
+bar (_Bool c)
+{
+ unsigned b = 0;
+ d = ((1 && b) < c) & c;
+ return a;
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix reassoc ICE (PR tree-optimization/69802)
2016-02-15 20:54 [PATCH] Fix reassoc ICE (PR tree-optimization/69802) Jakub Jelinek
@ 2016-02-15 21:27 ` Michael Matz
2016-02-15 21:41 ` Jakub Jelinek
2016-02-16 9:09 ` Richard Biener
1 sibling, 1 reply; 4+ messages in thread
From: Michael Matz @ 2016-02-15 21:27 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches
Hi,
On Mon, 15 Feb 2016, Jakub Jelinek wrote:
> + /* If op is default def SSA_NAME, there is no place to insert the
> + new comparison. Give up, unless we can use OP itself as the
> + range test. */
> + if (op && SSA_NAME_IS_DEFAULT_DEF (op))
> + {
> + if (op == range->exp
> + && ((TYPE_PRECISION (optype) == 1 && TYPE_UNSIGNED (optype))
> + || TREE_CODE (optype) == BOOLEAN_TYPE)
> + && (op == tem
> + || (TREE_CODE (tem) == EQ_EXPR
> + && TREE_OPERAND (tem, 0) == op
> + && integer_onep (TREE_OPERAND (tem, 1))))
> + && opcode != BIT_IOR_EXPR
> + && (opcode != ERROR_MARK || oe->rank != BIT_IOR_EXPR))
Perhaps just give up always, instead of this complicated (and hence
fragile) hackery? Are you 100% sure you catched everything, are there
testcases for each part of the condition (I miss at least one proving
that making the condition true is correct)?
Ciao,
Michael.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix reassoc ICE (PR tree-optimization/69802)
2016-02-15 21:27 ` Michael Matz
@ 2016-02-15 21:41 ` Jakub Jelinek
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2016-02-15 21:41 UTC (permalink / raw)
To: Michael Matz; +Cc: Richard Biener, gcc-patches
On Mon, Feb 15, 2016 at 10:27:16PM +0100, Michael Matz wrote:
> On Mon, 15 Feb 2016, Jakub Jelinek wrote:
>
> > + /* If op is default def SSA_NAME, there is no place to insert the
> > + new comparison. Give up, unless we can use OP itself as the
> > + range test. */
> > + if (op && SSA_NAME_IS_DEFAULT_DEF (op))
> > + {
> > + if (op == range->exp
> > + && ((TYPE_PRECISION (optype) == 1 && TYPE_UNSIGNED (optype))
> > + || TREE_CODE (optype) == BOOLEAN_TYPE)
> > + && (op == tem
> > + || (TREE_CODE (tem) == EQ_EXPR
> > + && TREE_OPERAND (tem, 0) == op
> > + && integer_onep (TREE_OPERAND (tem, 1))))
> > + && opcode != BIT_IOR_EXPR
> > + && (opcode != ERROR_MARK || oe->rank != BIT_IOR_EXPR))
>
> Perhaps just give up always, instead of this complicated (and hence
> fragile) hackery? Are you 100% sure you catched everything, are there
It is IMO not that fragile. The
op == range->exp is quite obvious condition, it could have been even assert
instead.
The second condition is what is used e.g. in init_range_test, i.e.
bool/unsigned :1 only. The third - op == tem is obviously good
transformation to tem = op, so the patch just makes sure we don't ICE
say in gsi_for_stmt (stmt); that is what we get for bool and is covered
in the testcase. The EQ_EXPR case is what we get for unsigned : 1 instead.
Perhaps it could be also op != 0 instead of just op == 1.
And lastly, the BIT_IOR_EXPR tests are to make sure we don't
if (opcode == BIT_IOR_EXPR
|| (opcode == ERROR_MARK && oe->rank == BIT_IOR_EXPR))
tem = invert_truthvalue_loc (loc, tem);
a few lines later. The reason to perform the check earlier is just
to avoid printing it in the dumpfile that we are changing something if we
give up instead.
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix reassoc ICE (PR tree-optimization/69802)
2016-02-15 20:54 [PATCH] Fix reassoc ICE (PR tree-optimization/69802) Jakub Jelinek
2016-02-15 21:27 ` Michael Matz
@ 2016-02-16 9:09 ` Richard Biener
1 sibling, 0 replies; 4+ messages in thread
From: Richard Biener @ 2016-02-16 9:09 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Mon, 15 Feb 2016, Jakub Jelinek wrote:
> Hi!
>
> The following patch fixes an ICE where one of the range tests
> is SSA_NAME_DEF_STMT of a bool/_Bool or unsigned : 1 bitfield.
> In that case, we don't know where to put the adjusted range test.
> The patch for this uncommon case gives up, unless the range test
> can be the SSA_NAME_DEF_STMT itself, and in that case makes sure we
> DTRT.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Richard.
> 2016-02-15 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/69802
> * tree-ssa-reassoc.c (update_range_test): If op is
> SSA_NAME_IS_DEFAULT_DEF, give up unless tem is a positive
> op == 1 test of precision 1 integral op, otherwise handle
> that case as op itself. Fix up formatting.
> (optimize_range_tests_to_bit_test, optimize_range_tests): Fix
> up formatting.
>
> * gcc.dg/pr69802.c: New test.
>
> --- gcc/tree-ssa-reassoc.c.jj 2016-02-12 10:23:51.000000000 +0100
> +++ gcc/tree-ssa-reassoc.c 2016-02-15 14:25:54.996572238 +0100
> @@ -2046,19 +2046,41 @@ update_range_test (struct range_entry *r
> {
> operand_entry *oe = (*ops)[range->idx];
> tree op = oe->op;
> - gimple *stmt = op ? SSA_NAME_DEF_STMT (op) :
> - last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
> + gimple *stmt = op ? SSA_NAME_DEF_STMT (op)
> + : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
> location_t loc = gimple_location (stmt);
> tree optype = op ? TREE_TYPE (op) : boolean_type_node;
> tree tem = build_range_check (loc, optype, unshare_expr (exp),
> in_p, low, high);
> enum warn_strict_overflow_code wc = WARN_STRICT_OVERFLOW_COMPARISON;
> gimple_stmt_iterator gsi;
> - unsigned int i;
> + unsigned int i, uid;
>
> if (tem == NULL_TREE)
> return false;
>
> + /* If op is default def SSA_NAME, there is no place to insert the
> + new comparison. Give up, unless we can use OP itself as the
> + range test. */
> + if (op && SSA_NAME_IS_DEFAULT_DEF (op))
> + {
> + if (op == range->exp
> + && ((TYPE_PRECISION (optype) == 1 && TYPE_UNSIGNED (optype))
> + || TREE_CODE (optype) == BOOLEAN_TYPE)
> + && (op == tem
> + || (TREE_CODE (tem) == EQ_EXPR
> + && TREE_OPERAND (tem, 0) == op
> + && integer_onep (TREE_OPERAND (tem, 1))))
> + && opcode != BIT_IOR_EXPR
> + && (opcode != ERROR_MARK || oe->rank != BIT_IOR_EXPR))
> + {
> + stmt = NULL;
> + tem = op;
> + }
> + else
> + return false;
> + }
> +
> if (strict_overflow_p && issue_strict_overflow_warning (wc))
> warning_at (loc, OPT_Wstrict_overflow,
> "assuming signed overflow does not occur "
> @@ -2096,12 +2118,22 @@ update_range_test (struct range_entry *r
> tem = invert_truthvalue_loc (loc, tem);
>
> tem = fold_convert_loc (loc, optype, tem);
> - gsi = gsi_for_stmt (stmt);
> - unsigned int uid = gimple_uid (stmt);
> + if (stmt)
> + {
> + gsi = gsi_for_stmt (stmt);
> + uid = gimple_uid (stmt);
> + }
> + else
> + {
> + gsi = gsi_none ();
> + uid = 0;
> + }
> + if (stmt == NULL)
> + gcc_checking_assert (tem == op);
> /* In rare cases range->exp can be equal to lhs of stmt.
> In that case we have to insert after the stmt rather then before
> it. If stmt is a PHI, insert it at the start of the basic block. */
> - if (op != range->exp)
> + else if (op != range->exp)
> {
> gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
> tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
> @@ -2489,7 +2521,7 @@ optimize_range_tests_to_bit_test (enum t
> operand_entry *oe = (*ops)[ranges[i].idx];
> tree op = oe->op;
> gimple *stmt = op ? SSA_NAME_DEF_STMT (op)
> - : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
> + : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
> location_t loc = gimple_location (stmt);
> tree optype = op ? TREE_TYPE (op) : boolean_type_node;
>
> @@ -2553,7 +2585,7 @@ optimize_range_tests_to_bit_test (enum t
> gcc_assert (TREE_CODE (exp) == SSA_NAME);
> gimple_set_visited (SSA_NAME_DEF_STMT (exp), true);
> gimple *g = gimple_build_assign (make_ssa_name (optype),
> - BIT_IOR_EXPR, tem, exp);
> + BIT_IOR_EXPR, tem, exp);
> gimple_set_location (g, loc);
> gimple_seq_add_stmt_without_update (&seq, g);
> exp = gimple_assign_lhs (g);
> @@ -2599,8 +2631,9 @@ optimize_range_tests (enum tree_code opc
> oe = (*ops)[i];
> ranges[i].idx = i;
> init_range_entry (ranges + i, oe->op,
> - oe->op ? NULL :
> - last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id)));
> + oe->op
> + ? NULL
> + : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id)));
> /* For | invert it now, we will invert it again before emitting
> the optimized expression. */
> if (opcode == BIT_IOR_EXPR
> --- gcc/testsuite/gcc.dg/pr69802.c.jj 2016-02-15 14:06:22.716491146 +0100
> +++ gcc/testsuite/gcc.dg/pr69802.c 2016-02-15 14:05:58.000000000 +0100
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/69802 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall" } */
> +
> +struct S { unsigned f : 1; };
> +int a, d;
> +
> +int
> +foo (void)
> +{
> + unsigned b = 0;
> + struct S c;
> + d = ((1 && b) < c.f) & c.f; /* { dg-warning "is used uninitialized" } */
> + return a;
> +}
> +
> +int
> +bar (_Bool c)
> +{
> + unsigned b = 0;
> + d = ((1 && b) < c) & c;
> + return a;
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-16 9:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-15 20:54 [PATCH] Fix reassoc ICE (PR tree-optimization/69802) Jakub Jelinek
2016-02-15 21:27 ` Michael Matz
2016-02-15 21:41 ` Jakub Jelinek
2016-02-16 9:09 ` 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).