On Sat, Apr 6, 2024 at 5:01 AM Dmitry Dolgov <9erthali...@gmail.com> wrote: > > > Yep, I think this is it. I've spent some hours trying to understand why > > > suddenly deform function has noundef ret attribute, when it shouldn't -- > > > this explains it and the proposed change fixes the crash. One thing that > > > is still not clear to me though is why this copied attribute doesn't > > > show up in the bitcode dumped right before running inline pass (I've > > > added this to troubleshoot the issue). > > > > One thing to consider in this context is indeed adding "verify" pass as > > suggested in the PR, at least for the debugging configuration. Without the > > fix > > it immediately returns: > > > > Running analysis: VerifierAnalysis on deform_0_1 > > Attribute 'noundef' applied to incompatible type! > > > > llvm error: Broken function found, compilation aborted! > > Here is what I have in mind. Interestingly enough, it also shows few > more errors besides "noundef": > > Intrinsic name not mangled correctly for type arguments! Should be: > llvm.lifetime.end.p0 > ptr @llvm.lifetime.end.p0i8 > > It refers to the function from create_LifetimeEnd, not sure how > important is this.
Would it be too slow to run the verify pass always, in assertion builds? Here's a patch for the original issue, and a patch to try that idea + a fix for that other complaint it spits out. The latter would only run for LLVM 17+, but that seems OK.
From 57af42d9a1b47b7361c7200a17a210f2ca37bd5d Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Tue, 9 Apr 2024 18:10:30 +1200 Subject: [PATCH 1/2] Fix illegal attribute LLVM attribute propagation. Commit 72559438 started copying more attributes from AttributeTemplate to the functions we generate on the fly. In the case of deform functions, which return void, this meant that "noundef", from AttributeTemplate's return value (a Datum) was copied to a void type. Older LLVM releases were OK with that, but LLVM 18 crashes. "noundef" is rejected for void by the verify pass, which would have alerted us to this problem (something we'll consider checking automatically in a later commit). Update our llvm_copy_attributes() function to skip copying the attribute for the return value, if the target function returns void. Thanks to Dmitry Dolgov for help chasing this down. Back-patch to all supported releases, like 72559438. Reported-by: Pavel Stehule <pavel.steh...@gmail.com> Reviewed-by: Dmitry Dolgov <9erthali...@gmail.com> Discussion: https://postgr.es/m/CAFj8pRACpVFr7LMdVYENUkScG5FCYMZDDdSGNU-tch%2Bw98OxYg%40mail.gmail.com --- src/backend/jit/llvm/llvmjit.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index ec0fdd49324..92b4993a98a 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -552,8 +552,11 @@ llvm_copy_attributes(LLVMValueRef v_from, LLVMValueRef v_to) /* copy function attributes */ llvm_copy_attributes_at_index(v_from, v_to, LLVMAttributeFunctionIndex); - /* and the return value attributes */ - llvm_copy_attributes_at_index(v_from, v_to, LLVMAttributeReturnIndex); + if (LLVMGetTypeKind(LLVMGetFunctionReturnType(v_to)) != LLVMVoidTypeKind) + { + /* and the return value attributes */ + llvm_copy_attributes_at_index(v_from, v_to, LLVMAttributeReturnIndex); + } /* and each function parameter's attribute */ param_count = LLVMCountParams(v_from); -- 2.44.0
From 91d8b747686aa07341337e1cd0b7c2244e955adb Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Tue, 9 Apr 2024 18:57:48 +1200 Subject: [PATCH 2/2] Run LLVM verify pass on all generated IR. We fixed a recent problem that crashed LLVM 18, but Dmitry pointed out that we'd have known about that all along if we'd automatically run the verify pass on the IR we generate. Turn that on in assertion builds. That reveals one other complaint about a name, which is also fixed here. Suggested-by: Dmitry Dolgov <9erthali...@gmail.com> Discussion: https://postgr.es/m/CAFj8pRACpVFr7LMdVYENUkScG5FCYMZDDdSGNU-tch%2Bw98OxYg%40mail.gmail.com --- src/backend/jit/llvm/llvmjit.c | 11 +++++++++-- src/backend/jit/llvm/llvmjit_expr.c | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 92b4993a98a..bc429e970f2 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -703,10 +703,17 @@ llvm_optimize_module(LLVMJitContext *context, LLVMModuleRef module) LLVMErrorRef err; const char *passes; + /* In assertion builds, run the LLVM verify pass. */ +#ifdef USE_ASSERT_CHECKING +#define PASSES_PREFIX "verify," +#else +#define PASSES_PREFIX "" +#endif + if (context->base.flags & PGJIT_OPT3) - passes = "default<O3>"; + passes = PASSES_PREFIX "default<O3>"; else - passes = "default<O0>,mem2reg"; + passes = PASSES_PREFIX "default<O0>,mem2reg"; options = LLVMCreatePassBuilderOptions(); diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index 9e0efd26687..cd956fa7e38 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -2765,7 +2765,7 @@ create_LifetimeEnd(LLVMModuleRef mod) LLVMContextRef lc; /* variadic pointer argument */ - const char *nm = "llvm.lifetime.end.p0i8"; + const char *nm = "llvm.lifetime.end.p0"; fn = LLVMGetNamedFunction(mod, nm); if (fn) -- 2.44.0
