Re: Fix compilation failure against LLVM 11
Hi, On 2020-05-27 07:49:45 -0700, Jesse Zhang wrote: > On the mensiversary of the last response, what can I do to help move > this along (aside from heeding the advice "don't use LLVM HEAD")? Sorry, I had looked at it at point with the intent to commit it, and hit some stupid small snags (*). And then I forgot about it. Pushed. Thanks for the patch! Andres Freund (*) first I didn't see the problem, because I accidentally had an old version of the header around. Then I couldn't immediately build old versions of pg + LLVM due to my existing installation needing to be rebuilt. Then there were compiler errors, due to a too new compiler. Etc.
Re: Fix compilation failure against LLVM 11
On 2020-05-28 17:07:46 +0900, Michael Paquier wrote: > Please note that I would still wait for their next GA release to plug > in any extra holes at the same time. @Jesse: or is this change > actually part of the upcoming 10.0.1? Why? I plan to just commit this change now.
Re: Fix compilation failure against LLVM 11
On Thu, May 28, 2020 at 1:07 AM Michael Paquier wrote: > Please note that I would still wait for their next GA release to plug > in any extra holes at the same time. @Jesse: or is this change > actually part of the upcoming 10.0.1? No a refactoring like this was not in the back branches (nor is it expected). Thanks, Jesse
Re: Fix compilation failure against LLVM 11
On Wed, May 27, 2020 at 07:49:45AM -0700, Jesse Zhang wrote: > For bystanders: Andres and I argued for "fixing this sooner and > backpatch" and Michael suggested "wait longer and whack all moles". We > have waited, and there seems to be only one mole (finding all dead > unbroken "include"s was left as an exercise for the reader). Have we > come to an agreement on this? If Andres could take care of this issue as he feels is suited, that's OK for me. I could look at that and play with llvm builds. Now I am not really familiar with it, so it would take me some time but we are all here to learn :) Please note that I would still wait for their next GA release to plug in any extra holes at the same time. @Jesse: or is this change actually part of the upcoming 10.0.1? -- Michael signature.asc Description: PGP signature
Re: Fix compilation failure against LLVM 11
Hi Andres, On the mensiversary of the last response, what can I do to help move this along (aside from heeding the advice "don't use LLVM HEAD")? Michael Paquier expressed concerns over flippant churn upthread: On Mon, Apr 27, 2020 at 10:19 PM Andres Freund wrote: AF> On 2020-04-28 13:56:23 +0900, Michael Paquier wrote: MP> > On Mon, Apr 27, 2020 at 07:48:54AM -0700, Jesse Zhang wrote: JZ> > > Are you expressing a concern against "churning" this part of the JZ> > > code in reaction to upstream LLVM changes? I'd agree with you in JZ> > > general. But then the question we need to ask is "will we need JZ> > > to revert this 3 weeks from now if upstream reverts their JZ> > > changes?", or "we change X to Y now, will we need to instead JZ> > > change X to Z 3 weeks later?". > > MP> > My concerns are a mix of all that, because we may finish by doing MP> > the same verification work multiple times instead of fixing all MP> > existing issues at once. A good thing is that we may be able to MP> > conclude rather soon, it looks like LLVM releases a new major MP> > version every 3 months or so. > AF> Given the low cost of a change like this, and the fact that we have AF> a buildfarm animal building recent trunk versions of llvm, I think AF> it's better to just backpatch now. For bystanders: Andres and I seemed to agree that this is unlikely to be flippant (in addition to other benefits mentioned below). We haven't discussed more on this, though I'm uncertain we had a consensus. > JZ> > > In that frame of mind, the answer is simply "no" w.r.t this JZ> > > patch: it's removing an #include that simply has been dead: the JZ> > > upstream change merely exposed it. > > MP> > The docs claim support for LLVM down to 3.9. Are versions older MP> > than 8 fine with your proposed change? > AF> I'll check. > How goes the checking? I was 99% certain it'd work but that might have been my excuse to be lazy. Do you need help on this? > JZ> > > OTOH, is your concern more around "how many more dead #include JZ> > > will LLVM 11 reveal before September?", I'm open to suggestions. JZ> > > I personally have a bias to keep things working. > > MP> > This position can have advantages, though it seems to me that we MP> > should still wait to see if there are more issues popping up. > AF> What's the benefit? The cost of checking this will be not AF> meaningfully lower if there's other things to check as well, given AF> their backward compat story presumably is different. For bystanders: Andres and I argued for "fixing this sooner and backpatch" and Michael suggested "wait longer and whack all moles". We have waited, and there seems to be only one mole (finding all dead unbroken "include"s was left as an exercise for the reader). Have we come to an agreement on this? Cheers, Jesse
Re: Fix compilation failure against LLVM 11
Hi Michael, On Mon, Apr 27, 2020 at 9:56 PM Michael Paquier wrote: > rather soon, it looks like LLVM releases a new major version every 3 > months or so. FYI LLVM has a six-month release cadence [1], the next release is expected coming September (I can't tell whether you were joking). Cheers, Jesse [1] https://llvm.org/docs/HowToReleaseLLVM.html#release-timeline
Re: Fix compilation failure against LLVM 11
Hi, On 2020-04-28 13:56:23 +0900, Michael Paquier wrote: > On Mon, Apr 27, 2020 at 07:48:54AM -0700, Jesse Zhang wrote: > > Are you expressing a concern against "churning" this part of the code in > > reaction to upstream LLVM changes? I'd agree with you in general. But > > then the question we need to ask is "will we need to revert this 3 weeks > > from now if upstream reverts their changes?", or "we change X to Y now, > > will we need to instead change X to Z 3 weeks later?". > > My concerns are a mix of all that, because we may finish by doing the > same verification work multiple times instead of fixing all existing > issues at once. A good thing is that we may be able to conclude > rather soon, it looks like LLVM releases a new major version every 3 > months or so. Given the low cost of a change like this, and the fact that we have a buildfarm animal building recent trunk versions of llvm, I think it's better to just backpatch now. > > In that frame of > > mind, the answer is simply "no" w.r.t this patch: it's removing an > > #include that simply has been dead: the upstream change merely exposed > > it. > > The docs claim support for LLVM down to 3.9. Are versions older than > 8 fine with your proposed change? I'll check. > > OTOH, is your concern more around "how many more dead #include will LLVM > > 11 reveal before September?", I'm open to suggestions. I personally have > > a bias to keep things working. > > This position can have advantages, though it seems to me that we > should still wait to see if there are more issues popping up. What's the benefit? The cost of checking this will be not meaningfully lower if there's other things to check as well, given their backward compat story presumably is different. Greetings, Andres Freund
Re: Fix compilation failure against LLVM 11
On Mon, Apr 27, 2020 at 07:48:54AM -0700, Jesse Zhang wrote: > Are you expressing a concern against "churning" this part of the code in > reaction to upstream LLVM changes? I'd agree with you in general. But > then the question we need to ask is "will we need to revert this 3 weeks > from now if upstream reverts their changes?", or "we change X to Y now, > will we need to instead change X to Z 3 weeks later?". My concerns are a mix of all that, because we may finish by doing the same verification work multiple times instead of fixing all existing issues at once. A good thing is that we may be able to conclude rather soon, it looks like LLVM releases a new major version every 3 months or so. > In that frame of > mind, the answer is simply "no" w.r.t this patch: it's removing an > #include that simply has been dead: the upstream change merely exposed > it. The docs claim support for LLVM down to 3.9. Are versions older than 8 fine with your proposed change? > OTOH, is your concern more around "how many more dead #include will LLVM > 11 reveal before September?", I'm open to suggestions. I personally have > a bias to keep things working. This position can have advantages, though it seems to me that we should still wait to see if there are more issues popping up. -- Michael signature.asc Description: PGP signature
Re: Fix compilation failure against LLVM 11
Hi Michael, On Sun, Apr 26, 2020 at 11:21 PM Michael Paquier wrote: > > On Sat, Apr 25, 2020 at 09:41:20PM -0700, Jesse Zhang wrote: > > I searched my inbox and the archive, strange that nobody else is seeing > > this. > > > > Turns out that LLVM has recently removed "llvm/IR/CallSite.h" in > > (unreleased) version 11 [1][2]. To fix the build I tried conditionally > > (on LLVM_VERSION_MAJOR < 11) including CallSite.h, but that looks yuck. > > Then I poked at llvmjit_inline.cpp a bit and found that CallSite.h > > doesn't seem to be really necessary. PFA a patch that simply removes > > this #include. > > > > In addition, I've done the due dilligence of trying to build against > > LLVM versions 8, 9, 10. > > LLVM 11 has not been released yet. Do you think that this part or > even more are subject to change before the 11 release? My take would > be to wait more before fixing this issue and make sure that our code > is fixed when their code is GA'd. > -- > Michael Are you expressing a concern against "churning" this part of the code in reaction to upstream LLVM changes? I'd agree with you in general. But then the question we need to ask is "will we need to revert this 3 weeks from now if upstream reverts their changes?", or "we change X to Y now, will we need to instead change X to Z 3 weeks later?". In that frame of mind, the answer is simply "no" w.r.t this patch: it's removing an #include that simply has been dead: the upstream change merely exposed it. OTOH, is your concern more around "how many more dead #include will LLVM 11 reveal before September?", I'm open to suggestions. I personally have a bias to keep things working. Cheers, Jesse
Re: Fix compilation failure against LLVM 11
On Sat, Apr 25, 2020 at 09:41:20PM -0700, Jesse Zhang wrote: > I searched my inbox and the archive, strange that nobody else is seeing > this. > > Turns out that LLVM has recently removed "llvm/IR/CallSite.h" in > (unreleased) version 11 [1][2]. To fix the build I tried conditionally > (on LLVM_VERSION_MAJOR < 11) including CallSite.h, but that looks yuck. > Then I poked at llvmjit_inline.cpp a bit and found that CallSite.h > doesn't seem to be really necessary. PFA a patch that simply removes > this #include. > > In addition, I've done the due dilligence of trying to build against > LLVM versions 8, 9, 10. LLVM 11 has not been released yet. Do you think that this part or even more are subject to change before the 11 release? My take would be to wait more before fixing this issue and make sure that our code is fixed when their code is GA'd. -- Michael signature.asc Description: PGP signature
Fix compilation failure against LLVM 11
Hi hackers, My local build of master started failing last night with this error: llvmjit_inline.cpp:59:10: fatal error: 'llvm/IR/CallSite.h' file not found #include ^~~~ I searched my inbox and the archive, strange that nobody else is seeing this. Turns out that LLVM has recently removed "llvm/IR/CallSite.h" in (unreleased) version 11 [1][2]. To fix the build I tried conditionally (on LLVM_VERSION_MAJOR < 11) including CallSite.h, but that looks yuck. Then I poked at llvmjit_inline.cpp a bit and found that CallSite.h doesn't seem to be really necessary. PFA a patch that simply removes this #include. In addition, I've done the due dilligence of trying to build against LLVM versions 8, 9, 10. Cheers, Jesse [1] LLVM Differential Revision: https://reviews.llvm.org/D78794 [2] LLVM commit https://github.com/llvm/llvm-project/commit/2c24051bacd2 "[CallSite removal] Rename CallSite.h to AbstractCallSite.h. NFC" From 27084a0ebd37176f0a831042457e27cad8ac70eb Mon Sep 17 00:00:00 2001 From: Jesse Zhang Date: Sat, 25 Apr 2020 16:24:33 -0700 Subject: [PATCH] Remove unused include LLVM has recently removed "llvm/IR/CallSite.h" in (unreleased) version 11 [1]. Instead of using preprocessor conditionals, just remove it because it doesn't seem like AbstractCallSite (or none of the other symbols declared in the header) is used by LLVM JIT. [1] LLVM Differential Revision: https://reviews.llvm.org/D78794 --- src/backend/jit/llvm/llvmjit_inline.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/backend/jit/llvm/llvmjit_inline.cpp b/src/backend/jit/llvm/llvmjit_inline.cpp index 95d4d854f43..2617a461cad 100644 --- a/src/backend/jit/llvm/llvmjit_inline.cpp +++ b/src/backend/jit/llvm/llvmjit_inline.cpp @@ -56,7 +56,6 @@ extern "C" #include #endif #include -#include #include #include #include -- 2.26.2
