Re: LRA has been merged into trunk.

[David Miller]

From: David Miller 
Date: Tue, 23 Oct 2012 22:06:55 -0400 (EDT)

> From: David Miller 
> Date: Tue, 23 Oct 2012 21:44:05 -0400 (EDT)
> 
>> The first issue sparc runs into is that it does not define it's
>> extra constraints properly.  In particular 'T' and 'W' must use
>> define_memory_constraint.
>> 
>> Otherwise the EXTRA_MEMORY_CONSTRAINT logic in process_alt_operands()
>> does not trigger and we therefore cannot even load a constant into
>> floating point registers properly.
> 
> Ok, the next problem we hit will be a little bit more difficult
> to solve.
> 
> Sparc accepts addresses of the form:
> 
> (plus:DI (lo_sum:DI (reg/f:DI 282)
> (symbol_ref:DI ("__mf_opts") ))
> (const_int 40 [0x28]))
> 
> These make use of Sparc's offsetable %lo() relocations.
> 
> But LRA is unable to cope with this expression when it is processed by
> extract_address_regs() because when the LO_SUM is inspected
> ad->disp_loc is already non-NULL triggering this assertion:

So I added a workaround for this, and the next problem we hit involves
PIC.  It stems from the HAVE_lo_sum code block in process_address.

It unconditionally tries to perform a HIGH/LO_SUM sequence to load an
address, and depends upon insn recognition and target backend address
validation to reject such sequences as needed.

Actually, this HAVE_lo_sum code block seems to be an ad-hoc
replacement for a target macro, namely LEGITIMIZE_RELOAD_ADDRESS.

However, on Sparc, LEGITIMIZE_RELOAD_ADDRESS would never emit the
HIGH/LO_SUM sequence for PIC.  Instead, it would leave it to reload to
push the reload and then emit a move of the address into a register,
which in turn would go through all the proper PIC handling logic in
the Sparc backend's move expander.

Before LRA, the target legitimate_address_p would never have to ever
see such strange "(LO_SUM REG SYMBOLIC)" expressions when PIC is
enabled, but now it can and they therefore have to now be explicitly
checked for.

This case triggers every time LRA does a force_const_mem().

I'll add the straightforward check to sparc's legitimate_address_p,
but I'm really surprised you never hit this case in your testing.

At this point I have the C testsuite regressions down to about 20
failures.

patch to fix PR55050 and PR55068

[Vladimir Makarov]

The following patch is to solve

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

and probably

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

Code updating equivalences was added to IRA.  It was supposed for LRA.  
It might create problems for reload because it creates inconsistency in 
dealing with equivs in reload and IRA.


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

Committed as rev. 192797.

2012-10-24  Vladimir Makarov  

PR bootstrap/55068
PR regression/55050
* ira.c (setup_reg_renumber): Fix assert.
* ira-emit.c (emit_move_list): Update equivalences only for LRA.

Index: ira.c
===
--- ira.c   (revision 192742)
+++ ira.c   (working copy)
@@ -1989,6 +1989,7 @@ setup_reg_renumber (void)
  ira_assert (!optimize || flag_caller_saves
  || (ALLOCNO_CALLS_CROSSED_NUM (a)
  == ALLOCNO_CHEAP_CALLS_CROSSED_NUM (a))
+ || regno >= ira_reg_equiv_len
  || ira_equiv_no_lvalue_p (regno));
  caller_save_needed = 1;
}
Index: ira-emit.c
===
--- ira-emit.c  (revision 192742)
+++ ira-emit.c  (working copy)
@@ -947,7 +947,8 @@ emit_move_list (move_t list, int freq)
= gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv_init (regno));
}
}
-  ira_update_equiv_info_by_shuffle_insn (to_regno, from_regno, list->insn);
+  if (ira_use_lra_p)
+   ira_update_equiv_info_by_shuffle_insn (to_regno, from_regno, 
list->insn);
   emit_insn (list->insn);
   mode = ALLOCNO_MODE (list->to);
   aclass = ALLOCNO_CLASS (list->to);

[patch,libgcc] sh-rtems: Add sh*-*-elf*'s extra_parts.

[Ralf Corsepius]

Hi,

I would like to apply the patch below to trunk and gcc-4.7 branch.
GCC doesn't build for sh-rtems* without it.

Ralf

2012-07-10  Ralf Corsépius 

	* config.host (sh*-*-rtems*): Add sh*-*-elf*'s extra_parts. 

diff --git a/libgcc/config.host b/libgcc/config.host
index ef9791b..6c5c825 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -995,7 +996,10 @@ sh-*-netbsdelf* | shl*-*-netbsdelf* | sh5-*-netbsd* | sh5l*-*-netbsd* | \
 sh-*-rtems*)
 	tmake_file="$tmake_file sh/t-sh t-crtstuff-pic t-fdpbit"
 	extra_parts="$extra_parts crt1.o crti.o crtn.o crtbeginS.o crtendS.o \
-		$sh_ic_extra_parts $sh_opt_extra_parts"
+		libic_invalidate_array_4-100.a \
+		libic_invalidate_array_4-200.a \
+		libic_invalidate_array_4a.a \
+		libgcc-Os-4-200.a libgcc-4-300.a"
 	;;
 sh-wrs-vxworks)
 	tmake_file="$tmake_file sh/t-sh t-crtstuff-pic t-fdpbit"

Re: [PATCH] GCC 4.7 and 4.8 PowerPC RTEMS

[Ralf Corsepius]

On 10/25/2012 04:40 AM, Ralf Corsepius wrote:

On 10/25/2012 01:28 AM, David Edelsohn wrote:

Joel and Ralf,

Would you please comment on this patch?

http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02154.html

Thanks, David

This patch had been part of the rtems gcc-patches for months and I
haven't received any complaint about it.

So, this patch is OK with me.


Patch applied to trunk and 4.7-branch.

Ralf

Re: [C++ Patch] PR 34892

[Jason Merrill]

OK.

Jason

Re: [C++ Patch] PR 34892

[Paolo Carlini]

Hi,

On 10/25/2012 03:53 AM, Jason Merrill wrote:

We can stop it even sooner:


  /* If the next token is an ellipsis, and we don't already have it
 marked as a parameter pack, then we have a parameter pack (that
 has no declarator).  */
  if (!*is_parameter_pack
  && cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS)
  && declarator_can_be_parameter_pack 
(parameter_declarator->declarator))


We shouldn't even consider this if there was a default argument.

Ah great, thanks. The below also regtests fine.

Thanks again,
Paolo.


Index: cp/parser.c
===
--- cp/parser.c (revision 192786)
+++ cp/parser.c (working copy)
@@ -12258,12 +12258,21 @@ cp_parser_template_parameter (cp_parser* parser, b
   parameter_declarator->declarator->parameter_pack_p = false;
 }
 
+  if (parameter_declarator
+  && parameter_declarator->default_argument)
+{
+  /* Can happen in some cases of erroneous input (c++/34892).  */
+  if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
+   /* Consume the `...' for better error recovery.  */
+   cp_lexer_consume_token (parser->lexer);
+}
   /* If the next token is an ellipsis, and we don't already have it
  marked as a parameter pack, then we have a parameter pack (that
  has no declarator).  */
-  if (!*is_parameter_pack
-  && cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS)
-  && declarator_can_be_parameter_pack (parameter_declarator->declarator))
+  else if (!*is_parameter_pack
+  && cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS)
+  && (declarator_can_be_parameter_pack
+  (parameter_declarator->declarator)))
 {
   /* Consume the `...'.  */
   cp_lexer_consume_token (parser->lexer);
Index: testsuite/g++.dg/template/crash114.C
===
--- testsuite/g++.dg/template/crash114.C(revision 0)
+++ testsuite/g++.dg/template/crash114.C(working copy)
@@ -0,0 +1,5 @@
+// PR c++/34892
+
+template struct A {};  // { dg-error "expected primary" }
+
+A<0> a;

Re: [PATCH] GCC 4.7 and 4.8 PowerPC RTEMS

[Ralf Corsepius]

On 10/25/2012 01:28 AM, David Edelsohn wrote:

Joel and Ralf,

Would you please comment on this patch?

http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02154.html

Thanks, David
This patch had been part of the rtems gcc-patches for months and I 
haven't received any complaint about it.


So, this patch is OK with me.

Ralf

patch to fix PR55067

[Vladimir Makarov]

The following patch is to fix

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

Committed as rev. 192794.

2012-10-24  Vladimir Makarov  

PR bootstrap/55067
* lra.c: Rename loc to sloc and loc_t to sloc_t.


Index: lra.c
===
--- lra.c   (revision 192742)
+++ lra.c   (working copy)
@@ -1859,19 +1859,19 @@ lra_process_new_insns (rtx insn, rtx bef
scratches at the end of LRA. */
 
 /* Description of location of a former scratch operand. */
-struct loc
+struct sloc
 {
   rtx insn; /* Insn where the scratch was.  */
   int nop;  /* Number of the operand which was a scratch.  */
 };
 
-typedef struct loc *loc_t;
+typedef struct sloc *sloc_t;
 
-DEF_VEC_P(loc_t);
-DEF_VEC_ALLOC_P(loc_t, heap);
+DEF_VEC_P(sloc_t);
+DEF_VEC_ALLOC_P(sloc_t, heap);
 
 /* Locations of the former scratches.  */
-static VEC (loc_t, heap) *scratches;
+static VEC (sloc_t, heap) *scratches;
 
 /* Bitmap of scratch regnos.  */
 static bitmap_head scratch_bitmap;
@@ -1902,11 +1902,11 @@ remove_scratches (void)
   bool insn_changed_p;
   basic_block bb;
   rtx insn, reg;
-  loc_t loc;
+  sloc_t loc;
   lra_insn_recog_data_t id;
   struct lra_static_insn_data *static_id;
 
-  scratches = VEC_alloc (loc_t, heap, get_max_uid ());
+  scratches = VEC_alloc (sloc_t, heap, get_max_uid ());
   bitmap_initialize (&scratch_bitmap, ®_obstack);
   bitmap_initialize (&scratch_operand_bitmap, ®_obstack);
   FOR_EACH_BB (bb)
@@ -1926,10 +1926,10 @@ remove_scratches (void)
  *id->operand_loc[i], ALL_REGS, NULL);
  add_reg_note (insn, REG_UNUSED, reg);
  lra_update_dup (id, i);
- loc = XNEW (struct loc);
+ loc = XNEW (struct sloc);
  loc->insn = insn;
  loc->nop = i;
- VEC_safe_push (loc_t, heap, scratches, loc);
+ VEC_safe_push (sloc_t, heap, scratches, loc);
  bitmap_set_bit (&scratch_bitmap, REGNO (*id->operand_loc[i]));
  bitmap_set_bit (&scratch_operand_bitmap,
  INSN_UID (insn) * MAX_RECOG_OPERANDS + i);
@@ -1950,11 +1950,11 @@ static void
 restore_scratches (void)
 {
   int i, regno;
-  loc_t loc;
+  sloc_t loc;
   rtx last = NULL_RTX;
   lra_insn_recog_data_t id = NULL;
 
-  for (i = 0; VEC_iterate (loc_t, scratches, i, loc); i++)
+  for (i = 0; VEC_iterate (sloc_t, scratches, i, loc); i++)
 {
   if (last != loc->insn)
{
@@ -1977,9 +1977,9 @@ restore_scratches (void)
 INSN_UID (loc->insn), loc->nop);
}
 }
-  for (i = 0; VEC_iterate (loc_t, scratches, i, loc); i++)
+  for (i = 0; VEC_iterate (sloc_t, scratches, i, loc); i++)
 free (loc);
-  VEC_free (loc_t, heap, scratches);
+  VEC_free (sloc_t, heap, scratches);
   bitmap_clear (&scratch_bitmap);
   bitmap_clear (&scratch_operand_bitmap);
 }

Re: [C++ Patch] PR 34892

[Jason Merrill]

We can stop it even sooner:


  /* If the next token is an ellipsis, and we don't already have it
 marked as a parameter pack, then we have a parameter pack (that
 has no declarator).  */
  if (!*is_parameter_pack
  && cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS)
  && declarator_can_be_parameter_pack (parameter_declarator->declarator))


We shouldn't even consider this if there was a default argument.

Jason

Re: [C++] Handle ?: for vectors

[Jason Merrill]

On 10/24/2012 04:15 PM, Marc Glisse wrote:

I don't understand how < 0 differs from != 0 for that purpose. In both
cases, the front-end will produce the extra comparison, and the
middle-end will need an optimization to detect "truth values" (vectors
of -1 and 0) and simplify trivial operations on those (<0, !=0, etc).


True; if we aren't going to assume that the operand is already a 
truthvalue vector, it would be the same.  So let's go with != 0 after all.



(at a point, based on Richard's words but apparently not on his
suggestion, I was tempted to introduce a new boolean vector type, whose
only values are 0 and -1, but that seems a bit complicated and doesn't
answer whether x?y:z should mean x<0 or x!=0)


That would make sense to me; it would avoid the extra comparison if the 
condition is a boolean vector.


Jason

Re: [C++ Patch] PR 34892

[Paolo Carlini]

Hi again,

On 10/24/2012 09:53 PM, Jason Merrill wrote:
The parm shouldn't be a parameter pack in that case; the ... is part 
of the default argument, not the parameter.
I tracked down where this goes wrong, in the parser: the below 
recognizes when cp_parser_parameter_declaration doesn't parse a valid 
default argument and in that case "*is_parameter_pack = true;" doesn't 
happen. This means we have a very good diagnostics, only a single error 
message and nothing else, not even the maybe_warn_variadic_templates 
message in C++98 mode because I don't think the code can make any sense 
in any C++ dialect whatsoever (the latter would be easy to change of course)


Tested x86_64-linux, as usual.

Thanks,
Paolo.

PS: interestingly, detecting error_mark_node toward the end of 
cp_parser_parameter_declaration and turning it into a NULL_TREE leads to 
a single regression, at the end of pr39060.C, because NULL_TREE implies 
that we don't do anymore the diagnostics about default argument part of 
duplicate_decls (which ICC also gives). Besides that, for 34892 we give 
like 6 different error messages ;)



/cp
2012-10-24  Paolo Carlini  

PR c++/34892
* parser.c (cp_parser_template_parameter): Notice when parsing the
default argument goes wrong in cp_parser_parameter_declaration and
don't set is_parameter_pack in that case.

/testsuite
2012-10-24  Paolo Carlini  

