Re: [PATCH] Avoid bugs like PR68273 to trigger

[Eric Botcazou]

> So you simply assume that exp_type is naturally aligned here.  I think
> you should test align < TYPE_ALIGN (TYPE_MAIN_VARIANT (exp_type)) here,
> no?

No strong opinion, but the patch is only intended to mitigate the effects of 
the PR65310 one-liner, including on the 5 branch, so it's minimal.

> And if you get enough supporters to apply this kind of workaround
> I'd prefer it to be in build_aligned_type itself, basically
> refusing to build over-aligned types.

I pondered that but rejected it for the 5 branch.

> And I'd rather make this controlled by an internal flag that backends should
> consciously set (aka a target hook).

I disagree, the onus is on the middle-end to fix its mess and the default 
should not be to generate wrong code in any case.

> The above is simply a bit too ugly IMHO
> and looks incomplete(?), cannot even the cummulative args machinery
> end up with type-align specifics or are you sure those can only
> be triggered from function_arg_boundary?

I think that testing function_arg_boundary is a conservative criterion, but I 
admittedly didn't check every architecture.

> Note the real issue is overaligned register types.

Agreed, and IMO the middle-end should not create them out of nowhere.

> At least it seems you have a testcase that reproduces the error
> so you should be able to point to a specific pass doing the wreckage
> and provide preprocessed source and a cross-compiler setup to
> investigate.

I'm just using Jeff's reduced testcase for MIPS.

-- 
Eric Botcazou

Re: [PATCH][GCC7] Remove scaling of COMPONENT_REF/ARRAY_REF ops 2/3

[Eric Botcazou]

> The following experiment resulted from looking at making
> array_ref_low_bound and array_ref_element_size non-mutating.  Again
> I wondered why we do this strange scaling by offset/element alignment.

I personally never really grasped it either...

> So - I hope somebody from Adacore can evaluate this patch code-generation
> wise.

I will, this looks like a valuable simplification to me.

-- 
Eric Botcazou

Re: [PATCH] Avoid bugs like PR68273 to trigger

[Richard Biener]

On Fri, 19 Feb 2016, Eric Botcazou wrote:

> > So you simply assume that exp_type is naturally aligned here.  I think
> > you should test align < TYPE_ALIGN (TYPE_MAIN_VARIANT (exp_type)) here,
> > no?
> 
> No strong opinion, but the patch is only intended to mitigate the effects of 
> the PR65310 one-liner, including on the 5 branch, so it's minimal.
> 
> > And if you get enough supporters to apply this kind of workaround
> > I'd prefer it to be in build_aligned_type itself, basically
> > refusing to build over-aligned types.
> 
> I pondered that but rejected it for the 5 branch.
> 
> > And I'd rather make this controlled by an internal flag that backends should
> > consciously set (aka a target hook).
> 
> I disagree, the onus is on the middle-end to fix its mess and the default 
> should not be to generate wrong code in any case.
> 
> > The above is simply a bit too ugly IMHO
> > and looks incomplete(?), cannot even the cummulative args machinery
> > end up with type-align specifics or are you sure those can only
> > be triggered from function_arg_boundary?
> 
> I think that testing function_arg_boundary is a conservative criterion, but I 
> admittedly didn't check every architecture.
> 
> > Note the real issue is overaligned register types.
> 
> Agreed, and IMO the middle-end should not create them out of nowhere.

Yup, so we should make sure we don't (even not "out of nowhere").  See
my attempts on adding some SSA verification for this (needs to be
restricted to overaligned, then it doesn't trigger that often...).
One issue is that we've often got DECLs that are overaligned but at
some point re-write them into SSA - and the SSA verifier expects the
SSA name types to agree with their decls type.  I didn't yet try to
fixup DECL alignment to be natural as we re-write it into SSA but
I believe that may be necessary in the end to make such SSA verification
not trigger.

> > At least it seems you have a testcase that reproduces the error
> > so you should be able to point to a specific pass doing the wreckage
> > and provide preprocessed source and a cross-compiler setup to
> > investigate.
> 
> I'm just using Jeff's reduced testcase for MIPS.

See the bug where I fixed up some more places in SRA after debugging
a cross to MIPS with the reduced testcase (fixing verification fails
with added SSA alignment checking).

Richard.

[Patch, fortran] PR69423 - [6 Regression] Invalid optimization with deferred-length character

[Paul Richard Thomas]

Dear All,

This has proven to be a rather vexatious bug to fix. On the face of
it, using the indirect reference to the passed string length for
deferred character length functions should have worked at all levels
of optimization. However, setting the string length within a do loop
resulted in the change not being visible within the rest of the
function scope, even though the correct result was returned. This was,
on the face of it, the same mechanism used for both dummies and
declared results, which works fine at all levels of optimization.

In order to be as conservative as possible at this stage in the
release cycle, I have resorted to the belt and braces approach of
using a local variable '..result', which is nulled and returned, as
appropriate, in a new helper function. So that the compiled code is
consistent, I have done the same for functions with and without
explicitly declared result variables.

There is some dead code in 'gfc_get_symbol_decl', which could, with
advantage, be replaced by a gcc_assert. In addition,
gfc_trans_deferred_vars could do with some further tidying up to
ensure that the logic is clear. These steps can easily be done now if
other maintainers think that it is timely.

Bootstraps and regtests on FC21/x86_64 - OK for trunk?

Paul

2016-02-19  Paul Thomas  

PR fortran/69423
* trans-decl.c (create_function_arglist): Deferred character
length functions, with and without declared results, address
the passed reference type as '.result' and the local string
length as '..result'.
(gfc_null_and_pass_deferred_len): Helper function to null and
return deferred string lengths, as needed.
(gfc_trans_deferred_vars): Call it, thereby reducing repeated
code, add call for deferred arrays and reroute pointer function
results. Avoid using 'tmp' for anything other that a temporary
tree by introducing 'type_of_array' for the arrayspec type.

2016-02-19  Paul Thomas  

PR fortran/69423
* gfortran.dg/deferred_character_15.f90 : New test.
Index: gcc/fortran/trans-decl.c
===
*** gcc/fortran/trans-decl.c(revision 233507)
--- gcc/fortran/trans-decl.c(working copy)
*** create_function_arglist (gfc_symbol * sy
*** 2234,2240 
   PARM_DECL,
   get_identifier (".__result"),
   len_type);
! if (!sym->ts.u.cl->length)
{
  sym->ts.u.cl->backend_decl = length;
  TREE_USED (length) = 1;
--- 2234,2245 
   PARM_DECL,
   get_identifier (".__result"),
   len_type);
! if (POINTER_TYPE_P (len_type))
!   {
! sym->ts.u.cl->passed_length = length;
! TREE_USED (length) = 1;
!   }
! else if (!sym->ts.u.cl->length)
{
  sym->ts.u.cl->backend_decl = length;
  TREE_USED (length) = 1;
*** create_function_arglist (gfc_symbol * sy
*** 2271,2283 
  type = gfc_sym_type (arg);
  arg->backend_decl = backend_decl;
  type = build_reference_type (type);
- 
- if (POINTER_TYPE_P (len_type))
-   {
- sym->ts.u.cl->passed_length = length;
- sym->ts.u.cl->backend_decl =
-   build_fold_indirect_ref_loc (input_location, length);
-   }
}
}
  
--- 2276,2281 
*** init_intent_out_dt (gfc_symbol * proc_sy
*** 3917,3922 
--- 3915,3976 
  }
  
  
+ /* Helper function to manage deferred string lengths.  */
+ 
+ static tree
+ gfc_null_and_pass_deferred_len (gfc_symbol *sym, stmtblock_t *init,
+   locus *loc)
+ {
+   tree tmp;
+ 
+   /* Character length passed by reference.  */
+   tmp = sym->ts.u.cl->passed_length;
+   tmp = build_fold_indirect_ref_loc (input_location, tmp);
+   tmp = fold_convert (gfc_charlen_type_node, tmp);
+ 
+   if (!sym->attr.dummy || sym->attr.intent == INTENT_OUT)
+ /* Zero the string length when entering the scope.  */
+ gfc_add_modify (init, sym->ts.u.cl->backend_decl,
+   build_int_cst (gfc_charlen_type_node, 0));
+   else
+ {
+   tree tmp2;
+ 
+   tmp2 = fold_build2_loc (input_location, MODIFY_EXPR,
+ gfc_charlen_type_node,
+ sym->ts.u.cl->backend_decl, tmp);
+   if (sym->attr.optional)
+   {
+ tree present = gfc_conv_expr_present (sym);
+ tmp2 = build3_loc (input_location, COND_EXPR,
+void_type_node, present, tmp2,
+build_empty_stmt (input_location));
+   }
+   gfc_add_expr_to_block (init, tmp2);
+ }
+ 
+   gfc_restore_backend_locus (loc);
+ 
+   /* Pass the final character length back.  */
+   if (sym

Re: [PATCH] Fix PR37448 (and dups?)

[Richard Biener]

On Thu, 18 Feb 2016, Richard Biener wrote:

> On Thu, 18 Feb 2016, Jan Hubicka wrote:
> 
> > > 
> > > This fixes the IPA inline analysis quadraticness that arises when we
> > > inline functions into the same function many times via
> > > inline_to_all_callers as we do in the testcase for PR37448.  In that
> > > case updating the overall summary of the caller is done for each
> > > inlined call instead of just once.
> > > 
> > > The solution is to keep track of which nodes we inlined to and
> > > delay the summary update.
> > > 
> > > Honza, I believe this is safe as the callers summary should only
> > > matter for further inline decisions and not this one (inlining
> > > into all callers).  Double-checking is appreciated - there should
> > > be no code changes by this patch (I verified this on the PR37448
> > > testcase).
> > 
> > I think it is not the case if one function contains multiple calls
> > and eventually hits large function growth.  In that case we inline
> > just some callers and not the others.
> 
> Are you sure?  I thought we compute that up-front ... at least
> we make sure we can_inline_edge_p all calls before even trying
> to inline all calls - that looks somewhat redundant then if it
> can happen that we give up anyway.  Ah, so can_inline_edge_p has
> 
>   /* Check if caller growth allows the inlining.  */
>   else if (!DECL_DISREGARD_INLINE_LIMITS (callee->decl)
>&& !disregard_limits
>&& !lookup_attribute ("flatten",
>  DECL_ATTRIBUTES (caller->decl))
>&& !caller_growth_limits (e))
> inlinable = false;
> 
> and we check that from when we do the inlining and upfront from
> want_inline_function_to_all_callers_p ... we also give up
> if we inline a recursive function it seems.
> 
> > For next stage1 I plan to finish the overahaul to sreals that will let
> > me to make update_overall_summary incremental (i.e. 
> > O(size_of_inlined_function)
> > and not O(size_of_inlined_function+size_of_function_inlined_to)) that will
> > also solve these issues.
> 
> Hopefully.
> 
> > One can probably do a pesimistic estimate on code size of the function 
> > (i.e. add the size of inlined function) and only if that hits the large 
> > function growth do the exact calculation. Or we can just decide that it 
> > is OK to ignore large function growht in this partiuclar case.
> 
> Ignoring it is probably not a good idea and will just lead to a different
> degenerate case.  As we update the summary afterwards it's probably
> ok to do incremental (pessimistic) updates on the size anyway?  In
> inline_merge_summary I mean?  Or should I open-code that into
> inline_call if ! update_overall_summary?

So like below - I update self-size by the growth estimate which is
supposed to match, this should keep the overall function growth
limits working, not sure about the stack stuff but that doesn't seem
to be updated by update_overall_summaries either.

This also fixes a similar issue in early inlining where we can then
trivially delay update_overall_summaries.  I remember seeing early
inline analysis behave similarly quadratic.

It leaves alone recursive and IPA small function inlining as those
update overall unit size as well - I first thought about somehow
making overall summary update lazy, but that looks quite complicated
and IIRC the round-off errors issue was when re-computing the
key for the fibheap after doing one inlining, right?

I hope you can get to use sreals early in next stage1 though I wonder
how that will avoid all corner-cases.

Bootstrap & regtest running on x86_64-unknown-linux-gnu.  Ok for trunk
and branch?

Thanks,
Richard.

2016-02-18  Richard Biener  

PR ipa/37448
* ipa-inline-transform.c (inline_call): When not updating
overall summaries adjust self size by the growth estimate.
* ipa-inline.c (inline_to_all_callers_1): Add to the callers
hash-set, do not update overall summaries here.  Renamed from ...
(inline_to_all_callers): ... this which is now wrapping the
above and performing delayed overall summary update.
(early_inline_small_functions): Delay updating of the overall
summary.

Index: gcc/ipa-inline-transform.c
===
--- gcc/ipa-inline-transform.c  (revision 233498)
+++ gcc/ipa-inline-transform.c  (working copy)
@@ -300,10 +300,12 @@ inline_call (struct cgraph_edge *e, bool
   struct cgraph_edge *curr = e;
   struct cgraph_node *callee = e->callee->ultimate_alias_target ();
   bool new_edges_found = false;
+  int estimated_growth;
 
+  if (! update_overall_summary)
+estimated_growth = estimate_edge_growth (e);
   /* This is used only for assert bellow.  */
 #if 0
-  int estimated_growth = estimate_edge_growth (e);
   bool predicated = inline_edge_summary (e)->predicate != NULL;
 #endif
 
@@ -373,7 +375,13 @@ inline_call (struct cgraph_edge *e, bool
 new_edges_found = ipa_propagat

Re: [PATCH] Add debug_function_to_file

[Richard Biener]

On Thu, Feb 18, 2016 at 8:41 PM, David Malcolm  wrote:
> On Thu, 2016-02-18 at 18:26 +0100, Tom de Vries wrote:
>> On 18/02/16 16:43, Tom de Vries wrote:
>> > On 18/02/16 16:27, Richard Biener wrote:
>> > > > > > I would be nice if we could avoid the ${1,2,3} printouts
>> > > > > > and value
>> > > > > > > > > > history
>> > > > > > > > > > assignments, but I'm not sure how to do that.
>> > > > > > > > > >
>> > > >
>> > > >
>> > > > Using gdb.parse_and_eval does the trick.
>> > > >
>> >
>> > This updated version uses gdb.parse_and_eval, and adds error
>> > handling.
>>
>> And this updated version adds handling different number of arguments,
>> and a help text. I think this could be ready for committing.
>>
>> Is a bootstrap/regtest useful/necessary?
>
> I don't think so; I don't think we have any automated coverage for
> gdb_hooks.py.  Presumably you've tested it by hand.
>
> What version of Python do you have embedded in gdb?  (IIRC some people
> have Python 2, others Python 3)
>
>> +print ("Too little arguments")
>
> Nit: can you change this to "Not enough arguments".
>
>
>> OK for stage4/stage1?
>
> Looks reasonable to me.

Ok for stage4.

Thanks,
Richard.

Re: [PATCH] Add debug_function_to_file

[Richard Biener]

On Fri, Feb 19, 2016 at 1:37 AM, Tom de Vries  wrote:
> On 18/02/16 16:27, Richard Biener wrote:
>>
>> Attached is what I have for now, it works if you call it like
>>
>> (gdb) dot-fn cfun
>> (gdb) dot-fn cfun, 1<<6
>>
>> w/o that arg parsing;)
>>
>> I'll play with it some more tomorrow.
>
>
> This version:
> - uses arg parsing
> - adds error handling
> - uses a temp file instead of a pipe
> - uses python os.system to call dot

I used popen specifically to allow you continue debugging while keeping the dot
process open and functional.  That would be restored with adding a '&' after
the command but then we race with the file removal ...

The following works for me though:

# Show graph in temp file
os.system("( dot -Tx11 %s; rm %s ) &" % (filename, filename) )

dot_fn()

ok for trunk with that change and thanks for the help!

Now if the TDF_ flags were available in gdb (without -g3) that would be
awsome.  I guess moving them into a enum would work but also require
some extra casts everywhere...

Thanks,
Richard.

> - adds documentation
>
> Thanks,
> - Tom

[PATCH] combine: Delete EQ* notes when pseudo mode changes (PR60818)

[Segher Boessenkool]

For some modifications combine does it changes the mode of a pseudo
because of SELECT_CC_MODE.  For example, it changes "unsigned >= 0" to
"unsigned != 0", and on some targets GTU and NE use different CC_MODEs.

Changing the mode of a pseudo has effect globally, so this changes any
REG_EQUAL and REG_EQUIV notes referring to the pseudo as well, which
makes them invalid.  This patch finds such notes and deletes them in
these cases.

Testing on powerpc64-linux; will also test on x86_64-linux before
committing.


Segher


2016-02-19  Segher Boessenkool  

PR 60818/rtl-optimization
* combine.c (combine_remove_reg_equal_equiv_notes_for_regno):
New function.
(try_combine): Call it when SELECT_CC_MODE makes us change the
mode of a pseudo.

---
 gcc/combine.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/gcc/combine.c b/gcc/combine.c
index 24dcefa..d3b69ac 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -2553,6 +2553,32 @@ can_split_parallel_of_n_reg_sets (rtx_insn *insn, int n)
   return true;
 }
 
+/* Remove all REG_EQUAL and REG_EQUIV notes referring to REGNO.  This is
+   like rtlanal's remove_reg_equal_equiv_notes_for_regno but with some big
+   differences, because combine does not keep the DF info up-to-date.
+   We do however never add or move these notes during combine, so we can
+   still use the DF info as it was at the start of combine to find all
+   such notes.  */
+
+static void
+combine_remove_reg_equal_equiv_notes_for_regno (unsigned int regno)
+{
+  gcc_assert (df);
+
+  rtx reg = regno_reg_rtx[regno];
+
+  for (df_ref eq_use = DF_REG_EQ_USE_CHAIN (regno);
+   eq_use;
+   eq_use = DF_REF_NEXT_REG (eq_use))
+{
+  rtx_insn *insn = DF_REF_INSN (eq_use);
+  rtx note = find_reg_equal_equiv_note (insn);
+
+  if (note && reg_overlap_mentioned_p (reg, XEXP (note, 0)))
+   remove_note (insn, note);
+}
+}
+
 /* Try to combine the insns I0, I1 and I2 into I3.
Here I0, I1 and I2 appear earlier than I3.
I0 and I1 can be zero; then we combine just I2 into I3, or I1 and I2 into
@@ -3184,6 +3210,8 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, 
rtx_insn *i0,
newpat_dest = gen_rtx_REG (compare_mode, regno);
  else
{
+ combine_remove_reg_equal_equiv_notes_for_regno (regno);
+
  SUBST_MODE (regno_reg_rtx[regno], compare_mode);
  newpat_dest = regno_reg_rtx[regno];
}
@@ -6638,6 +,12 @@ simplify_set (rtx x)
new_dest = gen_rtx_REG (compare_mode, regno);
  else
{
+ /* We change the mode of the result pseudo.  The expression
+in REG_EQ* notes refering to that pseudo will likely no
+longer make sense, so delete those notes.
+This is PR60818.  */
+ combine_remove_reg_equal_equiv_notes_for_regno (regno);
+
  SUBST_MODE (regno_reg_rtx[regno], compare_mode);
  new_dest = regno_reg_rtx[regno];
}
-- 
1.9.3

Re: [PATCH] Fix PR37448 (and dups?)

[Richard Biener]

On Fri, 19 Feb 2016, Richard Biener wrote:

> On Thu, 18 Feb 2016, Richard Biener wrote:
> 
> > On Thu, 18 Feb 2016, Jan Hubicka wrote:
> > 
> > > > 
> > > > This fixes the IPA inline analysis quadraticness that arises when we
> > > > inline functions into the same function many times via
> > > > inline_to_all_callers as we do in the testcase for PR37448.  In that
> > > > case updating the overall summary of the caller is done for each
> > > > inlined call instead of just once.
> > > > 
> > > > The solution is to keep track of which nodes we inlined to and
> > > > delay the summary update.
> > > > 
> > > > Honza, I believe this is safe as the callers summary should only
> > > > matter for further inline decisions and not this one (inlining
> > > > into all callers).  Double-checking is appreciated - there should
> > > > be no code changes by this patch (I verified this on the PR37448
> > > > testcase).
> > > 
> > > I think it is not the case if one function contains multiple calls
> > > and eventually hits large function growth.  In that case we inline
> > > just some callers and not the others.
> > 
> > Are you sure?  I thought we compute that up-front ... at least
> > we make sure we can_inline_edge_p all calls before even trying
> > to inline all calls - that looks somewhat redundant then if it
> > can happen that we give up anyway.  Ah, so can_inline_edge_p has
> > 
> >   /* Check if caller growth allows the inlining.  */
> >   else if (!DECL_DISREGARD_INLINE_LIMITS (callee->decl)
> >&& !disregard_limits
> >&& !lookup_attribute ("flatten",
> >  DECL_ATTRIBUTES (caller->decl))
> >&& !caller_growth_limits (e))
> > inlinable = false;
> > 
> > and we check that from when we do the inlining and upfront from
> > want_inline_function_to_all_callers_p ... we also give up
> > if we inline a recursive function it seems.
> > 
> > > For next stage1 I plan to finish the overahaul to sreals that will let
> > > me to make update_overall_summary incremental (i.e. 
> > > O(size_of_inlined_function)
> > > and not O(size_of_inlined_function+size_of_function_inlined_to)) that will
> > > also solve these issues.
> > 
> > Hopefully.
> > 
> > > One can probably do a pesimistic estimate on code size of the function 
> > > (i.e. add the size of inlined function) and only if that hits the large 
> > > function growth do the exact calculation. Or we can just decide that it 
> > > is OK to ignore large function growht in this partiuclar case.
> > 
> > Ignoring it is probably not a good idea and will just lead to a different
> > degenerate case.  As we update the summary afterwards it's probably
> > ok to do incremental (pessimistic) updates on the size anyway?  In
> > inline_merge_summary I mean?  Or should I open-code that into
> > inline_call if ! update_overall_summary?
> 
> So like below - I update self-size by the growth estimate which is
> supposed to match, this should keep the overall function growth
> limits working, not sure about the stack stuff but that doesn't seem
> to be updated by update_overall_summaries either.
> 
> This also fixes a similar issue in early inlining where we can then
> trivially delay update_overall_summaries.  I remember seeing early
> inline analysis behave similarly quadratic.
> 
> It leaves alone recursive and IPA small function inlining as those
> update overall unit size as well - I first thought about somehow
> making overall summary update lazy, but that looks quite complicated
> and IIRC the round-off errors issue was when re-computing the
> key for the fibheap after doing one inlining, right?

So like having a ->need_update flag in the summaries and in


...
  inline_call (edge, true, &new_indirect_edges, &overall_size, 
true);
  add_new_edges_to_heap (&edge_heap, new_indirect_edges);

  reset_edge_caches (edge->callee->function_symbol ());

  update_callee_keys (&edge_heap, where, updated_nodes);
}
..

update caller/callee keys with the "estimate" but set a ->need_key_update
flag on them.  When getting the new min key

  while (!edge_heap.empty ())
{
  int old_size = overall_size;
  struct cgraph_node *where, *callee;
  sreal badness = edge_heap.min_key ();

and ->need_update is set re-compute summaries and re-compute the
key (re-inserting it if it isn't still the minimum).

Of course making the incremental updates just work would be best.

> I hope you can get to use sreals early in next stage1 though I wonder
> how that will avoid all corner-cases.
> 
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.  Ok for trunk
> and branch?
> 
> Thanks,
> Richard.
> 
> 2016-02-18  Richard Biener  
> 
>   PR ipa/37448
>   * ipa-inline-transform.c (inline_call): When not updating
>   overall summaries adjust self size by the growth estimate.
>   * ipa-inline.c (inline_to_all_callers_1): Add to the callers
>   hash-set, do not update overall sum

Re: [PATCH] testsuite/s390: Add __morestack test.

[Andreas Krebbel]

On 02/07/2016 01:22 PM, Marcin Kościelnicki wrote:
> gcc/testsuite/ChangeLog:
> 
>   * gcc.target/s390/morestack.c: New test.

Applied.  Thanks!

-Andreas-

Re: [PATCH] S/390: PR 69625: Add test case

[Andreas Krebbel]

On 02/09/2016 02:18 PM, Dominik Vogt wrote:
> On Fri, Feb 05, 2016 at 05:07:57PM +0100, Dominik Vogt wrote:
>> The attached patch adds a testcase for PR 69625.
> 
> Version 2 also runs with -m31.

Applied. Thanks!

-Andreas-

Re: [PATCH] combine: Delete EQ* notes when pseudo mode changes (PR60818)

[Eric Botcazou]

> 2016-02-19  Segher Boessenkool  
> 
>   PR 60818/rtl-optimization
>   * combine.c (combine_remove_reg_equal_equiv_notes_for_regno):
>   New function.
>   (try_combine): Call it when SELECT_CC_MODE makes us change the
>   mode of a pseudo.

This looks like a big hammer to me though.

> +/* Remove all REG_EQUAL and REG_EQUIV notes referring to REGNO.  This is
> +   like rtlanal's remove_reg_equal_equiv_notes_for_regno but with some big
> +   differences, because combine does not keep the DF info up-to-date.
> +   We do however never add or move these notes during combine, so we can
> +   still use the DF info as it was at the start of combine to find all
> +   such notes.  */

The comment is wrong, or at least confusing, since distribute_notes does deal 
with REG_EQUAL and REG_EQUIV notes.

-- 
Eric Botcazou

Re: [PATCH] combine: Delete EQ* notes when pseudo mode changes (PR60818)

[Segher Boessenkool]

On Fri, Feb 19, 2016 at 11:27:48AM +0100, Eric Botcazou wrote:
> > 2016-02-19  Segher Boessenkool  
> > 
> > PR 60818/rtl-optimization
> > * combine.c (combine_remove_reg_equal_equiv_notes_for_regno):
> > New function.
> > (try_combine): Call it when SELECT_CC_MODE makes us change the
> > mode of a pseudo.
> 
> This looks like a big hammer to me though.

Do you have something smaller in mind that still works?  I'm all ears.

> > +/* Remove all REG_EQUAL and REG_EQUIV notes referring to REGNO.  This is
> > +   like rtlanal's remove_reg_equal_equiv_notes_for_regno but with some big
> > +   differences, because combine does not keep the DF info up-to-date.
> > +   We do however never add or move these notes during combine, so we can
> > +   still use the DF info as it was at the start of combine to find all
> > +   such notes.  */
> 
> The comment is wrong, or at least confusing, since distribute_notes does deal 
> with REG_EQUAL and REG_EQUIV notes.

But it never adds or moves these notes.  It even says so :-)


Segher

[C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

Hi,

as described in the PR 69865, some bits are not properly initialized when the 
-std=gnu++14
option is not present on the command line.  That includes the options 
-trigraphs and
-fno-extended-identifiers which are effectively overridden by the default 
handling.
Also the define __GNUC_GNU_INLINE__ vs. __GNUC_STDC_INLINE__ is differently
defined if -std=gnu++14 is given on the command line than when the default 
takes effect,
which is also supposed to be gnu++14 too.

While I think that we should probably not define __GNUC_GNU_INLINE__ at all for 
C++,
because it is meaningless, I am warned that this could break (already broken) 
header files.
I think the safest thing would be to unconditionally define __GNUC_GNU_INLINE__ 
for C++
and not use flag_isoc99 for that decision which is true for c++11 and above, 
but was undefined
previously when the -std option was not used on the command line.

Unfortunately this specific bug, cannot be tested in the test suite, especially 
the -trigraphs
have already test cases under c-c++common, which should have been failing all 
the time,
but, unfortunately the test suite always adds -std=xxx for c++ tests.  So I 
would like to make
an exception here to the general rule that every patch has to add a test case 
of its own.

I would like this patch to be considered for gcc-6 because the undefined state 
of the predefined
GNUC-define worries me a bit.


Boot-strapped and regression tested on x86_64-pc-linux-gnu.
OK for trunk?


Thanks
Bernd.2016-02-19  Bernd Edlinger  

PR c++/69865
* c-opts.c (c_common_post_options): Set flag_gnu89_inline for C++.
Move call to set_std_cxx14 from here...
(c_common_init_options): ...to here.
(set_std_cxx98): Initialize flag_isoc94 and flag_isoc99.
Index: gcc/c-family/c-opts.c
===
--- gcc/c-family/c-opts.c	(revision 233531)
+++ gcc/c-family/c-opts.c	(working copy)
@@ -246,6 +246,10 @@ c_common_init_options (unsigned int decoded_option
 	  }
 }
 
+  /* Set C++ standard to C++14 if not specified on the command line.  */
+  if (c_dialect_cxx ())
+set_std_cxx14 (/*ISO*/false);
+
   global_dc->colorize_source_p = true;
 }
 
@@ -786,7 +790,7 @@ c_common_post_options (const char **pfilename)
   /* By default we use C99 inline semantics in GNU99 or C99 mode.  C99
  inline semantics are not supported in GNU89 or C89 mode.  */
   if (flag_gnu89_inline == -1)
-flag_gnu89_inline = !flag_isoc99;
+flag_gnu89_inline = c_dialect_cxx () || !flag_isoc99;
   else if (!flag_gnu89_inline && !flag_isoc99)
 error ("-fno-gnu89-inline is only supported in GNU99 or C99 mode");
 
@@ -802,10 +806,6 @@ c_common_post_options (const char **pfilename)
   && flag_no_builtin)
 flag_tree_loop_distribute_patterns = 0;
 
