Horton

Thread

Commits

  1. Use appropriate -Wno-warning switches when compiling bitcode.

  1. Mixing CC and a different CLANG seems like a bad idea

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-11-18T16:56:59Z

    I noticed that, a week after Michael pushed 9ff47ea41 to silence
-Wcompound-token-split-by-macro warnings, buildfarm member sidewinder
is still spewing them.  Investigation shows that it's building with

configure: using compiler=cc (nb4 20200810) 7.5.0
configure: using CLANG=ccache clang

and the system cc doesn't know -Wcompound-token-split-by-macro,
so we don't use it, but the modules that are built into bytecode
still produce the warnings because they're built with clang.

I think this idea of using clang with switches selected for some other
compiler is completely horrid, and needs to be nuked from orbit before
it causes problems worse than mere warnings.  Why did we not simply
insist that if you want to use --with-llvm, the selected compiler must
be clang?  I cannot see any benefit of mix-and-match here.

			regards, tom lane

  2. Re: Mixing CC and a different CLANG seems like a bad idea

    Mikael Kjellström <mikael.kjellstrom@mksoft.nu> — 2021-11-18T17:24:36Z

    On 2021-11-18 17:56, Tom Lane wrote:
> I noticed that, a week after Michael pushed 9ff47ea41 to silence
> -Wcompound-token-split-by-macro warnings, buildfarm member sidewinder
> is still spewing them.  Investigation shows that it's building with
> 
> configure: using compiler=cc (nb4 20200810) 7.5.0
> configure: using CLANG=ccache clang


Hm, actually it's:

CC => "ccache cc",
CXX => "ccache c++",
CLANG => "ccache clang",

want me to change it to:

CC => "ccache clang",
CXX => "ccache c++",
CLANG => "ccache clang",

?

/Mikael

  3. Re: Mixing CC and a different CLANG seems like a bad idea

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-11-18T17:31:28Z

    =?UTF-8?Q?Mikael_Kjellstr=c3=b6m?= <mikael.kjellstrom@mksoft.nu> writes:
> Hm, actually it's:

> CC => "ccache cc",
> CXX => "ccache c++",
> CLANG => "ccache clang",

Right.

> want me to change it to:

> CC => "ccache clang",
> CXX => "ccache c++",
> CLANG => "ccache clang",

What I actually think is we should get rid of the separate CLANG
variable.  But don't do anything to the animal's configuration
till that's settled.

BTW, that would presumably lead to wanting to use CXX = "ccache clang++",
too.  I think we don't really want inconsistent CC/CXX either ...
although at least configure knows it needs to probe CXX's flags
separately.  I suppose another way to resolve this would be for
configure to make a third set of probes to see what switches CLANG
has --- but ick.

			regards, tom lane

  4. Re: Mixing CC and a different CLANG seems like a bad idea

    Andres Freund <andres@anarazel.de> — 2021-11-18T17:32:44Z

    Hi,

On 2021-11-18 11:56:59 -0500, Tom Lane wrote:
> I noticed that, a week after Michael pushed 9ff47ea41 to silence
> -Wcompound-token-split-by-macro warnings, buildfarm member sidewinder
> is still spewing them.  Investigation shows that it's building with
> 
> configure: using compiler=cc (nb4 20200810) 7.5.0
> configure: using CLANG=ccache clang
> 
> and the system cc doesn't know -Wcompound-token-split-by-macro,
> so we don't use it, but the modules that are built into bytecode
> still produce the warnings because they're built with clang.

> I think this idea of using clang with switches selected for some other
> compiler is completely horrid, and needs to be nuked from orbit before
> it causes problems worse than mere warnings.

We can test separately for flags, see BITCODE_CFLAGS/BITCODE_CXXFLAGS.


> Why did we not simply insist that if you want to use --with-llvm, the
> selected compiler must be clang?  I cannot see any benefit of mix-and-match
> here.

It seemed like a problematic restriction at the time. And still does to
me.

For one, gcc does generate somewhat better code. For another, extensions could
still compile with a different compiler, and generate bitcode with another
compiler, so it's good to have that easily testable.

It also just seems architecturally wrong: People pressed for making the choice
of JIT runtime replaceable, and it now is, at some pain. And forcing the main
compiler seems problematic from that angle.  With the meson port jit
compilation actually kinda works on windows - but it seems we shouldn't force
people to not use visual studio there, just for that?


I think the issue is more with trying to be miserly in the choice of compiler
flag tests to duplicate and how many places to change to choose the right flag
variable. It's taken a while for this to become a real issue, so it perhaps
was the right choice at the time.

Greetings,

Andres Freund

  5. Re: Mixing CC and a different CLANG seems like a bad idea

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-11-18T17:43:15Z

    Andres Freund <andres@anarazel.de> writes:
> On 2021-11-18 11:56:59 -0500, Tom Lane wrote:
>> Why did we not simply insist that if you want to use --with-llvm, the
>> selected compiler must be clang?  I cannot see any benefit of mix-and-match
>> here.

> It also just seems architecturally wrong: People pressed for making the choice
> of JIT runtime replaceable, and it now is, at some pain. And forcing the main
> compiler seems problematic from that angle.

OK, I concede that's a reasonable concern.  So we need to look more
carefully at how the switches for CLANG are being selected.

> I think the issue is more with trying to be miserly in the choice of compiler
> flag tests to duplicate and how many places to change to choose the right flag
> variable. It's taken a while for this to become a real issue, so it perhaps
> was the right choice at the time.

