Re: Patch ping

[Jeff Law]

On 03/04/2016 12:30 AM, Jakub Jelinek wrote:

Hi!

I'd like to ping a texinfo fix for __builtin_alloca*:
http://gcc.gnu.org/ml/gcc-patches/2016-02/msg01842.html

OK.

jeff

Re: [PATCH] S/390: Set GOARCH to the current target when testing multiarch.

[Andreas Krebbel]

On 03/02/2016 02:05 PM, Dominik Vogt wrote:
> gcc/testsuite/ChangeLog
>
>   * go.test/go-test.exp: S/390: Set GOARCH to the current target when
>   testing multiarch.

Applied. Thanks!

-Andreas-

Patch ping

[Jakub Jelinek]

Hi!

I'd like to ping a texinfo fix for __builtin_alloca*:
http://gcc.gnu.org/ml/gcc-patches/2016-02/msg01842.html

Jakub

Re: [PATCH] Fix PR70054

[Jeff Law]

On 03/03/2016 05:35 AM, Richard Biener wrote:


The following patch adjusts strict_aliasing_warning to use
proper alias_set_subset_of instead of relying on alias_sets_conflict_p
as after the PR66110 fix aggregates with a char[] member do not
automatically behave like having alias-set zero.  As a side-effect
the test will be somewhat stricter as well accessing a 'long' with
a struct { int i; long l; } * will now warn while it previously
didn't:

struct S { int i; long l; };
long x;
struct S foo () { return *(struct S *) }

now warns:

t.c: In function ‘foo’:
t.c:3:35: warning: dereferencing type-punned pointer will break
strict-aliasing rules [-Wstrict-aliasing]
  struct S foo () { return *(struct S *) }
^
Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk?

Thanks,
Richard.

2016-03-03  Richard Biener  

PR c++/70054
* c-common.c (strict_aliasing_warning): Use alias_set_subset_of
instead of alias_sets_conflict_p.

* g++.dg/warn/Wstrict-aliasing-bogus-union-2.C: New testcase.
* gcc.dg/Wstrict-aliasing-struct-member.c: New testcase.
The PR doesn't have a regression marker, but from reading the PR it's 
clearly a regression.


OK for the trunk,

Thanks,
Jeff

Re: Patch ping

[Jakub Jelinek]

On Fri, Mar 04, 2016 at 12:10:26AM -0700, Jeff Law wrote:
> On 03/03/2016 07:35 AM, Jakub Jelinek wrote:
> >Hi!
> >
> >I'd like to ping fix for P1 PR69947:
> >https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01743.html
> So essentially this is just marking more things so that we don't prune them
> away, right?
> 
> It's similar conceptually to one of Pierre-Marie's patches where he removed
> the switch and recursed anytime the operand's val_class matched
> dw_val_class_die_ref and was !external.  Yours just explicitly adds the new
> DW_OP_ things to the switch and has a slightly looser check (dropping the
> !external part of the check).

The !external part is IMHO not needed, as we only set external for
attributes, not for operands of DWARF expression opcodes.
And, while checking both operands for dw_val_class_die_ref is possible, it
is not enough, it doesn't cover the DW_OP_GNU_entry_value case where it is
a val_loc, and that case really depends on the context, other val_locs
shouldn't be traversed.  So, I think it is better to list the opcodes
explicitly, as that gives the needed context to what the arguments are,
perhaps at some point we'll refer to DIEs that we want to do something
different for.
When adding new DW_OP_* values, one has to change dozens of other places
anyway, so one further one doesn't matter, one has to search for all the
spots anyway.

Jakub

Re: [PATCH 2/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #1)

[Jeff Law]

On 03/03/2016 08:21 AM, David Malcolm wrote:

Comment #1 of PR c/68187 identified another overzealous warning
from -Wmisleading-indentation, with OpenSSL 1.0.1, on this poorly
indented code:

115if (locked)
116i = CRYPTO_add(>struct_ref, -1, CRYPTO_LOCK_ENGINE);
117else
118i = --e->struct_ref;
119engine_ref_debug(e, 0, -1)
120if (i > 0)
121return 1;
Egad.  How do people read this code when they have to understand it and 
make changes.What a steaming pile of .





Root cause is that "engine_ref_debug" is actually a debugging macro
that was empty in the given configuration, so the code effectively
was:

117else  // GUARD
118i = --e->struct_ref;  // BODY
119
120if (i > 0)// NEXT

hence the warning.

No surprise we triggered seeing that.



OK for trunk?

Note: one of the new test cases adds a dg-warning/dg-message pair, and so
would require updating if/when the wording change posted here:
   https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00068.html
   ("[PATCH] PR c/69993: improvements to wording of -Wmisleading-indentation")
is applied.

gcc/c-family/ChangeLog:
PR c/68187
* c-indentation.c (get_visual_column): Move code to determine next
tab stop to...
(next_tab_stop): ...this new function.
(line_contains_hash_if): Delete function.
(detect_preprocessor_logic): Delete function.
(get_first_nws_vis_column): New function.
(detect_intervening_unindent): New function.
(should_warn_for_misleading_indentation): Replace call to
detect_preprocessor_logic with a call to
detect_intervening_unindent.

gcc/testsuite/ChangeLog:
PR c/68187
* c-c++-common/Wmisleading-indentation.c (fn_42_a): New test
function.
(fn_42_b): Likewise.
(fn_42_c): Likewise.

OK.


Jeff

Re: [PATCH 1/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #0)

[Jeff Law]

On 03/03/2016 08:21 AM, David Malcolm wrote:

PR c/68187 covers two cases involving poor indentation where
the indentation is arguably not misleading, but for which
-Wmisleading-indentation emits a warning.

The two cases appear to be different in nature; one in comment #0
and the other in comment #1.  Richi marked the bug as a whole as
a P1 regression; it's not clear to me if he meant one or both of
these cases, so the following two patches fix both.

The rest of this post addresses the case in comment #0 of the PR;
the followup post addresses the other case, in comment #1 of the PR.

Building glibc (a9224562cbe9cfb0bd8d9e637a06141141f9e6e3) on x86_64
led to this diagnostic from -Wmisleading-indentation:

../stdlib/strtol_l.c: In function 'strtoul_l_internal':
../stdlib/strtol_l.c:356:9: error: statement is indented as if it were guarded 
by... [-Werror=misleading-indentation]
  cnt < thousands_len; })
  ^
../stdlib/strtol_l.c:353:9: note: ...this 'for' clause, but it is not
&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)
  ^

The code is question looks like this:

348for (c = *end; c != L_('\0'); c = *++end)
349  if (((STRING_TYPE) c < L_('0') || (STRING_TYPE) c > 
L_('9'))
350  # ifdef USE_WIDE_CHAR
351  && (wchar_t) c != thousands
352  # else
353  && ({ for (cnt = 0; cnt < thousands_len; ++cnt)
354if (thousands[cnt] != end[cnt])
355  break;
356cnt < thousands_len; })
357  # endif
358  && (!ISALPHA (c)
359  || (int) (TOUPPER (c) - L_('A') + 10) >= base))
360break;

Lines 354 and 355 are poorly indented, leading to the warning from
-Wmisleading-indentation at line 356.

The wording of the warning is clearly wrong: line 356 isn't indented as if
guarded by line 353, it's more that lines 354 and 355 *aren't* indented.

What's happening is that should_warn_for_misleading_indentation has a
heuristic for handling "} else", such as:

  if (p)
foo (1);
  } else   // GUARD
foo (2);   // BODY
foo (3);   // NEXT

and this heuristic uses the first non-whitespace character in the line
containing GUARD as the column of interest: the "}" character.

In this case we have:

353&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)  // GUARD
354  if (thousands[cnt] != end[cnt])// BODY
355break;
356  cnt < thousands_len; })// NEXT

and so it uses the column of the "&&", and treats it as if it were
indented thus:

353for (cnt = 0; cnt < thousands_len; ++cnt)// GUARD
354  if (thousands[cnt] != end[cnt])// BODY
355break;
356  cnt < thousands_len; })// NEXT

and thus issues a warning.

As far as I can tell the heuristic in question only makes sense for
"else" clauses, so the following patch updates it to only use the
special column when handling "else" clauses, eliminating the
overzealous warning.

Doing so led to a nonsensical warning for
libstdc++-v3/src/c++11/random.cc:random_device::_M_init:

random.cc: In member function ‘void std::random_device::_M_init(const string&)’:
random.cc:102:10: warning: this ‘if’ clause does not guard... 
[-Wmisleading-indentation]
  else if (token != "/dev/urandom" && token != "/dev/random")
   ^~
random.cc:107:5: note: ...this statement, but the latter is indented as if it 
does
  _M_file = static_cast(std::fopen(fname, "rb"));
  ^~~

so the patch addresses this by tweaking the heuristic that rejects
aligned BODY and NEXT so that it doesn't require them to be aligned with
the first non-whitespace of the GUARD, simply that they not be indented
relative to it.

Successfully bootstrapped on x86_64-pc-linux-gnu in
combination with the following patch; standalone bootstrap
is in progress.

OK for trunk if the latter is successful?

gcc/c-family/ChangeLog:
PR c/68187
* c-indentation.c (should_warn_for_misleading_indentation): When
suppressing warnings about cases where the guard and body are on
the same column, only use the first non-whitespace column in place
of the guard token column when dealing with "else" clauses.
When rejecting aligned BODY and NEXT, loosen the requirement
from equality with the first non-whitespace of guard to simply
that they not be indented relative to it.

gcc/testsuite/ChangeLog:
PR c/68187
* c-c++-common/Wmisleading-indentation.c (fn_40_a): New test
function.
(fn_40_b): Likewise.
(fn_41_a): Likewise.
(fn_41_b): Likewise.

OK.

FWIW, I think all of c-indentation would fall under the diagnostics 
maintainership you picked up recently.


jeff

Re: Patch ping

[Jeff Law]

On 03/03/2016 07:35 AM, Jakub Jelinek wrote:

Hi!

I'd like to ping fix for P1 PR69947:
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01743.html
So essentially this is just marking more things so that we don't prune 
them away, right?


It's similar conceptually to one of Pierre-Marie's patches where he 
removed the switch and recursed anytime the operand's val_class matched 
dw_val_class_die_ref and was !external.  Yours just explicitly adds the 
new DW_OP_ things to the switch and has a slightly looser check 
(dropping the !external part of the check).


I could argue for either approach.  Yours AFAICT is safer in that it 
won't recurse on unexpected DW_OP_ things.  Of course, it may 
require more long term maintenance to keep the list of things to recurse 
on up-to-date.


Either approach is OK with me, given you're a lot more familiar with our 
dwarf writer than I, I'll go with your judgment on which is the best 
approach to address the problem.


jeff

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Jakub Jelinek]

On Thu, Mar 03, 2016 at 11:31:08PM -0700, Jeff Law wrote:
> >2016-03-03  Marek Polacek  
> >
> > PR c/69798
> > * c-parser.c (c_parser_postfix_expression): Call
> > c_parser_cast_expression instead of c_parser_postfix_expression.
> >
> > * gcc.dg/cilk-plus/pr69798-1.c: New test.
> > * gcc.dg/cilk-plus/pr69798-2.c: New test.
> Do we need to do anything for the call into c_parser_postfix_expression that
> occurs between the two you changed in this patch.
> 
> I think you can get into that code with something like
> 
> _Cilk_spawn _Cilk_spawn ());

The _Cilk_spawn _Cilk_spawn (struct S);
case needs the same treatment as the other 2 spots, sure, and should be also
covered by a testcase.  Perhaps another testcase should test the various
other cases of the postfix vs. cast expression I've mentioned in the other
mail, just make sure the diagnostics is reasonable and we don't ICE.

Jakub

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Jeff Law]

On 03/03/2016 09:15 AM, Marek Polacek wrote:

On Thu, Mar 03, 2016 at 03:28:01PM +0100, Jakub Jelinek wrote:

On Thu, Mar 03, 2016 at 03:15:41PM +0100, Marek Polacek wrote:

This is ICE on invalid Cilk+ code.  cilk_spawn expects a function call, so e.g.
_Cilk_spawn (void) is invalid.  The function call after the cilk_spawn keyword
is parsed using recursive call in c_parser_postfix_expression (case
RID_CILK_SPAWN).  Now, c_parser_postfix_expression sees '(' followed by a
typename, so it thinks we're inside a compound literal, which means it expects
'{', but that isn't there, so we crash on the assert in c_parser_braced_init:
gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
But as the comment in c_parser_postfix_expression says, the code for parsing
a compound literal here is likely dead.  I made an experiment and added
gcc_unreachable () in that block, ran regtest, and there were no failures.
Thus it should be safe to just remove the code, which also fixes this ICE; with
the patch we just give a proper error and don't crash anymore.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'm actually slightly
nervous about the change, so maybe better table until gcc7?


This reminds me of PR67495.  Perhaps the answer here is also during the
_Cilk* stuff parsing don't call c_parser_postfix_expression, but call
c_parser_cast_expression instead and then verify what it got?


Alternatively this one works as well.  I don't know if any verification of the
result should be done (in the second call, the first one is invalid anyway).

2016-03-03  Marek Polacek  

PR c/69798
* c-parser.c (c_parser_postfix_expression): Call
c_parser_cast_expression instead of c_parser_postfix_expression.

* gcc.dg/cilk-plus/pr69798-1.c: New test.
* gcc.dg/cilk-plus/pr69798-2.c: New test.
Do we need to do anything for the call into c_parser_postfix_expression 
that occurs between the two you changed in this patch.


I think you can get into that code with something like

_Cilk_spawn _Cilk_spawn ());


For the non-error case (the second one you changed), I think we do want 
to verify that the expression we got back was a function call and issue 
an appropriate error otherwise.  You could make an argument that we 
should issue a diagnostic for the other cases as well, but it's less 
obviously correct.


jeff

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Jakub Jelinek]

On Thu, Mar 03, 2016 at 10:52:20PM -0700, Jeff Law wrote:
> On 03/03/2016 07:28 AM, Jakub Jelinek wrote:
> >On Thu, Mar 03, 2016 at 03:15:41PM +0100, Marek Polacek wrote:
> >>This is ICE on invalid Cilk+ code.  cilk_spawn expects a function call, so 
> >>e.g.
> >>_Cilk_spawn (void) is invalid.  The function call after the cilk_spawn 
> >>keyword
> >>is parsed using recursive call in c_parser_postfix_expression (case
> >>RID_CILK_SPAWN).  Now, c_parser_postfix_expression sees '(' followed by a
> >>typename, so it thinks we're inside a compound literal, which means it 
> >>expects
> >>'{', but that isn't there, so we crash on the assert in 
> >>c_parser_braced_init:
> >>gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
> >>But as the comment in c_parser_postfix_expression says, the code for parsing
> >>a compound literal here is likely dead.  I made an experiment and added
> >>gcc_unreachable () in that block, ran regtest, and there were no failures.
> >>Thus it should be safe to just remove the code, which also fixes this ICE; 
> >>with
> >>the patch we just give a proper error and don't crash anymore.
> >>
> >>Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'm actually slightly
> >>nervous about the change, so maybe better table until gcc7?
> >
> >This reminds me of PR67495.  Perhaps the answer here is also during the
> >_Cilk* stuff parsing don't call c_parser_postfix_expression, but call
> >c_parser_cast_expression instead and then verify what it got?
> That sounds more sensible to me -- as long as there aren't unintended side
> effects of calling c_parser_cast_expression.

The difference is that c_parser_cast_expression can parse (type)...,
++..., --..., &..., +..., -..., *..., ~..., !..., &&...,
sizeof/alignof/__extension__/__real/__imag/__transaction_{atomic,relaxed}...
Perhaps even safer version would be:
if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)
&& c_token_starts_typename (c_parser_peek_2nd_token (parser)))
  {
... error handling ...
  }
else
  ... c_parser_postfix_expression (parser);
But, even c_parser_postfix_expression parses tons of expressions that
aren't allowed there, so I think it is overkill.  It needs to check
afterwards, and from what I can see that happens in cilk_set_spawn_marker.
Thus I think c_parser_cast_expression is safe, it parses perhaps more
unexpected expressions, but will reject them all anyway, because they aren't
CALL_EXPR after parsing.

Jakub

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Jeff Law]

On 03/03/2016 07:28 AM, Jakub Jelinek wrote:

On Thu, Mar 03, 2016 at 03:15:41PM +0100, Marek Polacek wrote:

This is ICE on invalid Cilk+ code.  cilk_spawn expects a function call, so e.g.
_Cilk_spawn (void) is invalid.  The function call after the cilk_spawn keyword
is parsed using recursive call in c_parser_postfix_expression (case
RID_CILK_SPAWN).  Now, c_parser_postfix_expression sees '(' followed by a
typename, so it thinks we're inside a compound literal, which means it expects
'{', but that isn't there, so we crash on the assert in c_parser_braced_init:
gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
But as the comment in c_parser_postfix_expression says, the code for parsing
a compound literal here is likely dead.  I made an experiment and added
gcc_unreachable () in that block, ran regtest, and there were no failures.
Thus it should be safe to just remove the code, which also fixes this ICE; with
the patch we just give a proper error and don't crash anymore.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'm actually slightly
nervous about the change, so maybe better table until gcc7?


This reminds me of PR67495.  Perhaps the answer here is also during the
_Cilk* stuff parsing don't call c_parser_postfix_expression, but call
c_parser_cast_expression instead and then verify what it got?
That sounds more sensible to me -- as long as there aren't unintended 
side effects of calling c_parser_cast_expression.



Jeff

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Jeff Law]

On 03/03/2016 07:15 AM, Marek Polacek wrote:

This is ICE on invalid Cilk+ code.  cilk_spawn expects a function call, so e.g.
_Cilk_spawn (void) is invalid.  The function call after the cilk_spawn keyword
is parsed using recursive call in c_parser_postfix_expression (case
RID_CILK_SPAWN).  Now, c_parser_postfix_expression sees '(' followed by a
typename, so it thinks we're inside a compound literal, which means it expects
'{', but that isn't there, so we crash on the assert in c_parser_braced_init:
gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
But as the comment in c_parser_postfix_expression says, the code for parsing
a compound literal here is likely dead.  I made an experiment and added
gcc_unreachable () in that block, ran regtest, and there were no failures.
Thus it should be safe to just remove the code, which also fixes this ICE; with
the patch we just give a proper error and don't crash anymore.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'm actually slightly
nervous about the change, so maybe better table until gcc7?

2016-03-03  Marek Polacek  

PR c/69798
* c-parser.c (c_parser_postfix_expression): Remove code dealing with
compound literals.

* gcc.dg/cilk-plus/pr69798-1.c: New test.
* gcc.dg/cilk-plus/pr69798-2.c: New test.
FWIW, I don't particularly like this version.  My worry about just 
removing this code is that I expect our testsuite doesn't cover parsing 
issues all that well.


I'd feel better if it'd been run through one of the commercial suites. 
Even then those suites don't cover the GNU extensions, but at least the 
scope of things that might go wrong is significantly diminished.



Jeff

Re: Proposed Patch for Bug 69687

[Marcel Böhme]

On 4 Mar 2016, at 1:43 AM, Bernd Schmidt  wrote:
> 
> On 03/03/2016 04:18 PM, Mike Stump wrote:
>> On Mar 3, 2016, at 6:55 AM, Marcel Böhme  wrote:
>>> I have revised the patch and removed the limits.
>> 
>> I looked at the patch, I can find no more unreasonable limits!  Wonderful.  
>> Hope someone will finish off the review and approve.
> 
> Marcel, if you haven't contributed before, we'll need a copyright assignment 
> from you before we can accept the patch. Do you already have one on file?
> 
> 
> Bernd
> 


Hi Bernd, I have requested the disclaimer form as per 
/gd/gnuorg/Copyright/request-disclaim.changes

Thanks,
- Marcel

C++ PATCH for constexpr operator=

[Jason Merrill]

One thing I overlooked in my implementation of C++14 constexpr is that 
it changed operator= to be potentially constexpr.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit 445b5c0054f338e6c1904ae4ff09fa0761222fd3
Author: Jason Merrill 
Date:   Tue Mar 1 23:06:54 2016 -0500

	* method.c (synthesized_method_walk): operator= can also be constexpr.

diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 0235e6a..38f2a54 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1379,9 +1379,18 @@ synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
 
   /* If that user-written default constructor would satisfy the
  requirements of a constexpr constructor (7.1.5), the
- implicitly-defined default constructor is constexpr.  */
+ implicitly-defined default constructor is constexpr.
+
+ The implicitly-defined copy/move assignment operator is constexpr if
+  - X is a literal type, and
+  - the assignment operator selected to copy/move each direct base class
+	subobject is a constexpr function, and
+  - for each non-static data member of X that is of class type (or array
+	thereof), the assignment operator selected to copy/move that member is a
+	constexpr function.  */
   if (constexpr_p)
-*constexpr_p = ctor_p;
+*constexpr_p = ctor_p
+  || (assign_p && cxx_dialect >= cxx14);
 
   move_p = false;
   switch (sfk)
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-assign1.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-assign1.C
new file mode 100644
index 000..4583b64
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-assign1.C
@@ -0,0 +1,9 @@
+// { dg-do compile { target c++14 } }
+
+struct A { };
+
+struct B
+{
+  A a;
+  constexpr B& operator=(const B&) = default;
+};

C++ PATCH to instantiation of generic lambda

[Jason Merrill]

In this testcase, instantiating the return type doesn't work before 
we've instantiated the declaration of the operator().  Furthermore, 
instantiating the operator() declaration necessarily instantiates the 
return type, so we can wait and look it up from there.


Tested x86_64-pc-linux-gnu, applying to trunk.
commit 178af5b2e30a6efb8e49638c37dfea7320586c62
Author: Jason Merrill 
Date:   Wed Mar 2 14:53:31 2016 -0500

	* pt.c (tsubst_copy_and_build) [LAMBDA_EXPR]: Get
	LAMBDA_EXPR_RETURN_TYPE from the instantiated closure.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index c5b9201..e8cd736 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -17103,8 +17103,6 @@ tsubst_copy_and_build (tree t,
 	else
 	  gcc_unreachable ();
 	LAMBDA_EXPR_EXTRA_SCOPE (r) = scope;
-	LAMBDA_EXPR_RETURN_TYPE (r)
-	  = tsubst (LAMBDA_EXPR_RETURN_TYPE (t), args, complain, in_decl);
 
 	gcc_assert (LAMBDA_EXPR_THIS_CAPTURE (t) == NULL_TREE
 		&& LAMBDA_EXPR_PENDING_PROXIES (t) == NULL);
@@ -17115,6 +17113,9 @@ tsubst_copy_and_build (tree t,
 	   declaration of the op() for later calls to lambda_function.  */
 	complete_type (type);
 
+	if (tree fn = lambda_function (type))
+	  LAMBDA_EXPR_RETURN_TYPE (r) = TREE_TYPE (TREE_TYPE (fn));
+
 	LAMBDA_EXPR_THIS_CAPTURE (r) = NULL_TREE;
 
 	insert_pending_capture_proxies ();
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-trailing1.C b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-trailing1.C
new file mode 100644
index 000..96755b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-trailing1.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++14 } }
+
+template 
+void f()
+{
+  auto lam = [](auto a)->decltype(++a) { return a; };
+}
+
+int main()
+{
+  f();
+}

C++ PATCH for c++/67164 (error with variadic templates)

[Jason Merrill]

When we instantiate an element of a pack expansion, we replace the 
argument pack in the template argument vec with an ARGUMENT_PACK_SELECT 
which indicates the desired element of the vec.  If the args have been 
used to instantiate other templates as well, the args of those instances 
get modified as well, which can lead to strange results when we run into 
ARGUMENT_PACK_SELECT in inappropriate places.  This patch fixes this 
issue by making a copy of the template args before we start messing with 
them.


Tested x86_64-pc-linux-gnu, applying to trunk.

commit e32e331a8f6ada0c20ff13240bac030020f627bf
Author: Jason Merrill 
Date:   Wed Mar 2 09:17:20 2016 -0500

	PR c++/67164

	* pt.c (copy_template_args): New.
	(tsubst_pack_expansion): Use it.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b3681be..c5b9201 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -178,6 +178,7 @@ static int check_cv_quals_for_unify (int, tree, tree);
 static void template_parm_level_and_index (tree, int*, int*);
 static int unify_pack_expansion (tree, tree, tree,
  tree, unification_kind_t, bool, bool);
+static tree copy_template_args (tree);
 static tree tsubst_template_arg (tree, tree, tsubst_flags_t, tree);
 static tree tsubst_template_args (tree, tree, tsubst_flags_t, tree);
 static tree tsubst_template_parms (tree, tree, tsubst_flags_t);
@@ -11037,11 +11038,12 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
   /* For each argument in each argument pack, substitute into the
  pattern.  */
   result = make_tree_vec (len);
+  tree elem_args = copy_template_args (args);
   for (i = 0; i < len; ++i)
 {
   t = gen_elem_of_pack_expansion_instantiation (pattern, packs,
 		i,
-		args, complain,
+		elem_args, complain,
 		in_decl);
   TREE_VEC_ELT (result, i) = t;
   if (t == error_mark_node)
@@ -11136,6 +11138,32 @@ make_argument_pack (tree vec)
   return pack;
 }
 
