diff mbox series

[RS6000] Power10 vec-splati-runnable multiple test failures

Message ID 20201022065605.GH4898@bubble.grove.modra.org
State New
Headers show
Series [RS6000] Power10 vec-splati-runnable multiple test failures | expand

Commit Message

Alan Modra Oct. 22, 2020, 6:56 a.m. UTC
FAIL: gcc.target/powerpc/vec-splati-runnable.c 1 blank line(s) in output
FAIL: gcc.target/powerpc/vec-splati-runnable.c (test for excess errors)
Excess errors:
rs6000_emit_xxspltidp_v2df called ...

and running the test fails.  As the comment says
  /* Although the instruction says the results are not defined, it does seem
     to work, at least on Mambo.  But no guarentees!  */
So the simulator works but not real hardware.

Regstrapped powerpc64le-linux on power10 and power8.  OK?

gcc/
	* config/rs6000/rs6000.c (rs6000_emit_xxspltidp_v2df): Delete
	debug printf.  Remove trailing ".\n" from inform message.
	Break long line.
gcc/testsuite/
	* gcc.target/powerpc/vec-splati-runnable.c: Don't abort on
	undefined output.

Comments

Carl Love Oct. 22, 2020, 4:08 p.m. UTC | #1
On Thu, 2020-10-22 at 17:26 +1030, Alan Modra wrote:
> FAIL: gcc.target/powerpc/vec-splati-runnable.c 1 blank line(s) in
> output
> FAIL: gcc.target/powerpc/vec-splati-runnable.c (test for excess
> errors)
> Excess errors:
> rs6000_emit_xxspltidp_v2df called ...
> 
> and running the test fails.  As the comment says
>   /* Although the instruction says the results are not defined, it
> does seem
>      to work, at least on Mambo.  But no guarentees!  */
> So the simulator works but not real hardware.
> 
> Regstrapped powerpc64le-linux on power10 and power8.  OK?
> 
> gcc/
> 	* config/rs6000/rs6000.c (rs6000_emit_xxspltidp_v2df): Delete
> 	debug printf.  Remove trailing ".\n" from inform message.
> 	Break long line.
> gcc/testsuite/
> 	* gcc.target/powerpc/vec-splati-runnable.c: Don't abort on
> 	undefined output.
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index c0adc56387f..2f0ca7af030 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -27485,11 +27485,10 @@ rs6000_const_f32_to_i32 (rtx operand)
>  void
>  rs6000_emit_xxspltidp_v2df (rtx dst, long value)
>  {
> -  printf("rs6000_emit_xxspltidp_v2df called %ld\n", value);
> -  printf("rs6000_emit_xxspltidp_v2df called 0x%lx\n", value);
>    if (((value & 0x7F800000) == 0) && ((value & 0x7FFFFF) != 0))
>      inform (input_location,
> -	    "the result for the xxspltidp instruction is undefined for
> subnormal input values.\n");
> +	    "the result for the xxspltidp instruction "
> +	    "is undefined for subnormal input values");
>    emit_insn( gen_xxspltidp_v2df_inst (dst, GEN_INT (value)));
>  }
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> index afb0bfdef3a..e5a4935644f 100644
> --- a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
> @@ -5,7 +5,7 @@
> 
>  #define DEBUG 0
> 
> -#ifdef DEBUG
> +#if DEBUG
>  #include <stdio.h>
>  #endif
> 
> @@ -100,7 +100,7 @@ main (int argc, char *argv [])
>        printf(" vresult_d[%i] = %e, expected_vresult_d[%i] = %e\n",
>  	     i, vresult_d[i], i, expected_vresult_d[i]);
>  #else
> -    abort();
> +    ;
>  #endif
>    }

Alan:

Yea, the #ifdef should be #if.  

I see the error print statement you changed so that it would not wrap. 
I have always been told it is best not to break the print statement
across two lines.  The argument is it makes it harder to find it in the
code when using grep.  In this case, it should be clear what file the
error statement is in.  What is your take in general about breaking or
not breaking the body of an error print statement across lines?

                  Carl
