[PATCH, rs6000] Correct match pattern in pr56605.c

[PATCH, rs6000] Correct match pattern in pr56605.c

[HAO CHEN GUI]

Hi,
  This patch corrects the match pattern in pr56605.c. The former pattern
is wrong and test case fails with GCC11. It should match following insn on
each subtarget after mode promotion is disabled. The patch need to be
backported to GCC11.

//gimple
_17 = (unsigned int) _20;
 prolog_loop_niters.4_23 = _17 & 3;

//rtl
(insn 19 18 20 2 (parallel [
            (set (reg:CC 208)
                (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
                        (const_int 3 [0x3]))
                    (const_int 0 [0])))
            (set (reg:SI 129 [ prolog_loop_niters.5 ])
                (and:SI (subreg:SI (reg:DI 207) 0)
                    (const_int 3 [0x3])))
        ]) 197 {*andsi3_imm_mask_dot2}


  Bootstrapped and tested on powerpc64-linux BE/LE and AIX with no regressions.
Is this okay for trunk and GCC11? Any recommendations? Thanks a lot.

ChangeLog
2022-02-28 Haochen Gui <guihaoc@linux.ibm.com>

gcc/testsuite/
	PR target/102146
	* gcc.target/powerpc/pr56605.c: Correct match pattern in combine pass.


patch.diff
diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
index fdedbfc573d..231d808aa99 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
@@ -11,5 +11,5 @@ void foo (short* __restrict sb, int* __restrict ia)
     ia[i] = (int) sb[i];
 }

-/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
+/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */

Ping^1 [PATCH, rs6000] Correct match pattern in pr56605.c

[HAO CHEN GUI]

Hi,
  Gentle ping this:
  https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html
Thanks

On 28/2/2022 上午 11:17, HAO CHEN GUI wrote:
> Hi,
>   This patch corrects the match pattern in pr56605.c. The former pattern
> is wrong and test case fails with GCC11. It should match following insn on
> each subtarget after mode promotion is disabled. The patch need to be
> backported to GCC11.
> 
> //gimple
> _17 = (unsigned int) _20;
>  prolog_loop_niters.4_23 = _17 & 3;
> 
> //rtl
> (insn 19 18 20 2 (parallel [
>             (set (reg:CC 208)
>                 (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
>                         (const_int 3 [0x3]))
>                     (const_int 0 [0])))
>             (set (reg:SI 129 [ prolog_loop_niters.5 ])
>                 (and:SI (subreg:SI (reg:DI 207) 0)
>                     (const_int 3 [0x3])))
>         ]) 197 {*andsi3_imm_mask_dot2}
> 
> 
>   Bootstrapped and tested on powerpc64-linux BE/LE and AIX with no regressions.
> Is this okay for trunk and GCC11? Any recommendations? Thanks a lot.
> 
> ChangeLog
> 2022-02-28 Haochen Gui <guihaoc@linux.ibm.com>
> 
> gcc/testsuite/
> 	PR target/102146
> 	* gcc.target/powerpc/pr56605.c: Correct match pattern in combine pass.
> 
> 
> patch.diff
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> index fdedbfc573d..231d808aa99 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> @@ -11,5 +11,5 @@ void foo (short* __restrict sb, int* __restrict ia)
>      ia[i] = (int) sb[i];
>  }
> 
> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */
> 

Ping^2 [PATCH, rs6000] Correct match pattern in pr56605.c

[HAO CHEN GUI]

Hi,
  Gentle ping this:
   https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html
Thanks

