diff mbox series

[rs6000] fix failure test cases caused by disabling mode promotion for pseudos [PR100952]

Message ID 3544e148-761a-1691-0b49-7d131739dd76@linux.ibm.com
State New
Headers show
Series [rs6000] fix failure test cases caused by disabling mode promotion for pseudos [PR100952] | expand

Commit Message

HAO CHEN GUI July 6, 2021, 3:11 a.m. UTC 
Hi

    The patch changed matching conditions in pr81384.c and pr56605.c. 
The original conditions failed to match due to mode promotion disabled.

    The attachments are the patch diff and change log file.

    Bootstrapped and tested on powerpc64le-linux with no regressions. Is 
this okay for trunk? Any recommendations? Thanks a lot.
PR target/100952
	* gcc/testsuite/gcc.target/powerpc/pr56605.c: Change matching
	conditions.
	* gcc/testsuite/gcc.target/powerpc/pr81348.c: Likewise.

Comments

HAO CHEN GUI July 19, 2021, 2:41 a.m. UTC | #1 
Hi,

   Gentle ping this:

https://gcc.gnu.org/pipermail/gcc-patches/2021-July/574503.html

Thanks.

On 6/7/2021 上午 11:11, HAO CHEN GUI wrote:
>
> Hi
>
>    The patch changed matching conditions in pr81384.c and pr56605.c. 
> The original conditions failed to match due to mode promotion disabled.
>
>    The attachments are the patch diff and change log file.
>
>    Bootstrapped and tested on powerpc64le-linux with no regressions. 
> Is this okay for trunk? Any recommendations? Thanks a lot.
>
Segher Boessenkool July 21, 2021, 10:51 p.m. UTC | #2 
Hi!

On Tue, Jul 06, 2021 at 11:11:05AM +0800, HAO CHEN GUI wrote:
>    The patch changed matching conditions in pr81384.c and pr56605.c. 
> The original conditions failed to match due to mode promotion disabled.

> 	PR target/100952
> 	* gcc/testsuite/gcc.target/powerpc/pr56605.c: Change matching
> 	conditions.
> 	* gcc/testsuite/gcc.target/powerpc/pr81348.c: Likewise.
> 

> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
> index 29efd815adc..2b7ddbd7410 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 \\\(reg:\[SD\]I" 1 "combine" } } */
> +/* { dg-final { scan-rtl-dump-times "\\\(compare:CC \\\((?:and|zero_extend):SI \\\(subreg:SI \\\(reg:\[SD\]I" 1 "combine" } } */

So, this testcase only runs on 64-bit machines (even only on lp64
configurations).  But do we now always get a subreg?  And, can that
change again some time in the future?

Writing it as
/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):SI \(subreg:SI \(reg:[SD]I} 1 "combine" } } */
is easier to read btw.

If you get a subreg:SI of a reg:SI here, something is wrong.  And you
cannot have a zero_extend:SI of anything :SI either.

