* [PR68083] stop ifcombine from moving uninitialized uses before their guards
@ 2015-10-30 13:57 Alexandre Oliva
2015-10-30 18:03 ` Alexandre Oliva
2015-11-02 9:32 ` Richard Biener
0 siblings, 2 replies; 4+ messages in thread
From: Alexandre Oliva @ 2015-10-30 13:57 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
The ifcombine pass may move a conditional access to an uninitialized
value before the condition that ensures it is always well-defined,
thus introducing undefined behavior. Stop it from doing so.
Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install?
Incidentally, bb_no_side_effects_p (inner_cond_bb) is called in all four
tests in tree_ssa_ifcombine_bb_1, for each outer_cond_bb that
tree_ssa_ifcombine_bb might choose. Is there any reason to not factor
it out to the test that checks whether the inner_cond_bb is indeed an
if_then_else block, early in tree_ssa_ifcombine_bb, so as to
short-circuit the whole thing when the inner block is not viable?
for gcc/ChangeLog
PR tree-optimization/68083
* tree-ssa-ifcombine.c: Include tree-ssa.h.
(bb_no_side_effects_p): Test for undefined uses too.
* tree-ssa.c (gimple_uses_undefined_value_p): New.
* tree-ssa.h (gimple_uses_undefined_value_p): Declare.
for gcc/testsuite/ChangeLog
PR tree-optimization/68083
* gcc.dg/torture/pr68083.c: New. From Zhendong Su.
---
gcc/testsuite/gcc.dg/torture/pr68083.c | 35 ++++++++++++++++++++++++++++++++
gcc/tree-ssa-ifcombine.c | 2 ++
gcc/tree-ssa.c | 18 ++++++++++++++++
gcc/tree-ssa.h | 1 +
4 files changed, 56 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr68083.c
diff --git a/gcc/testsuite/gcc.dg/torture/pr68083.c b/gcc/testsuite/gcc.dg/torture/pr68083.c
new file mode 100644
index 0000000..ae24781
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr68083.c
@@ -0,0 +1,35 @@
+/* { dg-do run } */
+
+int a = 2, b = 1, c = 1;
+
+int
+fn1 ()
+{
+ int d;
+ for (; a; a--)
+ {
+ for (d = 0; d < 4; d++)
+ {
+ int k;
+ if (c < 1)
+ if (k)
+ c = 0;
+ if (b)
+ continue;
+ return 0;
+ }
+ b = !1;
+ }
+ return 0;
+}
+
+int
+main ()
+{
+ fn1 ();
+
+ if (a != 1)
+ __builtin_abort ();
+
+ return 0;
+}
diff --git a/gcc/tree-ssa-ifcombine.c b/gcc/tree-ssa-ifcombine.c
index ca55b57..622dc6b 100644
--- a/gcc/tree-ssa-ifcombine.c
+++ b/gcc/tree-ssa-ifcombine.c
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-iterator.h"
#include "gimplify-me.h"
#include "tree-cfg.h"
+#include "tree-ssa.h"
#ifndef LOGICAL_OP_NON_SHORT_CIRCUIT
#define LOGICAL_OP_NON_SHORT_CIRCUIT \
@@ -125,6 +126,7 @@ bb_no_side_effects_p (basic_block bb)
continue;
if (gimple_has_side_effects (stmt)
+ || gimple_uses_undefined_value_p (stmt)
|| gimple_could_trap_p (stmt)
|| gimple_vuse (stmt))
return false;
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index c7be442..8dc2d61 100644
--- a/gcc/tree-ssa.c
+++ b/gcc/tree-ssa.c
@@ -1210,6 +1210,24 @@ ssa_undefined_value_p (tree t, bool partial)
}
+/* Return TRUE iff STMT, a gimple statement, references an undefined
+ SSA name. */
+
+bool
+gimple_uses_undefined_value_p (gimple *stmt)
+{
+ ssa_op_iter iter;
+ tree op;
+
+ FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE)
+ if (ssa_undefined_value_p (op))
+ return true;
+
+ return false;
+}
+
+
+
/* If necessary, rewrite the base of the reference tree *TP from
a MEM_REF to a plain or converted symbol. */
diff --git a/gcc/tree-ssa.h b/gcc/tree-ssa.h
index 5a409e5..3b5bd70 100644
--- a/gcc/tree-ssa.h
+++ b/gcc/tree-ssa.h
@@ -51,6 +51,7 @@ extern bool tree_ssa_useless_type_conversion (tree);
extern tree tree_ssa_strip_useless_type_conversions (tree);
extern bool ssa_undefined_value_p (tree, bool = true);
+extern bool gimple_uses_undefined_value_p (gimple *);
extern void execute_update_addresses_taken (void);
/* Given an edge_var_map V, return the PHI arg definition. */
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PR68083] stop ifcombine from moving uninitialized uses before their guards
2015-10-30 13:57 [PR68083] stop ifcombine from moving uninitialized uses before their guards Alexandre Oliva
@ 2015-10-30 18:03 ` Alexandre Oliva
2015-11-02 9:32 ` Richard Biener
2015-11-02 9:32 ` Richard Biener
1 sibling, 1 reply; 4+ messages in thread
From: Alexandre Oliva @ 2015-10-30 18:03 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Oct 30, 2015, Alexandre Oliva <aoliva@redhat.com> wrote:
> Incidentally, bb_no_side_effects_p (inner_cond_bb) is called in all four
> tests in tree_ssa_ifcombine_bb_1, for each outer_cond_bb that
> tree_ssa_ifcombine_bb might choose. Is there any reason to not factor
> it out to the test that checks whether the inner_cond_bb is indeed an
> if_then_else block, early in tree_ssa_ifcombine_bb, so as to
> short-circuit the whole thing when the inner block is not viable?
Like this...
Bail out early if the inner block has side effects or is otherwise not
eligible for ifcombine.
Regstrapped on x86_64-linux-gnu. Ok to install?
for gcc/ChangeLog
* tree-ssa-ifcombine.c (tree_ssa_ifcombine_bb_1): Factor out
bb_no_side_effects_p tests...
(tree_ssa_ifcombine_bb): ... here.
---
gcc/tree-ssa-ifcombine.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/gcc/tree-ssa-ifcombine.c b/gcc/tree-ssa-ifcombine.c
index 622dc6b..3b60968 100644
--- a/gcc/tree-ssa-ifcombine.c
+++ b/gcc/tree-ssa-ifcombine.c
@@ -579,8 +579,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
the inner cond_bb having no side-effects. */
if (phi_pred_bb != else_bb
&& recognize_if_then_else (outer_cond_bb, &inner_cond_bb, &else_bb)
- && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb)
- && bb_no_side_effects_p (inner_cond_bb))
+ && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb))
{
/* We have
<outer_cond_bb>
@@ -598,8 +597,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
/* And a version where the outer condition is negated. */
if (phi_pred_bb != else_bb
&& recognize_if_then_else (outer_cond_bb, &else_bb, &inner_cond_bb)
- && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb)
- && bb_no_side_effects_p (inner_cond_bb))
+ && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb))
{
/* We have
<outer_cond_bb>
@@ -620,8 +618,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
having no side-effects. */
if (phi_pred_bb != then_bb
&& recognize_if_then_else (outer_cond_bb, &then_bb, &inner_cond_bb)
- && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb)
- && bb_no_side_effects_p (inner_cond_bb))
+ && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb))
{
/* We have
<outer_cond_bb>
@@ -638,8 +635,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
/* And a version where the outer condition is negated. */
if (phi_pred_bb != then_bb
&& recognize_if_then_else (outer_cond_bb, &inner_cond_bb, &then_bb)
- && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb)
- && bb_no_side_effects_p (inner_cond_bb))
+ && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb))
{
/* We have
<outer_cond_bb>
@@ -676,7 +672,8 @@ tree_ssa_ifcombine_bb (basic_block inner_cond_bb)
if (a && b)
;
This requires a single predecessor of the inner cond_bb. */
- if (single_pred_p (inner_cond_bb))
+ if (single_pred_p (inner_cond_bb)
+ && bb_no_side_effects_p (inner_cond_bb))
{
basic_block outer_cond_bb = single_pred (inner_cond_bb);
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PR68083] stop ifcombine from moving uninitialized uses before their guards
2015-10-30 13:57 [PR68083] stop ifcombine from moving uninitialized uses before their guards Alexandre Oliva
2015-10-30 18:03 ` Alexandre Oliva
@ 2015-11-02 9:32 ` Richard Biener
1 sibling, 0 replies; 4+ messages in thread
From: Richard Biener @ 2015-11-02 9:32 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: GCC Patches
On Fri, Oct 30, 2015 at 2:41 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> The ifcombine pass may move a conditional access to an uninitialized
> value before the condition that ensures it is always well-defined,
> thus introducing undefined behavior. Stop it from doing so.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install?
Ok.
Thanks,
Richard.
>
> Incidentally, bb_no_side_effects_p (inner_cond_bb) is called in all four
> tests in tree_ssa_ifcombine_bb_1, for each outer_cond_bb that
> tree_ssa_ifcombine_bb might choose. Is there any reason to not factor
> it out to the test that checks whether the inner_cond_bb is indeed an
> if_then_else block, early in tree_ssa_ifcombine_bb, so as to
> short-circuit the whole thing when the inner block is not viable?
>
>
> for gcc/ChangeLog
>
> PR tree-optimization/68083
> * tree-ssa-ifcombine.c: Include tree-ssa.h.
> (bb_no_side_effects_p): Test for undefined uses too.
> * tree-ssa.c (gimple_uses_undefined_value_p): New.
> * tree-ssa.h (gimple_uses_undefined_value_p): Declare.
>
> for gcc/testsuite/ChangeLog
>
> PR tree-optimization/68083
> * gcc.dg/torture/pr68083.c: New. From Zhendong Su.
> ---
> gcc/testsuite/gcc.dg/torture/pr68083.c | 35 ++++++++++++++++++++++++++++++++
> gcc/tree-ssa-ifcombine.c | 2 ++
> gcc/tree-ssa.c | 18 ++++++++++++++++
> gcc/tree-ssa.h | 1 +
> 4 files changed, 56 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr68083.c
>
> diff --git a/gcc/testsuite/gcc.dg/torture/pr68083.c b/gcc/testsuite/gcc.dg/torture/pr68083.c
> new file mode 100644
> index 0000000..ae24781
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr68083.c
> @@ -0,0 +1,35 @@
> +/* { dg-do run } */
> +
> +int a = 2, b = 1, c = 1;
> +
> +int
> +fn1 ()
> +{
> + int d;
> + for (; a; a--)
> + {
> + for (d = 0; d < 4; d++)
> + {
> + int k;
> + if (c < 1)
> + if (k)
> + c = 0;
> + if (b)
> + continue;
> + return 0;
> + }
> + b = !1;
> + }
> + return 0;
> +}
> +
> +int
> +main ()
> +{
> + fn1 ();
> +
> + if (a != 1)
> + __builtin_abort ();
> +
> + return 0;
> +}
> diff --git a/gcc/tree-ssa-ifcombine.c b/gcc/tree-ssa-ifcombine.c
> index ca55b57..622dc6b 100644
> --- a/gcc/tree-ssa-ifcombine.c
> +++ b/gcc/tree-ssa-ifcombine.c
> @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see
> #include "gimple-iterator.h"
> #include "gimplify-me.h"
> #include "tree-cfg.h"
> +#include "tree-ssa.h"
>
> #ifndef LOGICAL_OP_NON_SHORT_CIRCUIT
> #define LOGICAL_OP_NON_SHORT_CIRCUIT \
> @@ -125,6 +126,7 @@ bb_no_side_effects_p (basic_block bb)
> continue;
>
> if (gimple_has_side_effects (stmt)
> + || gimple_uses_undefined_value_p (stmt)
> || gimple_could_trap_p (stmt)
> || gimple_vuse (stmt))
> return false;
> diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
> index c7be442..8dc2d61 100644
> --- a/gcc/tree-ssa.c
> +++ b/gcc/tree-ssa.c
> @@ -1210,6 +1210,24 @@ ssa_undefined_value_p (tree t, bool partial)
> }
>
>
> +/* Return TRUE iff STMT, a gimple statement, references an undefined
> + SSA name. */
> +
> +bool
> +gimple_uses_undefined_value_p (gimple *stmt)
> +{
> + ssa_op_iter iter;
> + tree op;
> +
> + FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE)
> + if (ssa_undefined_value_p (op))
> + return true;
> +
> + return false;
> +}
> +
> +
> +
> /* If necessary, rewrite the base of the reference tree *TP from
> a MEM_REF to a plain or converted symbol. */
>
> diff --git a/gcc/tree-ssa.h b/gcc/tree-ssa.h
> index 5a409e5..3b5bd70 100644
> --- a/gcc/tree-ssa.h
> +++ b/gcc/tree-ssa.h
> @@ -51,6 +51,7 @@ extern bool tree_ssa_useless_type_conversion (tree);
> extern tree tree_ssa_strip_useless_type_conversions (tree);
>
> extern bool ssa_undefined_value_p (tree, bool = true);
> +extern bool gimple_uses_undefined_value_p (gimple *);
> extern void execute_update_addresses_taken (void);
>
> /* Given an edge_var_map V, return the PHI arg definition. */
>
>
> --
> Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/ FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PR68083] stop ifcombine from moving uninitialized uses before their guards
2015-10-30 18:03 ` Alexandre Oliva
@ 2015-11-02 9:32 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2015-11-02 9:32 UTC (permalink / raw)
To: Alexandre Oliva; +Cc: GCC Patches
On Fri, Oct 30, 2015 at 7:02 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Oct 30, 2015, Alexandre Oliva <aoliva@redhat.com> wrote:
>
>> Incidentally, bb_no_side_effects_p (inner_cond_bb) is called in all four
>> tests in tree_ssa_ifcombine_bb_1, for each outer_cond_bb that
>> tree_ssa_ifcombine_bb might choose. Is there any reason to not factor
>> it out to the test that checks whether the inner_cond_bb is indeed an
>> if_then_else block, early in tree_ssa_ifcombine_bb, so as to
>> short-circuit the whole thing when the inner block is not viable?
>
> Like this...
>
> Bail out early if the inner block has side effects or is otherwise not
> eligible for ifcombine.
>
> Regstrapped on x86_64-linux-gnu. Ok to install?
Ok.
Thanks,
Richard.
>
> for gcc/ChangeLog
>
> * tree-ssa-ifcombine.c (tree_ssa_ifcombine_bb_1): Factor out
> bb_no_side_effects_p tests...
> (tree_ssa_ifcombine_bb): ... here.
> ---
> gcc/tree-ssa-ifcombine.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/tree-ssa-ifcombine.c b/gcc/tree-ssa-ifcombine.c
> index 622dc6b..3b60968 100644
> --- a/gcc/tree-ssa-ifcombine.c
> +++ b/gcc/tree-ssa-ifcombine.c
> @@ -579,8 +579,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
> the inner cond_bb having no side-effects. */
> if (phi_pred_bb != else_bb
> && recognize_if_then_else (outer_cond_bb, &inner_cond_bb, &else_bb)
> - && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb)
> - && bb_no_side_effects_p (inner_cond_bb))
> + && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb))
> {
> /* We have
> <outer_cond_bb>
> @@ -598,8 +597,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
> /* And a version where the outer condition is negated. */
> if (phi_pred_bb != else_bb
> && recognize_if_then_else (outer_cond_bb, &else_bb, &inner_cond_bb)
> - && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb)
> - && bb_no_side_effects_p (inner_cond_bb))
> + && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb))
> {
> /* We have
> <outer_cond_bb>
> @@ -620,8 +618,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
> having no side-effects. */
> if (phi_pred_bb != then_bb
> && recognize_if_then_else (outer_cond_bb, &then_bb, &inner_cond_bb)
> - && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb)
> - && bb_no_side_effects_p (inner_cond_bb))
> + && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb))
> {
> /* We have
> <outer_cond_bb>
> @@ -638,8 +635,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
> /* And a version where the outer condition is negated. */
> if (phi_pred_bb != then_bb
> && recognize_if_then_else (outer_cond_bb, &inner_cond_bb, &then_bb)
> - && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb)
> - && bb_no_side_effects_p (inner_cond_bb))
> + && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb))
> {
> /* We have
> <outer_cond_bb>
> @@ -676,7 +672,8 @@ tree_ssa_ifcombine_bb (basic_block inner_cond_bb)
> if (a && b)
> ;
> This requires a single predecessor of the inner cond_bb. */
> - if (single_pred_p (inner_cond_bb))
> + if (single_pred_p (inner_cond_bb)
> + && bb_no_side_effects_p (inner_cond_bb))
> {
> basic_block outer_cond_bb = single_pred (inner_cond_bb);
>
>
>
> --
> Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/ FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-02 9:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-30 13:57 [PR68083] stop ifcombine from moving uninitialized uses before their guards Alexandre Oliva
2015-10-30 18:03 ` Alexandre Oliva
2015-11-02 9:32 ` Richard Biener
2015-11-02 9:32 ` 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).