Re: [PATCH v2] Test case for PR55033

[Sebastian Huber]

On 05/17/2013 01:27 AM, David Edelsohn wrote:

On Fri, May 10, 2013 at 11:43 PM, Chung-Ju Wu  wrote:

2013/5/10 Sebastian Huber :
v2: Format changes

gcc/testsuite/ChangeLog
2013-05-10  Sebastian Huber  

 PR target/55033
 * gcc.target/powerpc/pr55033.c: New.


The testcase is okay.

Do you have CVS write access or do you need someone to commit the patch for you?

Thanks, David



Sorry, I missed your reply.  I have no write access, so please commit it for me.

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

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

Re: Improve uncprop and coalescing

[Steven Bosscher]

On Fri, Jun 7, 2013 at 8:07 AM, Jeff Law wrote:
> Rather than using strict pointer equality, we can do better by looking at
> TYPE_CANONICAL when it's available.  Thus objects of the following two types
> (T1 & T2) become candidates for coalescing if they are tied together by a
> copy or PHI node.
>
> typedef int t1;
> typedef int t2;
>
>
> This typically eliminates necessary copies and constant initializations,
> which is good.

Hmm...  Can't you use types_compatible_p?

Ciao!
Steven

RE: [PATCH][ARM][5/n] Partial IT block deprecation in ARMv8 AArch32 - load/store multiple

[Kyrylo Tkachov]