Segher Boessenkool Oct. 22, 2020, 4:15 p.m. UTC | #2
Hi!

On Thu, Oct 22, 2020 at 09:08:58AM -0700, Carl Love wrote:
> On Thu, 2020-10-22 at 17:26 +1030, Alan Modra wrote:
> > FAIL: gcc.target/powerpc/vec-splati-runnable.c 1 blank line(s) in
> > output
> > FAIL: gcc.target/powerpc/vec-splati-runnable.c (test for excess
> > errors)
> > Excess errors:
> > rs6000_emit_xxspltidp_v2df called ...

> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -27485,11 +27485,10 @@ rs6000_const_f32_to_i32 (rtx operand)
> >  void
> >  rs6000_emit_xxspltidp_v2df (rtx dst, long value)
> >  {
> > -  printf("rs6000_emit_xxspltidp_v2df called %ld\n", value);
> > -  printf("rs6000_emit_xxspltidp_v2df called 0x%lx\n", value);
> >    if (((value & 0x7F800000) == 0) && ((value & 0x7FFFFF) != 0))
> >      inform (input_location,
> > -	    "the result for the xxspltidp instruction is undefined for
> > subnormal input values.\n");
> > +	    "the result for the xxspltidp instruction "
> > +	    "is undefined for subnormal input values");

> I see the error print statement you changed so that it would not wrap. 
> I have always been told it is best not to break the print statement
> across two lines.

It is only broken up in the source code :-)

> The argument is it makes it harder to find it in the
> code when using grep.  In this case, it should be clear what file the
> error statement is in.  What is your take in general about breaking or
> not breaking the body of an error print statement across lines?

Everyone agrees on that (I hope :-) )

The patch is okay for trunk.  Thanks!


Segher
Alan Modra Oct. 22, 2020, 11:59 p.m. UTC | #3
On Thu, Oct 22, 2020 at 09:08:58AM -0700, Carl Love wrote:
> I see the error print statement you changed so that it would not wrap. 
> I have always been told it is best not to break the print statement
> across two lines.  The argument is it makes it harder to find it in the
> code when using grep.  In this case, it should be clear what file the
> error statement is in.  What is your take in general about breaking or
> not breaking the body of an error print statement across lines?

Submit to https://gcc.gnu.org/codingconventions.html which says

"Formatting Conventions
 Line Length

 Lines shall be at most 80 columns."

Seriously, it doesn't matter what you or I think about the formatting
rules, they are in place for good reason.
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index c0adc56387f..2f0ca7af030 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -27485,11 +27485,10 @@  rs6000_const_f32_to_i32 (rtx operand)
 void
 rs6000_emit_xxspltidp_v2df (rtx dst, long value)
 {
-  printf("rs6000_emit_xxspltidp_v2df called %ld\n", value);
-  printf("rs6000_emit_xxspltidp_v2df called 0x%lx\n", value);
   if (((value & 0x7F800000) == 0) && ((value & 0x7FFFFF) != 0))
     inform (input_location,
-	    "the result for the xxspltidp instruction is undefined for subnormal input values.\n");
+	    "the result for the xxspltidp instruction "
+	    "is undefined for subnormal input values");
   emit_insn( gen_xxspltidp_v2df_inst (dst, GEN_INT (value)));
 }
 
diff --git a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
index afb0bfdef3a..e5a4935644f 100644
--- a/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
+++ b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c
@@ -5,7 +5,7 @@ 
 
 #define DEBUG 0
 
-#ifdef DEBUG
+#if DEBUG
 #include <stdio.h>
 #endif
 
@@ -100,7 +100,7 @@  main (int argc, char *argv [])
       printf(" vresult_d[%i] = %e, expected_vresult_d[%i] = %e\n",
 	     i, vresult_d[i], i, expected_vresult_d[i]);
 #else
-    abort();
+    ;
 #endif
   }