On 15/3/2022 上午 10:06, HAO CHEN GUI wrote:
> Hi,
>   Gentle ping this:
>   https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html
> Thanks
> 
> On 28/2/2022 上午 11:17, HAO CHEN GUI wrote:
>> Hi,
>>   This patch corrects the match pattern in pr56605.c. The former pattern
>> is wrong and test case fails with GCC11. It should match following insn on
>> each subtarget after mode promotion is disabled. The patch need to be
>> backported to GCC11.
>>
>> //gimple
>> _17 = (unsigned int) _20;
>>  prolog_loop_niters.4_23 = _17 & 3;
>>
>> //rtl
>> (insn 19 18 20 2 (parallel [
>>             (set (reg:CC 208)
>>                 (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
>>                         (const_int 3 [0x3]))
>>                     (const_int 0 [0])))
>>             (set (reg:SI 129 [ prolog_loop_niters.5 ])
>>                 (and:SI (subreg:SI (reg:DI 207) 0)
>>                     (const_int 3 [0x3])))
>>         ]) 197 {*andsi3_imm_mask_dot2}
>>
>>
>>   Bootstrapped and tested on powerpc64-linux BE/LE and AIX with no regressions.
>> Is this okay for trunk and GCC11? Any recommendations? Thanks a lot.
>>
>> ChangeLog
>> 2022-02-28 Haochen Gui <guihaoc@linux.ibm.com>
>>
>> gcc/testsuite/
>> 	PR target/102146
>> 	* gcc.target/powerpc/pr56605.c: Correct match pattern in combine pass.
>>
>>
>> patch.diff
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> index fdedbfc573d..231d808aa99 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> @@ -11,5 +11,5 @@ void foo (short* __restrict sb, int* __restrict ia)
>>      ia[i] = (int) sb[i];
>>  }
>>
>> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
>> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */
>>

Re: [PATCH, rs6000] Correct match pattern in pr56605.c

[will schmidt]

On Mon, 2022-02-28 at 11:17 +0800, HAO CHEN GUI via Gcc-patches wrote:
> Hi,
>   This patch corrects the match pattern in pr56605.c. The former pattern
> is wrong and test case fails with GCC11. It should match following insn on
> each subtarget after mode promotion is disabled. The patch need to be
> backported to GCC11.
> 

Hi,

I note This patch appears to (partially?) address the P1 [11 regression] pr.  
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102146


The issue makes reference to a different proposed patch 
in issue https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103197
titled  ppc inline expansion of memcpy/memmove should not use lxsibzx/stxsibx for a single byte
proposed patch named
    rs6000: Disparage lfiwzx and similar

I can't address any of the background or history there.  :-)


> //gimple
> _17 = (unsigned int) _20;
>  prolog_loop_niters.4_23 = _17 & 3;
> 
> //rtl
> (insn 19 18 20 2 (parallel [
>             (set (reg:CC 208)
>                 (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
>                         (const_int 3 [0x3]))
>                     (const_int 0 [0])))
>             (set (reg:SI 129 [ prolog_loop_niters.5 ])
>                 (and:SI (subreg:SI (reg:DI 207) 0)
>                     (const_int 3 [0x3])))
>         ]) 197 {*andsi3_imm_mask_dot2}
> 
> 
>   Bootstrapped and tested on powerpc64-linux BE/LE and AIX with no regressions.
> Is this okay for trunk and GCC11? Any recommendations? Thanks a lot.
> 
> ChangeLog
> 2022-02-28 Haochen Gui <guihaoc@linux.ibm.com>
> 
> gcc/testsuite/
> 	PR target/102146
> 	* gcc.target/powerpc/pr56605.c: Correct match pattern in combine pass.
> 
> 
> patch.diff
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> index fdedbfc573d..231d808aa99 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> @@ -11,5 +11,5 @@ void foo (short* __restrict sb, int* __restrict ia)
>      ia[i] = (int) sb[i];
>  }
> 
> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */


SO with the update, (i squint so this is an approximate handwave) this
drops the zero_extend and changes the destination type to be DI for the
scan-rtl.    This appears to match the rtl as mentioned in the patch
comments.


> 

Re: [PATCH, rs6000] Correct match pattern in pr56605.c

[Segher Boessenkool]

Hi!