-  /* Set C++ standard to C++14 if not specified on the command line.  */
-  if (c_dialect_cxx () && cxx_dialect == cxx_unset)
-set_std_cxx14 (/*ISO*/false);
-
   /* -Woverlength-strings is off by default, but is enabled by -Wpedantic.
  It is never enabled in C++, as the minimum limit is not normative
  in that standard.  */
@@ -1519,6 +1519,8 @@ set_std_cxx98 (int iso)
   flag_no_gnu_keywords = iso;
   flag_no_nonansi_builtin = iso;
   flag_iso = iso;
+  flag_isoc94 = 0;
+  flag_isoc99 = 0;
   cxx_dialect = cxx98;
   lang_hooks.name = "GNU C++98";
 }

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 10:50:34AM +, Bernd Edlinger wrote:
> While I think that we should probably not define __GNUC_GNU_INLINE__ at all 
> for C++,
> because it is meaningless, I am warned that this could break (already broken) 
> header files.

It is not meaningless.  The various headers need to know if it is safe to
use the gnu_inline attribute in C++.

In any case, the desirable state is that e.g. the -E -dD output should be
identical if you explicitly request the default -std= version vs. if it is
set implicitly.  We should verify it is the case even for C.

Jakub

Re: [PATCH] combine: Delete EQ* notes when pseudo mode changes (PR60818)

[Eric Botcazou]

> Do you have something smaller in mind that still works?  I'm all ears.

Try to adjust the notes instead of dropping them?

> But it never adds or moves these notes.  It even says so :-)

Right, but try_combine can do it, see line 4294 and below.

-- 
Eric Botcazou

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 11:56, Jakub Jelinek wrote:
>
> On Fri, Feb 19, 2016 at 10:50:34AM +, Bernd Edlinger wrote:
> > While I think that we should probably not define __GNUC_GNU_INLINE__ at all 
> > for C++,
> > because it is meaningless, I am warned that this could break (already 
> > broken) header files.
> 
> It is not meaningless.  The various headers need to know if it is safe to
> use the gnu_inline attribute in C++.
> 
> In any case, the desirable state is that e.g. the -E -dD output should be
> identical if you explicitly request the default -std= version vs. if it is
> set implicitly.  We should verify it is the case even for C.
> 
> Jakub

I absolutely agree with you.
The correct solution is probably doing this:

--- gcc/cp/cfns.h.jj2016-01-04 15:30:50.0 +0100
+++ gcc/cp/cfns.h   2016-02-19 12:00:15.730375049 +0100
@@ -124,9 +124,6 @@
 
 #ifdef __GNUC__
 __inline
-#ifdef __GNUC_STDC_INLINE__
-__attribute__ ((__gnu_inline__))
-#endif
 #endif
 const char *
 libc_name_p (register const char *str, register unsigned int len)

and remove this hunk instead:

@@ -786,7 +790,7 @@ c_common_post_options (const char **pfilename)
   /* By default we use C99 inline semantics in GNU99 or C99 mode.  C99
  inline semantics are not supported in GNU89 or C89 mode.  */
   if (flag_gnu89_inline == -1)
-flag_gnu89_inline = !flag_isoc99;
+flag_gnu89_inline = c_dialect_cxx () || !flag_isoc99;
   else if (!flag_gnu89_inline && !flag_isoc99)
 error ("-fno-gnu89-inline is only supported in GNU99 or C99 mode");


Would you like that better?


Thanks
Bernd.

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 11:08:48AM +, Bernd Edlinger wrote:
> On 19.02.2016 11:56, Jakub Jelinek wrote:
> >
> > On Fri, Feb 19, 2016 at 10:50:34AM +, Bernd Edlinger wrote:
> > > While I think that we should probably not define __GNUC_GNU_INLINE__ at 
> > > all for C++,
> > > because it is meaningless, I am warned that this could break (already 
> > > broken) header files.
> > 
> > It is not meaningless.  The various headers need to know if it is safe to
> > use the gnu_inline attribute in C++.
> > 
> > In any case, the desirable state is that e.g. the -E -dD output should be
> > identical if you explicitly request the default -std= version vs. if it is
> > set implicitly.  We should verify it is the case even for C.
> > 
> > Jakub
> 
> I absolutely agree with you.
> The correct solution is probably doing this:
> 
> --- gcc/cp/cfns.h.jj  2016-01-04 15:30:50.0 +0100
> +++ gcc/cp/cfns.h 2016-02-19 12:00:15.730375049 +0100
> @@ -124,9 +124,6 @@
>  
>  #ifdef __GNUC__
>  __inline
> -#ifdef __GNUC_STDC_INLINE__
> -__attribute__ ((__gnu_inline__))
> -#endif
>  #endif
>  const char *
>  libc_name_p (register const char *str, register unsigned int len)