+/* Return an exact copy of template args T that can be modified
+   independently.  */
+
+static tree
+copy_template_args (tree t)
+{
+  if (t == error_mark_node)
+return t;
+
+  int len = TREE_VEC_LENGTH (t);
+  tree new_vec = make_tree_vec (len);
+
+  for (int i = 0; i < len; ++i)
+{
+  tree elt = TREE_VEC_ELT (t, i);
+  if (elt && TREE_CODE (elt) == TREE_VEC)
+	elt = copy_template_args (elt);
+  TREE_VEC_ELT (new_vec, i) = elt;
+}
+
+  NON_DEFAULT_TEMPLATE_ARGS_COUNT (new_vec)
+= NON_DEFAULT_TEMPLATE_ARGS_COUNT (t);
+
+  return new_vec;
+}
+
 /* Substitute ARGS into the vector or list of template arguments T.  */
 
 static tree
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-tuple2.C b/gcc/testsuite/g++.dg/cpp0x/variadic-tuple2.C
new file mode 100644
index 000..43c00e9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic-tuple2.C
@@ -0,0 +1,29 @@
+// PR c++/67164
+// { dg-do compile { target c++11 } }
+
+#include 
+
+namespace detail {
+template 
+struct fast_and
+: std::is_same>
+{ };
+}
+
+template 
+struct tuple {
+tuple() { }
+
+template ::value...>::value
+>::type>
+tuple(Yn&& ...yn) { }
+
+template ::value...>::value
+>::type>
+tuple(tuple const& other) { }
+};
+
+tuple> t{};
+tuple> copy = t;

Re: [AArch64] Emit square root using the Newton series

[Evandro Menezes]

On 02/16/16 14:56, Evandro Menezes wrote:

On 12/08/15 15:35, Evandro Menezes wrote:

Emit square root using the Newton series

   2015-12-03  Evandro Menezes  

   gcc/
* config/aarch64/aarch64-protos.h (aarch64_emit_swsqrt):
   Declare new
function.
* config/aarch64/aarch64-simd.md (sqrt2): New
   expansion and
insn definitions.
* config/aarch64/aarch64-tuning-flags.def
(AARCH64_EXTRA_TUNE_FAST_SQRT): New tuning macro.
* config/aarch64/aarch64.c (aarch64_emit_swsqrt): Define
   new function.
* config/aarch64/aarch64.md (sqrt2): New expansion
   and insn
definitions.
* config/aarch64/aarch64.opt (mlow-precision-recip-sqrt):
   Expand option
description.
* doc/invoke.texi (mlow-precision-recip-sqrt): Likewise.

This patch extends the patch that added support for implementing 
x^-1/2 using the Newton series by adding support for x^1/2 as well.


Is it OK at this point of stage 3?

Thank you,



James,

As I was saying, this patch results in some validation errors in 
CPU2000 benchmarks using DF.  Although proving the algorithm to be 
pretty solid with a vast set of random values, I'm confused why some 
benchmarks fail to validate with this implementation of the Newton 
series for square root too, when they pass with the Newton series for 
reciprocal square root.


Since I had no problems with the same algorithm on x86-64, I wonder if 
the initial estimate on AArch64, which offers just 8 bits, whereas 
x86-64 offers 11 bits, has to do with it.  Then again, the algorithm 
iterated 1 less time on x86-64 than on AArch64.


Since it seems that the initial estimate is sufficient for CPU2000 to 
validate when using SF, I'm leaning towards restricting the Newton 
series for square root only for SF.


Your thoughts on the matter are appreciated,


Add choices for the reciprocal square root approximation

Allow a target to prefer such operation depending on the FP
   precision.

gcc/
* config/aarch64/aarch64-protos.h
(AARCH64_EXTRA_TUNE_APPROX_RSQRT): New macro.
* config/aarch64/aarch64-tuning-flags.def
(AARCH64_EXTRA_TUNE_APPROX_RSQRT_DF): New mask.
(AARCH64_EXTRA_TUNE_APPROX_RSQRT_SF): Likewise.
* config/aarch64/aarch64.c
(use_rsqrt_p): New argument for the mode.
(aarch64_builtin_reciprocal): Devise mode from builtin.
(aarch64_optab_supported_p): New argument for the mode.


Feedback appreciated.

Thank you,

--
Evandro Menezes

Re: [PATCH] libffi testsuite: Use split to ensure valid tcl list

[Mike Stump]

On Feb 25, 2016, at 12:15 PM, Thomas Schwinge  wrote:
> On Thu, 25 Feb 2016 11:45:06 -0800, Mike Stump  wrote:
>> On Feb 25, 2016, at 11:10 AM, Thomas Schwinge  
>> wrote:
>>> +set lines [libffi_target_compile $src /dev/null assembly “"]
>> 
>> Does this work on a dos box, or windows or other random non-posix systems?
> 
> I don't know, and can't easily test.  However, I had seen the same
> pattern be used elsewhere, for example:
> 
>$ git grep dev/null -- libstdc++-v3/testsuite/lib/
>libstdc++-v3/testsuite/lib/libstdc++.exp:   set lines 
> [v3_target_compile $src /dev/null executable ""]
>[several more]

Ah, I would not worry about it then.

Re: C++ PATCH to implement C++14 aggregate NSDMI (N3653)

[Jason Merrill]

This was missing a use of NSDMI when considering list-initialization via 
aggregate initialization in overload resolution.


Tested x86_64-pc-linux-gnu, applying to trunk and 5.

commit c2a038bbd3c2a82cc6f6679e5a70705f48571e07
Author: Jason Merrill 
Date:   Wed Mar 2 17:24:27 2016 -0500

	* call.c (build_aggr_conv): Use get_nsdmi.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 8d5582a..3ad3bd5 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -897,6 +897,8 @@ build_aggr_conv (tree type, tree ctor, int flags, tsubst_flags_t complain)
 
   if (i < CONSTRUCTOR_NELTS (ctor))
 	val = CONSTRUCTOR_ELT (ctor, i)->value;
+  else if (DECL_INITIAL (field))
+	val = get_nsdmi (field, /*ctor*/false);
   else if (TREE_CODE (ftype) == REFERENCE_TYPE)
 	/* Value-initialization of reference is ill-formed.  */
 	return NULL;
diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr4.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr4.C
new file mode 100644
index 000..71830cd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr4.C
@@ -0,0 +1,13 @@
+// { dg-do compile { target c++14 } }
+
+struct A
+{
+  A(int);
+};
+
+struct B
+{
+  A a{42};
+};
+
+B f() { return {}; }

Re: C++ PATCH for c++/51406, 51161 (wrong-code with static cast to rvalue ref)

[Jason Merrill]

On 12/14/2011 12:14 AM, Jason Merrill wrote:

The code for casting to rvalue ref was assuming that no base adjustment
would be necessary.  This patch delegates to the normal lvalue binding
code, and then changes the result to be an rvalue reference.


The test for DECL_P isn't sufficient to catch all cases where the result 
of cp_fold_convert is still an lvalue.  Now that we're doing delayed 
folding, let's replace the cp_fold_convert with an unconditional 
NON_LVALUE_EXPR.


Tested x86_64-pc-linux-gnu, applying to trunk.  For GCC 5 I'm changing 
DECL_P to real_lvalue_p instead.



commit 46ab061b39527bbdd427bdf887eb6954768d4b61
Author: Jason Merrill 
Date:   Wed Mar 2 01:11:30 2016 -0500

	PR c++/51406

	* typeck.c (build_static_cast_1): Avoid folding back to lvalue.

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 5145879..20f0afc 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6704,11 +6704,7 @@ build_static_cast_1 (tree type, tree expr, bool c_cast_p,
 	  tree lref = cp_build_reference_type (TREE_TYPE (type), false);
 	  result = (perform_direct_initialization_if_possible
 		(lref, expr, c_cast_p, complain));
-	  result = cp_fold_convert (type, result);
-	  /* Make sure we don't fold back down to a named rvalue reference,
-	 because that would be an lvalue.  */
-	  if (DECL_P (result))
-	result = build1 (NON_LVALUE_EXPR, type, result);
+	  result = build1 (NON_LVALUE_EXPR, type, result);
 	  return convert_from_reference (result);
 	}
   else
diff --git a/gcc/testsuite/g++.dg/cpp0x/rv-cast5.C b/gcc/testsuite/g++.dg/cpp0x/rv-cast5.C
new file mode 100644
index 000..c2473e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/rv-cast5.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++11 } }
+
+template 
+struct hold {
+  T value;
+  constexpr T&& operator()() && { return static_cast(value); }
+};
+
+int main()
+{
+  hold{42}();
+}

Re: [PING] genattrab.c generate switch

[Patrick Palka]

On Thu, Mar 3, 2016 at 4:29 PM, Jesper Broge Jørgensen
 wrote:
>
> On 18/02/16 13:22, Bernd Schmidt wrote:
>>
>> On 01/19/2016 12:47 PM, Jesper Broge Jørgensen wrote:
>>>
>>> Here is the reformatted patch:
>>
>>
>> This will probably have to wait until stage1.
>>
>>> +  const int code = GET_CODE (op2);
>>> +  if (code != IOR)
>>> +{
>>> +  if (code == EQ_ATTR)
>>
>>
>> All the formatting still looks completely mangled. This was probably done
>> by your mailer. Please try attaching the diff as text/plain.
>>
>>
>> Bernd
>>
> Hi i send the patch back as an attatchment as requested about two weeks ago
> (https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01256.html) but i have not
> received any response.
>
> If it has to wait for stage 1 are there anything else i can do for the patch
> untill then?

I still suggest to try making write_test_expr() avoid emitting
redundant parentheses for chains of || or &&, which would fix the
original issue all the same.  Previously you claimed that such a
change would not be simpler than your current patch, but I gave it a
quick try and ended up with a much smaller patch:

 gcc/genattrtab.c | 26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

Re: C++ PATCH for c++/67364 (constexpr vs. empty class)

[Jason Merrill]

On 02/25/2016 09:08 AM, Jason Merrill wrote:

We don't bother evaluating a store to an empty class member, and we
shouldn't complain about accesses either.


This needs to use really_empty_class, since that's what 
expand_aggr_init_1 uses.


Tested x86_64-pc-linux-gnu, applying to trunk and 5.


commit 9a1d6b7c9fa018033ff444829ad3c597e61f5120
Author: Jason Merrill 
Date:   Wed Mar 2 14:33:54 2016 -0500

	PR c++/67364

	* constexpr.c (cxx_eval_component_reference): Just return an empty
	CONSTRUCTOR for an empty class.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index bcb129f..5a81469 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1988,11 +1988,12 @@ cxx_eval_component_reference (const constexpr_ctx *ctx, tree t,
 }
 
   if (CONSTRUCTOR_NO_IMPLICIT_ZERO (whole)
-  && !is_empty_class (TREE_TYPE (part)))
+  && !is_really_empty_class (TREE_TYPE (t)))
 {
   /* 'whole' is part of the aggregate initializer we're currently
 	 building; if there's no initializer for this member yet, that's an
-	 error. */
+	 error.  But expand_aggr_init_1 doesn't bother to initialize really
+	 empty classes, so ignore them here, too.  */
   if (!ctx->quiet)
 	error ("accessing uninitialized member %qD", part);
   *non_constant_p = true;
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-empty11.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty11.C
new file mode 100644
index 000..7437367
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-empty11.C
@@ -0,0 +1,17 @@
+// PR c++/67364
+// { dg-do compile { target c++11 } }
+
+template 
+struct element : Xn {
+  constexpr element() : Xn() { }
+};
+
+template 
+struct closure {
+  element member;
+  constexpr closure() { }
+};
+
+struct empty { struct {} s; };
+constexpr closure tup{};
+constexpr empty first = tup.member;

Re: [PING] genattrab.c generate switch

[Jesper Broge Jørgensen]

On 18/02/16 13:22, Bernd Schmidt wrote:

On 01/19/2016 12:47 PM, Jesper Broge Jørgensen wrote:

Here is the reformatted patch:


This will probably have to wait until stage1.


+  const int code = GET_CODE (op2);
+  if (code != IOR)
+{
+  if (code == EQ_ATTR)


All the formatting still looks completely mangled. This was probably 
done by your mailer. Please try attaching the diff as text/plain.



Bernd

Hi i send the patch back as an attatchment as requested about two weeks 
ago (https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01256.html) but i 
have not received any response.


If it has to wait for stage 1 are there anything else i can do for the 
patch untill then?

Re: Reinstate generic stack checking warning with LRA

[Eric Botcazou]

> This works though, ok for trunk?
> 
> 2016-03-03  Jakub Jelinek  
> 
>   PR ada/70017
>   * gcc.dg/pr70017.c (foo): Store 0 to first element of each array.

Sure, thanks.

-- 
Eric Botcazou

Re: [Patch, fortran] PR69834 - Collision in derived type hashes

[Jerry DeLisle]

On 03/03/2016 07:59 AM, Paul Richard Thomas wrote:
> Dear All,
> 
> What started out as a provisional kludge, when first working on OOP,
> has come back to bite us after 7 years. A collision in derived type
> has values has been reported on clf. In principle, as pointed out in
> the clf thread, this could mean that existing code might be quietly
> confusing dynamic types. Fortunately, this is unlikely because the
> error in SELECT TYPE that flagged up this problem might appear or
> incorrect fields might be accessed, giving rise to runtime errors.
> 
> The fix uses a new vtable field, '_name' that is loaded with the
> value, "typename_scopename", which is used for the cases in SELECT
> TYPE and for comparison in SAME_TYPE_AS. I have retained the '_hash'
> field for compatibility with existing libraries. It could easily be
> removed, if that is preferred, but would require a publicity campaign
> to ensure that users recompile their code.
> 
> The changes are sufficiently well described in the ChangeLogs and the
> comments in the patch to not warrant further comment.
> 
> I have to confess to not knowing quite what to propose here. My gut
> feeling is that we should bite the bullet and the patch should be
> applied to trunk and 5-branch. However, I am open, on the grounds
> above, to wait until 7.0.0. It does bootstrap and regtest on trunk
> with FC23/x86_64.
> 
> Thanks to Dominique for testing an early version of the test and to
> Thomas for picking up on the clf thread.
> 

In my very humble opinion, I think you should commit the patch now before
release. As I have said before, people know major releases are bleeding edge,
bugs if any will be flushed out and can be fixed at 6.2 or 6.3.  It is the open
nature of our software and the user feedback that makes this all work. (also we
know Fortran is not release critical)

I tested with my own OOP code which is an adaptation of Metcalf's linked anylist
and it works fine.  Thats the best I can do and it is fairly complex code.  I
can send it to you if you would like to have it in your test pile.

Regards,

Jerry

Re: [PATCH] Fix vec_set_hi* patterns (PR target/70059)

[Jakub Jelinek]

On Thu, Mar 03, 2016 at 01:08:41PM +0100, Jakub Jelinek wrote:
> Fixed thusly, unfortunately I don't have access to avx512f (and not even to
> avx512dq) hw, so while I will bootstrap/regtest it on Haswell-E, can't test
> the tests if they now work at runtime (they link and the assembly of the foo
> routine has changed and looks good to me).  Can somebody test this please
> on real hw or emulator?
> Ok for trunk if it passes?

FYI, my bootstrap/regtest on Haswell-E (but without trying to run any
AVX512-* code, just link it at most) passed on both x86_64-linux and
i686-linux.

> 2016-03-03  Jakub Jelinek  
> 
>   PR target/70059
>   * config/i386/sse.md (vec_set_lo_,
>   _vinsert_mask): Formatting
>   fixes.
>   (vec_set_hi_): Likewise.  Swap VEC_CONCAT operands.
> 
>   * gcc.target/i386/avx512f-pr70059.c: New test.
>   * gcc.target/i386/avx512dq-pr70059.c: New test.

Jakub

[PATCH] Another fix for decide_alg (PR target/70062)

[Jakub Jelinek]

Hi!

Before my recent decide_alg change, *dynamic_check == -1 was indeed
guaranteed, because any_alg_usable_p doesn't depend on the arguments of
decide_alg that might change during recursive call, so we'd only recurse if
it wouldn't set *dynamic_check.  But, if we give up because we'd otherwise
recurse infinitely, we can set *dynamic_check to 128.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2016-03-03  Jakub Jelinek  

PR target/70062
* config/i386/i386.c (decide_alg): If
TARGET_INLINE_STRINGOPS_DYNAMICALLY, allow *dynamic_check to be also
128 from the recursive call.

* gcc.target/i386/pr70062.c: New test.

--- gcc/config/i386/i386.c.jj   2016-03-02 14:08:00.0 +0100
+++ gcc/config/i386/i386.c  2016-03-03 17:48:18.587450348 +0100
@@ -26170,11 +26170,15 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
 }
   alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
zero_memset, have_as, dynamic_check, noalign);
-  gcc_assert (*dynamic_check == -1);
   if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
-   *dynamic_check = max;
+   {
+ /* *dynamic_check could be set 128 above because we avoided
+infinite recursion.  */
+ gcc_assert (*dynamic_check == -1 || *dynamic_check == 128);
+ *dynamic_check = max;
+   }
   else
-   gcc_assert (alg != libcall);
+   gcc_assert (alg != libcall && *dynamic_check == -1);
   return alg;
 }
   return (alg_usable_p (algs->unknown_size, memset, have_as)
--- gcc/testsuite/gcc.target/i386/pr70062.c.jj  2016-03-03 17:54:13.167642050 
+0100
+++ gcc/testsuite/gcc.target/i386/pr70062.c 2016-03-03 17:54:58.753023808 
+0100
@@ -0,0 +1,11 @@
+/* PR target/70062 */
+/* { dg-options "-minline-all-stringops -minline-stringops-dynamically 
-mmemcpy-strategy=libcall:-1:noalign -Wno-psabi" } */
+/* { dg-additional-options "-mtune=k6-2" { target ia32 } } */
+
+typedef int V __attribute__ ((vector_size (32)));
+
+V
+foo (V x)
+{
+  return (V) { x[0] };
+}

Jakub

Re: [PATCH] Fix PR69951

[Christophe Lyon]

On 2 March 2016 at 10:49, Christophe Lyon  wrote:
> On 2 March 2016 at 10:16, James Greenhalgh  wrote:
>> On Tue, Mar 01, 2016 at 05:56:30PM +0100, Christophe Lyon wrote:
>>> On 1 March 2016 at 10:51, James Greenhalgh  wrote:
>>> > On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
>>> >> On Mon, 29 Feb 2016, James Greenhalgh wrote:
>>> >>
>>> >> > On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
>>> >> > >
>>> >> > > The following fixes PR69951, hopefully the last case of decl alias
>>> >> > > issues with alias analysis.  This time it's points-to and the 
>>> >> > > DECL_UIDs
>>> >> > > used in points-to sets not being canonicalized.
>>> >> > >
>>> >> > > The simplest (and cheapest) fix is to make aliases refer to the
>>> >> > > ultimate alias target via their DECL_PT_UID which we conveniently
>>> >> > > have available.
>>> >> > >
>>> >> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to 
>>> >> > > trunk.
>>> >> > >
>>> >> > > Richard.
>>> >> > >
>>> >> > > 2016-02-26  Richard Biener  
>>> >> > >
>>> >> > >   PR tree-optimization/69551
>>> >> > >   * tree-ssa-structalias.c (get_constraint_for_ssa_var): When
>>> >> > >   looking through aliases adjust DECL_PT_UID to refer to the
>>> >> > >   ultimate alias target.
>>> >> > >
>>> >> > >   * gcc.dg/torture/pr69951.c: New testcase.
>>> >> >
>>> >> > I see this new testcase failing on an ARM target as so:
>>> >> >
>>> >> > /tmp/ccChjoFc.s: Assembler messages:
>>> >> > /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a 
>>> >> > symbol match an ARM instruction: b
>>> >> >
>>> >> > FAIL: gcc.dg/torture/pr69951.c   -O0  (test for excess errors)
>>> >> >
>>> >> > But I haven't managed to reproduce it outside of the test environment.
>>> >> >
>>> >> > The fix looks trivial, rename b to anything else you fancy (well... 
>>> >> > stay
>>> >> > clear of add and ldr). I'll put a fix in myself if I can manage to get
>>> >> > this to reproduce - though if anyone else wants to do it I won't be
>>> >> > offended :-).
>>> >>
>>> >> Huh, I wonder what's the use of such warning.  After all 'ldr' is a valid
>>> >> C symbol name, too.  In fact my cross arm as doesn't report this
>>> >> warning (binutils 2.25.0)
>>> >>
>>> >> > arm-suse-linux-gnueabi-as t.s -mwarn-syms
>>> >> Assembler messages:
>>> >> Error: unrecognized option -mwarn-syms
>>> >
>>> > Right, I've figured out the set of conditions... You need to be targeting
>>> > an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
>>> > from config/arm/linux-elf.h is pulled in. This causes us to emit:
>>> >
>>> > b = a
>>> >
>>> > Rather than
>>> >
>>> > .setb,a
>>> >
>>> > Writing it as "b = a" causes the warning added to resolve binutils
>>> > PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
>>> > that patch).
>>> >
>>> James,
>>>
>>> What happens for you on arm-none-eabi configurations?
>>> I'm using binutils-2.25, so I can't see this warning whatever the target.
>>> However, I'm using qemu-arm and this test fails on arm-none-eabi,
>>> because argc is set to 0 during the startup-code.
>>>
>>> As I understand it, qemu-arm considers the code page as readonly,
>>> and thus the GetCmdLine semi hosting call cannot write argc/argv
>>> back to memory in the same code page (I'm using libgloss' crt0).
>>>
>>> I tried to play with qemu-system-arm, but then libgloss' crt0 does not
>>> set the reset vector and the simulation does random things, starting at
>>> address 0.
>>>
>>> Am I missing some newlib/libgloss configuration bits, or should I
>>> send a newlib patch to address this?
>>
>> Hi Christophe,
>>
>> I didn't get this running under arm-none-eabi. The testcase does have
>> undefined behaviour (too few arguments to main), but I'd be surprised if
>> that was the issue... I'm sure the testcase could be rewritten to avoid
>> the dependence on argc if this proves an issue for other bare-metal
>> testers running under an emulator.
>>
>
> Indeed, I'm wondering why the testcase depends on argc beeing non-zero?
>

To avoid modifying the testcase too much, I propose to replace
if (argc)
by
if (argc >= 0)
as in the attached patch.
This does make the trick on arm-non-eabi.

OK?

Christophe.