On Mon, Feb 28, 2022 at 11:17:27AM +0800, HAO CHEN GUI wrote:
>   This patch corrects the match pattern in pr56605.c. The former pattern
> is wrong and test case fails with GCC11. It should match following insn on
> each subtarget after mode promotion is disabled. The patch need to be
> backported to GCC11.
> 
> //gimple
> _17 = (unsigned int) _20;
>  prolog_loop_niters.4_23 = _17 & 3;
> 
> //rtl
> (insn 19 18 20 2 (parallel [
>             (set (reg:CC 208)
>                 (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
>                         (const_int 3 [0x3]))
>                     (const_int 0 [0])))
>             (set (reg:SI 129 [ prolog_loop_niters.5 ])
>                 (and:SI (subreg:SI (reg:DI 207) 0)
>                     (const_int 3 [0x3])))
>         ]) 197 {*andsi3_imm_mask_dot2}
> 
> 
>   Bootstrapped and tested on powerpc64-linux BE/LE and AIX with no regressions.
> Is this okay for trunk and GCC11? Any recommendations? Thanks a lot.
> 
> ChangeLog
> 2022-02-28 Haochen Gui <guihaoc@linux.ibm.com>
> 
> gcc/testsuite/
> 	PR target/102146
> 	* gcc.target/powerpc/pr56605.c: Correct match pattern in combine pass.
> 
> 
> patch.diff
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> index fdedbfc573d..231d808aa99 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> @@ -11,5 +11,5 @@ void foo (short* __restrict sb, int* __restrict ia)
>      ia[i] = (int) sb[i];
>  }
> 
> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */

The old pattern uses non-capturing braces here, which are required for
...-times to work correctly.  The zero_extend alternative is required as
well, as is making the subreg optional (we have an actual reg in one of
the cases currently).  What do you consider wrong about the old pattern,
what in the generated code is different from what you expect?

It works correctly on p7 etc. btw; where do you see it fail?  p10?


Segher

Re: [PATCH, rs6000] Correct match pattern in pr56605.c

[HAO CHEN GUI]

Hi,

On 9/4/2022 上午 12:48, will schmidt wrote:
> On Mon, 2022-02-28 at 11:17 +0800, HAO CHEN GUI via Gcc-patches wrote:
>> Hi,
>>   This patch corrects the match pattern in pr56605.c. The former pattern
>> is wrong and test case fails with GCC11. It should match following insn on
>> each subtarget after mode promotion is disabled. The patch need to be
>> backported to GCC11.
>>
> 
> Hi,
> 
> I note This patch appears to (partially?) address the P1 [11 regression] pr.  
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102146

There are two issues left in this PR. One is pr56605.c. My patch fixes it.
Another is prefix-no-update.c. The patch Segher proposed in 103197 could fix it.
Thanks.
> 
> 
> The issue makes reference to a different proposed patch 
> in issue https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103197
> titled  ppc inline expansion of memcpy/memmove should not use lxsibzx/stxsibx for a single byte
> proposed patch named
>     rs6000: Disparage lfiwzx and similar
> 
> I can't address any of the background or history there.  :-)
> 
> 
>> //gimple
>> _17 = (unsigned int) _20;
>>  prolog_loop_niters.4_23 = _17 & 3;
>>
>> //rtl
>> (insn 19 18 20 2 (parallel [
>>             (set (reg:CC 208)
>>                 (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
>>                         (const_int 3 [0x3]))
>>                     (const_int 0 [0])))
>>             (set (reg:SI 129 [ prolog_loop_niters.5 ])
>>                 (and:SI (subreg:SI (reg:DI 207) 0)
>>                     (const_int 3 [0x3])))
>>         ]) 197 {*andsi3_imm_mask_dot2}
>>
>>
>>   Bootstrapped and tested on powerpc64-linux BE/LE and AIX with no regressions.
>> Is this okay for trunk and GCC11? Any recommendations? Thanks a lot.
>>
>> ChangeLog
>> 2022-02-28 Haochen Gui <guihaoc@linux.ibm.com>
>>
>> gcc/testsuite/
>> 	PR target/102146
>> 	* gcc.target/powerpc/pr56605.c: Correct match pattern in combine pass.
>>
>>
>> patch.diff
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> index fdedbfc573d..231d808aa99 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> @@ -11,5 +11,5 @@ void foo (short* __restrict sb, int* __restrict ia)
>>      ia[i] = (int) sb[i];
>>  }
>>
>> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
>> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */
> 
> 
> SO with the update, (i squint so this is an approximate handwave) this
> drops the zero_extend and changes the destination type to be DI for the
> scan-rtl.    This appears to match the rtl as mentioned in the patch
> comments.
> 
> 
>>
> 

Re: [PATCH, rs6000] Correct match pattern in pr56605.c

[HAO CHEN GUI]

Hi,

