* [patch] Fix inconsistency in invert_tree_comparison
@ 2011-10-23 10:03 Eric Botcazou
2011-10-23 10:05 ` Richard Guenther
0 siblings, 1 reply; 3+ messages in thread
From: Eric Botcazou @ 2011-10-23 10:03 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1169 bytes --]
Hi,
the comment of the function reads:
/* Given a tree comparison code, return the code that is the logical inverse
of the given code. It is not safe to do this for floating-point
comparisons, except for NE_EXPR and EQ_EXPR, so we receive a machine mode
as well: if reversing the comparison is unsafe, return ERROR_MARK. */
but the function starts with:
if (honor_nans && flag_trapping_math)
return ERROR_MARK;
so, for example, it refuses to fold !(x == y) to x != y for FP, which is valid.
Fixed by letting EQ_EXPR and NE_EXPR go through. This makes tree-opt/44683
regress though, but it's clear that the original fix only papered over the
problem, as you can't infer a simple equivalence from a condition when you can
have signed zeros around; so the patch also includes the proper fix.
Tested on x86_64-suse-linux, OK for mainline?
2011-10-23 Eric Botcazou <ebotcazou@adacore.com>
* fold-const.c (invert_tree_comparison): Always invert EQ_EXPR/NE_EXPR.
PR tree-optimization/44683
* tree-ssa-dom.c (record_edge_info): Record simple equivalences only if
we can be sure that there are no signed zeros involved.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 3900 bytes --]
Index: fold-const.c
===================================================================
--- fold-const.c (revision 180235)
+++ fold-const.c (working copy)
@@ -2100,15 +2100,14 @@ pedantic_non_lvalue_loc (location_t loc,
return protected_set_expr_location_unshare (x, loc);
}
\f
-/* Given a tree comparison code, return the code that is the logical inverse
- of the given code. It is not safe to do this for floating-point
- comparisons, except for NE_EXPR and EQ_EXPR, so we receive a machine mode
- as well: if reversing the comparison is unsafe, return ERROR_MARK. */
+/* Given a tree comparison code, return the code that is the logical inverse.
+ It is generally not safe to do this for floating-point comparisons, except
+ for EQ_EXPR and NE_EXPR, so we return ERROR_MARK in this case. */
enum tree_code
invert_tree_comparison (enum tree_code code, bool honor_nans)
{
- if (honor_nans && flag_trapping_math)
+ if (honor_nans && flag_trapping_math && code != EQ_EXPR && code != NE_EXPR)
return ERROR_MARK;
switch (code)
Index: tree-ssa-dom.c
===================================================================
--- tree-ssa-dom.c (revision 180235)
+++ tree-ssa-dom.c (working copy)
@@ -1610,12 +1610,15 @@ record_edge_info (basic_block bb)
{
tree cond = build2 (code, boolean_type_node, op0, op1);
tree inverted = invert_truthvalue_loc (loc, cond);
+ enum machine_mode mode = TYPE_MODE (TREE_TYPE (op0));
+ bool can_infer_simple_equiv
+ = !(HONOR_SIGNED_ZEROS (mode) && real_zerop (op0));
struct edge_info *edge_info;
edge_info = allocate_edge_info (true_edge);
record_conditions (edge_info, cond, inverted);
- if (code == EQ_EXPR)
+ if (can_infer_simple_equiv && code == EQ_EXPR)
{
edge_info->lhs = op1;
edge_info->rhs = op0;
@@ -1624,7 +1627,7 @@ record_edge_info (basic_block bb)
edge_info = allocate_edge_info (false_edge);
record_conditions (edge_info, inverted, cond);
- if (TREE_CODE (inverted) == EQ_EXPR)
+ if (can_infer_simple_equiv && TREE_CODE (inverted) == EQ_EXPR)
{
edge_info->lhs = op1;
edge_info->rhs = op0;
@@ -1632,17 +1635,21 @@ record_edge_info (basic_block bb)
}
else if (TREE_CODE (op0) == SSA_NAME
- && (is_gimple_min_invariant (op1)
- || TREE_CODE (op1) == SSA_NAME))
+ && (TREE_CODE (op1) == SSA_NAME
+ || is_gimple_min_invariant (op1)))
{
tree cond = build2 (code, boolean_type_node, op0, op1);
tree inverted = invert_truthvalue_loc (loc, cond);
+ enum machine_mode mode = TYPE_MODE (TREE_TYPE (op1));
+ bool can_infer_simple_equiv
+ = !(HONOR_SIGNED_ZEROS (mode)
+ && (TREE_CODE (op1) == SSA_NAME || real_zerop (op1)));
struct edge_info *edge_info;
edge_info = allocate_edge_info (true_edge);
record_conditions (edge_info, cond, inverted);
- if (code == EQ_EXPR)
+ if (can_infer_simple_equiv && code == EQ_EXPR)
{
edge_info->lhs = op0;
edge_info->rhs = op1;
@@ -1651,7 +1658,7 @@ record_edge_info (basic_block bb)
edge_info = allocate_edge_info (false_edge);
record_conditions (edge_info, inverted, cond);
- if (TREE_CODE (inverted) == EQ_EXPR)
+ if (can_infer_simple_equiv && TREE_CODE (inverted) == EQ_EXPR)
{
edge_info->lhs = op0;
edge_info->rhs = op1;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] Fix inconsistency in invert_tree_comparison
2011-10-23 10:03 [patch] Fix inconsistency in invert_tree_comparison Eric Botcazou
@ 2011-10-23 10:05 ` Richard Guenther
2011-10-23 12:16 ` Eric Botcazou
0 siblings, 1 reply; 3+ messages in thread
From: Richard Guenther @ 2011-10-23 10:05 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches
On Sun, Oct 23, 2011 at 10:56 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> the comment of the function reads:
>
> /* Given a tree comparison code, return the code that is the logical inverse
> of the given code. It is not safe to do this for floating-point
> comparisons, except for NE_EXPR and EQ_EXPR, so we receive a machine mode
> as well: if reversing the comparison is unsafe, return ERROR_MARK. */
>
> but the function starts with:
>
> if (honor_nans && flag_trapping_math)
> return ERROR_MARK;
Do you have an idea why we test flag_trapping_math here?
> so, for example, it refuses to fold !(x == y) to x != y for FP, which is valid.
>
> Fixed by letting EQ_EXPR and NE_EXPR go through. This makes tree-opt/44683
> regress though, but it's clear that the original fix only papered over the
> problem, as you can't infer a simple equivalence from a condition when you can
> have signed zeros around; so the patch also includes the proper fix.
>
> Tested on x86_64-suse-linux, OK for mainline?
Ok.
Thanks,
Richard.
>
> 2011-10-23 Eric Botcazou <ebotcazou@adacore.com>
>
> * fold-const.c (invert_tree_comparison): Always invert EQ_EXPR/NE_EXPR.
>
> PR tree-optimization/44683
> * tree-ssa-dom.c (record_edge_info): Record simple equivalences only if
> we can be sure that there are no signed zeros involved.
>
>
> --
> Eric Botcazou
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] Fix inconsistency in invert_tree_comparison
2011-10-23 10:05 ` Richard Guenther
@ 2011-10-23 12:16 ` Eric Botcazou
0 siblings, 0 replies; 3+ messages in thread
From: Eric Botcazou @ 2011-10-23 12:16 UTC (permalink / raw)
To: Richard Guenther; +Cc: gcc-patches
> Do you have an idea why we test flag_trapping_math here?
Not really, the test was added with the contradictory comment:
http://gcc.gnu.org/ml/gcc-patches/2004-05/msg01674.html
--
Eric Botcazou
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-10-23 10:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-23 10:03 [patch] Fix inconsistency in invert_tree_comparison Eric Botcazou
2011-10-23 10:05 ` Richard Guenther
2011-10-23 12:16 ` Eric Botcazou
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).