Re: [patch] Do not include output.h everywhere

[Steven Bosscher]

On Fri, Jun 1, 2012 at 11:19 AM, Richard Guenther
 wrote:
>> What about toplev.h?  dump_file is defined there, after all.  I don't mind
>> it in system.h, if that's not a good place for it now.  The patch is OK
>> either way.
>
> I'd prefer toplev.h, too.  system.h is supposed to be for system header
> inclusion (and related workarounds).
(..)
> Thus, ok with moving to toplev.h instead.

I had already committed this on Friday.

The problem with toplev.h is that it isn't included in any tree-*.c
file (except tree-dump.c and tree-optimize.c). Also, there is nothing
"top-level" about an RTL or GIMPLE pass. Perhaps I should create a new
header with all the dump and statistics stuff?

Ciao!
Steven

Re: [gcov] a few improvements

[Nathan Sidwell]

On 06/03/12 21:40, Xinliang David Li wrote:


Can you explain this more -- what exactly are trying to do?  Are you trying
to rebuild multiple times with the same coverage data,


yes -- for instance, in the context of debugging a compiler problem,
you will need to compile the same file multiple times with the same
coverage data.


ok, debugging the compiler is not the typical user mode of use.

Do any of the following work for you?
*) don't use -frandom-seed (you don't say why you're using this flag)
*) copy and restore the coverage data file
*) stub out the unlink code at the end of coverage.c. (I presume that you're 
rebuilding cc1 with optimization off anyway, and as you're debugging a compiler 
problem with FDO you're somewhat familiar with coverage.c).


nathan

Re: [PATCH] Option to build bare-metal ARM cross-compiler for arm-none-eabi target without libunwind

[Fredrik Hederstierna]

You are absolutely right, I forgot to change this as well.
Thanks!

And I really really do hope that this patch will go into GCC 4.8 trunk.

Since arm-elf is marked 'obsolete' in GCC 4.7,
all old bare-metal arm-elf-users will need
to migrate to the the new arm-eabi-toolchain in GCC 4.8.0.

I fear it will cause a lot of people trouble, if we can't build a real pure 
bare-metal toolchain on arm-eabi target.

I have not seen many comments on the patch from the community yet.
Hopefully some arm-eabi maintainer will get his/her eyes on it eventually.

Thanks! and Best Regards
Fredrik

-Larry Doolittle  wrote: -
To: gcc-patches@gcc.gnu.org
From: Larry Doolittle 
Date: 06/03/2012 06:35PM
Cc: Fredrik Hederstierna 
Subject: [PATCH] Option to build bare-metal ARM cross-compiler for 
arm-none-eabi target without libunwind

This is an update to Fredrik Hederstierna's mail and patch from
12 Apr 2012.  I think he missed one place where -fexceptions needs
to be changed to -fno-exceptions.

With the attached patch, two of us (a friend and I, one on Linux and
one on Mac) can build Rockbox with gcc-4.7.0.  I'll admit I haven't
checked the repository for changes in this area, this patch is based
on stock released gcc-4.7.0.

I have no opinion on Sebastian Huber's idea of making functionality
of this kind implicit with target.

   - Larry


[attachment "libgcc-4.7.0-baremetal.patch" removed by Fredrik 
Hederstierna/INT/SE/DIR/SECURITAS]

[Doc] Add caveat to __builtin_object_size

[Florian Weimer]

This patch adds a cross-reference to GNU libc and _FORTIFY_SOURCE (which 
needs to be documented there) and mentions the optimization level 
requirements.


Okay for trunk?

2012-06-04  Florian Weimer  

* doc/extend.texi (Object Size Checking): Mention
 _FORTIFY_SOURCE, add caveat.

--
Florian Weimer / Red Hat Product Security Team
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi	(revision 187951)
+++ gcc/doc/extend.texi	(working copy)
@@ -7376,8 +7376,15 @@
 @findex __builtin___vfprintf_chk
 
 GCC implements a limited buffer overflow protection mechanism
-that can prevent some buffer overflow attacks.
+that can prevent some buffer overflow attacks.  GNU libc uses it
+in the implementation of the @code{_FORTIFY_SOURCE} functionality.
 
+This protection mechanism is only a last resort.  As a programmer, you
+must not rely on its presence, but use explicit buffer length checks
+to avoid buffer overflows.  GCC may not be able to determine buffer
+sizes accurately, and the accuracy depends on compiler version and
+optimization level (currently, at least @option{-O2} is required).
+
 @deftypefn {Built-in Function} {size_t} __builtin_object_size (void * @var{ptr}, int @var{type})
 is a built-in construct that returns a constant number of bytes from
 @var{ptr} to the end of the object @var{ptr} pointer points to

Re: [Doc] Add caveat to __builtin_object_size

[Florian Weimer]

On 06/04/2012 10:23 AM, Jakub Jelinek wrote:

On Mon, Jun 04, 2012 at 10:15:35AM +0200, Florian Weimer wrote:

--- gcc/doc/extend.texi (revision 187951)
+++ gcc/doc/extend.texi (working copy)
@@ -7376,8 +7376,15 @@
  @findex __builtin___vfprintf_chk

  GCC implements a limited buffer overflow protection mechanism
-that can prevent some buffer overflow attacks.
+that can prevent some buffer overflow attacks.  GNU libc uses it
+in the implementation of the @code{_FORTIFY_SOURCE} functionality.

+This protection mechanism is only a last resort.  As a programmer, you
+must not rely on its presence, but use explicit buffer length checks
+to avoid buffer overflows.  GCC may not be able to determine buffer
+sizes accurately, and the accuracy depends on compiler version and
+optimization level (currently, at least @option{-O2} is required).


That isn't true, at -O1 or -Os it should work just fine too, and
to some extent on the compiler side even at -O0.


Fold needs to run.  Without it, __builtin_object_size constantly returns 
(size_t)-1.  The documentation doesn't say which optimization level 
enables fold, so I have to guess.  -O0 apparently doesn't.



So I'd just replace optimization level.*\. with compiler options.


Okay.  What about this?

--
Florian Weimer / Red Hat Product Security Team
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi	(revision 187951)
+++ gcc/doc/extend.texi	(working copy)
@@ -7376,8 +7376,15 @@
 @findex __builtin___vfprintf_chk
 
 GCC implements a limited buffer overflow protection mechanism
-that can prevent some buffer overflow attacks.
+that can prevent some buffer overflow attacks.  GNU libc uses it
+in the implementation of the @code{_FORTIFY_SOURCE} functionality.
 
+This protection mechanism is only a last resort.  As a programmer, you
+must not rely on its presence, but use explicit buffer length checks
+to avoid buffer overflows.  GCC may not be able to determine buffer
+sizes accurately, and the accuracy depends on compiler version and
+options.
+
 @deftypefn {Built-in Function} {size_t} __builtin_object_size (void * @var{ptr}, int @var{type})
 is a built-in construct that returns a constant number of bytes from
 @var{ptr} to the end of the object @var{ptr} pointer points to

[patch] More places where not to include output.h

[Steven Bosscher]

Hello,

With the attached patch to move some non-output related prototypes
from output.h to tree.h, another batch of #include output.h cleanups
is possible.

After this patch, only 9 front-end files (out of 210 *.{c,cc,h} files)
still include output.h:

c-family/c-cppbuiltin.c:#include "output.h" /* For
user_label_prefix.  */
c-family/c-lex.c:#include "output.h" /* for asm_out_file */
c-family/c-pch.c:#include "output.h" /* for asm_out_file */
cp/decl.c:#include "output.h" /* for have_global_bss_p */
objc/objc-next-runtime-abi-01.c:#include "output.h" /* for asm_out_file */
ada/gcc-interface/trans.c:#include "output.h"
ada/gcc-interface/utils.c:#include "output.h"
go/go-backend.c:#include "output.h" /* for assemble_string */
java/class.c:#include "output.h" /* for switch_to_section and get_section */

Bootstrapped (with ada) on powerpc64-unknown-linux-gnu. OK?

Ciao!
Steven


cleanup_output_h_includes_2.diff
Description: Binary data

Re: [PATCH] Fix PR53471, remove DECL_ASSEMBLER_NAME deferred compute

[Richard Guenther]

On Fri, 1 Jun 2012, Jason Merrill wrote:

> On 06/01/2012 05:22 AM, Richard Guenther wrote:
> > On Thu, 31 May 2012, Jason Merrill wrote:
> >
> > > The comment mentions PCH in connection with deferred seting of
> > > DECL_ASSEMBLER_NAME; off the top of my head it occurs to me that that
> > > might be
> > > connected with anonymous unions, which need to have different linkage
> > > names in
> > > different translation units.
> >
> > Not sure when PCH generation happens (or when we call
> > rest_of_type_compilation), but shouldn't that be way earlier than
> > the debug output?
> 
> PCH generation happens at the beginning of cp_write_global_declarations;
> rest_of_type_compilation, which triggers the debug output, is called after
> each class definition.
> 
> > Anyway, any suggestion on how to tackle the issue
> > that we cannot compute new DECL_ASSEMBLER_NAMEs after the frontend
> > is finished?
> 
> Fix up the deferred_asm_name list somewhere between the call to
> c_common_write_pch and the call to free_lang_data.  I guess this would mean
> another debug hook for processing that needs to happen before free_lang_data.

That's certainly possible - add a finish_compilation_unit debug hook.
I'll think about that for 4.8.

> Or fix free_lang_data to deal with these types, too.

Unfortunately they are not "reachable" from anything but the
deferred_asm_name list.  So it would mean another debug hook.

> Or use your first patch, and decide that we don't care about the linkage name
> of unreachable types.  What types are affected by this, anyway?

Types affected by this are types not referenced from any object/function
that is being output as LTO bytecode (and the type will not be output
as LTO bytecode either, so at LTO time we'd not generate debuginfo for
it either).  It's basically completely "unused" types that are affected.

I suppose that's indeed the easiest fix - all "important" types should
have been assigned an assembler name by free-lang-data.  One issue
is that free-lang-data (and its assembler name assigning code) is not
run if -flto is not enabled ...

So I suppose for the 4.7 branch we'd go with

Index: dwarf2out.c
===
--- dwarf2out.c (revision 188106)
+++ dwarf2out.c (working copy)
@@ -22158,7 +22158,8 @@ dwarf2out_finish (const char *filename)
   for (node = deferred_asm_name; node; node = node->next)
 {
   tree decl = node->created_for;
-  if (DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl))
+  if ((!flag_generate_lto || DECL_ASSEMBLER_NAME_SET_P (decl))
+ && DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl))
{
  add_linkage_attr (node->die, decl);
  move_linkage_attr (node->die);

and for trunk change two things - make sure the assembler-name assigning
piece of free-lang-data runs unconditionally and a debug hook is added
so we can output debuginfo dependent on langhooks.

I am going to test the above and install it on trunk and the branch
for now to get into 4.7.1.

Thanks,
Richard.

[PATCH][5/n] loop distribution TLC

[Richard Guenther]

I noticed you cannot enable regular loop distribution at -O3 unless
you disable pattern recognition.  This lead me to simplifying the
code, changing semantics of the flags slightly (but still compatible
with what I read into their documentation).  -floop-distribution
enables loop distribution (but not pattern recognition) and
-floop-distribute-patterns enables pattern recognition but not
generic loop distribution.  The latter is still enabled by -O3,
if you enable both you get both.

The patch also removes one kind of "similar memory access" detection.
I have a followup that makes the remaining one cheaper.

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

Richard.

2012-06-04  Richard Guenther  

* tree-data-ref.c (have_similar_memory_accesses_1): Remove.
(ref_base_address_1): Likewise.
(remove_similar_memory_refs): Likewise.
* tree-data-ref.h (remove_similar_memory_refs): Remove.
* tree-loop-distribution.c (classify_partition): Do not classify
as builtin if -ftree-loop-distribute-patterns is not enabled.
(fuse_partitions_with_similar_memory_accesses): Inline ...
(ldist_gen): ... here.  Fuse all non-builtin partitions if
-ftree-loop-distribution is not enabled.  Properly return
the number of created partitions.  Do not update SSA form here
but ...
(tree_loop_distribution): ... once here for the whole function.
Only walk innermost loops, constrain loops we consider here
further.  Do not call remove_similar_memory_refs.
(distribute_loop): Do not check number of loop nodes here.

* gcc.dg/tree-ssa/ldist-11.c: Enable -ftree-loop-distribute-patterns.
* gcc.dg/tree-ssa/ldist-17.c: Likewise.
* gcc.dg/tree-ssa/ldist-pr45948.c: Likewise.

Index: gcc/tree-data-ref.c
===
*** gcc/tree-data-ref.c.orig2012-06-01 14:21:36.0 +0200
--- gcc/tree-data-ref.c 2012-06-01 15:26:42.826591786 +0200
*** have_similar_memory_accesses (gimple s1,
*** 5403,5467 
VEC_free (data_ref_loc, heap, refs2);
return res;
  }
- 
- /* Helper function for the hashtab.  */
- 
- static int
- have_similar_memory_accesses_1 (const void *s1, const void *s2)
- {
-   return have_similar_memory_accesses (CONST_CAST_GIMPLE ((const_gimple) s1),
-  CONST_CAST_GIMPLE ((const_gimple) s2));
- }
- 
- /* Helper function for the hashtab.  */
- 
- static hashval_t
- ref_base_address_1 (const void *s)
- {
-   gimple stmt = CONST_CAST_GIMPLE ((const_gimple) s);
-   unsigned i;
-   VEC (data_ref_loc, heap) *refs;
-   data_ref_loc *ref;
-   hashval_t res = 0;
- 
-   get_references_in_stmt (stmt, &refs);
- 
-   FOR_EACH_VEC_ELT (data_ref_loc, refs, i, ref)
- if (!ref->is_read)
-   {
-   res = htab_hash_pointer (ref_base_address (stmt, ref));
-   break;
-   }
- 
-   VEC_free (data_ref_loc, heap, refs);
-   return res;
- }
- 
- /* Try to remove duplicated write data references from STMTS.  */
- 
- void
- remove_similar_memory_refs (VEC (gimple, heap) **stmts)
- {
-   unsigned i;
-   gimple stmt;
-   htab_t seen = htab_create (VEC_length (gimple, *stmts), ref_base_address_1,
-have_similar_memory_accesses_1, NULL);
- 
-   for (i = 0; VEC_iterate (gimple, *stmts, i, stmt); )
- {
-   void **slot;
- 
-   slot = htab_find_slot (seen, stmt, INSERT);
- 
-   if (*slot)
-   VEC_ordered_remove (gimple, *stmts, i);
-   else
-   {
- *slot = (void *) stmt;
- i++;
-   }
- }
- 
-   htab_delete (seen);
- }
- 
--- 5403,5405 
Index: gcc/tree-data-ref.h
===
*** gcc/tree-data-ref.h.orig2012-06-01 14:21:36.0 +0200
--- gcc/tree-data-ref.h 2012-06-01 15:26:42.826591786 +0200
*** index_in_loop_nest (int var, VEC (loop_p
*** 607,613 
  
  void stores_from_loop (struct loop *, VEC (gimple, heap) **);
  void stores_zero_from_loop (struct loop *, VEC (gimple, heap) **);
- void remove_similar_memory_refs (VEC (gimple, heap) **);
  bool rdg_defs_used_in_other_loops_p (struct graph *, int);
  bool have_similar_memory_accesses (gimple, gimple);
  bool stmt_with_adjacent_zero_store_dr_p (gimple);
--- 607,612 
Index: gcc/tree-loop-distribution.c
===
*** gcc/tree-loop-distribution.c.orig   2012-06-01 14:21:36.0 +0200
--- gcc/tree-loop-distribution.c2012-06-01 15:45:22.848553012 +0200
*** destroy_loop (struct loop *loop)
*** 398,404 
rescan_loop_exit (exit, false, true);
  
for (i = 0; i < nbbs; i++)
! delete_basic_block (bbs[i]);
free (bbs);
  
set_immediate_dominator (CDI_DOMINATORS, dest,
--- 398,425 
rescan_loop_exit (exit, false, true);
  
for (i = 0; i < nbbs; i++)
! {
!   /* We have

Re: [Doc] Add caveat to __builtin_object_size

[Jakub Jelinek]

On Mon, Jun 04, 2012 at 10:34:21AM +0200, Florian Weimer wrote:
> >>+This protection mechanism is only a last resort.  As a programmer, you
> >>+must not rely on its presence, but use explicit buffer length checks
> >>+to avoid buffer overflows.  GCC may not be able to determine buffer
> >>+sizes accurately, and the accuracy depends on compiler version and
> >>+optimization level (currently, at least @option{-O2} is required).
> >
> >That isn't true, at -O1 or -Os it should work just fine too, and
> >to some extent on the compiler side even at -O0.
> 
> Fold needs to run.  Without it, __builtin_object_size constantly

Folding is performed at all optimization levels.
The objsz pass is only performed at -O1 and above (as it is among
optimization passes).  So say on:
char buf[64];
int f1 (void) { return __builtin_object_size (buf, 0); }
int f2 (void) { return __builtin_object_size (&buf[4], 0); }
static inline int __attribute__((always_inline)) f3 (char *p) { return 
__builtin_object_size (p, 0); }
int f4 (void) { return f3 (buf); }
int f5 (void) { return f3 (&buf[4]); }
int f6 (void) { char *q = buf; return f3 (q); }
int f7 (void) { struct S { char *q; } s; s.q = buf; return f3 (s.q); }
at -O0 f1 returns 64 and f2 returns 60, but due to not performing
objsz f[4567] already return -1.  By running the objsz pass
even at -O0 (as done in the attached pass) even f[456] could return
64 resp. 60, though of course as it isn't optimizing, many other
cases wouldn't be determined accurately, e.g f7 due to SRA not being
performed.  Still, it is purely glibc decision not to do fortification
at -O0.

--- gcc/passes.c.jj 2012-06-01 14:41:05.0 +0200
+++ gcc/passes.c2012-06-04 10:49:50.581693645 +0200
@@ -1501,6 +1501,7 @@ init_optimization_passes (void)
   NEXT_PASS (pass_tm_edges);
 }
   NEXT_PASS (pass_lower_complex_O0);
+  NEXT_PASS (pass_object_sizes_O0);
   NEXT_PASS (pass_cleanup_eh);
   NEXT_PASS (pass_lower_resx);
   NEXT_PASS (pass_nrv);
--- gcc/tree-pass.h.jj  2012-05-12 10:21:06.0 +0200
+++ gcc/tree-pass.h 2012-05-12 10:21:06.0 +0200
@@ -411,6 +411,7 @@ extern struct gimple_opt_pass pass_diagn
 extern struct gimple_opt_pass pass_expand_omp;
 extern struct gimple_opt_pass pass_expand_omp_ssa;
 extern struct gimple_opt_pass pass_object_sizes;
+extern struct gimple_opt_pass pass_object_sizes_O0;
 extern struct gimple_opt_pass pass_strlen;
 extern struct gimple_opt_pass pass_fold_builtins;
 extern struct gimple_opt_pass pass_stdarg;
--- gcc/tree-object-size.c.jj   2011-12-16 08:37:45.0 +0100
+++ gcc/tree-object-size.c  2012-06-04 10:52:35.033699016 +0200
@@ -1285,3 +1285,28 @@ struct gimple_opt_pass pass_object_sizes
   TODO_verify_ssa  /* todo_flags_finish */
  }
 };
+
+static bool
+gate_object_sizes_O0 (void)
+{
+  return !optimize;
+}
+
+struct gimple_opt_pass pass_object_sizes_O0 =
+{
+ {
+  GIMPLE_PASS,
+  "objsz0",/* name */
+  gate_object_sizes_O0,/* gate */
+  compute_object_sizes,/* execute */
+  NULL,/* sub */
+  NULL,/* next */
+  0,   /* static_pass_number */
+  TV_NONE, /* tv_id */
+  PROP_cfg | PROP_ssa, /* properties_required */
+  0,   /* properties_provided */
+  0,   /* properties_destroyed */
+  0,   /* todo_flags_start */
+  TODO_verify_ssa  /* todo_flags_finish */
+ }
+};


Jakub

Re: [Doc] Add caveat to __builtin_object_size

[Jakub Jelinek]

On Mon, Jun 04, 2012 at 10:15:35AM +0200, Florian Weimer wrote:
> --- gcc/doc/extend.texi   (revision 187951)
> +++ gcc/doc/extend.texi   (working copy)
> @@ -7376,8 +7376,15 @@
>  @findex __builtin___vfprintf_chk
>  
>  GCC implements a limited buffer overflow protection mechanism
> -that can prevent some buffer overflow attacks.
> +that can prevent some buffer overflow attacks.  GNU libc uses it
> +in the implementation of the @code{_FORTIFY_SOURCE} functionality.
>  
> +This protection mechanism is only a last resort.  As a programmer, you
> +must not rely on its presence, but use explicit buffer length checks
> +to avoid buffer overflows.  GCC may not be able to determine buffer
> +sizes accurately, and the accuracy depends on compiler version and
> +optimization level (currently, at least @option{-O2} is required).

That isn't true, at -O1 or -Os it should work just fine too, and
to some extent on the compiler side even at -O0.
So I'd just replace optimization level.*\. with compiler options.

Jakub

Re: [Doc] Add caveat to __builtin_object_size

[Florian Weimer]

On 06/04/2012 11:01 AM, Jakub Jelinek wrote:

On Mon, Jun 04, 2012 at 10:34:21AM +0200, Florian Weimer wrote:

+This protection mechanism is only a last resort.  As a programmer, you
+must not rely on its presence, but use explicit buffer length checks
+to avoid buffer overflows.  GCC may not be able to determine buffer
+sizes accurately, and the accuracy depends on compiler version and
+optimization level (currently, at least @option{-O2} is required).


That isn't true, at -O1 or -Os it should work just fine too, and
to some extent on the compiler side even at -O0.


Fold needs to run.  Without it, __builtin_object_size constantly


Folding is performed at all optimization levels.
The objsz pass is only performed at -O1 and above (as it is among
optimization passes).


Ah, interesting, thanks.  I think before we can apply this patch, we 
have to benchmark its impact on compile time.  I'll try to do that by 
compiling GCC.  Or are there better benchmarks for such a purpose?


I guess libc should remove the check for optimization, even though the 
fortify checks interfere with debugging a bit (because you have to set 
breakpoints on different libc symbols).


What about my documentation patch for GCC?

--
Florian Weimer / Red Hat Product Security Team

Re: [PATCH] Fix number_of_iterations_cond (PR tree-optimization/53550)

[Richard Guenther]

On Sat, Jun 2, 2012 at 9:48 AM, Jakub Jelinek  wrote:
> Hi!
>
> tree-chrec.c and tree-scalar-evolutions.c uses sizetype as type of
> iv->step, but the following spot would use POINTER_TYPE, which leads
> to ICEs.  Fixed thusly, bootstrapped/regtested onx 86_64-linux and
> i686-linux, ok for trunk/4.7?

Ok.

Thanks,
Richard.

> 2012-06-02  Jakub Jelinek  
>
>        PR tree-optimization/53550
>        * tree-ssa-loop-niter.c (number_of_iterations_cond): If type
>        is POINTER_TYPE_P, use sizetype as step type instead of type.
>
>        * gcc.dg/pr53550.c: New test.
>
> --- gcc/tree-ssa-loop-niter.c.jj        2012-06-01 14:40:59.336398386 +0200
> +++ gcc/tree-ssa-loop-niter.c   2012-06-01 17:52:09.128317401 +0200
> @@ -1275,13 +1275,14 @@ number_of_iterations_cond (struct loop *
>      practice, but it is simple enough to manage.  */
>   if (!integer_zerop (iv0->step) && !integer_zerop (iv1->step))
>     {
> +      tree step_type = POINTER_TYPE_P (type) ? sizetype : type;
>       if (code != NE_EXPR)
>        return false;
>
> -      iv0->step = fold_binary_to_constant (MINUS_EXPR, type,
> +      iv0->step = fold_binary_to_constant (MINUS_EXPR, step_type,
>                                           iv0->step, iv1->step);
>       iv0->no_overflow = false;
> -      iv1->step = build_int_cst (type, 0);
> +      iv1->step = build_int_cst (step_type, 0);
>       iv1->no_overflow = true;
>     }
>
> --- gcc/testsuite/gcc.dg/pr53550.c.jj   2012-06-01 17:52:09.128317401 +0200
> +++ gcc/testsuite/gcc.dg/pr53550.c      2012-06-02 08:53:53.345935193 +0200
> @@ -0,0 +1,12 @@
> +/* PR tree-optimization/53550 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fprefetch-loop-arrays -w" } */
> +
> +int *
> +foo (int *x)
> +{
> +  int *a = x + 10, *b = x, *c = a;
> +  while (b != c)
> +    *--c = *b++;
> +  return x;
> +}
>
>        Jakub

Re: [RFC] Fix SRA with respect of BIT_FIELD_REF

[Richard Guenther]

On Fri, Jun 1, 2012 at 1:44 PM, Martin Jambor  wrote:
> Hi,
>
> On Fri, Jun 01, 2012 at 11:31:20AM +0200, Richard Guenther wrote:
>> On Fri, Jun 1, 2012 at 6:02 AM, Andrew Pinski
>>  wrote:
>> > Hi,
>> >  When I modified GCC to change the majority of bitfield accesses
>> > which were done via component ref to BIT_FIELD_REF, SRA messes up
>> > because when it does the replacement it still tries to use the
>> > BIT_FIELD_REF except it never places the old value in the struct for
>> > the BIT_FIELD_REF to work correctly.
>> >
>> > This patch fixes the problem by expanding what BIT_FIELD_REF does
>> > internally for both writing and reading.  Note we can't use
>> > BIT_FIELD_REF directly on the left hand since we don't support partial
>> > writes yet (except for vector and complex types).
>> >
>> > This is only a RFC since I don't know a way to reproduce this bug on
>> > the trunk. I tested it on x86_64-linux-gnu with no regressions.
>>
>> I'd rather disqualify SRA of BIT_FIELD_REFs on the LHS for now.  My goal
>> was to enable SRA of bitfields using the DECL_BIT_FIELD_REPRESENTATIVE
>> work - something you go against with replacing them with BIT_FIELD_REFs.
>
> SRA of bit-fields works now, it is just rather simple and can't
> optimize the bit-field accesses as we probably should.  Therefore I am
> all for using DECL_BIT_FIELD_REPRESENTATIVEs and looked at those
> patches with interest, I'm just wondering why we'd want to do it for
> non-addressable structures only, which is something SRA would imply if
> this "lowering" was part of it.

SRA would not be the canonical lowering, it would just use the same
lowering way until we implement a generic lowering.  Given that when
there is an SRA opportunity the lowering is certainly profitable.

> BIT_FIELD_REFs of non-vectors on the LHS are not much tested, I'm
> afraid. I it is quite possible it does not do the right thing.
> Nevertheless, making regions accessed through them unscalarizable
> might also be an option, if it does not induce significant penalties
> anywhere.

Indeed.  I'd prefer that for now.  Btw, Andrew, your second patch doesn't
look sensible to me, at least without comments or a testcase to inspect.

Richard.

> Thanks,
>
> Martin
>
>>
>> You'd replace
>>
>>   x = a.b;
>>
>> with
>>
>>   tem = a.;
>>   x = BIT_FIELD_REF ;
>>
>> and for stores with a read-modify-write sequence, possibly adding
>> the bitfield-insert tree I proposed some time ago.  Replace
>>
>>  a.b = x;
>>
>> with
>>
>>  tem = a.
>>  tem = BIT_FIELD_EXPR ;
>>  a. = tem;
>>
>> and I'd do this replacement in SRA for now whenever it would decide to
>> SRA a bitfield.
>>
>> Richard.
>>
>> > Thanks,
>> > Andrew Pinski
>> >
>> > ChangeLog:
>> > * tree-sra.c (sra_modify_expr): Handle BIT_FIELD_REF specially if we
>> > are doing a replacement of the struct with one variable.

Re: [PATCH] vrp: fold ffs to ctz

[Richard Guenther]

On Sat, Jun 2, 2012 at 1:36 PM, Paolo Bonzini  wrote:
> The ffs function can be converted to ctz if the operand is known not
> to be zero, as is the case for example in
>
>    while (x != 0) {
>       bit = ffs(x) - 1;
>       ...
>       x &= ~(1 << bit);
>    }
>
> CSE can already do this on x86, but the attached patch implements this
> folding in VRP in a target-independent way.
>
> Bootstrapped/regtested x86_64-pc-linux-gnu, ok?

+val = compare_range_with_value (NE_EXPR, vr, integer_zero_node, &sop);
+if (!val || !integer_onep (val))
+  return false;

please add a value_range_nonzero_p helper alongside value_range_nonnegative_p.

+fndecl = builtin_decl_implicit (target_builtin_code);
+lhs = gimple_call_lhs (stmt);
+gcc_assert (TREE_TYPE (TREE_TYPE (fndecl)) == TREE_TYPE (lhs));

eh, if you care to check this please fail instead of ICEing ...

Do we always have CTZ if we have FFS?  Can't there be a target that
implements FFS as opcode but not CTZ, so you'd slow down things?
Thus, should the transform be conditonal on target support for CTZ
or no target support for FFS?
Please add a comment to the code as to what transform you are doing
here.

+/* Convert argument type.  */
+argtype = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
+tem = create_tmp_reg (argtype, NULL);
+newop = gimple_build_assign_with_ops (NOP_EXPR, tem, op0, NULL_TREE);
+tem = make_ssa_name (tem, newop);
+gimple_assign_set_lhs (newop, tem);
+gsi_insert_before (gsi, newop, GSI_SAME_STMT);

why is that necessary?  Can you at least wrap it inside a

  if (!useless_type_conversion_p (argtype, TREE_TYPE (op0)))

?

+/* Modify call in place.  */
+tem = create_tmp_reg (TREE_TYPE (lhs), NULL);
+tem = make_ssa_name (tem, stmt);
+gimple_call_set_lhs (stmt, tem);

you should be able to re-use SSA_NAME_VAR of the existing lhs
avoiding the create_tmp_reg.

+/* Sum one to the result.  */
+newop = gimple_build_assign_with_ops (PLUS_EXPR, lhs, tem,
+  fold_convert (TREE_TYPE (lhs),
+integer_one_node));

build_one_cst (TREE_TYPE (lhs)), or build_int_cst (TREE_TYPE (lhs), 1).

Otherwise looks ok to me.

Thanks,
Richard.


>
> Paolo

Re: Document sincos standard pattern name

[Richard Guenther]

On Sun, Jun 3, 2012 at 2:29 PM, Oleg Endo  wrote:
> Hi,
>
> The attached patch adds some documentation for the sincos standard
> pattern name.  Tested with 'make info dvi pdf'.
> Is the text correct and OK to apply?
> Maybe it would also make sense to apply it to the 4.7 branch?

Looks good to me.  Internal docs tweaks are enough to fix on trunk.

Thanks,
Richard.

> Cheers,
> Oleg
>
> ChangeLog:
>
>        * gcc/doc/md.texi (Standard Pattern Names For Generation):
>        Document sincos pattern.
>

Re: [patch] Do not include output.h everywhere

[Richard Guenther]

On Mon, Jun 4, 2012 at 9:00 AM, Steven Bosscher  wrote:
> On Fri, Jun 1, 2012 at 11:19 AM, Richard Guenther
>  wrote:
>>> What about toplev.h?  dump_file is defined there, after all.  I don't mind
>>> it in system.h, if that's not a good place for it now.  The patch is OK
>>> either way.
>>
>> I'd prefer toplev.h, too.  system.h is supposed to be for system header
>> inclusion (and related workarounds).
> (..)
>> Thus, ok with moving to toplev.h instead.
>
> I had already committed this on Friday.
>
> The problem with toplev.h is that it isn't included in any tree-*.c
> file (except tree-dump.c and tree-optimize.c). Also, there is nothing
> "top-level" about an RTL or GIMPLE pass. Perhaps I should create a new
> header with all the dump and statistics stuff?

There is already statistics.h and tree-dump.h, why is dump_file not in
tree-dump.h for example?  I admit it's all some of a mess ;)  But system.h
doesn't look right to me.

Richard.

> Ciao!
> Steven

Re: [patch] More places where not to include output.h

[Richard Guenther]

On Mon, Jun 4, 2012 at 10:38 AM, Steven Bosscher  wrote:
> Hello,
>
> With the attached patch to move some non-output related prototypes
> from output.h to tree.h, another batch of #include output.h cleanups
> is possible.
>
> After this patch, only 9 front-end files (out of 210 *.{c,cc,h} files)
> still include output.h:
>
> c-family/c-cppbuiltin.c:#include "output.h"             /* For
> user_label_prefix.  */
> c-family/c-lex.c:#include "output.h" /* for asm_out_file */
> c-family/c-pch.c:#include "output.h" /* for asm_out_file */
> cp/decl.c:#include "output.h" /* for have_global_bss_p */
> objc/objc-next-runtime-abi-01.c:#include "output.h" /* for asm_out_file */
> ada/gcc-interface/trans.c:#include "output.h"
> ada/gcc-interface/utils.c:#include "output.h"
> go/go-backend.c:#include "output.h"     /* for assemble_string */
> java/class.c:#include "output.h" /* for switch_to_section and get_section */
>
> Bootstrapped (with ada) on powerpc64-unknown-linux-gnu. OK?

Ok.

Thanks,
Richard.

> Ciao!
> Steven

Re: [Doc] Add caveat to __builtin_object_size

[Richard Guenther]

On Mon, Jun 4, 2012 at 11:09 AM, Florian Weimer  wrote:
> On 06/04/2012 11:01 AM, Jakub Jelinek wrote:
>>
>> On Mon, Jun 04, 2012 at 10:34:21AM +0200, Florian Weimer wrote:
>
> +This protection mechanism is only a last resort.  As a programmer, you
> +must not rely on its presence, but use explicit buffer length checks
> +to avoid buffer overflows.  GCC may not be able to determine buffer
> +sizes accurately, and the accuracy depends on compiler version and
> +optimization level (currently, at least @option{-O2} is required).

I think "accurately" is a wrong word here - doesn't that suggest it might
get "wrong"?  The answer should always be conservatively correct, so
it might just be not as constrained as it should.  So I think you need
to elaborate on "accurately" here.


 That isn't true, at -O1 or -Os it should work just fine too, and
 to some extent on the compiler side even at -O0.
>>>
>>>
>>> Fold needs to run.  Without it, __builtin_object_size constantly
>>
>>
>> Folding is performed at all optimization levels.
>> The objsz pass is only performed at -O1 and above (as it is among
>> optimization passes).
>
>
> Ah, interesting, thanks.  I think before we can apply this patch, we have to
> benchmark its impact on compile time.  I'll try to do that by compiling GCC.
>  Or are there better benchmarks for such a purpose?
>
> I guess libc should remove the check for optimization, even though the
> fortify checks interfere with debugging a bit (because you have to set
> breakpoints on different libc symbols).
>
> What about my documentation patch for GCC?
>
>
> --
> Florian Weimer / Red Hat Product Security Team

[PATCH][C++] Fix PR52841

[Richard Guenther]

We are using the following patch from Fabien in our tree to fix
a rejects-valid bug quite appearant in our own codebase.

Re-bootstrap and regtest on the branch head currently running,
ok if that succeeds?

Thanks,
Richard.

2012-06-04  Fabien Chêne  

PR c++/52841
* parser.c (cp_parser_alias_declaration): Return earlier
if an error occured.

* g++.dg/cpp0x/pr52841.C: New testcase.

Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 186200)
+++ gcc/cp/parser.c (working copy)
@@ -15072,6 +15072,9 @@ cp_parser_alias_declaration (cp_parser*
 
   cp_parser_require (parser, CPP_EQ, RT_EQ);
 
+  if (cp_parser_error_occurred (parser))
+return error_mark_node;
+
   /* Now we are going to parse the type-id of the declaration.  */
 
   /*
Index: gcc/testsuite/g++.dg/cpp0x/pr52841.C
===
--- gcc/testsuite/g++.dg/cpp0x/pr52841.C(revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/pr52841.C(revision 0)
@@ -0,0 +1,17 @@
+// { dg-do compile }
+
+struct Solvable;
+namespace sat
+{
+  class Solvable
+{
+  public:
+  typedef bool bool_type;
+};
+}
+
+class Resolvable : public sat::Solvable
+{
+public:
+  using sat::Solvable::bool_type;
+};

Re: [ARM Patch 1/n] PR53447: optimizations of 64bit ALU operation with constant

[Carrot Wei]

Hi

I updated the patch to correct the length of insn adddi3_neon.

thanks
Carrot


2012-06-04  Wei Guozhi  

PR target/53447
* gcc.target/arm/pr53447-1.c: New testcase.


2012-06-04  Wei Guozhi  

PR target/53447
* config/arm/arm-protos.h (const_ok_for_adddi): New prototype.
* config/arm/arm.c (const_ok_for_adddi): New function.
* config/arm/constraints.md (Dd): New constraint.
* config/arm/arm.md (adddi3): Extend it to handle constants.
(arm_adddi3): Likewise.
* config/arm/neon.md (adddi3_neon): Likewise.


Index: testsuite/gcc.target/arm/pr53447-1.c
===
--- testsuite/gcc.target/arm/pr53447-1.c(revision 0)
+++ testsuite/gcc.target/arm/pr53447-1.c(revision 0)
@@ -0,0 +1,8 @@
+/* { dg-options "-O2" }  */
+/* { dg-require-effective-target arm32 } */
+/* { dg-final { scan-assembler-not "mov" } } */
+
+void t0p(long long * p)
+{
+  *p += 0x10001;
+}
Index: config/arm/arm.c
===
--- config/arm/arm.c(revision 187751)
+++ config/arm/arm.c(working copy)
@@ -2497,6 +2497,17 @@
 }
 }

+/* Return TRUE if int I is a valid immediate constant used by pattern
+   arm_adddi3.  */
+int
+const_ok_for_adddi (HOST_WIDE_INT i)
+{
+  HOST_WIDE_INT high = (i >> 32) & 0x;
+  HOST_WIDE_INT low = i & 0x;
+  return (const_ok_for_arm (high)
+ && (const_ok_for_arm (low) || const_ok_for_arm (-low)));
+}
+
 /* Emit a sequence of insns to handle a large constant.
CODE is the code of the operation required, it can be any of SET, PLUS,
IOR, AND, XOR, MINUS;
Index: config/arm/arm-protos.h
===
--- config/arm/arm-protos.h (revision 187751)
+++ config/arm/arm-protos.h (working copy)
@@ -47,6 +47,7 @@
 extern bool arm_small_register_classes_for_mode_p (enum machine_mode);
 extern int arm_hard_regno_mode_ok (unsigned int, enum machine_mode);
 extern bool arm_modes_tieable_p (enum machine_mode, enum machine_mode);
+extern int const_ok_for_adddi (HOST_WIDE_INT);
 extern int const_ok_for_arm (HOST_WIDE_INT);
 extern int const_ok_for_op (HOST_WIDE_INT, enum rtx_code);
 extern int arm_split_constant (RTX_CODE, enum machine_mode, rtx,
Index: config/arm/neon.md
===
--- config/arm/neon.md  (revision 187751)
+++ config/arm/neon.md  (working copy)
@@ -588,9 +588,9 @@
 )

 (define_insn "adddi3_neon"
-  [(set (match_operand:DI 0 "s_register_operand" "=w,?&r,?&r,?w")
-(plus:DI (match_operand:DI 1 "s_register_operand" "%w,0,0,w")
- (match_operand:DI 2 "s_register_operand" "w,r,0,w")))
+  [(set (match_operand:DI 0 "s_register_operand" "=w,?&r,?&r,?w,?&r,?&r,?&r")
+(plus:DI (match_operand:DI 1 "s_register_operand" "%w,0,0,w,r,0,r")
+ (match_operand:DI 2 "reg_or_int_operand" "w,r,0,w,r,Dd,Dd")))
(clobber (reg:CC CC_REGNUM))]
   "TARGET_NEON"
 {
@@ -600,13 +600,16 @@
 case 3: return "vadd.i64\t%P0, %P1, %P2";
 case 1: return "#";
 case 2: return "#";
+case 4: return "#";
+case 5: return "#";
+case 6: return "#";
 default: gcc_unreachable ();
 }
 }
-  [(set_attr "neon_type" "neon_int_1,*,*,neon_int_1")
-   (set_attr "conds" "*,clob,clob,*")
-   (set_attr "length" "*,8,8,*")
-   (set_attr "arch" "nota8,*,*,onlya8")]
+  [(set_attr "neon_type" "neon_int_1,*,*,neon_int_1,*,*,*")
+   (set_attr "conds" "*,clob,clob,*,clob,clob,clob")
+   (set_attr "length" "*,8,8,*,8,8,8")
+   (set_attr "arch" "nota8,*,*,onlya8,*,*,*")]
 )

 (define_insn "*sub3_neon"
Index: config/arm/constraints.md
===
--- config/arm/constraints.md   (revision 187751)
+++ config/arm/constraints.md   (working copy)
@@ -29,7 +29,7 @@
 ;; in Thumb-1 state: I, J, K, L, M, N, O

 ;; The following multi-letter normal constraints have been used:
-;; in ARM/Thumb-2 state: Da, Db, Dc, Dn, Dl, DL, Dv, Dy, Di, Dt, Dz
+;; in ARM/Thumb-2 state: Da, Db, Dc, Dd, Dn, Dl, DL, Dv, Dy, Di, Dt, Dz
 ;; in Thumb-1 state: Pa, Pb, Pc, Pd, Pe
 ;; in Thumb-2 state: Pj, PJ, Ps, Pt, Pu, Pv, Pw, Px, Py

@@ -251,6 +251,12 @@
   (match_test "TARGET_32BIT && arm_const_double_inline_cost (op) == 4
   && !(optimize_size || arm_ld_sched)")))

+(define_constraint "Dd"
+ "@internal
+ In ARM/Thumb-2 state a const_int that can be used by insn adddi."
+ (and (match_code "const_int")
+  (match_test "TARGET_32BIT && const_ok_for_adddi (ival)")))
+
 (define_constraint "Di"
  "@internal
   In ARM/Thumb-2 state a const_int or const_double where both the high
Index: config/arm/arm.md
===
--- config/arm/arm.md   (revision 187751)
+++ config/arm/arm.md   (working copy)
@@ -574,10 +574,21 @@
  [(parallel
[(set (match_opera

Re: [Doc] Add caveat to __builtin_object_size

[Florian Weimer]

On 06/04/2012 11:46 AM, Richard Guenther wrote:

I think "accurately" is a wrong word here - doesn't that suggest it might
get "wrong"?  The answer should always be conservatively correct, so
it might just be not as constrained as it should.  So I think you need
to elaborate on "accurately" here.


It's wrong from the consumer point of view (i.e., the value returned by 
__builtin_object_size is too large, and one could argue that it is not 
conservative enough).  It's not wrong according to the specification, 
according to which (size_t)-1 is always correct (and thus a conservative 
result).  This mismatch is the main reason why _FORTIFY_SOURCE is a last 
resort mechanism only.


What about this?

+This protection mechanism is only a last resort.  As a programmer, you
+must not rely on its presence, but use explicit buffer length checks
+to avoid buffer overflows.  GCC may not be able to determine object
+sizes with sufficient precision (that is, the reported sizes can be
+larger than what is actually available at run time), and the accuracy
+depends on compiler version and options.

In a sense, GCC *is* wrong, but of course, the problem is unsolvable in 
the general case.


--
Florian Weimer / Red Hat Product Security Team

Re: [Doc] Add caveat to __builtin_object_size

[Jakub Jelinek]

On Mon, Jun 04, 2012 at 11:59:59AM +0200, Florian Weimer wrote:
> What about this?
> 
> +This protection mechanism is only a last resort.  As a programmer, you
> +must not rely on its presence, but use explicit buffer length checks
> +to avoid buffer overflows.  GCC may not be able to determine object
> +sizes with sufficient precision (that is, the reported sizes can be
> +larger than what is actually available at run time), and the accuracy
> +depends on compiler version and options.

That isn't correct.  __builtin_object_size, as documented a few lines below
what you are editing, is documented to have 4 different possible values of
second argument, and it can be larger than what is available only for values
0 and 1, for values 2 and 3 it must be equal or smaller only, and if you
want to test whether it is precise, you can compare results of two
__builtin_object_size calls.

> In a sense, GCC *is* wrong, but of course, the problem is unsolvable
> in the general case.

In a very weird sense.  The primary design goal of this feature is that
it is very cheap lightweight check, so no mudflap or similar instrumenting
of all pointers, changing the ABI of all function calls that pass around
pointers, changing the layout of structures etc. and no large on the side
hash tables.  Even for simple:
  char buf[64];
  strcpy (buf + (foo ? 4 : 8), bar);
__builtin_object_size can only return one value, so that would be
60 as upper bound and 56 as lower bound in the ideal case here.

Jakub

Re: [Doc] Add caveat to __builtin_object_size

[Richard Guenther]

On Mon, Jun 4, 2012 at 11:59 AM, Florian Weimer  wrote:
> On 06/04/2012 11:46 AM, Richard Guenther wrote:
>>
>> I think "accurately" is a wrong word here - doesn't that suggest it might
>> get "wrong"?  The answer should always be conservatively correct, so
>> it might just be not as constrained as it should.  So I think you need
>> to elaborate on "accurately" here.
>
>
> It's wrong from the consumer point of view (i.e., the value returned by
> __builtin_object_size is too large, and one could argue that it is not
> conservative enough).  It's not wrong according to the specification,
> according to which (size_t)-1 is always correct (and thus a conservative
> result).  This mismatch is the main reason why _FORTIFY_SOURCE is a last
> resort mechanism only.
>
> What about this?
>
>
> +This protection mechanism is only a last resort.  As a programmer, you
> +must not rely on its presence, but use explicit buffer length checks
> +to avoid buffer overflows.  GCC may not be able to determine object
> +sizes with sufficient precision (that is, the reported sizes can be
> +larger than what is actually available at run time), and the accuracy
> +depends on compiler version and options.
>
> In a sense, GCC *is* wrong, but of course, the problem is unsolvable in the
> general case.

Looks better.

Thanks,
Richard.

>
> --
> Florian Weimer / Red Hat Product Security Team

Re: [PATCH] Fix PR53471, remove DECL_ASSEMBLER_NAME deferred compute

[Richard Guenther]

On Mon, 4 Jun 2012, Richard Guenther wrote:

> On Fri, 1 Jun 2012, Jason Merrill wrote:
> 
> > Or use your first patch, and decide that we don't care about the linkage 
> > name
> > of unreachable types.  What types are affected by this, anyway?

So like the following.

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

Thanks,
Richard.

2012-06-04  Richard Guenther  

PR middle-end/53471
* dwarf2out.c (dwarf2out_finish): If generating LTO do not
create new assembler names.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 188168)
+++ gcc/dwarf2out.c (working copy)
@@ -22158,7 +22158,8 @@ dwarf2out_finish (const char *filename)
   for (node = deferred_asm_name; node; node = node->next)
 {
   tree decl = node->created_for;
-  if (DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl))
+  if ((!flag_generate_lto || DECL_ASSEMBLER_NAME_SET_P (decl))
+ && DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl))
{
  add_linkage_attr (node->die, decl);
  move_linkage_attr (node->die);

Re: [Doc] Add caveat to __builtin_object_size

[Florian Weimer]

On 06/04/2012 12:11 PM, Jakub Jelinek wrote:

On Mon, Jun 04, 2012 at 11:59:59AM +0200, Florian Weimer wrote:

What about this?

+This protection mechanism is only a last resort.  As a programmer, you
+must not rely on its presence, but use explicit buffer length checks
+to avoid buffer overflows.  GCC may not be able to determine object
+sizes with sufficient precision (that is, the reported sizes can be
+larger than what is actually available at run time), and the accuracy
+depends on compiler version and options.


That isn't correct.  __builtin_object_size, as documented a few lines below
what you are editing, is documented to have 4 different possible values of
second argument, and it can be larger than what is available only for values
0 and 1, for values 2 and 3 it must be equal or smaller only, and if you
want to test whether it is precise, you can compare results of two
__builtin_object_size calls.


For the explanation why this feature is unreliable and you still need 
manual bounds checks, the upper-bound-is-too-large case is the 
interesting one.  I don't think libc uses __builtin_object_size to 
compute lower bounds.


Hmm.  I'd like to remove the remark in parentheses and explain this in 
the libc documentation.


--
Florian Weimer / Red Hat Product Security Team

[C++] Return error_mark_node from cp_parser_constant_expression

[Florian Weimer]

Without this change, cp_parser_constant_expression reports errors in 
C++11 mode, but does not provide any indication to callers, which 
continue processing and produce misleading errors.


The changes to check_bitfield_decl and build_enumerator are required for 
identical error report in C++11 and C++98 mode.  This removes several 
spurious errors, and the testsuite is adjusted accordingly.


Bootstrapped on x86_64-linux-gnu with C, C++, TLO enabled, "make 
check-c++" passes with no regressions.


(Sorry if Thunderbird has garbled the changelog entries.)

2012-06-04  Florian Weimer  

* parser.c (cp_parser_constant_expression):
  Return error_mark_node on error if !allow_non_constant_p.
* class.c (check_bitfield_decl): Handle error_mark_node in
  width.
* decl.c (build_enumerator): Handle error_mark_node in value.


2012-06-04  Florian Weimer  

* g++.dg/warn/overflow-warn-4.C, g++.dg/cpp0x/pr51225.C,
g++.dg/cpp0x/regress/error-recovery1.C: Adjust error messages.


Index: gcc/testsuite/g++.dg/warn/overflow-warn-4.C
===
--- gcc/testsuite/g++.dg/warn/overflow-warn-4.C	(revision 188104)
+++ gcc/testsuite/g++.dg/warn/overflow-warn-4.C	(working copy)
@@ -20,11 +20,9 @@ enum e {
   /* { dg-error "enumerator value for 'E4' is not an integer constant" "enum error" { xfail *-*-* } 19 } */
   E5 = INT_MAX + 1, /* { dg-warning "integer overflow in expression" } */
   /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 21 } */
-  /* { dg-error "enumerator value for 'E5' is not an integer constant" "enum error" { target *-*-* } 21 } */
   /* Again, overflow in evaluated subexpression.  */
   E6 = 0 * (INT_MAX + 1), /* { dg-warning "integer overflow in expression" } */
-  /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 25 } */
-  /* { dg-error "enumerator value for 'E6' is not an integer constant" "enum error" { target *-*-* } 25 } */
+  /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 24 } */
   /* A cast does not constitute overflow in conversion.  */
   E7 = (char) INT_MAX
 };
