diff mbox series

rs6000/test: Fix empty TU in some cases of effective targets

Message ID 3d54f47a-983d-8900-7ebd-64ab55ed406c@linux.ibm.com
State New, archived
Headers
Series rs6000/test: Fix empty TU in some cases of effective targets |

Commit Message

Li, Pan2 via Gcc-patches July 20, 2022, 9:32 a.m. UTC 
  Hi,

As the failure of test case gcc.target/powerpc/pr92398.p9-.c in
PR106345 shows, some test sources for some powerpc effective
targets use empty translation unit wrongly.  The test sources
could go with options like "-ansi -pedantic-errors", then those
effective target checkings will fail unexpectedly with the
error messages like:

  error: ISO C forbids an empty translation unit [-Wpedantic]

This patch is to fix empty TUs with one dummy variable definition
accordingly.

Tested on powerpc64-linux-gnu P7 and P8 and
powerpc64le-linux-gnu P9 and P10.  Excepting for the failures
on gcc.target/powerpc/pr92398.p9-.c fixed, I can see it helps to
bring back some testing coverage like:

NA->PASS: gcc.target/powerpc/pr92398.p9+.c
NA->PASS: gcc.target/powerpc/pr93453-1.c

I'll push this soon if no objections.

BR,
Kewen
-----

	PR testsuite/106345

gcc/testsuite/ChangeLog:

	* lib/target-supports.exp (check_effective_target_powerpc_sqrt): Add
	a variable definition to avoid pedwarn about empty translation unit.
	(check_effective_target_has_arch_pwr5): Likewise.
	(check_effective_target_has_arch_pwr6): Likewise.
	(check_effective_target_has_arch_pwr7): Likewise.
	(check_effective_target_has_arch_pwr8): Likewise.
	(check_effective_target_has_arch_pwr9): Likewise.
	(check_effective_target_has_arch_pwr10): Likewise.
	(check_effective_target_has_arch_ppc64): Likewise.
	(check_effective_target_ppc_float128): Likewise.
	(check_effective_target_ppc_float128_insns): Likewise.
	(check_effective_target_powerpc_vsx): Likewise.
---
 gcc/testsuite/lib/target-supports.exp | 11 +++++++++++
 1 file changed, 11 insertions(+)

--
2.27.0

Comments

Segher Boessenkool July 21, 2022, 10:09 p.m. UTC | #1 
On Wed, Jul 20, 2022 at 05:32:01PM +0800, Kewen.Lin wrote:
> As the failure of test case gcc.target/powerpc/pr92398.p9-.c in
> PR106345 shows, some test sources for some powerpc effective
> targets use empty translation unit wrongly.  The test sources
> could go with options like "-ansi -pedantic-errors", then those
> effective target checkings will fail unexpectedly with the
> error messages like:
> 
>   error: ISO C forbids an empty translation unit [-Wpedantic]
> 
> This patch is to fix empty TUs with one dummy variable definition
> accordingly.

You can also use
  enum{a};
which is shorter, but more importantly does not generate any code.
You can also do
  extern int dummy;
of course -- same idea, no definitions, only declarations.

> I'll push this soon if no objections.

> @@ -6523,6 +6531,7 @@ proc check_effective_target_ppc_float128 { } {
>  	#ifndef __FLOAT128__
>  	  nope no good
>  	#endif
> +	int dummy;

At least put it in #else then?  Or just do things a bit more elegantly
(do a dummy function around this for example).


Segher
Li, Pan2 via Gcc-patches July 22, 2022, 12:41 a.m. UTC | #2 
Hi Segher,

Thanks for the comments!

on 2022/7/22 06:09, Segher Boessenkool wrote:
> On Wed, Jul 20, 2022 at 05:32:01PM +0800, Kewen.Lin wrote:
>> As the failure of test case gcc.target/powerpc/pr92398.p9-.c in
>> PR106345 shows, some test sources for some powerpc effective
>> targets use empty translation unit wrongly.  The test sources
>> could go with options like "-ansi -pedantic-errors", then those
>> effective target checkings will fail unexpectedly with the
>> error messages like:
>>
>>   error: ISO C forbids an empty translation unit [-Wpedantic]
>>
>> This patch is to fix empty TUs with one dummy variable definition
>> accordingly.
> 
> You can also use
>   enum{a};
> which is shorter, but more importantly does not generate any code.
> You can also do
>   extern int dummy;
> of course -- same idea, no definitions, only declarations.
> 

The used "int dummy" follows some existing practices, IMHO in this
context it doesn't matter that it will generate code or not, any of
these alternatives still generates an assembly or object file, but
the generated file gets removed after the checking.

May I still keep this "int dummy" to align with existing practices?
 
>> I'll push this soon if no objections.
> 
>> @@ -6523,6 +6531,7 @@ proc check_effective_target_ppc_float128 { } {
>>  	#ifndef __FLOAT128__
>>  	  nope no good
>>  	#endif
>> +	int dummy;
> 
> At least put it in #else then?  Or just do things a bit more elegantly
> (do a dummy function around this for example).
> 

OK, since it can still emit error even without "#else", I didn't bother
to add it.  I will add it, and update the "nope no good" to "#error
doesn't have float128 support".

BR,
Kewen
Segher Boessenkool July 22, 2022, 1:02 a.m. UTC | #3 
Hi!

On Fri, Jul 22, 2022 at 08:41:43AM +0800, Kewen.Lin wrote:
> Hi Segher,
> 
> Thanks for the comments!

Always.

> >> This patch is to fix empty TUs with one dummy variable definition
> >> accordingly.
> > 
> > You can also use
> >   enum{a};
> > which is shorter, but more importantly does not generate any code.
> > You can also do
> >   extern int dummy;
> > of course -- same idea, no definitions, only declarations.
> 
> The used "int dummy" follows some existing practices, IMHO in this
> context it doesn't matter that it will generate code or not, any of
> these alternatives still generates an assembly or object file, but
> the generated file gets removed after the checking.

It doesn't matter here, sure.  But it is certainly simple enough to make
it "extern int dummy" instead, not giving a bad example for future cases
where it may matter :-)

> May I still keep this "int dummy" to align with existing practices?

Of course, it was just advice.  If things are wrong (in my opinion that
is!), I'll say so.

> > At least put it in #else then?  Or just do things a bit more elegantly
> > (do a dummy function around this for example).
> 
> OK, since it can still emit error even without "#else", I didn't bother
> to add it.  I will add it, and update the "nope no good" to "#error
> doesn't have float128 support".

Just say

===
void nope (void)
{
#ifndef __FLOAT128__
	nope no good
#endif
}
===

which works in all cases?

Less maintenance is a good thing :-)


