Re: LLVM compile failing in seawasp
Let's just commit the #undef so that seawasp is green and back to being ready to tell us if something else breaks. +1. I was afraid that working around this would be impossibly painful ... but if it just takes one judiciously placed #undef, let's do that and not argue about it. Done. Seawasp is back to green. -- Fabien.
Re: LLVM compile failing in seawasp
Hi, On 2019-07-29 10:27:54 +1200, Thomas Munro wrote: > On Mon, Jul 29, 2019 at 9:55 AM Tom Lane wrote: > > Thomas Munro writes: > > > Let's just commit the #undef so that seawasp is green and back to > > > being ready to tell us if something else breaks. > > > > +1. I was afraid that working around this would be impossibly > > painful ... but if it just takes one judiciously placed #undef, > > let's do that and not argue about it. > > Done. cool, thanks. Greetings, Andres Freund
Re: LLVM compile failing in seawasp
On Mon, Jul 29, 2019 at 9:55 AM Tom Lane wrote: > Thomas Munro writes: > > Let's just commit the #undef so that seawasp is green and back to > > being ready to tell us if something else breaks. > > +1. I was afraid that working around this would be impossibly > painful ... but if it just takes one judiciously placed #undef, > let's do that and not argue about it. Done. -- Thomas Munro https://enterprisedb.com
Re: LLVM compile failing in seawasp
Thomas Munro writes: > Let's just commit the #undef so that seawasp is green and back to > being ready to tell us if something else breaks. +1. I was afraid that working around this would be impossibly painful ... but if it just takes one judiciously placed #undef, let's do that and not argue about it. regards, tom lane
Re: LLVM compile failing in seawasp
On Mon, Jul 29, 2019 at 8:03 AM Fabien COELHO wrote: > If reordering includes is not an option, too bad. Then "#undef Min" which > I find disputable, allthough I've done much worse... it might or might not > work depending on what is done afterwards. Or rename the macro, as I > suggested first, but there are many instances. Or convince LLVM people > that they should change their stuff. Or document that pg jit will cannot > use the latest LLVM, as a feature. Or find another solution:-) Let's just commit the #undef so that seawasp is green and back to being ready to tell us if something else breaks. Personally, I don't see any reason why should entertain a request to change their variable names to avoid our short common word macros that aren't even all-caps, but if someone asks them and they agree to do that before the final 9.0 release we can just revert. -- Thomas Munro https://enterprisedb.com
Re: LLVM compile failing in seawasp
Hello Tom, They should be fully independent anyway, so the order should not matter? On what grounds do you claim that's true anywhere, let alone everywhere? I mean that the intersection of Postgres realm, a database written in C, and LLVM realm, a compiler written in C++, should not interfere much one with the other, bar the jit compilation stuff which mixes both, so having one set of realm-specific includes before/after the other *should* not matter. Obviously the Min macro is a counter example of that, but that is indeed the problem to solve, and it is really accidental. It would be very unlucky if there was an issue the other way around. But maybe not. Anyway, I'm just trying to suggest a minimum fuss solution. One point of "seawasp" and "jellyfish" is to have early warning of compilation issues with future compilers, and it is serving this purpose beautifully. Another point is to detect compiler bugs early when compiling a significant project, and I have reported issues about both clang & gcc in the past, so it works there too. If reordering includes is not an option, too bad. Then "#undef Min" which I find disputable, allthough I've done much worse... it might or might not work depending on what is done afterwards. Or rename the macro, as I suggested first, but there are many instances. Or convince LLVM people that they should change their stuff. Or document that pg jit will cannot use the latest LLVM, as a feature. Or find another solution:-) -- Fabien.
Re: LLVM compile failing in seawasp
Fabien COELHO writes: > Otherwise, why not simply move llvm C++ includes *before* postgres > includes? We've been burnt in the past by putting other headers before postgres.h. (A typical issue is that the interpretation of varies depending on _LARGE_FILES or a similar macro, so you get problems if something causes that to be included before pg_config.h has set that macro.) Maybe none of the platforms where that's an issue have C++, but that doesn't seem like a great assumption. > They should be fully independent anyway, so the order should > not matter? On what grounds do you claim that's true anywhere, let alone everywhere? regards, tom lane
Re: LLVM compile failing in seawasp
Hello Thomas, I would just #undef Min for our small number of .cpp files that include LLVM headers. It's not as though you need it in C++, which has std::min() from . Like so. Fixes the problem for me (llvm-devel-9.0.d20190712). Hmmm. Not so nice, but if it works, why not, at least the impact is much smaller than renaming. Note that the Min macro is used in several pg headers (ginblock.h, ginxlog.h, hash.h, simplehash.h, spgist_private.h), so you might really need it depending on what is being done later. Otherwise, why not simply move llvm C++ includes *before* postgres includes? They should be fully independent anyway, so the order should not matter? -- Fabien.
Re: LLVM compile failing in seawasp
On Sat, Jul 27, 2019 at 7:12 PM Thomas Munro wrote: > On Sat, Jul 27, 2019 at 7:06 PM Fabien COELHO wrote: > > Maybe we should consider doing an explicit bug report, but I would not bet > > that they are going to fold… or fixing the issue pg side, eg "pg_Min", > > less than 400 hundred instances, and backpatch to all supported > > versions:-( > > I would just #undef Min for our small number of .cpp files that > include LLVM headers. It's not as though you need it in C++, which > has std::min() from . Like so. Fixes the problem for me (llvm-devel-9.0.d20190712). -- Thomas Munro https://enterprisedb.com 0001-Avoid-macro-clash-with-LLVM-9.patch Description: Binary data
Re: LLVM compile failing in seawasp
On Sat, Jul 27, 2019 at 7:06 PM Fabien COELHO wrote: > >>> c.h defines a C Min macro conflicting with llvm new class > >>> llvm:ElementCount Min member > >> > >> Really? Well, we will hardly be the only code they broke with that. > >> I think we can just wait for them to reconsider. > > > > FYI This is now on LLVM's release_90 branch, due out on August 28. > > Maybe we should consider doing an explicit bug report, but I would not bet > that they are going to fold… or fixing the issue pg side, eg "pg_Min", > less than 400 hundred instances, and backpatch to all supported > versions:-( I would just #undef Min for our small number of .cpp files that include LLVM headers. It's not as though you need it in C++, which has std::min() from . -- Thomas Munro https://enterprisedb.com
Re: LLVM compile failing in seawasp
c.h defines a C Min macro conflicting with llvm new class llvm:ElementCount Min member Really? Well, we will hardly be the only code they broke with that. I think we can just wait for them to reconsider. FYI This is now on LLVM's release_90 branch, due out on August 28. Maybe we should consider doing an explicit bug report, but I would not bet that they are going to fold… or fixing the issue pg side, eg "pg_Min", less than 400 hundred instances, and backpatch to all supported versions:-( -- Fabien.
Re: LLVM compile failing in seawasp
On Fri, Jun 7, 2019 at 12:13 PM Tom Lane wrote: > didier writes: > > c.h defines a C Min macro conflicting with llvm new class > > llvm:ElementCount Min member > > Really? Well, we will hardly be the only code they broke with that. > I think we can just wait for them to reconsider. FYI This is now on LLVM's release_90 branch, due out on August 28. -- Thomas Munro https://enterprisedb.com
Re: LLVM compile failing in seawasp
didier writes: > c.h defines a C Min macro conflicting with llvm new class > llvm:ElementCount Min member Really? Well, we will hardly be the only code they broke with that. I think we can just wait for them to reconsider. regards, tom lane
Re: LLVM compile failing in seawasp
failure). Apparently clang got upgraded from "trunk 361691" to "trunk 362290" ... is the new clang broken? I think that machine might also update llvm to a trunk checkout. Is that right Fabien? Yes, the version is recompiled from sources on every Saturday. -- Fabien.
Re: LLVM compile failing in seawasp
c.h defines a C Min macro conflicting with llvm new class
llvm:ElementCount Min member
On Thu, Jun 6, 2019 at 7:32 PM Alvaro Herrera wrote:
>
> Seawasp (using experimental clang 9.0) has been complaining of late:
>
> /home/fabien/clgtk/bin/clang -Wno-ignored-attributes -fno-strict-aliasing
> -fwrapv -O2 -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
> -D__STDC_CONSTANT_MACROS -D_DEBUG -D_GNU_SOURCE -I/home/fabien/clgtk/include
> -I../../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -flto=thin
> -emit-llvm -c -o llvmjit_types.bc llvmjit_types.c
> In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
> from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
> from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
> from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
> from llvmjit_inline.cpp:45:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:12: error: macro
> "Min" requires 2 arguments, but only 1 given
>: Min(Min), Scalable(Scalable) {}
> ^
> In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
> from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
> from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
> from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
> from llvmjit_inline.cpp:45:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h: In constructor
> \xe2\x80\x98llvm::ElementCount::ElementCount(unsigned int, bool)\xe2\x80\x99:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:13: error: expected
> \xe2\x80\x98(\xe2\x80\x99 before \xe2\x80\x98,\xe2\x80\x99 token
>: Min(Min), Scalable(Scalable) {}
> ^
> : recipe for target 'llvmjit_inline.o' failed
>
> This was working earlier, and as far as I can tell the cpluspluscheck
> fixes are not the cause (because those happened earlier than the first
> failure). Apparently clang got upgraded from "trunk 361691" to "trunk
> 362290" ... is the new clang broken?
>
> --
> Álvaro Herrera39°50'S 73°21'W
>
>
Re: LLVM compile failing in seawasp
Hi,
On 2019-06-06 13:32:16 -0400, Alvaro Herrera wrote:
> Seawasp (using experimental clang 9.0) has been complaining of late:
>
> /home/fabien/clgtk/bin/clang -Wno-ignored-attributes -fno-strict-aliasing
> -fwrapv -O2 -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
> -D__STDC_CONSTANT_MACROS -D_DEBUG -D_GNU_SOURCE -I/home/fabien/clgtk/include
> -I../../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -flto=thin
> -emit-llvm -c -o llvmjit_types.bc llvmjit_types.c
> In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
> from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
> from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
> from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
> from llvmjit_inline.cpp:45:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:12: error: macro
> "Min" requires 2 arguments, but only 1 given
>: Min(Min), Scalable(Scalable) {}
> ^
> In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
> from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
> from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
> from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
> from llvmjit_inline.cpp:45:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h: In constructor
> \xe2\x80\x98llvm::ElementCount::ElementCount(unsigned int, bool)\xe2\x80\x99:
> /home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:13: error: expected
> \xe2\x80\x98(\xe2\x80\x99 before \xe2\x80\x98,\xe2\x80\x99 token
>: Min(Min), Scalable(Scalable) {}
> ^
> : recipe for target 'llvmjit_inline.o' failed
>
> This was working earlier, and as far as I can tell the cpluspluscheck
> fixes are not the cause (because those happened earlier than the first
> failure). Apparently clang got upgraded from "trunk 361691" to "trunk
> 362290" ... is the new clang broken?
I think that machine might also update llvm to a trunk checkout. Is that
right Fabien? If so that's possible "just" a minor API break.
Greetings,
Andres Freund
LLVM compile failing in seawasp
Seawasp (using experimental clang 9.0) has been complaining of late:
/home/fabien/clgtk/bin/clang -Wno-ignored-attributes -fno-strict-aliasing
-fwrapv -O2 -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
-D__STDC_CONSTANT_MACROS -D_DEBUG -D_GNU_SOURCE -I/home/fabien/clgtk/include
-I../../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -flto=thin
-emit-llvm -c -o llvmjit_types.bc llvmjit_types.c
In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
from llvmjit_inline.cpp:45:
/home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:12: error: macro
"Min" requires 2 arguments, but only 1 given
: Min(Min), Scalable(Scalable) {}
^
In file included from /home/fabien/clgtk/include/llvm/ADT/DenseMapInfo.h:20:0,
from /home/fabien/clgtk/include/llvm/ADT/DenseMap.h:16,
from /home/fabien/clgtk/include/llvm/ADT/DenseSet.h:16,
from /home/fabien/clgtk/include/llvm/ADT/SetVector.h:23,
from llvmjit_inline.cpp:45:
/home/fabien/clgtk/include/llvm/Support/ScalableSize.h: In constructor
\xe2\x80\x98llvm::ElementCount::ElementCount(unsigned int, bool)\xe2\x80\x99:
/home/fabien/clgtk/include/llvm/Support/ScalableSize.h:27:13: error: expected
\xe2\x80\x98(\xe2\x80\x99 before \xe2\x80\x98,\xe2\x80\x99 token
: Min(Min), Scalable(Scalable) {}
^
: recipe for target 'llvmjit_inline.o' failed
This was working earlier, and as far as I can tell the cpluspluscheck
fixes are not the cause (because those happened earlier than the first
failure). Apparently clang got upgraded from "trunk 361691" to "trunk
362290" ... is the new clang broken?
--
Álvaro Herrera39°50'S 73°21'W
