* [PATCH GCC]Simplify (convert)(X op const) -> (convert)X op (convert)const by match&simplify
@ 2016-10-11 15:03 Bin Cheng
2016-10-11 21:34 ` Marc Glisse
0 siblings, 1 reply; 5+ messages in thread
From: Bin Cheng @ 2016-10-11 15:03 UTC (permalink / raw)
To: gcc-patches; +Cc: nd
[-- Attachment #1: Type: text/plain, Size: 711 bytes --]
Hi,
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.
Bootstrap and test along with the next patch on x86_64 and AArch64. Is it OK?
Thanks,
bin
2016-10-11 Bin Cheng <bin.cheng@arm.com>
* match.pd ((convert (X op C))): Fold type narrowing for unsigned
operation on constant.
gcc/testsuite/ChangeLog
2016-10-11 Bin Cheng <bin.cheng@arm.com>
* gcc.dg/fold-narrow-unsgn-opcst.c: New test.
[-- Attachment #2: simplify-narrow-unsigned-opcst-20160922.txt --]
[-- Type: text/plain, Size: 1380 bytes --]
diff --git a/gcc/match.pd b/gcc/match.pd
index 786cf4c..7875424 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1731,6 +1731,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>= inside_prec - !inside_unsignedp)
(convert @0)))))))
+/* (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
+ (convert (op @0 @1))
+ (if (INTEGRAL_TYPE_P (type)
+ && TYPE_OVERFLOW_WRAPS (type)
+ && TREE_CODE (@1) == INTEGER_CST
+ && INTEGRAL_TYPE_P (TREE_TYPE (@0))
+ && TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))
+ && 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" } } */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH GCC]Simplify (convert)(X op const) -> (convert)X op (convert)const by match&simplify
2016-10-11 15:03 [PATCH GCC]Simplify (convert)(X op const) -> (convert)X op (convert)const by match&simplify Bin Cheng
@ 2016-10-11 21:34 ` Marc Glisse
2016-10-12 8:48 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Marc Glisse @ 2016-10-11 21:34 UTC (permalink / raw)
To: Bin Cheng; +Cc: gcc-patches, nd
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.
+/* (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?
+ && 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH GCC]Simplify (convert)(X op const) -> (convert)X op (convert)const by match&simplify
2016-10-11 21:34 ` Marc Glisse
@ 2016-10-12 8:48 ` Richard Biener
2016-10-13 15:22 ` Jeff Law
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2016-10-12 8:48 UTC (permalink / raw)
To: GCC Patches; +Cc: Bin Cheng, nd
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH GCC]Simplify (convert)(X op const) -> (convert)X op (convert)const by match&simplify
2016-10-12 8:48 ` Richard Biener
@ 2016-10-13 15:22 ` Jeff Law
2016-10-14 9:34 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Law @ 2016-10-13 15:22 UTC (permalink / raw)
To: Richard Biener, GCC Patches; +Cc: Bin Cheng, nd
On 10/12/2016 02:48 AM, Richard Biener wrote:
> 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.
Right. Kai and I hadn't looked much at shorten_binary_op (focusing
more on shorten_compare). But the same principles apply to both.
Namely that the existing routines should be twiddled to handle warnings
only, but not modify the underlying IL. IL modifications
(canonicalization and optimization) should be moved into the match.pd
framework.
When Kai left Red Hat, that work stalled. I've got bits and pieces of
that work lying around, but I don't think they'd help Bin's work right now.
>
> There is also already a bunch of similar match.pd patterns here:
[ ... ]
Right. Those were a first start at handling some of the desired
narrowing, focused primarily on BZs that required narrowing to resolve.
Like Kai's work, I have some generalizations and improvements in a
half-completed state here, but haven't had time to work on them.
Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH GCC]Simplify (convert)(X op const) -> (convert)X op (convert)const by match&simplify
2016-10-13 15:22 ` Jeff Law
@ 2016-10-14 9:34 ` Richard Biener
0 siblings, 0 replies; 5+ messages in thread
From: Richard Biener @ 2016-10-14 9:34 UTC (permalink / raw)
To: Jeff Law; +Cc: GCC Patches, Bin Cheng, nd
On Thu, Oct 13, 2016 at 5:22 PM, Jeff Law <law@redhat.com> wrote:
> On 10/12/2016 02:48 AM, Richard Biener wrote:
>>
>> 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.
>
> Right. Kai and I hadn't looked much at shorten_binary_op (focusing more on
> shorten_compare). But the same principles apply to both.
>
> Namely that the existing routines should be twiddled to handle warnings
> only, but not modify the underlying IL. IL modifications (canonicalization
> and optimization) should be moved into the match.pd framework.
fortunately shorten_binary_op itself does not emit any warnings!
> When Kai left Red Hat, that work stalled. I've got bits and pieces of that
> work lying around, but I don't think they'd help Bin's work right now.
>
>>
>> There is also already a bunch of similar match.pd patterns here:
>
> [ ... ]
> Right. Those were a first start at handling some of the desired narrowing,
> focused primarily on BZs that required narrowing to resolve. Like Kai's
> work, I have some generalizations and improvements in a half-completed state
> here, but haven't had time to work on them.
>
>
> Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-14 9:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-11 15:03 [PATCH GCC]Simplify (convert)(X op const) -> (convert)X op (convert)const by match&simplify Bin Cheng
2016-10-11 21:34 ` Marc Glisse
2016-10-12 8:48 ` Richard Biener
2016-10-13 15:22 ` Jeff Law
2016-10-14 9:34 ` Richard Biener
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).