Yeah.  I'm inclined to think we ought to just bite the bullet and fold
CLANG/CLANGXX into the main list of compiler switch probes, so that we
check every interesting one four times.  That sounds fairly horrid,
but as long as you are using an accache file it's not really going
to cost that much.  (BTW, does meson have any comparable optimization?
If it doesn't, I bet that is going to be a problem.)

			regards, tom lane

  6. Re: Mixing CC and a different CLANG seems like a bad idea

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-11-18T18:39:04Z

    I wrote:
> Yeah.  I'm inclined to think we ought to just bite the bullet and fold
> CLANG/CLANGXX into the main list of compiler switch probes, so that we
> check every interesting one four times.

After studying configure's list more closely, that doesn't seem like
a great plan either.  There's a lot of idiosyncrasy in the tests,
such as things that only apply to C or to C++.

More, I think (though this ought to be documented in a comment) that
the policy is to not bother turning on extra -W options in the bitcode
switches, on the grounds that warning once in the main build is enough.
I follow that idea --- but what we missed is that we still need to
turn *off* the warnings we're actively disabling.  I shall go do that,
if no objections.

			regards, tom lane

  7. Re: Mixing CC and a different CLANG seems like a bad idea

    Andres Freund <andres@anarazel.de> — 2021-11-18T20:42:57Z

    Hi,

On 2021-11-18 12:43:15 -0500, Tom Lane wrote:
> Yeah.  I'm inclined to think we ought to just bite the bullet and fold
> CLANG/CLANGXX into the main list of compiler switch probes, so that we
> check every interesting one four times.  That sounds fairly horrid,
> but as long as you are using an accache file it's not really going
> to cost that much.  (BTW, does meson have any comparable optimization?
> If it doesn't, I bet that is going to be a problem.)
> 
> 			regards, tom lane
Greetings,

Andres Freund

  8. Re: Mixing CC and a different CLANG seems like a bad idea

    Robert Haas <robertmhaas@gmail.com> — 2021-11-18T21:13:50Z

    On Thu, Nov 18, 2021 at 3:43 PM Andres Freund <andres@anarazel.de> wrote:
> Hi,
>
> Greetings,
>
> Andres Freund

Greetings to you too, Andres. :-)

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  9. Re: Mixing CC and a different CLANG seems like a bad idea

    Andres Freund <andres@anarazel.de> — 2021-11-18T21:34:35Z

    Hi,

On 2021-11-18 13:39:04 -0500, Tom Lane wrote:
> After studying configure's list more closely, that doesn't seem like
> a great plan either.  There's a lot of idiosyncrasy in the tests,
> such as things that only apply to C or to C++.

Yea. It seems doable, but not really worth it for now.


> More, I think (though this ought to be documented in a comment) that
> the policy is to not bother turning on extra -W options in the bitcode
> switches, on the grounds that warning once in the main build is enough.
> I follow that idea --- but what we missed is that we still need to
> turn *off* the warnings we're actively disabling.  I shall go do that,
> if no objections.

Thanks for doing that, that does sounds like a good way, at least for now.


On 2021-11-18 13:39:04 -0500, Tom Lane wrote:
> That sounds fairly horrid, but as long as you are using an accache file it's
> not really going to cost that much.

> (BTW, does meson have any comparable optimization?
> If it doesn't, I bet that is going to be a problem.)

Yes - imo in a nicer, more reliable way. It caches most test results, but with
the complete input, including the commandline (i.e. compiler flags) as the
cache key. So no more errors about compile flags changing...

Greetings,

Andres Freund

  10. Re: Mixing CC and a different CLANG seems like a bad idea

    Andres Freund <andres@anarazel.de> — 2021-11-18T21:35:01Z

    Hi,

On 2021-11-18 16:13:50 -0500, Robert Haas wrote:
> On Thu, Nov 18, 2021 at 3:43 PM Andres Freund <andres@anarazel.de> wrote:
> > Hi,
> >
> > Greetings,
> >
> > Andres Freund
> 
> Greetings to you too, Andres. :-)

Oops I sent the email that I copied text from, rather than the one I wanted to
send...

Greetings,

Andres Freund

  11. Re: Mixing CC and a different CLANG seems like a bad idea

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-11-18T21:57:52Z

    Andres Freund <andres@anarazel.de> writes:
> On 2021-11-18 13:39:04 -0500, Tom Lane wrote:
>> More, I think (though this ought to be documented in a comment) that
>> the policy is to not bother turning on extra -W options in the bitcode
>> switches, on the grounds that warning once in the main build is enough.
>> I follow that idea --- but what we missed is that we still need to
>> turn *off* the warnings we're actively disabling.  I shall go do that,
>> if no objections.

> Thanks for doing that, that does sounds like a good way, at least for now.

Cool, thanks for confirming.

For the archives' sake: I thought originally that this was triggered
by having CC different from CLANG, and even wrote that in the commit
message; but I was mistaken.  I was misled by the fact that sidewinder
is the only animal still reporting the compound-token-split-by-macro
warnings, and jumped to the conclusion that its unusual configuration
was the cause.  But actually that't not it, because the flags we
feed to CLANG are *not* dependent on what CC will take.  I now think
the actual uniqueness is that sidewinder is the only animal that is
using clang >= 12 and has --with-llvm enabled.

>> (BTW, does meson have any comparable optimization?
>> If it doesn't, I bet that is going to be a problem.)

> Yes - imo in a nicer, more reliable way. It caches most test results, but with
> the complete input, including the commandline (i.e. compiler flags) as the
> cache key. So no more errors about compile flags changing...

Nice!

			regards, tom lane