>> Thanks,
>> James
>>
>>> > Resolving it by hacking the testcase would be one fix, but I wonder why 
>>> > the
>>> > ARM port prefers to emit "b = a" in a linux environment if .set does the
>>> > same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
>>> > remember? (AArch64 does the same thing, but the AArch64 gas port doesn't
>>> > have the PR18347 fix).
>>> >
>>> > Cheers,
>>> > James
>>> >
>>> > ---
>>> >
>>> > [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=18347
>>> >
>>> >>
>>> >> Richard.
>>> >>
>>>
2016-03-04  Christophe Lyon  

* gcc.dg/torture/pr69951.c: 

Re: [Patch X86_64] : Fix type attribute for sseimul reservations in znver1.md

[Uros Bizjak]

On Thu, Mar 3, 2016 at 7:05 PM, Kumar, Venkataramanan
 wrote:
> Hi Maintainers,
>
> The below patch corrects the type attribute  for "sseimul" type reservations 
> in znver1.md.
>
> (snip)
> diff --git a/gcc/config/i386/znver1.md b/gcc/config/i386/znver1.md
> index 3db3bed..feeccd7 100644
> --- a/gcc/config/i386/znver1.md
> +++ b/gcc/config/i386/znver1.md
> @@ -913,28 +913,28 @@
>  (define_insn_reservation "znver1_sseimul" 3
>  (and (eq_attr "cpu" "znver1")
>   (and (eq_attr "mode" "TI")
> -  (and (eq_attr "type" "ssemul")
> +  (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "none"
>  "znver1-direct,znver1-fp0*3")
>
>  (define_insn_reservation "znver1_sseimul_avx256" 4
>  (and (eq_attr "cpu" "znver1")
>   (and (eq_attr "mode" "OI")
> -  (and (eq_attr "type" "ssemul")
> +  (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "none"
>  "znver1-double,znver1-fp0*4")
>
>  (define_insn_reservation "znver1_sseimul_load" 7
>  (and (eq_attr "cpu" "znver1")
>   (and (eq_attr "mode" "TI")
> -  (and (eq_attr "type" "ssemul")
> +  (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "load"
>  "znver1-direct,znver1-load,znver1-fp0*3")
>
>  (define_insn_reservation "znver1_sseimul_avx256_load" 8
>  (and (eq_attr "cpu" "znver1")
>   (and (eq_attr "mode" "OI")
> -  (and (eq_attr "type" "ssemul")
> +  (and (eq_attr "type" "sseimul")
> (eq_attr "memory" "load"
>  "znver1-double,znver1-load,znver1-fp0*4")
>
> @@ -942,13 +942,13 @@
>  (and (eq_attr "cpu" "znver1")
>   (and (eq_attr "mode" "DI")
>(and (eq_attr "memory" "none")
> -   (eq_attr "type" "ssemul"
> +   (eq_attr "type" "sseimul"
>  "znver1-direct,znver1-fp0*4")
> (Snip)
>
> Changelog
>
> 2016-03-03  Venkataramanan Kumar  
>
>Fix sseimul type attribute.
>* config/i386/znver1.md
>(znver1_sseimul, znver1_sseimul_avx256, znver1_sseimul_load,
>znver1_sseimul_avx256_load,  znver1_sseimul_di,
>znver1_sseimul_load_di) : Fix the type attribute.
>
> Ok for trunk if bootstrap and testing passes?

OK.

BTW: This patch is trivial, according to [1]. The criteria is simply:

Obvious fixes can be committed without prior approval. Just check in
the fix and copy it to gcc-patches. A good test to determine whether a
fix is obvious: will the person who objects to my work the most be
able to find a fault with my fix? If the fix is later found to be
faulty, it can always be rolled back. We don't want to get overly
restrictive about checkin policies.

[1] https://gcc.gnu.org/svnwrite.html

Thanks,
Uros.

Re: Reinstate generic stack checking warning with LRA

[Jakub Jelinek]

On Thu, Mar 03, 2016 at 12:31:52PM +0100, Eric Botcazou wrote:
> > Anyway, looking at pro_and_epilogue dumps, with additional
> > -fstack-protector-strong we decrease sp only by 4176, while without it by
> > 8224 (on x86_64; the testcase fails on all targets I've tried so far
> > ({x86_64,i686,powerpc64{,le},s390{,x},aarch64,armv7hl}-linux).
> 
> Yeah, the threshold is around 4KB, feel free to add a few more HUNDREDs.

I've mislooked, with -fstack-protector-strong it just has 48 bytes, and
adding many more HUNDREDs doesn't change anything.

This works though, ok for trunk?

2016-03-03  Jakub Jelinek  

PR ada/70017
* gcc.dg/pr70017.c (foo): Store 0 to first element of each array.

--- gcc/testsuite/gcc.dg/pr70017.c.jj   2016-03-02 07:39:10.0 +0100
+++ gcc/testsuite/gcc.dg/pr70017.c  2016-03-03 19:22:02.098801850 +0100
@@ -13,4 +13,8 @@ void foo(void)
 {
   HUNDRED(a)
   HUNDRED(b)
+#undef ONE
+#define ONE(s) a##s[0] = 0;
+  HUNDRED(a)
+  HUNDRED(b)
 } /* { dg-warning "frame size too large for reliable stack checking" } */


Jakub

Re: [PATCH, ARM] Fix gcc.c-torture/execute/loop-2b.c execution failure on cortex-m0

[Thomas Preudhomme]

On Thursday 03 March 2016 15:32:27 Thomas Preudhomme wrote:
> On Thursday 03 March 2016 09:44:31 Ramana Radhakrishnan wrote:
> > On Thu, Mar 3, 2016 at 9:40 AM, Thomas Preudhomme
> > 
> >  wrote:
> > > On Friday 15 January 2016 12:45:04 Ramana Radhakrishnan wrote:
> > >> On Wed, Dec 16, 2015 at 9:11 AM, Thomas Preud'homme
> > >> 
> > >>  wrote:
> > >> > During reorg pass, thumb1_reorg () is tasked with rewriting mov rd,
> > >> > rn
> > >> > to
> > >> > subs rd, rn, 0 to avoid a comparison against 0 instruction before
> > >> > doing
> > >> > a
> > >> > conditional branch based on it. The actual avoiding of cmp is done in
> > >> > cbranchsi4_insn instruction C output template. When the condition is
> > >> > met,
> > >> > the source register (rn) is also propagated into the comparison in
> > >> > place
> > >> > the destination register (rd).
> > >> > 
> > >> > However, right now thumb1_reorg () only look for a mov followed by a
> > >> > cbranchsi but does not check whether the comparison in cbranchsi is
> > >> > against the constant 0. This is not safe because a non clobbering
> > >> > instruction could exist between the mov and the comparison that
> > >> > modifies
> > >> > the source register. This is what happens here with a post increment
> > >> > of
> > >> > the source register after the mov, which skip the [i] == [1]
> > >> > comparison for iteration i == 1.
> > >> > 
> > >> > This patch fixes the issue by checking that the comparison is against
> > >> > constant 0.
> > >> > 
> > >> > ChangeLog entry is as follow:
> > >> > 
> > >> > 
> > >> > *** gcc/ChangeLog ***
> > >> > 
> > >> > 2015-12-07  Thomas Preud'homme  
> > >> > 
> > >> > * config/arm/arm.c (thumb1_reorg): Check that the comparison
> > >> > is
> > >> > against the constant 0.
> > >> 
> > >> OK.
> > >> 
> > >> Ramana
> > >> 
> > >> > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> > >> > index 42bf272..49c0a06 100644
> > >> > --- a/gcc/config/arm/arm.c
> > >> > +++ b/gcc/config/arm/arm.c
> > >> > @@ -17195,7 +17195,7 @@ thumb1_reorg (void)
> > >> > 
> > >> >FOR_EACH_BB_FN (bb, cfun)
> > >> >
> > >> >  {
> > >> >  
> > >> >rtx dest, src;
> > >> > 
> > >> > -  rtx pat, op0, set = NULL;
> > >> > +  rtx cmp, op0, op1, set = NULL;
> > >> > 
> > >> >rtx_insn *prev, *insn = BB_END (bb);
> > >> >bool insn_clobbered = false;
> > >> > 
> > >> > @@ -17208,8 +17208,13 @@ thumb1_reorg (void)
> > >> > 
> > >> > continue;
> > >> >
> > >> >/* Get the register with which we are comparing.  */
> > >> > 
> > >> > -  pat = PATTERN (insn);
> > >> > -  op0 = XEXP (XEXP (SET_SRC (pat), 0), 0);
> > >> > +  cmp = XEXP (SET_SRC (PATTERN (insn)), 0);
> > >> > +  op0 = XEXP (cmp, 0);
> > >> > +  op1 = XEXP (cmp, 1);
> > >> > +
> > >> > +  /* Check that comparison is against ZERO.  */
> > >> > +  if (!CONST_INT_P (op1) || INTVAL (op1) != 0)
> > >> > +   continue;
> > >> > 
> > >> >/* Find the first flag setting insn before INSN in basic block
> > >> >BB.
> > >> >*/
> > >> >gcc_assert (insn != BB_HEAD (bb));
> > >> > 
> > >> > @@ -17249,7 +17254,7 @@ thumb1_reorg (void)
> > >> > 
> > >> >   PATTERN (prev) = gen_rtx_SET (dest, src);
> > >> >   INSN_CODE (prev) = -1;
> > >> >   /* Set test register in INSN to dest.  */
> > >> > 
> > >> > - XEXP (XEXP (SET_SRC (pat), 0), 0) = copy_rtx (dest);
> > >> > + XEXP (cmp, 0) = copy_rtx (dest);
> > >> > 
> > >> >   INSN_CODE (insn) = -1;
> > >> > 
> > >> > }
> > >> >  
> > >> >  }
> > >> > 
> > >> > Testsuite shows no regression when run for arm-none-eabi with
> > >> > -mcpu=cortex-m0 -mthumb
> > > 
> > > The patch applies cleanly on gcc-5-branch and also show no regression
> > > when
> > > run for arm-none-eabi with -mcpu=cortex-m0 -mthumb. Is it ok to
> > > backport?
> > 
> > This deserves a testcase.
> 
> The original patch don't have one initially because it fixes a fail of an
> existing testcase (loop-2b.c). However, the test pass on gcc 5 due to
> difference in code generation. I'm currently trying to come up with a
> testcase and will get back at you.

Sadly I did not manage to come up with a testcase that works on GCC 5. One 
need to reproduce a sequence of the form:

(set B A)
(insn clobbering A that is not a set, ie store with post increment)
(conditional branch between A and something else)

In that case, thumb1_reorg changes the set into (set B (minus A 0)) which is 
safe but also replace A by B in the conditional insn which is unsafe in the 
above situation. The problem I am having is to make GCC generate a move 
instruction because it's always optimized away. Using local register variable 
is not an option because the move should be between regular registers.

Any idea to construct a 

[Patch X86_64] : Fix type attribute for sseimul reservations in znver1.md

[Kumar, Venkataramanan]

Hi Maintainers,

The below patch corrects the type attribute  for "sseimul" type reservations in 
znver1.md.

(snip)
diff --git a/gcc/config/i386/znver1.md b/gcc/config/i386/znver1.md
index 3db3bed..feeccd7 100644
--- a/gcc/config/i386/znver1.md
+++ b/gcc/config/i386/znver1.md
@@ -913,28 +913,28 @@
 (define_insn_reservation "znver1_sseimul" 3
 (and (eq_attr "cpu" "znver1")
  (and (eq_attr "mode" "TI")
-  (and (eq_attr "type" "ssemul")
+  (and (eq_attr "type" "sseimul")
(eq_attr "memory" "none"
 "znver1-direct,znver1-fp0*3")

 (define_insn_reservation "znver1_sseimul_avx256" 4
 (and (eq_attr "cpu" "znver1")
  (and (eq_attr "mode" "OI")
-  (and (eq_attr "type" "ssemul")
+  (and (eq_attr "type" "sseimul")
(eq_attr "memory" "none"
 "znver1-double,znver1-fp0*4")

 (define_insn_reservation "znver1_sseimul_load" 7
 (and (eq_attr "cpu" "znver1")
  (and (eq_attr "mode" "TI")
-  (and (eq_attr "type" "ssemul")
+  (and (eq_attr "type" "sseimul")
(eq_attr "memory" "load"
 "znver1-direct,znver1-load,znver1-fp0*3")

 (define_insn_reservation "znver1_sseimul_avx256_load" 8
 (and (eq_attr "cpu" "znver1")
  (and (eq_attr "mode" "OI")
-  (and (eq_attr "type" "ssemul")
+  (and (eq_attr "type" "sseimul")
(eq_attr "memory" "load"
 "znver1-double,znver1-load,znver1-fp0*4")

@@ -942,13 +942,13 @@
 (and (eq_attr "cpu" "znver1")
  (and (eq_attr "mode" "DI")
   (and (eq_attr "memory" "none")
-   (eq_attr "type" "ssemul"
+   (eq_attr "type" "sseimul"
 "znver1-direct,znver1-fp0*4")
(Snip)

Changelog 

2016-03-03  Venkataramanan Kumar  

   Fix sseimul type attribute.
   * config/i386/znver1.md 
   (znver1_sseimul, znver1_sseimul_avx256, znver1_sseimul_load,
   znver1_sseimul_avx256_load,  znver1_sseimul_di,
   znver1_sseimul_load_di) : Fix the type attribute.

Ok for trunk if bootstrap and testing passes?

Regards,
Venkat.

Re: [PATCH]Replace -shared with -r -nostdlib in gcc.dg/lto/pr61526 pr54709 pr64415 test cases.

[Renlin Li]

Hi Richard,

On 03/03/16 12:47, Richard Biener wrote:

On Thu, Mar 3, 2016 at 1:07 PM, Renlin Li  wrote:

Hi Richard,


On 03/03/16 10:13, Richard Biener wrote:

On Wed, Mar 2, 2016 at 5:12 PM, Renlin Li  wrote:

Hi Richard,


On 02/03/16 13:35, Richard Biener wrote:

On Tue, Mar 1, 2016 at 4:56 PM, Renlin Li 
wrote:

Hi Richard,


On 01/03/16 09:16, Richard Biener wrote:

On Mon, Feb 29, 2016 at 5:13 PM, Renlin Li 
wrote:

Hi all,

The gcc.dg/lto/pr54709, pr61526, pr64415 linking testcases keep
failing
on
arm/aarch64 bare-metal target.

It's because statically built newlib library is used to link with
shared
object.
And the linker complains about relocations which cannot be used in
shared object.

For example, the following errors are produced:

crtbegin.o: relocation R_ARM_MOVW_ABS_NC against `a local symbol' can
not
be
used when making a shared object; recompile with -fPIC

crtbegin.o: relocation R_ARM_THM_MOVW_ABS_NC against `a local symbol'
can
not
be used when making a shared object; recompile with -fPIC

librdimon.a(rdimon-syscalls.o): relocation R_AARCH64_ADR_PREL_PG_HI21
against
external symbol `_impure_ptr' can not be used when making a shared
object;
recompile with -fPIC

Presumably, bare-metal toolchain for other architecture have those
test
case
failures as well?

In this patch, -shared option is replace by -r -nostdlib. So that the
standard
system startup files or libraries are not used when linking.

Note that -shared is not equivalent to -r -nostdlib so please verify
that
the
original issue can be still reproduced with its fix reverted but -r
-nostdlib
used with the new -r -nostdlib handling on trunk.


pr54709_0.c: Cannot be reproduced with even -shared. The error message
is
the same as shown above.
pr64415_0.c: Reproduced with "-r -nostdlib".
pr61526_0.c: Reproduced with "-r -nostdlib".

By the way, those linking test cases all pass for linux toolchain. Only
fail
for aarch64/arm baremetal toolchain.

Andrew, I saw you have done similar things in r153555
https://gcc.gnu.org/viewcvs/gcc?view=revision=153555

Do you have any thoughts?

And also here, the last comments in this ticket suggests to add
check_effective_target_shared to the exp file to limit it to linux
targets
only:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61526

As said LTO testcases tend to be somewhat fragile so limiting them to
targets known to work might be the best option.

Richard.


Forgot the mention that, by purely adding "-nostdlib" option (instead of
replacing -shared)
fixes the failures as well.

I test those test cases again with fix reverted, keep "-shared" option,
add
"-nostdlib" option.

Ok, so I discovered we have a "shared" target which means if a target
doesn't
support shared libs we can guard against it with using

/* { dg-require-effective-target shared } */

does adding that to the three testcases fix the issue for you?

By adding this target check
/* { dg-require-effective-target shared } */

Those test cases aredeemed to be unsupported, and thus skipped for
aarch64-none-elf target.

However, it's a little bit tricky for arm bare-metal target.

The shared option check actually successes for arm-none-eabi toolchain.
This is because the default cpu for arm-none-eabi toolchain is arm7tdmi. And
the start file crtbegin.o doesn't contains any modifications not allowed in
shared object.

arm-none-eabi is built with multilib. When running this testcase, it's
compiled with "-march=armv7-a".
The crtbegin.o for this architecture version contains relocations which
cannot be used in shared object.
That's why they fails to linking test.

For -shared it should provide a crtbeginS.o then.  Why not fix it properly?

Richard.


That's the case for linux toolchain. Multiple versions of startfile are 
generated.

crtbegin.o, crtbeginS.o, crtbeginT.o etc.

If I understand it correctly, this is not applicable to bare-metal 
tool-chain?
Because, normally bare-metal toolchain will not be used to create shared 
object.


I have double checked, almost all bare-metal toolchain only requires 
crtbegin.o.

The targets define STARTFILE_SPEC in a simple way.

The failures here are complaining creating shared object including 
statically generated object.

The code in start files is not used or interact with the test cases.
So I think it's reasonable to use "-nostdlib" to exclude standard 
startup file or libraries when testing the linking.


Certainly, we can skip the test cases for bare-metal toolchain.
However, as explained above, it seems this support checker is not fully 
capable to do this.

/* { dg-require-effective-target shared } */

Regards,
Renlin






Will adding "-nostdlib" (instead of replace -shared) option be an reasonable
fix given my previous check?

Regards,
Renlin




Thanks,
Richard.


pr54709_0.c: Cannot be reproduced even with test case unmodified.
The error message is the same as shown above. with "-nostdlib", no

Re: IRA costs tweaks, PR 56069

[Bernd Schmidt]

On 03/02/2016 10:53 PM, Vladimir Makarov wrote:

2. update_costs_from_allocno records a cost update not just for the
initial allocno, but for each of the visited ones. I can sort of see
an argument for doing that (let's say if you assign an allocno in the
middle of a copy chain you'd want the tail end of the chain to be
reset), but in practice I don't think the present algorithm can work
at all. In the case of an allocno in the middle of a copy chain the
restore would progress in both directions, and in any case it looks
like this approach can end up double-counting things when restoring
costs.


It is just a heuristic.  Richard Sandiford proposed this update
approach.  Before it we had only updates of allocnos directly connected
to allocno in question.  Richard's approach helped to improve code in
some cases.  If something works better we should use.  The bechmarking
is the best criterium.


Ccing Richard in case he has comments.


The patch is ok for me.  But I'd wait for GCC7.  People are sensitive to
their code performance degradation.  Even in most cases the patch
improves performance, in some case it can worsen code and people might
fill new PRs after such change.

Bernd, thanks for working on it and providing a fresh view of the code.
For me especially valuable when people benchmark their patches.
Sometimes I have to do it by myself.


Ok, I'll wait. I did not actually run any benchmarks either, but I 
looked at generated code for a large number of examples. Based on that I 
suspect the effect would be too small to detect with benchmark runs.



Bernd

Re: Proposed Patch for Bug 69687

[Bernd Schmidt]

On 03/03/2016 04:18 PM, Mike Stump wrote:

On Mar 3, 2016, at 6:55 AM, Marcel Böhme  wrote:

I have revised the patch and removed the limits.


I looked at the patch, I can find no more unreasonable limits!  Wonderful.  
Hope someone will finish off the review and approve.


Marcel, if you haven't contributed before, we'll need a copyright 
assignment from you before we can accept the patch. Do you already have 
one on file?



Bernd

Re: [PATCH 2/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #1)

[Patrick Palka]

On Thu, Mar 3, 2016 at 10:21 AM, David Malcolm  wrote:
> Comment #1 of PR c/68187 identified another overzealous warning
> from -Wmisleading-indentation, with OpenSSL 1.0.1, on this poorly
> indented code:
>
> 115if (locked)
> 116i = CRYPTO_add(>struct_ref, -1, CRYPTO_LOCK_ENGINE);
> 117else
> 118i = --e->struct_ref;
> 119engine_ref_debug(e, 0, -1)
> 120if (i > 0)
> 121return 1;
>
> eng_lib.c:120:9: warning: statement is indented as if it were guarded by... 
> [-Wmisleading-indentation]
>  if (i > 0)
>  ^~
> eng_lib.c:117:5: note: ...this 'else' clause, but it is not
>  else
>  ^~~~
>
> Line 120 is poorly indented, but the warning is arguably unjustified.
>
> Root cause is that "engine_ref_debug" is actually a debugging macro
> that was empty in the given configuration, so the code effectively
> was:
>
> 117else  // GUARD
> 118i = --e->struct_ref;  // BODY
> 119
> 120if (i > 0)// NEXT
>
> hence the warning.
>
> But the code as seen by a human is clearly *not* misleading, and
> hence arguably we shouldn't warn for this case.
>
> The following patch fixes this by ruling that if there is non-whitespace
> in a line between the BODY and the NEXT statements, and that this
> non-whitespace is effectively an "unindent" or "outdent" (it's not clear
> to me which of these terms is better), then to not issue a warning.

Cool, this also fixes the false-positives seen in bdwgc, whose coding
style suggests indenting things inside an #ifdef as if it were an
if(), e.g.:

if (a)
  foo ();
#   ifndef A
  bar ();
#   endif
...

Re: [committed] Fix libffi/70024

[Richard Henderson]

On 03/03/2016 01:55 AM, Richard Biener wrote:

Thanks for doing this.  I suppose this is also upstream now?


I'm re-syncing with upstream now.  I did just find that upstream's soname is 
already 6.4.0, so I may come back and bump both sonames to 7 instead of 5.



r~

Re: [committed] Fix libffi/70024

[Richard Henderson]

On 03/03/2016 05:35 AM, Rainer Orth wrote:

Hi Richard,


Unfortunately, even with this fixed, all Solaris/x86 tests now fail to
link:

FAIL: libffi.call/closure_fn0.c -W -Wall -Wno-psabi -O0 (test for excess errors)
Excess errors:
Undefined   first referenced
  symbol in file
ffi_closure_alloc   /var/tmp//ccIpq3qc.o
ffi_type_float  /var/tmp//ccIpq3qc.o
ffi_type_uint64 /var/tmp//ccIpq3qc.o
ffi_type_sint32 /var/tmp//ccIpq3qc.o
ffi_type_sint16 /var/tmp//ccIpq3qc.o
ffi_type_double /var/tmp//ccIpq3qc.o
ffi_prep_cif/var/tmp//ccIpq3qc.o
ld: fatal: symbol referencing errors


turned out to be easy, too: make_sunver.pl got passed a list of
non-existant object files:

.libs/src/prep_cif.o .libs/src/types.o .libs/src/raw_api.o 
.libs/src/java_raw_api.o .libs/src/closures.o

Treating $(libffi_la_OBJECTS) and $(libffi_la_LIBADD) the same fixed
this so all the symbols above got included in libffi.map-sun and all
tests pass.

Tested on i386-pc-solaris2.12, installed on mainline.


Thanks.


r~

Re: [PATCH 1/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #0)

[Patrick Palka]

On Thu, Mar 3, 2016 at 10:56 AM, David Malcolm  wrote:
> On Thu, 2016-03-03 at 10:24 -0500, Patrick Palka wrote:
>> On Thu, Mar 3, 2016 at 10:21 AM, David Malcolm 
>> wrote:
>> > PR c/68187 covers two cases involving poor indentation where
>> > the indentation is arguably not misleading, but for which
>> > -Wmisleading-indentation emits a warning.
>> >
>> > The two cases appear to be different in nature; one in comment #0
>> > and the other in comment #1.  Richi marked the bug as a whole as
>> > a P1 regression; it's not clear to me if he meant one or both of
>> > these cases, so the following two patches fix both.
>> >
>> > The rest of this post addresses the case in comment #0 of the PR;
>> > the followup post addresses the other case, in comment #1 of the
>> > PR.
>> >
>> > Building glibc (a9224562cbe9cfb0bd8d9e637a06141141f9e6e3) on x86_64
>> > led to this diagnostic from -Wmisleading-indentation:
>> >
>> > ../stdlib/strtol_l.c: In function 'strtoul_l_internal':
>> > ../stdlib/strtol_l.c:356:9: error: statement is indented as if it
>> > were guarded by... [-Werror=misleading-indentation]
>> >  cnt < thousands_len; })
>> >  ^
>> > ../stdlib/strtol_l.c:353:9: note: ...this 'for' clause, but it is
>> > not
>> >&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)
>> >  ^
>> >
>> > The code is question looks like this:
>> >
>> >348for (c = *end; c != L_('\0'); c = *++end)
>> >349  if (((STRING_TYPE) c < L_('0') || (STRING_TYPE)
>> > c > L_('9'))
>> >350  # ifdef USE_WIDE_CHAR
>> >351  && (wchar_t) c != thousands
>> >352  # else
>> >353  && ({ for (cnt = 0; cnt < thousands_len;
>> > ++cnt)
>> >354if (thousands[cnt] != end[cnt])
>> >355  break;
>> >356cnt < thousands_len; })
>> >357  # endif
>> >358  && (!ISALPHA (c)
>> >359  || (int) (TOUPPER (c) - L_('A') + 10)
>> > >= base))
>> >360break;
>> >
>> > Lines 354 and 355 are poorly indented, leading to the warning from
>> > -Wmisleading-indentation at line 356.
>> >
>> > The wording of the warning is clearly wrong: line 356 isn't
>> > indented as if
>> > guarded by line 353, it's more that lines 354 and 355 *aren't*
>> > indented.
>> >
>> > What's happening is that should_warn_for_misleading_indentation has
>> > a
>> > heuristic for handling "} else", such as:
>> >
>> >  if (p)
>> >foo (1);
>> >  } else   // GUARD
>> >foo (2);   // BODY
>> >foo (3);   // NEXT
>> >
>> > and this heuristic uses the first non-whitespace character in the
>> > line
>> > containing GUARD as the column of interest: the "}" character.
>> >
>> > In this case we have:
>> >
>> >353&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)  //
>> > GUARD
>> >354  if (thousands[cnt] != end[cnt])//
>> > BODY
>> >355break;
>> >356  cnt < thousands_len; })//
>> > NEXT
>> >
>> > and so it uses the column of the "&&", and treats it as if it were
>> > indented thus:
>> >
>> >353for (cnt = 0; cnt < thousands_len; ++cnt)//
>> > GUARD
>> >354  if (thousands[cnt] != end[cnt])//
>> > BODY
>> >355break;
>> >356  cnt < thousands_len; })//
>> > NEXT
>> >
>> > and thus issues a warning.
>> >
>> > As far as I can tell the heuristic in question only makes sense for
>> > "else" clauses, so the following patch updates it to only use the
>> > special column when handling "else" clauses, eliminating the
>> > overzealous warning.
>>
>> Wouldn't this change have the undesirable effect of no longer warning
>> about:
>>
>>   if (p)
>> foo (1);
>>   } else if (q)
>> foo (2);
>> foo (3);
>
> No, because the rejection based on indentation is done relative to
>  guard_line_first_nws, rather than guard_vis_column (I tried doing it
> via the latter in one version of the patch, and that broke some of the
> existing cases, so I didn't make that change).

I see. That clears things up for me, thanks.

>
> See the attached test file (which doesn't have dg-directives yet); the
> example you give is test1_d, with an open-brace added to the "if (p)".
>
> Trunk emits warnings for:
>   * test1_c
>   * test1_d
>   * test1_e
>   * test1_f (two warnings, one for the "if", one for the "else")
>   * test1_g
>   * test2_c
>   * test2_d
>   * test2_e
>
> With the patches, it emits warnings for:
>   * test1_c
>   * test1_d
>   * test1_e
>   * test1_f (just one warnings, for the "if")
>   * test1_g
>   * test2_c
>   * test2_d
>   * test2_e
>
> so the only change is the removal of the erroneous double warning for
> the "else" in test1_f.

Nice!

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Jakub Jelinek]

On Thu, Mar 03, 2016 at 09:24:36AM -0700, Jeff Law wrote:
> >2016-03-03  Marek Polacek  
> >
> > PR c/69798
> > * c-parser.c (c_parser_postfix_expression): Call
> > c_parser_cast_expression instead of c_parser_postfix_expression.
> >
> > * gcc.dg/cilk-plus/pr69798-1.c: New test.
> > * gcc.dg/cilk-plus/pr69798-2.c: New test.
> I'd wait for gcc-7.  There's actually further Cilk+ fixes queued up from
> Ryan.   I wanted to get those into gcc-6, but just flat ran out of time.
> Perhaps ask for an exception to address the queued up Cilk+ stuff in a minor
> release?

Well, this one looks fairly safe to me even for gcc-6, and has the additional 
benefit that
it affects solely Cilk+ and nothing else.  Furthermore, I believe the
difference between cast and postfix expression is just in what is invalid
for Cilk+, so it shouldn't affect valid Cilk+ code, only invalid.

Jakub

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Marek Polacek]

On Thu, Mar 03, 2016 at 09:24:36AM -0700, Jeff Law wrote:
> I'd wait for gcc-7.  There's actually further Cilk+ fixes queued up from
> Ryan.   I wanted to get those into gcc-6, but just flat ran out of time.
> Perhaps ask for an exception to address the queued up Cilk+ stuff in a minor
> release?

Works for me.

Marek

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Jeff Law]

On 03/03/2016 09:15 AM, Marek Polacek wrote:

On Thu, Mar 03, 2016 at 03:28:01PM +0100, Jakub Jelinek wrote:

On Thu, Mar 03, 2016 at 03:15:41PM +0100, Marek Polacek wrote:

This is ICE on invalid Cilk+ code.  cilk_spawn expects a function call, so e.g.
_Cilk_spawn (void) is invalid.  The function call after the cilk_spawn keyword
is parsed using recursive call in c_parser_postfix_expression (case
RID_CILK_SPAWN).  Now, c_parser_postfix_expression sees '(' followed by a
typename, so it thinks we're inside a compound literal, which means it expects
'{', but that isn't there, so we crash on the assert in c_parser_braced_init:
gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
But as the comment in c_parser_postfix_expression says, the code for parsing
a compound literal here is likely dead.  I made an experiment and added
gcc_unreachable () in that block, ran regtest, and there were no failures.
Thus it should be safe to just remove the code, which also fixes this ICE; with
the patch we just give a proper error and don't crash anymore.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'm actually slightly
nervous about the change, so maybe better table until gcc7?


This reminds me of PR67495.  Perhaps the answer here is also during the
_Cilk* stuff parsing don't call c_parser_postfix_expression, but call
c_parser_cast_expression instead and then verify what it got?


Alternatively this one works as well.  I don't know if any verification of the
result should be done (in the second call, the first one is invalid anyway).

2016-03-03  Marek Polacek  

PR c/69798
* c-parser.c (c_parser_postfix_expression): Call
c_parser_cast_expression instead of c_parser_postfix_expression.

* gcc.dg/cilk-plus/pr69798-1.c: New test.
* gcc.dg/cilk-plus/pr69798-2.c: New test.
I'd wait for gcc-7.  There's actually further Cilk+ fixes queued up from 
Ryan.   I wanted to get those into gcc-6, but just flat ran out of time. 
Perhaps ask for an exception to address the queued up Cilk+ stuff in a 
minor release?


jeff

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Marek Polacek]

On Thu, Mar 03, 2016 at 03:28:01PM +0100, Jakub Jelinek wrote:
> On Thu, Mar 03, 2016 at 03:15:41PM +0100, Marek Polacek wrote:
> > This is ICE on invalid Cilk+ code.  cilk_spawn expects a function call, so 
> > e.g.
> > _Cilk_spawn (void) is invalid.  The function call after the cilk_spawn 
> > keyword
> > is parsed using recursive call in c_parser_postfix_expression (case
> > RID_CILK_SPAWN).  Now, c_parser_postfix_expression sees '(' followed by a
> > typename, so it thinks we're inside a compound literal, which means it 
> > expects
> > '{', but that isn't there, so we crash on the assert in 
> > c_parser_braced_init:
> > gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
> > But as the comment in c_parser_postfix_expression says, the code for parsing
> > a compound literal here is likely dead.  I made an experiment and added
> > gcc_unreachable () in that block, ran regtest, and there were no failures.
> > Thus it should be safe to just remove the code, which also fixes this ICE; 
> > with
> > the patch we just give a proper error and don't crash anymore.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'm actually slightly
> > nervous about the change, so maybe better table until gcc7?
> 
> This reminds me of PR67495.  Perhaps the answer here is also during the
> _Cilk* stuff parsing don't call c_parser_postfix_expression, but call
> c_parser_cast_expression instead and then verify what it got?

Alternatively this one works as well.  I don't know if any verification of the
result should be done (in the second call, the first one is invalid anyway).

2016-03-03  Marek Polacek  

PR c/69798
* c-parser.c (c_parser_postfix_expression): Call
c_parser_cast_expression instead of c_parser_postfix_expression.

* gcc.dg/cilk-plus/pr69798-1.c: New test.
* gcc.dg/cilk-plus/pr69798-2.c: New test.

diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index bb508b7..ce00457 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -8024,8 +8024,8 @@ c_parser_postfix_expression (c_parser *parser)
{
  error_at (loc, "-fcilkplus must be enabled to use "
"%<_Cilk_spawn%>");
- expr = c_parser_postfix_expression (parser);
- expr.value = error_mark_node;   
+ expr = c_parser_cast_expression (parser, NULL);
+ expr.value = error_mark_node;
}
  else if (c_parser_peek_token (parser)->keyword == RID_CILK_SPAWN)
{
@@ -8038,7 +8038,7 @@ c_parser_postfix_expression (c_parser *parser)
}
  else
{
- expr = c_parser_postfix_expression (parser);
+ expr = c_parser_cast_expression (parser, NULL);
  expr.value = build_cilk_spawn (loc, expr.value);
}
  break; 
diff --git gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c 
gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
index e69de29..1120193 100644
--- gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
+++ gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
@@ -0,0 +1,12 @@
+/* PR c/69798 */
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+
+int
+main ()
+{
+  _Cilk_spawn (void); /* { dg-error "expected expression" } */
+  _Cilk_spawn (char []); /* { dg-error "expected expression" } */
+  _Cilk_spawn (int *); /* { dg-error "expected expression" } */
+  _Cilk_spawn ({}); /* { dg-error "only function calls can be spawned" } */
+}
diff --git gcc/testsuite/gcc.dg/cilk-plus/pr69798-2.c 
gcc/testsuite/gcc.dg/cilk-plus/pr69798-2.c
index e69de29..66bcdc8 100644
--- gcc/testsuite/gcc.dg/cilk-plus/pr69798-2.c
+++ gcc/testsuite/gcc.dg/cilk-plus/pr69798-2.c
@@ -0,0 +1,11 @@
+/* PR c/69798 */
+/* { dg-do compile } */
+
+int
+main ()
+{
+  _Cilk_spawn (void); /* { dg-error "expected expression" } */
+  _Cilk_spawn (char []); /* { dg-error "expected expression" } */
+  _Cilk_spawn (int *); /* { dg-error "expected expression" } */
+  _Cilk_spawn ({}); /* { dg-error "only function calls can be spawned" } */
+}

Marek

Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level

[Manuel López-Ibáñez]

On 3 March 2016 at 15:39, Patrick Palka  wrote:
> On Thu, Mar 3, 2016 at 10:22 AM, Manuel López-Ibáñez
>  wrote:
>> It would be an overall improvement if it was neither a TREE_LIST, nor a
>> TREE_VECTOR: https://gcc.gnu.org/wiki/Speedup_areas#Trees
>>
>> Those kinds of cleanups are always welcome even if they do not improve
>> performance noticeably at first glance. The speed-up will show up once
>> TREE_LIST is removed completely.
>
> Ah yeah, I meant if cp_binding_level::names were a vec
> since it would have to be resizable.

Sure, what I meant is that such a change is an improvement even if you
cannot measure any speed-up at all right now. Go for it!

Cheers,

Manuel.

[Patch, fortran] PR69834 - Collision in derived type hashes

[Paul Richard Thomas]

Dear All,

What started out as a provisional kludge, when first working on OOP,
has come back to bite us after 7 years. A collision in derived type
has values has been reported on clf. In principle, as pointed out in
the clf thread, this could mean that existing code might be quietly
confusing dynamic types. Fortunately, this is unlikely because the
error in SELECT TYPE that flagged up this problem might appear or
incorrect fields might be accessed, giving rise to runtime errors.

The fix uses a new vtable field, '_name' that is loaded with the
value, "typename_scopename", which is used for the cases in SELECT
TYPE and for comparison in SAME_TYPE_AS. I have retained the '_hash'
field for compatibility with existing libraries. It could easily be
removed, if that is preferred, but would require a publicity campaign
to ensure that users recompile their code.

The changes are sufficiently well described in the ChangeLogs and the
comments in the patch to not warrant further comment.

I have to confess to not knowing quite what to propose here. My gut
feeling is that we should bite the bullet and the patch should be
applied to trunk and 5-branch. However, I am open, on the grounds
above, to wait until 7.0.0. It does bootstrap and regtest on trunk
with FC23/x86_64.

Thanks to Dominique for testing an early version of the test and to
Thomas for picking up on the clf thread.

Regards

Paul

2016-03-03  Paul Thomas  

PR fortran/69834
* class.c (gfc_select_type_name): New function.
(gfc_find_derived_vtab, find_intrinsic_vtab): Add a new field
to the vtable '_name'. Initialize using gfc_select_type_name.
* expr.c : Clean up some trailing white space.
* gfortran.h : Define 'gfc_add_name_component' and provide
prototype for 'gfc_select_type_name'.
* module.c (mio_component): Deal with the initializer for the
'_name' field.
* resolve.c (resolve_select_type): Use the name generated by
'gfc_select_type_name' instead of the hash for the case labels.
* trans-expr.c : Generate the access functions for the vtable
'_name' field.
* trans-intrinsic.c (gfc_conv_same_type_as): Rework to use the
vtable '_name' field or, for derived types, the name produced
by 'gfc_select_type_name' for comparison, instead of the hash.

2016-03-03  Paul Thomas  

PR fortran/69834
* gfortran.dg/finalize_21.f90 : Remove the right brace in the
test for the tree dump to allow for the new field.
* gfortran.dg/select_type_35.f90 : New test.


-- 
The difference between genius and stupidity is; genius has its limits.

Albert Einstein
Index: gcc/fortran/class.c
===
*** gcc/fortran/class.c (revision 233626)
--- gcc/fortran/class.c (working copy)
*** gfc_intrinsic_hash_value (gfc_typespec *
*** 552,557 
--- 552,589 
return (hash % 1);
  }
  
+ /* Provide a full name for any arbitrary type that can be used in
+SELECT TYPE and the SAME_TYPE_AS intrinsic. This is loaded into the
+vtable '_name' field and is used for the case label in SELECT TYPE
+and for derived types in SAME_TYPE_AS. Unlike get_unique_type_string
+the derived type name is put before the scope name on the grounds
+that this will, most of the time, make distinguishing the names more
+efficient.  */
+ void
+ gfc_select_type_name (char *name, gfc_typespec *ts, gfc_symbol *type)
+ {
+   if (ts != NULL && (ts->type == BT_DERIVED || ts->type == BT_CLASS))
+ type = ts->u.derived;
+   else if (!type)
+ {
+   sprintf (name, "%s_%d", gfc_basic_typename (ts->type), ts->kind);
+   return;
+ }
+   gcc_assert (type);
+ 
+   if (type->attr.unlimited_polymorphic)
+ {
+   sprintf (name, "STAR");
+   return;
+ }
+ 
+   if (type->module)
+ sprintf (name, "%s_%s", type->name, type->module);
+   else if (type->ns->proc_name)
+ sprintf (name, "%s_%s", type->name, type->ns->proc_name->name);
+   else
+ sprintf (name, "%s", type->name);
+ }
  
  /* Get the _len component from a class/derived object storing a string.
 For unlimited polymorphic entities a ref to the _data component is 
available
*** gfc_find_derived_vtab (gfc_symbol *deriv
*** 2203,2208 
--- 2235,2241 
if (ns)
  {
char name[GFC_MAX_SYMBOL_LEN+1], tname[GFC_MAX_SYMBOL_LEN+1];
+   char *cname;
  
get_unique_hashed_string (tname, derived);
sprintf (name, "__vtab_%s", tname);
*** gfc_find_derived_vtab (gfc_symbol *deriv
*** 2405,2410 
--- 2438,2458 
  c->tb->ppc = 1;
  generate_finalization_wrapper (derived, ns, tname, c);
  
+ if (!gfc_add_component (vtype, "_name", ))
+   goto cleanup;
+ c->ts.type = BT_CHARACTER;
+ c->ts.kind = gfc_default_character_kind;
+ c->attr.access = ACCESS_PRIVATE;
+ 

Re: [PATCH 1/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #0)

[David Malcolm]

On Thu, 2016-03-03 at 10:24 -0500, Patrick Palka wrote:
> On Thu, Mar 3, 2016 at 10:21 AM, David Malcolm 
> wrote:
> > PR c/68187 covers two cases involving poor indentation where
> > the indentation is arguably not misleading, but for which
> > -Wmisleading-indentation emits a warning.
> > 
> > The two cases appear to be different in nature; one in comment #0
> > and the other in comment #1.  Richi marked the bug as a whole as
> > a P1 regression; it's not clear to me if he meant one or both of
> > these cases, so the following two patches fix both.
> > 
> > The rest of this post addresses the case in comment #0 of the PR;
> > the followup post addresses the other case, in comment #1 of the
> > PR.
> > 
> > Building glibc (a9224562cbe9cfb0bd8d9e637a06141141f9e6e3) on x86_64
> > led to this diagnostic from -Wmisleading-indentation:
> > 
> > ../stdlib/strtol_l.c: In function 'strtoul_l_internal':
> > ../stdlib/strtol_l.c:356:9: error: statement is indented as if it
> > were guarded by... [-Werror=misleading-indentation]
> >  cnt < thousands_len; })
> >  ^
> > ../stdlib/strtol_l.c:353:9: note: ...this 'for' clause, but it is
> > not
> >&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)
> >  ^
> > 
> > The code is question looks like this:
> > 
> >348for (c = *end; c != L_('\0'); c = *++end)
> >349  if (((STRING_TYPE) c < L_('0') || (STRING_TYPE)
> > c > L_('9'))
> >350  # ifdef USE_WIDE_CHAR
> >351  && (wchar_t) c != thousands
> >352  # else
> >353  && ({ for (cnt = 0; cnt < thousands_len;
> > ++cnt)
> >354if (thousands[cnt] != end[cnt])
> >355  break;
> >356cnt < thousands_len; })
> >357  # endif
> >358  && (!ISALPHA (c)
> >359  || (int) (TOUPPER (c) - L_('A') + 10)
> > >= base))
> >360break;
> > 
> > Lines 354 and 355 are poorly indented, leading to the warning from
> > -Wmisleading-indentation at line 356.
> > 
> > The wording of the warning is clearly wrong: line 356 isn't
> > indented as if
> > guarded by line 353, it's more that lines 354 and 355 *aren't*
> > indented.
> > 
> > What's happening is that should_warn_for_misleading_indentation has
> > a
> > heuristic for handling "} else", such as:
> > 
> >  if (p)
> >foo (1);
> >  } else   // GUARD
> >foo (2);   // BODY
> >foo (3);   // NEXT
> > 
> > and this heuristic uses the first non-whitespace character in the
> > line
> > containing GUARD as the column of interest: the "}" character.
> > 
> > In this case we have:
> > 
> >353&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)  //
> > GUARD
> >354  if (thousands[cnt] != end[cnt])//
> > BODY
> >355break;
> >356  cnt < thousands_len; })//
> > NEXT
> > 
> > and so it uses the column of the "&&", and treats it as if it were
> > indented thus:
> > 
> >353for (cnt = 0; cnt < thousands_len; ++cnt)//
> > GUARD
> >354  if (thousands[cnt] != end[cnt])//
> > BODY
> >355break;
> >356  cnt < thousands_len; })//
> > NEXT
> > 
> > and thus issues a warning.
> > 
> > As far as I can tell the heuristic in question only makes sense for
> > "else" clauses, so the following patch updates it to only use the
> > special column when handling "else" clauses, eliminating the
> > overzealous warning.
> 
> Wouldn't this change have the undesirable effect of no longer warning
> about:
> 
>   if (p)
> foo (1);
>   } else if (q)
> foo (2);
> foo (3);

No, because the rejection based on indentation is done relative to
 guard_line_first_nws, rather than guard_vis_column (I tried doing it
via the latter in one version of the patch, and that broke some of the
existing cases, so I didn't make that change).

See the attached test file (which doesn't have dg-directives yet); the
example you give is test1_d, with an open-brace added to the "if (p)".

Trunk emits warnings for:
  * test1_c
  * test1_d
  * test1_e
  * test1_f (two warnings, one for the "if", one for the "else")
  * test1_g
  * test2_c
  * test2_d
  * test2_e

With the patches, it emits warnings for:
  * test1_c
  * test1_d
  * test1_e
  * test1_f (just one warnings, for the "if")
  * test1_g
  * test2_c
  * test2_d
  * test2_e

so the only change is the removal of the erroneous double warning for
the "else" in test1_f.

I can add dg-directives and add the attachment to Wmisleading
-indentation.c as part of the patch (or keep it as a separate new test
file, the former is getting large)./* PR c/68187.  */
/* { dg-options "-Wmisleading-indentation" } */
/* { dg-do compile } */

extern int foo (int);
extern int bar (int, int);
extern int 

Re: Proposed Patch for Bug 69687

[Manuel López-Ibáñez]

On 03/03/16 14:21, Bernd Schmidt wrote:

On 03/02/2016 06:22 PM, Mike Stump wrote:


So, check for overflow, or better use unsigned values that are large
enough to never overflow.  With no possibility for overflow, you can
then retest the bug and see if there are any other failure modes and
fix those.


What C standard can we assume for libiberty? I was looking@patching this and
discovered that SIZE_MAX is defined only for C99, so I'm leaning towards
retaining the ints and using INT_MAX.


Retaining INT_MAX should be ok in this case, since that should allow pretty 
large mangled strings. As far as I know, the only users of libiberty are GDB 
and GCC, and GDB only because they have not completely moved to gnulib yet. GCC 
is C++, GDB assumes C90 but it is moving to C++ anyway, so it could be bumped 
to SIZE_MAX later.


However, it would be much better to add to libiberty something like gnulib's 
x2realloc and x2nrealloc and use that because:


* It is more concise.
* Avoid duplication.
* libiberty should be replaced by gnulib eventually
* error-handling is shared with xrealloc, which gives both more consistency and 
more flexibility.


Of course, there is an even better fix: Add to the GCC repository enough gnulib 
modules to use directly the x2realloc from gnulib, make the demangler use that. 
GDB is already using some gnulib modules, so it should not be a problem for 
them. It is a bit more work in the short term, but re-implementing function by 
function a lower quality implementation of the whole gnulib seems much worse in 
the long run.


Cheers,

Manuel.

Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level

[Patrick Palka]

On Thu, Mar 3, 2016 at 10:22 AM, Manuel López-Ibáñez
 wrote:
> On 03/03/16 14:49, Patrick Palka wrote:
>>
>> I think the slowness of this function is mostly due to the pointer
>> chasing performed in the function store_bindings, where we iterate
>> over all the names in each non-global scope to figure out whether to
>> preserve them.  It would probably improve performance if
>> cp_binding_level::names were a vector of trees instead of a linked
>> list of trees.
>
>
> It would be an overall improvement if it was neither a TREE_LIST, nor a
> TREE_VECTOR: https://gcc.gnu.org/wiki/Speedup_areas#Trees
>
> Those kinds of cleanups are always welcome even if they do not improve
> performance noticeably at first glance. The speed-up will show up once
> TREE_LIST is removed completely.

Ah yeah, I meant if cp_binding_level::names were a vec
since it would have to be resizable.

>
> Cheers,
>
> Manuel.
>
>

Re: [patch] libstdc++/69945 Add __gnu_cxx::__freeres hook

[Mark Wielaard]

On Wed, 2016-02-24 at 18:35 +, Jonathan Wakely wrote:
> This adds a new function to libsupc++ which will free the memory still
> in use by the pool used for allocating exceptions when malloc fails.
> 
> This is similar to glibc's __libc_freeres, which valgrind (and other
> tools?) use to tell glibc to deallocate everything before exiting.
> 
> I initially called it __gnu_cxx::__free_eh_pool() but I figured we
> might have other memory in use at some later date, and we wouldn't
> want valgrind to have to start calling a second function, nor make a
> function called __free_eh_pool() actually free other things.

I tested this on x86_64-pc-linux-gnu with Ivo's valgrind patch from
https://bugs.kde.org/show_bug.cgi?id=345307 and it works pretty nicely.
No more spurious still reachable memory issues with memcheck.

Is there any possibility to get this backported for 5.4?

Thanks,

Mark

Re: [PATCH, ARM] Fix gcc.c-torture/execute/loop-2b.c execution failure on cortex-m0

[Thomas Preudhomme]

On Thursday 03 March 2016 09:44:31 Ramana Radhakrishnan wrote:
> On Thu, Mar 3, 2016 at 9:40 AM, Thomas Preudhomme
> 
>  wrote:
> > On Friday 15 January 2016 12:45:04 Ramana Radhakrishnan wrote:
> >> On Wed, Dec 16, 2015 at 9:11 AM, Thomas Preud'homme
> >> 
> >>  wrote:
> >> > During reorg pass, thumb1_reorg () is tasked with rewriting mov rd, rn
> >> > to
> >> > subs rd, rn, 0 to avoid a comparison against 0 instruction before doing
> >> > a
> >> > conditional branch based on it. The actual avoiding of cmp is done in
> >> > cbranchsi4_insn instruction C output template. When the condition is
> >> > met,
> >> > the source register (rn) is also propagated into the comparison in
> >> > place
> >> > the destination register (rd).
> >> > 
> >> > However, right now thumb1_reorg () only look for a mov followed by a
> >> > cbranchsi but does not check whether the comparison in cbranchsi is
> >> > against the constant 0. This is not safe because a non clobbering
> >> > instruction could exist between the mov and the comparison that
> >> > modifies
> >> > the source register. This is what happens here with a post increment of
> >> > the source register after the mov, which skip the [i] == [1]
> >> > comparison for iteration i == 1.
> >> > 
> >> > This patch fixes the issue by checking that the comparison is against
> >> > constant 0.
> >> > 
> >> > ChangeLog entry is as follow:
> >> > 
> >> > 
> >> > *** gcc/ChangeLog ***
> >> > 
> >> > 2015-12-07  Thomas Preud'homme  
> >> > 
> >> > * config/arm/arm.c (thumb1_reorg): Check that the comparison is
> >> > against the constant 0.
> >> 
> >> OK.
> >> 
> >> Ramana
> >> 
> >> > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> >> > index 42bf272..49c0a06 100644
> >> > --- a/gcc/config/arm/arm.c
> >> > +++ b/gcc/config/arm/arm.c
> >> > @@ -17195,7 +17195,7 @@ thumb1_reorg (void)
> >> > 
> >> >FOR_EACH_BB_FN (bb, cfun)
> >> >
> >> >  {
> >> >  
> >> >rtx dest, src;
> >> > 
> >> > -  rtx pat, op0, set = NULL;
> >> > +  rtx cmp, op0, op1, set = NULL;
> >> > 
> >> >rtx_insn *prev, *insn = BB_END (bb);
> >> >bool insn_clobbered = false;
> >> > 
> >> > @@ -17208,8 +17208,13 @@ thumb1_reorg (void)
> >> > 
> >> > continue;
> >> >
> >> >/* Get the register with which we are comparing.  */
> >> > 
> >> > -  pat = PATTERN (insn);
> >> > -  op0 = XEXP (XEXP (SET_SRC (pat), 0), 0);
> >> > +  cmp = XEXP (SET_SRC (PATTERN (insn)), 0);
> >> > +  op0 = XEXP (cmp, 0);
> >> > +  op1 = XEXP (cmp, 1);
> >> > +
> >> > +  /* Check that comparison is against ZERO.  */
> >> > +  if (!CONST_INT_P (op1) || INTVAL (op1) != 0)
> >> > +   continue;
> >> > 
> >> >/* Find the first flag setting insn before INSN in basic block
> >> >BB.
> >> >*/
> >> >gcc_assert (insn != BB_HEAD (bb));
> >> > 
> >> > @@ -17249,7 +17254,7 @@ thumb1_reorg (void)
> >> > 
> >> >   PATTERN (prev) = gen_rtx_SET (dest, src);
> >> >   INSN_CODE (prev) = -1;
> >> >   /* Set test register in INSN to dest.  */
> >> > 
> >> > - XEXP (XEXP (SET_SRC (pat), 0), 0) = copy_rtx (dest);
> >> > + XEXP (cmp, 0) = copy_rtx (dest);
> >> > 
> >> >   INSN_CODE (insn) = -1;
> >> > 
> >> > }
> >> >  
> >> >  }
> >> > 
> >> > Testsuite shows no regression when run for arm-none-eabi with
> >> > -mcpu=cortex-m0 -mthumb
> > 
> > The patch applies cleanly on gcc-5-branch and also show no regression when
> > run for arm-none-eabi with -mcpu=cortex-m0 -mthumb. Is it ok to backport?
> This deserves a testcase.

The original patch don't have one initially because it fixes a fail of an 
existing testcase (loop-2b.c). However, the test pass on gcc 5 due to 
difference in code generation. I'm currently trying to come up with a testcase 
and will get back at you.

Best regards,

Thomas

Re: [PATCH 1/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #0)

[Patrick Palka]

On Thu, Mar 3, 2016 at 10:21 AM, David Malcolm  wrote:
> PR c/68187 covers two cases involving poor indentation where
> the indentation is arguably not misleading, but for which
> -Wmisleading-indentation emits a warning.
>
> The two cases appear to be different in nature; one in comment #0
> and the other in comment #1.  Richi marked the bug as a whole as
> a P1 regression; it's not clear to me if he meant one or both of
> these cases, so the following two patches fix both.
>
> The rest of this post addresses the case in comment #0 of the PR;
> the followup post addresses the other case, in comment #1 of the PR.
>
> Building glibc (a9224562cbe9cfb0bd8d9e637a06141141f9e6e3) on x86_64
> led to this diagnostic from -Wmisleading-indentation:
>
> ../stdlib/strtol_l.c: In function 'strtoul_l_internal':
> ../stdlib/strtol_l.c:356:9: error: statement is indented as if it were 
> guarded by... [-Werror=misleading-indentation]
>  cnt < thousands_len; })
>  ^
> ../stdlib/strtol_l.c:353:9: note: ...this 'for' clause, but it is not
>&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)
>  ^
>
> The code is question looks like this:
>
>348for (c = *end; c != L_('\0'); c = *++end)
>349  if (((STRING_TYPE) c < L_('0') || (STRING_TYPE) c > 
> L_('9'))
>350  # ifdef USE_WIDE_CHAR
>351  && (wchar_t) c != thousands
>352  # else
>353  && ({ for (cnt = 0; cnt < thousands_len; ++cnt)
>354if (thousands[cnt] != end[cnt])
>355  break;
>356cnt < thousands_len; })
>357  # endif
>358  && (!ISALPHA (c)
>359  || (int) (TOUPPER (c) - L_('A') + 10) >= base))
>360break;
>
> Lines 354 and 355 are poorly indented, leading to the warning from
> -Wmisleading-indentation at line 356.
>
> The wording of the warning is clearly wrong: line 356 isn't indented as if
> guarded by line 353, it's more that lines 354 and 355 *aren't* indented.
>
> What's happening is that should_warn_for_misleading_indentation has a
> heuristic for handling "} else", such as:
>
>  if (p)
>foo (1);
>  } else   // GUARD
>foo (2);   // BODY
>foo (3);   // NEXT
>
> and this heuristic uses the first non-whitespace character in the line
> containing GUARD as the column of interest: the "}" character.
>
> In this case we have:
>
>353&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)  // GUARD
>354  if (thousands[cnt] != end[cnt])// BODY
>355break;
>356  cnt < thousands_len; })// NEXT
>
> and so it uses the column of the "&&", and treats it as if it were
> indented thus:
>
>353for (cnt = 0; cnt < thousands_len; ++cnt)// GUARD
>354  if (thousands[cnt] != end[cnt])// BODY
>355break;
>356  cnt < thousands_len; })// NEXT
>
> and thus issues a warning.
>
> As far as I can tell the heuristic in question only makes sense for
> "else" clauses, so the following patch updates it to only use the
> special column when handling "else" clauses, eliminating the
> overzealous warning.

Wouldn't this change have the undesirable effect of no longer warning about:

  if (p)
foo (1);
  } else if (q)
foo (2);
foo (3);

Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level

[Manuel López-Ibáñez]

On 03/03/16 14:49, Patrick Palka wrote:

I think the slowness of this function is mostly due to the pointer
chasing performed in the function store_bindings, where we iterate
over all the names in each non-global scope to figure out whether to
preserve them.  It would probably improve performance if
cp_binding_level::names were a vector of trees instead of a linked
list of trees.


It would be an overall improvement if it was neither a TREE_LIST, nor a 
TREE_VECTOR: https://gcc.gnu.org/wiki/Speedup_areas#Trees


Those kinds of cleanups are always welcome even if they do not improve 
performance noticeably at first glance. The speed-up will show up once 
TREE_LIST is removed completely.


Cheers,

Manuel.

Re: Proposed Patch for Bug 69687

[Mike Stump]

On Mar 3, 2016, at 6:55 AM, Marcel Böhme  wrote:
> I have revised the patch and removed the limits.

I looked at the patch, I can find no more unreasonable limits!  Wonderful.  
Hope someone will finish off the review and approve.

Re: [PATCH] Fix vec_set_hi* patterns (PR target/70059)

[Kirill Yukhin]

Hi Jakub,
On 03 Mar 13:08, Jakub Jelinek wrote:
> routine has changed and looks good to me).  Can somebody test this please
> on real hw or emulator?
I'll run testing on the simulator.

--
Thanks, K

Re: Proposed Patch for Bug 69687

[Mike Stump]

On Mar 3, 2016, at 6:21 AM, Bernd Schmidt  wrote:
> What C standard can we assume for libiberty? I was looking at patching this 
> and discovered that SIZE_MAX is defined only for C99, so I'm leaning towards 
> retaining the ints and using INT_MAX.

As long as you don’t need a constant…  you can also do something like:

#ifndef SIZE_MAX
#define SIZE_MAX   (sizeof (size_t) == sizeof (int) ? INT_MAX : sizeof (size_t) 
== sizeof (long) ? LONG_MAX : (abort (), 0))
#endif

but, you need to consider the signedness of it.  A size bounded by int might be 
annoying if an int was 16 bits, but, we don’t care about such platforms hosting 
gcc, so, not a problem in reality.  Once we get to 32-biit (or more), we’re 
good.  No one better have a symbol >2 billion bytes.  And if they do, they can 
submit that patch to fix it in about 1000 years.  :-)  I think an INT_MAX only 
version is fine.

[PATCH 1/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #0)

[David Malcolm]

PR c/68187 covers two cases involving poor indentation where
the indentation is arguably not misleading, but for which
-Wmisleading-indentation emits a warning.

The two cases appear to be different in nature; one in comment #0
and the other in comment #1.  Richi marked the bug as a whole as
a P1 regression; it's not clear to me if he meant one or both of
these cases, so the following two patches fix both.

The rest of this post addresses the case in comment #0 of the PR;
the followup post addresses the other case, in comment #1 of the PR.

Building glibc (a9224562cbe9cfb0bd8d9e637a06141141f9e6e3) on x86_64
led to this diagnostic from -Wmisleading-indentation:

../stdlib/strtol_l.c: In function 'strtoul_l_internal':
../stdlib/strtol_l.c:356:9: error: statement is indented as if it were guarded 
by... [-Werror=misleading-indentation]
 cnt < thousands_len; })
 ^
../stdlib/strtol_l.c:353:9: note: ...this 'for' clause, but it is not
   && ({ for (cnt = 0; cnt < thousands_len; ++cnt)
 ^

The code is question looks like this:

   348for (c = *end; c != L_('\0'); c = *++end)
   349  if (((STRING_TYPE) c < L_('0') || (STRING_TYPE) c > L_('9'))
   350  # ifdef USE_WIDE_CHAR
   351  && (wchar_t) c != thousands
   352  # else
   353  && ({ for (cnt = 0; cnt < thousands_len; ++cnt)
   354if (thousands[cnt] != end[cnt])
   355  break;
   356cnt < thousands_len; })
   357  # endif
   358  && (!ISALPHA (c)
   359  || (int) (TOUPPER (c) - L_('A') + 10) >= base))
   360break;

Lines 354 and 355 are poorly indented, leading to the warning from
-Wmisleading-indentation at line 356.

The wording of the warning is clearly wrong: line 356 isn't indented as if
guarded by line 353, it's more that lines 354 and 355 *aren't* indented.

What's happening is that should_warn_for_misleading_indentation has a
heuristic for handling "} else", such as:

 if (p)
   foo (1);
 } else   // GUARD
   foo (2);   // BODY
   foo (3);   // NEXT

and this heuristic uses the first non-whitespace character in the line
containing GUARD as the column of interest: the "}" character.

In this case we have:

   353&& ({ for (cnt = 0; cnt < thousands_len; ++cnt)  // GUARD
   354  if (thousands[cnt] != end[cnt])// BODY
   355break;
   356  cnt < thousands_len; })// NEXT

and so it uses the column of the "&&", and treats it as if it were
indented thus:

   353for (cnt = 0; cnt < thousands_len; ++cnt)// GUARD
   354  if (thousands[cnt] != end[cnt])// BODY
   355break;
   356  cnt < thousands_len; })// NEXT

and thus issues a warning.

As far as I can tell the heuristic in question only makes sense for
"else" clauses, so the following patch updates it to only use the
special column when handling "else" clauses, eliminating the
overzealous warning.

Doing so led to a nonsensical warning for
libstdc++-v3/src/c++11/random.cc:random_device::_M_init:

random.cc: In member function ‘void std::random_device::_M_init(const string&)’:
random.cc:102:10: warning: this ‘if’ clause does not guard... 
[-Wmisleading-indentation]
 else if (token != "/dev/urandom" && token != "/dev/random")
  ^~
random.cc:107:5: note: ...this statement, but the latter is indented as if it 
does
 _M_file = static_cast(std::fopen(fname, "rb"));
 ^~~

so the patch addresses this by tweaking the heuristic that rejects
aligned BODY and NEXT so that it doesn't require them to be aligned with
the first non-whitespace of the GUARD, simply that they not be indented
relative to it.

Successfully bootstrapped on x86_64-pc-linux-gnu in
combination with the following patch; standalone bootstrap
is in progress.

OK for trunk if the latter is successful?

gcc/c-family/ChangeLog:
PR c/68187
* c-indentation.c (should_warn_for_misleading_indentation): When
suppressing warnings about cases where the guard and body are on
the same column, only use the first non-whitespace column in place
of the guard token column when dealing with "else" clauses.
When rejecting aligned BODY and NEXT, loosen the requirement
from equality with the first non-whitespace of guard to simply
that they not be indented relative to it.

gcc/testsuite/ChangeLog:
PR c/68187
* c-c++-common/Wmisleading-indentation.c (fn_40_a): New test
function.
(fn_40_b): Likewise.
(fn_41_a): Likewise.
(fn_41_b): Likewise.
---
 gcc/c-family/c-indentation.c   | 16 +++--
 .../c-c++-common/Wmisleading-indentation.c | 79 ++
 2 files changed, 89 insertions(+), 6 deletions(-)

diff 

[PATCH 2/2] PR c/68187: fix overzealous -Wmisleading-indentation (comment #1)

[David Malcolm]

Comment #1 of PR c/68187 identified another overzealous warning
from -Wmisleading-indentation, with OpenSSL 1.0.1, on this poorly
indented code:

115if (locked)
116i = CRYPTO_add(>struct_ref, -1, CRYPTO_LOCK_ENGINE);
117else
118i = --e->struct_ref;
119engine_ref_debug(e, 0, -1)
120if (i > 0)
121return 1;

eng_lib.c:120:9: warning: statement is indented as if it were guarded by... 
[-Wmisleading-indentation]
 if (i > 0)
 ^~
eng_lib.c:117:5: note: ...this 'else' clause, but it is not
 else
 ^~~~

Line 120 is poorly indented, but the warning is arguably unjustified.

Root cause is that "engine_ref_debug" is actually a debugging macro
that was empty in the given configuration, so the code effectively
was:

117else  // GUARD
118i = --e->struct_ref;  // BODY
119
120if (i > 0)// NEXT

hence the warning.

But the code as seen by a human is clearly *not* misleading, and
hence arguably we shouldn't warn for this case.

The following patch fixes this by ruling that if there is non-whitespace
in a line between the BODY and the NEXT statements, and that this
non-whitespace is effectively an "unindent" or "outdent" (it's not clear
to me which of these terms is better), then to not issue a warning.

In doing so I eliminated one of the existing heuristics: we already
had code to ignore preprocessor directives between BODY and NEXT for cases
like this:

  if (flagA)  // GUARD
foo (0);  // BODY
#if SOME_CONDITION_THAT_DOES_NOT_HOLD
  if (flagB)
#endif
foo (1);  // NEXT

This is handled by the new heuristic, so the new heuristic simply
replaces the old one.  Sadly the replacement of two old functions
with two new functions leads to a rather messy diff within
c-indentation.c; I can split it up into a removal/addition pair of
patches if that's easier to review.

Successfully bootstrapped on x86_64-pc-linux-gnu (in
combination with the previous patch).

OK for trunk?

Note: one of the new test cases adds a dg-warning/dg-message pair, and so
would require updating if/when the wording change posted here:
  https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00068.html
  ("[PATCH] PR c/69993: improvements to wording of -Wmisleading-indentation")
is applied.

gcc/c-family/ChangeLog:
PR c/68187
* c-indentation.c (get_visual_column): Move code to determine next
tab stop to...
(next_tab_stop): ...this new function.
(line_contains_hash_if): Delete function.
(detect_preprocessor_logic): Delete function.
(get_first_nws_vis_column): New function.
(detect_intervening_unindent): New function.
(should_warn_for_misleading_indentation): Replace call to
detect_preprocessor_logic with a call to
detect_intervening_unindent.

gcc/testsuite/ChangeLog:
PR c/68187
* c-c++-common/Wmisleading-indentation.c (fn_42_a): New test
function.
(fn_42_b): Likewise.
(fn_42_c): Likewise.
---
 gcc/c-family/c-indentation.c   | 141 -
 .../c-c++-common/Wmisleading-indentation.c |  72 +++
 2 files changed, 152 insertions(+), 61 deletions(-)

diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index c72192d..b84fbf4 100644
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -26,6 +26,16 @@ along with GCC; see the file COPYING3.  If not see
 
 extern cpp_options *cpp_opts;
 
+/* Round up VIS_COLUMN to nearest tab stop. */
+
+static unsigned int
+next_tab_stop (unsigned int vis_column)
+{
+  const unsigned int tab_width = cpp_opts->tabstop;
+  vis_column = ((vis_column + tab_width) / tab_width) * tab_width;
+  return vis_column;
+}
+
 /* Convert libcpp's notion of a column (a 1-based char count) to
the "visual column" (0-based column, respecting tabs), by reading the
relevant line.
@@ -77,11 +87,7 @@ get_visual_column (expanded_location exploc, location_t loc,
}
 
   if (ch == '\t')
-   {
-/* Round up to nearest tab stop. */
-const unsigned int tab_width = cpp_opts->tabstop;
-vis_column = ((vis_column + tab_width) / tab_width) * tab_width;
-   }
+   vis_column = next_tab_stop (vis_column);
   else
vis_column++;
 }
@@ -93,54 +99,49 @@ get_visual_column (expanded_location exploc, location_t loc,
   return true;
 }
 
-/* Does the given source line appear to contain a #if directive?
-   (or #ifdef/#ifndef).  Ignore the possibility of it being inside a
-   comment, for simplicity.
-   Helper function for detect_preprocessor_logic.  */
+/* Attempt to determine the first non-whitespace character in line LINE_NUM
+   of source line FILE.
+
+   If this is possible, return true and write its "visual column" to
+   *FIRST_NWS.
+   Otherwise, return false, leaving *FIRST_NWS untouched.  */
 
 static bool
-line_contains_hash_if (const char *file, int line_num)

Re: Proposed Patch for Bug 69687

[Marcel Böhme]

Thanks Mike. I have revised the patch and removed the limits.

While perhaps less security critical, without the limits on the loop count (r) 
the test cases will still consume all your memory and effectively freeze GDB.

* Before any realloc, check for overflow.
* string_need now returns 1 if the allocation was successful.
* all clients of string_need refrain from extending the string anything if 
string_need was unsuccessful.

>  better use unsigned values that are large enough to never overflow. 
Throughout cplus-dem.c, the length of a string is measured as pointer 
difference. So, technically length is of type ptrdiff_t which is signed.

— a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -55,6 +55,7 @@
 void * malloc ();
 void * realloc ();
 #endif
+#include 
 
 #include 
 #undef CURRENT_DEMANGLING_STYLE
@@ -379,7 +380,7 @@
 
 static int arm_special (const char **, string *);
 
-static void string_need (string *, int);
+static int string_need (string *, int);
 
 static void string_delete (string *);
 
@@ -4254,7 +4255,9 @@
}
   else
{
- work -> typevec_size *= 2;
+ if (work -> typevec_size > INT_MAX / 2)
+return;
+  work -> typevec_size *= 2;
  work -> typevec
= XRESIZEVEC (char *, work->typevec, work->typevec_size);
}
@@ -4281,7 +4284,9 @@
}
   else
{
- work -> ksize *= 2;
+ if (work -> ksize > INT_MAX / 2)
+return; 
+  work -> ksize *= 2;
  work -> ktypevec
= XRESIZEVEC (char *, work->ktypevec, work->ksize);
}
@@ -4294,7 +4299,8 @@
 
 /* Register a B code, and get an index for it. B codes are registered
as they are seen, rather than as they are completed, so map
-   registers map as B0, and temp as B1 */
+   registers map as B0, and temp as B1. Returns -1
+   if registration was unsuccessful. */
 
 static int
 register_Btype (struct work_stuff *work)
@@ -4310,7 +4316,9 @@
}
   else
{
- work -> bsize *= 2;
+ if (work -> bsize > INT_MAX / 2)
+return -1;
+  work -> bsize *= 2;
  work -> btypevec
= XRESIZEVEC (char *, work->btypevec, work->bsize);
}
@@ -4328,6 +4336,8 @@
 {
   char *tem;
 
+  if (index < 0)
+return;
   tem = XNEWVEC (char, len + 1);
   memcpy (tem, start, len);
   tem[len] = '\0';
@@ -4591,7 +4601,8 @@
   const char *tem;
 
   string_appendn (declp, (*mangled), scan - (*mangled));
-  string_need (declp, 1);
+  if (! string_need (declp, 1))
+ return 0;
   *(declp -> p) = '\0';
 
   /* Consume the function name, including the "__" separating the name
@@ -4747,7 +4758,7 @@
 
 /* a mini string-handling package */
 
-static void
+static int 
 string_need (string *s, int n)
 {
   int tem;
@@ -4765,11 +4776,14 @@
 {
   tem = s->p - s->b;
   n += tem;
+  if ( n > INT_MAX / 2) 
+return 0;
   n *= 2;
   s->b = XRESIZEVEC (char, s->b, n);
   s->p = s->b + tem;
   s->e = s->b + n;
 }
+return 1;
 }
 
 static void
@@ -4811,7 +4825,8 @@
   if (s == NULL || *s == '\0')
 return;
   n = strlen (s);
-  string_need (p, n);
+  if (! string_need (p, n))
+return;
   memcpy (p->p, s, n);
   p->p += n;
 }
@@ -4824,7 +4839,8 @@
   if (s->b != s->p)
 {
   n = s->p - s->b;
-  string_need (p, n);
+  if (! string_need (p, n))
+return;
   memcpy (p->p, s->b, n);
   p->p += n;
 }
@@ -4835,7 +4851,8 @@
 {
   if (n != 0)
 {
-  string_need (p, n);
+  if (! string_need (p, n))
+return;
   memcpy (p->p, s, n);
   p->p += n;
 }
@@ -4866,7 +4883,8 @@
 
   if (n != 0)
 {
-  string_need (p, n);
+  if (! string_need (p, n))
+return;
   for (q = p->p - 1; q >= p->b; q--)
{
  q[n] = q[0];

Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level

[Patrick Palka]

On Thu, Mar 3, 2016 at 9:22 AM, Markus Trippelsdorf
 wrote:
> On 2016.03.03 at 09:16 -0500, Patrick Palka wrote:
>> push_to_top_level gets called fairly frequently in template-heavy code
>> that performs a lot of instantiations, and we currently "leak" a lot of
>> GC memory when compiling such code since [push|pop]_to_top_level() do
>> not bother reusing or even freeing each saved_scope structure it
>> allocates.
>>
>> This patch makes push_to_top_level() reuse the saved_scope structures it
>> allocates.  This is similar to how begin_scope() reuses the
>> cp_binding_level structures it allocates.
>>
>> This patch reduces the maximum memory usage of the compiler by 4.5%,
>> from 525MB to 500MB, when compiling the Boost::Fusion test file
>> libs/fusion/test/compile_time/transform.cpp from the Boost 1.60 testsuite.
>>
>> Bootstrapped and tested on x86_64-pc-linux-gnu, OK for
>> trunk or for GCC 7?
>
> Great. push_to_top_level also shows up very high in profiles when
> building Chromium for example.
>
> There is an old bug for this issue:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64500
>
> --
> Markus

I forgot what exactly I was benchmarking but I also saw
push_to_top_level high on the list which is what made me interested in
this function in the first place.

I think the slowness of this function is mostly due to the pointer
chasing performed in the function store_bindings, where we iterate
over all the names in each non-global scope to figure out whether to
preserve them.  It would probably improve performance if
cp_binding_level::names were a vector of trees instead of a linked
list of trees.

Patch ping

[Jakub Jelinek]

Hi!

I'd like to ping fix for P1 PR69947:
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01743.html

Jakub

Re: [PATCH] Specify that new ports should use LRA

[Manuel López-Ibáñez]

On 2 March 2016 at 21:47, H.J. Lu  wrote:
> On Wed, Mar 2, 2016 at 12:18 PM, Manuel López-Ibáñez
>  wrote:
>> Pre-approved by Jeff here: 
>> https://gcc.gnu.org/ml/gcc-help/2016-03/msg6.html
>>
>> Committed as revision 233914.
>
> I checked in this missing patch.

Thanks H.J.!

Re: [C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Jakub Jelinek]

On Thu, Mar 03, 2016 at 03:15:41PM +0100, Marek Polacek wrote:
> This is ICE on invalid Cilk+ code.  cilk_spawn expects a function call, so 
> e.g.
> _Cilk_spawn (void) is invalid.  The function call after the cilk_spawn keyword
> is parsed using recursive call in c_parser_postfix_expression (case
> RID_CILK_SPAWN).  Now, c_parser_postfix_expression sees '(' followed by a
> typename, so it thinks we're inside a compound literal, which means it expects
> '{', but that isn't there, so we crash on the assert in c_parser_braced_init:
> gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
> But as the comment in c_parser_postfix_expression says, the code for parsing
> a compound literal here is likely dead.  I made an experiment and added
> gcc_unreachable () in that block, ran regtest, and there were no failures.
> Thus it should be safe to just remove the code, which also fixes this ICE; 
> with
> the patch we just give a proper error and don't crash anymore.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'm actually slightly
> nervous about the change, so maybe better table until gcc7?

This reminds me of PR67495.  Perhaps the answer here is also during the
_Cilk* stuff parsing don't call c_parser_postfix_expression, but call
c_parser_cast_expression instead and then verify what it got?

> 2016-03-03  Marek Polacek  
> 
>   PR c/69798
>   * c-parser.c (c_parser_postfix_expression): Remove code dealing with
>   compound literals.
> 
>   * gcc.dg/cilk-plus/pr69798-1.c: New test.
>   * gcc.dg/cilk-plus/pr69798-2.c: New test.

Jakub

Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level

[Markus Trippelsdorf]

On 2016.03.03 at 09:16 -0500, Patrick Palka wrote:
> push_to_top_level gets called fairly frequently in template-heavy code
> that performs a lot of instantiations, and we currently "leak" a lot of
> GC memory when compiling such code since [push|pop]_to_top_level() do
> not bother reusing or even freeing each saved_scope structure it
> allocates.
> 
> This patch makes push_to_top_level() reuse the saved_scope structures it
> allocates.  This is similar to how begin_scope() reuses the
> cp_binding_level structures it allocates.
> 
> This patch reduces the maximum memory usage of the compiler by 4.5%,
> from 525MB to 500MB, when compiling the Boost::Fusion test file
> libs/fusion/test/compile_time/transform.cpp from the Boost 1.60 testsuite.
> 
> Bootstrapped and tested on x86_64-pc-linux-gnu, OK for
> trunk or for GCC 7?

Great. push_to_top_level also shows up very high in profiles when
building Chromium for example. 

There is an old bug for this issue:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64500

-- 
Markus

Re: Proposed Patch for Bug 69687

[Bernd Schmidt]

On 03/02/2016 06:22 PM, Mike Stump wrote:


So, check for overflow, or better use unsigned values that are large
enough to never overflow.  With no possibility for overflow, you can
then retest the bug and see if there are any other failure modes and
fix those.


What C standard can we assume for libiberty? I was looking at patching 
this and discovered that SIZE_MAX is defined only for C99, so I'm 
leaning towards retaining the ints and using INT_MAX.



Bernd

[PATCH] Reuse the saved_scope structures allocated by push_to_top_level

[Patrick Palka]

push_to_top_level gets called fairly frequently in template-heavy code
that performs a lot of instantiations, and we currently "leak" a lot of
GC memory when compiling such code since [push|pop]_to_top_level() do
not bother reusing or even freeing each saved_scope structure it
allocates.

This patch makes push_to_top_level() reuse the saved_scope structures it
allocates.  This is similar to how begin_scope() reuses the
cp_binding_level structures it allocates.

This patch reduces the maximum memory usage of the compiler by 4.5%,
from 525MB to 500MB, when compiling the Boost::Fusion test file
libs/fusion/test/compile_time/transform.cpp from the Boost 1.60 testsuite.

Bootstrapped and tested on x86_64-pc-linux-gnu, OK for
trunk or for GCC 7?

gcc/cp/ChangeLog:

* name-lookup.c (free_saved_scope): New free list of saved_scope
structures.
(push_to_top_level): Attempt to reuse a saved_scope struct
from free_saved_scope instead of allocating a new one each time.
(pop_from_top_level_1): Chain the now-unused saved_scope structure
onto free_saved_scope.
---
 gcc/cp/name-lookup.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 89d84d7..3478b6a 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -6134,6 +6134,10 @@ store_class_bindings (vec 
*names,
   timevar_cond_stop (TV_NAME_LOOKUP, subtime);
 }
 
+/* A chain of saved_scope structures awaiting reuse.  */
+
+static GTY((deletable)) struct saved_scope *free_saved_scope;
+
 void
 push_to_top_level (void)
 {
@@ -6144,7 +6148,21 @@ push_to_top_level (void)
   bool need_pop;
 
   bool subtime = timevar_cond_start (TV_NAME_LOOKUP);
-  s = ggc_cleared_alloc ();
+
+  /* Reuse or create a new structure for this saved scope.  */
+  if (free_saved_scope != NULL)
+{
+  s = free_saved_scope;
+  free_saved_scope = s->prev;
+
+  vec *old_bindings = s->old_bindings;
+  memset (s, 0, sizeof (*s));
+  /* Also reuse the structure's old_bindings vector.  */
+  vec_safe_truncate (old_bindings, 0);
+  s->old_bindings = old_bindings;
+}
+  else
+s = ggc_cleared_alloc ();
 
   b = scope_chain ? current_binding_level : 0;
 
@@ -6237,6 +6255,11 @@ pop_from_top_level_1 (void)
   current_function_decl = s->function_decl;
   cp_unevaluated_operand = s->unevaluated_operand;
   c_inhibit_evaluation_warnings = s->inhibit_evaluation_warnings;
+
+  /* Make this saved_scope structure available for reuse by
+ push_to_top_level.  */
+  s->prev = free_saved_scope;
+  free_saved_scope = s;
 }
 
 /* Wrapper for pop_from_top_level_1.  */
-- 
2.8.0.rc0.11.g9bfbc33

[C PATCH] Fix ICE on invalid Cilk+ code (PR c/69798)

[Marek Polacek]

This is ICE on invalid Cilk+ code.  cilk_spawn expects a function call, so e.g.
_Cilk_spawn (void) is invalid.  The function call after the cilk_spawn keyword
is parsed using recursive call in c_parser_postfix_expression (case
RID_CILK_SPAWN).  Now, c_parser_postfix_expression sees '(' followed by a
typename, so it thinks we're inside a compound literal, which means it expects
'{', but that isn't there, so we crash on the assert in c_parser_braced_init:
gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE));
But as the comment in c_parser_postfix_expression says, the code for parsing
a compound literal here is likely dead.  I made an experiment and added
gcc_unreachable () in that block, ran regtest, and there were no failures.
Thus it should be safe to just remove the code, which also fixes this ICE; with
the patch we just give a proper error and don't crash anymore.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'm actually slightly
nervous about the change, so maybe better table until gcc7?

2016-03-03  Marek Polacek  

PR c/69798
* c-parser.c (c_parser_postfix_expression): Remove code dealing with
compound literals.

* gcc.dg/cilk-plus/pr69798-1.c: New test.
* gcc.dg/cilk-plus/pr69798-2.c: New test.

diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index bb508b7..9e8ac1b 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -7512,28 +7512,6 @@ c_parser_postfix_expression (c_parser *parser)
  set_c_expr_source_range (, loc, close_loc);
  mark_exp_read (expr.value);
}
-  else if (c_token_starts_typename (c_parser_peek_2nd_token (parser)))
-   {
- /* A compound literal.  ??? Can we actually get here rather
-than going directly to
-c_parser_postfix_expression_after_paren_type from
-elsewhere?  */
- location_t loc;
- struct c_type_name *type_name;
- c_parser_consume_token (parser);
- loc = c_parser_peek_token (parser)->location;
- type_name = c_parser_type_name (parser);
- c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
-"expected %<)%>");
- if (type_name == NULL)
-   {
- expr.value = error_mark_node;
-   }
- else
-   expr = c_parser_postfix_expression_after_paren_type (parser,
-type_name,
-loc);
-   }
   else
{
  /* A parenthesized expression.  */
diff --git gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c 
gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
index e69de29..1120193 100644
--- gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
+++ gcc/testsuite/gcc.dg/cilk-plus/pr69798-1.c
@@ -0,0 +1,12 @@
+/* PR c/69798 */
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+
+int
+main ()
+{
+  _Cilk_spawn (void); /* { dg-error "expected expression" } */
+  _Cilk_spawn (char []); /* { dg-error "expected expression" } */
+  _Cilk_spawn (int *); /* { dg-error "expected expression" } */
+  _Cilk_spawn ({}); /* { dg-error "only function calls can be spawned" } */
+}
diff --git gcc/testsuite/gcc.dg/cilk-plus/pr69798-2.c 
gcc/testsuite/gcc.dg/cilk-plus/pr69798-2.c
index e69de29..66bcdc8 100644
--- gcc/testsuite/gcc.dg/cilk-plus/pr69798-2.c
+++ gcc/testsuite/gcc.dg/cilk-plus/pr69798-2.c
@@ -0,0 +1,11 @@
+/* PR c/69798 */
+/* { dg-do compile } */
+
+int
+main ()
+{
+  _Cilk_spawn (void); /* { dg-error "expected expression" } */
+  _Cilk_spawn (char []); /* { dg-error "expected expression" } */
+  _Cilk_spawn (int *); /* { dg-error "expected expression" } */
+  _Cilk_spawn ({}); /* { dg-error "only function calls can be spawned" } */
+}

Marek

Re: [Ping^2][PATCH][GCC-5] Fix "#pragma GCC pop_options" warning.

[Andre Vieira (lists)]

On 03/03/16 12:11, Bernd Schmidt wrote:
> On 03/03/2016 11:45 AM, Andre Vieira (lists) wrote:
>> On 29/02/16 10:47, Andre Vieira (lists) wrote:
>>> On 15/02/16 10:33, Andre Vieira (lists) wrote:
 On 18/01/16 11:04, Andre Vieira (lists) wrote:
> Hi there,
>
> Can we have the "#pragma GCC pop_options" fix backported to GCC-5?
>
> Patch found in
> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg01261.html
> and was committed in r228794.
>
> The same patch applies cleanly to gcc-5, which would otherwise not be
> able to use this pragma even though the support is there.
> 
>> I understood it was a good idea to CC the appropriate maintainer on
>> this, so adding Bernd Schmidt to the CC.
> 
> Yeah, I think I remember this one. Ok.
> 
> 
> Bernd
> 

Thomas committed on my behalf at revision r233939.

2016-03-03  Andre Vieira  

Backport from mainline
2015-10-14  Dominik Vogt  

Fix "#pragma GCC pop_options"

gcc/ChangeLog

* targhooks.c (default_target_option_pragma_parse): Do not warn if
called on behalf of "#pragma GCC pop_options".

gcc/testsuite/ChangeLog
* gcc.dg/pragma-pop_options-1.c: New test.

Thank you Thomas and Bernd.

Cheers,
Andre

Re: [PATCH][ARM] PR target/70008

[Richard Earnshaw (lists)]

On 03/03/16 07:23, Michael Collison wrote:
> I have attached a new patch which hopefully address Richard's concerns.
> I modified the predicate on operand 1 to to "arm_rhs_operand" to be
> consistent with the constraints. I retained the split into two patterns;
> one for arm and another for thumb2. I thought this was cleaner.
> 
> Okay for trunk?
> 
> 2016-02-28  Michael Collison  
> 
> PR target/70008
> * config/arm/arm.md (*subsi3_carryin): Change predicate to
> arm_rhs_operand to be consistent with constraints.
> Only allow pattern for TARGET_ARM.
> * config/arm/thumb2.md (*thumb2_subsi3_carryin): New pattern.
> 

I don't think we need two patterns.  We could share this if we had a new
predicate that was called something like reg_or_arm_rhs_operand,  with a
rule that's something like:

  register_operand (op) || (TARGET_ARM && arm_immediate_operand (op))

R.
> On 02/29/2016 08:29 AM, Richard Earnshaw (lists) wrote:
>> On 29/02/16 11:21, Michael Collison wrote:
>>>
>>> On 2/29/2016 4:06 AM, Kyrill Tkachov wrote:
 Hi Michael,

 On 29/02/16 04:47, Michael Collison wrote:
> This patches address PR 70008, where a reverse subtract with carry
> instruction can be generated in thumb2 mode. It was tested with no
> regressions in arm and thumb modes on the following targets:
>
> arm-none-linux-gnueabi
> arm-none-linux-gnuabihf
> armeb-none-linux-gnuabihf
> arm-none-eabi
>
> Okay for trunk?
>
> 2016-02-28  Michael Collison 
>
>  PR target/70008
>  * config/arm/arm.md (*subsi3_carryin): Only match pattern if
>  TARGET_ARM due to 'rsc' instruction alternative.
>  * config/arm/thumb2.md (*thumb2_subsi3_carryin): New pattern.
>
>
 The *subsi3_carrying pattern has the arch attribute:
 (set_attr "arch" "*,a")

 That means that the second alternative that generates the RSC
 instruction is only enabled
 for ARM mode. Do you have a testcase where this doesn't happen and
 this pattern generates
 the second alternative for Thumb2?
>>> No I don't have a test case; i noticed the pattern when working on the
>>> overflow project. I did not realize
>>> that an attribute could affect the matching of an alternative. I will
>>> close the bug.
>>>

 Thanks,
 Kyrill
>> This is all true, but there is a potential performance issue with this
>> pattern though, that could lead to sub-optimal code.
>>
>> The predicate accepts reg-or-int, but in ARM state only simple
>> 'const-ok-for-arm' immediates are permitted by the predicates, and in
>> thumb code, no immediates are permitted at all.  This could potentially
>> result in sub-optimal code due to late splitting of the pattern.  It
>> would be better if the predicate understood these limitations and
>> restricted immediates accordingly.
>>
>> R.
>>
> 
> 
> bugzilla-70008-upstream-v2.patch
> 
> 
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index e67239d..e6bcd7f 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -867,15 +867,14 @@
>  
>  (define_insn "*subsi3_carryin"
>[(set (match_operand:SI 0 "s_register_operand" "=r,r")
> -(minus:SI (minus:SI (match_operand:SI 1 "reg_or_int_operand" "r,I")
> +(minus:SI (minus:SI (match_operand:SI 1 "arm_rhs_operand" "r,I")
>  (match_operand:SI 2 "s_register_operand" "r,r"))
>(ltu:SI (reg:CC_C CC_REGNUM) (const_int 0]
> -  "TARGET_32BIT"
> +  "TARGET_ARM"
>"@
> sbc%?\\t%0, %1, %2
> rsc%?\\t%0, %2, %1"
>[(set_attr "conds" "use")
> -   (set_attr "arch" "*,a")
> (set_attr "predicable" "yes")
> (set_attr "predicable_short_it" "no")
> (set_attr "type" "adc_reg,adc_imm")]
> diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
> index 9925365..79305c5 100644
> --- a/gcc/config/arm/thumb2.md
> +++ b/gcc/config/arm/thumb2.md
> @@ -848,6 +848,20 @@
> (set_attr "type" "multiple")]
>  )
>  
> +(define_insn "*thumb2_subsi3_carryin"
> +  [(set (match_operand:SI 0 "s_register_operand" "=r")
> +(minus:SI (minus:SI (match_operand:SI 1 "s_register_operand" "r")
> +(match_operand:SI 2 "s_register_operand" "r"))
> +  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0]
> +  "TARGET_THUMB2"
> +  "@
> +   sbc%?\\t%0, %1, %2"
> +  [(set_attr "conds" "use")
> +   (set_attr "predicable" "yes")
> +   (set_attr "predicable_short_it" "no")
> +   (set_attr "type" "adc_reg")]
> +)
> +
>  (define_insn "*thumb2_cond_sub"
>[(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts")
>  (minus:SI (match_operand:SI 1 "s_register_operand" "0,?Ts")
> 

Re: [committed] Fix libffi/70024

[Rainer Orth]

Hi Richard,

> Unfortunately, even with this fixed, all Solaris/x86 tests now fail to
> link:
>
> FAIL: libffi.call/closure_fn0.c -W -Wall -Wno-psabi -O0 (test for excess 
> errors)
> Excess errors:
> Undefined   first referenced
>  symbol in file
> ffi_closure_alloc   /var/tmp//ccIpq3qc.o
> ffi_type_float  /var/tmp//ccIpq3qc.o
> ffi_type_uint64 /var/tmp//ccIpq3qc.o
> ffi_type_sint32 /var/tmp//ccIpq3qc.o
> ffi_type_sint16 /var/tmp//ccIpq3qc.o
> ffi_type_double /var/tmp//ccIpq3qc.o
> ffi_prep_cif/var/tmp//ccIpq3qc.o
> ld: fatal: symbol referencing errors

turned out to be easy, too: make_sunver.pl got passed a list of
non-existant object files:

.libs/src/prep_cif.o .libs/src/types.o .libs/src/raw_api.o 
.libs/src/java_raw_api.o .libs/src/closures.o

Treating $(libffi_la_OBJECTS) and $(libffi_la_LIBADD) the same fixed
this so all the symbols above got included in libffi.map-sun and all
tests pass.

Tested on i386-pc-solaris2.12, installed on mainline.

Rainer

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


2016-03-03  Rainer Orth  

* Makefile.am (libffi.map-sun): Properly convert
$(libffi_la_OBJECTS) to object names.
* Makefile.in: Regenerate.

# HG changeset patch
# Parent  ce4aa09ea32c03f4d0ad44415ce066a861bda6c3
Fix passing object names to make_sunver.pl

diff --git a/libffi/Makefile.am b/libffi/Makefile.am
--- a/libffi/Makefile.am
+++ b/libffi/Makefile.am
@@ -215,8 +215,7 @@ libffi_version_dep = libffi.map-sun
 libffi.map-sun : libffi.map $(top_srcdir)/../contrib/make_sunver.pl \
 $(libffi_la_OBJECTS) $(libffi_la_LIBADD)
 	perl $(top_srcdir)/../contrib/make_sunver.pl libffi.map \
-	  $(libffi_la_OBJECTS:%.lo=.libs/%.o) \
-	 `echo $(libffi_la_LIBADD) | \
+	 `echo $(libffi_la_OBJECTS) $(libffi_la_LIBADD) | \
 	sed 's,\([^/]*\)\.l\([ao]\),.libs/\1.\2,g'` \
 	 > $@ || (rm -f $@ ; exit 1)
 endif

Re: [PATCH] Fix up vect pattern handling (PR target/70021)

[Richard Biener]

On Wed, 2 Mar 2016, Jakub Jelinek wrote:

> Hi!
> 
> This patch fixes two issues:
> 1) reverts part of https://gcc.gnu.org/ml/gcc-patches/2011-06/msg02183.html
>changes, my understanding is that we don't emit or support what has been
>mentioned as rationale for that, now that we can stick multiple pattern
>stmts into a sequence; without this reversion, we mark both the pattern
>and normal stmt relevant and then when vectorizing use in one spot
>the pattern stmt and in another one the original stmt, while we clearly
>want to use the pattern stmt always; also, without the reversion we
>consider the original stmt as needed for VF computation and thus think
>the loop needs QImode elements in addition to SImode and DImode.
> 2) if the shift count is coming from stmt with corresponding pattern stmt,
>we shouldn't look through it, because we might again refer to the middle
>of a pattern stmt (this is similar to the recently committed patch); we
>don't need to punt completely in that case though, the code can just
>add a cast into the pattern sequence as it does in many other cases.
> 
> Bootstrapped/regtested on {x86_64,i686,powerpc64,powerpc64le}-linux,
> bootstrap/regtest is still pending on {s390,s390x,aarch64,armv7hl}-linux,
> ok for trunk if it passes even there? 

Ok.  I've been trying to understand this code multiple times myself
and I'm happy to see it go ;)  Even though in all cases I remember
the issue was elsewhere...

Thanks,
Richard.

> 2016-03-02  Jakub Jelinek  
> 
>   PR target/70021
>   * tree-vect-stmts.c (vect_mark_relevant): Remove USED_IN_PATTERN
>   argument, if STMT_VINFO_IN_PATTERN_P (stmt_info), always mark
>   the pattern no matter if it is used just by non-pattern, pattern
>   or mix thereof.
>   (process_use, vect_mark_stmts_to_be_vectorized): Adjust callers.
>   * tree-vect-patterns.c (vect_recog_vector_vector_shift_pattern): If
>   oprnd1 def_stmt is in pattern, don't look through it.
> 
>   * gcc.dg/vect/pr70021.c: New test.
>   * gcc.target/i386/pr70021.c: New test.
> 
> --- gcc/tree-vect-stmts.c.jj  2016-03-01 19:23:51.0 +0100
> +++ gcc/tree-vect-stmts.c 2016-03-02 15:40:53.777231771 +0100
> @@ -181,8 +181,7 @@ create_array_ref (tree type, tree ptr, s
>  
>  static void
>  vect_mark_relevant (vec *worklist, gimple *stmt,
> - enum vect_relevant relevant, bool live_p,
> - bool used_in_pattern)
> + enum vect_relevant relevant, bool live_p)
>  {
>stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
>enum vect_relevant save_relevant = STMT_VINFO_RELEVANT (stmt_info);
> @@ -202,62 +201,22 @@ vect_mark_relevant (vec *workl
>   stmt itself should be marked.  */
>if (STMT_VINFO_IN_PATTERN_P (stmt_info))
>  {
> -  bool found = false;
> -  if (!used_in_pattern)
> -{
> -  imm_use_iterator imm_iter;
> -  use_operand_p use_p;
> -  gimple *use_stmt;
> -  tree lhs;
> -   loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
> -   struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
> -
> -  if (is_gimple_assign (stmt))
> -lhs = gimple_assign_lhs (stmt);
> -  else
> -lhs = gimple_call_lhs (stmt);
> -
> -  /* This use is out of pattern use, if LHS has other uses that are
> - pattern uses, we should mark the stmt itself, and not the 
> pattern
> - stmt.  */
> -   if (lhs && TREE_CODE (lhs) == SSA_NAME)
> - FOR_EACH_IMM_USE_FAST (use_p, imm_iter, lhs)
> -   {
> - if (is_gimple_debug (USE_STMT (use_p)))
> -   continue;
> - use_stmt = USE_STMT (use_p);
> -
> - if (!flow_bb_inside_loop_p (loop, gimple_bb (use_stmt)))
> -   continue;
> -
> - if (vinfo_for_stmt (use_stmt)
> - && STMT_VINFO_IN_PATTERN_P (vinfo_for_stmt (use_stmt)))
> -   {
> - found = true;
> - break;
> -   }
> -   }
> -}
> +  /* This is the last stmt in a sequence that was detected as a
> +  pattern that can potentially be vectorized.  Don't mark the stmt
> +  as relevant/live because it's not going to be vectorized.
> +  Instead mark the pattern-stmt that replaces it.  */
>  
> -  if (!found)
> -{
> -  /* This is the last stmt in a sequence that was detected as a
> - pattern that can potentially be vectorized.  Don't mark the stmt
> - as relevant/live because it's not going to be vectorized.
> - Instead mark the pattern-stmt that replaces it.  */
> -
> -  pattern_stmt = STMT_VINFO_RELATED_STMT (stmt_info);
> -
> -  if (dump_enabled_p ())
> -dump_printf_loc (MSG_NOTE, vect_location,
> - "last stmt in pattern. don't mark"
> -   

Re: [PATCH] Fix ICE with vector types in X % -Y pattern (PR middle-end/70050)

[Jakub Jelinek]

On Thu, Mar 03, 2016 at 01:56:05PM +0100, Marc Glisse wrote:
> On Thu, 3 Mar 2016, Marek Polacek wrote:
> 
> >We crashed on the attached testcase because we were trying to apply the
> >X % -Y -> X % Y transformation even on vectors.  That doesn't go well with 
> >the
> >check for !TYPE_UNSIGNED.  So I think let's limit the pattern to only work on
> >integral types.
> 
> I certainly hope the problem is not with TYPE_UNSIGNED, I think there are
> many places where we use it on vectors. I would expect the issue to be with
> TYPE_MIN_VALUE a few lines below. Not that it changes anything to the fix...
> 
> (we can revisit that if vectors ever get VRP support)

And expr_not_equal_to would need to be tought to use that...

Jakub

Re: [PATCH] Fix ICE with vector types in X % -Y pattern (PR middle-end/70050)

[Marek Polacek]

On Thu, Mar 03, 2016 at 01:56:05PM +0100, Marc Glisse wrote:
> On Thu, 3 Mar 2016, Marek Polacek wrote:
> 
> >We crashed on the attached testcase because we were trying to apply the
> >X % -Y -> X % Y transformation even on vectors.  That doesn't go well with 
> >the
> >check for !TYPE_UNSIGNED.  So I think let's limit the pattern to only work on
> >integral types.
> 
> I certainly hope the problem is not with TYPE_UNSIGNED, I think there are
> many places where we use it on vectors. I would expect the issue to be with
> TYPE_MIN_VALUE a few lines below. Not that it changes anything to the fix...

Yes, as Jakub already pointed out.  My bad, sorry about that.

Marek

Re: [PATCH] Fix ICE with vector types in X % -Y pattern (PR middle-end/70050)

[Marc Glisse]

On Thu, 3 Mar 2016, Marek Polacek wrote:


We crashed on the attached testcase because we were trying to apply the
X % -Y -> X % Y transformation even on vectors.  That doesn't go well with the
check for !TYPE_UNSIGNED.  So I think let's limit the pattern to only work on
integral types.


I certainly hope the problem is not with TYPE_UNSIGNED, I think there are 
many places where we use it on vectors. I would expect the issue to be 
with TYPE_MIN_VALUE a few lines below. Not that it changes anything to the 
fix...


(we can revisit that if vectors ever get VRP support)


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

2016-03-03  Marek Polacek  

PR middle-end/70050
* match.pd (X % -Y): Add INTEGRAL_TYPE_P check.

* gcc.dg/pr70050.c: New test.

diff --git gcc/match.pd gcc/match.pd
index 5903782..112deb3 100644
--- gcc/match.pd
+++ gcc/match.pd
@@ -293,7 +293,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* X % -Y is the same as X % Y.  */
(simplify
 (trunc_mod @0 (convert? (negate @1)))
- (if (!TYPE_UNSIGNED (type)
+ (if (INTEGRAL_TYPE_P (type)
+  && !TYPE_UNSIGNED (type)
  && !TYPE_OVERFLOW_TRAPS (type)
  && tree_nop_conversion_p (type, TREE_TYPE (@1))
  /* Avoid this transformation if X might be INT_MIN or
diff --git gcc/testsuite/gcc.dg/pr70050.c gcc/testsuite/gcc.dg/pr70050.c
index e69de29..610456f 100644
--- gcc/testsuite/gcc.dg/pr70050.c
+++ gcc/testsuite/gcc.dg/pr70050.c
@@ -0,0 +1,11 @@
+/* PR middle-end/70025 */
+/* { dg-do compile } */
+/* { dg-options "-Wno-psabi" } */
+
+typedef int v8si __attribute__ ((vector_size (32)));
+
+v8si
+foo (v8si v)
+{
+  return v %= -v;
+}


--
Marc Glisse

Re: [PATCH]Replace -shared with -r -nostdlib in gcc.dg/lto/pr61526 pr54709 pr64415 test cases.

[Richard Biener]

On Thu, Mar 3, 2016 at 1:07 PM, Renlin Li  wrote:
> Hi Richard,
>
>
> On 03/03/16 10:13, Richard Biener wrote:
>>
>> On Wed, Mar 2, 2016 at 5:12 PM, Renlin Li  wrote:
>>>
>>> Hi Richard,
>>>
>>>
>>> On 02/03/16 13:35, Richard Biener wrote:

 On Tue, Mar 1, 2016 at 4:56 PM, Renlin Li 
 wrote:
>
> Hi Richard,
>
>
> On 01/03/16 09:16, Richard Biener wrote:
>>
>> On Mon, Feb 29, 2016 at 5:13 PM, Renlin Li 
>> wrote:
>>>
>>> Hi all,
>>>
>>> The gcc.dg/lto/pr54709, pr61526, pr64415 linking testcases keep
>>> failing
>>> on
>>> arm/aarch64 bare-metal target.
>>>
>>> It's because statically built newlib library is used to link with
>>> shared
>>> object.
>>> And the linker complains about relocations which cannot be used in
>>> shared object.
>>>
>>> For example, the following errors are produced:
>>>
>>> crtbegin.o: relocation R_ARM_MOVW_ABS_NC against `a local symbol' can
>>> not
>>> be
>>> used when making a shared object; recompile with -fPIC
>>>
>>> crtbegin.o: relocation R_ARM_THM_MOVW_ABS_NC against `a local symbol'
>>> can
>>> not
>>> be used when making a shared object; recompile with -fPIC
>>>
>>> librdimon.a(rdimon-syscalls.o): relocation R_AARCH64_ADR_PREL_PG_HI21
>>> against
>>> external symbol `_impure_ptr' can not be used when making a shared
>>> object;
>>> recompile with -fPIC
>>>
>>> Presumably, bare-metal toolchain for other architecture have those
>>> test
>>> case
>>> failures as well?
>>>
>>> In this patch, -shared option is replace by -r -nostdlib. So that the
>>> standard
>>> system startup files or libraries are not used when linking.
>>
>> Note that -shared is not equivalent to -r -nostdlib so please verify
>> that
>> the
>> original issue can be still reproduced with its fix reverted but -r
>> -nostdlib
>> used with the new -r -nostdlib handling on trunk.
>
>
> pr54709_0.c: Cannot be reproduced with even -shared. The error message
> is
> the same as shown above.
> pr64415_0.c: Reproduced with "-r -nostdlib".
> pr61526_0.c: Reproduced with "-r -nostdlib".
>
> By the way, those linking test cases all pass for linux toolchain. Only
> fail
> for aarch64/arm baremetal toolchain.
>
> Andrew, I saw you have done similar things in r153555
> https://gcc.gnu.org/viewcvs/gcc?view=revision=153555
>
> Do you have any thoughts?
>
> And also here, the last comments in this ticket suggests to add
> check_effective_target_shared to the exp file to limit it to linux
> targets
> only:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61526

 As said LTO testcases tend to be somewhat fragile so limiting them to
 targets known to work might be the best option.

 Richard.
>>>
>>>
>>> Forgot the mention that, by purely adding "-nostdlib" option (instead of
>>> replacing -shared)
>>> fixes the failures as well.
>>>
>>> I test those test cases again with fix reverted, keep "-shared" option,
>>> add
>>> "-nostdlib" option.
>>
>> Ok, so I discovered we have a "shared" target which means if a target
>> doesn't
>> support shared libs we can guard against it with using
>>
>> /* { dg-require-effective-target shared } */
>>
>> does adding that to the three testcases fix the issue for you?
>
> By adding this target check
> /* { dg-require-effective-target shared } */
>
> Those test cases aredeemed to be unsupported, and thus skipped for
> aarch64-none-elf target.
>
> However, it's a little bit tricky for arm bare-metal target.
>
> The shared option check actually successes for arm-none-eabi toolchain.
> This is because the default cpu for arm-none-eabi toolchain is arm7tdmi. And
> the start file crtbegin.o doesn't contains any modifications not allowed in
> shared object.
>
> arm-none-eabi is built with multilib. When running this testcase, it's
> compiled with "-march=armv7-a".
> The crtbegin.o for this architecture version contains relocations which
> cannot be used in shared object.
> That's why they fails to linking test.

For -shared it should provide a crtbeginS.o then.  Why not fix it properly?

Richard.

> Will adding "-nostdlib" (instead of replace -shared) option be an reasonable
> fix given my previous check?
>
> Regards,
> Renlin
>
>
>
>>
>> Thanks,
>> Richard.
>>
>>> pr54709_0.c: Cannot be reproduced even with test case unmodified.
>>> The error message is the same as shown above. with "-nostdlib", no
>>> failure.
>>>
>>> pr64415_0.c: Reproduced.
>>> pr61526_0.c: Reproduced.
>>>
>>> Regards,
>>> Renlin
>>>
>>>
>>>
> Regards,
> Renlin
>
>
>> Otherwise simply dg-skip for aarch64.
>>
>> Richard.
>>
>>> arm-none-eabi, aarch64-none-elf regression test 

[PATCH] Fix PR70054

[Richard Biener]

The following patch adjusts strict_aliasing_warning to use
proper alias_set_subset_of instead of relying on alias_sets_conflict_p
as after the PR66110 fix aggregates with a char[] member do not
automatically behave like having alias-set zero.  As a side-effect
the test will be somewhat stricter as well accessing a 'long' with
a struct { int i; long l; } * will now warn while it previously
didn't:

struct S { int i; long l; };
long x;
struct S foo () { return *(struct S *) }

now warns:

t.c: In function ‘foo’:
t.c:3:35: warning: dereferencing type-punned pointer will break 
strict-aliasing rules [-Wstrict-aliasing]
 struct S foo () { return *(struct S *) }
   ^
Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk?

Thanks,
Richard.

2016-03-03  Richard Biener  

PR c++/70054
* c-common.c (strict_aliasing_warning): Use alias_set_subset_of
instead of alias_sets_conflict_p.

* g++.dg/warn/Wstrict-aliasing-bogus-union-2.C: New testcase.
* gcc.dg/Wstrict-aliasing-struct-member.c: New testcase.

Index: gcc/c-family/c-common.c
===
*** gcc/c-family/c-common.c (revision 233902)
--- gcc/c-family/c-common.c (working copy)
*** strict_aliasing_warning (tree otype, tre
*** 1568,1574 
alias_set_type set2 = get_alias_set (TREE_TYPE (type));
  
if (set1 != set2 && set2 != 0
! && (set1 == 0 || !alias_sets_conflict_p (set1, set2)))
{
  warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
   "pointer will break strict-aliasing rules");
--- 1568,1574 
alias_set_type set2 = get_alias_set (TREE_TYPE (type));
  
if (set1 != set2 && set2 != 0
! && (set1 == 0 || !alias_set_subset_of (set2, set1)))
{
  warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
   "pointer will break strict-aliasing rules");
Index: gcc/testsuite/g++.dg/warn/Wstrict-aliasing-bogus-union-2.C
===
*** gcc/testsuite/g++.dg/warn/Wstrict-aliasing-bogus-union-2.C  (revision 0)
--- gcc/testsuite/g++.dg/warn/Wstrict-aliasing-bogus-union-2.C  (working copy)
***
*** 0 
--- 1,14 
+ /* { dg-do compile { target c++11 } } */
+ /* { dg-options "-Wstrict-aliasing=2 -O2 -Wall" } */
+ 
+ #include 
+ 
+ struct foo
+ {
+   std::aligned_storage::type raw;
+ 
+   long& cooked()
+ {
+   return *static_cast(static_cast()); /* { dg-bogus 
"strict-aliasing" } */
+ }
+ };
Index: gcc/testsuite/gcc.dg/Wstrict-aliasing-struct-member.c
===
--- gcc/testsuite/gcc.dg/Wstrict-aliasing-struct-member.c   (revision 0)
+++ gcc/testsuite/gcc.dg/Wstrict-aliasing-struct-member.c   (working copy)
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+struct S { int i; long l; };
+long x;
+struct S foo () { return *(struct S *) } /* { dg-warning "will break 
strict-aliasing" } */

Re: [Ping^2][PATCH][GCC-5] Fix "#pragma GCC pop_options" warning.

[Bernd Schmidt]

On 03/03/2016 11:45 AM, Andre Vieira (lists) wrote:

On 29/02/16 10:47, Andre Vieira (lists) wrote:

On 15/02/16 10:33, Andre Vieira (lists) wrote:

On 18/01/16 11:04, Andre Vieira (lists) wrote:

Hi there,

Can we have the "#pragma GCC pop_options" fix backported to GCC-5?

Patch found in https://gcc.gnu.org/ml/gcc-patches/2015-10/msg01261.html
and was committed in r228794.

The same patch applies cleanly to gcc-5, which would otherwise not be
able to use this pragma even though the support is there.



I understood it was a good idea to CC the appropriate maintainer on
this, so adding Bernd Schmidt to the CC.


Yeah, I think I remember this one. Ok.


Bernd

[PATCH] Fix vec_set_hi* patterns (PR target/70059)

[Jakub Jelinek]

Hi!

Unlike the older vec_set_hi 256-bit patterns, which are correctly
using the VEC_SELECT as the first operand of VEC_CONCAT and
match_operand as second, because that is what the instruction does,
picks up low part from operand 1 and puts the operand 2 as the high part
of the result, the 512-bit vec_set_hi patterns (both the avx512f ones
and avx512dq ones) use the same order of VEC_CONCAT operands as vec_set_lo
(and differ just by different second operand of VEC_SELECT).
This leads to wrong-code on the following testcases, simplify-rtx.c during
combine simply looks at the RTL patterns and simplifies stuff according
to how the RTL patterns look like.

Fixed thusly, unfortunately I don't have access to avx512f (and not even to
avx512dq) hw, so while I will bootstrap/regtest it on Haswell-E, can't test
the tests if they now work at runtime (they link and the assembly of the foo
routine has changed and looks good to me).  Can somebody test this please
on real hw or emulator?
Ok for trunk if it passes?

2016-03-03  Jakub Jelinek  

PR target/70059
* config/i386/sse.md (vec_set_lo_,
_vinsert_mask): Formatting
fixes.
(vec_set_hi_): Likewise.  Swap VEC_CONCAT operands.

* gcc.target/i386/avx512f-pr70059.c: New test.
* gcc.target/i386/avx512dq-pr70059.c: New test.

--- gcc/config/i386/sse.md.jj   2016-03-02 20:14:11.0 +0100
+++ gcc/config/i386/sse.md  2016-03-03 10:40:58.325680697 +0100
@@ -12426,13 +12426,13 @@ (define_expand "_vinsert
 {
   int mask = INTVAL (operands[3]);
   if (mask == 0)
-emit_insn (gen_vec_set_lo__mask
-  (operands[0], operands[1], operands[2],
-   operands[4], operands[5]));
+emit_insn (gen_vec_set_lo__mask (operands[0], operands[1],
+  operands[2], operands[4],
+  operands[5]));
   else
-emit_insn (gen_vec_set_hi__mask
-  (operands[0], operands[1], operands[2],
-   operands[4], operands[5]));
+emit_insn (gen_vec_set_hi__mask (operands[0], operands[1],
+  operands[2], operands[4],
+  operands[5]));
   DONE;
 })
 
@@ -12443,9 +12443,9 @@ (define_insn "vec_set_lo_
(match_operand:V16FI 1 "register_operand" "v")
(parallel [(const_int 8) (const_int 9)
- (const_int 10) (const_int 11)
- (const_int 12) (const_int 13)
-  (const_int 14) (const_int 15)]]
+  (const_int 10) (const_int 11)
+  (const_int 12) (const_int 13)
+  (const_int 14) (const_int 15)]]
   "TARGET_AVX512DQ"
   "vinsert32x8\t{$0x0, %2, %1, 
%0|%0, %1, %2, $0x0}"
   [(set_attr "type" "sselog")
@@ -12456,13 +12456,13 @@ (define_insn "vec_set_lo_"
   [(set (match_operand:V16FI 0 "register_operand" "=v")
(vec_concat:V16FI
- (match_operand: 2 "nonimmediate_operand" "vm")
  (vec_select:
(match_operand:V16FI 1 "register_operand" "v")
(parallel [(const_int 0) (const_int 1)
- (const_int 2) (const_int 3)
- (const_int 4) (const_int 5)
-  (const_int 6) (const_int 7)]]
+  (const_int 2) (const_int 3)
+  (const_int 4) (const_int 5)
+  (const_int 6) (const_int 7)]))
+ (match_operand: 2 "nonimmediate_operand" "vm")))]
   "TARGET_AVX512DQ"
   "vinsert32x8\t{$0x1, %2, %1, 
%0|%0, %1, %2, $0x1}"
   [(set_attr "type" "sselog")
@@ -12477,7 +12477,7 @@ (define_insn "vec_set_lo_
(match_operand:V8FI 1 "register_operand" "v")
(parallel [(const_int 4) (const_int 5)
-  (const_int 6) (const_int 7)]]
+  (const_int 6) (const_int 7)]]
   "TARGET_AVX512F"
   "vinsert64x4\t{$0x0, %2, %1, 
%0|%0, %1, %2, $0x0}"
   [(set_attr "type" "sselog")
@@ -12488,11 +12488,11 @@ (define_insn "vec_set_lo_"
   [(set (match_operand:V8FI 0 "register_operand" "=v")
(vec_concat:V8FI
- (match_operand: 2 "nonimmediate_operand" "vm")
  (vec_select:
(match_operand:V8FI 1 "register_operand" "v")
(parallel [(const_int 0) (const_int 1)
-  (const_int 2) (const_int 3)]]
+  (const_int 2) (const_int 3)]))
+ (match_operand: 2 "nonimmediate_operand" "vm")))]
   "TARGET_AVX512F"
   "vinsert64x4\t{$0x1, %2, %1, 
%0|%0, %1, %2, $0x1}"
   [(set_attr "type" "sselog")
--- gcc/testsuite/gcc.target/i386/avx512f-pr70059.c.jj  2016-03-03 
11:06:00.949063626 +0100
+++ gcc/testsuite/gcc.target/i386/avx512f-pr70059.c 2016-03-03 
11:10:42.772195215 +0100
@@ -0,0 +1,33 @@
+/* PR target/70059 */
+/* { dg-do run } */
+/* { dg-require-effective-target avx512f } */
+/* { dg-options "-O2 -mavx512f" } */
+
+#include "avx512f-check.h"
+
+__attribute__((noinline, noclone)) __m512i
+foo (__m256i a, __m256i b)
+{
+ 

Re: [PATCH]Replace -shared with -r -nostdlib in gcc.dg/lto/pr61526 pr54709 pr64415 test cases.

[Renlin Li]

Hi Richard,

On 03/03/16 10:13, Richard Biener wrote:

On Wed, Mar 2, 2016 at 5:12 PM, Renlin Li  wrote:

Hi Richard,


On 02/03/16 13:35, Richard Biener wrote:

On Tue, Mar 1, 2016 at 4:56 PM, Renlin Li  wrote:

Hi Richard,


On 01/03/16 09:16, Richard Biener wrote:

On Mon, Feb 29, 2016 at 5:13 PM, Renlin Li 
wrote:

Hi all,

The gcc.dg/lto/pr54709, pr61526, pr64415 linking testcases keep failing
on
arm/aarch64 bare-metal target.

It's because statically built newlib library is used to link with
shared
object.
And the linker complains about relocations which cannot be used in
shared object.

For example, the following errors are produced:

crtbegin.o: relocation R_ARM_MOVW_ABS_NC against `a local symbol' can
not
be
used when making a shared object; recompile with -fPIC

crtbegin.o: relocation R_ARM_THM_MOVW_ABS_NC against `a local symbol'
can
not
be used when making a shared object; recompile with -fPIC

librdimon.a(rdimon-syscalls.o): relocation R_AARCH64_ADR_PREL_PG_HI21
against
external symbol `_impure_ptr' can not be used when making a shared
object;
recompile with -fPIC

Presumably, bare-metal toolchain for other architecture have those test
case
failures as well?

In this patch, -shared option is replace by -r -nostdlib. So that the
standard
system startup files or libraries are not used when linking.

Note that -shared is not equivalent to -r -nostdlib so please verify
that
the
original issue can be still reproduced with its fix reverted but -r
-nostdlib
used with the new -r -nostdlib handling on trunk.


pr54709_0.c: Cannot be reproduced with even -shared. The error message is
the same as shown above.
pr64415_0.c: Reproduced with "-r -nostdlib".
pr61526_0.c: Reproduced with "-r -nostdlib".

By the way, those linking test cases all pass for linux toolchain. Only
fail
for aarch64/arm baremetal toolchain.

Andrew, I saw you have done similar things in r153555
https://gcc.gnu.org/viewcvs/gcc?view=revision=153555

Do you have any thoughts?

And also here, the last comments in this ticket suggests to add
check_effective_target_shared to the exp file to limit it to linux
targets
only:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61526

As said LTO testcases tend to be somewhat fragile so limiting them to
targets known to work might be the best option.

Richard.


Forgot the mention that, by purely adding "-nostdlib" option (instead of
replacing -shared)
fixes the failures as well.

I test those test cases again with fix reverted, keep "-shared" option, add
"-nostdlib" option.

Ok, so I discovered we have a "shared" target which means if a target doesn't
support shared libs we can guard against it with using

/* { dg-require-effective-target shared } */

does adding that to the three testcases fix the issue for you?

By adding this target check
/* { dg-require-effective-target shared } */

Those test cases aredeemed to be unsupported, and thus skipped for 
aarch64-none-elf target.


However, it's a little bit tricky for arm bare-metal target.

The shared option check actually successes for arm-none-eabi toolchain.
This is because the default cpu for arm-none-eabi toolchain is arm7tdmi. And
the start file crtbegin.o doesn't contains any modifications not allowed 
in shared object.


arm-none-eabi is built with multilib. When running this testcase, it's 
compiled with "-march=armv7-a".
The crtbegin.o for this architecture version contains relocations which 
cannot be used in shared object.

That's why they fails to linking test.

Will adding "-nostdlib" (instead of replace -shared) option be an reasonable
fix given my previous check?

Regards,
Renlin




Thanks,
Richard.


pr54709_0.c: Cannot be reproduced even with test case unmodified.
The error message is the same as shown above. with "-nostdlib", no failure.

pr64415_0.c: Reproduced.
pr61526_0.c: Reproduced.

Regards,
Renlin




Regards,
Renlin



Otherwise simply dg-skip for aarch64.

Richard.


arm-none-eabi, aarch64-none-elf regression test OK, OK for trunk?

Regards,
Renlin Li

gcc/testsuite/ChangeLog:

2016-02-29  Renlin Li

   * gcc.dg/lto/pr54709_0.c: Replace -shard with -r -nostdlib.
   * gcc.dg/lto/pr61526_0.c: Ditto.
   * gcc.dg/lto/pr64415_0.c: Ditto.

Re: [PTX] port some cleanups from gomp4

[Alexander Monakov]

Hello Nathan,

On Wed, 9 Sep 2015, Nathan Sidwell wrote:
> I've applied this patch to port some cleanups, mainly formatting and loop
> idioms from the gomp4 branch.

This patch that you committed to trunk in September 2015 forcefully disables
generation of line number information, undoing a part of a patch by Bernd
Schmidt that was committed to trunk by Thomas Schwinge in August 2015.  I
couldn't find a corresponding email for the gomp4 branch (apparently the same
change was committed to the branch just a few hours prior to trunk), so I
wonder if there was any technical reason to disable linenumber info, or just
some kind of oversight?

Would you like if I prepared a patch for trunk that backs out the override?
It's useful not only for debugging, but also for instruction-level profiling
(which is my main motivation at the moment).

(relevant bit of the problematic patch quoted below)

Thanks.
Alexander

> 2015-09-09  Nathan Sidwell  
> 
>   * config/nvptx/nvptx.md (call_operation): Move bound out of loop.
>   (*cmp): Add assembler spacing.
>   (setcc_int, set_cc_float): Likewise.
>   * config/nvptx/nvptx.c (nvptx_option_override): Override debug
>   level.

(the hunk for the above entry)

>   (write_func_decl_from_insn): Refactor argument loops & comma emission.
>   (nvptx_expand_call): Likewise.
>   (nvptx_output_call_insn): Likewise.
>   (nvptx_reorg_subreg): Add spacing.
> 
> Index: src/gcc-mainline/gcc/config/nvptx/nvptx.c
> ===
> --- src/gcc-mainline/gcc/config/nvptx/nvptx.c (revision 227587)
> +++ src/gcc-mainline/gcc/config/nvptx/nvptx.c (working copy)
> @@ -102,6 +102,8 @@ nvptx_option_override (void)
>flag_toplevel_reorder = 1;
>/* Assumes that it will see only hard registers.  */
>flag_var_tracking = 0;
> +  write_symbols = NO_DEBUG;
> +  debug_info_level = DINFO_LEVEL_NONE;
>  
>declared_fndecls_htab = hash_table::create_ggc (17);
>needed_fndecls_htab = hash_table::create_ggc (17);

[PATCH][AArch64] PR target/70002: Make aarch64_set_current_function play nice with pragma resetting

[Kyrill Tkachov]

Hi all,

This patch fixes the ICE that was introduced by my earlier patch to 
aarch64_set_current_function:
FAIL: gcc.dg/torture/pr52429.c -O2 -flto -fno-use-linker-plugin 
-flto-partition=none (internal compiler error)

And it also fixes a bug that I was working on separately relating to popping 
pragmas.
The patch rewrites the aarch64_set_current_function implementation to be the 
same as the one in the arm port
that Christian wrote and which is simpler than the existing implementation and 
has been tested for some time
without problems. I've thought this was the way to go but was hoping to do it 
for GCC 7 instead, but I think
given the ICE we'd rather have consistent implementations of this hook between 
arm and aarch64 (and ideally
this should be moved into the midend for all targets, I don't see much 
target-specific information in the
implementation of this across the targets, but not at this stage).

Similar to that implementation the setting and restoring of the target globals 
is factored into a separate
function that is used in aarch64_set_current_function and the pragma handling 
function to tell the midend
when to reinitialise its structures.

This patch fixes the ICE, the testcase attached, and passes bootstrap and 
regression testing on
aarch64-none-linux-gnu.

Sorry for missing the ICE originally.
Ok for trunk?

Thanks,
Kyrill

P.S. The vector arguments re-layout hack that was in 
aarch64_set_current_function is removed because
it has become unneeded since Christian fixed some midend bugs so that it's done 
automatically there.

2016-03-03  Kyrylo Tkachov  

PR target/70002
* config/aarch64/aarch64-protos.h
(aarch64_save_restore_target_globals): New prototype.
* config/aarch64/aarch64-c.c (aarch64_pragma_target_parse):
Call the above when popping pragma.
* config/aarch64/aarch64.c (aarch64_save_restore_target_globals):
New function.
(aarch64_set_current_function): Rewrite using the above.

2016-03-03  Kyrylo Tkachov  

PR target/70002
PR target/69245
* gcc.target/aarch64/pr69245_2.c: New test.
diff --git a/gcc/config/aarch64/aarch64-c.c b/gcc/config/aarch64/aarch64-c.c
index 3590ae0daa5d80050b0f81cd6ab9a7779f463516..e057daaec24c0add673d0b2c776d4c4c43d1f0ea 100644
--- a/gcc/config/aarch64/aarch64-c.c
+++ b/gcc/config/aarch64/aarch64-c.c
@@ -178,6 +178,12 @@ aarch64_pragma_target_parse (tree args, tree pop_target)
 
   cpp_opts->warn_unused_macros = saved_warn_unused_macros;
 
+  /* If we're popping or reseting make sure to update the globals so that
+ the optab availability predicates get recomputed.  */
+  if (pop_target)
+aarch64_save_restore_target_globals (pop_target);
+
+
   /* Initialize SIMD builtins if we haven't already.
  Set current_target_pragma to NULL for the duration so that
  the builtin initialization code doesn't try to tag the functions
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index e4e49fc9ccc3d568c84b35c1a0c0733475017cca..c40d2b0c78494b50508c1b5135b8ee7676a61631 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -361,6 +361,7 @@ void aarch64_emit_call_insn (rtx);
 void aarch64_register_pragmas (void);
 void aarch64_relayout_simd_types (void);
 void aarch64_reset_previous_fndecl (void);
+void aarch64_save_restore_target_globals (tree);
 void aarch64_emit_approx_rsqrt (rtx, rtx);
 
 /* Initialize builtins for SIMD intrinsics.  */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1e10d9798ddc5f5d2aac4255d3a8fe4ecaf1402a..a05160e08d0474ed9c1e2afa1d00375839417034 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8570,6 +8570,21 @@ aarch64_reset_previous_fndecl (void)
   aarch64_previous_fndecl = NULL;
 }
 
+/* Restore or save the TREE_TARGET_GLOBALS from or to NEW_TREE.
+   Used by aarch64_set_current_function and aarch64_pragma_target_parse to
+   make sure optab availability predicates are recomputed when necessary.  */
+
+void
+aarch64_save_restore_target_globals (tree new_tree)
+{
+  if (TREE_TARGET_GLOBALS (new_tree))
+restore_target_globals (TREE_TARGET_GLOBALS (new_tree));
+  else if (new_tree == target_option_default_node)
+restore_target_globals (_target_globals);
+  else
+TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts ();
+}
+
 /* Implement TARGET_SET_CURRENT_FUNCTION.  Unpack the codegen decisions
like tuning and ISA features from the DECL_FUNCTION_SPECIFIC_TARGET
of the function, if such exists.  This function may be called multiple
@@ -8579,63 +8594,32 @@ aarch64_reset_previous_fndecl (void)
 static void
 aarch64_set_current_function (tree fndecl)
 {
+  if (!fndecl || fndecl == aarch64_previous_fndecl)
+return;
+
   tree old_tree = (aarch64_previous_fndecl
 		   ? DECL_FUNCTION_SPECIFIC_TARGET (aarch64_previous_fndecl)
 		   : NULL_TREE);
 
-  tree new_tree = (fndecl

Re: [PATCH] Fix ICE with vector types in X % -Y pattern (PR middle-end/70050)

[Jakub Jelinek]

On Thu, Mar 03, 2016 at 12:04:31PM +0100, Marek Polacek wrote:
> We crashed on the attached testcase because we were trying to apply the
> X % -Y -> X % Y transformation even on vectors.  That doesn't go well with the
> check for !TYPE_UNSIGNED.  So I think let's limit the pattern to only work on
> integral types.

I think TYPE_UNSIGNED is fine, but TYPE_MIN_VALUE is not.

> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2016-03-03  Marek Polacek  
> 
>   PR middle-end/70050
>   * match.pd (X % -Y): Add INTEGRAL_TYPE_P check.
> 
>   * gcc.dg/pr70050.c: New test.

Ok, thanks.

Jakub

Re: [PATCH 1/2][GCC][ARM] Add support for Cortex-R8

[Kyrill Tkachov]

On 03/03/16 11:31, Kyrill Tkachov wrote:


On 03/03/16 11:28, Kyrill Tkachov wrote:

Hi Andre,

On 02/03/16 12:20, Andre Vieira (lists) wrote:

gcc/ChangeLog:

2016-03-02  Andre Vieira 

  * config/arm/arm-cores.def (cortex-r8): New.
  * config/arm/arm-tables.opt (cortex-r8): New.
  * config/arm/arm-tune.md: Regenerate.
  * gcc/doc/invoke.texi: Add cortex-r8 to list of cpu values.




One nit I just noticed.
The arm-tables.opt entry should say "Renerate" as it's auto-generated from
arm-cores.def.



Of course, that should say "Regenerate."
Sorry, fingers slipping :(


Ok with that change to the ChangeLog

Kyrill


Ok.
Thanks,
Kyrill

Re: Reinstate generic stack checking warning with LRA

[Eric Botcazou]

> Anyway, looking at pro_and_epilogue dumps, with additional
> -fstack-protector-strong we decrease sp only by 4176, while without it by
> 8224 (on x86_64; the testcase fails on all targets I've tried so far
> ({x86_64,i686,powerpc64{,le},s390{,x},aarch64,armv7hl}-linux).

Yeah, the threshold is around 4KB, feel free to add a few more HUNDREDs.

-- 
Eric Botcazou

Re: [PATCH 1/2][GCC][ARM] Add support for Cortex-R8

[Kyrill Tkachov]

On 03/03/16 11:28, Kyrill Tkachov wrote:

Hi Andre,

On 02/03/16 12:20, Andre Vieira (lists) wrote:

gcc/ChangeLog:

2016-03-02  Andre Vieira  

  * config/arm/arm-cores.def (cortex-r8): New.
  * config/arm/arm-tables.opt (cortex-r8): New.
  * config/arm/arm-tune.md: Regenerate.
  * gcc/doc/invoke.texi: Add cortex-r8 to list of cpu values.




One nit I just noticed.
The arm-tables.opt entry should say "Renerate" as it's auto-generated from
arm-cores.def.

Ok with that change to the ChangeLog

Kyrill


Ok.
Thanks,
Kyrill

Re: [PATCH 2/2][GCC][ARM] Fix testcases after introduction of Cortex-R8

[Kyrill Tkachov]

Hi Andre,

On 02/03/16 12:21, Andre Vieira (lists) wrote:

Hi,

Tests used to check for "r8" which will not work because cortex-r8
string is now included in the assembly. Fixed by checking for "[^\-]r8".

Is this Ok?

Cheers,
Andre

gcc/testsuite/ChangeLog:

2016-03-02  Andre Vieira  

  * gcc.target/arm/pr45701-1.c: Change assembler scan to not
  trigger for cortex-r8, when scanning for register r8.
  * gcc.target/arm/pr45701-2.c: Likewise.


Ok.
Thanks,
Kyrill

Re: [PATCH 1/2][GCC][ARM] Add support for Cortex-R8

[Kyrill Tkachov]

Hi Andre,

On 02/03/16 12:20, Andre Vieira (lists) wrote:

gcc/ChangeLog:

2016-03-02  Andre Vieira  

  * config/arm/arm-cores.def (cortex-r8): New.
  * config/arm/arm-tables.opt (cortex-r8): New.
  * config/arm/arm-tune.md: Regenerate.
  * gcc/doc/invoke.texi: Add cortex-r8 to list of cpu values.


Ok.
Thanks,
Kyrill

Re: [PATCH][wwwdocs] Remove (Pending) tag from 5.3 notes, add 5.4 entry

[Kyrill Tkachov]

On 03/03/16 11:14, Gerald Pfeifer wrote:

On Thu, 3 Mar 2016, Kyrill Tkachov wrote:

Ok to commit?


Richi already approved, so this is only for future cases:
Please do consider changes like this either as trivial (and go ahead, just 
posting the patch) or pre-approved by me (and go ahead, just posting the 
patch).  As you prefer. ;-)



Thanks Gerald, I'll keep it in mind.

Kyrill


Thanks,
Gerald

Re: [PATCH][wwwdocs] Remove (Pending) tag from 5.3 notes, add 5.4 entry

[Gerald Pfeifer]

On Thu, 3 Mar 2016, Kyrill Tkachov wrote:

Ok to commit?


Richi already approved, so this is only for future cases:  

Please do consider changes like this either as trivial (and go 
ahead, just posting the patch) or pre-approved by me (and go 
ahead, just posting the patch).  As you prefer. ;-)


Thanks,
Gerald

[PATCH] Fix ICE with vector types in X % -Y pattern (PR middle-end/70050)

[Marek Polacek]

We crashed on the attached testcase because we were trying to apply the
X % -Y -> X % Y transformation even on vectors.  That doesn't go well with the
check for !TYPE_UNSIGNED.  So I think let's limit the pattern to only work on
integral types.

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

2016-03-03  Marek Polacek  

PR middle-end/70050
* match.pd (X % -Y): Add INTEGRAL_TYPE_P check.

* gcc.dg/pr70050.c: New test.

diff --git gcc/match.pd gcc/match.pd
index 5903782..112deb3 100644
--- gcc/match.pd
+++ gcc/match.pd
@@ -293,7 +293,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 /* X % -Y is the same as X % Y.  */
 (simplify
  (trunc_mod @0 (convert? (negate @1)))
- (if (!TYPE_UNSIGNED (type)
+ (if (INTEGRAL_TYPE_P (type)
+  && !TYPE_UNSIGNED (type)
   && !TYPE_OVERFLOW_TRAPS (type)
   && tree_nop_conversion_p (type, TREE_TYPE (@1))
   /* Avoid this transformation if X might be INT_MIN or
diff --git gcc/testsuite/gcc.dg/pr70050.c gcc/testsuite/gcc.dg/pr70050.c
index e69de29..610456f 100644
--- gcc/testsuite/gcc.dg/pr70050.c
+++ gcc/testsuite/gcc.dg/pr70050.c
@@ -0,0 +1,11 @@
+/* PR middle-end/70025 */
+/* { dg-do compile } */
+/* { dg-options "-Wno-psabi" } */
+
+typedef int v8si __attribute__ ((vector_size (32)));
+
+v8si
+foo (v8si v)
+{
+  return v %= -v;
+}

Marek

Re: [Ping^2][PATCH][GCC-5] Fix "#pragma GCC pop_options" warning.

[Andre Vieira (lists)]

On 29/02/16 10:47, Andre Vieira (lists) wrote:
> On 15/02/16 10:33, Andre Vieira (lists) wrote:
>> On 18/01/16 11:04, Andre Vieira (lists) wrote:
>>> Hi there,
>>>
>>> Can we have the "#pragma GCC pop_options" fix backported to GCC-5?
>>>
>>> Patch found in https://gcc.gnu.org/ml/gcc-patches/2015-10/msg01261.html
>>> and was committed in r228794.
>>>
>>> The same patch applies cleanly to gcc-5, which would otherwise not be
>>> able to use this pragma even though the support is there.
>>>
>>> Cheers,
>>> Andre
>>>
>>
>> Ping.
>>
> Ping.
> 
I understood it was a good idea to CC the appropriate maintainer on
this, so adding Bernd Schmidt to the CC.

Sorry for the noise.

Cheers,
Andre

Re: [committed] Fix libffi/70024

[Rainer Orth]

Hi Richard,

> As discussed in the PR, let's take the opportunity while bumping the soname
> to add symbol versioning.

great idea: I'd already suggested this back in 2010 when doing the bulk
of the Solaris symbol versioning work

http://gcc.gnu.org/ml/gcc/2010-02/msg00339.html
http://sourceware.org/ml/libffi-discuss/2010/msg00045.html

but there never was a conclusion on the questions I'd raised.

> Versioning is complicated by the fact that there are several pieces of API
> that are "optional" based on the target.  If these optional pieces are
> later implemented by targets that currently do not, we can't have these
> extra symbols suddenly appear in the base version -- that voids the promise
> that symbol versioning makes.
>
> So the symbols are split among 4 different versions.  Each version's set of
> symbols is either entirely present[*] or entirely absent.  Programs using
> the library will only depend on the subset of versions that they use.
>
> Tested on {x86_64,ppc64,aarch64}-linux.  Compile tested on mips64el-linux,
> which doesn't implement go closures or complex types.

Unfortunately, the patch broke Solaris bootstrap:

make[2]: Entering directory 
'/var/gcc/regression/trunk/12-gcc/build/i386-pc-solaris2.12/libffi'
Makefile:1906: *** missing separator (did you mean TAB instead of 8 spaces?).  
Stop.

Fixed as obvious like this, will commit shortly:

2016-03-03  Rainer Orth  

* Makefile.am (libffi.map-sun): Tabify:
* Makefile.in: Regenerate.

# HG changeset patch
# Parent  3195cd69f93aa48e3c342ea80e6f2660e30f33da
Tabify libffi/Makefile.am

diff --git a/libffi/Makefile.am b/libffi/Makefile.am
--- a/libffi/Makefile.am
+++ b/libffi/Makefile.am
@@ -214,11 +214,11 @@ libffi_version_script = -Wl,-M,libffi.ma
 libffi_version_dep = libffi.map-sun
 libffi.map-sun : libffi.map $(top_srcdir)/../contrib/make_sunver.pl \
 $(libffi_la_OBJECTS) $(libffi_la_LIBADD)
-perl $(top_srcdir)/../contrib/make_sunver.pl libffi.map \
-  $(libffi_la_OBJECTS:%.lo=.libs/%.o) \
- `echo $(libffi_la_LIBADD) | \
-sed 's,\([^/]*\)\.l\([ao]\),.libs/\1.\2,g'` \
- > $@ || (rm -f $@ ; exit 1)
+	perl $(top_srcdir)/../contrib/make_sunver.pl libffi.map \
+	  $(libffi_la_OBJECTS:%.lo=.libs/%.o) \
+	 `echo $(libffi_la_LIBADD) | \
+	sed 's,\([^/]*\)\.l\([ao]\),.libs/\1.\2,g'` \
+	 > $@ || (rm -f $@ ; exit 1)
 endif
 else
 libffi_version_script =

Unfortunately, even with this fixed, all Solaris/x86 tests now fail to
link:

FAIL: libffi.call/closure_fn0.c -W -Wall -Wno-psabi -O0 (test for excess errors)
Excess errors:
Undefined   first referenced
 symbol in file
ffi_closure_alloc   /var/tmp//ccIpq3qc.o
ffi_type_float  /var/tmp//ccIpq3qc.o
ffi_type_uint64 /var/tmp//ccIpq3qc.o
ffi_type_sint32 /var/tmp//ccIpq3qc.o
ffi_type_sint16 /var/tmp//ccIpq3qc.o
ffi_type_double /var/tmp//ccIpq3qc.o
ffi_prep_cif/var/tmp//ccIpq3qc.o
ld: fatal: symbol referencing errors

> For gcc7, we really should pull out these m4 macros to new config/ files.
> I didn't really want to touch anything except libffi for now.

Great idea: I'd meant to do this for a long time, but never got around
to it.

Rainer

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

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

[Alan Lawrence]

On 25/02/16 18:00, Alan Lawrence wrote:

On 22/02/16 12:03, Jakub Jelinek wrote:

(f) A global command-line option, which we check alongside DECL_COMMON and
further tests (basically, we want only DECL_COMMON decls that either have
ARRAY_TYPE, or some other aggregate type with flexible array member or some
other trailing array in the struct), and use the resulting predicate in
places where we optimize based on DECL_SIZE{,_UNIT} of the decl - if that
predicate returns true, we assume the DECL_SIZE is
"variable"/"unlimited"/whatever.
The two spots known to me that would need to use this new predicate would
be:
tree.c (array_at_struct_end_p):

[...]

tree-dfa.c (get_ref_base_and_extent):

[...]

Close enough to what I meant by (a)/(b), but never mind that, and I hadn't
looked at where the change for aggressive loop opts needed to be. However...
Well you are essentially writing the patch for me at this point (so, thanks!),
  but here's a patch that puts that under a global -funknown-commons flag.
Testcase as before.

Bootstrapped (with flag overridden to true) on x86_64 and aarch64, check-gcc,
check-fortran, and tested 416.gamess on AArch64, where this patch enables
running *without* the -fno-aggressive-loop-opts previously required.

In the gcc testsuite, these fail with the option turned on:
gcc.dg/pr53265.c  (test for warnings, line 137)
gcc.dg/pr53265.c  (test for warnings, line 138)
gcc.dg/pr64277.c scan-tree-dump cunroll ... (*2)
gcc.dg/tree-ssa/cunroll-{1,2,3,4,9,10,11}.c scan-tree-dump 
cunroll/cunrolli/ivcanon (various)
gcc.dg/tree-ssa/loop-38.c scan-tree-dump cunrolli "Loop 1 iterates at most 11 
times"
...which all appear harmless.

Thomas Koenig suggests (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69368#c80)
that this be combined with some flag fiddling and warnings in the Fortran
front-end; this patch doesn't do that, as I'm not very familiar with the
frontends, but that can follow in a separate patch. (Thomas?)

OK for trunk?

Cheers, Alan

gcc/ChangeLog:

DATE  Alan Lawrence  
   Jakub Jelinek  

* common.opt (funknown-commons, flag_unknown_commons): New.
* tree.c (array_at_struct_end_p): Do not limit to size of decl for
DECL_COMMONS if flag_unknown_commons is set.
* tree-dfa.c (get_ref_base_and_extent): Likewise.

gcc/testsuite/ChangeLog:

* gfortran.dg/unknown_commons.f: New.


Ping.

(Besides my original patch 
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01356.html which we decided was 
too fragile, I believe this is the only patch proposed that actually fixes the 
miscompare in 416.gamess.)


Thanks, Alan

Re: Fix/work around PR57676, LRA terminates prematurely

[Richard Biener]

On Wed, Mar 2, 2016 at 7:39 PM, Bernd Schmidt  wrote:
> On 02/24/2016 11:01 PM, Jeff Law wrote:
>>
>> As Vlad noted, the test is definitely a pathological case.  I think
>> Bernd's patch is a very reasonable approach to address the current
>> problem.  Namely that LRA can be making progress on a pathological
>> testcase, but it gets terminated by the anti-looping clamp.  The clamp
>> itself was put in place to catch bugs in LRA or ports that are in the
>> process of converting to LRA.
>
>
> Richard, are you upholding your objection?

No.

Richard.

>
> Bernd
>

Re: [PATCH]Replace -shared with -r -nostdlib in gcc.dg/lto/pr61526 pr54709 pr64415 test cases.

[Richard Biener]

On Wed, Mar 2, 2016 at 5:12 PM, Renlin Li  wrote:
> Hi Richard,
>
>
> On 02/03/16 13:35, Richard Biener wrote:
>>
>> On Tue, Mar 1, 2016 at 4:56 PM, Renlin Li  wrote:
>>>
>>> Hi Richard,
>>>
>>>
>>> On 01/03/16 09:16, Richard Biener wrote:

 On Mon, Feb 29, 2016 at 5:13 PM, Renlin Li 
 wrote:
>
> Hi all,
>
> The gcc.dg/lto/pr54709, pr61526, pr64415 linking testcases keep failing
> on
> arm/aarch64 bare-metal target.
>
> It's because statically built newlib library is used to link with
> shared
> object.
> And the linker complains about relocations which cannot be used in
> shared object.
>
> For example, the following errors are produced:
>
> crtbegin.o: relocation R_ARM_MOVW_ABS_NC against `a local symbol' can
> not
> be
> used when making a shared object; recompile with -fPIC
>
> crtbegin.o: relocation R_ARM_THM_MOVW_ABS_NC against `a local symbol'
> can
> not
> be used when making a shared object; recompile with -fPIC
>
> librdimon.a(rdimon-syscalls.o): relocation R_AARCH64_ADR_PREL_PG_HI21
> against
> external symbol `_impure_ptr' can not be used when making a shared
> object;
> recompile with -fPIC
>
> Presumably, bare-metal toolchain for other architecture have those test
> case
> failures as well?
>
> In this patch, -shared option is replace by -r -nostdlib. So that the
> standard
> system startup files or libraries are not used when linking.

 Note that -shared is not equivalent to -r -nostdlib so please verify
 that
 the
 original issue can be still reproduced with its fix reverted but -r
 -nostdlib
 used with the new -r -nostdlib handling on trunk.
>>>
>>>
>>> pr54709_0.c: Cannot be reproduced with even -shared. The error message is
>>> the same as shown above.
>>> pr64415_0.c: Reproduced with "-r -nostdlib".
>>> pr61526_0.c: Reproduced with "-r -nostdlib".
>>>
>>> By the way, those linking test cases all pass for linux toolchain. Only
>>> fail
>>> for aarch64/arm baremetal toolchain.
>>>
>>> Andrew, I saw you have done similar things in r153555
>>> https://gcc.gnu.org/viewcvs/gcc?view=revision=153555
>>>
>>> Do you have any thoughts?
>>>
>>> And also here, the last comments in this ticket suggests to add
>>> check_effective_target_shared to the exp file to limit it to linux
>>> targets
>>> only:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61526
>>
>> As said LTO testcases tend to be somewhat fragile so limiting them to
>> targets known to work might be the best option.
>>
>> Richard.
>
>
> Forgot the mention that, by purely adding "-nostdlib" option (instead of
> replacing -shared)
> fixes the failures as well.
>
> I test those test cases again with fix reverted, keep "-shared" option, add
> "-nostdlib" option.

Ok, so I discovered we have a "shared" target which means if a target doesn't
support shared libs we can guard against it with using

/* { dg-require-effective-target shared } */

does adding that to the three testcases fix the issue for you?

Thanks,
Richard.

> pr54709_0.c: Cannot be reproduced even with test case unmodified.
> The error message is the same as shown above. with "-nostdlib", no failure.
>
> pr64415_0.c: Reproduced.
> pr61526_0.c: Reproduced.
>
> Regards,
> Renlin
>
>
>
>>
>>> Regards,
>>> Renlin
>>>
>>>
 Otherwise simply dg-skip for aarch64.

 Richard.

> arm-none-eabi, aarch64-none-elf regression test OK, OK for trunk?
>
> Regards,
> Renlin Li
>
> gcc/testsuite/ChangeLog:
>
> 2016-02-29  Renlin Li
>
>   * gcc.dg/lto/pr54709_0.c: Replace -shard with -r -nostdlib.
>   * gcc.dg/lto/pr61526_0.c: Ditto.
>   * gcc.dg/lto/pr64415_0.c: Ditto.
>
>

Re: [PATCH][wwwdocs] Remove (Pending) tag from 5.3 notes, add 5.4 entry

[Richard Biener]

On Thu, Mar 3, 2016 at 10:49 AM, Kyrill Tkachov
 wrote:
> Hi all,
>
> I noticed that changes.html for GCC 5 has an entry for GCC 5.3 saying
> (Pending) and linking to the fixed PRs.
> 5.3 has already been released, so this patch removes it from there, and
> instead adds a similar entry for the pending 5.4 release.
>
> Ok to commit?

Ok.

Richard.

> Thanks,
> Kyrill

Re: [testsuite] Invoke gdb with -batch to avoid prompts

[Rainer Orth]

Mike Stump  writes:

> On Mar 1, 2016, at 6:20 AM, Rainer Orth  wrote:
>> When switching from gdb 7.10 to the newly released gdb 7.11 on Solaris,
>> all simulate-thread tests started to timeout
>
> Ok.  If a domain expert prefers a different strategy, I’m fine with
> anything better as well.

Given that there were neither objections nor better suggestions, I've
installed the patch everywhere.

Thanks.
Rainer

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

Re: [Patch testsuite] Change xfail conditions for bb-slp-34.c

[Richard Biener]

On Thu, 3 Mar 2016, James Greenhalgh wrote:

> 
> Hi,
> 
> ARM and AArch64 will still vectorize bb-slp-34.c - we're not operating
> with a cost model so we vectorize to a 64-bit vector of two ints, and the
> permutes are just element swaps.
> 
> So, don't mark this test xfail for arm/aarch64.
> 
> Checked on x86_64-none-linux-gnu, arm-none-eabi and aarch64-none-elf with
> no issues.
> 
> OK?

Ok.  Indeed with using V2SI vectors the vectorization is valid. 

Richard.

Re: [wwwdocs] Note for pr70024 for gcc-5

[Richard Biener]

On Thu, Mar 3, 2016 at 3:09 AM, Richard Henderson  wrote:
> Is there anything else we should say?
>
> I thought about recommending that distributions not install the libffi
> shared library from gcc and instead use upstream source.  But that doesn't
> really help one way or the other, so I dropped that language.

I wonder if it should go to the Caveats section but otherwise looks ok (noting
the breakage is on aarch64 only).

Thanks,
Richard.

>
> r~

Re: [committed] Fix libffi/70024

[Richard Biener]

On Thu, Mar 3, 2016 at 3:13 AM, Richard Henderson  wrote:
> [ Ho hum.  Re-send due to spam rejection.
>   Trying again with compressed patch.  ]
>
>
> As discussed in the PR, let's take the opportunity while bumping the soname
> to add symbol versioning.
>
> Versioning is complicated by the fact that there are several pieces of API
> that are "optional" based on the target.  If these optional pieces are later
> implemented by targets that currently do not, we can't have these extra
> symbols suddenly appear in the base version -- that voids the promise that
> symbol versioning makes.
>
> So the symbols are split among 4 different versions.  Each version's set of
> symbols is either entirely present[*] or entirely absent.  Programs using
> the library will only depend on the subset of versions that they use.
>
> Tested on {x86_64,ppc64,aarch64}-linux.  Compile tested on mips64el-linux,
> which doesn't implement go closures or complex types.
>
> For gcc7, we really should pull out these m4 macros to new config/ files.
> I didn't really want to touch anything except libffi for now.

Thanks for doing this.  I suppose this is also upstream now?

Richard.

>
> r~
>
>
> [*] Except for i386, where FFI_NATIVE_RAW_API modifies the API, suppressing
> ffi_java_raw_call, ffi_prep_java_raw_closure, ffi_prep_java_raw_closure_loc.
>
> IMO it's a libffi bug that these symbols are suppressed; they could just as
> easily have been aliases for other symbols within libffi.  But it's too late
> now.  Instead, users of libffi (e.g. libjava) are already checking
> FFI_NATIVE_RAW_API and adjusting the function called.
>
> Anyway, all is well so long as no target changes to define
> FFI_NATIVE_RAW_API that hasn't previously.  I'm going to assume that i386 is
> unique and no other target will ever define FFI_NATIVE_RAW_API.