Re: RFT: Please help testing the llvm/clang 3.5.0 import

From: Warner Losh <imp_at_bsdimp.com>
Date: Thu, 18 Dec 2014 14:29:49 -0700
> On Dec 18, 2014, at 12:01 PM, Dimitry Andric <dim_at_freebsd.org> wrote:
> 
> On 18 Dec 2014, at 15:47, Warner Losh <imp_at_bsdimp.com> wrote:
> ...
>>> * Mips will only have a chance with the upcoming clang 3.6.0, but that
>>> is way too late for this import.  It will probably require external
>>> toolchain support to get it working.
>> 
>> For native builds yes. For cross builds, clang 3.6 can be built on an
>> x86 host.
> 
> Yes, and it could even be one of the ports, if that is easier to use.
> 
> 
>> * Another ports exp-run was done [3], after fixing the problem with
>>> lang/gcc, which lead to many skipped dependent ports.
>>> * The second exp-run had much better results: the failure with the
>>> highest number of dependencies is devel/mingw32-gcc, but this seems
>>> to be due to a problem with makeinfo, not clang.  The next highest on
>>> the list is java/openjdk6, for which ports r374780 [4] was very
>>> recently committed.
>> 
>> Will users of our stable branch have code similar to the code that caused
>> problems?
> 
> I'm not sure which code you are referring to here, the openjdk6 code?
> The code itself is basically fine, but for reasons unknown to me, the
> port is compiled with -Werror (which is not the case for the other
> openjdk ports, apparently).  Since clang 3.5.0 adds a few new warnings
> for shaky C++ constructions, these appear during the openjdk6 build, but
> they are easily suppressed, if upstream does not fix them, or does not
> care to fix them.

I meant “similar code to what’s causing problems” with the build run in their
code they build on FreeBSD. If it is a few new warnings for obscure things,
we can advice to the release notes about what to avoid and how to mitigate
things.

> I already sent Jung-uk an alternative fix for openjkd6, similar to the
> one used for www/squid, where warnings are suppressed based on the
> COMPILER_VERSION variable provided the ports infrastructure.  In my
> opinion it would still be easier to just to turn off -Werror for any
> third-party code, if we don't feel like modifying it (with all the risks
> involved).

Yea, we can sort out the code in src and ports. I’m more worried about
what to tell our users that may be compiling their own code that we don’t
control. If these new warnings are ubiquitous, then that could be a problem
for adoption (since many shops mandate -Werror as much as possible, and
to comply with that mandate would require additional resources when trying
to upgrade). If there are a few, then we could just document them and move on.

>> One warning flag about your upgrade to the stable branch
>> would be if there’s a significant number of user-written programs that
>> suddenly become uncompilable with the new clang using the environment
>> that they have today. We know of some items that are issues, so careful
>> attention here is needed. Unless we go proactively looking for these,
>> there’s a good chance we won’t find them until users hit them and start
>> to complain (by which point it is likely too late). Could you post a summary
>> of the issues that ports have hit and the fixes necessary? We may need
>> to have that in the release notes and/or UPDATING file to help prepare
>> our users for the bumps and give them solutions over them.
> 
> The base system is already completely free of warnings, as far as I know
> of, so no action is needed there.  For ports, the number of failures
> introduced by new warnings are quite small, as far as I can see, and
> mostly for ports that are compiled with -Werror.

Yea, I wasn’t too worried about this aspect of things.

> The most encountered new warnings are, off the top of my head:
> 
> -Wabsolute-value
> 
> This warns in two cases, for both C and C++:
> * When the code is trying to take the absolute value of an unsigned
>  quantity, which is effectively a no-op, and almost never what was
>  intended.  The code should be fixed, if at all possible.
> * When the code is trying to take the absolute value, but the called
>  abs() variant is of the wrong type, which may lead to truncation.
>  If the warning is turned off, better make sure any truncation does
>  not lead to unwanted side-effects.
> 
> -Wtautological-undefined-compare and
> -Wundefined-bool-conversion
> 
> These warn when C++ code is trying to compare 'this' against NULL, while
> 'this' should never be NULL in well-defined C++ code.  However, there is
> some legacy (pre C++11) code out there, which actively abuses this
> feature, which was less strictly defined in previous C++ versions.
> 
> Squid does this, and apparently openjdk too.  The warning can be turned
> off for C++98 and earlier, but compiling the code in C++11 mode might
> result in unexpected behavior, for example the unreachable parts of the
> program could be optimized away.

