On Fri, 15 Dec 2017, Jakub Jelinek wrote:
> On Fri, Dec 15, 2017 at 10:28:52AM +0100, Richard Biener wrote:
> > > --- gcc/match.pd.jj 2017-12-07 14:00:51.083048186 +0100
> > > +++ gcc/match.pd 2017-12-07 15:17:49.132784931 +0100
> > > @@ -1784,8 +1784,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > >
> > > /* (T)(P + A) - (T)P -> (T) A */
> > > (simplify
> > > - (minus (convert (plus:c @0 @1))
> > > - (convert @0))
> > > + (minus (convert (plus:c @@0 @1))
> > > + (convert? @0))
> > > (if (element_precision (type) <= element_precision (TREE_TYPE (@1))
> > > /* For integer types, if A has a smaller type
> > > than T the result depends on the possible
> > > @@ -1794,10 +1794,29 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > > However, if an overflow in P + A would cause
> > > undefined behavior, we can assume that there
> > > is no overflow. */
> > > - || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > > - && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
> > > + || (INTEGRAL_TYPE_P (TREE_TYPE (@1))
> > > + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1))))
> >
> > Given @1 and @@0 are in the same plus this change isn't technically
> > necessary but it makes it clearer which type we look at (thus ok).
>
> My understanding is that it is necessary, because @@0 could have different
> type from @0 and TREE_TYPE (@0) is the type of where @0 is used rather
> than @@0.
Using @@0 also guarantees you to get this specific operand when
later refering to it via @0.
> > > (convert @1)))
> > > (simplify
> > > + (plus (convert (plus @1 INTEGER_CST@0)) INTEGER_CST@2)
> > > + (with { bool overflow;
> > > + wide_int w = wi::neg (wi::to_wide (@2), &overflow); }
> > > + (if (wi::to_widest (@0) == widest_int::from (w, TYPE_SIGN (TREE_TYPE
> > > (@2)))
> > > + && (!overflow
> > > + || (INTEGRAL_TYPE_P (TREE_TYPE (@2))
> > > + && TYPE_UNSIGNED (TREE_TYPE (@2))))
> > > + && (element_precision (type) <= element_precision (TREE_TYPE (@1))
> > > + /* For integer types, if A has a smaller type
> > > + than T the result depends on the possible
> > > + overflow in P + A.
> > > + E.g. T=size_t, A=(unsigned)429497295, P>0.
> > > + However, if an overflow in P + A would cause
> > > + undefined behavior, we can assume that there
> > > + is no overflow. */
> > > + || (INTEGRAL_TYPE_P (TREE_TYPE (@1))
> > > + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1)))))
> >
> > I think we don't need to worry about definedness of overflow. All
>
> We are talking about
> (int) (x + 0x80000000U) + INT_MIN
> I think you're right that we can still optimize that to (int) x.
>
> > that matters is whether twos complement arithmetic will simplify
> > the expression to (convert @1). Specifically the possible overflow
> > of the negation of @2 for the case element_precision (type) <=
> > element_precision (TREE_TYPE (@1)) shouldn't matter, likewise
> > for the widening case (we'd never get the equality).
> >
> > Don't we want to compare @0 and -@2 in the type of @2? Like
> > for (unsigned int)(unsigned-long-x + 0x100000005) + -5U which
> > we should be able to simplify? For the widening case that would
> > work as well as far as I can see?
>
> So, we can have several cases, the narrowing one, e.g.:
> (int)(unsigned-long-long-x + 0x100000005ULL) + -5
> (unsigned)(long-long-x + 0x100000005LL) + -5U
> (int)(unsigned-long-long-x + 0x1fffffffbULL) + 5
> (unsigned)(long-long-x + 0x1fffffffbLL) + 5U
> same precision:
> (int)(unsigned-x + 5U) + -5
> (unsigned)(int-x + 5) + -5U
> (int)(unsigned-x + -5U) + 5
> (unsigned)(int-x + -5) + 5
> and widening ones:
> (long long)(int-x + 5) + -5LL
> (unsigned long long)(int-x + 5) + -5ULL
> (long long)(int-x + -5) + 5LL
> (unsigned long long)(int-x + -5) + 5ULL
> You mean we should effectively (though on wide_int/widest_int)
> fold_unary (MINUS_EXPR, TREE_TYPE (@2), fold_convert (TREE_TYPE (@2), @0))
> and compare that to @2?
I think so.
> > If you can split out this new pattern the rest is ok with honoring
> > the comment below.
>
> Ok (will need to comment out the corresponding testcase, done below).
>
> > > - || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > > - && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
> > > + (if (((element_precision (type) <= element_precision (TREE_TYPE
> > > (@1)))
> > > + == (element_precision (type) <= element_precision (TREE_TYPE (@1))))
>
> Yeah, this @1 above should have been @2. Thanks for catching this.
>
> So for now like this?
Yes.
Thanks,
Richard.
> 2017-12-15 Jakub Jelinek <ja...@redhat.com>
>
> PR sanitizer/81281
> * match.pd ((T)(P + A) - (T)P -> (T) A): Use @@0 instead of @0 and
> convert? on @0 instead of convert. Check type of @1, not @0.
> ((T)P - (T)(P + A) -> -(T) A): Use @@0 instead of @0 and
> convert? on @0 instead of convert. Check type of @1, not @0.
> ((T)(P + A) - (T)(P + B) -> (T)A - (T)B): Use @@0 instead of @0,
> only optimize if either both @1 and @2 types are narrower
> precision, or both are wider or equal precision, and in the former
> case only if both have undefined overflow.
>
> * gcc.dg/pr81281-3.c: New test.
>
> --- gcc/match.pd.jj 2017-12-07 18:04:54.580750329 +0100
> +++ gcc/match.pd 2017-12-15 11:52:22.582118364 +0100
> @@ -1784,8 +1784,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>
> /* (T)(P + A) - (T)P -> (T) A */
> (simplify
> - (minus (convert (plus:c @0 @1))
> - (convert @0))
> + (minus (convert (plus:c @@0 @1))
> + (convert? @0))
> (if (element_precision (type) <= element_precision (TREE_TYPE (@1))
> /* For integer types, if A has a smaller type
> than T the result depends on the possible
> @@ -1794,8 +1794,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> However, if an overflow in P + A would cause
> undefined behavior, we can assume that there
> is no overflow. */
> - || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> - && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
> + || (INTEGRAL_TYPE_P (TREE_TYPE (@1))
> + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1))))
> (convert @1)))
> (simplify
> (minus (convert (pointer_plus @@0 @1))
> @@ -1818,8 +1818,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>
> /* (T)P - (T)(P + A) -> -(T) A */
> (simplify
> - (minus (convert @0)
> - (convert (plus:c @0 @1)))
> + (minus (convert? @0)
> + (convert (plus:c @@0 @1)))
> (if (INTEGRAL_TYPE_P (type)
> && TYPE_OVERFLOW_UNDEFINED (type)
> && element_precision (type) <= element_precision (TREE_TYPE (@1)))
> @@ -1833,8 +1833,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> However, if an overflow in P + A would cause
> undefined behavior, we can assume that there
> is no overflow. */
> - || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> - && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
> + || (INTEGRAL_TYPE_P (TREE_TYPE (@1))
> + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1))))
> (negate (convert @1)))))
> (simplify
> (minus (convert @0)
> @@ -1862,23 +1862,28 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>
> /* (T)(P + A) - (T)(P + B) -> (T)A - (T)B */
> (simplify
> - (minus (convert (plus:c @0 @1))
> + (minus (convert (plus:c @@0 @1))
> (convert (plus:c @0 @2)))
> (if (INTEGRAL_TYPE_P (type)
> && TYPE_OVERFLOW_UNDEFINED (type)
> - && element_precision (type) <= element_precision (TREE_TYPE (@1)))
> + && element_precision (type) <= element_precision (TREE_TYPE (@1))
> + && element_precision (type) <= element_precision (TREE_TYPE (@2)))
> (with { tree utype = unsigned_type_for (type); }
> (convert (minus (convert:utype @1) (convert:utype @2))))
> - (if (element_precision (type) <= element_precision (TREE_TYPE (@1))
> - /* For integer types, if A has a smaller type
> - than T the result depends on the possible
> - overflow in P + A.
> - E.g. T=size_t, A=(unsigned)429497295, P>0.
> - However, if an overflow in P + A would cause
> - undefined behavior, we can assume that there
> - is no overflow. */
> - || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> - && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
> + (if (((element_precision (type) <= element_precision (TREE_TYPE (@1)))
> + == (element_precision (type) <= element_precision (TREE_TYPE (@2))))
> + && (element_precision (type) <= element_precision (TREE_TYPE (@1))
> + /* For integer types, if A has a smaller type
> + than T the result depends on the possible
> + overflow in P + A.
> + E.g. T=size_t, A=(unsigned)429497295, P>0.
> + However, if an overflow in P + A would cause
> + undefined behavior, we can assume that there
> + is no overflow. */
> + || (INTEGRAL_TYPE_P (TREE_TYPE (@1))
> + && INTEGRAL_TYPE_P (TREE_TYPE (@2))
> + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1))
> + && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@2)))))
> (minus (convert @1) (convert @2)))))
> (simplify
> (minus (convert (pointer_plus @@0 @1))
> --- gcc/testsuite/gcc.dg/pr81281-3.c.jj 2017-12-15 11:51:38.294654394
> +0100
> +++ gcc/testsuite/gcc.dg/pr81281-3.c 2017-12-15 11:56:08.097388860 +0100
> @@ -0,0 +1,105 @@
> +/* PR sanitizer/81281 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-not "\[+=-] \?123\[ ;]" "optimized" } } */
> +
> +#ifdef __SIZEOF_INT128__
> +__int128
> +f1 (int a, long long b)
> +{
> + __int128 f = 123 + a;
> + __int128 g = 123 + b;
> + return f - g;
> +}
> +#endif
> +
> +signed char
> +f2 (int a, long long b)
> +{
> + signed char f = 123 + a;
> + signed char g = 123 + b;
> + return f - g;
> +}
> +
> +signed char
> +f3 (unsigned int a, unsigned long long b)
> +{
> + signed char f = 123 + a;
> + signed char g = 123 + b;
> + return f - g;
> +}
> +
> +unsigned char
> +f4 (unsigned int a, unsigned long long b)
> +{
> + unsigned char f = 123 + a;
> + unsigned char g = 123 + b;
> + return f - g;
> +}
> +
> +/* This isn't optimized yet. */
> +#if 0
> +long long
> +f5 (int a)
> +{
> + long long f = 123 + a;
> + long long g = 123;
> + return f - g;
> +}
> +#endif
> +
> +signed char
> +f6 (long long a)
> +{
> + signed char f = 123 + a;
> + signed char g = 123;
> + return f - g;
> +}
> +
> +signed char
> +f7 (unsigned int a)
> +{
> + signed char f = 123 + a;
> + signed char g = 123;
> + return f - g;
> +}
> +
> +unsigned char
> +f8 (unsigned long int a)
> +{
> + unsigned char f = 123 + a;
> + unsigned char g = 123;
> + return f - g;
> +}
> +
> +long long
> +f9 (int a)
> +{
> + long long f = 123;
> + long long g = 123 + a;
> + return f - g;
> +}
> +
> +signed char
> +f10 (long long a)
> +{
> + signed char f = 123;
> + signed char g = 123 + a;
> + return f - g;
> +}
> +
> +signed char
> +f11 (unsigned int a)
> +{
> + signed char f = 123;
> + signed char g = 123 + a;
> + return f - g;
> +}
> +
> +unsigned char
> +f12 (unsigned long int a)
> +{
> + unsigned char f = 123;
> + unsigned char g = 123 + a;
> + return f - g;
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
