Re: [PATCH] Avoid invoking ranlib on libbackend.a

2016-07-16 Thread Patrick Palka
On Sat, Jul 16, 2016 at 6:35 PM, Andrew Pinski  wrote:
> 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

2016-07-16 Thread Martin Sebor

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

2016-07-16 Thread Andrew Pinski
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 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

2016-07-16 Thread Patrick Palka
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

2016-07-16 Thread Martin Sebor

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

2016-07-16 Thread Patrick Palka
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

2016-07-16 Thread NightStrike
On Fri, Jul 15, 2016 at 1:07 PM, Bin Cheng  wrote:
> 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-16 Thread Daniel Krügler
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

2016-07-16 Thread John David Anglin
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

2016-07-16 Thread John David Anglin
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

2016-07-16 Thread Ville Voutilainen
On 16 July 2016 at 18:38, Ville Voutilainen  wrote:
> 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

2016-07-16 Thread Ville Voutilainen
> 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

2016-07-16 Thread Thomas Koenig

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 Koenig  

PR 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 Thread Daniel Krügler
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

2016-07-16 Thread ayush goel
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

2016-07-16 Thread Andreas Schwab
* 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

2016-07-16 Thread Jakub Jelinek
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