Re: [HACKERS] [PATCH] Generic type subscripting

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, David Steele <david(at)pgmasters(dot)net>, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Fetter <david(at)fetter(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Oleksandr Shulgin <oleksandr(dot)shulgin(at)zalando(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Oleg Bartunov <obartunov(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] [PATCH] Generic type subscripting
Date: 2020-12-07 21:21:42
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


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

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.


Andres Freund

Attachment Content-Type Size
0001-jit-wip-reference-function-types-instead-of-re-creat.patch text/x-diff 9.3 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-12-07 21:32:32 Re: [HACKERS] [PATCH] Generic type subscripting
Previous Message Anastasia Lubennikova 2020-12-07 21:16:11 Commitfest statistics