@@ -33,8 +31,7 @@ struct s {
   int a;
   int : 0 * (1 / 0); /* { dg-warning "division by zero" } */
   int : 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */
-  /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 35 } */
-  /* { dg-error "bit-field .* width not an integer constant" "" { target *-*-* } 35 } */
+  /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 33 } */
 };
 
 void
@@ -56,10 +53,10 @@ void *n = 0;
constants.  The third has the overflow in an unevaluated
subexpression, so is a null pointer constant.  */
 void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */
-/* { dg-error "invalid conversion from 'int' to 'void" "null" { target *-*-* } 58 } */
+/* { dg-error "invalid conversion from 'int' to 'void" "null" { target *-*-* } 55 } */
 
 void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
-/* { dg-error "invalid conversion from 'int' to 'void*'" "null" { xfail *-*-* } 61 } */
+/* { dg-error "invalid conversion from 'int' to 'void*'" "null" { xfail *-*-* } 58 } */
 void *r = (1 ? 0 : INT_MAX+1); /* { dg-bogus "integer overflow in expression" "" { xfail *-*-* } } */
 
 void
@@ -70,7 +67,7 @@ g (int i)
 case 0 * (1/0): /* { dg-warning "division by zero" } */
   ;
 case 1 + 0 * (INT_MAX + 1): /* { dg-warning "integer overflow in expression" } */