This is of course wrong.  cfns.h is a generated header, so you shouldn't
patch it.
If it is regenerated with a newer gperf (I have 3.0.4 installed), you get there:
@@ -124,7 +124,7 @@ hash (register const char *str, register
 
 #ifdef __GNUC__
 __inline
-#ifdef __GNUC_STDC_INLINE__
+#if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
 __attribute__ ((__gnu_inline__))
 #endif
 #endif

Jakub

Re: [PATCH] Add debug_function_to_file

[Tom de Vries]

On 18/02/16 20:41, David Malcolm wrote:

On Thu, 2016-02-18 at 18:26 +0100, Tom de Vries wrote:

On 18/02/16 16:43, Tom de Vries wrote:

On 18/02/16 16:27, Richard Biener wrote:

I would be nice if we could avoid the ${1,2,3} printouts
and value

history
assignments, but I'm not sure how to do that.




Using gdb.parse_and_eval does the trick.



This updated version uses gdb.parse_and_eval, and adds error
handling.


And this updated version adds handling different number of arguments,
and a help text. I think this could be ready for committing.

Is a bootstrap/regtest useful/necessary?


I don't think so; I don't think we have any automated coverage for
gdb_hooks.py.  Presumably you've tested it by hand.



Indeed, I did.


What version of Python do you have embedded in gdb?  (IIRC some people
have Python 2, others Python 3)



AFAIU, Python 3, more specifically:
...
$ cat /etc/issue
Ubuntu 14.04.3 LTS \n \l
$ gdb --version
GNU gdb (Ubuntu 7.7.1-0ubuntu5~14.04.2) 7.7.1
$ gdb
(gdb) python import sys; print(sys.version)
3.4.3 (default, Oct 14 2015, 20:31:36)
[GCC 4.8.4]
(gdb)
...


+print ("Too little arguments")


Nit: can you change this to "Not enough arguments".



In the attached reworked version, this error message no longer occurs.


OK for stage4/stage1?


Looks reasonable to me.



I've done two minor cleanups:
- I've moved casting the C call arguments out of the call expressions,
  and into the argument variables themselves
- I've changed the naming of the class to be consistent with the rest
  of the file (capitalize words, connect without underscore)

Furthermore, I've tried to improve usability:
- I've renamed the command to a shorter dump-fn (similar to dot-fn)
- I've added a pop-up mode (also similar to dot-fn), where we:
  - write to a temp file,
  - invoke an editor using the $EDITOR command (I needed to set
EDITOR=emacs to get this working, my default EDITOR="emacs -nw"
didn't work, giving not-a-tty error), and
  - remove the temp file after the editor is closed.
- the new pop-up mode is the default.
- the original file-only mode is selected by using /f  as first
  two arguments.

Thanks,
- Tom
Add debug-function-to-file to gdbhooks.py

2016-02-18  Tom de Vries  

	* gdbhooks.py (class debug_function_to_file): Add and instantiate.

---
 gcc/gdbhooks.py | 91 +
 1 file changed, 91 insertions(+)

diff --git a/gcc/gdbhooks.py b/gcc/gdbhooks.py
index 0d5cc97..ed72016 100644
--- a/gcc/gdbhooks.py
+++ b/gcc/gdbhooks.py
@@ -133,6 +133,7 @@ Instead (for now) you must access m_vecdata:
 import os.path
 import re
 import sys
+import tempfile
 
 import gdb
 import gdb.printing
@@ -589,4 +590,94 @@ class BreakOnPass(gdb.Command):
 
 BreakOnPass()
 
+class DumpFn(gdb.Command):
+"""
+A custom command to dump a gimple/rtl function to file.  By default, it
+dumps the current function using 0 as dump_flags, but the function and flags
+can also be specified. If /f  are passed as the first two arguments,
+the dump is written to that file.  Otherwise, a temporary file is created
+and opened in the text editor specified in the EDITOR environment variable.
+
+Examples of use:
+  (gdb) dump-fn
+  (gdb) dump-fn /f foo.1.txt
+  (gdb) dump-fn cfun->decl
+  (gdb) dump-fn /f foo.1.txt cfun->decl
+  (gdb) dump-fn cfun->decl 0
+  (gdb) dump-fn cfun->decl dump_flags
+"""
+
+def __init__(self):
+gdb.Command.__init__(self, 'dump-fn', gdb.COMMAND_USER)
+
+def invoke(self, arg, from_tty):
+# Parse args, check number of args
+args = gdb.string_to_argv(arg)
+if len(args) >= 1 and args[0] == "/f":
+if len(args) == 1:
+print ("Missing file argument")
+return
+filename = args[1]
+editor_mode = False
+base_arg = 2
+else:
+editor = os.getenv("EDITOR", "")
+if editor == "":
+print ("EDITOR environment variable not defined")
+return
+editor_mode = True
+base_arg = 0
+if len(args) - base_arg > 2:
+print ("Too many arguments")
+return
+
+# Set func
+if len(args) - base_arg >= 1:
+funcname = args[base_arg]
+printfuncname = "function %s" % funcname
+else:
+funcname = "cfun ? cfun->decl : current_function_decl"
+printfuncname = "current function"
+func = gdb.parse_and_eval(funcname)
+if func == 0:
+print ("Could not find %s" % printfuncname)
+return
+func = "(tree)%u" % func
+
+# Set flags
+if len(args) - base_arg >= 2:
+flags = gdb.parse_and_eval(args[base_arg + 1])
+else:
+flags = 0
+
+# Get tempory file, if necessary
+if editor_mode:
+f = tempfile.NamedTemporaryFile(delete=Fal

Re: [PATCH] combine: Delete EQ* notes when pseudo mode changes (PR60818)

[Segher Boessenkool]

On Fri, Feb 19, 2016 at 12:04:17PM +0100, Eric Botcazou wrote:
> > Do you have something smaller in mind that still works?  I'm all ears.
> 
> Try to adjust the notes instead of dropping them?

As far as I can see that is very hard to do though, and not really worth
it -- the notes can contain an arbitrary expression in general.

Not for stage 4 certainly.

> > But it never adds or moves these notes.  It even says so :-)
> 
> Right, but try_combine can do it, see line 4294 and below.

That moves those notes to i2notes, and then distribute_notes drops them?


Segher

[ping] Enable -mstackrealign with SSE on 32-bit Windows

[Eric Botcazou]

Any (global or platform) maintainer willing to approve this for GCC6?
  https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01458.html

We switched to SSE2 by default on Windows 32-bit at AdaCore and we ran into 
really nasty stability issues before applying this patch.

-- 
Eric Botcazou

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 12:31, Jakub Jelinek wrote:
> On Fri, Feb 19, 2016 at 11:08:48AM +, Bernd Edlinger wrote:
> > On 19.02.2016 11:56, Jakub Jelinek wrote:
> > >
> > > On Fri, Feb 19, 2016 at 10:50:34AM +, Bernd Edlinger wrote:
> > > > While I think that we should probably not define __GNUC_GNU_INLINE__ at 
> > > > all for C++,
> > > > because it is meaningless, I am warned that this could break (already 
> > > > broken) header files.
> > >
> > > It is not meaningless.  The various headers need to know if it is safe to
> > > use the gnu_inline attribute in C++.
> > >
> > > In any case, the desirable state is that e.g. the -E -dD output should be
> > > identical if you explicitly request the default -std= version vs. if it is
> > > set implicitly.  We should verify it is the case even for C.
> > >
> > > Jakub
> >
> > I absolutely agree with you.
> > The correct solution is probably doing this:
> >
> > --- gcc/cp/cfns.h.jj  2016-01-04 15:30:50.0 +0100
> > +++ gcc/cp/cfns.h 2016-02-19 12:00:15.730375049 +0100
> > @@ -124,9 +124,6 @@
> >
> >  #ifdef __GNUC__
> >  __inline
> > -#ifdef __GNUC_STDC_INLINE__
> > -__attribute__ ((__gnu_inline__))
> > -#endif
> >  #endif
> >  const char *
> >  libc_name_p (register const char *str, register unsigned int len)
> 
> This is of course wrong.  cfns.h is a generated header, so you shouldn't
> patch it.
> If it is regenerated with a newer gperf (I have 3.0.4 installed), you get 
> there:
> @@ -124,7 +124,7 @@ hash (register const char *str, register
> 
>  #ifdef __GNUC__
>  __inline
> -#ifdef __GNUC_STDC_INLINE__
> +#if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
>  __attribute__ ((__gnu_inline__))
>  #endif
>  #endif
> 
> Jakub

Damnit!!

I dont know how this file is ever supposed to compile on a C99 compiler.
but with this change it does not even compile with -std=c90 any more:

cat test.c
#include "cfns.h"

gcc -std=c90 test.c
In file included from test.c:1:0:
cfns.gperf:101:1: error: 'gnu_inline' attribute present on 'libc_name_p'
cfns.gperf:26:14: error: but not here

gcc -std=c99 test.c
In file included from test.c:1:0:
cfns.gperf:101:1: error: 'gnu_inline' attribute present on 'libc_name_p'
cfns.gperf:26:14: error: but not here
cfns.gperf: In function 'libc_name_p':
cfns.gperf:328:34: warning: implicit declaration of function 'strcmp' 
[-Wimplicit-function-declaration]


So this leaves only one solution, to define neither __GNUC_STDC_INLINE__
nor __GNUC_GNU_INLINE__ for C++, at least the inline and gnu_inline
is completely different on C and C++. But I am anxious this will break more
things than gperf, and I would like to fix this in a safe way.


Bernd.

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 11:53:03AM +, Bernd Edlinger wrote:
> >  #ifdef __GNUC__
> >  __inline
> > -#ifdef __GNUC_STDC_INLINE__
> > +#if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
> >  __attribute__ ((__gnu_inline__))
> >  #endif
> >  #endif
> > 
> > Jakub
> 
> Damnit!!
> 
> I dont know how this file is ever supposed to compile on a C99 compiler.
> but with this change it does not even compile with -std=c90 any more:
> 
> cat test.c
> #include "cfns.h"
> 
> gcc -std=c90 test.c
> In file included from test.c:1:0:
> cfns.gperf:101:1: error: 'gnu_inline' attribute present on 'libc_name_p'
> cfns.gperf:26:14: error: but not here
> 
> gcc -std=c99 test.c
> In file included from test.c:1:0:
> cfns.gperf:101:1: error: 'gnu_inline' attribute present on 'libc_name_p'
> cfns.gperf:26:14: error: but not here
> cfns.gperf: In function 'libc_name_p':
> cfns.gperf:328:34: warning: implicit declaration of function 'strcmp' 
> [-Wimplicit-function-declaration]
> 
> 
> So this leaves only one solution, to define neither __GNUC_STDC_INLINE__

Of course not, and that would be the wrong thing to do.
The definition spot of libc_name_p comes from gperf itself, the prototype
from cfns.gperf, which we can of course adjust.

> nor __GNUC_GNU_INLINE__ for C++, at least the inline and gnu_inline
> is completely different on C and C++. But I am anxious this will break more
> things than gperf, and I would like to fix this in a safe way.

IMNSHO we should just keep it consistent with what g++ e.g. 5.x did.
Thus,
$ g++ -E -dD -xc++ /dev/null -O2 -std=c++98 2>&1 | grep _INLINE_
#define __GNUC_GNU_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=c++11 2>&1 | grep _INLINE_
#define __GNUC_STDC_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=c++14 2>&1 | grep _INLINE_
#define __GNUC_STDC_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++98 2>&1 | grep _INLINE_
#define __GNUC_GNU_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++11 2>&1 | grep _INLINE_
#define __GNUC_STDC_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++14 2>&1 | grep _INLINE_
#define __GNUC_STDC_INLINE__ 1
$ g++ -E -dD -xc++ /dev/null -O2 2>&1 | grep _INLINE_
#define __GNUC_GNU_INLINE__ 1
We want to define what we did with the explicit -std= options, and just
change the output in the default case (last invocation), to
#define __GNUC_STDC_INLINE__ 1
because the default is different.

Jakub

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 12:59, Jakub Jelinek wrote:
> 
> Of course not, and that would be the wrong thing to do.
> The definition spot of libc_name_p comes from gperf itself, the prototype
> from cfns.gperf, which we can of course adjust.
>

Yes, now I understand.  Thanks.
 
>
> IMNSHO we should just keep it consistent with what g++ e.g. 5.x did.
>Thus,
> $ g++ -E -dD -xc++ /dev/null -O2 -std=c++98 2>&1 | grep _INLINE_
> #define __GNUC_GNU_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=c++11 2>&1 | grep _INLINE_
> #define __GNUC_STDC_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=c++14 2>&1 | grep _INLINE_
> #define __GNUC_STDC_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++98 2>&1 | grep _INLINE_
> #define __GNUC_GNU_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++11 2>&1 | grep _INLINE_
> #define __GNUC_STDC_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 -std=gnu++14 2>&1 | grep _INLINE_
> #define __GNUC_STDC_INLINE__ 1
> $ g++ -E -dD -xc++ /dev/null -O2 2>&1 | grep _INLINE_
> #define __GNUC_GNU_INLINE__ 1
> We want to define what we did with the explicit -std= options, and just
> change the output in the default case (last invocation), to
> #define __GNUC_STDC_INLINE__ 1
> because the default is different.
> 
>Jakub

OK. I can do that.
I will send a new patch in the evening.


Thanks
Bernd.

Re: [ping] Enable -mstackrealign with SSE on 32-bit Windows

[Richard Biener]

On Fri, Feb 19, 2016 at 12:50 PM, Eric Botcazou  wrote:
> Any (global or platform) maintainer willing to approve this for GCC6?
>   https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01458.html
>
> We switched to SSE2 by default on Windows 32-bit at AdaCore and we ran into
> really nasty stability issues before applying this patch.

Not sure - why doesn't mingw properly specify the default incoming
stack boundary?
If it does, why isn't there an issue elsewhere even on non-mingw if you use
-mincoming-stack-boundary=2?

Richard.

> --
> Eric Botcazou

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jakub Jelinek]

Hi!

On Fri, Feb 19, 2016 at 12:59:45PM +0100, Jakub Jelinek wrote:
> Of course not, and that would be the wrong thing to do.
> The definition spot of libc_name_p comes from gperf itself, the prototype
> from cfns.gperf, which we can of course adjust.

This is related to
https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00375.html
But, as we use it in a header, in C++ and don't provide an out of line
definition for it, we really don't want the gnu_inline attribute in this
case, because unlike in C, where gnu_inline attribute has one meaning when
applied to inline and another when applied to extern inline, in C++
it always means the extern inline GNU inline semantics.  And gperf clearly
wants the C inline __attribute__((gnu_inline)) semantics.
IMHO gperf should be fixed to only use __attribute__((gnu_inline)) for C++.

BTW, the prototypes aren't really needed anymore in cfns.gperf, we use it
in C++ only and C++ doesn't have -Wstrict-prototypes -Wmissing-prototypes.

So, for cfns.* I think we should use until gperf is fixed:

2016-02-19  Jakub Jelinek  

* cfns.gperf: Remove prototypes for hash and libc_name_p
inlines.  Undefine __GNUC_STDC_INLINE__ and
__GNUC_GNU_INLINE__ for C++.
* cfns.h: Regenerated.

--- gcc/cp/cfns.gperf.jj2016-01-04 14:55:57.0 +0100
+++ gcc/cp/cfns.gperf   2016-02-19 13:16:50.374311181 +0100
@@ -16,14 +16,13 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
+
+/* Don't use gnu_inline attribute that gperf 3.0.3 and later emits
+   for C++.  */
+#if defined(__cplusplus) && defined(__GNUC__)
+#  undef __GNUC_STDC_INLINE__
+#  undef __GNUC_GNU_INLINE__
 #endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
-#endif
-const char * libc_name_p (const char *, unsigned int);
 %}
 %%
 # The standard C library functions, for feeding to gperf; the result is used
--- gcc/cp/cfns.h.jj2016-01-04 14:55:57.0 +0100
+++ gcc/cp/cfns.h   2016-02-19 13:17:02.458142071 +0100
@@ -1,4 +1,4 @@
-/* ANSI-C code produced by gperf version 3.0.3 */
+/* ANSI-C code produced by gperf version 3.0.4 */
 /* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L ANSI-C 
cfns.gperf  */
 
 #if !((' ' == 32) && ('!' == 33) && ('"' == 34) && ('#' == 35) \
@@ -47,14 +47,13 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
-#endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
+
+/* Don't use gnu_inline attribute that gperf 3.0.3 and later emits
+   for C++.  */
+#if defined(__cplusplus) && defined(__GNUC__)
+#  undef __GNUC_STDC_INLINE__
+#  undef __GNUC_GNU_INLINE__
 #endif
-const char * libc_name_p (const char *, unsigned int);
 /* maximum key range = 391, duplicates = 0 */
 
 #ifdef __GNUC__
@@ -124,7 +123,7 @@ hash (register const char *str, register
 
 #ifdef __GNUC__
 __inline
-#ifdef __GNUC_STDC_INLINE__
+#if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
 __attribute__ ((__gnu_inline__))
 #endif
 #endif

Jakub

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 01:22:12PM +0100, Jakub Jelinek wrote:
> wants the C inline __attribute__((gnu_inline)) semantics.
> IMHO gperf should be fixed to only use __attribute__((gnu_inline)) for C++.

For C of course.
Thus emit
#ifndef __cplusplus
#if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
__attribute__ ((__gnu_inline__))
#endif
#endif
or something similar.

Jakub

Re: [PATCH] Avoid bugs like PR68273 to trigger

[Bernd Schmidt]

On 02/19/2016 09:36 AM, Richard Biener wrote:

Yup, so we should make sure we don't (even not "out of nowhere").  See
my attempts on adding some SSA verification for this (needs to be
restricted to overaligned, then it doesn't trigger that often...).
One issue is that we've often got DECLs that are overaligned but at
some point re-write them into SSA - and the SSA verifier expects the
SSA name types to agree with their decls type.


Sounds like the verifier needs to be relaxed a little?


Bernd

Re: [PATCH] Add debug_function_to_file

[Tom de Vries]

On 19/02/16 10:44, Richard Biener wrote:

On Fri, Feb 19, 2016 at 1:37 AM, Tom de Vries  wrote:

On 18/02/16 16:27, Richard Biener wrote:


Attached is what I have for now, it works if you call it like

(gdb) dot-fn cfun
(gdb) dot-fn cfun, 1<<6

w/o that arg parsing;)

I'll play with it some more tomorrow.



This version:
- uses arg parsing
- adds error handling
- uses a temp file instead of a pipe
- uses python os.system to call dot


I used popen


It was suggested in the other thread ( 
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01172.html ) that there 
were portability issues with popen, so I tried to avoid it.


Anyway, I guess it's probably best to try to minimize interaction with 
the inferior, if we can handle it in python instead.



specifically to allow you continue debugging while keeping the dot
process open and functional.


Ah, I see.


That would be restored with adding a '&' after
the command but then we race with the file removal ...

The following works for me though:

 # Show graph in temp file
 os.system("( dot -Tx11 %s; rm %s ) &" % (filename, filename) )

dot_fn()



Yep, that works for me too.

[ Btw, I prefer generating dot graps in pdf format. In the pdf viewer, 
you can search for variable names, bb numbers, etc.


It's easy enough to generate the pdf file, but I'm not sure how in 
python we can open the pdf viewer, and wait for it to finish in a 
portable way. xdg-open is linux-only and doesn't seem to wait-on 
-finish. And pythons webbrowser.open_new does wait-on-finish in some 
cases, but not in others, and I don't understand the documentation well 
enough to know if that's going to affect this use-case. ]



ok for trunk with that change and thanks for the help!



You're welcome :) . [ I used to use python regularly (though at a basic 
level) about 6 years ago, but haven't used it since, so it was good to 
revive that skill a bit. ]


I'll commit as attached, unless David has further review comments on 
dot-fn (or comments on dump-fn which would also apply for dot-fn).


Thanks,
- Tom
Add dot-fn to gdbhooks.py

2016-02-18  Richard Biener  

	* graph.c: Include dumpfile.h.
	(print_graph_cfg): Split into three overloads.
	* gdbhooks.py (dot-fn): New command.

---
 gcc/gdbhooks.py | 70 +
 gcc/graph.c | 32 ++
 2 files changed, 98 insertions(+), 4 deletions(-)

diff --git a/gcc/gdbhooks.py b/gcc/gdbhooks.py
index ed72016..1212312 100644
--- a/gcc/gdbhooks.py
+++ b/gcc/gdbhooks.py
@@ -680,4 +680,74 @@ class DumpFn(gdb.Command):
 
 DumpFn()
 
+class DotFn(gdb.Command):
+"""
+A custom command to show a gimple/rtl function control flow graph.
+By default, it show the current function, but the function can also be
+specified.
+
+Examples of use:
+  (gdb) dot-fn
+  (gdb) dot-fn cfun
+  (gdb) dot-fn cfun 0
+  (gdb) dot-fn cfun dump_flags
+"""
+def __init__(self):
+gdb.Command.__init__(self, 'dot-fn', gdb.COMMAND_USER)
+
+def invoke(self, arg, from_tty):
+# Parse args, check number of args
+args = gdb.string_to_argv(arg)
+if len(args) > 2:
+print("Too many arguments")
+return
+
+# Set func
+if len(args) >= 1:
+funcname = args[0]
+printfuncname = "function %s" % funcname
+else:
+funcname = "cfun"
+printfuncname = "current function"
+func = gdb.parse_and_eval(funcname)
+if func == 0:
+print("Could not find %s" % printfuncname)
+return
+func = "(struct function *)%s" % func
+
+# Set flags
+if len(args) >= 2:
+flags = gdb.parse_and_eval(args[1])
+else:
+flags = 0
+
+# Get temp file
+f = tempfile.NamedTemporaryFile(delete=False)
+filename = f.name
+
+# Close and reopen temp file to get C FILE*
+f.close()
+fp = gdb.parse_and_eval("fopen (\"%s\", \"w\")" % filename)
+if fp == 0:
+print("Cannot open temp file")
+return
+fp = "(FILE *)%u" % fp
+
+# Write graph to temp file
+_ = gdb.parse_and_eval("start_graph_dump (%s, \"\")" % fp)
+_ = gdb.parse_and_eval("print_graph_cfg (%s, %s, %u)"
+   % (fp, func, flags))
+_ = gdb.parse_and_eval("end_graph_dump (%s)" % fp)
+
+# Close temp file
+ret = gdb.parse_and_eval("fclose (%s)" % fp)
+if ret != 0:
+print("Could not close temp file: %s" % filename)
+return
+
+# Show graph in temp file
+os.system("( dot -Tx11 \"%s\"; rm \"%s\" ) &" % (filename, filename))
+
+DotFn()
+
 print('Successfully loaded GDB hooks for GCC')
diff --git a/gcc/graph.c b/gcc/graph.c
index 1b28c67..dd5bc4e 100644
--- a/gcc/graph.c
+++ b/gcc/graph.c
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.  If

Re: [PATCH] Avoid bugs like PR68273 to trigger

[Richard Biener]

On Fri, 19 Feb 2016, Bernd Schmidt wrote:

> On 02/19/2016 09:36 AM, Richard Biener wrote:
> > Yup, so we should make sure we don't (even not "out of nowhere").  See
> > my attempts on adding some SSA verification for this (needs to be
> > restricted to overaligned, then it doesn't trigger that often...).
> > One issue is that we've often got DECLs that are overaligned but at
> > some point re-write them into SSA - and the SSA verifier expects the
> > SSA name types to agree with their decls type.
> 
> Sounds like the verifier needs to be relaxed a little?

No, we really use those two types interchangeably.  If it's
a DECL_INGORED var adjusting it is fine I think, but if it's a
user var we'd drop some debug info (do we actually emit alignment
info?) and if it's a PARM_DECL then it may change the ABI
(in which case their SSA names maybe should have overaligned type...)

Richard.

> 
> Bernd
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

[patch] Clarify interaction of -Wnarrowing with -std

[Jonathan Wakely]

In PR69864 Manu suggests improving the docs to explain that
-Wnarrowing sometimes produces errors not warnings.

I think the right way to do that is clarify how it interacts with
-std. Specifically that the effect of -Wnarrowing listed first in the
manual *only* applies to C++98 modes, For all later modes (not just
with -std=c++11 as it says now), narrowing conversions produce errors
or warnings by default.

OK for trunk?


commit b78b2728d8d946bd92843f6155cdd2415682da09
Author: Jonathan Wakely 
Date:   Fri Feb 19 12:14:33 2016 +

	* doc/invoke.texi (C++ Dialect Options): Clarify interaction of
	-Wnarrowing with -std.

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2bd793d..c1ab788 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2756,7 +2756,8 @@ Requires @option{-flto} to be enabled.  Enabled by default.
 @item -Wnarrowing @r{(C++ and Objective-C++ only)}
 @opindex Wnarrowing
 @opindex Wno-narrowing
-Warn when a narrowing conversion prohibited by C++11 occurs within
+With @option{-std=gnu++98} or @option{-std=c++98}, warn when a narrowing
+conversion prohibited by C++11 occurs within
 @samp{@{ @}}, e.g.
 
 @smallexample
@@ -2765,10 +2766,13 @@ int i = @{ 2.2 @}; // error: narrowing from double to int
 
 This flag is included in @option{-Wall} and @option{-Wc++11-compat}.
 
-With @option{-std=c++11}, @option{-Wno-narrowing} suppresses the diagnostic
-required by the standard.  Note that this does not affect the meaning
-of well-formed code; narrowing conversions are still considered
-ill-formed in SFINAE context.
+When a later standard is in effect, e.g. when using @option{-std=c++11},
+narrowing conversions are diagnosed by default, as required by the standard.
+A narrowing conversion from a constant produces an error,
+and a narrowing conversion from a non-constant produces a warning,
+but @option{-Wno-narrowing} suppresses the diagnostic.
+Note that this does not affect the meaning of well-formed code;
+narrowing conversions are still considered ill-formed in SFINAE contexts.
 
 @item -Wnoexcept @r{(C++ and Objective-C++ only)}
 @opindex Wnoexcept

Re: [PATCH] Avoid bugs like PR68273 to trigger

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 01:42:20PM +0100, Richard Biener wrote:
> On Fri, 19 Feb 2016, Bernd Schmidt wrote:
> 
> > On 02/19/2016 09:36 AM, Richard Biener wrote:
> > > Yup, so we should make sure we don't (even not "out of nowhere").  See
> > > my attempts on adding some SSA verification for this (needs to be
> > > restricted to overaligned, then it doesn't trigger that often...).
> > > One issue is that we've often got DECLs that are overaligned but at
> > > some point re-write them into SSA - and the SSA verifier expects the
> > > SSA name types to agree with their decls type.
> > 
> > Sounds like the verifier needs to be relaxed a little?
> 
> No, we really use those two types interchangeably.  If it's
> a DECL_INGORED var adjusting it is fine I think, but if it's a
> user var we'd drop some debug info (do we actually emit alignment

I think we don't right now, but DWARF5 is going to have attributes for this,
so we'll start soon.

> info?) and if it's a PARM_DECL then it may change the ABI
> (in which case their SSA names maybe should have overaligned type...)

Jakub

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 13:22, Jakub Jelinek wrote:
> Hi!
>
> On Fri, Feb 19, 2016 at 12:59:45PM +0100, Jakub Jelinek wrote:
>> Of course not, and that would be the wrong thing to do.
>> The definition spot of libc_name_p comes from gperf itself, the prototype
>> from cfns.gperf, which we can of course adjust.
>
> This is related to
> https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00375.html
> But, as we use it in a header, in C++ and don't provide an out of line
> definition for it, we really don't want the gnu_inline attribute in this
> case, because unlike in C, where gnu_inline attribute has one meaning when
> applied to inline and another when applied to extern inline, in C++
> it always means the extern inline GNU inline semantics.  And gperf clearly
> wants the C inline __attribute__((gnu_inline)) semantics.
> IMHO gperf should be fixed to only use __attribute__((gnu_inline)) for C++.
>
> BTW, the prototypes aren't really needed anymore in cfns.gperf, we use it
> in C++ only and C++ doesn't have -Wstrict-prototypes -Wmissing-prototypes.
>
> So, for cfns.* I think we should use until gperf is fixed:
>

Yeah, that is ugly, but we don't have any alternatives.
So please commit then I can continue with my new patch.

Thanks
Bernd.

> 2016-02-19  Jakub Jelinek  
>
>   * cfns.gperf: Remove prototypes for hash and libc_name_p
>   inlines.  Undefine __GNUC_STDC_INLINE__ and
>   __GNUC_GNU_INLINE__ for C++.
>   * cfns.h: Regenerated.
>
> --- gcc/cp/cfns.gperf.jj  2016-01-04 14:55:57.0 +0100
> +++ gcc/cp/cfns.gperf 2016-02-19 13:16:50.374311181 +0100
> @@ -16,14 +16,13 @@ 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
>   .  */
> -#ifdef __GNUC__
> -__inline
> +
> +/* Don't use gnu_inline attribute that gperf 3.0.3 and later emits
> +   for C++.  */
> +#if defined(__cplusplus) && defined(__GNUC__)
> +#  undef __GNUC_STDC_INLINE__
> +#  undef __GNUC_GNU_INLINE__
>   #endif
> -static unsigned int hash (const char *, unsigned int);
> -#ifdef __GNUC__
> -__inline
> -#endif
> -const char * libc_name_p (const char *, unsigned int);
>   %}
>   %%
>   # The standard C library functions, for feeding to gperf; the result is used
> --- gcc/cp/cfns.h.jj  2016-01-04 14:55:57.0 +0100
> +++ gcc/cp/cfns.h 2016-02-19 13:17:02.458142071 +0100
> @@ -1,4 +1,4 @@
> -/* ANSI-C code produced by gperf version 3.0.3 */
> +/* ANSI-C code produced by gperf version 3.0.4 */
>   /* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L ANSI-C 
> cfns.gperf  */
>
>   #if !((' ' == 32) && ('!' == 33) && ('"' == 34) && ('#' == 35) \
> @@ -47,14 +47,13 @@ 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
>   .  */
> -#ifdef __GNUC__
> -__inline
> -#endif
> -static unsigned int hash (const char *, unsigned int);
> -#ifdef __GNUC__
> -__inline
> +
> +/* Don't use gnu_inline attribute that gperf 3.0.3 and later emits
> +   for C++.  */
> +#if defined(__cplusplus) && defined(__GNUC__)
> +#  undef __GNUC_STDC_INLINE__
> +#  undef __GNUC_GNU_INLINE__
>   #endif
> -const char * libc_name_p (const char *, unsigned int);
>   /* maximum key range = 391, duplicates = 0 */
>
>   #ifdef __GNUC__
> @@ -124,7 +123,7 @@ hash (register const char *str, register
>
>   #ifdef __GNUC__
>   __inline
> -#ifdef __GNUC_STDC_INLINE__
> +#if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
>   __attribute__ ((__gnu_inline__))
>   #endif
>   #endif
>
>   Jakub
>

Re: [PATCH] Fix ICE in vcond expansion with -mavx512f -mno-avx512bw (PR target/69820)

[Kirill Yukhin]

Hi Jakub!
On 15 Feb 22:00, Jakub Jelinek wrote:
> Hi!
> 
> We ICE on the following testcase, because vcondv32hiv32hi pattern
> really needs avx512bw, but it is enabled for avx512f.
> As VI_512 iterator is only used in vcond* patterns which need the
> avx512bw ISA for the V64QI and V32HI modes, I've changed that iterator.
> Or do you prefer to keep that iterator as is (so it will be unused)
> and another one with these conditions?  If yes, how should it be called.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux.
Patch is ok for trunk and branches.

> 2016-02-15  Jakub Jelinek  
> 
>   PR target/69820
>   * config/i386/sse.md (VI_512): Only include V64QImode and V32HImode
>   if TARGET_AVX512BW.
> 
>   * gcc.target/i386/pr69820.c: New test.
> 
> --- gcc/config/i386/sse.md.jj 2016-02-03 23:36:39.0 +0100
> +++ gcc/config/i386/sse.md2016-02-15 17:07:40.694352994 +0100
> @@ -522,7 +522,10 @@ (define_mode_iterator VI_128 [V16QI V8HI
>  (define_mode_iterator VI_256 [V32QI V16HI V8SI V4DI])
>  
>  ;; All 512bit vector integer modes
> -(define_mode_iterator VI_512 [V64QI V32HI V16SI V8DI])
> +(define_mode_iterator VI_512
> +  [(V64QI "TARGET_AVX512BW")
> +   (V32HI "TARGET_AVX512BW")
> +   V16SI V8DI])
>  
>  ;; Various 128bit vector integer mode combinations
>  (define_mode_iterator VI12_128 [V16QI V8HI])
> --- gcc/testsuite/gcc.target/i386/pr69820.c.jj2016-02-15 
> 17:13:57.397220839 +0100
> +++ gcc/testsuite/gcc.target/i386/pr69820.c   2016-02-15 17:13:28.0 
> +0100
> @@ -0,0 +1,14 @@
> +/* PR target/69820 */
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -mavx512f -mno-avx512bw" } */
> +
> +int a[100], b[100];
> +short c[100];
> +
> +void
> +foo ()
> +{
> +  int i;
> +  for (i = 0; i < 100; ++i)
> +b[i] = a[i] * (_Bool) c[i];
> +}
> 
>   Jakub
--
Thanks, K

[PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Jakub Jelinek]

Hi!

As described in the PR, in C++ we can have assignments
where both the lhs and rhs are COMPONENT_REFs with TREE_ADDRESSABLE types,
including padding, but the FIELD_DECLs are artificial fields that have
narrower bit sizes.
store_field in this case takes the path of bit-field handling (even when
it has bitpos and bitsize multiples of BITS_PER_UNIT (I think that is
necessarily true for the TREE_ADDRESSABLE types), which is incorrect,
because the rhs is expanded in that case through expand_normal, which
for a result type wider than the FIELD_DECL with forces it into a temporary.
In older GCCs that just generated inefficient code (copy the rhs into a
stack temporary, then copy that to lhs), but GCC trunk ICEs on that.
Fixed by not taking the bit-field path in that case after verifying
we'll be able to expand it properly using the normal store_expr.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-02-19  Jakub Jelinek  

PR c++/69851
* expr.c (store_field): Don't use bit-field path if exp is
COMPONENT_REF with TREE_ADDRESSABLE type, where TYPE_SIZE is
different from bitsize, but DECL_SIZE of FIELD_DECL is bitsize
and the assignment can be performed by bitwise copy.  Formatting
fix.

* g++.dg/torture/pr69851.C: New test.

--- gcc/expr.c.jj   2016-02-12 00:50:55.0 +0100
+++ gcc/expr.c  2016-02-19 10:43:59.639162531 +0100
@@ -6643,14 +6643,24 @@ store_field (rtx target, HOST_WIDE_INT b
  /* Except for initialization of full bytes from a CONSTRUCTOR, which
 we will handle specially below.  */
  && !(TREE_CODE (exp) == CONSTRUCTOR
-  && bitsize % BITS_PER_UNIT == 0))
+  && bitsize % BITS_PER_UNIT == 0)
+ /* And except for bitwise copying of TREE_ADDRESSABLE types,
+where the FIELD_DECL has the right bitsize, but TREE_TYPE (exp)
+includes some extra padding.  */
+ && (!TREE_ADDRESSABLE (TREE_TYPE (exp))
+ || TREE_CODE (exp) != COMPONENT_REF
+ || TREE_CODE (DECL_SIZE (TREE_OPERAND (exp, 1))) != INTEGER_CST
+ || (bitsize % BITS_PER_UNIT != 0)
+ || (bitpos % BITS_PER_UNIT != 0)
+ || (compare_tree_int (DECL_SIZE (TREE_OPERAND (exp, 1)), bitsize)
+ != 0)))
   /* If we are expanding a MEM_REF of a non-BLKmode non-addressable
  decl we must use bitfield operations.  */
   || (bitsize >= 0
  && TREE_CODE (exp) == MEM_REF
  && TREE_CODE (TREE_OPERAND (exp, 0)) == ADDR_EXPR
  && DECL_P (TREE_OPERAND (TREE_OPERAND (exp, 0), 0))
- && !TREE_ADDRESSABLE (TREE_OPERAND (TREE_OPERAND (exp, 0),0 ))
+ && !TREE_ADDRESSABLE (TREE_OPERAND (TREE_OPERAND (exp, 0), 0))
  && DECL_MODE (TREE_OPERAND (TREE_OPERAND (exp, 0), 0)) != BLKmode))
 {
   rtx temp;
--- gcc/testsuite/g++.dg/torture/pr69851.C.jj   2016-02-19 10:59:22.224438830 
+0100
+++ gcc/testsuite/g++.dg/torture/pr69851.C  2016-02-19 10:59:12.0 
+0100
@@ -0,0 +1,24 @@
+// PR c++/69851
+// { dg-do compile }
+// { dg-options "-std=c++11" }
+
+template 
+struct A { T a; };
+template 
+struct B;
+template 
+struct B : B<1, U...>, A
+{
+  B (B &) = default;
+  B (B &&x) : B(x) {}
+};
+template 
+struct B {};
+struct C { C (C &); };
+struct D {};
+
+void
+foo (B<0, C, D, int, int> a)
+{
+  B<0, C, D, int, int> b (a);
+}

Jakub

Re: [PATCH][AArch64][v2] Skip gcc.target/aarch64/assembler_arch_1.c if assembler does not support it

[Kyrill Tkachov]

On 18/02/16 14:24, James Greenhalgh wrote:

On Thu, Feb 18, 2016 at 11:31:02AM +0100, Christophe Lyon wrote:

On 17 February 2016 at 17:06, Kyrill Tkachov
 wrote:

Hi all,

I've thought about this check a bit more and I think we can compactly
auto-generate checks
for any aarch64 architecture extension support in the assembler.
This is done in a similar way we autogenerate the arm_arch_*_ok checks for
arm.

So in this revision we autogenerate aarch64_asm__ok checks for every
architecture extension
using some of the expect machinery. This should make this approach a bit
more general to handle
checks for any .arch_extension argument without much extra cost.

This still assumes that the assembler supports the .arch_extension
pseudo-op, the effective
target check will fail if it doesn't. This is what we want for this
testcase.

Is this patch ok instead of
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01052.html ?


Nice indeed.

Regarding the doc, it's not accurate to say that the values of ext
are defined in aarch64-option-extensions.def, since that file is not
actually parsed by DJ. I mean there is no guarantee the two lists
will be kept in sync.

In the new test itself, I think that
return [check_no_compiler_messages aarch64_lse_assembler object
should be:
return [check_no_compiler_messages aarch64_FUNC_assembler object

for consistency although your patch is functional as-is.

Agreed.

OK with that change.


Thanks, here's what I committed with r233559.

Kyrill

2016-02-19  Kyrylo Tkachov  

* lib/target-supports.exp: Define aarch64_asm_FUNC_ok checks
for fp, simd, crypto, crc, lse.
* doc/sourcebuild.texi (AArch64-specific attributes): Document the
above.
* gcc.target/aarch64/assembler_arch_1.c: Add aarch64_asm_lse_ok
effective target check.



Thanks,
James


2016-02-17  Kyrylo Tkachov  

 * lib/target-supports.exp: Define aarch64_asm_FUNC_ok checks
 for fp, simd, crypto, crc, lse.
 * doc/sourcebuild.texi (AArch64-specific attributes): Document the
 above.
 * gcc.target/aarch64/assembler_arch_1.c: Add aarch64_asm_lse_ok
 effective target check.


diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 6d548aad7aa24c59b40ec13d9c99733d94ec0aa6..c5354cfc8f36f453fedf0b6879b1dc1ec663f1b5 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -1606,6 +1606,9 @@ ARM target prefers @code{LDRD} and @code{STRD} instructions over
 @subsubsection AArch64-specific attributes
 
 @table @code
+@item aarch64_asm__ok
+AArch64 assembler supports the architecture extension @code{ext} via the
+@code{.arch_extension} pseudo-op.
 @item aarch64_tiny
 AArch64 target which generates instruction sequences for tiny memory model.
 @item aarch64_small
diff --git a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
index 901e50a178d7a4a443a5ad0abe63f624688db268..5deea5cf0ee9306743bc47bace6f762d0e35ce65 100644
--- a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
@@ -1,4 +1,5 @@
 /* { dg-do assemble } */
+/* { dg-require-effective-target aarch64_asm_lse_ok } */
 /* { dg-options "-march=armv8-a" } */
 
 /* Make sure that the function header in assembly doesn't override
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 66fb1eaf7bd4aa58d23cfc9203e9f27573c7a303..0b4252f6434fb8223423e06882a061ccf0f5a015 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -6719,6 +6719,23 @@ proc check_effective_target_aarch64_tiny { } {
 }
 }
 
+# Create functions to check that the AArch64 assembler supports the
+# various architecture extensions via the .arch_extension pseudo-op.
+
+foreach { aarch64_ext } { "fp" "simd" "crypto" "crc" "lse"} {
+eval [string map [list FUNC $aarch64_ext] {
+	proc check_effective_target_aarch64_asm_FUNC_ok { } {
+	  if { [istarget aarch64*-*-*] } {
+		return [check_no_compiler_messages aarch64_FUNC_assembler object {
+			__asm__ (".arch_extension FUNC");
+		} "-march=armv8-a+FUNC"]
+	  } else {
+		return 0
+	  }
+	}
+}]
+}
+
 proc check_effective_target_aarch64_small { } {
 if { [istarget aarch64*-*-*] } {
 	return [check_no_compiler_messages aarch64_small object {

Re: [PATCH][RFC][Offloading] Fix PR68463

[Jakub Jelinek]

On Wed, Feb 10, 2016 at 08:19:34PM +0300, Ilya Verbin wrote:
> This patch adds crtoffload{begin,end}.o to all -fopenmp programs, if they 
> exist.
> I couldn't think of a better solution...
> Tested using the testcase from the previous mail, e.g.:
> 
> $ gcc -DNUM=1 -c -fopenmp test.c -o obj1.o
> $ gcc -DNUM=2 -c -fopenmp test.c -o obj2.o
> $ gcc -DNUM=3 -c -fopenmp test.c -o obj3.o
> $ gcc -DNUM=4 -c -fopenmp test.c -o obj4.o -flto
> $ gcc -DNUM=5 -c -fopenmp test.c -o obj5.o
> $ gcc -DNUM=6 -c -fopenmp test.c -o obj6.o -flto
> $ gcc -DNUM=7 -c -fopenmp test.c -o obj7.o
> $ gcc-ar -cvq libtest.a obj3.o obj4.o obj5.o
> $ gcc -fopenmp main.c obj1.o obj2.o libtest.a obj6.o obj7.o
> 
> And other combinations.

Looking at this, I think I have no problem with crtoffloadbegin.o being
included in all -fopenmp/-fopenacc linked programs/shared libraries,
that just defines the symbols and nothing else.
I have no problem with the
__offload_funcs_end/__offload_vars_end part of crtoffloadend.o being
included too.
But, I really don't like __OFFLOAD_TABLE__ being added to all programs, that
wastes real space in data (rodata or relro?) section, and dynamic
relocations.
So, perhaps, can we split offloadstuff.c into 3 objects instead of 2,
crtoffload{begin,end,table}.o*, where the last one would be what
defines __OFFLOAD_TABLE__, and add the last one only by the linker
plugin/lto-wrapper/whatever, if any input objects had any offloading stuff
in it?

Jakub

[PATCH] Fix up handling of REG_EH_REGION notes in LRA (PR middle-end/69838)

[Jakub Jelinek]

Hi!

For -fnon-call-exceptions, if an instruction with REG_EH_REGION note is
reloaded, we should copy or move it to the instruction(s) corresponding to
the original one that could throw.  reload1.c apparently does this, but LRA
does not, so we can end up with REG_EH_REGION notes being dropped, or not
present on insns that actually can throw etc.

Fixed by calling the functions reload1.c does for this purpose.

Bootstrapped/regtested on x86_64-linux (including Ada) and i686-linux
(without Ada), and Dominik has kindly tested this on s390x-linux
(presumably with Ada, but don't know for sure).  Ok for trunk?

2016-02-19  Jakub Jelinek  

PR middle-end/69838
* lra.c (lra_process_new_insns): If non-call exceptions are enabled,
call copy_reg_eh_region_note_forward on before and/or after sequences
and remove note from insn if it no longer can throw.

--- gcc/lra.c.jj2016-01-04 14:55:52.0 +0100
+++ gcc/lra.c   2016-02-19 12:01:49.747724404 +0100
@@ -1742,20 +1742,29 @@ lra_process_new_insns (rtx_insn *insn, r
 }
   if (before != NULL_RTX)
 {
+  if (cfun->can_throw_non_call_exceptions)
+   copy_reg_eh_region_note_forward (insn, before, NULL);
   emit_insn_before (before, insn);
   push_insns (PREV_INSN (insn), PREV_INSN (before));
   setup_sp_offset (before, PREV_INSN (insn));
 }
   if (after != NULL_RTX)
 {
+  if (cfun->can_throw_non_call_exceptions)
+   copy_reg_eh_region_note_forward (insn, after, NULL);
   for (last = after; NEXT_INSN (last) != NULL_RTX; last = NEXT_INSN (last))
;
   emit_insn_after (after, insn);
   push_insns (last, insn);
   setup_sp_offset (after, last);
 }
+  if (cfun->can_throw_non_call_exceptions)
+{
+  rtx note = find_reg_note (insn, REG_EH_REGION, NULL_RTX);
+  if (note && !insn_could_throw_p (insn))
+   remove_note (insn, note);
+}
 }
-
 
 
 /* Replace all references to register OLD_REGNO in *LOC with pseudo

Jakub

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 13:26, Jakub Jelinek wrote:
> On Fri, Feb 19, 2016 at 01:22:12PM +0100, Jakub Jelinek wrote:
>> wants the C inline __attribute__((gnu_inline)) semantics.
>> IMHO gperf should be fixed to only use __attribute__((gnu_inline)) for C++.
>
> For C of course.
> Thus emit
> #ifndef __cplusplus
> #if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
> __attribute__ ((__gnu_inline__))
> #endif
> #endif
> or something similar.
>
>   Jakub
>

Hmm... wait a moment.
How about that?



Bernd.
Index: gcc/cp/Make-lang.in
===
--- gcc/cp/Make-lang.in	(revision 233557)
+++ gcc/cp/Make-lang.in	(working copy)
@@ -112,7 +112,7 @@ else
 # deleting the $(srcdir)/cp/cfns.h file.
 $(srcdir)/cp/cfns.h:
 endif
-	gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L ANSI-C \
+	gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L C++ \
 		$(srcdir)/cp/cfns.gperf --output-file $(srcdir)/cp/cfns.h
 
 #
Index: gcc/cp/cfns.gperf
===
--- gcc/cp/cfns.gperf	(revision 233557)
+++ gcc/cp/cfns.gperf	(working copy)
@@ -16,14 +16,7 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
-#endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
-#endif
-const char * libc_name_p (const char *, unsigned int);
+#define libc_name_p Perfect_Hash::libc_name_p
 %}
 %%
 # The standard C library functions, for feeding to gperf; the result is used
Index: gcc/cp/cfns.h
===
--- gcc/cp/cfns.h	(revision 233557)
+++ gcc/cp/cfns.h	(working copy)
@@ -1,5 +1,5 @@
-/* ANSI-C code produced by gperf version 3.0.3 */
-/* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L ANSI-C cfns.gperf  */
+/* C++ code produced by gperf version 3.0.4 */
+/* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L C++ --output-file cfns.h cfns.gperf  */
 
 #if !((' ' == 32) && ('!' == 33) && ('"' == 34) && ('#' == 35) \
   && ('%' == 37) && ('&' == 38) && ('\'' == 39) && ('(' == 40) \
@@ -47,26 +47,20 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
-#endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
-#endif
-const char * libc_name_p (const char *, unsigned int);
+#define libc_name_p Perfect_Hash::libc_name_p
 /* maximum key range = 391, duplicates = 0 */
 
-#ifdef __GNUC__
-__inline
-#else
-#ifdef __cplusplus
-inline
-#endif
-#endif
-static unsigned int
-hash (register const char *str, register unsigned int len)
+class Perfect_Hash
 {
+private:
+  static inline unsigned int hash (const char *str, unsigned int len);
+public:
+  static const char *libc_name_p (const char *str, unsigned int len);
+};
+
+inline unsigned int
+Perfect_Hash::hash (register const char *str, register unsigned int len)
+{
   static const unsigned short asso_values[] =
 {
   400, 400, 400, 400, 400, 400, 400, 400, 400, 400,
@@ -122,14 +116,8 @@ along with GCC; see the file COPYING3.  If not see
   return hval + asso_values[(unsigned char)str[len - 1]];
 }
 
-#ifdef __GNUC__
-__inline
-#ifdef __GNUC_STDC_INLINE__
-__attribute__ ((__gnu_inline__))
-#endif
-#endif
 const char *
-libc_name_p (register const char *str, register unsigned int len)
+Perfect_Hash::libc_name_p (register const char *str, register unsigned int len)
 {
   enum
 {

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 13:26, Jakub Jelinek wrote:
> On Fri, Feb 19, 2016 at 01:22:12PM +0100, Jakub Jelinek wrote:
>> wants the C inline __attribute__((gnu_inline)) semantics.
>> IMHO gperf should be fixed to only use __attribute__((gnu_inline)) for C++.
>
> For C of course.
> Thus emit
> #ifndef __cplusplus
> #if defined __GNUC_STDC_INLINE__ || defined __GNUC_GNU_INLINE__
> __attribute__ ((__gnu_inline__))
> #endif
> #endif
> or something similar.
>
>   Jakub
>

Hmm... wait a moment.
How about that?



Berns.
Index: gcc/cp/Make-lang.in
===
--- gcc/cp/Make-lang.in	(revision 233557)
+++ gcc/cp/Make-lang.in	(working copy)
@@ -112,7 +112,7 @@ else
 # deleting the $(srcdir)/cp/cfns.h file.
 $(srcdir)/cp/cfns.h:
 endif
-	gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L ANSI-C \
+	gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L C++ \
 		$(srcdir)/cp/cfns.gperf --output-file $(srcdir)/cp/cfns.h
 
 #
Index: gcc/cp/cfns.gperf
===
--- gcc/cp/cfns.gperf	(revision 233557)
+++ gcc/cp/cfns.gperf	(working copy)
@@ -16,14 +16,7 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
-#endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
-#endif
-const char * libc_name_p (const char *, unsigned int);
+#define libc_name_p Perfect_Hash::libc_name_p
 %}
 %%
 # The standard C library functions, for feeding to gperf; the result is used
Index: gcc/cp/cfns.h
===
--- gcc/cp/cfns.h	(revision 233557)
+++ gcc/cp/cfns.h	(working copy)
@@ -1,5 +1,5 @@
-/* ANSI-C code produced by gperf version 3.0.3 */
-/* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L ANSI-C cfns.gperf  */
+/* C++ code produced by gperf version 3.0.4 */
+/* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L C++ --output-file cfns.h cfns.gperf  */
 
 #if !((' ' == 32) && ('!' == 33) && ('"' == 34) && ('#' == 35) \
   && ('%' == 37) && ('&' == 38) && ('\'' == 39) && ('(' == 40) \
@@ -47,26 +47,20 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
-#endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
-#endif
-const char * libc_name_p (const char *, unsigned int);
+#define libc_name_p Perfect_Hash::libc_name_p
 /* maximum key range = 391, duplicates = 0 */
 
-#ifdef __GNUC__
-__inline
-#else
-#ifdef __cplusplus
-inline
-#endif
-#endif
-static unsigned int
-hash (register const char *str, register unsigned int len)
+class Perfect_Hash
 {
+private:
+  static inline unsigned int hash (const char *str, unsigned int len);
+public:
+  static const char *libc_name_p (const char *str, unsigned int len);
+};
+
+inline unsigned int
+Perfect_Hash::hash (register const char *str, register unsigned int len)
+{
   static const unsigned short asso_values[] =
 {
   400, 400, 400, 400, 400, 400, 400, 400, 400, 400,
@@ -122,14 +116,8 @@ along with GCC; see the file COPYING3.  If not see
   return hval + asso_values[(unsigned char)str[len - 1]];
 }
 
-#ifdef __GNUC__
-__inline
-#ifdef __GNUC_STDC_INLINE__
-__attribute__ ((__gnu_inline__))
-#endif
-#endif
 const char *
-libc_name_p (register const char *str, register unsigned int len)
+Perfect_Hash::libc_name_p (register const char *str, register unsigned int len)
 {
   enum
 {

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 03:09:25PM +, Bernd Edlinger wrote:
> Hmm... wait a moment.
> How about that?

Guess it is mostly reasonable (still, this is C++ FE, so I'd prefer
Jason to ack it), except for:

> +#define libc_name_p Perfect_Hash::libc_name_p

this.  Doesn't that cause
Perfect_Hash::libc_name_p (register const char *str, register unsigned int len)
to expand as
Perfect_Hash:: Perfect_Hash::libc_name_p (register const char *str, register 
unsigned int len)
?

So, I think better might be something like:

--- gcc/cp/Make-lang.in.jj  2016-01-04 14:55:57.0 +0100
+++ gcc/cp/Make-lang.in 2016-02-19 16:21:11.538734055 +0100
@@ -112,7 +112,7 @@ else
 # deleting the $(srcdir)/cp/cfns.h file.
 $(srcdir)/cp/cfns.h:
 endif
-   gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L ANSI-C \
+   gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L C++ \
$(srcdir)/cp/cfns.gperf --output-file $(srcdir)/cp/cfns.h
 
 #
--- gcc/cp/except.c.jj  2016-01-04 14:55:57.0 +0100
+++ gcc/cp/except.c 2016-02-19 16:20:37.134205968 +0100
@@ -1040,7 +1040,8 @@ nothrow_libfn_p (const_tree fn)
  unless the system headers are playing rename tricks, and if
  they are, we don't want to be confused by them.  */
   id = DECL_NAME (fn);
-  return !!libc_name_p (IDENTIFIER_POINTER (id), IDENTIFIER_LENGTH (id));
+  return !!libc_name::libc_name_p (IDENTIFIER_POINTER (id),
+  IDENTIFIER_LENGTH (id));
 }
 
 /* Returns nonzero if an exception of type FROM will be caught by a
--- gcc/cp/cfns.gperf.jj2016-02-19 14:41:14.0 +0100
+++ gcc/cp/cfns.gperf   2016-02-19 16:19:34.841060418 +0100
@@ -1,3 +1,5 @@
+%language=C++
+%define class-name libc_name
 %{
 /* Copyright (C) 2000-2016 Free Software Foundation, Inc.
 
@@ -16,14 +18,6 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
-#endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
-#endif
-const char * libc_name_p (const char *, unsigned int);
 %}
 %%
 # The standard C library functions, for feeding to gperf; the result is used
--- gcc/cp/cfns.h.jj2016-02-19 14:41:14.0 +0100
+++ gcc/cp/cfns.h   2016-02-19 16:19:53.533804017 +0100
@@ -1,5 +1,5 @@
-/* ANSI-C code produced by gperf version 3.0.3 */
-/* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L ANSI-C 
cfns.gperf  */
+/* C++ code produced by gperf version 3.0.4 */
+/* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L C++ 
cfns.gperf  */
 
 #if !((' ' == 32) && ('!' == 33) && ('"' == 34) && ('#' == 35) \
   && ('%' == 37) && ('&' == 38) && ('\'' == 39) && ('(' == 40) \
@@ -28,7 +28,7 @@
 #error "gperf generated tables don't work with this execution character set. 
Please report a bug to ."
 #endif
 
-#line 1 "cfns.gperf"
+#line 3 "cfns.gperf"
 
 /* Copyright (C) 2000-2016 Free Software Foundation, Inc.
 
@@ -47,25 +47,18 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
-#endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
-#endif
-const char * libc_name_p (const char *, unsigned int);
 /* maximum key range = 391, duplicates = 0 */
 
-#ifdef __GNUC__
-__inline
-#else
-#ifdef __cplusplus
-inline
-#endif
-#endif
-static unsigned int
-hash (register const char *str, register unsigned int len)
+class libc_name
+{
+private:
+  static inline unsigned int hash (const char *str, unsigned int len);
+public:
+  static const char *libc_name_p (const char *str, unsigned int len);
+};
+
+inline unsigned int
+libc_name::hash (register const char *str, register unsigned int len)
 {
   static const unsigned short asso_values[] =
 {
@@ -122,14 +115,8 @@ hash (register const char *str, register
   return hval + asso_values[(unsigned char)str[len - 1]];
 }
 
-#ifdef __GNUC__
-__inline
-#ifdef __GNUC_STDC_INLINE__
-__attribute__ ((__gnu_inline__))
-#endif
-#endif
 const char *
-libc_name_p (register const char *str, register unsigned int len)
+libc_name::libc_name_p (register const char *str, register unsigned int len)
 {
   enum
 {

Jakub

Re: [PATCH] Fix up handling of REG_EH_REGION notes in LRA (PR middle-end/69838)

[Dominik Vogt]

On Fri, Feb 19, 2016 at 04:08:37PM +0100, Jakub Jelinek wrote:
> For -fnon-call-exceptions, if an instruction with REG_EH_REGION note is
> reloaded, we should copy or move it to the instruction(s) corresponding to
> the original one that could throw.  reload1.c apparently does this, but LRA
> does not, so we can end up with REG_EH_REGION notes being dropped, or not
> present on insns that actually can throw etc.
> 
> Fixed by calling the functions reload1.c does for this purpose.
> 
> Bootstrapped/regtested on x86_64-linux (including Ada) and i686-linux
> (without Ada),

> and Dominik has kindly tested this on s390x-linux
> (presumably with Ada, but don't know for sure).

Yes, all languages with the whole testsuite.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[PATCH][ARM] PR target/69875 Fix atomic_loaddi expansion

[Kyrill Tkachov]

Hi all,

The atomic_loaddi expander on arm has some issues and can benefit from a 
rewrite to properly
perform double-word atomic loads on various architecture levels.

Consider the code:
--
#include 

atomic_ullong foo;

int glob;

int main(void) {
atomic_load_explicit(&foo, memory_order_acquire);
return glob;
}
-

Compiled with -O2 -march=armv7-a -std=c11 this gives:
movwr3, #:lower16:glob
movtr3, #:upper16:glob
dmb ish
movwr2, #:lower16:foo
movtr2, #:upper16:foo
ldrexd  r0, r1, [r2]
ldr r0, [r3]
bx  lr

For the acquire memory model the barrier should be after the ldrexd, not before.
The same code is generated when compiled with -march=armv7ve. However, we can 
get away with a single LDRD
on such systems. In issue C.c of The ARM Architecture Reference Manual for 
ARMv7-A and ARMv7-R
recommends at chapter A3.5.3:
"In an implementation that includes the Large Physical Address Extension, LDRD 
and STRD accesses to 64-bit aligned
locations are 64-bit single-copy atomic".
We still need the barrier after the LDRD to enforce the acquire ordering 
semantics.

For ARMv8-A we can do even better and use the load double-word acquire 
instruction: LDAEXD, with no need for
a barrier afterwards.

I've discussed the required sequences with some kernel folk and had a read 
through:
https://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html
and this is the patch I've come up with.

This patch handles all three of the above cases by rewriting the atomic_loaddi 
expander.
With this patch for the above code with -march=armv7-a we would now generate:
movwr3, #:lower16:foo
movtr3, #:upper16:foo
ldrexd  r0, r1, [r3]
movwr3, #:lower16:glob
movtr3, #:upper16:glob
dmb ish
ldr r0, [r3]
bx  lr

For -march=armv7ve:
movwr3, #:lower16:foo
movtr3, #:upper16:foo
ldrdr2, r3, [r3]
movwr3, #:lower16:glob
movtr3, #:upper16:glob
dmb ish
ldr r0, [r3]
bx  lr

and for -march=armv8-a:
movwr3, #:lower16:foo
movtr3, #:upper16:foo
ldaexd  r2, r3, [r3]
movwr3, #:lower16:glob
movtr3, #:upper16:glob
ldr r0, [r3]
bx  lr

For the relaxed memory model the armv7ve and armv8-a can be relaxed to a single
LDRD instruction, without any barriers.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk?

Thanks,
Kyrill

P.S. The backport to the previous branches will look a bit different because the
ARM_FSET_HAS_CPU1 machinery in arm.h was introduced for GCC 6. I'll prepare a 
backport
separately if this is accepted.

2016-02-19  Kyrylo Tkachov  

PR target/69875
* config/arm/arm.h (TARGET_HAVE_LPAE): Define.
* config/arm/unspecs.md (VUNSPEC_LDRD_ATOMIC): New value.
* config/arm/sync.md (arm_atomic_loaddi2_ldrd): New pattern.
(atomic_loaddi_1): Delete.
(atomic_loaddi): Rewrite expander using the above changes.

2016-02-19  Kyrylo Tkachov  

PR target/69875
* gcc.target/arm/atomic_loaddi_acquire.x: New file.
* gcc.target/arm/atomic_loaddi_relaxed.x: Likewise.
* gcc.target/arm/atomic_loaddi_seq_cst.x: Likewise.
* gcc.target/arm/atomic_loaddi_1.c: New test.
* gcc.target/arm/atomic_loaddi_2.c: Likewise.
* gcc.target/arm/atomic_loaddi_3.c: Likewise.
* gcc.target/arm/atomic_loaddi_4.c: Likewise.
* gcc.target/arm/atomic_loaddi_5.c: Likewise.
* gcc.target/arm/atomic_loaddi_6.c: Likewise.
* gcc.target/arm/atomic_loaddi_7.c: Likewise.
* gcc.target/arm/atomic_loaddi_8.c: Likewise.
* gcc.target/arm/atomic_loaddi_9.c: Likewise.
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index ca7c0e1e6c49e106cd871769dbf45e7b16861ea4..4a0e6aff8ec22c2c597684265cbbd10982bb81b7 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -253,6 +253,10 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
 /* Nonzero if this chip supports ldrex and strex */
 #define TARGET_HAVE_LDREX((arm_arch6 && TARGET_ARM) || arm_arch7)
 
+/* Nonzero if this chip supports LPAE.  */
+#define TARGET_HAVE_LPAE		\
+  (arm_arch7 && ARM_FSET_HAS_CPU1 (insn_flags, FL_FOR_ARCH7VE))
+
 /* Nonzero if this chip supports ldrex{bh} and strex{bh}.  */
 #define TARGET_HAVE_LDREXBH ((arm_arch6k && TARGET_ARM) || arm_arch7)
 
diff --git a/gcc/config/arm/sync.md b/gcc/config/arm/sync.md
index 8158f53025400045569533a1e8c6583025d490c8..abcfbcb1eacaabc597c9fde475c1b56624fb5a59 100644
--- a/gcc/config/arm/sync.md
+++ b/gcc/config/arm/sync.md
@@ -96,32 +96,62 @@ (define_insn "atomic_store"
   [(set_attr "predicable" "yes")
(set_attr "predicable_short_it" "no")])
 
-;; Note that ldrd and vldr are *not* guaranteed to be single-copy atomic,
-;; even for a 64-bit aligned address.  Instead we use a ldrexd unpar

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 16:22, Jakub Jelinek wrote:
> On Fri, Feb 19, 2016 at 03:09:25PM +, Bernd Edlinger wrote:
>> Hmm... wait a moment.
>> How about that?
>
> Guess it is mostly reasonable (still, this is C++ FE, so I'd prefer
> Jason to ack it), except for:
>
>> +#define libc_name_p Perfect_Hash::libc_name_p
>

Yeee. Right.

> this.  Doesn't that cause
> Perfect_Hash::libc_name_p (register const char *str, register unsigned int 
> len)
> to expand as
> Perfect_Hash:: Perfect_Hash::libc_name_p (register const char *str, register 
> unsigned int len)
> ?
>
> So, I think better might be something like:
>
> --- gcc/cp/Make-lang.in.jj2016-01-04 14:55:57.0 +0100
> +++ gcc/cp/Make-lang.in   2016-02-19 16:21:11.538734055 +0100
> @@ -112,7 +112,7 @@ else
>   # deleting the $(srcdir)/cp/cfns.h file.
>   $(srcdir)/cp/cfns.h:
>   endif
> - gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L ANSI-C \
> + gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L C++ \
>   $(srcdir)/cp/cfns.gperf --output-file $(srcdir)/cp/cfns.h
>
>   #
> --- gcc/cp/except.c.jj2016-01-04 14:55:57.0 +0100
> +++ gcc/cp/except.c   2016-02-19 16:20:37.134205968 +0100
> @@ -1040,7 +1040,8 @@ nothrow_libfn_p (const_tree fn)
>unless the system headers are playing rename tricks, and if
>they are, we don't want to be confused by them.  */
> id = DECL_NAME (fn);
> -  return !!libc_name_p (IDENTIFIER_POINTER (id), IDENTIFIER_LENGTH (id));
> +  return !!libc_name::libc_name_p (IDENTIFIER_POINTER (id),
> +IDENTIFIER_LENGTH (id));
>   }
>
>   /* Returns nonzero if an exception of type FROM will be caught by a
> --- gcc/cp/cfns.gperf.jj  2016-02-19 14:41:14.0 +0100
> +++ gcc/cp/cfns.gperf 2016-02-19 16:19:34.841060418 +0100
> @@ -1,3 +1,5 @@
> +%language=C++
> +%define class-name libc_name
>   %{

I like this idea with class-name.
Will post an updated patch in a minute.


Thanks
Bernd.

Re: [PATCH] Fix PR c++/68948 (wrong code generation due to invalid constructor call)

[Jason Merrill]

On 02/18/2016 01:25 PM, Patrick Palka wrote:

On Wed, Feb 17, 2016 at 10:51 PM, Jason Merrill  wrote:

OK.


Is this an approval of the 2nd patch for next stage 1?


Actually, I've been looking at this area a lot recently in the context 
of the 10200 fix, and now I think we can go ahead with the 2nd patch 
now, but without the assert; I think it would fire if we wrote A::A().


Jason

Re: [C++ PATCH] Some further -Wnonnull-compare fixes (PR c++/69850)

[Jason Merrill]

OK.

Jason

Re: [C++ PATCH] Fix handling of C++11 attributes (PR c++/67767)

[Jason Merrill]

OK.

Jason

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 16:22, Jakub Jelinek wrote:
>
> Guess it is mostly reasonable (still, this is C++ FE, so I'd prefer
> Jason to ack it), except for:
>

Here are updated versions of the cfns-patch and the pr69856-patch.
Are they OK for trunk when boot-strap and reg-testing succeeds?

Thanks
Bernd.
2016-02-19  Jakub Jelinek  
	Bernd Edlinger  

	* Make-lang.in: Invoke gpref with -L C++.
 	* cfns.gperf: Remove prototypes for hash and libc_name_p
 	inlines.
 	* cfns.h: Regenerated.
	* except.c (nothrow_libfn_p): Adjust.

Index: gcc/cp/Make-lang.in
===
--- gcc/cp/Make-lang.in	(revision 233557)
+++ gcc/cp/Make-lang.in	(working copy)
@@ -112,7 +112,7 @@ else
 # deleting the $(srcdir)/cp/cfns.h file.
 $(srcdir)/cp/cfns.h:
 endif
-	gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L ANSI-C \
+	gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L C++ \
 		$(srcdir)/cp/cfns.gperf --output-file $(srcdir)/cp/cfns.h
 
 #
Index: gcc/cp/cfns.gperf
===
--- gcc/cp/cfns.gperf	(revision 233557)
+++ gcc/cp/cfns.gperf	(working copy)
@@ -1,3 +1,5 @@
+%language=C++
+%define class-name libc_name
 %{
 /* Copyright (C) 2000-2016 Free Software Foundation, Inc.
 
@@ -16,14 +18,6 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
-#endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
-#endif
-const char * libc_name_p (const char *, unsigned int);
 %}
 %%
 # The standard C library functions, for feeding to gperf; the result is used
Index: gcc/cp/cfns.h
===
--- gcc/cp/cfns.h	(revision 233557)
+++ gcc/cp/cfns.h	(working copy)
@@ -1,5 +1,5 @@
-/* ANSI-C code produced by gperf version 3.0.3 */
-/* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L ANSI-C cfns.gperf  */
+/* C++ code produced by gperf version 3.0.4 */
+/* Command-line: gperf -o -C -E -k '1-6,$' -j1 -D -N libc_name_p -L C++ --output-file cfns.h cfns.gperf  */
 
 #if !((' ' == 32) && ('!' == 33) && ('"' == 34) && ('#' == 35) \
   && ('%' == 37) && ('&' == 38) && ('\'' == 39) && ('(' == 40) \
@@ -28,7 +28,7 @@
 #error "gperf generated tables don't work with this execution character set. Please report a bug to ."
 #endif
 
-#line 1 "cfns.gperf"
+#line 3 "cfns.gperf"
 
 /* Copyright (C) 2000-2016 Free Software Foundation, Inc.
 
@@ -47,26 +47,19 @@ 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
 .  */
-#ifdef __GNUC__
-__inline
-#endif
-static unsigned int hash (const char *, unsigned int);
-#ifdef __GNUC__
-__inline
-#endif
-const char * libc_name_p (const char *, unsigned int);
 /* maximum key range = 391, duplicates = 0 */
 
-#ifdef __GNUC__
-__inline
-#else
-#ifdef __cplusplus
-inline
-#endif
-#endif
-static unsigned int
-hash (register const char *str, register unsigned int len)
+class libc_name
 {
+private:
+  static inline unsigned int hash (const char *str, unsigned int len);
+public:
+  static const char *libc_name_p (const char *str, unsigned int len);
+};
+
+inline unsigned int
+libc_name::hash (register const char *str, register unsigned int len)
+{
   static const unsigned short asso_values[] =
 {
   400, 400, 400, 400, 400, 400, 400, 400, 400, 400,
@@ -122,14 +115,8 @@ along with GCC; see the file COPYING3.  If not see
   return hval + asso_values[(unsigned char)str[len - 1]];
 }
 
-#ifdef __GNUC__
-__inline
-#ifdef __GNUC_STDC_INLINE__
-__attribute__ ((__gnu_inline__))
-#endif
-#endif
 const char *
-libc_name_p (register const char *str, register unsigned int len)
+libc_name::libc_name_p (register const char *str, register unsigned int len)
 {
   enum
 {
Index: gcc/cp/except.c
===
--- gcc/cp/except.c	(revision 233557)
+++ gcc/cp/except.c	(working copy)
@@ -1040,7 +1040,7 @@ nothrow_libfn_p (const_tree fn)
  unless the system headers are playing rename tricks, and if
  they are, we don't want to be confused by them.  */
   id = DECL_NAME (fn);
-  return !!libc_name_p (IDENTIFIER_POINTER (id), IDENTIFIER_LENGTH (id));
+  return !!libc_name::libc_name_p (IDENTIFIER_POINTER (id), IDENTIFIER_LENGTH (id));
 }
 
 /* Returns nonzero if an exception of type FROM will be caught by a
2016-02-19  Bernd Edlinger  

	PR c++/69865
	* c-opts.c (c_common_post_options): Move call to set_std_cxx14 from
	here...
	(c_common_init_options): ...to here.
	(set_std_cxx98): Initialize flag_isoc94 and flag_isoc99.

Index: gcc/c-family/c-opts.c
===
--- gcc/c-family/c-opts.c	(revision 233557)
+++ gcc/c-family/c-opts.c	(working copy)
@@ 

Re: [PATCH] Fix PR37448 (and dups?)

[Jan Hubicka]

> > 
> > Are you sure?  I thought we compute that up-front ... at least
> > we make sure we can_inline_edge_p all calls before even trying
> > to inline all calls - that looks somewhat redundant then if it
> > can happen that we give up anyway.  Ah, so can_inline_edge_p has
> > 
> >   /* Check if caller growth allows the inlining.  */
> >   else if (!DECL_DISREGARD_INLINE_LIMITS (callee->decl)
> >&& !disregard_limits
> >&& !lookup_attribute ("flatten",
> >  DECL_ATTRIBUTES (caller->decl))
> >&& !caller_growth_limits (e))
> > inlinable = false;
> > 
> > and we check that from when we do the inlining and upfront from

> > want_inline_function_to_all_callers_p ... we also give up
> > if we inline a recursive function it seems.
Yep, there is early check for recursion and sizes. But you can still hit the
size limits later. If a function A calls 1000 times function B, each of the
calls individually may be inlinable, but not all together.
> > 
> > > For next stage1 I plan to finish the overahaul to sreals that will let
> > > me to make update_overall_summary incremental (i.e. 
> > > O(size_of_inlined_function)
> > > and not O(size_of_inlined_function+size_of_function_inlined_to)) that will
> > > also solve these issues.
> > 
> > Hopefully.
> > 
> > > One can probably do a pesimistic estimate on code size of the function 
> > > (i.e. add the size of inlined function) and only if that hits the large 
> > > function growth do the exact calculation. Or we can just decide that it 
> > > is OK to ignore large function growht in this partiuclar case.
> > 
> > Ignoring it is probably not a good idea and will just lead to a different
> > degenerate case.  As we update the summary afterwards it's probably
> > ok to do incremental (pessimistic) updates on the size anyway?  In
> > inline_merge_summary I mean?  Or should I open-code that into
> > inline_call if ! update_overall_summary?
> 
> So like below - I update self-size by the growth estimate which is
> supposed to match, this should keep the overall function growth
> limits working, not sure about the stack stuff but that doesn't seem
> to be updated by update_overall_summaries either.

Not really, the sizes are also computed in fixed point math, because we 
estimate probabilities
when insn is going to be removed. There are some cases wher estimate_growth is 
not as precise
as the real thing, you can see the #if 0 out asstert in ipa-inline-transform 
abou tthat.
Well, for inline functions called once, I suppose precision of these details is 
not as important,
so we may just go with the patch. 
I will finish the sreal conversion next stage1 (I got stuck on this patch 
series on gengtype change
to allow sreal inline_summary which I do not think ever got reviewed) and do 
the incremental update.
So if you fell this PR is important, go ahead with the patch.

Honza
> 
> This also fixes a similar issue in early inlining where we can then
> trivially delay update_overall_summaries.  I remember seeing early
> inline analysis behave similarly quadratic.
> 
> It leaves alone recursive and IPA small function inlining as those
> update overall unit size as well - I first thought about somehow
> making overall summary update lazy, but that looks quite complicated
> and IIRC the round-off errors issue was when re-computing the
> key for the fibheap after doing one inlining, right?
> 
> I hope you can get to use sreals early in next stage1 though I wonder
> how that will avoid all corner-cases.
> 
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.  Ok for trunk
> and branch?
> 
> Thanks,
> Richard.
> 
> 2016-02-18  Richard Biener  
> 
>   PR ipa/37448
>   * ipa-inline-transform.c (inline_call): When not updating
>   overall summaries adjust self size by the growth estimate.
>   * ipa-inline.c (inline_to_all_callers_1): Add to the callers
>   hash-set, do not update overall summaries here.  Renamed from ...
>   (inline_to_all_callers): ... this which is now wrapping the
>   above and performing delayed overall summary update.
>   (early_inline_small_functions): Delay updating of the overall
>   summary.
> 
> Index: gcc/ipa-inline-transform.c
> ===
> --- gcc/ipa-inline-transform.c(revision 233498)
> +++ gcc/ipa-inline-transform.c(working copy)
> @@ -300,10 +300,12 @@ inline_call (struct cgraph_edge *e, bool
>struct cgraph_edge *curr = e;
>struct cgraph_node *callee = e->callee->ultimate_alias_target ();
>bool new_edges_found = false;
> +  int estimated_growth;
>  
> +  if (! update_overall_summary)
> +estimated_growth = estimate_edge_growth (e);
>/* This is used only for assert bellow.  */
>  #if 0
> -  int estimated_growth = estimate_edge_growth (e);
>bool predicated = inline_edge_summary (e)->predicate != NULL;
>  #endif
>  
> @@ -373,7 +375,13 @@ inline_call (struct 

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jason Merrill]

On 02/19/2016 10:51 AM, Bernd Edlinger wrote:

+  flag_isoc94 = 0;
+  flag_isoc99 = 0;


Why?  These flags are global variables, so they're already zero-initialized.

Otherwise the changes look good to me.

Jason

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 11:03:23AM -0500, Jason Merrill wrote:
> On 02/19/2016 10:51 AM, Bernd Edlinger wrote:
> >+  flag_isoc94 = 0;
> >+  flag_isoc99 = 0;
> 
> Why?  These flags are global variables, so they're already zero-initialized.

That is true, but those global variables could have changed earlier.
Don't they e.g. get set if you do:
-std=c++14 -std=c++98
?

Jakub

Re: [PATCH] Fix up handling of REG_EH_REGION notes in LRA (PR middle-end/69838)

[Vladimir Makarov]

On 02/19/2016 10:08 AM, Jakub Jelinek wrote:

Hi!

For -fnon-call-exceptions, if an instruction with REG_EH_REGION note is
reloaded, we should copy or move it to the instruction(s) corresponding to
the original one that could throw.  reload1.c apparently does this, but LRA
does not, so we can end up with REG_EH_REGION notes being dropped, or not
present on insns that actually can throw etc.

Fixed by calling the functions reload1.c does for this purpose.

Bootstrapped/regtested on x86_64-linux (including Ada) and i686-linux
(without Ada), and Dominik has kindly tested this on s390x-linux
(presumably with Ada, but don't know for sure).  Ok for trunk?



Yes.  Thanks, Jakub.

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 17:09, Jakub Jelinek wrote:
> On Fri, Feb 19, 2016 at 11:03:23AM -0500, Jason Merrill wrote:
>> On 02/19/2016 10:51 AM, Bernd Edlinger wrote:
>>> +  flag_isoc94 = 0;
>>> +  flag_isoc99 = 0;
>>
>> Why?  These flags are global variables, so they're already zero-initialized.
>
> That is true, but those global variables could have changed earlier.
> Don't they e.g. get set if you do:
> -std=c++14 -std=c++98
> ?
>
>   Jakub
>

These are zero-initialized, but this:

@@ -246,6 +246,10 @@ c_common_init_options (unsigned int decoded_option
   }
  }

+  /* Set C++ standard to C++14 if not specified on the command line.  */
+  if (c_dialect_cxx ())
+set_std_cxx14 (/*ISO*/false);
+
global_dc->colorize_source_p = true;
  }


.. initializes them to 1, which is the default until we see
a -std=c++03.

I got 2 test cases FAIL without that hunk.

c-c++common/Wshift-negative-value-6.c and another similar one,
which I don't remember in the moment.


Bernd.

[pr 69666] No SRA default_def replacements for unscalarizable

[Martin Jambor]

Hi,

in PR 69666, SRA attempts to turn a load from an aggregate that is
uninitialized into a load from a default definition SSA name (which
something it does to generate an appropriate warning later) but
unfortunately it does so using an access structure which is
representable with __int128 when the load in question is smaller.  It
then attempts to fix it up only to create an invalid V_C_E.  In this
case, the correct thing to do is not to attempt the transformation,
when there are smaller accesses, which can be figured out by looking
at the unscalarizable_region flag of the access.

Bootstrapped and tested on x86_64, OK for trunk and later for the 5
branch?

Thanks,

Martin


2016-02-19  Martin Jambor  

PR tree-optimization/69666
* tree-sra.c (sra_modify_assign): Do not attempt to create
defaut_def replacements for unscalarizable regions.

testsuite/
* gcc.dg/tree-ssa/pr69666.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/pr69666.c | 16 
 gcc/tree-sra.c  |  1 +
 2 files changed, 17 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr69666.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr69666.c 
b/gcc/testsuite/gcc.dg/tree-ssa/pr69666.c
new file mode 100644
index 000..9be77ea
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr69666.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -w" } */
+
+int a, c, d;
+float b;
+void *memcpy();
+int fun1(int p1, unsigned char *p2) {
+  p2[p1] = b;
+  return a;
+}
+void fun2() {
+  unsigned char e[16];
+  fun1(16, e);
+  d = e[d];
+  memcpy(&c, e, sizeof(e));
+}
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 72157ed..663ded2 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -3339,6 +3339,7 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator 
*gsi)
 }
   else if (racc
   && !racc->grp_unscalarized_data
+  && !racc->grp_unscalarizable_region
   && TREE_CODE (lhs) == SSA_NAME
   && !access_has_replacements_p (racc))
 {
-- 
2.7.0

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 03:51:08PM +, Bernd Edlinger wrote:
> --- gcc/cp/except.c   (revision 233557)
> +++ gcc/cp/except.c   (working copy)
> @@ -1040,7 +1040,7 @@ nothrow_libfn_p (const_tree fn)
>   unless the system headers are playing rename tricks, and if
>   they are, we don't want to be confused by them.  */
>id = DECL_NAME (fn);
> -  return !!libc_name_p (IDENTIFIER_POINTER (id), IDENTIFIER_LENGTH (id));
> +  return !!libc_name::libc_name_p (IDENTIFIER_POINTER (id), 
> IDENTIFIER_LENGTH (id));

Too long line.

Jakub

Re: [PATCH] 69759 - document __builtin_alloca and __builtin_alloca_with_align

[Martin Sebor]

... Here I'd like to get my updated patch reviewed so that I
can move on to my other GCC 6 tasks.


I integrated the documentation update into the coding patch for bug
69780 - [4.9/5/6 Regression] ICE on __builtin_alloca_with_align, to
keep the two in sync.

Jakub has reviewed and approved the code changes but defers
the documentation review to you.  The most recent combined patch
is at the link below:

  https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01227.html

Thanks
Martin

Re: [pr 69666] No SRA default_def replacements for unscalarizable

[Richard Biener]

On February 19, 2016 5:21:56 PM GMT+01:00, Martin Jambor  
wrote:
>Hi,
>
>in PR 69666, SRA attempts to turn a load from an aggregate that is
>uninitialized into a load from a default definition SSA name (which
>something it does to generate an appropriate warning later) but
>unfortunately it does so using an access structure which is
>representable with __int128 when the load in question is smaller.  It
>then attempts to fix it up only to create an invalid V_C_E.  In this
>case, the correct thing to do is not to attempt the transformation,
>when there are smaller accesses, which can be figured out by looking
>at the unscalarizable_region flag of the access.
>
>Bootstrapped and tested on x86_64, OK for trunk and later for the 5
>branch?

OK.

Richard.

>Thanks,
>
>Martin
>
>
>2016-02-19  Martin Jambor  
>
>   PR tree-optimization/69666
>   * tree-sra.c (sra_modify_assign): Do not attempt to create
>   defaut_def replacements for unscalarizable regions.
>
>testsuite/
>   * gcc.dg/tree-ssa/pr69666.c: New test.
>---
> gcc/testsuite/gcc.dg/tree-ssa/pr69666.c | 16 
> gcc/tree-sra.c  |  1 +
> 2 files changed, 17 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr69666.c
>
>diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr69666.c
>b/gcc/testsuite/gcc.dg/tree-ssa/pr69666.c
>new file mode 100644
>index 000..9be77ea
>--- /dev/null
>+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr69666.c
>@@ -0,0 +1,16 @@
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -w" } */
>+
>+int a, c, d;
>+float b;
>+void *memcpy();
>+int fun1(int p1, unsigned char *p2) {
>+  p2[p1] = b;
>+  return a;
>+}
>+void fun2() {
>+  unsigned char e[16];
>+  fun1(16, e);
>+  d = e[d];
>+  memcpy(&c, e, sizeof(e));
>+}
>diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
>index 72157ed..663ded2 100644
>--- a/gcc/tree-sra.c
>+++ b/gcc/tree-sra.c
>@@ -3339,6 +3339,7 @@ sra_modify_assign (gimple *stmt,
>gimple_stmt_iterator *gsi)
> }
>   else if (racc
>  && !racc->grp_unscalarized_data
>+ && !racc->grp_unscalarizable_region
>  && TREE_CODE (lhs) == SSA_NAME
>  && !access_has_replacements_p (racc))
> {

Re: [PATCH] Fix PR c++/68948 (wrong code generation due to invalid constructor call)

[Patrick Palka]

On Fri, Feb 19, 2016 at 10:41 AM, Jason Merrill  wrote:
> On 02/18/2016 01:25 PM, Patrick Palka wrote:
>>
>> On Wed, Feb 17, 2016 at 10:51 PM, Jason Merrill  wrote:
>>>
>>> OK.
>>
>>
>> Is this an approval of the 2nd patch for next stage 1?
>
>
> Actually, I've been looking at this area a lot recently in the context of
> the 10200 fix, and now I think we can go ahead with the 2nd patch now, but
> without the assert; I think it would fire if we wrote A::A().

I w ill commit the version without the assert shortly, but...

I haven't been able to get the assert to fire even when the A in
A::A() is dependent because in that case FN should be dependent, so we
would already have exited out of finish_call_expr due to the
type_dependent_expression_p (fn) check near the top of
finish_call_expr.  (In particular for dependent A::A(), FN is a
SCOPE_REF whose 1st operand is the dependent type A and 2nd operand is
the identifier node A.)

So it seems to me that the assert at that location is safe, since the
dummy object should be dependent only if the constructor call FN is
dependent in which case we would never reach the assert.

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 17:38, Jakub Jelinek wrote:
> > -  return !!libc_name_p (IDENTIFIER_POINTER (id), IDENTIFIER_LENGTH (id));
> > +  return !!libc_name::libc_name_p (IDENTIFIER_POINTER (id), 
> > IDENTIFIER_LENGTH (id));
> 
> Too long line.
> 
> Jakub

Oh, thanks.  Consider it fixed.
Also the typo in the ChangeLog:

> 2016-02-19  Jakub Jelinek  
>   Bernd Edlinger  
> 
>   * Make-lang.in: Invoke gpref with -L C++.

We invoke gperf of course.

Bernd.

Re: [PATCH, PR middle-end/68134] Reject scalar modes in default get_mask_mode hook

[Alan Lawrence]

On 17/11/15 11:49, Ilya Enkovich wrote:

Hi,

Default hook for get_mask_mode is supposed to return integer vector modes.  
This means it should reject calar modes returned by mode_for_vector.  
Bootstrapped and regtested on x86_64-unknown-linux-gnu, regtested on 
aarch64-unknown-linux-gnu.  OK for trunk?

Thanks,
Ilya
--
gcc/

2015-11-17  Ilya Enkovich  

PR middle-end/68134
* targhooks.c (default_get_mask_mode): Filter out
scalar modes returned by mode_for_vector.


I've been playing around with a patch that expands arbitrary VEC_COND_EXPRs 
using the vec_cmp and vcond_mask optabs - allowing platforms to drop the ugly 
vcond pattern (for which a cross-product of modes is usually 
required) where vcond = vec_cmp + vcond_mask. (E.g. ARM and AArch64.)


Mostly this is fairly straightforward, relatively little midend code is 
required, and the backend cleans up quite a bit. However, I get stuck on the 
case of singleton vectors (64x1). No surprises there, then...


The PR/68134 fix, makes the 'mask mode' for comparing 64x1 vectors, into 
BLKmode, so that we get past this in expand_vector_operations:


/* A scalar operation pretending to be a vector one.  */
  if (VECTOR_BOOLEAN_TYPE_P (type)
  && !VECTOR_MODE_P (TYPE_MODE (type))
  && TYPE_MODE (type) != BLKmode)
return;

and we do the operation piecewise. (Which is what we want; there is only one 
piece!)

However, with my vec_cmp + vcond_mask changes, dropping vconddidi, this means we 
look for a vcond_maskdiblk and vec_cmpdiblk. Which doesn't really feel right - 
it feels like the 64x1 mask should be a DImode, just like other 64x1 vectors.


So, I'm left thinking - is there a better way to look for "scalar operations 
pretending to be vector ones", such that we can get a DImode vec through 
the above? What exactly do we want that check to do? In my AArch64 testing, I 
was able to take it out altogether and get all tests passing...? (I don't have 
AVX512 hardware)


Thanks, Alan

[PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006

[Alan Lawrence]

This relates to FORTRAN code where different modules give different sizes to the
same array in a COMMON block (contrary to the fortran language specification).
SPEC have refused to patch the source code
(https://www.spec.org/cpu2006/Docs/faq.html#Run.05).

Hence, this patch provides a Fortran-specific option -funknown-commons that
marks such arrays as having unknown size - that is, NULL_TREE for both
TYPE_SIZE and max value of TYPE_DOMAIN. DECL_SIZE is preserved for e.g. output
in varasm.c.

On AArch64, it fixes the 416.gamess issue, and allows compiling 416.gamess
without the -fno-aggressive-loop-optimizations previously required (numerous
other PRs relating to 416.gamess).

I had to fix up a couple of places to deal with null TYPE_SIZE but in most cases
a test for such was already present. (omp-low.c had too many so I gave up: in
practice I think it's OK to just not use the new flag at the same time as
-fopenmp).

Bootstrapped + check-gcc + check-g++ on AArch64 and x86.
Also check-fortran with a variant enabling the option in all cases (to allow
compilation of C): here, only gfortran.dg/gomp/appendix-a/a.24.1.f90 fails,
this uses -fopenmp mentioned above.

Testcase fails both the FIND and one of the assignments if -funknown-commons is 
removed.

OK for trunk? I'd be happy to move this under -std=legacy if the Fortran folks
prefer. (-funcommon-commons has also been humourously suggested!)

gcc/ChangeLog:

* expr.c (store_field): Handle null TYPE_SIZE.
* tree-vect-data-refs.c (vect_analyze_data_refs): Bail out if TYPE_SIZE
is null.
* tree-dfa.c (get_ref_base_and_extent): Keep maxsize==-1 when reading
the whole of a DECL.

gcc/fortran/ChangeLog:

* lang.opt: Add -funknown-commons.
* trans-common.ci (build_field): If flag_unknown_commons is on, set
upper bound and size of arrays in common block to NULL_TREE.
(create_common): Explicitly set DECL_SIZE and DECL_SIZE_UNIT.

gcc/testsuite/ChangeLog:
* gfortran.dg/unknown_commons.f: New.
---
 gcc/expr.c  |  1 +
 gcc/fortran/lang.opt|  4 
 gcc/fortran/trans-common.c  | 35 +
 gcc/testsuite/gfortran.dg/unknown_commons.f | 20 +
 gcc/tree-dfa.c  |  6 -
 gcc/tree-vect-data-refs.c   |  8 +++
 6 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/unknown_commons.f

diff --git a/gcc/expr.c b/gcc/expr.c
index f4bac36..609bf59 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -6638,6 +6638,7 @@ store_field (rtx target, HOST_WIDE_INT bitsize, 
HOST_WIDE_INT bitpos,
 RHS isn't the same size as the bitfield, we must use bitfield
 operations.  */
   || (bitsize >= 0
+ && TYPE_SIZE (TREE_TYPE (exp))
  && TREE_CODE (TYPE_SIZE (TREE_TYPE (exp))) == INTEGER_CST
  && compare_tree_int (TYPE_SIZE (TREE_TYPE (exp)), bitsize) != 0
  /* Except for initialization of full bytes from a CONSTRUCTOR, which
diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
index 45428d8..b1b4641 100644
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -686,6 +686,10 @@ funderscoring
 Fortran Var(flag_underscoring) Init(1)
 Append underscores to externally visible names.
 
+funknown-commons
+Fortran Var(flag_unknown_commons)
+Treat arrays in common blocks as having unknown size.
+
 fwhole-file
 Fortran Ignore
 Does nothing.  Preserved for backward compatibility.
diff --git a/gcc/fortran/trans-common.c b/gcc/fortran/trans-common.c
index 21d1928..bf99c56 100644
--- a/gcc/fortran/trans-common.c
+++ b/gcc/fortran/trans-common.c
@@ -284,8 +284,41 @@ build_field (segment_info *h, tree union_type, 
record_layout_info rli)
   unsigned HOST_WIDE_INT desired_align, known_align;
 
   name = get_identifier (h->sym->name);
+
+  gcc_assert (TYPE_P (h->field));
+  tree size = TYPE_SIZE (h->field);
+  tree size_unit = TYPE_SIZE_UNIT (h->field);
+  if (GFC_ARRAY_TYPE_P (h->field)
+  && !h->sym->value
+  && flag_unknown_commons)
+{
+  gcc_assert (!GFC_DESCRIPTOR_TYPE_P (h->field));
+  gcc_assert (TREE_CODE (h->field) == ARRAY_TYPE);
+
+  /* Could be merged at link time with a larger array whose size we don't
+know.  */
+  static tree range_type = NULL_TREE;
+  if (!range_type)
+   range_type = build_range_type (gfc_array_index_type,
+  gfc_index_zero_node, NULL_TREE);
+  tree type = copy_node (h->field);
+  TYPE_DOMAIN (type) = range_type;
+  TYPE_SIZE (type) = NULL_TREE;
+  TYPE_SIZE_UNIT (type) = NULL_TREE;
+  SET_TYPE_MODE (type, BLKmode);
+  gcc_assert (TYPE_CANONICAL (h->field) == h->field);
+  gcc_assert (TYPE_MAIN_VARIANT (h->field) == h->field);
+  TYPE_CANONICAL (type) = type;
+  TYPE_MAIN_VARIANT (type) = type;
+  layout_type (type);
+  h->field = type

Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 05:42:34PM +, Alan Lawrence wrote:
> This relates to FORTRAN code where different modules give different sizes to 
> the
> same array in a COMMON block (contrary to the fortran language specification).
> SPEC have refused to patch the source code
> (https://www.spec.org/cpu2006/Docs/faq.html#Run.05).
> 
> Hence, this patch provides a Fortran-specific option -funknown-commons that
> marks such arrays as having unknown size - that is, NULL_TREE for both
> TYPE_SIZE and max value of TYPE_DOMAIN. DECL_SIZE is preserved for e.g. output
> in varasm.c.
> 
> On AArch64, it fixes the 416.gamess issue, and allows compiling 416.gamess
> without the -fno-aggressive-loop-optimizations previously required (numerous
> other PRs relating to 416.gamess).
> 
> I had to fix up a couple of places to deal with null TYPE_SIZE but in most 
> cases

I think it is wrong to touch TYPE_SIZE/TYPE_SIZE_UNIT, IMHO it is much better 
just
to ignore DECL_SIZE/DECL_SIZE_UNIT in the selected few places
(get_ref_base_and_extent, the tree-ssa-loop-niters.c analysis) if the switch
is on, for selected decls (aggregates with flexible array members and other
similar trailing arrays, arrays themselves; all only if DECL_COMMON).

> a test for such was already present. (omp-low.c had too many so I gave up: in
> practice I think it's OK to just not use the new flag at the same time as
> -fopenmp).

That will just be an endless source of bugreports when people will report ICEs
with -funknown-commons -fopenmp (or -fopenacc, or -fcilkplus, or
-ftree-parallelize-loops, ...).  For OpenMP the TYPE_SIZE* is essential, if
you privatize variables, you need to know how big variable to allocate etc.

Jakub

Re: [PATCH][RFC][Offloading] Fix PR68463

[Mike Stump]

On Feb 19, 2016, at 6:53 AM, Jakub Jelinek  wrote:
> Looking at this, I think I have no problem with crtoffloadbegin.o being
> included in all -fopenmp/-fopenacc linked programs/shared libraries,

:-)  I have a problem with just the normal init path in most executables.  It 
adds a ton of stuff that can be empty at the bottom.  I sometimes wonder if we 
boosted it to -flto, and then let lto see the size of the table, and put all 
the init code under an early if (count) { do the init stuff; }, then given the 
count, lto can then just remove it all, reliably.

If the openmp people want to experiment with -flto and see if they can make the 
whole thing disappear that way, it might be worth considering.

But, yes, I agree, hard to want yet more included by default that just won’t go 
away.

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Jason Merrill]

On 02/19/2016 09:03 AM, Jakub Jelinek wrote:

As described in the PR, in C++ we can have assignments
where both the lhs and rhs are COMPONENT_REFs with TREE_ADDRESSABLE types,
including padding, but the FIELD_DECLs are artificial fields that have
narrower bit sizes.
store_field in this case takes the path of bit-field handling (even when
it has bitpos and bitsize multiples of BITS_PER_UNIT (I think that is
necessarily true for the TREE_ADDRESSABLE types), which is incorrect,
because the rhs is expanded in that case through expand_normal, which
for a result type wider than the FIELD_DECL with forces it into a temporary.
In older GCCs that just generated inefficient code (copy the rhs into a
stack temporary, then copy that to lhs), but GCC trunk ICEs on that.
Fixed by not taking the bit-field path in that case after verifying
we'll be able to expand it properly using the normal store_expr.


Won't store_expr clobber tail padding because it doesn't know about 
bitsize?  I would think that what we want is to use emit_block_move in 
the bit-field path whenever we're dealing with whole bytes in memory.


Jason

Re: [PATCH] combine: Delete EQ* notes when pseudo mode changes (PR60818)

[Eric Botcazou]

> As far as I can see that is very hard to do though, and not really worth
> it -- the notes can contain an arbitrary expression in general.

OK, your call.

> Not for stage 4 certainly.

If we go this way, is the bug a regression?  If no, why rushing the fix?

> That moves those notes to i2notes, and then distribute_notes drops them?

That's not why I understand though.  The code appends i2notes to i3notes and 
distribute_notes will preserve them on i3:

/* Distribute all the LOG_LINKS and REG_NOTES from I1, I2, and I3.  */
if (i3notes)
  distribute_notes (i3notes, i3, i3, newi2pat ? i2 : NULL,
elim_i2, elim_i1, elim_i0);

so the notes are effectively moved from I2 to I3.  distribute_notes will 
indeed drop the non-trivial REG_EQUAL and REG_EQUIV notes so your code is very 
likely OK, it's just the wording of the comment.  No big deal I agree.

-- 
Eric Botcazou

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 01:30:52PM -0500, Jason Merrill wrote:
> On 02/19/2016 09:03 AM, Jakub Jelinek wrote:
> >As described in the PR, in C++ we can have assignments
> >where both the lhs and rhs are COMPONENT_REFs with TREE_ADDRESSABLE types,
> >including padding, but the FIELD_DECLs are artificial fields that have
> >narrower bit sizes.
> >store_field in this case takes the path of bit-field handling (even when
> >it has bitpos and bitsize multiples of BITS_PER_UNIT (I think that is
> >necessarily true for the TREE_ADDRESSABLE types), which is incorrect,
> >because the rhs is expanded in that case through expand_normal, which
> >for a result type wider than the FIELD_DECL with forces it into a temporary.
> >In older GCCs that just generated inefficient code (copy the rhs into a
> >stack temporary, then copy that to lhs), but GCC trunk ICEs on that.
> >Fixed by not taking the bit-field path in that case after verifying
> >we'll be able to expand it properly using the normal store_expr.
> 
> Won't store_expr clobber tail padding because it doesn't know about bitsize?

It doesn't clobber it, because it uses get_inner_reference, expands the
inner reference (which is necessarily for something TREE_ADDRESSABLE either
a MEM_REF or some decl that lives in memory), and get_inner_reference in
that case gives it the bitsize/bitpos from the FIELD_DECL.
Which is why in the patch I've posted there is the comparison of DECL_SIZE
of the FIELD_DECL against the bitsize that is passed to store_field.

> I would think that what we want is to use emit_block_move in the bit-field
> path whenever we're dealing with whole bytes in memory.

I'm afraid we'd need to duplicate too much code if we'd wanted to bypass all
the layers in there.

Jakub

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Bernd Edlinger]

Hi,


> --- gcc/expr.c.jj 2016-02-12 00:50:55.0 +0100
> +++ gcc/expr.c2016-02-19 10:43:59.639162531 +0100
> @@ -6643,14 +6643,24 @@ store_field (rtx target, HOST_WIDE_INT b
> /* Except for initialization of full bytes from a CONSTRUCTOR, which
>we will handle specially below.  */
> && !(TREE_CODE (exp) == CONSTRUCTOR
> -&& bitsize % BITS_PER_UNIT == 0))
> +&& bitsize % BITS_PER_UNIT == 0)
> +   /* And except for bitwise copying of TREE_ADDRESSABLE types,
> +  where the FIELD_DECL has the right bitsize, but TREE_TYPE (exp)
> +  includes some extra padding.  */
> +   && (!TREE_ADDRESSABLE (TREE_TYPE (exp))
> +   || TREE_CODE (exp) != COMPONENT_REF
> +   || TREE_CODE (DECL_SIZE (TREE_OPERAND (exp, 1))) != INTEGER_CST
> +   || (bitsize % BITS_PER_UNIT != 0)
> +   || (bitpos % BITS_PER_UNIT != 0)
> +   || (compare_tree_int (DECL_SIZE (TREE_OPERAND (exp, 1)), bitsize)
> +   != 0)))
>/* If we are expanding a MEM_REF of a non-BLKmode non-addressable
>   decl we must use bitfield operations.  */

isn't the indentation of the new block wrong?


Bernd.

Re: [ping] Enable -mstackrealign with SSE on 32-bit Windows

[Eric Botcazou]

> Not sure - why doesn't mingw properly specify the default incoming
> stack boundary?
> If it does, why isn't there an issue elsewhere even on non-mingw if you use
> -mincoming-stack-boundary=2?

Wouldn't that pessimize over -mstackrealign?  That's my reading of the manual.
Note that the issue is present on Solaris 9 and was fixed this way.

-- 
Eric Botcazou

[patch, Fortran] Fix PR fortran/68147, temporaries for allocatable strings

[Thomas Koenig]

Hello world,

in the fix for PR 47674, there was a piece of unnecessary
code which meant that allocatable string assignments did
not get their temporary within IF statements (and others).

This patch is really simple and obvious.  If we weren't in
tstage 4, I would simply commit it.

Because allocatable strings are a major convenience for
users, I would still like to commit this to trunk and
to the affected branches.

So, OK for these branches?

Regards

Thomas

2016-02-18  Thomas Koenig  

PR fortran/68147
PR fortran/47674
* frontend-passes.c (realloc_string_callbac): Don't set
walk_subtrees.

2016-02-18  Thomas Koenig  

PR fortran/68147
PR fortran/47674
* gfortran.dg/realloc_on_assign_26.f90:  New test case.
Index: frontend-passes.c
===
--- frontend-passes.c	(Revision 233474)
+++ frontend-passes.c	(Arbeitskopie)
@@ -153,7 +153,7 @@ gfc_run_passes (gfc_namespace *ns)
  */
 
 static int
-realloc_string_callback (gfc_code **c, int *walk_subtrees,
+realloc_string_callback (gfc_code **c, int *walk_subtrees ATTRIBUTE_UNUSED,
 			 void *data ATTRIBUTE_UNUSED)
 {
   gfc_expr *expr1, *expr2;
@@ -160,7 +160,6 @@ static int
   gfc_code *co = *c;
   gfc_expr *n;
 
-  *walk_subtrees = 0;
   if (co->op != EXEC_ASSIGN)
 return 0;
 
! { dg-do run }
! PR 68147 - no temprorary within the IF statement.
! Original test case by Martin Reinecke.
program test
  implicit none
  character(len=:),allocatable ::name
  name="./a.out"
  if (index(name,"/")  /=  0) THEN
name=name(3:)
if (name .ne. "a.out") call abort
  endif
end program

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Jason Merrill]

On 02/19/2016 01:41 PM, Jakub Jelinek wrote:

On Fri, Feb 19, 2016 at 01:30:52PM -0500, Jason Merrill wrote:

On 02/19/2016 09:03 AM, Jakub Jelinek wrote:

As described in the PR, in C++ we can have assignments
where both the lhs and rhs are COMPONENT_REFs with TREE_ADDRESSABLE types,
including padding, but the FIELD_DECLs are artificial fields that have
narrower bit sizes.
store_field in this case takes the path of bit-field handling (even when
it has bitpos and bitsize multiples of BITS_PER_UNIT (I think that is
necessarily true for the TREE_ADDRESSABLE types), which is incorrect,
because the rhs is expanded in that case through expand_normal, which
for a result type wider than the FIELD_DECL with forces it into a temporary.
In older GCCs that just generated inefficient code (copy the rhs into a
stack temporary, then copy that to lhs), but GCC trunk ICEs on that.
Fixed by not taking the bit-field path in that case after verifying
we'll be able to expand it properly using the normal store_expr.


Won't store_expr clobber tail padding because it doesn't know about bitsize?


It doesn't clobber it, because it uses get_inner_reference, expands the
inner reference (which is necessarily for something TREE_ADDRESSABLE either
a MEM_REF or some decl that lives in memory), and get_inner_reference in
that case gives it the bitsize/bitpos from the FIELD_DECL.
Which is why in the patch I've posted there is the comparison of DECL_SIZE
of the FIELD_DECL against the bitsize that is passed to store_field.


Ah, that makes sense.  Please mention that in your added comment.

For GCC 7, can we drop the TREE_ADDRESSABLE check?

Jason

Re: [patch] Clarify interaction of -Wnarrowing with -std

[Jason Merrill]

On 02/19/2016 07:42 AM, Jonathan Wakely wrote:

In PR69864 Manu suggests improving the docs to explain that
-Wnarrowing sometimes produces errors not warnings.

I think the right way to do that is clarify how it interacts with
-std. Specifically that the effect of -Wnarrowing listed first in the
manual *only* applies to C++98 modes, For all later modes (not just
with -std=c++11 as it says now), narrowing conversions produce errors
or warnings by default.

OK for trunk?


OK, thanks.

Jason

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 06:42:32PM +, Bernd Edlinger wrote:
> Hi,
> 
> 
> > --- gcc/expr.c.jj   2016-02-12 00:50:55.0 +0100
> > +++ gcc/expr.c  2016-02-19 10:43:59.639162531 +0100
> > @@ -6643,14 +6643,24 @@ store_field (rtx target, HOST_WIDE_INT b
> >   /* Except for initialization of full bytes from a CONSTRUCTOR, which
> >  we will handle specially below.  */
> >   && !(TREE_CODE (exp) == CONSTRUCTOR
> > -  && bitsize % BITS_PER_UNIT == 0))
> > +  && bitsize % BITS_PER_UNIT == 0)
> > + /* And except for bitwise copying of TREE_ADDRESSABLE types,
> > +where the FIELD_DECL has the right bitsize, but TREE_TYPE (exp)
> > +includes some extra padding.  */
> > + && (!TREE_ADDRESSABLE (TREE_TYPE (exp))
> > + || TREE_CODE (exp) != COMPONENT_REF
> > + || TREE_CODE (DECL_SIZE (TREE_OPERAND (exp, 1))) != INTEGER_CST
> > + || (bitsize % BITS_PER_UNIT != 0)
> > + || (bitpos % BITS_PER_UNIT != 0)
> > + || (compare_tree_int (DECL_SIZE (TREE_OPERAND (exp, 1)), bitsize)
> > + != 0)))
> >/* If we are expanding a MEM_REF of a non-BLKmode non-addressable
> >   decl we must use bitfield operations.  */
> 
> isn't the indentation of the new block wrong?

No, I think it is correct.

Jakub

Re: [PATCH] Fix PR c++/68948 (wrong code generation due to invalid constructor call)

[Jason Merrill]

On 02/19/2016 11:56 AM, Patrick Palka wrote:

On Fri, Feb 19, 2016 at 10:41 AM, Jason Merrill  wrote:

On 02/18/2016 01:25 PM, Patrick Palka wrote:


On Wed, Feb 17, 2016 at 10:51 PM, Jason Merrill  wrote:


OK.


Is this an approval of the 2nd patch for next stage 1?


Actually, I've been looking at this area a lot recently in the context of
the 10200 fix, and now I think we can go ahead with the 2nd patch now, but
without the assert; I think it would fire if we wrote A::A().


I w ill commit the version without the assert shortly, but...

I haven't been able to get the assert to fire even when the A in
A::A() is dependent because in that case FN should be dependent, so we
would already have exited out of finish_call_expr due to the
type_dependent_expression_p (fn) check near the top of
finish_call_expr.  (In particular for dependent A::A(), FN is a
SCOPE_REF whose 1st operand is the dependent type A and 2nd operand is
the identifier node A.)

So it seems to me that the assert at that location is safe, since the
dummy object should be dependent only if the constructor call FN is
dependent in which case we would never reach the assert.


It might be safe given our current dependency analysis, but I'm planning 
to tighten that up in GCC 7, along the lines of my 69753 patch that I 
ended up backing out.  Why do you want the assert?


Jason

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 02:00:07PM -0500, Jason Merrill wrote:
> On 02/19/2016 01:41 PM, Jakub Jelinek wrote:
> >On Fri, Feb 19, 2016 at 01:30:52PM -0500, Jason Merrill wrote:
> >>On 02/19/2016 09:03 AM, Jakub Jelinek wrote:
> >>>As described in the PR, in C++ we can have assignments
> >>>where both the lhs and rhs are COMPONENT_REFs with TREE_ADDRESSABLE types,
> >>>including padding, but the FIELD_DECLs are artificial fields that have
> >>>narrower bit sizes.
> >>>store_field in this case takes the path of bit-field handling (even when
> >>>it has bitpos and bitsize multiples of BITS_PER_UNIT (I think that is
> >>>necessarily true for the TREE_ADDRESSABLE types), which is incorrect,
> >>>because the rhs is expanded in that case through expand_normal, which
> >>>for a result type wider than the FIELD_DECL with forces it into a 
> >>>temporary.
> >>>In older GCCs that just generated inefficient code (copy the rhs into a
> >>>stack temporary, then copy that to lhs), but GCC trunk ICEs on that.
> >>>Fixed by not taking the bit-field path in that case after verifying
> >>>we'll be able to expand it properly using the normal store_expr.
> >>
> >>Won't store_expr clobber tail padding because it doesn't know about bitsize?
> >
> >It doesn't clobber it, because it uses get_inner_reference, expands the
> >inner reference (which is necessarily for something TREE_ADDRESSABLE either
> >a MEM_REF or some decl that lives in memory), and get_inner_reference in
> >that case gives it the bitsize/bitpos from the FIELD_DECL.
> >Which is why in the patch I've posted there is the comparison of DECL_SIZE
> >of the FIELD_DECL against the bitsize that is passed to store_field.
> 
> Ah, that makes sense.  Please mention that in your added comment.
> 
> For GCC 7, can we drop the TREE_ADDRESSABLE check?

I think we can't drop it, but we could replace it with a check that
get_inner_reference is something that must live in memory
(MEM_REF/TARGET_MEM_REF of SSA_NAME, or of decl that lives in memory,
or decl itself that lives in memory).

Jakub

Re: [C++ PATCH] Fix -Wnonnull-compare warning from dynamic_cast <...> (this) (PR c++/69850)

[Jason Merrill]

OK.

Jason

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Jason Merrill]

On 02/19/2016 02:07 PM, Jakub Jelinek wrote:

On Fri, Feb 19, 2016 at 02:00:07PM -0500, Jason Merrill wrote:

On 02/19/2016 01:41 PM, Jakub Jelinek wrote:

On Fri, Feb 19, 2016 at 01:30:52PM -0500, Jason Merrill wrote:

On 02/19/2016 09:03 AM, Jakub Jelinek wrote:

As described in the PR, in C++ we can have assignments
where both the lhs and rhs are COMPONENT_REFs with TREE_ADDRESSABLE types,
including padding, but the FIELD_DECLs are artificial fields that have
narrower bit sizes.
store_field in this case takes the path of bit-field handling (even when
it has bitpos and bitsize multiples of BITS_PER_UNIT (I think that is
necessarily true for the TREE_ADDRESSABLE types), which is incorrect,
because the rhs is expanded in that case through expand_normal, which
for a result type wider than the FIELD_DECL with forces it into a temporary.
In older GCCs that just generated inefficient code (copy the rhs into a
stack temporary, then copy that to lhs), but GCC trunk ICEs on that.
Fixed by not taking the bit-field path in that case after verifying
we'll be able to expand it properly using the normal store_expr.


Won't store_expr clobber tail padding because it doesn't know about bitsize?


It doesn't clobber it, because it uses get_inner_reference, expands the
inner reference (which is necessarily for something TREE_ADDRESSABLE either
a MEM_REF or some decl that lives in memory), and get_inner_reference in
that case gives it the bitsize/bitpos from the FIELD_DECL.
Which is why in the patch I've posted there is the comparison of DECL_SIZE
of the FIELD_DECL against the bitsize that is passed to store_field.


Ah, that makes sense.  Please mention that in your added comment.

For GCC 7, can we drop the TREE_ADDRESSABLE check?


I think we can't drop it, but we could replace it with a check that
get_inner_reference is something that must live in memory
(MEM_REF/TARGET_MEM_REF of SSA_NAME, or of decl that lives in memory,
or decl itself that lives in memory).


Please mention that in the comment, as well.  OK with those comment changes.

Jason

Re: [Patch, fortran] PR69423 - [6 Regression] Invalid optimization with deferred-length character

[Dominique d'Humières]

With the patch I get an ICE when compiling gfortran.dg/allocate_error_5.f90

(lldb) target create 
"/opt/gcc/gcc6p-233563p2/libexec/gcc/x86_64-apple-darwin15.3.0/6.0.0/f951"
Current executable set to 
'/opt/gcc/gcc6p-233563p2/libexec/gcc/x86_64-apple-darwin15.3.0/6.0.0/f951' 
(x86_64).
(lldb) run /opt/gcc/_clean/gcc/testsuite/gfortran.dg/allocate_error_5.f90
Process 18138 launched: 
'/opt/gcc/gcc6p-233563p2/libexec/gcc/x86_64-apple-darwin15.3.0/6.0.0/f951' 
(x86_64)
 gProcess 18138 stopped
* thread #1: tid = 0x2ebfe4a, 0x0001000d5679 
f951`(chain=0x7fff5fbfef20, expr=0x0001427c56b8, 
front=)(tree *, tree, bool) + 41 at trans.c:1549, queue = 
'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, 
address=0x50004)
frame #0: 0x0001000d5679 f951`(chain=0x7fff5fbfef20, 
expr=0x0001427c56b8, front=)(tree *, tree, bool) + 41 at 
trans.c:1549
   1546 
   1547   if (*chain)
   1548 {
-> 1549   if (TREE_CODE (*chain) != STATEMENT_LIST)
   1550 {
   1551   tree tmp;
   1552 
(lldb) bt
* thread #1: tid = 0x2ebfe4a, 0x0001000d5679 
f951`(chain=0x7fff5fbfef20, expr=0x0001427c56b8, 
front=)(tree *, tree, bool) + 41 at trans.c:1549, queue = 
'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, 
address=0x50004)
  * frame #0: 0x0001000d5679 f951`(chain=0x7fff5fbfef20, 
expr=0x0001427c56b8, front=)(tree *, tree, bool) + 41 at 
trans.c:1549
frame #1: 0x0001000f045e 
f951`::gfc_null_and_pass_deferred_len(sym=0x000143d06d50, 
init=0x7fff5fbfef20, loc=0x7fff5fbfef10) + 174 at trans-decl.c:3965
frame #2: 0x0001000f9dba 
f951`gfc_trans_deferred_vars(proc_sym=0x000143d06a50, 
block=0x7fff5fbff0b0) + 2042 at trans-decl.c:4275
frame #3: 0x0001000fc5e4 
f951`gfc_generate_function_code(ns=) + 1332 at trans-decl.c:6269
frame #4: 0x00010008cb9c f951`gfc_parse_file() + 1644 at parse.c:5613
frame #5: 0x0001000d2d39 f951`::gfc_be_parse_file() + 57 at 
f95-lang.c:201
frame #6: 0x0001009ac32c f951`::compile_file() + 60 at toplev.c:465
frame #7: 0x000100d9caaf f951`toplev::main(int, char**) + 1154 at 
toplev.c:1988
frame #8: 0x000100d9c62d f951`toplev::main(this=, 
argc=, argv=) + 733
frame #9: 0x000100d9e479 f951`main(argc=2, argv=0x7fff5fbff318) + 
41 at main.c:39
frame #10: 0x7fff97c615ad libdyld.dylib`start + 1
frame #11: 0x7fff97c615ad libdyld.dylib`start + 1

Thanks for working on this PR,

Dominique

> Le 19 févr. 2016 à 09:47, Paul Richard Thomas  
> a écrit :
> 
> Dear All,
> 
> This has proven to be a rather vexatious bug to fix. On the face of
> it, using the indirect reference to the passed string length for
> deferred character length functions should have worked at all levels
> of optimization. However, setting the string length within a do loop
> resulted in the change not being visible within the rest of the
> function scope, even though the correct result was returned. This was,
> on the face of it, the same mechanism used for both dummies and
> declared results, which works fine at all levels of optimization.
> 
> In order to be as conservative as possible at this stage in the
> release cycle, I have resorted to the belt and braces approach of
> using a local variable '..result', which is nulled and returned, as
> appropriate, in a new helper function. So that the compiled code is
> consistent, I have done the same for functions with and without
> explicitly declared result variables.
> 
> There is some dead code in 'gfc_get_symbol_decl', which could, with
> advantage, be replaced by a gcc_assert. In addition,
> gfc_trans_deferred_vars could do with some further tidying up to
> ensure that the logic is clear. These steps can easily be done now if
> other maintainers think that it is timely.
> 
> Bootstraps and regtests on FC21/x86_64 - OK for trunk?
> 
> Paul
> 
> 2016-02-19  Paul Thomas  
> 
>PR fortran/69423
>* trans-decl.c (create_function_arglist): Deferred character
>length functions, with and without declared results, address
>the passed reference type as '.result' and the local string
>length as '..result'.
>(gfc_null_and_pass_deferred_len): Helper function to null and
>return deferred string lengths, as needed.
>(gfc_trans_deferred_vars): Call it, thereby reducing repeated
>code, add call for deferred arrays and reroute pointer function
>results. Avoid using 'tmp' for anything other that a temporary
>tree by introducing 'type_of_array' for the arrayspec type.
> 
> 2016-02-19  Paul Thomas  
> 
>PR fortran/69423
>* gfortran.dg/deferred_character_15.f90 : New test.
> 

Re: [PATCH] combine: Delete EQ* notes when pseudo mode changes (PR60818)

[Segher Boessenkool]

On Fri, Feb 19, 2016 at 07:39:03PM +0100, Eric Botcazou wrote:
> > Not for stage 4 certainly.
> 
> If we go this way, is the bug a regression?  If no, why rushing the fix?

It is not a regression (it is in all open release branches already).
I can postpone it if you think that is wiser?

> > That moves those notes to i2notes, and then distribute_notes drops them?
> 
> That's not why I understand though.  The code appends i2notes to i3notes and 
> distribute_notes will preserve them on i3:

I misread it as moving the notes from i3 to i2, ugh.  It looks like we
do have a problem if i2 is a parallel with only one SET; but we already
had a problem anyway?  The REG_EQ* is put on the wrong insn?


Segher

Re: [PATCH] combine: Delete EQ* notes when pseudo mode changes (PR60818)

[Eric Botcazou]

> It is not a regression (it is in all open release branches already).
> I can postpone it if you think that is wiser?

I do think that the combiner is one of those pieces of code that you ought not 
to touch unless you really need to.

> I misread it as moving the notes from i3 to i2, ugh.  It looks like we
> do have a problem if i2 is a parallel with only one SET; but we already
> had a problem anyway?  The REG_EQ* is put on the wrong insn?

Possibly indeed, we might need to filter them out during the move.

-- 
Eric Botcazou

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Bernd Edlinger]

On 19.02.2016 17:03, Jason Merrill wrote:
> On 02/19/2016 10:51 AM, Bernd Edlinger wrote:
>> +  flag_isoc94 = 0;
>> +  flag_isoc99 = 0;
>
> Why?  These flags are global variables, so they're already
> zero-initialized.
>
> Otherwise the changes look good to me.
>
> Jason
>

Hi Jason,

This hunk is really needed.

I can prove it:

Index: gcc/testsuite/c-c++-common/Wshift-negative-value-6.c
===
--- gcc/testsuite/c-c++-common/Wshift-negative-value-6.c(revision 
233557)
+++ gcc/testsuite/c-c++-common/Wshift-negative-value-6.c(working copy)
@@ -1,7 +1,7 @@
  /* PR c/65179 */
  /* { dg-do compile } */
  /* { dg-options "-O -Wextra" } */
-/* { dg-additional-options "-std=c++03" { target c++ } } */
+/* { dg-additional-options "-std=c++11 -std=c++03" { target c++ } } */
  /* { dg-additional-options "-std=c90" { target c } } */

  enum E {


unpatched gcc gives:

 === g++ tests ===


Running target unix
FAIL: c-c++-common/Wshift-negative-value-6.c(test for bogus 
messages, line 10)
FAIL: c-c++-common/Wshift-negative-value-6.c(test for bogus 
messages, line 26)
FAIL: c-c++-common/Wshift-negative-value-6.c(test for bogus 
messages, line 29)


Would you like me to commit the above test case change together with
both parts of the patch?

Do you think the patch is OK now?


Thanks
Bernd.

Re: [PATCH 4/n] OpenMP 4.0 offloading infrastructure: lto-wrapper

[Thomas Schwinge]

Hi!

On Thu, 2 Oct 2014 19:14:57 +0400, Ilya Verbin  wrote:
> With this patch lto-wrapper performs invocation of mkoffload tool for each
> offload target.  This tool [...]
> will compile IR from .gnu.offload_lto_* sections into offload
> target code and embed the resultant code (offload image) into the new host's
> object file.

Consider the following scenario:

$ cat < CSTS-214-acc.c
int acc (void)
{
  int a;

#pragma acc parallel num_gangs (1) copyout (a)
  a = 100;

  return a;
}
$ cat < CSTS-214-test.c
extern int acc (void);

int main (void)
{
  if (acc () != 100)
__builtin_abort ();
  
  return 0;
}

Compile these two files as follows:

$ [GCC] -fopenacc -c CSTS-214-acc.c
$ x86_64-linux-gnu-ar -cr CSTS-214-acc.a CSTS-214-acc.o
$ [GCC] -fopenacc CSTS-214-test.c CSTS-214-acc.a

The last step will fail -- with incomprehensible diagnostics, ;-) as so
often when offloading fails...  Here's what's going on: the
LTO/offloading machinery correctly identifies that it needs to process
the CSTS-214-acc.c:acc function, present in the CSTS-214-acc.a archive
file at a certain offset, and it "encodes" that as follows:
CSTS-214-acc.a@0x9e (see lto-plugin/lto-plugin.c:claim_file_handler, the
"file->offset != 0" code right at the beginning).  This makes its way
down through here:

> --- a/gcc/lto-wrapper.c
> +++ b/gcc/lto-wrapper.c

> +/* Copy a file from SRC to DEST.  */
> +
> +static void
> +copy_file (const char *dest, const char *src)
> +{
> +  [...]
> +}

> @@ -624,6 +852,54 @@ run_gcc (unsigned argc, char *argv[])

> +  /* If object files contain offload sections, but do not contain LTO 
> sections,
> + then there is no need to perform a link-time recompilation, i.e.
> + lto-wrapper is used only for a compilation of offload images.  */
> +  if (have_offload && !have_lto)
> +{
> +  for (i = 1; i < argc; ++i)
> + if ([...])
> +   {
> + char *out_file;
> + /* Can be ".o" or ".so".  */
> + char *ext = strrchr (argv[i], '.');
> + if (ext == NULL)
> +   out_file = make_temp_file ("");
> + else
> +   out_file = make_temp_file (ext);
> + /* The linker will delete the files we give it, so make copies.  */
> + copy_file (out_file, argv[i]);
> + printf ("%s\n", out_file);
> +   }
> +[...]
> +  goto finish;
> +}
> +
>if (lto_mode == LTO_MODE_LTO)
>  {
>flto_out = make_temp_file (".lto.o");
> @@ -850,6 +1126,10 @@ cont:
>obstack_free (&env_obstack, NULL);
>  }
>  
> + finish:
> +  if (offloadend)
> +printf ("%s\n", offloadend);
> +
>obstack_free (&argv_obstack, NULL);
>  }

When we hit this, for argv "CSTS-214-acc.a@0x9e", the copy_file call will
fail -- there is no "CSTS-214-acc.a@0x9e" file to copy.  If we strip off
the "@0x[...]" suffix (but still printf the filename including the
suffix), then things work.  I copied that bit of code from earlier in
this function, where the same archive offset handling needs to be done.
Probably that code should be refactored a bit.

Also, I wonder if the "ext == NULL" case can really happen, and needs to
be handled as done in the code cited above, or if that can be simplified?
(Not yet tested that.)

Will something like the following be OK to fix this issue, or is that
something "that should not happen", should be fixed differently?

--- gcc/lto-wrapper.c
+++ gcc/lto-wrapper.c
@@ -1161,15 +1161,31 @@ run_gcc (unsigned argc, char *argv[])
&& strncmp (argv[i], "-flinker-output=",
sizeof ("-flinker-output=") - 1) != 0)
  {
+   char *p;
+   off_t file_offset = 0;
+   long loffset;
+   int consumed;
+   char *filename = argv[i];
+
+   if ((p = strrchr (argv[i], '@'))
+   && p != argv[i] 
+   && sscanf (p, "@%li%n", &loffset, &consumed) >= 1
+   && strlen (p) == (unsigned int) consumed)
+ {
+   filename = XNEWVEC (char, p - argv[i] + 1);
+   memcpy (filename, argv[i], p - argv[i]);
+   filename[p - argv[i]] = '\0';
+   file_offset = (off_t) loffset;
+ }
+
char *out_file;
-   /* Can be ".o" or ".so".  */
-   char *ext = strrchr (argv[i], '.');
+   char *ext = strrchr (filename, '.');
if (ext == NULL)
  out_file = make_temp_file ("");
else
  out_file = make_temp_file (ext);
/* The linker will delete the files we give it, so make copies.  */
-   copy_file (out_file, argv[i]);
+   copy_file (out_file, filename);
printf ("%s\n", out_file);
  }
   goto finish;


Grüße
 Thomas


signature.asc
Description: PGP signature

Re: [C++ PATCH] Fix option handling when -std=gnu++14 is not used (PR 69865)

[Jason Merrill]

On 02/19/2016 02:37 PM, Bernd Edlinger wrote:

On 19.02.2016 17:03, Jason Merrill wrote:

On 02/19/2016 10:51 AM, Bernd Edlinger wrote:

+  flag_isoc94 = 0;
+  flag_isoc99 = 0;


Why?  These flags are global variables, so they're already
zero-initialized.

Otherwise the changes look good to me.

Jason



Hi Jason,

This hunk is really needed.

I can prove it:

Index: gcc/testsuite/c-c++-common/Wshift-negative-value-6.c
===
--- gcc/testsuite/c-c++-common/Wshift-negative-value-6.c(revision 
233557)
+++ gcc/testsuite/c-c++-common/Wshift-negative-value-6.c(working copy)
@@ -1,7 +1,7 @@
   /* PR c/65179 */
   /* { dg-do compile } */
   /* { dg-options "-O -Wextra" } */
-/* { dg-additional-options "-std=c++03" { target c++ } } */
+/* { dg-additional-options "-std=c++11 -std=c++03" { target c++ } } */
   /* { dg-additional-options "-std=c90" { target c } } */

   enum E {


unpatched gcc gives:

  === g++ tests ===


Running target unix
FAIL: c-c++-common/Wshift-negative-value-6.c(test for bogus
messages, line 10)
FAIL: c-c++-common/Wshift-negative-value-6.c(test for bogus
messages, line 26)
FAIL: c-c++-common/Wshift-negative-value-6.c(test for bogus
messages, line 29)


Would you like me to commit the above test case change together with
both parts of the patch?

Do you think the patch is OK now?


OK.

Jason

Re: [PATCH 4/n] OpenMP 4.0 offloading infrastructure: lto-wrapper

[Ilya Verbin]

On Fri, Feb 19, 2016 at 20:41:58 +0100, Thomas Schwinge wrote:
> Hi!
> 
> On Thu, 2 Oct 2014 19:14:57 +0400, Ilya Verbin  wrote:
> > With this patch lto-wrapper performs invocation of mkoffload tool for each
> > offload target.  This tool [...]
> > will compile IR from .gnu.offload_lto_* sections into offload
> > target code and embed the resultant code (offload image) into the new host's
> > object file.
> 
> Consider the following scenario:
> 
> $ cat < CSTS-214-acc.c
> int acc (void)
> {
>   int a;
> 
> #pragma acc parallel num_gangs (1) copyout (a)
>   a = 100;
> 
>   return a;
> }
> $ cat < CSTS-214-test.c
> extern int acc (void);
> 
> int main (void)
> {
>   if (acc () != 100)
> __builtin_abort ();
>   
>   return 0;
> }
> 
> Compile these two files as follows:
> 
> $ [GCC] -fopenacc -c CSTS-214-acc.c
> $ x86_64-linux-gnu-ar -cr CSTS-214-acc.a CSTS-214-acc.o
> $ [GCC] -fopenacc CSTS-214-test.c CSTS-214-acc.a
> 
> The last step will fail -- with incomprehensible diagnostics, ;-) as so
> often when offloading fails...  Here's what's going on: the
> LTO/offloading machinery correctly identifies that it needs to process
> the CSTS-214-acc.c:acc function, present in the CSTS-214-acc.a archive
> file at a certain offset, and it "encodes" that as follows:
> CSTS-214-acc.a@0x9e (see lto-plugin/lto-plugin.c:claim_file_handler, the
> "file->offset != 0" code right at the beginning).  This makes its way
> down through here:
> 
> > --- a/gcc/lto-wrapper.c
> > +++ b/gcc/lto-wrapper.c
> 
> > +/* Copy a file from SRC to DEST.  */
> > +
> > +static void
> > +copy_file (const char *dest, const char *src)
> > +{
> > +  [...]
> > +}
> 
> > @@ -624,6 +852,54 @@ run_gcc (unsigned argc, char *argv[])
> 
> > +  /* If object files contain offload sections, but do not contain LTO 
> > sections,
> > + then there is no need to perform a link-time recompilation, i.e.
> > + lto-wrapper is used only for a compilation of offload images.  */
> > +  if (have_offload && !have_lto)
> > +{
> > +  for (i = 1; i < argc; ++i)
> > +   if ([...])
> > + {
> > +   char *out_file;
> > +   /* Can be ".o" or ".so".  */
> > +   char *ext = strrchr (argv[i], '.');
> > +   if (ext == NULL)
> > + out_file = make_temp_file ("");
> > +   else
> > + out_file = make_temp_file (ext);
> > +   /* The linker will delete the files we give it, so make copies.  */
> > +   copy_file (out_file, argv[i]);
> > +   printf ("%s\n", out_file);
> > + }
> > +[...]
> > +  goto finish;
> > +}
> > +
> >if (lto_mode == LTO_MODE_LTO)
> >  {
> >flto_out = make_temp_file (".lto.o");
> > @@ -850,6 +1126,10 @@ cont:
> >obstack_free (&env_obstack, NULL);
> >  }
> >  
> > + finish:
> > +  if (offloadend)
> > +printf ("%s\n", offloadend);
> > +
> >obstack_free (&argv_obstack, NULL);
> >  }
> 
> When we hit this, for argv "CSTS-214-acc.a@0x9e", the copy_file call will
> fail -- there is no "CSTS-214-acc.a@0x9e" file to copy.  If we strip off
> the "@0x[...]" suffix (but still printf the filename including the
> suffix), then things work.  I copied that bit of code from earlier in
> this function, where the same archive offset handling needs to be done.
> Probably that code should be refactored a bit.
> 
> Also, I wonder if the "ext == NULL" case can really happen, and needs to
> be handled as done in the code cited above, or if that can be simplified?
> (Not yet tested that.)
> 
> Will something like the following be OK to fix this issue, or is that
> something "that should not happen", should be fixed differently?
> 
> --- gcc/lto-wrapper.c
> +++ gcc/lto-wrapper.c
> @@ -1161,15 +1161,31 @@ run_gcc (unsigned argc, char *argv[])
>   && strncmp (argv[i], "-flinker-output=",
>   sizeof ("-flinker-output=") - 1) != 0)
> {
> + char *p;
> + off_t file_offset = 0;
> + long loffset;
> + int consumed;
> + char *filename = argv[i];
> +
> + if ((p = strrchr (argv[i], '@'))
> + && p != argv[i] 
> + && sscanf (p, "@%li%n", &loffset, &consumed) >= 1
> + && strlen (p) == (unsigned int) consumed)
> +   {
> + filename = XNEWVEC (char, p - argv[i] + 1);
> + memcpy (filename, argv[i], p - argv[i]);
> + filename[p - argv[i]] = '\0';
> + file_offset = (off_t) loffset;
> +   }
> +
>   char *out_file;
> - /* Can be ".o" or ".so".  */
> - char *ext = strrchr (argv[i], '.');
> + char *ext = strrchr (filename, '.');
>   if (ext == NULL)
> out_file = make_temp_file ("");
>   else
> out_file = make_temp_file (ext);
>   /* The linker will delete the files we give it, so make copies.  */
> - copy_file (out_file, argv[i]);
> + co

Re: [PATCH] combine: Delete EQ* notes when pseudo mode changes (PR60818)

[Segher Boessenkool]

On Fri, Feb 19, 2016 at 08:34:03PM +0100, Eric Botcazou wrote:
> > It is not a regression (it is in all open release branches already).
> > I can postpone it if you think that is wiser?
> 
> I do think that the combiner is one of those pieces of code that you ought 
> not 
> to touch unless you really need to.

Oh I agree, which is why I throw all patches through testing on a zillion
different archs, and bootstraps on a few.

I'll postpone this patch to GCC 7, stage 4 is too late for it.


Segher

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Bernd Edlinger]

On 19.02.2016 20:04, Jakub Jelinek wrote:
> On Fri, Feb 19, 2016 at 06:42:32PM +, Bernd Edlinger wrote:
>> Hi,
>>
>>
>>> --- gcc/expr.c.jj   2016-02-12 00:50:55.0 +0100
>>> +++ gcc/expr.c  2016-02-19 10:43:59.639162531 +0100
>>> @@ -6643,14 +6643,24 @@ store_field (rtx target, HOST_WIDE_INT b
>>>   /* Except for initialization of full bytes from a CONSTRUCTOR, which
>>>  we will handle specially below.  */
>>>   && !(TREE_CODE (exp) == CONSTRUCTOR
>>> -  && bitsize % BITS_PER_UNIT == 0))
>>> +  && bitsize % BITS_PER_UNIT == 0)
>>> + /* And except for bitwise copying of TREE_ADDRESSABLE types,
>>> +where the FIELD_DECL has the right bitsize, but TREE_TYPE (exp)
>>> +includes some extra padding.  */
>>> + && (!TREE_ADDRESSABLE (TREE_TYPE (exp))
>>> + || TREE_CODE (exp) != COMPONENT_REF
>>> + || TREE_CODE (DECL_SIZE (TREE_OPERAND (exp, 1))) != INTEGER_CST
>>> + || (bitsize % BITS_PER_UNIT != 0)
>>> + || (bitpos % BITS_PER_UNIT != 0)
>>> + || (compare_tree_int (DECL_SIZE (TREE_OPERAND (exp, 1)), bitsize)
>>> + != 0)))
>>> /* If we are expanding a MEM_REF of a non-BLKmode non-addressable
>>>decl we must use bitfield operations.  */
>>
>> isn't the indentation of the new block wrong?
>
> No, I think it is correct.
>
>   Jakub
>


but you are just adding another term to this expression:
  !(TREE_CODE (exp) == CONSTRUCTOR
&& bitsize % BITS_PER_UNIT == 0)

so the result should look like
  !(TREE_CODE (exp) == CONSTRUCTOR
   && bitsize % BITS_PER_UNIT == 0
   && (!TREE_ADDRESSABLE ...
   || TREE_CODE () ...
   ...
   || (compare_tree_int ...
   != 0)))



Bernd.

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 08:04:39PM +, Bernd Edlinger wrote:
> but you are just adding another term to this expression:
>   !(TREE_CODE (exp) == CONSTRUCTOR
> && bitsize % BITS_PER_UNIT == 0)

No.  Please read the code again.  I'm adding another case
after this one.

> so the result should look like
>   !(TREE_CODE (exp) == CONSTRUCTOR
>&& bitsize % BITS_PER_UNIT == 0
>&& (!TREE_ADDRESSABLE ...
>|| TREE_CODE () ...
>...
>|| (compare_tree_int ...
>!= 0)))

Jakub

Re: [patch] Clarify interaction of -Wnarrowing with -std

[Sandra Loosemore]

On 02/19/2016 12:01 PM, Jason Merrill wrote:

On 02/19/2016 07:42 AM, Jonathan Wakely wrote:

In PR69864 Manu suggests improving the docs to explain that
-Wnarrowing sometimes produces errors not warnings.

I think the right way to do that is clarify how it interacts with
-std. Specifically that the effect of -Wnarrowing listed first in the
manual *only* applies to C++98 modes, For all later modes (not just
with -std=c++11 as it says now), narrowing conversions produce errors
or warnings by default.

OK for trunk?


OK, thanks.


I suppose the patch is OK as it stands, but I was going to suggest 
restructuring it so that it talks about the default behavior first and 
what it does with non-default -std= options after that, instead of 
vice-versa.  Unfortunately I am backlogged on other things right now and 
it might take me a day or two before I have time to come up with some 
alternate wording.  If we are in a rush, go ahead and commit the 
existing patch meanwhile, I guess.


-Sandra

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Bernd Edlinger]

On 19.02.2016 21:08, Jakub Jelinek wrote:
> On Fri, Feb 19, 2016 at 08:04:39PM +, Bernd Edlinger wrote:
>> but you are just adding another term to this expression:
>>!(TREE_CODE (exp) == CONSTRUCTOR
>>  && bitsize % BITS_PER_UNIT == 0)
>
> No.  Please read the code again.  I'm adding another case
> after this one.
>

Ok, now I see.
Sorry for the noise.


Bernd.

Re: [patch] Clarify interaction of -Wnarrowing with -std

[Jonathan Wakely]

On 19/02/16 13:17 -0700, Sandra Loosemore wrote:

On 02/19/2016 12:01 PM, Jason Merrill wrote:

On 02/19/2016 07:42 AM, Jonathan Wakely wrote:

In PR69864 Manu suggests improving the docs to explain that
-Wnarrowing sometimes produces errors not warnings.

I think the right way to do that is clarify how it interacts with
-std. Specifically that the effect of -Wnarrowing listed first in the
manual *only* applies to C++98 modes, For all later modes (not just
with -std=c++11 as it says now), narrowing conversions produce errors
or warnings by default.

OK for trunk?


OK, thanks.


I suppose the patch is OK as it stands, but I was going to suggest 
restructuring it so that it talks about the default behavior first and 
what it does with non-default -std= options after that, instead of 
vice-versa.  Unfortunately I am backlogged on other things right now 
and it might take me a day or two before I have time to come up with 
some alternate wording.  If we are in a rush, go ahead and commit the 
existing patch meanwhile, I guess.


I did wonder about that but couldn't come up with wording I liked.

I went ahead and committed it after Jason's OK, but I can take another
look at it next week.

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Bernd Edlinger]

Excuse me,

I have still another question.

Why are you adding braces here?

+ || (bitsize % BITS_PER_UNIT != 0)
+ || (bitpos % BITS_PER_UNIT != 0)
+ || (compare_tree_int (DECL_SIZE (TREE_OPERAND (exp, 1)), bitsize)
+ != 0)))

I think everywhere in that function we omit braces around == terms
inside || terms even long ones.

|| a == b
|| c == d
|| e == f)


I know many like to add braces here, but GCC does not do that
unnecessarily, right?


Thanks
Bernd.

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Bernd Edlinger]

Excuse me,

I have still another question.

Why are you adding braces here?

+ || (bitsize % BITS_PER_UNIT != 0)
+ || (bitpos % BITS_PER_UNIT != 0)
+ || (compare_tree_int (DECL_SIZE (TREE_OPERAND (exp, 1)), bitsize)
+ != 0)))

I think everywhere in that function we omit braces around == terms
inside || terms even long ones.

|| a == b
|| c == d
|| e == f)


I know many like to add braces here, but GCC does not do that
unnecessarily, right?


Thanks
Bernd.

Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes

[Paul E. Murphy]

On 02/17/2016 08:37 PM, Alan Modra wrote:
>> +/* A set bit indicates an exception is trapping.  */
>> +# define FP_TRAPPING_EXCEPTIONS ((_fpscr.i << 22) & FP_EX_ALL)
> 
> why then a shift here, since FP_EX_* are defined as the actual
> register bits?  Oh, I see.  FP_EX_* are the status bits, and you want
> the enable bits.  ie. bit 56 rather than bit 34, bit 57 rather than
> bit 35 and so on (bits numbered from 0 as msb).  A comment to that
> effect might reduce head scratching.

Thanks for taking a look Alan, and Joseph.  I've expanded the comment
to clarify the otherwise mysterious shift with the updated patch.

libgcc
* config/rs6000/sfp-machine.h:
(_FP_DECL_EX): Declare _fpsr as a union of u64 and double.
(FP_TRAPPING_EXCEPTIONS): Return a bitmask of trapping
exceptions.
(FP_INIT_ROUNDMODE): Read the fpscr instead of writing
a mystery value.
(FP_ROUNDMODE): Update the usage of _fpscr.

diff --git a/libgcc/config/rs6000/sfp-machine.h b/libgcc/config/rs6000/sfp-machine.h
index 75d5e1a..8dd8ea3 100644
--- a/libgcc/config/rs6000/sfp-machine.h
+++ b/libgcc/config/rs6000/sfp-machine.h
@@ -130,9 +130,13 @@ void __sfp_handle_exceptions (int);
 if (__builtin_expect (_fex, 0))		\
   __sfp_handle_exceptions (_fex);		\
   } while (0);
-/* A set bit indicates an exception is masked and a clear bit indicates it is
-   trapping.  */
-# define FP_TRAPPING_EXCEPTIONS (~_fpscr & (FP_EX_ALL >> 22))
+
+/* The FP_EX_* bits track whether the exception has occurred.  This macro
+   must set the FP_EX_* bits of those exceptions which are configured to
+   trap.  The FPSCR bit which indicates this is 22 ISA bits above the
+   respective FP_EX_* bit.  Note, the ISA labels bits from msb to lsb,
+   so 22 ISA bits above is 22 bits below when counted from the lsb.  */
+# define FP_TRAPPING_EXCEPTIONS ((_fpscr.i << 22) & FP_EX_ALL)
 
 # define FP_RND_NEAREST	0x0
 # define FP_RND_ZERO	0x1
@@ -141,16 +145,16 @@ void __sfp_handle_exceptions (int);
 # define FP_RND_MASK	0x3
 
 # define _FP_DECL_EX \
-  unsigned long long _fpscr __attribute__ ((unused)) = FP_RND_NEAREST
+  union { unsigned long long i; double d; } _fpscr __attribute__ ((unused)) = \
+	 { .i = FP_RND_NEAREST }
 
 #define FP_INIT_ROUNDMODE			\
   do {		\
-__asm__ __volatile__ ("mtfsf 255, %0"	\
-			  :			\
-			  : "f" (_fpscr));	\
+__asm__ __volatile__ ("mffs %0"		\
+			  : "=f" (_fpscr.d));	\
   } while (0)
 
-# define FP_ROUNDMODE	(_fpscr & FP_RND_MASK)
+# define FP_ROUNDMODE	(_fpscr.i & FP_RND_MASK)
 #endif	/* !__FLOAT128__ */
 
 /* Define ALIASNAME as a strong alias for NAME.  */

Re: [PATCH] Fix expansion of TREE_ADDRESSABLE bitwise copies (PR c++/69851)

[Jakub Jelinek]

On Fri, Feb 19, 2016 at 08:45:09PM +, Bernd Edlinger wrote:
> I have still another question.
> 
> Why are you adding braces here?
> 
> +   || (bitsize % BITS_PER_UNIT != 0)
> +   || (bitpos % BITS_PER_UNIT != 0)

These two are not really needed, but I've already committed the patch.

> +   || (compare_tree_int (DECL_SIZE (TREE_OPERAND (exp, 1)), bitsize)
> +   != 0)))
> 
> I think everywhere in that function we omit braces around == terms
> inside || terms even long ones.

