Re: [PATCH] Avoid invoking ranlib on libbackend.a
On Sat, Jul 16, 2016 at 6:35 PM, Andrew Pinskiwrote: > On Sat, Jul 16, 2016 at 3:13 PM, Patrick Palka wrote: >> On Sat, 16 Jul 2016, Patrick Palka wrote: >> >>> This patch makes the 0.5GB libbackend.a get built via "ar rcs" instead >>> of via "ar rc" + "ranlib", if possible. The problem with the latter is >>> that it's about twice as slow as the former and it makes libbackend.a >>> get written twice which is wasteful. On my machine this optimization >>> reduces rebuild time in a --disable-bootstrap tree after touching tree.c >>> by around 25%, from ~28s to ~21s. This optimization is only performed >>> when the current ar is GNU ar where we can be reasonably sure that the >>> "rcs" directive is supported. >>> >>> Does this change look reasonable? I am currently bootstrapping it on >>> x86_64-pc-linux-gnu. >>> >>> gcc/ChangeLog: >>> >>> * configure.ac (ar_is_gnu): New variable. AC_SUBST it. >>> * configure: Regenerate. >>> * Makefile.in (AR_IS_GNU): New variable. >>> (USE_AR_RCS): New variable. >>> (libbackend.a): When building this archive, if USE_AR_RCS then >>> just invoke "ar rcs" instead of invoking "ar rc" followed by >>> "ranlib". >>> --- >>> gcc/Makefile.in | 18 ++ >>> gcc/configure| 15 +-- >>> gcc/configure.ac | 8 >>> 3 files changed, 39 insertions(+), 2 deletions(-) >>> >>> diff --git a/gcc/Makefile.in b/gcc/Makefile.in >>> index 0786fa3..5ae6100 100644 >>> --- a/gcc/Makefile.in >>> +++ b/gcc/Makefile.in >>> @@ -237,10 +237,22 @@ FLEX = @FLEX@ >>> FLEXFLAGS = >>> AR = @AR@ >>> AR_FLAGS = rc >>> +AR_IS_GNU = @ar_is_gnu@ >>> NM = @NM@ >>> RANLIB = @RANLIB@ >>> RANLIB_FLAGS = @ranlib_flags@ >>> >>> +# Whether invocations of ar + ranlib can be replaced by a single >>> +# invocation of "ar rcs". >>> +USE_AR_RCS = no >>> +ifeq ($(AR_IS_GNU),yes) >>> +ifeq ($(AR_FLAGS),rc) >>> +ifeq ($(RANLIB_FLAGS),) >>> +USE_AR_RCS = yes >>> +endif >>> +endif >>> +endif >>> + >>> # Libraries to use on the host. >>> HOST_LIBS = @HOST_LIBS@ >>> >>> @@ -1882,8 +1894,14 @@ compilations: $(BACKEND) >>> # This archive is strictly for the host. >>> libbackend.a: $(OBJS) >>> -rm -rf libbackend.a >>> + @# Elide the invocation of ranlib if possible, as doing so >>> + @# significantly reduces the build time of this archive. >>> +ifeq ($(USE_AR_RCS),yes) >>> + $(AR) $(AR_FLAGS)s libbackend.a $(OBJS) >>> +else >>> $(AR) $(AR_FLAGS) libbackend.a $(OBJS) >>> -$(RANLIB) $(RANLIB_FLAGS) libbackend.a >>> +endif >>> >>> libcommon-target.a: $(OBJS-libcommon-target) >>> -rm -rf libcommon-target.a >>> diff --git a/gcc/configure b/gcc/configure >>> index ed44472..db761c8 100755 >>> --- a/gcc/configure >>> +++ b/gcc/configure >>> @@ -749,6 +749,7 @@ CXXDEPMODE >>> DEPDIR >>> am__leading_dot >>> doc_build_sys >>> +ar_is_gnu >>> AR >>> NM >>> BISON >>> @@ -8459,6 +8460,16 @@ fi >>> >>> fi >>> >>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ar is GNU ar" >&5 >>> +$as_echo_n "checking whether ar is GNU ar... " >&6; } >>> +ar_is_gnu=no >>> +if $AR --version 2>/dev/null | sed 1q | grep "GNU ar" > /dev/null; then >>> + ar_is_gnu=yes >>> +fi >>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ar_is_gnu" >&5 >>> +$as_echo "$ar_is_gnu" >&6; } >>> + >>> + >>> # The jit documentation looks better if built with sphinx, but can be >>> # built with texinfo if sphinx is not available. >>> # Set "doc_build_sys" to "sphinx" or "texinfo" accordingly. >>> @@ -18475,7 +18486,7 @@ else >>>lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 >>>lt_status=$lt_dlunknown >>>cat > conftest.$ac_ext <<_LT_EOF >>> -#line 18478 "configure" >>> +#line 18489 "configure" >>> #include "confdefs.h" >>> >>> #if HAVE_DLFCN_H >>> @@ -18581,7 +18592,7 @@ else >>>lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 >>>lt_status=$lt_dlunknown >>>cat > conftest.$ac_ext <<_LT_EOF >>> -#line 18584 "configure" >>> +#line 18595 "configure" >>> #include "confdefs.h" >>> >>> #if HAVE_DLFCN_H >>> diff --git a/gcc/configure.ac b/gcc/configure.ac >>> index 086d0fc..3749b21 100644 >>> --- a/gcc/configure.ac >>> +++ b/gcc/configure.ac >>> @@ -1081,6 +1081,14 @@ else >>>AC_CHECK_PROG(AR, ar, ar, ${CONFIG_SHELL-/bin/sh} ${srcdir}/../missing >>> ar) >>> fi >>> >>> +AC_MSG_CHECKING(whether ar is GNU ar) >>> +ar_is_gnu=no >>> +if $AR --version 2>/dev/null | sed 1q | grep "GNU ar" > /dev/null; then >>> + ar_is_gnu=yes >>> +fi >>> +AC_MSG_RESULT($ar_is_gnu) >>> +AC_SUBST(ar_is_gnu) >>> + >>> # The jit documentation looks better if built with sphinx, but can be >>> # built with texinfo if sphinx is not available. >>> # Set "doc_build_sys" to "sphinx" or "texinfo" accordingly. >>> -- >>> 2.9.2.321.g3e7e728 >>> >>> >> >> Even better, why build libbackend.a at all? Why not just pass the list >> of object files directly to the linker instead of going through
Re: RFA: new pass to warn on questionable uses of alloca() and VLAs
I forgot to ask, Aldy: Have you tried using LTO with the warning? I haven't managed to get it to warn even though I expected it to. The test I used is below. [...more testing...] After some more testing I discovered that LTO somehow seems to suppress this and other warnings (including the new -Wformat-length I've been working on). I haven't spent any time figuring out why but I have raised bug 71907 for it. Thanks Martin $ (CC='/build/gcc-walloca/gcc/xgcc -B /build/gcc-walloca/gcc'; CFLAGS='-O2 -Wall -Wextra -Wpedantic -Walloca-larger-than=32 -flto'; cat x.c && $CC -c x.c && $CC -DMAIN -DN=12345678 -c -o main.o x.c && $CC $CFLAGS x.o main.o && gdb -batch -q -ex r -ex bt ./a.out ) extern void f0 (void*, void*); extern unsigned f1 (void); extern void f2 (void); #if MAIN void f0 (void *p, void *q) { __builtin_printf ("p = %p, q = %p, d = %td\n", p, q, (char*)p - (char*)q); } unsigned f1 (void) { return N; } int main (void) { f2 (); } #else void f2 (void) { void *p = void *q = __builtin_alloca (f1 ()); f0 (p, q); } #endif Program received signal SIGSEGV, Segmentation fault. 0x00400592 in f2 () #0 0x00400592 in f2 () #1 0x004005e9 in main () On 07/16/2016 03:07 PM, Martin Sebor wrote: Done. -Walloca and -Wvla warn on any use of alloca and VLAs accordingly, with or without optimization. I sorry() on the bounded cases. I think it's an improvement though I suspect we each have a slightly different understanding of what the sorry message is meant to be used for. It's documented in the Diagnostics Conventions section of the GCC Coding Conventions as: sorry is for correct user input programs but unimplemented functionalities. I take that to mean that it should be used for what is considered valid user input that cannot be processed because the functionality is not yet implemented (but eventually will be). So unless this case falls into this category I would expect GCC to issue a warning saying that the options have no effect (or limited effect, whatever the case may be) without optimization. But maybe I'm not reading the coding conventions text right. 2) When passed an argument of a signed type, GCC prints warning: cast from signed type in alloca even though there is no explicit cast in the code. It may not be obvious why the conversion is a problem in this context. I would suggest to rephrase the warning along the lines of -Wsign-conversion which prints: conversion to ‘long unsigned int’ from ‘int’ may change the sign of the result and add why it's a potential problem. Perhaps something like: argument to alloca may be too large due to conversion from 'int to 'long unsigned int' Fixed: Cool. FWIW, by coincidence I was just educated about the subtle nuances of quoting in GCC messages in a discussion with David and Manu. Types, functions, variables, and literals that appear in the source code should be referenced in diagnostics by using the "%qT", "%qD", and "%qE" directives so that GCC can add the right quotes and highlighting. Enclosing "'%T'" in quotes will not use the same kind of quotes as "%qT" and won't highlight the type name. https://gcc.gnu.org/wiki/DiagnosticsGuidelines There is a "documented" reason for this: :) // Do not warn on VLAs occurring in a loop, since VLAs are // guaranteed to be cleaned up when they go out of scope. // That is, there is a corresponding __builtin_stack_restore // at the end of the scope in which the VLA occurs. Yes, I understand that VLAs in loops are treated differently than alloca. But I don't think this is quite how the logic should work. I.e., an excessively large VLA should be diagnosed regardless of whether it's in a loop or outside. Consider the following case where with the patch as is, the warning is issued only for one of the two functions, even though they both allocate a VLA in excess of the threshold. #define FOO(n) if (1) { \ char a [n]; \ f (a); \ } else (void)0 #define BAR(n) do { \ char a [n]; \ f (a); \ } while (0) void f (void*); void foo (void) { int n = 8000; FOO (n);// warning with -Wla-larger-than=4000 } void bar (void) { int n = 8000; BAR (n);// no warning } 5) The -Wvla=N logic only seems to take into consideration the number of elements but not the size of the element type. For example, I wasn't able to get it to warn on the following with -Wvla=255 or greater: void f0 (void*); void f1 (unsigned char a) { int x [a]; // or even char a [n][__INT_MAX__]; f0 (x); } That was a huge oversight (or should I say over-engineering) on my part. Fixed. Looks good. I did notice one minor glitch, though not one caused by your patch. GCC apparently transforms simple VLAs that are 256 bytes or less in size into ordinary arrays (i.e., it doesn't call __builtin_alloca_with_align). Because of that,
Re: [PATCH] Avoid invoking ranlib on libbackend.a
On Sat, Jul 16, 2016 at 3:13 PM, Patrick Palkawrote: > On Sat, 16 Jul 2016, Patrick Palka wrote: > >> This patch makes the 0.5GB libbackend.a get built via "ar rcs" instead >> of via "ar rc" + "ranlib", if possible. The problem with the latter is >> that it's about twice as slow as the former and it makes libbackend.a >> get written twice which is wasteful. On my machine this optimization >> reduces rebuild time in a --disable-bootstrap tree after touching tree.c >> by around 25%, from ~28s to ~21s. This optimization is only performed >> when the current ar is GNU ar where we can be reasonably sure that the >> "rcs" directive is supported. >> >> Does this change look reasonable? I am currently bootstrapping it on >> x86_64-pc-linux-gnu. >> >> gcc/ChangeLog: >> >> * configure.ac (ar_is_gnu): New variable. AC_SUBST it. >> * configure: Regenerate. >> * Makefile.in (AR_IS_GNU): New variable. >> (USE_AR_RCS): New variable. >> (libbackend.a): When building this archive, if USE_AR_RCS then >> just invoke "ar rcs" instead of invoking "ar rc" followed by >> "ranlib". >> --- >> gcc/Makefile.in | 18 ++ >> gcc/configure| 15 +-- >> gcc/configure.ac | 8 >> 3 files changed, 39 insertions(+), 2 deletions(-) >> >> diff --git a/gcc/Makefile.in b/gcc/Makefile.in >> index 0786fa3..5ae6100 100644 >> --- a/gcc/Makefile.in >> +++ b/gcc/Makefile.in >> @@ -237,10 +237,22 @@ FLEX = @FLEX@ >> FLEXFLAGS = >> AR = @AR@ >> AR_FLAGS = rc >> +AR_IS_GNU = @ar_is_gnu@ >> NM = @NM@ >> RANLIB = @RANLIB@ >> RANLIB_FLAGS = @ranlib_flags@ >> >> +# Whether invocations of ar + ranlib can be replaced by a single >> +# invocation of "ar rcs". >> +USE_AR_RCS = no >> +ifeq ($(AR_IS_GNU),yes) >> +ifeq ($(AR_FLAGS),rc) >> +ifeq ($(RANLIB_FLAGS),) >> +USE_AR_RCS = yes >> +endif >> +endif >> +endif >> + >> # Libraries to use on the host. >> HOST_LIBS = @HOST_LIBS@ >> >> @@ -1882,8 +1894,14 @@ compilations: $(BACKEND) >> # This archive is strictly for the host. >> libbackend.a: $(OBJS) >> -rm -rf libbackend.a >> + @# Elide the invocation of ranlib if possible, as doing so >> + @# significantly reduces the build time of this archive. >> +ifeq ($(USE_AR_RCS),yes) >> + $(AR) $(AR_FLAGS)s libbackend.a $(OBJS) >> +else >> $(AR) $(AR_FLAGS) libbackend.a $(OBJS) >> -$(RANLIB) $(RANLIB_FLAGS) libbackend.a >> +endif >> >> libcommon-target.a: $(OBJS-libcommon-target) >> -rm -rf libcommon-target.a >> diff --git a/gcc/configure b/gcc/configure >> index ed44472..db761c8 100755 >> --- a/gcc/configure >> +++ b/gcc/configure >> @@ -749,6 +749,7 @@ CXXDEPMODE >> DEPDIR >> am__leading_dot >> doc_build_sys >> +ar_is_gnu >> AR >> NM >> BISON >> @@ -8459,6 +8460,16 @@ fi >> >> fi >> >> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ar is GNU ar" >&5 >> +$as_echo_n "checking whether ar is GNU ar... " >&6; } >> +ar_is_gnu=no >> +if $AR --version 2>/dev/null | sed 1q | grep "GNU ar" > /dev/null; then >> + ar_is_gnu=yes >> +fi >> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ar_is_gnu" >&5 >> +$as_echo "$ar_is_gnu" >&6; } >> + >> + >> # The jit documentation looks better if built with sphinx, but can be >> # built with texinfo if sphinx is not available. >> # Set "doc_build_sys" to "sphinx" or "texinfo" accordingly. >> @@ -18475,7 +18486,7 @@ else >>lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 >>lt_status=$lt_dlunknown >>cat > conftest.$ac_ext <<_LT_EOF >> -#line 18478 "configure" >> +#line 18489 "configure" >> #include "confdefs.h" >> >> #if HAVE_DLFCN_H >> @@ -18581,7 +18592,7 @@ else >>lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 >>lt_status=$lt_dlunknown >>cat > conftest.$ac_ext <<_LT_EOF >> -#line 18584 "configure" >> +#line 18595 "configure" >> #include "confdefs.h" >> >> #if HAVE_DLFCN_H >> diff --git a/gcc/configure.ac b/gcc/configure.ac >> index 086d0fc..3749b21 100644 >> --- a/gcc/configure.ac >> +++ b/gcc/configure.ac >> @@ -1081,6 +1081,14 @@ else >>AC_CHECK_PROG(AR, ar, ar, ${CONFIG_SHELL-/bin/sh} ${srcdir}/../missing ar) >> fi >> >> +AC_MSG_CHECKING(whether ar is GNU ar) >> +ar_is_gnu=no >> +if $AR --version 2>/dev/null | sed 1q | grep "GNU ar" > /dev/null; then >> + ar_is_gnu=yes >> +fi >> +AC_MSG_RESULT($ar_is_gnu) >> +AC_SUBST(ar_is_gnu) >> + >> # The jit documentation looks better if built with sphinx, but can be >> # built with texinfo if sphinx is not available. >> # Set "doc_build_sys" to "sphinx" or "texinfo" accordingly. >> -- >> 2.9.2.321.g3e7e728 >> >> > > Even better, why build libbackend.a at all? Why not just pass the list > of object files directly to the linker instead of going through an > intermediate archive? The following patch cuts rebuild time by 50%, > from 28s to 14s by making libbackend.a no longer get built. (The > Make-lang.in hunk is apparently necessary to avoid duplicate-reference > link
Re: [PATCH] Avoid invoking ranlib on libbackend.a
On Sat, 16 Jul 2016, Patrick Palka wrote: > This patch makes the 0.5GB libbackend.a get built via "ar rcs" instead > of via "ar rc" + "ranlib", if possible. The problem with the latter is > that it's about twice as slow as the former and it makes libbackend.a > get written twice which is wasteful. On my machine this optimization > reduces rebuild time in a --disable-bootstrap tree after touching tree.c > by around 25%, from ~28s to ~21s. This optimization is only performed > when the current ar is GNU ar where we can be reasonably sure that the > "rcs" directive is supported. > > Does this change look reasonable? I am currently bootstrapping it on > x86_64-pc-linux-gnu. > > gcc/ChangeLog: > > * configure.ac (ar_is_gnu): New variable. AC_SUBST it. > * configure: Regenerate. > * Makefile.in (AR_IS_GNU): New variable. > (USE_AR_RCS): New variable. > (libbackend.a): When building this archive, if USE_AR_RCS then > just invoke "ar rcs" instead of invoking "ar rc" followed by > "ranlib". > --- > gcc/Makefile.in | 18 ++ > gcc/configure| 15 +-- > gcc/configure.ac | 8 > 3 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index 0786fa3..5ae6100 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -237,10 +237,22 @@ FLEX = @FLEX@ > FLEXFLAGS = > AR = @AR@ > AR_FLAGS = rc > +AR_IS_GNU = @ar_is_gnu@ > NM = @NM@ > RANLIB = @RANLIB@ > RANLIB_FLAGS = @ranlib_flags@ > > +# Whether invocations of ar + ranlib can be replaced by a single > +# invocation of "ar rcs". > +USE_AR_RCS = no > +ifeq ($(AR_IS_GNU),yes) > +ifeq ($(AR_FLAGS),rc) > +ifeq ($(RANLIB_FLAGS),) > +USE_AR_RCS = yes > +endif > +endif > +endif > + > # Libraries to use on the host. > HOST_LIBS = @HOST_LIBS@ > > @@ -1882,8 +1894,14 @@ compilations: $(BACKEND) > # This archive is strictly for the host. > libbackend.a: $(OBJS) > -rm -rf libbackend.a > + @# Elide the invocation of ranlib if possible, as doing so > + @# significantly reduces the build time of this archive. > +ifeq ($(USE_AR_RCS),yes) > + $(AR) $(AR_FLAGS)s libbackend.a $(OBJS) > +else > $(AR) $(AR_FLAGS) libbackend.a $(OBJS) > -$(RANLIB) $(RANLIB_FLAGS) libbackend.a > +endif > > libcommon-target.a: $(OBJS-libcommon-target) > -rm -rf libcommon-target.a > diff --git a/gcc/configure b/gcc/configure > index ed44472..db761c8 100755 > --- a/gcc/configure > +++ b/gcc/configure > @@ -749,6 +749,7 @@ CXXDEPMODE > DEPDIR > am__leading_dot > doc_build_sys > +ar_is_gnu > AR > NM > BISON > @@ -8459,6 +8460,16 @@ fi > > fi > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ar is GNU ar" >&5 > +$as_echo_n "checking whether ar is GNU ar... " >&6; } > +ar_is_gnu=no > +if $AR --version 2>/dev/null | sed 1q | grep "GNU ar" > /dev/null; then > + ar_is_gnu=yes > +fi > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ar_is_gnu" >&5 > +$as_echo "$ar_is_gnu" >&6; } > + > + > # The jit documentation looks better if built with sphinx, but can be > # built with texinfo if sphinx is not available. > # Set "doc_build_sys" to "sphinx" or "texinfo" accordingly. > @@ -18475,7 +18486,7 @@ else >lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 >lt_status=$lt_dlunknown >cat > conftest.$ac_ext <<_LT_EOF > -#line 18478 "configure" > +#line 18489 "configure" > #include "confdefs.h" > > #if HAVE_DLFCN_H > @@ -18581,7 +18592,7 @@ else >lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 >lt_status=$lt_dlunknown >cat > conftest.$ac_ext <<_LT_EOF > -#line 18584 "configure" > +#line 18595 "configure" > #include "confdefs.h" > > #if HAVE_DLFCN_H > diff --git a/gcc/configure.ac b/gcc/configure.ac > index 086d0fc..3749b21 100644 > --- a/gcc/configure.ac > +++ b/gcc/configure.ac > @@ -1081,6 +1081,14 @@ else >AC_CHECK_PROG(AR, ar, ar, ${CONFIG_SHELL-/bin/sh} ${srcdir}/../missing ar) > fi > > +AC_MSG_CHECKING(whether ar is GNU ar) > +ar_is_gnu=no > +if $AR --version 2>/dev/null | sed 1q | grep "GNU ar" > /dev/null; then > + ar_is_gnu=yes > +fi > +AC_MSG_RESULT($ar_is_gnu) > +AC_SUBST(ar_is_gnu) > + > # The jit documentation looks better if built with sphinx, but can be > # built with texinfo if sphinx is not available. > # Set "doc_build_sys" to "sphinx" or "texinfo" accordingly. > -- > 2.9.2.321.g3e7e728 > > Even better, why build libbackend.a at all? Why not just pass the list of object files directly to the linker instead of going through an intermediate archive? The following patch cuts rebuild time by 50%, from 28s to 14s by making libbackend.a no longer get built. (The Make-lang.in hunk is apparently necessary to avoid duplicate-reference link errors since incpath.o is already listed in $(OBJS)). diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 5ae6100..7b8ec16 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1591,7 +1591,7 @@ endif #
Re: RFA: new pass to warn on questionable uses of alloca() and VLAs
Done. -Walloca and -Wvla warn on any use of alloca and VLAs accordingly, with or without optimization. I sorry() on the bounded cases. I think it's an improvement though I suspect we each have a slightly different understanding of what the sorry message is meant to be used for. It's documented in the Diagnostics Conventions section of the GCC Coding Conventions as: sorry is for correct user input programs but unimplemented functionalities. I take that to mean that it should be used for what is considered valid user input that cannot be processed because the functionality is not yet implemented (but eventually will be). So unless this case falls into this category I would expect GCC to issue a warning saying that the options have no effect (or limited effect, whatever the case may be) without optimization. But maybe I'm not reading the coding conventions text right. 2) When passed an argument of a signed type, GCC prints warning: cast from signed type in alloca even though there is no explicit cast in the code. It may not be obvious why the conversion is a problem in this context. I would suggest to rephrase the warning along the lines of -Wsign-conversion which prints: conversion to ‘long unsigned int’ from ‘int’ may change the sign of the result and add why it's a potential problem. Perhaps something like: argument to alloca may be too large due to conversion from 'int to 'long unsigned int' Fixed: Cool. FWIW, by coincidence I was just educated about the subtle nuances of quoting in GCC messages in a discussion with David and Manu. Types, functions, variables, and literals that appear in the source code should be referenced in diagnostics by using the "%qT", "%qD", and "%qE" directives so that GCC can add the right quotes and highlighting. Enclosing "'%T'" in quotes will not use the same kind of quotes as "%qT" and won't highlight the type name. https://gcc.gnu.org/wiki/DiagnosticsGuidelines There is a "documented" reason for this: :) // Do not warn on VLAs occurring in a loop, since VLAs are // guaranteed to be cleaned up when they go out of scope. // That is, there is a corresponding __builtin_stack_restore // at the end of the scope in which the VLA occurs. Yes, I understand that VLAs in loops are treated differently than alloca. But I don't think this is quite how the logic should work. I.e., an excessively large VLA should be diagnosed regardless of whether it's in a loop or outside. Consider the following case where with the patch as is, the warning is issued only for one of the two functions, even though they both allocate a VLA in excess of the threshold. #define FOO(n) if (1) { \ char a [n]; \ f (a); \ } else (void)0 #define BAR(n) do { \ char a [n]; \ f (a); \ } while (0) void f (void*); void foo (void) { int n = 8000; FOO (n);// warning with -Wla-larger-than=4000 } void bar (void) { int n = 8000; BAR (n);// no warning } 5) The -Wvla=N logic only seems to take into consideration the number of elements but not the size of the element type. For example, I wasn't able to get it to warn on the following with -Wvla=255 or greater: void f0 (void*); void f1 (unsigned char a) { int x [a]; // or even char a [n][__INT_MAX__]; f0 (x); } That was a huge oversight (or should I say over-engineering) on my part. Fixed. Looks good. I did notice one minor glitch, though not one caused by your patch. GCC apparently transforms simple VLAs that are 256 bytes or less in size into ordinary arrays (i.e., it doesn't call __builtin_alloca_with_align). Because of that, specifying -Wvla-larger-than=N with N less than 256 may not give a warning, as in the example below. I suspect there may not be anything the Walloca pass can do about this so perhaps just mentioning it in the manual might be enough to avoid bug reports about false negatives. void f0 (void*); unsigned f1 (void) { return 256; } void f2 (void) { unsigned n = f1 (); char a [n]; f0 (a); } GCC doesn't do the same transformation for alloca calls so the -Walloca-larger-than warning doesn't have this quirk. This is a problem with the generic machinery so I would prefer someone file a bugzilla report :), as this affects other options. I raised bug 71905 for this. Ughhh...you're making me write user friendly stuff. The reason I got into compilers was so I wouldn't have to deal with the user :). if (n < 2000) { p = __builtin_alloca (n); f (p); } ./cc1 a.c -fdump-tree-all-vops-alias-details-graph -quiet -I/tmp -Walloca-larger-than=100 -O2 a.c: In function ‘g2’: a.c:9:7: warning: argument to alloca may be too large [-Walloca-larger-than=] p = __builtin_alloca (n); ~~^~ a.c:9:7: note: limit is '100' bytes, but argument may be '1999' Happy? :-) I along with untold numbers of users
[PATCH] Avoid invoking ranlib on libbackend.a
This patch makes the 0.5GB libbackend.a get built via "ar rcs" instead of via "ar rc" + "ranlib", if possible. The problem with the latter is that it's about twice as slow as the former and it makes libbackend.a get written twice which is wasteful. On my machine this optimization reduces rebuild time in a --disable-bootstrap tree after touching tree.c by around 25%, from ~28s to ~21s. This optimization is only performed when the current ar is GNU ar where we can be reasonably sure that the "rcs" directive is supported. Does this change look reasonable? I am currently bootstrapping it on x86_64-pc-linux-gnu. gcc/ChangeLog: * configure.ac (ar_is_gnu): New variable. AC_SUBST it. * configure: Regenerate. * Makefile.in (AR_IS_GNU): New variable. (USE_AR_RCS): New variable. (libbackend.a): When building this archive, if USE_AR_RCS then just invoke "ar rcs" instead of invoking "ar rc" followed by "ranlib". --- gcc/Makefile.in | 18 ++ gcc/configure| 15 +-- gcc/configure.ac | 8 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 0786fa3..5ae6100 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -237,10 +237,22 @@ FLEX = @FLEX@ FLEXFLAGS = AR = @AR@ AR_FLAGS = rc +AR_IS_GNU = @ar_is_gnu@ NM = @NM@ RANLIB = @RANLIB@ RANLIB_FLAGS = @ranlib_flags@ +# Whether invocations of ar + ranlib can be replaced by a single +# invocation of "ar rcs". +USE_AR_RCS = no +ifeq ($(AR_IS_GNU),yes) +ifeq ($(AR_FLAGS),rc) +ifeq ($(RANLIB_FLAGS),) +USE_AR_RCS = yes +endif +endif +endif + # Libraries to use on the host. HOST_LIBS = @HOST_LIBS@ @@ -1882,8 +1894,14 @@ compilations: $(BACKEND) # This archive is strictly for the host. libbackend.a: $(OBJS) -rm -rf libbackend.a + @# Elide the invocation of ranlib if possible, as doing so + @# significantly reduces the build time of this archive. +ifeq ($(USE_AR_RCS),yes) + $(AR) $(AR_FLAGS)s libbackend.a $(OBJS) +else $(AR) $(AR_FLAGS) libbackend.a $(OBJS) -$(RANLIB) $(RANLIB_FLAGS) libbackend.a +endif libcommon-target.a: $(OBJS-libcommon-target) -rm -rf libcommon-target.a diff --git a/gcc/configure b/gcc/configure index ed44472..db761c8 100755 --- a/gcc/configure +++ b/gcc/configure @@ -749,6 +749,7 @@ CXXDEPMODE DEPDIR am__leading_dot doc_build_sys +ar_is_gnu AR NM BISON @@ -8459,6 +8460,16 @@ fi fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ar is GNU ar" >&5 +$as_echo_n "checking whether ar is GNU ar... " >&6; } +ar_is_gnu=no +if $AR --version 2>/dev/null | sed 1q | grep "GNU ar" > /dev/null; then + ar_is_gnu=yes +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ar_is_gnu" >&5 +$as_echo "$ar_is_gnu" >&6; } + + # The jit documentation looks better if built with sphinx, but can be # built with texinfo if sphinx is not available. # Set "doc_build_sys" to "sphinx" or "texinfo" accordingly. @@ -18475,7 +18486,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18478 "configure" +#line 18489 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -18581,7 +18592,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18584 "configure" +#line 18595 "configure" #include "confdefs.h" #if HAVE_DLFCN_H diff --git a/gcc/configure.ac b/gcc/configure.ac index 086d0fc..3749b21 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -1081,6 +1081,14 @@ else AC_CHECK_PROG(AR, ar, ar, ${CONFIG_SHELL-/bin/sh} ${srcdir}/../missing ar) fi +AC_MSG_CHECKING(whether ar is GNU ar) +ar_is_gnu=no +if $AR --version 2>/dev/null | sed 1q | grep "GNU ar" > /dev/null; then + ar_is_gnu=yes +fi +AC_MSG_RESULT($ar_is_gnu) +AC_SUBST(ar_is_gnu) + # The jit documentation looks better if built with sphinx, but can be # built with texinfo if sphinx is not available. # Set "doc_build_sys" to "sphinx" or "texinfo" accordingly. -- 2.9.2.321.g3e7e728
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On Fri, Jul 15, 2016 at 1:07 PM, Bin Chengwrote: > Hi, > This patch removes support for -funsafe-loop-optimizations, as well as > -Wunsafe-loop-optimizations. By its name, this option does unsafe > optimizations by assuming all loops must terminate and doesn't wrap. > Unfortunately, it's not as useful as expected because: > 1) Simply assuming loop must terminate isn't enough. What we really want is > to analyze scalar evolution and loop niter bound under such assumptions. > This option does nothing in this aspect. > 2) IIRC, this option generates bogus code for some common programs, that's > why it's disabled by default even at Ofast level. > > After I sent patches handling possible infinite loops in both (scev/niter) > analyzer and vectorizer, it's a natural step to remove such options in GCC. > This patch does so by deleting code for -funsafe-loop-optimizations, as well > as -Wunsafe-loop-optimizations. It also deletes the two now useless tests, > while the option interface is preserved for backward compatibility purpose. There are a number of bugs opened against those options, including one that I just opened rather recently: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71769 but some go back far, in this case 9 years: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34114 If you are going to remove the options, you should address open bugs related to those options.
Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
2016-07-13 20:32 GMT+02:00 Ville Voutilainen: > On 13 July 2016 at 21:25, Daniel Krügler wrote: >> How would you feel about the introduction of an internal trait >> __is_boolean_testable, that would test both is_convertible> bool> and is_constructible for now, so that we could >> reuse that at places like these and others pointed out by LWG 2114? >> >> If you like that idea, I would work on a contribution. > > Sounds like an excellent idea to me. I have opened an enhancement request and have now a (actually two) proposal(s) for this. I would appreciate if you could express your opinion here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71899#c1 Thanks, - Daniel
Re: [committed] Improve profile support on hppa
Oops, here is patch. Dave -- John David Anglin dave.ang...@bell.net 2016-07-16 John David Anglin* config/pa/pa.c (hppa_profile_hook): Allocate stack space for register parameters. Remove code to initialize argument pointer on TARGET_64BIT. Optimize call to _mcount when it can be reached using a pc-relative branch. Cleanup conditional code. * config/pa/pa.md (call_mcount): New expander. (call_mcount_nonpic): New insn. (call_mcount_pic): New insn and split. (call_mcount_pic_post_reload): New insn. (call_mcount_64bit): New insn and split. (call_mcount_64bit_post_reload): New insn. Index: config/pa/pa.c === --- config/pa/pa.c (revision 238404) +++ config/pa/pa.c (working copy) @@ -4532,64 +4532,79 @@ lcla2 and load_offset_label_address insn patterns. */ rtx reg = gen_reg_rtx (SImode); rtx_code_label *label_rtx = gen_label_rtx (); - rtx begin_label_rtx; + rtx mcount = gen_rtx_MEM (Pmode, gen_rtx_SYMBOL_REF (Pmode, "_mcount")); + int reg_parm_stack_space = REG_PARM_STACK_SPACE (NULL_TREE); + rtx arg_bytes, begin_label_rtx; rtx_insn *call_insn; char begin_label_name[16]; + bool use_mcount_pcrel_call; + /* If we can reach _mcount with a pc-relative call, we can optimize + loading the address of the current function. This requires linker + long branch stub support. */ + if (!TARGET_PORTABLE_RUNTIME + && !TARGET_LONG_CALLS + && (TARGET_SOM || flag_function_sections)) +use_mcount_pcrel_call = TRUE; + else +use_mcount_pcrel_call = FALSE; + ASM_GENERATE_INTERNAL_LABEL (begin_label_name, FUNC_BEGIN_PROLOG_LABEL, label_no); begin_label_rtx = gen_rtx_SYMBOL_REF (SImode, ggc_strdup (begin_label_name)); - if (TARGET_64BIT) -emit_move_insn (arg_pointer_rtx, - gen_rtx_PLUS (word_mode, virtual_outgoing_args_rtx, - GEN_INT (64))); - emit_move_insn (gen_rtx_REG (word_mode, 26), gen_rtx_REG (word_mode, 2)); - /* The address of the function is loaded into %r25 with an instruction- - relative sequence that avoids the use of relocations. The sequence - is split so that the load_offset_label_address instruction can - occupy the delay slot of the call to _mcount. */ - if (TARGET_PA_20) -emit_insn (gen_lcla2 (reg, label_rtx)); - else -emit_insn (gen_lcla1 (reg, label_rtx)); + if (!use_mcount_pcrel_call) +{ + /* The address of the function is loaded into %r25 with an instruction- +relative sequence that avoids the use of relocations. The sequence +is split so that the load_offset_label_address instruction can +occupy the delay slot of the call to _mcount. */ + if (TARGET_PA_20) + emit_insn (gen_lcla2 (reg, label_rtx)); + else + emit_insn (gen_lcla1 (reg, label_rtx)); - emit_insn (gen_load_offset_label_address (gen_rtx_REG (SImode, 25), - reg, begin_label_rtx, label_rtx)); + emit_insn (gen_load_offset_label_address (gen_rtx_REG (SImode, 25), + reg, + begin_label_rtx, + label_rtx)); +} -#if !NO_DEFERRED_PROFILE_COUNTERS - { -rtx count_label_rtx, addr, r24; -char count_label_name[16]; + if (!NO_DEFERRED_PROFILE_COUNTERS) +{ + rtx count_label_rtx, addr, r24; + char count_label_name[16]; -funcdef_nos.safe_push (label_no); -ASM_GENERATE_INTERNAL_LABEL (count_label_name, "LP", label_no); -count_label_rtx = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (count_label_name)); + funcdef_nos.safe_push (label_no); + ASM_GENERATE_INTERNAL_LABEL (count_label_name, "LP", label_no); + count_label_rtx = gen_rtx_SYMBOL_REF (Pmode, + ggc_strdup (count_label_name)); -addr = force_reg (Pmode, count_label_rtx); -r24 = gen_rtx_REG (Pmode, 24); -emit_move_insn (r24, addr); + addr = force_reg (Pmode, count_label_rtx); + r24 = gen_rtx_REG (Pmode, 24); + emit_move_insn (r24, addr); -call_insn = - emit_call_insn (gen_call (gen_rtx_MEM (Pmode, -gen_rtx_SYMBOL_REF (Pmode, -"_mcount")), - GEN_INT (TARGET_64BIT ? 24 : 12))); + arg_bytes = GEN_INT (TARGET_64BIT ? 24 : 12); + if (use_mcount_pcrel_call) + call_insn = emit_call_insn (gen_call_mcount (mcount, arg_bytes, +begin_label_rtx)); + else + call_insn = emit_call_insn (gen_call (mcount, arg_bytes)); -use_reg (_INSN_FUNCTION_USAGE (call_insn), r24); - }
[committed] Improve profile support on hppa
The attached patch implements a suggestion from Helge Deller to optimize profile support. When _mcount is reachable using a pc-relative branch, we can use the delay slot of the branch to compute the selfpc argument of _mcount. In working on this change, I noticed a couple of problems with the current implementation of hppa_profile_hook. Firstly, the argument point setup on TARGET_64BIT was being done twice - once in hppa_profile_hook and again in the call expander. Secondly, the call frame and 64-bit argument were setup incorrectly in leaf functions. Because of the call to _mcount, we need to reserve stack space for the fixed arguments. Guess there's a reason why most targets do a libcall. The attached change has been tested on hppa-unknown-linux-gnu, hppa64-hp-hpux11.11 and hppa2.0w-hp-hpux11.11. Committed to trunk. Dave -- John David Anglin dave.ang...@bell.net
Re: [PATCH] Add constexpr to iterator adaptors, array and range access
On 16 July 2016 at 18:38, Ville Voutilainenwrote: > I think it's reasonable to make the functions constexpr even when the standard > doesn't allow it, and if c++17 allows them to be constexpr, make the functions > constexpr in other standard modes as well. We could of course ask whether > LWG thought about applying the changes to previous standards or not. Let me elaborate a bit. I think it's perfectly sane to enable constexpr for c++11 and c++14 when c++17 enables it, and talk to other vendors and LEWG/LWG about syncing such changes. >> How strict do we want to be about obeying the "implementors can't add >> constexpr" rule in these cases? > > I'd rather not be strict. ..and here I would prefer accepting some amounts of non-compliance and feeding the results back to LEWG/LWG and other vendors. If/when users run into trouble, we get better constexpr support in the spec, and we gain better performance for cases where constexpr evaluation isn't strictly required. I have had some ruminations for an annotation of some kind that would allow the front-end to detect non-portable uses of constexpr but allow the performance increase for portable cases. I haven't had the time to do the actual work for the front-end to be able to tell the difference based on such an annotation.
Re: [PATCH] Add constexpr to iterator adaptors, array and range access
> This patch implements http://wg21.link/p0031 which adds constexpr to > most operations on std::reverse_iterator, std::move_iterator, > std::array, as well as std::advance, std::distance, and the > range-access functions std::begin, std::rbegin etc. > > Strictly speaking, those functions should only be constexpr in C++17 > and not before, but this patch makes them constexpr whenever possible. > That means the changes are fully implemented for C++14 (and the > feature-test macro is defined) but only partially implemented for > C++11, because some of the functions can't be constexpr in C++11. > > My thinking is that if the committee has decided that these functions > *should* be constexpr, and changed them for C++17, then it doesn't > serve users to make them non-constexpr in C++11 and C++14 just because > the standard says so. > > How do other people feel about that? I think it's reasonable to make the functions constexpr even when the standard doesn't allow it, and if c++17 allows them to be constexpr, make the functions constexpr in other standard modes as well. We could of course ask whether LWG thought about applying the changes to previous standards or not. > The alternative would be to define a new _GLIBCXX17_CONSTEXPR macro > and use it in all these places, so they're only constexpr in C++17 > (and probably for -std=gnu++14 too, but not -std=c++14). That's certainly "safer". > How strict do we want to be about obeying the "implementors can't add > constexpr" rule in these cases? I'd rather not be strict.
[patch, Fortran] Fix some string temporaries
Hello world, this fixes PR 71902. The recent fix for PR 71783 introduced a performance and code size regression a string temporary was created for the test case when it was not actually needed. I also took the opportunity of renaming the misnomed temporary variable. Regression-tested. OK for trunk? Do we actually want to backport this? Technically, it is a regression, but people are not likely to notice much. Regards Thomas 2016-07-16 Thomas KoenigPR fortran/71902 * dependency.c (gfc_check_dependency): Use dep_ref. Handle case if identical is true and two array element references differ. (gfc_dep_resovler): Move most of the code to dep_ref. (dep_ref): New function. * frontend-passes.c (realloc_string_callback): Name temporary variable "realloc_string". 2016-07-16 Thomas Koenig PR fortran/71902 * gfortran.dg/dependency_47.f90: New test. Index: dependency.c === --- dependency.c (Revision 238223) +++ dependency.c (Arbeitskopie) @@ -54,6 +54,8 @@ enum gfc_dependency static gfc_dependency check_section_vs_section (gfc_array_ref *, gfc_array_ref *, int); +static gfc_dependency dep_ref (gfc_ref *, gfc_ref *, gfc_reverse *); + /* Returns 1 if the expr is an integer constant value 1, 0 if it is not or def if the value could not be determined. */ @@ -1316,14 +1318,34 @@ gfc_check_dependency (gfc_expr *expr1, gfc_expr *e return 0; } - if (identical) - return 1; - /* Identical and disjoint ranges return 0, overlapping ranges return 1. */ if (expr1->ref && expr2->ref) - return gfc_dep_resolver (expr1->ref, expr2->ref, NULL); + { + gfc_dependency dep; + dep = dep_ref (expr1->ref, expr2->ref, NULL); + switch (dep) + { + case GFC_DEP_EQUAL: + return identical; + case GFC_DEP_FORWARD: + return 0; + + case GFC_DEP_BACKWARD: + return 1; + + case GFC_DEP_OVERLAP: + return 1; + + case GFC_DEP_NODEP: + return 0; + + case GFC_DEP_ERROR: + return 0; + } + } + return 1; case EXPR_FUNCTION: @@ -2052,11 +2074,44 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref 2 : array references are overlapping but reversal of one or more dimensions will clear the dependency. 1 : array references are overlapping. - 0 : array references are identical or not overlapping. */ + 0 : array references are identical or can be handled in a forward loop. */ int gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse) { + enum gfc_dependency dep; + dep = dep_ref (lref, rref, reverse); + switch (dep) +{ +case GFC_DEP_EQUAL: + return 0; + +case GFC_DEP_FORWARD: + return 0; + +case GFC_DEP_BACKWARD: + return 2; + +case GFC_DEP_OVERLAP: + return 1; + +case GFC_DEP_NODEP: + return 0; + +case GFC_DEP_ERROR: + return 0; + +default: + gcc_unreachable(); +} +} + +/* Compare two references, returning an enum gfc_dependency depending on the + "worst" type of dependency found. */ + +static gfc_dependency +dep_ref (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse) +{ int n; int m; gfc_dependency fin_dep; @@ -2079,21 +2134,21 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf /* The two ranges can't overlap if they are from different components. */ if (lref->u.c.component != rref->u.c.component) - return 0; + return GFC_DEP_NODEP; break; case REF_SUBSTRING: /* Substring overlaps are handled by the string assignment code if there is not an underlying dependency. */ - return (fin_dep == GFC_DEP_OVERLAP) ? 1 : 0; + return fin_dep; case REF_ARRAY: if (ref_same_as_full_array (lref, rref)) - return 0; + return GFC_DEP_EQUAL; if (ref_same_as_full_array (rref, lref)) - return 0; + return GFC_DEP_EQUAL; if (lref->u.ar.dimen != rref->u.ar.dimen) { @@ -2104,7 +2159,7 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf fin_dep = gfc_full_array_ref_p (lref, NULL) ? GFC_DEP_EQUAL : GFC_DEP_OVERLAP; else - return 1; + return GFC_DEP_OVERLAP; break; } @@ -2148,7 +2203,7 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf /* If any dimension doesn't overlap, we have no dependency. */ if (this_dep == GFC_DEP_NODEP) - return 0; + return GFC_DEP_NODEP; /* Now deal with the loop reversal logic: This only works on ranges and is activated by setting @@ -2215,7 +2270,7 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf /* Exactly matching and forward overlapping ranges don't cause a dependency. */ if (fin_dep < GFC_DEP_BACKWARD) - return 0; + return fin_dep; /* Keep checking. We only have a dependency if
Re: [PATCH] Add constexpr to iterator adaptors, array and range access
2016-07-16 1:09 GMT+02:00 Jonathan Wakely: > This patch implements http://wg21.link/p0031 which adds constexpr to > most operations on std::reverse_iterator, std::move_iterator, > std::array, as well as std::advance, std::distance, and the > range-access functions std::begin, std::rbegin etc. > > Strictly speaking, those functions should only be constexpr in C++17 > and not before, but this patch makes them constexpr whenever possible. > That means the changes are fully implemented for C++14 (and the > feature-test macro is defined) but only partially implemented for > C++11, because some of the functions can't be constexpr in C++11. > > My thinking is that if the committee has decided that these functions > *should* be constexpr, and changed them for C++17, then it doesn't > serve users to make them non-constexpr in C++11 and C++14 just because > the standard says so. > > How do other people feel about that? > > The alternative would be to define a new _GLIBCXX17_CONSTEXPR macro > and use it in all these places, so they're only constexpr in C++17 > (and probably for -std=gnu++14 too, but not -std=c++14). > > How strict do we want to be about obeying the "implementors can't add > constexpr" rule in these cases? As much as I hate the current restrictions, I tend to suggest to follow them strictly in __STRICT_ANSI__ mode. > Here's the patch, but I'm not committing it yet. Since I made a similar (unintentional) omission recently myself: Shouldn't you touch std::__debug::array as well? What about other functions from std::__debug? - Daniel
Re: Importing gnulib into the gcc tree
Hi, Thanks for the feedbacks. —> I’m already configuring gcc with multiple languages and multilib enabled —> The changes have been bootstrapped and regression tested (complete check, make -k -j20 check). —> As mentioned, I have locally removed obstack.[ch] from libiberty and built and tested the entire thing. PFA the patch ChangeLog: 15-7-16 Ayush Goel* Makefile.def: Added gnulib as build & host library and dependency of all-gcc on gnulib * Makefile.in: regenerated * configure.ac: Added gnulib as build and host library * configure: regenerated * gcc/Makefile.in: Added path to gnulib static library (libgnu.a) and gnulib header files * gnulib: created directory * gnulib/Makefile.in: new file * gnulib/configure.ac: new file * gnulib/update-gnulib.sh: script to import gnulib modules using gnulib-tool * gnulib/import: created by update-gnulib.sh * gnulib/import/Makefile.in: imported from gnulib * gnulib/import/alignof.h: Imported from gnulib * gnulib/import/exitfail.c: Imported from gnulib * gnulib/import/exitfail.h: Imported from gnulib * gnulib/import/extra: Imported from gnulib * gnulib/import/extra/snippet: Imported from gnulib * gnulib/import/extra/snippet/_Noreturn.h: Imported from gnulib * gnulib/import/extra/snippet/arg-nonnull.h: Imported from gnulib * gnulib/import/extra/snippet/c++defs.h: Imported from gnulib * gnulib/import/extra/snippet/warn-on-use.h: Imported from gnulib * gnulib/import/gettext.h: Imported from gnulib * gnulib/import/m4: Imported from gnulib * gnulib/import/m4/00gnulib.m4: Imported from gnulib * gnulib/import/m4/absolute-header.m4: Imported from gnulib * gnulib/import/m4/extern-inline.m4: Imported from gnulib * gnulib/import/m4/gnulib-cache.m4: Imported from gnulib * gnulib/import/m4/gnulib-common.m4: Imported from gnulib * gnulib/import/m4/gnulib-comp.m4: Imported from gnulib * gnulib/import/m4/gnulib-tool.m4: Imported from gnulib * gnulib/import/m4/include_next.m4: Imported from gnulib * gnulib/import/m4/longlong.m4: Imported from gnulib * gnulib/import/m4/multiarch.m4: Imported from gnulib * gnulib/import/m4/obstack.m4: Imported from gnulib * gnulib/import/m4/off_t.m4: Imported from gnulib * gnulib/import/m4/ssize_t.m4: Imported from gnulib * gnulib/import/m4/stddef_h.m4: Imported from gnulib * gnulib/import/m4/stdint.m4: Imported from gnulib * gnulib/import/m4/stdlib_h.m4: Imported from gnulib * gnulib/import/m4/sys_types_h.m4: Imported from gnulib * gnulib/import/m4/unistd_h.m4: Imported from gnulib * gnulib/import/m4/warn-on-use.m4: Imported from gnulib * gnulib/import/m4/wchar_t.m4: Imported from gnulib * gnulib/import/obstack.c: Imported from gnulib * gnulib/import/obstack.h: Imported from gnulib * gnulib/import/stddef.in.h: Imported from gnulib * gnulib/import/stdint.in.h: Imported from gnulib * gnulib/import/stdlib.in.h: Imported from gnulib * gnulib/import/sys: Imported from gnulib * gnulib/import/sys_types.in.h: Imported from gnulib * gnulib/import/unistd.c: Imported from gnulib * gnulib/import/unistd.in.h: Imported from gnulib * gnulib/stamp-h1: generated Also note that I have a copyright assignment in place already. -- Thanks, Ayush Goel On 10 July 2016 at 9:35:27 PM, Manuel López-Ibáñez (lopeziba...@gmail.com) wrote: > Hi Ayush, > > Some suggestions: > > * When resubmitting a patch, also add again the Changelog. > * Use '.diff' or '.patch' as an extension, so that mail readers can > open the file as text. > * Mention that you have a copyright assignment in place already (I'm > assuming you have one already, no?). > * Mention how you regression tested your changes (on which targets?) > * In the case of these changes, and as further testing, it would be > good if you tried locally to remove obstack.[ch] from libiberty to > make sure nothing in GCC is using it. I don't think we can actually > remove it because it may be used by other projects using libiberty, > however, as your first submission showed, it is not always evident > that everything in GCC is using the gnulib version. > > Cheers, > > Manuel. > > > On 8 July 2016 at 20:30, ayush goel wrote: > > Yes, that’s correct. It has been moved before the libiberty library in the > > list now. > Bootstrapped the system with the changes as well. > > > > PFA the updated patch > > > > -- > > Thanks, > > Ayush Goel > > > > On 8 July 2016 at 2:29:04 AM, Manuel López-Ibáñez (lopeziba...@gmail.com) > > wrote: > >> On 7 July 2016 at 13:48, ayush goel wrote: > >> > In order to show the setup works, I’ve replaced libiberty’s version by > >> > obstack by > gnulib’s. > >> This was made possible by replacing the corresponding header file and then > >> including > >> gnulib headers and gnulib static library in the build path required to > >> compile gcc > files. > >> > >> Hi Ayush, > >> > >> I'm not an expert on the build machinery, so this question might be > >>
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
* g++.dg/cpp0x/nullptr35.C (caught): Fix typo. diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr35.C b/gcc/testsuite/g++.dg/cpp0x/nullptr35.C index 2f93ce1..c84966f 100644 --- a/gcc/testsuite/g++.dg/cpp0x/nullptr35.C +++ b/gcc/testsuite/g++.dg/cpp0x/nullptr35.C @@ -11,7 +11,7 @@ int result = 0; void __attribute__((noinline)) caught(int bit) { - result &= bit; + result |= bit; } struct A { }; -- 2.9.1 -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH] c++/58796 Make nullptr match exception handlers of pointer type
On Fri, Jul 15, 2016 at 11:14:03PM +0100, Jonathan Wakely wrote: > On 15/07/16 22:53 +0100, Jonathan Wakely wrote: > >On 15/07/16 23:36 +0200, Jakub Jelinek wrote: > >>On Fri, Jul 15, 2016 at 10:07:03PM +0100, Jonathan Wakely wrote: > + if (typeid (*this) == typeid(__pointer_type_info)) > +{ > + *thr_obj = nullptr; > + return true; > >> > >>But you have the above store too. > > > >That doesn't write to the exception object, it only does a single > >dereference (compared to the double dereference of the racy write), so > >it writes to the local variable in the PERSONALITY_FUNCTION in > >eh_personality.cc > > > >So that shouldn't race with other threads. I think. > > > > TSan agrees. When I compile my test and yours (and include > libsupc++/pbase_type_info.cc in the executable, so the writes are also > instrumented by tsan) then I see races for the *(ptrdiff_t*)*thr_obj > writes but not the *thr_obj ones. > > It's only the ptr-to-data-member case that scribbles in the actual > exception object. In: struct A { int a; }; int a; int main () { try { throw nullptr; } catch (int * const ) { __builtin_printf ("%p %p\n", p, ); } try { throw nullptr; } catch (int A::* const ) { __builtin_printf ("%p\n", ); asm volatile ("" : : "r" ()); } try { throw } catch (int * const ) { __builtin_printf ("%p %p\n", p, ); } } I see being the address passed to __cxa_throw only in the second case, where does the reference point to in the other two cases? Some temporary in main? Does that mean if you rethrow the exception multiple times in different threads you get references to the same object for PMF and to different objects for pointers? Jakub