-  /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 72 } */
+  /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 69 } */
   ;
 }
 }
Index: gcc/testsuite/g++.dg/cpp0x/regress/error-recovery1.C
===
--- gcc/testsuite/g++.dg/cpp0x/regress/error-recovery1.C	(revision 188104)
+++ gcc/testsuite/g++.dg/cpp0x/regress/error-recovery1.C	(working copy)
@@ -7,5 +7,3 @@ foo ()
   const bool b =;		// { dg-error "" }
   foo < b > ();			// { dg-error "constant expression" }
 };
-
-// { dg-error "no match" "" { target *-*-* } 8 }
Index: gcc/testsuite/g++.dg/cpp0x/pr51225.C
===
--- gcc/testsuite/g++.dg/cpp0x/pr51225.C	(revision 188104)
+++ gcc/testsuite/g++.dg/cpp0x/pr51225.C	(working copy)
@@ -12,3 +12,5 @@ template struct bar
 {
   static constexpr A<1> b = A<1>(x); // { dg-error "not declared" }
 };
+
+// { dg-error "template argument.*invalid" "" { target *-*-* } 8 }
Index: gcc/cp/init.c
===
--- gcc/cp/init.c	(revision 188104)
+++ gcc/cp/init.c	(working copy)
@@ -2805,6 +2805,34 @@ build_new (VEC(tree,gc) **placement, tree type, tr
   make_args_non_dependent (*init);
 }
 
+  /* If the type is variably modified (a GNU ex

Re: [patch] Do not include output.h everywhere

[Steven Bosscher]

On Mon, Jun 4, 2012 at 11:42 AM, Richard Guenther
 wrote:
> There is already statistics.h and tree-dump.h, why is dump_file not in
> tree-dump.h for example?  I admit it's all some of a mess ;)

Because tree-dump.h is about tree dumps (and gimple dumps). Yes, it is a mess.

> But system.h
> doesn't look right to me.

Fully agreed there.

I'll see if I can come up with a plan to remove some of this mess.

Ciao!
Steven

Re: [PATCH 2/2] Better system header location detection for built-in macro tokens

[Dodji Seketeli]

> > -  s = s; // { dg-message "synthesized|deleted" }
> > +  s = s;
> 
> Why do we lose this error?

Good question.

This is the result of a weird behaviour of DejaGNU, I think.

For the record, here is the whole test:

 1  // { dg-do compile  }
 2  // Origin: Mark Mitchell 
 3  
 4  template 
 5  struct S {  // { dg-error "const|operator=" }
 6S();
 7T t;
 8  };
 9  
10  void f()
11  {
12S s;
13s = s; // { dg-message "synthesized|deleted" }
14  }

And here is the output of the test, when run in trunk, without my
patch: http://people.redhat.com/dseketel/gcc/master-g++.log

Then, look at the output with the patch:
http://people.redhat.com/dseketel/gcc/PR53463-g++.log.

It says:

FAIL: g++.old-deja/g++.pt/assign1.C -std=c++98  (test for warnings, line 13)

But in the compiler output, we duly have:

assign1.C:13:5: note: synthesized method 'S& S::operator=(const
S&)' first required here 

which seems correct and shouldn't have triggered a FAIL there, IMHO.
Which, after much punching in my TCL darkness, brings me to this
theory:

It seems the { dg-error "const|operator=" } at line 5 is "swallowing"
somehow the compiler output above.  Why exactly? I have no idea.

One thing I noted though, is that there is no error emitted by g++ on
line 5 that has the string "operator=".  There is just one "note" with
that string.  So the "|operator=" seems superfluous there.  Note
however that the note above, on line 13, has the string "operator=".

Incidentally, changing that DejaGNU directive at line 5 with the more
specific:

// { dg-error "const member\[^\n\r\]*can't use default assignment operator" 
}

makes the whole shebang work.

And of course, I sent you the wrong patch.  The one I sent you does
pass testing, but it was just an attempt for me to understand the
issue.  The patch below is the one that uses the more specific DejaGNU
directive above.

libcpp/

PR preprocessor/53463
* line-map.c (linemap_location_in_system_header_p): For built-in
macro tokens, check the first expansion point location for that is
not for a token coming from a built-in macro.

gcc/cp/

PR preprocessor/53463
* parser.c (cp_parser_assignment_expression): Use the location
for the LHS as the default location for the expression.

gcc/testsuite/

PR preprocessor/53463
* g++.dg/cpp/limits.C: New test.
* g++.dg/parse/error19.C: Adjust.
* g++.dg/warn/Wconversion-real-integer2.C: Likewise.
* g++.dg/warn/pr35635.C: Likewise.
* g++.old-deja/g++.pt/assign1.C: Likewise.
---
 gcc/cp/parser.c|4 ++-
 gcc/testsuite/g++.dg/cpp/limits.C  |   21 ++
 gcc/testsuite/g++.dg/parse/error19.C   |2 +-
 .../g++.dg/warn/Wconversion-real-integer2.C|4 +-
 gcc/testsuite/g++.dg/warn/pr35635.C|4 +-
 gcc/testsuite/g++.old-deja/g++.pt/assign1.C|2 +-
 libcpp/line-map.c  |   30 +--
 7 files changed, 56 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp/limits.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 9fd8c84..e393f48 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7491,7 +7491,9 @@ cp_parser_assignment_expression (cp_parser* parser, bool 
cast_p,
  if (cp_parser_non_integral_constant_expression (parser,
  NIC_ASSIGNMENT))
return error_mark_node;
- /* Build the assignment expression.  */
+ /* Build the assignment expression.  Its default
+location is the location of the '=' token.  */
+ input_location = loc;
  expr = build_x_modify_expr (loc, expr,
  assignment_operator,
  rhs,
diff --git a/gcc/testsuite/g++.dg/cpp/limits.C 
b/gcc/testsuite/g++.dg/cpp/limits.C
new file mode 100644
index 000..b64e1e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp/limits.C
@@ -0,0 +1,21 @@
+// { dg-options "-pedantic" }
+// { dg-do compile }
+
+#include 
+
+// Compiling this with -pedantic was wrongly triggering this error:
+// libstdc++-v3/include/limits:1269:45: warning : use of C++0x long long 
integer constant [-Wlong-long]
+//   min() _GLIBCXX_USE_NOEXCEPT { return -__LONG_LONG_MAX__ - 1; }
+// ^
+// libstdc++-v3/include/limits:1272:44: warning : use of C++0x long long 
integer constant [-Wlong-long]
+//   max() _GLIBCXX_USE_NOEXCEPT { return __LONG_LONG_MAX__; }
+//^
+// libstdc++-v3/include/limits:1342:44: warning : use of C++0x long long 
integer constant [-Wlong-long]
+//   max() _GLIBCXX_USE_NOEXCEPT { return __LONG_LONG_MAX__ * 2ULL + 

Re: [C++ Patch] for c++/51214

[Fabien Chêne]

2012/6/3 Gabriel Dos Reis :
> On Sun, Jun 3, 2012 at 10:56 AM, Fabien Chêne  wrote:
>> 2012/6/3 Jason Merrill :
>>> On 05/24/2012 09:18 AM, Jason Merrill wrote:

 OK, thanks.
>>>
>>>
>>> I notice you haven't checked the patch in yet, is there a problem?
>>
>> Not at all, just lack of time, so many problems/holidays to tackle at
>> the moment... That is May month in France ;-)
>
> It must be distressing to make up for those 35 hours ;-p

Provided we do not respond as an ideal gas (P = nRT/V), it should be fine ;-)

-- 
Fabien

Re: PATCH: PR target/53539: Different __WCHAR_TYPE__/wchar_t for gcc -m32 on Linux/i386 and Linux/x86-64

[H.J. Lu]

On Thu, May 31, 2012 at 7:10 AM, H.J. Lu  wrote:
> On Thu, May 31, 2012 at 6:50 AM, Jakub Jelinek  wrote:
>> On Thu, May 31, 2012 at 06:35:19AM -0700, H.J. Lu wrote:
>>> This patch makes __WCHAR_TYPE__/wchar_t the same for gcc -m32 on
>>> Linux/i386 and Linux/x86-64.  OK for trunk?
>>
>> That looks wrong.  For Linux/i386, the 32-bit only compiler should be
>> the standard, rather than x86_64 -m32 if they differ.
>> So, IMHO you should keep gnu-user.h as is, and just use
>> #define WCHAR_TYPE (TARGET_64BIT ? "int" : "long int")
>> in gcc/config/i386/x86-64.h.
>>
>
> You are right on Linux/i386.  But we should change config/i386/gnu-user64.h,
> not x86-64.h since most of other i386 OSes use int for wchar.  Also we should
> check TARGET_LP64 to keep x32 as close to i386 as possible.  OK for
> trunk and 4.7?
>
> Thanks.

Hi  Richard, Honza, Uros,

Is this patch OK for trunk and 4.7?

H.J.
--
>
> --
> H.J.
> 
> 2012-05-31  H.J. Lu  
>
>        PR target/53539
>        * config/i386/gnu-user64.h (WCHAR_TYPE): Use "int" only for
>        TARGET_LP64.
>
> diff --git a/gcc/config/i386/gnu-user64.h b/gcc/config/i386/gnu-user64.h
> index 0e66d26..2941332 100644
> --- a/gcc/config/i386/gnu-user64.h
> +++ b/gcc/config/i386/gnu-user64.h
> @@ -91,3 +91,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If n
> ot, see
>  #define TARGET_THREAD_SPLIT_STACK_OFFSET \
>   (TARGET_64BIT ? (TARGET_X32 ? 0x40 : 0x70) : 0x30)
>  #endif
> +
> +#undef WCHAR_TYPE
> +#define WCHAR_TYPE (TARGET_LP64 ? "int" : "long int")



-- 
H.J.

Re: [PATCH 2/2] Better system header location detection for built-in macro tokens

[Jason Merrill]

On 06/04/2012 08:36 AM, Dodji Seketeli wrote:

- /* Build the assignment expression.  */
+ /* Build the assignment expression.  Its default
+location is the location of the '=' token.  */
+ input_location = loc;
  expr = build_x_modify_expr (loc, expr,
  assignment_operator,
  rhs,


Hmm, I meant to say this before, but apparently forgot: We should the 
old input_location after build_x_modify_expr returns.



This is the result of a weird behaviour of DejaGNU, I think.


Weird, I don't see how that could happen.  The full regexp in dg.exp is

(^|\n)(\[^\n\]+$line\[^\n\]*($pattern)\[^\n\]*\n?)+

I don't see how that could swallow a newline and some of another 
diagnostic.  But your workaround is OK.


Jason

Re: [PATCH] Fix PR53471, remove DECL_ASSEMBLER_NAME deferred compute

[Jason Merrill]

Please add a comment explaining why flag_generate_lto matters there.  OK 
with that change.


Jason

[m68k] Fix check for CASE_VECTOR_PC_RELATIVE

[Andreas Schwab]

When the default for CASE_VECTOR_PC_RELATIVE was added to defaults.h
back in 2004 the check in m68k.md wasn't updated.

Andreas.

PR target/53461
* config/m68k/m68k.md ("tablejump"): Fix check for
CASE_VECTOR_PC_RELATIVE.

Index: config/m68k/m68k.md
===
--- config/m68k/m68k.md (revision 188178)
+++ config/m68k/m68k.md (working copy)
@@ -6899,7 +6899,7 @@
  (use (label_ref (match_operand 1 "" "")))])]
   ""
 {
-#ifdef CASE_VECTOR_PC_RELATIVE
+#if CASE_VECTOR_PC_RELATIVE
 operands[0] = gen_rtx_PLUS (SImode, pc_rtx,
gen_rtx_SIGN_EXTEND (SImode, operands[0]));
 #endif
-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[PATCH][6/n] loop distribution TLC

[Richard Guenther]

This removes a pointless stmt -> RDG index hash (simply use gimple_uid)
and arranges to compute and analyze data-references once instead of
O(stmts^4) when looking for common data accesses (ugh!).

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

Richard.

2012-06-04  Richard Guenther  

* tree-data-ref.c (struct rdg_vertex_info): Remove.
(rdg_vertex_for_stmt): Simplify using gimple_uid.
(create_rdg_vertices): Pass loop argument, remove stmt to RDG index
hashtable.   Record stmt data-references.
(hash_stmt_vertex_info): Remove.
(eq_stmt_vertex_info): Likewise.
(hash_stmt_vertex_del): Likewise.
(build_empty_rdg): Simplify.
(build_rdg): Adjust.
(free_rdg): Likewise.
(ref_base_address): Remove.
(have_similar_memory_accesses): Likewise.
* tree-data-ref.h (create_rdg_vertices): Remove.
(struct rdg_vertex): Add datarefs member.
(RDGV_DATAREFS): New define.
(RDG_DATAREFS): Likewise.
(have_similar_memory_accesses): Remove.
(rdg_has_similar_memory_accesses): Likewise.
* tree-loop-distribution.c (ref_base_address): Re-implement here.
(similar_memory_accesses): Re-implement using existing data-references.
(tree_loop_distribution): Initialize stmt uids for the stmt to
RDG index mapping.
* tree-vect-loop.c (vect_create_epilog_for_reduction): Only
access stmt vinfo for stmts in loop.

Index: trunk/gcc/tree-data-ref.c
===
*** trunk.orig/gcc/tree-data-ref.c  2012-06-04 15:09:18.0 +0200
--- trunk/gcc/tree-data-ref.c   2012-06-04 15:09:40.605652827 +0200
*** dot_rdg (struct graph *rdg)
*** 4924,4952 
  #endif
  }
  
- /* This structure is used for recording the mapping statement index in
-the RDG.  */
- 
- struct GTY(()) rdg_vertex_info
- {
-   gimple stmt;
-   int index;
- };
- 
  /* Returns the index of STMT in RDG.  */
  
  int
! rdg_vertex_for_stmt (struct graph *rdg, gimple stmt)
  {
!   struct rdg_vertex_info rvi, *slot;
! 
!   rvi.stmt = stmt;
!   slot = (struct rdg_vertex_info *) htab_find (rdg->indices, &rvi);
! 
!   if (!slot)
! return -1;
! 
!   return slot->index;
  }
  
  /* Creates an edge in RDG for each distance vector from DDR.  The
--- 4924,4937 
  #endif
  }
  
  /* Returns the index of STMT in RDG.  */
  
  int
! rdg_vertex_for_stmt (struct graph *rdg ATTRIBUTE_UNUSED, gimple stmt)
  {
!   int index = gimple_uid (stmt);
!   gcc_checking_assert (index == -1 || RDG_STMT (rdg, index) == stmt);
!   return index;
  }
  
  /* Creates an edge in RDG for each distance vector from DDR.  The
*** create_rdg_edges (struct graph *rdg, VEC
*** 5041,5048 
  
  /* Build the vertices of the reduced dependence graph RDG.  */
  