emacs users keep saying that emacs needs this to indent it properly.

> || a == b
> || c == d
> || e == f)

These are on a single line, so it is not a problem.

Jakub

Re: [RFC] [P2] [PR tree-optimization/33562] Lowering more complex assignments.

[Jeff Law]

On 02/18/2016 02:56 AM, Richard Biener wrote:

Just a short quick comment - the above means you only handle partial
stores
with no interveaning uses.  You don't handle, say

struct S { struct R { int x; int y; } r; int z; } s;

   s = { {1, 2}, 3 };
   s.r.x = 1;
   s.r.y = 2;
   struct R r = s.r;
   s.z = 3;

where s = { {1, 2}, 3} is still dead.


Right.  But handling that has never been part of DSE's design goals. Once
there's a use, DSE has always given up.


Yeah, which is why I in the end said we need a "better" DSE ...
So I cobbled up a quick test for this.  I only handle assignments which 
may reference the same memory as the currently tracked store.  Obviously 
that could be extended to handle certain builtins and the like.


It triggers a bit here and there.  While looking at those cases it 
occurred to me that, we could also look at this as a failure earlier in 
the optimization pipeline.  In fact DOM already has code to handle a 
closely related situation.


When DOM sees a store to memory, it creates a new fake statement with 
the RHS and LHS reversed.  So in the case above DOM creates statements 
that look like:


