A little more progress today with clang/LLVM

A little more progress today with clang/LLVM

[Richard Pennington]

I've decided to keep the musl build scheme with a few minor changes (e.g. 
replace libgcc.a with libcompiler-rt.a).

I'm now building for i386, arm, and x86_64. I enabled clang warnings -Werror 
and had to inhibit a few, some of which look like they need some attention. 
Here's my current list:

-Werror -Qunused-arguments -Wno-unneeded-internal-declaration -Wno-cast-align 
-Wno-incompatible-pointer-types -Wno-implicit-function-declaration -Wno-
string-plus-int -Wno-pointer-sign -Wno-array-bounds

I think that bits/alltypes.h might need some clang specific tweaking.

I'm going to start looking at the mips, ppc, and microblaze bits now, and will 
look at warning elimination after.

-Rich

Re: A little more progress today with clang/LLVM

[Rich Felker]

On Mon, May 21, 2012 at 08:47:04PM -0500, Richard Pennington wrote:
> I've decided to keep the musl build scheme with a few minor changes (e.g. 
> replace libgcc.a with libcompiler-rt.a).
> 
> I'm now building for i386, arm, and x86_64. I enabled clang warnings -Werror 
> and had to inhibit a few, some of which look like they need some attention. 
> Here's my current list:
> 
> -Werror -Qunused-arguments -Wno-unneeded-internal-declaration -Wno-cast-align 
> -Wno-incompatible-pointer-types -Wno-implicit-function-declaration -Wno-
> string-plus-int -Wno-pointer-sign -Wno-array-bounds

Aside from unused arguments (a bogus warning IMO) and string plus int
(why is this a warning?), these sound like they could be issues to
address. I cleaned up all the pointer signedness warnings (which
should be errors, per the C language) with gcc, so I'm unsure why
they're showing up again. Implicit function declarations sound
particularly bad, but gcc didn't report any to me. The rest are
*probably* non-issues, but I'd like to check them if you could send
the warning output to the list.

> I think that bits/alltypes.h might need some clang specific tweaking.

Can you elaborate?

> I'm going to start looking at the mips, ppc, and microblaze bits now, and will 
> look at warning elimination after.

I'll be happy to look at it for you. :)

Rich

Re: A little more progress today with clang/LLVM

[Richard Pennington]

On Monday, May 21, 2012 09:59:35 PM Rich Felker wrote:
> On Mon, May 21, 2012 at 08:47:04PM -0500, Richard Pennington wrote:
> > I've decided to keep the musl build scheme with a few minor changes (e.g.
> > replace libgcc.a with libcompiler-rt.a).
> > 
> > I'm now building for i386, arm, and x86_64. I enabled clang warnings
> > -Werror and had to inhibit a few, some of which look like they need some
> > attention. Here's my current list:
> > 
> > -Werror -Qunused-arguments -Wno-unneeded-internal-declaration
> > -Wno-cast-align -Wno-incompatible-pointer-types
> > -Wno-implicit-function-declaration -Wno- string-plus-int
> > -Wno-pointer-sign -Wno-array-bounds
> 
> Aside from unused arguments (a bogus warning IMO) and string plus int
> (why is this a warning?), these sound like they could be issues to
> address. I cleaned up all the pointer signedness warnings (which
> should be errors, per the C language) with gcc, so I'm unsure why
> they're showing up again. Implicit function declarations sound
> particularly bad, but gcc didn't report any to me. The rest are
> *probably* non-issues, but I'd like to check them if you could send
> the warning output to the list.
> 
> > I think that bits/alltypes.h might need some clang specific tweaking.
> 
> Can you elaborate?

There are some differences between what clang thinks should be the parameters 
of well known functions and what your header files say. I'll get you more 
details.

> 
> > I'm going to start looking at the mips, ppc, and microblaze bits now, and
> > will look at warning elimination after.
> 
> I'll be happy to look at it for you. :)
> 
> Rich

I'll send you the list. The only one that looked sort of bad to me was the out 
of bounds array reference. It was on the x86_64 in the pthread_barrier_t 
access. clang complained about referencing __p[4], which is out of bounds. 
That code is a little cryptic to me right now, since I'm a newbi. ;-)