! void
! create_rdg_vertices (struct graph *rdg, VEC (gimple, heap) *stmts)
  {
int i, j;
gimple stmt;
--- 5026,5033 
  
  /* Build the vertices of the reduced dependence graph RDG.  */
  
! static void
! create_rdg_vertices (struct graph *rdg, VEC (gimple, heap) *stmts, loop_p 
loop)
  {
int i, j;
gimple stmt;
*** create_rdg_vertices (struct graph *rdg,
*** 5052,5084 
VEC (data_ref_loc, heap) *references;
data_ref_loc *ref;
struct vertex *v = &(rdg->vertices[i]);
-   struct rdg_vertex_info *rvi = XNEW (struct rdg_vertex_info);
-   struct rdg_vertex_info **slot;
  
!   rvi->stmt = stmt;
!   rvi->index = i;
!   slot = (struct rdg_vertex_info **) htab_find_slot (rdg->indices, rvi, 
INSERT);
! 
!   if (!*slot)
!   *slot = rvi;
!   else
!   free (rvi);
  
v->data = XNEW (struct rdg_vertex);
!   RDG_STMT (rdg, i) = stmt;
! 
!   RDG_MEM_WRITE_STMT (rdg, i) = false;
!   RDG_MEM_READS_STMT (rdg, i) = false;
if (gimple_code (stmt) == GIMPLE_PHI)
continue;
  
get_references_in_stmt (stmt, &references);
FOR_EACH_VEC_ELT (data_ref_loc, references, j, ref)
!   if (!ref->is_read)
! RDG_MEM_WRITE_STMT (rdg, i) = true;
!   else
! RDG_MEM_READS_STMT (rdg, i) = true;
! 
VEC_free (data_ref_loc, heap, references);
  }
  }
--- 5037,5067 
VEC (data_ref_loc, heap) *references;
data_ref_loc *ref;
struct vertex *v = &(rdg->vertices[i]);
  
!   /* Record statement to vertex mapping.  */
!   gimple_set_uid (stmt, i);
  
v->data = XNEW (struct rdg_vertex);
!   RDGV_STMT (v) = stmt;
!   RDGV_DATAREFS (v) = NULL;
!   RDGV_HAS_MEM_WRITE (v) = false;
!   RDGV_HAS_MEM_READS (v) = false;
if (gimple_code (stmt) == GIMPLE_PHI)
continue;
  
get_references_in_stmt (stmt, &references);
FOR_EACH_VEC_ELT (data_ref_loc, references, j, ref)
!   {
! data_reference_p dr;
! if (!ref->is_read)
!   RDGV_HAS_MEM_WRITE (v) = tru

Re: [PATCH] Fix PR53471, remove DECL_ASSEMBLER_NAME deferred compute

[Richard Guenther]

On Mon, 4 Jun 2012, Jason Merrill wrote:

> Please add a comment explaining why flag_generate_lto matters there.  OK with
> that change.

Committed as follows.

Thanks,
Richard.

2012-06-04  Richard Guenther  

PR middle-end/53471
* dwarf2out.c (dwarf2out_finish): If generating LTO do not
create new assembler names.

Index: gcc/dwarf2out.c
===
*** gcc/dwarf2out.c (revision 188176)
--- gcc/dwarf2out.c (working copy)
*** dwarf2out_finish (const char *filename)
*** 22158,22164 
for (node = deferred_asm_name; node; node = node->next)
  {
tree decl = node->created_for;
!   if (DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl))
{
  add_linkage_attr (node->die, decl);
  move_linkage_attr (node->die);
--- 22158,22168 
for (node = deferred_asm_name; node; node = node->next)
  {
tree decl = node->created_for;
!   /* When generating LTO bytecode we can not generate new assembler
!  names at this point and all important decls got theirs via
!free-lang-data.  */
!   if ((!flag_generate_lto || DECL_ASSEMBLER_NAME_SET_P (decl))
! && DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl))
{
  add_linkage_attr (node->die, decl);
  move_linkage_attr (node->die);

Re: [google] Add options to pattern match function name for hotness attributes

[Jan Hubicka]

> On Sat, Jun 2, 2012 at 11:11 AM, Jan Hubicka  wrote:
> >> Actually Dehao also plans to teach the static predictor to understand
> >> standard library functions more (e.g IO functions) and add more naming
> >
> > How this differ from annotating the library?
> 
> I find them more suitable to be compiler heuristic than being
> function's attribute -- attribute is a much stronger assertion.

Why so? Special matching the function names in compiler is pretty much
the same thing as annotating them in the source, right?
> 
> >
> > There are indeed quite some possibilities to do about library calls
> >
> > One thing I always wondered about is how to tell predictor that paths 
> > containing
> > heavy IO functions don't need to be really opimized to death, since their 
> > execution
> > time is dominated by the IO...
> >
> 
> Yes -- if branch predictor does the right thing and if function
> splitter is powerful enough, the IO code can be outlined and optimized
> for size :)

We have infrastructure for optimizing for size regions of functions, so this 
should
be easy. With flag_reorder_blocks_and_partition we can even "outline"
but problem with these things is that they do not really match the usual vision
of profile prediction...

Honza
> 
> 
> >> based heuristics such as 'error, success, failure, fatal etc).
> >
> > yeah, this is also mentioned by some branch prediction papers. It is bit 
> > kludgy
> > in nature (i.e. it won't understand programs written in Czech language) but 
> > it
> > is an heuristics after all.
> >
> 
> right.
> 
> thanks,
> 
> David
> 
> > Honza
> >>
> >> thanks,
> >>
> >> David
> >>
> >> > Honza
> >> >>
> >> >> thanks,
> >> >>
> >> >> David
> >> >> > Honza

Re: [PATCH] Hoist adjacent pointer loads

[William J. Schmidt]

Hi Richard,

Here's a revision of the hoist-adjacent-loads patch.  I'm sorry for the
delay since the last revision, but my performance testing has been
blocked waiting for a fix to PR53487.  I ended up applying a test
version of the patch to 4.7 and ran performance numbers with that
instead, with no degradations.

In addition to addressing your comments, this patch contains one bug fix
where local_mem_dependence was called on the wrong blocks after swapping
def1 and def2.

Bootstrapped with no regressions on powerpc64-unknown-linux-gnu.  Is
this version ok for trunk?  I won't commit it until I can do final
testing on trunk in conjunction with a fix for PR53487.

Thanks,
Bill


2012-06-04  Bill Schmidt  

* opts.c: Add -fhoist_adjacent_loads to -O2 and above.
* tree-ssa-phiopt.c (tree_ssa_phiopt_worker): Add argument to forward
declaration.
(hoist_adjacent_loads, gate_hoist_loads): New forward declarations.
(tree_ssa_phiopt): Call gate_hoist_loads.
(tree_ssa_cs_elim): Add parm to tree_ssa_phiopt_worker call.
(tree_ssa_phiopt_worker): Add do_hoist_loads to formal arg list; call
hoist_adjacent_loads.
(local_mem_dependence): New function.
(hoist_adjacent_loads): Likewise.
(gate_hoist_loads): Likewise.
* common.opt (fhoist-adjacent-loads): New switch.
* Makefile.in (tree-ssa-phiopt.o): Added dependencies.
* params.def (PARAM_MIN_CMOVE_STRUCT_ALIGN): New param.


Index: gcc/opts.c
===
--- gcc/opts.c  (revision 187805)
+++ gcc/opts.c  (working copy)
@@ -489,6 +489,7 @@ static const struct default_options default_option
 { OPT_LEVELS_2_PLUS, OPT_falign_functions, NULL, 1 },
 { OPT_LEVELS_2_PLUS, OPT_ftree_tail_merge, NULL, 1 },
 { OPT_LEVELS_2_PLUS_SPEED_ONLY, OPT_foptimize_strlen, NULL, 1 },
+{ OPT_LEVELS_2_PLUS, OPT_fhoist_adjacent_loads, NULL, 1 },
 
 /* -O3 optimizations.  */
 { OPT_LEVELS_3_PLUS, OPT_ftree_loop_distribute_patterns, NULL, 1 },
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c   (revision 187805)
+++ gcc/tree-ssa-phiopt.c   (working copy)
@@ -37,9 +37,17 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "tree-data-ref.h"
 #include "tree-pretty-print.h"
+#include "gimple-pretty-print.h"
+#include "insn-config.h"
+#include "expr.h"
+#include "optabs.h"
 
+#ifndef HAVE_conditional_move
+#define HAVE_conditional_move (0)
+#endif
+
 static unsigned int tree_ssa_phiopt (void);
-static unsigned int tree_ssa_phiopt_worker (bool);
+static unsigned int tree_ssa_phiopt_worker (bool, bool);
 static bool conditional_replacement (basic_block, basic_block,
 edge, edge, gimple, tree, tree);
 static int value_replacement (basic_block, basic_block,
@@ -53,6 +61,9 @@ static bool cond_store_replacement (basic_block, b
 static bool cond_if_else_store_replacement (basic_block, basic_block, 
basic_block);
 static struct pointer_set_t * get_non_trapping (void);
 static void replace_phi_edge_with_variable (basic_block, edge, gimple, tree);
+static void hoist_adjacent_loads (basic_block, basic_block,
+ basic_block, basic_block);
+static bool gate_hoist_loads (void);
 
 /* This pass tries to replaces an if-then-else block with an
assignment.  We have four kinds of transformations.  Some of these
@@ -138,12 +149,56 @@ static void replace_phi_edge_with_variable (basic_
  bb2:
x = PHI ;
 
-   A similar transformation is done for MAX_EXPR.  */
+   A similar transformation is done for MAX_EXPR.
 
+
+   This pass also performs a fifth transformation of a slightly different
+   flavor.
+
+   Adjacent Load Hoisting
+   --
+   
+   This transformation replaces
+
+ bb0:
+   if (...) goto bb2; else goto bb1;
+ bb1:
+   x1 = ().field1;
+   goto bb3;
+ bb2:
+   x2 = ().field2;
+ bb3:
+   # x = PHI ;
+
+   with
+
+ bb0:
+   x1 = ().field1;
+   x2 = ().field2;
+   if (...) goto bb2; else goto bb1;
+ bb1:
+   goto bb3;
+ bb2:
+ bb3:
+   # x = PHI ;
+
+   The purpose of this transformation is to enable generation of conditional
+   move instructions such as Intel CMOVE or PowerPC ISEL.  Because one of
+   the loads is speculative, the transformation is restricted to very
+   specific cases to avoid introducing a page fault.  We are looking for
+   the common idiom:
+
+ if (...)
+   x = y->left;
+ else
+   x = y->right;
+
+   where left and right are typically adjacent pointers in a tree structure.  
*/
+
 static unsigned int
 tree_ssa_phiopt (void)
 {
-  return tree_ssa_phiopt_worker (false);
+  return tree_ssa_phiopt_worker (false, gate_hoist_loads ());
 }
 
 /* This pass tries to transform conditional stores into unconditional
@@ -190,7

Re: [PATCH] Hoist adjacent pointer loads

[Jakub Jelinek]

On Mon, Jun 04, 2012 at 08:45:20AM -0500, William J. Schmidt wrote:
> 2012-06-04  Bill Schmidt  
> 
>   * opts.c: Add -fhoist_adjacent_loads to -O2 and above.

The option name has hyphens instead of underscores in it, so the above is
wrong.

> --- gcc/common.opt(revision 187805)
> +++ gcc/common.opt(working copy)
> @@ -1186,6 +1186,11 @@ fgraphite-identity
>  Common Report Var(flag_graphite_identity) Optimization
>  Enable Graphite Identity transformation
>  
> +fhoist-adjacent-loads
> +Common Report Var(flag_hoist_adjacent_loads) Optimization
> +Enable hoisting adjacent loads to encourage generating conditional move
> +instructions
> +

Jakub

[PATCH][7/7] loop distribution TLC

[Richard Guenther]

This is the last TLC kind of patches for loop distribution.  It moves
more loop distribution specific code from tree-data-ref.c to
tree-loop-distribution.c thereby simplifying it.  It should now be
reasonably easy and efficient to add more pattern recognitions
(I was thinking about even BLAS-like loops, though for being more
useful loop distribution first has to be extended to work on loop nests
instead of only innermost loops).

Bootstrap and regtest running on x86_64-unknown-linux-gnu (as always
with loop-distribution and pattern recognition enabled).

Richard.

2012-06-04  Richard Guenther  

* tree-data-ref.c (stores_from_loop): Remove.
(stmt_with_adjacent_zero_store_dr_p): Likewise.
(stores_zero_from_loop): Likewise.
* tree-data-ref.h (stores_from_loop, stores_zero_from_loop,
stmt_with_adjacent_zero_store_dr_p, stride_of_unit_type_p): Remove.
(adjacent_store_dr_p): New function.
* tree-loop-distribution.c (generate_memset_builtin): Pass
the RDG, use the already available data-reference.
(generate_code_for_partition): Pass down RDG.
(classify_partition): Inline parts of the former
stmt_with_adjacent_zero_store_dr_p here and use adjacent_store_dr_p.
(ldist_gen): Remember if there was any detected builtin and
do less work if not and flag_tree_loop_distribution is not set.
(tree_loop_distribution): Inline and fuse stores_from_loop
and stores_zero_from_loop here.

Index: trunk/gcc/tree-data-ref.c
===
*** trunk.orig/gcc/tree-data-ref.c  2012-06-04 13:47:31.0 +0200
--- trunk/gcc/tree-data-ref.c   2012-06-04 14:29:16.648737016 +0200
*** free_rdg (struct graph *rdg)
*** 5174,5261 
free_graph (rdg);
  }
  
- /* Initialize STMTS with all the statements of LOOP that contain a
-store to memory.  */
- 
- void
- stores_from_loop (struct loop *loop, VEC (gimple, heap) **stmts)
- {
-   unsigned int i;
-   basic_block *bbs = get_loop_body_in_dom_order (loop);
- 
-   for (i = 0; i < loop->num_nodes; i++)
- {
-   basic_block bb = bbs[i];
-   gimple_stmt_iterator bsi;
- 
-   for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi))
-   if (gimple_vdef (gsi_stmt (bsi)))
- VEC_safe_push (gimple, heap, *stmts, gsi_stmt (bsi));
- }
- 
-   free (bbs);
- }
- 
- /* Returns true when the statement at STMT is of the form "A[i] = 0"
-that contains a data reference on its LHS with a stride of the same
-size as its unit type.  */
- 
- bool
- stmt_with_adjacent_zero_store_dr_p (gimple stmt)
- {
-   tree lhs, rhs;
-   bool res;
-   struct data_reference *dr;
- 
-   if (!stmt
-   || !gimple_vdef (stmt)
-   || !gimple_assign_single_p (stmt))
- return false;
- 
-   lhs = gimple_assign_lhs (stmt);
-   rhs = gimple_assign_rhs1 (stmt);
- 
-   /* If this is a bitfield store bail out.  */
-   if (TREE_CODE (lhs) == COMPONENT_REF
-   && DECL_BIT_FIELD (TREE_OPERAND (lhs, 1)))
- return false;
- 
-   if (!(integer_zerop (rhs) || real_zerop (rhs)))
- return false;
- 
-   dr = XCNEW (struct data_reference);
- 
-   DR_STMT (dr) = stmt;
-   DR_REF (dr) = lhs;
- 
-   res = dr_analyze_innermost (dr, loop_containing_stmt (stmt))
- && stride_of_unit_type_p (DR_STEP (dr), TREE_TYPE (lhs));
- 
-   free_data_ref (dr);
-   return res;
- }
- 
- /* Initialize STMTS with all the statements of LOOP that contain a
-store to memory of the form "A[i] = 0".  */
- 
- void
- stores_zero_from_loop (struct loop *loop, VEC (gimple, heap) **stmts)
- {
-   unsigned int i;
-   basic_block bb;
-   gimple_stmt_iterator si;
-   gimple stmt;
-   basic_block *bbs = get_loop_body_in_dom_order (loop);
- 
-   for (i = 0; i < loop->num_nodes; i++)
- for (bb = bbs[i], si = gsi_start_bb (bb); !gsi_end_p (si); gsi_next (&si))
-   if ((stmt = gsi_stmt (si))
- && stmt_with_adjacent_zero_store_dr_p (stmt))
-   VEC_safe_push (gimple, heap, *stmts, gsi_stmt (si));
- 
-   free (bbs);
- }
- 
  /* Determines whether the statement from vertex V of the RDG has a
 definition used outside the loop that contains this statement.  */
  
--- 5174,5179 
Index: trunk/gcc/tree-data-ref.h
===
*** trunk.orig/gcc/tree-data-ref.h  2012-06-04 13:47:31.0 +0200
--- trunk/gcc/tree-data-ref.h   2012-06-04 14:29:35.097736096 +0200
*** index_in_loop_nest (int var, VEC (loop_p
*** 609,629 
return var_index;
  }
  
- void stores_from_loop (struct loop *, VEC (gimple, heap) **);
- void stores_zero_from_loop (struct loop *, VEC (gimple, heap) **);
  bool rdg_defs_used_in_other_loops_p (struct graph *, int);
- bool stmt_with_adjacent_zero_store_dr_p (gimple);
  
! /* Returns true when STRIDE is equal in absolute value to the size of
!the unit type of TYPE.  */
  
  static inline bool
! stride_of_unit

Re: [PATCH 1/2] gcc symbol database

[Dodji Seketeli]

Hello YunFeng,

Thank you for taking the time to work on this.  I cannot accept or
deny your patches, but I thought I could maybe comment on some parts
of them.

First, IMHO, some of the new plugin events you are proposing to add to
libcpp seem to make sense, at least so that people can write
etags/ctags-like cross-reference tools like the one you are proposing.

For now, I will not comment on the choices you've made for the
cross-reference tool itself, even though they are interesting to see,
at least to understand why you need the libcpp events you are
proposing.

Thus, I'll start with the libcpp changes.

[...]

> diff -upr .pc/symdb_enhance_libcpp/libcpp/directives.c libcpp/directives.c

[...]

> @@ -492,6 +492,8 @@ _cpp_handle_directive (cpp_reader *pfile
>else if (skip == 0)
>  _cpp_backup_tokens (pfile, 1);
> 
> +  if (pfile->cb.end_directive)
> +pfile->cb.end_directive (pfile);

It seems to me that there are possibly several places where we handle
directives.  In those places, the function start_directive and
end_directive are called.  So IMHO, it would seems more appropriate
and maintainable to call pfile->cb.start_directive in the
start_directive function, and likewise, call pfile->cg.end_directive
in the end_directive function.

You would then remove the call below from _cpp_lex_token:

@@ -1886,6 +1889,8 @@ _cpp_lex_token (cpp_reader *pfile)
  handles the directive as normal.  */
   && pfile->state.parsing_args != 1)
 {
+  if (pfile->cb.start_directive)
+pfile->cb.start_directive (pfile, result);
   if (_cpp_handle_directive (pfile, result->flags & PREV_WHITE))
 {
   if (pfile->directive_result.type == CPP_PADDING)

[...]


> +++ libcpp/include/cpplib.h2012-05-25 14:56:56.745507332 +0800
> @@ -218,10 +218,10 @@ struct GTY(()) cpp_identifier {
> node;
>  };
> 
> -/* A preprocessing token.  This has been carefully packed and should
> -   occupy 16 bytes on 32-bit hosts and 24 bytes on 64-bit hosts.  */
> +/* A preprocessing token. */
>  struct GTY(()) cpp_token {
>source_location src_loc;/* Location of first char of token.  */
> +  int file_offset;

Enlarging this struct that is *widely* used might not be accepted by
the maintainers, because of the memory toll it takes.  Or you'd need
to prove that you cannot do otherwise.

So why is this file_offset member needed?

>From the rest of the patch, it looks like you are using it to store
the offset of the token, from the beginning of its line.  What is the
difference between that, and the column number of the token, as you
could get by calling linemap_expand_location on the src_loc of the
token, effectively doing away with this new file_offset member?  Note
that the linemap_expand_location call was introduced in 4.7.

[...]

> +++ libcpp/macro.c2012-05-25 14:56:56.749508416 +0800
> @@ -1029,9 +1029,13 @@ enter_macro_context (cpp_reader *pfile,
>if (pragma_buff)
>  _cpp_release_buff (pfile, pragma_buff);
> 
> +  if (pfile->cb.macro_end_arg)
> +pfile->cb.macro_end_arg (pfile, true);

Presumably, if we reach this point, I believe it means there was no
argument to what seemed to be a function-like macro.  Was it?

Thus, I don't understand why calling this pfile->cb.macro_end_arg call
achieves.  Actually, I think I don't understand the meaning of that
event callback, to begin with.  I have read and re-read the comment
below:

> @@ -522,6 +522,24 @@ struct cpp_callbacks
>   be expanded.  */

[...]

> +  /* Called when a function-like macro stops collecting macro parameters,
> +   * cancel = true, macro expansion is canceled. */
> +  void (*macro_end_arg) (cpp_reader *, bool cancel);

and I don't understand why you need the information conveyed by this
macro_end_arg event.

Maybe if you put the call to macro_start_expand in this
enter_macro_context function only after we are sure we are going to
actually proceed with the expansion 

>return 0;
>  }

... here, then you wouldn't need the macro_end_arg at all.  Would that
make sense?

>if (macro->paramc > 0)
>  replace_args (pfile, node, macro,
>(macro_arg *) buff->base,
> @@ -2263,6 +2267,8 @@ cpp_get_token_1 (cpp_reader *pfile, sour
>if (pfile->context->c.macro)
>  ++num_expanded_macros_counter;
>_cpp_pop_context (pfile);
> +  if (pfile->cb.macro_end_expand)
> +pfile->cb.macro_end_expand (pfile);

_cpp_pop_context is really the function that marks the end of a given
macro expansion, especially when the predicate in_macro_expansion_p
(introduced recently in trunk for gcc 4.8) returns true.

So IMHO, I seems more maintainable to move the call to
pfile->cb.macro_end_expand in _cpp_pop_context.

>if (pfile->state.in_directive)
>  continue;
>result = &pfile->avoid_paste;
> @@ -2321,8 +2327,14 @@ cpp_get_token_1 (cpp_reader *pfile, sour
>  }
>  }

[C++ Patch] PR 53567

[Paolo Carlini]

Hi,

in this "error reporting routines re-entered" ICE we try to emit a 
warning from ocp_convert while we are already producing an error message 
(we get to ocp_convert via the usual tsubst (tf_none) called by 
dump_template_bindings). I considered whether we could avoid the 
specific problem by passing different FLAGS to ocp_convert when 
build_static_cast_1 is called with tf_none, but I think it would be 
tricky and error-prone, because ocp_convert has a mix of errors, 
warnings, errors not protected by flags & LOOKUP_COMPLAIN. Anyway, when 
I tried the usual tsubst_flags_t route, I saw a large patch but also 
quite a few loose ends solved (which I noticed other times), 
convert_to_reference isn't special anymore in having to "synthesize" a 
complain, all in all very few explicit tf_warning_or_error remain, and 
in safe places, as far as I can see.


Bootstrapped and tested x86_64-linux.

Thanks,
Paolo.

/
/cp
2012-06-04  Paolo Carlini  

PR c++/53567
* typeck.c (cp_perform_integral_promotions): New, like
perform_integral_promotions but also takes a tsubst_flags_t parameter.
(pointer_diff): Add tsubst_flags_t parameter.
(decay_conversion, cp_default_conversion, cp_build_array_ref,
cp_build_binary_op, cp_build_unary_op, build_static_cast_1,
build_reinterpret_cast_1, cp_build_modify_expr,
convert_for_assignment): Adjust.
* init.c (expand_virtual_init, expand_default_init,
build_new, build_vec_delete_1, build_vec_init, build_delete): Adjust.
* class.c (build_base_path): Likewise.
* decl.c (compute_array_index_type): Likewise.
* rtti.c (ifnonnull): Add tsubst_flags_t parameter.
(build_typeid, build_dynamic_cast_1): Adjust.
* except.c (initialize_handler_parm): Likewise.
* typeck2.c (process_init_constructor_record): Likewise.
* semantics.c (finish_goto_stmt, handle_omp_for_class_iterator,
finish_static_assert): Likewise.
* call.c (build_op_delete_call, convert_like_real,
convert_arg_to_ellipsis, convert_for_arg_passing): Likewise.
* cvt.c (cp_convert_to_pointer, convert_to_pointer_force,
build_up_reference, convert_to_reference, cp_convert,
cp_convert_and_check, ocp_convert, convert_force): Add tsubst_flags_t
parameter.
* cp-tree.h: Adjust prototypes.

/testsuite
2012-06-04  Paolo Carlini  

PR c++/53567
* g++.dg/cpp0x/alias-decl-19.C: New.

Index: testsuite/g++.dg/cpp0x/alias-decl-19.C
===
--- testsuite/g++.dg/cpp0x/alias-decl-19.C  (revision 0)
+++ testsuite/g++.dg/cpp0x/alias-decl-19.C  (revision 0)
@@ -0,0 +1,31 @@
+// PR c++/53567
+// { dg-do compile { target c++11 } }
+
+template  struct IntegerType { typedef unsigned type; };
+
+template 
+using UnderlyingEnumType = typename IntegerType 
EnumT(0))>::type;
+
+template >
+struct EnumMask
+{
+  constexpr EnumMask(EnumT val) : m_val(val) {}
+  operator EnumT() { return m_val; }
+
+  EnumT m_val;
+};
+
+enum class A : unsigned { x };
+
+template 
+EnumMask operator ~(EnumT lhs)
+{
+  return EnumT(~unsigned(lhs) & unsigned(EnumT::maskAll)); // { dg-error "not 
a member" }
+
+}
+
+int main()
+{
+  ~A::x;
+  return 0;
+}
Index: cp/typeck.c
===
--- cp/typeck.c (revision 188167)
+++ cp/typeck.c (working copy)
@@ -53,7 +53,7 @@ static tree rationalize_conditional_expr (enum tre
 static int comp_ptr_ttypes_real (tree, tree, int);
 static bool comp_except_types (tree, tree, bool);
 static bool comp_array_types (const_tree, const_tree, bool);
-static tree pointer_diff (tree, tree, tree);
+static tree pointer_diff (tree, tree, tree, tsubst_flags_t);
 static tree get_delta_difference (tree, tree, bool, bool, tsubst_flags_t);
 static void casts_away_constness_r (tree *, tree *, tsubst_flags_t);
 static bool casts_away_constness (tree, tree, tsubst_flags_t);
@@ -1907,7 +1907,7 @@ decay_conversion (tree exp, tsubst_flags_t complai
   /* This way is better for a COMPONENT_REF since it can
 simplify the offset for a component.  */
   adr = cp_build_addr_expr (exp, complain);
-  return cp_convert (ptrtype, adr);
+  return cp_convert (ptrtype, adr, complain);
 }
 
   /* If a bitfield is used in a context where integral promotion
@@ -1951,12 +1951,12 @@ cp_default_conversion (tree exp, tsubst_flags_t co
   /* Check for target-specific promotions.  */
   tree promoted_type = targetm.promoted_type (TREE_TYPE (exp));
   if (promoted_type)
-exp = cp_convert (promoted_type, exp);
+exp = cp_convert (promoted_type, exp, complain);
   /* Perform the integral promotions first so that bitfield
  expressions (which may promote to "int", even if the bitfield is
  declared "unsigned") are promoted correctly.  */
   else if (INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (TREE_TYPE (ex

Re: [PATCH 2/2] Better system header location detection for built-in macro tokens

[Dodji Seketeli]

Jason Merrill  writes:

> On 06/04/2012 08:36 AM, Dodji Seketeli wrote:
>> -  /* Build the assignment expression.  */
>> +  /* Build the assignment expression.  Its default
>> + location is the location of the '=' token.  */
>> +  input_location = loc;
>>expr = build_x_modify_expr (loc, expr,
>>assignment_operator,
>>rhs,
>
> Hmm, I meant to say this before, but apparently forgot: We should the
> old input_location after build_x_modify_expr returns.

Woops, I wonder how I forgot that.

Fixed thus.  Testing is still underway.


libcpp/

PR preprocessor/53463
* line-map.c (linemap_location_in_system_header_p): For built-in
macro tokens, check the first expansion point location for that is
not for a token coming from a built-in macro.

gcc/cp/

PR preprocessor/53463
* parser.c (cp_parser_assignment_expression): Use the location
for the LHS as the default location for the expression.

gcc/testsuite/

PR preprocessor/53463
* g++.dg/cpp/limits.C: New test.
* g++.dg/parse/error19.C: Adjust.
* g++.dg/warn/Wconversion-real-integer2.C: Likewise.
* g++.dg/warn/pr35635.C: Likewise.
* g++.old-deja/g++.pt/assign1.C: Likewise.
---
 gcc/cp/parser.c|7 -
 gcc/testsuite/g++.dg/cpp/limits.C  |   21 ++
 gcc/testsuite/g++.dg/parse/error19.C   |2 +-
 .../g++.dg/warn/Wconversion-real-integer2.C|4 +-
 gcc/testsuite/g++.dg/warn/pr35635.C|4 +-
 gcc/testsuite/g++.old-deja/g++.pt/assign1.C|2 +-
 libcpp/line-map.c  |   30 +--
 7 files changed, 59 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp/limits.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 9fd8c84..7e71888 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7479,6 +7479,7 @@ cp_parser_assignment_expression (cp_parser* parser, bool 
cast_p,
  if (assignment_operator != ERROR_MARK)
{
  bool non_constant_p;
+ location_t saved_input_location;
 
  /* Parse the right-hand side of the assignment.  */
  tree rhs = cp_parser_initializer_clause (parser, &non_constant_p);
@@ -7491,11 +7492,15 @@ cp_parser_assignment_expression (cp_parser* parser, 
bool cast_p,
  if (cp_parser_non_integral_constant_expression (parser,
  NIC_ASSIGNMENT))
return error_mark_node;
- /* Build the assignment expression.  */
+ /* Build the assignment expression.  Its default
+location is the location of the '=' token.  */
+ saved_input_location = input_location;
+ input_location = loc;
  expr = build_x_modify_expr (loc, expr,
  assignment_operator,
  rhs,
  tf_warning_or_error);
+ input_location = saved_input_location;
}
}
 }
diff --git a/gcc/testsuite/g++.dg/cpp/limits.C 
b/gcc/testsuite/g++.dg/cpp/limits.C
new file mode 100644
index 000..b64e1e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp/limits.C
@@ -0,0 +1,21 @@
+// { dg-options "-pedantic" }
+// { dg-do compile }
+
+#include 
+
+// Compiling this with -pedantic was wrongly triggering this error:
+// libstdc++-v3/include/limits:1269:45: warning : use of C++0x long long 
integer constant [-Wlong-long]
+//   min() _GLIBCXX_USE_NOEXCEPT { return -__LONG_LONG_MAX__ - 1; }
+// ^
+// libstdc++-v3/include/limits:1272:44: warning : use of C++0x long long 
integer constant [-Wlong-long]
+//   max() _GLIBCXX_USE_NOEXCEPT { return __LONG_LONG_MAX__; }
+//^
+// libstdc++-v3/include/limits:1342:44: warning : use of C++0x long long 
integer constant [-Wlong-long]
+//   max() _GLIBCXX_USE_NOEXCEPT { return __LONG_LONG_MAX__ * 2ULL + 1
+//^
+
+int
+main ()
+{
+return 0;
+}
diff --git a/gcc/testsuite/g++.dg/parse/error19.C 
b/gcc/testsuite/g++.dg/parse/error19.C
index 010a403..6d84f71 100644
--- a/gcc/testsuite/g++.dg/parse/error19.C
+++ b/gcc/testsuite/g++.dg/parse/error19.C
@@ -10,6 +10,6 @@ const A& foo();
 
 void bar()
 {
-  foo()=A(0); // { dg-error "12:no match for 'operator='" }
+  foo()=A(0); // { dg-error "8:no match for 'operator='" }
   // { dg-message "candidate" "candidate note" { target *-*-* } 13 }
 }
diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C 
b/gcc/testsuite/g++.dg/warn/Wconversion-real-integer2.C
index 6a95b0e..0494588 100644
--- a/gcc/testsuite/g++.dg/warn/Wcon

Re: [testsuite] Allow for ! comments in g++.dg/debug/dwarf2/nested-3.C

[Andreas Schwab]

Likewise for | comments on m68k.  Checked in as obvious.

Andreas.

* g++.dg/debug/dwarf2/nested-3.C: Allow for | comments.

diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C 
b/gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C
index 56f3dfb..ab8e0d0 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/nested-3.C
@@ -59,4 +59,4 @@ main ()
 //
 // Hence the scary regexp:
 //
-// { dg-final { scan-assembler "\[^\n\r\]*\\(DIE \\(0x(\[0-9a-f\]+)\\) 
DW_TAG_namespace\\)\[\n\r\]+\[^\n\r\]*\"thread\[\^\n\r]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\\(DIE
 \\(0x(\[0-9a-f\]+)\\) 
DW_TAG_class_type\\)(\[\n\r\]+\[^\n\r\]*)+\"Executor\[^\n\r\]+\[\n\r\]+\[^\n\r\]*DW_AT_declaration\[\n\r\]+\[^\n\r\]*DW_AT_signature\[^#/!\]*\[#/!\]
 
\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_subprogram\\)\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\"CurrentExecutor\[^\n\r\]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*end
 of children of DIE 0x\\3\[\n\r]+\[^\n\r\]*end of children of DIE 
0x\\1\[\n\r]+" } }
+// { dg-final { scan-assembler "\[^\n\r\]*\\(DIE \\(0x(\[0-9a-f\]+)\\) 
DW_TAG_namespace\\)\[\n\r\]+\[^\n\r\]*\"thread\[\^\n\r]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\\(DIE
 \\(0x(\[0-9a-f\]+)\\) 
DW_TAG_class_type\\)(\[\n\r\]+\[^\n\r\]*)+\"Executor\[^\n\r\]+\[\n\r\]+\[^\n\r\]*DW_AT_declaration\[\n\r\]+\[^\n\r\]*DW_AT_signature\[^#/!|\]*\[#/!|\]
 
\[^\n\r\]*\\(DIE\[^\n\r\]*DW_TAG_subprogram\\)\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*\"CurrentExecutor\[^\n\r\]+\[\n\r\]+(\[^\n\r\]*\[\n\r\]+)+(\[^\n\r\]*\[\n\r\]+)+\[^\n\r\]*end
 of children of DIE 0x\\3\[\n\r]+\[^\n\r\]*end of children of DIE 
0x\\1\[\n\r]+" } }

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[PATCH] [PowerPC] [RFA] Fix PR target/53559 ICE on altivec builtins stv[l|r]x[l]

[Edmar]

I regression tested the patch presented in the PR:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53559

for the following branches:
4.5 branch svn revision 188125
4.6 branch svn revision 188124
4.7 branch svn revision 188128
4.8 trunk svn revision 187734

All bootstrapped with no regressions for powerpc64 target.

Thanks
Edmar

Re: [PATCH] [PowerPC] [RFA] Fix PR target/53559 ICE on altivec builtins stv[l|r]x[l]

[David Edelsohn]

On Mon, Jun 4, 2012 at 10:08 AM, Edmar  wrote:
> I regression tested the patch presented in the PR:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53559
>
> for the following branches:
> 4.5 branch svn revision 188125
> 4.6 branch svn revision 188124
> 4.7 branch svn revision 188128
> 4.8 trunk svn revision 187734
>
> All bootstrapped with no regressions for powerpc64 target.

This patch is okay.

In the future, please include the ChangeLog in the body of the email
message and the patch itself as an attachment, not simply pointing to
Bugzilla.

Thanks, David

[patch] PR middle-end/47530: tail call optimization corrupts stack (trans-mem)

[Aldy Hernandez]

This PR was reopened because Luke Dalessandro found a testcase that was 
still causing the stack corruption.


The problem here is that we split the block and call gsi_start_bb() to 
start scanning the new block, but immediately after we call gsi_next() 
which essentially skips the first instruction.


OK for branch and mainline?  (Pending tests currently currently running 
for both 4.7 and trunk).
PR middle-end/47530
* trans-mem.c (expand_block_edges): Do not skip the first
statement when resetting the BB.

Index: testsuite/g++.dg/tm/pr47530-2.C
===
--- testsuite/g++.dg/tm/pr47530-2.C (revision 0)
+++ testsuite/g++.dg/tm/pr47530-2.C (revision 0)
@@ -0,0 +1,34 @@
+// { dg-do compile }
+// { dg-options "-fgnu-tm -O2 -fno-inline" }
+
+class RBTree
+{
+struct RBNode
+{
+  RBNode* next;
+};
+
+  public:
+RBNode* sentinel;
+__attribute__((transaction_safe)) bool lookup();
+};
+
+bool RBTree::lookup()
+{
+  RBNode* x = sentinel;
+  while (x)
+x = x->next;
+  return false;
+}
+
+
+RBTree* SET;
+
+void bench_test()
+{
+  __transaction_atomic { 
+  SET->lookup();
+}
+}
+
+// { dg-final { scan-assembler-not "jmp.*ITM_commitTransaction" } }
Index: trans-mem.c
===
--- trans-mem.c (revision 188081)
+++ trans-mem.c (working copy)
@@ -2591,6 +2591,7 @@ expand_block_edges (struct tm_region *re
 
   for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
 {
+  bool do_next = true;
   gimple stmt = gsi_stmt (gsi);
 
   /* ??? TM_COMMIT (and any other tm builtin function) in a nested
@@ -2612,6 +2613,7 @@ expand_block_edges (struct tm_region *re
  make_tm_edge (stmt, bb, region);
  bb = e->dest;
  gsi = gsi_start_bb (bb);
+ do_next = false;
}
 
  /* Delete any tail-call annotation that may have been added.
@@ -2620,7 +2622,8 @@ expand_block_edges (struct tm_region *re
  gimple_call_set_tail (stmt, false);
}
 
-  gsi_next (&gsi);
+  if (do_next)
+   gsi_next (&gsi);
 }
 }
 

Re: [patch] PR middle-end/47530: tail call optimization corrupts stack (trans-mem)

[Richard Henderson]

On 06/04/2012 09:04 AM, Aldy Hernandez wrote:
>   PR middle-end/47530
>   * trans-mem.c (expand_block_edges): Do not skip the first
>   statement when resetting the BB.

Ok.


r~

Re: [line-map] simple oneliner that speeds up track-macro-expansion

[Dodji Seketeli]

Hello Dimitrios,

I cannot approve or deny your patch, but I have one question.

[...]


> 2012-06-04  Dimitrios Apostolou 
> 
>   * line-map.c (linemap_enter_macro): Don't zero max_column_hint in
>   every macro. This improves performance by reducing the number of
>   reallocations when track-macro-expansion is on.

[...]

> === modified file 'libcpp/line-map.c'

[...]

> @@ -331,7 +331,6 @@ linemap_enter_macro (struct line_maps *s
> num_tokens * sizeof (source_location));
>  
>LINEMAPS_MACRO_CACHE (set) = LINEMAPS_MACRO_USED (set) - 1;
> -  set->max_column_hint = 0;

I am wondering why this change implies better performance.

Is this because when we later want to encode a new line/column, and
hit the spot below, (in linemap_line_start via
linemap_position_for_column), we call less linemap_add (and thus
allocate less maps):

  if (line_delta < 0
  || last_line != ORDINARY_MAP_STARTING_LINE_NUMBER (map)
  || SOURCE_COLUMN (map, highest) >= (1U << column_bits))
map = (struct line_map *) linemap_add (set, LC_RENAME,
   ORDINARY_MAP_IN_SYSTEM_HEADER_P
   (map),
   ORDINARY_MAP_FILE_NAME (map),
   to_line);

The reason why the "if" would fail more, is that the condition

"SOURCE_COLUMN (map, highest) >= (1U << column_bits)

would condition would fail more often, because the column_bits would
be smaller more often.

Does that match your measurements?

Thanks.

-- 
Dodji

[Patch, Fortran] PR50619 - fix ASSOCIATE with -finit-*

[Tobias Burnus]

First, I would like to ping my move_alloc coarray patches. The handling 
is in line with the J3-approved interpretation request F08/0040 with 
some remarks of the WG5 convenor (John Reid), cf. PR for the links:

- http://gcc.gnu.org/ml/fortran/2012-05/msg00171.html
- http://gcc.gnu.org/ml/fortran/2012-05/msg00173.html

 * * *

ASSOCIATE names are (wrongly) initialized by -finit-real=*, thus, their 
values are wrong.


The fix seems to be rather obvious, but I am not really an associate expert.

Build and regtested on x86-64-linux.
OK for the trunk? How about backporting? It's not a regression and it 
requires a nondefault flag; on the other hand, it can be rather 
suprising wrong-code bug and the fix is simple.


Tobias
2012-06-04  Tobias Burnus  

	PR fortran/50619
	* resolve.c (build_default_init_expr): Don't initialize
	ASSOCIATE names.

2012-06-04  Tobias Burnus  

	PR fortran/50619
	* gfortran.dg/init_flag_10.f90: New.

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 3627b71..61a1381 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -10103,7 +10103,8 @@ build_default_init_expr (gfc_symbol *sym)
   || sym->attr.data
   || sym->module
   || sym->attr.cray_pointee
-  || sym->attr.cray_pointer)
+  || sym->attr.cray_pointer
+  || sym->assoc)
 return NULL;
 
   /* Now we'll try to build an initializer expression.  */
--- /dev/null	2012-06-04 08:06:14.603785028 +0200
+++ gcc/gcc/testsuite/gfortran.dg/init_flag_10.f90	2012-06-04 14:42:30.0 +0200
@@ -0,0 +1,43 @@
+! { dg-do run }
+! { dg-options "-finit-real=NAN" }
+! { dg-add-options ieee }
+! { dg-skip-if "NaN not supported" { spu-*-* } { "*" } { "" } }
+!
+! PR fortran/50619
+!
+! Contributed by Fred Krogh
+!
+! The NaN initialization used to set the associate name to NaN!
+!
+
+module testa2
+type, public ::  test_ty
+  real :: rmult = 1.0e0
+end type test_ty
+
+contains
+  subroutine test(e, var1)
+type(test_ty) :: e
+real :: var1, var2 ! Should get NaN initialized
+
+! Should be the default value
+if (e%rmult /= 1.0) call abort ()
+
+! Check that NaN initialization is really turned on
+if (var1 == var1) call abort () 
+if (var2 == var2) call abort () 
+
+! The following was failing:
+associate (rmult=>e%rmult)
+  if (e%rmult /= 1.0) call abort ()
+end associate
+  end subroutine test
+end module testa2
+
+program testa1
+  use testa2
+  type(test_ty) :: e
+  real :: var1 ! Should get NaN initialized
+  call test(e, var1)
+  stop
+end program testa1

Re: [gcov] a few improvements

[Xinliang David Li]

On Mon, Jun 4, 2012 at 12:49 AM, Nathan Sidwell  wrote:
> On 06/03/12 21:40, Xinliang David Li wrote:
>
>>> Can you explain this more -- what exactly are trying to do?  Are you
>>> trying
>>> to rebuild multiple times with the same coverage data,
>>
>>
>> yes -- for instance, in the context of debugging a compiler problem,
>> you will need to compile the same file multiple times with the same
>> coverage data.
>
>
> ok, debugging the compiler is not the typical user mode of use.

Another usage is FDO performance tuning trying build with different
options/parameters, so it is common.


>
> Do any of the following work for you?
> *) don't use -frandom-seed (you don't say why you're using this flag)

This is in our build system for build reproducibility.

> *) copy and restore the coverage data file
> *) stub out the unlink code at the end of coverage.c. (I presume that you're
> rebuilding cc1 with optimization off anyway, and as you're debugging a
> compiler problem with FDO you're somewhat familiar with coverage.c).

I know how to workaround/fix the problem --- I am asking the
motivation for deleting the coverage file? What does it buy us? This
looks like a regression in gcc4.7 to me.

thanks,

David


>
> nathan

Re: [PATCH 2/2] Better system header location detection for built-in macro tokens

[Jason Merrill]

OK.

Jason

Re: [C++] Reject variably modified types in operator new

[Jason Merrill]

On 06/01/2012 03:55 PM, Florian Weimer wrote:

On 06/01/2012 06:19 PM, Jason Merrill wrote:

On 06/01/2012 11:40 AM, Florian Weimer wrote:

I'm puzzled why build_new is even invoked after detecting that there is
a non-constant expression.


I'd accept a patch to change that.


I don't really now what I'm doing here. But I noticed that in
cp_parser_constant_expression, a failure in
require_potential_rvalue_constant_expression is not reported to the caller.

This changes error reporting, and a few test cases need adjustment. In
some, reporting is better, in others, it's slightly worse. I need to
make a second pass over the changes to make sure that they are alright.


Oh, right.  I think I tried this change once and then decided that 
things were better without it, but I don't remember why.  The change 
looks reasonable by itself; what changes are you seeing in error reporting.


Jason

Re: [C++ Patch] PR 53524

[Jason Merrill]

On 05/31/2012 10:55 AM, Paolo Carlini wrote:

On 05/31/2012 04:43 PM, Jason Merrill wrote:

Does the C front end warn about this mismatch?

I just tried the first test of g++.old-deja/g++.other/cond5.C and the C
front-end does *not* warn neither by default, neither with -Wall.

  Do we warn about mismatch in comparisons?

Comparisons are dealt with separately, we have -Wenum-compare:


It seems to me that these should be covered by the same flag.  Maybe 
just add the warning to -Wenum-compare to start with; this isn't exactly 
comparison, but it's another case of the usual arithmetic conversions 
bringing them to a common type.  For 4.8 I think we also want to improve 
our handling of use of enums within their enum-specifier.


Jason

Re: [google] Add options to pattern match function name for hotness attributes

[Xinliang David Li]

On Sun, Jun 3, 2012 at 11:17 PM, Dehao Chen  wrote:
> I've updated the patch using deferred option.
>
> http://codereview.appspot.com/6281047
>
> Thanks,
> Dehao
>
> The new patch:
>
> 2012-06-01  Dehao Chen  
>
>        * gcc/cgraph.c (cgraph_match_attribute_by_name): New function.
>        (cgraph_node): Add attribute to function decl.
>        * gcc/opts-global.c (add_attribute_list_to_vector): New function.
>        (handle_common_deferred_options): Handle new options.
>        * gcc/opts.c (common_handle_option): Handle new options.
>        * gcc/opts.h (attribute_pair): New type.
>        * gcc/common.opt (flag_function_attribute_list): New option.
> Index: gcc/doc/invoke.texi
> ===
> --- gcc/doc/invoke.texi (revision 188050)
> +++ gcc/doc/invoke.texi (working copy)
> @@ -362,7 +362,8 @@
>  -fdelete-null-pointer-checks -fdse -fdevirtualize -fdse @gol
>  -fearly-inlining -fipa-sra -fexpensive-optimizations -ffast-math @gol
>  -ffinite-math-only -ffloat-store -fexcess-precision=@var{style} @gol
> --fforward-propagate -ffp-contract=@var{style} -ffunction-sections @gol
> +-fforward-propagate -ffp-contract=@var{style} @gol
> +-ffunction-attribute-list -ffunction-sections @gol
>  -fgcse -fgcse-after-reload -fgcse-las -fgcse-lm -fgraphite-identity @gol
>  -fgcse-sm -fif-conversion -fif-conversion2 -findirect-inlining @gol
>  -finline-functions -finline-functions-called-once -finline-limit=@var{n} @gol
> @@ -8585,6 +8586,10 @@
>  specify this option and you may have problems with debugging if
>  you specify both this option and @option{-g}.
>
> +@item -ffunction-attribute-list
> +@opindex ffunction-attribute-list
> +List of function name patterns that will be applied specified attribute.
> +
>  @item -fbranch-target-load-optimize
>  @opindex fbranch-target-load-optimize
>  Perform branch target register load optimization before prologue / epilogue
> Index: gcc/cgraph.c
> ===
> --- gcc/cgraph.c        (revision 188050)
> +++ gcc/cgraph.c        (working copy)
> @@ -99,6 +99,7 @@
>  #include "ipa-utils.h"
>  #include "lto-streamer.h"
>  #include "l-ipo.h"
> +#include "opts.h"
>
>  const char * const ld_plugin_symbol_resolution_names[]=
>  {
> @@ -520,6 +521,32 @@
>     }
>  }
>
> +/* Match FNDECL's name with user specified patterns. If match is found, add
> +   attributes to FNDECL.
> +   name matches with pattern, iff one of the following conditions satisfy:
> +     1. strcmp (name, pattern) != 0
> +     2. pattern[len - 1] = '*' && strncmp (name, pattern, len - 1) != 0  */

Fix the comment -- !strcmp ...

> +static void
> +cgraph_match_attribute_by_name (tree fndecl)
> +{
> +  unsigned i;
> +  attribute_pair_p p;
> +  const char *name = lang_hooks.decl_printable_name(fndecl, 0);

It should probably use mangled name here.

> +
> +  if (!name)
> +    return;
> +
> +  FOR_EACH_VEC_ELT (attribute_pair_p, function_attribute_list, i, p)
> +    {
> +      char *n = p->name;
> +      int len = strlen (n);
> +      if ((n[len - 1] == '*' && !strncmp (name, n, len - 1))
> +          || !strcmp (name, n))
> +        decl_attributes (&fndecl, tree_cons (
> +            get_identifier (p->attribute), NULL, NULL), 0);
> +    }
> +}
> +
>  /* Return cgraph node assigned to DECL.  Create new one when needed.  */
>
>  struct cgraph_node *
> @@ -554,6 +581,7 @@
>       node->origin->nested = node;
>     }
>   cgraph_add_assembler_hash_node (node);
> +  cgraph_match_attribute_by_name (decl);
>   return node;
>  }
>
> Index: gcc/opts.c
> ===
> --- gcc/opts.c  (revision 188050)
> +++ gcc/opts.c  (working copy)
> @@ -1647,6 +1647,10 @@
>       /* Deferred.  */
>       break;
>
> +    case OPT_ffunction_attribute_list_:
> +      /* Deferred.  */
> +      break;
> +
>     case OPT_fsched_verbose_:
>  #ifdef INSN_SCHEDULING
>       /* Handled with Var in common.opt.  */
> Index: gcc/opts.h
> ===
> --- gcc/opts.h  (revision 188050)
> +++ gcc/opts.h  (working copy)
> @@ -272,6 +272,15 @@
>   struct cl_option_handler_func handlers[3];
>  };
>
> +typedef struct {
> +  char *name;
> +  char *attribute;
> +} attribute_pair;
> +typedef attribute_pair *attribute_pair_p;
> +DEF_VEC_P(attribute_pair_p);
> +DEF_VEC_ALLOC_P(attribute_pair_p,heap);
> +extern VEC(attribute_pair_p,heap) *function_attribute_list;
> +
>  /* Input file names.  */
>
>  extern const char **in_fnames;
> Index: gcc/common.opt
> ===
> --- gcc/common.opt      (revision 188050)
> +++ gcc/common.opt      (working copy)
> @@ -1242,6 +1242,10 @@
>  Common Report Var(flag_function_sections)
>  Place each function into its own section
>
> +ffunction-attribute-list=
> +Common Joined RejectNegative Var(common_deferred_options) Defer
> +-ffunction-attribute-list=attr

Re: [google] Add options to pattern match function name for hotness attributes

[Xinliang David Li]

On Mon, Jun 4, 2012 at 6:24 AM, Jan Hubicka  wrote:
>> On Sat, Jun 2, 2012 at 11:11 AM, Jan Hubicka  wrote:
>> >> Actually Dehao also plans to teach the static predictor to understand
>> >> standard library functions more (e.g IO functions) and add more naming
>> >
>> > How this differ from annotating the library?
>>
>> I find them more suitable to be compiler heuristic than being
>> function's attribute -- attribute is a much stronger assertion.
>
> Why so? Special matching the function names in compiler is pretty much
> the same thing as annotating them in the source, right?

Compiler can choose to do fine grained tuning based on heuristics
which can be hard for user to do/maintain using annotation. For
instance, we provide user __builtin_expect to annotate branches, but
not a more general builtin to annotate the branch with specific
probabilities -- the latter is taken care of by  the compiler.

thanks,

David

>>
>> >
>> > There are indeed quite some possibilities to do about library calls
>> >
>> > One thing I always wondered about is how to tell predictor that paths 
>> > containing
>> > heavy IO functions don't need to be really opimized to death, since their 
>> > execution
>> > time is dominated by the IO...
>> >
>>
>> Yes -- if branch predictor does the right thing and if function
>> splitter is powerful enough, the IO code can be outlined and optimized
>> for size :)
>
> We have infrastructure for optimizing for size regions of functions, so this 
> should
> be easy. With flag_reorder_blocks_and_partition we can even "outline"
> but problem with these things is that they do not really match the usual 
> vision
> of profile prediction...
>
> Honza
>>
>>
>> >> based heuristics such as 'error, success, failure, fatal etc).
>> >
>> > yeah, this is also mentioned by some branch prediction papers. It is bit 
>> > kludgy
>> > in nature (i.e. it won't understand programs written in Czech language) 
>> > but it
>> > is an heuristics after all.
>> >
>>
>> right.
>>
>> thanks,
>>
>> David
>>
>> > Honza
>> >>
>> >> thanks,
>> >>
>> >> David
>> >>
>> >> > Honza
>> >> >>
>> >> >> thanks,
>> >> >>
>> >> >> David
>> >> >> > Honza

fix t-vxworks fetching of $(MULTIDIR) for libgcc

[Olivier Hainque]

Hello,

libgcc/config/t-vxworks has

<< LIBGCC2_INCLUDES = -nostdinc -I \
   `case "/$$(MULTIDIR)" in \
  */mrtp*) echo $(WIND_USR)/h ;; \
  *) echo $(WIND_BASE)/target/h ;; \
esac`
>>

The double "$" was useful back when libgcc was built
within gcc/ and we had

<< gcc/Makefile.in

   libgcc.mvars:
...
echo LIBGCC2_CFLAGS = '$(LIBGCC2_CFLAGS)' >> tmp-libgcc.mvars
>>

The latter disappeared when the libgcc build was moved to toplevel,
per svn rev 180774.

LIBGCC2_INCLUDES is now expanded in a more straightforward
fashion and the second "$" breaks the build. With bash for
example, we get "MULTIDIR: command not found" from the shell
trying to expand $(MULTIDIR) and a bogus -I argument value.

The attached patch fixes this by removing the extra '$'.

Tested by checking that a cross build for powerpc-wrs-vxworks on
x86-linux now proceeds past libgcc and finishes.

OK to commit ?

Thanks in advance,

Olivier

2012-06-04  Olivier Hainque  

libgcc/
* config/t-vxworks (LIBGCC2_INCLUDES): Remove extraneous $
in attempt at fetching the value of MULTIDIR.



multidir.diff
Description: Binary data

Re: [C++ Patch] Produce canonical names for debug info without changing normal pretty-printing (issue6215052)

[Sterling Augustine]

On Fri, Jun 1, 2012 at 4:38 PM, Gabriel Dos Reis
 wrote:
> On Fri, Jun 1, 2012 at 6:07 PM, Sterling Augustine
>  wrote:
>> After finding yet another bug in the previous patch dealing with 
>> pretty-printing
>> decls for dwarf in canonical form, I have figured out a way to do it that is
>> less invasive and much cleaner.
>>
>> This updated patch simply wraps the two entry points into the decl pretty-
>> printer called from cxx_dwarf_name with new functions that set the
>> appropriate flag on the pretty printer. This is much cleaner and avoids
>> the need for translating flags for C++ pretty-printing into standard C
>> pretty printing flags.
>>
>> OK for mainline?
>
> OK.

Thanks! Committed as posted.

Sterling

Re: [C++ Patch] PR 53524

[Paolo Carlini]

Hi,

On 06/04/2012 06:55 PM, Jason Merrill wrote:

On 05/31/2012 10:55 AM, Paolo Carlini wrote:

On 05/31/2012 04:43 PM, Jason Merrill wrote:

Does the C front end warn about this mismatch?

I just tried the first test of g++.old-deja/g++.other/cond5.C and the C
front-end does *not* warn neither by default, neither with -Wall.

  Do we warn about mismatch in comparisons?

Comparisons are dealt with separately, we have -Wenum-compare:


It seems to me that these should be covered by the same flag.  Maybe 
just add the warning to -Wenum-compare to start with; this isn't 
exactly comparison, but it's another case of the usual arithmetic 
conversions bringing them to a common type.
Ok, this would be simple to do. The only issue I can see, is that in C++ 
-Wenum-compare has a name, thus can be easily disabled but it's ON by 
default. Anyway, I'll send a patchlet should be just a couple of lines 
in call.c + a few words in the doc.


Paolo.

Re: _FORTIFY_SOURCE for std::vector

[Florian Weimer]

On 06/01/2012 01:34 PM, Jakub Jelinek wrote:

Have you looked at the assembly differences with this in?


It's not great.

Here's an example:

void
write(std::vector& blob, unsigned n, float v1, float v2, float 
v3, float v4)

{
  blob[n] = v1;
  blob[n + 1] = v2;
  blob[n + 2] = v3;
  blob[n + 3] = v4;
}

Without checks, it turns into:

movq(%rdi), %rax
movl%esi, %edx
movss   %xmm0, (%rax,%rdx,4)
leal1(%rsi), %edx
movss   %xmm1, (%rax,%rdx,4)
leal2(%rsi), %edx
movss   %xmm2, (%rax,%rdx,4)
leal3(%rsi), %edx
movss   %xmm3, (%rax,%rdx,4)
ret

With checks, it's:

subq$8, %rsp
movq(%rdi), %rdx
movq8(%rdi), %rax
movl%esi, %ecx
subq%rdx, %rax
sarq$2, %rax
cmpq%rax, %rcx
jae .L8
movss   %xmm0, (%rdx,%rcx,4)
leal1(%rsi), %ecx
cmpq%rax, %rcx
jae .L8
movss   %xmm1, (%rdx,%rcx,4)
leal2(%rsi), %ecx
cmpq%rax, %rcx
jae .L8
movss   %xmm2, (%rdx,%rcx,4)
leal3(%rsi), %ecx
cmpq%rax, %rcx
jae .L8
movss   %xmm3, (%rdx,%rcx,4)
addq$8, %rsp
ret
.L8:
call__chk_fail

For char/signed char/unsigned char, it's even worse because there's an 
additional pointer load per store (bringing up the total to two—the 
compiler cannot see that the _M_start pointer cannot point into the 
std::vector object).


I'm no longer sure that's acceptable overhead.  We really don't want 
programmers to switch off _FORTIFY_SOURCE because of its overhead.


I'm attaching the patch just in case.  I'm not sure the test case 
actually ran (if it did, it passed on first try, which would be unusual.)


--
Florian Weimer / Red Hat Product Security Team
Index: libstdc++-v3/include/bits/c++config
===
--- libstdc++-v3/include/bits/c++config	(revision 187951)
+++ libstdc++-v3/include/bits/c++config	(working copy)
@@ -370,6 +370,13 @@
   } while (false)
 #endif
 
+// _FORTIFY_SOURCE support.  Only available on GNU libc.
+
+namespace __gnu_cxx
+{
+  extern "C" void __chk_fail () __attribute__((noreturn));
+}
+
 // Macros for race detectors.
 // _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(A) and
 // _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(A) should be used to explain
Index: libstdc++-v3/include/bits/stl_vector.h
===
--- libstdc++-v3/include/bits/stl_vector.h	(revision 187951)
+++ libstdc++-v3/include/bits/stl_vector.h	(working copy)
@@ -768,7 +768,10 @@
*/
   reference
   operator[](size_type __n)
-  { return *(this->_M_impl._M_start + __n); }
+  {
+	_M_fortify_range_check(__n);
+	return *(this->_M_impl._M_start + __n);
+  }
 
   /**
*  @brief  Subscript access to the data contained in the %vector.
@@ -783,7 +786,10 @@
*/
   const_reference
   operator[](size_type __n) const
-  { return *(this->_M_impl._M_start + __n); }
+  { 
+	_M_fortify_range_check(__n);
+	return *(this->_M_impl._M_start + __n);
+  }
 
 protected:
   /// Safety check used only from at().
@@ -794,6 +800,17 @@
 	  __throw_out_of_range(__N("vector::_M_range_check"));
   }
 
+  /// Range check used by operator[].
+  /// No-op unless _FORTIFY_SOURCE is enabled.
+  void
+  _M_fortify_range_check(size_type __n) const
+  {
+#if defined __USE_FORTIFY_LEVEL && __USE_FORTIFY_LEVEL > 0
+	if (__n >= this->size())
+	__gnu_cxx::__chk_fail();
+#endif
+  }
+
 public:
   /**
*  @brief  Provides access to the data contained in the %vector.
Index: libstdc++-v3/testsuite/23_containers/vector/element_access/2.cc
===
--- libstdc++-v3/testsuite/23_containers/vector/element_access/2.cc	(revision 0)
+++ libstdc++-v3/testsuite/23_containers/vector/element_access/2.cc	(revision 0)
@@ -0,0 +1,100 @@
+// Copyright (C) 2012 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library 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.
+
+// This library 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 Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// 23.2.4 vector 
+
+// { dg-add-options no_pch }
+// { dg-target *-*-linux* }
+
+#undef _FORTIFY_SO

PATCH: --with-abi=x32 without --with-multilib-list doesn't work

[H.J. Lu]

Hi,

We should enable x32 run-time library if --with-abi={x32|mx32} is used
to configure GCC i[34567]86-*-* and x86_64-*-*.  Tested on Linux/x86-64.
OK for trunk?

Thanks.


H.J.
---
2012-06-04  H.J. Lu  

PR target/53575
* config.gcc: Enable x32 run-time library if --with-abi={x32|mx32}
is used for i[34567]86-*-* and x86_64-*-*.

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 61adc89..3f66bd2 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1233,7 +1233,14 @@ i[34567]86-*-linux* | i[34567]86-*-kfreebsd*-gnu | 
i[34567]86-*-knetbsd*-gnu | i
tmake_file="${tmake_file} i386/t-linux64"
x86_multilibs="${with_multilib_list}"
if test "$x86_multilibs" = "default"; then
-   x86_multilibs="m64,m32"
+   case ${with_abi} in
+   x32 | mx32)
+   x86_multilibs="m64,m32,mx32"
+   ;;
+   *)
+   x86_multilibs="m64,m32"
+   ;;
+   esac
fi
x86_multilibs=`echo $x86_multilibs | sed -e 's/,/ /g'`
for x86_multilib in ${x86_multilibs}; do
@@ -1299,7 +1306,14 @@ x86_64-*-linux* | x86_64-*-kfreebsd*-gnu | 
x86_64-*-knetbsd*-gnu)
tmake_file="${tmake_file} i386/t-linux64"
x86_multilibs="${with_multilib_list}"
if test "$x86_multilibs" = "default"; then
-   x86_multilibs="m64,m32"
+   case ${with_abi} in
+   x32 | mx32)
+   x86_multilibs="m64,m32,mx32"
+   ;;
+   *)
+   x86_multilibs="m64,m32"
+   ;;
+   esac
fi
x86_multilibs=`echo $x86_multilibs | sed -e 's/,/ /g'`
for x86_multilib in ${x86_multilibs}; do

Re: [C++ Patch] PR 53567

[Jason Merrill]

On 06/04/2012 10:15 AM, Paolo Carlini wrote:

@@ -5413,7 +5413,7 @@ build_op_delete_call (enum tree_code code, tree ad
  fns = lookup_name_nonclass (fnname);

/* Strip const and volatile from addr.  */
-  addr = cp_convert (ptr_type_node, addr);
+  addr = cp_convert (ptr_type_node, addr, tf_warning_or_error);


build_op_delete_call is called from build_new_1, so it should be 
complainified.



+ if (complain & tf_error)
+   {
+ if (flags & LOOKUP_COMPLAIN)
+   permerror (loc, "conversion from %q#T to %q#T",
+  intype, type);
+ if (!flag_permissive)
+   return error_mark_node;
+   }
+ else
return error_mark_node;


I don't think we want to check both tf_error and LOOKUP_COMPLAIN 
everywhere.  We could use one to set the other, but at this point I 
think we want to do away with LOOKUP_COMPLAIN entirely as its function 
has been taken over.


Jason

[patch][PCH] Do not write/read asm_out_file

[Steven Bosscher]

Hello,

The attached patch removes one more #include output.h, this time from
c-family/c-pch.c.

Anything written out to asm_out_file between pch_init and
c_common_write_pch is read back in by c_common_write_pch and dumped to
the PCH that's being written out. In c_common_read_pch this data is
written out verbatim to asm_out_file again.

But nothing should write to asm_out_file between pch_init and
c_common_write_pch. I suppose this happened before unit-at-a-time
became the only supported compilation mode, but these days there's
nothing, AFAICT, that should be written to asm_out_file by a front end
during PCH generation.

This patch was bootstrapped&tested on powerpc64-unknown-linux-gnu. OK for trunk?

Ciao!
Steven


c_pch_no_asm_out_file.diff
Description: Binary data

Re: [C++ Patch] PR 53524

[Jason Merrill]

On 06/04/2012 01:52 PM, Paolo Carlini wrote:

Ok, this would be simple to do. The only issue I can see, is that in C++
-Wenum-compare has a name, thus can be easily disabled but it's ON by
default.


The warning is already on by default, so that wouldn't be a change; this 
just creates a way for users to turn it off until we deal with the 
unhelpful case.


Jason

patch to fix reported worsening allocno class choice on the 2nd pass of ira reg costs on MIPS

[Vladimir Makarov]

  The following patch solves the problem.  Instead of pressure classes 
which are focused on relation register moves with memory costs, we use 
an uniform classes whose register move costs are the same for all their 
sublcasses.  The unifrom classes is used not to slow down the 2nd pass.


  The patch does not changed the generated code on x86/x86-64 and does 
not slow IRA down (I've tried it on variety of tests) while it does 
solve the reported problem.  The patch also adds priniting debug info 
about uniform and important classes.


  The patch was successfully bootstrapped on x86/x86-64.

  Committed as rev. 188199.

 2012-06-04  Vladimir Makarov 

* ira-int.h (struct target_ira_int): Add member
x_ira_uniform_class_p.
(ira_uniform_class_p): New macro.

* ira.c (setup_uniform_class_p): New function.
(setup_allocno_and_important_classes): Call the function.
(print_unform_and_important_classes): New function.
(print_classes): Rename to print_translated_classes.
(ira_debug_allocno_classes): Add call of
print_unform_and_important_classes.

* ira-costs.c (setup_regno_cost_classes_by_aclass): Use uniform
classes instead of pressure classes.


Index: ira.c
===
--- ira.c	(revision 188124)
+++ ira.c	(working copy)
@@ -903,6 +903,45 @@ setup_pressure_classes (void)
   setup_stack_reg_pressure_class ();
 }
 
+/* Set up IRA_UNIFORM_CLASS_P.  Uniform class is a register class
+   whose register move cost between any registers of the class is the
+   same as for all its subclasses.  We use the data to speed up the
+   2nd pass of calculations of allocno costs.  */
+static void
+setup_uniform_class_p (void)
+{
+  int i, cl, cl2, m;
+
+  for (cl = 0; cl < N_REG_CLASSES; cl++)
+{
+  ira_uniform_class_p[cl] = false;
+  if (ira_class_hard_regs_num[cl] == 0)
+	continue;
+  /* We can not use alloc_reg_class_subclasses here because move
+	 cost hooks does not take into account that some registers are
+	 unavailable for the subtarget.  E.g. for i686, INT_SSE_REGS
+	 is element of alloc_reg_class_subclasses for GENERAL_REGS
+	 because SSE regs are unavailable.  */
+  for (i = 0; (cl2 = reg_class_subclasses[cl][i]) != LIM_REG_CLASSES; i++)
+	{
+	  if (ira_class_hard_regs_num[cl2] == 0)
+	continue;
+  	  for (m = 0; m < NUM_MACHINE_MODES; m++)
+	if (contains_reg_of_mode[cl][m] && contains_reg_of_mode[cl2][m])
+	  {
+		ira_init_register_move_cost_if_necessary ((enum machine_mode) m);
+		if (ira_register_move_cost[m][cl][cl]
+		!= ira_register_move_cost[m][cl2][cl2])
+		  break;
+	  }
+	  if (m < NUM_MACHINE_MODES)
+	break;
+	}
+  if (cl2 == LIM_REG_CLASSES)
+	ira_uniform_class_p[cl] = true;
+}
+}
+
 /* Set up IRA_ALLOCNO_CLASSES, IRA_ALLOCNO_CLASSES_NUM,
IRA_IMPORTANT_CLASSES, and IRA_IMPORTANT_CLASSES_NUM.
 
@@ -1008,6 +1047,7 @@ setup_allocno_and_important_classes (voi
   for (j = 0; j < ira_allocno_classes_num; j++)
 ira_reg_allocno_class_p[ira_allocno_classes[j]] = true;
   setup_pressure_classes ();
+  setup_uniform_class_p ();
 }
 
 /* Setup translation in CLASS_TRANSLATE of all classes into a class
@@ -1292,10 +1332,27 @@ setup_reg_class_relations (void)
 }
 }
 
-/* Output all possible allocno classes and the translation map into
-   file F.  */
+/* Output all unifrom and important classes into file F.  */
+static void
+print_unform_and_important_classes (FILE *f)
+{
+  static const char *const reg_class_names[] = REG_CLASS_NAMES;
+  int i, cl;
+
+  fprintf (f, "Uniform classes:\n");
+  for (cl = 0; cl < N_REG_CLASSES; cl++)
+if (ira_uniform_class_p[cl])
+  fprintf (f, " %s", reg_class_names[cl]);
+  fprintf (f, "\nImportant classes:\n");
+  for (i = 0; i < ira_important_classes_num; i++)
+fprintf (f, " %s", reg_class_names[ira_important_classes[i]]);
+  fprintf (f, "\n");
+}
+
+/* Output all possible allocno or pressure classes and their
+   translation map into file F.  */
 static void
-print_classes (FILE *f, bool pressure_p)
+print_translated_classes (FILE *f, bool pressure_p)
 {
   int classes_num = (pressure_p
 		 ? ira_pressure_classes_num : ira_allocno_classes_num);
@@ -1321,8 +1378,9 @@ print_classes (FILE *f, bool pressure_p)
 void
 ira_debug_allocno_classes (void)
 {
-  print_classes (stderr, false);
-  print_classes (stderr, true);
+  print_unform_and_important_classes (stderr);
+  print_translated_classes (stderr, false);
+  print_translated_classes (stderr, true);
 }
 
 /* Set up different arrays concerning class subsets, allocno and
Index: ira-costs.c
===
--- ira-costs.c	(revision 188124)
+++ ira-costs.c	(working copy)
@@ -239,33 +239,19 @@ setup_regno_cost_classes_by_aclass (int
   COPY_HARD_REG_SET (temp, reg_class_contents[aclass]);
   AND_COMPL_HARD_REG_SET (temp, ira_no_alloc_regs);
   /* We exclu

Re: [C++] Return error_mark_node from cp_parser_constant_expression

[Jason Merrill]

On 06/04/2012 06:36 AM, Florian Weimer wrote:

(Sorry if Thunderbird has garbled the changelog entries.)


I add the ChangeLog to the top of the patch to avoid this.  :)


-  if (TREE_CODE (w) != INTEGER_CST)
+  if (w == error_mark_node)
+   ; /* Continue with error processing below. */
+  else if (TREE_CODE (w) != INTEGER_CST)



- error ("enumerator value for %qD is not an integer constant",
-name);
+ if (value != error_mark_node)
+   error ("enumerator value for %qD is not an integer constant",
+  name);


Hmm, I don't see these errors as redundant, but rather as giving context 
for the previous error.



+  else if (!parser->integral_constant_expression_p
+  && !parser->non_integral_constant_expression_p)
+expression = error_mark_node;


This doesn't make sense to me.  parser->integral_constant_expression_p 
should always be true at this point if you're moving the restore later 
(which also seems unnecessary).


Jason

Re: [C++ Patch] PR 53524

[Paolo Carlini]

On 06/04/2012 08:24 PM, Jason Merrill wrote:

On 06/04/2012 01:52 PM, Paolo Carlini wrote:

Ok, this would be simple to do. The only issue I can see, is that in C++
-Wenum-compare has a name, thus can be easily disabled but it's ON by
default.
The warning is already on by default, so that wouldn't be a change; 
this just creates a way for users to turn it off until we deal with 
the unhelpful case.
Ok. The below passes the testsuite on x86_64-linux. Ok for mainline and 
4.7.1?


Thanks,
Paolo.

/
2012-06-04  Paolo Carlini  

PR c++/53524
* doc/invoke.texi (Wenum-compare): Update documentation.

/cp
2012-06-04  Paolo Carlini  

PR c++/53524
* call.c (build_conditional_expr_1): Use OPT_Wenum_compare
to control enumeral mismatch in conditional expression too.

/testsuite
2012-06-04  Paolo Carlini  

PR c++/53524
* g++.dg/warn/Wenum-compare-no-2: New.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 188196)
+++ doc/invoke.texi (working copy)
@@ -4297,9 +4297,10 @@ while} statement.  This warning is also enabled by
 @item -Wenum-compare
 @opindex Wenum-compare
 @opindex Wno-enum-compare
-Warn about a comparison between values of different enumerated types. In C++
-this warning is enabled by default.  In C this warning is enabled by
-@option{-Wall}.
+Warn about a comparison between values of different enumerated types.
+In C++ enumeral mismatches in conditional expressions are also
+diagnosed and the warning is enabled by default.  In C this warning is 
+enabled by @option{-Wall}.
 
 @item -Wjump-misses-init @r{(C, Objective-C only)}
 @opindex Wjump-misses-init
Index: testsuite/g++.dg/warn/Wenum-compare-no-2.C
===
--- testsuite/g++.dg/warn/Wenum-compare-no-2.C  (revision 0)
+++ testsuite/g++.dg/warn/Wenum-compare-no-2.C  (revision 0)
@@ -0,0 +1,31 @@
+// PR c++/53524
+// { dg-options "-Wno-enum-compare" }
+
+template < typename > struct PointerLikeTypeTraits {
+  enum { NumLowBitsAvailable };
+};
+
+class CodeGenInstruction;
+class CodeGenInstAlias;
+
+template < typename T>
+struct PointerIntPair {
+  enum { IntShift = T::NumLowBitsAvailable };
+};
+
+template < typename PT1, typename PT2 > struct PointerUnionUIntTraits {
+  enum {
+PT1BitsAv = PointerLikeTypeTraits < PT1 >::NumLowBitsAvailable,
+PT2BitsAv = PointerLikeTypeTraits < PT2 >::NumLowBitsAvailable,
+NumLowBitsAvailable = 0 ? PT1BitsAv : PT2BitsAv
+  };
+};
+
+template < typename PT1, typename PT2 > class PointerUnion {
+  typedef PointerIntPair < PointerUnionUIntTraits < PT1, PT2 > > ValTy;
+  ValTy Val;
+};
+
+struct ClassInfo {
+  PointerUnion < CodeGenInstruction *, CodeGenInstAlias * > DefRec;
+};
Index: testsuite/g++.dg/cpp0x/alias-decl-19.C
===
Index: cp/call.c
===
--- cp/call.c   (revision 188196)
+++ cp/call.c   (working copy)
@@ -4696,7 +4696,7 @@ build_conditional_expr_1 (tree arg1, tree arg2, tr
  && TREE_CODE (arg3_type) == ENUMERAL_TYPE)
 {
   if (complain & tf_warning)
-warning (0, 
+warning (OPT_Wenum_compare, 
  "enumeral mismatch in conditional expression: %qT vs %qT",
  arg2_type, arg3_type);
 }

Re: [PATCH 2/2] Better system header location detection for built-in macro tokens

[Mike Stump]

On Jun 4, 2012, at 5:36 AM, Dodji Seketeli wrote:
>>> -  s = s; // { dg-message "synthesized|deleted" }
>>> +  s = s;
>> 
>> Why do we lose this error?
> 
> Good question.
> 
> This is the result of a weird behaviour of DejaGNU, I think.

Yup, that is truly weird.  The problem is that the | operator is binding funny 
because there are no () around the substituted variables in the matching 
expressions in the .exp files, and the messages for a single line get the 
opportunity to match and eat multiple lines.

So, dejagnu is using:

{[0-9]+: error:[^\n]*const|operator}

instead of:

{[0-9]+: error:[^\n]*(const|operator)}

and that makes all the difference.

If you want, you can use just const, like so:

template 
struct S {  // { dg-error "const" }
  S();
  T t;
};

The key is not to use |, or if you want to use |, you can (for now, until the 
underlying bug is fix) use:

template 
struct S {  // { dg-error "(const|operator)" }
  S();
  T t;
}

to fix the underlying problem, we need something like:

Index: gcc-dg.exp
===
--- gcc-dg.exp  (revision 188120)
+++ gcc-dg.exp  (working copy)
@@ -744,6 +744,12 @@ if { [info procs saved-dg-error] == [lis
 proc process-message { msgproc msgprefix dgargs } {
 upvar dg-messages dg-messages
 
+# We must enclose the expression in () to ensure any | in the
+# pattern doesn't escape from this part of the expression.
+set darg1 [lindex $dgargs 0]
+set dgargs [lindex $dgargs 1]
+set dgargs "$darg1 ($dgargs)"
+
 # Process the dg- directive, including adding the regular expression
 # to the new message entry in dg-messages.
 set msgcnt [llength ${dg-messages}]


which I'll test and see how bad off any other test cases are.  If people on 
fast machines want to weigh in, that'd be good.

Re: _FORTIFY_SOURCE for std::vector

[Marc Glisse]

On Mon, 4 Jun 2012, Florian Weimer wrote:


On 06/01/2012 01:34 PM, Jakub Jelinek wrote:

Have you looked at the assembly differences with this in?


It's not great.

Here's an example:

void
write(std::vector& blob, unsigned n, float v1, float v2, float v3, 
float v4)

{
 blob[n] = v1;
 blob[n + 1] = v2;
 blob[n + 2] = v3;
 blob[n + 3] = v4;
}


Would be great if it ended up testing only n and n+3.
__attribute__((__noreturn__)) is not quite strong enough to allow this 
optimization, it would require something like 
__attribute__((__crashing__)) to let the compiler know that if the 
function is called, you don't care what happens to blob. And possibly the 
use of a signed n.


Note that even when the optimization would be legal, gcc seems to have a 
few difficulties:


extern "C" void fail() __attribute((noreturn));
void write(signed m, signed n)
{
  if((n+3)>m) fail();
  if((n+2)>m) fail();
  if((n+1)>m) fail();
  if(n>m) fail();
}

keeps 3 tests.

--
Marc Glisse

Re: Fix typo in intrinsics/time_1.h

[Janne Blomqvist]

On Fri, Jun 1, 2012 at 8:08 PM, rbmj  wrote:
> Hi all,
>
> In libgfortran/intrinsics/time_1.h:181, there is a typo that refers to
> user_usecs (should be user_usec).  This patch fixes it.  I don't have commit
> access, so someone please apply this for me.  It should be obvious.

Thanks for catching this. Patch committed.


-- 
Janne Blomqvist

Re: [C++ Patch] PR 53524

[Jason Merrill]

On 06/04/2012 02:50 PM, Paolo Carlini wrote:

Ok. The below passes the testsuite on x86_64-linux. Ok for mainline and
4.7.1?


OK.  But let's leave the bug open until we fix the warning not to 
complain about this testcase even with -Wenum-compare on.


Jason

Re: [C++ Patch] PR 53524

[Paolo Carlini]

On 06/04/2012 09:16 PM, Jason Merrill wrote:

On 06/04/2012 02:50 PM, Paolo Carlini wrote:

Ok. The below passes the testsuite on x86_64-linux. Ok for mainline and
4.7.1?


OK.  But let's leave the bug open until we fix the warning not to 
complain about this testcase even with -Wenum-compare on.

Sure.

Paolo.

Re: Changed table name. (issue6244048)

[Tom Tromey]

> "Lawrence" == Lawrence Crowl  writes:

Lawrence> Because libcpp was using a typedef hash_table, that typedef has been
Lawrence> renamed cpp_hash_table and uses of it have changed.

The libcpp changes are ok.

Tom

Re: [PATCH 2/2] Better system header location detection for built-in macro tokens

[Jason Merrill]

On 06/04/2012 02:54 PM, Mike Stump wrote:

Yup, that is truly weird.  The problem is that the | operator is binding funny 
because there are no () around the substituted variables in the matching 
expressions in the .exp files, and the messages for a single line get the 
opportunity to match and eat multiple lines.

So, dejagnu is using:

{[0-9]+: error:[^\n]*const|operator}

instead of:

{[0-9]+: error:[^\n]*(const|operator)}

and that makes all the difference.


That was my guess, too, but when I look at dg.exp it seems that the 
pattern is properly wrapped in parens.


Jason

Re: _FORTIFY_SOURCE for std::vector

[Florian Weimer]

On 06/04/2012 09:07 PM, Marc Glisse wrote:

On Mon, 4 Jun 2012, Florian Weimer wrote:

void
write(std::vector& blob, unsigned n, float v1, float v2, float
v3, float v4)
{
blob[n] = v1;
blob[n + 1] = v2;
blob[n + 2] = v3;
blob[n + 3] = v4;
}


Would be great if it ended up testing only n and n+3.


True.


__attribute__((__noreturn__)) is not quite strong enough to allow this
optimization, it would require something like
__attribute__((__crashing__)) to let the compiler know that if the
function is called, you don't care what happens to blob. And possibly
the use of a signed n.


Interesting point, I had not realized that before.  Ada has a special 
rule for failures of language-defined checks, and they might give enough 
wiggle room to leave behind a partially updated vector in such situations.


But even without that, you could clone the if sequence, that is,

  if (blob.size() - n >= 4)
{
  blob[n] = v1;
  blob[n + 1] = v2;
  blob[n + 2] = v3;
  blob[n + 3] = v4;
}
  else
{
   ... // individual checks
}

Obviously, this has quite a bit of an impact on code size.

--
Florian Weimer / Red Hat Product Security Team

Ping - Re: [Patch] Add AC_ARG_ENABLE for libstdc++-v3

[rbmj]

On 05/22/2012 04:37 PM, rbmj wrote:
This patch adds an AC_ARG_ENABLE option to build/not build 
libstdc++-v3.  I wrote the patch in order to allow the user to 
override the automatic disable for libstdc++-v3 for certain targets.


Hi all,

What are the barriers to getting this in?  I wrote this so I don't have 
to continuously patch the configure script, and instead can override it 
with a command line option for my build targeting powerpc-wrs-vxworks.  
Even though VxWorks has it's own libstdc++, that doesn't mean that one 
wouldn't want to build it in order to get a more recent version than 
stock.  If it needs to be documented if someone can point me to the 
proper spot I can do that as well.


Thanks,

Robert Mason

[patch][gcc47] Deprecate -fconserve-space

[Steven Bosscher]

(xf. http://gcc.gnu.org/ml/gcc/2012-06/msg00043.html)

On Mon, Jun 4, 2012 at 7:05 PM, Jason Merrill  wrote:
>>> Jason, is this flag something that we could deprecate for GCC 4.7 and
>>> remove?
>>
>>
>> The docs say the flag is not useful these days as BSS is widely available.
>
>
> Right.  So yes, go ahead.

Hello,

Here are the patches for wwwdocs and GCC 4.7 branch. Bootstrapped and
tested on x86_64-unknown-linux-gnu. OK?

Ciao!
Steven


Index: htdocs/gcc-4.7/changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.110
diff -u -r1.110 changes.html
--- htdocs/gcc-4.7/changes.html 1 Jun 2012 09:27:10 -   1.110
+++ htdocs/gcc-4.7/changes.html 4 Jun 2012 19:35:22 -
@@ -15,6 +15,11 @@
 Caveats

   
+The -fconserve-space flag has been
+deprecated.  The flag only had an effect for G++ but could
+result in wrong semantics (please refer to the GCC manual
+for further details).  The flag will be removed in GCC 4.8
+
 Support for a number of older systems and recently
 unmaintained or untested target ports of GCC has been declared
 obsolete in GCC 4.7.  Unless there is activity to revive them, the


gcc/
* doc/invoke.texi (item -fconserve-space): This flag is deprecated

c-family/
* c.opt (fconserve-space): Deprecate.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 188202)
+++ doc/invoke.texi (working copy)
@@ -1940,6 +1940,8 @@ exhaustion is signalled by throwing @cod

 @item -fconserve-space
 @opindex fconserve-space
+This option is now deprecated.
+
 Put uninitialized or run-time-initialized global variables into the
 common segment, as C does.  This saves space in the executable at the
 cost of not diagnosing duplicate definitions.  If you compile with this
Index: c-family/c.opt
===
--- c-family/c.opt  (revision 188202)
+++ c-family/c.opt  (working copy)
@@ -745,7 +745,7 @@ Allow the arguments of the '?' operator

 fconserve-space
 C++ ObjC++ Var(flag_conserve_space)
-Reduce the size of object files
+Deprecated.  Reduce the size of object files, at the expense of wrong
semantics.

 fconstant-string-class=
 ObjC ObjC++ Joined MissingArgError(no class name specified with %qs)

Re: [patch][gcc47] Deprecate -fconserve-space

[Jason Merrill]

In wwwdocs I would also mention that the flag has no effect on most 
targets.  In fact, I'm not sure it affects any targets that we still 
actually support; are there any a.out targets left?


Jason

Re: [C++] Return error_mark_node from cp_parser_constant_expression

[Florian Weimer]

On 06/04/2012 08:40 PM, Jason Merrill wrote:

On 06/04/2012 06:36 AM, Florian Weimer wrote:

(Sorry if Thunderbird has garbled the changelog entries.)


I add the ChangeLog to the top of the patch to avoid this. :)


Good idea.


- if (TREE_CODE (w) != INTEGER_CST)
+ if (w == error_mark_node)
+ ; /* Continue with error processing below. */
+ else if (TREE_CODE (w) != INTEGER_CST)



- error ("enumerator value for %qD is not an integer constant",
- name);
+ if (value != error_mark_node)
+ error ("enumerator value for %qD is not an integer constant",
+ name);


Hmm, I don't see these errors as redundant, but rather as giving context
for the previous error.


Even with caret diagnostics?  Anyway, I will try to resolve this in a 
different way.



+ else if (!parser->integral_constant_expression_p
+ && !parser->non_integral_constant_expression_p)
+ expression = error_mark_node;


This doesn't make sense to me. parser->integral_constant_expression_p
should always be true at this point if you're moving the restore later
(which also seems unnecessary).


I think parser->integral_constant_expression_p reflects the result of 
the cp_parser_assignment_expression() call earlier in this function.


--
Florian Weimer / Red Hat Product Security Team

Re: [patch][gcc47] Deprecate -fconserve-space

[Steven Bosscher]

On Mon, Jun 4, 2012 at 9:45 PM, Jason Merrill  wrote:
> In wwwdocs I would also mention that the flag has no effect on most targets.

How about this:

Index: htdocs/gcc-4.7/changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.110
diff -u -r1.110 changes.html
--- htdocs/gcc-4.7/changes.html 1 Jun 2012 09:27:10 -   1.110
+++ htdocs/gcc-4.7/changes.html 4 Jun 2012 20:29:09 -
@@ -15,6 +15,14 @@
 Caveats

   
+The -fconserve-space flag has been
+deprecated.  The flag had no effect for most targets: only
+targets without a global .bss section and without
+support for switchable sections.  Furthermore, the flag only
+had an effect for G++, where it could result in wrong semantics
+(please refer to the GCC manual for further details).
+The flag will be removed in GCC 4.8
+
 Support for a number of older systems and recently
 unmaintained or untested target ports of GCC has been declared
 obsolete in GCC 4.7.  Unless there is activity to revive them, the

?

>  In fact, I'm not sure it affects any targets that we still actually
> support; are there any a.out targets left?

There is still a file config/arm/aout.h, and there appears to be code
in config/cris/cris.c to support an unnamed non-ELF target that may be
a.out. But it's hard to tell what other targets are doing.

Ciao!
Steven

Re: [C++] Return error_mark_node from cp_parser_constant_expression

[Gabriel Dos Reis]

>>> - error ("enumerator value for %qD is not an integer constant",
>>> - name);
>>> + if (value != error_mark_node)
>>> + error ("enumerator value for %qD is not an integer constant",
>>> + name);
>>
>>
>> Hmm, I don't see these errors as redundant, but rather as giving context
>> for the previous error.
>
>
> Even with caret diagnostics?

caret is optional.

> Anyway, I will try to resolve this in a different way.

Re: Ping - Re: [Patch] Add AC_ARG_ENABLE for libstdc++-v3

[Paolo Carlini]

On 06/04/2012 09:35 PM, rbmj wrote:

On 05/22/2012 04:37 PM, rbmj wrote:
This patch adds an AC_ARG_ENABLE option to build/not build 
libstdc++-v3.  I wrote the patch in order to allow the user to 
override the automatic disable for libstdc++-v3 for certain targets.


Hi all,

What are the barriers to getting this in?
Remember that all the libstdc++-v3 patches should also go to 
libstd...@gcc.gnu.org.


Thanks,
Paolo.

Re: [Patch, Fortran] PR50619 - fix ASSOCIATE with -finit-*

[Thomas Koenig]

Hi Tobias,


ASSOCIATE names are (wrongly) initialized by -finit-real=*, thus, their
values are wrong.

The fix seems to be rather obvious, but I am not really an associate
expert.


I'd say it is obvious. OK for trunk, also OK for backporting.
I am unsure whether to put it into 4.7.1, though. Opinions?

Regards

Thomas

Re: [patch][gcc47] Deprecate -fconserve-space

[Jason Merrill]

On 06/04/2012 04:30 PM, Steven Bosscher wrote:

How about this:


OK, looks good.


There is still a file config/arm/aout.h


I noticed that, but it seems to be used together with elfos.h, so it 
seems not to be a.out-specific.  :)


Jason

Re: [C++] Return error_mark_node from cp_parser_constant_expression

[Jason Merrill]

On 06/04/2012 04:12 PM, Florian Weimer wrote:

This doesn't make sense to me. parser->integral_constant_expression_p
should always be true at this point if you're moving the restore later
(which also seems unnecessary).


I think parser->integral_constant_expression_p reflects the result of
the cp_parser_assignment_expression() call earlier in this function.


parser->integral_constant_expression_p is set to indicate that the 
current context expects an integral constant expression; the call to 
cp_parser_assignment_expression will not affect that.


If the expression turns out not to be a valid constant expression, then 
we set parser->non_integral_constant_expression_p to true, but we don't 
touch parser->integral_constant_expression_p.  I think the condition you 
want is


if (parser->non_integral_constant_expression_p
&& !allow_non_constant_p)

Jason

Re: [PATCH 2/2] Better system header location detection for built-in macro tokens

[Mike Stump]

On Jun 4, 2012, at 12:30 PM, Jason Merrill wrote:
> On 06/04/2012 02:54 PM, Mike Stump wrote:
>> Yup, that is truly weird.  The problem is that the | operator is binding 
>> funny because there are no () around the substituted variables in the 
>> matching expressions in the .exp files, and the messages for a single line 
>> get the opportunity to match and eat multiple lines.
>> 
>> So, dejagnu is using:
>> 
>>  {[0-9]+: error:[^\n]*const|operator}
>> 
>> instead of:
>> 
>>  {[0-9]+: error:[^\n]*(const|operator)}
>> 
>> and that makes all the difference.
> 
> That was my guess, too, but when I look at dg.exp it seems that the pattern 
> is properly wrapped in parens.

That one is, the problem is the work that added error: and warning: processing 
differentiation puts together a new expression with concatenation and it is 
that use that _also_ needs protecting.

error: CONCAT regexp

is wrong, whereas:

error: CONCAT (regexp)

is correct.  This type of patch:

-   set expmsg "$column: $msgprefix\[^\n\]*$expmsg"
+   set expmsg "$column: $msgprefix\[^\n\]*($expmsg)"

might have been easier to understand, but, that repeats three times and is 
dependent upon people getting the obscure detail of () correct.  I instead add 
the () at the top, to ensure it is impossible to ever have this error again, no 
matter the code rearrangements and to have it appear just once, instead of 
three times.  Possibly:

-   set expmsg [lindex $newentry 2]
+   set expmsg "([lindex $newentry 2])"

might be better...

Re: [PATCH 2/2] Better system header location detection for built-in macro tokens

[Jason Merrill]

On 06/04/2012 05:27 PM, Mike Stump wrote:

That one is, the problem is the work that added error: and warning: processing 
differentiation puts together a new expression with concatenation and it is 
that use that _also_ needs protecting.


Aha!  Sneaky gcc-dg.exp.


-   set expmsg [lindex $newentry 2]
+   set expmsg "([lindex $newentry 2])"


Looks good to me.

Jason

Minor dwarf2out PATCH to add is_unit_die

[Jason Merrill]

Now that there are three different varieties of unit, it seems useful to 
have a single predicate for them.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit c879217dbfeaddebdde9121eb6750b75f650fe9f
Author: Jason Merrill 
Date:   Sun May 6 12:24:05 2012 -0400

	* dwarf2out.c (is_unit_die): New.
	(copy_declaration_context, copy_ancestor_tree): Use it.
	(copy_decls_walk): Likewise.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index babc5ba..3e92a14 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -5972,6 +5972,16 @@ is_cu_die (dw_die_ref c)
   return c && c->die_tag == DW_TAG_compile_unit;
 }
 
+/* Returns true iff C is a unit DIE of some sort.  */
+
+static inline bool
+is_unit_die (dw_die_ref c)
+{
+  return c && (c->die_tag == DW_TAG_compile_unit
+	   || c->die_tag == DW_TAG_partial_unit
+	   || c->die_tag == DW_TAG_type_unit);
+}
+
 static char *
 gen_internal_sym (const char *prefix)
 {
@@ -6379,8 +6389,7 @@ copy_declaration_context (dw_die_ref unit, dw_die_ref die)
 }
 
   if (decl->die_parent != NULL
-  && decl->die_parent->die_tag != DW_TAG_compile_unit
-  && decl->die_parent->die_tag != DW_TAG_type_unit)
+  && !is_unit_die (decl->die_parent))
 {
   new_decl = copy_ancestor_tree (unit, decl, NULL);
   if (new_decl != NULL)
@@ -6654,8 +6663,7 @@ copy_ancestor_tree (dw_die_ref unit, dw_die_ref die, htab_t decl_table)
   dw_die_ref spec = get_AT_ref (parent, DW_AT_specification);
   if (spec != NULL)
 parent = spec;
-  if (parent->die_tag != DW_TAG_compile_unit
-  && parent->die_tag != DW_TAG_type_unit)
+  if (!is_unit_die (parent))
 new_parent = copy_ancestor_tree (unit, parent, decl_table);
 }
 
@@ -6754,8 +6762,7 @@ copy_decls_walk (dw_die_ref unit, dw_die_ref die, htab_t decl_table)
   /* If TARG has surrounding context, copy its ancestor tree
  into the new type unit.  */
   if (targ->die_parent != NULL
-  && targ->die_parent->die_tag != DW_TAG_compile_unit
-  && targ->die_parent->die_tag != DW_TAG_type_unit)
+		  && !is_unit_die (targ->die_parent))
 parent = copy_ancestor_tree (unit, targ->die_parent,
  decl_table);
 