1 = s.r.x
2 = s.r.y

DOM then puts the RHS into the available expression table as equivalent 
to the LHS.  So if it finds a later load of s.r.x, it will replace that 
load with "1".  I haven't looked at it in a while, but it certainly was 
functional prior to the tuple merge.


Presumably DOM is not looking at r = s.r and realizing it could look s.r 
piece-wise in the available expression table.  If it did, it would 
effectively turn that fragment into:


s = { {1, 2}, 3 };
s.r.x = 1;
s.r.y = 2;
struct R r = {1, 2}
s.z = 3;

At which point we no longer have the may-read of s.r.{x,y} and DSE would 
see the initial assignment as dead.


I'm not sure if it's advisable to teach DOM how to lookup structure 
references piecewise or not.  The code to handle this case in DSE is 
relatively simple, so perhaps we just go with the DSE variant.


I also looked a bit at cases where we find that while an entire store 
(such as an aggregate initialization or mem*) may not be dead, pieces of 
the store may be dead.   That's trivial to detect.   It triggers 
relatively often.  The trick is once detected, we have to go back and 
rewrite the original statement to only store the live parts.  I've only 
written the detection code, the rewriting might be somewhat painful.


I'm starting to wonder if what we have is a 3-part series.

[1/3] The basic tracking to handle 33562, possibly included in gcc-6
[2/3] Ignore reads that reference stuff not in live_bytes for gcc-7
[3/3] Detect partially dead aggregate stores, rewriting the partially
  dead store to only store the live bytes.  Also for gcc-7.


