* [PATCH] Fix PR56365
@ 2016-03-14 11:58 Richard Biener
2016-03-14 18:26 ` Bernhard Reutner-Fischer
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2016-03-14 11:58 UTC (permalink / raw)
To: gcc-patches
I am testing the following patch to fix the regression in min/max
detection introduced by comparison canonicalization like a < 267
to a <= 266. The patch allows us to identify all four min/max
cases in the testcase below.
Bootstrap and regtest running on x86_64-unknown-linux-gnu.
Richard.
2016-03-14 Richard Biener <rguenther@suse.de>
PR tree-optimization/56365
* tree-ssa-phiopt.c (minmax_replacement): Handle alternate
constants to compare against.
* gcc.dg/tree-ssa/phi-opt-14.c: New testcase.
Index: gcc/tree-ssa-phiopt.c
===================================================================
*** gcc/tree-ssa-phiopt.c (revision 234180)
--- gcc/tree-ssa-phiopt.c (working copy)
*************** minmax_replacement (basic_block cond_bb,
*** 1045,1051 ****
gassign *new_stmt;
edge true_edge, false_edge;
enum tree_code cmp, minmax, ass_code;
! tree smaller, larger, arg_true, arg_false;
gimple_stmt_iterator gsi, gsi_from;
type = TREE_TYPE (PHI_RESULT (phi));
--- 1045,1051 ----
gassign *new_stmt;
edge true_edge, false_edge;
enum tree_code cmp, minmax, ass_code;
! tree smaller, alt_smaller, larger, alt_larger, arg_true, arg_false;
gimple_stmt_iterator gsi, gsi_from;
type = TREE_TYPE (PHI_RESULT (phi));
*************** minmax_replacement (basic_block cond_bb,
*** 1059,1073 ****
--- 1059,1117 ----
/* This transformation is only valid for order comparisons. Record which
operand is smaller/larger if the result of the comparison is true. */
+ alt_smaller = NULL_TREE;
+ alt_larger = NULL_TREE;
if (cmp == LT_EXPR || cmp == LE_EXPR)
{
smaller = gimple_cond_lhs (cond);
larger = gimple_cond_rhs (cond);
+ /* If we have smaller < CST it is equivalent to smaller <= CST-1.
+ Likewise smaller <= CST is equivalent to smaller < CST+1. */
+ if (TREE_CODE (larger) == INTEGER_CST)
+ {
+ if (cmp == LT_EXPR)
+ {
+ bool overflow;
+ wide_int alt = wi::sub (larger, 1, TYPE_SIGN (TREE_TYPE (larger)),
+ &overflow);
+ if (! overflow)
+ alt_larger = wide_int_to_tree (TREE_TYPE (larger), alt);
+ }
+ else
+ {
+ bool overflow;
+ wide_int alt = wi::add (larger, 1, TYPE_SIGN (TREE_TYPE (larger)),
+ &overflow);
+ if (! overflow)
+ alt_larger = wide_int_to_tree (TREE_TYPE (larger), alt);
+ }
+ }
}
else if (cmp == GT_EXPR || cmp == GE_EXPR)
{
smaller = gimple_cond_rhs (cond);
larger = gimple_cond_lhs (cond);
+ /* If we have larger > CST it is equivalent to larger >= CST+1.
+ Likewise larger >= CST is equivalent to larger > CST-1. */
+ if (TREE_CODE (smaller) == INTEGER_CST)
+ {
+ if (cmp == GT_EXPR)
+ {
+ bool overflow;
+ wide_int alt = wi::add (smaller, 1, TYPE_SIGN (TREE_TYPE (smaller)),
+ &overflow);
+ if (! overflow)
+ alt_smaller = wide_int_to_tree (TREE_TYPE (smaller), alt);
+ }
+ else
+ {
+ bool overflow;
+ wide_int alt = wi::sub (smaller, 1, TYPE_SIGN (TREE_TYPE (smaller)),
+ &overflow);
+ if (! overflow)
+ alt_smaller = wide_int_to_tree (TREE_TYPE (smaller), alt);
+ }
+ }
}
else
return false;
*************** minmax_replacement (basic_block cond_bb,
*** 1098,1105 ****
if (empty_block_p (middle_bb))
{
! if (operand_equal_for_phi_arg_p (arg_true, smaller)
! && operand_equal_for_phi_arg_p (arg_false, larger))
{
/* Case
--- 1142,1153 ----
if (empty_block_p (middle_bb))
{
! if ((operand_equal_for_phi_arg_p (arg_true, smaller)
! || (alt_smaller
! && operand_equal_for_phi_arg_p (arg_true, alt_smaller)))
! && (operand_equal_for_phi_arg_p (arg_false, larger)
! || (alt_larger
! && operand_equal_for_phi_arg_p (arg_true, alt_larger))))
{
/* Case
*************** minmax_replacement (basic_block cond_bb,
*** 1109,1116 ****
rslt = larger; */
minmax = MIN_EXPR;
}
! else if (operand_equal_for_phi_arg_p (arg_false, smaller)
! && operand_equal_for_phi_arg_p (arg_true, larger))
minmax = MAX_EXPR;
else
return false;
--- 1157,1168 ----
rslt = larger; */
minmax = MIN_EXPR;
}
! else if ((operand_equal_for_phi_arg_p (arg_false, smaller)
! || (alt_smaller
! && operand_equal_for_phi_arg_p (arg_false, alt_smaller)))
! && (operand_equal_for_phi_arg_p (arg_true, larger)
! || (alt_larger
! && operand_equal_for_phi_arg_p (arg_true, alt_larger))))
minmax = MAX_EXPR;
else
return false;
*************** minmax_replacement (basic_block cond_bb,
*** 1148,1154 ****
if (!operand_equal_for_phi_arg_p (lhs, arg_true))
return false;
! if (operand_equal_for_phi_arg_p (arg_false, larger))
{
/* Case
--- 1200,1208 ----
if (!operand_equal_for_phi_arg_p (lhs, arg_true))
return false;
! if (operand_equal_for_phi_arg_p (arg_false, larger)
! || (alt_larger
! && operand_equal_for_phi_arg_p (arg_false, alt_larger)))
{
/* Case
*************** minmax_replacement (basic_block cond_bb,
*** 1161,1169 ****
return false;
minmax = MIN_EXPR;
! if (operand_equal_for_phi_arg_p (op0, smaller))
bound = op1;
! else if (operand_equal_for_phi_arg_p (op1, smaller))
bound = op0;
else
return false;
--- 1215,1227 ----
return false;
minmax = MIN_EXPR;
! if (operand_equal_for_phi_arg_p (op0, smaller)
! || (alt_smaller
! && operand_equal_for_phi_arg_p (op0, alt_smaller)))
bound = op1;
! else if (operand_equal_for_phi_arg_p (op1, smaller)
! || (alt_smaller
! && operand_equal_for_phi_arg_p (op1, alt_smaller)))
bound = op0;
else
return false;
*************** minmax_replacement (basic_block cond_bb,
*** 1173,1179 ****
bound, larger)))
return false;
}
! else if (operand_equal_for_phi_arg_p (arg_false, smaller))
{
/* Case
--- 1231,1239 ----
bound, larger)))
return false;
}
! else if (operand_equal_for_phi_arg_p (arg_false, smaller)
! || (alt_smaller
! && operand_equal_for_phi_arg_p (arg_false, alt_smaller)))
{
/* Case
*************** minmax_replacement (basic_block cond_bb,
*** 1186,1194 ****
return false;
minmax = MAX_EXPR;
! if (operand_equal_for_phi_arg_p (op0, larger))
bound = op1;
! else if (operand_equal_for_phi_arg_p (op1, larger))
bound = op0;
else
return false;
--- 1246,1258 ----
return false;
minmax = MAX_EXPR;
! if (operand_equal_for_phi_arg_p (op0, larger)
! || (alt_larger
! && operand_equal_for_phi_arg_p (op0, alt_larger)))
bound = op1;
! else if (operand_equal_for_phi_arg_p (op1, larger)
! || (alt_larger
! && operand_equal_for_phi_arg_p (op1, alt_larger)))
bound = op0;
else
return false;
*************** minmax_replacement (basic_block cond_bb,
*** 1207,1213 ****
if (!operand_equal_for_phi_arg_p (lhs, arg_false))
return false;
! if (operand_equal_for_phi_arg_p (arg_true, larger))
{
/* Case
--- 1271,1279 ----
if (!operand_equal_for_phi_arg_p (lhs, arg_false))
return false;
! if (operand_equal_for_phi_arg_p (arg_true, larger)
! || (alt_larger
! && operand_equal_for_phi_arg_p (arg_true, alt_larger)))
{
/* Case
*************** minmax_replacement (basic_block cond_bb,
*** 1220,1228 ****
return false;
minmax = MAX_EXPR;
! if (operand_equal_for_phi_arg_p (op0, smaller))
bound = op1;
! else if (operand_equal_for_phi_arg_p (op1, smaller))
bound = op0;
else
return false;
--- 1286,1298 ----
return false;
minmax = MAX_EXPR;
! if (operand_equal_for_phi_arg_p (op0, smaller)
! || (alt_smaller
! && operand_equal_for_phi_arg_p (op0, alt_smaller)))
bound = op1;
! else if (operand_equal_for_phi_arg_p (op1, smaller)
! || (alt_smaller
! && operand_equal_for_phi_arg_p (op1, alt_smaller)))
bound = op0;
else
return false;
*************** minmax_replacement (basic_block cond_bb,
*** 1232,1238 ****
bound, larger)))
return false;
}
! else if (operand_equal_for_phi_arg_p (arg_true, smaller))
{
/* Case
--- 1302,1310 ----
bound, larger)))
return false;
}
! else if (operand_equal_for_phi_arg_p (arg_true, smaller)
! || (alt_smaller
! && operand_equal_for_phi_arg_p (arg_true, alt_smaller)))
{
/* Case
Index: gcc/testsuite/gcc.dg/tree-ssa/phi-opt-14.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/phi-opt-14.c (revision 0)
--- gcc/testsuite/gcc.dg/tree-ssa/phi-opt-14.c (working copy)
***************
*** 0 ****
--- 1,37 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O -fdump-tree-phiopt1" } */
+
+ int test_01 (int a)
+ {
+ if (127 <= a)
+ a = 127;
+ else if (a <= -128)
+ a = -128;
+ return a;
+ }
+ int test_02 (int a)
+ {
+ if (127 < a)
+ a = 127;
+ else if (a <= -128)
+ a = -128;
+ return a;
+ }
+ int test_03 (int a)
+ {
+ if (127 <= a)
+ a = 127;
+ else if (a < -128)
+ a = -128;
+ return a;
+ }
+ int test_04 (int a)
+ {
+ if (127 < a)
+ a = 127;
+ else if (a < -128)
+ a = -128;
+ return a;
+ }
+
+ /* { dg-final { scan-tree-dump-not "if" "phiopt1" } } */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix PR56365
2016-03-14 11:58 [PATCH] Fix PR56365 Richard Biener
@ 2016-03-14 18:26 ` Bernhard Reutner-Fischer
2016-03-14 20:21 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Bernhard Reutner-Fischer @ 2016-03-14 18:26 UTC (permalink / raw)
To: Richard Biener, gcc-patches
On March 14, 2016 12:58:20 PM GMT+01:00, Richard Biener <rguenther@suse.de> wrote:
>
>I am testing the following patch to fix the regression in min/max
>detection introduced by comparison canonicalization like a < 267
>to a <= 266. The patch allows us to identify all four min/max
>cases in the testcase below.
>
>Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>InLikew/testsuite/gcc.dg/tree-ssa/phi-opt-14.c
>===================================================================
>*** gcc/testsuite/gcc.dg/tree-ssa/phi-opt-14.c (revision 0)
>--- gcc/testsuite/gcc.dg/tree-ssa/phi-opt-14.c (working copy)
>***************
>*** 0 ****
>--- 1,37 ----
>+ /* { dg-do compile } */
>+ /* { dg-options "-O -fdump-tree-phiopt1" } */
>+
>+ int test_01 (int a)
>+ {
>+ if (127 <= a)
Shouldn't this be >= ?
>+ a = 127;
>+ else if (a <= -128)
>+ a = -128;
>+ return a;
>+ }
>+ int test_02 (int a)
>+ {
>+ if (127 < a)
and this >
>+ a = 127;
>+ else if (a <= -128)
>+ a = -128;
>+ return a;
>+ }
>+ int test_03 (int a)
>+ {
>+ if (127 <= a)
and this >=
>+ a = 127;
>+ else if (a < -128)
>+ a = -128;
>+ return a;
>+ }
>+ int test_04 (int a)
>+ {
>+ if (127 < a)
and >
TIA,
>+ a = 127;
>+ else if (a < -128)
>+ a = -128;
>+ return a;
>+ }
>+
>+ /* { dg-final { scan-tree-dump-not "if" "phiopt1" } } */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix PR56365
2016-03-14 18:26 ` Bernhard Reutner-Fischer
@ 2016-03-14 20:21 ` Richard Biener
2016-03-14 22:55 ` Bernhard Reutner-Fischer
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2016-03-14 20:21 UTC (permalink / raw)
To: Bernhard Reutner-Fischer, Richard Biener, gcc-patches
On March 14, 2016 7:25:31 PM GMT+01:00, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>On March 14, 2016 12:58:20 PM GMT+01:00, Richard Biener
><rguenther@suse.de> wrote:
>>
>>I am testing the following patch to fix the regression in min/max
>>detection introduced by comparison canonicalization like a < 267
>>to a <= 266. The patch allows us to identify all four min/max
>>cases in the testcase below.
>>
>>Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
>>InLikew/testsuite/gcc.dg/tree-ssa/phi-opt-14.c
>>===================================================================
>>*** gcc/testsuite/gcc.dg/tree-ssa/phi-opt-14.c (revision 0)
>>--- gcc/testsuite/gcc.dg/tree-ssa/phi-opt-14.c (working copy)
>>***************
>>*** 0 ****
>>--- 1,37 ----
>>+ /* { dg-do compile } */
>>+ /* { dg-options "-O -fdump-tree-phiopt1" } */
>>+
>>+ int test_01 (int a)
>>+ {
>>+ if (127 <= a)
>
>Shouldn't this be >= ?
No, note how the constant is left of the <=.
Richard.
>>+ a = 127;
>>+ else if (a <= -128)
>>+ a = -128;
>>+ return a;
>>+ }
>>+ int test_02 (int a)
>>+ {
>>+ if (127 < a)
>
>and this >
>
>>+ a = 127;
>>+ else if (a <= -128)
>>+ a = -128;
>>+ return a;
>>+ }
>>+ int test_03 (int a)
>>+ {
>>+ if (127 <= a)
>
>and this >=
>
>>+ a = 127;
>>+ else if (a < -128)
>>+ a = -128;
>>+ return a;
>>+ }
>>+ int test_04 (int a)
>>+ {
>>+ if (127 < a)
>
>and >
>
>TIA,
>>+ a = 127;
>>+ else if (a < -128)
>>+ a = -128;
>>+ return a;
>>+ }
>>+
>>+ /* { dg-final { scan-tree-dump-not "if" "phiopt1" } } */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix PR56365
2016-03-14 20:21 ` Richard Biener
@ 2016-03-14 22:55 ` Bernhard Reutner-Fischer
0 siblings, 0 replies; 4+ messages in thread
From: Bernhard Reutner-Fischer @ 2016-03-14 22:55 UTC (permalink / raw)
To: Richard Biener, Richard Biener, gcc-patches
On March 14, 2016 9:21:25 PM GMT+01:00, Richard Biener <richard.guenther@gmail.com> wrote:
>On March 14, 2016 7:25:31 PM GMT+01:00, Bernhard Reutner-Fischer
><rep.dot.nop@gmail.com> wrote:
>>On March 14, 2016 12:58:20 PM GMT+01:00, Richard Biener
>><rguenther@suse.de> wrote:
>>>+
>>>+ int test_01 (int a)
>>>+ {
>>>+ if (127 <= a)
>>
>>Shouldn't this be >= ?
>
>No, note how the constant is left of the <=.
>
Right, I managed to misread this.
Sorry for the noise!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-14 22:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-14 11:58 [PATCH] Fix PR56365 Richard Biener
2016-03-14 18:26 ` Bernhard Reutner-Fischer
2016-03-14 20:21 ` Richard Biener
2016-03-14 22:55 ` Bernhard Reutner-Fischer
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).