On 9/4/2022 上午 3:36, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, Feb 28, 2022 at 11:17:27AM +0800, HAO CHEN GUI wrote:
>>   This patch corrects the match pattern in pr56605.c. The former pattern
>> is wrong and test case fails with GCC11. It should match following insn on
>> each subtarget after mode promotion is disabled. The patch need to be
>> backported to GCC11.
>>
>> //gimple
>> _17 = (unsigned int) _20;
>>  prolog_loop_niters.4_23 = _17 & 3;
>>
>> //rtl
>> (insn 19 18 20 2 (parallel [
>>             (set (reg:CC 208)
>>                 (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
>>                         (const_int 3 [0x3]))
>>                     (const_int 0 [0])))
>>             (set (reg:SI 129 [ prolog_loop_niters.5 ])
>>                 (and:SI (subreg:SI (reg:DI 207) 0)
>>                     (const_int 3 [0x3])))
>>         ]) 197 {*andsi3_imm_mask_dot2}
>>
>>
>>   Bootstrapped and tested on powerpc64-linux BE/LE and AIX with no regressions.
>> Is this okay for trunk and GCC11? Any recommendations? Thanks a lot.
>>
>> ChangeLog
>> 2022-02-28 Haochen Gui <guihaoc@linux.ibm.com>
>>
>> gcc/testsuite/
>> 	PR target/102146
>> 	* gcc.target/powerpc/pr56605.c: Correct match pattern in combine pass.
>>
>>
>> patch.diff
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> index fdedbfc573d..231d808aa99 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> @@ -11,5 +11,5 @@ void foo (short* __restrict sb, int* __restrict ia)
>>      ia[i] = (int) sb[i];
>>  }
>>
>> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
>> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */
> 
> The old pattern uses non-capturing braces here, which are required for
> ...-times to work correctly.  The zero_extend alternative is required as
> well, as is making the subreg optional (we have an actual reg in one of
> the cases currently).  What do you consider wrong about the old pattern,
> what in the generated code is different from what you expect?
> 
> It works correctly on p7 etc. btw; where do you see it fail?  p10?
> 
> 
I saw it failed with GCC11.

FAIL: gcc.target/powerpc/pr56605.c scan-rtl-dump-times combine "\\(compare:CC \\((?:and|zero_extend):(?:DI) \\((?:sub)?reg:[SD]I" 1

On ppc64le with GCC11, it should match following insn.

(compare:CC (and:SI (subreg:SI (reg:DI 208) 0)

With GCC12, it should match following insn.

(compare:CC (and:SI (subreg:SI (reg:DI 207) 0)

With GCC12 the pattern actually matches:

(compare:CC (and:DI (subreg:DI (reg:SI 136 [ niters.6 ]) 0)

So GCC12 doesn't fail the case. But it actually match wrong insn.
There is no such insn in GCC11 combine dump. So GCC11 hits the problem.

Thanks.

> Segher

Re: Ping^2 [PATCH, rs6000] Correct match pattern in pr56605.c

[Alexandre Oliva]

On Apr  7, 2022, HAO CHEN GUI via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

>   Gentle ping this:
>    https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html
> Thanks

>> On 28/2/2022 上午 11:17, HAO CHEN GUI wrote:

>>> This patch corrects the match pattern in pr56605.c. The former pattern
>>> is wrong and test case fails with GCC11. It should match following insn on
>>> each subtarget after mode promotion is disabled. The patch need to be
>>> backported to GCC11.

>>> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
>>> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */


How about this less strict change instead?


ppc: testsuite: PROMOTE_MODE fallout pr56605 [PR102146]

The test expects a compare of DImode values, but after the removal of
PROMOTE_MODE from rs6000/, we get SImode.  Adjust the expectations.

Tested with gcc-11 targeting ppc64-vx7r2.  Ok to install?


for  gcc/testsuite/ChangeLog

	PR target/102146
	* gcc.target/powerpc/pr56605.c: Accept SImode compare operand.
---
 gcc/testsuite/gcc.target/powerpc/pr56605.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
index fdedbfc573dd8..7695f87db6f66 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
@@ -11,5 +11,5 @@ void foo (short* __restrict sb, int* __restrict ia)
     ia[i] = (int) sb[i];
 }
 
-/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
+/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:[SD]I) \((?:sub)?reg:[SD]I} 1 "combine" } } */
 



-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: Ping^2 [PATCH, rs6000] Correct match pattern in pr56605.c

[Segher Boessenkool]

On Mon, Apr 11, 2022 at 08:54:14PM -0300, Alexandre Oliva wrote:
> How about this less strict change instead?
> 
> ppc: testsuite: PROMOTE_MODE fallout pr56605 [PR102146]
> 
> The test expects a compare of DImode values, but after the removal of
> PROMOTE_MODE from rs6000/, we get SImode.  Adjust the expectations.

> 	PR target/102146
> 	* gcc.target/powerpc/pr56605.c: Accept SImode compare operand.

> --- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> @@ -11,5 +11,5 @@ void foo (short* __restrict sb, int* __restrict ia)
>      ia[i] = (int) sb[i];
>  }
>  
> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:[SD]I) \((?:sub)?reg:[SD]I} 1 "combine" } } */