Obviously [1/3] would need compile-time benchmarking, but I really don't 
expect any issues there.


Jeff

Re: [PATCH] Fix PR c++/68948 (wrong code generation due to invalid constructor call)

[Patrick Palka]

On Fri, Feb 19, 2016 at 2:06 PM, Jason Merrill  wrote:
> On 02/19/2016 11:56 AM, Patrick Palka wrote:
>>
>> On Fri, Feb 19, 2016 at 10:41 AM, Jason Merrill  wrote:
>>>
>>> On 02/18/2016 01:25 PM, Patrick Palka wrote:


 On Wed, Feb 17, 2016 at 10:51 PM, Jason Merrill 
 wrote:
>
>
> OK.


 Is this an approval of the 2nd patch for next stage 1?
>>>
>>>
>>> Actually, I've been looking at this area a lot recently in the context of
>>> the 10200 fix, and now I think we can go ahead with the 2nd patch now,
>>> but
>>> without the assert; I think it would fire if we wrote A::A().
>>
>>
>> I w ill commit the version without the assert shortly, but...
>>
>> I haven't been able to get the assert to fire even when the A in
>> A::A() is dependent because in that case FN should be dependent, so we
>> would already have exited out of finish_call_expr due to the
>> type_dependent_expression_p (fn) check near the top of
>> finish_call_expr.  (In particular for dependent A::A(), FN is a
>> SCOPE_REF whose 1st operand is the dependent type A and 2nd operand is
>> the identifier node A.)
>>
>> So it seems to me that the assert at that location is safe, since the
>> dummy object should be dependent only if the constructor call FN is
>> dependent in which case we would never reach the assert.
>
>
> It might be safe given our current dependency analysis, but I'm planning to
> tighten that up in GCC 7, along the lines of my 69753 patch that I ended up
> backing out.  Why do you want the assert?

