[PATCH GCC]Simplify (convert)(X op const) -> (convert)X op (convert)const by match&simplify

Richard Biener richard.guenther@gmail.com
Wed Oct 12 08:48:00 GMT 2016


On Tue, Oct 11, 2016 at 11:34 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Tue, 11 Oct 2016, Bin Cheng wrote:
>
>> We missed folding (convert)(X op const) -> (convert)X op (convert)const
>> for unsigned narrowing because of reason reported at
>> https://gcc.gnu.org/ml/gcc/2016-07/msg00126.html
>> This patch fixes the issue by adding new match&simplify pattern, it also
>> adds a test case.  This is the prerequisite patch for next patch adding new
>> vectorization pattern.
>
>
> Some technical comments below. I am sure Jeff and/or Richi will have more to
> say on the approach. I am a bit surprised to see it as adding a new
> transformation, instead of moving an old one.

The "old one" would be c-family/c-common.c:shorten_binary_op.  It's generally
prefered to move stuff, preserving semantics.

We do have a similar transform for MULT_EXPR in fold-const.c which also
applies to non-constant 2nd operand (likewise for shorten_binary_op).
The general issue with these "narrowings" is that it loses overflow information
if the original op was carried out in a TYPE_OVERFLOW_UNDEFINED type.

There is also already a bunch of similar match.pd patterns here:

/* Narrowing of arithmetic and logical operations.

   These are conceptually similar to the transformations performed for
   the C/C++ front-ends by shorten_binary_op and shorten_compare.  Long
   term we want to move all that code out of the front-ends into here.  */

/* If we have a narrowing conversion of an arithmetic operation where
   both operands are widening conversions from the same type as the outer
   narrowing conversion.  Then convert the innermost operands to a suitable
   unsigned type (to avoid introducing undefined behavior), perform the
   operation and convert the result to the desired type.  */
(for op (plus minus)
  (simplify
    (convert (op:s (convert@2 @0) (convert@3 @1)))
...

so it might be possible to amend these or at least move your pattern next
to those.

> +/* (convert (X op C)) -> ((convert)X op (convert)C) if it is narrowing
> +   conversion and both types wrap when overflow.  */
> +(for op (plus minus)
> +  (simplify
>
> We used to indent by a single space in this file, but I see that other
> transforms have made it in with double spacing, so I guess it doesn't
> matter.
>
> +    (convert (op @0 @1))
> +    (if (INTEGRAL_TYPE_P (type)
> +        && TYPE_OVERFLOW_WRAPS (type)
> +        && TREE_CODE (@1) == INTEGER_CST
>
> You can write (convert (op @0 INTEGER_CST@1)) and skip this line.
>
> +        && INTEGRAL_TYPE_P (TREE_TYPE (@0))
> +        && TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))
>
> This seems quite restrictive, TYPE_OVERFLOW_UNDEFINED should also be fine
> for this type. I guess you are trying to avoid saturating / trapping types?

Maybe he's avoiding the dropping of overflow info ... at least it
warrants a comment.

Richard.

> +        && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (@0)))
> +     (op (convert @0) (convert @1)))))
> +
>  /* If we have a narrowing conversion to an integral type that is fed by a
>     BIT_AND_EXPR, we might be able to remove the BIT_AND_EXPR if it merely
>     masks off bits outside the final type (and nothing else).  */
> diff --git a/gcc/testsuite/gcc.dg/fold-narrow-unsgn-opcst.c
> b/gcc/testsuite/gcc.dg/fold-narrow-unsgn-opcst.c
> new file mode 100644
> index 0000000..aff96a9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/fold-narrow-unsgn-opcst.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-gimple" } */
> +
> +unsigned char foo (unsigned short s)
> +{
> +  return (unsigned char)(s + 65530);
> +}
> +/* { dg-final { scan-tree-dump-not " 65530" "gimple" } } */
>
> As I understand it, C says that s is promoted to int and added to 65530, but
> int is not TYPE_OVERFLOW_WRAPS so your transformation doesn't apply (the
> test already passes without your patch). It is better to write tests for the
> gimple version of transformations, i.e. don't write everything as a single
> expression, use intermediate variables.
>
> --
> Marc Glisse



More information about the Gcc-patches mailing list