Ah, a nice small change.  This looks fine, thanks!  Okay for trunk.

It is fine to allow zero_extend:SI of :SI or :DI, those are invalid RTL,
testcases do not have to test there is no invalid RTL, in general.


Segher

Re: [PATCH, rs6000] Correct match pattern in pr56605.c

[Segher Boessenkool]

On Mon, Apr 11, 2022 at 10:47:53AM +0800, HAO CHEN GUI wrote:
> There are two issues left in this PR. One is pr56605.c. My patch fixes it.
> Another is prefix-no-update.c. The patch Segher proposed in 103197 could fix it.

So today all remaining problems will be fixed.  Thanks for shepherding
this PR!


Segher

Re: Ping^2 [PATCH, rs6000] Correct match pattern in pr56605.c

[Segher Boessenkool]

On Mon, Apr 11, 2022 at 08:54:14PM -0300, Alexandre Oliva wrote:
> On Apr  7, 2022, HAO CHEN GUI via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> >   Gentle ping this:
> >    https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html
> > Thanks
> 
> >> On 28/2/2022 上午 11:17, HAO CHEN GUI wrote:
> 
> >>> This patch corrects the match pattern in pr56605.c. The former pattern
> >>> is wrong and test case fails with GCC11. It should match following insn on
> >>> each subtarget after mode promotion is disabled. The patch need to be
> >>> backported to GCC11.
> 
> >>> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
> >>> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */
> 
> 
> How about this less strict change instead?
> 
> 
> ppc: testsuite: PROMOTE_MODE fallout pr56605 [PR102146]
> 
> The test expects a compare of DImode values, but after the removal of
> PROMOTE_MODE from rs6000/, we get SImode.  Adjust the expectations.
> 
> Tested with gcc-11 targeting ppc64-vx7r2.  Ok to install?

This should have been tested on Linux as well: it is now broken on both
-m32 and -m64 there.  Please revert?


Segher

Re: Ping^2 [PATCH, rs6000] Correct match pattern in pr56605.c

[Segher Boessenkool]

On Wed, Apr 13, 2022 at 04:30:36PM -0500, Segher Boessenkool wrote:
> This should have been tested on Linux as well: it is now broken on both
> -m32 and -m64 there.  Please revert?

Sorry, confusing with another new regression: this one is only -m64 of
course.


Segher

Re: Ping^2 [PATCH, rs6000] Correct match pattern in pr56605.c

[HAO CHEN GUI]

Hi,
   I tested the test case on Linux and AIX with both big and little endian.
The test case requires lp64 target, so it won't be tested on 32-bit targets.