PR c++/34892
* g++.dg/template/crash114.C: New.
Index: testsuite/g++.dg/template/crash114.C
===
--- testsuite/g++.dg/template/crash114.C(revision 0)
+++ testsuite/g++.dg/template/crash114.C(working copy)
@@ -0,0 +1,5 @@
+// PR c++/34892
+
+template struct A {};  // { dg-error "expected primary" }
+
+A<0> a;
Index: cp/parser.c
===
--- cp/parser.c (revision 192786)
+++ cp/parser.c (working copy)
@@ -12267,9 +12267,18 @@ cp_parser_template_parameter (cp_parser* parser, b
 {
   /* Consume the `...'.  */
   cp_lexer_consume_token (parser->lexer);
-  maybe_warn_variadic_templates ();
-  
-  *is_parameter_pack = true;
+
+  /* For an erroneous input like (c++/34892):
+
+  template struct A {};
+
+we gave an error in cp_parser_parameter_declaration and returned a
+parameter_declarator with error_mark_node as default_argument.  */
+  if (parameter_declarator->default_argument != error_mark_node)
+   {
+ maybe_warn_variadic_templates ();
+ *is_parameter_pack = true;
+   }
 }
   /* We might end up with a pack expansion as the type of the non-type
  template parameter, in which case this is a non-type template

Re: Make try_unroll_loop_completely to use loop bounds recorded

[Jan Hubicka]

> > * tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Add 
> > edge_to_cancel
> > parameter and use it to estimate code optimized out in the final 
> > iteration.
> > (loop_edge_to_cancel): New function.
> > (try_unroll_loop_completely): New IRRED_IVALIDATED parameter;
> > handle unrolling loops with bounds given via max_loop_iteratins;
> > handle unrolling non-inner loops when code size shrinks;
> > tidy dump output; when the last iteration loop still stays
> > as loop in the CFG forcongly redirect the latch to
> > __builtin_unreachable.
> > (canonicalize_loop_induction_variables): Add irred_invlaidated
> > parameter; record niter bound derrived; dump
> > max_loop_iterations bounds; call try_unroll_loop_completely
> > even if no niter bound is given.
> > (canonicalize_induction_variables): Handle irred_invalidated.
> > (tree_unroll_loops_completely): Handle non-innermost loops;
> > handle irred_invalidated.
> > * cfgloop.h (unlop): Declare.
> > * cfgloopmanip.c (unloop): Export.
> > * tree.c (build_common_builtin_nodes): Build BULTIN_UNREACHABLE.
> >
> 
> This caused:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55051
Oops, I did not really anticipated this to happen during profiledbootstrap, but 
I
discussed the problem in
http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01103.html
I suppose we will need to find porper fix for this long lasting issue more
urgently now :(

Honza
> 
> 
> -- 
> H.J.

Re: LRA has been merged into trunk.

[David Edelsohn]

And PR bootstrap/55068 due to assert failure in push_reload() .

Thanks, David

Re: Make try_unroll_loop_completely to use loop bounds recorded

[H.J. Lu]

On Tue, Oct 16, 2012 at 10:32 AM, Jan Hubicka  wrote:
> Hi,
> here is third revised version of the complette unroling changes.  While 
> working
> on the RTL variant I noticed PR54937 and the fact that I was overly aggressive
> on forcing single exit of the last iteration to be taken, because loop may 
> terminate
> otherwise (by EH or by exitting the program).  Same thinko is in loop-niter.
>
> This patch adds loop_edge_to_cancel that is more conservative: it looks for 
> the
> exit conditional where the non-exitting edges leads to latch and verifies that
> latch contains no statement with side effect that may terminate the loop.
> This still actually matches quite few non-single-exit loops and works well in
> practice.
>
> Unlike previous revision it also enables complette unrolling when code size
> does not grow even for non-innermost loops (with update in
> tree_unroll_loops_completely to walk them). This is something we did on RTL
> land but missed in trees.  This actually enables quite some optimizations when
> things can be propagated to the tiny inner loop body.
>
> I also fixed accounting in tree_estimate_loop_size for the cases where last
> iteration is not going to be updated.
>
> Finally I added code constructing __bulitin_unreachable as suggested by
> Ian.
>
> Bootstrapped/regtested x86_64-linux, also bootstrapped with -O3 and -Werror
> disabled and benchmarked. Among best benefits is about 7% improvement on 
> Applu,
> and it causes up to 15% improvements on vectorized loops with small iteration
> counts (by completelly peeling the precondition code).  There are no real
> performance regressions but there is some code size bloat.
>
> I plan to followup with strenghtening the heuristic to disable unrolling when
> benefits are absymal.  Easy is to limit unrolling on loops with CFG and/or
> calls in them.  We already have quite informed analysis in place.  I also plan
> to move simple FDO guided loop peeling from RTL level to trees to enable more
> propagation into peeled sequences.
>
> The patch also triggers bug in niter and requires xfailing do_1.f90 testcase.
> I filled PR 54932 to track this.
>
> There are also confused array bound warnings I hope to track incrementally, 
> too,
> by recording statements that are known to become unreachable in the last
> iteration and adding __buitin_unreachable in front of them. This is also
> important to avoid duplication leading to dead code: no other optimizers
> force paths leading to out of bound accesses to not happen.
>
> Honza
>
>
> * tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Add 
> edge_to_cancel
> parameter and use it to estimate code optimized out in the final 
> iteration.
> (loop_edge_to_cancel): New function.
> (try_unroll_loop_completely): New IRRED_IVALIDATED parameter;
> handle unrolling loops with bounds given via max_loop_iteratins;
> handle unrolling non-inner loops when code size shrinks;
> tidy dump output; when the last iteration loop still stays
> as loop in the CFG forcongly redirect the latch to
> __builtin_unreachable.
> (canonicalize_loop_induction_variables): Add irred_invlaidated
> parameter; record niter bound derrived; dump
> max_loop_iterations bounds; call try_unroll_loop_completely
> even if no niter bound is given.
> (canonicalize_induction_variables): Handle irred_invalidated.
> (tree_unroll_loops_completely): Handle non-innermost loops;
> handle irred_invalidated.
> * cfgloop.h (unlop): Declare.
> * cfgloopmanip.c (unloop): Export.
> * tree.c (build_common_builtin_nodes): Build BULTIN_UNREACHABLE.
>

This caused:

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


-- 
H.J.

Re: [PATCH] GCC 4.7 and 4.8 PowerPC RTEMS

[David Edelsohn]

Joel and Ralf,

Would you please comment on this patch?

http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02154.html

Thanks, David

Re: LRA has been merged into trunk.

[David Edelsohn]

This also causes PR bootstrap/55067 on AIX due to the use of typedef loc_t.

Thanks, David

[PATCH] obvious fix for rs6000 broken bootstrap committed

[Sharad Singhai]

As per discussion in http://gcc.gnu.org/ml/gcc/2012-10/msg00366.html.
I have applied the following obvious fix for rs6000 broken bootstrap.

2012-10-24  Sharad Singhai  

* config/rs6000/rs6000.c (rs6000_density_test): Use dump_enabled_p
  instead of dump_kind_p.

Index: config/rs6000/rs6000.c
===
--- config/rs6000/rs6000.c (revision 192787)
+++ config/rs6000/rs6000.c (working copy)
@@ -3547,7 +3547,7 @@ rs6000_density_test (rs6000_cost_data *data)
   && vec_cost + not_vec_cost > DENSITY_SIZE_THRESHOLD)
 {
   data->cost[vect_body] = vec_cost * (100 + DENSITY_PENALTY) / 100;
-  if (dump_kind_p (MSG_NOTE))
+  if (dump_enabled_p ())
  dump_printf_loc (MSG_NOTE, vect_location,
  "density %d%%, cost %d exceeds threshold, penalizing "
  "loop body cost by %d%%", density_pct,


Thanks,.
Sharad

Re: libgo patch committed: Update to current Go library

[Andreas Schwab]

Uros Bizjak  writes:

> Yes, I am running under gdb and all FDs are offset by +4 for some
> reason. So, FD 8 corresponds to FD4 in the strace log.

This is normal, gdb is leaking some fds to the inferior (which is a
bug).

Andreas.

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

RE: [Ping]FW: [PATCH] Cilk Plus merging to trunk (2 of n)

[Joseph S. Myers]

On Wed, 24 Oct 2012, Iyer, Balaji V wrote:

> > Where in the patch you use int for the size of something (e.g. a list) on 
> > the host,
> > please use size_t.
> 
> Can you please give me an example where I am violating this rule? Here 
> is a link to the last submitted patch in case you need it 
> (http://gcc.gnu.org/ml/gcc-patches/2012-10/msg00431.html).

For example, see build_array_notation_expr.  The variables ii, jj, 
rhs_list_size, lhs_list_size should be size_t.  I see no reason in 
principle those lists should not have 2^31 or more elements, on a 64-bit 
host.  Unless there is some reason why, whatever the host and target, the 
values of some variable in the compiler could never need to exceed 2^31, 
int is probably not the right type for that variable.

> Also you mention "host" and "target." What do you exactly mean by that? 
> I generally use those terms in like a cross-compiler setting (i.e. host 
> = the machine on which you are compiling and target means the target 
> architecture you are compiling for).

That's what I mean.  You're measuring the size of an array on the host, so 
use size_t not int to measure that size.  If you were measuring something 
on the target, you might need HOST_WIDE_INT (for example, a compiler on a 
32-bit host can reasonably build objects for a 64-bit target that declare 
arrays with more than 2^31 elements, so size_t is not the right type to 
use in the compiler for the size of a target array).

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

Re: patch to fix pr55049

[H.J. Lu]

On Wed, Oct 24, 2012 at 10:37 AM, Vladimir Makarov  wrote:
>   The following path shouldfix
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55049
>
>   The patch was successfully bootstrapped on x86-64. Sorry, I can not
> bootstrap with mx32 because of libraries absence.

It passed libgcc.  Now I got

[hjl@gnu-tools-1 gcc]$ cat /tmp/x.c
enum gomp_schedule_type
{
  GFS_GUIDED,
  GFS_AUTO
};
struct gomp_work_share
{
  enum gomp_schedule_type sched;
};
struct gomp_team_state
{
  struct gomp_work_share *work_share;
};
struct gomp_thread
{
  void *data;
  struct gomp_team_state ts;
};
extern __thread struct gomp_thread gomp_tls_data;
_Bool
foo (void)
{
  return gomp_tls_data.ts.work_share->sched == GFS_GUIDED;
}
[hjl@gnu-tools-1 gcc]$ ./xgcc -B./ -mx32 -S -O2 /tmp/x.c
/tmp/x.c: In function ‘foo’:
/tmp/x.c:24:1: internal compiler error: in check_rtl, at lra.c:2014
 }
 ^
0x8bfdbd check_rtl
/export/gnu/import/git/gcc/gcc/lra.c:2014
0x8c071d lra(_IO_FILE*)
/export/gnu/import/git/gcc/gcc/lra.c:2372
0x875f89 do_reload
/export/gnu/import/git/gcc/gcc/ira.c:4613
0x876182 rest_of_handle_reload
/export/gnu/import/git/gcc/gcc/ira.c:4719
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
[hjl@gnu-tools-1 gcc]$


-- 
H.J.

[lra] merged with trunk

[Vladimir Makarov]

LRA branch was merged with trunk @ 192779.

Committed as rev. 192787.

Re: patch to fix pr55049

[H.J. Lu]

On Wed, Oct 24, 2012 at 10:37 AM, Vladimir Makarov  wrote:
>   The following path shouldfix
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55049
>
>   The patch was successfully bootstrapped on x86-64. Sorry, I can not
> bootstrap with mx32 because of libraries absence.
>
>   Committed as rev. 192771.
>
> 2012-10-24  Vladimir Makarov  
>
> PR bootstrap/55049
> * lra-constraints.c (extract_loc_address_regs): Pass top_p for
> ZERO_EXTEND operand.
>

I checked in this testcase.  But I still got

libtool: compile:
/export/build/gnu/gcc-x32/build-x86_64-linux/./gcc/xgcc
-B/export/build/gnu/gcc-x32/build-x86_64-linux/./gcc/
-B/usr/gcc-4.8.0-x32/x86_64-unknown-linux-gnu/bin/
-B/usr/gcc-4.8.0-x32/x86_64-unknown-linux-gnu/lib/ -isystem
/usr/gcc-4.8.0-x32/x86_64-unknown-linux-gnu/include -isystem
/usr/gcc-4.8.0-x32/x86_64-unknown-linux-gnu/sys-include -mx32
-DHAVE_CONFIG_H -I. -I/export/gnu/import/git/gcc/libgomp
-I/export/gnu/import/git/gcc/libgomp/config/linux/x86
-I/export/gnu/import/git/gcc/libgomp/config/linux
-I/export/gnu/import/git/gcc/libgomp/config/posix
-I/export/gnu/import/git/gcc/libgomp -Wall -Werror -pthread
-ftls-model=initial-exec -O2 -g -pthread -pthread -MT loop.lo -MD -MP
-MF .deps/loop.Tpo -c /export/gnu/import/git/gcc/libgomp/loop.c  -fPIC
-DPIC -o .libs/loop.o
/export/gnu/import/git/gcc/libgomp/loop.c: In function ‘GOMP_loop_runtime_next’:
/export/gnu/import/git/gcc/libgomp/loop.c:355:1: internal compiler
error: in check_rtl, at lra.c:2014
 }
 ^
0x8bfdbd check_rtl
/export/gnu/import/git/gcc/gcc/lra.c:2014
0x8c071d lra(_IO_FILE*)
/export/gnu/import/git/gcc/gcc/lra.c:2372
0x875f89 do_reload
/export/gnu/import/git/gcc/gcc/ira.c:4613
0x876182 rest_of_handle_reload
/export/gnu/import/git/gcc/gcc/ira.c:4719
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
make[2]: *** [loop.lo] Error 1
make[2]: Leaving directory
`/export/build/gnu/gcc-x32/build-x86_64-linux/x86_64-unknown-linux-gnu/x32/libgomp'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory
`/export/build/gnu/gcc-x32/build-x86_64-linux/x86_64-unknown-linux-gnu/x32/libgomp'
make: *** [all] Error 2

I will extract a small tectase.

-- 
H.J.

PR bootstrap/55049
* gcc.target/i386/pr55049-1.c: New test.

Index: gcc.target/i386/pr55049-1.c
===
--- gcc.target/i386/pr55049-1.c (revision 0)
+++ gcc.target/i386/pr55049-1.c (revision 192785)
@@ -0,0 +1,11 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-require-effective-target fpic } */
+/* { dg-options "-O2 -fPIC -mx32" } */
+
+extern void __morestack_fail (const char *msg);
+void
+foo (void)
+{
+  static const char msg[] = "munmap of stack space failed: errno ";
+  __morestack_fail (msg);
+}

libbacktrace patch committed: Check whether -funwind-tables works

[Ian Lance Taylor]

PR 55061 is about a case in which the compiler used to build stage 1
(Xcode 3.1.4 on PPC Darwin) is a version of GCC that does not correctly
support the -funwind-tables option.  The libbacktrace configure script
was assuming that all versions of GCC support -funwind-tables.  This
patch changes it to actually test whether it works, using a test case
that fails with Xcode 3.1.4.  Bootstrapped and ran libbacktrace tests on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian


2012-10-24  Ian Lance Taylor  

PR target/55061
* configure.ac: Check whether -funwind-tables option works.
* configure: Rebuild.


Index: configure.ac
===
--- configure.ac	(revision 192781)
+++ configure.ac	(working copy)
@@ -97,8 +97,21 @@ fi
 AC_SUBST(BACKTRACE_FILE)
 
 EXTRA_FLAGS=
-if test "x$GCC" = "xyes"; then
+if test -n "${with_target_subdir}"; then
   EXTRA_FLAGS=-funwind-tables
+else
+  AC_CACHE_CHECK([for -funwind-tables option],
+[libbacktrace_cv_c_unwind_tables],
+[CFLAGS_hold="$CFLAGS"
+ CFLAGS="$CFLAGS -funwind-tables"
+ AC_COMPILE_IFELSE(
+   [AC_LANG_PROGRAM([static int f() { return 0; }], [return f();])],
+   [libbacktrace_cv_c_unwind_tables=yes],
+   [libbacktrace_cv_c_unwind_tables=no])
+ CFLAGS="$CFLAGS_hold"])
+   if test "$libbacktrace_cv_c_unwind_tables" = "yes"; then
+ EXTRA_FLAGS=-funwind-tables
+   fi
 fi
 AC_SUBST(EXTRA_FLAGS)
 

Re: [v3] Another try at LWG 2141

[Daniel Krügler]

2012/10/24 Marc Glisse :
> On Wed, 24 Oct 2012, Paolo Carlini wrote:
>
>> let's try again ;) In the light of discussion in Portland, which liked
>> Marc's idea of using std::decay in the unary common_type too, the below
>> seems good to go now, given that there are bad interactions with the
>> front-end bug we have got.
>
>
>template
>  struct common_type<_Tp>
> -{ typedef _Tp type; };
> +{ typedef typename decay<_Tp>::type type; };
>
> (talking to myself)
> I guess decay never has sfinae-type errors, so it doesn't matter whether we
> have this typedef or derive from decay.

Depending on the final resolution of

http://cplusplus.github.com/LWG/lwg-active.html#2101

(the worst IMO) std::decay could produce an invalid instantiation, a
less aggressive
variant would make this sfinae-friendly (not type), but hopefully it
will always be well-formed.
I'm awaiting the further development of that issue.

- Daniel

Re: libgo patch committed: Update to current Go library

[Ian Lance Taylor]

On Wed, Oct 24, 2012 at 11:03 AM, Ian Lance Taylor  wrote:
>
> OK, so it's a garbage collector problem.  Can you e-mail me the test
> binary offlist?  I will try to figure out where readFile lives.  The
> fact that I'm not seeing any GC problems on x86 or x86_64 suggests
> that this is something Alpha-specific.  Or, it could be something
> specific to the non-split-stack code.  What's weird is that it's
> unlikely to be a major error, since most of your tests pass.

Thanks for sending the binaries.  Unfortunately I don't see any
problems.  All the code looks reasonable, the registers should be on
the stack, and the garbage collector should see the stack.  I don't
see any way to make progress without doing some actual debugging.

Ian

Add myself to MAINTAINERS

[Sharad Singhai]

Added myself as write after approval maintainer in r192781.

Thanks,
Sharad

Index: ChangeLog
===
--- ChangeLog (revision 192779)
+++ ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2012-10-24  Sharad Singhai  
+
+ * MAINTAINERS (Write After Approval): Add myself.
+
 2012-10-24  Eric Christopher  

  * MAINTAINERS: Update email address.
Index: MAINTAINERS
===
--- MAINTAINERS (revision 192779)
+++ MAINTAINERS (working copy)
@@ -502,6 +502,7 @@ Dodji Seketeli do...@gcc.gnu.org
 Svein Seldal sv...@dev.seldal.com
 Thiemo Seufer t...@networkno.de
 Marcus Shawcroft marcus.shawcr...@arm.com
+Sharad Singhai sing...@google.com
 Johannes Singler sing...@kit.edu
 Franz Sirl franz.sirl-ker...@lauterbach.com
 Jan Sjodin jan.sjo...@amd.com

Re: [Patch] Fix the tests gcc.dg/vect/vect-8[23]_64.c

[Dominique Dhumieres]

Mike, Sharad,

> Committed in r192750.

Thanks for the review and the commit.

Dominique

Re: [v3] Another try at LWG 2141

[Marc Glisse]

On Wed, 24 Oct 2012, Paolo Carlini wrote:

let's try again ;) In the light of discussion in Portland, which liked Marc's 
idea of using std::decay in the unary common_type too, the below seems good 
to go now, given that there are bad interactions with the front-end bug we 
have got.


   template
 struct common_type<_Tp>
-{ typedef _Tp type; };
+{ typedef typename decay<_Tp>::type type; };

(talking to myself)
I guess decay never has sfinae-type errors, so it doesn't matter whether 
we have this typedef or derive from decay.


--
Marc Glisse

Re: [C++ Patch] PR 53761

[Paolo Carlini]

.. Oh well, and the details of this are even subtler, because, assuming 
we want the exact same behavior of the C front-end, we are going to accept:


typedef union {
  int* f;
  int y;
} __attribute__(( __transparent_union__ )) example_t;

and reject:

typedef union {
  int f;
  int* y;
} __attribute__(( __transparent_union__ )) example_t;

thus we can't even talk about integer and pointer types generically... 
Essentially, what really matters is that the first field must be a pointer.


Paolo.

Re: [C++] Handle ?: for vectors

[Marc Glisse]

On Wed, 24 Oct 2012, Mike Stump wrote:

Well, I suspect the OpenCL community had a ton of people sweat over the 
details and take into consideration the realities and the needs of 
people.  I'd like to believe they had more people in on this and that 
this was a compromise for someones vector unit.


Intel's, apparently...

The problem is, what if your vector unit produces 0, 1, to be compatible 
with C?  Suddenly, spilling this onto ? is annoying, both because it 
doesn't match hardware, nor the expected semantics of a person that just 
knows C.  Maybe we run a poll of vector units that prefer -1 or prefer 1 
for true, and then decide.  SSE has CMPPS, which likes the -1. 
Altivec's vec_cmpgt says true is all bits 1.  Gosh, I guess we can stop 
there.  Neon, for fun VCGE is defined to set to all ones for true. 
OpenCL it is.


We already decided that comparisons return -1, most processors agree on 
that except sparc, which doesn't return a vector at all. The question is 
about the selection instructions.


{-2,-1,0,1} ? {x,x,x,x} : {y,y,y,y}

OpenCL says this should be {x,x,y,y}. We are considering making it 
{x,x,y,x} instead. Hardware selection instructions vary a lot. OpenCL 
follows x86, what we are considering matches Power IIRC, and ARM only has 
a bitwise selection (I only quickly glanced at all of these, I may have 
read them wrong).



I am fine with both alternatives, but the choice is important...

--
Marc Glisse

[v3] Another try at LWG 2141

[Paolo Carlini]

Hi,

let's try again ;) In the light of discussion in Portland, which liked 
Marc's idea of using std::decay in the unary common_type too, the below 
seems good to go now, given that there are bad interactions with the 
front-end bug we have got.


Tested x86_64-linux.

Thanks,
Paolo.

///
2012-10-24  Daniel Krugler  

* include/std/type_traits (common_type): Implement LWG 2141.
* testsuite/20_util/duration/requirements/sfinae_friendly_1.cc:
Update.
* testsuite/20_util/common_type/requirements/typedefs-1.cc: Likewise.
* testsuite/20_util/common_type/requirements/sfinae_friendly_1.cc:
Likewise.
* testsuite/20_util/common_type/requirements/sfinae_friendly_2.cc:
Likewise.
* testsuite/20_util/common_type/requirements/typedefs-2.cc: Likewise.
Index: include/std/type_traits
===
--- include/std/type_traits (revision 192762)
+++ include/std/type_traits (working copy)
@@ -1792,9 +1792,9 @@
   struct __do_common_type_impl
   {
 template
-  static __success_type()
-: std::declval<_Up>())> _S_test(int);
+: std::declval<_Up>())>::type> _S_test(int);
 
 template
   static __failure_type _S_test(...);
@@ -1835,7 +1835,7 @@
 
   template
 struct common_type<_Tp>
-{ typedef _Tp type; };
+{ typedef typename decay<_Tp>::type type; };
 
   template
 struct common_type<_Tp, _Up>
Index: testsuite/20_util/duration/requirements/sfinae_friendly_1.cc
===
--- testsuite/20_util/duration/requirements/sfinae_friendly_1.cc
(revision 192762)
+++ testsuite/20_util/duration/requirements/sfinae_friendly_1.cc
(working copy)
@@ -21,9 +21,6 @@
 #include 
 #include 
 
