> On Fri, Apr 05, 2024 at 03:50:50PM +0200, Dmitry Dolgov wrote:
> > On Fri, Apr 05, 2024 at 03:21:06PM +0200, Dmitry Dolgov wrote:
> > > On Sat, Apr 06, 2024 at 02:00:38AM +1300, Thomas Munro wrote:
> > > On Sun, Mar 31, 2024 at 12:49 PM Thomas Munro <thomas.mu...@gmail.com> 
> > > wrote:
> > > > https://github.com/llvm/llvm-project/pull/87093
> > >
> > > Oh, with those clues, I think I might see...  It is a bit strange that
> > > we copy attributes from AttributeTemplate(), a function that returns
> > > Datum, to our void deform function.  It works (I mean doesn't crash)
> > > if you just comment this line out:
> > >
> > >     llvm_copy_attributes(AttributeTemplate, v_deform_fn);
> > >
> > > ... but I guess that disables inlining of the deform function?  So
> > > perhaps we just need to teach that thing not to try to copy the return
> > > value's attributes, which also seems to work here:
> >
> > 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.
>From 7a05bd48a4270998a08e63e07cdf1cb9932bfddd Mon Sep 17 00:00:00 2001
From: Dmitrii Dolgov <9erthali...@gmail.com>
Date: Fri, 5 Apr 2024 17:52:06 +0200
Subject: [PATCH v1] Add jit_verify_bitcode

When passing LLVM IR through optimizer, for troubleshooting purposes
it's useful to apply the "verify" pass as well. It can catch an invalid
IR, if such was produced by mistake, and output a meaningful message
about the error, e.g.:

    Attribute 'noundef' applied to incompatible type!
    ptr @deform_0_1

Control this via jit_verify_bitcode development guc.
---
 doc/src/sgml/config.sgml            | 19 +++++++++++++++++++
 src/backend/jit/jit.c               |  1 +
 src/backend/jit/llvm/llvmjit.c      | 11 ++++++++---
 src/backend/utils/misc/guc_tables.c | 11 +++++++++++
 src/include/jit/jit.h               |  2 +-
 5 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 624518e0b01..5f07ae099f4 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -11931,6 +11931,25 @@ LOG:  CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-jit-verify-bitcode" xreflabel="jit_verify_bitcode">
+      <term><varname>jit_verify_bitcode</varname> (<type>boolean</type>)
+      <indexterm>
+       <primary><varname>jit_verify_bitcode</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Adds a <literal>verify</literal> pass to the pipeline, when optimizing
+        generated <productname>LLVM</productname> IR. It helps to identify
+        cases when an invalid IR was generated due to errors, and is only
+        useful for working on the internals of the JIT implementation.
+        The default setting is <literal>off</literal>.
+        Only superusers and users with the appropriate <literal>SET</literal>
+        privilege can change this setting.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-jit-expressions" xreflabel="jit_expressions">
       <term><varname>jit_expressions</varname> (<type>boolean</type>)
       <indexterm>
diff --git a/src/backend/jit/jit.c b/src/backend/jit/jit.c
index 815b58f33c5..ad94e4b55ee 100644
--- a/src/backend/jit/jit.c
+++ b/src/backend/jit/jit.c
@@ -39,6 +39,7 @@ bool		jit_tuple_deforming = true;
 double		jit_above_cost = 100000;
 double		jit_inline_above_cost = 500000;
 double		jit_optimize_above_cost = 500000;
+bool		jit_verify_bitcode = false;
 
 static JitProviderCallbacks provider;
 static bool provider_successfully_loaded = false;
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index ec0fdd49324..a4b21abb83a 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -698,12 +698,17 @@ llvm_optimize_module(LLVMJitContext *context, LLVMModuleRef module)
 #else
 	LLVMPassBuilderOptionsRef options;
 	LLVMErrorRef err;
-	const char *passes;
+	const char *passes, *intermediate_passes;
 
 	if (context->base.flags & PGJIT_OPT3)
-		passes = "default<O3>";
+		intermediate_passes = "default<O3>";
 	else
-		passes = "default<O0>,mem2reg";
+		intermediate_passes = "default<O0>,mem2reg";
+
+	if (jit_verify_bitcode)
+		passes = psprintf("verify,%s", intermediate_passes);
+	else
+		passes = intermediate_passes;
 
 	options = LLVMCreatePassBuilderOptions();
 
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c12784cbec8..9b8f4249331 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1998,6 +1998,17 @@ struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"jit_verify_bitcode", PGC_SUSET, DEVELOPER_OPTIONS,
+			gettext_noop("Add verify pass when optimizing LLVM bitcode"),
+			NULL,
+			GUC_NOT_IN_SAMPLE
+		},
+		&jit_verify_bitcode,
+		false,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"data_sync_retry", PGC_POSTMASTER, ERROR_HANDLING_OPTIONS,
 			gettext_noop("Whether to continue running after a failure to sync data files."),
diff --git a/src/include/jit/jit.h b/src/include/jit/jit.h
index b7a1eed2810..4703b107cc4 100644
--- a/src/include/jit/jit.h
+++ b/src/include/jit/jit.h
@@ -89,11 +89,11 @@ extern PGDLLIMPORT bool jit_dump_bitcode;
 extern PGDLLIMPORT bool jit_expressions;
 extern PGDLLIMPORT bool jit_profiling_support;
 extern PGDLLIMPORT bool jit_tuple_deforming;
+extern PGDLLIMPORT bool jit_verify_bitcode;
 extern PGDLLIMPORT double jit_above_cost;
 extern PGDLLIMPORT double jit_inline_above_cost;
 extern PGDLLIMPORT double jit_optimize_above_cost;
 
-
 extern void jit_reset_after_error(void);
 extern void jit_release_context(JitContext *context);
 

base-commit: c9920a9068eac2e6c8fb34988d18c0b42b9bf811
-- 
2.41.0

Reply via email to