Re: User directed Function Multiversioning via Function Overloading (issue5752064)

[H.J. Lu]

On Mon, Jun 4, 2012 at 11:59 AM, Sriraman Tallam  wrote:
> Hi,
>
>   Attaching updated patch for function multiversioning which brings
> in plenty of changes.
>
> * As suggested by Richard earlier, I have made cgraph aware of
> function versions. All nodes of function versions are chained and the
> dispatcher bodies are created on demand while building cgraph edges.
> The dispatcher body will be created if and only if there is a call or
> reference to a versioned function. Previously, I was maintaining the
> list of versions separately in a hash map, all that is gone now.
> * Now, the file multiverison.c has some helper routines that are used
> in the context of function versioning. There are no new passes and no
> new globals.
> * More tests, updated existing tests.
> * Fixed lots of bugs.
> * Updated patch description.
>
> Patch attached. Patch also available for review at
> http://codereview.appspot.com/5752064
>
> Please let me know what you think,
>

Build failed in libstdc++-v3:

/export/build/gnu/gcc/build-x86_64-linux/x86_64-unknown-linux-gnu/libstdc++-v3/include/bits/locale_classes.h:546:59:
internal compiler error: tree check: expected function_decl, have
identifier_node in tourney, at cp/call.c:8498
  for (size_t __i = 0; __ret && __i < _S_categories_size - 1; ++__i)
   ^
