Hi Aaron,

On Fri, Jul 10, 2020 at 05:58:59PM -0500, Aaron Sawdey via Gcc-patches wrote:
> Add a test for dejagnu to determine if execution of MMA instructions is
> supported in the test environment. Add an execution test to make sure
> that __builtin_cpu_supports("mma") is true if we can execute MMA
> instructions.

> +int
> +main (int argc, char *argv[])
> +{
> +  int ret = 0;
> +#ifdef __BUILTIN_CPU_SUPPORTS__
> +  if ( !__builtin_cpu_supports ("mma"))
> +    {
> +      printf ("Error: __builtin_cpu_supports says mma not supported, but 
> ppc_mma_hw test passed.\n");
> +      ret++;
> +    }
> +#endif
> +  return ret;
> +}

This will complain about spurious test output, I think?  We usually put
#ifdef DEBUG  around such prints, but here you could just remove it, make
it a comment?  And maybe just do  exit(1);  or  abort();  or whatever.

> +             __vector_quad acc0;
> +             v4sf_t result[4];
> +             result[0][0] = 1.0;
> +             __builtin_mma_xxsetaccz (&acc0);
> +             __builtin_mma_disassemble_acc (result, &acc0);
> +             if ( result[0][0] != 0.0 )
> +             return 1;
> +             return 0;

No spaces inside ( ) please, and indent that first return statement?  We
don't care about the testcases themselves (more styles tests ever so
slightly more, even ;-) ), but this isn't a testcase :-)  Not that it is
important here, but hey.

> @@ -7865,6 +7891,7 @@ proc is-effective-target-keyword { arg } {
>         "named_sections" { return 1 }
>         "gc_sections"    { return 1 }
>         "cxa_atexit"     { return 1 }
> +       "ppc_mma_hw"     { return 1 }
>         default          { return 0 }
>       }

Hrm, do we want it to be named ppc_mma_hw?  Why not mma_hw just like
most other things?

And keep all rs6000 keywords together please (also power10_hw).

(I also wonder why rs6000 does things differently here, it's the only
arch that uses this apparently, hrm).


Okay for trunk (and backport to 10) modulo those nits.  Thanks!


Segher

Reply via email to