diff mbox

X /[ex] 4 < Y /[ex] 4

Message ID alpine.DEB.2.02.1704240822550.6994@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse April 24, 2017, 6:24 a.m. UTC
Hello,

we were missing this simplification on comparisons. Note that the testcase
still does not simplify as much as one might like, we don't turn x+z<y+z
into x<y. I'll try to add that next, but the interaction with
Wstrict-overflow may be painful... The goal is that comparing size and
capacity of a std::vector should turn into a simple comparison of the end
pointers.

I am only handling positive constants, because that's all we currently
have with exact division, we can revisit this place and many others when
that changes.

As usual, this might increase register pressure a bit in some cases, I
could ask that at least one of the quotients be single_use if you believe
that's better.

Bootstrap+regtest on powerpc64le-unknown-linux-gnu.

2017-04-24  Marc Glisse  <marc.glisse@inria.fr>

gcc/
 	* match.pd (X/[ex]C CMP Y/[ex]C): New transformation.

gcc/testsuite/
 	* gcc.dg/tree-ssa/cmpexactdiv-2.c: New file.

Comments

Jakub Jelinek April 24, 2017, 6:42 a.m. UTC | #1
On Mon, Apr 24, 2017 at 08:24:48AM +0200, Marc Glisse wrote:
> --- gcc/match.pd	(revision 247083)
> +++ gcc/match.pd	(working copy)
> @@ -1028,20 +1028,27 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>   (simplify
>    (cmp (mult:c @0 @1) (mult:c @2 @1))
>    (if (INTEGRAL_TYPE_P (TREE_TYPE (@1))
>         && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
>     (if (tree_expr_nonnegative_p (@1) && tree_expr_nonzero_p (@1))
>      (cmp @0 @2)
>     (if (TREE_CODE (@1) == INTEGER_CST
>  	&& wi::neg_p (@1, TYPE_SIGN (TREE_TYPE (@1))))
>      (cmp @2 @0))))))
>  
> +/* X / 4 < Y / 4 iif X < Y when the division is known to be exact.  */

s/iif/iff/ ?

	Jakub
Marc Glisse April 24, 2017, 7:04 a.m. UTC | #2
On Mon, 24 Apr 2017, Jakub Jelinek wrote:

>> +/* X / 4 < Y / 4 iif X < Y when the division is known to be exact.  */
>
> s/iif/iff/ ?

Indeed, thanks, I've fixed it locally.
Richard Biener April 24, 2017, 7:45 a.m. UTC | #3
On Mon, Apr 24, 2017 at 9:04 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Mon, 24 Apr 2017, Jakub Jelinek wrote:
>
>>> +/* X / 4 < Y / 4 iif X < Y when the division is known to be exact.  */
>>
>>
>> s/iif/iff/ ?
>
>
> Indeed, thanks, I've fixed it locally.

Ok.

As of strict-overflow warnings I'd like to kill -f[no-]strict-overflow
by aliasing
it to -f[no-]wrapv and thus removing the !TYPE_OVERFLOW_WRAPS
&& !TYPE_OVERFLOW_UNDEFINED && !TYPE_OVERFLOW_TRAPS case.

This means effectively killing -Wstrict-overflow which is IMHO reasonable
as it's quite broken plus we now have ubsan which does a better job.

Thanks,
Richard.

> --
> Marc Glisse
Jeff Law April 24, 2017, 3:48 p.m. UTC | #4
On 04/24/2017 12:24 AM, Marc Glisse wrote:
> Hello,
> 
> we were missing this simplification on comparisons. Note that the testcase
> still does not simplify as much as one might like, we don't turn x+z<y+z
> into x<y. I'll try to add that next, but the interaction with
> Wstrict-overflow may be painful... The goal is that comparing size and
> capacity of a std::vector should turn into a simple comparison of the end
> pointers.
I'm all for improving our ability to analyze and generate good code for 
std::vector.  So any steps you take in this space are greatly appreciated.

Martin Sebor was considering looking at a variety of issues affecting 
our ability to do a good job with std::vector.  You might want to 
coordinate with him to make sure y'all don't duplicate work.

jeff
Marc Glisse April 25, 2017, 7:30 a.m. UTC | #5
On Mon, 24 Apr 2017, Jeff Law wrote:

> Martin Sebor was considering looking at a variety of issues affecting our 
> ability to do a good job with std::vector.  You might want to coordinate with 
> him to make sure y'all don't duplicate work.

I don't really have any plans for std::vector that might overlap with 
someone else's. I just have a couple more transformations of the type (X 
OP Y) CMP (X OP Z), but that will not affect std::vector.

My first guess for improving std::vector would be around the lack of magic 
in new/delete, compared to malloc/free.
diff mbox

Patch

Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 247083)
+++ gcc/match.pd	(working copy)
@@ -1028,20 +1028,27 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (simplify
   (cmp (mult:c @0 @1) (mult:c @2 @1))
   (if (INTEGRAL_TYPE_P (TREE_TYPE (@1))
        && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
    (if (tree_expr_nonnegative_p (@1) && tree_expr_nonzero_p (@1))
     (cmp @0 @2)
    (if (TREE_CODE (@1) == INTEGER_CST
 	&& wi::neg_p (@1, TYPE_SIGN (TREE_TYPE (@1))))
     (cmp @2 @0))))))
 
+/* X / 4 < Y / 4 iif X < Y when the division is known to be exact.  */
+(for cmp (simple_comparison)
+ (simplify
+  (cmp (exact_div @0 INTEGER_CST@2) (exact_div @1 @2))
+  (if (wi::gt_p(@2, 0, TYPE_SIGN (TREE_TYPE (@2))))
+   (cmp @0 @1))))
+
 /* ((X inner_op C0) outer_op C1)
    With X being a tree where value_range has reasoned certain bits to always be
    zero throughout its computed value range,
    inner_op = {|,^}, outer_op = {|,^} and inner_op != outer_op
    where zero_mask has 1's for all bits that are sure to be 0 in
    and 0's otherwise.
    if (inner_op == '^') C0 &= ~C1;
    if ((C0 & ~zero_mask) == 0) then emit (X outer_op (C0 outer_op C1)
    if ((C1 & ~zero_mask) == 0) then emit (X inner_op (C0 outer_op C1)
 */
Index: gcc/testsuite/gcc.dg/tree-ssa/cmpexactdiv-2.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/cmpexactdiv-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/cmpexactdiv-2.c	(working copy)
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized-raw" } */
+
+int f (long *a, long *b, long *c) {
+    __PTRDIFF_TYPE__ l1 = b - a;
+    __PTRDIFF_TYPE__ l2 = c - a;
+    return l1 < l2;
+}
+
+/* Eventually we also want to remove all minus_expr.  */
+/* { dg-final { scan-tree-dump-not "exact_div_expr" "optimized" } } */