On big endian (both AIX and Linux), it should match
(compare:CC (and:SI (subreg:SI (reg:DI 207) 4)

On little endian (both AIX and Linux), it should match
(compare:CC (and:SI (subreg:SI (reg:DI 207) 0)

So, the pattern in my patch should work fine.

/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */

Thanks.

On 14/4/2022 上午 5:30, Segher Boessenkool wrote:
> On Mon, Apr 11, 2022 at 08:54:14PM -0300, Alexandre Oliva wrote:
>> On Apr  7, 2022, HAO CHEN GUI via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>
>>>   Gentle ping this:
>>>    https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html
>>> Thanks
>>
>>>> On 28/2/2022 上午 11:17, HAO CHEN GUI wrote:
>>
>>>>> This patch corrects the match pattern in pr56605.c. The former pattern
>>>>> is wrong and test case fails with GCC11. It should match following insn on
>>>>> each subtarget after mode promotion is disabled. The patch need to be
>>>>> backported to GCC11.
>>
>>>>> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) \((?:sub)?reg:[SD]I} 1 "combine" } } */
>>>>> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */
>>
>>
>> How about this less strict change instead?
>>
>>
>> ppc: testsuite: PROMOTE_MODE fallout pr56605 [PR102146]
>>
>> The test expects a compare of DImode values, but after the removal of
>> PROMOTE_MODE from rs6000/, we get SImode.  Adjust the expectations.
>>
>> Tested with gcc-11 targeting ppc64-vx7r2.  Ok to install?
> 
> This should have been tested on Linux as well: it is now broken on both
> -m32 and -m64 there.  Please revert?
> 
> 
> Segher

Re: Ping^2 [PATCH, rs6000] Correct match pattern in pr56605.c

[Segher Boessenkool]

On Tue, Apr 19, 2022 at 04:05:06PM +0800, HAO CHEN GUI wrote:
>    I tested the test case on Linux and AIX with both big and little endian.
> The test case requires lp64 target, so it won't be tested on 32-bit targets.
> 
> On big endian (both AIX and Linux), it should match
> (compare:CC (and:SI (subreg:SI (reg:DI 207) 4)
> 
> On little endian (both AIX and Linux), it should match
> (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
> 
> So, the pattern in my patch should work fine.
> 
> /* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */

On powerpc64-linux:

FAIL: gcc.target/powerpc/pr56605.c scan-rtl-dump-times combine "\\(compare:CC \\((?:and|zero_extend):(?:[SD]I) \\((?:sub)?reg:[SD]I" 1

It matches twice instead of once, namely:

(insn 19 18 20 2 (parallel [
            (set (reg:CC 208)
                (compare:CC (and:SI (subreg:SI (reg:DI 207) 4)
                        (const_int 3 [0x3]))
                    (const_int 0 [0])))
            (set (reg:SI 129 [ prolog_loop_niters.5 ])
                (and:SI (subreg:SI (reg:DI 207) 4)
                    (const_int 3 [0x3])))
        ]) 208 {*andsi3_imm_mask_dot2}
     (nil))

(insn 81 80 82 11 (parallel [
            (set (reg:CC 232)
                (compare:CC (and:DI (subreg:DI (reg:SI 136 [ niters.6 ]) 0)
                        (const_int 7 [0x7]))
                    (const_int 0 [0])))
            (clobber (scratch:DI))
        ]) 207 {*anddi3_imm_mask_dot}
     (expr_list:REG_DEAD (reg:SI 136 [ niters.6 ])
        (nil)))

The paradoxical subreg in the latter wasn't expected :-)


Segher

Re: Ping^2 [PATCH, rs6000] Correct match pattern in pr56605.c

[HAO CHEN GUI]

Hi Segher,
   Yes, the old committed patch caused it matches two insns.
So I submitted the new patch which fixes the problem. Here is
the new patch.
https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html

The new pattern is:
/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */

I tested it and it is fine on all sub-targets.
Thanks.

On 20/4/2022 上午 5:06, Segher Boessenkool wrote:
> On Tue, Apr 19, 2022 at 04:05:06PM +0800, HAO CHEN GUI wrote:
>>    I tested the test case on Linux and AIX with both big and little endian.
>> The test case requires lp64 target, so it won't be tested on 32-bit targets.
>>
>> On big endian (both AIX and Linux), it should match
>> (compare:CC (and:SI (subreg:SI (reg:DI 207) 4)
>>
>> On little endian (both AIX and Linux), it should match
>> (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
>>
>> So, the pattern in my patch should work fine.
>>
>> /* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */
> 
> On powerpc64-linux:
> 
> FAIL: gcc.target/powerpc/pr56605.c scan-rtl-dump-times combine "\\(compare:CC \\((?:and|zero_extend):(?:[SD]I) \\((?:sub)?reg:[SD]I" 1
> 
> It matches twice instead of once, namely:
> 
> (insn 19 18 20 2 (parallel [
>             (set (reg:CC 208)
>                 (compare:CC (and:SI (subreg:SI (reg:DI 207) 4)
>                         (const_int 3 [0x3]))
>                     (const_int 0 [0])))
>             (set (reg:SI 129 [ prolog_loop_niters.5 ])
>                 (and:SI (subreg:SI (reg:DI 207) 4)
>                     (const_int 3 [0x3])))
>         ]) 208 {*andsi3_imm_mask_dot2}
>      (nil))
> 
> (insn 81 80 82 11 (parallel [
>             (set (reg:CC 232)
>                 (compare:CC (and:DI (subreg:DI (reg:SI 136 [ niters.6 ]) 0)
>                         (const_int 7 [0x7]))
>                     (const_int 0 [0])))
>             (clobber (scratch:DI))
>         ]) 207 {*anddi3_imm_mask_dot}
>      (expr_list:REG_DEAD (reg:SI 136 [ niters.6 ])
>         (nil)))
> 
> The paradoxical subreg in the latter wasn't expected :-)
> 
> 
> Segher

Re: [PATCH] [rs6000] Correct match pattern in pr56605.c

[Kewen.Lin]

Hi Haochen,

on 2023/3/27 17:46, HAO CHEN GUI wrote:
> Kewen,
>   The case still fails with trunk.
> 

OK, thanks for checking, the proposed patch can catch the expected pattern
accurately (excluding noises), so okay for trunk and branches, thanks!

BR,
Kewen

> FAIL: gcc.target/powerpc/pr56605.c scan-rtl-dump-times combine "\\(compare:CC \\((?:and|zero_extend):(?:[SD]I) \\((?:sub)?reg:[SD]I" 1
> 
>                 === gcc Summary ===
> 
> # of expected passes            1
> # of unexpected failures        1
> 
>   With the trunk, it should match the pattern.
> (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)

Re: [PATCH] [rs6000] Correct match pattern in pr56605.c

[HAO CHEN GUI]

Kewen,
  The case still fails with trunk.

FAIL: gcc.target/powerpc/pr56605.c scan-rtl-dump-times combine "\\(compare:CC \\((?:and|zero_extend):(?:[SD]I) \\((?:sub)?reg:[SD]I" 1

                === gcc Summary ===

# of expected passes            1
# of unexpected failures        1

  With the trunk, it should match the pattern.
(compare:CC (and:SI (subreg:SI (reg:DI 207) 0)

Thanks
Gui Haochen


在 2023/3/27 15:41, Kewen.Lin 写道:
> Hi Alexandre and Haochen,
> 
> on 2023/3/25 16:42, Alexandre Oliva via Gcc-patches wrote:
>>
>> Ping https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html
>>
>> From: Haochen Gui <guihaoc@linux.ibm.com>
>>
>> This patch corrects the match pattern in pr56605.c. The former pattern
>> is wrong and test case fails with GCC11. It should match following
>> insn on each subtarget after mode promotion is disabled. The patch
>> need to be backported to GCC11.
> 
> Comment https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102146#c21 made me
> feel that this test issue was just in branches, but this proposed patch
> seems to say it still exists on trunk, could you confirm that?
> 
> BR,
> Kewen
> 
>>
>> //gimple
>> _17 = (unsigned int) _20;
>>  prolog_loop_niters.4_23 = _17 & 3;
>>
>> //rtl
>> (insn 19 18 20 2 (parallel [
>>             (set (reg:CC 208)
>>                 (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
>>                         (const_int 3 [0x3]))
>>                     (const_int 0 [0])))
>>             (set (reg:SI 129 [ prolog_loop_niters.5 ])
>>                 (and:SI (subreg:SI (reg:DI 207) 0)
>>                     (const_int 3 [0x3])))
>>         ]) 197 {*andsi3_imm_mask_dot2}
>>
>> Rebased.  Regstrapped on ppc64-linux-gnu.  Also tested with
>> ppc64-vxworks7r2 (gcc-12), where it's also needed.  Ok to install?
>>
>>
>> for  gcc/testsuite/ChangeLog
>>
>> 	PR target/102146
>> 	* gcc.target/powerpc/pr56605.c: Correct match pattern in
>> 	combine pass.
>> ---
>>  gcc/testsuite/gcc.target/powerpc/pr56605.c |    3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> index 7695f87db6f66..651a88e3cc7f9 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> @@ -11,5 +11,4 @@ void foo (short* __restrict sb, int* __restrict ia)
>>      ia[i] = (int) sb[i];
>>  }
>>  
>> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:[SD]I) \((?:sub)?reg:[SD]I} 1 "combine" } } */
>> -
>> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */
> 

Re: [PATCH] [rs6000] Correct match pattern in pr56605.c

[Kewen.Lin]

Hi Alexandre and Haochen,

on 2023/3/25 16:42, Alexandre Oliva via Gcc-patches wrote:
> 
> Ping https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html
> 
> From: Haochen Gui <guihaoc@linux.ibm.com>
> 
> This patch corrects the match pattern in pr56605.c. The former pattern
> is wrong and test case fails with GCC11. It should match following
> insn on each subtarget after mode promotion is disabled. The patch
> need to be backported to GCC11.

Comment https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102146#c21 made me
feel that this test issue was just in branches, but this proposed patch
seems to say it still exists on trunk, could you confirm that?

BR,
Kewen

> 
> //gimple
> _17 = (unsigned int) _20;
>  prolog_loop_niters.4_23 = _17 & 3;
> 
> //rtl
> (insn 19 18 20 2 (parallel [
>             (set (reg:CC 208)
>                 (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
>                         (const_int 3 [0x3]))
>                     (const_int 0 [0])))
>             (set (reg:SI 129 [ prolog_loop_niters.5 ])
>                 (and:SI (subreg:SI (reg:DI 207) 0)
>                     (const_int 3 [0x3])))
>         ]) 197 {*andsi3_imm_mask_dot2}
> 
> Rebased.  Regstrapped on ppc64-linux-gnu.  Also tested with
> ppc64-vxworks7r2 (gcc-12), where it's also needed.  Ok to install?
> 
> 
> for  gcc/testsuite/ChangeLog
> 
> 	PR target/102146
> 	* gcc.target/powerpc/pr56605.c: Correct match pattern in
> 	combine pass.
> ---
>  gcc/testsuite/gcc.target/powerpc/pr56605.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> index 7695f87db6f66..651a88e3cc7f9 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> @@ -11,5 +11,4 @@ void foo (short* __restrict sb, int* __restrict ia)
>      ia[i] = (int) sb[i];
>  }
>  
> -/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:[SD]I) \((?:sub)?reg:[SD]I} 1 "combine" } } */
> -
> +/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */

[PATCH] [rs6000] Correct match pattern in pr56605.c

[Alexandre Oliva]

Ping https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590958.html

From: Haochen Gui <guihaoc@linux.ibm.com>

This patch corrects the match pattern in pr56605.c. The former pattern
is wrong and test case fails with GCC11. It should match following
insn on each subtarget after mode promotion is disabled. The patch
need to be backported to GCC11.

//gimple
_17 = (unsigned int) _20;
 prolog_loop_niters.4_23 = _17 & 3;

//rtl
(insn 19 18 20 2 (parallel [
            (set (reg:CC 208)
                (compare:CC (and:SI (subreg:SI (reg:DI 207) 0)
                        (const_int 3 [0x3]))
                    (const_int 0 [0])))
            (set (reg:SI 129 [ prolog_loop_niters.5 ])
                (and:SI (subreg:SI (reg:DI 207) 0)
                    (const_int 3 [0x3])))
        ]) 197 {*andsi3_imm_mask_dot2}

Rebased.  Regstrapped on ppc64-linux-gnu.  Also tested with
ppc64-vxworks7r2 (gcc-12), where it's also needed.  Ok to install?


for  gcc/testsuite/ChangeLog

	PR target/102146
	* gcc.target/powerpc/pr56605.c: Correct match pattern in
	combine pass.
---
 gcc/testsuite/gcc.target/powerpc/pr56605.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
index 7695f87db6f66..651a88e3cc7f9 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
@@ -11,5 +11,4 @@ void foo (short* __restrict sb, int* __restrict ia)
     ia[i] = (int) sb[i];
 }
 
-/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:[SD]I) \((?:sub)?reg:[SD]I} 1 "combine" } } */
-
+/* { dg-final { scan-rtl-dump-times {\(compare:CC \(and:SI \(subreg:SI \(reg:DI} 1 "combine" } } */

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>