Re: [patch, bz #58312] Fix libssp handling of vsnprintf for cross-compilers

[Jakub Jelinek]

On Tue, Sep 03, 2013 at 06:59:30PM -0700, Brooks Moses wrote:
> The attached patch fixes bug 58312, by replacing the runtime check of
> vsnprintf with a compile-time check -- which means that it now
> performs the same checks for both native and cross compilers, ensuring
> consistency instead of the previous behavior of just discarding some
> libssp functions when cross-compiling.
> 
> I've tested this by reconfiguring (on an x86-linux cross
> configuration) and confirming that it behaves as expected -- the
> HAVE_USABLE_VSNPRINTF macro is defined in confdefs.h, and the
> config.log has verbosity of output similar to the previous version.
> (The formerly-explicit output is included as part of the AC_CHECK_FUNC
> macro.)
> 
> It is possible that this will enable the macro on some native
> configurations where vsnprintf was present but is broken.  The effect
> is simply that a few additional functions (which rely on vsnprintf)
> are compiled into libssp; this seems relatively innocuous.
> 
> Jakub, I'm cc'ing you in hopes that you're a reasonably appropriate
> person to review this.

That looks wrong, the test was intentionally looking for badly implemented
vsnprintf, see
http://www.gnu.org/software/gnulib/manual/html_node/snprintf.html
"This function does not return a byte count as specified in C99 on some
platforms: HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 9, mingw."

The implementation relies on the returned byte count to be exactly correct,
so it can't be implemented on platforms where that is not the case.

Not sure which of the targets from the above list we still support,
certainly at least mingw, though in that case I don't know if it hasn't been
fixed there.  So, as Joseph said, you probably should keep the runtime test
as is, and just for cross compiling replace the unconditional =undef with
an optimistic assumption followed by a blacklist of platforms where it is
known not to work.

> 2013-09-03  Brooks Moses  
> 
> * configure.ac: Replace runtime vsnprintf check with
>   compile-time check.
> * configure: Regenerate.



Jakub

Re: [PATCH] Enable non-complex math builtins from C99 for Bionic

[Alexander Ivchenko]

Hi Maxim,

2013/9/4 Maxim Kuvyrkov :
> On 23/08/2013, at 1:04 AM, Alexander Ivchenko wrote:
>
>> Ugh.. thanks, you are right. That points to another problem that I
>> didn't see before:
>>
>> 3) *linux* targets that do not append to tm_p_file (s390x-*-linux* and
>> s390x-ibm-tpf* - your patch addresses that problem correctly) OR
>> tmake_file (bfin*-linux-uclibc* or crisv32-*-linux* | cris-*-linux*)
>
> Could you be more verbose, please?  What some of the *linux* target do not 
> append to tm_p_file?

*linux* targets that do not append to tm_p_file are 390x-*-linux* and
s390x-ibm-tpf* and Andrew is already addressed this problem.


> It seems that there are at least two separate problems:
>
> 1. OPTION_BIONIC is not defined in linux-android.c .  I think the right fix 
> here is to copy definitions of OPTION_BIONIC (and, optionally, OPTION_UCLIBC) 
> from gcc/config/linux.h to rs6000/linux.h, rs6000/linux64.h and alpha/linux.h 
> -- in other words, define OPTION_BIONIC and OPTION_UCLIBC whenever 
> OPTION_GLIBC is defined.

Why can't we just use the checks like "if (linux_libc == LIBC_GLIBC)".
Seems that we have all infrastructure in place (linux_libc,
LIBC_GLIBC, LIBC_BIONIC, LIBC_UCLIBC are defined for all *linux*
targets). I don't see the reason to make new defines.

> 2. The second problem is to do with definitions of TARGET_LIBC_HAS_FUNCTION 
> for bfin, c6x, lm32, m68k and moxie.  What is the failure scenario here?

"For them we have ar: linux-android.o: No such file or directory" So I
added t-linux-android rules to tmake_file.

(and for bfin additionally we have:
"../../gcc/gcc/config/bfin/bfin.c:5812:29: error:
‘linux_android_libc_has_function’ was not declared in this scope".
because
the tm_p.h is not included in bfin.c)

>
>> diff --git a/gcc/config.gcc b/gcc/config.gcc
>> index 7e1d529..89cf30a 100644
>> --- a/gcc/config.gcc
>> +++ b/gcc/config.gcc
>> @@ -1018,7 +1018,7 @@ bfin*-uclinux*)
>>  ;;
>> bfin*-linux-uclibc*)
>>  tm_file="${tm_file} dbxelf.h elfos.h bfin/elf.h gnu-user.h linux.h
>> glibc-stdint.h bfin/linux.h ./linux-sysroot-suffix.h"
>> - tmake_file="bfin/t-bfin-linux t-slibgcc"
>> + tmake_file="bfin/t-bfin-linux t-slibgcc t-linux-android"
>>  use_collect2=no
>>  ;;
>> bfin*-rtems*)
>
> Why?  Bfin has nothing to do with android.

Since ‘linux_android_libc_has_function’ handles all three libc options
for all linux targets, that's natural to use this function. May be it
should be called "linux_libc_has_function" and be located in linux.c
(we don't have this file..).

>> @@ -1053,7 +1053,7 @@ cris-*-elf | cris-*-none)
>> crisv32-*-linux* | cris-*-linux*)
>>  tm_file="dbxelf.h elfos.h ${tm_file} gnu-user.h linux.h
>> glibc-stdint.h cris/linux.h"
>>  # We need to avoid using t-linux, so override default tmake_file
>> - tmake_file="cris/t-cris cris/t-linux t-slibgcc"
>> + tmake_file="cris/t-cris cris/t-linux t-slibgcc t-linux-android"
>>  extra_options="${extra_options} cris/linux.opt"
>>  case $target in
>>   cris-*-*)
>
> Same question here.

Please, see above.

>> diff --git a/gcc/config/bfin/bfin.c b/gcc/config/bfin/bfin.c
>> index 7fab975..18457f8 100644
>> --- a/gcc/config/bfin/bfin.c
>> +++ b/gcc/config/bfin/bfin.c
>> @@ -46,6 +46,7 @@
>> #include "cgraph.h"
>> #include "langhooks.h"
>> #include "bfin-protos.h"
>> +#include "tm_p.h"
>> #include "tm-preds.h"
>> #include "tm-constrs.h"
>> #include "gt-bfin.h"
>> diff --git a/gcc/config/bfin/uclinux.h b/gcc/config/bfin/uclinux.h
>> index ca0f4ee..63cba99 100644
>> --- a/gcc/config/bfin/uclinux.h
>> +++ b/gcc/config/bfin/uclinux.h
>> @@ -44,3 +44,6 @@ see the files COPYING3 and COPYING.RUNTIME
>> respectively.  If not, see
>> #define TARGET_SUPPORTS_SYNC_CALLS 1
>>
>> #define SUBTARGET_FDPIC_NOT_SUPPORTED
>> +
>> +#undef TARGET_LIBC_HAS_FUNCTION
>> +#define TARGET_LIBC_HAS_FUNCTION no_c99_libc_has_function
>> diff --git a/gcc/config/c6x/uclinux-elf.h b/gcc/config/c6x/uclinux-elf.h
>> index 5d61f4d..fa0937e 100644
>> --- a/gcc/config/c6x/uclinux-elf.h
>> +++ b/gcc/config/c6x/uclinux-elf.h
>> @@ -62,3 +62,5 @@
>> : "0" (_beg), "b" (_end), "b" (_scno)); \
>> }
>>
>> +#undef TARGET_LIBC_HAS_FUNCTION
>> +#define TARGET_LIBC_HAS_FUNCTION no_c99_libc_has_function
>> diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c
>> index 4a4b48d..e9d9e9a 100644
>> --- a/gcc/config/linux-android.c
>> +++ b/gcc/config/linux-android.c
>> @@ -35,9 +35,9 @@ linux_android_has_ifunc_p (void)
>> bool
>> linux_android_libc_has_function (enum function_class fn_class)
>> {
>> -  if (OPTION_GLIBC)
>> +  if (linux_libc == LIBC_GLIBC)
>> return true;
>> -  if (OPTION_BIONIC)
>> +  if (linux_libc == LIBC_BIONIC)
>> if (fn_class == function_c94
>>  || fn_class == function_c99_misc
>>  || fn_class == function_sincos)
>
> The above hunk should not be necessary after (1).
>
>> diff --git a/gcc/config/lm32/uclinux-elf.h b/gcc/config/lm32/uclinux-elf.h
>> index 3a556d7..a5e8163 100644
>> --- a/gcc/config/lm32/uclinux-elf.h
>> +++ b/gcc/config/lm32/u

Re: [bootstrap] Fix build for several targets (including pr58242)

[Alexander Ivchenko]

Hi Jakub,

thanks for your review. I also answered Maxim in the initial thread.
I agree we your points about not enforcing Android stuff into all
*linux* targets, but for e.g

tm_file="$tm_file linux-android.h"

.. this enforcing been there before my patch and therefore it would be
better to address this in the follow-up patch (I can work on that).

and btw we define all supported by linux libcs (LIBC_GLIBC,
LIBC_BIONIC, LIBC_UCLIBC) in linux.opts and we also add it to all
*linux* targets

thanks,
Alexander

2013/9/3 Jakub Jelinek :
> On Tue, Sep 03, 2013 at 09:25:31AM +0400, Alexander Ivchenko wrote:
>> Several builds are broken after r201838.
>
> What targets actually support bionic?  If it is just arm, i?86/x86_64
> and perhaps aarch64 and nothing else, I'd like to question the way where
> you enforce all the
>   # Add Android userspace support to Linux targets.
>   case $target in
> *linux*)
>   tm_p_file="${tm_p_file} linux-protos.h"
>   tmake_file="${tmake_file} t-linux-android"
>   tm_file="$tm_file linux-android.h"
>   extra_options="$extra_options linux-android.opt"
>   extra_objs="$extra_objs linux-android.o"
>   ;;
>   esac
> stuff onto all Linux targets, without actually testing all of them.
> Wouldn't it be much better not to support Android in any way
> in config/linux.h (say at most #define TARGET_HAS_BIONIC 0 there,
> #ifdef HAVE_GNU_INDIRECT_FUNCTION
> #define TARGET_LIBC_HAS_FUNCTION hook_bool*true
> #endif
> etc.), and append the android stuff only to targets which you
> support, not before the target specific snippets in config.gcc,
> but after those (so linux-android.h, etc. come last, not first)?
>
> Jakub

Re: [bootstrap] Fix build for several targets (including pr58242)

[Maxim Kuvyrkov]

On 3/09/2013, at 5:53 PM, Jakub Jelinek wrote:

> On Tue, Sep 03, 2013 at 09:25:31AM +0400, Alexander Ivchenko wrote:
>> Several builds are broken after r201838.
> 
> What targets actually support bionic?  If it is just arm, i?86/x86_64
> and perhaps aarch64 and nothing else, I'd like to question the way where
> you enforce all the
>  # Add Android userspace support to Linux targets.
>  case $target in
>*linux*)
>  tm_p_file="${tm_p_file} linux-protos.h"
>  tmake_file="${tmake_file} t-linux-android"
>  tm_file="$tm_file linux-android.h"
>  extra_options="$extra_options linux-android.opt"
>  extra_objs="$extra_objs linux-android.o"
>  ;;
>  esac
> stuff onto all Linux targets, without actually testing all of them.
> Wouldn't it be much better not to support Android in any way
> in config/linux.h (say at most #define TARGET_HAS_BIONIC 0 there,
> #ifdef HAVE_GNU_INDIRECT_FUNCTION
> #define TARGET_LIBC_HAS_FUNCTION hook_bool*true
> #endif
> etc.), and append the android stuff only to targets which you
> support, not before the target specific snippets in config.gcc,
> but after those (so linux-android.h, etc. come last, not first)?

Jakub,

Nominal handling of Bionic by all *linux* targets is my fault, and it seemed 
like a good thing at the time.  With several iterations of improvements laid on 
top of this initial decision it now becoming a mess.  I'm thinking along 
similar lines as you to restrict Bionic/Android handling only to targets that 
actually support it.

As Alexander said in his email, this rework should be done as a separate patch.

Thanks,

--
Maxim Kuvyrkov
www.kugelworks.com

Re: [PATCH] Enable non-complex math builtins from C99 for Bionic

[Maxim Kuvyrkov]

On 4/09/2013, at 7:43 PM, Alexander Ivchenko wrote:

> Hi Maxim,
> 
> 2013/9/4 Maxim Kuvyrkov :
>> On 23/08/2013, at 1:04 AM, Alexander Ivchenko wrote:
>> 
>>> Ugh.. thanks, you are right. That points to another problem that I
>>> didn't see before:
>>> 
>>> 3) *linux* targets that do not append to tm_p_file (s390x-*-linux* and
>>> s390x-ibm-tpf* - your patch addresses that problem correctly) OR
>>> tmake_file (bfin*-linux-uclibc* or crisv32-*-linux* | cris-*-linux*)
>> 
>> Could you be more verbose, please?  What some of the *linux* target do not 
>> append to tm_p_file?
> 
> *linux* targets that do not append to tm_p_file are 390x-*-linux* and
> s390x-ibm-tpf* and Andrew is already addressed this problem.

OK.

> 
> 
>> It seems that there are at least two separate problems:
>> 
>> 1. OPTION_BIONIC is not defined in linux-android.c .  I think the right fix 
>> here is to copy definitions of OPTION_BIONIC (and, optionally, 
>> OPTION_UCLIBC) from gcc/config/linux.h to rs6000/linux.h, rs6000/linux64.h 
>> and alpha/linux.h -- in other words, define OPTION_BIONIC and OPTION_UCLIBC 
>> whenever OPTION_GLIBC is defined.
> 
> Why can't we just use the checks like "if (linux_libc == LIBC_GLIBC)".
> Seems that we have all infrastructure in place (linux_libc,
> LIBC_GLIBC, LIBC_BIONIC, LIBC_UCLIBC are defined for all *linux*
> targets). I don't see the reason to make new defines.

Same reason why the existing defines are in place.  OPTION_X is not always the 
same as (linux_libc == LIBC_X).

> 
>> 2. The second problem is to do with definitions of TARGET_LIBC_HAS_FUNCTION 
>> for bfin, c6x, lm32, m68k and moxie.  What is the failure scenario here?
> 
> "For them we have ar: linux-android.o: No such file or directory" So I
> added t-linux-android rules to tmake_file.
> 
> (and for bfin additionally we have:
> "../../gcc/gcc/config/bfin/bfin.c:5812:29: error:
> ‘linux_android_libc_has_function’ was not declared in this scope".
> because
> the tm_p.h is not included in bfin.c)

OK.