-//TODO: Uncomment this once gcc bug 53000 has been resolved:
-//#define HAS_53000_FIXED
-
 // Helper types:
 struct has_type_impl
 {
@@ -55,10 +52,8 @@
 typedef std::chrono::duration ddn;
 typedef std::chrono::duration dim;
 
-#ifdef HAS_53000_FIXED
-static_assert(is_type, din&&>(), "");
-static_assert(is_type, din&&>(), "");
-#endif
+static_assert(is_type, din>(), "");
+static_assert(is_type, din>(), "");
 
 static_assert(is_type, ddn>(), "");
 static_assert(is_type, ddn>(), "");
Index: testsuite/20_util/common_type/requirements/typedefs-1.cc
===
--- testsuite/20_util/common_type/requirements/typedefs-1.cc(revision 
192762)
+++ testsuite/20_util/common_type/requirements/typedefs-1.cc(working copy)
@@ -105,7 +105,7 @@
   COMMON_TYPE_TEST_ALL_2(int, int, int, 1);
   COMMON_TYPE_TEST_ALL_2(int, double, double, 2);
   COMMON_TYPE_TEST_2(NO_CV, A, A, A, 3);
-  COMMON_TYPE_TEST_2(const, A, A, const A, 4);
+  COMMON_TYPE_TEST_2(const, A, A, A, 4);
   COMMON_TYPE_TEST_2(NO_CV, B, A, A, 5);  
 }
 
Index: testsuite/20_util/common_type/requirements/sfinae_friendly_1.cc
===
--- testsuite/20_util/common_type/requirements/sfinae_friendly_1.cc 
(revision 192762)
+++ testsuite/20_util/common_type/requirements/sfinae_friendly_1.cc 
(working copy)
@@ -163,23 +163,19 @@
   };
 }
 
-#ifdef HAS_53000_FIXED
-static_assert(is_type, int&&>(), "");
-static_assert(is_type, ScEn&&>(), "");
-static_assert(is_type, UnscEn&&>(), "");
-#endif
+static_assert(is_type, int>(), "");
+static_assert(is_type, ScEn>(), "");
+static_assert(is_type, UnscEn>(), "");
 static_assert(is_type, int>(), "");
-#ifdef HAS_53000_FIXED
-static_assert(is_type, int&&>(), "");
-static_assert(is_type, int&&>(), "");
-static_assert(is_type, int&&>(), "");
-static_assert(is_type, S&&>(), "");
-static_assert(is_type, const S&&>(), "");
+static_assert(is_type, int>(), "");
+static_assert(is_type, int>(), "");
+static_assert(is_type, int>(), "");
+static_assert(is_type, S>(), "");
+static_assert(is_type, S>(), "");
 static_assert(is_type,
- std::initializer_list>, std::initializer_list&&>(), "");
-static_assert(is_type, B&&>(), "");
-static_assert(is_type, B&&>(), "");
-#endif
+ std::initializer_list>, std::initializer_list>(), "");
+static_assert(is_type, B>(), "");
+static_assert(is_type, B>(), "");
 static_assert(is_type, void*>(), "");
 static_assert(is_type, void*>(), "");
 static_assert(is_type, const volatile void*>(), "");
@@ -191,16 +187,12 @@
 static_assert(is_type, void>(), "");
 static_assert(is_type, void>(), "");
 static_assert(is_type, int>(), "");
-static_assert(is_type, int&>(), "");
-#ifdef HAS_53000_FIXED
-static_assert(is_type, int&&>(), "");
-static_assert(is_type, const int&&>(), 
"");
-#endif
-static_assert(is_type, const U>(), "");
-static_assert(is_type, U&>(), "");
-#ifdef HAS_53000_FIXED
-static_assert(is_type, U&&>(), "");
-#endif
+static_assert(is_type, int>(), "");
+static_assert(is_type, int>(), "");
+static_assert(i

Re: [C++] Handle ?: for vectors

[Mike Stump]

On Oct 24, 2012, at 12:38 PM, Jakub Jelinek  wrote:
> On Wed, Oct 24, 2012 at 08:36:19PM +0200, Marc Glisse wrote:
>> On Wed, 24 Oct 2012, Jason Merrill wrote:
>> 
>>> On 10/19/2012 04:40 PM, Marc Glisse wrote:
 So between the following:
 a) x?y:z means (x<0)?y:z
 b) x?y:z means (x!=0)?y:z
 c) x?y:z is rejected, only things like (x==t)?y:z are accepted
 d) other
 
 is the choice still b) ? That's an easy one, only 2 characters to change
 in the patch (assuming the rest is correct) :-)
>>> 
>>> That would be my inclination, but I'm not a vector programming expert,
>> 
>> I'm not an expert at all either, and I find the OpenCL semantics
>> quite confusing, where truth is implicitly x<0 for x?y:z but x!=0
>> for x&&y or !x, so in particular (x&&y)?z:t is not equivalent to
>> x?y?z:t:t, and I am quite happy with your suggestion to deviate. I
>> was surprised because I hadn't considered the possibility, but now I
>> actually like it better than OpenCL ;-)
>> 
>>> so I guess let's go with the OpenCL semantics.
>> 
>> Ok. I take it as: let's wait a few days in case people want to
>> comment, then you'll review the original patch?
> 
> I'd prefer b) as well, but I'm not a vector programming expert either.

Well, I suspect the OpenCL community had a ton of people sweat over the details 
and take into consideration the realities and the needs of people.  I'd like to 
believe they had more people in on this and that this was a compromise for 
someones vector unit.  I like b myself, however, following OpenCL I think might 
be better.  Tough choice.  The problem is, what if your vector unit produces 0, 
1, to be compatible with C?  Suddenly, spilling this onto ? is annoying, both 
because it doesn't match hardware, nor the expected semantics of a person that 
just knows C.  Maybe we run a poll of vector units that prefer -1 or prefer 1 
for true, and then decide.  SSE has CMPPS, which likes the -1.  Altivec's 
vec_cmpgt says true is all bits 1.  Gosh, I guess we can stop there.  Neon, for 
fun VCGE is defined to set to all ones for true.  OpenCL it is.

Re: [C++ Patch] PR 34892

[Paolo Carlini]

On 10/24/2012 09:53 PM, Jason Merrill wrote:

On 10/24/2012 02:11 PM, Paolo Carlini wrote:

The problem is that the first time we go through the loop, when parm_idx
== 0 and TREE_PURPOSE is error_mark_node, the condition:

   if (template_parameter_pack_p (TREE_VALUE (parm))
   && !(arg && ARGUMENT_PACK_P (arg)))

near the beginning of the loop is true


The parm shouldn't be a parameter pack in that case; the ... is part 
of the default argument, not the parameter.
Yes, I see that. Then it seems to me that we have to catch the problem 
(much) earlier...


Thanks,
Paolo.

Re: [C++ Patch] PR 53761

[Paolo Carlini]

Hi,

On 10/24/2012 09:55 PM, Jason Merrill wrote:

On 10/24/2012 03:48 PM, Paolo Carlini wrote:
+  error ("type transparent %q#T cannot be made transparent 
because "

+ "a field has neither pointer nor integer type", t);


I'd say "%q#T cannot be made transparent because %q#D has neither 
pointer nor integer type", t, field.
Unfortunately, I don't think we can name the specific field/member like 
this because it can be misleading in some cases (I had already tried ;) 
Consider:


typedef union {
  int f;
  double x;
} __attribute__(( __transparent_union__ )) example_t;

we end up saying:

‘union’ cannot be made transparent because ‘int union>::f’ has neither pointer nor integer type


whereas the problem is actually x! Shall we content ourselves of 
something not naming the field? We are already doing better than the C 
front-end ;)


Paolo.

Re: [C++] Handle ?: for vectors

[Marc Glisse]

On Wed, 24 Oct 2012, Jason Merrill wrote:

My guess for the reason why OpenCL has the semantics it does is that if you 
stored a boolean result in a variable earlier and then use it as the ? 
condition, that would require an extra comparison whereas if it's already a 
vector of 0 and -1 as expected it can be used directly.


I don't understand how < 0 differs from != 0 for that purpose. In both 
cases, the front-end will produce the extra comparison, and the middle-end 
will need an optimization to detect "truth values" (vectors of -1 and 0) 
and simplify trivial operations on those (<0, !=0, etc).


(at a point, based on Richard's words but apparently not on his 
suggestion, I was tempted to introduce a new boolean vector type, whose 
only values are 0 and -1, but that seems a bit complicated and doesn't 
answer whether x?y:z should mean x<0 or x!=0)


--
Marc Glisse

patch to fix PR55055

[Vladimir Makarov]

  The following patch fix

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

  In this case, operand was an address containing subreg. LRA before 
the patch processed only operands which are subregs of regs.


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

  Committed as rev. 192779.

2012-10-24  Vladimir Makarov  

PR rtl-optimization/55055
* lra-spills.c (alter_subregs): New function.
(lra_hard_reg_substitution): Use it.

Index: lra-spills.c
===
--- lra-spills.c(revision 192742)
+++ lra-spills.c(working copy)
@@ -571,6 +571,48 @@ lra_spill (void)
   free (pseudo_regnos);
 }
 
+/* Apply alter_subreg for subregs of regs in *LOC.  Use FINAL_P for
+   alter_subreg calls. Return true if any subreg of reg is
+   processed.  */
+static bool
+alter_subregs (rtx *loc, bool final_p)
+{
+  int i;
+  rtx x = *loc;
+  bool res;
+  const char *fmt;
+  enum rtx_code code;
+
+  if (x == NULL_RTX)
+return false;
+  code = GET_CODE (x);
+  if (code == SUBREG && REG_P (SUBREG_REG (x)))
+{
+  lra_assert (REGNO (SUBREG_REG (x)) < FIRST_PSEUDO_REGISTER);
+  alter_subreg (loc, final_p);
+  return true;
+}
+  fmt = GET_RTX_FORMAT (code);
+  res = false;
+  for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
+{
+  if (fmt[i] == 'e')
+   {
+ if (alter_subregs (&XEXP (x, i), final_p))
+   res = true;
+   }
+  else if (fmt[i] == 'E')
+   {
+ int j;
+ 
+ for (j = XVECLEN (x, i) - 1; j >= 0; j--)
+   if (alter_subregs (&XVECEXP (x, i, j), final_p))
+ res = true;
+   }
+}
+  return res;
+}
+
 /* Final change of pseudos got hard registers into the corresponding
hard registers.  */
 void
@@ -589,22 +631,15 @@ lra_hard_reg_substitution (void)
 FOR_BB_INSNS (bb, insn)
   if (INSN_P (insn))
{
- lra_insn_recog_data_t id;
+ lra_insn_recog_data_t id = lra_get_insn_recog_data (insn);
  bool insn_change_p = false;
 
- id = lra_get_insn_recog_data (insn);
  for (i = id->insn_static_data->n_operands - 1; i >= 0; i--)
-   {
- rtx op = *id->operand_loc[i];
-
- if (GET_CODE (op) == SUBREG && REG_P (SUBREG_REG (op)))
-   {
- lra_assert (REGNO (SUBREG_REG (op)) < FIRST_PSEUDO_REGISTER);
- alter_subreg (id->operand_loc[i], ! DEBUG_INSN_P (insn));
- lra_update_dup (id, i);
- insn_change_p = true;
-   }
-   }
+   if (alter_subregs (id->operand_loc[i], ! DEBUG_INSN_P (insn)))
+ {
+   lra_update_dup (id, i);
+   insn_change_p = true;
+ }
  if (insn_change_p)
lra_update_operator_dups (id);
}

Re: [C++ Patch] PR 53761

[Jason Merrill]

On 10/24/2012 03:48 PM, Paolo Carlini wrote:

+ error ("type transparent %q#T cannot be made transparent because "
+"a field has neither pointer nor integer type", t);


I'd say "%q#T cannot be made transparent because %q#D has neither 
pointer nor integer type", t, field.


OK with that change.

Jason

Re: [C++ Patch] PR 34892

[Jason Merrill]

On 10/24/2012 02:11 PM, Paolo Carlini wrote:

The problem is that the first time we go through the loop, when parm_idx
== 0 and TREE_PURPOSE is error_mark_node, the condition:

   if (template_parameter_pack_p (TREE_VALUE (parm))
   && !(arg && ARGUMENT_PACK_P (arg)))

near the beginning of the loop is true


The parm shouldn't be a parameter pack in that case; the ... is part of 
the default argument, not the parameter.


Jason

Re: [C++] Handle ?: for vectors

[Jason Merrill]

My guess for the reason why OpenCL has the semantics it does is that if 
you stored a boolean result in a variable earlier and then use it as the 
? condition, that would require an extra comparison whereas if it's 
already a vector of 0 and -1 as expected it can be used directly.


Jason

Re: [PATCH][RFC] Fix PR54824, deal with BBs with no successor

[Steven Bosscher]

On Wed, Oct 24, 2012 at 4:25 PM, Richard Biener  wrote:
> Comments?  Should find_many_sub_basic_blocks really do what
> it does here?  Or is the CFG in fact "invalid" (but we don't
> check that in verification)?

If a "noreturn" function is inlined, doesn't the inliner detect that
the inlined body does in fact return?? The error is that the function
"bar()" returns, the documentation for noreturn makes it pretty clear
that such functions should never return:

In this case, bar() returns but the compiler has already used its
noreturn attribute to construct a dead end in the CFG (i.e. your basic
block without successors). But the fact that bar() returns is
undefined behavior. I'm not sure it is possible to know where the
function would continue after returning from bar(), after lowering to
GIMPLE.

IMHO find_many_sub_basic_blocks should not connect this block to the
rest of the CFG, so expanding a trap or a barrier is my preferred
solution, like so:

Index: cfgexpand.c
===
--- cfgexpand.c (revision 192696)
+++ cfgexpand.c (working copy)
@@ -3989,6 +3989,12 @@ expand_gimple_basic_block (basic_block b

   do_pending_stack_adjust ();

+  /* If this block has no successors at all, make sure that
+ find_many_sub_basic_blocks does not connect this "dead end"
+ to the rest of the function.  */
+  if (EDGE_COUNT (bb->succs) == 0)
+expand_builtin_unreachable ();
+
   /* Find the block tail.  The last insn in the block is the insn
  before a barrier and/or table jump insn.  */
   last = get_last_insn ();


At least this way the GIMPLE and RTL CFGs should be more alike than
with an infinite loop.

But, really, why inline noreturn functions at all?

Ciao!
Steven

Re: [C++ Patch] PR 53761

[Paolo Carlini]

Hi,

On 10/24/2012 06:57 PM, Jason Merrill wrote:

On 10/10/2012 11:13 AM, Paolo Carlini wrote:

-  error ("type transparent class %qT does not have any fields", t);
+  if (TREE_CODE (t) == UNION_TYPE)
+error ("type transparent union %qT does not have any 
fields", t);

+  else
+error ("type transparent class %qT does not have any 
fields", t);

If you use %q#T you don't need to repeat the class-key.

Indeed.


+error ("type transparent union %qT cannot be made 
transparent", t);


Let's say why not.

Thus I tested the below.

Thanks,
Paolo.

//
Index: cp/class.c
===
--- cp/class.c  (revision 192762)
+++ cp/class.c  (working copy)
@@ -6261,7 +6261,7 @@ finish_struct_1 (tree t)
   tree field = first_field (t);
   if (field == NULL_TREE || error_operand_p (field))
{
- error ("type transparent class %qT does not have any fields", t);
+ error ("type transparent %q#T does not have any fields", t);
  TYPE_TRANSPARENT_AGGR (t) = 0;
}
   else if (DECL_ARTIFICIAL (field))
@@ -6275,6 +6275,12 @@ finish_struct_1 (tree t)
}
  TYPE_TRANSPARENT_AGGR (t) = 0;
}
+  else if (TYPE_MODE (t) != DECL_MODE (field))
+   {
+ error ("type transparent %q#T cannot be made transparent because "
+"a field has neither pointer nor integer type", t);
+ TYPE_TRANSPARENT_AGGR (t) = 0;
+   }
 }
 }
 
Index: testsuite/g++.dg/ext/transparent-union.C
===
--- testsuite/g++.dg/ext/transparent-union.C(revision 0)
+++ testsuite/g++.dg/ext/transparent-union.C(working copy)
@@ -0,0 +1,5 @@
+// PR c++/53761
+
+typedef union {// { dg-error "type transparent" }
+   double x;
+} __attribute__(( __transparent_union__ )) example_t;

Re: [C++] Handle ?: for vectors

[Jakub Jelinek]

On Wed, Oct 24, 2012 at 08:36:19PM +0200, Marc Glisse wrote:
> On Wed, 24 Oct 2012, Jason Merrill wrote:
> 
> >On 10/19/2012 04:40 PM, Marc Glisse wrote:
> >>So between the following:
> >>a) x?y:z means (x<0)?y:z
> >>b) x?y:z means (x!=0)?y:z
> >>c) x?y:z is rejected, only things like (x==t)?y:z are accepted
> >>d) other
> >>
> >>is the choice still b) ? That's an easy one, only 2 characters to change
> >>in the patch (assuming the rest is correct) :-)
> >
> >That would be my inclination, but I'm not a vector programming expert,
> 
> I'm not an expert at all either, and I find the OpenCL semantics
> quite confusing, where truth is implicitly x<0 for x?y:z but x!=0
> for x&&y or !x, so in particular (x&&y)?z:t is not equivalent to
> x?y?z:t:t, and I am quite happy with your suggestion to deviate. I
> was surprised because I hadn't considered the possibility, but now I
> actually like it better than OpenCL ;-)
> 
> >so I guess let's go with the OpenCL semantics.
> 
> Ok. I take it as: let's wait a few days in case people want to
> comment, then you'll review the original patch?

I'd prefer b) as well, but I'm not a vector programming expert either.

Jakub

libgo patch committed: Define SIGPOLL and SIGCLD if necessary

[Ian Lance Taylor]

This patch to libgo arranges to define SIGPOLL and SIGCLD if necessary,
as SIGIO and SIGCHLD respectively.  This is necessary on GNU/Linux
because SIGPOLL is #define'd as SIGIO before SIGIO is #define'd, and gcc
-fdump-go-spec doesn't understand that.  Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu.  Committed to mainline and 4.7
branch.

Ian

diff -r 961c515492df libgo/mksysinfo.sh
--- a/libgo/mksysinfo.sh	Tue Oct 23 11:00:44 2012 -0700
+++ b/libgo/mksysinfo.sh	Wed Oct 24 11:57:46 2012 -0700
@@ -225,6 +225,16 @@
 grep '^const _SIG[^_]' gen-sysinfo.go | \
   grep -v '^const _SIGEV_' | \
   sed -e 's/^\(const \)_\(SIG[^= ]*\)\(.*\)$/\1\2 = Signal(_\2)/' >> ${OUT}
+if ! grep '^const SIGPOLL ' ${OUT} >/dev/null 2>&1; then
+  if grep '^const SIGIO ' ${OUT} > /dev/null 2>&1; then
+echo "const SIGPOLL = SIGIO" >> ${OUT}
+  fi
+fi
+if ! grep '^const SIGCLD ' ${OUT} >/dev/null 2>&1; then
+  if grep '^const SIGCHLD ' ${OUT} >/dev/null 2>&1; then
+echo "const SIGCLD = SIGCHLD" >> ${OUT}
+  fi
+fi
 
 # The syscall numbers.  We force the names to upper case.
 grep '^const _SYS_' gen-sysinfo.go | \

Re: PR tree-optimization/54985

[Jeff Law]

On 10/23/2012 03:43 PM, Jakub Jelinek wrote:

On Tue, Oct 23, 2012 at 03:34:46PM -0600, Jeff Law wrote:

I think it should be backported to 4.7, perhaps with a few days delay after the
trunk commit.

Do we even have debug statements after control flow statements?


They shouldn't be there, so if you just give up the same way for gsi_stmt
NULL as well as non-control stmt, it shouldn't make a difference.
So last_stmt might be just shorter to type and more commonly used, nothing
more.
From looking at the existing code and last_stmt; for the cases we care 
about, they ought to be equivalent.


Using last_stmt seems marginally clearer.  I'll go ahead and make that 
change after the usual bootstrap & test cycle.


jeff

Re: [C++] Handle ?: for vectors

[Marc Glisse]

On Wed, 24 Oct 2012, Jason Merrill wrote:


On 10/19/2012 04:40 PM, Marc Glisse wrote:

So between the following:
a) x?y:z means (x<0)?y:z
b) x?y:z means (x!=0)?y:z
c) x?y:z is rejected, only things like (x==t)?y:z are accepted
d) other

is the choice still b) ? That's an easy one, only 2 characters to change
in the patch (assuming the rest is correct) :-)


That would be my inclination, but I'm not a vector programming expert,


I'm not an expert at all either, and I find the OpenCL semantics quite 
confusing, where truth is implicitly x<0 for x?y:z but x!=0 for x&&y or 
!x, so in particular (x&&y)?z:t is not equivalent to x?y?z:t:t, and I am 
quite happy with your suggestion to deviate. I was surprised because I 
hadn't considered the possibility, but now I actually like it better than 
OpenCL ;-)



so I guess let's go with the OpenCL semantics.


Ok. I take it as: let's wait a few days in case people want to comment, 
then you'll review the original patch?


Thanks,

--
Marc Glisse

Re: [C++ Patch] PR 34892

[Paolo Carlini]

Hi,

On 10/24/2012 07:30 PM, Jason Merrill wrote:

On 10/24/2012 01:20 PM, Paolo Carlini wrote:

+  if (parm == error_mark_node
+  || TREE_PURPOSE (parm) == error_mark_node)


It seems odd to bail out early if the default argument is bad even if 
we aren't trying to use it.  Doesn't it work to check this further 
down where we actually look at the default argument?
The problem is that the first time we go through the loop, when parm_idx 
== 0 and TREE_PURPOSE is error_mark_node, the condition:


  if (template_parameter_pack_p (TREE_VALUE (parm))
  && !(arg && ARGUMENT_PACK_P (arg)))

near the beginning of the loop is true, thus we call 
coerce_template_parameter_pack, no error and we continue to the next 
iteration. The next iteration, arg == NULL_TREE and require_all_args is 
true, we actually use TREE_PURPOSE (parm) but for the next parm, when 
things are fine, then we get to the:


TREE_VEC_ELT (new_inner_args, arg_idx) = arg;

at the bottom of the loop and we are in big troubles because 
new_inner_args has only 1 element and arg_idx is 2.


Thus, in summary, it really seems to me that we should do *something* 
the *first* time through the loop, when currently we only use TREE_VALUE 
per the above and we don't notice that TREE_PURPOSE is error_mark_node. 
Did I explain myself well enough?


Thanks,
Paolo.

Re: PR c++/54928 infinite ICE when reporting ICE on macro expansion

[Jason Merrill]

On 10/24/2012 01:54 PM, Manuel López-Ibáñez wrote:

/* Add a note with text GMSGID and with LOCATION to the diagnostic CONTEXT.  */


Sure.


Actually, I don't know why I call it "attach". diagnostic_add_note()
or diagnostic_print_note() seems clearer. What do you think?


How about "append"?

Jason

Re: libgo patch committed: Update to current Go library

[Ian Lance Taylor]

On Wed, Oct 24, 2012 at 10:52 AM, Uros Bizjak  wrote:
> On Wed, Oct 24, 2012 at 7:46 PM, Ian Lance Taylor  wrote:
>> On Wed, Oct 24, 2012 at 9:34 AM, Uros Bizjak  wrote:
>>>
>>> Continuing.
>>> [New Thread 0x2000307b280 (LWP 8059)]
>>>
>>> Breakpoint 18, 0x020002e378c0 in socketpair () from /lib/libc.so.6.1
>>>
>>> Continuing.
>>> [New Thread 0x20003083280 (LWP 8065)]
>>> [Switching to Thread 0x20003083280 (LWP 8065)]
>>>
>>> [...]
>>>
>>> The first call with relevant FD is from:
>>>
>>> Breakpoint 21, 0x020002e243f8 in close () from /lib/libc.so.6.1
>>> (gdb) i r a0
>>> a0 0x8  8
>>
>> Does this mean that this is a call to close file descriptor 8?
>> According to the strace log, the file descriptor we care about is 4.
>> Although it is also true that I don't see a close of file descriptor 8
>> at all in the strace log.  Or is the change from 4 to 8 due somehow to
>> running the program under gdb?
>
> Yes, I am running under gdb and all FDs are offset by +4 for some
> reason. So, FD 8 corresponds to FD4 in the strace log.
>>
>>> (gdb) bt
>>> #0  0x020002e243f8 in close () from /lib/libc.so.6.1
>>> #1  0x00012003559c in syscall.Close (fd=) at 
>>> libcalls.go:271
>>> #2  0x025d3cfc in os.close.pN7_os.file (file=0xf840414b70) at
>>> ../../../gcc-svn/trunk/libgo/go/os/file_unix.go:106
>>> #3  0x02888f18 in ffi_call_osf () at
>>> ../../../gcc-svn/trunk/libffi/src/alpha/osf.S:79
>>> #4  0x028889c4 in ffi_call (cif=, fn=>> out>, rvalue=, avalue=0xf840c87fe8)
>>> at ../../../gcc-svn/trunk/libffi/src/alpha/ffi.c:169
>>> #5  0x02558204 in reflect.call (func_type=0x29e9650
>>> <__go_td_FppN7_os.fileerN5_erroree>,
>>> func_addr=0x25d3c60 ,
>>> is_interface=, is_method=,
>>> params=0xf840c87fe0, results=0x0) at
>>> ../../../gcc-svn/trunk/libgo/runtime/go-reflect-call.c:498
>>> #6  0x025620b8 in runfinq (dummy=) at
>>> ../../../gcc-svn/trunk/libgo/runtime/mgc0.c:1168
>>> #7  0x02566b20 in kickoff () at
>>> ../../../gcc-svn/trunk/libgo/runtime/proc.c:338
>>> #8  0x020002d8d024 in ?? () from /lib/libc.so.6.1
>>
>> If this is indeed the file descriptor we care about, then this is
>> interesting, because it is being closed by a finalizer run by the
>> garbage collector.  That implies that the garbage collector collected
>> the local variable readFile in TestPassFD in passfd_test.go, which
>> would be clearly wrong.  Unfortunately this could be rather difficult
>> to debug.
>
> Yes, this is correct descriptor. For added fun, a descriptor that
> corresponds to writeFile closes through the same mechanism.

OK, so it's a garbage collector problem.  Can you e-mail me the test
binary offlist?  I will try to figure out where readFile lives.  The
fact that I'm not seeing any GC problems on x86 or x86_64 suggests
that this is something Alpha-specific.  Or, it could be something
specific to the non-split-stack code.  What's weird is that it's
unlikely to be a major error, since most of your tests pass.

Ian

Re: [C++] Handle ?: for vectors

[Jason Merrill]

On 10/19/2012 04:40 PM, Marc Glisse wrote:

So between the following:
a) x?y:z means (x<0)?y:z
b) x?y:z means (x!=0)?y:z
c) x?y:z is rejected, only things like (x==t)?y:z are accepted
d) other

is the choice still b) ? That's an easy one, only 2 characters to change
in the patch (assuming the rest is correct) :-)


That would be my inclination, but I'm not a vector programming expert, 
so I guess let's go with the OpenCL semantics.


Jason

[committed] Unbreak powerpc bootstrap

[Richard Sandiford]

