Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Mon, May 26, 2014 at 08:36:31AM -0700, Mike Stump wrote: On May 26, 2014, at 2:22 AM, FX fxcoud...@gmail.com wrote: This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? The best coarse of action, post a patch, have it reviewed and put in. Current action, a patch has been posted, the review is outstanding, I’d like to see it put in; though, I am curious why the casts were there in the first place. Note, haven't added them there, but from what I can test, the casts there can serve as a compile time check that the right type is used, e.g. unsigned long i; void foo (void) { asm volatile (# %0 %1 : =r ((unsigned long long) i) : 0 ((unsigned long long) 0)); } errors out on x86_64 -m32, but compiles fine with -m64, because in the latter case the type has the correct size, while in the former case it doesn't. So, perhaps instead of removing the casts we should replace them with some kind of static assertions (whether extern char foobar[sizeof (arg) == sizeof (UDItype) __builtin_classify_type (arg) == 1 ? 1 : -1]; or __builtin_types_compatible_p, or C++ templates for C++, ... Jakub
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Wed, May 28, 2014 at 8:50 AM, Jakub Jelinek ja...@redhat.com wrote: On Mon, May 26, 2014 at 08:36:31AM -0700, Mike Stump wrote: On May 26, 2014, at 2:22 AM, FX fxcoud...@gmail.com wrote: This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? The best coarse of action, post a patch, have it reviewed and put in. Current action, a patch has been posted, the review is outstanding, I’d like to see it put in; though, I am curious why the casts were there in the first place. Note, haven't added them there, but from what I can test, the casts there can serve as a compile time check that the right type is used, e.g. unsigned long i; void foo (void) { asm volatile (# %0 %1 : =r ((unsigned long long) i) : 0 ((unsigned long long) 0)); } Ah, interesting. A not-so-hineous extension then. errors out on x86_64 -m32, but compiles fine with -m64, because in the latter case the type has the correct size, while in the former case it doesn't. So, perhaps instead of removing the casts we should replace them with some kind of static assertions (whether extern char foobar[sizeof (arg) == sizeof (UDItype) __builtin_classify_type (arg) == 1 ? 1 : -1]; or __builtin_types_compatible_p, or C++ templates for C++, ... Yeah, a portable (C and C++) static assert would be nice. And also pushing this to gmp then. In the meantime I see nothing wrong in merging from GMP. Richard. Jakub
Re: Darwin bootstrap failure following wide int merge
Richard Biener richard.guent...@gmail.com writes: On Wed, May 28, 2014 at 8:50 AM, Jakub Jelinek ja...@redhat.com wrote: On Mon, May 26, 2014 at 08:36:31AM -0700, Mike Stump wrote: On May 26, 2014, at 2:22 AM, FX fxcoud...@gmail.com wrote: This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? The best coarse of action, post a patch, have it reviewed and put in. Current action, a patch has been posted, the review is outstanding, I’d like to see it put in; though, I am curious why the casts were there in the first place. Note, haven't added them there, but from what I can test, the casts there can serve as a compile time check that the right type is used, e.g. unsigned long i; void foo (void) { asm volatile (# %0 %1 : =r ((unsigned long long) i) : 0 ((unsigned long long) 0)); } Ah, interesting. A not-so-hineous extension then. In that case, how about just protecting the include with: #if GCC_VERSION = 4300 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) rather than: #if GCC_VERSION = 3000 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) so that clang will fail the version check? At the end of the day we only really care what happens during stage 2 and 3. Cross-compilers built with recentish gccs will still benefit. Thanks, Richard
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Wed, May 28, 2014 at 10:24 AM, FX fxcoud...@gmail.com wrote: Yeah, a portable (C and C++) static assert would be nice. And also pushing this to gmp then. In the meantime I see nothing wrong in merging from GMP. One question, one comment: 1. can I count your “I see nothing wrong” as an approval, as in “global reviewers can approve changes to any part of the compiler or associated libraries”? Well, kind of. But Jakub is as well, so I don't want to override him. So please wait for an ack from Jakub. I agree with him that the casts served a purpose and that, if removed, they need to be replaced with an appropriate assertion measure. Given that inline asm is a GCC extension calling those casts another extension from the LLVM side is really odd. In fact I think the casts are a very good way of doing this kind of assertions. Are they documented in that regard? If so I'd say it's really really LLVM that should be fixed and the workaround on the GCC side is to pass that -fhineous-gnu-extensions flag. 2. I think your quotes around “merging” mean you’re not actually thinking of a merge, but for clarification’s sake: GMP’s longlong.h has apparently a long history of its own, and has many differences with GCC’s version. The closest thing to an “upstream” for us would probably be glibc (see the diff attached), from which we last merged on 2014-04-22. I see. I suppose the gcc side includes the proposed patch and glibc still has those casts, right? In that case there is nothing to merge from glibc (but to glibc eventually). Richard. Thanks, FX
Re: Darwin bootstrap failure following wide int merge
On Wed, May 28, 2014 at 10:24 AM, Richard Sandiford rdsandif...@googlemail.com wrote: Richard Biener richard.guent...@gmail.com writes: On Wed, May 28, 2014 at 8:50 AM, Jakub Jelinek ja...@redhat.com wrote: On Mon, May 26, 2014 at 08:36:31AM -0700, Mike Stump wrote: On May 26, 2014, at 2:22 AM, FX fxcoud...@gmail.com wrote: This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? The best coarse of action, post a patch, have it reviewed and put in. Current action, a patch has been posted, the review is outstanding, I’d like to see it put in; though, I am curious why the casts were there in the first place. Note, haven't added them there, but from what I can test, the casts there can serve as a compile time check that the right type is used, e.g. unsigned long i; void foo (void) { asm volatile (# %0 %1 : =r ((unsigned long long) i) : 0 ((unsigned long long) 0)); } Ah, interesting. A not-so-hineous extension then. In that case, how about just protecting the include with: #if GCC_VERSION = 4300 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) rather than: #if GCC_VERSION = 3000 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) so that clang will fail the version check? At the end of the day we only really care what happens during stage 2 and 3. Cross-compilers built with recentish gccs will still benefit. Works for me (thus, pre-approved with a comment explaining the version choice). Thanks, Richard. Thanks, Richard
Re: Darwin bootstrap failure following wide int merge
On Wed, May 28, 2014 at 11:40 AM, Richard Biener richard.guent...@gmail.com wrote: On Wed, May 28, 2014 at 10:24 AM, Richard Sandiford rdsandif...@googlemail.com wrote: Richard Biener richard.guent...@gmail.com writes: On Wed, May 28, 2014 at 8:50 AM, Jakub Jelinek ja...@redhat.com wrote: On Mon, May 26, 2014 at 08:36:31AM -0700, Mike Stump wrote: On May 26, 2014, at 2:22 AM, FX fxcoud...@gmail.com wrote: This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? The best coarse of action, post a patch, have it reviewed and put in. Current action, a patch has been posted, the review is outstanding, I’d like to see it put in; though, I am curious why the casts were there in the first place. Note, haven't added them there, but from what I can test, the casts there can serve as a compile time check that the right type is used, e.g. unsigned long i; void foo (void) { asm volatile (# %0 %1 : =r ((unsigned long long) i) : 0 ((unsigned long long) 0)); } Ah, interesting. A not-so-hineous extension then. In that case, how about just protecting the include with: #if GCC_VERSION = 4300 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) rather than: #if GCC_VERSION = 3000 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) so that clang will fail the version check? At the end of the day we only really care what happens during stage 2 and 3. Cross-compilers built with recentish gccs will still benefit. Works for me (thus, pre-approved with a comment explaining the version choice). Btw, testing coverage would ask for defined (__OPTIMIZE__), too, disabling that path in stage1 unconditionally even for new GCC. Richard. Thanks, Richard. Thanks, Richard
Re: Darwin bootstrap failure following wide int merge
Richard Biener richard.guent...@gmail.com writes: On Wed, May 28, 2014 at 11:40 AM, Richard Biener richard.guent...@gmail.com wrote: On Wed, May 28, 2014 at 10:24 AM, Richard Sandiford rdsandif...@googlemail.com wrote: Richard Biener richard.guent...@gmail.com writes: On Wed, May 28, 2014 at 8:50 AM, Jakub Jelinek ja...@redhat.com wrote: On Mon, May 26, 2014 at 08:36:31AM -0700, Mike Stump wrote: On May 26, 2014, at 2:22 AM, FX fxcoud...@gmail.com wrote: This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? The best coarse of action, post a patch, have it reviewed and put in. Current action, a patch has been posted, the review is outstanding, I’d like to see it put in; though, I am curious why the casts were there in the first place. Note, haven't added them there, but from what I can test, the casts there can serve as a compile time check that the right type is used, e.g. unsigned long i; void foo (void) { asm volatile (# %0 %1 : =r ((unsigned long long) i) : 0 ((unsigned long long) 0)); } Ah, interesting. A not-so-hineous extension then. In that case, how about just protecting the include with: #if GCC_VERSION = 4300 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) rather than: #if GCC_VERSION = 3000 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) so that clang will fail the version check? At the end of the day we only really care what happens during stage 2 and 3. Cross-compilers built with recentish gccs will still benefit. Works for me (thus, pre-approved with a comment explaining the version choice). Btw, testing coverage would ask for defined (__OPTIMIZE__), too, disabling that path in stage1 unconditionally even for new GCC. Ah, but surely the day is near when we use -O or -Og for stage1? I've been using -O for a good few years now and it definitely makes things faster (and without affecting debugging too much -- in the rare cases where it does affect debugging, a recompile with -g is very quick). ATM we get the testing coverage for i686 and ppc32 hosts. So TBH I'd prefer to keep it simple and just bump the version number. Thanks, Richard
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Wed, May 28, 2014 at 11:38:55AM +0200, Richard Biener wrote: On Wed, May 28, 2014 at 10:24 AM, FX fxcoud...@gmail.com wrote: Yeah, a portable (C and C++) static assert would be nice. And also pushing this to gmp then. In the meantime I see nothing wrong in merging from GMP. One question, one comment: 1. can I count your “I see nothing wrong” as an approval, as in “global reviewers can approve changes to any part of the compiler or associated libraries”? Well, kind of. But Jakub is as well, so I don't want to override him. So please wait for an ack from Jakub. I agree with him that the casts served a purpose and that, if removed, they need to be replaced with an appropriate assertion measure. I think my preference would be to change include/ansidecl.h to: /* This macro simplifies testing whether we are using gcc, and if it is of a particular minimum version. (Both major minor numbers are significant.) This macro will evaluate to 0 if we are not using gcc at all.*/ #ifndef GCC_VERSION +/* Some compilers pretend to be GCC, even when they are not. */ +#if defined(__clang__) || defined(__INTEL_COMPILER) +#define GCC_VERSION 0 +#else #define GCC_VERSION (__GNUC__ * 1000 + __GNUC_MINOR__) +#endif #endif /* GCC_VERSION */ so that we really can trust the GCC_VERSION macro, casts in lhs of inline asm isn't the only incompatibility clang has. Any other compilers that define __GNUC__? Jakub
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Wed, May 28, 2014 at 11:48 AM, Jakub Jelinek ja...@redhat.com wrote: On Wed, May 28, 2014 at 11:38:55AM +0200, Richard Biener wrote: On Wed, May 28, 2014 at 10:24 AM, FX fxcoud...@gmail.com wrote: Yeah, a portable (C and C++) static assert would be nice. And also pushing this to gmp then. In the meantime I see nothing wrong in merging from GMP. One question, one comment: 1. can I count your “I see nothing wrong” as an approval, as in “global reviewers can approve changes to any part of the compiler or associated libraries”? Well, kind of. But Jakub is as well, so I don't want to override him. So please wait for an ack from Jakub. I agree with him that the casts served a purpose and that, if removed, they need to be replaced with an appropriate assertion measure. I think my preference would be to change include/ansidecl.h to: /* This macro simplifies testing whether we are using gcc, and if it is of a particular minimum version. (Both major minor numbers are significant.) This macro will evaluate to 0 if we are not using gcc at all.*/ #ifndef GCC_VERSION +/* Some compilers pretend to be GCC, even when they are not. */ +#if defined(__clang__) || defined(__INTEL_COMPILER) +#define GCC_VERSION 0 +#else #define GCC_VERSION (__GNUC__ * 1000 + __GNUC_MINOR__) +#endif #endif /* GCC_VERSION */ so that we really can trust the GCC_VERSION macro, casts in lhs of inline asm isn't the only incompatibility clang has. Any other compilers that define __GNUC__? Every one I guess. ICC 9 has it defined to 4, __GNUC_MINOR__ to 0. Richard. Jakub
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Wed, May 28, 2014 at 12:03 PM, Richard Biener richard.guent...@gmail.com wrote: On Wed, May 28, 2014 at 11:48 AM, Jakub Jelinek ja...@redhat.com wrote: On Wed, May 28, 2014 at 11:38:55AM +0200, Richard Biener wrote: On Wed, May 28, 2014 at 10:24 AM, FX fxcoud...@gmail.com wrote: Yeah, a portable (C and C++) static assert would be nice. And also pushing this to gmp then. In the meantime I see nothing wrong in merging from GMP. One question, one comment: 1. can I count your “I see nothing wrong” as an approval, as in “global reviewers can approve changes to any part of the compiler or associated libraries”? Well, kind of. But Jakub is as well, so I don't want to override him. So please wait for an ack from Jakub. I agree with him that the casts served a purpose and that, if removed, they need to be replaced with an appropriate assertion measure. I think my preference would be to change include/ansidecl.h to: /* This macro simplifies testing whether we are using gcc, and if it is of a particular minimum version. (Both major minor numbers are significant.) This macro will evaluate to 0 if we are not using gcc at all.*/ #ifndef GCC_VERSION +/* Some compilers pretend to be GCC, even when they are not. */ +#if defined(__clang__) || defined(__INTEL_COMPILER) +#define GCC_VERSION 0 +#else #define GCC_VERSION (__GNUC__ * 1000 + __GNUC_MINOR__) +#endif #endif /* GCC_VERSION */ so that we really can trust the GCC_VERSION macro, casts in lhs of inline asm isn't the only incompatibility clang has. Any other compilers that define __GNUC__? Every one I guess. ICC 9 has it defined to 4, __GNUC_MINOR__ to 0. So if we want to go down that route I'd rather change the configury that checks whether we are using GCC to be more pedantic and for example parse $CC --version output (not for the actual version but for whether $CC is GCC). Richard. Richard. Jakub
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Wed, May 28, 2014 at 12:07:41PM +0200, Richard Biener wrote: #ifndef GCC_VERSION +/* Some compilers pretend to be GCC, even when they are not. */ +#if defined(__clang__) || defined(__INTEL_COMPILER) +#define GCC_VERSION 0 +#else #define GCC_VERSION (__GNUC__ * 1000 + __GNUC_MINOR__) +#endif #endif /* GCC_VERSION */ so that we really can trust the GCC_VERSION macro, casts in lhs of inline asm isn't the only incompatibility clang has. Any other compilers that define __GNUC__? Every one I guess. ICC 9 has it defined to 4, __GNUC_MINOR__ to 0. ICC is included in the above check, that is the __INTEL_COMPILER check. So if we want to go down that route I'd rather change the configury that checks whether we are using GCC to be more pedantic and for example parse $CC --version output (not for the actual version but for whether $CC is GCC). include/ is shared among many GCC/binutils/GDB uses, using autoconf macros there would be nightmare, since you'd need to replicate those tests everywhere you'd use the header, plus lots of people customize the gcc --version output and it has changed over the years. Jakub
Re: Darwin bootstrap failure following wide int merge
On Wed, May 28, 2014 at 11:49 AM, Richard Sandiford rsand...@linux.vnet.ibm.com wrote: Richard Biener richard.guent...@gmail.com writes: On Wed, May 28, 2014 at 11:40 AM, Richard Biener richard.guent...@gmail.com wrote: On Wed, May 28, 2014 at 10:24 AM, Richard Sandiford rdsandif...@googlemail.com wrote: Richard Biener richard.guent...@gmail.com writes: On Wed, May 28, 2014 at 8:50 AM, Jakub Jelinek ja...@redhat.com wrote: On Mon, May 26, 2014 at 08:36:31AM -0700, Mike Stump wrote: On May 26, 2014, at 2:22 AM, FX fxcoud...@gmail.com wrote: This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? The best coarse of action, post a patch, have it reviewed and put in. Current action, a patch has been posted, the review is outstanding, I’d like to see it put in; though, I am curious why the casts were there in the first place. Note, haven't added them there, but from what I can test, the casts there can serve as a compile time check that the right type is used, e.g. unsigned long i; void foo (void) { asm volatile (# %0 %1 : =r ((unsigned long long) i) : 0 ((unsigned long long) 0)); } Ah, interesting. A not-so-hineous extension then. In that case, how about just protecting the include with: #if GCC_VERSION = 4300 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) rather than: #if GCC_VERSION = 3000 (W_TYPE_SIZE == 32 || defined (__SIZEOF_INT128__)) so that clang will fail the version check? At the end of the day we only really care what happens during stage 2 and 3. Cross-compilers built with recentish gccs will still benefit. Works for me (thus, pre-approved with a comment explaining the version choice). Btw, testing coverage would ask for defined (__OPTIMIZE__), too, disabling that path in stage1 unconditionally even for new GCC. Ah, but surely the day is near when we use -O or -Og for stage1? I've been using -O for a good few years now and it definitely makes things faster (and without affecting debugging too much -- in the rare cases where it does affect debugging, a recompile with -g is very quick). ATM we get the testing coverage for i686 and ppc32 hosts. So TBH I'd prefer to keep it simple and just bump the version number. Works for me (though see Jakubs idea on the other thread, so please wait until we settled on a solution). Richard. Thanks, Richard
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Wed, May 28, 2014 at 12:03:39PM +0200, Richard Biener wrote: Any other compilers that define __GNUC__? Every one I guess. ICC 9 has it defined to 4, __GNUC_MINOR__ to 0. For ICC it seems to depend on the ICC version, newer ICC versions report newer __GNUC_MINOR__. That said, ICC doesn't error out on the LHS cast for -m32, just silently drops the cast, so we really can't say it is compatible in this regard. unsigned long i; void foo (void) { asm volatile (# %0 %1 : =r ((unsigned long long) i) : 0 ((unsigned long long) 0)); } Jakub
Re: Darwin bootstrap failure following wide int merge
On Wed, May 28, 2014 at 12:18:13PM +0200, Richard Biener wrote: ATM we get the testing coverage for i686 and ppc32 hosts. So TBH I'd prefer to keep it simple and just bump the version number. Works for me (though see Jakubs idea on the other thread, so please wait until we settled on a solution). After lengthy IRC discussions, what Richard and I can live with is !defined(__clang__) in this particular case that uses longlong.h in GCC sources, with a comment why. If we get too many of these workarounds, we should reconsider. Jakub
Re: Darwin bootstrap failure following wide int merge
After lengthy IRC discussions, what Richard and I can live with is !defined(__clang__) in this particular case that uses longlong.h in GCC sources, with a comment why. I’ll test this patch and commit if there is no problem. But right now, current trunk doesn’t build on x86_64-apple-darwin due to error below. Richard, could this be due to your revision 211013? FX ../../trunk/gcc/rtl.c: In function ‘void cwi_output_hex(FILE*, const_rtx)’: ../../trunk/gcc/rtl.c:239:62: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long int’ [-Werror=format=] fprintf (outfile, HOST_WIDE_INT_PRINT_HEX, CWI_ELT (x, --i)); ^ ../../trunk/gcc/rtl.c:239:62: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long int’ [-Werror=format=] ../../trunk/gcc/rtl.c:241:69: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long int’ [-Werror=format=] fprintf (outfile, HOST_WIDE_INT_PRINT_PADDED_HEX, CWI_ELT (x, i)); ^ ../../trunk/gcc/rtl.c:241:69: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long int’ [-Werror=format=] cc1plus: all warnings being treated as errors make[3]: *** [build/rtl.o] Error 1 make[2]: *** [all-stage2-gcc] Error 2 make[1]: *** [stage2-bubble] Error 2 make: *** [all] Error 2
Re: Darwin bootstrap failure following wide int merge
On Wed, May 28, 2014 at 3:15 PM, FX fxcoud...@gmail.com wrote: After lengthy IRC discussions, what Richard and I can live with is !defined(__clang__) in this particular case that uses longlong.h in GCC sources, with a comment why. I’ll test this patch and commit if there is no problem. But right now, current trunk doesn’t build on x86_64-apple-darwin due to error below. Richard, could this be due to your revision 211013? Hum, yeah. But why does it even warn if sizeof (long) == sizeof (long long)? I suppose casting the result of CWI_ELT () to uint64_t fixes this. Do similar errors happen elsewhere? (the hex printfs expect unsigned types but CWI_ELT returns a signed HWI) Richard. FX ../../trunk/gcc/rtl.c: In function ‘void cwi_output_hex(FILE*, const_rtx)’: ../../trunk/gcc/rtl.c:239:62: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long int’ [-Werror=format=] fprintf (outfile, HOST_WIDE_INT_PRINT_HEX, CWI_ELT (x, --i)); ^ ../../trunk/gcc/rtl.c:239:62: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long int’ [-Werror=format=] ../../trunk/gcc/rtl.c:241:69: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long int’ [-Werror=format=] fprintf (outfile, HOST_WIDE_INT_PRINT_PADDED_HEX, CWI_ELT (x, i)); ^ ../../trunk/gcc/rtl.c:241:69: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long int’ [-Werror=format=] cc1plus: all warnings being treated as errors make[3]: *** [build/rtl.o] Error 1 make[2]: *** [all-stage2-gcc] Error 2 make[1]: *** [stage2-bubble] Error 2 make: *** [all] Error 2
Re: Darwin bootstrap failure following wide int merge
On Wed, May 28, 2014 at 03:47:52PM +0200, Richard Biener wrote: On Wed, May 28, 2014 at 3:15 PM, FX fxcoud...@gmail.com wrote: After lengthy IRC discussions, what Richard and I can live with is !defined(__clang__) in this particular case that uses longlong.h in GCC sources, with a comment why. I’ll test this patch and commit if there is no problem. But right now, current trunk doesn’t build on x86_64-apple-darwin due to error below. Richard, could this be due to your revision 211013? Hum, yeah. But why does it even warn if sizeof (long) == sizeof (long long)? Because using the right format strings is important for portability. I suppose casting the result of CWI_ELT () to uint64_t fixes this. Do similar errors happen elsewhere? (the hex printfs expect unsigned types but CWI_ELT returns a signed HWI) I think the sign shouldn't be a problem, but perhaps there is a mismatch between HOST_WIDE_INT definition and HOST_WIDE_INT_PRINT_HEX? Defining HOST_WIDE_INT to long long or long based on whether long is 64-bit or not, but using PRIx64 etc. unconditionally is just wrong, either HOST_WIDE_INT should be uint64_t and then you should use PRI*64, or it is not, and then you should keep using what has been used in the past, different macros depending on how HOST_WIDE_INT is defined. Jakub
Re: Darwin bootstrap failure following wide int merge
On Wed, May 28, 2014 at 04:00:32PM +0200, Jakub Jelinek wrote: Defining HOST_WIDE_INT to long long or long based on whether long is 64-bit or not, but using PRIx64 etc. unconditionally is just wrong, either HOST_WIDE_INT should be uint64_t and then you should use PRI*64, or it is not, and then you should keep using what has been used in the past, different macros depending on how HOST_WIDE_INT is defined. And, note that even the fallback PRI*64 definitions if not defined in system headers already probably need to depend on autoconf test finding out whether uint64_t is unsigned long or unsigned long long (and failing otherwise, unless PRI*64 is defined in the headers). Guess the test can easily try to compile some proglet with a template to find out what uint64_t is. Jakub
Re: Darwin bootstrap failure following wide int merge
I suppose casting the result of CWI_ELT () to uint64_t fixes this. Do similar errors happen elsewhere? I don’t think you can cast to uint64_t, as host wide int might be some other type, no? There are others: ../../trunk/gcc/print-rtl.c: In function ‘void print_rtx(const_rtx)’: ../../trunk/gcc/print-rtl.c:385:62: error: format ‘%lld’ expects argument of type ‘long long int’, but argument 3 has type ‘long int’ [-Werror=format=] fprintf (outfile, HOST_WIDE_INT_PRINT_DEC, XWINT (in_rtx, i)); ^ ../../trunk/gcc/print-rtl.c:385:62: error: format ‘%lld’ expects argument of type ‘long long int’, but argument 3 has type ‘long int’ [-Werror=format=] ../../trunk/gcc/print-rtl.c:388:48: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long unsigned int’ [-Werror=format=] (unsigned HOST_WIDE_INT) XWINT (in_rtx, i)); ^ ../../trunk/gcc/print-rtl.c:388:48: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘long unsigned int’ [-Werror=format=] ../../trunk/gcc/genemit.c: In function ‘void gen_exp(rtx, rtx_code, char*)’: ../../trunk/gcc/genemit.c:200:49: error: format ‘%lld’ expects argument of type ‘long long int’, but argument 2 has type ‘long int’ [-Werror=format=] printf (HOST_WIDE_INT_PRINT_DEC_C, INTVAL (x)); ^ ../../trunk/gcc/genemit.c:200:49: error: format ‘%lld’ expects argument of type ‘long long int’, but argument 2 has type ‘long int’ [-Werror=format=] ../../trunk/gcc/genattrtab.c: In function ‘unsigned int write_test_expr(FILE*, rtx, unsigned int, int)’: ../../trunk/gcc/genattrtab.c:3758:61: error: format ‘%lld’ expects argument of type ‘long long int’, but argument 3 has type ‘long int’ [-Werror=format=] fprintf (outf, HOST_WIDE_INT_PRINT_DEC, XWINT (exp, 0)); ^ ../../trunk/gcc/genattrtab.c:3758:61: error: format ‘%lld’ expects argument of type ‘long long int’, but argument 3 has type ‘long int’ [-Werror=format=] ../../trunk/gcc/genattrtab.c: In function ‘void write_attr_value(FILE*, attr_desc*, rtx)’: ../../trunk/gcc/genattrtab.c:4350:61: error: format ‘%lld’ expects argument of type ‘long long int’, but argument 3 has type ‘long int’ [-Werror=format=] fprintf (outf, HOST_WIDE_INT_PRINT_DEC, INTVAL (value)); ^ ../../trunk/gcc/genattrtab.c:4350:61: error: format ‘%lld’ expects argument of type ‘long long int’, but argument 3 has type ‘long int’ [-Werror=format=]
Re: Darwin bootstrap failure following wide int merge
After lengthy IRC discussions, what Richard and I can live with is !defined(__clang__) in this particular case that uses longlong.h in GCC sources, with a comment why. If we get too many of these workarounds, we should reconsider. Committed as revision 211023, after bootstrap on x86_64-apple-darwin13. Thanks! FX patch Description: application/applefile CL Description: Binary data
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
Ping? Or, I can ask, any objections? In https://gcc.gnu.org/PR61146 it is stated that GMP removed the casts in 2005. On May 26, 2014, at 4:26 AM, FX fxcoud...@gmail.com wrote: So changing just 2 of them doesn't feel right to me… [Again, with the patch actually attached… sorry] Here’s a patch that removes all the casts on output operands in x86/x86_64 code in longlong.h. Again bootstrapped on x86_64-apple-darwin13, passing both stage1 (system compiler) and stages 2-3 (gcc). OK to commit? Other archs which have such code are arc, arm, hppa, m32r, mc68000, mc68020, ns32000, ibm032, sparc, and vax. Since I don’t have any of those to test on, I can’t test it there. If you or another global reviewer indicate that a patch extending the work attached to these other archs is suitable, I’m willing to do the tedious work of proposing a full patch, but I won’t be able to test it (and I didn’t want to do it if it had no chance of being accepted). Thanks, FX longlong.diff Description: Binary data longlong.ChangeLog Description: Binary data
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
Or, I can ask, any objections? In https://gcc.gnu.org/PR61146 it is stated that GMP removed the casts in 2005. Among the many many versions of longlong.h that one can find around the web, many have don’t have these casts, including GMP and coreutils (http://code.metager.de/source/xref/gnu/coreutils/src/longlong.h). FX
Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
.././../gcc-4.10-20140518/gcc/wide-int.cc:1274:23: error: invalid use of a cast in a inline asm context requiring an l-value: remove the cast or build with -fheinous-gnu-extensions umul_ppmm (val[1], val[0], op1.ulow (), op2.ulow ()); ~~~^ This is PR 61146. You can get around it by adding -fheinous-gnu-extensions to BOOT_CFLAGS. This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Regarding PR 61146, I agree with Marc Glisse (comment #3) that the casts in question look weird and should probably be removed, as was done in GMP. This code should be cleaned up, and it will fix bootstrap on clang-based target coincidentally, without adding another kludge. FX
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? FX
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Mon, May 26, 2014 at 2:22 AM, FX fxcoud...@gmail.com wrote: This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? The failure is again a bug in clang compiler of defining __GNUC__ when they don't fully support GNU C if they want to say it is not a bug to define __GNUC__ I give up. This is just like the bug a while back where ICC did the same thing. We need to shame compiler developers to stop saying they support GNU C without really supporting it. Work arounds for broken commercial compilers is something which we tried back in the 90s; We really should not still be keeping around any work around for them either. Thanks, Andrew PInski FX
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Mon, May 26, 2014 at 10:14 AM, FX fxcoud...@gmail.com wrote: .././../gcc-4.10-20140518/gcc/wide-int.cc:1274:23: error: invalid use of a cast in a inline asm context requiring an l-value: remove the cast or build with -fheinous-gnu-extensions umul_ppmm (val[1], val[0], op1.ulow (), op2.ulow ()); ~~~^ This is PR 61146. You can get around it by adding -fheinous-gnu-extensions to BOOT_CFLAGS. This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Regarding PR 61146, I agree with Marc Glisse (comment #3) that the casts in question look weird and should probably be removed, as was done in GMP. This code should be cleaned up, and it will fix bootstrap on clang-based target coincidentally, without adding another kludge. Please post a patch. Thanks, Richard. FX
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Mon, May 26, 2014 at 1:14 AM, FX fxcoud...@gmail.com wrote: .././../gcc-4.10-20140518/gcc/wide-int.cc:1274:23: error: invalid use of a cast in a inline asm context requiring an l-value: remove the cast or build with -fheinous-gnu-extensions umul_ppmm (val[1], val[0], op1.ulow (), op2.ulow ()); ~~~^ This is PR 61146. You can get around it by adding -fheinous-gnu-extensions to BOOT_CFLAGS. This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Regarding PR 61146, I agree with Marc Glisse (comment #3) that the casts in question look weird and should probably be removed, as was done in GMP. This code should be cleaned up, and it will fix bootstrap on clang-based target coincidentally, without adding another kludge. I think that posting a patch is probably the best bet. Then the various merits of the patch to clean up the code can be argued. As far as some sort of workaround, I'd suggest seeing if there's something else that can be done first. Thanks. -eric
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
Please post a patch. How about that? I’m not doing a full clean-up of the longlong.h code outside the area affected. This restores bootstrap on darwin, confirming that both the system compiler and later-stage-gcc accepts it. FX longlong.diff Description: Binary data longlong.ChangeLog Description: Binary data
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On Mon, May 26, 2014 at 12:32:15PM +0200, FX wrote: Please post a patch. How about that? I’m not doing a full clean-up of the longlong.h code outside the area affected. This restores bootstrap on darwin, confirming that both the system compiler and later-stage-gcc accepts it. grep '=.*\(U[SD]Itype\|unsigned int\)' longlong.h | wc -l 98 So changing just 2 of them doesn't feel right to me... Jakub
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
So changing just 2 of them doesn't feel right to me… Here’s a patch that removes all the casts on output operands in x86/x86_64 code in longlong.h. Again bootstrapped on x86_64-apple-darwin13, passing both stage1 (system compiler) and stages 2-3 (gcc). OK to commit? Other archs which have such code are arc, arm, hppa, m32r, mc68000, mc68020, ns32000, ibm032, sparc, and vax. Since I don’t have any of those to test on, I can’t test it there. If you or another global reviewer indicate that a patch extending the work attached to these other archs is suitable, I’m willing to do the tedious work of proposing a full patch, but I won’t be able to test it (and I didn’t want to do it if it had no chance of being accepted). Thanks, FX longlong.ChangeLog Description: Binary data longlong.ChangeLog Description: Binary data
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
So changing just 2 of them doesn't feel right to me… [Again, with the patch actually attached… sorry] Here’s a patch that removes all the casts on output operands in x86/x86_64 code in longlong.h. Again bootstrapped on x86_64-apple-darwin13, passing both stage1 (system compiler) and stages 2-3 (gcc). OK to commit? Other archs which have such code are arc, arm, hppa, m32r, mc68000, mc68020, ns32000, ibm032, sparc, and vax. Since I don’t have any of those to test on, I can’t test it there. If you or another global reviewer indicate that a patch extending the work attached to these other archs is suitable, I’m willing to do the tedious work of proposing a full patch, but I won’t be able to test it (and I didn’t want to do it if it had no chance of being accepted). Thanks, FX longlong.diff Description: Binary data longlong.ChangeLog Description: Binary data
Re: Darwin bootstrap failure following wide int merge (was: we are starting the wide int merge)
On May 26, 2014, at 2:22 AM, FX fxcoud...@gmail.com wrote: This causes GCC bootstrap to fail on Darwin systems (whose system compiler is clang-based). Since PR 61146 was resolved as INVALID (but I’m not sure it’s the right call, see below), I’ve filed a separate report for the bootstrap issue (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61315). Since my PR has been closed twice by Andrew Pinski (“it’s clang’s fault, bouh ouh”), I’d ask the maintainers to step in. Can we please provide a GCC that works for the default darwin setup? Or at least drop darwin as secondary target and document the failure? The best coarse of action, post a patch, have it reviewed and put in. Current action, a patch has been posted, the review is outstanding, I’d like to see it put in; though, I am curious why the casts were there in the first place. Another way out, #ifndef clang the entire thing and disappear the contents for clang. I’d be fine with that as well, ultimately the longlong.h people will have to choose how they want to solve the issue. The state of the PR system isn’t as concerning to me as the state of the software tree. The bug _will_ get fixed, one way, or, another. The current patch posted that removes the casts seems like the best possible solution to me.