Please submit a full bug report,
with preprocessed source if appropriate.
See  for instructions.
make[5]: *** [x86_64-unknown-linux-gnu/bits/stdc++.h.gch/O2g.gch] Erro

on Linux/x86-64.


-- 
H.J.

Re: [patch][gcc47] Deprecate -fconserve-space

[Mike Stump]

On Jun 4, 2012, at 12:45 PM, Jason Merrill wrote:
> In wwwdocs I would also mention that the flag has no effect on most targets.  
> In fact, I'm not sure it affects any targets that we still actually support; 
> are there any a.out targets left?

The flag can go...  The days of not having bss are long gone...  Even AT&T had 
bss support for a.out.  The issue was if the assembler and linker didn't 
supported .bss, one would get hosed on ROM size.

[PATCH] PR 53487 - Fix isel on powerpc to work again

[Michael Meissner]

This patch fixes PR 53487, so that -misel will no longer cause an unknown insn
error.  Andrew Pinksi's changes on May 4th, 2012, changed to use the
movcc pattern instead of the cstore4 in some cases, when a target
machine defines both patterns.  The cstore4 pattern had checks in it to
prevent ISELs involving floating point compares from being generated, but
movcc did not.  In addition, building Spec showed that going through
movcc also needed to make sure a comparison against a negative integer
constant like cstore4 also had.

