Re: [RS6000] Power10 vec-splati-runnable multiple test failures
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. -- Alan Modra Australia Development Lab, IBM
Re: [RS6000] Power10 vec-splati-runnable multiple test failures
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 & 0x7F80) == 0) && ((value & 0x7F) != 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
Re: [RS6000] Power10 vec-splati-runnable multiple test failures
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 & 0x7F80) == 0) && ((value & 0x7F) != 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
> #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
[RS6000] Power10 vec-splati-runnable multiple test failures
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 & 0x7F80) == 0) && ((value & 0x7F) != 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
#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 Modra
Australia Development Lab, IBM
