Re: [PATCH] Fix build with LLVM 15 or above

Hi,

On 2022-10-03 18:34:18 +1300, Thomas Munro wrote:
> One option I thought about as a stopgap measure is to use
> LLVMContextSetOpaquePointers(context, false) to turn the new code
> paths off, but it doesn't seem to work for me and I couldn't figure
> out why yet (it still aborts -- probably there are more 'contexts'
> around that I didn't handle, something like that).

I think that's just because of this hunk:

@@ -992,7 +1000,12 @@ llvm_create_types(void)
}

/* eagerly load contents, going to need it all */
+#if LLVM_VERSION_MAJOR > 14
+ if (LLVMParseBitcodeInContext2(LLVMOrcThreadSafeContextGetContext(llvm_ts_context),
+ buf, &llvm_types_module))
+#else
if (LLVMParseBitcode2(buf, &llvm_types_module))
+#endif
{
elog(ERROR, "LLVMParseBitcode2 of %s failed", path);
}

This is the wrong context to use here. Because of that we end up with types
from two different contexts being used, which leads to this assertion to fail:

#5 0x00007f945a036ab2 in __GI___assert_fail (
assertion=0x7f93cf5a4a1b "getOperand(0)->getType() == getOperand(1)->getType() && \"Both operands to ICmp instruction are not of the same type!\"",
file=0x7f93cf66062a "/home/andres/src/llvm-project/llvm/include/llvm/IR/Instructions.h", line=1191,
function=0x7f93cf5f2db6 "void llvm::ICmpInst::AssertOK()") at ./assert/assert.c:101
#6 0x00007f93cf9e3a3c in llvm::ICmpInst::AssertOK (this=0x56482c3b4b50) at /home/andres/src/llvm-project/llvm/include/llvm/IR/Instructions.h:1190
#7 0x00007f93cf9e38ca in llvm::ICmpInst::ICmpInst (this=0x56482c3b4b50, pred=llvm::CmpInst::ICMP_UGE, LHS=0x56482c3b98d0, RHS=0x56482c3b9920, NameStr="")
at /home/andres/src/llvm-project/llvm/include/llvm/IR/Instructions.h:1245
#8 0x00007f93cf9dc6f9 in llvm::IRBuilderBase::CreateICmp (this=0x56482c3b4650, P=llvm::CmpInst::ICMP_UGE, LHS=0x56482c3b98d0, RHS=0x56482c3b9920, Name="")
at /home/andres/src/llvm-project/llvm/include/llvm/IR/IRBuilder.h:2212
#9 0x00007f93cfa650cd in LLVMBuildICmp (B=0x56482c3b4650, Op=LLVMIntUGE, LHS=0x56482c3b98d0, RHS=0x56482c3b9920, Name=0x7f9459722cf2 "")
at /home/andres/src/llvm-project/llvm/lib/IR/Core.cpp:3883
#10 0x00007f945971b4d7 in llvm_compile_expr (state=0x56482c31f878) at /home/andres/src/postgresql/src/backend/jit/llvm/llvmjit_expr.c:302
#11 0x000056482a28f76b in jit_compile_expr (state=state(at)entry=0x56482c31f878) at /home/andres/src/postgresql/src/backend/jit/jit.c:177
#12 0x0000564829f44e62 in ExecReadyExpr (state=state(at)entry=0x56482c31f878) at /home/andres/src/postgresql/src/backend/executor/execExpr.c:885

because types (compared by pointer value) are only unique within a context.

I think all that is needed for this aspect would be:

#if LLVM_VERSION_MAJOR > 14
LLVMContextSetOpaquePointers(LLVMGetGlobalContext(), false);
#endif

I haven't yet run through the whole regression test with an assert enabled
llvm because an assert-enabled llvm is *SLOW*, but it got through the first
few parallel groups ok. Using an optimized llvm 15, all tests pass with
PGOPTIONS=-cjit_above_cost=0.

> That option is available for LLVM 15 but will be taken out in LLVM 16, so
> that's supposed to be the last chance to stop using pre-opaque pointers; see
> the bottom of the page I linked above for that, where they call it
> setOpaquePointers(false) (the C++ version of
> LLVMContextSetOpaquePointers()). I don't really want to go with that if we
> can avoid it, though, because it says "Opaque pointers are enabled by
> default. Typed pointers are still available, but only supported on a
> best-effort basis and may be untested" so I expect it to be blighted with
> problems.

I think it'd be ok for the back branches, while we figure out the opaque stuff
in HEAD.

Greetings,

Andres Freund