Segher
Li, Pan2 via Gcc-patches July 22, 2022, 1:26 a.m. UTC | #4 
Hi!

on 2022/7/22 09:02, Segher Boessenkool wrote:
> Hi!
> 
> On Fri, Jul 22, 2022 at 08:41:43AM +0800, Kewen.Lin wrote:
>> Hi Segher,
>>
>> Thanks for the comments!
> 
> Always.
> 
>>>> This patch is to fix empty TUs with one dummy variable definition
>>>> accordingly.
>>>
>>> You can also use
>>>   enum{a};
>>> which is shorter, but more importantly does not generate any code.
>>> You can also do
>>>   extern int dummy;
>>> of course -- same idea, no definitions, only declarations.
>>
>> The used "int dummy" follows some existing practices, IMHO in this
>> context it doesn't matter that it will generate code or not, any of
>> these alternatives still generates an assembly or object file, but
>> the generated file gets removed after the checking.
> 
> It doesn't matter here, sure.  But it is certainly simple enough to make
> it "extern int dummy" instead, not giving a bad example for future cases
> where it may matter :-)
> 

OK.

>> May I still keep this "int dummy" to align with existing practices?
> 
> Of course, it was just advice.  If things are wrong (in my opinion that
> is!), I'll say so.
> 

Got it, thanks!  :)

>>> At least put it in #else then?  Or just do things a bit more elegantly
>>> (do a dummy function around this for example).
>>
>> OK, since it can still emit error even without "#else", I didn't bother
>> to add it.  I will add it, and update the "nope no good" to "#error
>> doesn't have float128 support".
> 
> Just say
> 
> ===
> void nope (void)
> {
> #ifndef __FLOAT128__
> 	nope no good
> #endif
> }
> ===
> 
> which works in all cases?

Yeah, good idea, I'll make a new version of patch based on this.

Thanks again!

BR,
Kewen

> 
> Less maintenance is a good thing :-)
> 
> 
> Segher
diff mbox series

Patch

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 4ed7b25b9a4..aac2a557f5d 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -6262,6 +6262,7 @@  proc check_effective_target_powerpc_sqrt { } {
 	#ifndef _ARCH_PPCSQ
 	#error _ARCH_PPCSQ is not defined
 	#endif
+	int dummy;
     } {}]
 }

@@ -6373,6 +6374,7 @@  proc check_effective_target_has_arch_pwr5 { } {
 		#error does not have power5 support.
 		#else
 		/* "has power5 support" */
+		int dummy;
 		#endif
 	} [current_compiler_flags]]
 }
@@ -6383,6 +6385,7 @@  proc check_effective_target_has_arch_pwr6 { } {
 		#error does not have power6 support.
 		#else
 		/* "has power6 support" */
+		int dummy;
 		#endif
 	} [current_compiler_flags]]
 }
@@ -6393,6 +6396,7 @@  proc check_effective_target_has_arch_pwr7 { } {
 		#error does not have power7 support.
 		#else
 		/* "has power7 support" */
+		int dummy;
 		#endif
 	} [current_compiler_flags]]
 }
@@ -6403,6 +6407,7 @@  proc check_effective_target_has_arch_pwr8 { } {
 		#error does not have power8 support.
 		#else
 		/* "has power8 support" */
+		int dummy;
 		#endif
 	} [current_compiler_flags]]
 }
@@ -6413,6 +6418,7 @@  proc check_effective_target_has_arch_pwr9 { } {
 		#error does not have power9 support.
 		#else
 		/* "has power9 support" */
+		int dummy;
 		#endif
 	} [current_compiler_flags]]
 }
@@ -6423,6 +6429,7 @@  proc check_effective_target_has_arch_pwr10 { } {
 		#error does not have power10 support.
 		#else
 		/* "has power10 support" */
+		int dummy;
 		#endif
 	} [current_compiler_flags]]
 }
@@ -6433,6 +6440,7 @@  proc check_effective_target_has_arch_ppc64 { } {
 		#error does not have ppc64 support.
 		#else
 		/* "has ppc64 support" */
+		int dummy;
 		#endif
 	} [current_compiler_flags]]
 }
@@ -6523,6 +6531,7 @@  proc check_effective_target_ppc_float128 { } {
 	#ifndef __FLOAT128__
 	  nope no good
 	#endif
+	int dummy;
     }]
 }

@@ -6533,6 +6542,7 @@  proc check_effective_target_ppc_float128_insns { } {
 	#ifndef __FLOAT128_HARDWARE__
 	  nope no good
 	#endif
+	int dummy;
     }]
 }

@@ -6543,6 +6553,7 @@  proc check_effective_target_powerpc_vsx { } {
 	#ifndef __VSX__
 	  nope no vsx
 	#endif
+	int dummy;
     }]
 }