On Fri, 2023-11-17 at 17:36 -0500, Antoni Boucher wrote: > Hi. > This patch adds a vector permutation and vector access operations > (bug > 112602). > > This was split from this patch: > https://gcc.gnu.org/pipermail/jit/2023q1/001606.html >
Thanks for the patch. Overall, looks good, but 3 minor nitpicks: [...snip...] > diff --git a/gcc/jit/docs/topics/compatibility.rst > b/gcc/jit/docs/topics/compatibility.rst > index ebede440ee4..a764e3968d1 100644 > --- a/gcc/jit/docs/topics/compatibility.rst > +++ b/gcc/jit/docs/topics/compatibility.rst > @@ -378,3 +378,13 @@ alignment of a variable: > -------------------- > ``LIBGCCJIT_ABI_25`` covers the addition of > :func:`gcc_jit_type_get_restrict` > + > + > +.. _LIBGCCJIT_ABI_26: > + > +``LIBGCCJIT_ABI_26`` > +-------------------- > +``LIBGCCJIT_ABI_26`` covers the addition of functions to manipulate vectors: > + > + * :func:`gcc_jit_context_new_rvalue_vector_perm` > + * :func:`gcc_jit_context_new_vector_access` > diff --git a/gcc/jit/docs/topics/expressions.rst > b/gcc/jit/docs/topics/expressions.rst > index 42cfee36302..4a45aa13f5c 100644 > --- a/gcc/jit/docs/topics/expressions.rst > +++ b/gcc/jit/docs/topics/expressions.rst > @@ -295,6 +295,35 @@ Vector expressions > > #ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_rvalue_from_vector > > +.. function:: gcc_jit_rvalue * \ > + gcc_jit_context_new_rvalue_vector_perm (gcc_jit_context *ctxt, > \ > + gcc_jit_location *loc, > \ > + gcc_jit_rvalue > *elements1, \ > + gcc_jit_rvalue > *elements2, \ > + gcc_jit_rvalue *mask); > + > + Build a permutation of two vectors. > + > + "elements1" and "elements2" should have the same type. > + The length of "mask" and "elements1" should be the same. > + The element type of "mask" should be integral. > + The size of the element type of "mask" and "elements1" should be the same. > + > + This entrypoint was added in :ref:`LIBGCCJIT_ABI_25`; you can test for ^^ Should be 26 [...snip...] > Unary Operations > **************** > > @@ -1020,3 +1049,27 @@ Field access is provided separately for both lvalues > and rvalues. > PTR[INDEX] > > in C (or, indeed, to ``PTR + INDEX``). > + > +.. function:: gcc_jit_lvalue *\ > + gcc_jit_context_new_vector_access (gcc_jit_context *ctxt,\ > + gcc_jit_location *loc,\ > + gcc_jit_rvalue *vector,\ > + gcc_jit_rvalue *index) > + > + Given an rvalue of vector type ``T __attribute__ ((__vector_size__ > (SIZE)))``, get the element `T` at > + the given index. > + > + This entrypoint was added in :ref:`LIBGCCJIT_ABI_25`; you can test for ^^ Likewise here. [...snip...] > @@ -4071,6 +4107,79 @@ gcc_jit_context_new_rvalue_from_vector > (gcc_jit_context *ctxt, > (gcc::jit::recording::rvalue **)elements); > } > > +/* Public entrypoint. See description in libgccjit.h. > + > + After error-checking, the real work is done by the > + gcc::jit::recording::context::new_rvalue_vector_perm method, in > + jit-recording.cc. */ > + > +gcc_jit_rvalue * > +gcc_jit_context_new_rvalue_vector_perm (gcc_jit_context *ctxt, > + gcc_jit_location *loc, > + gcc_jit_rvalue *elements1, > + gcc_jit_rvalue *elements2, > + gcc_jit_rvalue *mask) > +{ > + RETURN_NULL_IF_FAIL (ctxt, NULL, loc, "NULL ctxt"); > + JIT_LOG_FUNC (ctxt->get_logger ()); > + > + /* LOC can be NULL. */ ...but "elements1", "elements2", and "mask" must not be NULL, as they're dereferenced below. So this is going to need something like the following (untested): RETURN_NULL_IF_FAIL (elements1, ctxt, loc, "NULL elements1"); RETURN_NULL_IF_FAIL (elements2, ctxt, loc, "NULL elements2"); RETURN_NULL_IF_FAIL (mask, ctxt, loc, "NULL mask"); [...snip...] OK with those fixed. Thanks Dave