libgccjit: Add support for bitcasts [PR104071]
Commit Message
Hi.
This patch add support for bitcasts in libgccjit.
It passes the JIT tests, but since I added a function in tree.c, I
wonder if I should run the whole testsuite.
David, you can now disregard my question in my email about 128-bit
integers regarding my issue with initialize_sizetypes being called
multiple times because this patch fix this issue.
I turns out there was a cache of types that needed to be cleared when
you initialize the JIT.
The check for sizes is pending, because it requires the updates to
get_size I made in my patch for 128-bit integers.
Thanks for the review!
Comments
I was missing the define, so I added it.
Here's the new patch with it.
Le lundi 17 janvier 2022 à 17:18 -0500, Antoni Boucher via Jit a
écrit :
> Hi.
> This patch add support for bitcasts in libgccjit.
>
> It passes the JIT tests, but since I added a function in tree.c, I
> wonder if I should run the whole testsuite.
>
> David, you can now disregard my question in my email about 128-bit
> integers regarding my issue with initialize_sizetypes being called
> multiple times because this patch fix this issue.
> I turns out there was a cache of types that needed to be cleared when
> you initialize the JIT.
>
> The check for sizes is pending, because it requires the updates to
> get_size I made in my patch for 128-bit integers.
>
> Thanks for the review!
On Mon, 2022-01-17 at 19:30 -0500, Antoni Boucher via Gcc-patches
wrote:
> I was missing the define, so I added it.
> Here's the new patch with it.
Thanks for the patch.
> Le lundi 17 janvier 2022 à 17:18 -0500, Antoni Boucher via Jit a
> écrit :
> > Hi.
> > This patch add support for bitcasts in libgccjit.
> >
> > It passes the JIT tests, but since I added a function in tree.c, I
> > wonder if I should run the whole testsuite.
We're in stage 4 for GCC 12 now, so we need to be especially careful
and conservative about every change. A strict reading on the rules is
that we shouldn't be adding new features - but if they're confined to
libgccjit we may be able to get release manager approval.
> >
> > David, you can now disregard my question in my email about 128-bit
> > integers regarding my issue with initialize_sizetypes being called
> > multiple times because this patch fix this issue.
> > I turns out there was a cache of types that needed to be cleared
> > when
> > you initialize the JIT.
> >
> > The check for sizes is pending, because it requires the updates to
> > get_size I made in my patch for 128-bit integers.
Sorry, I seem to have mislaid that patch; do you have the "Subject"
line handy?
Do you have a list of the patches I need to review?
As for this patch, overall I like it, but there are various nits...
> >
> > Thanks for the review!
> 2022-01-17 Antoni Boucher <bouanto@zoho.com>
>
> gcc/jit/
> PR target/104071
Should be "jit" rather than "target".
Various source files are now .cc rather than .c after yesterday's big
renaming.
> * docs/topics/compatibility.rst (LIBGCCJIT_ABI_20): New ABI tag.
> * docs/topics/expressions.rst: Add documentation for the
> function gcc_jit_context_new_bitcast.
> * dummy-frontend.c: clear the cache of non-standard integer
> types to avoid having issues with some optimizations of
> bitcast where the SSA_NAME will have a size of a cached
> integer type that should have been invalidated, causing a
> comparison of integer constant to fail.
> * jit-playback.c: New function (new_bitcast).
> * jit-playback.h: New function (new_bitcast).
> * jit-recording.c: New functions (new_bitcast,
> bitcast::replay_into, bitcast::visit_children,
> bitcast::make_debug_string, bitcast::write_reproducer).
> * jit-recording.h: New calss (bitcast) and new function
> (new_bitcast, bitcast::replay_into, bitcast::visit_children,
> bitcast::make_debug_string, bitcast::write_reproducer,
> bitcast::get_precedence).
> * libgccjit.c: New function (gcc_jit_context_new_bitcast)
> * libgccjit.h: New function (gcc_jit_context_new_bitcast)
> * libgccjit.map (LIBGCCJIT_ABI_20): New ABI tag.
>
> gcc/testsuite/
> PR target/104071
> * jit.dg/all-non-failing-tests.h: Add new test-bitcast.
> * jit.dg/test-bitcast.c: New test.
>
> gcc/
> PR target/104071
> * tree.c: New function (clear_nonstandard_integer_type_cache).
> * tree.h: New function (clear_nonstandard_integer_type_cache).
> ---
> gcc/jit/docs/topics/compatibility.rst | 9 +++
> gcc/jit/docs/topics/expressions.rst | 17 +++++
> gcc/jit/dummy-frontend.c | 2 +
> gcc/jit/jit-playback.c | 13 ++++
> gcc/jit/jit-playback.h | 5 ++
> gcc/jit/jit-recording.c | 66 ++++++++++++++++++++
> gcc/jit/jit-recording.h | 32 ++++++++++
> gcc/jit/libgccjit.c | 28 +++++++++
> gcc/jit/libgccjit.h | 15 +++++
> gcc/jit/libgccjit.map | 6 ++
> gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
> gcc/testsuite/jit.dg/test-bitcast.c | 60 ++++++++++++++++++
> gcc/tree.c | 8 +++
> gcc/tree.h | 1 +
> 14 files changed, 272 insertions(+)
> create mode 100644 gcc/testsuite/jit.dg/test-bitcast.c
>
> diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
> index 16cebe31a10..b5a6b704dda 100644
> --- a/gcc/jit/docs/topics/compatibility.rst
> +++ b/gcc/jit/docs/topics/compatibility.rst
> @@ -302,3 +302,12 @@ thread-local storage model of a variable:
> section of a variable:
>
> * :func:`gcc_jit_lvalue_set_link_section`
> +
> +.. _LIBGCCJIT_ABI_20:
> +
> +``LIBGCCJIT_ABI_20``
> +-----------------------
> +``LIBGCCJIT_ABI_20`` covers the addition of an API entrypoint to bitcast a
> +value from one type to another:
> +
> + * :func:`gcc_jit_context_new_bitcast`
> diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
> index 791a20398ca..1328a53f70f 100644
> --- a/gcc/jit/docs/topics/expressions.rst
> +++ b/gcc/jit/docs/topics/expressions.rst
> @@ -649,6 +649,23 @@ Type-coercion
> * int <-> bool
> * P* <-> Q*, for pointer types P and Q
>
> +.. function:: gcc_jit_rvalue *\
> + gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,\
> + gcc_jit_location *loc,\
> + gcc_jit_rvalue *rvalue,\
> + gcc_jit_type *type)
> +
> + Given an rvalue of T, bitcast it to another type.
I think it's worth defining what "bitcast" means in the docs here;
presumably you mean something like "generating a new rvalue by
interpreting the bits of the input rvalue according to the layout of
type" or somesuch.
> +
> + The type of rvalue must be the same size as the size of ``type``.
> +
> + This entrypoint was added in :ref:`LIBGCCJIT_ABI_20`; you can test for
> + its presence using
> +
> + .. code-block:: c
> +
> + #ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast
> +
> Lvalues
> -------
>
> diff --git a/gcc/jit/dummy-frontend.c b/gcc/jit/dummy-frontend.c
> index 84ff359bfe3..c3da97642e3 100644
> --- a/gcc/jit/dummy-frontend.c
> +++ b/gcc/jit/dummy-frontend.c
> @@ -592,6 +592,8 @@ jit_langhook_init (void)
> global_dc->begin_diagnostic = jit_begin_diagnostic;
> global_dc->end_diagnostic = jit_end_diagnostic;
>
> + clear_nonstandard_integer_type_cache ();
> +
I've been putting code to cleanup global state in foo.c into
foo_c_finalize functions; I'm testing a patch right now to rename these
to foo_cc_finalize functions to reflect the renaming of .c to .cc
These cleanup functions are called from toplev::finalize.
I think it would be better to invoke this cleanup from a new
tree_cc_finalize that follows that pattern.
> build_common_tree_nodes (false);
>
> /* I don't know why this has to be done explicitly. */
[...snip...]
> diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
> index 03704ef10b8..cd8516d1c4d 100644
> --- a/gcc/jit/libgccjit.c
> +++ b/gcc/jit/libgccjit.c
> @@ -2405,6 +2405,34 @@ gcc_jit_context_new_cast (gcc_jit_context *ctxt,
> return static_cast <gcc_jit_rvalue *> (ctxt->new_cast (loc, rvalue, type));
> }
>
> +/* Public entrypoint. See description in libgccjit.h.
> +
> + After error-checking, the real work is done by the
> + gcc::jit::recording::context::new_bitcast method in jit-recording.c. */
> +
> +gcc_jit_rvalue *
> +gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,
> + gcc_jit_location *loc,
> + gcc_jit_rvalue *rvalue,
> + gcc_jit_type *type)
> +{
> + RETURN_NULL_IF_FAIL (ctxt, NULL, loc, "NULL context");
> + JIT_LOG_FUNC (ctxt->get_logger ());
> + /* LOC can be NULL. */
> + RETURN_NULL_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue");
> + RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type");
> + // TODO: check the sizes.
> + /*RETURN_NULL_IF_FAIL_PRINTF3 (
> + is_valid_cast (rvalue->get_type (), type),
> + ctxt, loc,
> + "cannot cast %s from type: %s to type: %s",
> + rvalue->get_debug_string (),
> + rvalue->get_type ()->get_debug_string (),
> + type->get_debug_string ());*/
I belive that in general we can't do the checking for equal size until
we have trees i.e. at playback time. I suspect that if the user gets
this wrong, it will lead to an internal compiler error at playback
time.
Please can you add a testcase that tests mismatching sizes e.g. trying
to interpret an int as a array of 4096 bytes, or something similar
that's wildly wrong (ideally coverage for casting in both directions).
Ideally we can generate a useful error message, or, at least, not
crash.
> +
> + return static_cast <gcc_jit_rvalue *> (ctxt->new_bitcast (loc, rvalue, type));
> +}
> +
> /* Public entrypoint. See description in libgccjit.h.
>
> After error-checking, the real work is done by the
[...snip...]
> diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> index 29afe064db6..656351edce1 100644
> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> @@ -77,6 +77,13 @@
> /* test-builtin-unreachable.c: We don't add this one, since it touches
> the optimization level of the context as a whole. */
>
> +/* test-bitcast.c */
> +#define create_code create_code_bitcast
> +#define verify_code verify_code_bitcast
> +#include "test-bitcast.c"
> +#undef create_code
> +#undef verify_code
> +
> /* test-calling-external-function.c */
> #define create_code create_code_calling_external_function
> #define verify_code verify_code_calling_external_function
> @@ -400,6 +407,9 @@ const struct testcase testcases[] = {
> {"builtin-memcpy",
> create_code_builtin_memcpy,
> verify_code_builtin_memcpy},
> + {"bitcast",
> + create_code_bitcast,
> + verify_code_bitcast},
> {"calling_external_function",
> create_code_calling_external_function,
> verify_code_calling_external_function},
> diff --git a/gcc/testsuite/jit.dg/test-bitcast.c b/gcc/testsuite/jit.dg/test-bitcast.c
> new file mode 100644
> index 00000000000..2d70622051a
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-bitcast.c
> @@ -0,0 +1,60 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include "libgccjit.h"
> +
> +#include "harness.h"
> +
> +void
> +create_code (gcc_jit_context *ctxt, void *user_data)
> +{
> + /* Let's try to inject the equivalent of:
> +int
> +my_bitcast (double x)
> +{
> + return bitcast(x, int);
> +}
> + */
> + gcc_jit_type *int_type =
> + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
> + gcc_jit_type *double_type =
> + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_FLOAT);
This says "double_type" but is GCC_JIT_TYPE_FLOAT.
I don't think we're guaranteed that sizeof(int) == sizeof(float) on all targets.
> +
> + gcc_jit_param *x =
> + gcc_jit_context_new_param (
> + ctxt,
> + NULL,
> + double_type, "x");
> + gcc_jit_param *params[1] = {x};
> + gcc_jit_function *func =
> + gcc_jit_context_new_function (ctxt,
> + NULL,
> + GCC_JIT_FUNCTION_EXPORTED,
> + int_type,
> + "my_bitcast",
> + 1, params, 0);
> +
> + gcc_jit_block *initial =
> + gcc_jit_function_new_block (func, "initial");
> +
> + gcc_jit_block_end_with_return(initial, NULL,
> + gcc_jit_context_new_bitcast(ctxt,
> + NULL,
> + gcc_jit_param_as_rvalue(x),
> + int_type
> + ));
> +}
> +
> +void
> +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> +{
> + typedef int (*my_bitcast_fn_type) (double);
> + CHECK_NON_NULL (result);
> + my_bitcast_fn_type my_bitcast =
> + (my_bitcast_fn_type)gcc_jit_result_get_code (result,
"my_bitcast");
> + CHECK_NON_NULL (my_bitcast);
> + int val = my_bitcast (-5.1298714);
> + note ("my_bitcast returned: %d", val);
> + CHECK_VALUE (val, 35569201);
> +}
> diff --git a/gcc/tree.c b/gcc/tree.c
> index d98b77db50b..e4bf4e84675 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -6963,6 +6963,14 @@ build_reference_type (tree to_type)
> (HOST_BITS_PER_WIDE_INT > 64 ? HOST_BITS_PER_WIDE_INT : 64)
> static GTY(()) tree nonstandard_integer_type_cache[2 *
MAX_INT_CACHED_PREC + 2];
>
> +void clear_nonstandard_integer_type_cache (void)
> +{
> + for (size_t i = 0 ; i < 2 * MAX_INT_CACHED_PREC + 2 ; i++)
> + {
> + nonstandard_integer_type_cache[i] = NULL;
> + }
> +}
> +
As noted above (for gcc/jit/dummy-frontend.c), make this static, and
call it from a new tree_cc_finalize function, and call that from
toplev::finalize.
> /* Builds a signed or unsigned integer type of precision PRECISION.
> Used for C bitfields whose precision does not match that of
> built-in target types. */
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 318019c4dc5..640b492802c 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -5385,6 +5385,7 @@ extern bool real_minus_onep (const_tree);
> extern void init_ttree (void);
> extern void build_common_tree_nodes (bool);
> extern void build_common_builtin_nodes (void);
> +extern void clear_nonstandard_integer_type_cache (void);
...and get rid of this in favor of a tree_cc_finalize.
> extern tree build_nonstandard_integer_type (unsigned HOST_WIDE_INT,
int);
> extern tree build_nonstandard_boolean_type (unsigned HOST_WIDE_INT);
> extern tree build_range_type (tree, tree, tree);
> --
> 2.26.2.7.g19db9cfb68.dirty
>
Hope this is constructive; thanks again for the patch
Dave
Hi.
Here's the updated patch.
See comments below.
Le mardi 18 janvier 2022 à 18:06 -0500, David Malcolm a écrit :
> On Mon, 2022-01-17 at 19:30 -0500, Antoni Boucher via Gcc-patches
> wrote:
> > I was missing the define, so I added it.
> > Here's the new patch with it.
>
> Thanks for the patch.
>
> > Le lundi 17 janvier 2022 à 17:18 -0500, Antoni Boucher via Jit a
> > écrit :
> > > Hi.
> > > This patch add support for bitcasts in libgccjit.
> > >
> > > It passes the JIT tests, but since I added a function in tree.c,
> > > I
> > > wonder if I should run the whole testsuite.
>
> We're in stage 4 for GCC 12 now, so we need to be especially careful
> and conservative about every change. A strict reading on the rules
> is
> that we shouldn't be adding new features - but if they're confined to
> libgccjit we may be able to get release manager approval.
Ok, if the 4 patches currently being reviewed (and listed here:
https://github.com/antoyo/libgccjit-patches) were included in gcc 12,
I'd be able to build rustc_codegen_gcc with an unpatched gcc.
It is to be noted however, that I'll need more patches for future work.
Off the top of my head, I'll at least need a patch for the inline
attribute, try/catch and target-specific builtins.
The last 2 features will probably take some time to implement, so I'll
let you judge if you think it's worth merging the 4 patches currently
being reviewed for gcc 12.
>
> > >
> > > David, you can now disregard my question in my email about 128-
> > > bit
> > > integers regarding my issue with initialize_sizetypes being
> > > called
> > > multiple times because this patch fix this issue.
> > > I turns out there was a cache of types that needed to be cleared
> > > when
> > > you initialize the JIT.
> > >
> > > The check for sizes is pending, because it requires the updates
> > > to
> > > get_size I made in my patch for 128-bit integers.
>
> Sorry, I seem to have mislaid that patch; do you have the "Subject"
> line handy?
I recently sent an email with that patch updated, but here's the
subject line:
[PATCH] libgccjit: Add support for sized integer types, including 128-
bit integers [PR95325]
>
> Do you have a list of the patches I need to review?
Yes, on this repo:
https://github.com/antoyo/libgccjit-patches
They are outdated but I can update them if you want.
>
> As for this patch, overall I like it, but there are various nits...
>
> > >
> > > Thanks for the review!
>
> > 2022-01-17 Antoni Boucher <bouanto@zoho.com>
> >
> > gcc/jit/
> > PR target/104071
>
> Should be "jit" rather than "target".
>
> Various source files are now .cc rather than .c after yesterday's big
> renaming.
>
> > * docs/topics/compatibility.rst (LIBGCCJIT_ABI_20): New ABI
> > tag.
> > * docs/topics/expressions.rst: Add documentation for the
> > function gcc_jit_context_new_bitcast.
> > * dummy-frontend.c: clear the cache of non-standard integer
> > types to avoid having issues with some optimizations of
> > bitcast where the SSA_NAME will have a size of a cached
> > integer type that should have been invalidated, causing a
> > comparison of integer constant to fail.
> > * jit-playback.c: New function (new_bitcast).
> > * jit-playback.h: New function (new_bitcast).
> > * jit-recording.c: New functions (new_bitcast,
> > bitcast::replay_into, bitcast::visit_children,
> > bitcast::make_debug_string, bitcast::write_reproducer).
> > * jit-recording.h: New calss (bitcast) and new function
> > (new_bitcast, bitcast::replay_into,
> > bitcast::visit_children,
> > bitcast::make_debug_string, bitcast::write_reproducer,
> > bitcast::get_precedence).
> > * libgccjit.c: New function (gcc_jit_context_new_bitcast)
> > * libgccjit.h: New function (gcc_jit_context_new_bitcast)
> > * libgccjit.map (LIBGCCJIT_ABI_20): New ABI tag.
> >
> > gcc/testsuite/
> > PR target/104071
> > * jit.dg/all-non-failing-tests.h: Add new test-bitcast.
> > * jit.dg/test-bitcast.c: New test.
> >
> > gcc/
> > PR target/104071
> > * tree.c: New function
> > (clear_nonstandard_integer_type_cache).
> > * tree.h: New function
> > (clear_nonstandard_integer_type_cache).
> > ---
> > gcc/jit/docs/topics/compatibility.rst | 9 +++
> > gcc/jit/docs/topics/expressions.rst | 17 +++++
> > gcc/jit/dummy-frontend.c | 2 +
> > gcc/jit/jit-playback.c | 13 ++++
> > gcc/jit/jit-playback.h | 5 ++
> > gcc/jit/jit-recording.c | 66
> > ++++++++++++++++++++
> > gcc/jit/jit-recording.h | 32 ++++++++++
> > gcc/jit/libgccjit.c | 28 +++++++++
> > gcc/jit/libgccjit.h | 15 +++++
> > gcc/jit/libgccjit.map | 6 ++
> > gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
> > gcc/testsuite/jit.dg/test-bitcast.c | 60
> > ++++++++++++++++++
> > gcc/tree.c | 8 +++
> > gcc/tree.h | 1 +
> > 14 files changed, 272 insertions(+)
> > create mode 100644 gcc/testsuite/jit.dg/test-bitcast.c
> >
> > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > b/gcc/jit/docs/topics/compatibility.rst
> > index 16cebe31a10..b5a6b704dda 100644
> > --- a/gcc/jit/docs/topics/compatibility.rst
> > +++ b/gcc/jit/docs/topics/compatibility.rst
> > @@ -302,3 +302,12 @@ thread-local storage model of a variable:
> > section of a variable:
> >
> > * :func:`gcc_jit_lvalue_set_link_section`
> > +
> > +.. _LIBGCCJIT_ABI_20:
> > +
> > +``LIBGCCJIT_ABI_20``
> > +-----------------------
> > +``LIBGCCJIT_ABI_20`` covers the addition of an API entrypoint to
> > bitcast a
> > +value from one type to another:
> > +
> > + * :func:`gcc_jit_context_new_bitcast`
> > diff --git a/gcc/jit/docs/topics/expressions.rst
> > b/gcc/jit/docs/topics/expressions.rst
> > index 791a20398ca..1328a53f70f 100644
> > --- a/gcc/jit/docs/topics/expressions.rst
> > +++ b/gcc/jit/docs/topics/expressions.rst
> > @@ -649,6 +649,23 @@ Type-coercion
> > * int <-> bool
> > * P* <-> Q*, for pointer types P and Q
> >
> > +.. function:: gcc_jit_rvalue *\
> > + gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,\
> > + gcc_jit_location *loc,\
> > + gcc_jit_rvalue
> > *rvalue,\
> > + gcc_jit_type *type)
> > +
> > + Given an rvalue of T, bitcast it to another type.
>
> I think it's worth defining what "bitcast" means in the docs here;
> presumably you mean something like "generating a new rvalue by
> interpreting the bits of the input rvalue according to the layout of
> type" or somesuch.
Done.
>
>
> > +
> > + The type of rvalue must be the same size as the size of
> > ``type``.
> > +
> > + This entrypoint was added in :ref:`LIBGCCJIT_ABI_20`; you can
> > test for
> > + its presence using
> > +
> > + .. code-block:: c
> > +
> > + #ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast
> > +
> > Lvalues
> > -------
> >
> > diff --git a/gcc/jit/dummy-frontend.c b/gcc/jit/dummy-frontend.c
> > index 84ff359bfe3..c3da97642e3 100644
> > --- a/gcc/jit/dummy-frontend.c
> > +++ b/gcc/jit/dummy-frontend.c
> > @@ -592,6 +592,8 @@ jit_langhook_init (void)
> > global_dc->begin_diagnostic = jit_begin_diagnostic;
> > global_dc->end_diagnostic = jit_end_diagnostic;
> >
> > + clear_nonstandard_integer_type_cache ();
> > +
>
> I've been putting code to cleanup global state in foo.c into
> foo_c_finalize functions; I'm testing a patch right now to rename
> these
> to foo_cc_finalize functions to reflect the renaming of .c to .cc
> These cleanup functions are called from toplev::finalize.
>
> I think it would be better to invoke this cleanup from a new
> tree_cc_finalize that follows that pattern.
Done.
>
> > build_common_tree_nodes (false);
> >
> > /* I don't know why this has to be done explicitly. */
>
> [...snip...]
>
> > diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
> > index 03704ef10b8..cd8516d1c4d 100644
> > --- a/gcc/jit/libgccjit.c
> > +++ b/gcc/jit/libgccjit.c
> > @@ -2405,6 +2405,34 @@ gcc_jit_context_new_cast (gcc_jit_context
> > *ctxt,
> > return static_cast <gcc_jit_rvalue *> (ctxt->new_cast (loc,
> > rvalue, type));
> > }
> >
> > +/* Public entrypoint. See description in libgccjit.h.
> > +
> > + After error-checking, the real work is done by the
> > + gcc::jit::recording::context::new_bitcast method in jit-
> > recording.c. */
> > +
> > +gcc_jit_rvalue *
> > +gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,
> > + gcc_jit_location *loc,
> > + gcc_jit_rvalue *rvalue,
> > + gcc_jit_type *type)
> > +{
> > + RETURN_NULL_IF_FAIL (ctxt, NULL, loc, "NULL context");
> > + JIT_LOG_FUNC (ctxt->get_logger ());
> > + /* LOC can be NULL. */
> > + RETURN_NULL_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue");
> > + RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type");
> > + // TODO: check the sizes.
> > + /*RETURN_NULL_IF_FAIL_PRINTF3 (
> > + is_valid_cast (rvalue->get_type (), type),
> > + ctxt, loc,
> > + "cannot cast %s from type: %s to type: %s",
> > + rvalue->get_debug_string (),
> > + rvalue->get_type ()->get_debug_string (),
> > + type->get_debug_string ());*/
>
> I belive that in general we can't do the checking for equal size
> until
> we have trees i.e. at playback time. I suspect that if the user gets
> this wrong, it will lead to an internal compiler error at playback
> time.
Done.
>
> Please can you add a testcase that tests mismatching sizes e.g.
> trying
> to interpret an int as a array of 4096 bytes, or something similar
> that's wildly wrong (ideally coverage for casting in both
> directions).
Done.
>
> Ideally we can generate a useful error message, or, at least, not
> crash.
>
> > +
> > + return static_cast <gcc_jit_rvalue *> (ctxt->new_bitcast (loc,
> > rvalue, type));
> > +}
> > +
> > /* Public entrypoint. See description in libgccjit.h.
> >
> > After error-checking, the real work is done by the
>
> [...snip...]
>
> > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > index 29afe064db6..656351edce1 100644
> > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > @@ -77,6 +77,13 @@
> > /* test-builtin-unreachable.c: We don't add this one, since it
> > touches
> > the optimization level of the context as a whole. */
> >
> > +/* test-bitcast.c */
> > +#define create_code create_code_bitcast
> > +#define verify_code verify_code_bitcast
> > +#include "test-bitcast.c"
> > +#undef create_code
> > +#undef verify_code
> > +
> > /* test-calling-external-function.c */
> > #define create_code create_code_calling_external_function
> > #define verify_code verify_code_calling_external_function
> > @@ -400,6 +407,9 @@ const struct testcase testcases[] = {
> > {"builtin-memcpy",
> > create_code_builtin_memcpy,
> > verify_code_builtin_memcpy},
> > + {"bitcast",
> > + create_code_bitcast,
> > + verify_code_bitcast},
> > {"calling_external_function",
> > create_code_calling_external_function,
> > verify_code_calling_external_function},
> > diff --git a/gcc/testsuite/jit.dg/test-bitcast.c
> > b/gcc/testsuite/jit.dg/test-bitcast.c
> > new file mode 100644
> > index 00000000000..2d70622051a
> > --- /dev/null
> > +++ b/gcc/testsuite/jit.dg/test-bitcast.c
> > @@ -0,0 +1,60 @@
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +
> > +#include "libgccjit.h"
> > +
> > +#include "harness.h"
> > +
> > +void
> > +create_code (gcc_jit_context *ctxt, void *user_data)
> > +{
> > + /* Let's try to inject the equivalent of:
> > +int
> > +my_bitcast (double x)
> > +{
> > + return bitcast(x, int);
> > +}
> > + */
> > + gcc_jit_type *int_type =
> > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
> > + gcc_jit_type *double_type =
> > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_FLOAT);
>
> This says "double_type" but is GCC_JIT_TYPE_FLOAT.
>
> I don't think we're guaranteed that sizeof(int) == sizeof(float) on
> all targets.
I switched to using `gcc_jit_context_get_int_type (ctxt, 4, 1)` for the
integer type.
>
> > +
> > + gcc_jit_param *x =
> > + gcc_jit_context_new_param (
> > + ctxt,
> > + NULL,
> > + double_type, "x");
> > + gcc_jit_param *params[1] = {x};
> > + gcc_jit_function *func =
> > + gcc_jit_context_new_function (ctxt,
> > + NULL,
> > + GCC_JIT_FUNCTION_EXPORTED,
> > + int_type,
> > + "my_bitcast",
> > + 1, params, 0);
> > +
> > + gcc_jit_block *initial =
> > + gcc_jit_function_new_block (func, "initial");
> > +
> > + gcc_jit_block_end_with_return(initial, NULL,
> > + gcc_jit_context_new_bitcast(ctxt,
> > + NULL,
> > + gcc_jit_param_as_rvalue(x),
> > + int_type
> > + ));
> > +}
> > +
> > +void
> > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> > +{
> > + typedef int (*my_bitcast_fn_type) (double);
> > + CHECK_NON_NULL (result);
> > + my_bitcast_fn_type my_bitcast =
> > + (my_bitcast_fn_type)gcc_jit_result_get_code (result,
> "my_bitcast");
> > + CHECK_NON_NULL (my_bitcast);
> > + int val = my_bitcast (-5.1298714);
> > + note ("my_bitcast returned: %d", val);
> > + CHECK_VALUE (val, 35569201);
> > +}
> > diff --git a/gcc/tree.c b/gcc/tree.c
> > index d98b77db50b..e4bf4e84675 100644
> > --- a/gcc/tree.c
> > +++ b/gcc/tree.c
> > @@ -6963,6 +6963,14 @@ build_reference_type (tree to_type)
> > (HOST_BITS_PER_WIDE_INT > 64 ? HOST_BITS_PER_WIDE_INT : 64)
> > static GTY(()) tree nonstandard_integer_type_cache[2 *
> MAX_INT_CACHED_PREC + 2];
> >
> > +void clear_nonstandard_integer_type_cache (void)
> > +{
> > + for (size_t i = 0 ; i < 2 * MAX_INT_CACHED_PREC + 2 ; i++)
> > + {
> > + nonstandard_integer_type_cache[i] = NULL;
> > + }
> > +}
> > +
>
> As noted above (for gcc/jit/dummy-frontend.c), make this static, and
> call it from a new tree_cc_finalize function, and call that from
> toplev::finalize.
>
> > /* Builds a signed or unsigned integer type of precision
> > PRECISION.
> > Used for C bitfields whose precision does not match that of
> > built-in target types. */
> > diff --git a/gcc/tree.h b/gcc/tree.h
> > index 318019c4dc5..640b492802c 100644
> > --- a/gcc/tree.h
> > +++ b/gcc/tree.h
> > @@ -5385,6 +5385,7 @@ extern bool real_minus_onep (const_tree);
> > extern void init_ttree (void);
> > extern void build_common_tree_nodes (bool);
> > extern void build_common_builtin_nodes (void);
> > +extern void clear_nonstandard_integer_type_cache (void);
>
> ...and get rid of this in favor of a tree_cc_finalize.
>
> > extern tree build_nonstandard_integer_type (unsigned
> > HOST_WIDE_INT,
> int);
> > extern tree build_nonstandard_boolean_type (unsigned
> > HOST_WIDE_INT);
> > extern tree build_range_type (tree, tree, tree);
> > --
> > 2.26.2.7.g19db9cfb68.dirty
> >
>
> Hope this is constructive; thanks again for the patch
> Dave
>
On Fri, 2022-01-21 at 18:41 -0500, Antoni Boucher wrote:
> Hi.
> Here's the updated patch.
>
Thanks. Review below:
[...snip...]
> diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
> index 4c352e8c93d..6bf1e1ceee0 100644
> --- a/gcc/jit/libgccjit.cc
> +++ b/gcc/jit/libgccjit.cc
> @@ -2405,6 +2405,34 @@ gcc_jit_context_new_cast (gcc_jit_context *ctxt,
> return static_cast <gcc_jit_rvalue *> (ctxt->new_cast (loc, rvalue, type));
> }
>
> +/* Public entrypoint. See description in libgccjit.h.
> +
> + After error-checking, the real work is done by the
> + gcc::jit::recording::context::new_bitcast method in jit-recording.c. */
> +
> +gcc_jit_rvalue *
> +gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,
> + gcc_jit_location *loc,
> + gcc_jit_rvalue *rvalue,
> + gcc_jit_type *type)
> +{
> + RETURN_NULL_IF_FAIL (ctxt, NULL, loc, "NULL context");
> + JIT_LOG_FUNC (ctxt->get_logger ());
> + /* LOC can be NULL. */
> + RETURN_NULL_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue");
> + RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type");
> + // TODO: check the sizes.
> + /*RETURN_NULL_IF_FAIL_PRINTF3 (
> + is_valid_cast (rvalue->get_type (), type),
> + ctxt, loc,
> + "cannot cast %s from type: %s to type: %s",
> + rvalue->get_debug_string (),
> + rvalue->get_type ()->get_debug_string (),
> + type->get_debug_string ());*/
I think we agreed that we can't check the sizes at this point, so this
commented-out check would be better replaced with a comment explaining
that we have to defer the check to playback time, when we have the
trees.
> +
> + return static_cast <gcc_jit_rvalue *> (ctxt->new_bitcast (loc, rvalue, type));
> +}
> +
> /* Public entrypoint. See description in libgccjit.h.
>
> After error-checking, the real work is done by the
[...snip...]
> diff --git a/gcc/testsuite/jit.dg/test-bitcast.c b/gcc/testsuite/jit.dg/test-bitcast.c
> new file mode 100644
> index 00000000000..a092fa117e6
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-bitcast.c
> @@ -0,0 +1,60 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include "libgccjit.h"
> +
> +#include "harness.h"
> +
> +void
> +create_code (gcc_jit_context *ctxt, void *user_data)
> +{
> + /* Let's try to inject the equivalent of:
> +int
> +my_bitcast (double x)
> +{
> + return bitcast(x, int);
> +}
> + */
> + gcc_jit_type *int_type =
> + gcc_jit_context_get_int_type (ctxt, 4, 1);
> + gcc_jit_type *float_type =
> + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_FLOAT);
This uses GCC_JIT_TYPE_FLOAT for the param...
> +
> + gcc_jit_param *x =
> + gcc_jit_context_new_param (
> + ctxt,
> + NULL,
> + float_type, "x");
> + gcc_jit_param *params[1] = {x};
> + gcc_jit_function *func =
> + gcc_jit_context_new_function (ctxt,
> + NULL,
> + GCC_JIT_FUNCTION_EXPORTED,
> + int_type,
> + "my_bitcast",
> + 1, params, 0);
[..snip...]
> +
> +void
> +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> +{
> + typedef int (*my_bitcast_fn_type) (double);
...but this uses "double". Presumably these should agree, and have the
same sizeof as the integer type.
> + CHECK_NON_NULL (result);
> + my_bitcast_fn_type my_bitcast =
> + (my_bitcast_fn_type)gcc_jit_result_get_code (result, "my_bitcast");
> + CHECK_NON_NULL (my_bitcast);
> + int val = my_bitcast (-5.1298714);
> + note ("my_bitcast returned: %d", val);
> + CHECK_VALUE (val, 35569201);
Out of curiosity, is there any particular significance for these
values? FWIW I rather like:
http://evanw.github.io/float-toy/
for directly manipulating the bits of floating point numbers.
[...snip...]
> diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> index 534da1462e8..bc4921974eb 100644
> --- a/gcc/toplev.cc
> +++ b/gcc/toplev.cc
> @@ -2368,6 +2368,7 @@ toplev::finalize (void)
> gcse_c_finalize ();
> ipa_cp_c_finalize ();
> ira_costs_c_finalize ();
> + tree_cc_finalize ();
>
> /* save_decoded_options uses opts_obstack, so these must
> be cleaned up together. */
> diff --git a/gcc/tree.cc b/gcc/tree.cc
> index ae159ee20ce..fe9d9083026 100644
> --- a/gcc/tree.cc
> +++ b/gcc/tree.cc
> @@ -6963,6 +6963,15 @@ build_reference_type (tree to_type)
> (HOST_BITS_PER_WIDE_INT > 64 ? HOST_BITS_PER_WIDE_INT : 64)
> static GTY(()) tree nonstandard_integer_type_cache[2 * MAX_INT_CACHED_PREC + 2];
>
> +static void
> +clear_nonstandard_integer_type_cache (void)
> +{
> + for (size_t i = 0 ; i < 2 * MAX_INT_CACHED_PREC + 2 ; i++)
> + {
> + nonstandard_integer_type_cache[i] = NULL;
> + }
> +}
> +
> /* Builds a signed or unsigned integer type of precision PRECISION.
> Used for C bitfields whose precision does not match that of
> built-in target types. */
> @@ -14565,6 +14574,12 @@ get_attr_nonstring_decl (tree expr, tree *ref)
> return NULL_TREE;
> }
>
> +void
> +tree_cc_finalize (void)
> +{
> + clear_nonstandard_integer_type_cache ();
> +}
> +
> #if CHECKING_P
>
> namespace selftest {
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 30bc53c2996..bf886fc2472 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -5385,6 +5385,7 @@ extern bool real_minus_onep (const_tree);
> extern void init_ttree (void);
> extern void build_common_tree_nodes (bool);
> extern void build_common_builtin_nodes (void);
> +extern void tree_cc_finalize (void);
> extern tree build_nonstandard_integer_type (unsigned HOST_WIDE_INT, int);
> extern tree build_nonstandard_boolean_type (unsigned HOST_WIDE_INT);
> extern tree build_range_type (tree, tree, tree);
Looks OK to me, but am not officially a maintainer of these parts.
LGTM with those nits fixed - for next stage 1, or for trunk now if the
release maintainers are OK with it.
Thanks again for the patch, and sorry about the belated review.
Dave
Here's the updated patch.
On Fri, 2022-04-08 at 15:22 -0400, David Malcolm wrote:
> On Fri, 2022-01-21 at 18:41 -0500, Antoni Boucher wrote:
> > Hi.
> > Here's the updated patch.
> >
>
> Thanks. Review below:
>
> [...snip...]
>
> > diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
> > index 4c352e8c93d..6bf1e1ceee0 100644
> > --- a/gcc/jit/libgccjit.cc
> > +++ b/gcc/jit/libgccjit.cc
> > @@ -2405,6 +2405,34 @@ gcc_jit_context_new_cast (gcc_jit_context
> > *ctxt,
> > return static_cast <gcc_jit_rvalue *> (ctxt->new_cast (loc,
> > rvalue, type));
> > }
> >
> > +/* Public entrypoint. See description in libgccjit.h.
> > +
> > + After error-checking, the real work is done by the
> > + gcc::jit::recording::context::new_bitcast method in jit-
> > recording.c. */
> > +
> > +gcc_jit_rvalue *
> > +gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,
> > + gcc_jit_location *loc,
> > + gcc_jit_rvalue *rvalue,
> > + gcc_jit_type *type)
> > +{
> > + RETURN_NULL_IF_FAIL (ctxt, NULL, loc, "NULL context");
> > + JIT_LOG_FUNC (ctxt->get_logger ());
> > + /* LOC can be NULL. */
> > + RETURN_NULL_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue");
> > + RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type");
> > + // TODO: check the sizes.
> > + /*RETURN_NULL_IF_FAIL_PRINTF3 (
> > + is_valid_cast (rvalue->get_type (), type),
> > + ctxt, loc,
> > + "cannot cast %s from type: %s to type: %s",
> > + rvalue->get_debug_string (),
> > + rvalue->get_type ()->get_debug_string (),
> > + type->get_debug_string ());*/
>
> I think we agreed that we can't check the sizes at this point, so
> this
> commented-out check would be better replaced with a comment
> explaining
> that we have to defer the check to playback time, when we have the
> trees.
>
> > +
> > + return static_cast <gcc_jit_rvalue *> (ctxt->new_bitcast (loc,
> > rvalue, type));
> > +}
> > +
> > /* Public entrypoint. See description in libgccjit.h.
> >
> > After error-checking, the real work is done by the
>
> [...snip...]
>
> > diff --git a/gcc/testsuite/jit.dg/test-bitcast.c
> > b/gcc/testsuite/jit.dg/test-bitcast.c
> > new file mode 100644
> > index 00000000000..a092fa117e6
> > --- /dev/null
> > +++ b/gcc/testsuite/jit.dg/test-bitcast.c
> > @@ -0,0 +1,60 @@
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +
> > +#include "libgccjit.h"
> > +
> > +#include "harness.h"
> > +
> > +void
> > +create_code (gcc_jit_context *ctxt, void *user_data)
> > +{
> > + /* Let's try to inject the equivalent of:
> > +int
> > +my_bitcast (double x)
> > +{
> > + return bitcast(x, int);
> > +}
> > + */
> > + gcc_jit_type *int_type =
> > + gcc_jit_context_get_int_type (ctxt, 4, 1);
> > + gcc_jit_type *float_type =
> > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_FLOAT);
>
> This uses GCC_JIT_TYPE_FLOAT for the param...
>
> > +
> > + gcc_jit_param *x =
> > + gcc_jit_context_new_param (
> > + ctxt,
> > + NULL,
> > + float_type, "x");
> > + gcc_jit_param *params[1] = {x};
> > + gcc_jit_function *func =
> > + gcc_jit_context_new_function (ctxt,
> > + NULL,
> > + GCC_JIT_FUNCTION_EXPORTED,
> > + int_type,
> > + "my_bitcast",
> > + 1, params, 0);
>
> [..snip...]
>
> > +
> > +void
> > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> > +{
> > + typedef int (*my_bitcast_fn_type) (double);
>
> ...but this uses "double". Presumably these should agree, and have
> the
> same sizeof as the integer type.
>
> > + CHECK_NON_NULL (result);
> > + my_bitcast_fn_type my_bitcast =
> > + (my_bitcast_fn_type)gcc_jit_result_get_code (result,
> > "my_bitcast");
> > + CHECK_NON_NULL (my_bitcast);
> > + int val = my_bitcast (-5.1298714);
> > + note ("my_bitcast returned: %d", val);
> > + CHECK_VALUE (val, 35569201);
>
> Out of curiosity, is there any particular significance for these
> values? FWIW I rather like:
> http://evanw.github.io/float-toy/
> for directly manipulating the bits of floating point numbers.
The given float values, when bitcast to an int, gives the given int
value.
>
>
> [...snip...]
>
> > diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> > index 534da1462e8..bc4921974eb 100644
> > --- a/gcc/toplev.cc
> > +++ b/gcc/toplev.cc
> > @@ -2368,6 +2368,7 @@ toplev::finalize (void)
> > gcse_c_finalize ();
> > ipa_cp_c_finalize ();
> > ira_costs_c_finalize ();
> > + tree_cc_finalize ();
> >
> > /* save_decoded_options uses opts_obstack, so these must
> > be cleaned up together. */
> > diff --git a/gcc/tree.cc b/gcc/tree.cc
> > index ae159ee20ce..fe9d9083026 100644
> > --- a/gcc/tree.cc
> > +++ b/gcc/tree.cc
> > @@ -6963,6 +6963,15 @@ build_reference_type (tree to_type)
> > (HOST_BITS_PER_WIDE_INT > 64 ? HOST_BITS_PER_WIDE_INT : 64)
> > static GTY(()) tree nonstandard_integer_type_cache[2 *
> > MAX_INT_CACHED_PREC + 2];
> >
> > +static void
> > +clear_nonstandard_integer_type_cache (void)
> > +{
> > + for (size_t i = 0 ; i < 2 * MAX_INT_CACHED_PREC + 2 ; i++)
> > + {
> > + nonstandard_integer_type_cache[i] = NULL;
> > + }
> > +}
> > +
> > /* Builds a signed or unsigned integer type of precision
> > PRECISION.
> > Used for C bitfields whose precision does not match that of
> > built-in target types. */
> > @@ -14565,6 +14574,12 @@ get_attr_nonstring_decl (tree expr, tree
> > *ref)
> > return NULL_TREE;
> > }
> >
> > +void
> > +tree_cc_finalize (void)
> > +{
> > + clear_nonstandard_integer_type_cache ();
> > +}
> > +
> > #if CHECKING_P
> >
> > namespace selftest {
> > diff --git a/gcc/tree.h b/gcc/tree.h
> > index 30bc53c2996..bf886fc2472 100644
> > --- a/gcc/tree.h
> > +++ b/gcc/tree.h
> > @@ -5385,6 +5385,7 @@ extern bool real_minus_onep (const_tree);
> > extern void init_ttree (void);
> > extern void build_common_tree_nodes (bool);
> > extern void build_common_builtin_nodes (void);
> > +extern void tree_cc_finalize (void);
> > extern tree build_nonstandard_integer_type (unsigned
> > HOST_WIDE_INT, int);
> > extern tree build_nonstandard_boolean_type (unsigned
> > HOST_WIDE_INT);
> > extern tree build_range_type (tree, tree, tree);
>
> Looks OK to me, but am not officially a maintainer of these parts.
>
> LGTM with those nits fixed - for next stage 1, or for trunk now if
> the
> release maintainers are OK with it.
>
> Thanks again for the patch, and sorry about the belated review.
>
> Dave
>
>
>
On Sat, 2022-04-09 at 14:05 -0400, Antoni Boucher wrote:
> Here's the updated patch.
Thanks.
I updated the patch somewhat:
* fixed up some hunks that didn't quite apply
* whitespace fixes
* added a missing comment
* regenerated .texinfo from .rst
* test-bitcast.c failed for me; the bitcast of -5.1298714 float to int
didn't give me the expected 35569201; I got this value:
(gdb) p val
$1 = -1062983704
(gdb) p /x val
$2 = 0xc0a427e8
(gdb) p /x 35569201
$3 = 0x21ebe31
and I couldn't figure out what the relationship between these two
values could be.
The expected: 35569201 == 0x21ebe31
0x021ebe31 as float is 1 × 2^-123 × 1.2401792 = 1.1662589e-37
I rewrote the test to explicitly use int32_t rather than int, but this
didn't help.
I wondered if this was an endianness issue, but:
-5.1298713 is -1 × 2^2 × 1.2824678, which is 0xc0a427e8
In the end, I changed the constants in the test to these:
(gdb) p *(float *)&val
$25 = 3.14159274
(gdb) p *(int32_t *)&val
$26 = 1078530011
(gdb) p /x *(int32_t *)&val
$27 = 0x40490fdb
which passes for me.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
I've pushed the patch to trunk for GCC 12 as r12-8117-g30f7c83e9cfe7c
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=30f7c83e9cfe7c015448d72f63c4c39d14bc6de6
Dave
>
> On Fri, 2022-04-08 at 15:22 -0400, David Malcolm wrote:
> > On Fri, 2022-01-21 at 18:41 -0500, Antoni Boucher wrote:
> > > Hi.
> > > Here's the updated patch.
> > >
> >
> > Thanks. Review below:
> >
> > [...snip...]
> >
> > > diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
> > > index 4c352e8c93d..6bf1e1ceee0 100644
> > > --- a/gcc/jit/libgccjit.cc
> > > +++ b/gcc/jit/libgccjit.cc
> > > @@ -2405,6 +2405,34 @@ gcc_jit_context_new_cast (gcc_jit_context
> > > *ctxt,
> > > return static_cast <gcc_jit_rvalue *> (ctxt->new_cast (loc,
> > > rvalue, type));
> > > }
> > >
> > > +/* Public entrypoint. See description in libgccjit.h.
> > > +
> > > + After error-checking, the real work is done by the
> > > + gcc::jit::recording::context::new_bitcast method in jit-
> > > recording.c. */
> > > +
> > > +gcc_jit_rvalue *
> > > +gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,
> > > + gcc_jit_location *loc,
> > > + gcc_jit_rvalue *rvalue,
> > > + gcc_jit_type *type)
> > > +{
> > > + RETURN_NULL_IF_FAIL (ctxt, NULL, loc, "NULL context");
> > > + JIT_LOG_FUNC (ctxt->get_logger ());
> > > + /* LOC can be NULL. */
> > > + RETURN_NULL_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue");
> > > + RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type");
> > > + // TODO: check the sizes.
> > > + /*RETURN_NULL_IF_FAIL_PRINTF3 (
> > > + is_valid_cast (rvalue->get_type (), type),
> > > + ctxt, loc,
> > > + "cannot cast %s from type: %s to type: %s",
> > > + rvalue->get_debug_string (),
> > > + rvalue->get_type ()->get_debug_string (),
> > > + type->get_debug_string ());*/
> >
> > I think we agreed that we can't check the sizes at this point, so
> > this
> > commented-out check would be better replaced with a comment
> > explaining
> > that we have to defer the check to playback time, when we have the
> > trees.
> >
> > > +
> > > + return static_cast <gcc_jit_rvalue *> (ctxt->new_bitcast (loc,
> > > rvalue, type));
> > > +}
> > > +
> > > /* Public entrypoint. See description in libgccjit.h.
> > >
> > > After error-checking, the real work is done by the
> >
> > [...snip...]
> >
> > > diff --git a/gcc/testsuite/jit.dg/test-bitcast.c
> > > b/gcc/testsuite/jit.dg/test-bitcast.c
> > > new file mode 100644
> > > index 00000000000..a092fa117e6
> > > --- /dev/null
> > > +++ b/gcc/testsuite/jit.dg/test-bitcast.c
> > > @@ -0,0 +1,60 @@
> > > +#include <stdlib.h>
> > > +#include <stdio.h>
> > > +#include <string.h>
> > > +
> > > +#include "libgccjit.h"
> > > +
> > > +#include "harness.h"
> > > +
> > > +void
> > > +create_code (gcc_jit_context *ctxt, void *user_data)
> > > +{
> > > + /* Let's try to inject the equivalent of:
> > > +int
> > > +my_bitcast (double x)
> > > +{
> > > + return bitcast(x, int);
> > > +}
> > > + */
> > > + gcc_jit_type *int_type =
> > > + gcc_jit_context_get_int_type (ctxt, 4, 1);
> > > + gcc_jit_type *float_type =
> > > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_FLOAT);
> >
> > This uses GCC_JIT_TYPE_FLOAT for the param...
> >
> > > +
> > > + gcc_jit_param *x =
> > > + gcc_jit_context_new_param (
> > > + ctxt,
> > > + NULL,
> > > + float_type, "x");
> > > + gcc_jit_param *params[1] = {x};
> > > + gcc_jit_function *func =
> > > + gcc_jit_context_new_function (ctxt,
> > > + NULL,
> > > + GCC_JIT_FUNCTION_EXPORTED,
> > > + int_type,
> > > + "my_bitcast",
> > > + 1, params, 0);
> >
> > [..snip...]
> >
> > > +
> > > +void
> > > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> > > +{
> > > + typedef int (*my_bitcast_fn_type) (double);
> >
> > ...but this uses "double". Presumably these should agree, and have
> > the
> > same sizeof as the integer type.
> >
> > > + CHECK_NON_NULL (result);
> > > + my_bitcast_fn_type my_bitcast =
> > > + (my_bitcast_fn_type)gcc_jit_result_get_code (result,
> > > "my_bitcast");
> > > + CHECK_NON_NULL (my_bitcast);
> > > + int val = my_bitcast (-5.1298714);
> > > + note ("my_bitcast returned: %d", val);
> > > + CHECK_VALUE (val, 35569201);
> >
> > Out of curiosity, is there any particular significance for these
> > values? FWIW I rather like:
> > http://evanw.github.io/float-toy/
> > for directly manipulating the bits of floating point numbers.
>
> The given float values, when bitcast to an int, gives the given int
> value.
>
> >
> >
> > [...snip...]
> >
> > > diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> > > index 534da1462e8..bc4921974eb 100644
> > > --- a/gcc/toplev.cc
> > > +++ b/gcc/toplev.cc
> > > @@ -2368,6 +2368,7 @@ toplev::finalize (void)
> > > gcse_c_finalize ();
> > > ipa_cp_c_finalize ();
> > > ira_costs_c_finalize ();
> > > + tree_cc_finalize ();
> > >
> > > /* save_decoded_options uses opts_obstack, so these must
> > > be cleaned up together. */
> > > diff --git a/gcc/tree.cc b/gcc/tree.cc
> > > index ae159ee20ce..fe9d9083026 100644
> > > --- a/gcc/tree.cc
> > > +++ b/gcc/tree.cc
> > > @@ -6963,6 +6963,15 @@ build_reference_type (tree to_type)
> > > (HOST_BITS_PER_WIDE_INT > 64 ? HOST_BITS_PER_WIDE_INT : 64)
> > > static GTY(()) tree nonstandard_integer_type_cache[2 *
> > > MAX_INT_CACHED_PREC + 2];
> > >
> > > +static void
> > > +clear_nonstandard_integer_type_cache (void)
> > > +{
> > > + for (size_t i = 0 ; i < 2 * MAX_INT_CACHED_PREC + 2 ; i++)
> > > + {
> > > + nonstandard_integer_type_cache[i] = NULL;
> > > + }
> > > +}
> > > +
> > > /* Builds a signed or unsigned integer type of precision
> > > PRECISION.
> > > Used for C bitfields whose precision does not match that of
> > > built-in target types. */
> > > @@ -14565,6 +14574,12 @@ get_attr_nonstring_decl (tree expr, tree
> > > *ref)
> > > return NULL_TREE;
> > > }
> > >
> > > +void
> > > +tree_cc_finalize (void)
> > > +{
> > > + clear_nonstandard_integer_type_cache ();
> > > +}
> > > +
> > > #if CHECKING_P
> > >
> > > namespace selftest {
> > > diff --git a/gcc/tree.h b/gcc/tree.h
> > > index 30bc53c2996..bf886fc2472 100644
> > > --- a/gcc/tree.h
> > > +++ b/gcc/tree.h
> > > @@ -5385,6 +5385,7 @@ extern bool real_minus_onep (const_tree);
> > > extern void init_ttree (void);
> > > extern void build_common_tree_nodes (bool);
> > > extern void build_common_builtin_nodes (void);
> > > +extern void tree_cc_finalize (void);
> > > extern tree build_nonstandard_integer_type (unsigned
> > > HOST_WIDE_INT, int);
> > > extern tree build_nonstandard_boolean_type (unsigned
> > > HOST_WIDE_INT);
> > > extern tree build_range_type (tree, tree, tree);
> >
> > Looks OK to me, but am not officially a maintainer of these parts.
> >
> > LGTM with those nits fixed - for next stage 1, or for trunk now if
> > the
> > release maintainers are OK with it.
> >
> > Thanks again for the patch, and sorry about the belated review.
> >
> > Dave
> >
> >
> >
>
From 35e113b51c416af0c13bc7eb160ff6c0cbcff813 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Wed, 9 Jun 2021 18:29:14 -0400
Subject: [PATCH] libgccjit: Add support for bitcasts [PR104071]
2022-01-17 Antoni Boucher <bouanto@zoho.com>
gcc/jit/
PR target/104071
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_20): New ABI tag.
* docs/topics/expressions.rst: Add documentation for the
function gcc_jit_context_new_bitcast.
* dummy-frontend.c: clear the cache of non-standard integer
types to avoid having issues with some optimizations of
bitcast where the SSA_NAME will have a size of a cached
integer type that should have been invalidated, causing a
comparison of integer constant to fail.
* jit-playback.c: New function (new_bitcast).
* jit-playback.h: New function (new_bitcast).
* jit-recording.c: New functions (new_bitcast,
bitcast::replay_into, bitcast::visit_children,
bitcast::make_debug_string, bitcast::write_reproducer).
* jit-recording.h: New calss (bitcast) and new function
(new_bitcast, bitcast::replay_into, bitcast::visit_children,
bitcast::make_debug_string, bitcast::write_reproducer,
bitcast::get_precedence).
* libgccjit.c: New function (gcc_jit_context_new_bitcast)
* libgccjit.h: New function (gcc_jit_context_new_bitcast)
* libgccjit.map (LIBGCCJIT_ABI_20): New ABI tag.
gcc/testsuite/
PR target/104071
* jit.dg/all-non-failing-tests.h: Add new test-bitcast.
* jit.dg/test-bitcast.c: New test.
gcc/
PR target/104071
* tree.c: New function (clear_nonstandard_integer_type_cache).
* tree.h: New function (clear_nonstandard_integer_type_cache).
---
gcc/jit/docs/topics/compatibility.rst | 9 +++
gcc/jit/docs/topics/expressions.rst | 10 +++
gcc/jit/dummy-frontend.c | 2 +
gcc/jit/jit-playback.c | 13 ++++
gcc/jit/jit-playback.h | 5 ++
gcc/jit/jit-recording.c | 66 ++++++++++++++++++++
gcc/jit/jit-recording.h | 32 ++++++++++
gcc/jit/libgccjit.c | 28 +++++++++
gcc/jit/libgccjit.h | 9 +++
gcc/jit/libgccjit.map | 6 ++
gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
gcc/testsuite/jit.dg/test-bitcast.c | 60 ++++++++++++++++++
gcc/tree.c | 8 +++
gcc/tree.h | 1 +
14 files changed, 259 insertions(+)
create mode 100644 gcc/testsuite/jit.dg/test-bitcast.c
@@ -302,3 +302,12 @@ thread-local storage model of a variable:
section of a variable:
* :func:`gcc_jit_lvalue_set_link_section`
+
+.. _LIBGCCJIT_ABI_20:
+
+``LIBGCCJIT_ABI_20``
+-----------------------
+``LIBGCCJIT_ABI_20`` covers the addition of an API entrypoint to bitcast a
+value from one type to another:
+
+ * :func:`gcc_jit_context_new_bitcast`
@@ -649,6 +649,16 @@ Type-coercion
* int <-> bool
* P* <-> Q*, for pointer types P and Q
+.. function:: gcc_jit_rvalue *\
+ gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,\
+ gcc_jit_location *loc,\
+ gcc_jit_rvalue *rvalue,\
+ gcc_jit_type *type)
+
+ Given an rvalue of T, bitcast it to another type.
+
+ The type of rvalue must be the same size as the size of ``type``.
+
Lvalues
-------
@@ -592,6 +592,8 @@ jit_langhook_init (void)
global_dc->begin_diagnostic = jit_begin_diagnostic;
global_dc->end_diagnostic = jit_end_diagnostic;
+ clear_nonstandard_integer_type_cache ();
+
build_common_tree_nodes (false);
/* I don't know why this has to be done explicitly. */
@@ -1390,6 +1390,19 @@ new_cast (playback::location *loc,
return new rvalue (this, t_cast);
}
+playback::rvalue *
+playback::context::
+new_bitcast (location *loc,
+ rvalue *expr,
+ type *type_)
+{
+ tree t_bitcast = build1 (VIEW_CONVERT_EXPR,
+ type_->as_tree (), expr->as_tree ());
+ if (loc)
+ set_tree_location (t_bitcast, loc);
+ return new rvalue (this, t_bitcast);
+}
+
/* Construct a playback::lvalue instance (wrapping a tree) for an
array access. */
@@ -180,6 +180,11 @@ public:
rvalue *expr,
type *type_);
+ rvalue *
+ new_bitcast (location *loc,
+ rvalue *expr,
+ type *type_);
+
lvalue *
new_array_access (location *loc,
rvalue *ptr,
@@ -1242,6 +1242,22 @@ recording::context::new_cast (recording::location *loc,
return result;
}
+/* Create a recording::bitcast instance and add it to this context's list
+ of mementos.
+
+ Implements the post-error-checking part of
+ gcc_jit_context_new_bitcast. */
+
+recording::rvalue *
+recording::context::new_bitcast (location *loc,
+ rvalue *expr,
+ type *type_)
+{
+ recording::rvalue *result = new bitcast (this, loc, expr, type_);
+ record (result);
+ return result;
+}
+
/* Create a recording::call instance and add it to this context's list
of mementos.
@@ -5740,6 +5756,56 @@ recording::cast::write_reproducer (reproducer &r)
r.get_identifier_as_type (get_type ()));
}
+/* Implementation of pure virtual hook recording::memento::replay_into
+ for recording::bitcast. */
+
+void
+recording::bitcast::replay_into (replayer *r)
+{
+ set_playback_obj (r->new_bitcast (playback_location (r, m_loc),
+ m_rvalue->playback_rvalue (),
+ get_type ()->playback_type ()));
+}
+
+/* Implementation of pure virtual hook recording::rvalue::visit_children
+ for recording::bitcast. */
+void
+recording::bitcast::visit_children (rvalue_visitor *v)
+{
+ v->visit (m_rvalue);
+}
+
+/* Implementation of recording::memento::make_debug_string for
+ casts. */
+
+recording::string *
+recording::bitcast::make_debug_string ()
+{
+ enum precedence prec = get_precedence ();
+ return string::from_printf (m_ctxt,
+ "bitcast(%s, %s)",
+ m_rvalue->get_debug_string_parens (prec),
+ get_type ()->get_debug_string ());
+}
+
+/* Implementation of recording::memento::write_reproducer for casts. */
+
+void
+recording::bitcast::write_reproducer (reproducer &r)
+{
+ const char *id = r.make_identifier (this, "rvalue");
+ r.write (" gcc_jit_rvalue *%s =\n"
+ " gcc_jit_context_new_bitcast (%s,\n"
+ " %s, /* gcc_jit_location *loc */\n"
+ " %s, /* gcc_jit_rvalue *rvalue */\n"
+ " %s); /* gcc_jit_type *type */\n",
+ id,
+ r.get_identifier (get_context ()),
+ r.get_identifier (m_loc),
+ r.get_identifier_as_rvalue (m_rvalue),
+ r.get_identifier_as_type (get_type ()));
+}
+
/* The implementation of class gcc::jit::recording::base_call. */
/* The constructor for gcc::jit::recording::base_call. */
@@ -205,6 +205,11 @@ public:
rvalue *expr,
type *type_);
+ rvalue *
+ new_bitcast (location *loc,
+ rvalue *expr,
+ type *type_);
+
lvalue *
new_array_access (location *loc,
rvalue *ptr,
@@ -1691,6 +1696,33 @@ private:
rvalue *m_rvalue;
};
+class bitcast : public rvalue
+{
+public:
+ bitcast (context *ctxt,
+ location *loc,
+ rvalue *a,
+ type *type_)
+ : rvalue (ctxt, loc, type_),
+ m_rvalue (a)
+ {}
+
+ void replay_into (replayer *r) FINAL OVERRIDE;
+
+ void visit_children (rvalue_visitor *v) FINAL OVERRIDE;
+
+private:
+ string * make_debug_string () FINAL OVERRIDE;
+ void write_reproducer (reproducer &r) FINAL OVERRIDE;
+ enum precedence get_precedence () const FINAL OVERRIDE
+ {
+ return PRECEDENCE_CAST;
+ }
+
+private:
+ rvalue *m_rvalue;
+};
+
class base_call : public rvalue
{
public:
@@ -2405,6 +2405,34 @@ gcc_jit_context_new_cast (gcc_jit_context *ctxt,
return static_cast <gcc_jit_rvalue *> (ctxt->new_cast (loc, rvalue, type));
}
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, the real work is done by the
+ gcc::jit::recording::context::new_bitcast method in jit-recording.c. */
+
+gcc_jit_rvalue *
+gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,
+ gcc_jit_location *loc,
+ gcc_jit_rvalue *rvalue,
+ gcc_jit_type *type)
+{
+ RETURN_NULL_IF_FAIL (ctxt, NULL, loc, "NULL context");
+ JIT_LOG_FUNC (ctxt->get_logger ());
+ /* LOC can be NULL. */
+ RETURN_NULL_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue");
+ RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type");
+ // TODO: check the sizes.
+ /*RETURN_NULL_IF_FAIL_PRINTF3 (
+ is_valid_cast (rvalue->get_type (), type),
+ ctxt, loc,
+ "cannot cast %s from type: %s to type: %s",
+ rvalue->get_debug_string (),
+ rvalue->get_type ()->get_debug_string (),
+ type->get_debug_string ());*/
+
+ return static_cast <gcc_jit_rvalue *> (ctxt->new_bitcast (loc, rvalue, type));
+}
+
/* Public entrypoint. See description in libgccjit.h.
After error-checking, the real work is done by the
@@ -1206,6 +1206,15 @@ gcc_jit_context_new_cast (gcc_jit_context *ctxt,
gcc_jit_rvalue *rvalue,
gcc_jit_type *type);
+/* Reinterpret a value as another type.
+
+ The types must be of the same size. */
+extern gcc_jit_rvalue *
+gcc_jit_context_new_bitcast (gcc_jit_context *ctxt,
+ gcc_jit_location *loc,
+ gcc_jit_rvalue *rvalue,
+ gcc_jit_type *type);
+
extern gcc_jit_lvalue *
gcc_jit_context_new_array_access (gcc_jit_context *ctxt,
gcc_jit_location *loc,
@@ -238,8 +238,14 @@ LIBGCCJIT_ABI_18 {
} LIBGCCJIT_ABI_17;
LIBGCCJIT_ABI_19 {
+ global:
gcc_jit_context_new_array_constructor;
gcc_jit_context_new_struct_constructor;
gcc_jit_context_new_union_constructor;
gcc_jit_global_set_initializer_rvalue;
} LIBGCCJIT_ABI_18;
+
+LIBGCCJIT_ABI_20 {
+ global:
+ gcc_jit_context_new_bitcast;
+} LIBGCCJIT_ABI_19;
@@ -77,6 +77,13 @@
/* test-builtin-unreachable.c: We don't add this one, since it touches
the optimization level of the context as a whole. */
+/* test-bitcast.c */
+#define create_code create_code_bitcast
+#define verify_code verify_code_bitcast
+#include "test-bitcast.c"
+#undef create_code
+#undef verify_code
+
/* test-calling-external-function.c */
#define create_code create_code_calling_external_function
#define verify_code verify_code_calling_external_function
@@ -400,6 +407,9 @@ const struct testcase testcases[] = {
{"builtin-memcpy",
create_code_builtin_memcpy,
verify_code_builtin_memcpy},
+ {"bitcast",
+ create_code_bitcast,
+ verify_code_bitcast},
{"calling_external_function",
create_code_calling_external_function,
verify_code_calling_external_function},
new file mode 100644
@@ -0,0 +1,60 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ /* Let's try to inject the equivalent of:
+int
+my_bitcast (double x)
+{
+ return bitcast(x, int);
+}
+ */
+ gcc_jit_type *int_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+ gcc_jit_type *double_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_FLOAT);
+
+ gcc_jit_param *x =
+ gcc_jit_context_new_param (
+ ctxt,
+ NULL,
+ double_type, "x");
+ gcc_jit_param *params[1] = {x};
+ gcc_jit_function *func =
+ gcc_jit_context_new_function (ctxt,
+ NULL,
+ GCC_JIT_FUNCTION_EXPORTED,
+ int_type,
+ "my_bitcast",
+ 1, params, 0);
+
+ gcc_jit_block *initial =
+ gcc_jit_function_new_block (func, "initial");
+
+ gcc_jit_block_end_with_return(initial, NULL,
+ gcc_jit_context_new_bitcast(ctxt,
+ NULL,
+ gcc_jit_param_as_rvalue(x),
+ int_type
+ ));
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ typedef int (*my_bitcast_fn_type) (double);
+ CHECK_NON_NULL (result);
+ my_bitcast_fn_type my_bitcast =
+ (my_bitcast_fn_type)gcc_jit_result_get_code (result, "my_bitcast");
+ CHECK_NON_NULL (my_bitcast);
+ int val = my_bitcast (-5.1298714);
+ note ("my_bitcast returned: %d", val);
+ CHECK_VALUE (val, 35569201);
+}
@@ -6963,6 +6963,14 @@ build_reference_type (tree to_type)
(HOST_BITS_PER_WIDE_INT > 64 ? HOST_BITS_PER_WIDE_INT : 64)
static GTY(()) tree nonstandard_integer_type_cache[2 * MAX_INT_CACHED_PREC + 2];
+void clear_nonstandard_integer_type_cache (void)
+{
+ for (size_t i = 0 ; i < 2 * MAX_INT_CACHED_PREC + 2 ; i++)
+ {
+ nonstandard_integer_type_cache[i] = NULL;
+ }
+}
+
/* Builds a signed or unsigned integer type of precision PRECISION.
Used for C bitfields whose precision does not match that of
built-in target types. */
@@ -5385,6 +5385,7 @@ extern bool real_minus_onep (const_tree);
extern void init_ttree (void);
extern void build_common_tree_nodes (bool);
extern void build_common_builtin_nodes (void);
+extern void clear_nonstandard_integer_type_cache (void);
extern tree build_nonstandard_integer_type (unsigned HOST_WIDE_INT, int);
extern tree build_nonstandard_boolean_type (unsigned HOST_WIDE_INT);
extern tree build_range_type (tree, tree, tree);
--
2.26.2.7.g19db9cfb68.dirty