This is the kind of information I was talking about. Do we have a process
to make sure this gets into the release notes?

>> I would really like to merge this branch to head in about a week,
>>> pending portmgr approvall; I don't expect the base system (outside of
>>> llvm/clang) to need any further updates.
>> 
>> I think there’s good reason to do this, but we should chat about the
>> build issues below before doing it. They are minor, but an important
>> detail. I’ll see if I can find a few minutes to pull the branch and send
>> patches.
>> 
>>> Lastly, to clear things up about the requirements for this branch (and
>>> thus for head, in a while); to build it, you need to have:
>>> * A C++11 capable "host" compiler, e.g. clang >= 3.3 or later, or gcc
>>>> = 4.8 (I'm not 100% sure if gcc 4.7 will work, reports welcome)
>>> * A C++11 standard library, e.g. libc++, or libstdc++ from gcc >= 4.8.
>>> 
>>> So from any earlier standard 10.x or 11.x installation, you should be
>>> good, unless you explicitly disabled clang or libc++.  In that case,
>>> you must build and install both of those first.
>> 
>> This is true only on i386, amd64, and arm hosts. Given that some people
>> do try to do weird things, tightening up how you present this will get the
>> word out a little better.
>> 
>>> On a 9.x installation, you will have clang by default, but not libc++,
>>> so libc++ should be built and installed first, before attempting to
>>> build the clang350-import branch.
>> 
>> Can you make sure that the UPDATING entry you are writing for this
>> contains explicit instructions.
> 
> I'm quite bad at writing UPDATING entries, so any help there is much
> appreciated. :-)

I’m happy to help with this, but I work best from a rough draft ...

>> On 8.x an earlier, you need to upgrade to at least 9.x first, follow
>>> the previous instruction.
>> 
>> We should remove building on 8 support then, unless there external
>> toolchain stuff is up to the task (e.g. build gcc 4.9, libstc++, etc).
> 
> The problem with 8.x is that it still has the old binutils 2.15, and
> neither clang nor libc++.  It would really require some externally
> supplied parts.  Maybe this could be done with ports, but I'm not sure
> how long ports still supports the 8.x branch?

If we can’t bootstrap from an 8.x system and have it work, we need to
remove support for doing that from Makefile.inc1 and friends. If we think
we can still support it with the external tool chain stuff, or with the ‘end
goal’ for the external toolchain stuff, we should leave it in. At this point,
I think it would be best to add a .warning for 8.x saying this isn’t supported
without an external toolchain, and then a month out from the branch point
we can decide what to do.

8.x is supported through the summer, iirc, on ports.

Right now, for example, we say if the host is < 8.0<mumble> then we don’t
support it. I’ll add something that says if the host is < 9<mumble> then bootstrapping
clang won’t work, but I won’t make it an error just yet and send it to you for
your branch.

>>> As for MFC'ing, I plan on merging clang 3.5.x to 10.x in a while
>>> (roughly a month), but this will cause upgrades from 9.x to 10.x to
>>> start requiring the build of libc++, as described above.  I don't think
>>> we can merge clang 3.5.x to 9.x, unless clang becomes the default
>>> compiler there (but that is very unlikely).
>> 
>> Let’s see how it goes, and what the upgrade issues wind up being
>> before doing this merge back. New “major” compilers on stable branches
>> traditionally haven’t been done, but if clang is better about being ABIly
>> identical to prior releases than gcc was, it might not have the same issues
>> associated with it.
> 
> We don't really use llvm or clang's own ABI for anything at the moment,
> just the resulting compiler executable, which is actually one big binary
> (and it is even statically linked, by default).

Right, this isn’t what I’m worried about...

> The code output by clang 3.4.1 or 3.5.0 is not different in any "ABI"
> sense of the word.  Of course it will be different in absolute sense,
> since optimizations were improved, and so on.

This is what I’m worried about given our past experiences with updating
compilers. If we believe there’s no issues here, we can tick that box
but it was traumatic enough the last time we tried this (admittedly with
gcc) that I have to ask. Similar representations were made, though
it turned out people forgot to ask “including C++” until people noticed
that it failed in some cases and started asking…

> The only real issue is how to bootstrap the compiler itself, since it
> requires working C++11 support.  In 10.x, we provide that by default,
> but not in earlier releases.

Yea, that’s something we can document with release notes.

Cool! Thanks for taking the time to address my concerns.

Warner

Received on Thu Dec 18 2014 - 20:29:53 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:54 UTC