* [PATCH v2] gimple-match.pd Add more optimization for gimple_cond
@ 2023-11-28 2:55 Feng Wang
2023-11-28 3:06 ` Andrew Pinski
0 siblings, 1 reply; 5+ messages in thread
From: Feng Wang @ 2023-11-28 2:55 UTC (permalink / raw)
To: gcc-patches; +Cc: kito.cheng, jeffreyalaw, pinskia, Feng Wang
The link of PATCH v1: https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg326661.html
This patch add another condition for gimple-cond optimization. Refer to
the following test case.
int foo1 (int data, int res)
{
res = data & 0xf;
res |= res << 4;
if (res < 0x22)
return 0x22;
return res;
}
with the compilation flag "-O2",
before this patch the log info of phiopt2 pass is
<bb 2> [local count: 1073741824]:
res_5 = data_1(D) & 15;
_6 = (unsigned int) res_5;
_7 = _6 * 17;
res_8 = (int) _7;
if (_7 <= 33)
goto <bb 3>; [21.72%]
else
goto <bb 4>; [78.28%]
<bb 3> [local count: 233216728]:
<bb 4> [local count: 1073741824]:
# _9 = PHI <res_8(2), 34(3)>
return _9;
after this patch the the log info of phiopt2 pass is
<bb 2> [local count: 1073741824]:
res_5 = data_1(D) & 15;
_6 = (unsigned int) res_5;
_7 = _6 * 17;
res_8 = (int) _7;
_10 = MAX_EXPR <_7, 34>;
_3 = (int) _10;
return _3;
This patch optimizes the phi node to generate "MAX_EXPR".
The root cause of minmax replacement failure is the type of "_7"
is unsigned, but the type of const_int "34" is signed. It makes
types_match (c2_type, from_type) return false. So I add another
condition to process this scenario.
gcc/ChangeLog:
* match.pd: Add another condition to process type mismatch.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/phi-opt-41.c: New test.
---
gcc/match.pd | 5 ++++-
gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c | 24 ++++++++++++++++++++++
2 files changed, 28 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
diff --git a/gcc/match.pd b/gcc/match.pd
index 95225e4ca5f..e864845bfa9 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -5419,7 +5419,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
&& (types_match (c2_type, from_type)
|| (TYPE_PRECISION (c2_type) > TYPE_PRECISION (from_type)
&& (TYPE_UNSIGNED (from_type)
- || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))))
+ || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))
+ || (TYPE_UNSIGNED (from_type) != TYPE_UNSIGNED (c2_type)
+ && TYPE_PRECISION (c2_type) == TYPE_PRECISION (from_type)
+ && !TYPE_OVERFLOW_WRAPS (c2_type))))
{
if (cmp != EQ_EXPR)
code = minmax_from_comparison (cmp, @1, @3, @1, @2);
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
new file mode 100644
index 00000000000..d1101c2f9f7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-phiopt2" } */
+
+int foo1 (int data, int res)
+{
+ res = data & 0xf;
+ res |= res << 4;
+ if (res < 0x22)
+ return 0x22;
+ return res;
+}
+
+int foo2 (int data, int res)
+{
+ res = data & 0xf;
+ unsigned int r = res;
+ r*=17;
+ res = r;
+ if (r < 0x22)
+ return 0x22;
+ return res;
+}
+
+/* { dg-final { scan-tree-dump-times "MAX_EXPR" 2 "phiopt2" } } */
\ No newline at end of file
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] gimple-match.pd Add more optimization for gimple_cond
2023-11-28 2:55 [PATCH v2] gimple-match.pd Add more optimization for gimple_cond Feng Wang
@ 2023-11-28 3:06 ` Andrew Pinski
2023-11-28 6:04 ` Feng Wang
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Pinski @ 2023-11-28 3:06 UTC (permalink / raw)
To: Feng Wang; +Cc: gcc-patches, kito.cheng, jeffreyalaw
On Mon, Nov 27, 2023 at 6:56 PM Feng Wang <wangfeng@eswincomputing.com> wrote:
>
> The link of PATCH v1: https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg326661.html
> This patch add another condition for gimple-cond optimization. Refer to
> the following test case.
> int foo1 (int data, int res)
> {
> res = data & 0xf;
> res |= res << 4;
> if (res < 0x22)
> return 0x22;
> return res;
> }
> with the compilation flag "-O2",
> before this patch the log info of phiopt2 pass is
> <bb 2> [local count: 1073741824]:
> res_5 = data_1(D) & 15;
> _6 = (unsigned int) res_5;
> _7 = _6 * 17;
> res_8 = (int) _7;
> if (_7 <= 33)
> goto <bb 3>; [21.72%]
> else
> goto <bb 4>; [78.28%]
>
> <bb 3> [local count: 233216728]:
>
> <bb 4> [local count: 1073741824]:
> # _9 = PHI <res_8(2), 34(3)>
> return _9;
> after this patch the the log info of phiopt2 pass is
> <bb 2> [local count: 1073741824]:
> res_5 = data_1(D) & 15;
> _6 = (unsigned int) res_5;
> _7 = _6 * 17;
> res_8 = (int) _7;
> _10 = MAX_EXPR <_7, 34>;
> _3 = (int) _10;
> return _3;
> This patch optimizes the phi node to generate "MAX_EXPR".
> The root cause of minmax replacement failure is the type of "_7"
> is unsigned, but the type of const_int "34" is signed. It makes
> types_match (c2_type, from_type) return false. So I add another
> condition to process this scenario.
>
> gcc/ChangeLog:
>
> * match.pd: Add another condition to process type mismatch.
This should most likely be:
((cond (cmp (convert1? x) c1) (convert2? x) c2) pattern): Also allow
conversions that only change the sign.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/phi-opt-41.c: New test.
> ---
> gcc/match.pd | 5 ++++-
> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c | 24 ++++++++++++++++++++++
> 2 files changed, 28 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 95225e4ca5f..e864845bfa9 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -5419,7 +5419,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> && (types_match (c2_type, from_type)
> || (TYPE_PRECISION (c2_type) > TYPE_PRECISION (from_type)
> && (TYPE_UNSIGNED (from_type)
> - || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))))
> + || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))
> + || (TYPE_UNSIGNED (from_type) != TYPE_UNSIGNED (c2_type)
> + && TYPE_PRECISION (c2_type) == TYPE_PRECISION (from_type)
> + && !TYPE_OVERFLOW_WRAPS (c2_type))))
What is the need for TYPE_OVERFLOW_WRAPS here? Also I think you just
need the check for TYPE_PRECISION instead of the rest.
Maybe instead of types_match here, tree_nop_conversion_p could be used
instead. I am not 100% sure though.
I also suspect you should add a few other testcases that don't depend
on VRP changing things. Maybe a runtime test too.
Thanks,
Andrew
> {
> if (cmp != EQ_EXPR)
> code = minmax_from_comparison (cmp, @1, @3, @1, @2);
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
> new file mode 100644
> index 00000000000..d1101c2f9f7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-phiopt2" } */
> +
> +int foo1 (int data, int res)
> +{
> + res = data & 0xf;
> + res |= res << 4;
> + if (res < 0x22)
> + return 0x22;
> + return res;
> +}
> +
> +int foo2 (int data, int res)
> +{
> + res = data & 0xf;
> + unsigned int r = res;
> + r*=17;
> + res = r;
> + if (r < 0x22)
> + return 0x22;
> + return res;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 2 "phiopt2" } } */
> \ No newline at end of file
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH v2] gimple-match.pd Add more optimization for gimple_cond
2023-11-28 3:06 ` Andrew Pinski
@ 2023-11-28 6:04 ` Feng Wang
2023-11-28 6:08 ` Andrew Pinski
0 siblings, 1 reply; 5+ messages in thread
From: Feng Wang @ 2023-11-28 6:04 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches, kito.cheng, Jeff Law
On 2023-11-28 11:06 Andrew Pinski <pinskia@gmail.com> wrote:
>On Mon, Nov 27, 2023 at 6:56 PM Feng Wang <wangfeng@eswincomputing.com> wrote:
>>
>> The link of PATCH v1: https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg326661.html
>> This patch add another condition for gimple-cond optimization. Refer to
>> the following test case.
>> int foo1 (int data, int res)
>> {
>> res = data & 0xf;
>> res |= res << 4;
>> if (res < 0x22)
>> return 0x22;
>> return res;
>> }
>> with the compilation flag "-O2",
>> before this patch the log info of phiopt2 pass is
>> <bb 2> [local count: 1073741824]:
>> res_5 = data_1(D) & 15;
>> _6 = (unsigned int) res_5;
>> _7 = _6 * 17;
>> res_8 = (int) _7;
>> if (_7 <= 33)
>> goto <bb 3>; [21.72%]
>> else
>> goto <bb 4>; [78.28%]
>>
>> <bb 3> [local count: 233216728]:
>>
>> <bb 4> [local count: 1073741824]:
>> # _9 = PHI <res_8(2), 34(3)>
>> return _9;
>> after this patch the the log info of phiopt2 pass is
>> <bb 2> [local count: 1073741824]:
>> res_5 = data_1(D) & 15;
>> _6 = (unsigned int) res_5;
>> _7 = _6 * 17;
>> res_8 = (int) _7;
>> _10 = MAX_EXPR <_7, 34>;
>> _3 = (int) _10;
>> return _3;
>> This patch optimizes the phi node to generate "MAX_EXPR".
>> The root cause of minmax replacement failure is the type of "_7"
>> is unsigned, but the type of const_int "34" is signed. It makes
>> types_match (c2_type, from_type) return false. So I add another
>> condition to process this scenario.
>>
>> gcc/ChangeLog:
>>
>> * match.pd: Add another condition to process type mismatch.
>
>This should most likely be:
> ((cond (cmp (convert1? x) c1) (convert2? x) c2) pattern): Also allow
>conversions that only change the sign.
>
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.dg/tree-ssa/phi-opt-41.c: New test.
>> ---
>> gcc/match.pd | 5 ++++-
>> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c | 24 ++++++++++++++++++++++
>> 2 files changed, 28 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
>>
>> diff --git a/gcc/match.pd b/gcc/match.pd
>> index 95225e4ca5f..e864845bfa9 100644
>> --- a/gcc/match.pd
>> +++ b/gcc/match.pd
>> @@ -5419,7 +5419,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>> && (types_match (c2_type, from_type)
>> || (TYPE_PRECISION (c2_type) > TYPE_PRECISION (from_type)
>> && (TYPE_UNSIGNED (from_type)
>> - || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))))
>> + || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))
>> + || (TYPE_UNSIGNED (from_type) != TYPE_UNSIGNED (c2_type)
>> + && TYPE_PRECISION (c2_type) == TYPE_PRECISION (from_type)
>> + && !TYPE_OVERFLOW_WRAPS (c2_type))))
>
>What is the need for TYPE_OVERFLOW_WRAPS here? Also I think you just
>need the check for TYPE_PRECISION instead of the rest.
>Maybe instead of types_match here, tree_nop_conversion_p could be used
>instead. I am not 100% sure though.
>
>I also suspect you should add a few other testcases that don't depend
>on VRP changing things. Maybe a runtime test too.
>
>Thanks,
>Andrew
>
I want to make sure the cont_int "c2" won't be overflow,so I use the TYPE_OVERFLOW_WRAPS.
I checked the code , tree_nop_conversion_p judge the TYPE_PRECISION or TYPE_MODE and doesn't
care the unsigned_flag, it should be fine for this scenario, I'm not sure if there's a problem with this
modification, I will run the regression to check whether it causes other issues.
Thanks,
Feng Wang
>> {
>> if (cmp != EQ_EXPR)
>> code = minmax_from_comparison (cmp, @1, @3, @1, @2);
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
>> new file mode 100644
>> index 00000000000..d1101c2f9f7
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
>> @@ -0,0 +1,24 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fdump-tree-phiopt2" } */
>> +
>> +int foo1 (int data, int res)
>> +{
>> + res = data & 0xf;
>> + res |= res << 4;
>> + if (res < 0x22)
>> + return 0x22;
>> + return res;
>> +}
>> +
>> +int foo2 (int data, int res)
>> +{
>> + res = data & 0xf;
>> + unsigned int r = res;
>> + r*=17;
>> + res = r;
>> + if (r < 0x22)
>> + return 0x22;
>> + return res;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 2 "phiopt2" } } */
>> \ No newline at end of file
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH v2] gimple-match.pd Add more optimization for gimple_cond
2023-11-28 6:04 ` Feng Wang
@ 2023-11-28 6:08 ` Andrew Pinski
2023-11-29 12:33 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Pinski @ 2023-11-28 6:08 UTC (permalink / raw)
To: Feng Wang; +Cc: gcc-patches, kito.cheng, Jeff Law
On Mon, Nov 27, 2023 at 10:04 PM Feng Wang <wangfeng@eswincomputing.com> wrote:
>
> On 2023-11-28 11:06 Andrew Pinski <pinskia@gmail.com> wrote:
> >On Mon, Nov 27, 2023 at 6:56 PM Feng Wang <wangfeng@eswincomputing.com> wrote:
> >>
> >> The link of PATCH v1: https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg326661.html
> >> This patch add another condition for gimple-cond optimization. Refer to
> >> the following test case.
> >> int foo1 (int data, int res)
> >> {
> >> res = data & 0xf;
> >> res |= res << 4;
> >> if (res < 0x22)
> >> return 0x22;
> >> return res;
> >> }
> >> with the compilation flag "-O2",
> >> before this patch the log info of phiopt2 pass is
> >> <bb 2> [local count: 1073741824]:
> >> res_5 = data_1(D) & 15;
> >> _6 = (unsigned int) res_5;
> >> _7 = _6 * 17;
> >> res_8 = (int) _7;
> >> if (_7 <= 33)
> >> goto <bb 3>; [21.72%]
> >> else
> >> goto <bb 4>; [78.28%]
> >>
> >> <bb 3> [local count: 233216728]:
> >>
> >> <bb 4> [local count: 1073741824]:
> >> # _9 = PHI <res_8(2), 34(3)>
> >> return _9;
> >> after this patch the the log info of phiopt2 pass is
> >> <bb 2> [local count: 1073741824]:
> >> res_5 = data_1(D) & 15;
> >> _6 = (unsigned int) res_5;
> >> _7 = _6 * 17;
> >> res_8 = (int) _7;
> >> _10 = MAX_EXPR <_7, 34>;
> >> _3 = (int) _10;
> >> return _3;
> >> This patch optimizes the phi node to generate "MAX_EXPR".
> >> The root cause of minmax replacement failure is the type of "_7"
> >> is unsigned, but the type of const_int "34" is signed. It makes
> >> types_match (c2_type, from_type) return false. So I add another
> >> condition to process this scenario.
> >>
> >> gcc/ChangeLog:
> >>
> >> * match.pd: Add another condition to process type mismatch.
> >
> >This should most likely be:
> > ((cond (cmp (convert1? x) c1) (convert2? x) c2) pattern): Also allow
> >conversions that only change the sign.
> >
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> * gcc.dg/tree-ssa/phi-opt-41.c: New test.
> >> ---
> >> gcc/match.pd | 5 ++++-
> >> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c | 24 ++++++++++++++++++++++
> >> 2 files changed, 28 insertions(+), 1 deletion(-)
> >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
> >>
> >> diff --git a/gcc/match.pd b/gcc/match.pd
> >> index 95225e4ca5f..e864845bfa9 100644
> >> --- a/gcc/match.pd
> >> +++ b/gcc/match.pd
> >> @@ -5419,7 +5419,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >> && (types_match (c2_type, from_type)
> >> || (TYPE_PRECISION (c2_type) > TYPE_PRECISION (from_type)
> >> && (TYPE_UNSIGNED (from_type)
> >> - || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))))
> >> + || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))
> >> + || (TYPE_UNSIGNED (from_type) != TYPE_UNSIGNED (c2_type)
> >> + && TYPE_PRECISION (c2_type) == TYPE_PRECISION (from_type)
> >> + && !TYPE_OVERFLOW_WRAPS (c2_type))))
> >
> >What is the need for TYPE_OVERFLOW_WRAPS here? Also I think you just
> >need the check for TYPE_PRECISION instead of the rest.
> >Maybe instead of types_match here, tree_nop_conversion_p could be used
> >instead. I am not 100% sure though.
> >
> >I also suspect you should add a few other testcases that don't depend
> >on VRP changing things. Maybe a runtime test too.
> >
> >Thanks,
> >Andrew
> >
>
>
> I want to make sure the cont_int "c2" won't be overflow,so I use the TYPE_OVERFLOW_WRAPS.
That is exactly what the `int_fits_type_p (@2, from_type)` check is
there for in the first place.
Thanks,
Andrew
> I checked the code , tree_nop_conversion_p judge the TYPE_PRECISION or TYPE_MODE and doesn't
> care the unsigned_flag, it should be fine for this scenario, I'm not sure if there's a problem with this
> modification, I will run the regression to check whether it causes other issues.
> Thanks,
> Feng Wang
>
>
> >> {
> >> if (cmp != EQ_EXPR)
> >> code = minmax_from_comparison (cmp, @1, @3, @1, @2);
> >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
> >> new file mode 100644
> >> index 00000000000..d1101c2f9f7
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
> >> @@ -0,0 +1,24 @@
> >> +/* { dg-do compile } */
> >> +/* { dg-options "-O2 -fdump-tree-phiopt2" } */
> >> +
> >> +int foo1 (int data, int res)
> >> +{
> >> + res = data & 0xf;
> >> + res |= res << 4;
> >> + if (res < 0x22)
> >> + return 0x22;
> >> + return res;
> >> +}
> >> +
> >> +int foo2 (int data, int res)
> >> +{
> >> + res = data & 0xf;
> >> + unsigned int r = res;
> >> + r*=17;
> >> + res = r;
> >> + if (r < 0x22)
> >> + return 0x22;
> >> + return res;
> >> +}
> >> +
> >> +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 2 "phiopt2" } } */
> >> \ No newline at end of file
> >> --
> >> 2.17.1
> >>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH v2] gimple-match.pd Add more optimization for gimple_cond
2023-11-28 6:08 ` Andrew Pinski
@ 2023-11-29 12:33 ` Richard Biener
0 siblings, 0 replies; 5+ messages in thread
From: Richard Biener @ 2023-11-29 12:33 UTC (permalink / raw)
To: Andrew Pinski; +Cc: Feng Wang, gcc-patches, kito.cheng, Jeff Law
On Tue, Nov 28, 2023 at 7:08 AM Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Mon, Nov 27, 2023 at 10:04 PM Feng Wang <wangfeng@eswincomputing.com> wrote:
> >
> > On 2023-11-28 11:06 Andrew Pinski <pinskia@gmail.com> wrote:
> > >On Mon, Nov 27, 2023 at 6:56 PM Feng Wang <wangfeng@eswincomputing.com> wrote:
> > >>
> > >> The link of PATCH v1: https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg326661.html
> > >> This patch add another condition for gimple-cond optimization. Refer to
> > >> the following test case.
> > >> int foo1 (int data, int res)
> > >> {
> > >> res = data & 0xf;
> > >> res |= res << 4;
> > >> if (res < 0x22)
> > >> return 0x22;
> > >> return res;
> > >> }
> > >> with the compilation flag "-O2",
> > >> before this patch the log info of phiopt2 pass is
> > >> <bb 2> [local count: 1073741824]:
> > >> res_5 = data_1(D) & 15;
> > >> _6 = (unsigned int) res_5;
> > >> _7 = _6 * 17;
> > >> res_8 = (int) _7;
> > >> if (_7 <= 33)
> > >> goto <bb 3>; [21.72%]
> > >> else
> > >> goto <bb 4>; [78.28%]
> > >>
> > >> <bb 3> [local count: 233216728]:
> > >>
> > >> <bb 4> [local count: 1073741824]:
> > >> # _9 = PHI <res_8(2), 34(3)>
> > >> return _9;
> > >> after this patch the the log info of phiopt2 pass is
> > >> <bb 2> [local count: 1073741824]:
> > >> res_5 = data_1(D) & 15;
> > >> _6 = (unsigned int) res_5;
> > >> _7 = _6 * 17;
> > >> res_8 = (int) _7;
> > >> _10 = MAX_EXPR <_7, 34>;
> > >> _3 = (int) _10;
> > >> return _3;
> > >> This patch optimizes the phi node to generate "MAX_EXPR".
> > >> The root cause of minmax replacement failure is the type of "_7"
> > >> is unsigned, but the type of const_int "34" is signed. It makes
> > >> types_match (c2_type, from_type) return false. So I add another
> > >> condition to process this scenario.
> > >>
> > >> gcc/ChangeLog:
> > >>
> > >> * match.pd: Add another condition to process type mismatch.
> > >
> > >This should most likely be:
> > > ((cond (cmp (convert1? x) c1) (convert2? x) c2) pattern): Also allow
> > >conversions that only change the sign.
> > >
> > >>
> > >> gcc/testsuite/ChangeLog:
> > >>
> > >> * gcc.dg/tree-ssa/phi-opt-41.c: New test.
> > >> ---
> > >> gcc/match.pd | 5 ++++-
> > >> gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c | 24 ++++++++++++++++++++++
> > >> 2 files changed, 28 insertions(+), 1 deletion(-)
> > >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
> > >>
> > >> diff --git a/gcc/match.pd b/gcc/match.pd
> > >> index 95225e4ca5f..e864845bfa9 100644
> > >> --- a/gcc/match.pd
> > >> +++ b/gcc/match.pd
> > >> @@ -5419,7 +5419,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > >> && (types_match (c2_type, from_type)
> > >> || (TYPE_PRECISION (c2_type) > TYPE_PRECISION (from_type)
> > >> && (TYPE_UNSIGNED (from_type)
> > >> - || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))))
> > >> + || TYPE_SIGN (c2_type) == TYPE_SIGN (from_type)))
> > >> + || (TYPE_UNSIGNED (from_type) != TYPE_UNSIGNED (c2_type)
> > >> + && TYPE_PRECISION (c2_type) == TYPE_PRECISION (from_type)
> > >> + && !TYPE_OVERFLOW_WRAPS (c2_type))))
> > >
> > >What is the need for TYPE_OVERFLOW_WRAPS here? Also I think you just
> > >need the check for TYPE_PRECISION instead of the rest.
> > >Maybe instead of types_match here, tree_nop_conversion_p could be used
> > >instead. I am not 100% sure though.
> > >
> > >I also suspect you should add a few other testcases that don't depend
> > >on VRP changing things. Maybe a runtime test too.
> > >
> > >Thanks,
> > >Andrew
> > >
> >
> >
> > I want to make sure the cont_int "c2" won't be overflow,so I use the TYPE_OVERFLOW_WRAPS.
> That is exactly what the `int_fits_type_p (@2, from_type)` check is
> there for in the first place.
I wonder why the types_match is there, if we know the constant fits the type
that should be sufficient if we properly care about the actual sign of
the comparison, that is.
Note we're passing down "mismatched" typed args to minmax_from_comparison, so
we probably have to be careful there, possibly passing down the type
of the comparison
explicitly.
Richard.
> Thanks,
> Andrew
>
> > I checked the code , tree_nop_conversion_p judge the TYPE_PRECISION or TYPE_MODE and doesn't
> > care the unsigned_flag, it should be fine for this scenario, I'm not sure if there's a problem with this
> > modification, I will run the regression to check whether it causes other issues.
>
>
> > Thanks,
> > Feng Wang
> >
> >
> > >> {
> > >> if (cmp != EQ_EXPR)
> > >> code = minmax_from_comparison (cmp, @1, @3, @1, @2);
> > >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
> > >> new file mode 100644
> > >> index 00000000000..d1101c2f9f7
> > >> --- /dev/null
> > >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-41.c
> > >> @@ -0,0 +1,24 @@
> > >> +/* { dg-do compile } */
> > >> +/* { dg-options "-O2 -fdump-tree-phiopt2" } */
> > >> +
> > >> +int foo1 (int data, int res)
> > >> +{
> > >> + res = data & 0xf;
> > >> + res |= res << 4;
> > >> + if (res < 0x22)
> > >> + return 0x22;
> > >> + return res;
> > >> +}
> > >> +
> > >> +int foo2 (int data, int res)
> > >> +{
> > >> + res = data & 0xf;
> > >> + unsigned int r = res;
> > >> + r*=17;
> > >> + res = r;
> > >> + if (r < 0x22)
> > >> + return 0x22;
> > >> + return res;
> > >> +}
> > >> +
> > >> +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 2 "phiopt2" } } */
> > >> \ No newline at end of file
> > >> --
> > >> 2.17.1
> > >>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-29 12:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-28 2:55 [PATCH v2] gimple-match.pd Add more optimization for gimple_cond Feng Wang
2023-11-28 3:06 ` Andrew Pinski
2023-11-28 6:04 ` Feng Wang
2023-11-28 6:08 ` Andrew Pinski
2023-11-29 12:33 ` 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).