I have bootstrapped the compiler (using BOOT_CFLAGS='-g -O2 -mcpu=power7
-misel) and there were no regressions in the test suite against an unpatched
compiler.  I have also built the Spec 2006 suite with isel, and it generated no
errors.  Is this patch ok to apply?

[gcc]
2012-06-04  Michael Meissner  

PR target/53487
* config/rs6000/rs6000.c (rs6000_generate_compare): If we are
doing an unsigned compare, make sure the second argument is not a
negative constant.
(rs6000_emit_cmove): Don't allow floating point comparisons when
generating ISEL moves.

[gcc/testsuite]
2012-06-04  Michael Meissner  

* gcc.target/powerpc/pr53487.c: New test.

-- 
Michael Meissner, IBM
5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 188186)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -15361,6 +15361,16 @@ rs6000_generate_compare (rtx cmp, enum m
   else
 comp_mode = CCmode;
 
+  /* If we have an unsigned compare, make sure we don't have a signed value as
+ an immediate.  */
+  if (comp_mode == CCUNSmode && GET_CODE (op1) == CONST_INT
+  && INTVAL (op1) < 0)
+{
+  op0 = copy_rtx_if_shared (op0);
+  op1 = force_reg (GET_MODE (op0), op1);
+  cmp = gen_rtx_fmt_ee (code, GET_MODE (cmp), op0, op1);
+}
+
   /* First, the compare.  */
   compare_result = gen_reg_rtx (comp_mode);
 
@@ -16114,6 +16124,11 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx
   if (GET_MODE (false_cond) != result_mode)
 return 0;
 
+  /* Don't allow using floating point comparisons for integer results for
+ now.  */
+  if (FLOAT_MODE_P (compare_mode) && !FLOAT_MODE_P (result_mode))
+return 0;
+
   /* First, work out if the hardware can do this at all, or
  if it's too slow  */
   if (!FLOAT_MODE_P (compare_mode))
Index: gcc/testsuite/gcc.target/powerpc/pr53487.c
===
--- gcc/testsuite/gcc.target/powerpc/pr53487.c  (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr53487.c  (revision 0)
@@ -0,0 +1,27 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-O3 -mcpu=power7 -misel -ffast-math" } */
+
+struct phylo_s {
+  int left;
+};
+
+int Cluster(float **dmx, int N, struct phylo_s *tree)
+{
+  float **mx;
+  int *coord;
+  int i;
+  int Np;
+  int row, col;
+  float min;
+  for (col = 0; col < N; Np--)
+{
+  for (row = 0; row < Np; row++)
+   for (col = row+1; col < Np; col++)
+ if (mx[row][col] < min)
+   i = row;
+  tree[Np-2].left = coord[i];
+}
+  Free2DArray((void **) mx, N);
+}

Re: [google/main] Copy TREE_STATIC() property from id in dwarf2asm.c (issue 6273045)

[asharif]

On 2012/06/02 01:52:28, asharif wrote:

A trunk version of this patch is already under review. I'm creating

this CL in

order to get this patch in before the branch date.


Ping.

http://codereview.appspot.com/6273045/

[PATCH][Cilkplus] Fix a bug when using throw () in function

[Iyer, Balaji V]

 
Hello Everyone,
   This patch is for the Cilkplus branch affecting the C++ compiler. The 
compiler crashes when a throw() is set for a spawned function. This patch 
should fix that.

Thanks,

Balaji V. iyer.Index: gcc/cp/cilk.c
===
--- gcc/cp/cilk.c   (revision 188209)
+++ gcc/cp/cilk.c   (working copy)
@@ -872,7 +872,6 @@
  obody, dtor),
  &body);
 
