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) {}
^
<builtin>: 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 Herrera 39�50'S 73�21'W
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) {}
^
<builtin>: recipe for target 'llvmjit_inline.o' failedThis 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
c.h defines a C Min macro conflicting with llvm new class
llvm:ElementCount Min member
Show quoted text
On Thu, Jun 6, 2019 at 7:32 PM Alvaro Herrera <alvherre@2ndquadrant.com> 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) {}
^
<builtin>: recipe for target 'llvmjit_inline.o' failedThis 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 Herrera 39°50'S 73°21'W
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.
didier <did447@gmail.com> 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
On Fri, Jun 7, 2019 at 12:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
didier <did447@gmail.com> writes:
c.h defines a C Min macro conflicting with llvm new class
llvm:ElementCount Min memberReally? 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
c.h defines a C Min macro conflicting with llvm new class
llvm:ElementCount Min memberReally? 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.
On Sat, Jul 27, 2019 at 7:06 PM Fabien COELHO <coelho@cri.ensmp.fr> wrote:
c.h defines a C Min macro conflicting with llvm new class
llvm:ElementCount Min memberReally? 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 <algorithm>.
--
Thomas Munro
https://enterprisedb.com
On Sat, Jul 27, 2019 at 7:12 PM Thomas Munro <thomas.munro@gmail.com> wrote:
On Sat, Jul 27, 2019 at 7:06 PM Fabien COELHO <coelho@cri.ensmp.fr> 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 <algorithm>.
Like so. Fixes the problem for me (llvm-devel-9.0.d20190712).
--
Thomas Munro
https://enterprisedb.com
Attachments:
0001-Avoid-macro-clash-with-LLVM-9.patchapplication/octet-stream; name=0001-Avoid-macro-clash-with-LLVM-9.patchDownload
From f63802e7e8eba0114dc103bd35bd8800ef5f1ed0 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sat, 27 Jul 2019 21:16:01 +1200
Subject: [PATCH] Avoid macro clash with LLVM 9.
Early previews of LLVM 9 reveal that our Min() macro causes compiler
errors in LLVM headers reached by the include directives in
llvm_inline.cpp. Let's just undefine it.
Discussion: https://postgr.es/m/20190606173216.GA6306%40alvherre.pgsql
---
src/backend/jit/llvm/llvmjit_inline.cpp | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/backend/jit/llvm/llvmjit_inline.cpp b/src/backend/jit/llvm/llvmjit_inline.cpp
index 8005d43a84..1ac1d9deae 100644
--- a/src/backend/jit/llvm/llvmjit_inline.cpp
+++ b/src/backend/jit/llvm/llvmjit_inline.cpp
@@ -24,6 +24,9 @@ extern "C"
#include "postgres.h"
}
+/* Avoid macro clash with LLVM headers */
+#undef Min
+
#include "jit/llvmjit.h"
extern "C"
--
2.22.0
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 <algorithm>.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.
Fabien COELHO <coelho@cri.ensmp.fr> 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 <stdio.h> 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
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.
On Mon, Jul 29, 2019 at 8:03 AM Fabien COELHO <coelho@cri.ensmp.fr> 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 <random other project> 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
Thomas Munro <thomas.munro@gmail.com> 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
On Mon, Jul 29, 2019 at 9:55 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Thomas Munro <thomas.munro@gmail.com> 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
Hi,
On 2019-07-29 10:27:54 +1200, Thomas Munro wrote:
On Mon, Jul 29, 2019 at 9:55 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Thomas Munro <thomas.munro@gmail.com> 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
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.