Обсуждение: clang 15 doesn't like our JIT code
According to https://bugzilla.redhat.com/show_bug.cgi?id=2127503 bleeding-edge clang complains thusly: llvmjit_inline.cpp: In function 'std::unique_ptr<llvm::ModuleSummaryIndex> llvm_load_summary(llvm::StringRef)': llvmjit_inline.cpp:771:37: error: incomplete type 'llvm::MemoryBuffer' used in nested name specifier 771 | llvm::MemoryBuffer::getFile(path); | ^~~~~~~ In file included from /usr/include/c++/12/memory:76, from /usr/include/llvm/ADT/SmallVector.h:28, from /usr/include/llvm/ADT/ArrayRef.h:14, from /usr/include/llvm/ADT/SetVector.h:23, from llvmjit_inline.cpp:48: /usr/include/c++/12/bits/unique_ptr.h: In instantiation of 'void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp= llvm::MemoryBuffer]': /usr/include/c++/12/bits/unique_ptr.h:396:17: required from 'std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = llvm::MemoryBuffer;_Dp = std::default_delete<llvm::MemoryBuffer>]' /usr/include/llvm/Support/ErrorOr.h:142:34: required from 'llvm::ErrorOr<T>::~ErrorOr() [with T = std::unique_ptr<llvm::MemoryBuffer>]' llvmjit_inline.cpp:771:35: required from here /usr/include/c++/12/bits/unique_ptr.h:93:23: error: invalid application of 'sizeof' to incomplete type 'llvm::MemoryBuffer' 93 | static_assert(sizeof(_Tp)>0, | ^~~~~~~~~~~ I suspect this is less about clang and more about LLVM APIs, but anyway it seems like we gotta fix something. regards, tom lane
Hi, On 2022-09-16 11:40:46 -0400, Tom Lane wrote: > According to > > https://bugzilla.redhat.com/show_bug.cgi?id=2127503 > > bleeding-edge clang complains thusly: > > llvmjit_inline.cpp: In function 'std::unique_ptr<llvm::ModuleSummaryIndex> llvm_load_summary(llvm::StringRef)': > llvmjit_inline.cpp:771:37: error: incomplete type 'llvm::MemoryBuffer' used in nested name specifier > 771 | llvm::MemoryBuffer::getFile(path); > | ^~~~~~~ > In file included from /usr/include/c++/12/memory:76, > from /usr/include/llvm/ADT/SmallVector.h:28, > from /usr/include/llvm/ADT/ArrayRef.h:14, > from /usr/include/llvm/ADT/SetVector.h:23, > from llvmjit_inline.cpp:48: > /usr/include/c++/12/bits/unique_ptr.h: In instantiation of 'void std::default_delete<_Tp>::operator()(_Tp*) const [with_Tp = llvm::MemoryBuffer]': > /usr/include/c++/12/bits/unique_ptr.h:396:17: required from 'std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = llvm::MemoryBuffer;_Dp = std::default_delete<llvm::MemoryBuffer>]' > /usr/include/llvm/Support/ErrorOr.h:142:34: required from 'llvm::ErrorOr<T>::~ErrorOr() [with T = std::unique_ptr<llvm::MemoryBuffer>]' > llvmjit_inline.cpp:771:35: required from here > /usr/include/c++/12/bits/unique_ptr.h:93:23: error: invalid application of 'sizeof' to incomplete type 'llvm::MemoryBuffer' > 93 | static_assert(sizeof(_Tp)>0, > | ^~~~~~~~~~~ > > I suspect this is less about clang and more about LLVM APIs, > but anyway it seems like we gotta fix something. Yea, there's definitely a bunch of llvm 15 issues that need to be fixed - this particular failure is pretty easy to fix, but there's some others that are harder. They redesigned a fairly core part of the IR representation. Thomas has a WIP fix, I think. Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2022-09-16 11:40:46 -0400, Tom Lane wrote:
>> I suspect this is less about clang and more about LLVM APIs,
>> but anyway it seems like we gotta fix something.
> Yea, there's definitely a bunch of llvm 15 issues that need to be fixed - this
> particular failure is pretty easy to fix, but there's some others that are
> harder. They redesigned a fairly core part of the IR representation. Thomas
> has a WIP fix, I think.
I'm more and more getting the feeling that we're interfacing with LLVM
at too low a level, because it seems like our code is constantly breaking.
Do they just not have any stable API at all?
regards, tom lane
Hi, On 2022-09-16 16:07:51 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2022-09-16 11:40:46 -0400, Tom Lane wrote: > >> I suspect this is less about clang and more about LLVM APIs, > >> but anyway it seems like we gotta fix something. > > > Yea, there's definitely a bunch of llvm 15 issues that need to be fixed - this > > particular failure is pretty easy to fix, but there's some others that are > > harder. They redesigned a fairly core part of the IR representation. Thomas > > has a WIP fix, I think. > > I'm more and more getting the feeling that we're interfacing with LLVM > at too low a level, because it seems like our code is constantly breaking. > Do they just not have any stable API at all? I don't think it's the wrong level. While LLVM has a subset of the API that's supposed to be stable, and we mostly use only that subset, they've definitely are breaking it more and more frequently. Based on my observation that's because more and more of the development is done by google and facebook, which internally use monorepos, and vendor LLVM - that kind of model makes API changes much less of an issue. OTOH, the IR breakage (and a few prior related ones) is about fixing a design issue they've been talking about fixing for 10+ years... Greetings, Andres Freund
On Sat, Sep 17, 2022 at 6:45 AM Andres Freund <andres@anarazel.de> wrote: > On 2022-09-16 11:40:46 -0400, Tom Lane wrote: > > According to > > > > https://bugzilla.redhat.com/show_bug.cgi?id=2127503 > > > > bleeding-edge clang complains thusly: > > > > llvmjit_inline.cpp: In function 'std::unique_ptr<llvm::ModuleSummaryIndex> llvm_load_summary(llvm::StringRef)': > > llvmjit_inline.cpp:771:37: error: incomplete type 'llvm::MemoryBuffer' used in nested name specifier > > 771 | llvm::MemoryBuffer::getFile(path); > > | ^~~~~~~ > > In file included from /usr/include/c++/12/memory:76, > > from /usr/include/llvm/ADT/SmallVector.h:28, > > from /usr/include/llvm/ADT/ArrayRef.h:14, > > from /usr/include/llvm/ADT/SetVector.h:23, > > from llvmjit_inline.cpp:48: > > /usr/include/c++/12/bits/unique_ptr.h: In instantiation of 'void std::default_delete<_Tp>::operator()(_Tp*) const [with_Tp = llvm::MemoryBuffer]': > > /usr/include/c++/12/bits/unique_ptr.h:396:17: required from 'std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = llvm::MemoryBuffer;_Dp = std::default_delete<llvm::MemoryBuffer>]' > > /usr/include/llvm/Support/ErrorOr.h:142:34: required from 'llvm::ErrorOr<T>::~ErrorOr() [with T = std::unique_ptr<llvm::MemoryBuffer>]' > > llvmjit_inline.cpp:771:35: required from here > > /usr/include/c++/12/bits/unique_ptr.h:93:23: error: invalid application of 'sizeof' to incomplete type 'llvm::MemoryBuffer' > > 93 | static_assert(sizeof(_Tp)>0, > > | ^~~~~~~~~~~ > > > > I suspect this is less about clang and more about LLVM APIs, > > but anyway it seems like we gotta fix something. > > Yea, there's definitely a bunch of llvm 15 issues that need to be fixed - this > particular failure is pretty easy to fix, but there's some others that are > harder. They redesigned a fairly core part of the IR representation. Thomas > has a WIP fix, I think. Yes, I've been working on this and will try to have a patch on the list in a few days. There are also a few superficial changes to names, arguments, headers etc like the one reported there, but the real problem is that it aborts at runtime when JIT stuff happens, so I didn't want to push changes for the superficial things without addressing that or someone might get a nasty surprise. Separately, there's also the walker stuff[1] to address. [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGKpHPDTv67Y%2Bs6yiC8KH5OXeDg6a-twWo_xznKTcG0kSA%40mail.gmail.com
Thomas Munro <thomas.munro@gmail.com> writes:
> there's also the walker stuff[1] to address.
Yeah. I just did some experimentation with that, and it looks like
neither gcc nor clang will cut you any slack at all for declaring
an argument as "void *": given say
typedef bool (*tree_walker_callback) (Node *node, void *context);
the walker functions also have to be declared with exactly "void *"
as their second argument. So it's going to be just as messy and
full-of-casts as we feared. Still, I'm not sure we have any
alternative.
regards, tom lane