Re: JIT compiling with LLVM v9.1

[Andreas Karlsson]

On 02/15/2018 06:23 PM, Andres Freund wrote:

2) When I build with --with-cassert I expect the assertions to be there,
both in the binaries and the bitcode. Is that just a bug or is there any
thought behind this?


Not sure what you mean by that. NDEBUG and cassert are independent
mechanisms, no?


Yeah, I think I just managed to confuse myself there.

The actual issue is that --with-llvm changes if NDEBUG is set or not, 
which is quite surprising. I would not expect assertions to be disabled 
in the fronted code just because I compiled PostgreSQL with llvm.


Andreas

Re: JIT compiling with LLVM v9.1

[Andres Freund]

Hi,

On 2018-02-15 12:54:34 +0100, Andreas Karlsson wrote:
> 1) Why does config/llvm.m4 modify CPPFLAGS? That affects the building of the
> binaries too which may be done with gcc like in my case. Shouldn't it use a
> LLVM_CPPFLAGS or something?

Well, most of the time cppflags just are things like additional include
directories. And the established precedent is to just add those to the
global cppflags (c.f. libxml stuff in configure in).  I've no problem
changing this, I just followed established practice.


> 2) When I build with --with-cassert I expect the assertions to be there,
> both in the binaries and the bitcode. Is that just a bug or is there any
> thought behind this?

Not sure what you mean by that. NDEBUG and cassert are independent
mechanisms, no?

Greetings,

Andres Freund

Re: JIT compiling with LLVM v9.1

[Andreas Karlsson]

On 02/05/2018 10:44 PM, Pierre Ducroquet wrote:

psqlscanslash.l: In function ‘psql_scan_slash_option’:
psqlscanslash.l:550:8: warning: variable ‘lexresult’ set but not used
[-Wunused-but-set-variable]
int   final_state;
  ^


I'm not sure Andres's patches have anything to do with psql, it's surprising.


I managed to track down the bug and apparently when building with 
--with-llvm the -DNDEBUG option is added to CPPFLAGS, but I am not 
entirely sure what the code in config/llvm.m4 is trying to do in the 
first place.


The two issues I see with what the code does are:

1) Why does config/llvm.m4 modify CPPFLAGS? That affects the building of 
the binaries too which may be done with gcc like in my case. Shouldn't 
it use a LLVM_CPPFLAGS or something?


2) When I build with --with-cassert I expect the assertions to be there, 
both in the binaries and the bitcode. Is that just a bug or is there any 
thought behind this?


Below is the diff in src/Makefile.global between when I run configure 
with --with-llvm or not.


diff src/Makefile.global-nollvm  src/Makefile.global-llvm
78c78
< configure_args =  '--prefix=/home/andreas/dev/postgresql-inst' 
'--enable-tap-tests' '--enable-cassert' '--enable-debug'

---
> configure_args =  '--prefix=/home/andreas/dev/postgresql-inst' 
'--enable-tap-tests' '--enable-cassert' '--enable-debug' '--with-llvm'

190c190
< with_llvm  = no
---
> with_llvm  = yes
227,229c227,229
< LLVM_CONFIG =
< LLVM_BINPATH =
< CLANG =
---
> LLVM_CONFIG = /usr/bin/llvm-config
> LLVM_BINPATH = /usr/lib/llvm-4.0/bin
> CLANG = /usr/bin/clang
238c238
< CPPFLAGS =  -D_GNU_SOURCE
---
> CPPFLAGS = -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_CONSTANT_MACROS -D_GNU_SOURCE -DNDEBUG 
-I/usr/lib/llvm-4.0/include  -D_GNU_SOURCE

261c261
< LLVM_CXXFLAGS =
---
> LLVM_CXXFLAGS =  -std=c++0x -std=c++11 -fno-exceptions
283c283
< LLVM_LIBS=
---
> LLVM_LIBS= -lLLVM-4.0
297c297
< LDFLAGS +=   -Wl,--as-needed
---
> LDFLAGS +=  -L/usr/lib/llvm-4.0/lib  -Wl,--as-needed

Re: JIT compiling with LLVM v9.1

[Pierre Ducroquet]

On Monday, February 5, 2018 10:20:27 PM CET Andreas Karlsson wrote:
> OK that fixed the issue, but you have a typo in your patch set.
> 
> diff --git a/src/backend/lib/llvmjit_inline.cpp
> b/src/backend/lib/llvmjit_inline.cpp
> index a785261bea..51f38e10d2 100644
> --- a/src/backend/lib/llvmjit_inline.cpp
> +++ b/src/backend/lib/llvmjit_inline.cpp
> @@ -37,7 +37,7 @@ extern "C"
>   #include 
>   #include 
>   #include 
> -#if LLVM_MAJOR_VERSION > 3
> +#if LLVM_VERSION_MAJOR > 3
>   #include 
>   #else
>   #include "llvm/Bitcode/ReaderWriter.h"

Thanks, it's weird I had no issue with it. I will fix in the next patch set.