No good reason.

>
> Jason
>

Fix/work around PR57676, LRA terminates prematurely

[Bernd Schmidt]

The testcase in this PR causes gcc to abort with

internal compiler error: Maximum number of LRA constraint passes is 
achieved (30)


[in theory - I've not managed to reproduce this on my system with any 
compiler]


The abort is premature, allowing LRA to continue would allow the 
testcase to compile. Vlad would like to leave the check in, however, as 
it is a good way to catch problems with machine descriptions as they are 
converted to use LRA.


The following is a compromise I came up with after some internal 
discussion. The idea is to leave the assert in under -fchecking, so that 
release compilers do not prematurely abort for valid testcases, while 
development builds get an additional sanity check.


Bootstrapped and tested on x86_64-linux. Ok?


Bernd

PR rtl-optimization/57676
* lra-assigns.c (lra_assign): Guard test for maximum iterations
with flag_checking.

PR rtl-optimization/57676
* gcc.dg/torture/pr57676.c: New test.

Index: gcc/lra-assigns.c
===
--- gcc/lra-assigns.c	(revision 233451)
+++ gcc/lra-assigns.c	(working copy)
@@ -1620,7 +1620,12 @@ lra_assign (void)
   timevar_pop (TV_LRA_ASSIGN);
   if (former_reload_pseudo_spill_p)
 lra_assignment_iter_after_spill++;
