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@entry=0x56482c31f878)
at /home/andres/src/postgresql/src/backend/jit/jit.c:177
#12 0x0000564829f44e62 in ExecReadyExpr (state=state@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
>From d2a383fe07f962e3a1e17a7f8f112a868cbd8175 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Thu, 22 Sep 2022 23:38:56 +1200
Subject: [PATCH v1] WIP: jit: LLVM 15: Minimal changes.
Per https://llvm.org/docs/OpaquePointers.html, support for non-opaque
pointers still exists and we can request that on our context. We have
until LLVM 16 to move to opaque pointers.
---
src/backend/jit/llvm/llvmjit.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index fd3eecf27d3..bccfcfa9698 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -798,6 +798,16 @@ llvm_session_initialize(void)
LLVMInitializeNativeAsmPrinter();
LLVMInitializeNativeAsmParser();
+ /*
+ * When targetting an llvm version with opaque pointers enabled by
+ * default, turn them off for the context we build our code in. Don't need
+ * to do so for other contexts (e.g. llvm_ts_context) - once the IR is
+ * generated, it carries the necessary information.
+ */
+#if LLVM_VERSION_MAJOR > 14
+ LLVMContextSetOpaquePointers(LLVMGetGlobalContext(), false);
+#endif
+
/*
* Synchronize types early, as that also includes inferring the target
* triple.
@@ -1112,7 +1122,11 @@ llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx,
LLVMOrcJITDylibRef JD, LLVMOrcJITDylibLookupFlags JDLookupFlags,
LLVMOrcCLookupSet LookupSet, size_t LookupSetSize)
{
+#if LLVM_VERSION_MAJOR > 14
+ LLVMOrcCSymbolMapPairs symbols = palloc0(sizeof(LLVMOrcCSymbolMapPair) * LookupSetSize);
+#else
LLVMOrcCSymbolMapPairs symbols = palloc0(sizeof(LLVMJITCSymbolMapPair) * LookupSetSize);
+#endif
LLVMErrorRef error;
LLVMOrcMaterializationUnitRef mu;
@@ -1230,7 +1244,11 @@ llvm_create_jit_instance(LLVMTargetMachineRef tm)
* Symbol resolution support for "special" functions, e.g. a call into an
* SQL callable function.
*/
+#if LLVM_VERSION_MAJOR > 14
+ ref_gen = LLVMOrcCreateCustomCAPIDefinitionGenerator(llvm_resolve_symbols, NULL, NULL);
+#else
ref_gen = LLVMOrcCreateCustomCAPIDefinitionGenerator(llvm_resolve_symbols, NULL);
+#endif
LLVMOrcJITDylibAddGenerator(LLVMOrcLLJITGetMainJITDylib(lljit), ref_gen);
return lljit;
--
2.37.3.542.gdd3f6c4cae