> On 06/06/13 17:26, Richard Henderson wrote:
> > On 06/06/2013 08:02 AM, Richard Earnshaw wrote:
> >>   (define_insn "add3"
> >> -  [(set (match_operand:FIXED 0 "s_register_operand" "=r")
> >> -(plus:FIXED (match_operand:FIXED 1 "s_register_operand"
> "r")
> >> -(match_operand:FIXED 2 "s_register_operand" "r")))]
> >> +  [(set (match_operand:FIXED 0 "s_register_operand" "=r,l")
> >> +(plus:FIXED (match_operand:FIXED 1 "s_register_operand"
> "r,l")
> >> +(match_operand:FIXED 2 "s_register_operand"
> "r,l")))]
> >>
> >> It would probably be better to put the 'l' variant first. This
> should encourage
> >> register allocation to prefer low registers and that might lead
> to other
> >> optimizations later on.  Similarly for sub3.
> >
> > It's also 100% required in order to make the l alternative ever
> chosen.
> >
> > When we compute which_alternative post-reload, we'll see that r
> matches
> > and always choose alternative 0.  If you've been examining asm
> dumps of
> > various test cases, in addition to your bootstrapping, you'll
> have seen
> > no IT predicated addition insns after this patch.
> >
> >
> > r~
> >
> 
> Not quite.  In the cond-exec case the first alternative can be
> disabled,
> then only those cases that match the second set of constraints
> could be
> conditionalized.

In any case, I've committed the patch with the 'l' constraint going
before 'r' as r199739.
Thanks for the review.

Kyrill

Re: force_const_mem VOIDmode

[Richard Biener]

On Fri, Jun 7, 2013 at 4:00 AM, Alan Modra  wrote:
> force_const_mem() isn't supposed to handle VOIDmode or BLKmode, so the
> check for VOIDmode when aligning is needless.  If we ever did get one
> of these modes in a constant pool, this
>
>   pool->offset += GET_MODE_SIZE (mode);
>
> won't add to the pool size, and output_constant_pool_2() will hit a
> gcc_unreachable().  Bootstrapped etc. powerpc64-linux.

Ok.

Thanks,
Richard.

> * varasm.c (force_const_mem): Assert mode is not VOID or BLK.
>
> Index: gcc/varasm.c
> ===
> --- gcc/varasm.c(revision 199718)
> +++ gcc/varasm.c(working copy)
> @@ -3567,7 +3575,8 @@
>*slot = desc;
>
>/* Align the location counter as required by EXP's data type.  */
> -  align = GET_MODE_ALIGNMENT (mode == VOIDmode ? word_mode : mode);
> +  gcc_checking_assert (mode != VOIDmode && mode != BLKmode);
> +  align = GET_MODE_ALIGNMENT (mode);
>  #ifdef CONSTANT_ALIGNMENT
>{
>  tree type = lang_hooks.types.type_for_mode (mode, 0);
>
> --
> Alan Modra
> Australia Development Lab, IBM

Re: [GOOGLE] More strict checking for call args

[Richard Biener]

On Thu, Jun 6, 2013 at 5:10 PM, Dehao Chen  wrote:
> Hi, Martin,
>
> Yes, your patch can fix my case. Thanks a lot for the fix.
>
> With the fix, value profiling will still promote the wrong indirect
> call target. Though it will not be inlining, but it results in an
> additional check. How about in check_ic_target, after calling
> gimple_check_call_matching_types, we also check if number of args
> match number of params in target->symbol.decl?

I wonder what's the point in the gimple_check_call_matching_types check
in the profiling case.  It's at least no longer

/* Perform sanity check on the indirect call target. Due to race conditions,
   false function target may be attributed to an indirect call site. If the
   call expression type mismatches with the target function's type, expand_call
   may ICE.

because since the introduction of gimple_call_fntype we will _not_ ICE.

Thus I argue that check_ic_target should be even removed instead of
enhancing it!

How does IC profiling determine the called target?  That is, what does it
do when the target is not always the same?  (because the checking code
talks about race conditions for example)

Richard.


> Thanks,
> Dehao
>
>
> On Thu, Jun 6, 2013 at 7:11 AM, Martin Jambor  wrote:
>>
>> Hi,
>>
>> On Tue, Jun 04, 2013 at 05:19:02PM -0700, Dehao Chen wrote:
>> > attached is a testcase that would cause problem when source has changed:
>> >
>> > $ g++ test.cc -O2 -fprofile-generate -DOLD
>> > $ ./a.out
>> > $ g++ test.cc -O2 -fprofile-use
>> > test.cc:34:1: internal compiler error: in operator[], at vec.h:815
>> >  }
>> >  ^
>> > 0x512740 vec::operator[](unsigned int)
>> > ../../gcc/vec.h:815
>> > 0x512740 vec::operator[](unsigned int)
>> > ../../gcc/vec.h:1244
>> > 0xf24464 vec::operator[](unsigned int)
>> > ../../gcc/vec.h:815
>> > 0xf24464 vec::operator[](unsigned int)
>> > ../../gcc/vec.h:1244
>> > 0xf24464 ipa_get_indirect_edge_target_1
>> > ../../gcc/ipa-cp.c:1535
>> > 0x971b9a estimate_edge_devirt_benefit
>> > ../../gcc/ipa-inline-analysis.c:2757
>>
>> Hm, this seems rather like an omission in ipa_get_indirect_edge_target_1.
>> Since it is called also from inlining, we can have parameter count
>> mismatches... and in fact in non-virtual paths of that function we do
>> check that we don't.  Because all callers have to pass known_vals
>> describing all formal parameters of the inline tree root, we should
>> apply the fix below (I've only just started running a bootstrap and
>> testsuite on x86_64, though).
>>
>> OTOH, while I understand that FDO can change inlining sufficiently so
>> that this error occurs, IMHO this should not be caused by outdated
>> profiles but there is somewhere a parameter mismatch in the source.
>>
>> Dehao, can you please check that this patch helps?
>>
>> Richi, if it does and the patch passes bootstrap and tests, is it OK
>> for trunk and 4.8 branch?
>>
>> Thanks and sorry for the trouble,
>>
>> Martin
>>
>>
>> 2013-06-06  Martin Jambor  
>>
>> * ipa-cp.c (ipa_get_indirect_edge_target_1): Check that param_index 
>> is
>> within bounds at the beginning of the function.
>>
>> Index: src/gcc/ipa-cp.c
>> ===
>> --- src.orig/gcc/ipa-cp.c
>> +++ src/gcc/ipa-cp.c
>> @@ -1481,7 +1481,8 @@ ipa_get_indirect_edge_target_1 (struct c
>>tree otr_type;
>>tree t;
>>
>> -  if (param_index == -1)
>> +  if (param_index == -1
>> +  || known_vals.length () <= (unsigned int) param_index)
>>  return NULL_TREE;
>>
>>if (!ie->indirect_info->polymorphic)
>> @@ -1516,8 +1517,7 @@ ipa_get_indirect_edge_target_1 (struct c
>> t = NULL;
>> }
>>else
>> -   t = (known_vals.length () > (unsigned int) param_index
>> -? known_vals[param_index] : NULL);
>> +   t = NULL;
>>
>>if (t &&
>>   TREE_CODE (t) == ADDR_EXPR

Re: Improve uncprop and coalescing

[Richard Biener]

On Fri, Jun 7, 2013 at 8:07 AM, Jeff Law  wrote:
>
> I stumbled over this while looking at regressions triggered when moving
> certain branch-cost driven transformations from fold-const.c to a later
> point in the pipeline.
>
> The coalescing we do as part of the out-of-ssa process restricts itself to
> only coalescing when the types of the underlying objects are the same.
> Where same is currently defined as pointer equality on the type.
>
> So if we have a copy or PHI node where the source & dest have equivalent,
> but different types we will not currently coalesce the source and
> destination.
>
> We also have a pass which un-propagates certain equivalences appearing as
> PHI args  when the equivalence is derived from conditionals.  This
> un-propagation pass is primarily meant to improve coalescing and ultimately
> eliminate silly constant initializations and useless copies.  That pass also
> used pointer equality of types when determining if unpropagation was
> profitable.
>
> Rather than using strict pointer equality, we can do better by looking at
> TYPE_CANONICAL when it's available.  Thus objects of the following two types
> (T1 & T2) become candidates for coalescing if they are tied together by a
> copy or PHI node.
>
> typedef int t1;
> typedef int t2;
>
>
> This typically eliminates necessary copies and constant initializations,
> which is good.  The only regression I saw when reviewing the generated code
> & dumps was a case where we got different register allocation on one path
> which in turn inhibited a tail merging opportunity.
>
>
>
> Bootstrapped and regression tested on x86_64-linux-gnu.  OK for the trunk?
>
>
>
>
> * gimple.h (gimple_can_coalesce_p): Prototype.
> * tree-ssa-coalesce.c (gimple_can_coalesce_p): New function.
> (create_outofssa_var_map, coalesce_partitions): Use it.
> * tree-ssa-uncprop.c (uncprop_into_successor_phis): Similarly.
> * tree-ssa-live.c (var_map_base_init): Use TYPE_CANONICAL
> if it's available.
>
> * gcc.dg/tree-ssa/coalesce-1.c: New test.
>
>
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index b4de403..8ae07c9 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -1101,6 +1101,9 @@ extern tree tree_ssa_strip_useless_type_conversions
> (tree);
>  extern bool useless_type_conversion_p (tree, tree);
>  extern bool types_compatible_p (tree, tree);
>
> +/* In tree-ssa-coalesce.c */
> +extern bool gimple_can_coalesce_p (tree, tree);
> +
>  /* Return the first node in GIMPLE sequence S.  */
>
>  static inline gimple_seq_node
> diff --git a/gcc/tree-ssa-coalesce.c b/gcc/tree-ssa-coalesce.c
> index 354b5f1..42bea5d 100644
> --- a/gcc/tree-ssa-coalesce.c
> +++ b/gcc/tree-ssa-coalesce.c
> @@ -943,8 +943,7 @@ create_outofssa_var_map (coalesce_list_p cl, bitmap
> used_in_copy)
> continue;
>
>   register_ssa_partition (map, arg);
> - if ((SSA_NAME_VAR (arg) == SSA_NAME_VAR (res)
> -  && TREE_TYPE (arg) == TREE_TYPE (res))
> + if (gimple_can_coalesce_p (arg, res)
>   || (e->flags & EDGE_ABNORMAL))
> {
>   saw_copy = true;
> @@ -985,8 +984,7 @@ create_outofssa_var_map (coalesce_list_p cl, bitmap
> used_in_copy)
> if (gimple_assign_copy_p (stmt)
>  && TREE_CODE (lhs) == SSA_NAME
> && TREE_CODE (rhs1) == SSA_NAME
> -   && SSA_NAME_VAR (lhs) == SSA_NAME_VAR (rhs1)
> -   && TREE_TYPE (lhs) == TREE_TYPE (rhs1))
> +   && gimple_can_coalesce_p (lhs, rhs1))
>   {
> v1 = SSA_NAME_VERSION (lhs);
> v2 = SSA_NAME_VERSION (rhs1);
> @@ -1037,8 +1035,7 @@ create_outofssa_var_map (coalesce_list_p cl, bitmap
> used_in_copy)
> v1 = SSA_NAME_VERSION (outputs[match]);
> v2 = SSA_NAME_VERSION (input);
>
> -   if (SSA_NAME_VAR (outputs[match]) == SSA_NAME_VAR
> (input)
> -   && TREE_TYPE (outputs[match]) == TREE_TYPE (input))
> +   if (gimple_can_coalesce_p (outputs[match], input))
>   {
> cost = coalesce_cost (REG_BR_PROB_BASE,
>   optimize_bb_for_size_p (bb));
> @@ -1072,8 +1069,7 @@ create_outofssa_var_map (coalesce_list_p cl, bitmap
> used_in_copy)
> first = var;
>   else
> {
> - gcc_assert (SSA_NAME_VAR (var) == SSA_NAME_VAR (first)
> - && TREE_TYPE (var) == TREE_TYPE (first));
> + gcc_assert (gimple_can_coalesce_p (var, first));
>   v1 = SSA_NAME_VERSION (first);
>   v2 = SSA_NAME_VERSION (var);
>   bitmap_set_bit (used_in_copy, v1);
> @@ -1210,8 +1206,7 @@ coalesce_partitions (var_map map, ssa_conflicts_p
> graph, coalesce_list_p cl,
> 

Re: [PATCH, alpha]: Update baseline_symbols.txt

[Paolo Carlini]

On 06/06/2013 06:34 PM, Uros Bizjak wrote:

Hello!

This patch avoids ABI check failure on alpha.

2013-06-06  Uros Bizjak  

 * config/abi/post/alpha-linux-gnu/baseline_symbols.txt: Update.

Tested on alphaev68-pc-linux-gnu.

OK for mainline?

Ok, thanks. Don't you need to update 4_8-branch too?

Paolo.

Re: [PATCH, alpha]: Update baseline_symbols.txt

[Uros Bizjak]

On Fri, Jun 7, 2013 at 11:40 AM, Paolo Carlini  wrote:

>> This patch avoids ABI check failure on alpha.
>>
>> 2013-06-06  Uros Bizjak  
>>
>>  * config/abi/post/alpha-linux-gnu/baseline_symbols.txt: Update.
>>
>> Tested on alphaev68-pc-linux-gnu.
>>
>> OK for mainline?
>
> Ok, thanks. Don't you need to update 4_8-branch too?

Yes, I have similar update for 4.8 branch, too.

Uros.

Re: [PATCH v2] Test case for PR55033

[Chung-Ju Wu]

2013/6/7 Sebastian Huber :
> On 05/17/2013 01:27 AM, David Edelsohn wrote:
>>
>> On Fri, May 10, 2013 at 11:43 PM, Chung-Ju Wu  wrote:
>>>
>>> 2013/5/10 Sebastian Huber :
>>> v2: Format changes
>>>
>>> gcc/testsuite/ChangeLog
>>> 2013-05-10  Sebastian Huber  
>>>
>>>  PR target/55033
>>>  * gcc.target/powerpc/pr55033.c: New.
>>
>>
>> The testcase is okay.
>>
>> Do you have CVS write access or do you need someone to commit the patch
>> for you?
>>
>> Thanks, David
>>
>
> Sorry, I missed your reply.  I have no write access, so please commit it for
> me.
>

I can help it.

Committed as Rev.199795
http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=199795


Best regards,
jasonwucj

[RS6000] little-endian va_arg

[Alan Modra]

tree-stdarg.c:check_all_va_list_escapes looks at va_arg accesses
to determine cfun->va_list_gpr_size.  The value set is the offset of
the highest access in the va_list area, plus the size of that access.
For big-endian rs6000 this always results in a multiple of the gpr
register size, but that isn't true for little-endian.  We currently
round down the size, resulting in gcc.c-torture/execute/20071213-1.c
failure on powerpc64le at -O3.

Bootstrapped and regression tested powerpc64-linux.  OK to apply?

* config/rs6000/rs6000.c (setup_incoming_varargs): Round up
va_list_gpr_size.

Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 199781)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -9371,19 +9371,20 @@
   && cfun->va_list_gpr_size)
 {
   int nregs = GP_ARG_NUM_REG - first_reg_offset;
+  int n_gpr;
 
   if (va_list_gpr_counter_field)
{
  /* V4 va_list_gpr_size counts number of registers needed.  */
- if (nregs > cfun->va_list_gpr_size)
-   nregs = cfun->va_list_gpr_size;
+ n_gpr = cfun->va_list_gpr_size;
}
   else
{
  /* char * va_list instead counts number of bytes needed.  */
- if (nregs > cfun->va_list_gpr_size / reg_size)
-   nregs = cfun->va_list_gpr_size / reg_size;
+ n_gpr = (cfun->va_list_gpr_size + reg_size - 1) / reg_size;
}
+  if (nregs > n_gpr)
+   nregs = n_gpr;
 
   mem = gen_rtx_MEM (BLKmode,
 plus_constant (Pmode, save_area,

-- 
Alan Modra
Australia Development Lab, IBM

Re: [RS6000] little-endian va_arg

[David Edelsohn]

On Fri, Jun 7, 2013 at 6:04 AM, Alan Modra  wrote:
> tree-stdarg.c:check_all_va_list_escapes looks at va_arg accesses
> to determine cfun->va_list_gpr_size.  The value set is the offset of
> the highest access in the va_list area, plus the size of that access.
> For big-endian rs6000 this always results in a multiple of the gpr
> register size, but that isn't true for little-endian.  We currently
> round down the size, resulting in gcc.c-torture/execute/20071213-1.c
> failure on powerpc64le at -O3.
>
> Bootstrapped and regression tested powerpc64-linux.  OK to apply?
>
> * config/rs6000/rs6000.c (setup_incoming_varargs): Round up
> va_list_gpr_size.
>
> Index: gcc/config/rs6000/rs6000.c
> ===
> --- gcc/config/rs6000/rs6000.c  (revision 199781)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -9371,19 +9371,20 @@
>&& cfun->va_list_gpr_size)
>  {
>int nregs = GP_ARG_NUM_REG - first_reg_offset;
> +  int n_gpr;

Declare n_gpr on the same line.

>
>if (va_list_gpr_counter_field)
> {
>   /* V4 va_list_gpr_size counts number of registers needed.  */
> - if (nregs > cfun->va_list_gpr_size)
> -   nregs = cfun->va_list_gpr_size;
> + n_gpr = cfun->va_list_gpr_size;
> }

Remove the braces.

>else
> {
>   /* char * va_list instead counts number of bytes needed.  */
> - if (nregs > cfun->va_list_gpr_size / reg_size)
> -   nregs = cfun->va_list_gpr_size / reg_size;
> + n_gpr = (cfun->va_list_gpr_size + reg_size - 1) / reg_size;
> }

Remove the braces.  Blank line.

> +  if (nregs > n_gpr)
> +   nregs = n_gpr;
>
>mem = gen_rtx_MEM (BLKmode,
>  plus_constant (Pmode, save_area,

Okay with those changes.

Thanks, David

Re: [PATCH][ARM] Cleanup anddi3 constraints

[Richard Earnshaw]

On 06/06/13 17:08, Kyrylo Tkachov wrote:

Hi all,

This patch cleans up the anddi3_insn pattern by removing duplicate
alternatives and redundant attribute setters.
It also restricts the splitting to after reload and when we know that
we're not using the NEON versions.

Tested arm-none-eabi on qemu.

Ok for trunk?

Thanks,
Kyrill

2013-06-06  Kyrylo Tkachov  

* config/arm/arm.md (anddi3_insn): Remove duplicate
alternatives. Clean up.


Shouldn't the second sentence be "Clean up attributes"?

Two spaces after full stop before new sentence.

Otherwise OK.

R.

Re: [RFC] Implement Undefined Behavior Sanitizer

[Marek Polacek]

On Wed, Jun 05, 2013 at 07:50:52PM +, Joseph S. Myers wrote:
> On Wed, 5 Jun 2013, Marek Polacek wrote:
> 
> > It works by creating a COMPOUND_EXPR around original expression, so e.g.
> > it creates:
> > 
> > if (b < 0 || (b > 31 || a < 0))
> >   {
> > __builtin___ubsan_handle_shift_out_of_bounds ();
> >   }
> > else
> >   {
> > 0
> >   }, a << b;
> > 
> > from original "a <<= b;".
> 
> For the "a < 0" here, and signed left shift of a positive value shifting a 
> 1 into or past the sign bit, I think it should be possible to control the 
> checks separately from other checks on shifts - both because those cases 
> were implementation-defined in C90, only undefined in C99/C11, and because 
> they are widely used in practice.

Ok, I see.

> > There is of course a lot of stuff that needs to be done, more
> > specifically:
> 
> 5) Testcases (or if applicable, running existing testcases coming with the 
> library).

Yeah -- we definitely want to have some testcases; the trouble is
that, like for tsan, we don't have any infrastructure for that yet.
Probably we could just put new tests into gcc.dg and put
-fsanitize=undefined into dg-options?  Or maybe tweak .exp files and
run some testcases also with -fsanitize=undefined, but the thing is
that we can't use dg-do compile tests, we need dg-do run tests.

> 6) Map -ftrapv onto an appropriate subset of this option that handles the 
> cases -ftrapv was meant to handle (so arithmetic overflow, which I'd say 
> should include INT_MIN / -1).

Ok, we can look at this maybe later when ubsan is more mature.

> >   4) and of course, more instrumentation (C/C++ FE, gimple level)
> >  What comes to mind is:
> >  - float/double to integer conversions,
> 
> Under Annex F, these return an unspecified value rather than being 
> undefined behavior.

Aha, good to know.  I've mentioned it because clang instruments that.

> >  - integer overflows (a long list of various cases here),
> 
> Strictly, including INT_MIN % -1 (both / and % are undefined if the result 
> of either is unrepresentable) - it appears you've already got that.  Of 
> course INT_MIN % -1 and INT_MIN / -1 should *work* reliably with -fwrapv, 
> which is another bug (30484).
> 
> >  - invalid conversions of int to bool,
> 
> What do you mean?  Conversion to bool is just a comparison != 0.

Something like e.g.:

unsigned char c = 42;
int
main (void)
{
  _Bool *b = (_Bool *) &c;
  return *b;
}

(clang catches this.)

> >  - VLAs size (e.g. negative size),
> 
> Or the multiplication used to compute the size in bytes overflows (really, 
> there should be some code generated expanding the stack bit by bit to 
> avoid it accidentally overflowing into another allocated area of memory, I 
> suppose).

Yeah, that sounds interesting as well.

> > +@item -fsanitize=undefined
> > +Enable UndefinedBehaviorSanitizer, a fast undefined behavior detector
> > +Various computations will be instrumented to detect
> > +undefined behavior, e.g. division by zero or various overflows.
> 
> e.g.@:

Fixed.  Thanks!

Marek

Re: [PATCH] Fix incorrect discriminator assignment.

[Rainer Orth]

Andreas Schwab  writes:

> Rainer Orth  writes:
>
>> Indeed, and I see -ansi -pedantic-errors -gdwarf-2 -O0 used for the test
>> on both x86_64-unknown-linux-gnu and i386-pc-solaris2.10 (if I augment
>> the target clause).
>
> DEFAULT_CFLAGS is set by many *.exp files.  dwarf2.exp will not
> overwrite an existing definition, so using anthing different from "-ansi
> -pedantic-errors" will not work.

Right, I noticed this myself.  Most likely just another one of the many
instances of mindless copy-and-paste programming in our testsuite ;-(

I've now applied the following patch to fix this, after testing on
x86_64-unknown-linux-gnu.

Btw., I've started implementing a dwarf2_debug_line effective-target
keyword for the testsuite.

Rainer


2013-06-07  Rainer Orth  

* gcc.dg/debug/dwarf2/discriminator.c: Fix wording.
Revert to dg-options.

diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/discriminator.c b/gcc/testsuite/gcc.dg/debug/dwarf2/discriminator.c
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/discriminator.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/discriminator.c
@@ -1,7 +1,7 @@
 /* HAVE_AS_DWARF2_DEBUG_LINE macro needs to be defined to pass the unittest.
-   However, there dg cannot defin it as, so we restrict the target to linux.  */
+   However, dg cannot access it, so we restrict the target to linux.  */
 /* { dg-do compile { target *-*-linux-gnu } } */
-/* { dg-additional-options "-O0" } */
+/* { dg-options "-O0 -gdwarf-2" } */
 /* { dg-final { scan-assembler "loc \[0-9] 11 \[0-9]( is_stmt \[0-9])?\n" } } */
 /* { dg-final { scan-assembler "loc \[0-9] 11 \[0-9]( is_stmt \[0-9])? discriminator 2\n" } } */
 /* { dg-final { scan-assembler "loc \[0-9] 11 \[0-9]( is_stmt \[0-9])? discriminator 1\n" } } */

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: C++ PATCH for c++/52377 (NSDMI and unions)

[Rainer Orth]

Jason Merrill  writes:

> On 05/31/2013 07:13 AM, Rainer Orth wrote:
>> Also, both testcases are missing from gcc/testsuite/ChangeLog.
>
> I prefer not to mess with logging testcase changes.
>
> http://gcc.gnu.org/codingconventions.html#ChangeLogs

I hadn't seen this before.  I think this policy is highly unfortunate:
unreliable entries are worse than none at all.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Improve uncprop and coalescing

[Jeff Law]

On 06/07/13 02:30, Steven Bosscher wrote:

On Fri, Jun 7, 2013 at 8:07 AM, Jeff Law wrote:

Rather than using strict pointer equality, we can do better by looking at
TYPE_CANONICAL when it's available.  Thus objects of the following two types
(T1 & T2) become candidates for coalescing if they are tied together by a
copy or PHI node.

typedef int t1;
typedef int t2;


This typically eliminates necessary copies and constant initializations,
which is good.


Hmm...  Can't you use types_compatible_p?

No.  That was my first inclination.

jeff

[PATCH][ARM] PR56315 Improve codegen for DImode immediates - XOR

[Kyrylo Tkachov]

Hi all,

In a similar vein to
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01838.html this patch lets
us use the immediate forms of the eor instruction when dealing with
64-bit constants.
The insn is changed into an insn_and_split and merged with the NEON
version. A new constraint is introduced to make sure the pattern accepts
the correct range of immediates.

Now for code:
  unsigned long long xor64 (unsigned long long input)
  {
return input ^ 0x20004ULL;
  }

we generate:
 eor r0, r0, #4
 eor r1, r1, #2

without any mov immediates.

Regtested arm-none-eabi on qemu with and without -mthumb.

Ok for trunk?

Thanks,
Kyrill

gcc/
2013-06-07  Kyrylo Tkachov  

PR target/56315
* config/arm/arm.md (*xordi3_insn): Change to insn_and_split.
(xordi3): Change operand 2 constraint to arm_xordi_operand.
* config/arm/constraints.md (Dg): New constraint.
* config/arm/neon.md (xordi3_neon): Remove.
(neon_veor): Generate xordi3 instead of xordi3_neon.
* config/arm/predicates.md (arm_xordi_operand): New predicate.


gcc/testsuite/
2013-06-07  Kyrylo Tkachov  

PR target/56315
* gcc.target/arm/xordi3-opt.c: New test.

RE: [PATCH][ARM] PR56315 Improve codegen for DImode immediates - XOR

[Kyrylo Tkachov]

Oops, once again with patch attached...

gcc/
2013-06-07  Kyrylo Tkachov  

PR target/56315
* config/arm/arm.md (*xordi3_insn): Change to insn_and_split.
(xordi3): Change operand 2 constraint to arm_xordi_operand.
* config/arm/constraints.md (Dg): New constraint.
* config/arm/neon.md (xordi3_neon): Remove.
(neon_veor): Generate xordi3 instead of xordi3_neon.
* config/arm/predicates.md (arm_xordi_operand): New predicate.


gcc/testsuite/
2013-06-07  Kyrylo Tkachov  

PR target/56315
* gcc.target/arm/xordi3-opt.c: New test.


> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Kyrylo Tkachov
> Sent: 07 June 2013 14:03
> To: gcc-patches@gcc.gnu.org
> Cc: Ramana Radhakrishnan; Richard Earnshaw
> Subject: [PATCH][ARM] PR56315 Improve codegen for DImode immediates
> - XOR
> 
> Hi all,
> 
> In a similar vein to
> http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01838.html this patch
> lets
> us use the immediate forms of the eor instruction when dealing with
> 64-bit constants.
> The insn is changed into an insn_and_split and merged with the NEON
> version. A new constraint is introduced to make sure the pattern
> accepts
> the correct range of immediates.
> 
> Now for code:
>   unsigned long long xor64 (unsigned long long input)
>   {
> return input ^ 0x20004ULL;
>   }
> 
> we generate:
>  eor r0, r0, #4
>  eor r1, r1, #2
> 
> without any mov immediates.
> 
> Regtested arm-none-eabi on qemu with and without -mthumb.
> 
> Ok for trunk?
> 
> Thanks,
> Kyrill
> 
> gcc/
> 2013-06-07  Kyrylo Tkachov  
> 
>   PR target/56315
>   * config/arm/arm.md (*xordi3_insn): Change to insn_and_split.
>   (xordi3): Change operand 2 constraint to arm_xordi_operand.
>   * config/arm/constraints.md (Dg): New constraint.
>   * config/arm/neon.md (xordi3_neon): Remove.
>   (neon_veor): Generate xordi3 instead of xordi3_neon.
>   * config/arm/predicates.md (arm_xordi_operand): New
> predicate.
> 
> 
> gcc/testsuite/
> 2013-06-07  Kyrylo Tkachov  
> 
>   PR target/56315
>   * gcc.target/arm/xordi3-opt.c: New test.
> 
> 
> 


xordi.patch
Description: Binary data

Add error message for mismatched parentheses in reservation string

[Tom de Vries]

Vladimir,

If I introduce an unbalanced parentheses error in a reservation string, f.i. in
athlon.md using the following patch:
...
diff --git a/gcc/config/i386/athlon.md b/gcc/config/i386/athlon.md
index d872b8f..b1ed5cd 100644
--- a/gcc/config/i386/athlon.md
+++ b/gcc/config/i386/athlon.md
@@ -90,7 +90,7 @@
 (athlon-decode0 | athlon-decode1
 | athlon-decode2)")
 ;; Double instructions behaves like two direct instructions.
-(define_reservation "athlon-double" "((athlon-decode2, athlon-decode0)
+(define_reservation "athlon-double" "(athlon-decode2, athlon-decode0)
 | (nothing,(athlon-decode0 + 
athlon-decode1))
 | (nothing,(athlon-decode1 + 
athlon-decode2)))")
...

and rebuild cc1, I get a segmentation fault:
...
build/genautomata gcc/config/i386/i386.md \
  insn-conditions.md > tmp-automata.c
/bin/bash: line 1: 18077 Segmentation fault  (core dumped) build/genautomata
gcc/config/i386/i386.md insn-conditions.md > tmp-automata.c
make: *** [s-automata] Error 139
...

The segmentation fault happens because sequence_vect is set to NULL here in
gen_regexp_sequence in genautomata.c:
...
  sequence_vect = get_str_vect (str, &els_num, ',', TRUE);
  if (els_num > 1)
...

and sequence_vect is dereferenced here:
...
  else
return gen_regexp_oneof (sequence_vect[0]);
...

The patch adds error checking for the specific case of unbalanced parentheses,
and for sequence_vect == NULL in general.

Using the patch the error message becomes:
...
genautomata: unbalanced parentheses in reservation `(athlon-decode2, 
athlon-decode0)
 | (nothing,(athlon-decode0 + 
athlon-decode1))
 | (nothing,(athlon-decode1 + 
athlon-decode2)))'
...

Tested by completing a non-bootstrap build.

OK for trunk?

Thanks,
- Tom

2013-06-07  Tom de Vries  

* genautomata.c (gen_regexp_sequence): Handle els_num == -1.  Handle
sequence_vect == NULL.
diff --git a/gcc/genautomata.c b/gcc/genautomata.c
index 3665d95..add4624 100644
--- a/gcc/genautomata.c
+++ b/gcc/genautomata.c
@@ -1672,6 +1672,10 @@ gen_regexp_sequence (const char *str)
   int i;
 
   sequence_vect = get_str_vect (str, &els_num, ',', TRUE);
+  if (els_num == -1)
+fatal ("unbalanced parentheses in reservation `%s'", str);
+  if (sequence_vect == NULL)
+fatal ("invalid reservation `%s'", str);
   if (els_num > 1)
 {
   sequence = XCREATENODEVAR (struct regexp, sizeof (struct regexp)

Re: [PATCH][ARM] PR56315 Improve codegen for DImode immediates - XOR

[Richard Earnshaw]

On 07/06/13 14:04, Kyrylo Tkachov wrote:

Oops, once again with patch attached...

gcc/
2013-06-07  Kyrylo Tkachov  

PR target/56315
* config/arm/arm.md (*xordi3_insn): Change to insn_and_split.
(xordi3): Change operand 2 constraint to arm_xordi_operand.
* config/arm/constraints.md (Dg): New constraint.
* config/arm/neon.md (xordi3_neon): Remove.
(neon_veor): Generate xordi3 instead of xordi3_neon.
* config/arm/predicates.md (arm_xordi_operand): New predicate.


gcc/testsuite/
2013-06-07  Kyrylo Tkachov  

PR target/56315
* gcc.target/arm/xordi3-opt.c: New test.



-Original Message-
From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
ow...@gcc.gnu.org] On Behalf Of Kyrylo Tkachov
Sent: 07 June 2013 14:03
To: gcc-patches@gcc.gnu.org
Cc: Ramana Radhakrishnan; Richard Earnshaw
Subject: [PATCH][ARM] PR56315 Improve codegen for DImode immediates
- XOR

Hi all,

In a similar vein to
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01838.html this patch
lets
us use the immediate forms of the eor instruction when dealing with
64-bit constants.
The insn is changed into an insn_and_split and merged with the NEON
version. A new constraint is introduced to make sure the pattern
accepts
the correct range of immediates.

Now for code:
   unsigned long long xor64 (unsigned long long input)
   {
 return input ^ 0x20004ULL;
   }

we generate:
  eor r0, r0, #4
  eor r1, r1, #2

without any mov immediates.

Regtested arm-none-eabi on qemu with and without -mthumb.

Ok for trunk?

Thanks,
Kyrill

gcc/
2013-06-07  Kyrylo Tkachov  

PR target/56315
* config/arm/arm.md (*xordi3_insn): Change to insn_and_split.
(xordi3): Change operand 2 constraint to arm_xordi_operand.
* config/arm/constraints.md (Dg): New constraint.
* config/arm/neon.md (xordi3_neon): Remove.
(neon_veor): Generate xordi3 instead of xordi3_neon.
* config/arm/predicates.md (arm_xordi_operand): New
predicate.


gcc/testsuite/
2013-06-07  Kyrylo Tkachov  

PR target/56315
* gcc.target/arm/xordi3-opt.c: New test.



OK.

R.

Re: [GOOGLE] More strict checking for call args

[Xinliang David Li]

On Fri, Jun 7, 2013 at 2:05 AM, Richard Biener
 wrote:
> On Thu, Jun 6, 2013 at 5:10 PM, Dehao Chen  wrote:
>> Hi, Martin,
>>
>> Yes, your patch can fix my case. Thanks a lot for the fix.
>>
>> With the fix, value profiling will still promote the wrong indirect
>> call target. Though it will not be inlining, but it results in an
>> additional check. How about in check_ic_target, after calling
>> gimple_check_call_matching_types, we also check if number of args
>> match number of params in target->symbol.decl?
>
> I wonder what's the point in the gimple_check_call_matching_types check
> in the profiling case.  It's at least no longer
>
> /* Perform sanity check on the indirect call target. Due to race conditions,
>false function target may be attributed to an indirect call site. If the
>call expression type mismatches with the target function's type, 
> expand_call
>may ICE.
>
> because since the introduction of gimple_call_fntype we will _not_ ICE.
>
> Thus I argue that check_ic_target should be even removed instead of
> enhancing it!
>

Another reason is what Dehao had mentioned -- wrong target leads to
useless transformation.

> How does IC profiling determine the called target?  That is, what does it
> do when the target is not always the same?  (because the checking code
> talks about race conditions for example)


The race condition is the happening at instrumentation time -- the
indirect call counters are not thread local. We have seen this a lot
in the past that a totally bogus target is attributed to a indirect
callsite.

thanks,

David
>
> Richard.
>
>
>> Thanks,
>> Dehao
>>
>>
>> On Thu, Jun 6, 2013 at 7:11 AM, Martin Jambor  wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Jun 04, 2013 at 05:19:02PM -0700, Dehao Chen wrote:
>>> > attached is a testcase that would cause problem when source has changed:
>>> >
>>> > $ g++ test.cc -O2 -fprofile-generate -DOLD
>>> > $ ./a.out
>>> > $ g++ test.cc -O2 -fprofile-use
>>> > test.cc:34:1: internal compiler error: in operator[], at vec.h:815
>>> >  }
>>> >  ^
>>> > 0x512740 vec::operator[](unsigned int)
>>> > ../../gcc/vec.h:815
>>> > 0x512740 vec::operator[](unsigned int)
>>> > ../../gcc/vec.h:1244
>>> > 0xf24464 vec::operator[](unsigned int)
>>> > ../../gcc/vec.h:815
>>> > 0xf24464 vec::operator[](unsigned int)
>>> > ../../gcc/vec.h:1244
>>> > 0xf24464 ipa_get_indirect_edge_target_1
>>> > ../../gcc/ipa-cp.c:1535
>>> > 0x971b9a estimate_edge_devirt_benefit
>>> > ../../gcc/ipa-inline-analysis.c:2757
>>>
>>> Hm, this seems rather like an omission in ipa_get_indirect_edge_target_1.
>>> Since it is called also from inlining, we can have parameter count
>>> mismatches... and in fact in non-virtual paths of that function we do
>>> check that we don't.  Because all callers have to pass known_vals
>>> describing all formal parameters of the inline tree root, we should
>>> apply the fix below (I've only just started running a bootstrap and
>>> testsuite on x86_64, though).
>>>
>>> OTOH, while I understand that FDO can change inlining sufficiently so
>>> that this error occurs, IMHO this should not be caused by outdated
>>> profiles but there is somewhere a parameter mismatch in the source.
>>>
>>> Dehao, can you please check that this patch helps?
>>>
>>> Richi, if it does and the patch passes bootstrap and tests, is it OK
>>> for trunk and 4.8 branch?
>>>
>>> Thanks and sorry for the trouble,
>>>
>>> Martin
>>>
>>>
>>> 2013-06-06  Martin Jambor  
>>>
>>> * ipa-cp.c (ipa_get_indirect_edge_target_1): Check that param_index 
>>> is
>>> within bounds at the beginning of the function.
>>>
>>> Index: src/gcc/ipa-cp.c
>>> ===
>>> --- src.orig/gcc/ipa-cp.c
>>> +++ src/gcc/ipa-cp.c
>>> @@ -1481,7 +1481,8 @@ ipa_get_indirect_edge_target_1 (struct c
>>>tree otr_type;
>>>tree t;
>>>
>>> -  if (param_index == -1)
>>> +  if (param_index == -1
>>> +  || known_vals.length () <= (unsigned int) param_index)
>>>  return NULL_TREE;
>>>
>>>if (!ie->indirect_info->polymorphic)
>>> @@ -1516,8 +1517,7 @@ ipa_get_indirect_edge_target_1 (struct c
>>> t = NULL;
>>> }
>>>else
>>> -   t = (known_vals.length () > (unsigned int) param_index
>>> -? known_vals[param_index] : NULL);
>>> +   t = NULL;
>>>
>>>if (t &&
>>>   TREE_CODE (t) == ADDR_EXPR

[Patch, Fortran, committed] PR57556 - Fix ordering issue in gfc_build_final_call

[Tobias Burnus]

It helps to initialize a block before calling "gfc_add_block_to_block 
(&block" ...


Committed as Rev. 199812 after building and regtesting on x86-64-gnu-linux.

Tobias
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog	(Revision 199811)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,8 @@
+2013-06-07  Tobias Burnus  
+
+	PR fortran/57556
+	* trans.c (gfc_build_final_call): Init block before use.
+
 2013-06-06  Tobias Burnus  
 
 	PR fortran/57542
Index: gcc/fortran/trans.c
===
--- gcc/fortran/trans.c	(Revision 199811)
+++ gcc/fortran/trans.c	(Arbeitskopie)
@@ -852,6 +852,7 @@ gfc_build_final_call (gfc_typespec ts, gfc_expr *f
   gcc_assert (final_wrapper->expr_type == EXPR_VARIABLE);
   gcc_assert (var);
 
+  gfc_start_block (&block);
   gfc_init_se (&se, NULL);
   gfc_conv_expr (&se, final_wrapper);
   final_fndecl = se.expr;
@@ -936,7 +937,6 @@ gfc_build_final_call (gfc_typespec ts, gfc_expr *f
   if (!POINTER_TYPE_P (TREE_TYPE (array)))
 array = gfc_build_addr_expr (NULL, array);
 
-  gfc_start_block (&block);
   gfc_add_block_to_block (&block, &se.pre);
   tmp = build_call_expr_loc (input_location,
 			 final_fndecl, 3, array,

Re: [GOOGLE] More strict checking for call args

[Richard Biener]

On Fri, Jun 7, 2013 at 3:30 PM, Xinliang David Li  wrote:
> On Fri, Jun 7, 2013 at 2:05 AM, Richard Biener
>  wrote:
>> On Thu, Jun 6, 2013 at 5:10 PM, Dehao Chen  wrote:
>>> Hi, Martin,
>>>
>>> Yes, your patch can fix my case. Thanks a lot for the fix.
>>>
>>> With the fix, value profiling will still promote the wrong indirect
>>> call target. Though it will not be inlining, but it results in an
>>> additional check. How about in check_ic_target, after calling
>>> gimple_check_call_matching_types, we also check if number of args
>>> match number of params in target->symbol.decl?
>>
>> I wonder what's the point in the gimple_check_call_matching_types check
>> in the profiling case.  It's at least no longer
>>
>> /* Perform sanity check on the indirect call target. Due to race conditions,
>>false function target may be attributed to an indirect call site. If the
>>call expression type mismatches with the target function's type, 
>> expand_call
>>may ICE.
>>
>> because since the introduction of gimple_call_fntype we will _not_ ICE.
>>
>> Thus I argue that check_ic_target should be even removed instead of
>> enhancing it!
>>
>
> Another reason is what Dehao had mentioned -- wrong target leads to
> useless transformation.

Sure, but a not wrong in the sense of the predicate does not guarantee
a useful transformation either.

>> How does IC profiling determine the called target?  That is, what does it
>> do when the target is not always the same?  (because the checking code
>> talks about race conditions for example)
>
>
> The race condition is the happening at instrumentation time -- the
> indirect call counters are not thread local. We have seen this a lot
> in the past that a totally bogus target is attributed to a indirect
> callsite.

So it simply uses whatever function was called last?  Instead of
using the function that was called most of the time?

Richard.

> thanks,
>
> David
>>
>> Richard.
>>
>>
>>> Thanks,
>>> Dehao
>>>
>>>
>>> On Thu, Jun 6, 2013 at 7:11 AM, Martin Jambor  wrote:

 Hi,

 On Tue, Jun 04, 2013 at 05:19:02PM -0700, Dehao Chen wrote:
 > attached is a testcase that would cause problem when source has changed:
 >
 > $ g++ test.cc -O2 -fprofile-generate -DOLD
 > $ ./a.out
 > $ g++ test.cc -O2 -fprofile-use
 > test.cc:34:1: internal compiler error: in operator[], at vec.h:815
 >  }
 >  ^
 > 0x512740 vec::operator[](unsigned int)
 > ../../gcc/vec.h:815
 > 0x512740 vec::operator[](unsigned int)
 > ../../gcc/vec.h:1244
 > 0xf24464 vec::operator[](unsigned int)
 > ../../gcc/vec.h:815
 > 0xf24464 vec::operator[](unsigned int)
 > ../../gcc/vec.h:1244
 > 0xf24464 ipa_get_indirect_edge_target_1
 > ../../gcc/ipa-cp.c:1535
 > 0x971b9a estimate_edge_devirt_benefit
 > ../../gcc/ipa-inline-analysis.c:2757

 Hm, this seems rather like an omission in ipa_get_indirect_edge_target_1.
 Since it is called also from inlining, we can have parameter count
 mismatches... and in fact in non-virtual paths of that function we do
 check that we don't.  Because all callers have to pass known_vals
 describing all formal parameters of the inline tree root, we should
 apply the fix below (I've only just started running a bootstrap and
 testsuite on x86_64, though).

 OTOH, while I understand that FDO can change inlining sufficiently so
 that this error occurs, IMHO this should not be caused by outdated
 profiles but there is somewhere a parameter mismatch in the source.

 Dehao, can you please check that this patch helps?

 Richi, if it does and the patch passes bootstrap and tests, is it OK
 for trunk and 4.8 branch?

 Thanks and sorry for the trouble,

 Martin


 2013-06-06  Martin Jambor  

 * ipa-cp.c (ipa_get_indirect_edge_target_1): Check that 
 param_index is
 within bounds at the beginning of the function.

 Index: src/gcc/ipa-cp.c
 ===
 --- src.orig/gcc/ipa-cp.c
 +++ src/gcc/ipa-cp.c
 @@ -1481,7 +1481,8 @@ ipa_get_indirect_edge_target_1 (struct c
tree otr_type;
tree t;

 -  if (param_index == -1)
 +  if (param_index == -1
 +  || known_vals.length () <= (unsigned int) param_index)
  return NULL_TREE;

if (!ie->indirect_info->polymorphic)
 @@ -1516,8 +1517,7 @@ ipa_get_indirect_edge_target_1 (struct c
 t = NULL;
 }
else
 -   t = (known_vals.length () > (unsigned int) param_index
 -? known_vals[param_index] : NULL);
 +   t = NULL;

if (t &&
   TREE_CODE (t) == ADDR_EXPR

Re: [PATCH] PR55033

[Richard Henderson]

On 06/06/2013 09:50 PM, Alan Modra wrote:
>   * varasm.c (get_section): Don't die on !DECL_P decl.  Tidy error
>   reporting.
>   (get_named_section): Don't NULL !DECL_P decl.


Ok.


r~

[Patch, Fortran] PR57549 - fix type-spec handling with array constructors

[Tobias Burnus]

Unreviewed patches:
* http://gcc.gnu.org/ml/fortran/2013-06/msg00027.html
* http://gcc.gnu.org/ml/fortran/2013-06/msg00048.html

 * * *

As with ALLOCATE (type-spec :: ...), also array constructors take as 
type-spec a type name ("integer, my_dt, etc.) without an extra TYPE().


Thus, instead of using the decl.c var decl function, we use the same as 
for allocate.


Build and regtested on x86-64-gnu-linux.
OK for the trunk?


From the standard:

The Fortran 2008 standard has:

R469  ac-spec  is  type-spec ::
   or  [type-spec ::] ac-value-list

R402  type-spec  is  intrinsic-type-spec
 or  derived-type-spec

R453  derived-type-spec  is  type-name [ ( type-param-spec-list ) ]


Tobias
2013-06-07  Tobias Burnus  

	PR fortran/57549
	* array.c (gfc_match_array_constructor): Call
	gfc_match_type_spec instead of gfc_match_decl_type_spec.
	* match.c (gfc_match_type_spec): Renamed from match_type_spec.
	(gfc_match_type_is, gfc_match_allocate): Update call.
	* match.h (gfc_match_type_spec): Add prototype.

2013-06-07  Tobias Burnus  

	PR fortran/57549
	* gfortran.dg/array_constructor_48.f90: New.
	* gfortran.dg/array_constructor_type_14.f03: Correct test case.
	* gfortran.dg/array_constructor_type_15.f03: Ditto.

diff --git a/gcc/fortran/array.c b/gcc/fortran/array.c
index c2ac1ec..c6b8eb9 100644
--- a/gcc/fortran/array.c
+++ b/gcc/fortran/array.c
@@ -1073,7 +1073,7 @@ gfc_match_array_constructor (gfc_expr **result)
   /* Try to match an optional "type-spec ::"  */
   gfc_clear_ts (&ts);
   gfc_new_undo_checkpoint (changed_syms);
-  if (gfc_match_decl_type_spec (&ts, 0) == MATCH_YES)
+  if (gfc_match_type_spec (&ts) == MATCH_YES)
 {
   seen_ts = (gfc_match (" ::") == MATCH_YES);
 
diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
index b44d815..2533584 100644
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -1937,8 +1937,8 @@ match_derived_type_spec (gfc_typespec *ts)
the implicit_flag is not needed, so it was removed. Derived types are
identified by their name alone.  */
 
-static match
-match_type_spec (gfc_typespec *ts)
+match
+gfc_match_type_spec (gfc_typespec *ts)
 {
   match m;
   locus old_locus;
@@ -3426,7 +3426,7 @@ gfc_match_allocate (void)
 
   /* Match an optional type-spec.  */
   old_locus = gfc_current_locus;
-  m = match_type_spec (&ts);
+  m = gfc_match_type_spec (&ts);
   if (m == MATCH_ERROR)
 goto cleanup;
   else if (m == MATCH_NO)
@@ -5502,7 +5502,7 @@ gfc_match_type_is (void)
   c = gfc_get_case ();
   c->where = gfc_current_locus;
 
-  if (match_type_spec (&c->ts) == MATCH_ERROR)
+  if (gfc_match_type_spec (&c->ts) == MATCH_ERROR)
 goto cleanup;
 
   if (gfc_match_char (')') != MATCH_YES)
diff --git a/gcc/fortran/match.h b/gcc/fortran/match.h
index ac8b9f8..1a701f0 100644
--- a/gcc/fortran/match.h
+++ b/gcc/fortran/match.h
@@ -59,6 +59,8 @@ match gfc_match_char (char);
 match gfc_match (const char *, ...);
 match gfc_match_iterator (gfc_iterator *, int);
 match gfc_match_parens (void);
+match gfc_match_type_spec (gfc_typespec *);
+
 
 /* Statement matchers.  */
 match gfc_match_program (void);
--- /dev/null	2013-06-07 09:13:23.024185858 +0200
+++ gcc/gcc/testsuite/gfortran.dg/array_constructor_48.f90	2013-06-07 15:52:28.578691018 +0200
@@ -0,0 +1,17 @@
+! { dg-do compile }
+!
+! PR fortran/57549
+!
+! Contributed by Vladimir Fuka
+!
+ type t
+ end type
+ type(t),allocatable :: a(:)
+ a = [t::t()]
+ print *, [ integer :: ]
+end
+
+subroutine invalid()
+print *, [ type(integer) :: ] ! { dg-error "Syntax error in array constructor" }
+print *, [ type(tt) :: ]  ! { dg-error "Syntax error in array constructor" }
+end subroutine invalid
diff --git a/gcc/testsuite/gfortran.dg/array_constructor_type_14.f03 b/gcc/testsuite/gfortran.dg/array_constructor_type_14.f03
index 04ac728..0e24334 100644
--- a/gcc/testsuite/gfortran.dg/array_constructor_type_14.f03
+++ b/gcc/testsuite/gfortran.dg/array_constructor_type_14.f03
@@ -16,7 +16,7 @@ PROGRAM test
 
   TYPE(foo), DIMENSION(2) :: arr
 
-  arr = (/ TYPE(foo) :: x, foo(0, 1.) /)
+  arr = (/ foo :: x, foo(0, 1.) /)
   IF (arr(1)%i /= 42 .OR. arr(1)%x /= 42. .OR. &
   arr(2)%i /= 0 .OR. arr(2)%x /= 1.) THEN
 CALL abort()
diff --git a/gcc/testsuite/gfortran.dg/array_constructor_type_15.f03 b/gcc/testsuite/gfortran.dg/array_constructor_type_15.f03
index 2073698..a946555 100644
--- a/gcc/testsuite/gfortran.dg/array_constructor_type_15.f03
+++ b/gcc/testsuite/gfortran.dg/array_constructor_type_15.f03
@@ -18,5 +18,5 @@ PROGRAM test
 
   TYPE(foo), PARAMETER :: x = foo(42, 42.)
 
-  WRITE (*,*) (/ TYPE(foo) :: x, foo(0, 1.), bar(.TRUE.) /) ! { dg-error "convert TYPE" }
+  WRITE (*,*) (/ foo :: x, foo(0, 1.), bar(.TRUE.) /) ! { dg-error "convert TYPE" }
 END PROGRAM test

Re: [PATCH] gfortran testsuite: implicitly cleanup-modules

[Bernhard Reutner-Fischer]

On 29 June 2012 04:59, Mike Stump  wrote:
> On Jun 28, 2012, at 5:15 PM, Bernhard Reutner-Fischer wrote:
>> On Thu, Jun 28, 2012 at 04:43:05PM -0700, Mike Stump wrote:
>>> On Jun 28, 2012, at 3:27 PM, Bernhard Reutner-Fischer wrote:
 Perhaps you want to pursue this? We'd need to suggest this to dejagnu,
>>>
>>> Actually, we have the technology, so that isn't necessary.  :-)  You can 
>>> install replacements for any procs you want, not pretty, but... it does 
>>> work.  I think this is a more deterministic path forward than waiting for a 
>>> mythical dejagnu release.  Also, we then can avoid the hassle of requiring 
>>> a new dejagnu.
>>
>> Wouldn't that mean that we have to completely replace proc load_lib?
>
> Yes; worse, it is a cut-n-paste from dejagnu and can effectively rev lock us 
> to the current dejagnu release...  One can delegate, but I don't think any 
> pre or post processing in this case is enough to `fix' the issue, so it would 
> be a wholesale replacement.

Ben,

Would you accept something like the patch in the message below into dejagnu?
http://gcc.gnu.org/ml/fortran/2012-03/msg00094.html

Above patch was motivated by these (unpleasant) observations:
http://gcc.gnu.org/ml/fortran/2012-03/msg00092.html

thanks,
>
>> But anyway.
>> Mike, it would be nice if you could fix
>>> +# BUG: gcc-dg calls gcc-set-multilib-library-path but does not load 
>>> gcc-defs!
>
> Sounds like a single line fix.  It is the testing of that fix that is the 
> annoying part.

Re: patch to fix PR57468

[Vladimir Makarov]

On 13-06-06 9:22 PM, David Edelsohn wrote:

The patch actually restore the LRA behaviour for x86/x86-64 before rev. 199298. 
The revision was added for PPC SDmode value correct generation. So it is really 
needed for PPC64 and badly hurts x86/x86-64 performance (by doing secondary 
memory reloads when one pseudo is spilled).

 Should the solution for PPC64 be further limited, even on
PPC64? Is this going to hurt more normal spilling code on PPC64 that
does not have the strange restrictions of SDmode?


  No, it does not hurt ppc performance.  In ppc case, it works the same 
way as reload does (and only for SDmode values).  I also found LRA 
generates a better code for SDmode than reload.  I did not figure out 
why reload pass uses secondary memory when one pseudo is spilled for ppc 
and does not use this for x86/x86-64 although both targets report 
necessity of secondary memory in case when one pseudo is spilled.  It 
requires a lot of time but I should probably still do this.


  I hope that Mike checks performance of LRA as I have no hardware to 
do this.  If he finds any performance degradation, I look at it.

Clean up after standard literals patch.

[Ed Smith-Rowland]

Greetings,

Jonathan had asked if I could make some simplifications to the 
organization of C++ versioning macros and namespaces in my standard 
literals patch.  I plum forgot to do it before I put in the patch.


Also, I noticed I had picked up some garbage comments in the test cases.

This patch cleans all that up.

Built and tested on x86_64-linux.

Thanks,
Ed Smith-Rowland


2013-06-07  Ed Smith-Rowland  <3dw...@verizon.net>

Simplify and clean up library literals.
* include/std/chrono: Simplify namespace and versioning management.
* include/bits/basic_string.h: Ditto.
* testsuite/20_util/duration/literals/types.cc: Remove bogus comment.
* testsuite/20_util/duration/literals/values.cc: Ditto.
* testsuite/21_strings/basic_string/literals/types.cc: Ditto.
* testsuite/21_strings/basic_string/literals/values.cc: Ditto.

Index: include/std/chrono
===
--- include/std/chrono  (revision 199730)
+++ include/std/chrono  (working copy)
@@ -713,7 +713,7 @@
  *
  *  Time returned represents wall time from the system-wide clock.
 */
- struct system_clock
+struct system_clock
 {
   typedef chrono::nanoseconds  duration;
   typedef duration::reprep;
@@ -775,148 +775,137 @@
 */
 using high_resolution_clock = system_clock;
 
-  } // end inline namespace _V2
+} // end inline namespace _V2
 
   _GLIBCXX_END_NAMESPACE_VERSION
   } // namespace chrono
 
-  // @} group chrono
-} // namespace
+#if __cplusplus > 201103L
 
-#endif //_GLIBCXX_USE_C99_STDINT_TR1
+  inline namespace literals {
+  inline namespace chrono_literals {
 
-#endif // C++11
+namespace __detail {
 
-#if __cplusplus > 201103L
+  using namespace __parse_int;
 
-#ifdef _GLIBCXX_USE_C99_STDINT_TR1
+  template
+   struct _Select_type
+   : conditional<
+   _Val <= static_cast
+ (numeric_limits::max()),
+   _Dur, void>
+   {
+ static constexpr typename _Select_type::type
+   value{static_cast(_Val)};
+   };
 
-namespace std _GLIBCXX_VISIBILITY(default)
-{
-_GLIBCXX_BEGIN_NAMESPACE_VERSION
+  template
+   constexpr typename _Select_type<_Val, _Dur>::type
+   _Select_type<_Val, _Dur>::value;
 
-inline namespace literals {
-inline namespace chrono_literals {
+} // __detail
 
-  namespace __detail {
+constexpr chrono::duration>
+operator"" h(long double __hours)
+{ return chrono::duration>{__hours}; }
 
-using namespace __parse_int;
-
-template
-  struct _Select_type
-  : conditional<
- _Val <= static_cast
-   (numeric_limits::max()),
- _Dur, void>
+template 
+  constexpr typename
+  __detail::_Select_type<__select_int::_Select_int<_Digits...>::value,
+chrono::hours>::type
+  operator"" h()
   {
-   static constexpr typename _Select_type::type
- value{static_cast(_Val)};
-  };
+   return __detail::_Select_type<
+ __select_int::_Select_int<_Digits...>::value,
+ chrono::hours>::value;
+  }
 
-template
-  constexpr typename _Select_type<_Val, _Dur>::type
-  _Select_type<_Val, _Dur>::value;
+constexpr chrono::duration>
+operator"" min(long double __mins)
+{ return chrono::duration>{__mins}; }
 
-  } // __detail
+template 
+  constexpr typename
+  __detail::_Select_type<__select_int::_Select_int<_Digits...>::value,
+chrono::minutes>::type
+  operator"" min()
+  {
+   return __detail::_Select_type<
+ __select_int::_Select_int<_Digits...>::value,
+ chrono::minutes>::value;
+  }
 
-  constexpr chrono::duration>
-  operator"" h(long double __hours)
-  { return chrono::duration>{__hours}; }
+constexpr chrono::duration
+operator"" s(long double __secs)
+{ return chrono::duration{__secs}; }
 
-  template 
-constexpr typename
-__detail::_Select_type<__select_int::_Select_int<_Digits...>::value,
-  chrono::hours>::type
-operator"" h()
-{
-  return __detail::_Select_type<
-   __select_int::_Select_int<_Digits...>::value,
-   chrono::hours>::value;
-}
+template 
+  constexpr typename
+  __detail::_Select_type<__select_int::_Select_int<_Digits...>::value,
+chrono::seconds>::type
+  operator"" s()
+  {
+   return __detail::_Select_type<
+ __select_int::_Select_int<_Digits...>::value,
+ chrono::seconds>::value;
+  }
 
-  constexpr chrono::duration>
-  operator"" min(long double __mins)
-  { return chrono::duration>{__mins}; }
+constexpr chrono::duratio

Re: [Patch, Fortran] PR57549 - fix type-spec handling with array constructors

[Steve Kargl]

On Fri, Jun 07, 2013 at 04:03:52PM +0200, Tobias Burnus wrote:
> Unreviewed patches:
> * http://gcc.gnu.org/ml/fortran/2013-06/msg00027.html
> * http://gcc.gnu.org/ml/fortran/2013-06/msg00048.html
> 
>   * * *
> 
> As with ALLOCATE (type-spec :: ...), also array constructors take as 
> type-spec a type name ("integer, my_dt, etc.) without an extra TYPE().
> 
> Thus, instead of using the decl.c var decl function, we use the same as 
> for allocate.
> 
> Build and regtested on x86-64-gnu-linux.
> OK for the trunk?
> 

OK.  Thanks for the patch.

-- 
Steve

Re: RFA: Switching LRA on for s390

[Andreas Krebbel]

On Fri, May 31, 2013 at 02:11:38PM -0400, Vladimir Makarov wrote:
>   The following patch switches LRA on for s390.  The patch introduces
> a new option -mlra to use LRA instead of reload.  I did not document
> the option as I'd like to delete it for gcc4.9.  By default, LRA will
> be used for s390 instead of reload for better testing LRA for s390.
> 
>   Changes in s390.md are because of define_splits for the last
> alternative *movmem_short, *clrmem_short, *cmpmem_short have guard
> TARGET_CPU_ZARCH.
> 
>   The patch was successfully bootstrapped and tested on s390x.
> 
>   Any comments?
>   Is it ok to commit the patch into trunk?
> 
>   Thanks.
> 
> 
> 2013-05-31  Vladimir Makarov  
> 
> * config/s390/s390.opt (mlra): New option.
> * config/s390/s390.c (s390_decompose_address): Check displacement
> for all registers for LRA.
> (s390_secondary_reload): Don't used secondary reloads for LRA.
> (s390_lra_p): New function.
> (TARGET_LRA_P): Define.
> * config/s390/s390.md (*movmem_short, *clrmem_short): Change value
> of attribute cpu_facility to zarch for the last alternative.
> (*cmpmem_short): Ditto.

I've applied the attached patch. This helps me getting a little
further when bootstrapping with lra and --with-arch=zEC12.

2013-06-07  Andreas Krebbel  

* config/s390/s390.md (cpu_facility): Add cpu_zarch.
("*movmem_short", "*clrmem_short", "*cmpmem_short): Use cpu_zarch
for last alternative in the cpu_facility attribute.

---
 gcc/config/s390/s390.md |   13 !
 1 file changed, 4 insertions(+), 9 modifications(!)

Index: gcc/config/s390/s390.md
===
*** gcc/config/s390/s390.md.orig
--- gcc/config/s390/s390.md
***
*** 277,283 
  (define_attr "cpu" "g5,g6,z900,z990,z9_109,z9_ec,z10,z196,zEC12"
(const (symbol_ref "s390_tune_attr")))
  
! (define_attr "cpu_facility" 
"standard,ieee,zarch,longdisp,extimm,dfp,z10,z196,zEC12"
(const_string "standard"))
  
  (define_attr "enabled" ""
--- 277,284 
  (define_attr "cpu" "g5,g6,z900,z990,z9_109,z9_ec,z10,z196,zEC12"
(const (symbol_ref "s390_tune_attr")))
  
! (define_attr "cpu_facility"
!   "standard,ieee,zarch,cpu_zarch,longdisp,extimm,dfp,z10,z196,zEC12"
(const_string "standard"))
  
  (define_attr "enabled" ""
***
*** 304,309 
--- 305,314 
  (match_test "TARGET_DFP"))
 (const_int 1)
  
+  (and (eq_attr "cpu_facility" "cpu_zarch")
+   (match_test "TARGET_CPU_ZARCH"))
+(const_int 1)
+ 
   (and (eq_attr "cpu_facility" "z10")
(match_test "TARGET_Z10"))
 (const_int 1)
***
*** 2690,2696 
"(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)"
"#"
[(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,zarch")])
  
  (define_split
[(set (match_operand:BLK 0 "memory_operand" "")
--- 2695,2701 
"(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)"
"#"
[(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,cpu_zarch")])
  
  (define_split
[(set (match_operand:BLK 0 "memory_operand" "")
***
*** 2899,2905 
"(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)"
"#"
[(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,zarch")])
  
  (define_split
[(set (match_operand:BLK 0 "memory_operand" "")
--- 2904,2910 
"(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)"
"#"
[(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,cpu_zarch")])
  
  (define_split
[(set (match_operand:BLK 0 "memory_operand" "")
***
*** 3075,3081 
"(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)"
"#"
[(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,zarch")])
  
  (define_split
[(set (reg:CCU CC_REGNUM)
--- 3080,3086 
"(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)"
"#"
[(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,cpu_zarch")])
  
  (define_split
[(set (reg:CCU CC_REGNUM)

Re: [GOOGLE] More strict checking for call args

[Dehao Chen]

On Fri, Jun 7, 2013 at 6:47 AM, Richard Biener
 wrote:
> On Fri, Jun 7, 2013 at 3:30 PM, Xinliang David Li  wrote:
>> On Fri, Jun 7, 2013 at 2:05 AM, Richard Biener
>>  wrote:
>>> On Thu, Jun 6, 2013 at 5:10 PM, Dehao Chen  wrote:
 Hi, Martin,

 Yes, your patch can fix my case. Thanks a lot for the fix.

 With the fix, value profiling will still promote the wrong indirect
 call target. Though it will not be inlining, but it results in an
 additional check. How about in check_ic_target, after calling
 gimple_check_call_matching_types, we also check if number of args
 match number of params in target->symbol.decl?
>>>
>>> I wonder what's the point in the gimple_check_call_matching_types check
>>> in the profiling case.  It's at least no longer
>>>
>>> /* Perform sanity check on the indirect call target. Due to race conditions,
>>>false function target may be attributed to an indirect call site. If the
>>>call expression type mismatches with the target function's type, 
>>> expand_call
>>>may ICE.
>>>
>>> because since the introduction of gimple_call_fntype we will _not_ ICE.
>>>
>>> Thus I argue that check_ic_target should be even removed instead of
>>> enhancing it!
>>>
>>
>> Another reason is what Dehao had mentioned -- wrong target leads to
>> useless transformation.
>
> Sure, but a not wrong in the sense of the predicate does not guarantee
> a useful transformation either.

True, but at least it will filter some obvious bad transformations.

Another important factor is for AutoFDO, which uses source lineno to
map profile to IR. So when the source has changed, it could literally
map anything to an indirect call target. check_ic_target can actually
filter out most of the incorrect targets.

Thanks,
Dehao

>
>>> How does IC profiling determine the called target?  That is, what does it
>>> do when the target is not always the same?  (because the checking code
>>> talks about race conditions for example)
>>
>>
>> The race condition is the happening at instrumentation time -- the
>> indirect call counters are not thread local. We have seen this a lot
>> in the past that a totally bogus target is attributed to a indirect
>> callsite.
>
> So it simply uses whatever function was called last?  Instead of
> using the function that was called most of the time?
>
> Richard.
>
>> thanks,
>>
>> David
>>>
>>> Richard.
>>>
>>>
 Thanks,
 Dehao


 On Thu, Jun 6, 2013 at 7:11 AM, Martin Jambor  wrote:
>
> Hi,
>
> On Tue, Jun 04, 2013 at 05:19:02PM -0700, Dehao Chen wrote:
> > attached is a testcase that would cause problem when source has changed:
> >
> > $ g++ test.cc -O2 -fprofile-generate -DOLD
> > $ ./a.out
> > $ g++ test.cc -O2 -fprofile-use
> > test.cc:34:1: internal compiler error: in operator[], at vec.h:815
> >  }
> >  ^
> > 0x512740 vec::operator[](unsigned int)
> > ../../gcc/vec.h:815
> > 0x512740 vec::operator[](unsigned int)
> > ../../gcc/vec.h:1244
> > 0xf24464 vec::operator[](unsigned int)
> > ../../gcc/vec.h:815
> > 0xf24464 vec::operator[](unsigned int)
> > ../../gcc/vec.h:1244
> > 0xf24464 ipa_get_indirect_edge_target_1
> > ../../gcc/ipa-cp.c:1535
> > 0x971b9a estimate_edge_devirt_benefit
> > ../../gcc/ipa-inline-analysis.c:2757
>
> Hm, this seems rather like an omission in ipa_get_indirect_edge_target_1.
> Since it is called also from inlining, we can have parameter count
> mismatches... and in fact in non-virtual paths of that function we do
> check that we don't.  Because all callers have to pass known_vals
> describing all formal parameters of the inline tree root, we should
> apply the fix below (I've only just started running a bootstrap and
> testsuite on x86_64, though).
>
> OTOH, while I understand that FDO can change inlining sufficiently so
> that this error occurs, IMHO this should not be caused by outdated
> profiles but there is somewhere a parameter mismatch in the source.
>
> Dehao, can you please check that this patch helps?
>
> Richi, if it does and the patch passes bootstrap and tests, is it OK
> for trunk and 4.8 branch?
>
> Thanks and sorry for the trouble,
>
> Martin
>
>
> 2013-06-06  Martin Jambor  
>
> * ipa-cp.c (ipa_get_indirect_edge_target_1): Check that 
> param_index is
> within bounds at the beginning of the function.
>
> Index: src/gcc/ipa-cp.c
> ===
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -1481,7 +1481,8 @@ ipa_get_indirect_edge_target_1 (struct c
>tree otr_type;
>tree t;
>
> -  if (param_index == -1)
> +  if (param_index == -1
> +  || known_vals.length () <= (unsigned int) param_index)
>  return NULL_TREE;
>
>if (!ie->indirect_inf

Re: RFA: Switching LRA on for s390

[Vladimir Makarov]

On 13-06-07 10:57 AM, Andreas Krebbel wrote:

On Fri, May 31, 2013 at 02:11:38PM -0400, Vladimir Makarov wrote:

   The following patch switches LRA on for s390.  The patch introduces
a new option -mlra to use LRA instead of reload.  I did not document
the option as I'd like to delete it for gcc4.9.  By default, LRA will
be used for s390 instead of reload for better testing LRA for s390.

   Changes in s390.md are because of define_splits for the last
alternative *movmem_short, *clrmem_short, *cmpmem_short have guard
TARGET_CPU_ZARCH.

   The patch was successfully bootstrapped and tested on s390x.

   Any comments?
   Is it ok to commit the patch into trunk?

   Thanks.


2013-05-31  Vladimir Makarov  

 * config/s390/s390.opt (mlra): New option.
 * config/s390/s390.c (s390_decompose_address): Check displacement
 for all registers for LRA.
 (s390_secondary_reload): Don't used secondary reloads for LRA.
 (s390_lra_p): New function.
 (TARGET_LRA_P): Define.
 * config/s390/s390.md (*movmem_short, *clrmem_short): Change value
 of attribute cpu_facility to zarch for the last alternative.
 (*cmpmem_short): Ditto.

I've applied the attached patch. This helps me getting a little
further when bootstrapping with lra and --with-arch=zEC12.

2013-06-07  Andreas Krebbel  

* config/s390/s390.md (cpu_facility): Add cpu_zarch.
("*movmem_short", "*clrmem_short", "*cmpmem_short): Use cpu_zarch
for last alternative in the cpu_facility attribute.

---
  gcc/config/s390/s390.md |   13 !
  1 file changed, 4 insertions(+), 9 modifications(!)

Index: gcc/config/s390/s390.md
===
*** gcc/config/s390/s390.md.orig
--- gcc/config/s390/s390.md
***
*** 277,283 
   (define_attr "cpu" "g5,g6,z900,z990,z9_109,z9_ec,z10,z196,zEC12"
 (const (symbol_ref "s390_tune_attr")))
   
! (define_attr "cpu_facility" "standard,ieee,zarch,longdisp,extimm,dfp,z10,z196,zEC12"

 (const_string "standard"))
   
   (define_attr "enabled" ""

--- 277,284 
   (define_attr "cpu" "g5,g6,z900,z990,z9_109,z9_ec,z10,z196,zEC12"
 (const (symbol_ref "s390_tune_attr")))
   
! (define_attr "cpu_facility"

!   "standard,ieee,zarch,cpu_zarch,longdisp,extimm,dfp,z10,z196,zEC12"
 (const_string "standard"))
   
   (define_attr "enabled" ""

***
*** 304,309 
--- 305,314 
  (match_test "TARGET_DFP"))
 (const_int 1)
   
+  (and (eq_attr "cpu_facility" "cpu_zarch")

+   (match_test "TARGET_CPU_ZARCH"))
+(const_int 1)
+
(and (eq_attr "cpu_facility" "z10")
 (match_test "TARGET_Z10"))
 (const_int 1)
***
*** 2690,2696 
 "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)"
 "#"
 [(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,zarch")])
   
   (define_split

 [(set (match_operand:BLK 0 "memory_operand" "")
--- 2695,2701 
 "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)"
 "#"
 [(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,cpu_zarch")])
   
   (define_split

 [(set (match_operand:BLK 0 "memory_operand" "")
***
*** 2899,2905 
 "(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)"
 "#"
 [(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,zarch")])
   
   (define_split

 [(set (match_operand:BLK 0 "memory_operand" "")
--- 2904,2910 
 "(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)"
 "#"
 [(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,cpu_zarch")])
   
   (define_split

 [(set (match_operand:BLK 0 "memory_operand" "")
***
*** 3075,3081 
 "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)"
 "#"
 [(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,zarch")])
   
   (define_split

 [(set (reg:CCU CC_REGNUM)
--- 3080,3086 
 "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)"
 "#"
 [(set_attr "type" "cs")
!(set_attr "cpu_facility" "*,*,z10,cpu_zarch")])
   
   (define_split

 [(set (reg:CCU CC_REGNUM)

Thanks, Andreas.  I am not a specialist in 390 architecture.  You are 
the expert.  I'll check the bootstrap with this option too on a machine 
available for me.  I've checked the bootstrap without any -with-arch 
option before submitting the patch for approval.  If I find some 
problems, I'll work on them too.  I'll work on PR57599 you just reported 
too.

[Patch tree-ssa] RFC: Enable path threading for control variables (PR tree-optimization/54742).

[James Greenhalgh]

Hello,

After seeing Steve Ellcey's patch at:
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00667.html
I've been playing with pushing the same logic closer to the
current jump-threading framework.

I've split off Steve's patch into path search code (added to
tree-ssa-threadedge.c) and path copy code (added to
tree-ssa-threadupdate.c).

This patch survives bootstrap and regression runs on arm and
x86 with no problems. As it stands it is unconditionally added
in the same places jump-threading would be, making it active
at -O1 and above. With this, I saw no significant impact in
compile times measured by --with-build-config=bootstrap-time.

I'm not sure this is what Jeff Law intended when he asked that
the code be moved to the existing jump-threading framework, I
found gimple_duplicate_sese_region did a much better job of
doing the right thing than I had expected, thus had no need to
play with the block update code in tree-ssa-threadupdate.c.

As for deciding when it is good to perform this optimisation,
I use Steve's max_insn_count and max_path_count values, though
clearly if this approach seems acceptable I can add command line
flags as appropriate.

In terms of touching generic code I had to make a change to the
gimple_duplicate_sese_region code to patch up when I am copying
across loop headers/latches which are not entry->dest/src. I use
the assumption that, having copied a region, if block n was the
loop header of the original region, block n will be the loop
header of the new region and use this to patch up new loop
information.

My other generic change ensures that
simplify_control_stmt_condition in tree-ssa-threadedge.c returns
something for us to analyse even if it cannot simplify to a
gimple min invariant.

Beyond that, the path search code is modified from Steve's patch
to only perform one depth first search, and the path copy code
is modified to purge the region entry and exit edges for those
which traditional jump-threading may consider.

Opinions?

Thanks,
James Greenhalgh

---
gcc/

2013-06-07  James Greenhalgh  

PR tree-optimization/54742
* tree.cfg (gimple_duplicate_sese_region): Memoize loop latch
and loop header blocks if copying across a latch/header.
* tree-flow.h (thread_paths): New struct.
(register_thread_paths): Declare.
* tree-ssa-threadedge.c
(simplify_control_stmt_condition): Permit returning something
not in gimple_min_invariant form.
(max_insn_count): Define.
(max_path_count): Likewise.
(find_thread_path_1): New function.
(find_thread_path): Likewise.
(save_new_thread_path): Likewise.
(find_control_statement_thread_paths): Likewise.
(thread_across_edge): Handle non gimple_min_invariant cases.
* tree-ssa-threadupdate.c (thread_paths_vec): New.
(remove_edge_from_thread_candidates): New function.
(duplicate_thread_path): Likewise.
(copy_control_statement_thread_paths): Likewise.
(thread_through_all_blocks): Handle thread_paths.
(register_thread_paths): New function.
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 4b91a35..6dcd2e4 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -5717,10 +5717,12 @@ gimple_duplicate_sese_region (edge entry, edge exit,
 {
   unsigned i;
   bool free_region_copy = false, copying_header = false;
+  bool save_loop_details = false;
   struct loop *loop = entry->dest->loop_father;
   edge exit_copy;
   vec doms;
   edge redirected;
+  int memo_loop_header_no = 0, memo_loop_latch_no = 0;
   int total_freq = 0, entry_freq = 0;
   gcov_type total_count = 0, entry_count = 0;
 
@@ -5738,9 +5740,15 @@ gimple_duplicate_sese_region (edge entry, edge exit,
   if (region[i]->loop_father != loop)
 	return false;
 
-  if (region[i] != entry->dest
-	  && region[i] == loop->header)
-	return false;
+  /* If we are copying an abnormally shaped region, keep track of
+	 which block will become our loop header.  */
+  if ((region[i] != entry->dest && region[i] == loop->header)
+	  ||(region[i] != entry->src && region[i] == loop->latch))
+	{
+	  save_loop_details = true;
+	  memo_loop_latch_no = i;
+	  memo_loop_header_no = i;
+	}
 }
 
   set_loop_copy (loop, loop);
@@ -5821,6 +5829,13 @@ gimple_duplicate_sese_region (edge entry, edge exit,
   loop->latch = exit->src;
 }
 
+  /* Restore loop details if we were asked to save them.  */
+  if (save_loop_details)
+{
+  loop->header = region_copy[memo_loop_header_no];
+  loop->latch = region_copy[memo_loop_latch_no];
+}
+
   /* Redirect the entry and add the phi node arguments.  */
   redirected = redirect_edge_and_branch (entry, get_bb_copy (entry->dest));
   gcc_assert (redirected != NULL);
diff --git a/gcc/tree-flow.h b/gcc/tree-flow.h
index 24fcfbf..c16a1ba 100644
--- a/gcc/tree-flow.h
+++ b/gcc/tree-flow.h
@@ -604,6 +604,13 @@ struct tree_niter_desc
   enum tree_code cmp;
 };
 
+typedef struct thread_paths
+{
+

[c++, driver] Add -lrt on Solaris

[Rainer Orth]

This is a resubmission of a patch originally posted here

http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01488.html

It was only intended as a stop-gap measure to avoid a couple of
libstdc++ testsuite failures on Solaris 9 and 10 where nanosleep only
lives in librt, which is now required since --enable-libstdcxx-time=auto
became the default.  On Solaris 11, librt has been folded into libc and
librt only remains as a filter on libc, so linking with unnecessary,
though not really harmful.

As I mentioned in the thread above, I don't really like this solution
for two reasons:

* It splits the knowledge about libstdc++'s need for librt between
  libstdc++ and gcc configury.

* It unconditionally links librt into every C++ program instead of doing
  this only when necessary.

Therefore I'd suggested an alternative approach, which unfortunately is
far more intrusive:

Follow libgfortran's lead and introduce libstdc++.spec.  Here,
target-specific linker switches can be determined by the libstdc++
configury, with configuration well localized, and one can easily handle
stuff like linking -lrt with -z ignore/-z record
resp. --as-needed/--no-as-needed.  This provides us with a general
extension mechanism for linking C++ programs, which can be used for
other purposes if the need arises.

The downside of course is that the testsuite needs to be taught about
that file by passing the appropriate -B switches to find it at compile
time.

One way or another, this needs to be adressed to fix the libstdc++
testsuite failures on Solaris.

Thoughts?

Rainer


2013-05-24  Rainer Orth  

gcc/cp:
* g++spec.c (TIMELIB): Define.
(WITHLIBC, SKIPOPT): Adjust values.
(lang_specific_driver): Add TIME_LIBRARY if not passed explicitly.

gcc:
* config/sol2.h (TIME_LIBRARY): Define.

# HG changeset patch
# Parent d6881ec042d3a6328b763cbf0f38e61bdbb64d79
Add -lrt on Solaris

diff --git a/gcc/config/sol2.h b/gcc/config/sol2.h
--- a/gcc/config/sol2.h
+++ b/gcc/config/sol2.h
@@ -194,6 +194,9 @@ along with GCC; see the file COPYING3.  
 #endif /* HAVE_LD_EH_FRAME && TARGET_DL_ITERATE_PHDR */
 #endif
 
+/* C++11 programs need -lrt for nanosleep.  */
+#define TIME_LIBRARY "rt"
+
 #ifndef USE_GLD
 /* The default MFLIB_SPEC is GNU ld specific.  */
 #define MFLIB_SPEC ""
diff --git a/gcc/cp/g++spec.c b/gcc/cp/g++spec.c
--- a/gcc/cp/g++spec.c
+++ b/gcc/cp/g++spec.c
@@ -28,10 +28,12 @@ along with GCC; see the file COPYING3.  
 #define LANGSPEC	(1<<1)
 /* This bit is set if they did `-lm' or `-lmath'.  */
 #define MATHLIB		(1<<2)
+/* This bit is set if they did `-lrt' or equivalent.  */
+#define TIMELIB		(1<<3)
 /* This bit is set if they did `-lc'.  */
-#define WITHLIBC	(1<<3)
+#define WITHLIBC	(1<<4)
 /* Skip this option.  */
-#define SKIPOPT		(1<<4)
+#define SKIPOPT		(1<<5)
 
 #ifndef MATH_LIBRARY
 #define MATH_LIBRARY "m"
@@ -40,6 +42,10 @@ along with GCC; see the file COPYING3.  
 #define MATH_LIBRARY_PROFILE MATH_LIBRARY
 #endif
 
+#ifndef TIME_LIBRARY
+#define TIME_LIBRARY ""
+#endif
+
 #ifndef LIBSTDCXX
 #define LIBSTDCXX "stdc++"
 #endif
@@ -83,16 +89,22 @@ lang_specific_driver (struct cl_decoded_
   /* "-lm" or "-lmath" if it appears on the command line.  */
   const struct cl_decoded_option *saw_math = NULL;
 
+  /* "-lrt" or eqivalent if it appears on the command line.  */
+  const struct cl_decoded_option *saw_time = NULL;
+
   /* "-lc" if it appears on the command line.  */
   const struct cl_decoded_option *saw_libc = NULL;
 
   /* An array used to flag each argument that needs a bit set for
- LANGSPEC, MATHLIB, or WITHLIBC.  */
+ LANGSPEC, MATHLIB, TIMELIB, or WITHLIBC.  */
   int *args;
 
   /* By default, we throw on the math library if we have one.  */
   int need_math = (MATH_LIBRARY[0] != '\0');
 
+  /* By default, we throw on the time library if we have one.  */
+  int need_time = (TIME_LIBRARY[0] != '\0');
+
   /* True if we saw -static.  */
   int static_link = 0;
 
@@ -136,6 +148,11 @@ lang_specific_driver (struct cl_decoded_
 	  args[i] |= MATHLIB;
 	  need_math = 0;
 	}
+	  else if (strcmp (arg, TIME_LIBRARY) == 0)
+	{
+	  args[i] |= TIMELIB;
+	  need_time = 0;
+	}
 	  else if (strcmp (arg, "c") == 0)
 	args[i] |= WITHLIBC;
 	  else
@@ -268,6 +285,12 @@ lang_specific_driver (struct cl_decoded_
 	  saw_math = &decoded_options[i];
 	}
 
+  if (!saw_time && (args[i] & TIMELIB) && library > 0)
+	{
+	  --j;
+	  saw_time = &decoded_options[i];
+	}
+
   if (!saw_libc && (args[i] & WITHLIBC) && library > 0)
 	{
 	  --j;
@@ -352,6 +375,15 @@ lang_specific_driver (struct cl_decoded_
   added_libraries++;
   j++;
 }
+  if (saw_time)
+new_decoded_options[j++] = *saw_time;
+  else if (library > 0 && need_time)
+{
+  generate_option (OPT_l, TIME_LIBRARY, 1, CL_DRIVER,
+		   &new_decoded_options[j]);
+  added_libraries++;
+  j++;
+}
   if (saw_libc)
 new_decoded_options[j++] = *saw_libc;
  

Re: [c++, driver] Add -lrt on Solaris

[Jonathan Wakely]

I haven't looked at the patch yet and my knowledge of spec files is
pretty weak, so sorry if these are stupid questions ...


On 7 June 2013 16:45, Rainer Orth wrote:
>
> Follow libgfortran's lead and introduce libstdc++.spec.  Here,
> target-specific linker switches can be determined by the libstdc++
> configury, with configuration well localized, and one can easily handle
> stuff like linking -lrt with -z ignore/-z record
> resp. --as-needed/--no-as-needed.  This provides us with a general
> extension mechanism for linking C++ programs, which can be used for
> other purposes if the need arises.

Could this later be extended to serve as the basis for a -cxxthread
switch that does platform-specific things to allow the C++11 thread
library to be used?  I don't like that users must use -pthread for
std::thread and std::mutex to work, because they shouldn't need to
know or care whether it uses Pthreads internally, or some other thread
model.


> The downside of course is that the testsuite needs to be taught about
> that file by passing the appropriate -B switches to find it at compile
> time.

Is that only necessary for testing an uninstalled libstdc++?  In other
words, would the spec file be installed and used automatically for an
installed libstdc++?

> One way or another, this needs to be adressed to fix the libstdc++
> testsuite failures on Solaris.

The other option is to make --enable-libstdcxx-time=auto only
effective for Solaris 11, but I think you said you'd prefer to keep it
for 9 and 10.

Re: [c++, driver] Add -lrt on Solaris

[Rainer Orth]

Jonathan Wakely  writes:

> On 7 June 2013 16:45, Rainer Orth wrote:
>>
>> Follow libgfortran's lead and introduce libstdc++.spec.  Here,
>> target-specific linker switches can be determined by the libstdc++
>> configury, with configuration well localized, and one can easily handle
>> stuff like linking -lrt with -z ignore/-z record
>> resp. --as-needed/--no-as-needed.  This provides us with a general
>> extension mechanism for linking C++ programs, which can be used for
>> other purposes if the need arises.
>
> Could this later be extended to serve as the basis for a -cxxthread
> switch that does platform-specific things to allow the C++11 thread
> library to be used?  I don't like that users must use -pthread for

Sure.

> std::thread and std::mutex to work, because they shouldn't need to
> know or care whether it uses Pthreads internally, or some other thread
> model.

Exactly: that's the sort of implementation detail users shouldn't need
to know or care about.

>> The downside of course is that the testsuite needs to be taught about
>> that file by passing the appropriate -B switches to find it at compile
>> time.
>
> Is that only necessary for testing an uninstalled libstdc++?  In other
> words, would the spec file be installed and used automatically for an
> installed libstdc++?

Of course: that's what gfortran does with libgfortran.spec: it gets
installed into $libdir and is searched there automatically.

>> One way or another, this needs to be adressed to fix the libstdc++
>> testsuite failures on Solaris.
>
> The other option is to make --enable-libstdcxx-time=auto only
> effective for Solaris 11, but I think you said you'd prefer to keep it
> for 9 and 10.

Right: if the target (independent of Solaris) has the necessary support
for some feature, libstdc++ should be able to use it.  You wouldn't
think about disabling thread support before Solaris 11 to avoid linking
libstdc++.so with -lpthread?

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

[PATCH] Move sharable cilkplus functions to c-family

[Iyer, Balaji V]

Hello Richard et al.,
I looked at my C++ implementation for Array Notations and a bunch of 
helper functions (and one structure) can be shared between C and C++. Attached 
is a patch that will move these functions from c/c-array-notation.c to 
c-family/array-notation-common.c. 

Is this OK for the trunk?

Here are the ChangeLog entries

gcc/c/ChangeLog
2013-06-07  Balaji V. Iyer  

* array-notation-common.c (length_mismatch_in_expr_p): Moved this
function to c-family/array-notation-common.c.
(is_cilkplus_reduce_builtin): Likewise.
(find_rank): Likewise.
(extract_array_notation_exprs): Likewise.
(replace_array_notations): Likewise.
(find_inv_trees): Likewise.
(replace_inv_trees): Likewise.
(contains_array_notation_expr): Likewise.
(find_correct_array_notation_type): Likewise.
(replace_invariant_exprs): Initialized additional_tcodes to NULL.
(struct inv_list): Moved this to c-family/array-notation-common.c.
* c-tree.h (is_cilkplus_builtin_reduce): Remove prototype.

gcc/c-family/ChangeLog
2013-06-07  Balaji V. Iyer  

* array-notation-common.c (length_mismatch_in_expr_p): Moved this
function from c/c-array-notation.c.
(is_cilkplus_reduce_builtin): Likewise.
(find_rank): Likewise.
(extract_array_notation_exprs): Likewise.
(replace_array_notations): Likewise.
(find_inv_trees): Likewise.
(replace_inv_trees): Likewise.
(contains_array_notation_expr): Likewise.
(find_correct_array_notation_type): Likewise.
* c-common.h (struct inv_list): Moved this struct from the file
c/c-array-notation.c and added a new field called additional tcodes.
(length_mismatch_in_expr_p): New prototype.
(is_cilkplus_reduce_builtin): Likewise.
(find_rank): Likewise.
(extract_array_notation_exprs): Likewise.
(replace_array_notation): Likewise.
(find_inv_trees): Likewise.
(replace_inv_trees): Likewise.
(find_correct_array_notation_type): Likewise.

Thanks,

Balaji V. Iyer.
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
old mode 100644
new mode 100755
index d434a2f..8fcedc6
Binary files a/gcc/c-family/ChangeLog and b/gcc/c-family/ChangeLog differ
diff --git a/gcc/c-family/array-notation-common.c 
b/gcc/c-family/array-notation-common.c
old mode 100644
new mode 100755
index 1d28846..489b67c
--- a/gcc/c-family/array-notation-common.c
+++ b/gcc/c-family/array-notation-common.c
@@ -27,9 +27,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree.h"
 #include "langhooks.h" 
 #include "tree-iterator.h"
+#include "c-family/c-common.h"
 #include "diagnostic-core.h"
 
-
 /* Returns true if the function call in FNDECL is  __sec_implicit_index.  */
 
 bool
@@ -74,3 +74,489 @@ extract_sec_implicit_index_arg (location_t location, tree 
fn)
 }
   return return_int;
 }
+
+/* Returns true if there is length mismatch among expressions
+   on the same dimension and on the same side of the equal sign.  The
+   expressions (or ARRAY_NOTATION lengths) are passed in through 2-D array
+   **LIST where X and Y indicate first and second dimension sizes of LIST,
+   respectively.  */
+
+bool
+length_mismatch_in_expr_p (location_t loc, tree **list, size_t x, size_t y)
+{
+  size_t ii, jj;
+  tree start = NULL_TREE;
+  HOST_WIDE_INT l_start, l_node;
+  for (jj = 0; jj < y; jj++)
+{
+  start = NULL_TREE;
+  for (ii = 0; ii < x; ii++)
+   {
+ if (!start)
+   start = list[ii][jj];
+ else if (TREE_CODE (start) == INTEGER_CST)
+   {
+ /* If start is a INTEGER, and list[ii][jj] is an integer then
+check if they are equal.  If they are not equal then return
+true.  */
+ if (TREE_CODE (list[ii][jj]) == INTEGER_CST)
+   {
+ l_node = int_cst_value (list[ii][jj]);
+ l_start = int_cst_value (start);
+ if (absu_hwi (l_start) != absu_hwi (l_node))
+   {
+ error_at (loc, "length mismatch in expression");
+ return true;
+   }
+   }
+   }
+ else
+   /* We set the start node as the current node just in case it turns
+  out to be an integer.  */
+   start = list[ii][jj];
+   }
+}
+  return false;
+}
+
+/* Given an FNDECL of type FUNCTION_DECL or ADDR_EXPR, return the corresponding
+   BUILT_IN_CILKPLUS_SEC_REDUCE_* being called.  If none, return
+   BUILT_IN_NONE.  */
+
+enum built_in_function
+is_cilkplus_reduce_builtin (tree fndecl)
+{
+  if (!fndecl)
+return BUILT_IN_NONE;
+  if (TREE_CODE (fndecl) == ADDR_EXPR)
+fndecl = TREE_OPERAND (fndecl, 0);
+
+  if (TREE_CODE (fndecl) == FUNCTION_DECL
+  && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
+switch (DECL_FUNCTION_CODE (

Re: Symtab cleanups 2/17 - merge alias code

[Marcus Shawcroft]

On 1 June 2013 14:06, Jan Hubicka  wrote:
> Hi,
> this patch cleanups way we handle aliases.  The main point is to merge code
> that was previously done separately for variables and functions.
>

Hello,  This patch appears to break both arm and aarch64.  I don't
fully understand the mechanism.  The issue shows up when building
res_libc.o over in glibc. The following fragment demonstrates the
issue:

/* aarch64-none-linux-gnueabi-gcc -O2 -c -std=gnu99 -fgnu89-inline r.c  */
struct __res_state {
 int retrans;
};
extern __thread struct __res_state *__libc_resp __attribute__
((tls_model ("initial-exec")));

struct __res_state _res __attribute__((section (".bss")));
__thread struct __res_state *__resp = &_res;
extern __thread struct __res_state *__libc_resp  __attribute__ ((alias
("__resp"))) ;

int
__res_init(void) {
  (*__libc_resp).retrans = 5;
}


We seem to lose track of the alias between _resp and __llbc_resp
resulting in the following output:

.size __res_init, .-__res_init
.global __resp
.global __libc_resp
__libc_resp = __resp
.global _res
.section .bss,"aw",%nobits
.align 3
.type _res, %object
.size _res, 4
_res:
.zero 4
.section .tbss,"awT",%nobits
.align 3
.LANCHOR0 = . + 0
.type __libc_resp, %object
.size __libc_resp, 8
__libc_resp:
.zero 8
.section .tdata,"awT",%progbits
.align 3
.type __resp, %object
.size __resp, 8
__resp:
.xword _res

Note the definition of both __libc_resp and __resp, where previously
we would have defined only __resp.

I suspect the section anchor appeared because
varasm.c:use_blocks_for_decl_p test for an 'alias' failed.

/Marcus

[Patch, Fortran] PR57553 - fix two STORAGE_SIZE bugs

[Tobias Burnus]

This patch fixes two issues:
*  storage_size('aa')  was rejected as constant expression - as 
ts.u.cl->length == 0.
* In trans*.c, there was a fold_convert missing (-> ICE). Additionally, 
I have replaced the detour to generate a tree containing the value "8" 
via a fortran expression.


Build and regtested on x86-64-gnu-linux.
OK for the trunk?

Tobias

2013-06-07  Tobias Burnus  

	PR fortran/57553
	* simplify.c (gfc_simplify_storage_size): Handle literal
	strings.
	* trans-intrinsic.c (gfc_conv_intrinsic_storage_size):
	Add missing fold_convert.

diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 815043b..683d58b 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -5717,7 +5717,7 @@ gfc_simplify_storage_size (gfc_expr *x,
   if (x->ts.type == BT_CLASS || x->ts.deferred)
 return NULL;
 
-  if (x->ts.type == BT_CHARACTER
+  if (x->ts.type == BT_CHARACTER && x->expr_type != EXPR_CONSTANT
   && (!x->ts.u.cl || !x->ts.u.cl->length
 	  || x->ts.u.cl->length->expr_type != EXPR_CONSTANT))
 return NULL;
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index eca907e..3fbf193 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -5249,12 +5249,10 @@ static void
 gfc_conv_intrinsic_storage_size (gfc_se *se, gfc_expr *expr)
 {
   gfc_expr *arg;
-  gfc_se argse,eight;
+  gfc_se argse;
   tree type, result_type, tmp;
 
   arg = expr->value.function.actual->expr;
-  gfc_init_se (&eight, NULL);
-  gfc_conv_expr (&eight, gfc_get_int_expr (expr->ts.kind, NULL, 8));
   
   gfc_init_se (&argse, NULL);
   result_type = gfc_get_int_type (expr->ts.kind);
@@ -5285,11 +5283,12 @@ gfc_conv_intrinsic_storage_size (gfc_se *se, gfc_expr *expr)
   if (arg->ts.type == BT_CHARACTER)
 tmp = size_of_string_in_bytes (arg->ts.kind, argse.string_length);
   else
-tmp = fold_convert (result_type, size_in_bytes (type)); 
+tmp = size_in_bytes (type); 
+  tmp = fold_convert (result_type, tmp);
 
 done:
   se->expr = fold_build2_loc (input_location, MULT_EXPR, result_type, tmp,
-			  eight.expr);
+			  build_int_cst (result_type, BITS_PER_UNIT));
   gfc_add_block_to_block (&se->pre, &argse.pre);
 }
 
--- /dev/null	2013-06-07 09:13:23.024185858 +0200
+++ gcc/gcc/testsuite/gfortran.dg/storage_size_4.f90	2013-06-07 17:34:12.719284544 +0200
@@ -0,0 +1,23 @@
+! { dg-do compile }
+! { dg-options "-fdump-tree-original" }
+!
+! PR fortran/57553
+!
+! Ensure that there is no ICE and that compile-time simplication works.
+!
+  use iso_fortran_env
+  implicit none
+  integer, parameter :: ESize = storage_size('a')
+  integer, parameter :: ESize2 = storage_size('aa')
+  if ( ESize/CHARACTER_STORAGE_SIZE /= 1) call abort()
+  if ( ESize2/CHARACTER_STORAGE_SIZE /= 2) call abort()
+end
+
+subroutine S ( A )
+  character(len=*), intent(in) :: A
+  integer :: ESize = 4
+  esize = ( storage_size(a) + 7 ) / 8
+end
+
+! { dg-final { scan-tree-dump-not "abort" "original" } }
+! { dg-final { cleanup-tree-dump "original" } }

Re: [PATCH] Move sharable cilkplus functions to c-family

[Richard Henderson]

On 06/07/2013 09:08 AM, Iyer, Balaji V wrote:
> Hello Richard et al.,
>   I looked at my C++ implementation for Array Notations and a bunch of 
> helper functions (and one structure) can be shared between C and C++. 
> Attached is a patch that will move these functions from c/c-array-notation.c 
> to c-family/array-notation-common.c. 
> 
> Is this OK for the trunk?

Yes, this is ok.


r~

Re: Symtab cleanups 2/17 - merge alias code

[Jan Hubicka]

> On 1 June 2013 14:06, Jan Hubicka  wrote:
> > Hi,
> > this patch cleanups way we handle aliases.  The main point is to merge code
> > that was previously done separately for variables and functions.
> >
> 
> Hello,  This patch appears to break both arm and aarch64.  I don't
> fully understand the mechanism.  The issue shows up when building
> res_libc.o over in glibc. The following fragment demonstrates the
> issue:
> 
> I suspect the section anchor appeared because
> varasm.c:use_blocks_for_decl_p test for an 'alias' failed.

You are probably right.  I added code that removes alias attributes once
they land symtab. The reason is that I run into wrong code issues with code
picking up the symbol name from alias attribute that is no longer valid
after LTO renaming.
Apparently this code and also symbol equality testing in fold-const rely
on alias attribute but do not worry about what alias it is.
I will revert this part of patch and try to solve this incrementally.
(the code in fold-const is wrong anyway)

Honza
> 
> /Marcus

Re: [PATCH] ARMv6-M MI thunk fix

[Cesar Philippidis]

On 6/6/13 9:00 AM, Richard Earnshaw wrote:
> The pipeline offset is 4 for Thumb2 as well.  So at the very least you
> need to explain why your change doesn't apply then as well.

Yes some context is lost in that comment.  Thunks are usually emitted in
ARM mode, except for Thumb-only targets.  Is the new comment OK?  If so,
please check it in since I do not have SVN write access.

Thanks,
Cesar


2013-06-07  Julian Brown  
Cesar Philippidis  

gcc/
* config/arm/arm.c (arm_output_mi_thunk): Fix offset for
TARGET_THUMB1_ONLY.
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c(revision 199695)
+++ gcc/config/arm/arm.c(working copy)
@@ -25217,7 +25217,12 @@
{
  /* Output ".word .LTHUNKn-7-.LTHUNKPCn".  */
  rtx tem = XEXP (DECL_RTL (function), 0);
- tem = gen_rtx_PLUS (GET_MODE (tem), tem, GEN_INT (-7));
+ /* When supported, thunks are generated in ARM mode.  But for 
+TARGET_THUMB1_ONLY the thunk is in Thumb mode, so the PC 
+pipeline offset is four rather than eight.  Adjust the 
+offset accordingly.  */
+ tem = gen_rtx_PLUS (GET_MODE (tem), tem,
+ GEN_INT (TARGET_THUMB1_ONLY ? -3 : -7));
  tem = gen_rtx_MINUS (GET_MODE (tem),
   tem,
   gen_rtx_SYMBOL_REF (Pmode,

Fix tree-ssa/attr-alias.c testcase

[Jan Hubicka]

Hi,
on some testers attr-alias.c fails because ( in the template is interpreted by
dejagnu rather than part of the pattern.  This patch fixes it by simply testing
for indentation of the call (to not match the declaration nor function body
dump header).

Comitted as obvious.

Honza

Index: ChangeLog
===
--- ChangeLog   (revision 199820)
+++ ChangeLog   (working copy)
@@ -1,3 +1,7 @@
+2013-06-07  Jan Hubicka  
+
+   * gcc.dg/tree-ssa/attr-alias.c: Remove brackets in template.
+
 2013-06-07  Tobias Burnus  
 
PR fortran/57549
Index: gcc.dg/tree-ssa/attr-alias.c
===
--- gcc.dg/tree-ssa/attr-alias.c(revision 199820)
+++ gcc.dg/tree-ssa/attr-alias.c(working copy)
@@ -21,8 +21,8 @@
 /* calls to test1 and test2 can be inlined and optmized away. Calls
to test and test4 are overwritable.  */
 
-/* { dg-final { scan-tree-dump-times "test (" 2 "optimized" } } */
-/* { dg-final { scan-tree-dump-times "test4 (" 1 "optimized" } } */
-/* { dg-final { scan-tree-dump-not "test1 (" "optimized" } } */
-/* { dg-final { scan-tree-dump-not "test2 (" "optimized" } } */
+/* { dg-final { scan-tree-dump-times "  test " 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "  test4 " 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "  test1 " "optimized" } } */
+/* { dg-final { scan-tree-dump-not "  test2 " "optimized" } } */
 /* { dg-final { cleanup-tree-dump "optimized" } } */

[cilkplus] cleanup C++ pragma simd implementation

[Aldy Hernandez]

The attached patch cleans up the C++ pragma simd implementation to share 
more things with the C front-end, particularly the type checking.  For 
instance, I got rid of all the specialized parsing routines to just use 
generic cp_parser_*expression* and then perform the actual typechecking 
in the C shared routines.


I also reworded some of the tests to match the C FE so we can share the 
tests there too.


There are also some fixes scattered throughout, all in an effort to pass 
all the c-c++-common Cilk Plus pragma simd tests.  Almost there... :).


One bug I found that I don't know how to handle is:

@@ -30315,6 +30319,9 @@ cp_parser_simd_for_init_statement (cp_parser 
*parser, tree *init,

  && CLASS_TYPE_P (TREE_TYPE (decl)))
{
  tree rhs, new_expr;
+ // ?? FIXME: I don't see any definition for *init in this
+ // code path. ??
+ gcc_unreachable ();
  cp_parser_parse_definitely (parser);
  cp_parser_require (parser, CPP_EQ, RT_EQ);

AFAICT, *init is not initialized in this code path and the caller uses 
it.  Balaji, what's the intent here?


Committing to the aldyh/cilk-in-gomp branch.
commit 51d8e99dd59ec4beb523101a35eedd21f2d439a4
Author: Aldy Hernandez 
Date:   Fri Jun 7 12:06:12 2013 -0500

Clean up C++ implementation of pragma simd to use shared C/C++ type
checking.

Change error messages to match C front-end when possible.

Misc fixes.

diff --git a/gcc/c-family/c-cilkplus.c b/gcc/c-family/c-cilkplus.c
index 84f1645..c02851e 100644
--- a/gcc/c-family/c-cilkplus.c
+++ b/gcc/c-family/c-cilkplus.c
@@ -207,12 +207,16 @@ c_check_cilk_loop_body (tree body)
 /* Validate a _Cilk_for construct (or a #pragma simd for loop, which
has the same syntactic restrictions).  Returns TRUE if there were
no errors, FALSE otherwise.  LOC is the location of the for.  DECL
-   is the controlling variable.  COND is the condition.  INCR is the
-   increment expression.  BODY is the body of the LOOP.  */
+   is the controlling variable.  COND is the condition.  INCRP is a
+   pointer the increment expression (in case, the increment needs to
+   be canonicalized).  BODY is the body of the LOOP.  */
 
 static bool
-c_check_cilk_loop (location_t loc, tree decl, tree cond, tree incr, tree body)
+c_check_cilk_loop (location_t loc, tree decl, tree cond, tree *incrp,
+  tree body)
 {
+  tree incr = *incrp;
+
   if (decl == error_mark_node
   || cond == error_mark_node 
   || incr == error_mark_node
@@ -284,10 +288,11 @@ c_check_cilk_loop (location_t loc, tree decl, tree cond, 
tree incr, tree body)
   return false;
 }
 
-  /* Validate the increment.  */
+  /* Validate and canonicalize the increment.  */
   incr = c_check_cilk_loop_incr (loc, decl, incr);
   if (incr == error_mark_node)
 return false;
+  *incrp = incr;
 
   if (!c_check_cilk_loop_body (body))
 return false;
@@ -325,7 +330,7 @@ c_finish_cilk_simd_loop (location_t loc,
 {
   location_t rhs_loc;
 
-  if (!c_check_cilk_loop (loc, decl, cond, incr, body))
+  if (!c_check_cilk_loop (loc, decl, cond, &incr, body))
 return NULL;
 
   /* In the case of "for (int i = 0...)", init will be a decl.  It should
@@ -345,7 +350,11 @@ c_finish_cilk_simd_loop (location_t loc,
   init = build_modify_expr (loc, decl, NULL_TREE, NOP_EXPR, rhs_loc,
init, NULL_TREE);
 }
-  gcc_assert (TREE_CODE (init) == MODIFY_EXPR);
+
+  // The C++ parser just gives us the rhs.
+  if (TREE_CODE (init) != MODIFY_EXPR)
+init = build2 (MODIFY_EXPR, void_type_node, decl, init);
+
   gcc_assert (TREE_OPERAND (init, 0) == decl);
 
   tree initv = make_tree_vec (1);
diff --git a/gcc/cp/ChangeLog.cilkplus b/gcc/cp/ChangeLog.cilkplus
index 7254b81..e7f7596 100644
--- a/gcc/cp/ChangeLog.cilkplus
+++ b/gcc/cp/ChangeLog.cilkplus
@@ -1,9 +1,12 @@
 2013-05-21  Balaji V. Iyer  
+   Aldy Hernandez  
 
* cp-tree.h (p_simd_valid_stmts_in_body_p): New prototype.
+   (finish_cilk_for_cond): Likewise.
* parser.h (IN_CILK_P_SIMD_FOR): New #define.
* Make-lang.in (CXX_AND_OBJCXX_OBJS): Added new obj-file cp-cilkplus.o
* cp-cilkplus.c: New file.
+   * semantics.c (finish_cilk_for_cond): New.
* parser.c (cp_parser_pragma): Added a PRAGMA_CILK_SIMD case.
(cp_parser_cilk_simd_vectorlength): New function.
(cp_parser_cilk_simd_linear): Likewise.
diff --git a/gcc/cp/cp-cilkplus.c b/gcc/cp/cp-cilkplus.c
index f387e2f..e6bdf8a 100644
--- a/gcc/cp/cp-cilkplus.c
+++ b/gcc/cp/cp-cilkplus.c
@@ -46,9 +46,6 @@ cpp_validate_cilk_plus_loop_aux (tree *tp, int 
*walk_subtrees, void *data)
   if (!tp || !*tp)
 return NULL_TREE;
 
-  // Call generic C version.
-  (void) c_validate_cilk_plus_loop (tp, walk_subtrees, data);
-  
   if (TREE_CODE (*tp) == THROW_EXPR)
 {
   error_at (loc, "throw expressions are not allowed inside loops "
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[David Edelsohn]

On Thu, May 30, 2013 at 6:10 PM, Iyer, Balaji V  wrote:

> I think David is getting the correct output but just that dg-error is not 
> catching it correctly.

What version of expect are you using?

Fedora 18 apparently has 5.45, others are using 5.44, and AIX 7.1
provides 5.42.1.

Thanks, David

[PATCH] tree-into-ssa.c: make interesting_blocks static

[David Malcolm]

"interesting_blocks" is only used inside tree-into-ssa.c, so it can be
made static.

Successfully bootstrapped on x86_64-unknown-linux-gnu (using
gcc-4.7.2-2.fc17.x86_64).

OK for trunk?

2013-06-07  David Malcolm  

* tree-into-ssa.c (interesting_blocks): Make static.

Index: gcc/tree-into-ssa.c
===
--- gcc/tree-into-ssa.c	(revision 199816)
+++ gcc/tree-into-ssa.c	(working copy)
@@ -93,7 +93,7 @@
the operations done on them are presence tests.  */
 static sbitmap new_ssa_names;
 
-sbitmap interesting_blocks;
+static sbitmap interesting_blocks;
 
 /* Set of SSA names that have been marked to be released after they
were registered in the replacement table.  They will be finally

[PATCH] tree-object-size.c: mark "unknown" as const

[David Malcolm]

The array "unknown" within tree-object-size.c is only ever read from,
not written to, so it can be marked as const.

Successfully bootstrapped on x86_64-unknown-linux-gnu (using
gcc-4.7.2-2.fc17.x86_64).

OK for trunk?

2013-06-07  David Malcolm  

* tree-object-size.c (unknown): Make const.


Index: gcc/tree-object-size.c
===
--- gcc/tree-object-size.c	(revision 199816)
+++ gcc/tree-object-size.c	(working copy)
@@ -39,7 +39,7 @@
   unsigned int *stack, *tos;
 };
 
-static unsigned HOST_WIDE_INT unknown[4] = { -1, -1, 0, 0 };
+static const unsigned HOST_WIDE_INT unknown[4] = { -1, -1, 0, 0 };
 
 static tree compute_object_offset (const_tree, const_tree);
 static unsigned HOST_WIDE_INT addr_object_size (struct object_size_info *,

PR57548 - Call to multiversioned function from global namespace

[Sriraman Tallam]

Hi,

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

The ICE here is because of a multi-versioned function called from
global namespace and has no caller.  This ICEs in target hook
ix86_can_inline_p as caller is 0x0.  The following simple patch
attached fixes this problem.

* cp/call.c (build_over_call):  Check if current_function_decl is
NULL.
* testsuite/g++.dg/ext/pr57548.C: New test.

Ok to submit?

Thanks
Sri
This patch fixes PR 57548.  The problem here is that the caller to fum
not from a function and current_function_decl is NULL when processing the
call.  The simple fix in call.c to check current_function_decl before
calling can_inline_p target hook.

* cp/call.c (build_over_call):  Check if current_function_decl is
NULL.
* testsuite/g++.dg/ext/pr57548.C: New test.


Index: cp/call.c
===
--- cp/call.c   (revision 199662)
+++ cp/call.c   (working copy)
@@ -7053,7 +7053,8 @@ build_over_call (struct z_candidate *cand, int fla
  otherwise the call should go through the dispatcher.  */
 
   if (DECL_FUNCTION_VERSIONED (fn)
-  && !targetm.target_option.can_inline_p (current_function_decl, fn))
+  && (current_function_decl == NULL
+ || !targetm.target_option.can_inline_p (current_function_decl, fn)))
 {
   fn = get_function_version_dispatcher (fn);
   if (fn == NULL)
Index: testsuite/g++.dg/ext/pr57548.C
===
--- testsuite/g++.dg/ext/pr57548.C  (revision 0)
+++ testsuite/g++.dg/ext/pr57548.C  (revision 0)
@@ -0,0 +1,25 @@
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-require-ifunc "" }  */
+
+int fum (); // Extra declaration that is merged with the second one.
+int fum () __attribute__ ((target("default")));
+
+
+int fum () __attribute__((target( "mmx")));
+int fum () __attribute__((target( "popcnt")));
+int fum () __attribute__((target( "sse")));
+int fum () __attribute__((target( "sse2")));
+int fum () __attribute__((target( "sse3")));
+int fum () __attribute__((target( "ssse3")));
+int fum () __attribute__((target( "sse4.1")));
+int fum () __attribute__((target( "sse4.2")));
+int fum () __attribute__((target( "avx")));
+int fum () __attribute__((target( "avx2")));
+
+int fum () __attribute__((target("arch=core2")));
+int fum () __attribute__((target("arch=corei7")));
+int fum () __attribute__((target("arch=atom")));
+
+int (*p)() = &fum;
+
+int j = fum();

Re: [PATCH] tree-object-size.c: mark "unknown" as const

[Jakub Jelinek]

On Fri, Jun 07, 2013 at 01:40:00PM -0400, David Malcolm wrote:
> The array "unknown" within tree-object-size.c is only ever read from,
> not written to, so it can be marked as const.
> 
> Successfully bootstrapped on x86_64-unknown-linux-gnu (using
> gcc-4.7.2-2.fc17.x86_64).
> 
> OK for trunk?
> 
> 2013-06-07  David Malcolm  
> 
>   * tree-object-size.c (unknown): Make const.

Ok, thanks.

Jakub

Re: [PATCH] tree-object-size.c: mark "unknown" as const

[David Malcolm]

On Fri, 2013-06-07 at 20:49 +0200, Jakub Jelinek wrote:
> On Fri, Jun 07, 2013 at 01:40:00PM -0400, David Malcolm wrote:
> > The array "unknown" within tree-object-size.c is only ever read from,
> > not written to, so it can be marked as const.
> > 
> > Successfully bootstrapped on x86_64-unknown-linux-gnu (using
> > gcc-4.7.2-2.fc17.x86_64).
> > 
> > OK for trunk?
> > 
> > 2013-06-07  David Malcolm  
> > 
> > * tree-object-size.c (unknown): Make const.
> 
> Ok, thanks.

Thanks; committed to SVN trunk as r199832.

Re: [PATCH, rs6000] power8 patches, patch #9, power8 scheduling

[Pat Haugen]

This patch adds instruction scheduling support for the Power8 processor. 
Bootstrap/regression test with no new failures. Ok for trunk?



2013-06-07  Michael Meissner  
Pat Haugen 
Peter Bergner 

* config/rs6000/power8.md: New.
* config/rs6000/rs6000-cpus.def (RS6000_CPU table): Adjust processor
setting for power8 entry.
* config/rs6000/t-rs6000 (MD_INCLUDES): Add power8.md.
* config/rs6000/rs6000.c (is_microcoded_insn, is_cracked_insn): Adjust
test for Power4/Power5 only.
(insn_must_be_first_in_group, insn_must_be_last_in_group): Add Power8
support.
(force_new_group): Adjust comment.
* config/rs6000/rs6000.md: Include power8.md.



Index: gcc/config/rs6000/power8.md
===
--- gcc/config/rs6000/power8.md (revision 0)
+++ gcc/config/rs6000/power8.md (revision 0)
@@ -0,0 +1,373 @@
+;; Scheduling description for IBM POWER8 processor.
+;; Copyright (C) 2013 Free Software Foundation, Inc.
+;;
+;; Contributed by Pat Haugen (pthau...@us.ibm.com).
+
+;; This file is part of GCC.
+;;
+;; GCC is free software; you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published
+;; by the Free Software Foundation; either version 3, or (at your
+;; option) any later version.
+;;
+;; GCC is distributed in the hope that it will be useful, but WITHOUT
+;; ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+;; or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+;; License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with GCC; see the file COPYING3.  If not see
+;; .
+
+(define_automaton "power8fxu,power8lsu,power8vsu,power8misc")
+
+(define_cpu_unit "fxu0_power8,fxu1_power8" "power8fxu")
+(define_cpu_unit "lu0_power8,lu1_power8" "power8lsu")
+(define_cpu_unit "lsu0_power8,lsu1_power8" "power8lsu")
+(define_cpu_unit "vsu0_power8,vsu1_power8" "power8vsu")
+(define_cpu_unit "bpu_power8,cru_power8" "power8misc")
+(define_cpu_unit "du0_power8,du1_power8,du2_power8,du3_power8,du4_power8,\
+ du5_power8,du6_power8"  "power8misc")
+
+
+; Dispatch group reservations
+(define_reservation "DU_any_power8"
+   "du0_power8|du1_power8|du2_power8|du3_power8|du4_power8|\
+du5_power8")
+
+; 2-way Cracked instructions go in slots 0-1
+;   (can also have a second in slots 3-4 if insns are adjacent)
+(define_reservation "DU_cracked_power8"
+   "du0_power8+du1_power8")
+
+; Insns that are first in group
+(define_reservation "DU_first_power8"
+   "du0_power8")
+
+; Insns that are first and last in group
+(define_reservation "DU_both_power8"
+   "du0_power8+du1_power8+du2_power8+du3_power8+du4_power8+\
+du5_power8+du6_power8")
+
+; Dispatch slots are allocated in order conforming to program order.
+(absence_set "du0_power8" "du1_power8,du2_power8,du3_power8,du4_power8,\
+ du5_power8,du6_power8")
+(absence_set "du1_power8" "du2_power8,du3_power8,du4_power8,du5_power8,\
+ du6_power8")
+(absence_set "du2_power8" "du3_power8,du4_power8,du5_power8,du6_power8")
+(absence_set "du3_power8" "du4_power8,du5_power8,du6_power8")
+(absence_set "du4_power8" "du5_power8,du6_power8")
+(absence_set "du5_power8" "du6_power8")
+
+
+; Execution unit reservations
+(define_reservation "FXU_power8"
+"fxu0_power8|fxu1_power8")
+
+(define_reservation "LU_power8"
+"lu0_power8|lu1_power8")
+
+(define_reservation "LSU_power8"
+"lsu0_power8|lsu1_power8")
+
+(define_reservation "LU_or_LSU_power8"
+"lu0_power8|lu1_power8|lsu0_power8|lsu1_power8")
+
+(define_reservation "VSU_power8"
+"vsu0_power8|vsu1_power8")
+
+
+; LS Unit
+(define_insn_reservation "power8-load" 3
+  (and (eq_attr "type" "load")
+   (eq_attr "cpu" "power8"))
+  "DU_any_power8,LU_or_LSU_power8")
+
+(define_insn_reservation "power8-load-update" 3
+  (and (eq_attr "type" "load_u,load_ux")
+   (eq_attr "cpu" "power8"))
+  "DU_cracked_power8,LU_or_LSU_power8+FXU_power8")
+
+(define_insn_reservation "power8-load-ext" 3
+  (and (eq_attr "type" "load_ext")
+   (eq_attr "cpu" "power8"))
+  "DU_cracked_power8,LU_or_LSU_power8,FXU_power8")
+
+(define_insn_reservation "power8-load-ext-update" 3
+  (and (eq_attr "type" "load_ext_u,load_ext_ux")
+   (eq_attr "cpu" "power8"))
+  "DU_both_power8,LU_or_LSU_power8+FXU_power8,FXU_power8")
+
+(define_insn_reservation "power8-fpload" 5
+  (and (eq_attr "type" "fpload,vecload")
+   (eq_attr "cpu" "power8"))
+  "DU_any_power8,LU_power8")
+
+(define_insn_reservation "power8-fpload-update" 5
+  (and (eq_attr "type" "fpload_u,fpload_ux")
+   (eq_attr "cpu" "power8"))
+  "DU_cracked_power8,LU_power8+FXU_power8")
+
+(define_insn_reservation "power8-sto

[PATCH] DATA_ALIGNMENT vs. DATA_ABI_ALIGNMENT (PR target/56564)

[Jakub Jelinek]

Hi!

This PR is about DATA_ALIGNMENT macro increasing alignment of some decls
for optimization purposes beyond ABI mandated levels.  It is fine to emit
the vars aligned as much as we want for optimization purposes, but if we
can't be sure that references to that decl bind to the definition we
increased the alignment on (e.g. common variables, or -fpic code without
hidden visibility, weak vars etc.), we can't assume that alignment.
As DECL_ALIGN is used for both the alignment emitted for the definitions
and alignment assumed on code referring to it, this patch increases
DECL_ALIGN only on decls where decl_binds_to_current_def_p is true,
and otherwise the optimization part on top of that emits only when
aligning definition.
On x86_64, DATA_ALIGNMENT macro was partly an optimization, partly ABI
mandated alignment increase, so I've introduced a new macro,
DATA_ABI_ALIGNMENT, which is the ABI mandated increase only (on x86-64
I think the only one is that arrays with size 16 bytes or more (and VLAs,
but that is not handled by DATA*ALIGNMENT) are at least 16 byte aligned).

Bootstrapped/regtested on x86_64-linux and i686-linux.  No idea about other
targets, I've kept them all using DATA_ALIGNMENT, which is considered
optimization increase only now, if there is some ABI mandated alignment
increase on other targets, that should be done in DATA_ABI_ALIGNMENT as
well as DATA_ALIGNMENT.  The patch causes some vectorization regressions
(tweaked in the testsuite), especially for common vars where we used to
align say common arrays to 256 bits rather than the ABI mandated 128 bits,
or for -fpic code, but I'm afraid we need to live with that, if you compile
another file with say icc or some other compiler which doesn't increase
alignment beyond ABI mandated level and that other file defines the var say
as non-common, we have wrong-code.

2013-06-07  Jakub Jelinek  

PR target/56564
* varasm.c (align_variable): Don't use DATA_ALIGNMENT or
CONSTANT_ALIGNMENT if !decl_binds_to_current_def_p (decl).
Use DATA_ABI_ALIGNMENT for that case instead if defined.
(get_variable_align): New function.
(get_variable_section, emit_bss, emit_common,
assemble_variable_contents, place_block_symbol): Use
get_variable_align instead of DECL_ALIGN.
(assemble_noswitch_variable): Add align argument, use it
instead of DECL_ALIGN.
(assemble_variable): Adjust caller.  Use get_variable_align
instead of DECL_ALIGN.
* config/i386/i386.h (DATA_ALIGNMENT): Adjust x86_data_alignment
caller.
(DATA_ABI_ALIGNMENT): Define.
* config/i386/i386-protos.h (x86_data_alignment): Adjust prototype.
* config/i386/i386.c (x86_data_alignment): Add opt argument.  If
opt is false, only return the psABI mandated alignment increase.
* doc/tm.texi.in (DATA_ABI_ALIGNMENT): Document.
* doc/tm.texi: Regenerated.

* gcc.target/i386/pr56564-1.c: New test.
* gcc.target/i386/pr56564-2.c: New test.
* gcc.target/i386/pr56564-3.c: New test.
* gcc.target/i386/pr56564-4.c: New test.
* gcc.target/i386/avx256-unaligned-load-4.c: Add -fno-common.
* gcc.target/i386/avx256-unaligned-store-1.c: Likewise.
* gcc.target/i386/avx256-unaligned-store-3.c: Likewise.
* gcc.target/i386/avx256-unaligned-store-4.c: Likewise.
* gcc.target/i386/vect-sizes-1.c: Likewise.
* gcc.target/i386/memcpy-1.c: Likewise.
* gcc.dg/vect/costmodel/i386/costmodel-vect-31.c (tmp): Initialize.
* gcc.dg/vect/costmodel/x86_64/costmodel-vect-31.c (tmp): Likewise.

--- gcc/varasm.c.jj 2013-06-07 13:17:17.0 +0200
+++ gcc/varasm.c2013-06-07 15:38:36.710908852 +0200
@@ -966,8 +966,12 @@ align_variable (tree decl, bool dont_out
   align = MAX_OFILE_ALIGNMENT;
 }
 
-  /* On some machines, it is good to increase alignment sometimes.  */
-  if (! DECL_USER_ALIGN (decl))
+  /* On some machines, it is good to increase alignment sometimes.
+ But as DECL_ALIGN is used both for actually emitting the variable
+ and for code accessing the variable as guaranteed alignment, we
+ can only increase the alignment if it is a performance optimization
+ if the references to it must bind to the current definition.  */
+  if (! DECL_USER_ALIGN (decl) && decl_binds_to_current_def_p (decl))
 {
 #ifdef DATA_ALIGNMENT
   unsigned int data_align = DATA_ALIGNMENT (TREE_TYPE (decl), align);
@@ -988,12 +992,69 @@ align_variable (tree decl, bool dont_out
}
 #endif
 }
+#ifdef DATA_ABI_ALIGNMENT
+  else if (! DECL_USER_ALIGN (decl))
+{
+  unsigned int data_align = DATA_ABI_ALIGNMENT (TREE_TYPE (decl), align);
+  /* For backwards compatibility, don't assume the ABI alignment for
+TLS variables.  */
+  if (! DECL_THREAD_LOCAL_P (decl) || data_align <= BITS_PER_WORD)
+   align = data_align;
+}
+#endif
 
   /* Reset the 

Re: PR57548 - Call to multiversioned function from global namespace

[Jason Merrill]

OK.

Jason

Re: Symtab cleanups 2/17 - merge alias code

[Jan Hubicka]

> > On 1 June 2013 14:06, Jan Hubicka  wrote:
> > > Hi,
> > > this patch cleanups way we handle aliases.  The main point is to merge 
> > > code
> > > that was previously done separately for variables and functions.
> > >
> > 
> > Hello,  This patch appears to break both arm and aarch64.  I don't
> > fully understand the mechanism.  The issue shows up when building
> > res_libc.o over in glibc. The following fragment demonstrates the
> > issue:
> > 
> > I suspect the section anchor appeared because
> > varasm.c:use_blocks_for_decl_p test for an 'alias' failed.
> 
> You are probably right.  I added code that removes alias attributes once
> they land symtab. The reason is that I run into wrong code issues with code
> picking up the symbol name from alias attribute that is no longer valid
> after LTO renaming.
> Apparently this code and also symbol equality testing in fold-const rely
> on alias attribute but do not worry about what alias it is.
> I will revert this part of patch and try to solve this incrementally.
> (the code in fold-const is wrong anyway)
Hi,
I have comitted the following.  Does it solve your problem?

Honza

Index: ChangeLog
===
--- ChangeLog   (revision 199834)
+++ ChangeLog   (working copy)
@@ -1,3 +1,7 @@
+2013-06-07  Jan Hubicka  
+
+   * symtab.c (symtab_resolve_alias): Do not remove alias attribute.
+
 2013-06-07  David Malcolm  
 
* tree-object-size.c (unknown): Make const.
Index: symtab.c
===
--- symtab.c(revision 199698)
+++ symtab.c(working copy)
@@ -978,8 +978,6 @@ symtab_resolve_alias (symtab_node node,
  We do not want to keep it around or we would have to mind updating them
  when renaming symbols.  */
   node->symbol.alias_target = NULL;
-  DECL_ATTRIBUTES (node->symbol.decl)
- = remove_attribute ("alias", DECL_ATTRIBUTES (node->symbol.decl));
 
   if (node->symbol.cpp_implicit_alias && cgraph_state >= 
CGRAPH_STATE_CONSTRUCTION)
 fixup_same_cpp_alias_visibility (node, target);

[C++ patch] Fix PR 57551

[Jan Hubicka]

Hi,
please see the PR log for details.  My symtab patch made testcase anon6.C to
fail because it tests that unused static var is output into the assembly and we
now optimize it out.

Adding attribute used however shows problem that the var suddenly becomes WEAK
and COMDAT. It seems to me that this is bug in C++ FE. Watchpointing the decl
I found that the decl is first correctly brought static by constrain_visibility
but later it is exported again by mark_decl_instantiated.

I think mark_decl_instantiated is wrong. I am not at all sure if the following
patch is proper fix though.

After this patch at least GCC agrees with clang on the testcase.

Bootstrapped/regtested x86_64-linux, OK?

Honza

PR c++/57551
* cp/pt.c (mark_decl_instantiated): Do not export explicit 
instantiations
of anonymous namespace templates.

* g++.dg/ext/visibility/anon6.C: Update testcase.

Index: cp/pt.c
===
--- cp/pt.c (revision 199698)
+++ cp/pt.c (working copy)
@@ -17402,6 +17402,13 @@ mark_decl_instantiated (tree result, int
   if (TREE_ASM_WRITTEN (result))
 return;
 
+  /* For anonymous namespace we don't need to do anything.  */
+  if (decl_anon_ns_mem_p (result))
+{
+  gcc_assert (!TREE_PUBLIC (result));
+  return;
+}
+
   if (TREE_CODE (result) != FUNCTION_DECL)
 /* The TREE_PUBLIC flag for function declarations will have been
set correctly by tsubst.  */
Index: testsuite/g++.dg/ext/visibility/anon6.C
===
--- testsuite/g++.dg/ext/visibility/anon6.C (revision 199698)
+++ testsuite/g++.dg/ext/visibility/anon6.C (working copy)
@@ -1,6 +1,8 @@
 // PR c++/33094
 // { dg-final { scan-assembler "1BIiE1cE" } }
 // { dg-final { scan-assembler-not "globl.*1BIiE1cE" } }
+// { dg-final { scan-assembler-not "comdat" } }
+// { dg-final { scan-assembler-not "weak" } }
 // { dg-final { scan-assembler-not "1CIiE1cE" } }
 
 // Test that B::c is emitted as an internal symbol, and C::c is
@@ -18,7 +20,7 @@ namespace
   template 
   class B
   {
-static const T c = 0;
+__attribute__ ((__used__)) static const T c = 0;
   };
 
   template  const T B::c;

Re: [PATCH] DATA_ALIGNMENT vs. DATA_ABI_ALIGNMENT (PR target/56564)

[Richard Henderson]

On 06/07/2013 12:25 PM, Jakub Jelinek wrote:
> This PR is about DATA_ALIGNMENT macro increasing alignment of some decls
> for optimization purposes beyond ABI mandated levels.  It is fine to emit
> the vars aligned as much as we want for optimization purposes, but if we
> can't be sure that references to that decl bind to the definition we
> increased the alignment on (e.g. common variables, or -fpic code without
> hidden visibility, weak vars etc.), we can't assume that alignment.

When the linker merges common blocks, it chooses both maximum size and maximum
alignment.  Thus for any common block for which we can prove the block must
reside in the module (any executable, or hidden common in shared object), we
can go ahead and use the increased alignment.

It's only in shared objects with non-hidden common blocks that we have a
problem, since in that case we may resolve the common block via copy reloc to a
memory block in another module.

So while decl_binds_to_current_def_p is a good starting point, I think we can
do a little better with common blocks.  Which ought to take care of those
vectorization regressions you mention.

> @@ -966,8 +966,12 @@ align_variable (tree decl, bool dont_out
>align = MAX_OFILE_ALIGNMENT;
>  }
>  
> -  /* On some machines, it is good to increase alignment sometimes.  */
> -  if (! DECL_USER_ALIGN (decl))
> +  /* On some machines, it is good to increase alignment sometimes.
> + But as DECL_ALIGN is used both for actually emitting the variable
> + and for code accessing the variable as guaranteed alignment, we
> + can only increase the alignment if it is a performance optimization
> + if the references to it must bind to the current definition.  */
> +  if (! DECL_USER_ALIGN (decl) && decl_binds_to_current_def_p (decl))
>  {
>  #ifdef DATA_ALIGNMENT
>unsigned int data_align = DATA_ALIGNMENT (TREE_TYPE (decl), align);
> @@ -988,12 +992,69 @@ align_variable (tree decl, bool dont_out
>   }
>  #endif
>  }
> +#ifdef DATA_ABI_ALIGNMENT
> +  else if (! DECL_USER_ALIGN (decl))
> +{
> +  unsigned int data_align = DATA_ABI_ALIGNMENT (TREE_TYPE (decl), align);
> +  /* For backwards compatibility, don't assume the ABI alignment for
> +  TLS variables.  */
> +  if (! DECL_THREAD_LOCAL_P (decl) || data_align <= BITS_PER_WORD)
> + align = data_align;
> +}
> +#endif

This structure would seem to do the wrong thing if DATA_ABI_ALIGNMENT is
defined, but DATA_ALIGNMENT isn't.  And while I realize you documented it, I
don't like the restriction that D_A /must/ return something larger than D_A_A.
 All that means is that in complex cases D_A will have to call D_A_A itself.

I would think that it would be better to rearrange as

  if (!D_U_A)
{
  #ifdef D_A_A
  align = ...
  #endif
  #ifdef D_A
  if (d_b_t_c_d_p)
align = ...
  #endif
}

Why the special case for TLS?  If we really want that special case surely that
test should go into D_A_A itself, and not here in generic code.

> Bootstrapped/regtested on x86_64-linux and i686-linux.  No idea about other
> targets, I've kept them all using DATA_ALIGNMENT, which is considered
> optimization increase only now, if there is some ABI mandated alignment
> increase on other targets, that should be done in DATA_ABI_ALIGNMENT as
> well as DATA_ALIGNMENT.

I've had a brief look over the instances of D_A within the tree atm.  Most of
them carry the cut-n-paste comment "for the same reasons".  These I believe
never intended an ABI change, and were really only interested in optimization.

But these I think require a good hard look to see if they really intended an
ABI alignment:

c6x comment explicitly mentions abi
criscompiler options for alignment -- systemwide or local?
mmixcomment mentions GETA instruction
s390comment mentions LARL instruction
rs6000  SPE and E500 portion of the alignment non-optional?

Relevant port maintainers CCed.


r~

Re: [PATCH] DATA_ALIGNMENT vs. DATA_ABI_ALIGNMENT (PR target/56564)

[Jakub Jelinek]

On Fri, Jun 07, 2013 at 01:43:27PM -0700, Richard Henderson wrote:
> On 06/07/2013 12:25 PM, Jakub Jelinek wrote:
> > This PR is about DATA_ALIGNMENT macro increasing alignment of some decls
> > for optimization purposes beyond ABI mandated levels.  It is fine to emit
> > the vars aligned as much as we want for optimization purposes, but if we
> > can't be sure that references to that decl bind to the definition we
> > increased the alignment on (e.g. common variables, or -fpic code without
> > hidden visibility, weak vars etc.), we can't assume that alignment.
> 
> When the linker merges common blocks, it chooses both maximum size and maximum
> alignment.  Thus for any common block for which we can prove the block must
> reside in the module (any executable, or hidden common in shared object), we
> can go ahead and use the increased alignment.

But consider say:
one TU:
struct S { char buf[15]; } s __attribute__((aligned (32)));

another TU:
char c = 7;
struct S { char buf[15]; } s = { { 1, 2 } };
char d = 8;
int main () { return 0; }

(the aligned(32) is there just to simulate the DATA_ALIGNMENT optimization
increase).  Linker warns about this (thus the question is if we want to
increase the alignment for optimization on commons at all) and doesn't align
it.

> This structure would seem to do the wrong thing if DATA_ABI_ALIGNMENT is
> defined, but DATA_ALIGNMENT isn't.  And while I realize you documented it, I
> don't like the restriction that D_A /must/ return something larger than D_A_A.
>  All that means is that in complex cases D_A will have to call D_A_A itself.

Yeah, I guess I can rearrange it.  The reason I wrote it that way was to
avoid an extra function call, but that is probably not big enough overhead.

> I would think that it would be better to rearrange as
> 
>   if (!D_U_A)
> {
>   #ifdef D_A_A
>   align = ...
>   #endif
>   #ifdef D_A
>   if (d_b_t_c_d_p)
> align = ...
>   #endif
> }
> 
> Why the special case for TLS?  If we really want that special case surely that
> test should go into D_A_A itself, and not here in generic code.

I special case TLS for backwards ABI compatibility with GCC <= 4.8.1,
because we ignored DATA_ALIGNMENT for TLS vars, even if what it returned
wasn't just an optimization, but ABI requirement.  So, if you have:
__thread char a[16] = { 1, 2 };
in one shared library and
__thread char a[16] = { 1, 2 };
use (a);
in another shared library, compile one with GCC 4.8.1 e.g. and the other one
with 4.9.0, if use (a) would assume the psABI mandated 16 byte alignment,
but the actual definition was from 4.8.1 compiled object and was only 1 byte
aligned, it could crash.  Also, neither DATA_ALIGNMENT nor DATA_ABI_ALIGNMENT 
sees
the decl itself, it just looks at type and previously assumed alignment.

> > Bootstrapped/regtested on x86_64-linux and i686-linux.  No idea about other
> > targets, I've kept them all using DATA_ALIGNMENT, which is considered
> > optimization increase only now, if there is some ABI mandated alignment
> > increase on other targets, that should be done in DATA_ABI_ALIGNMENT as
> > well as DATA_ALIGNMENT.
> 
> I've had a brief look over the instances of D_A within the tree atm.  Most of
> them carry the cut-n-paste comment "for the same reasons".  These I believe
> never intended an ABI change, and were really only interested in optimization.

Thanks for looking into this.

> But these I think require a good hard look to see if they really intended an
> ABI alignment:
> 
> c6x   comment explicitly mentions abi
> cris  compiler options for alignment -- systemwide or local?
> mmix  comment mentions GETA instruction
> s390  comment mentions LARL instruction
> rs6000SPE and E500 portion of the alignment non-optional?
> 
> Relevant port maintainers CCed.

Jakub

[PATCH] PR 57541

[Iyer, Balaji V]

Hello Everyone,
This patch below should fix the bug reported in PR 57541. The following 
statements were not caught by the array notation expander, and they should be 
caught and replaced with zero nodes:

A[:];
A[x:y];
A[x:y:z];

Here are the Changelogs 

gcc/c/ChangeLog
2013-06-07  Balaji V. Iyer  
* c-array-notation.c (expand_array_notation_exprs): Added
ARRAY_NOTATION_REF case.

gcc/testsuite/ChangeLog
2013-06-07  Balaji V. Iyer  

PR middle-end/57541
* c-c++-common/cilk-plus/AN/pr57541.c: New test case.


... and the patch cut and pasted below:

Index: gcc/c/c-array-notation.c
===
--- gcc/c/c-array-notation.c(revision 199825)
+++ gcc/c/c-array-notation.c(working copy)
@@ -2317,6 +2317,14 @@
 case RETURN_EXPR:
   if (contains_array_notation_expr (t))
t = fix_return_expr (t);
+  return t;
+case ARRAY_NOTATION_REF:
+  /* IF we are here, then we are dealing with cases like this:
+A[:];
+A[x:y:z];
+A[x:y];
+Replace those with just void zero node.  */
+  t = void_zero_node;
 default:
   return t;
 }
Index: gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541.c
===
--- gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541.c   (revision 0)
+++ gcc/testsuite/c-c++-common/cilk-plus/AN/pr57541.c   (revision 0)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+
+int A[10];
+
+int main () {
+  char c = (char)N; /* { dg-error "undeclared" } */
+  short s = (short)N;
+  long l = (long)N;
+  A[l:s:c];
+}
+
+/* { dg-message "note: each" "defined" { target *-*-* }  7 } */
+

Since this is a trivial patch, I will commit this in. I am willing to revert it 
(and/or fix it) if anyone has objections.

Thanks,

Balaji V. Iyer.

Re: [PATCH] DATA_ALIGNMENT vs. DATA_ABI_ALIGNMENT (PR target/56564)

[Hans-Peter Nilsson]

On Fri, 7 Jun 2013, Richard Henderson wrote:
> I've had a brief look over the instances of D_A within the tree atm.  Most of
> them carry the cut-n-paste comment "for the same reasons".  These I believe
> never intended an ABI change, and were really only interested in optimization.
>
> But these I think require a good hard look to see if they really intended an
> ABI alignment:

I'm not sure what is about to change how?

> cris  compiler options for alignment -- systemwide or local?

No, DATA_ALIGNMENT in cris.h is not intended as an ABI
indication, but as an optimization when emitting data.
(This was the way to do it at the time.  Has this changed?)

The ABI is as indicated by BIGGEST_ALIGNMENT: 8 (bits; one
byte).  Nothing is guaranteed (to the data referer) to have a
bigger alignment - unless otherwise indicated by attribute-align.

(Unfortunately I can't change BIGGEST_ALIGNMENT to indicate that
atomic variables require "natural alignment", or actually not to
straddle a cache-boundary, as increasing BIGGEST_ALIGNMENT makes
GCC change the ABI.  But that's a slightly different issue.)

> mmix  comment mentions GETA instruction

Yep, data must be at least 32-bit-aligned so addresses can be
formed with a GETA insn.  BIGGEST_ALIGNMENT is 64 though and
STRICT_ALIGNMENT; natural alignment is required for proper
interpretation as the low bits are ignored.

brgds, H-P

Re: [GOOGLE] More strict checking for call args

[Xinliang David Li]

On Fri, Jun 7, 2013 at 6:47 AM, Richard Biener
 wrote:
> On Fri, Jun 7, 2013 at 3:30 PM, Xinliang David Li  wrote:
>> On Fri, Jun 7, 2013 at 2:05 AM, Richard Biener
>>  wrote:
>>> On Thu, Jun 6, 2013 at 5:10 PM, Dehao Chen  wrote:
 Hi, Martin,

 Yes, your patch can fix my case. Thanks a lot for the fix.

 With the fix, value profiling will still promote the wrong indirect
 call target. Though it will not be inlining, but it results in an
 additional check. How about in check_ic_target, after calling
 gimple_check_call_matching_types, we also check if number of args
 match number of params in target->symbol.decl?
>>>
>>> I wonder what's the point in the gimple_check_call_matching_types check
>>> in the profiling case.  It's at least no longer
>>>
>>> /* Perform sanity check on the indirect call target. Due to race conditions,
>>>false function target may be attributed to an indirect call site. If the
>>>call expression type mismatches with the target function's type, 
>>> expand_call
>>>may ICE.
>>>
>>> because since the introduction of gimple_call_fntype we will _not_ ICE.
>>>
>>> Thus I argue that check_ic_target should be even removed instead of
>>> enhancing it!
>>>
>>
>> Another reason is what Dehao had mentioned -- wrong target leads to
>> useless transformation.
>
> Sure, but a not wrong in the sense of the predicate does not guarantee
> a useful transformation either.

The case in reality is very rare -- most of the cases, the
transformation is good.

>
>>> How does IC profiling determine the called target?  That is, what does it
>>> do when the target is not always the same?  (because the checking code
>>> talks about race conditions for example)
>>
>>
>> The race condition is the happening at instrumentation time -- the
>> indirect call counters are not thread local. We have seen this a lot
>> in the past that a totally bogus target is attributed to a indirect
>> callsite.
>
> So it simply uses whatever function was called last?  Instead of
> using the function that was called most of the time?

It uses the most frequent target -- but the target id recorded for the
most frequent target might be corrupted and got mapped to a false
target during profile-use.

David

>
> Richard.
>
>> thanks,
>>
>> David
>>>
>>> Richard.
>>>
>>>
 Thanks,
 Dehao


 On Thu, Jun 6, 2013 at 7:11 AM, Martin Jambor  wrote:
>
> Hi,
>
> On Tue, Jun 04, 2013 at 05:19:02PM -0700, Dehao Chen wrote:
> > attached is a testcase that would cause problem when source has changed:
> >
> > $ g++ test.cc -O2 -fprofile-generate -DOLD
> > $ ./a.out
> > $ g++ test.cc -O2 -fprofile-use
> > test.cc:34:1: internal compiler error: in operator[], at vec.h:815
> >  }
> >  ^
> > 0x512740 vec::operator[](unsigned int)
> > ../../gcc/vec.h:815
> > 0x512740 vec::operator[](unsigned int)
> > ../../gcc/vec.h:1244
> > 0xf24464 vec::operator[](unsigned int)
> > ../../gcc/vec.h:815
> > 0xf24464 vec::operator[](unsigned int)
> > ../../gcc/vec.h:1244
> > 0xf24464 ipa_get_indirect_edge_target_1
> > ../../gcc/ipa-cp.c:1535
> > 0x971b9a estimate_edge_devirt_benefit
> > ../../gcc/ipa-inline-analysis.c:2757
>
> Hm, this seems rather like an omission in ipa_get_indirect_edge_target_1.
> Since it is called also from inlining, we can have parameter count
> mismatches... and in fact in non-virtual paths of that function we do
> check that we don't.  Because all callers have to pass known_vals
> describing all formal parameters of the inline tree root, we should
> apply the fix below (I've only just started running a bootstrap and
> testsuite on x86_64, though).
>
> OTOH, while I understand that FDO can change inlining sufficiently so
> that this error occurs, IMHO this should not be caused by outdated
> profiles but there is somewhere a parameter mismatch in the source.
>
> Dehao, can you please check that this patch helps?
>
> Richi, if it does and the patch passes bootstrap and tests, is it OK
> for trunk and 4.8 branch?
>
> Thanks and sorry for the trouble,
>
> Martin
>
>
> 2013-06-06  Martin Jambor  
>
> * ipa-cp.c (ipa_get_indirect_edge_target_1): Check that 
> param_index is
> within bounds at the beginning of the function.
>
> Index: src/gcc/ipa-cp.c
> ===
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -1481,7 +1481,8 @@ ipa_get_indirect_edge_target_1 (struct c
>tree otr_type;
>tree t;
>
> -  if (param_index == -1)
> +  if (param_index == -1
> +  || known_vals.length () <= (unsigned int) param_index)
>  return NULL_TREE;
>
>if (!ie->indirect_info->polymorphic)
> @@ -1516,8 +1517,7 @@ ipa_get_indirect_edge_target_1 (str

Re: [C++ patch] Fix PR 57551

[Jason Merrill]

OK.

Jason

Re: [libstdc++-v3][C++14] Implement N3654 - Quoted Strings

[Ed Smith-Rowland]

On 06/06/2013 10:55 AM, Ed Smith-Rowland wrote:

On 06/05/2013 04:01 PM, Jonathan Wakely wrote:

On 5 June 2013 20:18, Ed Smith-Rowland wrote:

Greetings,
This patch implements quoted string manipulators for C++14.

27.7.6 - Quoted manipulators[quoted.manip].

The idea is to allow round trip insert and extract of strings with 
spaces.


   std::stringstream ss;
   std::string original = "thing1  thing1";
   std::string round_trip;
   ss << std::quoted(original);
   ss >> std::quoted(round_trip);
   assert( original == round_trip );

Builds and tests clean on x86-64-linux.

As I suggested for your literals patch, couldn't the test for:
#if __cplusplus > 201103L
go inside the existing one?

i.e.

#if __cplusplus >= 201103L
[...]
#if __cplusplus > 201103L
[...]
#endif
#endif
Certainly.  I forgot that in the last literals patch.  I'll fix that 
after I finish this one. (I just noticed junk comments in the 
testcases for literals also).



_Quoted_string appears to do two copies of the string, one for the
constructor argument and one for the member variable, do they
definitely get elided?
I looks that way.  But all used of the template parm String are either 
references or pointers so these operations should be efficient.

_Quoted_string should be used as a non-owning string thing.


The members of _Quoted_string should be named _M_xxx not __xxx, to
follow the coding style guidelines.

Done.


What is __delim2 for?

What if the first extraction in the operator>> fails, is doing
__is.unget() the right thing to do?

Thanks. I'll return with __is rather than attempting to continue reading.


You could simplify the quoted() overloads by using auto return type
deduction, is it an intentional choice not to use that?
For some reason I forgot about auto return type in C++14.  It sure 
cleans things up nicely.  Done.

Rebuilt and retested on x86_64



OK, I added a static_assert to check that _String is only reference or 
pointer.
I also added a tests that check the case where _String is 'const 
basic_string<>&'.


Built and tested on x86_64-linux.

OK?

Ed


2013-06-08  Ed Smith-Rowland  <3dw...@verizon.net>

Implement N3654 - Quoted Strings Library Proposal
* include/std/iomanip: Add quoted(String, Char delim, Char escape)
manipulators and supporting machinery in c++1y mode.
* testsuite/27_io/manipulators/standard/char/quoted.cc: New.
* testsuite/27_io/manipulators/standard/wchar_t/quoted.cc: New.
Index: include/std/iomanip
===
--- include/std/iomanip (revision 199730)
+++ include/std/iomanip (working copy)
@@ -334,8 +334,161 @@
   return __os; 
 }
 
-#endif
+#if __cplusplus > 201103L
 
+  namespace __detail {
+
+/**
+ * @brief Struct for delimited strings.
+ *The left and right delimiters can be different.
+ */
+template
+  struct _Quoted_string
+  {
+   static_assert(is_reference<_String>::value
+  || is_pointer<_String>::value,
+ "String type must be pointer or reference");
+
+   _Quoted_string(_String __str, _CharT __del, _CharT __esc)
+   : _M_string(__str), _M_delim{__del}, _M_escape{__esc}
+   { }
+
+   _Quoted_string&
+   operator=(_Quoted_string&) = delete;
+
+   _String _M_string;
+   _CharT _M_delim;
+   _CharT _M_escape;
+  };
+
+/**
+ * @brief Inserter for delimited strings.
+ *The left and right delimiters can be different.
+ */
+template
+  auto&
+  operator<<(std::basic_ostream<_CharT, _Traits>& __os,
+const _Quoted_string& __str)
+  {
+   __os << __str._M_delim;
+   for (const _CharT* __c = __str._M_string; *__c; ++__c)
+ {
+   if (*__c == __str._M_delim || *__c == __str._M_escape)
+ __os << __str._M_escape;
+   __os << *__c;
+ }
+   __os << __str._M_delim;
+
+   return __os;
+  }
+
+/**
+ * @brief Inserter for delimited strings.
+ *The left and right delimiters can be different.
+ */
+template
+  auto&
+  operator<<(std::basic_ostream<_CharT, _Traits>& __os,
+const _Quoted_string<_String, _CharT>& __str)
+  {
+   __os << __str._M_delim;
+   for (auto& __c : __str._M_string)
+ {
+   if (__c == __str._M_delim || __c == __str._M_escape)
+ __os << __str._M_escape;
+   __os << __c;
+ }
+   __os << __str._M_delim;
+
+   return __os;
+  }
+
+/**
+ * @brief Extractor for delimited strings.
+ *The left and right delimiters can be different.
+ */
+template
+  auto&
+  operator>>(std::basic_istream<_CharT, _Traits>& __is,
+const _Quoted_string&,
+ _CharT>& __str)
+  {
+   __str._M_string.clear();
+
+   _CharT __c;
+   __is >> __c;
+   if (!_