Horton

Re: [HACKERS] [PATCH] Generic type subscripting

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>, Pavel Stehule <pavel.stehule@gmail.com>, Alexander Korotkov <aekorotkov@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, David Steele <david@pgmasters.net>, Nikita Glukhov <n.gluhov@postgrespro.ru>, Alvaro Herrera <alvherre@2ndquadrant.com>, David Fetter <david@fetter.org>, Thomas Munro <thomas.munro@gmail.com>, Oleksandr Shulgin <oleksandr.shulgin@zalando.de>, Robert Haas <robertmhaas@gmail.com>, Michael Paquier <michael.paquier@gmail.com>, Oleg Bartunov <obartunov@gmail.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-12-07T21:21:42Z
Lists: pgsql-hackers

Attachments

Hi,

On 2020-12-07 14:08:35 -0500, Tom Lane wrote:
> 1. I'm still wondering if TypeParamBool is the right thing to pass to
> LLVMFunctionType() to describe a function-returning-bool.  It does
> seem to work on x64_64 and aarch64, for what that's worth.

> -					v_ret = build_EvalXFunc(b, mod, "ExecEvalSubscriptingRef",
> -											v_state, op);
> +					param_types[0] = l_ptr(StructExprState);
> +					param_types[1] = l_ptr(TypeSizeT);
> +					param_types[2] = l_ptr(StructExprContext);
> +
> +					v_functype = LLVMFunctionType(TypeParamBool,
> +												  param_types,
> +												  lengthof(param_types),
> +												  false);
> +					v_func = l_ptr_const(op->d.sbsref_subscript.subscriptfunc,
> +										 l_ptr(v_functype));
> +
> +					v_params[0] = v_state;
> +					v_params[1] = l_ptr_const(op, l_ptr(TypeSizeT));
> +					v_params[2] = v_econtext;
> +					v_ret = LLVMBuildCall(b,
> +										  v_func,
> +										  v_params, lengthof(v_params), "");
>  					v_ret = LLVMBuildZExt(b, v_ret, TypeStorageBool, "");

The TypeParamBool stuff here is ok. Basically LLVM uses a '1bit' integer
to represent booleans in the IR. But when it comes to storing such a
value in memory, it uses 1 byte, for obvious reasons. Hence the two
types.

We infer it like this:

> /*
>  * Clang represents stdbool.h style booleans that are returned by functions
>  * differently (as i1) than stored ones (as i8). Therefore we do not just need
>  * TypeBool (above), but also a way to determine the width of a returned
>  * integer. This allows us to keep compatible with non-stdbool using
>  * architectures.
>  */
> extern bool FunctionReturningBool(void);
> bool
> FunctionReturningBool(void)
> {
> 	return false;
> }

so you should be good.


I think it'd be a better to rely on the backend's definition of
ExecEvalBoolSubroutine etc. For the functions implementing expression
steps I've found that far easier to work with over time (because you can
get LLVM to issue type mismatch errors when the signature changes,
instead of seeing compile failures).

I've attached a prototype conversion for two other such places. Which
immediately pointed to a bug. And one harmless issue (using a pointer to
size_t instead of ExprEvalOp* to represent the 'op' parameter), which
you promptly copied...

If I pushed a slightly cleaned up version of that, it should be fairly
easy to adapt your code to it, I think?


WRT the prototype, I think it may be worth removing most of the types
from llvmjit.h. Worth keeping the most common ones, but most aren't used
all the time so terseness doesn't matter that much, and
the llvm_pg_var_type() would suffice.

Greetings,

Andres Freund

Commits

  1. Throw error when assigning jsonb scalar instead of a composite object

  2. Filling array gaps during jsonb subscripting

  3. Implementation of subscripting for jsonb

  4. Allow ALTER TYPE to update an existing type's typsubscript value.

  5. Allow subscripting of hstore values.

  6. Support subscripting of arbitrary types, not only arrays.

  7. jit: Reference function pointer types via llvmjit_types.c.

  8. Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.

  9. jit: Correct parameter type for generated expression evaluation functions.

  10. Renaming for new subscripting mechanism

  11. Fix assertion failure for SSL connections.

  12. Teach eval_const_expressions() to handle some more cases.