-Rich

Re: A little more progress today with clang/LLVM

[Rich Felker]

On Mon, May 21, 2012 at 09:35:59PM -0500, Richard Pennington wrote:
> I'll send you the list. The only one that looked sort of bad to me was the out 
> of bounds array reference. It was on the x86_64 in the pthread_barrier_t 
> access. clang complained about referencing __p[4], which is out of bounds. 
> That code is a little cryptic to me right now, since I'm a newbi. ;-)

Thanks! Indeed, that's broken. I must not have noticed since I'm on
x86 (32-bit). The situation with the definition of these types is
unfortunate and I might eventually change it; basically, to avoid
leaking implementation details in the public headers, and to avoid
having to change the bits headers for all archs if I change the way
these primitives work, the public headers just have a union with ints
and pointers matching the ABI's size for the type, and pthread_impl.h
has macros that map fields into these slots in such a way that the
mapping works on both 32- and 64-bit systems.

Fixing now...

Rich

Re: A little more progress today with clang/LLVM

[Richard Pennington]

On Monday, May 21, 2012 10:53:06 PM Rich Felker wrote:
> On Mon, May 21, 2012 at 09:35:59PM -0500, Richard Pennington wrote:
> > I'll send you the list. The only one that looked sort of bad to me was the
> > out of bounds array reference. It was on the x86_64 in the
> > pthread_barrier_t access. clang complained about referencing __p[4],
> > which is out of bounds. That code is a little cryptic to me right now,
> > since I'm a newbi. ;-)
> Thanks! Indeed, that's broken. I must not have noticed since I'm on
> x86 (32-bit). The situation with the definition of these types is
> unfortunate and I might eventually change it; basically, to avoid
> leaking implementation details in the public headers, and to avoid
> having to change the bits headers for all archs if I change the way
> these primitives work, the public headers just have a union with ints
> and pointers matching the ABI's size for the type, and pthread_impl.h
> has macros that map fields into these slots in such a way that the
> mapping works on both 32- and 64-bit systems.
> 
> Fixing now...
> 
> Rich