-  TREE_CHAIN (obody) = body;
   *saved_tree = body;
 }
 
Index: gcc/cp/ChangeLog.cilk
===
--- gcc/cp/ChangeLog.cilk   (revision 188209)
+++ gcc/cp/ChangeLog.cilk   (working copy)
@@ -1,3 +1,8 @@
+2012-06-04  Balaji V. Iyer  
+
+   * cilk.c (cp_make_cilk_frame): Removed adding body to the orig body's
+   tree chain.
+
 2012-05-30  Balaji V. Iyer  
 
* semantics.c (finish_call_expr): Used spawning for call_type instead of

[wwwdocs] Make codingconventions.html pass W3 validator.

[Lawrence Crowl]

The following source change enables coddingconventions.html to
pass the HTML validator at validator.w3.org.

Okay?

Index: codingconventions.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/codingconventions.html,v
retrieving revision 1.66
diff -u -r1.66 codingconventions.html
--- codingconventions.html  19 Feb 2012 00:45:34 -  1.66
+++ codingconventions.html  4 Jun 2012 22:39:15 -
@@ -1,6 +1,9 @@
+http://www.w3.org/TR/html4/strict.dtd";>
 

 
+
 GCC Coding Conventions
 

-- 
Lawrence Crowl

Re: [wwwdocs] Make codingconventions.html pass W3 validator.

[Gabriel Dos Reis]

On Mon, Jun 4, 2012 at 5:42 PM, Lawrence Crowl  wrote:
> The following source change enables coddingconventions.html to
> pass the HTML validator at validator.w3.org.
>
> Okay?

Looks obvious! :-)

-- gaby

Re: PATCH: PR bootstrap/53555: [4.8 Regression] Bootstrap failure

[Uros Bizjak]

On Sat, Jun 2, 2012 at 3:27 PM, H.J. Lu  wrote:

> ix86_sched_reorder should skip debug insns.  Tested on Linux/ia32.
> OK to install?
>
> 2012-06-02  H.J. Lu  
>
>        PR bootstrap/53555
>        * config/i386/i386.c (ix86_sched_reorder) Skip debug insns.

OK.

Thanks,
Uros.

Re: [C++ Patch] PR 53567

[Jason Merrill]

On 06/04/2012 06:55 PM, Paolo Carlini wrote:

If, more generally, you mean we should remove it completely, I'm afraid
some cases are rather nasty. For example, I think that whenever we go
through implicit_conversion, thus its flags &=, the following function
calls can easily have at the same time complain & tf_error true and
flags & LOOKUP_COMPLAIN false.


In cases where we currently don't set LOOKUP_COMPLAIN in flags, we can 
instead set complain to tf_none.


Jason

Re: [C++ Patch] PR 53567

[Paolo Carlini]

On 06/05/2012 01:23 AM, Jason Merrill wrote:

On 06/04/2012 06:55 PM, Paolo Carlini wrote:

If, more generally, you mean we should remove it completely, I'm afraid
some cases are rather nasty. For example, I think that whenever we go
through implicit_conversion, thus its flags &=, the following function
calls can easily have at the same time complain & tf_error true and
flags & LOOKUP_COMPLAIN false.


In cases where we currently don't set LOOKUP_COMPLAIN in flags, we can 
instead set complain to tf_none.
Ok, I'll see what happens in the various specific circumstances. Indeed, 
earlier today I wondered myself if in principle not setting 
LOOKUP_COMPLAIN was just tf_none, but looking at the code things seemed 
more complex, when eg we are currently warning without checking 
LOOKUP_COMPLAIN and with complain available normally we would protect 
the warning with complain & tf_warning. But I don't want to speak too 
early ;) Let me see.


Thanks!
Paolo.

Re: [C++ Patch] PR 53567

[Paolo Carlini]

On 06/05/2012 01:45 AM, Paolo Carlini wrote:

On 06/05/2012 01:23 AM, Jason Merrill wrote:

On 06/04/2012 06:55 PM, Paolo Carlini wrote:

If, more generally, you mean we should remove it completely, I'm afraid
some cases are rather nasty. For example, I think that whenever we go
through implicit_conversion, thus its flags &=, the following function
calls can easily have at the same time complain & tf_error true and
flags & LOOKUP_COMPLAIN false.


In cases where we currently don't set LOOKUP_COMPLAIN in flags, we 
can instead set complain to tf_none.
Ok, I'll see what happens in the various specific circumstances. 
Indeed, earlier today I wondered myself if in principle not setting 
LOOKUP_COMPLAIN was just tf_none, but looking at the code things 
seemed more complex, when eg we are currently warning without checking 
LOOKUP_COMPLAIN and with complain available normally we would protect 
the warning with complain & tf_warning. But I don't want to speak too 
early ;) Let me see.

In fact, I think setting complain to tf_warning may work better.

Paolo.

[contrib] Extend validate_failures.py to run outside the build directory (issue6268051)

[Diego Novillo]

This patch adds three new arguments to validate_failures.py so
it can be used outside the build directory:

--ignore_missing_failures
When a failure is expected in the manifest but it is not found in
the actual results, the script produces a note alerting to this
fact. This means that the expected failure has been fixed, or it
did not run, or it may simply be flaky.

With this option, one can ask the script not to show the missing
failures.

--manifest
Name of the manifest file to use.  By default, the script will
look for the manifest file in the source directory associated with
this build.  With this option, one can point to any arbitrary
manifest file.  I renamed the old --manifest flag to
--produce_manifest.

--results
Space-separated list of .sum files with the testing results to
check. The only content needed from these files are the lines
starting with FAIL, XPASS or UNRESOLVED.


2012-06-04   Diego Novillo  

* testsuite-management/validate_failures.py (GetResults):
Change argument to accept list of summary files to analyze.
Update callers.
(CheckExpectedResults): Handle options.manifest, options.results
and options.ignore_missing_failures.
(Main): Add options --manifest, --results and --ignore_missing_failures.

diff --git a/contrib/testsuite-management/validate_failures.py 
b/contrib/testsuite-management/validate_failures.py
index c0436e5..c07be84 100755
--- a/contrib/testsuite-management/validate_failures.py
+++ b/contrib/testsuite-management/validate_failures.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python2.6
 
 # Script to compare testsuite failures against a list of known-to-fail
 # tests.
@@ -206,10 +206,8 @@ def GetSumFiles(builddir):
   return sum_files
 
 
-def GetResults(builddir):
-  """Collect all the test results from .sum files under the given build
-  directory."""
-  sum_files = GetSumFiles(builddir)
+def GetResults(sum_files):
+  """Collect all the test results from the given .sum files."""
   build_results = set()
   for sum_fname in sum_files:
 print '\t%s' % sum_fname
@@ -258,16 +256,26 @@ def PrintSummary(msg, summary):
 
 
 def CheckExpectedResults(options):
-  (srcdir, target, valid_build) = GetBuildData(options)
-  if not valid_build:
-return False
+  if not options.manifest:
+(srcdir, target, valid_build) = GetBuildData(options)
+if not valid_build:
+  return False
+manifest_name = _MANIFEST_PATH_PATTERN % (srcdir, target)
+  else:
+manifest_name = options.manifest
+if not os.path.exists(manifest_name):
+  Error('Manifest file %s does not exist.' % manifest_name)
 
-  manifest_name = _MANIFEST_PATH_PATTERN % (srcdir, target)
   print 'Manifest: %s' % manifest_name
   manifest = GetManifest(manifest_name)
 
-  print 'Getting actual results from build'
-  actual = GetResults(options.build_dir)
+  if not options.results:
+print 'Getting actual results from build'
+sum_files = GetSumFiles(options.build_dir)
+  else:
+print 'Getting actual results from user-provided results'
+sum_files = options.results.split()
+  actual = GetResults(sum_files)
 
   if options.verbosity >= 1:
 PrintSummary('Tests expected to fail', manifest)
@@ -280,7 +288,7 @@ def CheckExpectedResults(options):
 PrintSummary('Build results not in the manifest', actual_vs_manifest)
 tests_ok = False
 
-  if len(manifest_vs_actual) > 0:
+  if not options.ignore_missing_failures and len(manifest_vs_actual) > 0:
 PrintSummary('Manifest results not present in the build'
  '\n\nNOTE: This is not a failure.  It just means that the '
  'manifest expected\nthese tests to fail, '
@@ -314,20 +322,42 @@ def ProduceManifest(options):
 
 def Main(argv):
   parser = optparse.OptionParser(usage=__doc__)
+
+  # Keep the following list sorted by option name.
   parser.add_option('--build_dir', action='store', type='string',
 dest='build_dir', default='.',
 help='Build directory to check (default = .)')
-  parser.add_option('--manifest', action='store_true', dest='manifest',
-default=False, help='Produce the manifest for the current '
-'build (default = False)')
   parser.add_option('--force', action='store_true', dest='force',
-default=False, help='When used with --manifest, it will '
-'overwrite an existing manifest file (default = False)')
+default=False, help='When used with --produce_manifest, '
+'it will overwrite an existing manifest file '
+'(default = False)')
+  parser.add_option('--ignore_missing_failures', action='store_true',
+dest='ignore_missing_failures', default=False,
+help='When a failure is expected in the manifest but '
+'it is not found in the actual resu

Re: [PATCH] PR 53487 - Fix isel on powerpc to work again

[David Edelsohn]

On Mon, Jun 4, 2012 at 5:51 PM, Michael Meissner
 wrote:
> This patch fixes PR 53487, so that -misel will no longer cause an unknown insn
> error.  Andrew Pinksi's changes on May 4th, 2012, changed to use the
> movcc pattern instead of the cstore4 in some cases, when a target
> machine defines both patterns.  The cstore4 pattern had checks in it to
> prevent ISELs involving floating point compares from being generated, but
> movcc did not.  In addition, building Spec showed that going through
> movcc also needed to make sure a comparison against a negative integer
> constant like cstore4 also had.
>
> I have bootstrapped the compiler (using BOOT_CFLAGS='-g -O2 -mcpu=power7
> -misel) and there were no regressions in the test suite against an unpatched
> compiler.  I have also built the Spec 2006 suite with isel, and it generated 
> no
> errors.  Is this patch ok to apply?
>
> [gcc]
> 2012-06-04  Michael Meissner  
>
>        PR target/53487
>        * config/rs6000/rs6000.c (rs6000_generate_compare): If we are
>        doing an unsigned compare, make sure the second argument is not a
>        negative constant.
>        (rs6000_emit_cmove): Don't allow floating point comparisons when
>        generating ISEL moves.
>
> [gcc/testsuite]
> 2012-06-04  Michael Meissner  
>
>        * gcc.target/powerpc/pr53487.c: New test.

Okay.

Thanks, David