Hi, On 2020-09-25 16:38:41 -0700, Andres Freund via llvm-dev wrote:> On 2020-09-24 16:34:30 -0700, Lang Hames wrote: > > If anyone wants to check out the OrcV1 removal branch and provide feedback > > now is the time. Otherwise I will aim to land the work in the mainline > > early next week. > > I'm trying to get it to work with postgres. Unfortunately this week was a bit less > productive than I had hoped... Hope to get there Mon/Tues.Have the basics working. Should I open a PR to your branch for what I got so far? There's currently two things I am still missing from the C API. The first is to expose more of symbol resolution. Postgres can be extended via dynamically loaded libraries, and we need to be careful to look up symbols in the correct library. In the generated JIT IR references to functions in such libraries use a distinguishable symbol name (see example below), indicating that they should not be looked up in the global namespace. Previously we used the symbol resolution callback to LLVMOrcAddEagerlyCompiledIR to implement this. If a symbol name was scoped to a library, we only searched inside that, rather than globally: /* * Attempt to resolve symbol, so LLVM can emit a reference to it. */ static uint64_t llvm_resolve_symbol(const char *symname, void *ctx) { ... llvm_split_symbol_name(symname, &modname, &funcname); ... if (modname) addr = (uintptr_t) load_external_function(modname, funcname, true, NULL); else addr = (uintptr_t) LLVMSearchForAddressOfSymbol(symname); ... return (uint64_t) addr; } I don't think that can be implemented with the current C API. Related to that, when hitting this case I currently get, on stderr: JIT session error: Symbols not found: [ pgextern./home/andres/build/postgres/dev-assert/vpath/src/test/regress/regress.so.pt_in_widget ] but currently that error is not bubbled up to the C API. There I just get: ERROR: XX000: failed to look up symbol "evalexpr_0_0": Failed to materialize symbols: { (main, { evalexpr_0_0 }) } (obviously the first part is postgres code) Do you have thoughts on what that should look like? The other part that I am still missing is replacment for LLVMCreateGDBRegistrationListener(); LLVMCreatePerfJITEventListener(); LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); I haven't yet looked at whether there is replacement infrastructure for this already. Is there? Will look after the next few meetings... Greetings, Andres Freund
Hi Andres, Have the basics working. Should I open a PR to your branch for what I> got so far?Sounds good to me. There's currently two things I am still missing from the C API.>The first is to expose more of symbol resolution. Postgres can be> extended via dynamically loaded libraries, and we need to be careful to > look up symbols in the correct library. In the generated JIT IR > references to functions in such libraries use a distinguishable symbol > name (see example below), indicating that they should not be looked up > in the global namespace.>From the example code I guess the namespacing for symbols from externallibraries is something like: "<modulename>.<symbolname>" ? This is what definition generators are for. We can add an API for attaching generators and provide facilities to define a generator in C. Here you'd want a generator with a map of module names to dylib handles. First you'd search for a handle for <modulename> (loading it if it didn't already exist) then you'd dlsym <symbolname> in that handle. Related to that, when hitting this case I currently get, on stderr:> JIT session error: Symbols not found: [ > pgextern./home/andres/build/postgres/dev-assert/vpath/src/test/regress/regress.so.pt_in_widget > ] > but currently that error is not bubbled up to the C API. There I just > get: > ERROR: XX000: failed to look up symbol "evalexpr_0_0": Failed to > materialize symbols: { (main, { evalexpr_0_0 }) }(obviously the first part is postgres code)> Do you have thoughts on what that should look like?We need an API to install an error reporter on the session: That's where the SymbolsNotFound error will be delivered. I'll add that in a minute. The other part that I am still missing is replacment for LLVMCreateGDBRegistrationListener();> LLVMCreatePerfJITEventListener(); > LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); > I haven't yet looked at whether there is replacement infrastructure for > this already. Is there? Will look after the next few meetings...You can continue to use LLVMCreateGDBRegistrationListener and LLVMCreatePerfJITEventListener from ExecutionEngine.h for now. We just need to add an OrcV2 equivalent of LLVMOrcRegisterJITEventListener. I'll do that today. -- Lang. On Tue, Sep 29, 2020 at 11:58 AM Andres Freund <andres at anarazel.de> wrote:> Hi, > > On 2020-09-25 16:38:41 -0700, Andres Freund via llvm-dev wrote: > > On 2020-09-24 16:34:30 -0700, Lang Hames wrote: > > > If anyone wants to check out the OrcV1 removal branch and provide > feedback > > > now is the time. Otherwise I will aim to land the work in the mainline > > > early next week. > > > > I'm trying to get it to work with postgres. Unfortunately this week was > a bit less > > productive than I had hoped... Hope to get there Mon/Tues. > > Have the basics working. Should I open a PR to your branch for what I > got so far? > > > There's currently two things I am still missing from the C API. > > The first is to expose more of symbol resolution. Postgres can be > extended via dynamically loaded libraries, and we need to be careful to > look up symbols in the correct library. In the generated JIT IR > references to functions in such libraries use a distinguishable symbol > name (see example below), indicating that they should not be looked up > >> in the global namespace. > > > Previously we used the symbol resolution callback to > LLVMOrcAddEagerlyCompiledIR to implement this. If a symbol name was > scoped to a library, we only searched inside that, rather than globally: > > /* > * Attempt to resolve symbol, so LLVM can emit a reference to it. > */ > static uint64_t > llvm_resolve_symbol(const char *symname, void *ctx) > { > ... > llvm_split_symbol_name(symname, &modname, &funcname); > ... > > if (modname) > addr = (uintptr_t) load_external_function(modname, > funcname, > true, NULL); > else > addr = (uintptr_t) LLVMSearchForAddressOfSymbol(symname); > ... > return (uint64_t) addr; > } > > I don't think that can be implemented with the current C API. > > > Related to that, when hitting this case I currently get, on stderr: > JIT session error: Symbols not found: [ > pgextern./home/andres/build/postgres/dev-assert/vpath/src/test/regress/regress.so.pt_in_widget > ] > > but currently that error is not bubbled up to the C API. There I just > get: > ERROR: XX000: failed to look up symbol "evalexpr_0_0": Failed to > materialize symbols: { (main, { evalexpr_0_0 }) } > > (obviously the first part is postgres code) > > Do you have thoughts on what that should look like? > > > The other part that I am still missing is replacment for > > LLVMCreateGDBRegistrationListener(); > LLVMCreatePerfJITEventListener(); > LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); > > I haven't yet looked at whether there is replacement infrastructure for > this already. Is there? Will look after the next few meetings... > > Greetings, > > Andres Freund >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200929/ab4c2775/attachment.html>
Hi Andres, ... I'll add that in a minute.> ... I'll do that today.That may have been overly optimistic -- I had to jump on to another bug for a bit. I'll try to get these done in the next day or so though. -- Lang. On Tue, Sep 29, 2020 at 1:58 PM Lang Hames <lhames at gmail.com> wrote:> Hi Andres, > > Have the basics working. Should I open a PR to your branch for what I >> got so far? > > > Sounds good to me. > > There's currently two things I am still missing from the C API. >> > > > The first is to expose more of symbol resolution. Postgres can be >> extended via dynamically loaded libraries, and we need to be careful to >> look up symbols in the correct library. In the generated JIT IR >> references to functions in such libraries use a distinguishable symbol >> name (see example below), indicating that they should not be looked up >> in the global namespace. > > > From the example code I guess the namespacing for symbols from external > libraries is something like: "<modulename>.<symbolname>" ? > > This is what definition generators are for. We can add an API for > attaching generators and provide facilities to define a generator in C. > > Here you'd want a generator with a map of module names to dylib handles. > First you'd search for a handle for <modulename> (loading it if it didn't > already exist) then you'd dlsym <symbolname> in that handle. > > Related to that, when hitting this case I currently get, on stderr: >> JIT session error: Symbols not found: [ >> pgextern./home/andres/build/postgres/dev-assert/vpath/src/test/regress/regress.so.pt_in_widget >> ] >> but currently that error is not bubbled up to the C API. There I just >> get: >> ERROR: XX000: failed to look up symbol "evalexpr_0_0": Failed to >> materialize symbols: { (main, { evalexpr_0_0 }) } > > (obviously the first part is postgres code) >> Do you have thoughts on what that should look like? > > > We need an API to install an error reporter on the session: That's where > the SymbolsNotFound error will be delivered. I'll add that in a minute. > > The other part that I am still missing is replacment for > > LLVMCreateGDBRegistrationListener(); >> LLVMCreatePerfJITEventListener(); >> LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); >> I haven't yet looked at whether there is replacement infrastructure for >> this already. Is there? Will look after the next few meetings... > > > You can continue to use LLVMCreateGDBRegistrationListener and > LLVMCreatePerfJITEventListener from ExecutionEngine.h for now. We just need > to add an OrcV2 equivalent of LLVMOrcRegisterJITEventListener. I'll do that > today. > > -- Lang. > > On Tue, Sep 29, 2020 at 11:58 AM Andres Freund <andres at anarazel.de> wrote: > >> Hi, >> >> On 2020-09-25 16:38:41 -0700, Andres Freund via llvm-dev wrote: >> > On 2020-09-24 16:34:30 -0700, Lang Hames wrote: >> > > If anyone wants to check out the OrcV1 removal branch and provide >> feedback >> > > now is the time. Otherwise I will aim to land the work in the mainline >> > > early next week. >> > >> > I'm trying to get it to work with postgres. Unfortunately this week was >> a bit less >> > productive than I had hoped... Hope to get there Mon/Tues. >> >> Have the basics working. Should I open a PR to your branch for what I >> got so far? >> >> >> There's currently two things I am still missing from the C API. >> >> The first is to expose more of symbol resolution. Postgres can be >> extended via dynamically loaded libraries, and we need to be careful to >> look up symbols in the correct library. In the generated JIT IR >> references to functions in such libraries use a distinguishable symbol >> name (see example below), indicating that they should not be looked up >> >>> in the global namespace. >> >> >> Previously we used the symbol resolution callback to >> LLVMOrcAddEagerlyCompiledIR to implement this. If a symbol name was >> scoped to a library, we only searched inside that, rather than globally: >> >> /* >> * Attempt to resolve symbol, so LLVM can emit a reference to it. >> */ >> static uint64_t >> llvm_resolve_symbol(const char *symname, void *ctx) >> { >> ... >> llvm_split_symbol_name(symname, &modname, &funcname); >> ... >> >> if (modname) >> addr = (uintptr_t) load_external_function(modname, >> funcname, >> true, NULL); >> else >> addr = (uintptr_t) LLVMSearchForAddressOfSymbol(symname); >> ... >> return (uint64_t) addr; >> } >> >> I don't think that can be implemented with the current C API. >> >> >> Related to that, when hitting this case I currently get, on stderr: >> JIT session error: Symbols not found: [ >> pgextern./home/andres/build/postgres/dev-assert/vpath/src/test/regress/regress.so.pt_in_widget >> ] >> >> but currently that error is not bubbled up to the C API. There I just >> get: >> ERROR: XX000: failed to look up symbol "evalexpr_0_0": Failed to >> materialize symbols: { (main, { evalexpr_0_0 }) } >> >> (obviously the first part is postgres code) >> >> Do you have thoughts on what that should look like? >> >> >> The other part that I am still missing is replacment for >> >> LLVMCreateGDBRegistrationListener(); >> LLVMCreatePerfJITEventListener(); >> LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); >> >> I haven't yet looked at whether there is replacement infrastructure for >> this already. Is there? Will look after the next few meetings... >> >> Greetings, >> >> Andres Freund >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200929/a4c21e0c/attachment.html>
Hi, On 2020-09-29 13:58:35 -0700, Lang Hames wrote:> > Have the basics working. Should I open a PR to your branch for what I > > got so far?> Sounds good to me.Done, https://github.com/lhames/llvm-project/pull/2 .> The first is to expose more of symbol resolution. Postgres can be > > extended via dynamically loaded libraries, and we need to be careful to > > look up symbols in the correct library. In the generated JIT IR > > references to functions in such libraries use a distinguishable symbol > > name (see example below), indicating that they should not be looked up > > in the global namespace. > > > From the example code I guess the namespacing for symbols from external > libraries is something like: "<modulename>.<symbolname>" ?It's currently pgextern.<path_to_library>.<symbolname>> This is what definition generators are for. We can add an API for attaching > generators and provide facilities to define a generator in C. > Here you'd want a generator with a map of module names to dylib > handles. > First you'd search for a handle for <modulename> (loading it if it didn't > already exist) then you'd dlsym <symbolname> in that handle.Ok, cool.> Related to that, when hitting this case I currently get, on stderr: > > JIT session error: Symbols not found: [ > > pgextern./home/andres/build/postgres/dev-assert/vpath/src/test/regress/regress.so.pt_in_widget > > ] > > but currently that error is not bubbled up to the C API. There I just > > get: > > ERROR: XX000: failed to look up symbol "evalexpr_0_0": Failed to > > materialize symbols: { (main, { evalexpr_0_0 }) } > > (obviously the first part is postgres code) > > Do you have thoughts on what that should look like? > > > We need an API to install an error reporter on the session: That's where > the SymbolsNotFound error will be delivered. I'll add that in a minute.Sounds good.> > The other part that I am still missing is replacment for > > LLVMCreateGDBRegistrationListener(); > > LLVMCreatePerfJITEventListener(); > > LLVMOrcRegisterJITEventListener(llvm_opt0_orc, l); > > I haven't yet looked at whether there is replacement infrastructure for > > this already. Is there? Will look after the next few meetings...> You can continue to use LLVMCreateGDBRegistrationListener and > LLVMCreatePerfJITEventListener from ExecutionEngine.h for now. We just need > to add an OrcV2 equivalent of LLVMOrcRegisterJITEventListener. I'll do that > today.Cool. Greetings, Andres Freund