I understand.  I'm just afraid about my next update..  I'm doing a git to svn 
sync and I know nothing about git. :-(  Google is my friend, however.

-Rich

Re: A little more progress today with clang/LLVM

[Richard Pennington]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

On Monday, May 21, 2012 09:59:35 PM Rich Felker wrote:
> > I think that bits/alltypes.h might need some clang specific tweaking.
> 
> Can you elaborate?
> 

I've done a little hacking on alltypes.h.sh which I'm in the process of 
testing. I have two goals:
	1. Make it work with clang's headers.
	2. Make it slightly easier to add additional processor support by having
	    a single alltypes.h for all processors.

Does this look like I'm going down a reasonable path?

-Rich

[-- Attachment #2: alltypes.h.sh --]
[-- Type: application/x-shellscript, Size: 8342 bytes --]

Re: A little more progress today with clang/LLVM

[Rich Felker]

On Fri, May 25, 2012 at 01:56:56PM -0500, Richard Pennington wrote:
> I've done a little hacking on alltypes.h.sh which I'm in the process of 
> testing. I have two goals:
> 	1. Make it work with clang's headers.

Can you explain what the issue is? Are you talking about issues
building clang itself, or building programs against musl using clang?
In the latter case, musl does not use or support using
compiler-provided headers. All of the standard headers are provided
fully by musl.

> 	2. Make it slightly easier to add additional processor support by having
> 	    a single alltypes.h for all processors.
> 
> Does this look like I'm going down a reasonable path?

No, what you've done is taken header files that work with ANY compiler
meeting the target ABI and that are optimized to avoid unnecessary
runtime checks, and made them so that they only work with compilers
which define particular macros for the name of the target arch and
which go through a lot of unnecessary extra conditional tests. Part of
the (admittedly not documented anywhere) philosophy of musl's headers
is that they should avoid preprocessor conditionals in the headers for
"parameters" that are actually constant.

> #if defined(__clang__)
>   TYPEDEF __typeof__(sizeof(int)) size_t;
> #else
>   TYPEDEF unsigned long size_t;
> #endif

The #else is actually wrong. The i386 ABI uses unsigned int, not
unsigned long, for size_t. It's stupid, but it matters because even
though the representations and range of values are identical, the
types unsigned long * and unsigned int * are not compatible. Also, it
breaks C++ name mangling if you use the wrong type.

Is clang using different types than gcc for size_t? If so, it means
it's impossible to have a shared C++ ABI between the two. If not,
what's the problem with the current definition?

> #if defined(__clang__)
>   TYPEDEF __typeof__(((int*)0)-((int*)0)) ptrdiff_t;
> #else
>   #if defined(__x86_64__) || defined(__arm__) || defined(__i386__)
>     TYPEDEF long ptrdiff_t;
>   #else
>     #error ptrdiff_t is not defined for this processor
>   #endif
> #endif

One thing that _would_ work for these __typeof__ cases is to put it
under __GNUC__, which encompasses ALL compilers that have "GNU C"
extensions like typeof. That would definitely be an acceptable change,
but unless there are any compilers where it's necessary, I'd rather
just leave the types explicit for the arch's ABI. One of the most
frustrating things with glibc headers is never being able to figure
out the actual underlying definition of a type, and I'd like not to
recreate that frustration.

> TYPEDEF __builtin_va_list va_list;

This does not work with some older compilers if I remember correctly.
That's why it's conditional on i386 in the current version.

A couple things I'm _NOT_ happy about in my current system are that
the whole alltypes.h gets parsed again and again even (for each header
that includes it) even if only a few types are needed each time. One
thing I'm considering (but not yet decided on) is dropping it and
instead having the build system generate all the headers from
templates when musl is built, and put the expanded TYPEDEF templates
right in the headers that use them.

In any case, for now don't worry about the potential
ugliness/duplication in alltypes.h.sh for new archs. It's not a large
file (the bits/*.h headers are much worse when it comes to
duplication) and I'm happy to take responsibility for cleaning them
all up later if a better system is devised.

Rich

Re: A little more progress today with clang/LLVM

[Richard Pennington]

On Friday, May 25, 2012 07:09:38 PM Rich Felker wrote:
> On Fri, May 25, 2012 at 01:56:56PM -0500, Richard Pennington wrote:
> > I've done a little hacking on alltypes.h.sh which I'm in the process of
> > 
> > testing. I have two goals:
> > 	1. Make it work with clang's headers.
> 
> Can you explain what the issue is? Are you talking about issues
> building clang itself, or building programs against musl using clang?
> In the latter case, musl does not use or support using
> compiler-provided headers. All of the standard headers are provided
> fully by musl.

This is good to know. I'll stop using the compiler supplied headers.

[snip]
> One thing that _would_ work for these __typeof__ cases is to put it
> under __GNUC__, which encompasses ALL compilers that have "GNU C"
> extensions like typeof. That would definitely be an acceptable change,
> but unless there are any compilers where it's necessary, I'd rather
> just leave the types explicit for the arch's ABI. One of the most
> frustrating things with glibc headers is never being able to figure
> out the actual underlying definition of a type, and I'd like not to
> recreate that frustration.

You've convinced me. ;-)

> 
> A couple things I'm _NOT_ happy about in my current system are that
> the whole alltypes.h gets parsed again and again even (for each header
> that includes it) even if only a few types are needed each time. One
> thing I'm considering (but not yet decided on) is dropping it and
> instead having the build system generate all the headers from
> templates when musl is built, and put the expanded TYPEDEF templates
> right in the headers that use them.
> 
> In any case, for now don't worry about the potential
> ugliness/duplication in alltypes.h.sh for new archs. It's not a large
> file (the bits/*.h headers are much worse when it comes to
> duplication) and I'm happy to take responsibility for cleaning them
> all up later if a better system is devised.
> 

OK, and thanks for being patient. I'm still trying to absorb the musl 
philosophy.

-Rich

Re: A little more progress today with clang/LLVM

[Richard Pennington]

On Saturday, May 26, 2012 06:30:15 AM Richard Pennington wrote:
> On Friday, May 25, 2012 07:09:38 PM Rich Felker wrote:
> > On Fri, May 25, 2012 at 01:56:56PM -0500, Richard Pennington wrote:
> > > I've done a little hacking on alltypes.h.sh which I'm in the process of
> > > 
> > > testing. I have two goals:
> > > 	1. Make it work with clang's headers.
> > 
> > Can you explain what the issue is? Are you talking about issues
> > building clang itself, or building programs against musl using clang?
> > In the latter case, musl does not use or support using
> > compiler-provided headers. All of the standard headers are provided
> > fully by musl.
> 
> This is good to know. I'll stop using the compiler supplied headers.
> 
[snip]

Which reminds me why I went down the supplied header path in the first place.  
My tgmath.h test cases fail, for example (f is a float, d is a double):

../../../../../src/libs/math/001tgmath.c:15:7: error: assigning to 'float' from 
incompatible type 'typeof (*(0
      ? (typeof (0 ? (double *)0 : (void *)!!((1 ? 1 : ((f))) / 2)))0 : 
(typeof (0 ? (typeof ((f)) *)0 :
      (void *)!!!((1 ? 1 : ((f))) / 2)))0))' (aka 'void')
    f = acos(f);
      ^ ~~~~~~~
../../../../../src/libs/math/001tgmath.c:16:7: error: assigning to 'double' 
from incompatible type 'typeof    
      (*(0 ? (typeof (0 ? (double *)0 : (void *)!!((1 ? 1 : ((d))) / 2)))0 : 
(typeof (0 ? (typeof ((d)) *)0 :
      (void *)!!!((1 ? 1 : ((d))) / 2)))0))' (aka 'void')
    d = acos(d);
      ^ ~~~~~~~

-Rich

Re: A little more progress today with clang/LLVM

[Szabolcs Nagy]

* Richard Pennington <rich@pennware.com> [2012-05-26 06:39:25 -0500]:
> My tgmath.h test cases fail, for example (f is a float, d is a double):
> 

tgmath.h is non-trivial and is not extensively tested so there can be issues

> ../../../../../src/libs/math/001tgmath.c:15:7: error: assigning to 'float' from 
> incompatible type 'typeof (*(0
>       ? (typeof (0 ? (double *)0 : (void *)!!((1 ? 1 : ((f))) / 2)))0 : 
> (typeof (0 ? (typeof ((f)) *)0 :
>       (void *)!!!((1 ? 1 : ((f))) / 2)))0))' (aka 'void')

sounds like a clang bug

this code is marked with #ifdef __GNUC__ and
uses __typeof__ and ?: trics to be able to cast
the return value to the correct type


to try the messy expression above in isolation you can use:

#define T __typeof__
#define ISFP(x) !!((1?1:(x))/2)

float f = 0.1;
T(*(0 ? (T(0 ? (double *)0 : (void *)ISFP(f)))0 :
        (T(0 ? (T(f) *)0 : (void *)!ISFP(f)))0)) *x = &f;


but i think clang gets the ?: thing wrong
eg. try

__typeof__(0 ? (char*)0 : (void*)1) p;
void **pp = &p;

__typeof__(0 ? (char*)0 : (void*)0) q;
char **qq = &q;


(this should compile cleanly)

Re: A little more progress today with clang/LLVM

[Richard Pennington]

On Saturday, May 26, 2012 06:30:15 AM Richard Pennington wrote:
[snip]
> > A couple things I'm _NOT_ happy about in my current system are that
> > the whole alltypes.h gets parsed again and again even (for each header
> > that includes it) even if only a few types are needed each time. One
> > thing I'm considering (but not yet decided on) is dropping it and
> > instead having the build system generate all the headers from
> > templates when musl is built, and put the expanded TYPEDEF templates
> > right in the headers that use them.

One somewhat silly reason that I wouldn't want this: a single ELLCC 
build/install supports all the targets. Currently I have a single include 
directory with target specific headers in target specific subdirectories, e.g. 
include/x86_64/bits/alltypes.h with all the other header files common.

Also, I agree with your previous point that it is nice to go to one place 
(alltypes.h) to find the processor specific types. If you make this change, at 
least keep alltypes.h for reference. ;-)

-Rich