So what the original matched were
  (compare:CC (and:DI (reg:DI
and
  (compare:CC (zero_extend:DI (reg:SI
and now you want to allow a subreg:SI in that last one as well (and you
do not really care what it is a subreg of, you don't check what offset
anyway), so maybe just
/* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI \((?:sub)?reg:[SD]I} 1 "combine" } } */
will do what you want?

> --- a/gcc/testsuite/gcc.target/powerpc/pr81348.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr81348.c
> @@ -19,5 +19,5 @@ void d(void)
>          ***c = e;
>  }
>  
> -/* { dg-final { scan-assembler {\mlxsihzx\M}  } } */
> -/* { dg-final { scan-assembler {\mvextsh2d\M} } } */
> +/* { dg-final { scan-assembler {\mlha\M}  } } */
> +/* { dg-final { scan-assembler {\mmtvsrwa\M} } } */

(This test should not test for powerpc64*-*-* but powerpc*-*-* btw,
and that means it can just be left out, so just
/* { dg-do compile { target lp64 } } */
and nothing more).

Okay for trunk with those changes (the RE and lp64).  Thanks!
(Test if it works of course; I did not :-) )


Segher
HAO CHEN GUI July 23, 2021, 12:46 a.m. UTC | #3 
Segher,

    Thanks for your advice. I tested it. "{ dg-final { 
scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI) 
\((?:sub)?reg:[SD]I} 1 "combine" } }" works well.

On 22/7/2021 上午 6:51, Segher Boessenkool wrote:
> Hi!
>
> On Tue, Jul 06, 2021 at 11:11:05AM +0800, HAO CHEN GUI wrote:
>>     The patch changed matching conditions in pr81384.c and pr56605.c.
>> The original conditions failed to match due to mode promotion disabled.
>> 	PR target/100952
>> 	* gcc/testsuite/gcc.target/powerpc/pr56605.c: Change matching
>> 	conditions.
>> 	* gcc/testsuite/gcc.target/powerpc/pr81348.c: Likewise.
>>
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
>> index 29efd815adc..2b7ddbd7410 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 \\\(reg:\[SD\]I" 1 "combine" } } */
>> +/* { dg-final { scan-rtl-dump-times "\\\(compare:CC \\\((?:and|zero_extend):SI \\\(subreg:SI \\\(reg:\[SD\]I" 1 "combine" } } */
> So, this testcase only runs on 64-bit machines (even only on lp64
> configurations).  But do we now always get a subreg?  And, can that
> change again some time in the future?
>
> Writing it as
> /* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):SI \(subreg:SI \(reg:[SD]I} 1 "combine" } } */
> is easier to read btw.
>
> If you get a subreg:SI of a reg:SI here, something is wrong.  And you
> cannot have a zero_extend:SI of anything :SI either.
>
> So what the original matched were
>    (compare:CC (and:DI (reg:DI
> and
>    (compare:CC (zero_extend:DI (reg:SI
> and now you want to allow a subreg:SI in that last one as well (and you
> do not really care what it is a subreg of, you don't check what offset
> anyway), so maybe just
> /* { dg-final { scan-rtl-dump-times {\(compare:CC \((?:and|zero_extend):(?:DI \((?:sub)?reg:[SD]I} 1 "combine" } } */
> will do what you want?
>
>> --- a/gcc/testsuite/gcc.target/powerpc/pr81348.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr81348.c
>> @@ -19,5 +19,5 @@ void d(void)
>>           ***c = e;
>>   }
>>   
>> -/* { dg-final { scan-assembler {\mlxsihzx\M}  } } */
>> -/* { dg-final { scan-assembler {\mvextsh2d\M} } } */
>> +/* { dg-final { scan-assembler {\mlha\M}  } } */
>> +/* { dg-final { scan-assembler {\mmtvsrwa\M} } } */
> (This test should not test for powerpc64*-*-* but powerpc*-*-* btw,
> and that means it can just be left out, so just
> /* { dg-do compile { target lp64 } } */
> and nothing more).
>
> Okay for trunk with those changes (the RE and lp64).  Thanks!
> (Test if it works of course; I did not :-) )
>
>
> Segher
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c b/gcc/testsuite/gcc.target/powerpc/pr56605.c
index 29efd815adc..2b7ddbd7410 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 \\\(reg:\[SD\]I" 1 "combine" } } */
+/* { dg-final { scan-rtl-dump-times "\\\(compare:CC \\\((?:and|zero_extend):SI \\\(subreg:SI \\\(reg:\[SD\]I" 1 "combine" } } */
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr81348.c b/gcc/testsuite/gcc.target/powerpc/pr81348.c
index 7037acf0c22..8043d06bcde 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr81348.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr81348.c
@@ -19,5 +19,5 @@  void d(void)
         ***c = e;
 }
 
-/* { dg-final { scan-assembler {\mlxsihzx\M}  } } */
-/* { dg-final { scan-assembler {\mvextsh2d\M} } } */
+/* { dg-final { scan-assembler {\mlha\M}  } } */
+/* { dg-final { scan-assembler {\mmtvsrwa\M} } } */