Re: [PATCH] libgccjit: Add support for the type bfloat16

[David Malcolm <dmalcolm@redhat.com>]

On Thu, 2023-11-16 at 17:20 -0500, Antoni Boucher wrote:
> I forgot to attach the patch.
> 
> On Thu, 2023-11-16 at 17:19 -0500, Antoni Boucher wrote:
> > Hi.
> > This patch adds the support for the type bfloat16 (bug 112574).
> > 
> > This was asked to be splitted from a another patch sent here:
> > https://gcc.gnu.org/pipermail/jit/2023q1/001607.html
> > 
> > Thanks for the review.
> 

Thanks for the patch.

> diff --git a/gcc/jit/jit-playback.cc b/gcc/jit/jit-playback.cc
> index 18cc4da25b8..7e1c97a4638 100644
> --- a/gcc/jit/jit-playback.cc
> +++ b/gcc/jit/jit-playback.cc
> @@ -280,6 +280,8 @@ get_tree_node_for_type (enum gcc_jit_types type_)
>  
>      case GCC_JIT_TYPE_FLOAT:
>        return float_type_node;
> +    case GCC_JIT_TYPE_BFLOAT16:
> +      return bfloat16_type_node;

The code to create bfloat16_type_node (in build_common_tree_nodes) is
guarded by #ifdef HAVE_BFmode, so we should probably have a test for
this in case GCC_JIT_TYPE_BFLOAT16 to at least add an error message
when it's NULL_TREE, rather than silently returning NULL_TREE and
crashing.

[...]

> diff --git a/gcc/testsuite/jit.dg/test-bfloat16.c b/gcc/testsuite/jit.dg/test-bfloat16.c
> new file mode 100644
> index 00000000000..6aed3920351
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-bfloat16.c
> @@ -0,0 +1,37 @@
> +/* { dg-do compile { target x86_64-*-* } } */
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "libgccjit.h"
> +
> +/* We don't want set_options() in harness.h to set -O3 so our little local
> +   is optimized away. */
> +#define TEST_ESCHEWS_SET_OPTIONS
> +static void set_options (gcc_jit_context *ctxt, const char *argv0)
> +{
> +}


Please add a comment to all-non-failing-tests.h noting the exclusion of
this test case from the array.

[...]

> diff --git a/gcc/testsuite/jit.dg/test-types.c b/gcc/testsuite/jit.dg/test-types.c
> index a01944e35fa..9e7c4f3e046 100644
> --- a/gcc/testsuite/jit.dg/test-types.c
> +++ b/gcc/testsuite/jit.dg/test-types.c
> @@ -1,3 +1,4 @@
> +#include <immintrin.h>
>  #include <stdint.h>
>  #include <stdlib.h>
>  #include <stdio.h>
> @@ -492,4 +493,5 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
>  
>    CHECK_VALUE (gcc_jit_type_get_size (gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_FLOAT)), sizeof (float));
>    CHECK_VALUE (gcc_jit_type_get_size (gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_DOUBLE)), sizeof (double));
> +  CHECK_VALUE (gcc_jit_type_get_size (gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_BFLOAT16)), sizeof (__bfloat16));


This is only going to work on targets which #ifdef HAVE_BFmode, so this
CHECK_VALUE needs to be conditionalized somehow, to avoid having this,
test-combination, and test-threads from bailing out early on targets
without BFmode.

Dave