[PATCH v2] gimple-match.pd Add more optimization for gimple_cond

[PATCH v2] gimple-match.pd Add more optimization for gimple_cond

[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

Re: [PATCH v2] gimple-match.pd Add more optimization for gimple_cond

[Andrew Pinski]

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
>

Re: Re: [PATCH v2] gimple-match.pd Add more optimization for gimple_cond

[Feng Wang]

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
>>

Re: Re: [PATCH v2] gimple-match.pd Add more optimization for gimple_cond

[Andrew Pinski]

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
> >>

Re: Re: [PATCH v2] gimple-match.pd Add more optimization for gimple_cond

[Richard Biener]

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
> > >>