* [PATCH] Fix PR64614
@ 2015-01-16 13:16 Richard Biener
2015-01-16 13:25 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2015-01-16 13:16 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek
The following fixes PR64614, maybe-uninit warnings in switches
and guards like X & 3.
The support for switches is quite limited (because the predicate
infrastructure in the pass is quite limited), but it seems to
handle this particular case at least.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Any idea how to better handle the case label lookup? Unfortunately
the pass doesn't have a general information gathering phase or a
lattice... Maybe we just don't care? Or put in an arbitrary
limit on gimple_switch_num_labels we handle?
Thanks,
Richard.
2015-01-16 Richard Biener <rguenther@suse.de>
PR middle-end/64614
* tree-ssa-uninit.c: Include tree-cfg.h.
(convert_control_dep_chain_into_preds): Handle switch statements.
(is_pred_expr_subset_of): Handle x == CST vs. (x & CST) != 0.
(normalize_one_pred_1): Do not split bit-manipulations.
Record (x & CST).
* gcc.dg/uninit-18.c: New testcase.
Index: gcc/tree-ssa-uninit.c
===================================================================
--- gcc/tree-ssa-uninit.c (revision 219724)
+++ gcc/tree-ssa-uninit.c (working copy)
@@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.
#include "tree-pass.h"
#include "diagnostic-core.h"
#include "params.h"
+#include "tree-cfg.h"
/* This implements the pass that does predicate aware warning on uses of
possibly uninitialized variables. The pass first collects the set of
@@ -592,17 +593,58 @@ convert_control_dep_chain_into_preds (ve
if (skip)
continue;
}
- if (gimple_code (cond_stmt) != GIMPLE_COND)
+ if (gimple_code (cond_stmt) == GIMPLE_COND)
+ {
+ one_pred.pred_lhs = gimple_cond_lhs (cond_stmt);
+ one_pred.pred_rhs = gimple_cond_rhs (cond_stmt);
+ one_pred.cond_code = gimple_cond_code (cond_stmt);
+ one_pred.invert = !!(e->flags & EDGE_FALSE_VALUE);
+ t_chain.safe_push (one_pred);
+ has_valid_pred = true;
+ }
+ else if (gswitch *gs = dyn_cast <gswitch *> (cond_stmt))
+ {
+ /* Find the case label.
+ ??? Ugh, this is quadratic. */
+ tree l = NULL_TREE;
+ unsigned idx;
+ for (idx = 0; idx < gimple_switch_num_labels (gs); ++idx)
+ {
+ tree tl = gimple_switch_label (gs, idx);
+ if (e->dest == label_to_block (CASE_LABEL (tl)))
+ {
+ if (!l)
+ l = tl;
+ else
+ {
+ l = NULL_TREE;
+ break;
+ }
+ }
+ }
+ /* If more than one label reaches this block or the case
+ label doesn't have a single value (like the default one)
+ fail. */
+ if (!l
+ || !CASE_LOW (l)
+ || (CASE_HIGH (l) && !operand_equal_p (CASE_LOW (l),
+ CASE_HIGH (l), 0)))
+ {
+ has_valid_pred = false;
+ break;
+ }
+ one_pred.pred_lhs = gimple_switch_index (gs);
+ one_pred.pred_rhs = CASE_LOW (l);
+ one_pred.cond_code = EQ_EXPR;
+ one_pred.invert = false;
+ t_chain.safe_push (one_pred);
+ has_valid_pred = true;
+ }
+ else
{
has_valid_pred = false;
break;
}
- one_pred.pred_lhs = gimple_cond_lhs (cond_stmt);
- one_pred.pred_rhs = gimple_cond_rhs (cond_stmt);
- one_pred.cond_code = gimple_cond_code (cond_stmt);
- one_pred.invert = !!(e->flags & EDGE_FALSE_VALUE);
- t_chain.safe_push (one_pred);
- has_valid_pred = true;
}
if (!has_valid_pred)
@@ -1329,6 +1371,10 @@ is_pred_expr_subset_of (pred_info expr1,
if (expr2.invert)
code2 = invert_tree_comparison (code2, false);
+ if (code1 == EQ_EXPR && code2 == BIT_AND_EXPR)
+ return wi::eq_p (expr1.pred_rhs,
+ wi::bit_and (expr1.pred_rhs, expr2.pred_rhs));
+
if (code1 != code2 && code2 != NE_EXPR)
return false;
@@ -1970,8 +2016,25 @@ normalize_one_pred_1 (pred_chain_union *
}
else if (gimple_assign_rhs_code (def_stmt) == and_or_code)
{
- push_to_worklist (gimple_assign_rhs1 (def_stmt), work_list, mark_set);
- push_to_worklist (gimple_assign_rhs2 (def_stmt), work_list, mark_set);
+ /* Avoid splitting up bit manipulations like x & 3 or y | 1. */
+ if (is_gimple_min_invariant (gimple_assign_rhs2 (def_stmt)))
+ {
+ /* But treat x & 3 as condition. */
+ if (and_or_code == BIT_AND_EXPR)
+ {
+ pred_info n_pred;
+ n_pred.pred_lhs = gimple_assign_rhs1 (def_stmt);
+ n_pred.pred_rhs = gimple_assign_rhs2 (def_stmt);
+ n_pred.cond_code = and_or_code;
+ n_pred.invert = false;
+ norm_chain->safe_push (n_pred);
+ }
+ }
+ else
+ {
+ push_to_worklist (gimple_assign_rhs1 (def_stmt), work_list, mark_set);
+ push_to_worklist (gimple_assign_rhs2 (def_stmt), work_list, mark_set);
+ }
}
else if (TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt))
== tcc_comparison)
Index: gcc/testsuite/gcc.dg/uninit-18.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-18.c (revision 0)
+++ gcc/testsuite/gcc.dg/uninit-18.c (working copy)
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wuninitialized" } */
+
+char *foo(int bar, char *baz)
+{
+ char *tmp;
+
+ if (bar & 3)
+ tmp = baz;
+
+ switch (bar) {
+ case 1:
+ tmp[5] = 7; /* { dg-bogus "may be used uninitialized" } */
+ break;
+ case 2:
+ tmp[11] = 15; /* { dg-bogus "may be used uninitialized" } */
+ break;
+ default:
+ tmp = 0;
+ break;
+ }
+
+ return tmp; /* { dg-bogus "may be used uninitialized" } */
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix PR64614
2015-01-16 13:16 [PATCH] Fix PR64614 Richard Biener
@ 2015-01-16 13:25 ` Jakub Jelinek
2015-01-16 13:38 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2015-01-16 13:25 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Fri, Jan 16, 2015 at 02:02:37PM +0100, Richard Biener wrote:
>
> The following fixes PR64614, maybe-uninit warnings in switches
> and guards like X & 3.
>
> The support for switches is quite limited (because the predicate
> infrastructure in the pass is quite limited), but it seems to
> handle this particular case at least.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> Any idea how to better handle the case label lookup? Unfortunately
> the pass doesn't have a general information gathering phase or a
> lattice... Maybe we just don't care? Or put in an arbitrary
> limit on gimple_switch_num_labels we handle?
The pass already has various other limits:
#define MAX_NUM_CHAINS 8
#define MAX_CHAIN_LEN 5
#define MAX_POSTDOM_CHECK 8
I'd just add MAX_SWITCH_CASES 40
or similar. Or, turn all those into proper params ;)
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix PR64614
2015-01-16 13:25 ` Jakub Jelinek
@ 2015-01-16 13:38 ` Richard Biener
0 siblings, 0 replies; 3+ messages in thread
From: Richard Biener @ 2015-01-16 13:38 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Fri, 16 Jan 2015, Jakub Jelinek wrote:
> On Fri, Jan 16, 2015 at 02:02:37PM +0100, Richard Biener wrote:
> >
> > The following fixes PR64614, maybe-uninit warnings in switches
> > and guards like X & 3.
> >
> > The support for switches is quite limited (because the predicate
> > infrastructure in the pass is quite limited), but it seems to
> > handle this particular case at least.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > Any idea how to better handle the case label lookup? Unfortunately
> > the pass doesn't have a general information gathering phase or a
> > lattice... Maybe we just don't care? Or put in an arbitrary
> > limit on gimple_switch_num_labels we handle?
>
> The pass already has various other limits:
> #define MAX_NUM_CHAINS 8
> #define MAX_CHAIN_LEN 5
> #define MAX_POSTDOM_CHECK 8
> I'd just add MAX_SWITCH_CASES 40
Done, committed as follows.
Richard.
2015-01-16 Richard Biener <rguenther@suse.de>
PR middle-end/64614
* tree-ssa-uninit.c: Include tree-cfg.h.
(MAX_SWITCH_CASES): New define.
(convert_control_dep_chain_into_preds): Handle switch statements.
(is_pred_expr_subset_of): Handle x == CST vs. (x & CST) != 0.
(normalize_one_pred_1): Do not split bit-manipulations.
Record (x & CST).
* gcc.dg/uninit-18.c: New testcase.
Index: gcc/tree-ssa-uninit.c
===================================================================
*** gcc/tree-ssa-uninit.c.orig 2015-01-16 14:14:18.135193759 +0100
--- gcc/tree-ssa-uninit.c 2015-01-16 14:16:16.635189656 +0100
*************** along with GCC; see the file COPYING3.
*** 58,63 ****
--- 58,64 ----
#include "tree-pass.h"
#include "diagnostic-core.h"
#include "params.h"
+ #include "tree-cfg.h"
/* This implements the pass that does predicate aware warning on uses of
possibly uninitialized variables. The pass first collects the set of
*************** find_control_equiv_block (basic_block bb
*** 411,416 ****
--- 412,418 ----
#define MAX_NUM_CHAINS 8
#define MAX_CHAIN_LEN 5
#define MAX_POSTDOM_CHECK 8
+ #define MAX_SWITCH_CASES 40
/* Computes the control dependence chains (paths of edges)
for DEP_BB up to the dominating basic block BB (the head node of a
*************** convert_control_dep_chain_into_preds (ve
*** 592,608 ****
if (skip)
continue;
}
! if (gimple_code (cond_stmt) != GIMPLE_COND)
{
has_valid_pred = false;
break;
}
- one_pred.pred_lhs = gimple_cond_lhs (cond_stmt);
- one_pred.pred_rhs = gimple_cond_rhs (cond_stmt);
- one_pred.cond_code = gimple_cond_code (cond_stmt);
- one_pred.invert = !!(e->flags & EDGE_FALSE_VALUE);
- t_chain.safe_push (one_pred);
- has_valid_pred = true;
}
if (!has_valid_pred)
--- 594,656 ----
if (skip)
continue;
}
! if (gimple_code (cond_stmt) == GIMPLE_COND)
! {
! one_pred.pred_lhs = gimple_cond_lhs (cond_stmt);
! one_pred.pred_rhs = gimple_cond_rhs (cond_stmt);
! one_pred.cond_code = gimple_cond_code (cond_stmt);
! one_pred.invert = !!(e->flags & EDGE_FALSE_VALUE);
! t_chain.safe_push (one_pred);
! has_valid_pred = true;
! }
! else if (gswitch *gs = dyn_cast <gswitch *> (cond_stmt))
! {
! /* Avoid quadratic behavior. */
! if (gimple_switch_num_labels (gs) > MAX_SWITCH_CASES)
! {
! has_valid_pred = false;
! break;
! }
! /* Find the case label. */
! tree l = NULL_TREE;
! unsigned idx;
! for (idx = 0; idx < gimple_switch_num_labels (gs); ++idx)
! {
! tree tl = gimple_switch_label (gs, idx);
! if (e->dest == label_to_block (CASE_LABEL (tl)))
! {
! if (!l)
! l = tl;
! else
! {
! l = NULL_TREE;
! break;
! }
! }
! }
! /* If more than one label reaches this block or the case
! label doesn't have a single value (like the default one)
! fail. */
! if (!l
! || !CASE_LOW (l)
! || (CASE_HIGH (l) && !operand_equal_p (CASE_LOW (l),
! CASE_HIGH (l), 0)))
! {
! has_valid_pred = false;
! break;
! }
! one_pred.pred_lhs = gimple_switch_index (gs);
! one_pred.pred_rhs = CASE_LOW (l);
! one_pred.cond_code = EQ_EXPR;
! one_pred.invert = false;
! t_chain.safe_push (one_pred);
! has_valid_pred = true;
! }
! else
{
has_valid_pred = false;
break;
}
}
if (!has_valid_pred)
*************** is_pred_expr_subset_of (pred_info expr1,
*** 1329,1334 ****
--- 1377,1386 ----
if (expr2.invert)
code2 = invert_tree_comparison (code2, false);
+ if (code1 == EQ_EXPR && code2 == BIT_AND_EXPR)
+ return wi::eq_p (expr1.pred_rhs,
+ wi::bit_and (expr1.pred_rhs, expr2.pred_rhs));
+
if (code1 != code2 && code2 != NE_EXPR)
return false;
*************** normalize_one_pred_1 (pred_chain_union *
*** 1970,1977 ****
}
else if (gimple_assign_rhs_code (def_stmt) == and_or_code)
{
! push_to_worklist (gimple_assign_rhs1 (def_stmt), work_list, mark_set);
! push_to_worklist (gimple_assign_rhs2 (def_stmt), work_list, mark_set);
}
else if (TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt))
== tcc_comparison)
--- 2022,2046 ----
}
else if (gimple_assign_rhs_code (def_stmt) == and_or_code)
{
! /* Avoid splitting up bit manipulations like x & 3 or y | 1. */
! if (is_gimple_min_invariant (gimple_assign_rhs2 (def_stmt)))
! {
! /* But treat x & 3 as condition. */
! if (and_or_code == BIT_AND_EXPR)
! {
! pred_info n_pred;
! n_pred.pred_lhs = gimple_assign_rhs1 (def_stmt);
! n_pred.pred_rhs = gimple_assign_rhs2 (def_stmt);
! n_pred.cond_code = and_or_code;
! n_pred.invert = false;
! norm_chain->safe_push (n_pred);
! }
! }
! else
! {
! push_to_worklist (gimple_assign_rhs1 (def_stmt), work_list, mark_set);
! push_to_worklist (gimple_assign_rhs2 (def_stmt), work_list, mark_set);
! }
}
else if (TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt))
== tcc_comparison)
Index: gcc/testsuite/gcc.dg/uninit-18.c
===================================================================
*** /dev/null 1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/uninit-18.c 2015-01-16 14:14:28.304193407 +0100
***************
*** 0 ****
--- 1,24 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O -Wuninitialized" } */
+
+ char *foo(int bar, char *baz)
+ {
+ char *tmp;
+
+ if (bar & 3)
+ tmp = baz;
+
+ switch (bar) {
+ case 1:
+ tmp[5] = 7; /* { dg-bogus "may be used uninitialized" } */
+ break;
+ case 2:
+ tmp[11] = 15; /* { dg-bogus "may be used uninitialized" } */
+ break;
+ default:
+ tmp = 0;
+ break;
+ }
+
+ return tmp; /* { dg-bogus "may be used uninitialized" } */
+ }
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-01-16 13:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-16 13:16 [PATCH] Fix PR64614 Richard Biener
2015-01-16 13:25 ` Jakub Jelinek
2015-01-16 13:38 ` 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).