In the process of factoring out the "lowpart bit field" check
from an earlier patch, I somehow managed to drop an "== 0" condition.
It seems I then compounded that by screwing up the powerpc64 testing
(still not sure how :-().

Anyway, fixed with the patch below, tested on powerpc64-linux-gnu.
Sorry for the breakage, and thanks to Segher for the heads-up.

Richard


gcc/
* expmed.c (lowpart_bit_field_p): Add missing == 0 check.

Index: gcc/expmed.c
===
--- gcc/expmed.c2012-10-24 11:32:33.0 +0100
+++ gcc/expmed.c2012-10-24 18:52:58.303570945 +0100
@@ -402,7 +402,7 @@ lowpart_bit_field_p (unsigned HOST_WIDE_
 enum machine_mode struct_mode)
 {
   if (BYTES_BIG_ENDIAN)
-return (bitnum % BITS_PER_UNIT
+return (bitnum % BITS_PER_UNIT == 0
&& (bitnum + bitsize == GET_MODE_BITSIZE (struct_mode)
|| (bitnum + bitsize) % BITS_PER_WORD == 0));
   else

Re: PR c++/54928 infinite ICE when reporting ICE on macro expansion

[Manuel López-Ibáñez]

What about?

/* Add a note with text GMSGID and with LOCATION to the diagnostic CONTEXT.  */

Actually, I don't know why I call it "attach". diagnostic_add_note()
or diagnostic_print_note() seems clearer. What do you think?

Cheers,

Manuel.


On 24 October 2012 19:27, Jason Merrill  wrote:
> Agreed.  OK with the comment added.
>
> Jason

Re: libgo patch committed: Update to current Go library

[Uros Bizjak]

On Wed, Oct 24, 2012 at 7:46 PM, Ian Lance Taylor  wrote:
> On Wed, Oct 24, 2012 at 9:34 AM, Uros Bizjak  wrote:
>>
>> Continuing.
>> [New Thread 0x2000307b280 (LWP 8059)]
>>
>> Breakpoint 18, 0x020002e378c0 in socketpair () from /lib/libc.so.6.1
>>
>> Continuing.
>> [New Thread 0x20003083280 (LWP 8065)]
>> [Switching to Thread 0x20003083280 (LWP 8065)]
>>
>> [...]
>>
>> The first call with relevant FD is from:
>>
>> Breakpoint 21, 0x020002e243f8 in close () from /lib/libc.so.6.1
>> (gdb) i r a0
>> a0 0x8  8
>
> Does this mean that this is a call to close file descriptor 8?
> According to the strace log, the file descriptor we care about is 4.
> Although it is also true that I don't see a close of file descriptor 8
> at all in the strace log.  Or is the change from 4 to 8 due somehow to
> running the program under gdb?

Yes, I am running under gdb and all FDs are offset by +4 for some
reason. So, FD 8 corresponds to FD4 in the strace log.
>
>> (gdb) bt
>> #0  0x020002e243f8 in close () from /lib/libc.so.6.1
>> #1  0x00012003559c in syscall.Close (fd=) at 
>> libcalls.go:271
>> #2  0x025d3cfc in os.close.pN7_os.file (file=0xf840414b70) at
>> ../../../gcc-svn/trunk/libgo/go/os/file_unix.go:106
>> #3  0x02888f18 in ffi_call_osf () at
>> ../../../gcc-svn/trunk/libffi/src/alpha/osf.S:79
>> #4  0x028889c4 in ffi_call (cif=, fn=> out>, rvalue=, avalue=0xf840c87fe8)
>> at ../../../gcc-svn/trunk/libffi/src/alpha/ffi.c:169
>> #5  0x02558204 in reflect.call (func_type=0x29e9650
>> <__go_td_FppN7_os.fileerN5_erroree>,
>> func_addr=0x25d3c60 ,
>> is_interface=, is_method=,
>> params=0xf840c87fe0, results=0x0) at
>> ../../../gcc-svn/trunk/libgo/runtime/go-reflect-call.c:498
>> #6  0x025620b8 in runfinq (dummy=) at
>> ../../../gcc-svn/trunk/libgo/runtime/mgc0.c:1168
>> #7  0x02566b20 in kickoff () at
>> ../../../gcc-svn/trunk/libgo/runtime/proc.c:338
>> #8  0x020002d8d024 in ?? () from /lib/libc.so.6.1
>
> If this is indeed the file descriptor we care about, then this is
> interesting, because it is being closed by a finalizer run by the
> garbage collector.  That implies that the garbage collector collected
> the local variable readFile in TestPassFD in passfd_test.go, which
> would be clearly wrong.  Unfortunately this could be rather difficult
> to debug.

Yes, this is correct descriptor. For added fun, a descriptor that
corresponds to writeFile closes through the same mechanism.

Uros.

Re: libgo patch committed: Update to current Go library

[Ian Lance Taylor]

On Wed, Oct 24, 2012 at 9:34 AM, Uros Bizjak  wrote:
>
> Continuing.
> [New Thread 0x2000307b280 (LWP 8059)]
>
> Breakpoint 18, 0x020002e378c0 in socketpair () from /lib/libc.so.6.1
>
> Continuing.
> [New Thread 0x20003083280 (LWP 8065)]
> [Switching to Thread 0x20003083280 (LWP 8065)]
>
> [...]
>
> The first call with relevant FD is from:
>
> Breakpoint 21, 0x020002e243f8 in close () from /lib/libc.so.6.1
> (gdb) i r a0
> a0 0x8  8

Does this mean that this is a call to close file descriptor 8?
According to the strace log, the file descriptor we care about is 4.
Although it is also true that I don't see a close of file descriptor 8
at all in the strace log.  Or is the change from 4 to 8 due somehow to
running the program under gdb?

> (gdb) bt
> #0  0x020002e243f8 in close () from /lib/libc.so.6.1
> #1  0x00012003559c in syscall.Close (fd=) at 
> libcalls.go:271
> #2  0x025d3cfc in os.close.pN7_os.file (file=0xf840414b70) at
> ../../../gcc-svn/trunk/libgo/go/os/file_unix.go:106
> #3  0x02888f18 in ffi_call_osf () at
> ../../../gcc-svn/trunk/libffi/src/alpha/osf.S:79
> #4  0x028889c4 in ffi_call (cif=, fn= out>, rvalue=, avalue=0xf840c87fe8)
> at ../../../gcc-svn/trunk/libffi/src/alpha/ffi.c:169
> #5  0x02558204 in reflect.call (func_type=0x29e9650
> <__go_td_FppN7_os.fileerN5_erroree>,
> func_addr=0x25d3c60 ,
> is_interface=, is_method=,
> params=0xf840c87fe0, results=0x0) at
> ../../../gcc-svn/trunk/libgo/runtime/go-reflect-call.c:498
> #6  0x025620b8 in runfinq (dummy=) at
> ../../../gcc-svn/trunk/libgo/runtime/mgc0.c:1168
> #7  0x02566b20 in kickoff () at
> ../../../gcc-svn/trunk/libgo/runtime/proc.c:338
> #8  0x020002d8d024 in ?? () from /lib/libc.so.6.1

If this is indeed the file descriptor we care about, then this is
interesting, because it is being closed by a finalizer run by the
garbage collector.  That implies that the garbage collector collected
the local variable readFile in TestPassFD in passfd_test.go, which
would be clearly wrong.  Unfortunately this could be rather difficult
to debug.

Ian

patch to fix pr55049

[Vladimir Makarov]

  The following path shouldfix

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

  The patch was successfully bootstrapped on x86-64. Sorry, I can not 
bootstrap with mx32 because of libraries absence.


  Committed as rev. 192771.

2012-10-24  Vladimir Makarov  

PR bootstrap/55049
* lra-constraints.c (extract_loc_address_regs): Pass top_p for
ZERO_EXTEND operand.

svn diff --diff-cmd diff -x -up lra-constraints.c 
Index: lra-constraints.c
===
--- lra-constraints.c   (revision 192770)
+++ lra-constraints.c   (working copy)
@@ -515,6 +515,12 @@ extract_loc_address_regs (bool top_p, en
 case PC:
   return;
 
+case ZERO_EXTEND:
+  /* Pass TOP_P for displacement.  */
+  extract_loc_address_regs (top_p, mode, as, &XEXP (*loc, 0), context_p,
+   code, index_code, modify_p, ad);
+  return;
+
 case PLUS:
 case LO_SUM:
   /* When we have an address that is a sum, we must determine

Re: [C++ Patch] PR 34892

[Jason Merrill]

On 10/24/2012 01:20 PM, Paolo Carlini wrote:

+  if (parm == error_mark_node
+ || TREE_PURPOSE (parm) == error_mark_node)


It seems odd to bail out early if the default argument is bad even if we 
aren't trying to use it.  Doesn't it work to check this further down 
where we actually look at the default argument?


Jason

Re: PR c++/54928 infinite ICE when reporting ICE on macro expansion

[Jason Merrill]

Agreed.  OK with the comment added.

Jason

Re: patch to fix constant math - 4th patch - the wide-int class.

[Mike Stump]

On Oct 24, 2012, at 2:43 AM, Richard Biener  wrote:
> On Tue, Oct 23, 2012 at 6:12 PM, Kenneth Zadeck
>  wrote:
>> 
>> On 10/23/2012 10:12 AM, Richard Biener wrote:
>>> 
>>> +  HOST_WIDE_INT val[2 * MAX_BITSIZE_MODE_ANY_INT /
>>> HOST_BITS_PER_WIDE_INT];
>>> 
>>> are we sure this rounds properly?  Consider a port with max byte mode
>>> size 4 on a 64bit host.
>> 
>> I do not believe that this can happen.   The core compiler includes all
>> modes up to TI mode, so by default we already up to 128 bits.
> 
> And mode bitsizes are always power-of-two?  I suppose so.

Actually, no, they are not.  Partial int modes can have bit sizes that are not 
power of two, and, if there isn't an int mode that is bigger, we'd want to 
round up the partial int bit size.  Something like ((2 * 
MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) /  
HOST_BITS_PER_WIDE_INT should do it.

>>> I still would like to have the ability to provide specializations of
>>> wide_int
>>> for "small" sizes, thus ideally wide_int would be a template templated
>>> on the number of HWIs in val.  Interface-wise wide_int<2> should be
>>> identical to double_int, thus we should be able to do
>>> 
>>> typedef wide_int<2> double_int;
>> 
>> If you want to go down this path after the patches get in, go for it.I
>> see no use at all for this.
>> This was not meant to be a plug in replacement for double int. This goal of
>> this patch is to get the compiler to do the constant math the way that the
>> target does it.   Any such instantiation is by definition placing some
>> predefined limit that some target may not want.
> 
> Well, what I don't really like is that we now have two implementations
> of functions that perform integer math on two-HWI sized integers.  What
> I also don't like too much is that we have two different interfaces to operate
> on them!  Can't you see how I come to not liking this?  Especially the
> latter …

double_int is logically dead.  Reactoring wide-int and double-int is a waste of 
time, as the time is better spent removing double-int from the compiler.  All 
the necessary semantics and code of double-int _has_ been refactored into 
wide-int already.  Changing wide-int in any way to vend anything to double-int 
is wrong, as once double-int is removed, then all the api changes to make 
double-int share from wide-int is wasted and must then be removed.  The path 
forward is the complete removal of double-int; it is wrong, has been wrong and 
always will be wrong, nothing can change that.

[C++ Patch] PR 34892

[Paolo Carlini]

Hi,

a *very* old ICE on invalid, even a regression (from before variadic 
templates, I guess!). I tried various other tweaks, like catching the 
issue earlier but diagnostic quality decreases, too many cascading error 
messages. The below means I have to tweak only a couple of existing 
testcases, and I'm actually pretty happy with that, because we produce 
much less verbose diagnostic in both cases (1 error message less / 2 
respectively).


Tested x86_64-linux.

Thanks,
Paolo.

//



/cp
2012-10-24  Paolo Carlini  

PR c++/34892
* pt.c (coerce_template_parms): Check TREE_PURPOSE (parm)
for error_mark_node.

/testsuite
2012-10-24  Paolo Carlini  

PR c++/34892
* g++.dg/template/crash114.C: New.
* g++.dg/template/crash55.C: Tweak dg-error directive.
* g++.dg/template/crash57.C: Likewise.
Index: cp/pt.c
===
--- cp/pt.c (revision 192762)
+++ cp/pt.c (working copy)
@@ -6645,11 +6645,12 @@ coerce_template_parms (tree parms,
   /* Get the Ith template parameter.  */
   parm = TREE_VEC_ELT (parms, parm_idx);
  
-  if (parm == error_mark_node)
-  {
-TREE_VEC_ELT (new_inner_args, arg_idx) = error_mark_node;
-continue;
-  }
+  if (parm == error_mark_node
+ || TREE_PURPOSE (parm) == error_mark_node)
+   {
+ TREE_VEC_ELT (new_inner_args, arg_idx) = error_mark_node;
+ continue;
+   }
 
   /* Calculate the next argument.  */
   if (arg_idx < nargs)
Index: testsuite/g++.dg/template/crash114.C
===
--- testsuite/g++.dg/template/crash114.C(revision 0)
+++ testsuite/g++.dg/template/crash114.C(working copy)
@@ -0,0 +1,5 @@
+// PR c++/34892
+
+template struct A {}; // { dg-error "expected" }
+// { dg-message "variadic" "" { target c++98 } 3 }
+A<0> a;   // { dg-error "type" }
Index: testsuite/g++.dg/template/crash55.C
===
--- testsuite/g++.dg/template/crash55.C (revision 192762)
+++ testsuite/g++.dg/template/crash55.C (working copy)
@@ -3,4 +3,4 @@
 template // { dg-error "nested-name-specifier|two 
or more|valid type" }
 struct A {};
 
-template void foo(A); // { dg-error "mismatch|constant|template 
argument" }
+template void foo(A); // { dg-error "type|template" }
Index: testsuite/g++.dg/template/crash57.C
===
--- testsuite/g++.dg/template/crash57.C (revision 192762)
+++ testsuite/g++.dg/template/crash57.C (working copy)
@@ -7,4 +7,4 @@ template struct B
 template struct C;// { dg-error "token" }
 };
 
-A a;  // { dg-error "type/value 
mismatch|constant|declaration" }
+A a;  // { dg-error "type" }

Re: ARC port (5/5): rest of gcc/{,common/}config/arc/

[Joseph S. Myers]

Don't use ATTRIBUTE_UNUSED on a parameter that, as in 
arc_option_init_struct, is in fact used unconditionally.

handle_option hooks take a location, so use warning_at not plain warning 
in arc_handle_option.  Same diagnostic issue as noted before applies.

There are lots of obsolete bits in your ASM_SPEC and LINK_SPEC, such as 
%{v}, %{version:-v}, %{b}, %{Wl,*:%*}., %{!dynamic-linker:...}.  Remove 
them.

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

Re: [C++ Patch] PR 53761

[Jason Merrill]

On 10/10/2012 11:13 AM, Paolo Carlini wrote:

- error ("type transparent class %qT does not have any fields", t);
+ if (TREE_CODE (t) == UNION_TYPE)
+   error ("type transparent union %qT does not have any fields", t);
+ else
+   error ("type transparent class %qT does not have any fields", t);


If you use %q#T you don't need to repeat the class-key.


+   error ("type transparent union %qT cannot be made transparent", t);


Let's say why not.

Jason

Re: ARC port (4/5): libgcc/config/arc/

[Joseph S. Myers]

If you need special t-* logit for fp-bit.c / dp-bit.c, rather than being 
able to use t-fdpbit, then you need comments explaining why the special 
logic rather than the generic code is used.

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

Re: ARC port (3/5): gcc/config/arc/arc.md

[Joseph S. Myers]

Same diagnostic comment regarding one fatal_error call here; in addition, 
diagnostic functions should not be called directly from .md files at all, 
because .md files aren't processed by exgettext to extract messages for 
translation.

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

Re: ARC port (2/5): gcc/config/arc/arc.c

[Joseph S. Myers]

Diagnostics should not end with '.' (or '\n', in some cases here); also 
avoid starting with a capital letter in cases such as "Operand".

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

Re: RFA: Add lock_lenth attribute to support the ARC port (Was: Re: Ping: RFA: add lock_length attribute to break branch-shortening cycles)

[Joseph S. Myers]

On Tue, 23 Oct 2012, Joern Rennecke wrote:

> I'll be posting the ARC port shortly; it does not fit into a single 100 KB
> posting, so I'm thinking of splitting it in a configury patch and zx
> compressed files/tarballs for arc.c, arc.md, libgcc, and the rest of the port.

The size limit is 400 kB, not 100 kB.  Hopefully that means you don't need 
to compress so many bits and can attach more as plain patches for future 
revisions.

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

Re: ARC port (1/5): configuration file patches

[Joseph S. Myers]

On Tue, 23 Oct 2012, Joern Rennecke wrote:

> + tm_file="dbxelf.h elfos.h linux.h  ${tm_file}"

Should be using gnu-user.h linux.h glibc-stdint.h, not linux.h on its own.

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

Re: libgo patch committed: Update to current Go library

[Uros Bizjak]

On Wed, Oct 24, 2012 at 4:30 PM, Ian Lance Taylor  wrote:
> On Wed, Oct 24, 2012 at 6:19 AM, Uros Bizjak  wrote:
>> On Wed, Oct 24, 2012 at 3:10 PM, Ian Lance Taylor  wrote:
>>> On Wed, Oct 24, 2012 at 5:31 AM, Uros Bizjak  wrote:
 On Wed, Oct 24, 2012 at 2:18 PM, Andreas Schwab  
 wrote:
> Uros Bizjak  writes:
>
>> To answer my own question:
>>
>> dup(4)  = 9
>> ...
>> close(9)= 0
>> dup(4)  = -1 EBADF (Bad file descriptor)
>>
>> Test is calling dup on a closed file descriptor.
>
> FD 4 is most likely closed by one of the cloned threads.  Use strace -f
> to follow them.

 Yes, indeed! Attached strace -f record confirms this on alpha.

 The same happens on x86_64, but for some reason x86_64 doesn't
 complain when executing dup(2) on closed FD.
>>>
>>> The test execs itself by calling the fork and execve functions in
>>> libc.  It is the child process that closes the FD after it forks.
>>> From the point of view of the parent process, the FD should still be
>>> open.  I don't think you attached the strace -f output so I can't
>>> confirm this.
>>
>> Eh, sorry, attached now.
>>
>> After the second socketpair and before dup, there is a close in the same 
>> thread.
>
> Thanks.  I agree.  Unfortunately, I can't figure out what is causing
> it.  These seem to be the relevant calls.
>
> 16252 socketpair(PF_FILE, SOCK_STREAM, 0, [3, 4]) = 0
>
> 16252 clone(child_stack=0,
> flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD,
> child_tidptr=0x20002f0ecd0) = 16259
>
> 16259 execve("./a.out", ["./a.out", "-test.run=^TestPassFD$", "--",
> "/tmp/TestPassFD684357043"], [/* 33 vars */] 
>
> 16252 clone( 
> 16252 <... clone resumed> child_stack=0x200031e2b70,
> flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
> parent_tidptr=0x200031e3350, tls=0x200031e3970,
> child_tidptr=0x200031e3350) = 16260
>
> * This is the bad call, but where does this come from?
> 16252 close(4 
> 16252 <... close resumed> ) = 0
>
> 16260 wait4(16259,  
>
> 16261 read(9,  
>
> 16259 sendmsg(5, {msg_name(0)=NULL, msg_iov(1)=[{"x", 1}],
> msg_controllen=24, {cmsg_len=20, cmsg_level=SOL_SOCKET,
> cmsg_type=SCM_RIGHTS, {9}}, msg_flags=0}, 0) = 1
>
> 16259 exit_group(0) = ?
>
> 16261 <... read resumed> "", 512)   = 0
>
> 16260 <... wait4 resumed> [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0,
> {ru_utime={0, 46875}, ru_stime={0, 26367}, ...}) = 16259
>
> 16261 dup(4)= -1 EBADF (Bad file descriptor)
>
>
> Here is an approachj that might help.  Set a breakpoint on socketpair.
>  The failure comes after the second call to socketpair, the one from
> the function TestPassFD.  After that breakpoint is reached, set a
> breakpoint on close.  Look for the call to close(4).  Get a backtrace
> from there so we can see what is calling it.

Continuing.
[New Thread 0x2000307b280 (LWP 8059)]

Breakpoint 18, 0x020002e378c0 in socketpair () from /lib/libc.so.6.1

Continuing.
[New Thread 0x20003083280 (LWP 8065)]
[Switching to Thread 0x20003083280 (LWP 8065)]

[...]

The first call with relevant FD is from:

Breakpoint 21, 0x020002e243f8 in close () from /lib/libc.so.6.1
(gdb) i r a0
a0 0x8  8
(gdb) bt
#0  0x020002e243f8 in close () from /lib/libc.so.6.1
#1  0x00012003559c in syscall.Close (fd=) at libcalls.go:271
#2  0x025d3cfc in os.close.pN7_os.file (file=0xf840414b70) at
../../../gcc-svn/trunk/libgo/go/os/file_unix.go:106
#3  0x02888f18 in ffi_call_osf () at
../../../gcc-svn/trunk/libffi/src/alpha/osf.S:79
#4  0x028889c4 in ffi_call (cif=, fn=, rvalue=, avalue=0xf840c87fe8)
at ../../../gcc-svn/trunk/libffi/src/alpha/ffi.c:169
#5  0x02558204 in reflect.call (func_type=0x29e9650
<__go_td_FppN7_os.fileerN5_erroree>,
func_addr=0x25d3c60 ,
is_interface=, is_method=,
params=0xf840c87fe0, results=0x0) at
../../../gcc-svn/trunk/libgo/runtime/go-reflect-call.c:498
#6  0x025620b8 in runfinq (dummy=) at
../../../gcc-svn/trunk/libgo/runtime/mgc0.c:1168
#7  0x02566b20 in kickoff () at
../../../gcc-svn/trunk/libgo/runtime/proc.c:338
#8  0x020002d8d024 in ?? () from /lib/libc.so.6.1

[... dup call follows from the same thread ...]

Breakpoint 22, 0x020002e24e90 in dup () from /lib/libc.so.6.1
(gdb) i r a0
a0 0x8  8
(gdb) bt
#0  0x020002e24e90 in dup () from /lib/libc.so.6.1
#1  0x00012003578c in syscall.Dup (oldfd=8) at libcalls.go:314
#2  0x025b2794 in net.newFileFD (f=0xf844a0) at
../../../gcc-svn/trunk/libgo/go/net/file_unix.go:16
#3  0x025c0dcc in net.FileConn (f=0x8) at
../../../gcc-svn/trunk/libgo/go/net/file_unix.go:78
#4  0x0001200433e4 in syscall_test.TestPassFD (t.param=) at passfd_test.go:60
#5  

Re: [PATCH 3/3] [asan] Instrument built-in memory access function calls

[Jakub Jelinek]

On Wed, Oct 24, 2012 at 05:11:23PM +0200, Dodji Seketeli wrote:
> If I write exactly what you wrote here, I am getting an error for e.g:
> 
> void
> bar ()
> {
>   char bar[1] = {0};
>   int n = 0;
> 
>   __builtin_memset (bar, 0, n);
> }

I see, the problem is that build_check_stmt is initially called on an
iterator in empty bb.  Your solution is fine.

> > On the other side, you IMHO want to handle here also __atomic_* and
> > __sync_* builtins (not by using instrument_mem_region_access, but
> > just instrument_derefs
> 
> Updated in the patch below.

case BUILT_IN_ATOMIC_ALWAYS_LOCK_FREE:
case BUILT_IN_ATOMIC_IS_LOCK_FREE:
I think don't touch the memory at all (or not necessarily),
and IMHO you don't want to handle the BUILT_IN_*_N variants either,
those are just FE builtins that are lowered to the corresponding
_{1,2,4,8,16} variants.

> @@ -661,13 +702,477 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t,
>int volatilep = 0, unsignedp = 0;
>get_inner_reference (t, &bitsize, &bitpos, &offset,
>  &mode, &unsignedp, &volatilep, false);
> -  if (bitpos != 0 || bitsize != size_in_bytes * BITS_PER_UNIT)
> +  if (bitpos % BITS_PER_UNIT || bitsize != size_in_bytes * BITS_PER_UNIT)
>  return;

Shouldn't that be bitpos % (size_in_bytes * BITS_PER_UNIT) ?
1 byte or 2 byte access at bitpos 80 is fine, but 4 byte access is not.

Jakub

Re: PR c++/54928 infinite ICE when reporting ICE on macro expansion

[Dodji Seketeli]

I am not a maintainer so the below are just some casual comments of
mine.  I am deferring to the maintainers for a real review.

Manuel López-Ibáñez  writes:

> gcc/
>   * tree-diagnostic.c (maybe_unwind_expanded_macro_loc):
>   Use diagnostic_attach_note.
>   * diagnostic.c (diagnostic_build_prefix): Make diagnostic const.
>   (default_diagnostic_finalizer): Do not destroy prefix here.
>   (diagnostic_report_diagnostic): Destroy it here.
>   (diagnostic_attach_note): New.
>   * diagnostic.h (diagnostic_attach_note): Declare.
>

These bits looks good to me, barring ...

[...]

> +void
> +diagnostic_attach_note (diagnostic_context *context,
> +location_t location,
> +const char * gmsgid, ...)
> +{

... this function that lacks a comment.

Thanks.

-- 
Dodji

Re: [PATCH 3/3] [asan] Instrument built-in memory access function calls

[Dodji Seketeli]

Jakub Jelinek  writes:

> On Wed, Oct 24, 2012 at 05:16:26PM +0200, Dodji Seketeli wrote:
>> Jakub Jelinek  writes:
>> 
>> >> For 'strlen', can the memory check be done at the end of the string
>> >> using the returned length?
>> >
>> > Guess strlen is commonly expanded inline, so it would be worthwhile to 
>> > check
>> > the shadow memory after the call (well, we could check the first byte
>> > before the call and the last one after the call).
>> 
>> How do I get the result of the (strlen) call in gimple?
>
> That is gimple_call_lhs (call_stmt).

OK, thank you.

> So for insturmenting strlen, you want to ammend:
>   tmp = strlen (ptr);
> as
>   asan_addr_check (ptr);
>   tmp = strlen (ptr);
>   asan_addr_check (ptr + tmp);

I see, thanks.

-- 
Dodji

Re: [PATCH 3/3] [asan] Instrument built-in memory access function calls

[Jakub Jelinek]

On Wed, Oct 24, 2012 at 05:16:26PM +0200, Dodji Seketeli wrote:
> Jakub Jelinek  writes:
> 
> >> For 'strlen', can the memory check be done at the end of the string
> >> using the returned length?
> >
> > Guess strlen is commonly expanded inline, so it would be worthwhile to check
> > the shadow memory after the call (well, we could check the first byte
> > before the call and the last one after the call).
> 
> How do I get the result of the (strlen) call in gimple?

That is gimple_call_lhs (call_stmt).
So for insturmenting strlen, you want to ammend:
  tmp = strlen (ptr);
as
  asan_addr_check (ptr);
  tmp = strlen (ptr);
  asan_addr_check (ptr + tmp);

Jakub

patch to fix PR55048

[Vladimir Makarov]

  The following patch fixes

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

  LRA tried to take BB from non-NOTE_INSN_BASIC_BLOCK note and got NULL 
which resulted in SIGSEGV.


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

  Committed as rev. 192770.

2012-10-24  Vladimir Makarov  

PR bootstrap/55048
* lra-constraints.c (update_ebb_live_info): Skip
non-NOTE_INSN_BASIC_BLOCK notes.


Index: lra-constraints.c
===
--- lra-constraints.c   (revision 192743)
+++ lra-constraints.c   (working copy)
@@ -4300,6 +4300,10 @@ update_ebb_live_info (rtx head, rtx tail
curr_insn = prev_insn)
 {
   prev_insn = PREV_INSN (curr_insn);
+  /* We need to process empty blocks too.  They contain
+NOTE_INSN_BASIC_BLOCK referring for the basic block.  */
+  if (NOTE_P (curr_insn) && NOTE_KIND (curr_insn) != NOTE_INSN_BASIC_BLOCK)
+   continue;
   curr_bb = BLOCK_FOR_INSN (curr_insn);
   if (curr_bb != prev_bb)
{

Re: [PATCH 2/3] [asan] Factorize condition insertion code out of build_check_stmt

[Jakub Jelinek]

On Wed, Oct 24, 2012 at 04:46:05PM +0200, Dodji Seketeli wrote:
> Fixed.  Below is the updated patch.

Ok, thanks.

Jakub

[Patch, Fortran, committed] PR 55037: [4.8 Regression] [OOP] ICE with local allocatable variable of abstract type

[Janus Weil]

Hi all,

I have just committed an obvious fix for a recent OOP regression:

http://gcc.gnu.org/viewcvs?view=revision&revision=192768

Cheers,
Janus

Re: [PATCH 3/3] [asan] Instrument built-in memory access function calls

[Dodji Seketeli]

Jakub Jelinek  writes:

>> For 'strlen', can the memory check be done at the end of the string
>> using the returned length?
>
> Guess strlen is commonly expanded inline, so it would be worthwhile to check
> the shadow memory after the call (well, we could check the first byte
> before the call and the last one after the call).

How do I get the result of the (strlen) call in gimple?

-- 
Dodji

Re: [PATCH 3/3] [asan] Instrument built-in memory access function calls

[Dodji Seketeli]

Jakub Jelinek  writes:

> On Tue, Oct 23, 2012 at 03:11:29PM +0200, Dodji Seketeli wrote:
>> +  /* (src, n) style memops.  */
>> +case BUILT_IN_STRNDUP:
>> +  source0 = gimple_call_arg (call, 0);
>> +  len = gimple_call_arg (call, 1);
>> +  break;
>
> I think you can't instrument strndup either, the length is just a limit
> there, it can copy fewer characters than that if strlen (source0) is
> shorter.  libasan intercepts strndup I think.

Fixed.

>> +  /* (src, x, n) style memops.  */  
>> +case BUILT_IN_MEMCHR:
>> +  source0 = gimple_call_arg (call, 0);
>> +  len = gimple_call_arg (call, 2);
>
> And similarly for memchr, you could call
> p = malloc (4096);
> p[4095] = 1;
> x = memchr (p, 1, 8192);
> and it shouldn't read anything past the end of the
> allocated area.

Fixed as well.

> On Tue, Oct 23, 2012 at 03:11:29PM +0200, Dodji Seketeli wrote:
> > * asan.c (insert_if_then_before_iter)
> > (instrument_mem_region_access)
> > (maybe_instrument_builtin_call, maybe_instrument_call): New static
> 
> Why not just write it:
>   * asan.c (insert_if_then_before_iter, instrument_mem_region_access,
>   maybe_instrument_builtin_call, maybe_instrument_call): New static
> ?

It's emacs that formats it like that automatically.  I am not sure how
to teach him otherwise.  I have fixed this as you want by doing it "by
hand".

> 
> > functions.
> 
> + tree pointed_to_type = TREE_TYPE (TREE_TYPE (base));
> 
> Shouldn't pointed_to_type be always char_type_node?
> I mean it shouldn't be VOID_TYPE, even when the argument is (void *),
> etc.

Right.  Fixed.

> 
> > +  /* The 'then block' of the 'if (len != 0) condition is where
> > +we'll generate the asan instrumentation code now.  */
> > +  gsi = gsi_start_bb (then_bb);
> > +
> > +  /* Instrument the beginning of the memory region to be accessed,
> > +and arrange for the rest of the intrumentation code to be
> > +inserted in the then block *after* the current gsi.  */
> > +  build_check_stmt (base, &gsi, location, is_store,
> > +   int_size_in_bytes (pointed_to_type));
> > +  gsi = gsi_last_bb (then_bb);
> > +}
> > +  else
> > +{
> > +  /* Instrument the beginning of the memory region to be
> > +accessed.  */
> > +  build_check_stmt (base, iter, location, is_store,
> > +   int_size_in_bytes (pointed_to_type));
> > +  gsi = *iter;
> > +}
> 
> Is there any reason why you can't call build_check_stmt just once, after
> the conditional?  I.e. do
>   ...
>   gsi = gsi_start_bb (then_bb);
> }
>   else
> gsi = *iter;
> 
>   build_check_stmt (base, &gsi, location, is_store,
>   int_size_in_bytes (pointed_to_type));


If I write exactly what you wrote here, I am getting an error for e.g:

void
bar ()
{
  char bar[1] = {0};
  int n = 0;

  __builtin_memset (bar, 0, n);
}

test.cc: In function ‘void bar()’:
test.cc:11:1: erreur: definition in block 9 does not dominate use in block 7
 bar ()
 ^
for SSA_NAME: _28 in statement:
_29 = (unsigned long) _28;
test.cc:11:1: erreur interne du compilateur: verify_ssa failed

I think the issue is that when we insert the "if (len != 0)" condition
and 'then block',  we want the instrumentation to happen in the then
block;  but then build_check_stmt as in

gsi = gsi_start_bb (then_bb);
build_check_stmt (base, &gsi, location, is_store,
  int_size_in_bytes (pointed_to_type));

arranges to leave gsi to point at the beginning of the then_bb, and
then the subsequent insertion of instrumentation code that happens
later at:

gsi_insert_before (&gsi, offset, GSI_NEW_STMT);

puts said instrumentation code logically /before/ the beginning of the
then_bb; so bad things happen.  Or maybe I am missing something?

To avoid calling build_check_stmt twice I am proposing this change:

diff --git a/gcc/asan.c b/gcc/asan.c
index ecc0d0b..2ccc082 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -721,6 +721,7 @@ instrument_mem_region_access (tree base, tree len,
   gimple_stmt_iterator gsi = *iter;
   tree pointed_to_type = TREE_TYPE (TREE_TYPE (base));
 
+  basic_block fallthrough_bb = NULL, then_bb = NULL;
   if (!is_gimple_constant (len))
 {
   /* So, the length of the memory area to asan-protect is
@@ -733,7 +734,6 @@ instrument_mem_region_access (tree base, tree len,
}
   // falltrough instructions, starting with *ITER.  */
 
-  basic_block fallthrough_bb, then_bb;
   gimple g = gimple_build_cond (NE_EXPR,
len,
build_int_cst (TREE_TYPE (len), 0),
@@ -747,22 +747,22 @@ instrument_mem_region_access (tree base, tree len,
   /* The 'then block' of the 'if (len != 0) condition is where
 we'll generate the asan instrumentation code now.  */
   gsi = gsi_start_bb (then_bb);
-
-  /* Instrument the beginning 

Re: [Patch] Potential fix for PR55033

[Alan Modra]

On Tue, Oct 23, 2012 at 06:25:43PM +0200, Sebastian Huber wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55033
> 
> This patch fixes my problem, but I am absolutely not sure if this is the
> right way.
[snip]

This is http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9571 all over again.

IMHO your patch is not wrong, but a better idea is that if we've used
categorize_decl_for_section to choose a section name, then we ought to
also use categorize_decl_for_section to choose section flags.

My original fix for pr9571 was to pass the decl down to
get_named_section.
http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02487.html
That hit the assert in get_named_section when building libgfortran for
ia64, and my knee-jerk patch to fix that was to simply satisfy the
assert.  After looking at all the target section_type_flags functions,
I believe the following is what I should have done way back then.
Bootstrapped and regression tested powerpc64-linux.

* varasm.c (default_elf_select_section): Move !DECL_P check..
(get_named_section): ..to here before calling get_section_name.
Adjust assertion.
(default_section_type_flags): Add DECL_P check.
* config/i386/winnt.c (i386_pe_section_type_flags): Likewise.
* config/rs6000/rs6000.c (rs6000_xcoff_section_type_flags): Likewise.

Index: gcc/varasm.c
===
--- gcc/varasm.c(revision 192660)
+++ gcc/varasm.c(working copy)
@@ -403,12 +403,16 @@ get_named_section (tree decl, const char *name, in
 {
   unsigned int flags;
 
-  gcc_assert (!decl || DECL_P (decl));
   if (name == NULL)
-name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl));
+{
+  gcc_assert (decl && DECL_P (decl) && DECL_SECTION_NAME (decl));
+  name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl));
+}
 
   flags = targetm.section_type_flags (decl, name, reloc);
 
+  if (decl && !DECL_P (decl))
+decl = NULL_TREE;
   return get_section (name, flags, decl);
 }
 
@@ -5943,7 +5947,7 @@ default_section_type_flags (tree decl, const char
flags |= SECTION_RELRO;
 }
 
-  if (decl && DECL_ONE_ONLY (decl))
+  if (decl && DECL_P (decl) && DECL_ONE_ONLY (decl))
 flags |= SECTION_LINKONCE;
 
   if (decl && TREE_CODE (decl) == VAR_DECL && DECL_THREAD_LOCAL_P (decl))
@@ -6299,8 +6303,6 @@ default_elf_select_section (tree decl, int reloc,
   gcc_unreachable ();
 }
 
-  if (!DECL_P (decl))
-decl = NULL_TREE;
   return get_named_section (decl, sname, reloc);
 }
 
Index: gcc/config/i386/winnt.c
===
--- gcc/config/i386/winnt.c (revision 192660)
+++ gcc/config/i386/winnt.c (working copy)
@@ -476,7 +476,7 @@ i386_pe_section_type_flags (tree decl, const char
flags |= SECTION_PE_SHARED;
 }
 
-  if (decl && DECL_ONE_ONLY (decl))
+  if (decl && DECL_P (decl) && DECL_ONE_ONLY (decl))
 flags |= SECTION_LINKONCE;
 
   /* See if we already have an entry for this section.  */
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 192660)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -25689,7 +25689,7 @@ rs6000_xcoff_section_type_flags (tree decl, const
   unsigned int flags = default_section_type_flags (decl, name, reloc);
 
   /* Align to at least UNIT size.  */
-  if (flags & SECTION_CODE || !decl)
+  if ((flags & SECTION_CODE) != 0 || !decl || !DECL_P (decl))
 align = MIN_UNITS_PER_WORD;
   else
 /* Increase alignment of large objects if not already stricter.  */

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/3] [asan] Factorize condition insertion code out of build_check_stmt

[Dodji Seketeli]

Jakub Jelinek  writes:

> On Tue, Oct 23, 2012 at 03:08:07PM +0200, Dodji Seketeli wrote:
>> +static gimple_stmt_iterator
>> +create_cond_insert_point_before_iter (gimple_stmt_iterator *iter,
>> +  bool then_more_likely_p,
>> +  basic_block *then_block,
>> +  basic_block *fallthrough_block)
>> +{
>> +  gcc_assert (then_block != NULL && fallthrough_block != NULL);
>
> I think this assert is useless, if they are NULL
>
>> +  *then_block = then_bb;
>> +  *fallthrough_block = fallthru_bb;
>
> the above two stmts will just crash and be as useful for debugging
> as the assert.

Fixed.  Below is the updated patch.

gcc/

* asan.c (create_cond_insert_point_before_iter): Factorize out of ...
(build_check_stmt): ... here.
---
 gcc/asan.c | 120 ++---
 1 file changed, 76 insertions(+), 44 deletions(-)

diff --git a/gcc/asan.c b/gcc/asan.c
index e8660a6..39e77e6 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -397,6 +397,75 @@ asan_init_func (void)
 #define PROB_VERY_UNLIKELY (REG_BR_PROB_BASE / 2000 - 1)
 #define PROB_ALWAYS(REG_BR_PROB_BASE)
 
+/* Split the current basic block and create a condition statement
+   insertion point right before the statement pointed to by ITER.
+   Return an iterator to the point at which the caller might safely
+   insert the condition statement.
+
+   THEN_BLOCK must be set to the address of an uninitialized instance
+   of basic_block.  The function will then set *THEN_BLOCK to the
+   'then block' of the condition statement to be inserted by the
+   caller.
+
+   Similarly, the function will set *FALLTRHOUGH_BLOCK to the 'else
+   block' of the condition statement to be inserted by the caller.
+
+   Note that *FALLTHROUGH_BLOCK is a new block that contains the
+   statements starting from *ITER, and *THEN_BLOCK is a new empty
+   block.
+
+   *ITER is adjusted to still point to the same statement it was
+   *pointing to initially.  */
+
+static gimple_stmt_iterator
+create_cond_insert_point_before_iter (gimple_stmt_iterator *iter,
+ bool then_more_likely_p,
+ basic_block *then_block,
+ basic_block *fallthrough_block)
+{
+  gimple_stmt_iterator gsi = *iter;
+
+  if (!gsi_end_p (gsi))
+gsi_prev (&gsi);
+
+  basic_block cur_bb = gsi_bb (*iter);
+
+  edge e = split_block (cur_bb, gsi_stmt (gsi));
+
+  /* Get a hold on the 'condition block', the 'then block' and the
+ 'else block'.  */
+  basic_block cond_bb = e->src;
+  basic_block fallthru_bb = e->dest;
+  basic_block then_bb = create_empty_bb (cond_bb);
+
+  /* Set up the newly created 'then block'.  */
+  e = make_edge (cond_bb, then_bb, EDGE_TRUE_VALUE);
+  int fallthrough_probability =
+then_more_likely_p
+? PROB_VERY_UNLIKELY
+: PROB_ALWAYS - PROB_VERY_UNLIKELY;
+  e->probability = PROB_ALWAYS - fallthrough_probability;
+  make_single_succ_edge (then_bb, fallthru_bb, EDGE_FALLTHRU);
+
+  /* Set up the fallthrough basic block.  */
+  e = find_edge (cond_bb, fallthru_bb);
+  e->flags = EDGE_FALSE_VALUE;
+  e->count = cond_bb->count;
+  e->probability = fallthrough_probability;
+
+  /* Update dominance info for the newly created then_bb; note that
+ fallthru_bb's dominance info has already been updated by
+ split_bock.  */
+  if (dom_info_available_p (CDI_DOMINATORS))
+set_immediate_dominator (CDI_DOMINATORS, then_bb, cond_bb);
+
+  *then_block = then_bb;
+  *fallthrough_block = fallthru_bb;
+  *iter = gsi_start_bb (fallthru_bb);
+
+  return gsi_last_bb (cond_bb);
+}
+
 /* Instrument the memory access instruction BASE.  Insert new
statements before ITER.
 
@@ -411,8 +480,7 @@ build_check_stmt (tree base, gimple_stmt_iterator *iter,
  int size_in_bytes)
 {
   gimple_stmt_iterator gsi;
-  basic_block cond_bb, then_bb, else_bb;
-  edge e;
+  basic_block then_bb, else_bb;
   tree t, base_addr, shadow;
   gimple g;
   tree shadow_ptr_type = shadow_ptr_types[size_in_bytes == 16 ? 1 : 0];
@@ -421,51 +489,15 @@ build_check_stmt (tree base, gimple_stmt_iterator *iter,
 = build_nonstandard_integer_type (TYPE_PRECISION (TREE_TYPE (base)), 1);
   tree base_ssa = base;
 
-  /* We first need to split the current basic block, and start altering
- the CFG.  This allows us to insert the statements we're about to
- construct into the right basic blocks.  */
-
-  cond_bb = gimple_bb (gsi_stmt (*iter));
-  gsi = *iter;
-  gsi_prev (&gsi);
-  if (!gsi_end_p (gsi))
-e = split_block (cond_bb, gsi_stmt (gsi));
-  else
-e = split_block_after_labels (cond_bb);
-  cond_bb = e->src;
-  else_bb = e->dest;
-
-  /* A recap at this point: else_bb is the basic block at whose head
- is the gimple statement for which this check expression is being
- built.  cond_bb is the (possibly new, synthetic) basic blo

Re: libgo patch committed: Update to current Go library

[Ian Lance Taylor]

On Wed, Oct 24, 2012 at 6:19 AM, Uros Bizjak  wrote:
> On Wed, Oct 24, 2012 at 3:10 PM, Ian Lance Taylor  wrote:
>> On Wed, Oct 24, 2012 at 5:31 AM, Uros Bizjak  wrote:
>>> On Wed, Oct 24, 2012 at 2:18 PM, Andreas Schwab  
>>> wrote:
 Uros Bizjak  writes:

> To answer my own question:
>
> dup(4)  = 9
> ...
> close(9)= 0
> dup(4)  = -1 EBADF (Bad file descriptor)
>
> Test is calling dup on a closed file descriptor.

 FD 4 is most likely closed by one of the cloned threads.  Use strace -f
 to follow them.
>>>
>>> Yes, indeed! Attached strace -f record confirms this on alpha.
>>>
>>> The same happens on x86_64, but for some reason x86_64 doesn't
>>> complain when executing dup(2) on closed FD.
>>
>> The test execs itself by calling the fork and execve functions in
>> libc.  It is the child process that closes the FD after it forks.
>> From the point of view of the parent process, the FD should still be
>> open.  I don't think you attached the strace -f output so I can't
>> confirm this.
>
> Eh, sorry, attached now.
>
> After the second socketpair and before dup, there is a close in the same 
> thread.

Thanks.  I agree.  Unfortunately, I can't figure out what is causing
it.  These seem to be the relevant calls.

16252 socketpair(PF_FILE, SOCK_STREAM, 0, [3, 4]) = 0

16252 clone(child_stack=0,
flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD,
child_tidptr=0x20002f0ecd0) = 16259

16259 execve("./a.out", ["./a.out", "-test.run=^TestPassFD$", "--",
"/tmp/TestPassFD684357043"], [/* 33 vars */] 

16252 clone( 
16252 <... clone resumed> child_stack=0x200031e2b70,
flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0x200031e3350, tls=0x200031e3970,
child_tidptr=0x200031e3350) = 16260

* This is the bad call, but where does this come from?
16252 close(4 
16252 <... close resumed> ) = 0

16260 wait4(16259,  

16261 read(9,  

16259 sendmsg(5, {msg_name(0)=NULL, msg_iov(1)=[{"x", 1}],
msg_controllen=24, {cmsg_len=20, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_RIGHTS, {9}}, msg_flags=0}, 0) = 1

16259 exit_group(0) = ?

16261 <... read resumed> "", 512)   = 0

16260 <... wait4 resumed> [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0,
{ru_utime={0, 46875}, ru_stime={0, 26367}, ...}) = 16259

16261 dup(4)= -1 EBADF (Bad file descriptor)


Here is an approachj that might help.  Set a breakpoint on socketpair.
 The failure comes after the second call to socketpair, the one from
the function TestPassFD.  After that breakpoint is reached, set a
breakpoint on close.  Look for the call to close(4).  Get a backtrace
from there so we can see what is calling it.

Ian

[PATCH][RFC] Fix PR54824, deal with BBs with no successor

[Richard Biener]

This fixes PR54824, an ICE in loop structure verification, by
avoiding the situation of a BB with no successor and no control
statement (such as a noreturn call).  This situation leads
RTL expansions call to find_many_sub_basic_blocks to connect such
block to the "next" block, in this testcase forming a loop
which verification rightfully complains about.

My fix is to simply connect such blocks to themselves during
execute_fixup_cfg (the place we deal with suddenly appearing
noreturn attributes vs. vanishing noreturn properties which
is what we have here).  Another possibility would be to
insert __builtin_unreachable () calls, but I suppose instead
of falling through to a random BB at runtime endless looping
is nicer.  We could also insert __builtin_trap ().  As the
situation only appears when inlining such a function a place
to fix it up would be the inliner as well.

Comments?  Should find_many_sub_basic_blocks really do what
it does here?  Or is the CFG in fact "invalid" (but we don't
check that in verification)?

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Thanks,
Richard.

2012-10-24  Richard Biener  

PR middle-end/54824
* tree-optimize.c (execute_fixup_cfg): Connect blocks with
no successor to themselves.

* gcc.dg/torture/pr54824.c: New testcase.

Index: gcc/tree-optimize.c
===
*** gcc/tree-optimize.c (revision 192760)
--- gcc/tree-optimize.c (working copy)
*** execute_fixup_cfg (void)
*** 180,185 
--- 180,199 
FOR_EACH_EDGE (e, ei, bb->succs)
  e->count = (e->count * count_scale
+ REG_BR_PROB_BASE / 2) / REG_BR_PROB_BASE;
+ 
+   /* If we have a basic-block with no successors that does not
+end with a control statement or a noreturn call connect it
+to itself.  This situation can occur when inlining a noreturn
+call that does in fact return.  */
+   if (EDGE_COUNT (bb->succs) == 0)
+   {
+ gimple stmt = last_stmt (bb);
+ if (!stmt
+ || (!is_ctrl_stmt (stmt)
+ && (!is_gimple_call (stmt)
+ || (gimple_call_flags (stmt) & ECF_NORETURN) == 0)))
+   make_edge (bb, bb, EDGE_FALLTHRU);
+   }
  }
if (count_scale != REG_BR_PROB_BASE)
  compute_function_frequency ();
Index: gcc/testsuite/gcc.dg/torture/pr54824.c
===
*** gcc/testsuite/gcc.dg/torture/pr54824.c  (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr54824.c  (working copy)
***
*** 0 
--- 1,16 
+ /* { dg-do compile } */
+ /* { dg-options "-w" } */
+ 
+ void __attribute__((noreturn)) bar(void)
+ {
+ }
+ 
+ void foo(int i, char *p, char *q)
+ {
+   while (*p++) {
+   if (i)
+   p++;
+   if (!*q++)
+   bar();
+   }
+ }

[toplevel patch] Simplify FLAGS_FOR_TARGET for Cygwin

[Corinna Vinschen]

Hi guys,

I just applied the below patch to the sourceware src repo.  The reason
for the patch is that Cygwin won't be using the in-tree mingw and w32api
any longer, but instead it requires an external installation of a
Mingw64 based w32api, and a Mingw64 build environment to build the
native Windows utilities.  Additionally the FLAGS_FOR_TARGET contains
one dir which doesn't contain any libs (winsup) and one dir which doesn't
exist (winsup/include).  The below patch only changes FLAGS_FOR_TARGET
accordingly.

Could somebody with toplevel checkin rights in the gcc repo please apply
this patch there, too?


Thanks,
Corinna


2012-10-24  Corinna Vinschen  

* configure.ac (FLAGS_FOR_TARGET,target=cygwin): Fix for building
against Mingw64 w32api.
* configure: Regenerate.


Index: configure.ac
===
RCS file: /cvs/src/src/configure.ac,v
retrieving revision 1.176
diff -u -p -r1.176 configure.ac
--- configure.ac23 Oct 2012 23:02:33 -  1.176
+++ configure.ac24 Oct 2012 13:39:56 -
@@ -2827,7 +2827,7 @@ case " $target_configdirs " in
   *" --with-newlib "*)
case "$target" in
 *-cygwin*)
-  FLAGS_FOR_TARGET=$FLAGS_FOR_TARGET' -L$$r/$(TARGET_SUBDIR)/winsup 
-L$$r/$(TARGET_SUBDIR)/winsup/cygwin -L$$r/$(TARGET_SUBDIR)/winsup/w32api/lib 
-isystem $$s/winsup/include -isystem $$s/winsup/cygwin/include -isystem 
$$s/winsup/w32api/include'
+  FLAGS_FOR_TARGET=$FLAGS_FOR_TARGET' -L$$r/$(TARGET_SUBDIR)/winsup/cygwin 
-isystem $$s/winsup/cygwin/include'
   ;;
esac
 
Index: configure
===
RCS file: /cvs/src/src/configure,v
retrieving revision 1.432
diff -u -p -r1.432 configure
--- configure   23 Oct 2012 23:02:33 -  1.432
+++ configure   24 Oct 2012 13:39:55 -
@@ -7301,7 +7301,7 @@ case " $target_configdirs " in
   *" --with-newlib "*)
case "$target" in
 *-cygwin*)
-  FLAGS_FOR_TARGET=$FLAGS_FOR_TARGET' -L$$r/$(TARGET_SUBDIR)/winsup 
-L$$r/$(TARGET_SUBDIR)/winsup/cygwin -L$$r/$(TARGET_SUBDIR)/winsup/w32api/lib 
-isystem $$s/winsup/include -isystem $$s/winsup/cygwin/include -isystem 
$$s/winsup/w32api/include'
+  FLAGS_FOR_TARGET=$FLAGS_FOR_TARGET' -L$$r/$(TARGET_SUBDIR)/winsup/cygwin 
-isystem $$s/winsup/cygwin/include'
   ;;
esac
 

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Replace const_vector with match_operand in sse.md

[Uros Bizjak]

On Wed, Oct 24, 2012 at 3:01 PM, Andrey Turetskiy
 wrote:

> On Tue, Oct 23, 2012 at 2:45 PM, Andrey Turetskiy
>  wrote:
>> Hi,
>>
>> This patch replaces large const_vector constructions with
>> match_operand in sse.md to decrease its size.
>> Is it ok?

No, you don't have to touch generic expand machinery.

In the expander, use (match_dup X), and declare "operands[X] =
CONST1_RTX (..some_mode..)" in preparation statement. In unnamed
patterns, use const1_operand operand predicate. You should extend
existing const1_operand in the same way as const0_operand.

This approach is not compatible with named insn patterns, which
duplicate its functionality as pattern matcher and as an expander.

Uros.

Re: RFA: Add lock_lenth attribute to support the ARC port (Was: Re: Ping: RFA: add lock_length attribute to break branch-shortening cycles)

[Joern Rennecke]

Quoting Richard Biener :


Just to add some extra information, can you quote your ports
ADJUST_INSN_LENGTH and one example instruction with length/lock_length
attribute the above applies to?


This is a bit besides the point, since lock_length does mostly the traditional
branch shortening (with alignment effects rolled in for branches - I'm not
really happy about this, but that was required for convergence), length
does alignment effects on top, and ADJUST_INSN_LENGTH does random
context-sensitive adjustments related to pipeline quirks and conditional
execution.

I'm afraid this gcc port's logic in this area is both more complicated than
what you would expect in an average gcc port, and much more simplistic than
what a competent assembler programmer uses to optimize code for these
microarchitectures.

from arc.h:

#define ADJUST_INSN_LENGTH(X, LENGTH)   \
  (LENGTH) += arc_adjust_insn_length ((X), (LENGTH))

from arc.c:

/* Return length adjustment for INSN.  */
int
arc_adjust_insn_length (rtx insn, int len)
{
  int adj = 0;

  if (!INSN_P (insn))
return 0;
  if (GET_CODE (PATTERN (insn)) == SEQUENCE)
{
  int adj0, adj1, len1;
  rtx pat = PATTERN (insn);
  rtx i0 = XVECEXP (pat, 0, 0);
  rtx i1 = XVECEXP (pat, 0, 1);

  len1 = get_attr_lock_length (i1);
  gcc_assert (!len || len >= 4 || (len == 2 && get_attr_iscompact (i1)));
  if (!len1)
len1 = get_attr_iscompact (i1) != ISCOMPACT_FALSE ? 2 : 4;
  adj0 = arc_adjust_insn_length (i0, len - len1);
  adj1 = arc_adjust_insn_length (i1, len1);
  return adj0 + adj1;
}
  if (recog_memoized (insn) == CODE_FOR_doloop_end_i)
{
  rtx prev = prev_nonnote_insn (insn);

  return ((LABEL_P (prev)
   || (TARGET_ARC600
   && (JUMP_P (prev) || GET_CODE (PATTERN (prev)) ==  
SEQUENCE)))

  ? 4 : 0);
}

  /* Check for return with but one preceding insn since function
 start / call.  */
  if (TARGET_PAD_RETURN
  && JUMP_P (insn)
  && GET_CODE (PATTERN (insn)) != ADDR_VEC
  && GET_CODE (PATTERN (insn)) != ADDR_DIFF_VEC
  && get_attr_type (insn) == TYPE_RETURN)
{
  rtx prev = prev_active_insn (insn);

  if (!prev || !(prev = prev_active_insn (prev))
  || ((NONJUMP_INSN_P (prev)
   && GET_CODE (PATTERN (prev)) == SEQUENCE)
  ? CALL_ATTR (XVECEXP (PATTERN (prev), 0, 0), NON_SIBCALL)
  : CALL_ATTR (prev, NON_SIBCALL)))
return 4;
}
  /* Rtl changes too much before arc_reorg to keep ccfsm state.
 But we are not required to give exact answers then.  */
  if (cfun->machine->arc_reorg_started
  && (JUMP_P (insn) || (len & 2)))
{
  struct arc_ccfsm *statep = &cfun->machine->ccfsm_current;

  arc_ccfsm_advance_to (insn);
  switch (statep->state)
{
case 0:
  break;
case 1: case 2:
  /* Deleted branch.  */
  return -len;
case 3: case 4: case 5:
  /* Conditionalized insn.  */
  if ((!JUMP_P (insn)
   || (get_attr_type (insn) != TYPE_BRANCH
   && get_attr_type (insn) != TYPE_UNCOND_BRANCH
   && (get_attr_type (insn) != TYPE_RETURN
   || (statep->cc != ARC_CC_EQ && statep->cc != ARC_CC_NE)
   || NEXT_INSN (PREV_INSN (insn)) != insn)))
  && (len & 2))
adj = 2;
  break;
default:
  gcc_unreachable ();
}
}
  if (TARGET_ARC600)
{
  rtx succ = next_real_insn (insn);

  if (succ && INSN_P (succ))
adj += arc600_corereg_hazard (insn, succ);
}

  /* Restore extracted operands - otherwise splitters like the  
addsi3_mixed one

 can go awry.  */
  extract_constrain_insn_cached (insn);

  return adj;
}

From arc.md:

; Since the demise of REG_N_SETS, it is no longer possible to find out
; in the prologue / epilogue expanders how many times blink is set.
; Using df_regs_ever_live_p to decide if blink needs saving means that
; any explicit use of blink will cause it to be saved; hence we cannot
; represent the blink use in return / sibcall instructions themselves, and
; instead have to show it in EPILOGUE_USES and must explicitly
; forbid instructions that change blink in the return / sibcall delay slot.
(define_insn "return_i"
  [(return)]
  "reload_completed"
{
  rtx reg
= gen_rtx_REG (Pmode,
   arc_return_address_regs[arc_compute_function_type (cfun)]);

  if (TARGET_PAD_RETURN)
arc_pad_return ();
  output_asm_insn (\"j%!%* [%0]%&\", ®);
  return \"\";
}
  [(set_attr "type" "return")
   (set_attr "cond" "canuse")
   (set (attr "iscompact")
(cond [(eq (symbol_ref "arc_compute_function_type (cfun)")
   (symbol_ref "ARC_FUNCTION_NORMAL"))
   (const_string "maybe")]
  (const_string "false")))
   (set (attr "length")
(cond [(eq (symbol_ref "arc

Re: libgo patch committed: Update to current Go library

[Uros Bizjak]

On Wed, Oct 24, 2012 at 3:10 PM, Ian Lance Taylor  wrote:
> On Wed, Oct 24, 2012 at 5:31 AM, Uros Bizjak  wrote:
>> On Wed, Oct 24, 2012 at 2:18 PM, Andreas Schwab  
>> wrote:
>>> Uros Bizjak  writes:
>>>
 To answer my own question:

 dup(4)  = 9
 ...
 close(9)= 0
 dup(4)  = -1 EBADF (Bad file descriptor)

 Test is calling dup on a closed file descriptor.
>>>
>>> FD 4 is most likely closed by one of the cloned threads.  Use strace -f
>>> to follow them.
>>
>> Yes, indeed! Attached strace -f record confirms this on alpha.
>>
>> The same happens on x86_64, but for some reason x86_64 doesn't
>> complain when executing dup(2) on closed FD.
>
> The test execs itself by calling the fork and execve functions in
> libc.  It is the child process that closes the FD after it forks.
> From the point of view of the parent process, the FD should still be
> open.  I don't think you attached the strace -f output so I can't
> confirm this.

Eh, sorry, attached now.

After the second socketpair and before dup, there is a close in the same thread.

Uros.


strace.txt.gz
Description: GNU Zip compressed data

Re: libgo patch committed: Update to current Go library

[Ian Lance Taylor]

On Wed, Oct 24, 2012 at 5:31 AM, Uros Bizjak  wrote:
> On Wed, Oct 24, 2012 at 2:18 PM, Andreas Schwab  wrote:
>> Uros Bizjak  writes:
>>
>>> To answer my own question:
>>>
>>> dup(4)  = 9
>>> ...
>>> close(9)= 0
>>> dup(4)  = -1 EBADF (Bad file descriptor)
>>>
>>> Test is calling dup on a closed file descriptor.
>>
>> FD 4 is most likely closed by one of the cloned threads.  Use strace -f
>> to follow them.
>
> Yes, indeed! Attached strace -f record confirms this on alpha.
>
> The same happens on x86_64, but for some reason x86_64 doesn't
> complain when executing dup(2) on closed FD.

The test execs itself by calling the fork and execve functions in
libc.  It is the child process that closes the FD after it forks.
>From the point of view of the parent process, the FD should still be
open.  I don't think you attached the strace -f output so I can't
confirm this.

Ian

Re: [PATCH] Replace const_vector with match_operand in sse.md

[Jakub Jelinek]

On Wed, Oct 24, 2012 at 05:01:15PM +0400, Andrey Turetskiy wrote:
> > This patch replaces large const_vector constructions with
> > match_operand in sse.md to decrease its size.
> > Is it ok?

The *intrin.h changes look all wrong to me, why should one pass a dummy
uninitialized argument to the builtins?

Jakub

Re: [PATCH] Replace const_vector with match_operand in sse.md

[Andrey Turetskiy]

Add Uros to Cc.

On Tue, Oct 23, 2012 at 2:45 PM, Andrey Turetskiy
 wrote:
> Hi,
>
> This patch replaces large const_vector constructions with
> match_operand in sse.md to decrease its size.
> Is it ok?
>
> Changelog:
>
> 2012-10-23  Andrey Turetskiy  
>
>* config/i386/avx2intrin.h (_mm256_avg_epu8): Add new operand.
>(_mm256_avg_epu16): Ditto.
>(_mm256_mulhrs_epi16): Ditto.
>* config/i386/emmintrin.h (_mm_avg_epu8): Ditto.
>(_mm_avg_epu16): Ditto.
>* config/i386/tmmintrin.h (_mm_mulhrs_epi16): Ditto.
>(_mm_mulhrs_pi16): Ditto.
>* config/i386/i386-builtin-types.def
>DEF_FUNCTION_TYPE (V4HI, V4HI, V4HI, V4HI): New type.
>* config/i386/i386.c (bdesc_args): Extend builtins types.
>(ix86_expand_args_builtin): Add support for new types and allow expander
>to generate const_vector filled with all ones.
>* config/i386/predicates.md (vector1_operand): New predicate which match
>const_vector filled with all ones.
>* config/i386/sse.md (sse2_uavgv16qi3): Replace const_vector with
>vector1_operand.
>(sse2_uavgv8hi3): Ditto.
>(ssse3_pmulhrswv8hi3): Ditto.
>(ssse3_pmulhrswv4hi3): Ditto.
>(avx2_uavgv32qi3): Ditto.
>(avx2_uavgv16hi3): Ditto.
>(avx2_umulhrswv16hi3): Ditto.
>
> ---
> Best regards,
> Andrey Turetskiy

Re: libgo patch committed: Update to current Go library

[Uros Bizjak]

On Wed, Oct 24, 2012 at 2:18 PM, Andreas Schwab  wrote:
> Uros Bizjak  writes:
>
>> To answer my own question:
>>
>> dup(4)  = 9
>> ...
>> close(9)= 0
>> dup(4)  = -1 EBADF (Bad file descriptor)
>>
>> Test is calling dup on a closed file descriptor.
>
> FD 4 is most likely closed by one of the cloned threads.  Use strace -f
> to follow them.

Yes, indeed! Attached strace -f record confirms this on alpha.

The same happens on x86_64, but for some reason x86_64 doesn't
complain when executing dup(2) on closed FD.

Uros.

[v3] libstdc++/55047

[Paolo Carlini]

Hi

tested x86_64-linux multilib, committed to mainline. A similar fix will 
go in 4_7-branch too.


Thanks,
Paolo.


2012-10-24   Haakan Younes  
 Paolo Carlini  

PR libstdc++/55047
* include/bits/random.h (exponential_distribution<>::operator):
Fix formula to std::log(result_type(1) - __aurng()).
* include/bits/random.tcc: Likewise, everywhere.
Index: include/bits/random.tcc
===
--- include/bits/random.tcc (revision 192718)
+++ include/bits/random.tcc (working copy)
@@ -1,6 +1,6 @@
 // random number generation (out of line) -*- C++ -*-
 
-// Copyright (C) 2009, 2010, 2011, 2012 Free Software Foundation, Inc.
+// Copyright (C) 2009-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
@@ -1187,7 +1187,7 @@
 
double __cand;
do
- __cand = std::floor(std::log(__aurng()) / __param._M_log_1_p);
+ __cand = std::floor(std::log(1.0 - __aurng()) / __param._M_log_1_p);
while (__cand >= __thr);
 
return result_type(__cand + __naf);
@@ -1217,7 +1217,8 @@
  {
double __cand;
do
- __cand = std::floor(std::log(__aurng()) / __param._M_log_1_p);
+ __cand = std::floor(std::log(1.0 - __aurng())
+ / __param._M_log_1_p);
while (__cand >= __thr);
 
*__f++ = __cand + __naf;
@@ -1464,7 +1465,7 @@
do
  {
const double __u = __c * __aurng();
-   const double __e = -std::log(__aurng());
+   const double __e = -std::log(1.0 - __aurng());
 
double __w = 0.0;
 
@@ -1496,7 +1497,7 @@
  __x = 1;
else
  {
-   const double __v = -std::log(__aurng());
+   const double __v = -std::log(1.0 - __aurng());
const double __y = __param._M_d
 + __v * __2cx / __param._M_d;
__x = std::ceil(__y);
@@ -1655,7 +1656,7 @@
 
do
  {
-   const double __e = -std::log(__aurng());
+   const double __e = -std::log(1.0 - __aurng());
__sum += __e / (__t - __x);
__x += 1;
  }
@@ -1723,7 +1724,7 @@
__reject = __y >= __param._M_d1;
if (!__reject)
  {
-   const double __e = -std::log(__aurng());
+   const double __e = -std::log(1.0 - __aurng());
__x = std::floor(__y);
__v = -__e - __n * __n / 2 + __param._M_c;
  }
@@ -1735,15 +1736,15 @@
__reject = __y >= __param._M_d2;
if (!__reject)
  {
-   const double __e = -std::log(__aurng());
+   const double __e = -std::log(1.0 - __aurng());
__x = std::floor(-__y);
__v = -__e - __n * __n / 2;
  }
  }
else if (__u <= __a123)
  {
-   const double __e1 = -std::log(__aurng());
-   const double __e2 = -std::log(__aurng());
+   const double __e1 = -std::log(1.0 - __aurng());
+   const double __e2 = -std::log(1.0 - __aurng());
 
const double __y = __param._M_d1
 + 2 * __s1s * __e1 / __param._M_d1;
@@ -1754,8 +1755,8 @@
  }
else
  {
-   const double __e1 = -std::log(__aurng());
-   const double __e2 = -std::log(__aurng());
+   const double __e1 = -std::log(1.0 - __aurng());
+   const double __e2 = -std::log(1.0 - __aurng());
 
const double __y = __param._M_d2
 + 2 * __s2s * __e1 / __param._M_d2;
@@ -1869,7 +1870,7 @@
__detail::_Adaptor<_UniformRandomNumberGenerator, result_type>
  __aurng(__urng);
while (__f != __t)
- *__f++ = -std::log(__aurng()) / __p.lambda();
+ *__f++ = -std::log(result_type(1) - __aurng()) / __p.lambda();
   }
 
   template
@@ -2628,7 +2629,7 @@
   {
__detail::_Adaptor<_UniformRandomNumberGenerator, result_type>
  __aurng(__urng);
-   return __p.b() * std::pow(-std::log(__aurng()),
+   return __p.b() * std::pow(-std::log(result_type(1) - __aurng()),
  result_type(1) / __p.a());
   }
 
@@ -2644,10 +2645,11 @@
__glibcxx_function_requires(_ForwardIteratorConcept<_ForwardIterator>)
__detail::_Adaptor<_UniformRandomNumberGenerat

Re: libgo patch committed: Update to current Go library

[Andreas Schwab]

Uros Bizjak  writes:

> To answer my own question:
>
> dup(4)  = 9
> ...
> close(9)= 0
> dup(4)  = -1 EBADF (Bad file descriptor)
>
> Test is calling dup on a closed file descriptor.

FD 4 is most likely closed by one of the cloned threads.  Use strace -f
to follow them.

Andreas.

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

Re: [PATCH] Intrinsics for fxsave[,64], xsave[,64], xsaveopt[,64]

[Uros Bizjak]

On Wed, Oct 24, 2012 at 1:06 PM, Uros Bizjak  wrote:
>>> Please take a look at the attached patch.
>>>
>>> I changed the asm-pattern implementation according to your recomendation.
>>> Changed the name of feature option from -mfxsave to -mfxsr, as it is in
>>> Intel SDM. Corrected the arguments name in the headers.
>>>
>>> Bootstrap passes
>>
>> I think you should add bit_FXSR.  Also you should turn off XSAVE if OSXSAVE 
>> is
>> off, according to Intel SDM.
>
> There is no need for additional bit_FXSR. We already have plenty of
> "misnamed" bits in cpuid.h and we can live with them just fine.
>
>> I think we should check OSFXSR bit for OS SSE support in driver-i386.c before
>> enabling SSE and FXSR, similar to OSXSAVE bit.  If we do that, we should 
>> enable
>> FXSR when enabling SSE.
>
> This was already discussed, and the conclusion was that SSE and FXSR
> are orhogonal, so we don't need to tie them together.

Oh, I see. Actually, we have to _disable_ SSE related features in
driver-i386 when FXSR cpuid bit is not set. But I think this is not
necessary, since probably all processors with SSE bit set also have
FXSR bit set. Opposite is not necessary true.

Uros.

Re: [PATCH] Intrinsics for fxsave[,64], xsave[,64], xsaveopt[,64]

[Uros Bizjak]

On Wed, Oct 24, 2012 at 12:52 PM, H.J. Lu  wrote:
>> Please take a look at the attached patch.
>>
>> I changed the asm-pattern implementation according to your recomendation.
>> Changed the name of feature option from -mfxsave to -mfxsr, as it is in
>> Intel SDM. Corrected the arguments name in the headers.
>>
>> Bootstrap passes
>
> I think you should add bit_FXSR.  Also you should turn off XSAVE if OSXSAVE is
> off, according to Intel SDM.

There is no need for additional bit_FXSR. We already have plenty of
"misnamed" bits in cpuid.h and we can live with them just fine.

> I think we should check OSFXSR bit for OS SSE support in driver-i386.c before
> enabling SSE and FXSR, similar to OSXSAVE bit.  If we do that, we should 
> enable
> FXSR when enabling SSE.

This was already discussed, and the conclusion was that SSE and FXSR
are orhogonal, so we don't need to tie them together.

Uros.

Re: [PATCH] Intrinsics for fxsave[,64], xsave[,64], xsaveopt[,64]

[H.J. Lu]

On Tue, Oct 23, 2012 at 3:14 AM, Alexander Ivchenko  wrote:
> Please take a look at the attached patch.
>
> I changed the asm-pattern implementation according to your recomendation.
> Changed the name of feature option from -mfxsave to -mfxsr, as it is in
> Intel SDM. Corrected the arguments name in the headers.
>
> Bootstrap passes

I think you should add bit_FXSR.  Also you should turn off XSAVE if OSXSAVE is
off, according to Intel SDM.

I think we should check OSFXSR bit for OS SSE support in driver-i386.c before
enabling SSE and FXSR, similar to OSXSAVE bit.  If we do that, we should enable
FXSR when enabling SSE.


-- 
H.J.

Re: RFA: Add lock_lenth attribute to support the ARC port (Was: Re: Ping: RFA: add lock_length attribute to break branch-shortening cycles)

[Richard Biener]

On Wed, Oct 24, 2012 at 3:42 AM, Joern Rennecke
 wrote:
> Quoting Richard Biener :
>
>> On Tue, Oct 16, 2012 at 9:35 PM, Joern Rennecke
>>  wrote:
>
> ..
>>>
>>> Well, we could split it anyway, and give ports without the need for
>>> multiple length attributes the benefit of the optimistic algorithm.
>>>
>>> I have attached a patch that implements this.
>>
>>
>> Looks reasonable to me, though I'm not familiar enough with the code
>> to approve it.
>
>
> Now that Richard Sandiford has reviewed that split-off part and it's in
> the source tree, we can return to the remaining functionality needed
> by for the ARC port.
>
>> I'd strongly suggest to try harder to make things work for you without
>> the new attribute even though I wasn't really able to follow your
>> reasoning
>> on why that wouldn't work.  It may be easier to motivate this change
>> once the port is in without that attribute so one can actually look at
>> the machine description and port details.
>
>
> Well, it doesn't simply drop in with the existing branch shortening -
> libgcc won't compile because of out-of-range branches.
> I tried to lump the length and lock_length atribute together, and that
> just gives genattrtab indigestion.  It sits there looping forever.
> I could start debugging this, but that would take an unknown amount of
> time, and then the review of the fix would take an unknown amount of time,
> and then the ARC port probably needs fixing up again because it just
> doesn't work right with these fudged lengths.  And even if we could get
> everything required in before the close of phase 1, the branch shortening
> would be substandard.
> It seems more productive to get the branch shortening working now.
> The lock_length atrtibute is completely optional, so no port maintainer
> would be forced to use the functionality if it's not desired.
>
> The issue is that the some instructions need to be aligned or unaligned
> for performance or in a few cases even for correctness.  Just inserting
> random nops would be a waste of code space and cycles, since most of the
> time, the desired (mis)alignment can be archived by selectively making
> a short instruction long.  If an instruction that is long once was forced
> to stay long henceforth, that would actually defeat the purpose of getting
> the desired alignment.  Then another short instruction - if it can be found
> -
> would need to be upsized.  So a size increase could ripple through as
> alignments are distorted.  The natural thing to do is really when the
> alignemnt changes is really to let the upsized instruction be short again.
> Only length-determined branch sizes have to be locked to avoid cycles.

Just to add some extra information, can you quote your ports
ADJUST_INSN_LENGTH and one example instruction with length/lock_length
attribute the above applies to?

> This is the documentation for the new role of the lock_length attribute
> (reduced from my previous attempt):
>
> @cindex lock_length
> Usually, when doing optimizing branch shortening, the instruction length
> is calculated by avaluating the @code{length} attribute, applying
> @code{ADJUST_INSN_LENGTH}, and taking the maximum of the resultant
> value and the length of the instruction in the previous iteration.

Which sounds straight-forward.  The docs of ADJUST_INSN_LENGTH
are not entirely clear, but it sounds like it may only increase length, correct?
I see that ADJUST_INSN_LENGTH is really not needed as everything
should be expressable in the length attribute of an instruction?

> If you define the @code{lock_length} attribute, the @code{lock_length}
> attribute will be evaluated, and then the maximum with of @code{lock_length}

with of?  I read it as 'of the'

> value from the previous iteration will be formed and saved.

So lock_length will only increase during iteration.

> Then the maximum of that value with the @code{length} attribute will
> be formed, and @code{ADJUST_INSN_LENGTH} will be applied.

ADJUST_INSN_LENGTH will be applied to the maximum?  What will
be the 'instruction length' equivalent to the simple non-lock_length case?
Is it the above, max (ADJUST_INSN_LENGTH (lock-length-max, length))?

> Thus, you can allow the length to vary downwards as well as upwards
> across iterations with suitable definitions of the @code{length} attribute
> and/or @code{ADJUST_INSN_LENGTH}.  Care has to be taken that this does not
> lead to infinite loops.

I don't see that you can shrink length with just suitable lock_length and
length attributes.  What seems to be the cruical difference is that you
apply ADJUST_INSN_LENGTH after combining lock-length-max and length.
But then you _decrease_ length with ADJUST_INSN_LENGHT ...

Maybe what you really want is ADJUST_INSN_LENGTH_AFTER which is
applied afterwards?  Thus,

> Usually, when doing optimizing branch shortening, the instruction length
> is calculated by avaluating the @code{length} attribute, applying
> @code{ADJUST_INSN_LENGTH}, and taking the maximum of the re

Re: [PATCH] GCC 4.7 and 4.8 PowerPC RTEMS

[Sebastian Huber]

On 03/09/2012 10:01 AM, Sebastian Huber wrote:

Hi,

please have a look at the attached patch.  Test suite results for GCC 4.7

http://gcc.gnu.org/ml/gcc-testresults/2012-03/msg00986.html



This patch is still pending for GCC 4.7 and 4.8.  Can someone please review and 
commit it.  Without this patch the RTEMS PowerPC support is broken.  Thanks.


--
Sebastian Huber, embedded brains GmbH

Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany
Phone   : +49 89 18 90 80 79-6
Fax : +49 89 18 90 80 79-9
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.


2012-03-08  Sebastian Huber  

* config.host (powerpc-*-rtems*): Add rs6000/t-savresfgpr to
tmake_file.

diff --git a/libgcc/config.host b/libgcc/config.host
index 257622a..f6432c5 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -884,7 +884,7 @@ powerpc-*-eabi*)
 	extra_parts="$extra_parts crtbegin.o crtend.o crtbeginS.o crtendS.o crtbeginT.o ecrti.o ecrtn.o ncrti.o ncrtn.o"
 	;;
 powerpc-*-rtems*)
-	tmake_file="${tmake_file} rs6000/t-ppccomm rs6000/t-crtstuff t-crtstuff-pic t-fdpbit"
+	tmake_file="${tmake_file} rs6000/t-ppccomm rs6000/t-savresfgpr rs6000/t-crtstuff t-crtstuff-pic t-fdpbit"
 	extra_parts="$extra_parts crtbeginS.o crtendS.o crtbeginT.o ecrti.o ecrtn.o ncrti.o ncrtn.o"
 	;;
 powerpc-*-linux* | powerpc64-*-linux*)

Re: LRA has been merged into trunk.

[H.J. Lu]

On Tue, Oct 23, 2012 at 8:46 AM, Vladimir Makarov  wrote:
>   Hi, I was going to merge LRA into trunk last Sunday.  It did not happen.
> LRA was actively changed last 4 weeks by implementing reviewer's proposals
> which resulted in a lot of new LRA regressions on GCC testsuite in
> comparison with reload.  Finally, they were fixed and everything looks ok to
> me.
>
>   So I've committed the patch into trunk as rev. 192719. The final patch is
> in the attachment.
>

It caused:

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

-- 
H.J.

Re: LRA has been merged into trunk.

[Ramana Radhakrishnan]

On 10/23/12 16:46, Vladimir Makarov wrote:

Hi, I was going to merge LRA into trunk last Sunday.  It did not
happen.  LRA was actively changed last 4 weeks by implementing
reviewer's proposals which resulted in a lot of new LRA regressions on
GCC testsuite in comparison with reload.  Finally, they were fixed and
everything looks ok to me.

So I've committed the patch into trunk as rev. 192719. The final
patch is in the attachment.



This prima-facie caused PR55050 on arm-linux-gnueabi . CC'd you on the 
bug report.



regards,
Ramana

Re: patch to fix constant math - 4th patch - the wide-int class.

[Richard Biener]

On Tue, Oct 23, 2012 at 6:12 PM, Kenneth Zadeck
 wrote:
>
> On 10/23/2012 10:12 AM, Richard Biener wrote:
>>
>> On Tue, Oct 9, 2012 at 5:09 PM, Kenneth Zadeck 
>> wrote:
>>>
>>> This patch implements the wide-int class.this is a more general
>>> version
>>> of the double-int class and is meant to be the eventual replacement for
>>> that
>>> class.The use of this class removes all dependencies of the host from
>>> the target compiler's integer math.
>>>
>>> I have made all of the changes i agreed to in the earlier emails. In
>>> particular, this class internally maintains a bitsize and precision but
>>> not
>>> a mode. The class now is neutral about modes and tree-types.the
>>> functions that take modes or tree-types are just convenience functions
>>> that
>>> translate the parameters into bitsize and precision and where ever there
>>> is
>>> a call that takes a mode, there is a corresponding call that takes a
>>> tree-type.
>>>
>>> All of the little changes that richi suggested have also been made.
>>>
>>> The buffer sizes is now twice the size needed by the largest integer
>>> mode.
>>> This gives enough room for tree-vrp to do full multiplies on any type
>>> that
>>> the target supports.
>>>
>>> Tested on x86-64.
>>>
>>> This patch depends on the first three patches.   I am still waiting on
>>> final
>>> approval on the hwint.h patch.
>>>
>>> Ok to commit?
>>
>> diff --git a/gcc/wide-int.h b/gcc/wide-int.h
>> new file mode 100644
>> index 000..efd2c01
>> --- /dev/null
>> +++ b/gcc/wide-int.h
>> ...
>> +#ifndef GENERATOR_FILE
>
>
>> The whole file is guarded with that ... why?  That is bound to be fragile
>> once
>> use of wide-int spreads?  How do generator programs end up including
>> this file if they don't need it at all?
>
> This is so that wide-int can be included at the level of the generators.
> There some stuff that needs to see this type that is done during the build
> build phase that cannot see the types that are included in wide-int.h.
>
>> +#include "tree.h"
>> +#include "hwint.h"
>> +#include "options.h"
>> +#include "tm.h"
>> +#include "insn-modes.h"
>> +#include "machmode.h"
>> +#include "double-int.h"
>> +#include 
>> +#include "insn-modes.h"
>> +
>>
>> That's a lot of tree and rtl dependencies.  double-int.h avoids these by
>> placing conversion routines in different headers or by only resorting to
>> types in coretypes.h.  Please try to reduce the above to a minimum.
>>
>> +  HOST_WIDE_INT val[2 * MAX_BITSIZE_MODE_ANY_INT /
>> HOST_BITS_PER_WIDE_INT];
>>
>> are we sure this rounds properly?  Consider a port with max byte mode
>> size 4 on a 64bit host.
>
> I do not believe that this can happen.   The core compiler includes all
> modes up to TI mode, so by default we already up to 128 bits.

And mode bitsizes are always power-of-two?  I suppose so.

>> I still would like to have the ability to provide specializations of
>> wide_int
>> for "small" sizes, thus ideally wide_int would be a template templated
>> on the number of HWIs in val.  Interface-wise wide_int<2> should be
>> identical to double_int, thus we should be able to do
>>
>> typedef wide_int<2> double_int;
>
> If you want to go down this path after the patches get in, go for it.I
> see no use at all for this.
> This was not meant to be a plug in replacement for double int. This goal of
> this patch is to get the compiler to do the constant math the way that the
> target does it.   Any such instantiation is by definition placing some
> predefined limit that some target may not want.

Well, what I don't really like is that we now have two implementations
of functions that perform integer math on two-HWI sized integers.  What
I also don't like too much is that we have two different interfaces to operate
on them!  Can't you see how I come to not liking this?  Especially the
latter ...

>> in double-int.h and replace its implementation with a specialization of
>> wide_int.  Due to a number of divergences (double_int is not a subset
>> of wide_int) that doesn't seem easily possible (one reason is the
>> ShiftOp and related enums you use).  Of course wide_int is not a
>> template either.  For the hypotetical embedded target above we'd
>> end up using wide_int<1>, a even more trivial specialization.
>>
>> I realize again this wide-int is not what your wide-int is (because you
>> add a precision member).  Still factoring out the "common"s of
>> wide-int and double-int into a wide_int_raw <> template should be
>> possible.
>>
>> +class wide_int {
>> +  /* Internal representation.  */
>> +
>> +  /* VAL is set to a size that is capable of computing a full
>> + multiplication on the largest mode that is represented on the
>> + target.  The full multiplication is use by tree-vrp.  If
>> + operations are added that require larger buffers, then VAL needs
>> + to be changed.  */
>> +  HOST_WIDE_INT val[2 * MAX_BITSIZE_MODE_ANY_INT /
>> HOST_BITS_PER_WIDE_INT];
>> +  unsigned short len;
>> +  unsigned in

Re: LRA has been merged into trunk.

[Richard Sandiford]

Jakub Jelinek  writes:
> On Wed, Oct 24, 2012 at 10:17:48AM +0100, Richard Sandiford wrote:
>> > Sparc accepts addresses of the form:
>> >
>> > (plus:DI (lo_sum:DI (reg/f:DI 282)
>> > (symbol_ref:DI ("__mf_opts") ))
>> > (const_int 40 [0x28]))
>> >
>> > These make use of Sparc's offsetable %lo() relocations.
>> 
>> Hmm, this looks a bit risky.  In terms of RTL semantics, this
>> (plus:DI ...) is a full 64-bit addition of the result of the
>> (lo_sum:DI ...), whatever that (lo_sum:DI ...) result happens to be.
>> I assume the offset is really folded into the %lo() constant itself,
>> in which case the usual form would be:
>> 
>> (lo_sum:DI (reg/f:DI 282)
>>(const:DI (plus:DI (symbol_ref:DI ...) (const_int 40
>
> R_SPARC_OLO10 relocation has two addends though, please see
> http://sourceware.org/ml/binutils/1999-q3/msg00099.html
> so the plus there is right.  The constant in second operand of PLUS
> where first operand is LO_SUM needs to be RTX_OK_FOR_OLO10_P, so that
> there is no overflow on the lo_sum operand & 0x3ff plus the
> RTX_OK_FOR_OLO10_P offset (it needs to fit into signed 13 bit field).

OK, that blasts that argument out of the water then. :-)

Richard

[wwwdocs] changes.html update

[Jan Hubicka]

Hi,
few things I think are worth to be mentioned in changes.html.
Index: changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.8/changes.html,v
retrieving revision 1.48
diff -c -3 -p -r1.48 changes.html
*** changes.html21 Oct 2012 11:01:37 -  1.48
--- changes.html24 Oct 2012 09:29:49 -
*** by this change.
*** 76,81 
--- 76,103 
   "Eigen" C++ linear algebra templates library, is significantly
   faster than previous releases of GCC.
  
+ Link-time optimization (LTO) improvements:
+ 
+  LTO partitioning has been rewritten for better reliability
+and maintanibility. Several important bugs leading to link
+failures has been fixed.
+ 
+ Interprocedural optimization improvements:
+ 
+  New symbol table has been implemented. The symbol table
+builds on earlier callgraph and varpool modules and 
+provide a new API. Unusual symbol visibilities and aliases
+are handled more consistently leading to, for example,
+more aggrssive unreachable code removal with LTO.
+  The inline heuristic can now bypass limits on size of
+inlined function when the inlining is particularly profitable.
+This happens, for example, when loop bounds or array strides
+gets propagated.
+  Values passed through aggregates (either by value or refernece)
+are now propagated at inter-procedural level leading to better
+inlining decisions (for exmaple in the case of Fortran
+array descriptors) and devirtualization.
+ 

  
  

Re: LRA has been merged into trunk.

[Jakub Jelinek]

On Wed, Oct 24, 2012 at 10:17:48AM +0100, Richard Sandiford wrote:
> > Sparc accepts addresses of the form:
> >
> > (plus:DI (lo_sum:DI (reg/f:DI 282)
> > (symbol_ref:DI ("__mf_opts") ))
> > (const_int 40 [0x28]))
> >
> > These make use of Sparc's offsetable %lo() relocations.
> 
> Hmm, this looks a bit risky.  In terms of RTL semantics, this
> (plus:DI ...) is a full 64-bit addition of the result of the
> (lo_sum:DI ...), whatever that (lo_sum:DI ...) result happens to be.
> I assume the offset is really folded into the %lo() constant itself,
> in which case the usual form would be:
> 
> (lo_sum:DI (reg/f:DI 282)
>(const:DI (plus:DI (symbol_ref:DI ...) (const_int 40

R_SPARC_OLO10 relocation has two addends though, please see
http://sourceware.org/ml/binutils/1999-q3/msg00099.html
so the plus there is right.  The constant in second operand of PLUS
where first operand is LO_SUM needs to be RTX_OK_FOR_OLO10_P, so that
there is no overflow on the lo_sum operand & 0x3ff plus the
RTX_OK_FOR_OLO10_P offset (it needs to fit into signed 13 bit field).

Jakub