> 
>> 
>>> diff --git a/gcc/config.gcc b/gcc/config.gcc
>>> index 7e1d529..89cf30a 100644
>>> --- a/gcc/config.gcc
>>> +++ b/gcc/config.gcc
>>> @@ -1018,7 +1018,7 @@ bfin*-uclinux*)
>>> ;;
>>> bfin*-linux-uclibc*)
>>> tm_file="${tm_file} dbxelf.h elfos.h bfin/elf.h gnu-user.h linux.h
>>> glibc-stdint.h bfin/linux.h ./linux-sysroot-suffix.h"
>>> - tmake_file="bfin/t-bfin-linux t-slibgcc"
>>> + tmake_file="bfin/t-bfin-linux t-slibgcc t-linux-android"
>>> use_collect2=no
>>> ;;
>>> bfin*-rtems*)
>> 
>> Why?  Bfin has nothing to do with android.
> 
> Since ‘linux_android_libc_has_function’ handles all three libc options
> for all linux targets, that's natural to use this function. May be it
> should be called "linux_libc_has_function" and be located in linux.c
> (we don't have this file..).

OK.

Ideally we would have linux.c with linux_libc_has_function that knows nothing 
about Bionic and linux-android.c with linux_android_libc_has_function.

The patch is OK with definitions of OPTION_GLIBC, OPTION_UCLIBC and 
OPTION_BIONIC copied verbatim from gcc/config/linux.h to rs6000 and alpha 
versions.

It is then on my plate to refactor handling of Bionic libc and remove it from 
targets that don't support it.

Thanks,

--
Maxim Kuvyrkov
www.kugelworks.com

RE: [patch 4/4] -fstrict-volatile-bitfields cleanup v3: remove from defaults on all targets

[Bernd Edlinger]

On Tue, 3 Sep 2013 21:20:04, Joseph S. Myers wrote:
> On Tue, 3 Sep 2013, Bernd Edlinger wrote:
>
 The trouble is that AAPCS semantics are incompatible with the default GNU
 semantics for non-packed structures as well - AAPCS
 strict-volatile-bitfields is only compatible with --param
 allow-store-data-races=1, which is not the default for any language
 variant accepted by GCC (and I say that the default language semantics
 here should not depend on the target architecture).
>>>
>>> As I said it should be easy to fulfil AAPCS requirements if they do not 
>>> violate
>>> language constrains during code generation and thus warn about accesses
>>> that are emitted in a way not conforming to AAPCS (a warning at struct
>>> declaration time would be nicer, but I guess requires more coding and 
>>> thought,
>>> though at the point we compute DECL_BIT_FIELD_REPRESENTATIVE in
>>> stor-layout.c would be a suitable place).
>>>
>>
>> Just to make that clear, Sandra's patch tries to follow the AAPCS 
>> requirements,
>> _even if_ they violate the language constraints.
>>
>> Thus -fstrict-volatile-bitfields implies --param allow-store-data-races=1.
>
> And my concern is specifically about the defaults - the default for ARM
> should be the same C/C++ language as on other targets - rather than what
> happens if someone specifies -fstrict-volatile-bitfields explicitly.
>

I fully agree with you, the current default state of -fstrict-volatile-bitfields
should be disabled for all targets. That will be changed in part 4 of Sandra's 
patch.

For portability of application code, the default should be always off, unless
specifically requested.

Even driver code rarely uses bit-fields for register access, because that is
inherently non-portabe. Reason: the bit positions are completely different
on little- and big-endian targets. Most drivers I have seen use some macros
and explicit bit operations for register accesses.


> --
> Joseph S. Myers
> jos...@codesourcery.com 

Re: [bootstrap] Fix build for several targets (including pr58242)

[Jakub Jelinek]

On Wed, Sep 04, 2013 at 08:02:13PM +1200, Maxim Kuvyrkov wrote:
> Nominal handling of Bionic by all *linux* targets is my fault, and it
> seemed like a good thing at the time.  With several iterations of
> improvements laid on top of this initial decision it now becoming a mess. 
> I'm thinking along similar lines as you to restrict Bionic/Android
> handling only to targets that actually support it.
> 
> As Alexander said in his email, this rework should be done as a separate
> patch.

Yeah, doing it in a separate patch is just fine.  AFAIK richi has already
approved the pending fix.

Jakub

Re: [PATCH] Convert more passes to new dump framework

[Richard Biener]

On Tue, Sep 3, 2013 at 9:39 PM, Teresa Johnson  wrote:
> On Fri, Aug 30, 2013 at 11:28 PM, Sharad Singhai  wrote:
>>> Found the issue. The stream was incorrectly being closed when it was
>>> stderr/stdout. So only the dump output before the first dump_finish
>>> call was being emitted to stderr. I fixed this the same way the
>>> alt_dump_file was being handled just below - don't close if it is
>>> stderr/stdout. Confirmed that this fixes the problem.
>>>
>>> (So the real ratio between the volume of -fdump-...=stderr and
>>> -fopt-info is much higher than what I reported in an earlier email)
>>>
>>> Is the following patch ok, pending regression tests?
>>>
>>> 2013-08-30  Teresa Johnson  
>>>
>>> * dumpfile.c (dump_finish): Don't close stderr/stdout.
>>>
>>> Index: dumpfile.c
>>> ===
>>> --- dumpfile.c  (revision 202059)
>>> +++ dumpfile.c  (working copy)
>>> @@ -450,7 +450,8 @@ dump_finish (int phase)
>>>if (phase < 0)
>>>  return;
>>>dfi = get_dump_file_info (phase);
>>> -  if (dfi->pstream)
>>> +  if (dfi->pstream && strcmp("stderr", dfi->pfilename) != 0
>>> +  && strcmp("stdout", dfi->pfilename) != 0)
>>>  fclose (dfi->pstream);
>>>
>>>if (dfi->alt_stream && strcmp("stderr", dfi->alt_filename) != 0
>>
>> Yes, this is clearly a bug which I missed. Thanks for fixing it. Is it
>> feasible to add a test case for it?
>>
>> Thanks,
>> Sharad
>
>
> Good idea. I modified an existing test to dump to stderr instead of a
> dump file. Since it has 2 functions with messages from each, I
> confirmed that it exposed the bug.
>
> Here is the full patch. Bootstrapped and tested on
> x86_64-unknown-linux-gnu. Ok for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> Teresa
>
> 2013-09-03  Teresa Johnson  
>
> * dumpfile.c (dump_finish): Don't close stderr/stdout.
>
> * testsuite/gcc.dg/unroll_1.c: Test dumping to stderr.
>
> Index: dumpfile.c
> ===
> --- dumpfile.c  (revision 202121)
> +++ dumpfile.c  (working copy)
> @@ -450,7 +450,9 @@ dump_finish (int phase)
>if (phase < 0)
>  return;
>dfi = get_dump_file_info (phase);
> -  if (dfi->pstream)
> +  if (dfi->pstream && (!dfi->pfilename
> +   || (strcmp("stderr", dfi->pfilename) != 0
> +   && strcmp("stdout", dfi->pfilename) != 0)))
>  fclose (dfi->pstream);
>
>if (dfi->alt_stream && strcmp("stderr", dfi->alt_filename) != 0
> Index: testsuite/gcc.dg/unroll_1.c
> ===
> --- testsuite/gcc.dg/unroll_1.c (revision 202121)
> +++ testsuite/gcc.dg/unroll_1.c (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops
> -fdisable-tree-cunroll -fdisable-tree-cunrolli
> -fenable-rtl-loop2_unroll" } */
> +/* { dg-options "-O2 -fdump-rtl-loop2_unroll=stderr -fno-peel-loops
> -fdisable-tree-cunroll -fdisable-tree-cunrolli
> -fenable-rtl-loop2_unroll" } */
>
>  unsigned a[100], b[100];
>  inline void bar()
> @@ -11,7 +11,7 @@ int foo(void)
>  {
>int i;
>bar();
> -  for (i = 0; i < 2; i++)
> +  for (i = 0; i < 2; i++) /* { dg-message "note: loop turned into
> non-loop; it never loops" } */
>{
>   a[i]= b[i] + 1;
>}
> @@ -21,12 +21,10 @@ int foo(void)
>  int foo2(void)
>  {
>int i;
> -  for (i = 0; i < 2; i++)
> +  for (i = 0; i < 2; i++) /* { dg-message "note: loop turned into
> non-loop; it never loops" } */
>{
>   a[i]= b[i] + 1;
>}
>return 1;
>  }
> -
> -/* { dg-final { scan-rtl-dump-times "loop turned into non-loop; it
> never loops" 2 "loop2_unroll" } } */
> -/* { dg-final { cleanup-rtl-dump "loop2_unroll" } } */
> +/* { dg-prune-output ".*" } */
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: [PATCH] manage dom-walk_data initialization and finalization with constructors and destructors

[Richard Biener]

On Wed, Sep 4, 2013 at 5:16 AM,   wrote:
> From: Trevor Saunders 
>
> bootstrapped on x86_64-unknown-linux-gnu with same test results as unpatched 
> r202185 ok?

That looks like a not too useful part-C++-ification of domwalk.  A proper
C++-ification would include the use of functors and possibly templating it
on the user specific data type.

So, what do you think is the advantage (other than dropping two or three lines
of code in the users)?

Richard.

>
> * compare-elim.c (find_comparisons_in_bb): adjust
> * domwalk.c (init_walk_dominator_tree): Convert to dom_walk_data constructor.
>   (fini_walk_dominator_tree): Convert to dom_walk_data destructor.
> * domwalk.h (dom_walk_data::dom_walk_data): declare
>   (dom_walk_data::~dom_walk_data): declare
>   (init_walk_dominator_tree): remove
>   (fini_walk_dominator_tree): remove
> * fwprop.c (build_single_def_use_links): adjust
> * gimple-ssa-strength-reduction.c (execute_strength_reduction): adjust
> * graphite-sese-to-poly.c (build_sese_conditions_after): adjust
> * tree-into-ssa.c (rewrite_blocks): adjust
>   (mark_def_site_blocks): adjust
> * tree-ssa-dom.c (tree_ssa_dominator_optimize): adjust
> * tree-ssa-dse.c (tree_ssa_dse): adjust
> * tree-ssa-loop-im.c (determine_invariantness): adjust
>   (move_computations): adjust
> * tree-ssa-phiopt.c (get_non_trapping): adjust
> * tree-ssa-pre.c (eliminate): adjust
> * tree-ssa-strlen.c (tree_ssa_strlen): adjust
> * tree-ssa-uncprop.c (tree_ssa_uncprop): adjust
>
> diff --git a/gcc/compare-elim.c b/gcc/compare-elim.c
> index e907376..8a1e68f 100644
> --- a/gcc/compare-elim.c
> +++ b/gcc/compare-elim.c
> @@ -403,17 +403,13 @@ find_comparisons_in_bb (struct dom_walk_data *data 
> ATTRIBUTE_UNUSED,
>  static void
>  find_comparisons (void)
>  {
> -  struct dom_walk_data data;
> +  struct dom_walk_data data (CDI_DOMINATORS);
>
> -  memset (&data, 0, sizeof(data));
> -  data.dom_direction = CDI_DOMINATORS;
>data.before_dom_children = find_comparisons_in_bb;
>
>calculate_dominance_info (CDI_DOMINATORS);
>
> -  init_walk_dominator_tree (&data);
>walk_dominator_tree (&data, ENTRY_BLOCK_PTR);
> -  fini_walk_dominator_tree (&data);
>
>clear_aux_for_blocks ();
>free_dominance_info (CDI_DOMINATORS);
> diff --git a/gcc/domwalk.c b/gcc/domwalk.c
> index 8c1ddc6..bc7a93f 100644
> --- a/gcc/domwalk.c
> +++ b/gcc/domwalk.c
> @@ -261,22 +261,26 @@ walk_dominator_tree (struct dom_walk_data *walk_data, 
> basic_block bb)
>free (worklist);
>  }
>
> -void
> -init_walk_dominator_tree (struct dom_walk_data *walk_data)
> +dom_walk_data::dom_walk_data (cdi_direction direction)
> +  : dom_direction(direction),
> +  initialize_block_local_data (NULL),
> +before_dom_children (NULL),
> +  after_dom_children (NULL),
> +  global_data (NULL),
> +block_local_data_size (0)
>  {
> -  walk_data->free_block_data.create (0);
> -  walk_data->block_data_stack.create (0);
> +  free_block_data.create (0);
> +  block_data_stack.create (0);
>  }
>
> -void
> -fini_walk_dominator_tree (struct dom_walk_data *walk_data)
> +dom_walk_data::~dom_walk_data ()
>  {
> -  if (walk_data->initialize_block_local_data)
> +  if (initialize_block_local_data)
>  {
> -  while (walk_data->free_block_data.length () > 0)
> -   free (walk_data->free_block_data.pop ());
> +  while (free_block_data.length () > 0)
> +   free (free_block_data.pop ());
>  }
>
> -  walk_data->free_block_data.release ();
> -  walk_data->block_data_stack.release ();
> +  free_block_data.release ();
> +  block_data_stack.release ();
>  }
> diff --git a/gcc/domwalk.h b/gcc/domwalk.h
> index 54b7f3c..94986c7 100644
> --- a/gcc/domwalk.h
> +++ b/gcc/domwalk.h
> @@ -26,6 +26,10 @@ typedef void *void_p;
>
>  struct dom_walk_data
>  {
> +public:
> +  dom_walk_data (cdi_direction);
> +  ~dom_walk_data ();
> +
>/* This is the direction of the dominator tree we want to walk.  i.e.,
>   if it is set to CDI_DOMINATORS, then we walk the dominator tree,
>   if it is set to CDI_POST_DOMINATORS, then we walk the post
> @@ -70,5 +74,3 @@ struct dom_walk_data
>  };
>
>  void walk_dominator_tree (struct dom_walk_data *, basic_block);
> -void init_walk_dominator_tree (struct dom_walk_data *);
> -void fini_walk_dominator_tree (struct dom_walk_data *);
> diff --git a/gcc/fwprop.c b/gcc/fwprop.c
> index 8fe02ac..e10754b 100644
> --- a/gcc/fwprop.c
> +++ b/gcc/fwprop.c
> @@ -269,8 +269,6 @@ single_def_use_leave_block (struct dom_walk_data 
> *walk_data ATTRIBUTE_UNUSED,
>  static void
>  build_single_def_use_links (void)
>  {
> -  struct dom_walk_data walk_data;
> -
>/* We use the multiple definitions problem to compute our restricted
>   use-def chains.  */
>df_set_flags (DF_EQ_NOTES);
> @@ -291,14 +289,12 @@ build_single_def_use_links (void)
>
>/* Walk the dominator tree looking for single reaching definitions
>   dominating the uses.  This is similar to how SSA form is built.  */
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_w

[PATCH] More comments about DECL_BUILT_IN and DECL_IS_BUILTIN (was Re: Why DECL_BUILT_IN and DECL_IS_BUILTIN?)

[Dodji Seketeli]

Hello,

Richard Biener  a écrit:

[...]

> DECL_IS_BUILTIN is true if the decl was created by the frontend / backend
> rather than by user code (indicated by source location).  DECL_BUILT_IN
> is true if the decl represents a function of the standard library, a
> builtin that is
> recognized by optimization / expansion.  User declared prototypes of
> C library functions are not DECL_IS_BUILTIN but may be DECL_BUILT_IN.

Every time I see these macros I have a hard time telling which is which
:-) So I felt I'd stand on your shoulders (if you don't mind) and add
these comments right in tree.h so that they are easier to find next
time.

OK to commit this comment-only patchlet to trunk then?

Thanks.

From 1ad29143764a72d27b1ecf3c06b4ba72bfaf4fe8 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli 
Date: Wed, 4 Sep 2013 10:32:36 +0200
Subject: [PATCH] More comments about DECL_BUILT_IN and DECL_IS_BUILTIN

gcc/ChangeLog

* tree.h (DECL_BUILT_IN, DECL_IS_BUILTIN): Add more comments
explaining their differences.
---
 gcc/tree.h | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gcc/tree.h b/gcc/tree.h
index 718d8f4..88d527a 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1838,6 +1838,9 @@ extern enum machine_mode vector_type_mode (const_tree);
 #define DECL_SOURCE_FILE(NODE) LOCATION_FILE (DECL_SOURCE_LOCATION (NODE))
 #define DECL_SOURCE_LINE(NODE) LOCATION_LINE (DECL_SOURCE_LOCATION (NODE))
 #define DECL_SOURCE_COLUMN(NODE) LOCATION_COLUMN (DECL_SOURCE_LOCATION (NODE))
+/* This accessor returns TRUE if the decl it operates on was created
+   by a front-end or back-end rather than by user code.  In this case
+   builtin-ness is indicated by source location.  */
 #define DECL_IS_BUILTIN(DECL) \
   (LOCATION_LOCUS (DECL_SOURCE_LOCATION (DECL)) <= BUILTINS_LOCATION)
 
@@ -2486,7 +2489,13 @@ extern vec **decl_debug_args_insert (tree);
 #define DECL_STRUCT_FUNCTION(NODE) \
   (FUNCTION_DECL_CHECK (NODE)->function_decl.f)
 
-/* In a FUNCTION_DECL, nonzero means a built in function.  */
+/* In a FUNCTION_DECL, nonzero means a built in function of a
+   standard library or more generally a built in function that is
+   recognized by optimizers and expanders.
+
+   Note that it is different from the DECL_IS_BUILTIN accessor.  For
+   instance, user declarated prototypes of C library functions are not
+   DECL_IS_BUILTIN but may be DECL_BUILT_IN.  */
 #define DECL_BUILT_IN(NODE) (DECL_BUILT_IN_CLASS (NODE) != NOT_BUILT_IN)
 
 /* For a builtin function, identify which part of the compiler defined it.  */
-- 

Dodji

Re: Fwd: [PATCH] Scheduling result adjustment to enable macro-fusion

[Alexander Monakov]

Hello,

Could you use the existing facilities instead, such as adjust_priority hook,
or making the compare-branch insn sequence a SCHED_GROUP?

Alexander

[PATCH][RFC] Move IVOPTs closer to RTL expansion

[Richard Biener]

The patch below moves IVOPTs out of the GIMPLE loop pipeline more
closer to RTL expansion.  That's done for multiple reasons.

First, the loop passes that at the moment preceede IVOPTs leave
around IL that is in desparate need of basic re-optimization
like CSE, constant propagation and DCE.  That puts extra load
on IVOPTs and its cost model, increasing compile-time and
possibly confusing it.

Second, IVOPTs introduces lowered memory accesses that it
expects to stay as is, likewise it produces auto-inc/dec
sequences that it expects to stay as is until RTL expansion.
Passes such as DOM can break this expectation and make the
work done by IVOPTs a waste.

I remember doing this excercise in the GCC 4.3 timeframe where
benchmarking on x86_64 showed no gains or losses (but x86_64
isn't very sensitive to IV choices).

Any help with benchmarking this on targets other than x86_64
is appreciated (I'll re-do x86_64).

Bootstrapped and tested on x86_64-unknown-linux-gnu.

General comments are of course also welcome.

Thanks,
Richard.

2013-09-04  Richard Biener  

* passes.def: Move IVOPTs before final DCE pass.
* tree-ssa-loop.c (tree_ssa_loop_ivopts): Adjust for being
outside of the loop pipeline.

* gcc.dg/tree-ssa/ivopts-3.c: Scan non-details dump.
* gcc.dg/tree-ssa/reassoc-19.c: Be more permissive.

Index: gcc/passes.def
===
*** gcc/passes.def.orig 2013-09-04 10:57:33.0 +0200
--- gcc/passes.def  2013-09-04 11:11:27.535952665 +0200
*** along with GCC; see the file COPYING3.
*** 221,227 
  NEXT_PASS (pass_complete_unroll);
  NEXT_PASS (pass_slp_vectorize);
  NEXT_PASS (pass_loop_prefetch);
- NEXT_PASS (pass_iv_optimize);
  NEXT_PASS (pass_lim);
  NEXT_PASS (pass_tree_loop_done);
POP_INSERT_PASSES ()
--- 221,226 
*** along with GCC; see the file COPYING3.
*** 237,242 
--- 236,246 
 opportunities.  */
NEXT_PASS (pass_phi_only_cprop);
NEXT_PASS (pass_vrp);
+   /* IVOPTs lowers memory accesses and exposes auto-inc/dec
+  opportunities.  Run it after the above passes cleaned up
+the loop optimized IL but before DCE as IVOPTs generates
+quite some garbage.  */
+   NEXT_PASS (pass_iv_optimize);
NEXT_PASS (pass_cd_dce);
NEXT_PASS (pass_tracer);
  
Index: gcc/tree-ssa-loop.c
===
*** gcc/tree-ssa-loop.c.orig2013-09-04 10:57:32.0 +0200
--- gcc/tree-ssa-loop.c 2013-09-04 11:11:27.536952677 +0200
*** make_pass_loop_prefetch (gcc::context *c
*** 906,915 
  static unsigned int
  tree_ssa_loop_ivopts (void)
  {
!   if (number_of_loops (cfun) <= 1)
! return 0;
  
-   tree_ssa_iv_optimize ();
return 0;
  }
  
--- 906,924 
  static unsigned int
  tree_ssa_loop_ivopts (void)
  {
!   loop_optimizer_init (LOOPS_NORMAL
!  | LOOPS_HAVE_RECORDED_EXITS);
! 
!   if (number_of_loops (cfun) > 1)
! {
!   rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa);
!   scev_initialize ();
!   tree_ssa_iv_optimize ();
!   scev_finalize ();
! }
! 
!   loop_optimizer_finalize ();
  
return 0;
  }
  
Index: gcc/testsuite/gcc.dg/tree-ssa/ivopts-3.c
===
*** gcc/testsuite/gcc.dg/tree-ssa/ivopts-3.c.orig   2013-09-04 
10:57:33.0 +0200
--- gcc/testsuite/gcc.dg/tree-ssa/ivopts-3.c2013-09-04 11:11:27.559952952 
+0200
***
*** 1,5 
  /* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-ivopts-details" } */
  
  void main (void)
  {
--- 1,5 
  /* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-ivopts" } */
  
  void main (void)
  {
*** void main (void)
*** 8,12 
  f2 ();
  }
  
! /* { dg-final { scan-tree-dump-times "!= 0" 5 "ivopts" } }  */
  /* { dg-final { cleanup-tree-dump "ivopts" } }  */
--- 8,12 
  f2 ();
  }
  
! /* { dg-final { scan-tree-dump-times "!= 0" 1 "ivopts" } }  */
  /* { dg-final { cleanup-tree-dump "ivopts" } }  */
Index: gcc/testsuite/gcc.dg/tree-ssa/reassoc-19.c
===
*** gcc/testsuite/gcc.dg/tree-ssa/reassoc-19.c.orig 2012-12-18 
14:24:58.0 +0100
--- gcc/testsuite/gcc.dg/tree-ssa/reassoc-19.c  2013-09-04 11:13:30.895416700 
+0200
*** void foo(char* left, char* rite, int ele
*** 16,22 
}
  }
  
! /* { dg-final { scan-tree-dump-times "= \\\(sizetype\\\) element" 1 
"optimized" } } */
  /* { dg-final { scan-tree-dump-times "= -" 1 "optimized" } } */
  /* { dg-final { scan-tree-dump-times " \\\+ " 1 "optimized" } } */
  /* { dg-final { cleanup-tree-dump "optimized" } } */
--- 16,22 
}
  }
  
! /* { dg-final { scan-tree-dump-times "= \\\(\[^)\]*\\\) element" 1 
"optimized" } } */

[patch] boehm-gc: link libgcjgc with -ldl

[Matthias Klose]

The boehm-gc tests currently fail to build with a linker with
--no-copy-dt-needed-entries as the default. dlopen is referenced in the libgcjgc
library itself, so link it with -ldl.  The macro name EXTRA_TEST_LIBS is a bit
unfortunate now, but it is the right way to find the library name, as done for
the tests itself.

Ok for the trunk and the 4.8 branch?

  Matthias

* Makefile.am (libgcjgc_la_LIBADD): Add EXTRA_TEST_LIBS.
* Makefile.in: Regenerate.

--- a/src/boehm-gc/Makefile.am
+++ a/src/boehm-gc/Makefile.am
@@ -35,7 +35,7 @@
 
 # Include THREADLIBS here to ensure that the correct versions of
 # linuxthread semaphore functions get linked:
-libgcjgc_la_LIBADD = $(addobjs) $(THREADLIBS)
+libgcjgc_la_LIBADD = $(addobjs) $(THREADLIBS) $(EXTRA_TEST_LIBS)
 libgcjgc_la_DEPENDENCIES = $(addobjs)
 libgcjgc_la_LDFLAGS = $(extra_ldflags_libgc) -version-info 1:2:0 -rpath 
$(toolexeclibdir)
 libgcjgc_la_LINK = $(LINK) $(libgcjgc_la_LDFLAGS)

Re: [C++ Patch] PR 24926

[Paolo Carlini]

... assuming the general idea makes sense, this version may be better 
because, at the cost of 3 lines of code duplication, it cuts the 
unnecessary function calls, eg exactly zero if the struct doesn't have 
anonymous aggregates at all. The patch is also easier to read ;)


Booted and tested x86_64-linux.

Paolo.

/
Index: cp/class.c
===
--- cp/class.c  (revision 202241)
+++ cp/class.c  (working copy)
@@ -2773,15 +2773,96 @@ warn_hidden (tree t)
 }
 }
 
+/* Recursive helper for finish_struct_anon.  */
+
+static void
+finish_struct_anon_r (tree field, bool complain)
+{
+  bool is_union = TREE_CODE (TREE_TYPE (field)) == UNION_TYPE;
+  tree elt = TYPE_FIELDS (TREE_TYPE (field));
+  for (; elt; elt = DECL_CHAIN (elt))
+{
+  /* We're generally only interested in entities the user
+declared, but we also find nested classes by noticing
+the TYPE_DECL that we create implicitly.  You're
+allowed to put one anonymous union inside another,
+though, so we explicitly tolerate that.  We use
+TYPE_ANONYMOUS_P rather than ANON_AGGR_TYPE_P so that
+we also allow unnamed types used for defining fields.  */
+  if (DECL_ARTIFICIAL (elt)
+ && (!DECL_IMPLICIT_TYPEDEF_P (elt)
+ || TYPE_ANONYMOUS_P (TREE_TYPE (elt
+   continue;
+
+  if (!complain && TREE_STATIC (elt))
+   continue;
+
+  if (TREE_CODE (elt) != FIELD_DECL)
+   {
+ if (complain)
+   {
+ if (is_union)
+   permerror (input_location,
+  "%q+#D invalid; an anonymous union can "
+  "only have non-static data members", elt);
+ else
+   permerror (input_location,
+  "%q+#D invalid; an anonymous struct can "
+  "only have non-static data members", elt);
+   }
+ continue;
+   }
+
+  if (complain)
+   {
+ if (TREE_PRIVATE (elt))
+   {
+ if (is_union)
+   permerror (input_location,
+  "private member %q+#D in anonymous union", elt);
+ else
+   permerror (input_location,
+  "private member %q+#D in anonymous struct", elt);
+   }
+ else if (TREE_PROTECTED (elt))
+   {
+ if (is_union)
+   permerror (input_location,
+  "protected member %q+#D in anonymous union", elt);
+ else
+   permerror (input_location,
+  "protected member %q+#D in anonymous struct", elt);
+   }
+   }
+
+  TREE_PRIVATE (elt) = TREE_PRIVATE (field);
+  TREE_PROTECTED (elt) = TREE_PROTECTED (field);
+
+  /* Recur inside the anonymous aggregates to handle correctly
+access control (c++/24926):
+
+class A {
+  union {
+union {
+  int i;
+};
+  };
+};
+
+int j=A().i;  */
+  if (DECL_NAME (elt) == NULL_TREE
+ && ANON_AGGR_TYPE_P (TREE_TYPE (elt)))
+   finish_struct_anon_r (elt, /*complain=*/false);
+}
+}
+
 /* Check for things that are invalid.  There are probably plenty of other
things we should check for also.  */
 
 static void
 finish_struct_anon (tree t)
 {
-  tree field;
-
-  for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
+  for (tree field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
 {
   if (TREE_STATIC (field))
continue;
@@ -2790,53 +2871,7 @@ finish_struct_anon (tree t)
 
   if (DECL_NAME (field) == NULL_TREE
  && ANON_AGGR_TYPE_P (TREE_TYPE (field)))
-   {
- bool is_union = TREE_CODE (TREE_TYPE (field)) == UNION_TYPE;
- tree elt = TYPE_FIELDS (TREE_TYPE (field));
- for (; elt; elt = DECL_CHAIN (elt))
-   {
- /* We're generally only interested in entities the user
-declared, but we also find nested classes by noticing
-the TYPE_DECL that we create implicitly.  You're
-allowed to put one anonymous union inside another,
-though, so we explicitly tolerate that.  We use
-TYPE_ANONYMOUS_P rather than ANON_AGGR_TYPE_P so that
-we also allow unnamed types used for defining fields.  */
- if (DECL_ARTIFICIAL (elt)
- && (!DECL_IMPLICIT_TYPEDEF_P (elt)
- || TYPE_ANONYMOUS_P (TREE_TYPE (elt
-   continue;
-
- if (TREE_CODE (elt) != FIELD_DECL)
-   {
- if (is_union)
-   permerror (input_location, "%q+#D invalid; an anonymous 
union can "
-  "only have non-static data members", elt);
- else
-   permerror (input_location, "%q

Re: [PATCH] More comments about DECL_BUILT_IN and DECL_IS_BUILTIN (was Re: Why DECL_BUILT_IN and DECL_IS_BUILTIN?)

[Richard Biener]

On Wed, Sep 4, 2013 at 10:46 AM, Dodji Seketeli  wrote:
> Hello,
>
> Richard Biener  a écrit:
>
> [...]
>
>> DECL_IS_BUILTIN is true if the decl was created by the frontend / backend
>> rather than by user code (indicated by source location).  DECL_BUILT_IN
>> is true if the decl represents a function of the standard library, a
>> builtin that is
>> recognized by optimization / expansion.  User declared prototypes of
>> C library functions are not DECL_IS_BUILTIN but may be DECL_BUILT_IN.
>
> Every time I see these macros I have a hard time telling which is which
> :-) So I felt I'd stand on your shoulders (if you don't mind) and add
> these comments right in tree.h so that they are easier to find next
> time.
>
> OK to commit this comment-only patchlet to trunk then?

Ok.

Thanks,
Richard.

> Thanks.
>
> From 1ad29143764a72d27b1ecf3c06b4ba72bfaf4fe8 Mon Sep 17 00:00:00 2001
> From: Dodji Seketeli 
> Date: Wed, 4 Sep 2013 10:32:36 +0200
> Subject: [PATCH] More comments about DECL_BUILT_IN and DECL_IS_BUILTIN
>
> gcc/ChangeLog
>
> * tree.h (DECL_BUILT_IN, DECL_IS_BUILTIN): Add more comments
> explaining their differences.
> ---
>  gcc/tree.h | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 718d8f4..88d527a 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -1838,6 +1838,9 @@ extern enum machine_mode vector_type_mode (const_tree);
>  #define DECL_SOURCE_FILE(NODE) LOCATION_FILE (DECL_SOURCE_LOCATION (NODE))
>  #define DECL_SOURCE_LINE(NODE) LOCATION_LINE (DECL_SOURCE_LOCATION (NODE))
>  #define DECL_SOURCE_COLUMN(NODE) LOCATION_COLUMN (DECL_SOURCE_LOCATION 
> (NODE))
> +/* This accessor returns TRUE if the decl it operates on was created
> +   by a front-end or back-end rather than by user code.  In this case
> +   builtin-ness is indicated by source location.  */
>  #define DECL_IS_BUILTIN(DECL) \
>(LOCATION_LOCUS (DECL_SOURCE_LOCATION (DECL)) <= BUILTINS_LOCATION)
>
> @@ -2486,7 +2489,13 @@ extern vec **decl_debug_args_insert 
> (tree);
>  #define DECL_STRUCT_FUNCTION(NODE) \
>(FUNCTION_DECL_CHECK (NODE)->function_decl.f)
>
> -/* In a FUNCTION_DECL, nonzero means a built in function.  */
> +/* In a FUNCTION_DECL, nonzero means a built in function of a
> +   standard library or more generally a built in function that is
> +   recognized by optimizers and expanders.
> +
> +   Note that it is different from the DECL_IS_BUILTIN accessor.  For
> +   instance, user declarated prototypes of C library functions are not
> +   DECL_IS_BUILTIN but may be DECL_BUILT_IN.  */
>  #define DECL_BUILT_IN(NODE) (DECL_BUILT_IN_CLASS (NODE) != NOT_BUILT_IN)
>
>  /* For a builtin function, identify which part of the compiler defined it.  
> */
> --
>
> Dodji

Re: [patch] boehm-gc: link libgcjgc with -ldl

[Andrew Haley]

On 09/04/2013 11:00 AM, Matthias Klose wrote:
> The boehm-gc tests currently fail to build with a linker with
> --no-copy-dt-needed-entries as the default.

Hmm, isn't that a bug in the linker?

Andrew.

Re: [patch] boehm-gc: link libgcjgc with -ldl

[Matthias Klose]

Am 04.09.2013 12:21, schrieb Andrew Haley:
> On 09/04/2013 11:00 AM, Matthias Klose wrote:
>> The boehm-gc tests currently fail to build with a linker with
>> --no-copy-dt-needed-entries as the default.
> 
> Hmm, isn't that a bug in the linker?

No, it's the default in gold and in the bfd linker since 2.23.

  Matthias

Re: [patch] boehm-gc: link libgcjgc with -ldl

[Andrew Haley]

On 09/04/2013 11:24 AM, Matthias Klose wrote:
> Am 04.09.2013 12:21, schrieb Andrew Haley:
>> On 09/04/2013 11:00 AM, Matthias Klose wrote:
>>> The boehm-gc tests currently fail to build with a linker with
>>> --no-copy-dt-needed-entries as the default.
>>
>> Hmm, isn't that a bug in the linker?
> 
> No, it's the default in gold and in the bfd linker since 2.23.

Oh, right.  I wonder how many other things this will break.  OK.

Andrew.

Re: [patch, fortran, docs] Unformatted sequential and special files

[Janne Blomqvist]

On Tue, Sep 3, 2013 at 3:04 PM, Thomas Koenig  wrote:
> Hello world,
>
> here is a rewrite, which I hope make things more clear.
> Unformatted sequential files are now made up of subrecords, where
> a logical record may only have one.

Looks ok.

> Regarding block devices: I don't know anybody who ever used them
> from gfortran, so I tried to be as vague as possible.
>
> Any more suggestions?  OK for trunk otherwise?

I'm still not comfortable with the wording. As I've argued before,
special files are special in different ways, and can behave
differently on different systems, so it's difficult to say anything
definitive about their behavior in general. Maybe being more explicit
about what is supported for a limited subset would help. E.g. starting
the section with something like

"For terminal devices, pipes, FIFO's and sockets the following file
modes are supported:
- ...

For other special files and other file modes, the result is undefined."

("undefined" rather than "not supported", as we're not going out of
our way to prevent it if somebody wants to do it either)

- Wrt the POS= specifier with INQUIRE, even it it "works" (as in, does
not generate an error), there is no sensible concept of file position
for a stream file anyway, so perhaps we shouldn't explicitly say it's
supported either.

- Wrt. block devices, perhaps remove that section and cover it just
with the "...implementation defined" sentence above.



-- 
Janne Blomqvist

Clean up pretty printers [21/n]

[Gabriel Dos Reis]

Tested on an x86_64-suse-linux.  Applied to trunk.

-- Gaby

2013-09-04  Gabriel Dos Reis  
c-family/
* c-pretty-print.h (c_pretty_printer::simple_type_specifier): Now
a virtual member function.
(pp_simple_type_specifier): Remove.
(pp_c_type_specifier): Likewise.
* c-pretty-print.c (c_pretty_printer::simple_type_specifier):
Rename from pp_c_type_specifier.  Adjust.
(c_pretty_printer::c_pretty_printer): Do not assign to
simple_type_specifier.
cp/
* cxx-pretty-print.h (cxx_pretty_printer::simple_type_specifier):
Declare as overrider.
* cxx-pretty-print.c (cxx_pretty_printer::simple_type_specifier):
Rename from pp_cxx_simple_type_specifier.
(cxx_pretty_printer::cxx_pretty_printer): Do not assign to
simple_type_specifier.

Index: c-family/c-pretty-print.c
===
--- c-family/c-pretty-print.c   (revision 202240)
+++ c-family/c-pretty-print.c   (working copy)
@@ -305,7 +305,10 @@
 }
 }
 
-/* type-specifier:
+/* simple-type-specifier:
+ type-specifier
+
+   type-specifier:
   void
   char
   short
@@ -328,17 +331,17 @@
   __vector__   */
 
 void
-pp_c_type_specifier (c_pretty_printer *pp, tree t)
+c_pretty_printer::simple_type_specifier (tree t)
 {
   const enum tree_code code = TREE_CODE (t);
   switch (code)
 {
 case ERROR_MARK:
-  pp->translate_string ("");
+  translate_string ("");
   break;
 
 case IDENTIFIER_NODE:
-  pp_c_identifier (pp, IDENTIFIER_POINTER (t));
+  pp_c_identifier (this, IDENTIFIER_POINTER (t));
   break;
 
 case VOID_TYPE:
@@ -349,7 +352,7 @@
   if (TYPE_NAME (t))
{
  t = TYPE_NAME (t);
- pp_c_type_specifier (pp, t);
+ simple_type_specifier (t);
}
   else
{
@@ -360,11 +363,11 @@
t = c_common_type_for_mode (TYPE_MODE (t), TYPE_UNSIGNED (t));
  if (TYPE_NAME (t))
{
- pp_c_type_specifier (pp, t);
+ simple_type_specifier (t);
  if (TYPE_PRECISION (t) != prec)
{
- pp_colon (pp);
- pp_decimal_int (pp, prec);
+ pp_colon (this);
+ pp_decimal_int (this, prec);
}
}
  else
@@ -372,52 +375,52 @@
  switch (code)
{
case INTEGER_TYPE:
- pp->translate_string (TYPE_UNSIGNED (t)
-? "translate_string ("translate_string ("id_expression (t);
+   id_expression (t);
   else
-   pp->translate_string ("");
+   translate_string ("");
   break;
 
 case UNION_TYPE:
 case RECORD_TYPE:
 case ENUMERAL_TYPE:
   if (code == UNION_TYPE)
-   pp_c_ws_string (pp, "union");
+   pp_c_ws_string (this, "union");
   else if (code == RECORD_TYPE)
-   pp_c_ws_string (pp, "struct");
+   pp_c_ws_string (this, "struct");
   else if (code == ENUMERAL_TYPE)
-   pp_c_ws_string (pp, "enum");
+   pp_c_ws_string (this, "enum");
   else
-   pp->translate_string ("");
+   translate_string ("");
 
   if (TYPE_NAME (t))
-   pp->id_expression (TYPE_NAME (t));
+   id_expression (TYPE_NAME (t));
   else
-   pp->translate_string ("");
+   translate_string ("");
   break;
 
 default:
-  pp_unsupported_tree (pp, t);
+  pp_unsupported_tree (this, t);
   break;
 }
 }
@@ -483,7 +486,7 @@
   break;
 
 default:
-  pp_simple_type_specifier (pp, t);
+  pp->simple_type_specifier (t);
   break;
 }
   if ((pp->flags & pp_c_flag_gnu_v3) && code != POINTER_TYPE)
@@ -2328,7 +2331,6 @@
   type_specifier_seq= pp_c_specifier_qualifier_list;
   ptr_operator  = pp_c_pointer;
   parameter_list= pp_c_parameter_type_list;
-  simple_type_specifier = pp_c_type_specifier;
 }
 
 
Index: c-family/c-pretty-print.h
===
--- c-family/c-pretty-print.h   (revision 202240)
+++ c-family/c-pretty-print.h   (working copy)
@@ -69,6 +69,7 @@
 
   virtual void declaration (tree);
   virtual void declaration_specifiers (tree);
+  virtual void simple_type_specifier (tree);
   virtual void function_specifier (tree);
   virtual void storage_class_specifier (tree);
   virtual void declarator (tree);
@@ -88,7 +89,6 @@
   c_pretty_print_fn type_specifier_seq;
   c_pretty_print_fn ptr_operator;
   c_pretty_print_fn parameter_list;
-  c_pretty_print_fn simple_type_specifier;
 };
 
 #define pp_c_tree_identifier(PPI, ID)  \
@@ -97,7 +97,6 @@
 #define pp_type_specifier_seq(PP, D)(PP)->type_specifier_seq (PP, D)
 #define pp_ptr_operator(PP, D)  (PP)->ptr_operator (PP, D)
 #define pp_parameter_list(PP, T)(PP)->parameter_list (PP, T)
-#define pp_simple_type_s

Re: [C++ Patch] PR 24926

[Jason Merrill]

It looks to me like this will result in duplicate diagnostics for 
invalid members in a nested anonymous union.  Maybe make the recursive 
part only handle access setting?


Jason

[Patch AArch64] Obvious - Fix return types for vaddvq_64

[James Greenhalgh]

The vaddvq_s64 and vaddvq_u64 intrinsics are defined to return 32-bit
types. This is clearly wrong, so fix them to return int64_t and uint64_t
as expected.

Regression tested with a run through aarch64.exp and sanity checked.

OK for trunk?

Thanks,
James

---
gcc/

2013-09-04  James Greenhalgh  

* config/aarch64/arm_neon.h (vaddvq_64): Fix return types.
diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
index e289a0d..29d1378 100644
--- a/gcc/config/aarch64/arm_neon.h
+++ b/gcc/config/aarch64/arm_neon.h
@@ -17033,7 +17033,7 @@ vaddvq_s32 (int32x4_t __a)
   return vgetq_lane_s32 (__builtin_aarch64_reduc_splus_v4si (__a), 0);
 }
 
-__extension__ static __inline int32_t __attribute__ ((__always_inline__))
+__extension__ static __inline int64_t __attribute__ ((__always_inline__))
 vaddvq_s64 (int64x2_t __a)
 {
   return vgetq_lane_s64 (__builtin_aarch64_reduc_splus_v2di (__a), 0);
@@ -17060,7 +17060,7 @@ vaddvq_u32 (uint32x4_t __a)
 		__builtin_aarch64_reduc_uplus_v4si ((int32x4_t) __a), 0);
 }
 
-__extension__ static __inline uint32_t __attribute__ ((__always_inline__))
+__extension__ static __inline uint64_t __attribute__ ((__always_inline__))
 vaddvq_u64 (uint64x2_t __a)
 {
   return vgetq_lane_u64 ((uint64x2_t)

Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy

[Michael Matz]

Hi,

On Tue, 3 Sep 2013, David Malcolm wrote:

> > I can't really say I find this shorter, easier to read, more
> > expressive or even safer than what was there before.  And the 
> > repetition for adding the helpers for const and non-const types
> > all the time doesn't make it better.
> Part of this is the verbose struct names.  I mentioned getting rid of
> the "_statement" part of the typenames, I think I'll do that.

Yep, IMO makes sense.

> The other part is that the accessor functions become redundant, and that 
> you'd be able to do the cast once, and then use all of the various 
> fields of a gimple_whatever, bypassing the getters/setters.

Well, you can do that today with unions too, it's just not prevalent 
style; but you could do:

if (gimple_has_mem_ops (g))
  {
struct gimple_statement_with_memory_ops_base *gm = &g->gsmembase;
gm->vuse = ...;
  }

Obviously the naming of the struct here also is a bit excessive.  Using 
accessors has one large advantage over accessing the fields directly, you 
can change the semantics of them.  One reason why the above style isn't 
used.  But if that is true one of your reasons doing the change (downcast 
once, access fields directly, obsoleting the accessors) becomes moot, 
because we don't _want_ to access the fields directly.  That is, until you 
add member functions doing the accesses, which has its own problems (of 
stylistic nature, because then we'd have a very weird and clumsy mix in 
GCC sources of some data structures having member function accessors and 
others using the traditional C style).

Hmm.  After some nights sleeping over this, I'm oscillating again between 
not liking the change and being indifferent; as in, I do see some of the 
advantages you mentioned but I don't regard them outweighing the 
disadvantages.


Ciao,
Michael.

Re: [patch 4/4] -fstrict-volatile-bitfields cleanup v3: remove from defaults on all targets

[DJ Delorie]

> I fully agree with you, the current default state of
> -fstrict-volatile-bitfields should be disabled for all targets.

Please don't do that until gcc produces code that does the same
things.  Most of my targets rely on gcc not doing the old behavior, to
generate correct code.

> For portability of application code, the default should be always
> off, unless specifically requested.

The vendors of my targets specificially requested it be the default.

> Even driver code rarely uses bit-fields for register access,

People keep saying this, and people are wrong.  For the targets I
support, they *all* use bitfields for *all* the peripherals, because
that's what's in the headers the vendor ships with each chip.

Re: [Patch] Fix infinite loop/crash if array initializer index equals max value

[Senthil Kumar Selvaraj]

On Fri, Aug 23, 2013 at 09:49:55PM +, Joseph S. Myers wrote:
> On Thu, 22 Aug 2013, Selvaraj, Senthil_Kumar wrote:
> 
> > 2013-08-23  Senthil Kumar Selvaraj  
> > * c-typeck.c (output_pending_init_elements): Handle overflow of
> > constructor_unfilled_index.
> 
> This patch needs to add include a testcase to the testsuite that fails 
> before and passes after the patch.  (I realise such a test may only be 
> able to run for a subset of targets.)

Reattaching the patch with a testcase for the AVR target. I'm not sure
how to generalize the testcase for other targets - the constant is the
max value (unsigned) of the mode used to represent initialized array
indices.

The attached test fails with a timeout before applying the patch, and
passes after applying it.

Regards
Senthil

gcc/c/ChangeLog

2013-09-04  Senthil Kumar Selvaraj  
* c-typeck.c (output_pending_init_elements): Handle overflow of
constructor_unfilled_index.

gcc/testsuite/ChangeLog

2013-09-04  Senthil Kumar Selvaraj  
* gcc.dg/large-size-array-7.c: New test to verify overflow handling
of constructor_unfilled_index.

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 30871db..ed2e37a 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -7953,8 +7953,9 @@ output_pending_init_elements (int all, struct obstack * 
braced_init_obstack)
 TREE_TYPE (constructor_type),
 constructor_unfilled_index, 0, false,
 braced_init_obstack);
- else if (tree_int_cst_lt (constructor_unfilled_index,
-   elt->purpose))
+  else if (!TREE_OVERFLOW_P (constructor_unfilled_index)
+&& tree_int_cst_lt (constructor_unfilled_index,
+   elt->purpose))
{
  /* Advance to the next smaller node.  */
  if (elt->left)
@@ -7979,7 +7980,8 @@ output_pending_init_elements (int all, struct obstack * 
braced_init_obstack)
  while (elt->parent && elt->parent->right == elt)
elt = elt->parent;
  elt = elt->parent;
- if (elt && tree_int_cst_lt (constructor_unfilled_index,
+  if (elt && !TREE_OVERFLOW_P (constructor_unfilled_index)
+  && tree_int_cst_lt (constructor_unfilled_index,
  elt->purpose))
{
  next = elt->purpose;
diff --git gcc/testsuite/gcc.dg/large-size-array-7.c 
gcc/testsuite/gcc.dg/large-size-array-7.c
new file mode 100644
index 000..196767d
--- /dev/null
+++ gcc/testsuite/gcc.dg/large-size-array-7.c
@@ -0,0 +1,5 @@
+/* { dg-do compile { target "avr-*-*" } } */
+/* { dg-options "-O2" } */
+static char * name[] = {
+[0xFF]  = "bar"
+  }; /* { dg-error "too large" } */

RE: [patch 4/4] -fstrict-volatile-bitfields cleanup v3: remove from defaults on all targets

[Bernd Edlinger]

On Wed, 4 Sep 2013 09:29:02, DJ Delorie wrote:
>
>> I fully agree with you, the current default state of
>> -fstrict-volatile-bitfields should be disabled for all targets.
>
> Please don't do that until gcc produces code that does the same
> things. Most of my targets rely on gcc not doing the old behavior, to
> generate correct code.
>
>> For portability of application code, the default should be always
>> off, unless specifically requested.
>
> The vendors of my targets specificially requested it be the default.
>
>> Even driver code rarely uses bit-fields for register access,
>
> People keep saying this, and people are wrong. For the targets I
> support, they *all* use bitfields for *all* the peripherals, because
> that's what's in the headers the vendor ships with each chip.

I really do respect your customer's requirements.

I do not even think about changing the default state of
-fstrict-volatile-bitfields on gcc 4.7.x or gcc 4.8.x, although
I hope that the other parts of this patch will make their way
to these old versions too.

How about this for a compromise: Let's make the
default of -fstrict-volatile-bitfields an optional
configure argument for gcc 4.9, that can be used for every
target, even for X86_64 of you want ?

Regards
Bernd.

Re: [PATCH] More comments about DECL_BUILT_IN and DECL_IS_BUILTIN (was Re: Why DECL_BUILT_IN and DECL_IS_BUILTIN?)

[Ian Lance Taylor]

On Wed, Sep 4, 2013 at 1:46 AM, Dodji Seketeli  wrote:
>
> +/* In a FUNCTION_DECL, nonzero means a built in function of a
> +   standard library or more generally a built in function that is
> +   recognized by optimizers and expanders.
> +
> +   Note that it is different from the DECL_IS_BUILTIN accessor.  For
> +   instance, user declarated prototypes of C library functions are not
> +   DECL_IS_BUILTIN but may be DECL_BUILT_IN.  */
>  #define DECL_BUILT_IN(NODE) (DECL_BUILT_IN_CLASS (NODE) != NOT_BUILT_IN)

s/declarated/declared/

Ian

Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy

[Jan Hubicka]

> On Mon, 2013-09-02 at 14:35 +0200, Martin Jambor wrote:
> > Hi,
> > 
> > On Fri, Aug 30, 2013 at 03:21:22PM -0400, David Malcolm wrote:
> > > Apart from the GTY aspect, how do people feel about the patch series?
> > > FWIW I have vague thoughts about doing something similar for tree -
> > > doing so *might* give an easier route to the type vs expression
> > > separation that Andrew spoke about at the Cauldron rearchitecture BoF.
> > > (I started looking at doing a similar C++-ification of rtx, but...
> > > gah)
> > > 
> > 
> > I like it but before you start looking at the biger things, could you
> > perhpas proceed with the symtab?  It has much fewer classes, will
> > probably affect private development of fewer people, the accessor
> > macros/functions of symtab are less developed so it will immediately
> > really make code nicer, Honza has approved it and I'm really looking
> > forward to it.  Also, perhaps it will show us at much saller scale
> > potential problems with the general scheme.
> 
> Sorry about the delay.  I wasn't aware that it had been approved; there
> seemed to be a lot of caveats and objections in that thread.  On
> re-reading, http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01147.html
> could be seen as approval, but I guess I was making a conservative
> reading of that post.  I hope to refresh the patches and
> reboostrap/repost them at some point this week.

I think the patch was generally in right direction.  I would welcome
if you got rid of symtab_node_base (and made symtab_node the basetype)
and possibly also did the suggested grand renaming.  But the second
can be probably done incrementally.
> 
> > I'm only writing this because the development there seems a bit
> > stalled and it it a shame.  Of course, you ay want to simplify the
> > manual markings first.  I'd perfectly understand that.
> 
> I've been poking at gengtype (and running benchmarks; see other post),
> which would affect the symtab patch, though it's something of a
> quagmire...

Making gengtype to generate ggc_mark for each type would make hand writting
easier - you can use C++ overloading instead of trying to guess the funny
names gengtype uses right now.
But that is independent of this change.  I am slowly getting used to the
world of hand written gengtype markings.

Honza

Re: [patch 4/4] -fstrict-volatile-bitfields cleanup v3: remove from defaults on all targets

[Richard Biener]

On Wed, Sep 4, 2013 at 3:55 PM, DJ Delorie  wrote:
>
>> How about this for a compromise: Let's make the default of
>> -fstrict-volatile-bitfields an optional configure argument for gcc
>> 4.9, that can be used for every target, even for X86_64 of you want ?
>
> I don't care how it's enabled (currently, each target that wants it,
> sets it) as long as a plain "./configure" gives the correct defaults.
> "Don't forget this option or your code won't work" isn't an option.

You mean the C++11 application or the driver?  You mean
-fstrict-volatile-bitfields or -fno-strict-volatile-bitfields?

Not to think about C++11 drivers ;)

Richard.

Re: [Patch AArch64] Obvious - Fix return types for vaddvq_64

[Richard Earnshaw]

On 04/09/13 14:12, James Greenhalgh wrote:
> 
> The vaddvq_s64 and vaddvq_u64 intrinsics are defined to return 32-bit
> types. This is clearly wrong, so fix them to return int64_t and uint64_t
> as expected.
> 
> Regression tested with a run through aarch64.exp and sanity checked.
> 
> OK for trunk?
> 

OK.

There's no need to seek approval for really obvious patches like this.
 But please remember to post saying what you've done, just in case it
wasn't as obvious as you thought it was... :-)

R.

Re: [patch 4/4] -fstrict-volatile-bitfields cleanup v3: remove from defaults on all targets

[DJ Delorie]

> You mean the C++11 application or the driver?  You mean
> -fstrict-volatile-bitfields or -fno-strict-volatile-bitfields?

I mean, if the typedef for a volatile bitfield says "char" gcc can't
generate an HImode access, by default.

Re: [PATCH] manage dom-walk_data initialization and finalization with constructors and destructors

[Jeff Law]

On 09/04/2013 08:59 AM, Trevor Saunders wrote:

On Wed, Sep 04, 2013 at 10:32:17AM +0200, Richard Biener wrote:

On Wed, Sep 4, 2013 at 5:16 AM,   wrote:

From: Trevor Saunders 

bootstrapped on x86_64-unknown-linux-gnu with same test results as unpatched 
r202185 ok?


That looks like a not too useful part-C++-ification of domwalk.  A proper
C++-ification would include the use of functors and possibly templating it
on the user specific data type.


  It's true this patch isn't that aggressive, I was thinking about going
  farther incrementally.  I also haven't quiet convinced my self inlining
  all of domwalk.c or adding a whole bunch of explicit template
  instantiations to domwalk.c was really a good idea.  Are you saying you
  believe the removal of the indirect calls is worth the extra code size?


So, what do you think is the advantage (other than dropping two or three lines
of code in the users)?


I'd say its the same as for all other raii things, you don't have to be
sure to free data when you're done with it, and imho reducing boiler
plate is a fairly good thing on its own.
More importantly, I think converting the walker to a C++ object with 
methods ought to be rather straightforward.   The state each optimizer 
attaches to the walker naturally falls into a derived class.


I wouldn't go down templatizing it without a proof of concept which 
shows there's a performance improvement.


RAII is a good model for things like handling mutexes and simple 
allocation issues.


Jeff

[RFC] Fix for PR58201

[Jan Hubicka]

Hi,
this is third fallout of my change to remove DECL_ARGUMENTS/DECL_RESULT for 
functions w/o
bodies I did not really anticipate.

Here removal of the arguments changes mangling algorithm if
set_decl_assembler_name is invoked late.  This is something I wanted to get rid
of for a long time:  we already compute assembler names for every symbol that
lands symbol table after the early cleanups for every unit that is LTOed and
every unit containing any alias directive.  I think it will make things
smoother if we computed it always early: other persistent source of problems
are same body aliases that are created as a side effect of langhook of
set_decl_assembler_name and that may happen at a time IPA code does not really
expect new functions/variables to appear.

So independently of DECL_ARGUMENTS/DECL_RESULT issues, I would like to propose 
the
following patch that triggers unconditional computation of DECL_ASSEMBLER_NAME 
and the
real symbol table construction.  I already benchmarked it few months ago and the
erformance implications seems in wash.

Just to keep things linked, the other two fallouts are
 1) problem with thunks needing DECL_ARGUMENTS when they are output in gimple 
way
but these are not streamed, handled by 
http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00057.html
 2) problem with variable sized arguments and return values
"fixed" by http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00078.html
(the fix restored old behaviour, but I do not think it fixes the whole issue
as discussed in the thread and provided with another testcases that ICEs the
compiler independently of my changes).

I would like to basically ask if it seems to make sense to go this route and
try to get rid of those declarations.

For LTO it is really nice optimization - instead of streaming 4 decls at
average for a function (function-decl, result-decl and parm-decls), we need
just one. This change was originally motivated by memory analysis of firefox
WPA build where PARM_DECL was the most common type of tree just after
TREE_LIST.  I also think it makes sense from backend point of view to consider
these part of the function body representation, just as DECL_STRUCT_FUNCTOIN
and DECL_INITIAL is.

Even in non-LTO we save a lot of decls for all the external declarations that
are kept in memory.

I would be willing to try to analyze/fix the issues if they appear and I tried
quite curefuly to examine the existing code dealing with arguments. There
are obviously interesting scenarios, like this mangling issue, I missed.

If this does not seem to make sense, I will prepare patch to revert the changes.
If it does, I will commit those fixes and hope things will stabilize quickly.

Honza

* cgraphunit.c (analyze_functions): Populate assembler names once done
with early unreachable function removal.

Index: cgraphunit.c
===
--- cgraphunit.c(revision 202199)
+++ cgraphunit.c(working copy)
@@ -1074,6 +1086,7 @@
   bitmap_obstack_release (NULL);
   pointer_set_destroy (reachable_call_targets);
   ggc_collect ();
+  symtab_initialize_asm_name_hash ();
 }
 
 /* Translate the ugly representation of aliases as alias pairs into nice

Re: [Patch] Fix infinite loop/crash if array initializer index equals max value

[Joseph S. Myers]

On Wed, 4 Sep 2013, Senthil Kumar Selvaraj wrote:

> Reattaching the patch with a testcase for the AVR target. I'm not sure
> how to generalize the testcase for other targets - the constant is the
> max value (unsigned) of the mode used to represent initialized array
> indices.

Logically that should be __SIZE_MAX__, unless there's an unusual choice of 
size_t.

Are there any issues with large compiler memory consumption if you use 
__SIZE_MAX__ there for ordinary 32-bit or 64-bit targets?  If so, that 
would be a reason to keep the test restricted to AVR; if not, it would 
seem better to have __SIZE_MAX__ in the test (or a macro defined in the 
test that defaults to __SIZE_MAX__ but is overridden for particular 
targets, if some targets need it overridden).

-- 
Joseph S. Myers
jos...@codesourcery.com

C++ demangler fix

[Gary Benson]

Hi all,

d_print_comp maintains a certain amount of scope across calls (namely
a stack of templates) which is used when evaluating references in
template argument lists.  If such a reference is later used from a
subtitution then the scope in force at the time of the substitution is
used.  This appears to be wrong (I say appears because I couldn't find
anything in http://mentorembedded.github.io/cxx-abi/abi.html#mangling
to clarify this).

The attached patch causes the demangler to capture the scope the first
time such a reference is traversed, and to use that captured scope on
subsequent traversals.  This fixes
http://sourceware.org/bugzilla/show_bug.cgi?id=14963 whereby a
reference is resolved against the wrong template, causing an infinite
loop and eventual stack overflow and segmentation fault.

I've added the result to the demangler test suite, but I know of
no way to check the validity of the demangled symbol other than by
inspection (and I am no expert here!)  If anybody knows a way to
check this then please let me know!  Otherwise, I hope this
not-really-checked demangled version is acceptable.

Thanks,
Gary

--
http://gbenson.net/
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index e4ce0b9..a084282 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,22 @@
+2013-09-04  Gary Benson  
+
+   * cp-demangle.c: Include hashtab.h.
+   (struct d_print_info): New field saved_scopes.
+   (d_print_init): Initialize the above.
+   (d_print_free): New function.
+   (cplus_demangle_print_callback): Call the above.
+   (struct d_saved_scope): New structure.
+   (d_store_scope): New function.
+   (d_free_scope) Likewise.
+   (d_restore_scope) Likewise.
+   (d_hash_saved_scope) Likewise.
+   (d_equal_saved_scope) Likewise.
+   (d_print_comp): New variable saved_scope.
+   [DEMANGLE_COMPONENT_REFERENCE,
+   DEMANGLE_COMPONENT_RVALUE_REFERENCE]: Capture scope the first
+   time the component is traversed, and use the captured scope for
+   subsequent traversals.
+
 2013-08-20  Alan Modra  
 
* floatformat.c (floatformat_ibm_long_double): Rename to..
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 70f5438..d44e82a 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -128,6 +128,7 @@ extern char *alloca ();
 #include "libiberty.h"
 #include "demangle.h"
 #include "cp-demangle.h"
+#include "hashtab.h"
 
 /* If IN_GLIBCPP_V3 is defined, some functions are made static.  We
also rename them via #define to avoid compiler errors when the
@@ -302,6 +303,10 @@ struct d_print_info
   int pack_index;
   /* Number of d_print_flush calls so far.  */
   unsigned long int flush_count;
+  /* Table mapping demangle components to scopes saved when first
+ traversing those components.  These are used while evaluating
+ substitutions.  */
+  htab_t saved_scopes;
 };
 
 #ifdef CP_DEMANGLE_DEBUG
@@ -3665,6 +3670,17 @@ d_print_init (struct d_print_info *dpi, 
demangle_callbackref callback,
   dpi->opaque = opaque;
 
   dpi->demangle_failure = 0;
+
+  dpi->saved_scopes = NULL;
+}
+
+/* Free a print information structure.  */
+
+static void
+d_print_free (struct d_print_info *dpi)
+{
+  if (dpi->saved_scopes != NULL)
+htab_delete (dpi->saved_scopes);
 }
 
 /* Indicate that an error occurred during printing, and test for error.  */
@@ -3749,6 +3765,7 @@ cplus_demangle_print_callback (int options,
demangle_callbackref callback, void *opaque)
 {
   struct d_print_info dpi;
+  int success;
 
   d_print_init (&dpi, callback, opaque);
 
@@ -3756,7 +3773,9 @@ cplus_demangle_print_callback (int options,
 
   d_print_flush (&dpi);
 
-  return ! d_print_saw_error (&dpi);
+  success = ! d_print_saw_error (&dpi);
+  d_print_free (&dpi);
+  return success;
 }
 
 /* Turn components into a human readable string.  OPTIONS is the
@@ -3913,6 +3932,114 @@ d_print_subexpr (struct d_print_info *dpi, int options,
 d_append_char (dpi, ')');
 }
 
+/* A demangle component and some scope captured when it was first
+   traversed.  */
+
+struct d_saved_scope
+{
+  /* The component whose scope this is.  Used as the key for the
+ saved_scopes hashtable in d_print_info.  May be NULL if this
+ scope will not be inserted into that table.  */
+  const struct demangle_component *container;
+  /* Nonzero if the below items are copies and require freeing
+ when this scope is freed.  */
+  int is_copy;
+  /* The list of templates, if any, that was current when this
+ scope was captured.  */
+  struct d_print_template *templates;
+};
+
+/* Allocate a scope and populate it with the current values from DPI.
+   CONTAINER is the demangle component to which the scope refers, and
+   is used as the key for the saved_scopes hashtable in d_print_info.
+   CONTAINER may be NULL if this scope will not be inserted into that
+   table.  If COPY is nonzero then items that may have been alloca

Re: [c++-concepts] Class template constraints

[Jason Merrill]

On 09/03/2013 11:01 AM, Andrew Sutton wrote:

Attached is a patch for constrained class templates. It's the 3rd time
I've sent it.


Please feel free to ping me if you're waiting for a patch review; once a 
week is not too much.



1. Type constraints are checked on lookup rather than instantiation.


How is this different from function template constraints?  Is this just 
a difference in internal function name (instantiate_template vs 
lookup_template_class)?



+// Returns the type of a template specialization only if that
+// specializaiton needs to defined. Otherwise (e.g., if the type has


specialization


+  // Do the constraints match the most general template? Note that
+  // the absence of constraints will also match.
+  if (equivalent_constraints (cur_constr, DECL_CONSTRAINTS (tmpl)))


If absence matches, I think the name "equivalent" is misleading.  But 
the implementation seems to require subsumes in both directions.  What's 
up here?



+  // Find the template parameter list at the a depth appropriate to
+  // the scope we're trying to enter.
+  tree parms = current_template_parms;
+  int depth = template_class_depth (type);
+  for (int n = processing_template_decl; n > depth && parms; --n)
+parms = TREE_CHAIN (parms);


If you're going to use this function from lookup_template_class_1, it 
can't use current_template_*, since those are parser state which might 
be something completely unrelated when we get here during instantiation.



+return resolve_template_scope(gen_tmpl, template_type);


Space before (.


+  // Diaagnose constraints here since they are not diagnosed


Extra 'a'.


+  // If not, it is equivalent to have failed to compute the binding.


"to having failed"

Jason

Re: [RFC] Fix for PR58201

[Jakub Jelinek]

On Wed, Sep 04, 2013 at 06:04:09PM +0200, Jan Hubicka wrote:
>   * cgraphunit.c (analyze_functions): Populate assembler names once done
>   with early unreachable function removal.

Please add some of the testcases from the PRs and mention the PRs in the
ChangeLog entry.

> --- cgraphunit.c  (revision 202199)
> +++ cgraphunit.c  (working copy)
> @@ -1074,6 +1086,7 @@
>bitmap_obstack_release (NULL);
>pointer_set_destroy (reachable_call_targets);
>ggc_collect ();
> +  symtab_initialize_asm_name_hash ();
>  }
>  
>  /* Translate the ugly representation of aliases as alias pairs into nice

Jakub

[PATCH, AArch64] support extension option 'crc' in -march and -mcpu

[Yufeng Zhang]

Hi,

This patch adds the support for the crc extension option to the aarch64 
gcc driver.


OK for the trunk?

Thanks,
Yufeng

gcc/

* config/aarch64/aarch64-option-extensions.def: Add
AARCH64_OPT_EXTENSION of 'crc'.
* config/aarch64/aarch64.h (AARCH64_FL_CRC): New define.
(AARCH64_ISA_CRC): Ditto.
* doc/invoke.texi (-march and -mcpu feature modifiers): Add
description of the CRC extension.diff --git a/gcc/config/aarch64/aarch64-option-extensions.def 
b/gcc/config/aarch64/aarch64-option-extensions.def
index 58e8154..371e74c 100644
--- a/gcc/config/aarch64/aarch64-option-extensions.def
+++ b/gcc/config/aarch64/aarch64-option-extensions.def
@@ -35,3 +35,4 @@
 AARCH64_OPT_EXTENSION("fp",AARCH64_FL_FP,  AARCH64_FL_FPSIMD | 
AARCH64_FL_CRYPTO)
 AARCH64_OPT_EXTENSION("simd",  AARCH64_FL_FPSIMD,  AARCH64_FL_SIMD | 
AARCH64_FL_CRYPTO)
 AARCH64_OPT_EXTENSION("crypto",AARCH64_FL_CRYPTO | AARCH64_FL_FPSIMD,  
AARCH64_FL_CRYPTO)
+AARCH64_OPT_EXTENSION("crc",   AARCH64_FL_CRC, AARCH64_FL_CRC)
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 0924269..d8012f8 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -158,6 +158,7 @@
 #define AARCH64_FL_FP (1 << 1) /* Has FP.  */
 #define AARCH64_FL_CRYPTO (1 << 2) /* Has crypto.  */
 #define AARCH64_FL_SLOWMUL(1 << 3) /* A slow multiply core.  */
+#define AARCH64_FL_CRC(1 << 4) /* Has CRC.  */
 
 /* Has FP and SIMD.  */
 #define AARCH64_FL_FPSIMD (AARCH64_FL_FP | AARCH64_FL_SIMD)
@@ -170,6 +171,7 @@
 
 /* Macros to test ISA flags.  */
 extern unsigned long aarch64_isa_flags;
+#define AARCH64_ISA_CRC(aarch64_isa_flags & AARCH64_FL_CRC)
 #define AARCH64_ISA_CRYPTO (aarch64_isa_flags & AARCH64_FL_CRYPTO)
 #define AARCH64_ISA_FP (aarch64_isa_flags & AARCH64_FL_FP)
 #define AARCH64_ISA_SIMD   (aarch64_isa_flags & AARCH64_FL_SIMD)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 14955dd..0843178 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11138,6 +11138,8 @@ Feature modifiers used with @option{-march} and 
@option{-mcpu} can be one
 the following:
 
 @table @samp
+@item crc
+Enable CRC extension.
 @item crypto
 Enable Crypto extension.  This implies Advanced SIMD is enabled.
 @item fp

Re: [patch 4/4] -fstrict-volatile-bitfields cleanup v3: remove from defaults on all targets

[DJ Delorie]

> How about this for a compromise: Let's make the default of
> -fstrict-volatile-bitfields an optional configure argument for gcc
> 4.9, that can be used for every target, even for X86_64 of you want ?

I don't care how it's enabled (currently, each target that wants it,
sets it) as long as a plain "./configure" gives the correct defaults.
"Don't forget this option or your code won't work" isn't an option.

Re: [c++-concepts] Class template constraints

[Andrew Sutton]

>> 1. Type constraints are checked on lookup rather than instantiation.
>
>
> How is this different from function template constraints?  Is this just a
> difference in internal function name (instantiate_template vs
> lookup_template_class)?

It's not supposed to be different. Checking constraints in
instantiate_template is actually too late. We want to check before
instantiation, at the point of use. This also means we don't need
complete types to check constraints. So this:

  template
struct X;

  X* x;

Should fail and does. This change also makes it impossible to have
partial specializations that are more general than the primary
template. Checking in lookup_class_template does not consult the
specializations.

Constraints on partial specializations are checked in
most_specialized_class (or one of its subroutines).

>
>> +// Returns the type of a template specialization only if that
>> +// specializaiton needs to defined. Otherwise (e.g., if the type has
>
>
> specialization
>
>> +  // Do the constraints match the most general template? Note that
>> +  // the absence of constraints will also match.
>> +  if (equivalent_constraints (cur_constr, DECL_CONSTRAINTS (tmpl)))
>
>
> If absence matches, I think the name "equivalent" is misleading.  But the
> implementation seems to require subsumes in both directions.  What's up
> here?

Subsumption is essentially computing an implication between
constraints, so that if P subsumes Q, you could also say that P => Q.
Checking in both directions gives P => Q and Q => P, or P <=> Q, which
is logical equivalence.

I think that the absence of constraints fits into those definition
nicely, since it represents the empty set of propositions.

>> +  // Find the template parameter list at the a depth appropriate to
>> +  // the scope we're trying to enter.
>> +  tree parms = current_template_parms;
>> +  int depth = template_class_depth (type);
>> +  for (int n = processing_template_decl; n > depth && parms; --n)
>> +parms = TREE_CHAIN (parms);
>
>
> If you're going to use this function from lookup_template_class_1, it can't
> use current_template_*, since those are parser state which might be
> something completely unrelated when we get here during instantiation.

I was worried about that. I'm not sure how this gets invoked during
instantiation. I'll look at it.

-- 
Andrew Sutton
andrew.n.sut...@gmail.com

Re: [C++ Patch] PR 24926

[Paolo Carlini]

Hi,

On 09/04/2013 03:11 PM, Jason Merrill wrote:
It looks to me like this will result in duplicate diagnostics for 
invalid members in a nested anonymous union.  Maybe make the recursive 
part only handle access setting?
Indeed. I think the recursive part already does that, because only the 
first time is called complain == true (thus does exactly what the 
current code does), then when the recursion proper starts, complain == 
false. Or you mean something else?


Paolo.

[PATCH, committed] Skip undefined weak in testsuite on AIX

[David Edelsohn]

AIX does not support undefined weak. Skip those tests in the testsuite.

Also, select COFF as AIX file format without requiring objdump.

* gcc.dg/attr-weakref-1.c: Skip on AIX.
* gcc.dg/torture/pr53922.c: Skip on AIX.
* lib/file-format.exp (gcc_target_object_format): AIX is COFF.

Bootstrapped on powerpc-ibm-aix7.1.0.0.

Thanks, David

Index: gcc.dg/attr-weakref-1.c
===
--- gcc.dg/attr-weakref-1.c (revision 202260)
+++ gcc.dg/attr-weakref-1.c (working copy)
@@ -4,7 +4,7 @@
 // This test requires support for undefined weak symbols.  This support
 // is not available on hppa*-*-hpux*.  The test is skipped rather than
 // xfailed to suppress the warning that would otherwise arise.
-// { dg-skip-if "" { "*-*-darwin*" "hppa*-*-hpux*" } "*" { "" } }
+// { dg-skip-if "" { "*-*-darwin*" "hppa*-*-hpux*" "*-*-aix*" } "*" { "" } }
 // For kernel modules and static RTPs, the loader treats undefined weak
 // symbols in the same way as undefined strong symbols.  The test
 // therefore fails to load, so skip it.Index: gcc.dg/torture/pr53922.c
===
--- gcc.dg/torture/pr53922.c(revision 202260)
+++ gcc.dg/torture/pr53922.c(working copy)
@@ -1,6 +1,7 @@
 /* { dg-do run } */
 /* { dg-require-weak "" } */
 /* { dg-skip-if "No undefined" { *-*-mingw* } { "*" } { "" } } */
+/* { dg-skip-if "No undefined weak" { *-*-aix* } { "*" } { "" } } */
 /* { dg-skip-if "No undefined weak" { hppa*-*-hpux* && { ! lp64 } } {
"*" } { "" } } */
 /* { dg-options "-Wl,-undefined,dynamic_lookup" { target *-*-darwin* } } */

Index: lib/file-format.exp
===
--- lib/file-format.exp (revision 202260)
+++ lib/file-format.exp (working copy)
@@ -38,6 +38,9 @@
} else {
  set gcc_target_object_format_saved som
}
+} elseif { [istarget *-*-aix*] } {
+   # AIX doesn't necessarily have objdump, so hand-code it.
+   set gcc_target_object_format_saved coff
 } else {
 set objdump_name [find_binutils_prog objdump]
 set open_file [open objfmtst.c w]

Re: [stdc++] Mark global new/delete operators as weak symbols

[Romain Geissler]

Ping

2013/8/20 Romain Geissler :
> Hi,
>
> According to the C++ standard §17.4.3.4(2,3) the global new and delete
> operator implementation can be replaced by the program. For example,
> tcmalloc does redefine them. On ELF targets, it is currently OK if you
> link libstc++ dynamically. However, if you try to link both tcmalloc
> and stdc++ statically, then the linker will complain about multiple
> symbol definition.
>
> This patch fixes this by marking the global new/delete operator as
> weaks for linux. I may be extended to all ELF targets. Bootstrapped
> and tested on SLES 11 SP1 x86_64.
>
> Cheers,
> Romain
>
> /libstc++-v3/
>
> 2013-08-20  Romain Geissler  
>
> * config/os/gnu-linux/os_defines.h: (_GLIBCXX_WEAK_DEFINITION): Define.

Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy

[Mike Stump]

On Sep 4, 2013, at 7:09 AM, Jan Hubicka  wrote:
> Making gengtype to generate ggc_mark for each type would make hand writting
> easier - you can use C++ overloading instead of trying to guess the funny
> names gengtype uses right now.
> But that is independent of this change.  I am slowly getting used to the
> world of hand written gengtype markings.

I'd rather plugin generated code that applies general rules to the transitive 
closure of all types needed.  The markings would be for exceptions to those 
rules, which should be very, very rare.  The load for someone writing and 
maintaining code is then reduced and the fear of GTY doesn't play a role in 
activities such as refactoring, as there would be nothing to do.  I know it, 
and I have a healthy fear of it.  :-)

RE: [patch 4/4] -fstrict-volatile-bitfields cleanup v3: remove from defaults on all targets

[Joseph S. Myers]

On Wed, 4 Sep 2013, Bernd Edlinger wrote:

> How about this for a compromise: Let's make the
> default of -fstrict-volatile-bitfields an optional
> configure argument for gcc 4.9, that can be used for every
> target, even for X86_64 of you want ?

I think it's generally a bad idea for semantic options, such as --param 
allow-store-data-races=1, to depend on configure options like that (as 
opposed to non-semantic options such as what architecture variant is 
optimized for by default).

Of course, *if* the user passes --param allow-store-data-races=1, then 
-fstrict-volatile-bitfields becomes non-semantic at the C/C++ language 
level and it's reasonable for an architecture to default it to on.

Any configure option related to this should maybe be a stronger version of 
--disable-threads, meaning that not only is no thread library support to 
be included in any of GCC's libraries but that the compiler will never be 
used to build any code for which any concurrency or data race issues arise 
(including through signals) and so --param allow-store-data-races=1 can be 
the default (along with any target-specific consequences of that default).  
Call it --enable-data-races and require it to be used along with 
--disable-threads, or something like that.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH] manage dom-walk_data initialization and finalization with constructors and destructors

[Trevor Saunders]

On Wed, Sep 04, 2013 at 10:32:17AM +0200, Richard Biener wrote:
> On Wed, Sep 4, 2013 at 5:16 AM,   wrote:
> > From: Trevor Saunders 
> >
> > bootstrapped on x86_64-unknown-linux-gnu with same test results as 
> > unpatched r202185 ok?
> 
> That looks like a not too useful part-C++-ification of domwalk.  A proper
> C++-ification would include the use of functors and possibly templating it
> on the user specific data type.

 It's true this patch isn't that aggressive, I was thinking about going
 farther incrementally.  I also haven't quiet convinced my self inlining
 all of domwalk.c or adding a whole bunch of explicit template
 instantiations to domwalk.c was really a good idea.  Are you saying you
 believe the removal of the indirect calls is worth the extra code size?

> So, what do you think is the advantage (other than dropping two or three lines
> of code in the users)?

I'd say its the same as for all other raii things, you don't have to be
sure to free data when you're done with it, and imho reducing boiler
plate is a fairly good thing on its own.

Trev

> 
> Richard.
> 
> >
> > * compare-elim.c (find_comparisons_in_bb): adjust
> > * domwalk.c (init_walk_dominator_tree): Convert to dom_walk_data 
> > constructor.
> >   (fini_walk_dominator_tree): Convert to dom_walk_data destructor.
> > * domwalk.h (dom_walk_data::dom_walk_data): declare
> >   (dom_walk_data::~dom_walk_data): declare
> >   (init_walk_dominator_tree): remove
> >   (fini_walk_dominator_tree): remove
> > * fwprop.c (build_single_def_use_links): adjust
> > * gimple-ssa-strength-reduction.c (execute_strength_reduction): adjust
> > * graphite-sese-to-poly.c (build_sese_conditions_after): adjust
> > * tree-into-ssa.c (rewrite_blocks): adjust
> >   (mark_def_site_blocks): adjust
> > * tree-ssa-dom.c (tree_ssa_dominator_optimize): adjust
> > * tree-ssa-dse.c (tree_ssa_dse): adjust
> > * tree-ssa-loop-im.c (determine_invariantness): adjust
> >   (move_computations): adjust
> > * tree-ssa-phiopt.c (get_non_trapping): adjust
> > * tree-ssa-pre.c (eliminate): adjust
> > * tree-ssa-strlen.c (tree_ssa_strlen): adjust
> > * tree-ssa-uncprop.c (tree_ssa_uncprop): adjust
> >
> > diff --git a/gcc/compare-elim.c b/gcc/compare-elim.c
> > index e907376..8a1e68f 100644
> > --- a/gcc/compare-elim.c
> > +++ b/gcc/compare-elim.c
> > @@ -403,17 +403,13 @@ find_comparisons_in_bb (struct dom_walk_data *data 
> > ATTRIBUTE_UNUSED,
> >  static void
> >  find_comparisons (void)
> >  {
> > -  struct dom_walk_data data;
> > +  struct dom_walk_data data (CDI_DOMINATORS);
> >
> > -  memset (&data, 0, sizeof(data));
> > -  data.dom_direction = CDI_DOMINATORS;
> >data.before_dom_children = find_comparisons_in_bb;
> >
> >calculate_dominance_info (CDI_DOMINATORS);
> >
> > -  init_walk_dominator_tree (&data);
> >walk_dominator_tree (&data, ENTRY_BLOCK_PTR);
> > -  fini_walk_dominator_tree (&data);
> >
> >clear_aux_for_blocks ();
> >free_dominance_info (CDI_DOMINATORS);
> > diff --git a/gcc/domwalk.c b/gcc/domwalk.c
> > index 8c1ddc6..bc7a93f 100644
> > --- a/gcc/domwalk.c
> > +++ b/gcc/domwalk.c
> > @@ -261,22 +261,26 @@ walk_dominator_tree (struct dom_walk_data *walk_data, 
> > basic_block bb)
> >free (worklist);
> >  }
> >
> > -void
> > -init_walk_dominator_tree (struct dom_walk_data *walk_data)
> > +dom_walk_data::dom_walk_data (cdi_direction direction)
> > +  : dom_direction(direction),
> > +  initialize_block_local_data (NULL),
> > +before_dom_children (NULL),
> > +  after_dom_children (NULL),
> > +  global_data (NULL),
> > +block_local_data_size (0)
> >  {
> > -  walk_data->free_block_data.create (0);
> > -  walk_data->block_data_stack.create (0);
> > +  free_block_data.create (0);
> > +  block_data_stack.create (0);
> >  }
> >
> > -void
> > -fini_walk_dominator_tree (struct dom_walk_data *walk_data)
> > +dom_walk_data::~dom_walk_data ()
> >  {
> > -  if (walk_data->initialize_block_local_data)
> > +  if (initialize_block_local_data)
> >  {
> > -  while (walk_data->free_block_data.length () > 0)
> > -   free (walk_data->free_block_data.pop ());
> > +  while (free_block_data.length () > 0)
> > +   free (free_block_data.pop ());
> >  }
> >
> > -  walk_data->free_block_data.release ();
> > -  walk_data->block_data_stack.release ();
> > +  free_block_data.release ();
> > +  block_data_stack.release ();
> >  }
> > diff --git a/gcc/domwalk.h b/gcc/domwalk.h
> > index 54b7f3c..94986c7 100644
> > --- a/gcc/domwalk.h
> > +++ b/gcc/domwalk.h
> > @@ -26,6 +26,10 @@ typedef void *void_p;
> >
> >  struct dom_walk_data
> >  {
> > +public:
> > +  dom_walk_data (cdi_direction);
> > +  ~dom_walk_data ();
> > +
> >/* This is the direction of the dominator tree we want to walk.  i.e.,
> >   if it is set to CDI_DOMINATORS, then we walk the dominator tree,
> >   if it is set to CDI_POST_DOMINATORS, then we walk the post
> > @@ -70,5 +74,3 @@ struct dom_walk_data
> >  };
> >
> >  void walk_dominator_tree (struct dom_walk_

RE: [PATCH] PR58143/58227 wrong code at -O3

[Bernd Edlinger]

On Tue, 3 Sep 2013 12:31:50, Richard Biener wrote:
> On Fri, Aug 30, 2013 at 6:43 PM, Bernd Edlinger
>  wrote:
>> Now I think this is good opportunity for a simple heuristic:
>>
>> If a statement is at loop level 1 we can move it out of the loop,
>> regardless of the fact, that it may invoke undefined behavior, because if it 
>> is
>> moved then out of any loops, and thus it cannot be an induction variable and
>> cause problems with aggressive loop optimizations later.
>
> VRP can still cause wrong-code issues (it's probably hard to generate a
> testcase though). Also a less conservative check would be to see
> whether we hoist _into_ loop level 0 (though we cannot check that at
> the point where you placed the check).

Well, then I should better revert this heuristic again.
 
>> Only statements with possible undefined behavior in nested loops can become
>> induction variable if lim moves them from one loop into an outer loop.
>>
>> Therefore with extremely much luck the test case will pass unmodified.
>> I tried it, and the patch passes bootstrap and causes zero regressions
>> with this heuristic.
>>
>> Ok for trunk now?
>
> Jakub mentioned another possibility - make sure the moved expression
> does not invoke undefined behavior by computing in an unsigned type.

That is a possibility, but on the other hand, that would obscure the undefined
behavior and thus prevent other possible optimizations later.

Another possibility would be to move the statement together with the
enclosing if-statement, thus really preserving the execution.
 
> That said, for the sake of backporting we need a patch as simple as
> possible - so it would be interesting to see whether the patch without
> the loop 1 heuristic has any effect on say SPEC CPU 2006 performance.

I do not have access to that test, but on the dhrystone benchmark this patch
has no influence whatsoever.

Attached you'll find the latest version of my patch without the heuristic.
Bootstrapped on i686-pc-linux-gnu and regression tested again.

Ok for trunk and 4.8 branch?  2013-09-03  Bernd Edlinger  

PR tree-optimization/58143
PR tree-optimization/58227
* gimple.c (gimple_could_trap_p_1): Rename to ...
(gimple_could_trap_or_invoke_undefined_p): ... this
and add new parameter include_undefined.
(gimple_could_trap_or_invoke_undefined_p): Adjust.
(gimple_assign_rhs_could_trap_p): Adjust.
* gimple.h (gimple_could_trap_p_1): Renamed.
* tree-if-conv.c (ifcvt_could_trap_p): Adjust.
* tree-ssa-loop-im.c (movement_possibility): Prevent
lim from moving conditional expressions if that could
trigger undefined behavior.


testsuite/

* gcc.dg/pr58143.c: New test.
* gcc.target/i386/pr53397-1.c: Adjust.
* gcc.target/i386/pr53397-2.c: Adjust.



patch-lim.diff
Description: Binary data

Re: [stdc++] Mark global new/delete operators as weak symbols

[Mike Stump]

On Sep 4, 2013, at 7:30 AM, Romain Geissler  wrote:
>> 2013-08-20  Romain Geissler  
>> 
>>* config/os/gnu-linux/os_defines.h: (_GLIBCXX_WEAK_DEFINITION): Define.

Strikes me as wrong.  Using weak should be autoconfed or driven by the compiler 
and then respected by the library.  With that change, all targets, wether they 
are linux or not, then work.  The above fixes one target, at the expense of not 
fixing any other targets.

Re: [patch, bz #58312] Fix libssp handling of vsnprintf for cross-compilers

[Brooks Moses]

On Wed, Sep 4, 2013 at 12:01 AM, Jakub Jelinek  wrote:
> That looks wrong, the test was intentionally looking for badly implemented
> vsnprintf, see
> http://www.gnu.org/software/gnulib/manual/html_node/snprintf.html
> "This function does not return a byte count as specified in C99 on some
> platforms: HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 9, mingw."
>
> The implementation relies on the returned byte count to be exactly correct,
> so it can't be implemented on platforms where that is not the case.

Ah; thanks for the pointer.

> Not sure which of the targets from the above list we still support,
> certainly at least mingw, though in that case I don't know if it hasn't been
> fixed there.  So, as Joseph said, you probably should keep the runtime test
> as is, and just for cross compiling replace the unconditional =undef with
> an optimistic assumption followed by a blacklist of platforms where it is
> known not to work.

That makes sense.  Am I correct in understanding that the list I need
to be concerned with is only the one you quote?

I have access to a mingw compiler, so I can easily give that a test in
a current version.

The glibc manual has a list of supported/tested platforms, and all of
the above other than mingw are listed as "occasionally tested" as of
2011, so they should probably all be listed in the blacklist:
https://www.gnu.org/software/gnulib/manual/html_node/Target-Platforms.html

Thanks,
- Brooks

[PATCH, AArch64] Improve handling of constants destined for FP_REGS

[Ian Bolton]

(This patch supercedes this one:
http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01462.html)

The movdi_aarch64 pattern allows moving a constant into an FP_REG,
but has the constraint Dd, which is stricter than the constraint N for
moving a constant into a CORE_REG.  This is due to restricted values
allowed for MOVI instruction.

Due to the predicate allowing any constant that is valid for the
CORE_REGs, we can run into situations where IRA/reload has decided
to use FP_REGs but the value is not actually valid for MOVI.

This patch makes use of TARGET_PREFERRED_RELOAD_CLASS to ensure that
NO_REGS (which leads to literal pool) is returned, when the immediate
can't be put directly into FP_REGS.

A testcase is included.

Linux regressions all came back good.

OK for trunk?

Cheers,
Ian


2013-09-04  Ian Bolton  

gcc/
* config/aarch64/aarch64.c (aarch64_preferred_reload_class):
Return NO_REGS for immediate that can't be moved directly
into FP_REGS.

testsuite/
* gcc.target/aarch64/movdi_1.c: New test.diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index aed035a..2c07ccf 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -4236,10 +4236,18 @@ aarch64_class_max_nregs (reg_class_t regclass, enum 
machine_mode mode)
 }
 
 static reg_class_t
-aarch64_preferred_reload_class (rtx x ATTRIBUTE_UNUSED, reg_class_t regclass)
+aarch64_preferred_reload_class (rtx x, reg_class_t regclass)
 {
-  return ((regclass == POINTER_REGS || regclass == STACK_REG)
- ? GENERAL_REGS : regclass);
+  if (regclass == POINTER_REGS || regclass == STACK_REG)
+return GENERAL_REGS;
+
+  /* If it's an integer immediate that MOVI can't handle, then
+ FP_REGS is not an option, so we return NO_REGS instead.  */
+  if (CONST_INT_P (x) && reg_class_subset_p (regclass, FP_REGS)
+  && !aarch64_simd_imm_scalar_p (x, GET_MODE (x)))
+return NO_REGS;
+
+  return regclass;
 }
 
 void
diff --git a/gcc/testsuite/gcc.target/aarch64/movdi_1.c 
b/gcc/testsuite/gcc.target/aarch64/movdi_1.c
new file mode 100644
index 000..a22378d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/movdi_1.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-inline" } */
+
+#include 
+
+void
+foo1 (uint64_t *a)
+{
+  uint64x1_t val18;
+  uint32x2_t val19;
+  uint64x1_t val20;
+  val19 = vcreate_u32 (0x80004cf3dffbUL);
+  val20 = vrsra_n_u64 (val18, vreinterpret_u64_u32 (val19), 34);
+  vst1_u64 (a, val20);
+}
+
+void
+foo2 (uint64_t *a)
+{
+  uint64x1_t val18;
+  uint32x2_t val19;
+  uint64x1_t val20;
+  val19 = vcreate_u32 (0xdffbUL);
+  val20 = vrsra_n_u64 (val18, vreinterpret_u64_u32 (val19), 34);
+  vst1_u64 (a, val20);
+}

Fix long lines in cgraphunit.c

[Jan Hubicka]

Hi,
as Michael pointed out, I introduced long lines primarily because of large
indentation in cgraph_analyze_function.  It makes the code to look better
if the polymorphic call logic in broken out.

Bootstrapping/regtesting x86_64-linux, will commit it once testing conlcude.
Honza
* cgraphunit.c (walk_polymorphic_call_targets): Break out from...
(cgraph_analyze_functions): ... here.
Index: cgraphunit.c
===
--- cgraphunit.c(revision 202199)
+++ cgraphunit.c(working copy)
@@ -821,7 +821,77 @@
 varpool_assemble_decl (node);
 }
 
+/* EDGE is an polymorphic call.  Mark all possible targets as reachable
+   and if there is only one target, perform trivial devirtualization. 
+   REACHABLE_CALL_TARGETS collects target lists we already walked to
+   avoid udplicate work.  */
 
+static void
+walk_polymorphic_call_targets (pointer_set_t *reachable_call_targets,
+  struct cgraph_edge *edge)
+{
+  unsigned int i;
+  void *cache_token;
+  bool final;
+  vec targets
+= possible_polymorphic_call_targets
+   (edge, &final, &cache_token);
+
+  if (!pointer_set_insert (reachable_call_targets,
+  cache_token))
+{
+  if (cgraph_dump_file)
+   dump_possible_polymorphic_call_targets 
+ (cgraph_dump_file, edge);
+
+  for (i = 0; i < targets.length(); i++)
+   {
+ /* Do not bother to mark virtual methods in anonymous namespace;
+either we will find use of virtual table defining it, or it is
+unused.  */
+ if (targets[i]->symbol.definition
+ && TREE_CODE
+ (TREE_TYPE (targets[i]->symbol.decl))
+  == METHOD_TYPE
+ && !type_in_anonymous_namespace_p
+  (method_class_type
+(TREE_TYPE (targets[i]->symbol.decl
+ enqueue_node ((symtab_node) targets[i]);
+   }
+}
+
+  /* Very trivial devirtualization; when the type is
+ final or anonymous (so we know all its derivation)
+ and there is only one possible virtual call target,
+ make the edge direct.  */
+  if (final)
+{
+  gcc_assert (targets.length());
+  if (targets.length() == 1)
+   {
+ if (cgraph_dump_file)
+   {
+ fprintf (cgraph_dump_file,
+  "Devirtualizing call: ");
+ print_gimple_stmt (cgraph_dump_file,
+edge->call_stmt, 0,
+TDF_SLIM);
+   }
+ cgraph_make_edge_direct (edge, targets[0]);
+ cgraph_redirect_edge_call_stmt_to_callee (edge);
+ if (cgraph_dump_file)
+   {
+ fprintf (cgraph_dump_file,
+  "Devirtualized as: ");
+ print_gimple_stmt (cgraph_dump_file,
+edge->call_stmt, 0,
+TDF_SLIM);
+   }
+   }
+}
+}
+
+
 /* Discover all functions and variables that are trivially needed, analyze
them as well as all functions and variables referred by them  */
 
@@ -923,71 +993,13 @@
  if (optimize && flag_devirtualize)
{
  struct cgraph_edge *next;
- for (edge = cnode->indirect_calls; edge; edge = next)
+
+ for (edge = cnode->indirect_calls; edge; edge = next)
{
  next = edge->next_callee;
  if (edge->indirect_info->polymorphic)
-   {
- unsigned int i;
- void *cache_token;
- bool final;
- vec targets
-   = possible_polymorphic_call_targets
-   (edge, &final, &cache_token);
-
- if (!pointer_set_insert (reachable_call_targets,
-  cache_token))
-   {
- if (cgraph_dump_file)
-   dump_possible_polymorphic_call_targets 
- (cgraph_dump_file, edge);
-
- for (i = 0; i < targets.length(); i++)
-   {
- /* Do not bother to mark virtual methods in 
anonymous namespace;
-either we will find use of virtual table 
defining it, or it is
-unused.  */
- if (targets[i]->symbol.definition
- && TREE_CODE
- (TREE_TYPE (targets[i]->symbol.decl))
-  == METHOD_TYPE
- && !type_in_anonymous_namespace_p
-  (method_cla

Re: [PATCH 6/6] Add manual GTY hooks

[David Malcolm]

On Sat, 2013-08-31 at 19:27 +0200, Richard Biener wrote:
> David Malcolm  wrote:
> >On Fri, 2013-08-30 at 10:09 +0200, Richard Biener wrote:
> >> On Thu, Aug 29, 2013 at 9:44 PM, Steven Bosscher
> > wrote:
> >> > On Thu, Aug 29, 2013 at 6:20 PM, David Malcolm
> > wrote:
> >> >> * gimple.c (gt_ggc_mx (gimple)): New, as required by
> >GTY((user)).
> >> >> (gt_pch_nx (gimple)): Likewise.
> >> >
> >> > GIMPLE isn't supposed to end up in a PCH. Can you please make this
> >> > function simply call gcc_unreachable()?
> >> >
> >> > FWIW 1: I really think all these hand-written markers aren't a good
> >> > idea, we should really figure out a way to have automatic marker
> >> > function generators, something less complex than gengtype, of
> >course.
> >> > But to have all these calls to the type-mangled marker functions
> >> > (gt_pch_n_9tree_node, etc.) is going to be a problem in the long
> >term.
> >> >
> >> > It seems to me that the first step in all these conversions to
> >> > hand-written markers should be to make gengtype spit out the marker
> >> > functions *without* the type name mangling, i.e. all marker
> >functions
> >> > should just be gt_ggc_mx(type) / gt_pch_nx(type).
> >> 
> >> Yes, the original idea was that gengtype would do that.  For things
> >we like
> >> to optimize the GTY((user)) thing would tell it that we do provide
> >the markers.
> >> Like when you want to look through a non-GCed intermediate object. 
> >Or
> >> for things like GTY((chain)) stuff that doesn't really work if you
> >have multiple
> >> chains (without clever GTY((skip))s...).
> >> 
> >> The lack of the unmangled overloads is annoying :/  IIRC Diego
> >halfway completed
> >> the transition to unmangled overloads / specializations?
> >
> >How would that work, and is that something that it would be productive
> >for me to work on?
> >
> >Currently AIUI gtype-desc.h contains mangled macros and decls e.g.:
> >  extern void gt_ggc_mx_rtvec_def (void *);
> >  #define gt_ggc_m_9rtvec_def(X) do { \
> >if (X != NULL) gt_ggc_mx_rtvec_def (X);\
> >} while (0)
> >
> >and the corresponding functions in gtype-desc.c contain:
> >
> >void
> >gt_ggc_mx_rtvec_def (void *x_p)
> >{
> >  struct rtvec_def * const x = (struct rtvec_def *)x_p;
> >  if (ggc_test_and_set_mark (x))
> >{
> > /* visit fields of x, invoking the macros */
> >}
> >}
> >
> >Is the goal for the field-visiting code to all be able to simply do:
> >   gt_ggc_mx (field)
> 
> Yes. The advantage is that gengtype this way only needs to parse field names 
> and not types which is a lot easier.
> 
> >and have overloading resolve it all? (and handle e.g. chain_next etc
> >etc)
> 
> Yes. All bits that would require more complex parsing should instead be 
> telled explicit via a GTY annotation.
> 
> >Presumably if this were implemented, then hand-written GTY functions
> >would be that much easier to maintain, and perhaps this gimple
> >conversion idea would be more acceptable?  (or, in other words, should
> >I
> >work on this?)
> 
> That would be very nice! IIRC Diego had issues with making all declarations 
> visible to make this work. Diego?

An update: I've been trying this, but I'm running into what may be a
blocker: types need to be visible in the declaration of the marker
function.  Doing this in gtype-desc.h runs into a tangle of dependency
issues.  For example:

  ./gtype-desc.h:2842:28: error: ‘ivarref_entry’ was not declared in this scope
  extern void gt_ggc_mx (vec *p);
  ^
In the extreme case, the underlying types in question might be only
declared in one specific .c file.  In other cases, there's likely to be
some set of header files needed to get at the types.  Also, vec.h uses
ggc.h, which uses gtype-desc.h, and so we have a loop...

AIUI, the mangled names of types in the function names are doing the job
of hiding all the types in gtype-desc.h, so that they are only needed in
the various gt-*.h files and in gtype-desc.c.  By changing things so
that the marker function declarations refer to the types directly, all
types need to be known to users of such functions.  I'm not sure that
that's possible - or, at least, not if the declarations are in a shared
header file.

Perhaps it could work with some scheme in which the types are
partitioned by .h/.c file, and copies of forward declarations are added
only as-needed to the appropriate gt-*.h/gtype-desc.c files?

I'm trying this now.

BTW, what is the etymology of "gt_ggc_mx" and "gt_pch_nx", and how do
people pronounce them? (I call them the "object-marking" and
"object-noting" hooks" fwiw).  As far as I can see, the "m" and and the
"n" come from "mark" and "note"; it's not clear to me where the trailing
"x" came from.

Dave

Re: [RFC] Fix for PR58201

[Bernd Schmidt]

On 09/04/2013 06:04 PM, Jan Hubicka wrote:
> this is third fallout of my change to remove DECL_ARGUMENTS/DECL_RESULT for 
> functions w/o
> bodies I did not really anticipate.
[...]
> I would like to basically ask if it seems to make sense to go this route and
> try to get rid of those declarations.

I'm currently working on a new target, ptx, which uses a
pseudo-assembler where functions (even extern ones) need to be declared
with their arguments and return types. With my current code I have to
look at DECL_ARGUMENTS fairly late in the compilation. I'm not quite
sure yet whether the change to delete them will break the backend.


Bernd

Re: [patch, bz #58312] Fix libssp handling of vsnprintf for cross-compilers

[Jakub Jelinek]

On Wed, Sep 04, 2013 at 09:41:20AM -0700, Brooks Moses wrote:
> > Not sure which of the targets from the above list we still support,
> > certainly at least mingw, though in that case I don't know if it hasn't been
> > fixed there.  So, as Joseph said, you probably should keep the runtime test
> > as is, and just for cross compiling replace the unconditional =undef with
> > an optimistic assumption followed by a blacklist of platforms where it is
> > known not to work.
> 
> That makes sense.  Am I correct in understanding that the list I need
> to be concerned with is only the one you quote?

I don't know.  I only have access to linux targets, where glibc thankfully
does the right thing.  Guess you should ask the maintainers of the targets
mentioned in that list, perhaps ask about other C libraries.  Ask also
people with non-glibc linux systems, like uclibc, bionic, musl, ...

Jakub

Re: [RFC] Fix for PR58201

[Jan Hubicka]

> On 09/04/2013 06:04 PM, Jan Hubicka wrote:
> > this is third fallout of my change to remove DECL_ARGUMENTS/DECL_RESULT for 
> > functions w/o
> > bodies I did not really anticipate.
> [...]
> > I would like to basically ask if it seems to make sense to go this route and
> > try to get rid of those declarations.
> 
> I'm currently working on a new target, ptx, which uses a
> pseudo-assembler where functions (even extern ones) need to be declared
> with their arguments and return types. With my current code I have to
> look at DECL_ARGUMENTS fairly late in the compilation. I'm not quite
> sure yet whether the change to delete them will break the backend.

How do you support K&R functions here?  My basic idea was that TYPE_ARG_TYPES
should give enough information about external function calling convention
anyone will ever need. I would hope that this will be sufficient for your
use, too, despite the fact you no longer have parameter names at hand
and you also lose info about external inline K&R-style delcared functions
that has been optimized out.

If not that indeed, you will not see DECL_ARGUMENTS for external function
anytime after cgraph_remove_unreachable_functions is called.

Honza

Re: [c++-concepts] Class template constraints

[Jason Merrill]

On 09/04/2013 11:59 AM, Andrew Sutton wrote:

It's not supposed to be different. Checking constraints in
instantiate_template is actually too late. We want to check before
instantiation, at the point of use.


Right, what I was getting at is that instantiate_template actually only 
instantiates the declaration of a function, not the definition, so it 
corresponds to lookup_template_class for class templates.


instantiate_decl is what actually instantiates the body.  Confusing 
internal function naming.



I think that the absence of constraints fits into those definition
nicely, since it represents the empty set of propositions.


Oh, did the comment just mean that absence is equivalent to absence?  I 
thought the comment was saying that absence is considered equivalent to 
anything else.  Just tweak the comment, then.


Jason

Re: [PATCH, C++, PR58282] Handle noexcept on transactions with -fno-exceptions

[Jason Merrill]

On 09/03/2013 06:03 AM, Tom de Vries wrote:

* semantics.c (finish_transaction_stmt, build_transaction_expr): Handle
flag_exceptions.


I'd rather handle this at a lower level, by making 
build_must_not_throw_expr return its argument if -fno-exceptions.


Jason

Re: [RFC] Fix for PR58201

[Bernd Schmidt]

On 09/04/2013 07:09 PM, Jan Hubicka wrote:
> How do you support K&R functions here?  My basic idea was that TYPE_ARG_TYPES
> should give enough information about external function calling convention
> anyone will ever need. I would hope that this will be sufficient for your
> use, too, despite the fact you no longer have parameter names at hand
> and you also lose info about external inline K&R-style delcared functions
> that has been optimized out.

TYPE_ARG_TYPES doesn't exist for all functions, so right now the backend
is using whichever of the two is available. It seems that TYPE_ARG_TYPES
is actually NULL for K&R functions (gcc.c-torture/compile/2403-1.c
is the first one that fails if I try to use only TYPE_ARG_TYPES).


Bernd

Re: [C++ Patch] PR 24926

[Jason Merrill]

On 09/04/2013 10:42 AM, Paolo Carlini wrote:

Indeed. I think the recursive part already does that, because only the
first time is called complain == true (thus does exactly what the
current code does), then when the recursion proper starts, complain ==
false.


Ah yes, I see.


Or you mean something else?


I was thinking that the recursive part could be a simple loop to set 
access, but your way is fine too.


Jason

Re: [C++ Patch] PR 24926

[Paolo Carlini]

Hi,

>> Or you mean something else?
>
>I was thinking that the recursive part could be a simple loop to set
>access, but your way is fine too.

Ok, great. Note, before committing I mean to also simplify it a bit, the 
TREE_STATIC check of the recursive part has no reason to exist, doesn't exist 
in the current code handling the fields of an anon aggr. All in all I rather 
prefer this kind of solution, in my opinion the typical recursion here is very 
shallow and in this way the amount of new code is minimized.

Thanks,
Paolo

Re: [c++-concepts] Class template constraints

[Andrew Sutton]

>> It's not supposed to be different. Checking constraints in
>> instantiate_template is actually too late. We want to check before
>> instantiation, at the point of use.
>
> Right, what I was getting at is that instantiate_template actually only
> instantiates the declaration of a function, not the definition, so it
> corresponds to lookup_template_class for class templates.

Ah. The goal is to check after we've deduced/coerced template
arguments into a valid substitution. With functions, that's in
fn_type_unification (hopefully called from instantiate_template), and
for classes in lookup_template_class.

There are some other places too: get_class_bindings for partial
specializations, and determine_specialization for explicit
specializations.


> Oh, did the comment just mean that absence is equivalent to absence?  I
> thought the comment was saying that absence is considered equivalent to
> anything else.  Just tweak the comment, then.

Sounds good.

Andrew

Re: Fix PR middle-end/57370

[Easwaran Raman]

Submitted the patch (r202262) without the insert_stmt_after hunk. Also
fixed nits pointed out by Jakub.

- Easwaran

On Mon, Sep 2, 2013 at 2:31 AM, Richard Biener
 wrote:
> On Fri, Aug 30, 2013 at 6:13 PM, Easwaran Raman  wrote:
>> On Fri, Aug 30, 2013 at 1:25 AM, Richard Biener
>>  wrote:
>>> On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman  wrote:
 Richard,
 Do you want me to commit everything but the  insert_stmt_after hunk
 now?
>>>
>>> Yes.
>>>
 There are more similar failures reported in
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached
 the updated patch there. Shall I send that for review? Apart from the
 debug statement issue, almost all the bugs are due to dependence
 violation because certain newly inserted statements do not have the
 right UID. Instead of trying to catch all of them, will it be better
 if I check if the stmt has a proper uid (non-zero if it is not the
 first stmt) and assign a sensible value at the point where it is used
 (find_insert_point and appears_later_in_bb) instead of where the stmt
 is created? I think that would be less brittle.
>>>
>>> In the end all this placement stuff should be reverted and done as
>>> post-processing after reassoc is finished.  You can remember the
>>> inserted stmts that are candidates for moving (just set a pass-local
>>> flag on them) and assign UIDs for the whole function after all stmts
>>> are inserted.
>>
>> The problem is we need sane UID values as reassoc is happening - not
>> after it is finished. As it process each stmt in reassoc_bb, it might
>> generate new stmts which might have to be compared in
>> find_insert_point or appears_later_in_bb.
>
> But if you no longer find_insert_point during reassoc but instead do
> a "scheduling" pass after it finished you won't need sane UIDs during
> reassoc.
>
> Richard.
>
>> - Easwaran
>>
>>> Richard.
>>>
>>>
 - Easwaran



 On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
  wrote:
> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman  wrote:
>> I have a new patch that supersedes this. The new patch also fixes PR
>> tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
>> no test regression on x86_64/linux. Ok for trunk?
>>
>> 2013-07-31  Easwaran Raman  
>>
>> PR middle-end/57370
>> * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and 
>> reset
>> of debug statements that cause inconsistent IR.
>
> Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
> at all.  The other hunks are ok, but we need to work harder to preserve
> debug stmts - simply removing all is not going to fly.
>
> Richard.
>
>>
>> testsuite/ChangeLog:
>> 2013-07-31  Easwaran Raman  
>>
>> PR middle-end/57370
>> PR tree-optimization/57393
>> PR tree-optimization/58011
>> * gfortran.dg/reassoc_12.f90: New testcase.
>> * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
>> * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
>>
>>
>> On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman  
>> wrote:
>>> Ping.
>>>
>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman  
>>> wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID 
 of a phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options "-O2 -ffast-math" }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, &
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, &
 +   e_ndrho_ndrho_rho
 +  DO

Re: [PATCH] [lambda] Extract lambda functions from semantics.c.

[Adam Butcher]

On 04.09.2013 03:41, Gabriel Dos Reis wrote:
On Tue, Sep 3, 2013 at 9:33 PM, Mike Stump  
wrote:
On Jul 12, 2013, at 11:18 PM, Adam Butcher  
wrote:

  * gcc/cp/semantics.c (build_lambda_expr),
  (build_lambda_object), (begin_lambda_type), 
(lambda_return_type),
  (lambda_function), (lambda_capture_field_type), 
(is_capture_proxy),

  (is_normal_capture_proxy), (insert_capture_proxy),
  (insert_pending_capture_proxies), (lambda_proxy_type),
  (build_capture_proxy), (vla_capture_type),
  (register_capture_members), (add_default_capture),
  (lambda_expr_this_capture), (maybe_resolve_dummy),
  (nonlambda_method_basetype), (maybe_add_lambda_conv_op) and
  (is_lambda_ignored_entity): Moved definitions into ...
  * gcc/cp/lambda.c: ... this new file.



This can cause an incremental build failure because there are no 
dependencies:


diff --git a/gcc/cp/Make-lang.in b/gcc/cp/Make-lang.in
index 2c1774f..65dfe08 100644
--- a/gcc/cp/Make-lang.in
+++ b/gcc/cp/Make-lang.in
@@ -351,6 +351,7 @@ cp/vtable-class-hierarchy.o: 
cp/vtable-class-hierarchy.c \
 cp/name-lookup.o: cp/name-lookup.c $(CONFIG_H) $(SYSTEM_H) 
coretypes.h \
$(TM_H) $(CXX_TREE_H) $(TIMEVAR_H) gt-cp-name-lookup.h 
$(PARAMS_H) \

$(DIAGNOSTIC_CORE_H) $(FLAGS_H) debug.h pointer-set.h
+cp/lambda.o: cp/lambda.c $(CXX_TREE_H) $(CGRAPH_H) $(VEC_H) 
$(SYSTEM_H) coretypes.h


 cp/cxx-pretty-print.o: cp/cxx-pretty-print.c $(CXX_PRETTY_PRINT_H) 
\
   $(CONFIG_H) $(SYSTEM_H) $(TM_H) coretypes.h $(CXX_TREE_H) 
tree-pretty-print.h


When tree codes are added or moved, the check is then against the 
wrong number, and this will kill the build.


I'm still looking forward to the day when all the dependancies are 
unceremoniously ripped out, until then...


Ok?


OK.

Eek.  I didn't realize dependencies had to be manually specified.  
That's prompted me to update a more recent patchset I'm working on where 
I've introduced a new header.


Is anyone working on using some use, perhaps filtered, of -MD (or -MDD) 
to generate deps on the fly?  I haven't looked into the GCC makefile 
system in any detail but I assume dependency handling is more complex 
than the standard usage pattern for -MD or I guess someone would have 
done it already.  Or are maintainers worried that auto deps will slow 
the build down too much?  In our team's experience with using -MD the 
overhead is negligible; especially when weighed up against the effort 
required to manually maintain deps.  It just takes make a little longer 
to start actually building anything.

Re: Fwd: [PATCH] Scheduling result adjustment to enable macro-fusion

[Steven Bosscher]

On Wed, Sep 4, 2013 at 10:58 AM, Alexander Monakov wrote:
> Hello,
>
> Could you use the existing facilities instead, such as adjust_priority hook,
> or making the compare-branch insn sequence a SCHED_GROUP?


Or a define_bypass?

Ciao!
Steven

Re: [RFC] Fix for PR58201

[Jeff Law]

On 09/04/2013 10:49 AM, Bernd Schmidt wrote:

On 09/04/2013 06:04 PM, Jan Hubicka wrote:

this is third fallout of my change to remove DECL_ARGUMENTS/DECL_RESULT for 
functions w/o
bodies I did not really anticipate.

[...]

I would like to basically ask if it seems to make sense to go this route and
try to get rid of those declarations.


I'm currently working on a new target, ptx, which uses a
pseudo-assembler where functions (even extern ones) need to be declared
with their arguments and return types. With my current code I have to
look at DECL_ARGUMENTS fairly late in the compilation. I'm not quite
sure yet whether the change to delete them will break the backend.
IIRC the PA had similar requirements as well -- in 
ASM_DECLARE_FUNCTION_NAME we have to peek at DECL_ARGUMENTS so we can 
pass to the assembler & linker which registers hold arguments.


Jeff

Re: [RFC] Fix for PR58201

[Richard Biener]

Jan Hubicka  wrote:
>> On 09/04/2013 06:04 PM, Jan Hubicka wrote:
>> > this is third fallout of my change to remove
>DECL_ARGUMENTS/DECL_RESULT for functions w/o
>> > bodies I did not really anticipate.
>> [...]
>> > I would like to basically ask if it seems to make sense to go this
>route and
>> > try to get rid of those declarations.
>> 
>> I'm currently working on a new target, ptx, which uses a
>> pseudo-assembler where functions (even extern ones) need to be
>declared
>> with their arguments and return types. With my current code I have to
>> look at DECL_ARGUMENTS fairly late in the compilation. I'm not quite
>> sure yet whether the change to delete them will break the backend.
>
>How do you support K&R functions here?  My basic idea was that
>TYPE_ARG_TYPES
>should give enough information about external function calling
>convention
>anyone will ever need. I would hope that this will be sufficient for
>your
>use, too, despite the fact you no longer have parameter names at hand
>and you also lose info about external inline K&R-style delcared
>functions
>that has been optimized out.
>
>If not that indeed, you will not see DECL_ARGUMENTS for external
>function
>anytime after cgraph_remove_unreachable_functions is called.

In fact it has to work because of indirect calls and how we now handle gimple 
call abi via gimple_call_fntype.

Richard.

>Honza

Re: [PATCH i386 2/8] [AVX512] Add mask registers.

[Kirill Yukhin]

Hello,

PING.

--
Thanks, K

Re: Fwd: [PATCH] Scheduling result adjustment to enable macro-fusion

[Alexander Monakov]

On Wed, Sep 4, 2013 at 9:53 PM, Steven Bosscher  wrote:
>
> On Wed, Sep 4, 2013 at 10:58 AM, Alexander Monakov wrote:
> > Hello,
> >
> > Could you use the existing facilities instead, such as adjust_priority hook,
> > or making the compare-branch insn sequence a SCHED_GROUP?
>
>
> Or a define_bypass?

Hm, I don't think define_bypass would work: it still leaves the
scheduler freedom to move the compare up.

IMO adjust_priority would be preferable if it allows to achieve the goal.

Alexander

Re: [PATCH] [lambda] Extract lambda functions from semantics.c.

[Adam Butcher]

On 04.09.2013 20:39, Gabriel Dos Reis wrote:
On Wed, Sep 4, 2013 at 12:55 PM, Adam Butcher  
wrote:


Is anyone working on using some use, perhaps filtered, of -MD (or 
-MDD) to

generate deps on the fly?


See Tom's patch series.

Ah, yes.  Cool.  I guess it's just waiting on approval for each part; I 
note that a few front ends have already been OK'd.

Re: [patch] Make vector::at() assertion message more useful (try #2)

[Daniel Krügler]

2013/9/4 Paul Pluzhnikov :
> Greetings,
>
> This is a followup to:
> http://gcc.gnu.org/ml/libstdc++/2013-08/msg00096.html
>
> Without this patch, the user on vector::at out of bounds sees:
>
> terminate called after throwing an instance of 'std::out_of_range'
>   what():  vector::_M_range_check
> Aborted (core dumped)
>
> With the patch:
>
> terminate called after throwing an instance of 'std::out_of_range'
>   what():  vector::_M_range_check: __n (which is 3) >= this->size() (which is 
> 2)
> Aborted (core dumped)
>
>
> I am not at all sure the names I choose here are good ones. Suggestions
> welcome.
>
> I also shudder at the idea of repeating _M_range_check code in
> e.g. string::at(), and elsewhere. Perhaps we need a snprintf_lite, that
> only understands '%zu' and literal characters, e.g.:

I expect that this kind of index violation is a rather often occurring
pattern and should be isolated. IMO the _M_range
_check now pessimisms the normal, non-violating case. Why not simply
replacing it by something along the lines of

 _M_range_check(size_type __n) const
   {
if (__n >= this->size())
 
__throw_out_of_range(__index_out_of_range_msg(__N("vector::_M_range_check"),
__n, this->size()));
   }

where __index_out_of_range_msg is a reusable function that uses
something like snprintf_lite internally?

- Daniel

Re: [PATCH] Fixing improper conversion from sin() to sinf() in optimization mode.

[Xinliang David Li]

On Wed, Sep 4, 2013 at 1:53 PM, Cong Hou  wrote:
> I have made a new patch according to your comments. I found several
> references saying that the precision 2p+2 is OK for the sqrt
> conversion (one here:
> http://www.cs.berkeley.edu/~fateman/generic/algorithms.pdf). The new
> patch is pasted as below.
>
> Thank you for all the suggestions, Joseph!
>
>
> Cong
>
>
> Index: gcc/testsuite/gcc.c-torture/execute/20030125-1.c
> ===
> --- gcc/testsuite/gcc.c-torture/execute/20030125-1.c (revision 201891)
> +++ gcc/testsuite/gcc.c-torture/execute/20030125-1.c (working copy)
> @@ -44,11 +44,11 @@ __attribute__ ((noinline))
>  double
>  sin(double a)
>  {
> - abort ();
> + return a;
>  }
>  __attribute__ ((noinline))
>  float
>  sinf(float a)
>  {
> - return a;
> + abort ();
>  }
> Index: gcc/convert.c
> ===
> --- gcc/convert.c (revision 201891)
> +++ gcc/convert.c (working copy)
> @@ -135,16 +135,34 @@ convert_to_real (tree type, tree expr)
>CASE_MATHFN (COS)
>CASE_MATHFN (ERF)
>CASE_MATHFN (ERFC)
> -  CASE_MATHFN (FABS)
>CASE_MATHFN (LOG)
>CASE_MATHFN (LOG10)
>CASE_MATHFN (LOG2)
>CASE_MATHFN (LOG1P)
> -  CASE_MATHFN (LOGB)
>CASE_MATHFN (SIN)
> -  CASE_MATHFN (SQRT)
>CASE_MATHFN (TAN)
>CASE_MATHFN (TANH)
> +  CASE_MATHFN (SQRT)
> +
> +/* The above functions (except sqrt) are not safe to do
> this conversion. */
> +if (!flag_unsafe_math_optimizations)
> +{
> +  /* sqrtl?(T1) could be safely converted into sqrtf?(T2) only if
> +   * p1 >= p2*2+2, where p1 and p2 are precisions of T1 and T2. 
> */

Two spaces after T2.

Perhaps making the comment clearer?

  it is safe to do the following:
float f1 = sqrt ((double) f2);
 -->
float f1 = sqrtf (f2);

 But conditionally safe for the following
double d1 = sqrtl ((long double) d2);
  -->
double d1 = sqrt (d2);

 depending on the precision of the long double type on
the target. ...< Add your
 reference here.>


David

> +  if ((fcode == BUILT_IN_SQRT || fcode == BUILT_IN_SQRTL))
> +  {

Fix indentation.

> +int p1 = REAL_MODE_FORMAT (TYPE_MODE (type))->p;
> +int p2 = (fcode == BUILT_IN_SQRTL) ?
> +REAL_MODE_FORMAT (TYPE_MODE (long_double_type_node))->p :
> +REAL_MODE_FORMAT (TYPE_MODE (double_type_node))->p;
> +if (p2 < p1 * 2 + 2)
> +  break;
> +  }
> +  else
> +break;
> +}
> +  CASE_MATHFN (FABS)
> +  CASE_MATHFN (LOGB)
>  #undef CASE_MATHFN
>  {
>tree arg0 = strip_float_extensions (CALL_EXPR_ARG (expr, 0));
>
> On Tue, Sep 3, 2013 at 3:38 PM, Joseph S. Myers  
> wrote:
>> On Tue, 3 Sep 2013, Cong Hou wrote:
>>
>>> Could you please tell me how to check the precision of long double in
>>> GCC on different platforms?
>>
>> REAL_MODE_FORMAT (TYPE_MODE (long_double_type_node))->p
>>
>> (but you should be referring to the relevant types - "type", the type
>> being converted to, "itype", the type of the function being called in the
>> source code, "TREE_TYPE (arg0)", the type of the argument after extensions
>> have been removed, and "newtype", computed from those - so you should have
>> expressions like the above with two or more of those four types, but not
>> with long_double_type_node directly).
>>
>> The patch submission will need to include a proper analysis to justify to
>> the reader why the particular inequality with particular types from those
>> four is correct in all cases where the relevant code may be executed.
>>
>> --
>> Joseph S. Myers
>> jos...@codesourcery.com

[patch] Make vector::at() assertion message more useful (try #2)

[Paul Pluzhnikov]

Greetings,

This is a followup to:
http://gcc.gnu.org/ml/libstdc++/2013-08/msg00096.html

Without this patch, the user on vector::at out of bounds sees:

terminate called after throwing an instance of 'std::out_of_range'
  what():  vector::_M_range_check
Aborted (core dumped)

With the patch:

terminate called after throwing an instance of 'std::out_of_range'
  what():  vector::_M_range_check: __n (which is 3) >= this->size() (which is 2)
Aborted (core dumped)


I am not at all sure the names I choose here are good ones. Suggestions
welcome.

I also shudder at the idea of repeating _M_range_check code in
e.g. string::at(), and elsewhere. Perhaps we need a snprintf_lite, that
only understands '%zu' and literal characters, e.g.:

  snprintf_lite(__s, sizeof(__s),
_N("vector::_M_range_check: __n (which is %zu) >= "
   "this->size() (which is %zu)"), __n, this->size());


[The patch also doesn't include libstdc++-v3/libsupc++/Makefile.in,
which I'll regenerate before submitting.]

[Please CC me on any replies.]

--
Paul Pluzhnikov

2013-09-04  Paul Pluzhnikov  

* libstdc++-v3/config/abi/pre/gnu.ver: Add
_ZN9__gnu_cxx13concat_size_tEPcm
* libstdc++-v3/include/bits/stl_vector.h (_M_range_check): Print
additional assertion details
* libstdc++-v3/libsupc++/Makefile.am: Add support.cc
* libstdc++-v3/libsupc++/support.cc: New
* 
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc: 
Adjust.
* testsuite/23_containers/vector/requirements/dr438/insert_neg.cc:
Likewise.
* 
testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc:
Likewise.
* 
testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc:
Likewise.


Index: libstdc++-v3/config/abi/pre/gnu.ver
===
--- libstdc++-v3/config/abi/pre/gnu.ver (revision 202262)
+++ libstdc++-v3/config/abi/pre/gnu.ver (working copy)
@@ -1365,6 +1365,9 @@
 # std::get_unexpected()
 _ZSt14get_unexpectedv;
 
+# __gnu_cxx::concat_size_t(char*, unsigned long)
+_ZN9__gnu_cxx13concat_size_tEPcm;
+
 } GLIBCXX_3.4.19;
 
 # Symbols in the support library (libsupc++) have their own tag.
Index: libstdc++-v3/include/bits/stl_vector.h
===
--- libstdc++-v3/include/bits/stl_vector.h  (revision 202262)
+++ libstdc++-v3/include/bits/stl_vector.h  (working copy)
@@ -63,6 +63,10 @@
 #include 
 #endif
 
+namespace __gnu_cxx {
+  int concat_size_t(char *, size_t);
+}
+
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
@@ -791,7 +795,26 @@
   _M_range_check(size_type __n) const
   {
if (__n >= this->size())
- __throw_out_of_range(__N("vector::_M_range_check"));
+ {
+   const char __p[] = __N("vector::_M_range_check: __n (which is ");
+   const char __q[] = __N(") >= this->size() (which is ");
+
+   // Enough space for __p, __q, size and __n (in decimal).
+   const int __alloc_size =
+ sizeof(__p) + sizeof(__q) + 3 * 2 * sizeof(size_type) + 10;
+
+   char *__s = static_cast(__builtin_alloca(__alloc_size));
+   char *__ps = __s;
+   __builtin_memcpy(__ps, __p, sizeof(__p));
+   __ps += sizeof(__p) - 1;
+   __ps += __gnu_cxx::concat_size_t(__ps, __n);
+   __builtin_memcpy(__ps, __q, sizeof(__q));
+   __ps += sizeof(__q) - 1;
+   __ps += __gnu_cxx::concat_size_t(__ps, this->size());
+   *(__ps++) = __N(')');
+   *(__ps++) = '\0';
+   __throw_out_of_range(__s);
+ }
   }
 
 public:
Index: libstdc++-v3/libsupc++/Makefile.am
===
--- libstdc++-v3/libsupc++/Makefile.am  (revision 202262)
+++ libstdc++-v3/libsupc++/Makefile.am  (working copy)
@@ -91,6 +91,7 @@
pointer_type_info.cc \
pure.cc \
si_class_type_info.cc \
+   support.cc \
tinfo.cc \
tinfo2.cc \
vec.cc \
Index: libstdc++-v3/libsupc++/support.cc
===
--- libstdc++-v3/libsupc++/support.cc   (revision 0)
+++ libstdc++-v3/libsupc++/support.cc   (revision 0)
@@ -0,0 +1,53 @@
+// Debugging support -*- C++ -*-
+
+// Copyright (C) 2013 Free Software Foundation, Inc.
+//
+// This file is part of GCC.
+//
+// GCC is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// GCC is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General P

Re: [PATCH] Fixing improper conversion from sin() to sinf() in optimization mode.

[Xinliang David Li]

On Wed, Sep 4, 2013 at 1:59 PM, Joseph S. Myers  wrote:
> On Wed, 4 Sep 2013, Cong Hou wrote:
>
>> I have made a new patch according to your comments. I found several
>> references saying that the precision 2p+2 is OK for the sqrt
>> conversion (one here:
>> http://www.cs.berkeley.edu/~fateman/generic/algorithms.pdf). The new
>> patch is pasted as below.
>
> This patch submission still fails to pay attention to various of my
> comments.
>

If you can provide inlined comments in the patch, that will be more
useful and productive.

thanks,

David


> --
> Joseph S. Myers
> jos...@codesourcery.com

Re: [PATCH] Fixing improper conversion from sin() to sinf() in optimization mode.

[Cong Hou]

I have made a new patch according to your comments. I found several
references saying that the precision 2p+2 is OK for the sqrt
conversion (one here:
http://www.cs.berkeley.edu/~fateman/generic/algorithms.pdf). The new
patch is pasted as below.

Thank you for all the suggestions, Joseph!


Cong


Index: gcc/testsuite/gcc.c-torture/execute/20030125-1.c
===
--- gcc/testsuite/gcc.c-torture/execute/20030125-1.c (revision 201891)
+++ gcc/testsuite/gcc.c-torture/execute/20030125-1.c (working copy)
@@ -44,11 +44,11 @@ __attribute__ ((noinline))
 double
 sin(double a)
 {
- abort ();
+ return a;
 }
 __attribute__ ((noinline))
 float
 sinf(float a)
 {
- return a;
+ abort ();
 }
Index: gcc/convert.c
===
--- gcc/convert.c (revision 201891)
+++ gcc/convert.c (working copy)
@@ -135,16 +135,34 @@ convert_to_real (tree type, tree expr)
   CASE_MATHFN (COS)
   CASE_MATHFN (ERF)
   CASE_MATHFN (ERFC)
-  CASE_MATHFN (FABS)
   CASE_MATHFN (LOG)
   CASE_MATHFN (LOG10)
   CASE_MATHFN (LOG2)
   CASE_MATHFN (LOG1P)
-  CASE_MATHFN (LOGB)
   CASE_MATHFN (SIN)
-  CASE_MATHFN (SQRT)
   CASE_MATHFN (TAN)
   CASE_MATHFN (TANH)
+  CASE_MATHFN (SQRT)
+
+/* The above functions (except sqrt) are not safe to do
this conversion. */
+if (!flag_unsafe_math_optimizations)
+{
+  /* sqrtl?(T1) could be safely converted into sqrtf?(T2) only if
+   * p1 >= p2*2+2, where p1 and p2 are precisions of T1 and T2. */
+  if ((fcode == BUILT_IN_SQRT || fcode == BUILT_IN_SQRTL))
+  {
+int p1 = REAL_MODE_FORMAT (TYPE_MODE (type))->p;
+int p2 = (fcode == BUILT_IN_SQRTL) ?
+REAL_MODE_FORMAT (TYPE_MODE (long_double_type_node))->p :
+REAL_MODE_FORMAT (TYPE_MODE (double_type_node))->p;
+if (p2 < p1 * 2 + 2)
+  break;
+  }
+  else
+break;
+}
+  CASE_MATHFN (FABS)
+  CASE_MATHFN (LOGB)
 #undef CASE_MATHFN
 {
   tree arg0 = strip_float_extensions (CALL_EXPR_ARG (expr, 0));

On Tue, Sep 3, 2013 at 3:38 PM, Joseph S. Myers  wrote:
> On Tue, 3 Sep 2013, Cong Hou wrote:
>
>> Could you please tell me how to check the precision of long double in
>> GCC on different platforms?
>
> REAL_MODE_FORMAT (TYPE_MODE (long_double_type_node))->p
>
> (but you should be referring to the relevant types - "type", the type
> being converted to, "itype", the type of the function being called in the
> source code, "TREE_TYPE (arg0)", the type of the argument after extensions
> have been removed, and "newtype", computed from those - so you should have
> expressions like the above with two or more of those four types, but not
> with long_double_type_node directly).
>
> The patch submission will need to include a proper analysis to justify to
> the reader why the particular inequality with particular types from those
> four is correct in all cases where the relevant code may be executed.
>
> --
> Joseph S. Myers
> jos...@codesourcery.com

Re: [PATCH] Fixing improper conversion from sin() to sinf() in optimization mode.

[Joseph S. Myers]

On Wed, 4 Sep 2013, Cong Hou wrote:

> I have made a new patch according to your comments. I found several
> references saying that the precision 2p+2 is OK for the sqrt
> conversion (one here:
> http://www.cs.berkeley.edu/~fateman/generic/algorithms.pdf). The new
> patch is pasted as below.

This patch submission still fails to pay attention to various of my 
comments.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [RFC] Changes to the wide-int classes

[Kenneth Zadeck]

Richi, and the rest of the community,

Richard Sandiford has proposed a set of patches that change the wide-int 
api in a significant way. We think that we really need some input from 
the community as to if this is what we want using C++ in gcc is going to 
look like. There are, as I see it, two issues that are raised by these 
patches:


1) Visibility. In my original wide-int patch, there were things that 
were public and things that were private. In general, the implementation 
details were private, but also the privacy was used to enforce the 
readonlyness of the representation. A wide int is an object and that 
object has value that cannot be changed. In Richard's restructuring, 
everything is visible in the wi namespace. The privacy of the internals 
can only be enforced by review of the maintainers.


It is possible restore the privacy of the original patches but this 
seems to involve using a private class inside the namespace and using 
C++ friends.


Adding visibility adds extra code

namespace wi {
class impl {
private:
add_large() { }
}
friend add(…);
};

that is roughly 1 line per client that uses an internal routine for the friend 
declaration and 5 extra lines for the other stuff.

Richard worries that using friends is not within the scope acceptable 
C++ mechanisms to use within the gcc community. We are, of course, open 
to other suggestions.


My personal view is that privacy is important and I do not want to let 
that go. In past comments, Richi, asked that we remove all of the public 
method calls that modified the internals of a wide-int. We did this and 
we generally are happy that we did this. So there is the question is the 
privacy important and if it is what is the proper mechanism to implement it.


2) In my original patch, almost all of the operations either used 
operators or method calls. In richard's patch, those method calls have 
all become regular function calls. This is not just a syntactic change. 
The wide-int operations are fairly generic. The operations are not just 
defined on wide-ints but also on tree and rtx constants. In my patch the 
tree and rtx arguments were not converted to wide-ints. instead, the 
code looked inside of the representation of the tree and rtl and just 
operated on that. However, in my patch the first parameter had to be a 
wide-int to make the method call. By making everything a regular 
function call, richard's patch avoids having to make the wide-int object 
just to make the method call. This seems like a big win, but it means 
that the wide-int code does not "look like" the double-int code anymore. 
I am in favor of this change, but it i (we) fear that there will be 
pushback from those that wanted this to look more oo-like as double-int 
currently does.


comments suggestions, and questions are appreciated.

thanks,

kenny

Re: [c++-concepts] Class template constraints

[Jason Merrill]

On 09/04/2013 01:33 PM, Andrew Sutton wrote:

Ah. The goal is to check after we've deduced/coerced template
arguments into a valid substitution. With functions, that's in
fn_type_unification (hopefully called from instantiate_template)


Actually fn_type_unification calls instantiate_template, but yep, we're 
on the same page.


Jason

Re: [PATCH] Fixing improper conversion from sin() to sinf() in optimization mode.

[Cong Hou]

Updated patch according to your comment (tabs are not pasted here).

Cong


Index: gcc/convert.c
===
--- gcc/convert.c (revision 201891)
+++ gcc/convert.c (working copy)
@@ -135,16 +135,40 @@ convert_to_real (tree type, tree expr)
   CASE_MATHFN (COS)
   CASE_MATHFN (ERF)
   CASE_MATHFN (ERFC)
-  CASE_MATHFN (FABS)
   CASE_MATHFN (LOG)
   CASE_MATHFN (LOG10)
   CASE_MATHFN (LOG2)
   CASE_MATHFN (LOG1P)
-  CASE_MATHFN (LOGB)
   CASE_MATHFN (SIN)
-  CASE_MATHFN (SQRT)
   CASE_MATHFN (TAN)
   CASE_MATHFN (TANH)
+  CASE_MATHFN (SQRT)
+
+/* The above functions (except sqrt) are not safe to do this conversion. */
+if (!flag_unsafe_math_optimizations)
+  {
+ /* sqrtl?(T1) could be safely converted into sqrtf?(T2) only if
+   p1 >= p2*2+2, where p1 and p2 are precisions of T1 and T2.
+   For example, on x86 the conversion from float(sqrt((double)f) to
+   sqrtf(f) is safe where f has the type float, since float has 23 bits
+   precision and double has 52 bits precision, and 52 > 23*2+2.
+   However, the conversion from double(sqrtl((long double)d) to
+   sqrt(d) is unsafe where d has the type double. This is because
+   long double has 63 bits precision and then 63 < 52*2+2.  */
+ if ((fcode == BUILT_IN_SQRT || fcode == BUILT_IN_SQRTL))
+  {
+int p1 = REAL_MODE_FORMAT (TYPE_MODE (type))->p;
+int p2 = (fcode == BUILT_IN_SQRTL) ?
+ REAL_MODE_FORMAT (TYPE_MODE (long_double_type_node))->p :
+ REAL_MODE_FORMAT (TYPE_MODE (double_type_node))->p;
+if (p2 < p1 * 2 + 2)
+  break;
+  }
+ else
+  break;
+  }
+  CASE_MATHFN (FABS)
+  CASE_MATHFN (LOGB)
 #undef CASE_MATHFN
 {
   tree arg0 = strip_float_extensions (CALL_EXPR_ARG (expr, 0));
Index: gcc/testsuite/gcc.c-torture/execute/20030125-1.c
===
--- gcc/testsuite/gcc.c-torture/execute/20030125-1.c (revision 201891)
+++ gcc/testsuite/gcc.c-torture/execute/20030125-1.c (working copy)
@@ -44,11 +44,11 @@ __attribute__ ((noinline))
 double
 sin(double a)
 {
- abort ();
+ return a;
 }
 __attribute__ ((noinline))
 float
 sinf(float a)
 {
- return a;
+ abort ();
 }

On Wed, Sep 4, 2013 at 2:21 PM, Xinliang David Li  wrote:
> On Wed, Sep 4, 2013 at 1:59 PM, Joseph S. Myers  
> wrote:
>> On Wed, 4 Sep 2013, Cong Hou wrote:
>>
>>> I have made a new patch according to your comments. I found several
>>> references saying that the precision 2p+2 is OK for the sqrt
>>> conversion (one here:
>>> http://www.cs.berkeley.edu/~fateman/generic/algorithms.pdf). The new
>>> patch is pasted as below.
>>
>> This patch submission still fails to pay attention to various of my
>> comments.
>>
>
> If you can provide inlined comments in the patch, that will be more
> useful and productive.
>
> thanks,
>
> David
>
>
>> --
>> Joseph S. Myers
>> jos...@codesourcery.com

Re: [PATCH] Fixing improper conversion from sin() to sinf() in optimization mode.

[Joseph S. Myers]

On Wed, 4 Sep 2013, Xinliang David Li wrote:

> > This patch submission still fails to pay attention to various of my
> > comments.
> >
> 
> If you can provide inlined comments in the patch, that will be more
> useful and productive.

I have explained things several times in this thread already.  I see no 
point in repeating things when what I would say has already been said and 
ignored.  As far as I can tell, this latest patch submission has taken one 
line from the message it is in response to, and largely ignored the 
following two paragraphs (including where I explicitly say that said line 
should not appear literally in the source code at all).  But, repeating 
what I said before yet again:

  (but you should be referring to the relevant types

The patch does not do properly that.  It refers explicitly to 
long_double_type_node and double_type_node.

  - "type", the type 
  being converted to, "itype", the type of the function being called in the 
  source code, "TREE_TYPE (arg0)", the type of the argument after extensions 
  have been removed, and "newtype", computed from those

The patch only engages with "type".  I suspect "newtype" is what it really 
means there when using "type".  When it uses long_double_type_node and 
double_type_node, those should be "itype".

  - so you should have 
  expressions like the above with two or more of those four types, but not 
  with long_double_type_node directly).

See above.  The patch uses long_double_type_node directly, contrary to 
what I explicitly said.  You are free to disagree with something I say in 
a review - but in that case you need to reply specifically to the review 
comment explaining your rationale for disagreeing with it.  Just ignoring 
the comment and not mentioning the disagreement wastes the time of 
reviewers.

  The patch submission will need to include a proper analysis to justify to 
  the reader why the particular inequality with particular types from those 
  four is correct in all cases where the relevant code may be executed.

The comments only refer to "T1" and "T2" without explaining how they 
relate to the original expression (three types) or the variables within 
GCC dealing with various types (four types, because newtype gets 
involved).  I said back in 
 and 
 that three types 
are involved - when I say "the patch submission needs to include its own 
analysis for the full generality of three types", again, it's 
inappropriate for a patch to omit such an analysis without justification.  
The patch submission needs to include an analysis that properly explains 
the transformation involved and the conditions under which it is safe.  
Maybe starting along the lines of:

We have an expression of the form (T1) sqrtT2 ((T2) exprT3), where exprT3 
has type T3 (TREE_TYPE (ARG0)), T2 is the type of the floating-point 
square root function being used (ITYPE), T1 is TYPE and all these types 
are binary floating-point types.  We wish to optimize if possible to an 
expression of the form (T1) sqrtT4 ((T4) exprT3), where T4 (NEWTYPE) is 
narrower than T2.  (Then explain the choice of T4 and the conditions under 
which the transformation is safe, with appropriate references.)

I suggest that for the next patch submission you (the patch submitter) 
make sure you spend at least an hour properly understanding the issues and 
all the previous messages in the thread and writing up the detailed, 
coherent explanation of the transformation and analysis of the issues that 
I asked for some time ago, as if for a reader who hasn't read any of this 
thread or looked at this transformation in GCC before.  I've certainly 
spent longer than that on review in this thread.  It's normal for a patch 
involving anything at all tricky to take hours to write up (and also 
normal for one to discover, in the course of writing the detailed coherent 
analysis for people who aren't familiar with the code and the issues 
involved, that there's in fact something wrong with the patch and it needs 
revisiting before submission).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [patch] Make vector::at() assertion message more useful (try #2)

[Paul Pluzhnikov]

On Wed, Sep 4, 2013 at 2:10 PM, Daniel Krügler
 wrote:

> I expect that this kind of index violation is a rather often occurring
> pattern and should be isolated. IMO the _M_range
> _check now pessimisms the normal, non-violating case.

Did you mean "pessimises code size", or something else?


> Why not simply
> replacing it by something along the lines of
>
>  _M_range_check(size_type __n) const
>{
> if (__n >= this->size())
>  
> __throw_out_of_range(__index_out_of_range_msg(__N("vector::_M_range_check"),
>__n, this->size()));
>}
>
> where __index_out_of_range_msg is a reusable function that uses
> something like snprintf_lite internally?

Some disadvantages to doing that:
1. The actual message is now "magically" transformed inside
   __index_out_of_range_msg into the final message, making translation
   more difficult.
2. __index_out_of_range_msg() would have to return a string, which is heavier
   weight (in try#1 I just used snprintf, which was considered "too heavy").

Thanks,
-- 
Paul Pluzhnikov

Re: [patch] Make vector::at() assertion message more useful (try #2)

[Paolo Carlini]

Hi,

On 09/04/2013 10:53 PM, Paul Pluzhnikov wrote:

I am not at all sure the names I choose here are good ones. Suggestions
welcome.
For sure concat_size would not be Ok, isn't uglified. Thanks for the 
code, you understand isn't really something we can imagine committing.

I also shudder at the idea of repeating _M_range_check code in
e.g. string::at(), and elsewhere. Perhaps we need a snprintf_lite, that
only understands '%zu' and literal characters, e.g.:

   snprintf_lite(__s, sizeof(__s),
 _N("vector::_M_range_check: __n (which is %zu) >= "
"this->size() (which is %zu)"), __n, this->size());

That seems worth exploring, I agree.

Paolo.

Re: Fwd: [PATCH] Scheduling result adjustment to enable macro-fusion

[Wei Mi]

Thanks for the suggestions! I take a look at adjust_priority, and find
it may not guarantee to schedule cmp and jmp together. The priority is
used to choose a candidate from ready list. If cmp is the only insn in
ready list and there is another insn-A in queued set (insn-A's
dependence has been resolved, but it is not ready because of data
delay or resource delay), then cmp will be scheduled before insn-A no
matter what their priorities are.

I will take a look at whether SCHED_GROUP is going to work.

On Wed, Sep 4, 2013 at 12:33 PM, Alexander Monakov  wrote:
> On Wed, Sep 4, 2013 at 9:53 PM, Steven Bosscher  wrote:
>>
>> On Wed, Sep 4, 2013 at 10:58 AM, Alexander Monakov wrote:
>> > Hello,
>> >
>> > Could you use the existing facilities instead, such as adjust_priority 
>> > hook,
>> > or making the compare-branch insn sequence a SCHED_GROUP?
>>
>>
>> Or a define_bypass?
>
> Hm, I don't think define_bypass would work: it still leaves the
> scheduler freedom to move the compare up.
>
> IMO adjust_priority would be preferable if it allows to achieve the goal.
>
> Alexander

Re: [patch] Make vector::at() assertion message more useful (try #2)

[Paul Pluzhnikov]

On Wed, Sep 4, 2013 at 4:26 PM, Paolo Carlini  wrote:

> For sure concat_size would not be Ok, isn't uglified.

I didn't uglify it because it's inside __gnu_cxx namespace.
Does it still need uglification?

>>snprintf_lite(__s, sizeof(__s),
>>  _N("vector::_M_range_check: __n (which is %zu) >= "
>> "this->size() (which is %zu)"), __n, this->size());
>
> That seems worth exploring, I agree.

Should snprintf_lite be in __gnu_cxx namespace, or be global and be called
__snprintf_lite(), or ...?

Is the location of the out-of-line code in libstdc++-v3/libsupc++/ ok?
(Would probably be called snprintf_lite.cc or some such.)

Is the version I've assigned to the symbol -- GLIBCXX_3.4.20 -- ok?

Thanks,
-- 
Paul Pluzhnikov

Re: [patch] Make vector::at() assertion message more useful (try #2)

[Paolo Carlini]

Hi,

On 09/05/2013 01:36 AM, Paul Pluzhnikov wrote:

On Wed, Sep 4, 2013 at 4:26 PM, Paolo Carlini  wrote:


For sure concat_size would not be Ok, isn't uglified.

I didn't uglify it because it's inside __gnu_cxx namespace.
Does it still need uglification?

Yes.



snprintf_lite(__s, sizeof(__s),
  _N("vector::_M_range_check: __n (which is %zu) >= "
 "this->size() (which is %zu)"), __n, this->size());

That seems worth exploring, I agree.

Should snprintf_lite be in __gnu_cxx namespace, or be global and be called
__snprintf_lite(), or ...?
In any case it needs the __ in front. Like the rest of the library, to 
protect vs


#define snprintf_lite 1

in user code. Well known issue...

Is the location of the out-of-line code in libstdc++-v3/libsupc++/ ok?
(Would probably be called snprintf_lite.cc or some such.)
I don't think we want to fiddle with libsupc++, for the time being at 
least. src/c++98 seem ok to me.

Is the version I've assigned to the symbol -- GLIBCXX_3.4.20 -- ok?

Is a release out with 3.4.20? If not, it's fine. I think it's fine.

Paolo.

Re: [PATCH] Portable Volatility Warning

[Hans-Peter Nilsson]

On Tue, 3 Sep 2013, Richard Biener wrote:
> I think the warning can be completely implemented inside struct-layout.c
> for example in finish_bitfield_representative (if you pass that the first 
> field
> in the group, too).  Of course that is at the expense of warning for
> struct declarations rather than actual code differences (the struct may
> not be used)

FWIW, I don't feeling stringly whether this is ok or not, but I
*think* missing warnings for (unused) declarations is ok; we are
(ok, I am) interested in semantic differences for generated
code, not declarations.

brgds, H-P

Re: RFC - Refactor tree.h

[Mike Stump]

On Aug 30, 2013, at 4:22 PM, Diego Novillo  wrote:
> Thanks for the suggestions.  I've incorporated them into the patch.
> It now adds tree-core.h with all the structures, enums, typedefs and
> some fundamental declarations from tree.h.  Everything else stays in
> tree.h for now.

So, the comments for the data structures that moved, should also move?!

The below moves one comment that I noticed.

Ok?


Index: tree-core.h
===
--- tree-core.h (revision 202238)
+++ tree-core.h (working copy)
@@ -1328,6 +1328,11 @@ struct GTY(()) tree_decl_non_common {
   tree vindex;
 };
 
+/* FUNCTION_DECL inherits from DECL_NON_COMMON because of the use of the
+   arguments/result/saved_tree fields by front ends.   It was either inherit
+   FUNCTION_DECL from non_common, or inherit non_common from FUNCTION_DECL,
+   which seemed a bit strange.  */
+
 struct GTY(()) tree_function_decl {
   struct tree_decl_non_common common;
 
Index: tree.h
===
--- tree.h  (revision 202238)
+++ tree.h  (working copy)
@@ -2534,11 +2534,6 @@ extern vec **decl_debug_arg
 #define DECL_FINAL_P(NODE)\
(FUNCTION_DECL_CHECK (NODE)->decl_with_vis.final)
 
-/* FUNCTION_DECL inherits from DECL_NON_COMMON because of the use of the
-   arguments/result/saved_tree fields by front ends.   It was either inherit
-   FUNCTION_DECL from non_common, or inherit non_common from FUNCTION_DECL,
-   which seemed a bit strange.  */
-
 /* The source language of the translation-unit.  */
 #define TRANSLATION_UNIT_LANGUAGE(NODE) \
   (TRANSLATION_UNIT_DECL_CHECK (NODE)->translation_unit_decl.language)

Re: [patch 4/4] -fstrict-volatile-bitfields cleanup v3: remove from defaults on all targets

[Hans-Peter Nilsson]

> From: Bernd Edlinger 
> Date: Wed, 4 Sep 2013 10:15:22 +0200

> Even driver code rarely uses bit-fields for register access,
> because that is inherently non-portabe. Reason: the bit
> positions are completely different on little- and big-endian
> targets. Most drivers I have seen use some macros and explicit
> bit operations for register accesses.

Counter-observation, FWIW: most drivers I work on (...) use
bit-field structures filled to capacity of the I/O access size
(almost always 32 bits), read and written volatile as the full
structure.  None of the I/O accesses are per *individual*
bit-field, so this particular discussion does not apply, but I
very much would like a warning for unportable code (meaning:
across targets and ABIs within gcc).  The bit-field-structure
declarations are in turn generated by a (supposedly :)
target-endian-aware generator from an I/O register description
(also layout-aware except all intended targets have a common
layout for the declarations used).  If both-endian declarations
were desirable, both could easily be generated and selected #if
THATENDIAN-style.

Not my idea but I dare defend it: using bit-field syntax is
quite a bit more readable than shifting-and-masking macros.
Unfortunately, sometimes there's a gcc-bug or two, tree-sra
being a usual suspect. :/

brgds, H-P

Re: [patch] Make vector::at() assertion message more useful (try #2)

[Daniel Krügler]

2013/9/5 Paul Pluzhnikov :
> On Wed, Sep 4, 2013 at 2:10 PM, Daniel Krügler
>  wrote:
>
>> I expect that this kind of index violation is a rather often occurring
>> pattern and should be isolated. IMO the _M_range
>> _check now pessimisms the normal, non-violating case.
>
> Did you mean "pessimises code size", or something else?

Yes.

- Daniel