On Thu, 2021-05-27 at 21:51 -0400, Antoni Boucher wrote:
> I chose option A, so everything is a size_t, now.
> I also renamed the dyncast functions.
> Here's the new patch.
Thanks, sorry again about the delays in reviewing your work.
You didn't specify how you tested the patch; are you running the full
jit regression test suite?
This is looking close to done; a few nits inline below:
[...snip...]
> diff --git a/gcc/jit/docs/topics/functions.rst
> b/gcc/jit/docs/topics/functions.rst
> index b2d9239aa0a..1d20e3045a0 100644
> --- a/gcc/jit/docs/topics/functions.rst
> +++ b/gcc/jit/docs/topics/functions.rst
[...snip...]
> + The API entrypoints relating to getting info about parameters and return
> + types:
> +
> + * :c:func:`gcc_jit_function_get_return_type`
> +
> + * :c:func:`gcc_jit_function_get_param_count`
> +
> + were added in :ref:`LIBGCCJIT_ABI_15`; you can test for their presence
> + using
16, rather than 15.
> + The API entrypoints related to the reflection API:
> +
> + * :c:func:`gcc_jit_function_type_get_return_type`
> +
[...snip...]
> +
> + * :c:func:`gcc_jit_struct_get_field_count`
> +
> + were added in :ref:`LIBGCCJIT_ABI_15`; you can test for their presence
> + using
16, rather than 15 again.
> +
> + .. code-block:: c
> +
> + #ifdef LIBGCCJIT_HAVE_REFLECTION
> +
> + .. type:: gcc_jit_case
[...snip...]
> +gcc_jit_type *
> +gcc_jit_function_type_get_param_type (gcc_jit_function_type *function_type,
> + size_t index)
> +{
> + RETURN_NULL_IF_FAIL (function_type, NULL, NULL, "NULL function_type");
> + size_t num_params = function_type->get_param_types ().length ();
> + gcc::jit::recording::context *ctxt = function_type->m_ctxt;
> + RETURN_NULL_IF_FAIL_PRINTF3 (index < num_params,
> + ctxt, NULL,
> + "index of %ld is too large (%s has %ld params)",
> + index,
> + function_type->get_debug_string (),
> + num_params);
> + return (gcc_jit_type *)function_type->get_param_types ()[index];
> +}
> +
I'm retaining the above, since...
[...snip...]
>
> +
> +/* Public entrypoint. See description in libgccjit.h.
> +
> + After error-checking, the real work is done by the
> + gcc::jit::recording::fields::get_field method in
> + jit-recording.c. */
> +extern gcc_jit_field *
> +gcc_jit_struct_get_field (gcc_jit_struct *struct_type,
> + size_t index)
> +{
> + RETURN_NULL_IF_FAIL (struct_type, NULL, NULL, "NULL struct type");
> + RETURN_NULL_IF_FAIL (struct_type->get_fields (), NULL, NULL,
> + "NULL struct fields");
> + RETURN_NULL_IF_FAIL ((int) index < struct_type->get_fields ()->length (),
> + NULL, NULL, "NULL struct type");
...copy&paste error here; the message for this kind of failure needs
updating. Do it in a similar way to how you did
gcc_jit_function_type_get_param_type above, using the ctxt of the
struct_type.
[...snip...]
> +#define LIBGCCJIT_HAVE_REFLECTION
> +
> +/* Reflection functions to get the number of parameters, return type of
> + a function and whether a type is a bool from the C API.
> +
> + This API entrypoint was added in LIBGCCJIT_ABI_15; you can test for its
16, rather than 15, again.
> + presence using
> + #ifdef LIBGCCJIT_HAVE_REFLECTION
[...snip...]
OK for trunk with the above nits fixed, assuming that you have run the
full regression test suite (or do you need help with that?)
I can't remember, sorry, do you have push rights to the gcc git
repository?
Do you have a preference as to which patch you want me to look at next?
Otherwise I'll go through them in the order in
https://github.com/antoyo/rustc_codegen_gcc/tree/master/gcc-patches
Dave