> Also I get some warning. Not sure if they are from your patches or from
> Andres's.
> 
> llvmjit_error.cpp:118:1: warning: unused function
> 'fatal_llvm_new_handler' [-Wunused-function]
> fatal_llvm_new_handler(void *user_data,
> ^
> 1 warning generated.
> llvmjit_inline.cpp:114:6: warning: no previous prototype for function
> 'operator!' [-Wmissing-prototypes]
> bool operator!(const llvm::ValueInfo ) {
>   ^
> 1 warning generated.

Both are mine, I knew about the first one, but I did not see the second one. I 
will fix them too, thanks for the review!

> psqlscanslash.l: In function ‘psql_scan_slash_option’:
> psqlscanslash.l:550:8: warning: variable ‘lexresult’ set but not used
> [-Wunused-but-set-variable]
>int   final_state;
>  ^

I'm not sure Andres's patches have anything to do with psql, it's surprising.

Re: JIT compiling with LLVM v9.1

[Andreas Karlsson]

OK that fixed the issue, but you have a typo in your patch set.

diff --git a/src/backend/lib/llvmjit_inline.cpp 
b/src/backend/lib/llvmjit_inline.cpp

index a785261bea..51f38e10d2 100644
--- a/src/backend/lib/llvmjit_inline.cpp
+++ b/src/backend/lib/llvmjit_inline.cpp
@@ -37,7 +37,7 @@ extern "C"
 #include 
 #include 
 #include 
-#if LLVM_MAJOR_VERSION > 3
+#if LLVM_VERSION_MAJOR > 3
 #include 
 #else
 #include "llvm/Bitcode/ReaderWriter.h"

Also I get some warning. Not sure if they are from your patches or from 
Andres's.


llvmjit_error.cpp:118:1: warning: unused function 
'fatal_llvm_new_handler' [-Wunused-function]

fatal_llvm_new_handler(void *user_data,
^
1 warning generated.
llvmjit_inline.cpp:114:6: warning: no previous prototype for function 
'operator!' [-Wmissing-prototypes]

bool operator!(const llvm::ValueInfo ) {
 ^
1 warning generated.
psqlscanslash.l: In function ‘psql_scan_slash_option’:
psqlscanslash.l:550:8: warning: variable ‘lexresult’ set but not used 
[-Wunused-but-set-variable]

  int   final_state;
^

Andreas

On 02/05/2018 11:39 AM, Pierre Ducroquet wrote:

On Sunday, February 4, 2018 12:45:50 AM CET Andreas Karlsson wrote:

On 02/02/2018 10:48 AM, Pierre Ducroquet wrote:

I have successfully built the JIT branch against LLVM 4.0.1 on Debian
testing. This is not enough for Debian stable (LLVM 3.9 is the latest
available there), but it's a first step.
I've split the patch in four files. The first three fix the build issues,
the last one fixes a runtime issue.
I think they are small enough to not be a burden for you in your
developments. But if you don't want to carry these ifdefs right now, I
maintain them in a branch on a personal git and rebase as frequently as I
can.


I tested these patches and while the code built for me and passed the
test suite on Debian testing I have a weird bug where the very first
query fails to JIT while the rest work as they should. I think I need to
dig into LLVM's codebase to see what it is, but can you reproduce this
bug at your machine?

Code to reproduce:

SET jit_expressions = true;
SET jit_above_cost = 0;
SELECT 1;
SELECT 1;

Output:

postgres=# SELECT 1;
ERROR:  failed to jit module
postgres=# SELECT 1;
   ?column?
--
  1
(1 row)

Config:

Version: You patches applied on top of
302b7a284d30fb0e00eb5f0163aa933d4d9bea10
OS: Debian testing
llvm/clang: 4.0.1-8

Andreas



I have fixed the patches, I was wrong on 'guessing' the migration of the API
for one function.
I have rebuilt the whole patch set. It is still based on 302b7a284d and has
been tested with both LLVM 3.9 and 4.0 on Debian testing.

Thanks for your feedback !

Re: JIT compiling with LLVM v9.1

[Pierre Ducroquet]

On Sunday, February 4, 2018 12:45:50 AM CET Andreas Karlsson wrote:
> On 02/02/2018 10:48 AM, Pierre Ducroquet wrote:
> > I have successfully built the JIT branch against LLVM 4.0.1 on Debian
> > testing. This is not enough for Debian stable (LLVM 3.9 is the latest
> > available there), but it's a first step.
> > I've split the patch in four files. The first three fix the build issues,
> > the last one fixes a runtime issue.
> > I think they are small enough to not be a burden for you in your
> > developments. But if you don't want to carry these ifdefs right now, I
> > maintain them in a branch on a personal git and rebase as frequently as I
> > can.
> 
> I tested these patches and while the code built for me and passed the
> test suite on Debian testing I have a weird bug where the very first
> query fails to JIT while the rest work as they should. I think I need to
> dig into LLVM's codebase to see what it is, but can you reproduce this
> bug at your machine?
> 
> Code to reproduce:
> 
> SET jit_expressions = true;
> SET jit_above_cost = 0;
> SELECT 1;
> SELECT 1;
> 
> Output:
> 
> postgres=# SELECT 1;
> ERROR:  failed to jit module
> postgres=# SELECT 1;
>   ?column?
> --
>  1
> (1 row)
> 
> Config:
> 
> Version: You patches applied on top of
> 302b7a284d30fb0e00eb5f0163aa933d4d9bea10
> OS: Debian testing
> llvm/clang: 4.0.1-8
> 
> Andreas


I have fixed the patches, I was wrong on 'guessing' the migration of the API 
for one function.
I have rebuilt the whole patch set. It is still based on 302b7a284d and has 
been tested with both LLVM 3.9 and 4.0 on Debian testing.

Thanks for your feedback !
>From ebf577fc6b1e74ddb2f6b9aef1c2632ab807dd70 Mon Sep 17 00:00:00 2001
From: Pierre 
Date: Fri, 2 Feb 2018 09:11:55 +0100
Subject: [PATCH 1/6] Add support for LLVM4 in llvmjit.c

---
 src/backend/lib/llvmjit.c | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/backend/lib/llvmjit.c b/src/backend/lib/llvmjit.c
index 8e5ba94c98..046cd53ef3 100644
--- a/src/backend/lib/llvmjit.c
+++ b/src/backend/lib/llvmjit.c
@@ -230,12 +230,19 @@ llvm_get_function(LLVMJitContext *context, const char *funcname)
 
 		addr = 0;
 		if (LLVMOrcGetSymbolAddressIn(handle->stack, , handle->orc_handle, mangled))
-			elog(ERROR, "failed to lookup symbol");
+			elog(ERROR, "failed to lookup symbol %s", mangled);
 		if (addr)
 			return (void *) addr;
 	}
 #endif
 
+#if LLVM_VERSION_MAJOR < 5
+	if ((addr = LLVMOrcGetSymbolAddress(llvm_opt0_orc, mangled)))
+		return (void *) addr;
+	if ((addr = LLVMOrcGetSymbolAddress(llvm_opt3_orc, mangled)))
+		return (void *) addr;
+	elog(ERROR, "failed to lookup symbol %s for %s", mangled, funcname);
+#else
 	if (LLVMOrcGetSymbolAddress(llvm_opt0_orc, , mangled))
 		elog(ERROR, "failed to lookup symbol");
 	if (addr)
@@ -244,7 +251,7 @@ llvm_get_function(LLVMJitContext *context, const char *funcname)
 		elog(ERROR, "failed to lookup symbol");
 	if (addr)
 		return (void *) addr;
-
+#endif
 	elog(ERROR, "failed to JIT: %s", funcname);
 
 	return NULL;
@@ -380,11 +387,18 @@ llvm_compile_module(LLVMJitContext *context)
 	 * faster instruction selection mechanism is used.
 	 */
 	{
-		LLVMSharedModuleRef smod;
 		instr_time tb, ta;
 
 		/* emit the code */
 		INSTR_TIME_SET_CURRENT(ta);
+#if LLVM_VERSION_MAJOR < 5
+		orc_handle = LLVMOrcAddEagerlyCompiledIR(compile_orc, context->module,
+			 llvm_resolve_symbol, NULL);
+		// It seems there is no error return from that function in LLVM < 5.
+#else
+		LLVMSharedModuleRef smod;
+
+		LLVMSharedModuleRef smod;
 		smod = LLVMOrcMakeSharedModule(context->module);
 		if (LLVMOrcAddEagerlyCompiledIR(compile_orc, _handle, smod,
 		llvm_resolve_symbol, NULL))
@@ -392,6 +406,7 @@ llvm_compile_module(LLVMJitContext *context)
 			elog(ERROR, "failed to jit module");
 		}
 		LLVMOrcDisposeSharedModuleRef(smod);
+#endif
 		INSTR_TIME_SET_CURRENT(tb);
 		INSTR_TIME_SUBTRACT(tb, ta);
 		ereport(DEBUG1, (errmsg("time to emit: %.3fs",
-- 
2.15.1

>From f90c156c9b26eda38d3a1312c3de87ffba50340a Mon Sep 17 00:00:00 2001
From: Pierre 
Date: Fri, 2 Feb 2018 09:13:40 +0100
Subject: [PATCH 2/6] Add LLVM4 support in llvmjit_error.cpp

---
 src/backend/lib/llvmjit_error.cpp | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/backend/lib/llvmjit_error.cpp b/src/backend/lib/llvmjit_error.cpp
index 70cecd114b..04e51b2a31 100644
--- a/src/backend/lib/llvmjit_error.cpp
+++ b/src/backend/lib/llvmjit_error.cpp
@@ -56,7 +56,9 @@ llvm_enter_fatal_on_oom(void)
 	if (fatal_new_handler_depth == 0)
 	{
 		old_new_handler = std::set_new_handler(fatal_system_new_handler);
+#if LLVM_VERSION_MAJOR > 4
 		llvm::install_bad_alloc_error_handler(fatal_llvm_new_handler);
+#endif
 		llvm::install_fatal_error_handler(fatal_llvm_error_handler);
 	}
 	fatal_new_handler_depth++;
@@ -72,7 +74,9 @@ llvm_leave_fatal_on_oom(void)
 	if (fatal_new_handler_depth == 0)
 	{
 

Re: JIT compiling with LLVM v9.1

[Pierre Ducroquet]

On Sunday, February 4, 2018 12:45:50 AM CET Andreas Karlsson wrote:
> On 02/02/2018 10:48 AM, Pierre Ducroquet wrote:
> > I have successfully built the JIT branch against LLVM 4.0.1 on Debian
> > testing. This is not enough for Debian stable (LLVM 3.9 is the latest
> > available there), but it's a first step.
> > I've split the patch in four files. The first three fix the build issues,
> > the last one fixes a runtime issue.
> > I think they are small enough to not be a burden for you in your
> > developments. But if you don't want to carry these ifdefs right now, I
> > maintain them in a branch on a personal git and rebase as frequently as I
> > can.
> 
> I tested these patches and while the code built for me and passed the
> test suite on Debian testing I have a weird bug where the very first
> query fails to JIT while the rest work as they should. I think I need to
> dig into LLVM's codebase to see what it is, but can you reproduce this
> bug at your machine?
> 
> Code to reproduce:
> 
> SET jit_expressions = true;
> SET jit_above_cost = 0;
> SELECT 1;
> SELECT 1;
> 
> Output:
> 
> postgres=# SELECT 1;
> ERROR:  failed to jit module
> postgres=# SELECT 1;
>   ?column?
> --
>  1
> (1 row)
> 
> Config:
> 
> Version: You patches applied on top of
> 302b7a284d30fb0e00eb5f0163aa933d4d9bea10
> OS: Debian testing
> llvm/clang: 4.0.1-8
> 
> Andreas

Hi

Indeed, thanks for reporting this. I scripted the testing but failed to see 
it, I forgot to set on_error_stop.
I will look into this and fix it.

Thanks

 Pierre

Re: JIT compiling with LLVM v9.1

[Andreas Karlsson]

On 02/02/2018 10:48 AM, Pierre Ducroquet wrote:

I have successfully built the JIT branch against LLVM 4.0.1 on Debian testing.
This is not enough for Debian stable (LLVM 3.9 is the latest available there),
but it's a first step.
I've split the patch in four files. The first three fix the build issues, the
last one fixes a runtime issue.
I think they are small enough to not be a burden for you in your developments.
But if you don't want to carry these ifdefs right now, I maintain them in a
branch on a personal git and rebase as frequently as I can.


I tested these patches and while the code built for me and passed the 
test suite on Debian testing I have a weird bug where the very first 
query fails to JIT while the rest work as they should. I think I need to 
dig into LLVM's codebase to see what it is, but can you reproduce this 
bug at your machine?


Code to reproduce:

SET jit_expressions = true;
SET jit_above_cost = 0;
SELECT 1;
SELECT 1;

Output:

postgres=# SELECT 1;
ERROR:  failed to jit module
postgres=# SELECT 1;
 ?column?
--
1
(1 row)

Config:

Version: You patches applied on top of 
302b7a284d30fb0e00eb5f0163aa933d4d9bea10

OS: Debian testing
llvm/clang: 4.0.1-8

Andreas

Re: JIT compiling with LLVM v9.1

[Andres Freund]

Hi,

On 2018-02-03 01:13:21 -0800, Andres Freund wrote:
> On 2018-02-02 18:21:12 -0800, Jeff Davis wrote:
> > I think I saw about a 2% gain here over master, but when I applied it
> > on top of the fast scans it did not seem to add anything on top of
> > fast scans. Seems reproducible, but I don't have an explanation.
> 
> Yea, that makes sense. The primary reason the patch is beneficial is
> that it centralizes the place where the HeapTupleHeader is accessed to a
> single piece of code (slot_deform_tuple()). In a lot of cases that first
> access will result in a cache miss in all layers, requiring a memory
> access. In slot_getsomeattrs() there's very little that can be done in
> an out-of-order manner, whereas slot_deform_tuple() can continue
> execution a bit further. Also, the latter will then go and sequentially
> access the rest (or a significant part of) the tuple, so a centralized
> access is more prefetchable.

Oops missed part of the argument here: The reason that isn't that large
an effect anymore with the scan order patch applied is that suddenly the
accesses are, due to the better scan order, more likely to be cacheable
and prefetchable. So in that case the few additional instructions and
branches in slot_getsomeattrs/slot_getattr don't hurt as much
anymore. IIRC I could still show it up, but it's a much smaller win.

Greetings,

Andres Freund

Re: JIT compiling with LLVM v9.1

[Andres Freund]

Hi,

On 2018-02-02 18:21:12 -0800, Jeff Davis wrote:
> On Mon, Jan 29, 2018 at 1:53 AM, Andres Freund  wrote:
> >>   https://git.postgresql.org/git/users/andresfreund/postgres.git
> 
> There's a patch in there to change the scan order.

Yes - note it's "deactivated" at the moment in the series. I primarily
have it in there because I found profiles to be a lot more useful if
it's enabled, as otherwise the number of cache misses and related stalls
from heap accesses completely swamp everything else.

FWIW, there's 
http://archives.postgresql.org/message-id/20161030073655.rfa6nvbyk4w2kkpk%40alap3.anarazel.de


> I suggest that you rename the GUC "synchronize_seqscans" to something
> more generic like "optimize_scan_order", and use it to control your
> feature as well (after all, it's the same trade-off: weird scan order
> vs.  performance). Then, go ahead and commit it. FWIW I see about a 7%
> boost on my laptop[1] from that patch on master, without JIT or
> anything else.

Yea, that's roughly the same magnitude of what I'm seeing, some queries
even bigger.

I'm not sure I want to commit this right now - ISTM we couldn't default
this to on without annoying a lot of people, and letting the performance
wins on the table by default seems like a shame.  I think we should
probably either change the order we store things on the page by default
or only use the faster order if the scan above doesn't care about order
- the planner could figure that out easily.

I personally don't think it is necessary to get this committed at the
same time as the JIT stuff, so I'm not planning to push very hard on
that front. Should you be interested in taking it up, please feel
entirely free.


> I also see you dropped "7ae518bf Centralize slot deforming logic a
> bit.". Was that intentional? Do we want it?

The problem is that there's probably some controversial things in
there. I think the checks I dropped largely make no sense, but I don't
really want to push for that hard.  I suspect we probably still want it,
but I do not want to put into the critical path right now.


> I think I saw about a 2% gain here over master, but when I applied it
> on top of the fast scans it did not seem to add anything on top of
> fast scans. Seems reproducible, but I don't have an explanation.

Yea, that makes sense. The primary reason the patch is beneficial is
that it centralizes the place where the HeapTupleHeader is accessed to a
single piece of code (slot_deform_tuple()). In a lot of cases that first
access will result in a cache miss in all layers, requiring a memory
access. In slot_getsomeattrs() there's very little that can be done in
an out-of-order manner, whereas slot_deform_tuple() can continue
execution a bit further. Also, the latter will then go and sequentially
access the rest (or a significant part of) the tuple, so a centralized
access is more prefetchable.


> And you are probably already working on this, but it would be helpful
> to get the following two patches in also:
> * 3c22065f Do execGrouping via expression eval
> * a9dde4aa Allow tupleslots to have a fixed tupledesc

Yes, I plan to resume working in whipping them up into shape as soon as
I've finished the move to a shared library. This weekend I'm at fosdem,
so that's going to be after...

Thanks for looking!

Andres Freund

Re: JIT compiling with LLVM v9.1

[Jeff Davis]

On Mon, Jan 29, 2018 at 1:53 AM, Andres Freund  wrote:
>>   https://git.postgresql.org/git/users/andresfreund/postgres.git

There's a patch in there to change the scan order. I suggest that you
rename the GUC "synchronize_seqscans" to something more generic like
"optimize_scan_order", and use it to control your feature as well
(after all, it's the same trade-off: weird scan order vs.
performance). Then, go ahead and commit it. FWIW I see about a 7%
boost on my laptop[1] from that patch on master, without JIT or
anything else.

I also see you dropped "7ae518bf Centralize slot deforming logic a
bit.". Was that intentional? Do we want it? I think I saw about a 2%
gain here over master, but when I applied it on top of the fast scans
it did not seem to add anything on top of fast scans. Seems
reproducible, but I don't have an explanation.

And you are probably already working on this, but it would be helpful
to get the following two patches in also:
* 3c22065f Do execGrouping via expression eval
* a9dde4aa Allow tupleslots to have a fixed tupledesc

I took a brief look at those two, but will review them in more detail.

Regards,
 Jeff Davis

[1] Simple scan with simple predicate on 50M tuples, after pg_prewarm.

Re: JIT compiling with LLVM v9.1

[Pierre Ducroquet]

On Friday, February 2, 2018 10:48:16 AM CET Pierre Ducroquet wrote:
> On Monday, January 29, 2018 10:53:50 AM CET Andres Freund wrote:
> > Hi,
> > 
> > On 2018-01-23 23:20:38 -0800, Andres Freund wrote:
> > > == Code ==
> > > 
> > > As the patchset is large (500kb) and I'm still quickly evolving it, I do
> > > not yet want to attach it. The git tree is at
> > > 
> > >   https://git.postgresql.org/git/users/andresfreund/postgres.git
> > > 
> > > in the jit branch
> > > 
> > >   https://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a
> > >   =s
> > >   hortlog;h=refs/heads/jit
> > 
> > I've just pushed an updated and rebased version of the tree:
> > - Split the large "jit infrastructure" commits into a number of smaller
> > 
> >   commits
> > 
> > - Split the C++ file
> > - Dropped some of the performance stuff done to heaptuple.c - that was
> > 
> >   mostly to make performance comparisons a bit more interesting, but
> >   doesn't seem important enough to deal with.
> > 
> > - Added a commit renaming datetime.h symbols so they don't conflict with
> > 
> >   LLVM variables anymore, removing ugly #undef PM/#define PM dance
> >   around includes. Will post separately.
> > 
> > - Reduced the number of pointer constants in the generated LLVM IR, by
> > 
> >   doing more getelementptr accesses (stem from before the time types
> >   were automatically synced)
> > 
> > - Increased number of comments a bit
> > 
> > There's a jit-before-rebase-2018-01-29 tag, for the state of the tree
> > before the rebase.
> > 
> > Regards,
> > 
> > Andres
> 
> Hi
> 
> I have successfully built the JIT branch against LLVM 4.0.1 on Debian
> testing. This is not enough for Debian stable (LLVM 3.9 is the latest
> available there), but it's a first step.
> I've split the patch in four files. The first three fix the build issues,
> the last one fixes a runtime issue.
> I think they are small enough to not be a burden for you in your
> developments. But if you don't want to carry these ifdefs right now, I
> maintain them in a branch on a personal git and rebase as frequently as I
> can.
> 
> LLVM 3.9 support isn't going to be hard, but I prefer splitting. I also hope
> this will help more people test this wonderful toy… :)
> 
> Regards
> 
>  Pierre

For LLVM 3.9, only small changes were needed.
I've attached the patches to this email.
I only did very basic, primitive testing, but it seems to work.
I'll do more testing in the next days.

 Pierre

>From 5ca9594a7f52b7daab8562293010fe8c807107ee Mon Sep 17 00:00:00 2001
From: Pierre 
Date: Fri, 2 Feb 2018 11:29:45 +0100
Subject: [PATCH 1/2] Fix building with LLVM 3.9

---
 src/backend/lib/llvmjit_inline.cpp | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/backend/lib/llvmjit_inline.cpp b/src/backend/lib/llvmjit_inline.cpp
index 8a747cbfc0..a785261bea 100644
--- a/src/backend/lib/llvmjit_inline.cpp
+++ b/src/backend/lib/llvmjit_inline.cpp
@@ -37,7 +37,12 @@ extern "C"
 #include 
 #include 
 #include 
+#if LLVM_MAJOR_VERSION > 3
 #include 
+#else
+#include "llvm/Bitcode/ReaderWriter.h"
+#include "llvm/Support/Error.h"
+#endif
 #include 
 #include 
 #include 
@@ -100,7 +105,12 @@ llvm_inline(LLVMModuleRef M)
 	llvm_execute_inline_plan(mod, globalsToInline.get());
 }
 
-#if LLVM_VERSION_MAJOR < 5
+#if LLVM_VERSION_MAJOR < 4
+bool operator!(const llvm::ValueInfo ) {
+	return !(  (vi.Kind == llvm::ValueInfo::VI_GUID && vi.TheValue.Id)
+		|| (vi.Kind == llvm::ValueInfo::VI_Value && vi.TheValue.V));
+}
+#elif LLVM_VERSION_MAJOR < 5
 bool operator!(const llvm::ValueInfo ) {
 	return !(  (vi.Kind == llvm::ValueInfo::VI_GUID && vi.TheValue.Id)
 		|| (vi.Kind == llvm::ValueInfo::VI_Value && vi.TheValue.GV));
@@ -188,12 +198,15 @@ llvm_build_inline_plan(llvm::Module *mod)
  funcName.data(),
  modPath.data());
 
+// XXX Missing in LLVM < 4.0 ?
+#if LLVM_VERSION_MAJOR > 3
 			if (gvs->notEligibleToImport())
 			{
 elog(DEBUG1, "uneligible to import %s due to summary",
 	 funcName.data());
 continue;
 			}
+#endif
 
 			if ((int) fs->instCount() > threshold)
 			{
@@ -339,8 +352,10 @@ llvm_execute_inline_plan(llvm::Module *mod, ImportMapTy *globalsToInline)
 
 #if LLVM_VERSION_MAJOR > 4
 #define IRMOVE_PARAMS , /*IsPerformingImport=*/false
-#else
+#elif LLVM_VERSION_MAJOR > 3
 #define IRMOVE_PARAMS , /*LinkModuleInlineAsm=*/false, /*IsPerformingImport=*/false
+#else
+#define IRMOVE_PARAMS
 #endif
 		if (Mover.move(std::move(importMod), GlobalsToImport.getArrayRef(),
 	   [](llvm::GlobalValue &, llvm::IRMover::ValueAdder) {}
@@ -648,7 +663,11 @@ llvm_load_index(void)
 			if (e)
 elog(ERROR, "could not load summary at %s", subpath);
 #else
+#if LLVM_VERSION_MAJOR > 3
 			std::unique_ptr subindex = std::move(llvm::getModuleSummaryIndex(ref).get());
+#else
+			std::unique_ptr subindex = std::move(llvm::getModuleSummaryIndex(ref, [](const llvm::DiagnosticInfo &) {}).get());
+#endif
 			if 

Re: JIT compiling with LLVM v9.1

[Pierre Ducroquet]

On Monday, January 29, 2018 10:53:50 AM CET Andres Freund wrote:
> Hi,
> 
> On 2018-01-23 23:20:38 -0800, Andres Freund wrote:
> > == Code ==
> > 
> > As the patchset is large (500kb) and I'm still quickly evolving it, I do
> > not yet want to attach it. The git tree is at
> > 
> >   https://git.postgresql.org/git/users/andresfreund/postgres.git
> > 
> > in the jit branch
> > 
> >   https://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=s
> >   hortlog;h=refs/heads/jit
> I've just pushed an updated and rebased version of the tree:
> - Split the large "jit infrastructure" commits into a number of smaller
>   commits
> - Split the C++ file
> - Dropped some of the performance stuff done to heaptuple.c - that was
>   mostly to make performance comparisons a bit more interesting, but
>   doesn't seem important enough to deal with.
> - Added a commit renaming datetime.h symbols so they don't conflict with
>   LLVM variables anymore, removing ugly #undef PM/#define PM dance
>   around includes. Will post separately.
> - Reduced the number of pointer constants in the generated LLVM IR, by
>   doing more getelementptr accesses (stem from before the time types
>   were automatically synced)
> - Increased number of comments a bit
> 
> There's a jit-before-rebase-2018-01-29 tag, for the state of the tree
> before the rebase.
> 
> Regards,
> 
> Andres

Hi

I have successfully built the JIT branch against LLVM 4.0.1 on Debian testing. 
This is not enough for Debian stable (LLVM 3.9 is the latest available there), 
but it's a first step.
I've split the patch in four files. The first three fix the build issues, the 
last one fixes a runtime issue.
I think they are small enough to not be a burden for you in your developments. 
But if you don't want to carry these ifdefs right now, I maintain them in a 
branch on a personal git and rebase as frequently as I can.

LLVM 3.9 support isn't going to be hard, but I prefer splitting. I also hope 
this will help more people test this wonderful toy… :)

Regards

 Pierre


>From 770104331a36a8d207053227b850396f1392939a Mon Sep 17 00:00:00 2001
From: Pierre 
Date: Fri, 2 Feb 2018 09:11:55 +0100
Subject: [PATCH 1/4] Add support for LLVM4 in llvmjit.c

---
 src/backend/lib/llvmjit.c | 24 +---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/backend/lib/llvmjit.c b/src/backend/lib/llvmjit.c
index 8e5ba94c98..d0c5537610 100644
--- a/src/backend/lib/llvmjit.c
+++ b/src/backend/lib/llvmjit.c
@@ -230,12 +230,19 @@ llvm_get_function(LLVMJitContext *context, const char *funcname)
 
 		addr = 0;
 		if (LLVMOrcGetSymbolAddressIn(handle->stack, , handle->orc_handle, mangled))
-			elog(ERROR, "failed to lookup symbol");
+			elog(ERROR, "failed to lookup symbol %s", mangled);
 		if (addr)
 			return (void *) addr;
 	}
 #endif
 
+#if LLVM_VERSION_MAJOR < 5
+	if ((addr = LLVMOrcGetSymbolAddress(llvm_opt0_orc, mangled)))
+		return (void *) addr;
+	if ((addr = LLVMOrcGetSymbolAddress(llvm_opt3_orc, mangled)))
+		return (void *) addr;
+	elog(ERROR, "failed to lookup symbol %s for %s", mangled, funcname);
+#else
 	if (LLVMOrcGetSymbolAddress(llvm_opt0_orc, , mangled))
 		elog(ERROR, "failed to lookup symbol");
 	if (addr)
@@ -244,7 +251,7 @@ llvm_get_function(LLVMJitContext *context, const char *funcname)
 		elog(ERROR, "failed to lookup symbol");
 	if (addr)
 		return (void *) addr;
-
+#endif
 	elog(ERROR, "failed to JIT: %s", funcname);
 
 	return NULL;
@@ -380,11 +387,21 @@ llvm_compile_module(LLVMJitContext *context)
 	 * faster instruction selection mechanism is used.
 	 */
 	{
-		LLVMSharedModuleRef smod;
 		instr_time tb, ta;
 
 		/* emit the code */
 		INSTR_TIME_SET_CURRENT(ta);
+#if LLVM_VERSION < 5
+		orc_handle = LLVMOrcAddEagerlyCompiledIR(compile_orc, context->module,
+		llvm_resolve_symbol, NULL);
+		if (!orc_handle)
+		{
+			elog(ERROR, "failed to jit module");
+		}
+#else
+		LLVMSharedModuleRef smod;
+
+		LLVMSharedModuleRef smod;
 		smod = LLVMOrcMakeSharedModule(context->module);
 		if (LLVMOrcAddEagerlyCompiledIR(compile_orc, _handle, smod,
 		llvm_resolve_symbol, NULL))
@@ -392,6 +409,7 @@ llvm_compile_module(LLVMJitContext *context)
 			elog(ERROR, "failed to jit module");
 		}
 		LLVMOrcDisposeSharedModuleRef(smod);
+#endif
 		INSTR_TIME_SET_CURRENT(tb);
 		INSTR_TIME_SUBTRACT(tb, ta);
 		ereport(DEBUG1, (errmsg("time to emit: %.3fs",
-- 
2.15.1

>From 079ad7087e2ab106c0f04fa9056c93afa9a43b7c Mon Sep 17 00:00:00 2001
From: Pierre 
Date: Fri, 2 Feb 2018 09:13:40 +0100
Subject: [PATCH 2/4] Add LLVM4 support in llvmjit_error.cpp

---
 src/backend/lib/llvmjit_error.cpp | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/backend/lib/llvmjit_error.cpp b/src/backend/lib/llvmjit_error.cpp
index 70cecd114b..04e51b2a31 100644
--- a/src/backend/lib/llvmjit_error.cpp
+++ b/src/backend/lib/llvmjit_error.cpp
@@ -56,7 +56,9 @@ 

Re: JIT compiling with LLVM v9.1

[Craig Ringer]

On 29 January 2018 at 22:53, Andres Freund  wrote:

> Hi,
>
> On 2018-01-23 23:20:38 -0800, Andres Freund wrote:
> > == Code ==
> >
> > As the patchset is large (500kb) and I'm still quickly evolving it, I do
> > not yet want to attach it. The git tree is at
> >   https://git.postgresql.org/git/users/andresfreund/postgres.git
> > in the jit branch
> >   https://git.postgresql.org/gitweb/?p=users/andresfreund/
> postgres.git;a=shortlog;h=refs/heads/jit
>
> I've just pushed an updated and rebased version of the tree:
> - Split the large "jit infrastructure" commits into a number of smaller
>   commits
> - Split the C++ file
> - Dropped some of the performance stuff done to heaptuple.c - that was
>   mostly to make performance comparisons a bit more interesting, but
>   doesn't seem important enough to deal with.
> - Added a commit renaming datetime.h symbols so they don't conflict with
>   LLVM variables anymore, removing ugly #undef PM/#define PM dance
>   around includes. Will post separately.
> - Reduced the number of pointer constants in the generated LLVM IR, by
>   doing more getelementptr accesses (stem from before the time types
>   were automatically synced)
> - Increased number of comments a bit
>
> There's a jit-before-rebase-2018-01-29 tag, for the state of the tree
> before the rebase.


If you submit the C++ support separately I'd like to sign up as reviewer
and get that in. It's non-intrusive and just makes our existing c++
compilation support actually work properly. Your patch is a more complete
version of the C++ support I hacked up during linux.conf.au - I should've
thought to look in your tree.

The only part I had to add that I don't see in yours is a workaround for
mismatched throw() annotations on our redefinition of inet_net_ntop :


src/include/port.h:

@@ -421,7 +425,7 @@ extern int   pg_codepage_to_encoding(UINT cp);

 /* port/inet_net_ntop.c */
 extern char *inet_net_ntop(int af, const void *src, int bits,
-  char *dst, size_t size);
+  char *dst, size_t size) __THROW;


src/include/c.h:

@@ -1131,6 +1131,16 @@ extern intfdatasync(int fildes);
 #define NON_EXEC_STATIC static
 #endif

+/*
+ * glibc uses __THROW when compiling with the c++ compiler, but port.h
reclares
+ * inet_net_ntop. If we don't annotate it the same way as the prototype in
+ *  we'll upset g++, so we must use __THROW from
. If
+ * we're not on glibc, we need to define it away.
+ */
+#ifndef __GNU_LIBRARY__
+#define __THROW
+#endif
+
 /* /port compatibility functions */
 #include "port.h"


This might be better solved by renaming it to pg_inet_net_ntop so we don't
conflict with a standard name.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: JIT compiling with LLVM v9.1

[Andres Freund]

Hi,

On 2018-01-23 23:20:38 -0800, Andres Freund wrote:
> == Code ==
> 
> As the patchset is large (500kb) and I'm still quickly evolving it, I do
> not yet want to attach it. The git tree is at
>   https://git.postgresql.org/git/users/andresfreund/postgres.git
> in the jit branch
>   
> https://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/jit

I've just pushed an updated and rebased version of the tree:
- Split the large "jit infrastructure" commits into a number of smaller
  commits
- Split the C++ file
- Dropped some of the performance stuff done to heaptuple.c - that was
  mostly to make performance comparisons a bit more interesting, but
  doesn't seem important enough to deal with.
- Added a commit renaming datetime.h symbols so they don't conflict with
  LLVM variables anymore, removing ugly #undef PM/#define PM dance
  around includes. Will post separately.
- Reduced the number of pointer constants in the generated LLVM IR, by
  doing more getelementptr accesses (stem from before the time types
  were automatically synced)
- Increased number of comments a bit

There's a jit-before-rebase-2018-01-29 tag, for the state of the tree
before the rebase.

Regards,

Andres