-  if (lra_assignment_iter_after_spill > LRA_MAX_ASSIGNMENT_ITERATION_NUMBER)
+  /* This is conditional on flag_checking because valid code can take
+ more than this maximum number of iteration, but at the same time
+ the test can uncover errors in machine descriptions.  */
+  if (flag_checking
+  && (lra_assignment_iter_after_spill
+	  > LRA_MAX_ASSIGNMENT_ITERATION_NUMBER))
 internal_error
   ("Maximum number of LRA assignment passes is achieved (%d)\n",
LRA_MAX_ASSIGNMENT_ITERATION_NUMBER);
Index: gcc/testsuite/gcc.dg/torture/pr57676.c
===
--- gcc/testsuite/gcc.dg/torture/pr57676.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr57676.c	(working copy)
@@ -0,0 +1,28 @@
+/* Verify that LRA does not abort prematurely in a release build of the
+   compiler.  */
+/* { dg-do compile } */
+/* { dg-options "-fno-checking -w -funroll-loops" } */
+
+int a, b, c;
+
+void f(p1)
+{
+for(;;)
+{
+if(p1 ? : (c /= 0))
+{
+int d;
+
+for(; d; d++)
+{
+for(b = 0; b < 4; b++)
+p1 /= p1;
+lbl:
+while(a);
+}
+}
+
+if((c &= 1))
+goto lbl;
+}
+}

Re: Fix/work around PR57676, LRA terminates prematurely

[Jeff Law]

On 02/19/2016 02:32 PM, Bernd Schmidt wrote:

The testcase in this PR causes gcc to abort with

internal compiler error: Maximum number of LRA constraint passes is
achieved (30)

[in theory - I've not managed to reproduce this on my system with any
compiler]

The abort is premature, allowing LRA to continue would allow the
testcase to compile. Vlad would like to leave the check in, however, as
it is a good way to catch problems with machine descriptions as they are
converted to use LRA.

The following is a compromise I came up with after some internal
discussion. The idea is to leave the assert in under -fchecking, so that
release compilers do not prematurely abort for valid testcases, while
development builds get an additional sanity check.

Bootstrapped and tested on x86_64-linux. Ok?


Bernd

 PR rtl-optimization/57676
 * lra-assigns.c (lra_assign): Guard test for maximum iterations
 with flag_checking.

 PR rtl-optimization/57676
 * gcc.dg/torture/pr57676.c: New test.

OK.  Your call on backporting the release branches.

jeff

i386: relax scan-assembler test in lzcnt-1 testcase

[Bernd Schmidt]

I'm working on some IRA cost fixes, and I've had the lzcnt-1.c test fail 
because the register allocator started making different decisions. In 
both cases we end up generating two instructions, but with slightly 
different register assignments. Hence, this patch, which relaxes the 
test slightly.


Bootstrapped and tested on x86_64-linux (with my IRA changes, so I 
probably ought to try without those as well). Ok?



Bernd
	* gcc.target/i386/lzcnt-1.c: Allow a different lzcntw output register.

Index: gcc/testsuite/gcc.target/i386/lzcnt-1.c
===
--- gcc/testsuite/gcc.target/i386/lzcnt-1.c	(revision 233451)
+++ gcc/testsuite/gcc.target/i386/lzcnt-1.c	(working copy)
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-options "-O2 -mlzcnt " } */
-/* { dg-final { scan-assembler "lzcntw\[^\\n]*(%|)ax" } } */
+/* { dg-final { scan-assembler "lzcntw\[^\\n]*(%|)\[ad\]\[xi\]" } } */
 
 #include 
 

Re: i386: relax scan-assembler test in lzcnt-1 testcase

[Jeff Law]

On 02/19/2016 02:36 PM, Bernd Schmidt wrote:

I'm working on some IRA cost fixes, and I've had the lzcnt-1.c test fail
because the register allocator started making different decisions. In
both cases we end up generating two instructions, but with slightly
different register assignments. Hence, this patch, which relaxes the
test slightly.

Bootstrapped and tested on x86_64-linux (with my IRA changes, so I
probably ought to try without those as well). Ok?

OK.
jeff

Re: [PATCH] Add configure flag for operator new (std::nothrow)

[Daniel Gutson]

On Tue, Nov 10, 2015 at 10:10 AM, Jonathan Wakely  wrote:
> On 06/11/15 09:59 +, Pedro Alves wrote:
>>
>> On 11/06/2015 01:56 AM, Jonathan Wakely wrote:
>>>
>>> On 5 November 2015 at 23:31, Daniel Gutson
>>
>>
 The issue is, as I understand it, to do the actual work of operator
 new, i.e. allocate memory. It should force
 us to copy most of the code of the original code of operator new,
 which may change on new versions of the
 STL, forcing us to keep updated.
>>>
>>>
>>> It can just call malloc, and the replacement operator delete can call
>>> free.
>>>
>>> That is very unlikely to need to change (which is corroborated by the
>>> fact that the default definitions in libsupc++ change very rarely).
>>
>>
>> Or perhaps libsupc++ could provide the default operator new under
>> a __default_operator_new alias or some such, so that the user-defined
>> replacement can fallback to calling it.  Likewise for op delete.

that would allow us to overload operator new as something like this:

void* operator new  ( std::size_t count, const std::nothrow_t& tag)
noexcept(true)
{
const auto old_handler = std::set_new_handler(nullptr);
const auto ret = __default_operator_new(count, tag);
std::set_new_handler(old_handler);
return ret;
}

This is a non-iterating operator new.

This additional user defined operator new would be possible:

void* operator new  ( std::size_t count, const std::nothrow_t& tag)
noexcept(true)
{
const auto old_handler = std::set_new_handler(nullptr);
const auto ret = __default_operator_new(count, tag);
std::set_new_handler(old_handler);

if (ret == nullptr && old_handler != nullptr)
old_handler();

return ret;
}

So I like the idea.


>
>
> That could be useful, please file an enhancement request in bugzilla
> if you'd like that done.



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson