Re: [PATCH] c++: Diagnose when "requires" is used instead of "requires requires" [PR94306]

[Jason Merrill via Gcc-patches]

On 3/26/20 11:49 PM, Patrick Palka wrote:

On Thu, 26 Mar 2020, Jason Merrill wrote:


On 3/26/20 6:36 PM, Patrick Palka wrote:

On Thu, 26 Mar 2020, Jason Merrill wrote:


On 3/26/20 5:28 PM, Patrick Palka wrote:

This adds support to detect and recover from the case where an opening
brace
immediately follows the start of a requires-clause.  So rather than
emitting
the
error

 error: expected primary-expression before '{' token

followed by a slew of irrevelant errors, we now assume the user had
intended
to
write "requires requires {" and diagnose and recover accordingly.

Tested on x86_64-pc-linux-gnu, does this look OK?

gcc/cp/ChangeLog:

PR c++/94306
* parser.c (cp_parser_requires_clause_opt): Diagnose and recover from
"requires {" when "requires requires {" was probably intended.

gcc/testsuite/ChangeLog:

PR c++/94306
* g++.dg/concepts/diagnostic8.C: New test.
---
gcc/cp/parser.c | 17 -
gcc/testsuite/g++.dg/concepts/diagnostic8.C |  6 ++
2 files changed, 22 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 05363653691..73c2c2cb010 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser
*parser,
bool lambda_p)
}
  return NULL_TREE;
}
-  cp_lexer_consume_token (parser->lexer);
+
+  cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2);
+  if (tok2->type == CPP_OPEN_BRACE)


Do we want to handle an open paren the same way?


Hmm, wouldn't the error recovery be significantly different for an open
paren here?  Just pretending that there's another "requires" token would
mean we proceed as if we got "requires requires (" but that is still
ill-formed I think.


Not ill-formed; a requires-expression can have a parameter-list.


Ah, of course..  But then again, wouldn't "requires (" be not
necessarily ill-formed, since the open paren could possibly be a part of
the constraint-expression e.g. "requires (a || b)"?


Ah, yes.


We could perhaps look ahead of the open paren and check if we match the
ill-formed pattern "requires (...) {...}" and if so, handle the open
paren the same way.  Oh, but even that pattern is not necessarily
ill-formed, at least when parsing a lambda: the following
 "[]  () -> void requires (true) { }"
is apparently a well-formed lambda-expression whose tokens starting from
its requires-clause would match that pattern.  So we would probably have
to condition this open-paren handling on !lambda_p.


Yeah, this is too much trouble.  Your patch is OK as is.






+{
+  /* An opening brace following the start of a requires-clause is
+ill-formed; the user likely forgot the second `requires' that
+would start a requires-expression.  */
+  gcc_rich_location richloc (tok2->location);
+  richloc.add_fixit_insert_before (" requires");
+  error_at (, "missing additional % to start "
+   "a requires-expression");
+  /* Don't consume the `requires', so that it's reused as the start
of
a
+requires-expression.  */
+}
+  else
+cp_lexer_consume_token (parser->lexer);
if (!flag_concepts_ts)
return cp_parser_requires_clause_expression (parser, lambda_p);
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic8.C
b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
new file mode 100644
index 000..70d7e4a9cc1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
@@ -0,0 +1,6 @@
+// PR c++/94306
+// { dg-do compile { target c++2a } }
+
+template struct S { };
+template requires { typename T::type; } struct S { };
+// { dg-error "missing additional .requires." "" { target *-*-* } .-1 }

Re: [PATCH 2/2] c++: Respect current_constraint_diagnosis_depth in diagnose_compound_requirement

[Patrick Palka via Gcc-patches]

On Mon, 9 Mar 2020, Patrick Palka wrote:

> The first patch tries to avoid changing our current default diagnostics.  But
> for the sake of consistency we arguably should also respect
> current_constraint_diagnosis_depth in diagnose_compound_requirement() like we 
> do
> in the other error-replaying diagnostic routine.  But doing so would be a 
> change
> to our default diagnostics behavior, so the change has been split out into 
> this
> separate patch for separate consideration.

Ping.  Here's a rebased version of this patch.

-- >8 --

gcc/cp/ChangeLog:

* constraint.cc (diagnose_compound_requirement): When diagnosing a
compound requirement, maybe replay the satisfaction failure, subject to
the current diagnosis depth.

gcc/testsuite/ChangeLog:

* g++.dg/concepts/diagnostic1.C: Pass -fconcepts-diagnostics-depth=2.
* g++.dg/concepts/diagnostic5.C: Adjust expected diagnostics.
* g++.dg/cpp2a/concepts-requires5.C: Pass
-fconcepts-diagnostics-depth=2.
---
 gcc/cp/constraint.cc  | 28 +--
 gcc/testsuite/g++.dg/concepts/diagnostic1.C   |  1 +
 gcc/testsuite/g++.dg/concepts/diagnostic5.C   |  5 +---
 gcc/testsuite/g++.dg/cpp2a/concepts-iconv1.C  |  1 +
 .../g++.dg/cpp2a/concepts-requires5.C |  2 +-
 5 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 76c520318c3..571c7cbdd38 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -3308,20 +3308,30 @@ diagnose_compound_requirement (tree req, tree args, 
tree in_decl)
  if (!type_deducible_p (expr, type, placeholder, args, quiet))
{
  tree orig_expr = TREE_OPERAND (req, 0);
- inform (loc, "%qE does not satisfy return-type-requirement",
- orig_expr);
-
- /* Further explain the reason for the error.  */
- type_deducible_p (expr, type, placeholder, args, noisy);
+ if (diagnosing_failed_constraint::replay_errors_p ())
+   {
+ inform (loc,
+ "%qE does not satisfy return-type-requirement, "
+ "because", orig_expr);
+ /* Further explain the reason for the error.  */
+ type_deducible_p (expr, type, placeholder, args, noisy);
+   }
+ else
+   inform (loc, "%qE does not satisfy return-type-requirement",
+   orig_expr);
}
}
   else if (!expression_convertible_p (expr, type, quiet))
{
  tree orig_expr = TREE_OPERAND (req, 0);
- inform (loc, "cannot convert %qE to %qT", orig_expr, type);
-
- /* Further explain the reason for the error.  */
- expression_convertible_p (expr, type, noisy);
+ if (diagnosing_failed_constraint::replay_errors_p ())
+   {
+ inform (loc, "cannot convert %qE to %qT because", orig_expr, 
type);
+ /* Further explain the reason for the error.  */
+ expression_convertible_p (expr, type, noisy);
+   }
+ else
+   inform (loc, "cannot convert %qE to %qT", orig_expr, type);
}
 }
 }
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic1.C 
b/gcc/testsuite/g++.dg/concepts/diagnostic1.C
index 7da08db2792..c6589e2e671 100644
--- a/gcc/testsuite/g++.dg/concepts/diagnostic1.C
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic1.C
@@ -1,6 +1,7 @@
 // PR c++/67159
 // { dg-do compile { target c++17_only } }
 // { dg-options "-fconcepts" }
+// { dg-additional-options "-fconcepts-diagnostics-depth=2" }
 
 template 
 concept bool SameAs = __is_same_as(T, U);
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C 
b/gcc/testsuite/g++.dg/concepts/diagnostic5.C
index 2641dc18423..0d890d6f548 100644
--- a/gcc/testsuite/g++.dg/concepts/diagnostic5.C
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C
@@ -4,8 +4,7 @@
 template
   concept c1 = requires { typename T::blah; };
 // { dg-message "satisfaction of .c1. .with T = char." "" { target *-*-* } 
.-1 }
-// { dg-message "satisfaction of .c1." "" { target *-*-* } .-2 }
-// { dg-message ".typename T::blah. is invalid" "" { target *-*-* } .-3 }
+// { dg-message ".typename T::blah. is invalid" "" { target *-*-* } .-2 }
 
 template
   concept c2 = requires (T x) { *x; };
@@ -27,8 +26,6 @@ template
   concept c5 = requires (T x) { {  } -> c1; };
 // { dg-message "satisfaction of .c5. .with T = char." "" { target *-*-* } 
.-1 }
 // { dg-message "in requirements with .char x." "" { target *-*-* } .-2 }
-// { dg-message "does not satisfy return-type-requirement" "" { target *-*-* } 
.-3 }
-// { dg-error "deduced expression type does not satisfy" "" { target *-*-* } 
.-4 }
 
 template
   requires (c1 || c2) || (c3 || c4) || c5 // { dg-message "49: 
no operand" }
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-iconv1.C 

Re: [PATCH 1/2] c++: Replay errors during diagnosis of constraint satisfaction failures

[Patrick Palka via Gcc-patches]

On Mon, 16 Mar 2020, Patrick Palka wrote:

> Hi Martin,
> 
> On Sun, 15 Mar 2020, Martin Sebor wrote:
> 
> > On 3/11/20 4:18 PM, Patrick Palka via Gcc-patches wrote:
> > ...
> > > Hmm, like this?  This version introduces a new static member function
> > > diagnosing_failed_constraint::replay_errors_p that checks
> > > current_constraint_diagnosis_depth, and also sets max_depth_exceeded_p
> > > when appropriate.
> > > 
> > ...
> > 
> > Just one small quoting nit:
> > 
> > 
> > > @@ -3368,11 +3464,25 @@ diagnose_constraints (location_t loc, tree t, tree
> > > args)
> > >   {
> > > inform (loc, "constraints not satisfied");
> > >   +  if (concepts_diagnostics_max_depth == 0)
> > > +return;
> > > +
> > > /* Replay satisfaction, but diagnose errors.  */
> > > if (!args)
> > >   constraint_satisfaction_value (t, tf_warning_or_error);
> > > else
> > >   constraint_satisfaction_value (t, args, tf_warning_or_error);
> > > +
> > > +  static bool suggested_p;
> > > +  if (concepts_diagnostics_max_depth_exceeded_p
> > > +  && current_constraint_diagnosis_depth == 0
> > > +  && !suggested_p)
> > > +{
> > > +  inform (UNKNOWN_LOCATION,
> > > +   "set -fconcepts-diagnostics-depth= to at least %d for more
> > > detail",
> > > +   concepts_diagnostics_max_depth + 1);
> > 
> > 
> > Can you please quote the option name in the diagnostic (e.g., by using
> > "set %qs to...", "-fconcepts-diagnostics-depth=" or equivalent) to avoid
> > -Wformat-diag?  It won't cause errors (yet) but will eventually need to
> > be cleaned up.
> 
> Yes sure, consider it done.  I've amended the patch locally with this
> change.

Here's a rebased version of this patch with the above diagnostic change,
no other changes made (aside from a minor adjustment to diagnostic5.C):

-- >8 --

gcc/c-family/ChangeLog:

* c.opt: Add -fconcepts-diagnostics-depth.

gcc/cp/ChangeLog:

* constraint.cc (finish_constraint_binary_op): Set the location of EXPR
as well as its range, because build_x_binary_op doesn't always do so.
(current_constraint_diagnosis_depth): New.
(concepts_diagnostics_max_depth_exceeded_p): New.
(collect_operands_of_disjunction): New.
(satisfy_disjunction): When diagnosing a satisfaction failure, maybe
replay each branch of the disjunction, subject to the current diagnosis
depth.
(diagnose_valid_expression): When diagnosing a satisfaction failure,
maybe replay the substitution error, subject to the current diagnosis
recursion.
(diagnose_valid_type): Likewise.
(diagnose_nested_requiremnet): Likewise.
(diagnosing_failed_constraint::diagnosing_failed_constraint): Increment
current_constraint_diagnosis_depth when diagnosing.
(diagnosing_failed_constraint::~diagnosing_failed_constraint): Decrement
current_constraint_diagnosis_depth when diagnosing.
(diagnosing_failed_constraint::replay_errors_p): New static member
function.
(diagnose_constraints): Don't diagnose if concepts_diagnostics_max_depth
is 0.  Emit a one-off note to increase -fconcepts-diagnostics-depth if
the limit was exceeded.
* cp-tree.h (diagnosing_failed_constraint::replay_errors_p): Declare.

gcc/testsuite/ChangeLog:

* g++.dg/concepts/diagnostic2.C: Expect "no operand" instead of
"neither operand".
* g++.dg/concepts/diagnostic5.C: New test.
---
 gcc/c-family/c.opt  |   4 +
 gcc/cp/constraint.cc| 150 +---
 gcc/cp/cp-tree.h|   1 +
 gcc/testsuite/g++.dg/concepts/diagnostic2.C |   2 +-
 gcc/testsuite/g++.dg/concepts/diagnostic5.C |  46 ++
 5 files changed, 182 insertions(+), 21 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic5.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index a1e0f4cdc3a..c49da99d395 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1453,6 +1453,10 @@ fconcepts-ts
 C++ ObjC++ Var(flag_concepts_ts) Init(0)
 Enable certain features present in the Concepts TS.
 
+fconcepts-diagnostics-depth=
+C++ ObjC++ Joined RejectNegative UInteger Var(concepts_diagnostics_max_depth) 
Init(1)
+Specify maximum error replay depth during recursive diagnosis of a constraint 
satisfaction failure.
+
 fcond-mismatch
 C ObjC C++ ObjC++
 Allow the arguments of the '?' operator to have different types.
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index a86bcdf603a..76c520318c3 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -162,6 +162,7 @@ finish_constraint_binary_op (location_t loc,
   /* When either operand is dependent, the overload set may be non-empty.  */
   if (expr == error_mark_node)
 return error_mark_node;
+  expr.set_location (loc);
   expr.set_range (lhs.get_start (), rhs.get_finish ());
   return expr;
 }
@@ -2395,6 +2396,49 @@ 

Re: [PATCH] c++: Diagnose when "requires" is used instead of "requires requires" [PR94306]

[Patrick Palka via Gcc-patches]

On Thu, 26 Mar 2020, Jason Merrill wrote:

> On 3/26/20 6:36 PM, Patrick Palka wrote:
> > On Thu, 26 Mar 2020, Jason Merrill wrote:
> > 
> > > On 3/26/20 5:28 PM, Patrick Palka wrote:
> > > > This adds support to detect and recover from the case where an opening
> > > > brace
> > > > immediately follows the start of a requires-clause.  So rather than
> > > > emitting
> > > > the
> > > > error
> > > > 
> > > > error: expected primary-expression before '{' token
> > > > 
> > > > followed by a slew of irrevelant errors, we now assume the user had
> > > > intended
> > > > to
> > > > write "requires requires {" and diagnose and recover accordingly.
> > > > 
> > > > Tested on x86_64-pc-linux-gnu, does this look OK?
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > PR c++/94306
> > > > * parser.c (cp_parser_requires_clause_opt): Diagnose and 
> > > > recover from
> > > > "requires {" when "requires requires {" was probably intended.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > PR c++/94306
> > > > * g++.dg/concepts/diagnostic8.C: New test.
> > > > ---
> > > >gcc/cp/parser.c | 17 -
> > > >gcc/testsuite/g++.dg/concepts/diagnostic8.C |  6 ++
> > > >2 files changed, 22 insertions(+), 1 deletion(-)
> > > >create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C
> > > > 
> > > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > > > index 05363653691..73c2c2cb010 100644
> > > > --- a/gcc/cp/parser.c
> > > > +++ b/gcc/cp/parser.c
> > > > @@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser
> > > > *parser,
> > > > bool lambda_p)
> > > > }
> > > >  return NULL_TREE;
> > > >}
> > > > -  cp_lexer_consume_token (parser->lexer);
> > > > +
> > > > +  cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2);
> > > > +  if (tok2->type == CPP_OPEN_BRACE)
> > > 
> > > Do we want to handle an open paren the same way?
> > 
> > Hmm, wouldn't the error recovery be significantly different for an open
> > paren here?  Just pretending that there's another "requires" token would
> > mean we proceed as if we got "requires requires (" but that is still
> > ill-formed I think.
> 
> Not ill-formed; a requires-expression can have a parameter-list.

Ah, of course..  But then again, wouldn't "requires (" be not
necessarily ill-formed, since the open paren could possibly be a part of
the constraint-expression e.g. "requires (a || b)"?

We could perhaps look ahead of the open paren and check if we match the
ill-formed pattern "requires (...) {...}" and if so, handle the open
paren the same way.  Oh, but even that pattern is not necessarily
ill-formed, at least when parsing a lambda: the following
"[]  () -> void requires (true) { }"
is apparently a well-formed lambda-expression whose tokens starting from
its requires-clause would match that pattern.  So we would probably have
to condition this open-paren handling on !lambda_p.

> 
> > > 
> > > > +{
> > > > +  /* An opening brace following the start of a requires-clause is
> > > > +ill-formed; the user likely forgot the second `requires' that
> > > > +would start a requires-expression.  */
> > > > +  gcc_rich_location richloc (tok2->location);
> > > > +  richloc.add_fixit_insert_before (" requires");
> > > > +  error_at (, "missing additional % to start "
> > > > +   "a requires-expression");
> > > > +  /* Don't consume the `requires', so that it's reused as the start
> > > > of
> > > > a
> > > > +requires-expression.  */
> > > > +}
> > > > +  else
> > > > +cp_lexer_consume_token (parser->lexer);
> > > >if (!flag_concepts_ts)
> > > >return cp_parser_requires_clause_expression (parser, lambda_p);
> > > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic8.C
> > > > b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
> > > > new file mode 100644
> > > > index 000..70d7e4a9cc1
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
> > > > @@ -0,0 +1,6 @@
> > > > +// PR c++/94306
> > > > +// { dg-do compile { target c++2a } }
> > > > +
> > > > +template struct S { };
> > > > +template requires { typename T::type; } struct S { };
> > > > +// { dg-error "missing additional .requires." "" { target *-*-* } .-1 }
> > > > 
> > > 
> > > 
> > 
> 
> 

Re: [PATCH] c++: Handle COMPOUND_EXPRs in ocp_convert [PR94339]

[Jason Merrill via Gcc-patches]

On 3/26/20 3:13 PM, Jakub Jelinek wrote:

Hi!

My recent change to get_narrower/warnings_for_convert_and_check broke
the following testcase, warnings_for_convert_and_check is upset that
expr is a COMPOUND_EXPR with INTEGER_CST at the rightmost operand, while
result is a COMPOUND_EXPR with a NOP_EXPR of INTEGER_CST at the rightmost
operand, it expects such conversions to be simplified.

The easiest fix seems to be to handle COMPOUND_EXPRs in ocp_convert too,
by converting the rightmost operand and recreating COMPOUND_EXPR(s) if that
changed.

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


OK.


The attr-copy-2.C change is a workaround for PR94346, where we now ICE on
the testcase, while previously we'd ICE only if it contained a comma
expression at the outer level rather than cast of a COMPOUND_EXPR to
something.  I'll defer that to Martin.

2020-03-26  Jakub Jelinek  

PR c++/94339
* cvt.c (ocp_convert): Handle COMPOUND_EXPR by recursion on the second
operand and creating a new COMPOUND_EXPR if anything changed.

* g++.dg/other/pr94339.C: New test.
* g++.dg/ext/attr-copy-2.C: Comment out failing tests due to PR94346.

--- gcc/cp/cvt.c.jj 2020-01-12 11:54:36.0 +0100
+++ gcc/cp/cvt.c2020-03-26 12:30:52.312170071 +0100
@@ -697,6 +697,17 @@ ocp_convert (tree type, tree expr, int c
if (error_operand_p (e) || type == error_mark_node)
  return error_mark_node;
  
+  if (TREE_CODE (e) == COMPOUND_EXPR)

+{
+  e = ocp_convert (type, TREE_OPERAND (e, 1), convtype, flags, complain);
+  if (e == error_mark_node)
+   return error_mark_node;
+  if (e == TREE_OPERAND (expr, 1))
+   return expr;
+  return build2_loc (EXPR_LOCATION (expr), COMPOUND_EXPR, TREE_TYPE (e),
+TREE_OPERAND (expr, 0), e);
+}
+
complete_type (type);
complete_type (TREE_TYPE (expr));
  
--- gcc/testsuite/g++.dg/other/pr94339.C.jj	2020-03-26 12:37:01.645664573 +0100

+++ gcc/testsuite/g++.dg/other/pr94339.C2020-03-26 12:32:36.217621191 
+0100
@@ -0,0 +1,11 @@
+// PR c++/94339
+// { dg-do compile }
+
+unsigned a;
+void bar ();
+
+unsigned
+foo (bool x)
+{
+  return x ? bar (), -1 : a;
+}
--- gcc/testsuite/g++.dg/ext/attr-copy-2.C.jj   2020-01-12 11:54:37.0 
+0100
+++ gcc/testsuite/g++.dg/ext/attr-copy-2.C  2020-03-26 20:05:34.464882638 
+0100
@@ -36,8 +36,8 @@ typedef struct C
ATTR (copy (*(struct A *)(0, 1))) long m_xpa_0_1;
  
ATTR (copy (((struct A *)0)[0])) short m_arpa_0;

-  ATTR (copy (((struct A *)(1, 0))[0])) int m_arpa_1_0;
-  ATTR (copy (((struct A *)(0, 1))[0])) long m_arpa_0_1;
+//  ATTR (copy (((struct A *)(1, 0))[0])) int m_arpa_1_0;
+//  ATTR (copy (((struct A *)(0, 1))[0])) long m_arpa_0_1;
  
ATTR (copy (a)) short m_a;

ATTR (copy (b.a)) int m_b_a;
@@ -86,8 +86,8 @@ static_assert (__builtin_has_attribute (
  static_assert (__builtin_has_attribute (((C*)0)->m_ra, packed));
  
  static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0, packed));

-static_assert (__builtin_has_attribute (((C*)0)->m_arpa_1_0, packed));
-static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0_1, packed));
+//static_assert (__builtin_has_attribute (((C*)0)->m_arpa_1_0, packed));
+//static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0_1, packed));
  
  static_assert (__builtin_has_attribute (((C*)0)->m_a, packed));

  static_assert (__builtin_has_attribute (((C*)0)->m_b_a, packed));

Jakub

Re: [PATCH] c++: Avoid calls in non-evaluated contexts affect whether function can or can't throw [PR94326]

[Jason Merrill via Gcc-patches]

On 3/26/20 3:39 PM, Jakub Jelinek wrote:

Hi!

The following testcase FAILs -fcompare-debug, because if we emit a
-Wreturn-local-addr warning, we tsubst decltype in order to print the
warning and as that function could throw, set_flags_from_callee during that
sets cp_function_chain->can_throw and later on we don't set TREE_NOTHROW
on foo.  While with -w or -Wno-return-local-addr, tsubst isn't called during
the warning_at, cp_function_chain->can_throw is kept clear and TREE_NOTHROW
is set on foo.
It isn't just a matter of the warning though, in
int foo ();
int bar () { return sizeof (foo ()); }
int baz () { return sizeof (int); }
I don't really see why we should mark only baz as TREE_NOTHROW and not bar
too, when neither can really throw.

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


OK.


2020-03-26  Jakub Jelinek  

PR c++/94326
* call.c (set_flags_from_callee): Don't update
cp_function_chain->can_throw or current_function_returns_abnormally
if cp_unevaluated_operand.

* g++.dg/other/pr94326.C: New test.

--- gcc/cp/call.c.jj2020-03-25 08:05:07.153731580 +0100
+++ gcc/cp/call.c   2020-03-26 15:03:42.432909693 +0100
@@ -333,11 +333,14 @@ set_flags_from_callee (tree call)
   && internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW)
  nothrow = true;
  
-  if (!nothrow && at_function_scope_p () && cfun && cp_function_chain)

-cp_function_chain->can_throw = 1;
+  if (cfun && cp_function_chain && !cp_unevaluated_operand)
+{
+  if (!nothrow && at_function_scope_p ())
+   cp_function_chain->can_throw = 1;
  
-  if (decl && TREE_THIS_VOLATILE (decl) && cfun && cp_function_chain)

-current_function_returns_abnormally = 1;
+  if (decl && TREE_THIS_VOLATILE (decl))
+   current_function_returns_abnormally = 1;
+}
  
TREE_NOTHROW (call) = nothrow;

  }
--- gcc/testsuite/g++.dg/other/pr94326.C.jj 2020-03-26 15:14:23.609400216 
+0100
+++ gcc/testsuite/g++.dg/other/pr94326.C2020-03-26 15:14:54.162947065 
+0100
@@ -0,0 +1,19 @@
+// PR c++/94326
+// { dg-do compile { target c++11 } }
+// { dg-options "-fcompare-debug" }
+
+template  struct A {
+  const int () { return 0; }   // { dg-warning "returning reference to 
temporary" }
+  template  void bar(_Kt) { foo(); }
+};
+struct B {
+  A<> b;
+  template  auto baz(_Kt p1) -> decltype(b.bar(p1)) {
+b.bar(p1);
+  }
+};
+struct C {};
+void operator<(C, int) {
+  B a;
+  a.baz(C{});
+}

Jakub

Re: [PATCH] c++: template keyword accepted before destructor names [PR94336]

[Jason Merrill via Gcc-patches]

On 3/26/20 6:31 PM, Marek Polacek wrote:

This came up on the C++ core list recently.  We don't diagnose the case
when 'template' is followed by a destructor name which is not permitted
by [temp.names]/5.


OK.


PR c++/94336 - template keyword accepted before destructor names.
* parser.c (cp_parser_unqualified_id): Give an error when 'template'
is followed by a destructor name.

* g++.dg/template/template-keyword2.C: New test.
---
  gcc/cp/parser.c   | 9 +
  gcc/testsuite/g++.dg/template/template-keyword2.C | 5 +
  2 files changed, 14 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/template/template-keyword2.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 05363653691..8c5fc5eea09 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6068,6 +6068,15 @@ cp_parser_unqualified_id (cp_parser* parser,
  cp_lexer_consume_token (parser->lexer);
return error_mark_node;
  }
+   if (template_keyword_p)
+ {
+   if (!cp_parser_uncommitted_to_tentative_parse_p (parser))
+ error_at (tilde_loc, "% keyword not permitted in "
+   "destructor name");
+   cp_parser_simulate_error (parser);
+   return error_mark_node;
+ }
+
gcc_assert (!scope || TYPE_P (scope));
  
  	token = cp_lexer_peek_token (parser->lexer);

diff --git a/gcc/testsuite/g++.dg/template/template-keyword2.C 
b/gcc/testsuite/g++.dg/template/template-keyword2.C
new file mode 100644
index 000..27d874ba7d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/template-keyword2.C
@@ -0,0 +1,5 @@
+// PR c++/94336 - template keyword accepted before destructor names.
+
+template void f(T *p) { p->template ~X(); } // { dg-error ".template. 
keyword not permitted in destructor name" }
+template struct X {};
+void g(X *p) { f(p); }

base-commit: 16948c54b7576fb4b27c59915eac71a0c6bf94f6

Re: [PATCH] c++: Diagnose when "requires" is used instead of "requires requires" [PR94306]

[Jason Merrill via Gcc-patches]

On 3/26/20 6:36 PM, Patrick Palka wrote:

On Thu, 26 Mar 2020, Jason Merrill wrote:


On 3/26/20 5:28 PM, Patrick Palka wrote:

This adds support to detect and recover from the case where an opening brace
immediately follows the start of a requires-clause.  So rather than emitting
the
error

error: expected primary-expression before '{' token

followed by a slew of irrevelant errors, we now assume the user had intended
to
write "requires requires {" and diagnose and recover accordingly.

Tested on x86_64-pc-linux-gnu, does this look OK?

gcc/cp/ChangeLog:

PR c++/94306
* parser.c (cp_parser_requires_clause_opt): Diagnose and recover from
"requires {" when "requires requires {" was probably intended.

gcc/testsuite/ChangeLog:

PR c++/94306
* g++.dg/concepts/diagnostic8.C: New test.
---
   gcc/cp/parser.c | 17 -
   gcc/testsuite/g++.dg/concepts/diagnostic8.C |  6 ++
   2 files changed, 22 insertions(+), 1 deletion(-)
   create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 05363653691..73c2c2cb010 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser *parser,
bool lambda_p)
}
 return NULL_TREE;
   }
-  cp_lexer_consume_token (parser->lexer);
+
+  cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2);
+  if (tok2->type == CPP_OPEN_BRACE)


Do we want to handle an open paren the same way?


Hmm, wouldn't the error recovery be significantly different for an open
paren here?  Just pretending that there's another "requires" token would
mean we proceed as if we got "requires requires (" but that is still
ill-formed I think.


Not ill-formed; a requires-expression can have a parameter-list.




+{
+  /* An opening brace following the start of a requires-clause is
+ill-formed; the user likely forgot the second `requires' that
+would start a requires-expression.  */
+  gcc_rich_location richloc (tok2->location);
+  richloc.add_fixit_insert_before (" requires");
+  error_at (, "missing additional % to start "
+   "a requires-expression");
+  /* Don't consume the `requires', so that it's reused as the start of
a
+requires-expression.  */
+}
+  else
+cp_lexer_consume_token (parser->lexer);
   if (!flag_concepts_ts)
   return cp_parser_requires_clause_expression (parser, lambda_p);
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic8.C
b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
new file mode 100644
index 000..70d7e4a9cc1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
@@ -0,0 +1,6 @@
+// PR c++/94306
+// { dg-do compile { target c++2a } }
+
+template struct S { };
+template requires { typename T::type; } struct S { };
+// { dg-error "missing additional .requires." "" { target *-*-* } .-1 }

[PATCH] avoid passing expressions to decl_attributes [PR94346]

[Martin Sebor via Gcc-patches]

Attribute copy can be invoked with an expression argument to copy from
the expression's type.  However, it must avoid passing the expression
as the last (optional) argument to decl_attributes because the function
is only prepared to deal with DECLs and types.

The attached patch passes null as the last argument in these situations
instead.  In addition, it makes more robust the handling of expressions
of pointers to function type (a subtle bug here was exposed by testing
of additional cases beyond the one in the report).

Tested on x86_64-linux.  I plan to commit the patch in the next day or
so unless there are concerns/suggestions.

Martin
PR c++/94346 - [9/10 Regression] ICE due to handle_copy_attribute since r9-3982

gcc/c-family/ChangeLog:

	PR c++/94346
	* c-attribs.c (handle_copy_attribute): Avoid passing expressions
	to decl_attributes.  Make handling of different kinds of entities
	more robust.
	
gcc/c-c++-common/ChangeLog:

	PR c++/94346
	* c-c++-common/attr-copy.c: New test.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index f30158a258b..93e740853a9 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -2526,17 +2526,21 @@ handle_copy_attribute (tree *node, tree name, tree args,
   && !FUNCTION_POINTER_TYPE_P (TREE_TYPE (ref)))
 ref = TREE_TYPE (ref);
 
+  tree reftype = DECL_P (ref) || EXPR_P (ref) ? TREE_TYPE (ref) : ref;
+
   if (DECL_P (decl))
 {
   if ((VAR_P (decl)
 	   && (TREE_CODE (ref) == FUNCTION_DECL
 	   || (EXPR_P (ref)
-		   && POINTER_TYPE_P (TREE_TYPE (ref))
-		   && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (ref))
+		   && POINTER_TYPE_P (reftype)
+		   && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (reftype)
 	  || (TREE_CODE (decl) == FUNCTION_DECL
 	  && (VAR_P (ref)
 		  || (EXPR_P (ref)
-		  && !FUNC_OR_METHOD_TYPE_P (TREE_TYPE (ref))
+		  && !FUNC_OR_METHOD_TYPE_P (reftype)
+		  && (!POINTER_TYPE_P (reftype)
+			  || !FUNC_OR_METHOD_TYPE_P (TREE_TYPE (reftype)))
 	{
 	  /* It makes no sense to try to copy function attributes
 	 to a variable, or variable attributes to a function.  */
@@ -2606,15 +2610,11 @@ handle_copy_attribute (tree *node, tree name, tree args,
 
   /* Similarly, a function declared with attribute noreturn has it
  attached on to it, but a C11 _Noreturn function does not.  */
-  tree reftype = ref;
   if (DECL_P (ref)
   && TREE_THIS_VOLATILE (ref)
-  && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (reftype)))
+  && FUNC_OR_METHOD_TYPE_P (reftype))
 TREE_THIS_VOLATILE (decl) = true;
 
-  if (DECL_P (ref) || EXPR_P (ref))
-reftype = TREE_TYPE (ref);
-
   if (POINTER_TYPE_P (reftype))
 reftype = TREE_TYPE (reftype);
 
@@ -2623,9 +2623,10 @@ handle_copy_attribute (tree *node, tree name, tree args,
 
   tree attrs = TYPE_ATTRIBUTES (reftype);
 
-  /* Copy type attributes from REF to DECL.  */
+  /* Copy type attributes from REF to DECL.  Pass in REF if it's a DECL
+ or a type but not if it's an expression.  */
   for (tree at = attrs; at; at = TREE_CHAIN (at))
-decl_attributes (node, at, flags, ref);
+decl_attributes (node, at, flags, EXPR_P (ref) ? NULL_TREE : ref);
 
   return NULL_TREE;
 }
diff --git a/gcc/testsuite/c-c++-common/attr-copy.c b/gcc/testsuite/c-c++-common/attr-copy.c
new file mode 100644
index 000..284088a8b97
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-copy.c
@@ -0,0 +1,43 @@
+/* PR c++/94346 - ICE due to handle_copy_attribute
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+#define ATTR(...) __attribute__ ((__VA_ARGS__))
+
+#if __cplusplus > 199711L
+#  define SA(expr) static_assert (expr, #expr)
+#elif __cplusplus
+#  define SA(expr)			\
+  typedef __attribute__ ((unused)) char Assert[!(expr) ? -1 : 1]
+#else
+#  define SA(expr) _Static_assert (expr, #expr)
+#endif
+
+typedef struct ATTR (packed) A { ATTR (packed) unsigned bf: 1; } A;
+
+int bar (void);
+
+struct C
+{
+  char c;
+  ATTR (copy ((bar (), ((struct A *)(0))[0]))) int i;
+};
+
+/* Verify the attribute has been copied.  */
+SA (__builtin_offsetof (struct C, i) == 1);
+
+
+
+/* Verify attribute copy can copy from the type a comma expression.  */
+ATTR (alloc_size (1)) void* alloc1 (int);
+
+ATTR (copy ((bar (), alloc1))) void* alloc2 (int, int);
+
+ATTR (copy ((bar (), alloc1))) void alloc3 (int);  /* { dg-warning "'alloc_size' attribute ignored on a function returning 'void'" } */
+
+
+typedef ATTR (alloc_size (1)) void* F (int);
+
+ATTR (copy ((bar (), (F*)0))) void* alloc4 (int, int);
+
+ATTR (copy ((bar (), (F*)0))) void alloc5 (int, int);  /* { dg-warning "'alloc_size' attribute ignored on a function returning 'void'" } */

Re: [PATCH] rs6000: Save/restore r31 if frame_pointer_needed is true

[luoxhu via Gcc-patches]

On 2020/3/27 07:59, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Mar 25, 2020 at 11:15:22PM -0500, luo...@linux.ibm.com wrote:
>> frame_pointer_needed is set to true in reload pass setup_can_eliminate,
>> but regs_ever_live[31] is false, so pro_and_epilogue doesn't save/restore
>> r31 even it is used actually, causing CPU2006 465.tonto segment fault of
>> loading from invalid addresses.
> 
> If df_regs_ever_live_p(31) is false there is no hard frame pointer
> anywhere in the program.  How can it be used then?

There is a piece of code emit move instruction to r31 even 
df_regs_ever_live_p(31) is false
in pro_and_epilogue.  As frame_point_needed is true and frame_pointer_needed is 
widely
used in this function, so I propose to save r31 in save_reg_p instead of check
(frame_pointer_needed && df_regs_ever_live_p(31), I haven't verify whether this 
works yet).
Is this reasonable?  Thanks.
 
rs6000-logue.c
void
rs6000_emit_prologue (void)
{
...
bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 26840)   /* Set frame 
pointer, if needed.  */
bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 26841)   if 
(frame_pointer_needed)
bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 26842) {
0d6c02bf24e4 (jakub  2005-06-30 14:26:32 + 26843)   insn = 
emit_move_insn (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM),
bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 26844)   
 sp_reg_rtx);
bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 26845)   
RTX_FRAME_RELATED_P (insn) = 1;
6b02f2a5c61e (meissner   1995-11-30 20:02:16 + 26846) }
d1bd513ed578 (kenner 1992-02-09 19:26:21 + 26847)
...
}

Xionghu

> 
> 
> Segher
> 

Re: [PATCH] rs6000: Check -+0 and NaN for smax/smin generation

[Jiufu Guo via Gcc-patches]

Matthias Klose  writes:

Thanks so much for all of you for pay attention and take care of
this.  Matthias and Segher point out this; Joseph helped remove this
file.  Sorry for spend your extra time on this.

Thanks again!

> diff --git a/a b/a
> new file mode 100644
> index 000..a4f422403ef
> --- /dev/null
> +++ b/a
> @@ -0,0 +1,81 @@
> +commit 5b075372b47b87bde46e5acc58531c410fb65f8c
> +Author: Jiufu Guo 
> +AuthorDate: Tue Mar 10 13:51:57 2020 +0800
> +Commit: Jiufu Guo 
> +CommitDate: Thu Mar 19 10:04:00 2020 +0800
>
> [...]
>
> please remove.

Re: [PATCH] tree: Fix -fcompare-debug issues due to protected_set_expr_location [PR94323]

[Alexandre Oliva]

On Mar 26, 2020, Jakub Jelinek  wrote:

> The hardest issue I gave up completely on after trying 3 different
> approaches is PR93786, I just don't know what to do in that case.

struct S {virtual void v();};
void f(S * s) {
  ({ s; })->v();
}

Thanks, I can now see how markers create situations that were not
possible with binds.

The statement 's;' in the expression statement doesn't have any side
effects, but it will have a marker associated with it.

Without side effects, a statement wouldn't give rise to binds, so this
situation wouldn't arise.

Such a simple, side-effect-free statement wouldn't even have location
data associated with it, I suppose.  We might have a wrapper to annotate
the expression with the location in which it is used, and we might
augment such a wrapper with pre- and post- debug stmts, but odds are
we'd eventually end up without means to carry the info any further.


This suggests two potential ways to address this problem:

- when gimplifying a STATEMENT_LIST without side effects, if it contains
a single nondebug statement, gimplify only that one stmt, discarding
any of the debug stmts that we can't hold on to.

- wherever we might have a sequence of statements, create a statement
list instead of working very hard in a premature (?) optimization that
makes it harder (impossible?) to retain debug info in cases we'd like
to.  This would grow memory use a little during parsing, but since we
gimplify functions early on and that would address most of the
differences, and optimizers would take care of any unnecessary
SAVE_EXPRs, it might not matter much, and avoiding the current runtime
overheads in skipping debug stmts early on might more than make up for
that.


-- 
Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/
Free Software Evangelist  Stallman was right, but he's left :(
GNU Toolchain Engineer   Live long and free, and prosper ethically

Re: [PATCH] avoid modifying type in attribute access handler [PR94098]

[Jason Merrill via Gcc-patches]

On 3/26/20 4:57 PM, Martin Sebor wrote:

On 3/25/20 3:56 PM, Jason Merrill wrote:

On 3/16/20 4:41 PM, Martin Sebor wrote:

The recent fix to avoid modifying in place the type argument in
handle_access_attribute (PR 92721) was incomplete and didn't fully
resolve the problem (an ICE in the C++ front-end).  The attached
patch removes the remaining modification that causes the ICE.  In
addition, the change adjusts checking calls to functions declared
with the attribute to scan all its instances.

The attached patch was tested on x86_64-linux.



   attrs = remove_attribute (IDENTIFIER_POINTER (name), attrs);


This unchanged line can still modify existing types; I think if attrs 
already has a matching attribute you need to work harder to replace it.


I have a number of questions.

1) There are two instances of the call above in the code (only one in
the context of the patch).  Are you referring specifically to the one
in the patch or to both?  The one in the patch manipulates the type
attributes of the last DECL (NODE[1]).  The other one, attributes of
the current type (NODE[0]).  Are both a problem?


Both, yes.  We should give the decl(s) a new type rather than modify 
their existing types.



2) What all do you include in "modifying existing types?"  A number
of attribute handlers unconditionally modify the type of a *NODE
without first testing ATTR_FLAG_TYPE_IN_PLACE (e.g., by setting
TYPE_USER_ALIGN, or TREE_USED).  Are those modifications safe?  If
they are, what makes the difference between those and the code above?


Those also seem unsafe.


3) Is the restriction on "modifying existing types" specific to
the C++ front end or a general one that applies to the whole C/C++
family?  (I've only seen a failure in the C++ FE.)


It's a general correctness issue, the C++ front end just catches the 
issues better.  If you have two declarations with the same type, and 
then you give one an attribute, it shouldn't give that same attribute to 
the other declaration that shared the type.



4) What exactly does "work harder" mean and how do I test it?  Are
you suggesting to call build_type_attribute_variant (or maybe
build_variant_type_copy like the existing handlers do) to replace
the type?  Or are you implying that unless I need to replace
the existing type  I should avoid modifying the TYPE's
TYPE_ATTRIBUTES and instead work with a copy of the attribute chain?


We can't modify the existing attribute chain; if we need to remove an 
attribute from the middle, it must be by copying all the list nodes 
before the attribute.



These restrictions and the lack of enforcement (only in the C++ FE)
make nontrivial handlers extremely fragile.  Unless the restrictions
are entirely C++-specific I would really like to add assertions to
decl_attributes to catch these problems earlier (and in C as well).


Sounds good.


Either way, I also want to add test cases to exercise them.  But
I need to understand them first (i.e., get answers to the questions
above).  Nothing I've tried so far has triggered a problem due to
the code you point out above.

Martin

[committed] coroutines, testsuite: Fix symmetric-transfer-00-basic.C on Linux.

[Iain Sandoe]

Hi.

I failed to comment out some test printing before committing the testcase.
However, for that to work, we need to include  on Linux platforms.

tested on x86_64-linux-gnu, applied to master.
thanks
Iain

2020-03-27  Iain Sandoe  

* g++.dg/coroutines/torture/symmetric-transfer-00-basic.C:
Add .

diff --git 
a/gcc/testsuite/g++.dg/coroutines/torture/symmetric-transfer-00-basic.C 
b/gcc/testsuite/g++.dg/coroutines/torture/symmetric-transfer-00-basic.C
index c445fc55a2c..864846e365c 100644
--- a/gcc/testsuite/g++.dg/coroutines/torture/symmetric-transfer-00-basic.C
+++ b/gcc/testsuite/g++.dg/coroutines/torture/symmetric-transfer-00-basic.C
@@ -18,7 +18,8 @@ namespace coro = std::experimental;
 
 #include 
 #include 
- 
+#include 
+
 template  
 struct Loopy {
   struct promise_type;
@@ -84,7 +85,7 @@ pingpong (const char *id)
   v = co_await aw;
   //fprintf (stderr, "%s = %d\n", id, v);
 }
- fprintf (stderr, "%s = %d\n", id, v);
+ //fprintf (stderr, "%s = %d\n", id, v);
 }
 
 int main ()

Re: [PATCH] rs6000: Save/restore r31 if frame_pointer_needed is true

[Segher Boessenkool]

Hi!

On Wed, Mar 25, 2020 at 11:15:22PM -0500, luo...@linux.ibm.com wrote:
> frame_pointer_needed is set to true in reload pass setup_can_eliminate,
> but regs_ever_live[31] is false, so pro_and_epilogue doesn't save/restore
> r31 even it is used actually, causing CPU2006 465.tonto segment fault of
> loading from invalid addresses.

If df_regs_ever_live_p(31) is false there is no hard frame pointer
anywhere in the program.  How can it be used then?


Segher

Re: [PATCH] tree: Fix -fcompare-debug issues due to protected_set_expr_location [PR94323]

[Jakub Jelinek via Gcc-patches]

On Thu, Mar 26, 2020 at 08:41:41PM -0300, Alexandre Oliva wrote:
> On Mar 26, 2020, Jakub Jelinek  wrote:
> 
> > Or disable -gstatement-frontiers by default and declare it -fcompare-debug
> > incompatible.
> 
> I don't get what makes debug stmts introduced by -gstatement-frontiers
> special in this regard.  I recall working a lot on making statement list
> management compatible with debug stmts for -fcompare-debug purposes back
> when introducing them, but not breaking new ground when extending them
> with markers rather than binds.

The hardest issue I gave up completely on after trying 3 different
approaches is PR93786, I just don't know what to do in that case.
Gimplification of a STATEMENT_LIST is destructive, especially the
voidify_wrapper_expr part of it, and if the STATEMENT_LIST is used multiple
times, that is a severe problem.

The PR94323 and PR94272 changes worked, but they are quite unclean and in
bugzilla there is at least one another -fcompare-debug issue that goes away
with -gno-statement-frontiers (PR94340).

Jakub

[PATCH] i386: Fix up *one_cmplv*2* insn with avx512f [PR94343]

[Jakub Jelinek via Gcc-patches]

Hi!

This define_insn has two issues.
One is that with -mavx512f -mno-avx512vl it can emit an AVX512VL-only insn
- 128-bit or 256-bit EVEX encoded vpternlog{d,q}.
Another one is that because there is no vpternlog{b,w}, we emit vpternlogd
instead, but then we shouldn't pretend we support masking of that, because
we don't.
The first one can be fixed by forcing the use of %zmm* registers instead of
%xmm* or %ymm* if AVX512F but not AVX512VL, like we do for a couple of other
insns (although that is primarily done in order to support %xmm16+ regs).
But we need to make sure that in that case the input operand isn't memory,
because while we can read and store the higher bits of registers, we don't
want to read from memory more bytes than what we should read.

A variant to these two if_then_else set attrs, condition in the output and
larger condition would be 4 different define_insns (one with something like
VI48_AVX512VL iterator, masking, no g modifiers and "vm" input constraint,
another one with VI48_AVX iterator, !TARGET_AVX512VL in condition,
no masking, g modifiers and "v" input constraint, one with VI12_AVX512VL
iterator, no masking, no g modifiers and "vm" input constraint and last one with
VI12_AVX2 iterator, !TARGET_AVX512VL in condition, no masking, g modifiers
and "v" input constraint, but I think having one pattern is shorter than
that.

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

2020-03-26  Jakub Jelinek  

PR target/94343
* config/i386/sse.md (one_cmpl2): If
!TARGET_AVX512VL, use 512-bit vpternlog and make sure the input
operand is a register.  Don't enable masked variants for V*[QH]Imode.

* gcc.target/i386/avx512f-pr94343.c: New test.
* gcc.target/i386/avx512vl-pr94343.c: New test.

--- gcc/config/i386/sse.md.jj   2020-03-06 11:35:46.284074858 +0100
+++ gcc/config/i386/sse.md  2020-03-26 18:49:39.644131577 +0100
@@ -12796,14 +12796,29 @@ (define_expand "one_cmpl2"
 })
 
 (define_insn "one_cmpl2"
-  [(set (match_operand:VI 0 "register_operand" "=v")
-   (xor:VI (match_operand:VI 1 "nonimmediate_operand" "vm")
-   (match_operand:VI 2 "vector_all_ones_operand" "BC")))]
-  "TARGET_AVX512F"
-  "vpternlog\t{$0x55, %1, %0, 
%0|%0, %0, %1, 0x55}"
+  [(set (match_operand:VI 0 "register_operand" "=v,v")
+   (xor:VI (match_operand:VI 1 "nonimmediate_operand" "v,m")
+   (match_operand:VI 2 "vector_all_ones_operand" "BC,BC")))]
+  "TARGET_AVX512F
+   && (!
+   || mode == SImode
+   || mode == DImode)"
+{
+  if (TARGET_AVX512VL)
+return "vpternlog\t{$0x55, %1, %0, 
%0|%0, %0, %1, 0x55}";
+  else
+return "vpternlog\t{$0x55, %g1, %g0, 
%g0|%g0, %g0, %g1, 0x55}";
+}
   [(set_attr "type" "sselog")
(set_attr "prefix" "evex")
-   (set_attr "mode" "")])
+   (set (attr "mode")
+(if_then_else (match_test "TARGET_AVX512VL")
+ (const_string "")
+ (const_string "XI")))
+   (set (attr "enabled")
+   (if_then_else (eq_attr "alternative" "1")
+ (symbol_ref " == 64 || TARGET_AVX512VL")
+ (const_int 1)))])
 
 (define_expand "_andnot3"
   [(set (match_operand:VI_AVX2 0 "register_operand")
--- gcc/testsuite/gcc.target/i386/avx512f-pr94343.c.jj  2020-03-26 
17:47:40.008654504 +0100
+++ gcc/testsuite/gcc.target/i386/avx512f-pr94343.c 2020-03-26 
17:48:37.169811375 +0100
@@ -0,0 +1,12 @@
+/* PR target/94343 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mavx512f -mno-avx512vl" } */
+/* { dg-final { scan-assembler-not "vpternlogd\[^\n\r]*xmm\[0-9]*" } } */
+
+typedef int __v4si __attribute__((vector_size (16)));
+
+__v4si
+foo (__v4si a)
+{
+  return ~a;
+}
--- gcc/testsuite/gcc.target/i386/avx512vl-pr94343.c.jj 2020-03-26 
17:48:53.232573115 +0100
+++ gcc/testsuite/gcc.target/i386/avx512vl-pr94343.c2020-03-26 
17:49:08.034352968 +0100
@@ -0,0 +1,12 @@
+/* PR target/94343 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mavx512vl" } */
+/* { dg-final { scan-assembler "vpternlogd\[^\n\r]*xmm\[0-9]*" } } */
+
+typedef int __v4si __attribute__((vector_size (16)));
+
+__v4si
+foo (__v4si a)
+{
+  return ~a;
+}

Jakub

Re: [PATCH] tree: Fix -fcompare-debug issues due to protected_set_expr_location [PR94323]

[Alexandre Oliva]

On Mar 26, 2020, Jakub Jelinek  wrote:

> Or disable -gstatement-frontiers by default and declare it -fcompare-debug
> incompatible.

I don't get what makes debug stmts introduced by -gstatement-frontiers
special in this regard.  I recall working a lot on making statement list
management compatible with debug stmts for -fcompare-debug purposes back
when introducing them, but not breaking new ground when extending them
with markers rather than binds.


> Or perhaps just a flag on the STATEMENT_LIST that would make it clear the
> STATEMENT_LIST wouldn't be there without -g.  Or different tree code like
> STATEMENT_LIST, except that it would be only this kind of container.

IIRC part of the problem in ensuring isomorphism for -fcompare-debug is
that sometimes we end up with redundant STATEMENT_LISTs in the non-debug
case, and then we can't tell whether a STATEMENT_LIST is to be
significant or not in the debug case.  A flag for STATEMENT_LIST, or
maybe an alternate node type that amounted to an easier to recognize and
manage transparent container for at most one nondebug stmt, and as many
debug stmts as needed before and after it, might help avoid unintended
differences.

I'm not sure whether the separate node with direct access to the
contained nondebug node would be advantageous.  Maybe just recognizing
this kind of node and turning it into a regular STATEMENT_LIST wherever
we'd have created one would suffice.  IIRC there are various operations
of inserting stmts that search for nondebug stmts, and the direct link
might help with them, if the flag alone wouldn't.

-- 
Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/
Free Software Evangelist  Stallman was right, but he's left :(
GNU Toolchain Engineer   Live long and free, and prosper ethically

[PATCH v2] Fix vextract* masked patterns (PR target/93069)

[Jakub Jelinek via Gcc-patches]

On Wed, Mar 25, 2020 at 05:59:36PM -0600, Jeff Law via Gcc-patches wrote:
> Sorry.  I know you asked me to look at this eons ago, but ever time I just get
> lost.
> 
> I get the distinct impression that we could do something much simpler (the 
> patch
> you initially proposed for backporting to the release branches).  Perhaps we 
> go
> with that now and the full patch for gcc-11?

So like this?
Bootstrapped/regtested on x86_64-linux and i686-linux.

2020-03-26  Jakub Jelinek  

PR target/93069
* config/i386/sse.md (vec_extract_lo_): Use
 instead of m in output operand constraint.
(vec_extract_hi_): Use  instead of
%{%3%}.

* gcc.target/i386/avx512vl-pr93069.c: New test.
* gcc.dg/vect/pr93069.c: New test.

--- gcc/config/i386/sse.md.jj   2019-12-27 18:16:48.146431083 +0100
+++ gcc/config/i386/sse.md  2019-12-28 14:43:29.181456611 +0100
@@ -8782,7 +8782,8 @@
 })
 
 (define_insn "vec_extract_lo_"
-  [(set (match_operand: 0 "nonimmediate_operand" "=v,v,m")
+  [(set (match_operand: 0 ""
+ "=v,v,")
(vec_select:
  (match_operand:V16FI 1 ""
 "v,,v")
@@ -8834,7 +8835,8 @@
 })
 
 (define_insn "vec_extract_lo_"
-  [(set (match_operand: 0 "" "=v,v,m")
+  [(set (match_operand: 0 ""
+ "=v,v,")
(vec_select:
  (match_operand:VI8F_256 1 ""
"v,,v")
@@ -8844,7 +8846,7 @@
&& ( || !(MEM_P (operands[0]) && MEM_P (operands[1])))"
 {
   if ()
-return "vextract64x2\t{$0x0, %1, %0%{%3%}|%0%{%3%}, %1, 0x0}";
+return "vextract64x2\t{$0x0, %1, 
%0|%0, %1, 0x0}";
   else
 return "#";
 }
--- gcc/testsuite/gcc.target/i386/avx512vl-pr93069.c.jj 2019-12-28 
16:31:30.118695074 +0100
+++ gcc/testsuite/gcc.target/i386/avx512vl-pr93069.c2019-12-28 
16:32:16.920990539 +0100
@@ -0,0 +1,12 @@
+/* PR target/93069 */
+/* { dg-do assemble { target vect_simd_clones } } */
+/* { dg-options "-O2 -fopenmp-simd -mtune=skylake-avx512" } */
+/* { dg-additional-options "-mavx512vl" { target avx512vl } } */
+/* { dg-additional-options "-mavx512dq" { target avx512dq } } */
+
+#pragma omp declare simd
+int
+foo (int x, int y)
+{
+  return x == 0 ? x : y;
+}
--- gcc/testsuite/gcc.dg/vect/pr93069.c.jj  2019-12-28 16:31:01.822121036 
+0100
+++ gcc/testsuite/gcc.dg/vect/pr93069.c 2019-12-28 16:30:35.503517205 +0100
@@ -0,0 +1,10 @@
+/* PR target/93069 */
+/* { dg-do assemble { target vect_simd_clones } } */
+/* { dg-options "-O2 -fopenmp-simd" } */
+
+#pragma omp declare simd
+int
+foo (int x, int y)
+{
+  return x == 0 ? x : y;
+}


Jakub

Re: [PATCH] avoid modifying type in attribute access handler [PR94098]

[Martin Sebor via Gcc-patches]

I have removed both calls to remove_attribute because with the changes
to calls.c they are no longer needed.  The revised patch is attached.
I will plan to commit it tomorrow unless there's something else.

In hindsight, I think changing the attribute from the human-readable
form to the internal, condensed, form was a mistake (it was done to
address concerns about the costs of parsing the attribute raised
during code review).  I don't want to make further changes here now
but I will plan to revisit it in stage 1.

I would still appreciate answers to the questions below so I can work
on beefing up the verification in decl_attributes (also in stage 1).

Thanks
Martin

On 3/26/20 2:57 PM, Martin Sebor wrote:

On 3/25/20 3:56 PM, Jason Merrill wrote:

On 3/16/20 4:41 PM, Martin Sebor wrote:

The recent fix to avoid modifying in place the type argument in
handle_access_attribute (PR 92721) was incomplete and didn't fully
resolve the problem (an ICE in the C++ front-end).  The attached
patch removes the remaining modification that causes the ICE.  In
addition, the change adjusts checking calls to functions declared
with the attribute to scan all its instances.

The attached patch was tested on x86_64-linux.



   attrs = remove_attribute (IDENTIFIER_POINTER (name), attrs);


This unchanged line can still modify existing types; I think if attrs 
already has a matching attribute you need to work harder to replace it.


I have a number of questions.

1) There are two instances of the call above in the code (only one in
the context of the patch).  Are you referring specifically to the one
in the patch or to both?  The one in the patch manipulates the type
attributes of the last DECL (NODE[1]).  The other one, attributes of
the current type (NODE[0]).  Are both a problem?

2) What all do you include in "modifying existing types?"  A number
of attribute handlers unconditionally modify the type of a *NODE
without first testing ATTR_FLAG_TYPE_IN_PLACE (e.g., by setting
TYPE_USER_ALIGN, or TREE_USED).  Are those modifications safe?  If
they are, what makes the difference between those and the code above?

3) Is the restriction on "modifying existing types" specific to
the C++ front end or a general one that applies to the whole C/C++
family?  (I've only seen a failure in the C++ FE.)

4) What exactly does "work harder" mean and how do I test it?  Are
you suggesting to call build_type_attribute_variant (or maybe
build_variant_type_copy like the existing handlers do) to replace
the type?  Or are you implying that unless I need to replace
the existing type  I should avoid modifying the TYPE's
TYPE_ATTRIBUTES and instead work with a copy of the attribute chain?

These restrictions and the lack of enforcement (only in the C++ FE)
make nontrivial handlers extremely fragile.  Unless the restrictions
are entirely C++-specific I would really like to add assertions to
decl_attributes to catch these problems earlier (and in C as well).
Either way, I also want to add test cases to exercise them.  But
I need to understand them first (i.e., get answers to the questions
above).  Nothing I've tried so far has triggered a problem due to
the code you point out above.

Martin


PR c++/94098 - ICE on attribute access redeclaration

gcc/c-family/ChangeLog:

	PR c++/94098
	* c-attribs.c (handle_access_attribute): Avoid setting TYPE_ATTRIBUTES
	here.

gcc/ChangeLog:

	PR c++/94098
	* calls.c (init_attr_rdwr_indices): Iterate over all access attributes.

gcc/testsuite/ChangeLog:

	PR c++/94098
	* g++.dg/ext/attr-access-2.C: New test.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 9abf81d0248..f30158a258b 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4182,7 +4182,6 @@ handle_access_attribute (tree *node, tree name, tree args,
 
   /* Replace any existing access attribute specification with
  the concatenation above.  */
-  attrs = remove_attribute (IDENTIFIER_POINTER (name), attrs);
   new_attrs = tree_cons (NULL_TREE, new_attrs, NULL_TREE);
   new_attrs = tree_cons (name, new_attrs, attrs);
 
@@ -4190,15 +4189,12 @@ handle_access_attribute (tree *node, tree name, tree args,
 {
   /* Repeat for the previously declared type.  */
   attrs = TYPE_ATTRIBUTES (TREE_TYPE (node[1]));
-  tree new_attrs
-	= append_access_attrs (node[1], attrs, attrstr, code, idxs);
-  if (!new_attrs)
+  tree attrs1 = append_access_attrs (node[1], attrs, attrstr, code, idxs);
+  if (!attrs1)
 	return NULL_TREE;
 
-  attrs = remove_attribute (IDENTIFIER_POINTER (name), attrs);
-  new_attrs = tree_cons (NULL_TREE, new_attrs, NULL_TREE);
-  new_attrs = tree_cons (name, new_attrs, attrs);
-  TYPE_ATTRIBUTES (TREE_TYPE (node[1])) = new_attrs;
+  attrs1 = tree_cons (NULL_TREE, attrs1, NULL_TREE);
+  new_attrs = tree_cons (name, attrs1, attrs);
 }
 
   /* Recursively call self to "replace" the documented/external form
diff --git a/gcc/calls.c b/gcc/calls.c
index 

Re: [PATCH] c++: Diagnose when "requires" is used instead of "requires requires" [PR94306]

[Patrick Palka via Gcc-patches]

On Thu, 26 Mar 2020, David Malcolm wrote:

> On Thu, 2020-03-26 at 17:28 -0400, Patrick Palka via Gcc-patches wrote:
> > This adds support to detect and recover from the case where an
> > opening brace
> > immediately follows the start of a requires-clause.  So rather than
> > emitting the
> > error
> > 
> >   error: expected primary-expression before '{' token
> > 
> > followed by a slew of irrevelant errors, we now assume the user had
> > intended to
> > write "requires requires {" and diagnose and recover accordingly.
> > 
> > Tested on x86_64-pc-linux-gnu, does this look OK?
> > 
> > gcc/cp/ChangeLog:
> > 
> > PR c++/94306
> > * parser.c (cp_parser_requires_clause_opt): Diagnose and
> > recover from
> > "requires {" when "requires requires {" was probably intended.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > PR c++/94306
> > * g++.dg/concepts/diagnostic8.C: New test.
> > ---
> >  gcc/cp/parser.c | 17 -
> >  gcc/testsuite/g++.dg/concepts/diagnostic8.C |  6 ++
> >  2 files changed, 22 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C
> > 
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index 05363653691..73c2c2cb010 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser
> > *parser, bool lambda_p)
> > }
> >return NULL_TREE;
> >  }
> > -  cp_lexer_consume_token (parser->lexer);
> > +
> > +  cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2);
> > +  if (tok2->type == CPP_OPEN_BRACE)
> > +{
> > +  /* An opening brace following the start of a requires-clause
> > is
> > +ill-formed; the user likely forgot the second `requires' that
> > +would start a requires-expression.  */
> > +  gcc_rich_location richloc (tok2->location);
> > +  richloc.add_fixit_insert_before (" requires");
> 
> Thanks for adding a fix-it hint.  That said, is this spacing here
> correct?  If I'm reading it right, this adds " requires" immediately
> before the open brace, leading to a suggestion of:
>   "requires {"
> becoming:
>   "requires  requires{"
> 
> Perhaps adding it immediately after the first requires via:
>   richloc.add_fixit_insert_after (first_requires_location,
>   " requires");
> would be better, which ought to lead to a suggestion of:
>   "requires {"
> becoming:
>   "requires requires {"
> 
> (assuming I'm reading the patch right)
> 
> Might be an idea to test the effect of the fix-it hint e.g. via
> -fdiagnostics-generate-patch.
> 
> Hope this is constructive
> Dave

That makes a lot of sense, thanks.  Luckily we have the location of the
first "requires" already handy.  Here's the updated patch which
incorporates the improved fix-it hint:

-- >8 --

gcc/cp/ChangeLog:

PR c++/94306
* parser.c (cp_parser_requires_clause_opt): Diagnose and recover from
"requires {" when "requires requires {" was probably intended.

gcc/testsuite/ChangeLog:

PR c++/94306
* g++.dg/concepts/diagnostic8.C: New test.
---
 gcc/cp/parser.c | 17 -
 gcc/testsuite/g++.dg/concepts/diagnostic8.C |  6 ++
 2 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 05363653691..5bc1ecd102e 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser *parser, bool 
lambda_p)
}
   return NULL_TREE;
 }
-  cp_lexer_consume_token (parser->lexer);
+
+  cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2);
+  if (tok2->type == CPP_OPEN_BRACE)
+{
+  /* An opening brace following the start of a requires-clause is
+ill-formed; the user likely forgot the second `requires' that
+would start a requires-expression.  */
+  gcc_rich_location richloc (tok2->location);
+  richloc.add_fixit_insert_after (tok->location, " requires");
+  error_at (, "missing additional % to start "
+   "a requires-expression");
+  /* Don't consume the `requires', so that it's reused as the start of a
+requires-expression.  */
+}
+  else
+cp_lexer_consume_token (parser->lexer);
 
   if (!flag_concepts_ts)
 return cp_parser_requires_clause_expression (parser, lambda_p);
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic8.C 
b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
new file mode 100644
index 000..70d7e4a9cc1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
@@ -0,0 +1,6 @@
+// PR c++/94306
+// { dg-do compile { target c++2a } }
+
+template struct S { };
+template requires { typename T::type; } struct S { };
+// { dg-error "missing additional .requires." "" { target *-*-* } .-1 }
-- 
2.26.0.rc1.11.g30e9940356

Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)

[Martin Sebor via Gcc-patches]

On 3/26/20 4:16 PM, Jason Merrill wrote:

On 3/26/20 2:58 PM, Martin Sebor wrote:

On 3/25/20 11:36 PM, Jason Merrill wrote:

On 3/23/20 12:50 PM, Martin Sebor wrote:

On 3/23/20 8:49 AM, Jason Merrill wrote:

On 3/21/20 5:59 PM, Martin Sebor wrote:
+  /* Diagnose class/struct/union mismatches.  IS_DECLARATION 
is false

+ for alias definition.  */
+  bool decl_class = (is_declaration
+ && cp_parser_declares_only_class_p (parser));
    cp_parser_check_class_key (parser, key_loc, tag_type, 
type, false,

   cp_parser_declares_only_class_p (parser));


Don't you need to use the new variable?

Don't your testcases exercise this?


Of course they do.  This was a leftover from an experiment after I put
the initial updated patch together.  On final review I decided to 
adjust

some comments and forgot to restore the original use of the variable.

+  /* When TYPE is the use of an implicit specialization of a 
previously
+ declared template set TYPE_DECL to the type of the primary 
template
+ for the specialization and look it up in CLASS2LOC below. 
For uses
+ of explicit or partial specializations TYPE_DECL already 
points to

+ the declaration of the specialization.
+ IS_USE is clear so that the type of an implicit 
instantiation rather

+ than that of a partial specialization is determined.  */
+  type_decl = TREE_TYPE (type_decl);
+  if (TREE_CODE (type_decl) != TEMPLATE_DECL)
+    type_decl = TYPE_MAIN_DECL (type_decl);


The comment is no longer relevant to the code.  The remaining code 
also seems like it would have no effect; we already know type_decl 
is TYPE_MAIN_DECL (type).


I removed the block of code.

Martin

PS I would have preferred to resolve just the reported problem in this
patch and deal with the template specializations more fully (and with
aliases) in a followup.  As it is, it has grown bigger and more complex
than I'm comfortable with, especially with the template 
specializations,

harder for me to follow, and obviously a lot more time-consuming not
just to put together but also to review.  Although this revision 
handles
many more template specialization cases correctly, there still are 
other

(arguably corner) cases that it doesn't.  I suspect getting those right
might even require a design change, which I see as out of scope at this
time (not to mention my ability).


Sure, at this point in the cycle there's always a tradeoff between 
better functionality and risk from ballooning changes.  It looks like 
the improved template handling could still be split out into a 
separate patch, if you'd prefer.


I would prefer to get this patch committed as is now.  I appreciate
there are improvements that can be made to the code (there always
are) but, unlike the bugs it fixes, they are invisible to users and
so don't seem essential at this point.


+  /* Number of usesn of the class.  */

Typo.


+ definintion if one exists or the first declaration otherwise.  */

typo.


+  if (CLASSTYPE_USE_TEMPLATE (type) == 1 && !is_decl (0))

...

+ the first reference to the instantiation.  The primary must
+ be (and inevitably is) at index zero.  */


I think CLASSTYPE_IMPLICIT_INSTANTIATION is more readable than 
testing the value 1.


Okay.



What is the !is_decl (0) intended to do?  Changing it to an assert 
inside the 'if' doesn't seem to affect any of the testcases.


Looks like the test is an unnecessary remnant and can be removed.
In fact, both is_decl() and decl_p member don't appear necessary
anymore so I've removed them too.


+ For implicit instantiations of a primary template it's
+ the class-key used to declare the primary with.  The primary
+ must be at index zero.  */
+  const tag_types xpect_key
+    = cdlguide->class_key (cdlguide == this ? idxguide : 0);


A template can also be declared before it's defined;


Obviously, just like a class.  Is there something you expect me to
change in response to this point?


You're hardcoding index zero for the template case, which seems to 
assume that the definition of a template is always at index zero.


Sounds like you're concerned about something like:

  template  class A;
  template  class A;// expect -Wmismatched-tags
  template  struct A { };   // ...because of this
  class A a;   // expect -Wmismatched-tags

The definition should be at index zero because once it's seen, entries
for prior declarations are purged (after diagnosing mismatches).  But
I'm sure the tests for these things could stand to beefed up so there
could easily be cases that aren't handled correctly.



I think you want to move the def_p/idxdef/idxguide logic into another 
member function that you invoke on cdlguide to perhaps get the 
class_key_loc_t that you want to use as the pattern.


I'm not quite sure what you have in mind here.  I agree the cdlcode
code looks a little cumbersome and perhaps could be restructured but
it's not 

Re: [PATCH] c++: Diagnose when "requires" is used instead of "requires requires" [PR94306]

[Patrick Palka via Gcc-patches]

On Thu, 26 Mar 2020, Jason Merrill wrote:

> On 3/26/20 5:28 PM, Patrick Palka wrote:
> > This adds support to detect and recover from the case where an opening brace
> > immediately follows the start of a requires-clause.  So rather than emitting
> > the
> > error
> > 
> >error: expected primary-expression before '{' token
> > 
> > followed by a slew of irrevelant errors, we now assume the user had intended
> > to
> > write "requires requires {" and diagnose and recover accordingly.
> > 
> > Tested on x86_64-pc-linux-gnu, does this look OK?
> > 
> > gcc/cp/ChangeLog:
> > 
> > PR c++/94306
> > * parser.c (cp_parser_requires_clause_opt): Diagnose and recover from
> > "requires {" when "requires requires {" was probably intended.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > PR c++/94306
> > * g++.dg/concepts/diagnostic8.C: New test.
> > ---
> >   gcc/cp/parser.c | 17 -
> >   gcc/testsuite/g++.dg/concepts/diagnostic8.C |  6 ++
> >   2 files changed, 22 insertions(+), 1 deletion(-)
> >   create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C
> > 
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index 05363653691..73c2c2cb010 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser *parser,
> > bool lambda_p)
> > }
> > return NULL_TREE;
> >   }
> > -  cp_lexer_consume_token (parser->lexer);
> > +
> > +  cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2);
> > +  if (tok2->type == CPP_OPEN_BRACE)
> 
> Do we want to handle an open paren the same way?

Hmm, wouldn't the error recovery be significantly different for an open
paren here?  Just pretending that there's another "requires" token would
mean we proceed as if we got "requires requires (" but that is still
ill-formed I think.

> 
> > +{
> > +  /* An opening brace following the start of a requires-clause is
> > +ill-formed; the user likely forgot the second `requires' that
> > +would start a requires-expression.  */
> > +  gcc_rich_location richloc (tok2->location);
> > +  richloc.add_fixit_insert_before (" requires");
> > +  error_at (, "missing additional % to start "
> > +   "a requires-expression");
> > +  /* Don't consume the `requires', so that it's reused as the start of
> > a
> > +requires-expression.  */
> > +}
> > +  else
> > +cp_lexer_consume_token (parser->lexer);
> >   if (!flag_concepts_ts)
> >   return cp_parser_requires_clause_expression (parser, lambda_p);
> > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic8.C
> > b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
> > new file mode 100644
> > index 000..70d7e4a9cc1
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
> > @@ -0,0 +1,6 @@
> > +// PR c++/94306
> > +// { dg-do compile { target c++2a } }
> > +
> > +template struct S { };
> > +template requires { typename T::type; } struct S { };
> > +// { dg-error "missing additional .requires." "" { target *-*-* } .-1 }
> > 
> 
> 

[PATCH] c++: template keyword accepted before destructor names [PR94336]

[Marek Polacek via Gcc-patches]

This came up on the C++ core list recently.  We don't diagnose the case
when 'template' is followed by a destructor name which is not permitted
by [temp.names]/5.

PR c++/94336 - template keyword accepted before destructor names.
* parser.c (cp_parser_unqualified_id): Give an error when 'template'
is followed by a destructor name.

* g++.dg/template/template-keyword2.C: New test.
---
 gcc/cp/parser.c   | 9 +
 gcc/testsuite/g++.dg/template/template-keyword2.C | 5 +
 2 files changed, 14 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/template-keyword2.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 05363653691..8c5fc5eea09 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6068,6 +6068,15 @@ cp_parser_unqualified_id (cp_parser* parser,
  cp_lexer_consume_token (parser->lexer);
return error_mark_node;
  }
+   if (template_keyword_p)
+ {
+   if (!cp_parser_uncommitted_to_tentative_parse_p (parser))
+ error_at (tilde_loc, "% keyword not permitted in "
+   "destructor name");
+   cp_parser_simulate_error (parser);
+   return error_mark_node;
+ }
+
gcc_assert (!scope || TYPE_P (scope));
 
token = cp_lexer_peek_token (parser->lexer);
diff --git a/gcc/testsuite/g++.dg/template/template-keyword2.C 
b/gcc/testsuite/g++.dg/template/template-keyword2.C
new file mode 100644
index 000..27d874ba7d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/template-keyword2.C
@@ -0,0 +1,5 @@
+// PR c++/94336 - template keyword accepted before destructor names.
+
+template void f(T *p) { p->template ~X(); } // { dg-error 
".template. keyword not permitted in destructor name" }
+template struct X {};
+void g(X *p) { f(p); }

base-commit: 16948c54b7576fb4b27c59915eac71a0c6bf94f6
-- 
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Re: [PATCH] c++: requires-expression outside of a template is misevaluated [PR94252]

[Patrick Palka via Gcc-patches]

On Thu, 26 Mar 2020, Jason Merrill wrote:

> On 3/26/20 5:24 PM, Patrick Palka wrote:
> > On Wed, 25 Mar 2020, Jason Merrill wrote:
> > 
> > > On 3/25/20 12:17 PM, Patrick Palka wrote:
> > > > This PR reports that the requires-expression in
> > > > 
> > > > auto f = [] { };
> > > > static_assert(requires { f(); });
> > > > 
> > > > erroneously evaluates to false.  The way we end up evaluating to false
> > > > goes
> > > > as
> > > > follows.  During the parsing of the requires-expression, we call
> > > > finish_call_expr from cp_parser_requires_expression with the arguments
> > > > 
> > > > fn = VIEW_CONVERT_EXPR(f);
> > > > args = {}
> > > > 
> > > > which does the full processing of the call (since we're not inside a
> > > > template)
> > > > and returns
> > > > 
> > > > ::operator() ();
> > > > 
> > > > Later, when evaluating the requires-expression, we call finish_call_expr
> > > > again,
> > > > this time from tsubst_expr from satisfy_atom, with the arguments
> > > > 
> > > > fn = operator()
> > > > args = {  }
> > > > 
> > > > (which, as expected, correspond to the CALL_EXPR returned by
> > > > finish_call_expr
> > > > the first time).  But this time, finish_call_expr returns
> > > > error_mark_node
> > > > because
> > > > it doesn't expect to see an explicit 'this' argument in the args array,
> > > > treating
> > > > it instead as a user-written argument which causes the only candidate
> > > > function
> > > > to be discarded.  This causes the requires-expression to evaluate to
> > > > false.
> > > > 
> > > > In short, it seems finish_call_expr is not idempotent on certain inputs
> > > > when
> > > > !processing_template_decl.  Assuming this idempotency issue is not
> > > > specific
> > > > to
> > > > finish_call_expr, it seems that the safest thing to do is to avoid doing
> > > > full
> > > > semantic processing twice when parsing and evaluating a
> > > > requires-expression
> > > > that
> > > > lives outside of a template definition.
> > > 
> > > Absolutely.  We shouldn't call tsubst_expr on non-template trees.
> > > 
> > > > This patch achieves this by temporarily setting processing_template_decl
> > > > to
> > > > non-zero when parsing the body of a requires-expression.  This way, full
> > > > semantic processing will always be done only during evaluation and not
> > > > during
> > > > parsing.
> > > 
> > > Hmm, interesting approach, but I think the standard requires us to treat
> > > requires-expressions outside of template context like normal non-template
> > > code: "[Note: If a requires-expression contains invalid
> > > types or expressions in its requirements, and it does not appear within
> > > the
> > > declaration of a templated entity, then the program is ill-formed. --end
> > > note]"
> > > 
> > > So I think better to avoid the tsubst_expr later, either by immediately
> > > evaluating the REQUIRES_EXPR or marking the ATOMIC_CONSTR.  We could do
> > > that
> > > by immediately resolving the requires-expression in non-template context,
> > > or
> > > by marking it up somehow to prevent the substitution.
> > 
> > If we go the route of marking the REQUIRES_EXPR or its subtrees, then we
> > would need to change tsubst_requires_expr and its callees as well as
> > changing diagnose_requires_expr and its callees so that they
> > conditionally avoid doing tsubst_expr, which seems likes like an
> > undesirable amount of churn.
> > 
> > Another downside is that we apparently already always pretend we're
> > inside a template when parsing a nested-requirement, which means marking
> > a REQUIRES_EXPR based on processing_template_decl won't work correctly
> > for a REQUIRES_EXPR inside a nested-requirement like this one:
> > 
> >  requires { requires requires { ... }; };
> > 
> > These two points nudged me to instead go the route of pretending we're
> > inside a template when parsing the body of a requires-expr, and then
> > immediately afterwards doing tsubst_requires_expr on the body to perform
> > the full semantic processing.
> > 
> > Does the following look OK?
> > 
> > > 
> > > I notice that we currently fail to handle requires-expressions in regular
> > > expression context:
> > > 
> > > int main() { return requires { 42; }; } // ICE
> > 
> > This remains unchanged with the patch.  We could return the result of
> > tsubst_requires_expr from cp_parser_requires_expression instead of
> > throwing it away, but that would break our support for explaining an
> > unsatisfied REQUIRES_EXPR inside a static_assert, that the testcase
> > g++.dg/concepts/diagnostic7.C verifies.
> > 
> > What would be the best way to handle these stray REQUIRES_EXPRs?
> 
> We probably want to lower them in cp_genericize_r.  And I suppose if we're
> doing that we may not need the tsubst_requires_expr at parse time.

Sounds good, I'll look into this.

> 
> > -- >8 --
> > 
> > gcc/cp/ChangeLog:
> > 
> > PR c++/94252
> > * constraint.cc (tsubst_compound_requirement): Always suppress 

Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)

[Jason Merrill via Gcc-patches]

On 3/26/20 2:58 PM, Martin Sebor wrote:

On 3/25/20 11:36 PM, Jason Merrill wrote:

On 3/23/20 12:50 PM, Martin Sebor wrote:

On 3/23/20 8:49 AM, Jason Merrill wrote:

On 3/21/20 5:59 PM, Martin Sebor wrote:
+  /* Diagnose class/struct/union mismatches.  IS_DECLARATION 
is false

+ for alias definition.  */
+  bool decl_class = (is_declaration
+ && cp_parser_declares_only_class_p (parser));
    cp_parser_check_class_key (parser, key_loc, tag_type, type, 
false,

   cp_parser_declares_only_class_p (parser));


Don't you need to use the new variable?

Don't your testcases exercise this?


Of course they do.  This was a leftover from an experiment after I put
the initial updated patch together.  On final review I decided to adjust
some comments and forgot to restore the original use of the variable.

+  /* When TYPE is the use of an implicit specialization of a 
previously
+ declared template set TYPE_DECL to the type of the primary 
template
+ for the specialization and look it up in CLASS2LOC below.  
For uses
+ of explicit or partial specializations TYPE_DECL already 
points to

+ the declaration of the specialization.
+ IS_USE is clear so that the type of an implicit instantiation 
rather

+ than that of a partial specialization is determined.  */
+  type_decl = TREE_TYPE (type_decl);
+  if (TREE_CODE (type_decl) != TEMPLATE_DECL)
+    type_decl = TYPE_MAIN_DECL (type_decl);


The comment is no longer relevant to the code.  The remaining code 
also seems like it would have no effect; we already know type_decl 
is TYPE_MAIN_DECL (type).


I removed the block of code.

Martin

PS I would have preferred to resolve just the reported problem in this
patch and deal with the template specializations more fully (and with
aliases) in a followup.  As it is, it has grown bigger and more complex
than I'm comfortable with, especially with the template specializations,
harder for me to follow, and obviously a lot more time-consuming not
just to put together but also to review.  Although this revision handles
many more template specialization cases correctly, there still are other
(arguably corner) cases that it doesn't.  I suspect getting those right
might even require a design change, which I see as out of scope at this
time (not to mention my ability).


Sure, at this point in the cycle there's always a tradeoff between 
better functionality and risk from ballooning changes.  It looks like 
the improved template handling could still be split out into a 
separate patch, if you'd prefer.


I would prefer to get this patch committed as is now.  I appreciate
there are improvements that can be made to the code (there always
are) but, unlike the bugs it fixes, they are invisible to users and
so don't seem essential at this point.


+  /* Number of usesn of the class.  */

Typo.


+ definintion if one exists or the first declaration otherwise.  */

typo.


+  if (CLASSTYPE_USE_TEMPLATE (type) == 1 && !is_decl (0))

...

+ the first reference to the instantiation.  The primary must
+ be (and inevitably is) at index zero.  */


I think CLASSTYPE_IMPLICIT_INSTANTIATION is more readable than testing 
the value 1.


Okay.



What is the !is_decl (0) intended to do?  Changing it to an assert 
inside the 'if' doesn't seem to affect any of the testcases.


Looks like the test is an unnecessary remnant and can be removed.
In fact, both is_decl() and decl_p member don't appear necessary
anymore so I've removed them too.


+ For implicit instantiations of a primary template it's
+ the class-key used to declare the primary with.  The primary
+ must be at index zero.  */
+  const tag_types xpect_key
+    = cdlguide->class_key (cdlguide == this ? idxguide : 0);


A template can also be declared before it's defined;


Obviously, just like a class.  Is there something you expect me to
change in response to this point?


You're hardcoding index zero for the template case, which seems to 
assume that the definition of a template is always at index zero.


I think you want to move the def_p/idxdef/idxguide logic into another 
member function that you invoke on cdlguide to perhaps get the 
class_key_loc_t that you want to use as the pattern.


I'm not quite sure what you have in mind here.  I agree the cdlcode
code looks a little cumbersome and perhaps could be restructured but
it's not obvious to me how.  Nothing I tried looked like a clear win
so unless you consider changing how this is done a prerequisite for
accepting the whole patch I'd rather not spend any more time at this
stage iterating over refinements of it.  Please let me know soon.


I mean that


+  const unsigned ndecls = locvec.length (); > +  const bool def_p = idxdef < 
ndecls;
+  const unsigned idxguide = def_p ? idxdef : 0;


are based on whether the instantiation, rather than the template, is 
defined.


I's probably enough to update ndecls to cdlguide->locvec.length() 

Re: [PATCH] c++: Diagnose when "requires" is used instead of "requires requires" [PR94306]

[David Malcolm via Gcc-patches]

On Thu, 2020-03-26 at 17:28 -0400, Patrick Palka via Gcc-patches wrote:
> This adds support to detect and recover from the case where an
> opening brace
> immediately follows the start of a requires-clause.  So rather than
> emitting the
> error
> 
>   error: expected primary-expression before '{' token
> 
> followed by a slew of irrevelant errors, we now assume the user had
> intended to
> write "requires requires {" and diagnose and recover accordingly.
> 
> Tested on x86_64-pc-linux-gnu, does this look OK?
> 
> gcc/cp/ChangeLog:
> 
>   PR c++/94306
>   * parser.c (cp_parser_requires_clause_opt): Diagnose and
> recover from
>   "requires {" when "requires requires {" was probably intended.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c++/94306
>   * g++.dg/concepts/diagnostic8.C: New test.
> ---
>  gcc/cp/parser.c | 17 -
>  gcc/testsuite/g++.dg/concepts/diagnostic8.C |  6 ++
>  2 files changed, 22 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 05363653691..73c2c2cb010 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser
> *parser, bool lambda_p)
>   }
>return NULL_TREE;
>  }
> -  cp_lexer_consume_token (parser->lexer);
> +
> +  cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2);
> +  if (tok2->type == CPP_OPEN_BRACE)
> +{
> +  /* An opening brace following the start of a requires-clause
> is
> +  ill-formed; the user likely forgot the second `requires' that
> +  would start a requires-expression.  */
> +  gcc_rich_location richloc (tok2->location);
> +  richloc.add_fixit_insert_before (" requires");

Thanks for adding a fix-it hint.  That said, is this spacing here
correct?  If I'm reading it right, this adds " requires" immediately
before the open brace, leading to a suggestion of:
  "requires {"
becoming:
  "requires  requires{"

Perhaps adding it immediately after the first requires via:
  richloc.add_fixit_insert_after (first_requires_location,
  " requires");
would be better, which ought to lead to a suggestion of:
  "requires {"
becoming:
  "requires requires {"

(assuming I'm reading the patch right)

Might be an idea to test the effect of the fix-it hint e.g. via
-fdiagnostics-generate-patch.

Hope this is constructive
Dave

> +  error_at (, "missing additional % to start
> "
> + "a requires-expression");
> +  /* Don't consume the `requires', so that it's reused as the
> start of a
> +  requires-expression.  */
> +}
> +  else
> +cp_lexer_consume_token (parser->lexer);
>  
>if (!flag_concepts_ts)
>  return cp_parser_requires_clause_expression (parser, lambda_p);
> diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic8.C
> b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
> new file mode 100644
> index 000..70d7e4a9cc1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
> @@ -0,0 +1,6 @@
> +// PR c++/94306
> +// { dg-do compile { target c++2a } }
> +
> +template struct S { };
> +template requires { typename T::type; } struct S { };
> +// { dg-error "missing additional .requires." "" { target *-*-* } .-
> 1 }

Re: rs6000: Update bswap64-4 test to reflect actual results

[Segher Boessenkool]

Hi!

On Tue, Mar 24, 2020 at 01:26:25PM -0500, will schmidt wrote:
>   Update existing testcase powerpc/bswap64-4.c to
> reflect that we generate ldbrx and stdbrx instructions
> for newer cpu targets.  This is in contrast to the 
> pair of lwbrx and stwbrx instructions that are
> generated with older cpu targets.

So, this has always been broken for p7 and later?  Or what changed?

Okay for trunk.  Thanks!


Segher


> testsuite/
>   * gcc.target/powerpc/bswap64-4.c: Update scan-assembler
>   expected results.

Re: [PATCH v6] c++: DR1710, template keyword in a typename-specifier [PR94057]

[Jason Merrill via Gcc-patches]

On 3/26/20 3:44 PM, Marek Polacek wrote:

On Thu, Mar 26, 2020 at 01:43:28AM -0400, Jason Merrill wrote:

I think you want to check all typedefs like in e.g. find_parameter_packs_r;
if the name is a typedef, it's only suitable if
alias_template_specialization_p.


..this: Since alias_template_specialization_p already checks TYPE_P and
typedef_variant_p, can we simply use that?


But not all typedefs are aliases; 'typedef A type;' does not make
'::template type' valid just because '::template A' would be.  If it's
an alias template specialization, OK; if it's any other typedef, not OK.


This is actually already handled well, '::template type' is rejected but
'::template A' works.  I've added alias-decl3.C to that effect but
otherwise there are no changes in this patch:


Then the patch is OK, thanks.


-- >8 --
Consider

   template  class A {
 template  class B {
   void fn(typename A::B);
 };
   };

which is rejected with
error: 'typename A::B' names 'template template class 
A::B', which is not a type
whereas clang/icc/msvc accept it.

"typename A::B" is a typename-specifier.  Sadly, our comments
don't mention it anywhere, because the typename-specifier wasn't in C++11;
it was only added to the language in N1376.  Instead, we handle it as
an elaborated-type-specifier (not a problem thus far).   So we get to
cp_parser_nested_name_specifier_opt which has a loop that breaks if we
don't see a < or ::, but that means we can -- tentatively -- parse even
B which is not a nested-name-specifier (it doesn't end with a ::).

I think this should compile because [temp.names]/4 says: "In a qualified-id
used as the name in a typename-specifier, elaborated-type-specifier,
using-declaration, or class-or-decltype, an optional keyword template
appearing at the top level is ignored.", added in DR 1710.  Also see
DR 1812.

This issue on its own is not a significant problem or a regression.
However, in C++20, the typename here becomes optional, and so this test
is rejected in C++20, but accepted in C++17:

   template  class A {
 template  class B {
   void fn(A::B);
 };
   };

Here we morph A::B into a typename-specifier, but that happens
in cp_parser_simple_type_specifier and we never handle it as above.
To fake the template keyword I'm afraid we need to use cp_parser_template_id
with template_keyword_p=true as in the patch below.  The tricky thing
is to avoid breaking concepts.

To handle DR 1710, I made cp_parser_nested_name_specifier_opt assume that
when we're naming a type, the template keyword is present, too.  That
revealed a bug: DR 1710 also says that the template keyword can be followed
by an alias template, but we weren't prepared to handle that.  alias-decl?.C
exercise this.

gcc/cp:

DR 1710
PR c++/94057 - template keyword in a typename-specifier.
* parser.c (check_template_keyword_in_nested_name_spec): New.
(cp_parser_nested_name_specifier_opt): Implement DR1710, optional
'template'.  Call check_template_keyword_in_nested_name_spec.
(cp_parser_simple_type_specifier): Assume that a <
following a qualified-id in a typename-specifier begins
a template argument list.

gcc/testsuite:

DR 1710
PR c++/94057 - template keyword in a typename-specifier.
* g++.dg/cpp1y/alias-decl1.C: New test.
* g++.dg/cpp1y/alias-decl2.C: New test.
* g++.dg/cpp1y/alias-decl3.C: New test.
* g++.dg/parse/missing-template1.C: Update dg-error.
* g++.dg/parse/template3.C: Likewise.
* g++.dg/template/error4.C: Likewise.
* g++.dg/template/meminit2.C: Likewise.
* g++.dg/template/dependent-name5.C: Likewise.
* g++.dg/template/dependent-name7.C: New test.
* g++.dg/template/dependent-name8.C: New test.
* g++.dg/template/dependent-name9.C: New test.
* g++.dg/template/dependent-name10.C: New test.
* g++.dg/template/dependent-name11.C: New test.
* g++.dg/template/dependent-name12.C: New test.
* g++.dg/template/dependent-name13.C: New test.
* g++.dg/template/dr1794.C: New test.
* g++.dg/template/dr314.C: New test.
* g++.dg/template/dr1710.C: New test.
* g++.dg/template/dr1710-2.C: New test.
* g++.old-deja/g++.pt/crash38.C: Update dg-error.
---
  gcc/cp/parser.c   | 79 ---
  gcc/testsuite/g++.dg/cpp1y/alias-decl1.C  |  9 +++
  gcc/testsuite/g++.dg/cpp1y/alias-decl2.C  |  8 ++
  gcc/testsuite/g++.dg/cpp1y/alias-decl3.C  |  9 +++
  .../g++.dg/parse/missing-template1.C  |  4 +-
  gcc/testsuite/g++.dg/parse/template3.C|  5 +-
  .../g++.dg/template/dependent-name10.C| 18 +
  .../g++.dg/template/dependent-name11.C| 15 
  .../g++.dg/template/dependent-name12.C|  7 ++
  .../g++.dg/template/dependent-name13.C|  8 ++
  .../g++.dg/template/dependent-name5.C |  2 -
  

Re: [PATCH v3 2/4] libffi/test: Fix compilation for build sysroot

[Maciej W. Rozycki via Gcc-patches]

On Tue, 24 Mar 2020, Mike Stump wrote:

> > Have we made any conclusions WRT the way to move forward with this stuff?  
> > Things remain broken and I'd prefer to get the issues off the plate while 
> > the stuff is hot, or at least mildly warm.  I'm about to get distracted 
> > with other work.
> 
> It's unfortunate that upstream has anything that prevents it from us 
> just importing it all and calling it done.
> 
> Anyway, if you see a path forward for grabbing all the Makefile/config 
> stuff and leaving the abi changing stuff out, and just important that, 
> we can do a partial import.  I say this without reviewing the diffs from 
> upstream and how many there are and what's in them.  I'm hoping things 
> are nicely segregated and reasonably small otherwise.

 Thank you for your input.

 I have actually considered extracting the bits already, but I hesitated 
putting that forward that as having looked at the part that we require I 
have thought it to be very messy: the .exp file is handcrafted with an 
inline piece of scriptery buried in `configure.ac' and never cleaned, not 
even with `make distclean', nor equipped with any Makefile dependencies.  
Clearly it must have been written by someone who hasn't been accustomed to 
working with GNU autotools.

 The ultimate change is very small, but it has only been created gradually 
with four commits in the upstream libffi repository, none of which applies
cleanly to ours and most of which include unrelated stuff.  They are:

- commit 8308984e479e ("[PATCH] Make sure we're running dejagnu tests with 
  the right compiler."),

- commit 2d9b3939751b ("[PATCH] Fix for closures with sunpro compiler"),

- commit 0c3824702d3d ("[PATCH] Always set CC_FOR_TARGET for dejagnu, to 
  make the testsuite respect $CC"),

- commit 7d698125b1f0 ("[PATCH] Use the proper C++ compiler to run C++ 
  tests") -- not yet needed in our libffi version as no tests are marked
  C++.

-- at .  I have now extracted the 
relevant bits from the four commits and the result is below.

 I have pushed it through my testing and it fixes the test results just 
like my earlier proposal; in fact libffi.log files are the same modulo 
timestamps and one number that is randomly generated.  It is worth noting 
however that the multilib discovery logic in `libffi-init' has not been 
updated unlike with my proposal and it continues using the compiler 
hardcoded within rather than one set with CC_FOR_TARGET/CXX_FOR_TARGET.

 That uses a mechanism (*_FOR_TARGET, interpreted within `target_compile') 
different from one we do (*_UNDER_TEST, used to set `compiler=' in the 
invocation of `target_compile'), and ignores TOOL_EXECUTABLE altogether.  
It makes sense however semantically to me for a standalone library test 
suite to consider the compiler just a tool in testing and not the object 
under test.  Plus it makes it easy to define compilers for the various 
languages required.

 So I am in favour of retaining the mechanism rather than using my earlier 
proposal, however I'm in two minds as to how to proceed.  Integrating the 
change as it is will make us having clutter left in the tree after `make 
distclean', but we can do it right away.  Fixing the problems with the 
change upstream in libffi first and then merging the result back into our 
tree will avoid getting the clutter, but will likely take time.

 I'll sleep on it yet, and meanwhile I'll be happy to hear suggestions.  
I have also cc-ed the libffi mailing list for a possible further insight.

  Maciej

---
 libffi/configure |5 +
 libffi/configure.ac  |5 +
 libffi/testsuite/Makefile.am |2 ++
 libffi/testsuite/Makefile.in |1 +
 4 files changed, 13 insertions(+)

Index: gcc/libffi/configure
===
--- gcc.orig/libffi/configure
+++ gcc/libffi/configure
@@ -14961,6 +14961,11 @@ _ACEOF
 
 
 
+cat > local.exp <&5
 $as_echo_n "checking whether to enable maintainer-specific portions of 
Makefiles... " >&6; }
Index: gcc/libffi/configure.ac
===
--- gcc.orig/libffi/configure.ac
+++ gcc/libffi/configure.ac
@@ -61,6 +61,11 @@ AC_PROG_LIBTOOL
 # Test for 64-bit build.
 AC_CHECK_SIZEOF([size_t])
 
+cat > local.exp <

Re: [committed] Add missing T register clobber for SH port

[Jeff Law via Gcc-patches]

On Thu, 2020-03-26 at 12:38 -0600, Jeff Law wrote:
> On Wed, 2020-03-25 at 23:10 -0400, Hans-Peter Nilsson wrote:
> > On Wed, 25 Mar 2020, Jeff Law via Gcc-patches wrote:
> > 
> > The patch you sent, as well as what you committed as r10-7383,
> > was just a ChangeLog entry.
> > 
> > > Bootstrapped on sh4-linux-gnu and sh4eb-linux-gnu.  Regression testing in
> > > progress (won't finish for ~12 hours).  No new test as it's covered by
> > > vector-
> > > compare-1 in the testsuite.
> > 
> > ...so I guess you've noticed by the time you read this. :]
> :-)
> 
> Here's the actual patch:
> 
> commit 48817fbd7616f086ac7bb1dd38b862f78762c9b8
> Author: Jeff Law 
> Date:   Wed Mar 25 14:33:08 2020 -0600
> 
> Fix vector-compare-1 regressions on sh4/sh4eb caused by pattern
> clobbering T reg without expressing that in its RTL.
> 
> PR rtl-optimization/90275
> * config/sh/sh.md (mov_neg_si_t): Clobber the T register in 
> the
> pattern.
And sadly, the only thing this fixed was the regressions recently flagged by the
tester.  It didn't fix any of the pre-existing failures.  Sigh.

jeff
> 

Re: [PATCH] c++: requires-expression outside of a template is misevaluated [PR94252]

[Jason Merrill via Gcc-patches]

On 3/26/20 5:24 PM, Patrick Palka wrote:

On Wed, 25 Mar 2020, Jason Merrill wrote:


On 3/25/20 12:17 PM, Patrick Palka wrote:

This PR reports that the requires-expression in

auto f = [] { };
static_assert(requires { f(); });

erroneously evaluates to false.  The way we end up evaluating to false goes
as
follows.  During the parsing of the requires-expression, we call
finish_call_expr from cp_parser_requires_expression with the arguments

fn = VIEW_CONVERT_EXPR(f);
args = {}

which does the full processing of the call (since we're not inside a
template)
and returns

::operator() ();

Later, when evaluating the requires-expression, we call finish_call_expr
again,
this time from tsubst_expr from satisfy_atom, with the arguments

fn = operator()
args = {  }

(which, as expected, correspond to the CALL_EXPR returned by
finish_call_expr
the first time).  But this time, finish_call_expr returns error_mark_node
because
it doesn't expect to see an explicit 'this' argument in the args array,
treating
it instead as a user-written argument which causes the only candidate
function
to be discarded.  This causes the requires-expression to evaluate to false.

In short, it seems finish_call_expr is not idempotent on certain inputs when
!processing_template_decl.  Assuming this idempotency issue is not specific
to
finish_call_expr, it seems that the safest thing to do is to avoid doing
full
semantic processing twice when parsing and evaluating a requires-expression
that
lives outside of a template definition.


Absolutely.  We shouldn't call tsubst_expr on non-template trees.


This patch achieves this by temporarily setting processing_template_decl to
non-zero when parsing the body of a requires-expression.  This way, full
semantic processing will always be done only during evaluation and not
during
parsing.


Hmm, interesting approach, but I think the standard requires us to treat
requires-expressions outside of template context like normal non-template
code: "[Note: If a requires-expression contains invalid
types or expressions in its requirements, and it does not appear within the
declaration of a templated entity, then the program is ill-formed. --end
note]"

So I think better to avoid the tsubst_expr later, either by immediately
evaluating the REQUIRES_EXPR or marking the ATOMIC_CONSTR.  We could do that
by immediately resolving the requires-expression in non-template context, or
by marking it up somehow to prevent the substitution.


If we go the route of marking the REQUIRES_EXPR or its subtrees, then we
would need to change tsubst_requires_expr and its callees as well as
changing diagnose_requires_expr and its callees so that they
conditionally avoid doing tsubst_expr, which seems likes like an
undesirable amount of churn.

Another downside is that we apparently already always pretend we're
inside a template when parsing a nested-requirement, which means marking
a REQUIRES_EXPR based on processing_template_decl won't work correctly
for a REQUIRES_EXPR inside a nested-requirement like this one:

 requires { requires requires { ... }; };

These two points nudged me to instead go the route of pretending we're
inside a template when parsing the body of a requires-expr, and then
immediately afterwards doing tsubst_requires_expr on the body to perform
the full semantic processing.

Does the following look OK?



I notice that we currently fail to handle requires-expressions in regular
expression context:

int main() { return requires { 42; }; } // ICE


This remains unchanged with the patch.  We could return the result of
tsubst_requires_expr from cp_parser_requires_expression instead of
throwing it away, but that would break our support for explaining an
unsatisfied REQUIRES_EXPR inside a static_assert, that the testcase
g++.dg/concepts/diagnostic7.C verifies.

What would be the best way to handle these stray REQUIRES_EXPRs?


We probably want to lower them in cp_genericize_r.  And I suppose if 
we're doing that we may not need the tsubst_requires_expr at parse time.



-- >8 --

gcc/cp/ChangeLog:

PR c++/94252
* constraint.cc (tsubst_compound_requirement): Always suppress errors
from type_deducible_p and expression_convertible_p, as they're not
substitution errors.
(diagnose_atomic_constraint) : Remove this case so
that we diagnose INTEGER_CST expressions of non-bool type via the
default case.
* parser.c (cp_parser_requires_expression): Always parse the requirement
body as if we're processing a template, by temporarily incrementing
processing_template_decl.  Afterwards, if we're not actually in a
template context, perform semantic processing to diagnose any invalid
types and expressions.
* pt.c (tsubst_copy_and_build) : Remove dead code.


Why is this dead code?


* semantics.c (finish_static_assert): Also explain an assertion failure
when the condition is a 

Re: [PATCH] c++: Diagnose when "requires" is used instead of "requires requires" [PR94306]

[Jason Merrill via Gcc-patches]

On 3/26/20 5:28 PM, Patrick Palka wrote:

This adds support to detect and recover from the case where an opening brace
immediately follows the start of a requires-clause.  So rather than emitting the
error

   error: expected primary-expression before '{' token

followed by a slew of irrevelant errors, we now assume the user had intended to
write "requires requires {" and diagnose and recover accordingly.

Tested on x86_64-pc-linux-gnu, does this look OK?

gcc/cp/ChangeLog:

PR c++/94306
* parser.c (cp_parser_requires_clause_opt): Diagnose and recover from
"requires {" when "requires requires {" was probably intended.

gcc/testsuite/ChangeLog:

PR c++/94306
* g++.dg/concepts/diagnostic8.C: New test.
---
  gcc/cp/parser.c | 17 -
  gcc/testsuite/g++.dg/concepts/diagnostic8.C |  6 ++
  2 files changed, 22 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 05363653691..73c2c2cb010 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser *parser, bool 
lambda_p)
}
return NULL_TREE;
  }
-  cp_lexer_consume_token (parser->lexer);
+
+  cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2);
+  if (tok2->type == CPP_OPEN_BRACE)


Do we want to handle an open paren the same way?


+{
+  /* An opening brace following the start of a requires-clause is
+ill-formed; the user likely forgot the second `requires' that
+would start a requires-expression.  */
+  gcc_rich_location richloc (tok2->location);
+  richloc.add_fixit_insert_before (" requires");
+  error_at (, "missing additional % to start "
+   "a requires-expression");
+  /* Don't consume the `requires', so that it's reused as the start of a
+requires-expression.  */
+}
+  else
+cp_lexer_consume_token (parser->lexer);
  
if (!flag_concepts_ts)

  return cp_parser_requires_clause_expression (parser, lambda_p);
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic8.C 
b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
new file mode 100644
index 000..70d7e4a9cc1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
@@ -0,0 +1,6 @@
+// PR c++/94306
+// { dg-do compile { target c++2a } }
+
+template struct S { };
+template requires { typename T::type; } struct S { };
+// { dg-error "missing additional .requires." "" { target *-*-* } .-1 }

[PATCH] c++: Diagnose when "requires" is used instead of "requires requires" [PR94306]

[Patrick Palka via Gcc-patches]

This adds support to detect and recover from the case where an opening brace
immediately follows the start of a requires-clause.  So rather than emitting the
error

  error: expected primary-expression before '{' token

followed by a slew of irrevelant errors, we now assume the user had intended to
write "requires requires {" and diagnose and recover accordingly.

Tested on x86_64-pc-linux-gnu, does this look OK?

gcc/cp/ChangeLog:

PR c++/94306
* parser.c (cp_parser_requires_clause_opt): Diagnose and recover from
"requires {" when "requires requires {" was probably intended.

gcc/testsuite/ChangeLog:

PR c++/94306
* g++.dg/concepts/diagnostic8.C: New test.
---
 gcc/cp/parser.c | 17 -
 gcc/testsuite/g++.dg/concepts/diagnostic8.C |  6 ++
 2 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic8.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 05363653691..73c2c2cb010 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27639,7 +27639,22 @@ cp_parser_requires_clause_opt (cp_parser *parser, bool 
lambda_p)
}
   return NULL_TREE;
 }
-  cp_lexer_consume_token (parser->lexer);
+
+  cp_token *tok2 = cp_lexer_peek_nth_token (parser->lexer, 2);
+  if (tok2->type == CPP_OPEN_BRACE)
+{
+  /* An opening brace following the start of a requires-clause is
+ill-formed; the user likely forgot the second `requires' that
+would start a requires-expression.  */
+  gcc_rich_location richloc (tok2->location);
+  richloc.add_fixit_insert_before (" requires");
+  error_at (, "missing additional % to start "
+   "a requires-expression");
+  /* Don't consume the `requires', so that it's reused as the start of a
+requires-expression.  */
+}
+  else
+cp_lexer_consume_token (parser->lexer);
 
   if (!flag_concepts_ts)
 return cp_parser_requires_clause_expression (parser, lambda_p);
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic8.C 
b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
new file mode 100644
index 000..70d7e4a9cc1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic8.C
@@ -0,0 +1,6 @@
+// PR c++/94306
+// { dg-do compile { target c++2a } }
+
+template struct S { };
+template requires { typename T::type; } struct S { };
+// { dg-error "missing additional .requires." "" { target *-*-* } .-1 }
-- 
2.26.0.rc1.11.g30e9940356

Re: [PATCH] c++: requires-expression outside of a template is misevaluated [PR94252]

[Patrick Palka via Gcc-patches]

On Wed, 25 Mar 2020, Jason Merrill wrote:

> On 3/25/20 12:17 PM, Patrick Palka wrote:
> > This PR reports that the requires-expression in
> > 
> >auto f = [] { };
> >static_assert(requires { f(); });
> > 
> > erroneously evaluates to false.  The way we end up evaluating to false goes
> > as
> > follows.  During the parsing of the requires-expression, we call
> > finish_call_expr from cp_parser_requires_expression with the arguments
> > 
> >fn = VIEW_CONVERT_EXPR(f);
> >args = {}
> > 
> > which does the full processing of the call (since we're not inside a
> > template)
> > and returns
> > 
> >::operator() ();
> > 
> > Later, when evaluating the requires-expression, we call finish_call_expr
> > again,
> > this time from tsubst_expr from satisfy_atom, with the arguments
> > 
> >fn = operator()
> >args = {  }
> > 
> > (which, as expected, correspond to the CALL_EXPR returned by
> > finish_call_expr
> > the first time).  But this time, finish_call_expr returns error_mark_node
> > because
> > it doesn't expect to see an explicit 'this' argument in the args array,
> > treating
> > it instead as a user-written argument which causes the only candidate
> > function
> > to be discarded.  This causes the requires-expression to evaluate to false.
> > 
> > In short, it seems finish_call_expr is not idempotent on certain inputs when
> > !processing_template_decl.  Assuming this idempotency issue is not specific
> > to
> > finish_call_expr, it seems that the safest thing to do is to avoid doing
> > full
> > semantic processing twice when parsing and evaluating a requires-expression
> > that
> > lives outside of a template definition.
> 
> Absolutely.  We shouldn't call tsubst_expr on non-template trees.
> 
> > This patch achieves this by temporarily setting processing_template_decl to
> > non-zero when parsing the body of a requires-expression.  This way, full
> > semantic processing will always be done only during evaluation and not
> > during
> > parsing.
> 
> Hmm, interesting approach, but I think the standard requires us to treat
> requires-expressions outside of template context like normal non-template
> code: "[Note: If a requires-expression contains invalid
> types or expressions in its requirements, and it does not appear within the
> declaration of a templated entity, then the program is ill-formed. --end
> note]"
> 
> So I think better to avoid the tsubst_expr later, either by immediately
> evaluating the REQUIRES_EXPR or marking the ATOMIC_CONSTR.  We could do that
> by immediately resolving the requires-expression in non-template context, or
> by marking it up somehow to prevent the substitution.

If we go the route of marking the REQUIRES_EXPR or its subtrees, then we
would need to change tsubst_requires_expr and its callees as well as
changing diagnose_requires_expr and its callees so that they
conditionally avoid doing tsubst_expr, which seems likes like an
undesirable amount of churn.

Another downside is that we apparently already always pretend we're
inside a template when parsing a nested-requirement, which means marking
a REQUIRES_EXPR based on processing_template_decl won't work correctly
for a REQUIRES_EXPR inside a nested-requirement like this one:

requires { requires requires { ... }; };

These two points nudged me to instead go the route of pretending we're
inside a template when parsing the body of a requires-expr, and then
immediately afterwards doing tsubst_requires_expr on the body to perform
the full semantic processing.

Does the following look OK?

> 
> I notice that we currently fail to handle requires-expressions in regular
> expression context:
> 
> int main() { return requires { 42; }; } // ICE

This remains unchanged with the patch.  We could return the result of
tsubst_requires_expr from cp_parser_requires_expression instead of
throwing it away, but that would break our support for explaining an
unsatisfied REQUIRES_EXPR inside a static_assert, that the testcase
g++.dg/concepts/diagnostic7.C verifies.

What would be the best way to handle these stray REQUIRES_EXPRs?

-- >8 --

gcc/cp/ChangeLog:

PR c++/94252
* constraint.cc (tsubst_compound_requirement): Always suppress errors
from type_deducible_p and expression_convertible_p, as they're not
substitution errors.
(diagnose_atomic_constraint) : Remove this case so
that we diagnose INTEGER_CST expressions of non-bool type via the
default case.
* parser.c (cp_parser_requires_expression): Always parse the requirement
body as if we're processing a template, by temporarily incrementing
processing_template_decl.  Afterwards, if we're not actually in a
template context, perform semantic processing to diagnose any invalid
types and expressions.
* pt.c (tsubst_copy_and_build) : Remove dead code.
* semantics.c (finish_static_assert): Also explain an assertion failure
when the condition is a 

Re: [PATCH] avoid modifying type in attribute access handler [PR94098]

[Martin Sebor via Gcc-patches]

On 3/25/20 3:56 PM, Jason Merrill wrote:

On 3/16/20 4:41 PM, Martin Sebor wrote:

The recent fix to avoid modifying in place the type argument in
handle_access_attribute (PR 92721) was incomplete and didn't fully
resolve the problem (an ICE in the C++ front-end).  The attached
patch removes the remaining modification that causes the ICE.  In
addition, the change adjusts checking calls to functions declared
with the attribute to scan all its instances.

The attached patch was tested on x86_64-linux.



   attrs = remove_attribute (IDENTIFIER_POINTER (name), attrs);


This unchanged line can still modify existing types; I think if attrs 
already has a matching attribute you need to work harder to replace it.


I have a number of questions.

1) There are two instances of the call above in the code (only one in
the context of the patch).  Are you referring specifically to the one
in the patch or to both?  The one in the patch manipulates the type
attributes of the last DECL (NODE[1]).  The other one, attributes of
the current type (NODE[0]).  Are both a problem?

2) What all do you include in "modifying existing types?"  A number
of attribute handlers unconditionally modify the type of a *NODE
without first testing ATTR_FLAG_TYPE_IN_PLACE (e.g., by setting
TYPE_USER_ALIGN, or TREE_USED).  Are those modifications safe?  If
they are, what makes the difference between those and the code above?

3) Is the restriction on "modifying existing types" specific to
the C++ front end or a general one that applies to the whole C/C++
family?  (I've only seen a failure in the C++ FE.)

4) What exactly does "work harder" mean and how do I test it?  Are
you suggesting to call build_type_attribute_variant (or maybe
build_variant_type_copy like the existing handlers do) to replace
the type?  Or are you implying that unless I need to replace
the existing type  I should avoid modifying the TYPE's
TYPE_ATTRIBUTES and instead work with a copy of the attribute chain?

These restrictions and the lack of enforcement (only in the C++ FE)
make nontrivial handlers extremely fragile.  Unless the restrictions
are entirely C++-specific I would really like to add assertions to
decl_attributes to catch these problems earlier (and in C as well).
Either way, I also want to add test cases to exercise them.  But
I need to understand them first (i.e., get answers to the questions
above).  Nothing I've tried so far has triggered a problem due to
the code you point out above.

Martin

Re: C++ 'NON_LVALUE_EXPR' in OMP array section handling

[Thomas Schwinge]

Hi!

On 2020-03-26T09:09:01-0600, Sandra Loosemore  wrote:
> On 3/26/20 8:27 AM, Thomas Schwinge wrote:
>> Note that as this code is shared between OpenACC/OpenMP, this might
>> affect OpenMP, too, as far as I can tell.  (Subject updated.)  Jakub, can
>> you please have a look, too?
>>
>> On 2020-03-25T23:02:38-0600, Sandra Loosemore  
>> wrote:
>>> The attached patch fixes a bug I found in the C++ front end's handling
>>> of OpenACC data clauses.  The problem here is that the C++ front end
>>> wraps the array bounds expressions in NON_LVALUE_EXPR tree nodes, and
>>> the code here (which appears to have been copied from similar code in
>>> the C front end) was failing to strip those before checking to see if
>>> they were INTEGER_CST nodes, etc.
>>
>> So, I had a quick look.  I'm confirming the 'NON_LVALUE_EXPR' (C++ only,
>> not seen for C) difference, and that 'STRIP_NOPS' gets rid of these.
>> However, I also in some code paths see, for example, 'integer_nonzerop'
>> calls, which internally do 'STRIP_ANY_LOCATION_WRAPPER'.
>>
>> I don't know if 'STRIP_NOPS' is the correct "hammer" to use here, I don't
>> know what the usual convention is: explicitly remove (via 'STRIP_NOPS' as
>> you suggested, or something else), or have the enquiry functions do it
>> ('STRIP_ANY_LOCATION_WRAPPER' as 'integer_nonzerop' is doing, for
>> example).
>>
>> Empirical data doesn't mean too much here, of course, I'm not seeing a
>> lot of explicit 'STRIP_*' calls in 'gcc/cp/semantics.c'.  ;-)
>
> I saw that STRIP_NOPS seem to be the preferred way to do things in e.g.
> fold-const.c.  I don't know if there's a reason to use some less general
> form of STRIP_* here?
>
>>> Sadly, I have no test case for this because it was only triggering an
>>> error in conjunction with some other OpenACC patches that are not yet on
>>> trunk
>>
>> So maybe the problem is actually that these "other OpenACC patches" are
>> not sufficiently sanitizing the input data they're doing checks on?
>
> No.  In the current code on trunk, both places that are being patched
> continue with checks like
>
> TREE_CODE (low_bound) == INTEGER_CST
>
> etc.  So when they're wrapped in NON_LVALUE_EXPRs, it's basically
> failing to detect constants in array dimension specifiers and falling
> through to other code.

Eh, indeed...  ;-\ (So we should be able to deduce some misbehavior from
that, to construct a test case.  I'll have a look.)

>>> and the tree dumps don't show anything useful.
>>
>> Well, the 'original' tree dumps do show (C++) vs. don't show (C) the
>> 'NON_LVALUE_EXPR's.

While true, that of course doesn't tell us anything what the OMP array
section handling is doing with these.

I guess I was half-asleep when I wrote my email...  ;-/

So.  I'm not objecting to handling 'NON_LVALUE_EXPR's locally here via
any kind of 'STRIP_*', but it somehow doesn't seem the general solution.
Another option seems to be to teach 'fold_simple' to handle
'NON_LVALUE_EXPR's, so that the existing code:

/* We need to reduce to real constant-values for checks below.  */
if (length)
  length = fold_simple (length);
if (low_bound)
  low_bound = fold_simple (low_bound);
if (low_bound
&& TREE_CODE (low_bound) == INTEGER_CST
&& [...])
  low_bound = fold_convert (sizetype, low_bound);
if (length
&& TREE_CODE (length) == INTEGER_CST
&& [...])
  length = fold_convert (sizetype, length);

... would then just work.  But: I don't know if 'fold_simple' (and
others?) should handle 'NON_LVALUE_EXPR's, or if there's a reason why it
doesn't.  (Have not yet tried to figure that out.)  What I can tell is
that the attached patch does solve the issue in the C++ OMP array section
handling, and survives a powerpc64le-unknown-linux-gnu
'--enable-checking=yes' (will now re-run with 'fold' checking) bootstrap,
with no testsuite regressions.

Hmm.


Grüße
 Thomas


-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
>From 611fbe24b7e459829c0a304a58963d4987c8de0a Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Thu, 26 Mar 2020 21:22:54 +0100
Subject: [PATCH] Fold 'NON_LVALUE_EXPR' some more

---
 gcc/cp/constexpr.c | 1 +
 gcc/fold-const.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 192face9a3a..f31d61c1460 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -6650,6 +6650,7 @@ fold_simple_1 (tree t)
 case BIT_NOT_EXPR:
 case TRUTH_NOT_EXPR:
 case NOP_EXPR:
+case NON_LVALUE_EXPR:
 case VIEW_CONVERT_EXPR:
 case CONVERT_EXPR:
 case FLOAT_EXPR:
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 71a1d3eb735..b6bc5080ff3 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -1739,6 +1739,7 @@ const_unop (enum tree_code code, tree type, tree arg0)
   switch (code)
 {
 CASE_CONVERT:
+case 

Re: [PATCH v6] c++: DR1710, template keyword in a typename-specifier [PR94057]

[Marek Polacek via Gcc-patches]

On Thu, Mar 26, 2020 at 01:43:28AM -0400, Jason Merrill wrote:
> > > I think you want to check all typedefs like in e.g. 
> > > find_parameter_packs_r;
> > > if the name is a typedef, it's only suitable if
> > > alias_template_specialization_p.
> > 
> > ..this: Since alias_template_specialization_p already checks TYPE_P and
> > typedef_variant_p, can we simply use that?
> 
> But not all typedefs are aliases; 'typedef A type;' does not make
> '::template type' valid just because '::template A' would be.  If it's
> an alias template specialization, OK; if it's any other typedef, not OK.

This is actually already handled well, '::template type' is rejected but
'::template A' works.  I've added alias-decl3.C to that effect but
otherwise there are no changes in this patch:

-- >8 --
Consider

  template  class A {
template  class B {
  void fn(typename A::B);
};
  };

which is rejected with
error: 'typename A::B' names 'template template class 
A::B', which is not a type
whereas clang/icc/msvc accept it.

"typename A::B" is a typename-specifier.  Sadly, our comments
don't mention it anywhere, because the typename-specifier wasn't in C++11;
it was only added to the language in N1376.  Instead, we handle it as
an elaborated-type-specifier (not a problem thus far).   So we get to
cp_parser_nested_name_specifier_opt which has a loop that breaks if we
don't see a < or ::, but that means we can -- tentatively -- parse even
B which is not a nested-name-specifier (it doesn't end with a ::).

I think this should compile because [temp.names]/4 says: "In a qualified-id
used as the name in a typename-specifier, elaborated-type-specifier,
using-declaration, or class-or-decltype, an optional keyword template
appearing at the top level is ignored.", added in DR 1710.  Also see
DR 1812.

This issue on its own is not a significant problem or a regression.
However, in C++20, the typename here becomes optional, and so this test
is rejected in C++20, but accepted in C++17:

  template  class A {
template  class B {
  void fn(A::B);
};
  };

Here we morph A::B into a typename-specifier, but that happens
in cp_parser_simple_type_specifier and we never handle it as above.
To fake the template keyword I'm afraid we need to use cp_parser_template_id
with template_keyword_p=true as in the patch below.  The tricky thing
is to avoid breaking concepts.

To handle DR 1710, I made cp_parser_nested_name_specifier_opt assume that
when we're naming a type, the template keyword is present, too.  That
revealed a bug: DR 1710 also says that the template keyword can be followed
by an alias template, but we weren't prepared to handle that.  alias-decl?.C
exercise this.

gcc/cp:

DR 1710
PR c++/94057 - template keyword in a typename-specifier.
* parser.c (check_template_keyword_in_nested_name_spec): New.
(cp_parser_nested_name_specifier_opt): Implement DR1710, optional
'template'.  Call check_template_keyword_in_nested_name_spec.
(cp_parser_simple_type_specifier): Assume that a <
following a qualified-id in a typename-specifier begins
a template argument list.

gcc/testsuite:

DR 1710
PR c++/94057 - template keyword in a typename-specifier.
* g++.dg/cpp1y/alias-decl1.C: New test.
* g++.dg/cpp1y/alias-decl2.C: New test.
* g++.dg/cpp1y/alias-decl3.C: New test.
* g++.dg/parse/missing-template1.C: Update dg-error.
* g++.dg/parse/template3.C: Likewise.
* g++.dg/template/error4.C: Likewise.
* g++.dg/template/meminit2.C: Likewise.
* g++.dg/template/dependent-name5.C: Likewise.
* g++.dg/template/dependent-name7.C: New test.
* g++.dg/template/dependent-name8.C: New test.
* g++.dg/template/dependent-name9.C: New test.
* g++.dg/template/dependent-name10.C: New test.
* g++.dg/template/dependent-name11.C: New test.
* g++.dg/template/dependent-name12.C: New test.
* g++.dg/template/dependent-name13.C: New test.
* g++.dg/template/dr1794.C: New test.
* g++.dg/template/dr314.C: New test.
* g++.dg/template/dr1710.C: New test.
* g++.dg/template/dr1710-2.C: New test.
* g++.old-deja/g++.pt/crash38.C: Update dg-error.
---
 gcc/cp/parser.c   | 79 ---
 gcc/testsuite/g++.dg/cpp1y/alias-decl1.C  |  9 +++
 gcc/testsuite/g++.dg/cpp1y/alias-decl2.C  |  8 ++
 gcc/testsuite/g++.dg/cpp1y/alias-decl3.C  |  9 +++
 .../g++.dg/parse/missing-template1.C  |  4 +-
 gcc/testsuite/g++.dg/parse/template3.C|  5 +-
 .../g++.dg/template/dependent-name10.C| 18 +
 .../g++.dg/template/dependent-name11.C| 15 
 .../g++.dg/template/dependent-name12.C|  7 ++
 .../g++.dg/template/dependent-name13.C|  8 ++
 .../g++.dg/template/dependent-name5.C |  2 -
 .../g++.dg/template/dependent-name7.C |  9 +++
 

[PATCH] c++: Avoid calls in non-evaluated contexts affect whether function can or can't throw [PR94326]

[Jakub Jelinek via Gcc-patches]

Hi!

The following testcase FAILs -fcompare-debug, because if we emit a
-Wreturn-local-addr warning, we tsubst decltype in order to print the
warning and as that function could throw, set_flags_from_callee during that
sets cp_function_chain->can_throw and later on we don't set TREE_NOTHROW
on foo.  While with -w or -Wno-return-local-addr, tsubst isn't called during
the warning_at, cp_function_chain->can_throw is kept clear and TREE_NOTHROW
is set on foo.
It isn't just a matter of the warning though, in
int foo ();
int bar () { return sizeof (foo ()); }
int baz () { return sizeof (int); }
I don't really see why we should mark only baz as TREE_NOTHROW and not bar
too, when neither can really throw.

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

2020-03-26  Jakub Jelinek  

PR c++/94326
* call.c (set_flags_from_callee): Don't update
cp_function_chain->can_throw or current_function_returns_abnormally
if cp_unevaluated_operand.

* g++.dg/other/pr94326.C: New test.

--- gcc/cp/call.c.jj2020-03-25 08:05:07.153731580 +0100
+++ gcc/cp/call.c   2020-03-26 15:03:42.432909693 +0100
@@ -333,11 +333,14 @@ set_flags_from_callee (tree call)
   && internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW)
 nothrow = true;
 
-  if (!nothrow && at_function_scope_p () && cfun && cp_function_chain)
-cp_function_chain->can_throw = 1;
+  if (cfun && cp_function_chain && !cp_unevaluated_operand)
+{
+  if (!nothrow && at_function_scope_p ())
+   cp_function_chain->can_throw = 1;
 
-  if (decl && TREE_THIS_VOLATILE (decl) && cfun && cp_function_chain)
-current_function_returns_abnormally = 1;
+  if (decl && TREE_THIS_VOLATILE (decl))
+   current_function_returns_abnormally = 1;
+}
 
   TREE_NOTHROW (call) = nothrow;
 }
--- gcc/testsuite/g++.dg/other/pr94326.C.jj 2020-03-26 15:14:23.609400216 
+0100
+++ gcc/testsuite/g++.dg/other/pr94326.C2020-03-26 15:14:54.162947065 
+0100
@@ -0,0 +1,19 @@
+// PR c++/94326
+// { dg-do compile { target c++11 } }
+// { dg-options "-fcompare-debug" }
+
+template  struct A {
+  const int () { return 0; }   // { dg-warning "returning reference to 
temporary" }
+  template  void bar(_Kt) { foo(); }
+};
+struct B {
+  A<> b;
+  template  auto baz(_Kt p1) -> decltype(b.bar(p1)) {
+b.bar(p1);
+  }
+};
+struct C {};
+void operator<(C, int) {
+  B a;
+  a.baz(C{});
+}

Jakub

Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]

[Jason Merrill via Gcc-patches]

On 3/22/20 9:21 PM, Patrick Palka wrote:

This patch relaxes an assertion in tsubst_default_argument that exposes a latent
bug in how we substitute an array type into a cv-qualified wildcard function
parameter type.  Concretely, the latent bug is that given the function template

   template void foo(const T t);

one would expect the type of foo to be void(const int*), but we
(seemingly prematurely) strip function parameter types of their top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead end up
obtaining void(int*) as the type of foo after substitution and decaying.

We still however correctly substitute into and decay the formal parameter type,
obtaining const int* as the type of t after substitution.  But this then leads
to us tripping over the assert in tsubst_default_argument that verifies the
formal parameter type and the function type are consistent.

Assuming it's too late at this stage to fix the substitution bug, we can still
relax the assertion like so.  Tested on x86_64-pc-linux-gnu, does this look OK?


This is core issues 1001/1322, which have not been resolved.  Clang does 
the substitution the way you suggest; EDG rejects the testcase because 
the two substitutions produce different results.  I think it would make 
sense to follow the EDG behavior until this issue is actually resolved.



gcc/cp/ChangeLog:

PR c++/92010
* pt.c (tsubst_default_argument): Relax assertion to permit
disagreements between the function type and the parameter type
about the cv-qualification of the pointed-to type.

gcc/testsuite/ChangeLog:

PR c++/92010
* g++.dg/template/defarg22.C: New test.
---
  gcc/cp/pt.c  | 17 -
  gcc/testsuite/g++.dg/template/defarg22.C | 11 +++
  2 files changed, 27 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/template/defarg22.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 9e1eb9416c9..923166276b8 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13337,7 +13337,22 @@ tsubst_default_argument (tree fn, int parmnum, tree 
type, tree arg,
if (parmtype == error_mark_node)
  return error_mark_node;
  
-  gcc_assert (same_type_ignoring_top_level_qualifiers_p (type, parmtype));

+  if (same_type_ignoring_top_level_qualifiers_p (type, parmtype))
+;
+  else if (POINTER_TYPE_P (type) && POINTER_TYPE_P (parmtype))
+{
+  /* The function type and the parameter type can disagree about the
+cv-qualification of the pointed-to type; see PR92010.  */
+  gcc_assert (same_type_p (TREE_TYPE (type),
+  strip_top_quals (TREE_TYPE (parmtype;
+  /* Verify that this happens only when the dependent parameter type is a
+cv-qualified wildcard type.  */
+  tree pattern_parm = get_pattern_parm (parm, DECL_TI_TEMPLATE (fn));
+  gcc_assert (WILDCARD_TYPE_P (TREE_TYPE (pattern_parm))
+ && cv_qualified_p (TREE_TYPE (pattern_parm)));
+}
+  else
+gcc_unreachable ();
  
tree *slot;

if (defarg_inst && (slot = defarg_inst->get (parm)))
diff --git a/gcc/testsuite/g++.dg/template/defarg22.C 
b/gcc/testsuite/g++.dg/template/defarg22.C
new file mode 100644
index 000..cf6261916d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/defarg22.C
@@ -0,0 +1,11 @@
+// PR c++/92010
+// { dg-do compile }
+
+template 
+void foo(const T t = 0)
+{ }
+
+int main()
+{
+  foo();
+}

Re: rs6000 - allow builtin initialization regardless of mask

[Segher Boessenkool]

Hi!

On Mon, Mar 23, 2020 at 03:18:25PM -0500, will schmidt wrote:
>   Disable the code that limits initialization of builtins based
> on the rs6000_builtin_mask.  This allows all built-ins to be
> properly referenced when building code using #pragma for cpu
> targets newer than what was specified by the -mcpu default.
> The use of built-ins is still properly limited by logic within
> altivec_resolve_overloaded_builtin().
> 
> I'm still reviewing test results for any regressions.
> 
> OK for master?

Okay (if those tests pass ;-) ), thanks!  Just a few nits:


>   * config/rs6000/rs6000-call.c altivec_init_builtins():  Remove code
>   to skip defining builtins based on builtin_mask.

* config/rs6000/rs6000-call.c (altivec_init_builtins): Remove code
to skip defining builtins based on builtin_mask.


> testsuite/

gcc/testsuite/


> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pragma_misc9.c
> @@ -0,0 +1,47 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */

This is the default for anything in gcc.target/powerpc?  { dg-do compile }
can be useful (for a reader; for the test itself it is default as well),
but drop the target selector please?


Segher

[PATCH] c++: Handle COMPOUND_EXPRs in ocp_convert [PR94339]

[Jakub Jelinek via Gcc-patches]

Hi!

My recent change to get_narrower/warnings_for_convert_and_check broke
the following testcase, warnings_for_convert_and_check is upset that
expr is a COMPOUND_EXPR with INTEGER_CST at the rightmost operand, while
result is a COMPOUND_EXPR with a NOP_EXPR of INTEGER_CST at the rightmost
operand, it expects such conversions to be simplified.

The easiest fix seems to be to handle COMPOUND_EXPRs in ocp_convert too,
by converting the rightmost operand and recreating COMPOUND_EXPR(s) if that
changed.

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

The attr-copy-2.C change is a workaround for PR94346, where we now ICE on
the testcase, while previously we'd ICE only if it contained a comma
expression at the outer level rather than cast of a COMPOUND_EXPR to
something.  I'll defer that to Martin.

2020-03-26  Jakub Jelinek  

PR c++/94339
* cvt.c (ocp_convert): Handle COMPOUND_EXPR by recursion on the second
operand and creating a new COMPOUND_EXPR if anything changed.

* g++.dg/other/pr94339.C: New test.
* g++.dg/ext/attr-copy-2.C: Comment out failing tests due to PR94346.

--- gcc/cp/cvt.c.jj 2020-01-12 11:54:36.0 +0100
+++ gcc/cp/cvt.c2020-03-26 12:30:52.312170071 +0100
@@ -697,6 +697,17 @@ ocp_convert (tree type, tree expr, int c
   if (error_operand_p (e) || type == error_mark_node)
 return error_mark_node;
 
+  if (TREE_CODE (e) == COMPOUND_EXPR)
+{
+  e = ocp_convert (type, TREE_OPERAND (e, 1), convtype, flags, complain);
+  if (e == error_mark_node)
+   return error_mark_node;
+  if (e == TREE_OPERAND (expr, 1))
+   return expr;
+  return build2_loc (EXPR_LOCATION (expr), COMPOUND_EXPR, TREE_TYPE (e),
+TREE_OPERAND (expr, 0), e);
+}
+
   complete_type (type);
   complete_type (TREE_TYPE (expr));
 
--- gcc/testsuite/g++.dg/other/pr94339.C.jj 2020-03-26 12:37:01.645664573 
+0100
+++ gcc/testsuite/g++.dg/other/pr94339.C2020-03-26 12:32:36.217621191 
+0100
@@ -0,0 +1,11 @@
+// PR c++/94339
+// { dg-do compile }
+
+unsigned a;
+void bar ();
+
+unsigned
+foo (bool x)
+{
+  return x ? bar (), -1 : a;
+}
--- gcc/testsuite/g++.dg/ext/attr-copy-2.C.jj   2020-01-12 11:54:37.0 
+0100
+++ gcc/testsuite/g++.dg/ext/attr-copy-2.C  2020-03-26 20:05:34.464882638 
+0100
@@ -36,8 +36,8 @@ typedef struct C
   ATTR (copy (*(struct A *)(0, 1))) long m_xpa_0_1;
 
   ATTR (copy (((struct A *)0)[0])) short m_arpa_0;
-  ATTR (copy (((struct A *)(1, 0))[0])) int m_arpa_1_0;
-  ATTR (copy (((struct A *)(0, 1))[0])) long m_arpa_0_1;
+//  ATTR (copy (((struct A *)(1, 0))[0])) int m_arpa_1_0;
+//  ATTR (copy (((struct A *)(0, 1))[0])) long m_arpa_0_1;
 
   ATTR (copy (a)) short m_a;
   ATTR (copy (b.a)) int m_b_a;
@@ -86,8 +86,8 @@ static_assert (__builtin_has_attribute (
 static_assert (__builtin_has_attribute (((C*)0)->m_ra, packed));
 
 static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0, packed));
-static_assert (__builtin_has_attribute (((C*)0)->m_arpa_1_0, packed));
-static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0_1, packed));
+//static_assert (__builtin_has_attribute (((C*)0)->m_arpa_1_0, packed));
+//static_assert (__builtin_has_attribute (((C*)0)->m_arpa_0_1, packed));
 
 static_assert (__builtin_has_attribute (((C*)0)->m_a, packed));
 static_assert (__builtin_has_attribute (((C*)0)->m_b_a, packed));

Jakub

Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824)

[Martin Sebor via Gcc-patches]

On 3/25/20 11:36 PM, Jason Merrill wrote:

On 3/23/20 12:50 PM, Martin Sebor wrote:

On 3/23/20 8:49 AM, Jason Merrill wrote:

On 3/21/20 5:59 PM, Martin Sebor wrote:
+  /* Diagnose class/struct/union mismatches.  IS_DECLARATION is 
false

+ for alias definition.  */
+  bool decl_class = (is_declaration
+ && cp_parser_declares_only_class_p (parser));
    cp_parser_check_class_key (parser, key_loc, tag_type, type, 
false,

   cp_parser_declares_only_class_p (parser));


Don't you need to use the new variable?

Don't your testcases exercise this?


Of course they do.  This was a leftover from an experiment after I put
the initial updated patch together.  On final review I decided to adjust
some comments and forgot to restore the original use of the variable.

+  /* When TYPE is the use of an implicit specialization of a 
previously
+ declared template set TYPE_DECL to the type of the primary 
template
+ for the specialization and look it up in CLASS2LOC below.  For 
uses
+ of explicit or partial specializations TYPE_DECL already 
points to

+ the declaration of the specialization.
+ IS_USE is clear so that the type of an implicit instantiation 
rather

+ than that of a partial specialization is determined.  */
+  type_decl = TREE_TYPE (type_decl);
+  if (TREE_CODE (type_decl) != TEMPLATE_DECL)
+    type_decl = TYPE_MAIN_DECL (type_decl);


The comment is no longer relevant to the code.  The remaining code 
also seems like it would have no effect; we already know type_decl is 
TYPE_MAIN_DECL (type).


I removed the block of code.

Martin

PS I would have preferred to resolve just the reported problem in this
patch and deal with the template specializations more fully (and with
aliases) in a followup.  As it is, it has grown bigger and more complex
than I'm comfortable with, especially with the template specializations,
harder for me to follow, and obviously a lot more time-consuming not
just to put together but also to review.  Although this revision handles
many more template specialization cases correctly, there still are other
(arguably corner) cases that it doesn't.  I suspect getting those right
might even require a design change, which I see as out of scope at this
time (not to mention my ability).


Sure, at this point in the cycle there's always a tradeoff between 
better functionality and risk from ballooning changes.  It looks like 
the improved template handling could still be split out into a separate 
patch, if you'd prefer.


I would prefer to get this patch committed as is now.  I appreciate
there are improvements that can be made to the code (there always
are) but, unlike the bugs it fixes, they are invisible to users and
so don't seem essential at this point.


+  /* Number of usesn of the class.  */

Typo.


+ definintion if one exists or the first declaration otherwise.  */

typo.


+  if (CLASSTYPE_USE_TEMPLATE (type) == 1 && !is_decl (0))

...

+ the first reference to the instantiation.  The primary must
+ be (and inevitably is) at index zero.  */


I think CLASSTYPE_IMPLICIT_INSTANTIATION is more readable than testing 
the value 1.


Okay.



What is the !is_decl (0) intended to do?  Changing it to an assert 
inside the 'if' doesn't seem to affect any of the testcases.


Looks like the test is an unnecessary remnant and can be removed.
In fact, both is_decl() and decl_p member don't appear necessary
anymore so I've removed them too.


+ For implicit instantiations of a primary template it's
+ the class-key used to declare the primary with.  The primary
+ must be at index zero.  */
+  const tag_types xpect_key
+    = cdlguide->class_key (cdlguide == this ? idxguide : 0);


A template can also be declared before it's defined;


Obviously, just like a class.  Is there something you expect me to
change in response to this point?

I think you want to 
move the def_p/idxdef/idxguide logic into another member function that 
you invoke on cdlguide to perhaps get the class_key_loc_t that you want 
to use as the pattern.


I'm not quite sure what you have in mind here.  I agree the cdlcode
code looks a little cumbersome and perhaps could be restructured but
it's not obvious to me how.  Nothing I tried looked like a clear win
so unless you consider changing how this is done a prerequisite for
accepting the whole patch I'd rather not spend any more time at this
stage iterating over refinements of it.  Please let me know soon.

Attached is a revised patch with the other changes above.

Martin
PR c++/94078 - bogus and missing -Wmismatched-tags on an instance of a template
PR c++/93824 - bogus -Wredundant-tags on a first declaration in use
PR c++/93810 - missing -Wmismatched-tags and -Wredundant-tags on a typedef of an implicit class template specialization

gcc/cp/ChangeLog:

	PR c++/94078
	PR c++/93824
	PR c++/93810
	* cp-tree.h (most_specialized_partial_spec): Declare.
	* parser.c 

Re: [committed] Add missing T register clobber for SH port

[Jeff Law via Gcc-patches]

On Wed, 2020-03-25 at 23:10 -0400, Hans-Peter Nilsson wrote:
> On Wed, 25 Mar 2020, Jeff Law via Gcc-patches wrote:
> 
> The patch you sent, as well as what you committed as r10-7383,
> was just a ChangeLog entry.
> 
> > Bootstrapped on sh4-linux-gnu and sh4eb-linux-gnu.  Regression testing in
> > progress (won't finish for ~12 hours).  No new test as it's covered by
> > vector-
> > compare-1 in the testsuite.
> 
> ...so I guess you've noticed by the time you read this. :]
:-)

Here's the actual patch:

commit 48817fbd7616f086ac7bb1dd38b862f78762c9b8
Author: Jeff Law 
Date:   Wed Mar 25 14:33:08 2020 -0600

Fix vector-compare-1 regressions on sh4/sh4eb caused by pattern
clobbering T reg without expressing that in its RTL.

PR rtl-optimization/90275
* config/sh/sh.md (mov_neg_si_t): Clobber the T register in the
pattern.

diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
index 4a1797160cf..fc80278a395 100644
--- a/gcc/config/sh/sh.md
+++ b/gcc/config/sh/sh.md
@@ -8395,9 +8395,15 @@
 ;; Store (negated) T bit as all zeros or ones in a reg.
 ;; subcRn,Rn   ! Rn = Rn - Rn - T; T = T
 ;; not Rn,Rn   ! Rn = 0 - Rn
+;;
+;; Note the call to sh_split_treg_set_expr may clobber
+;; the T reg.  We must express this, even though it's
+;; not immediately obvious this pattern changes the
+;; T register.
 (define_insn_and_split "mov_neg_si_t"
   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
-   (neg:SI (match_operand 1 "treg_set_expr")))]
+   (neg:SI (match_operand 1 "treg_set_expr")))
+   (clobber (reg:SI T_REG))]
   "TARGET_SH1"
 {
   gcc_assert (t_reg_operand (operands[1], VOIDmode));

Re: [PATCH] rs6000: Allow FPRs to change between SDmode and DDmode [PR94254]

[Segher Boessenkool]

Hi!

On Mon, Mar 23, 2020 at 06:37:52PM +, Richard Sandiford wrote:
> g:497498c878d48754318e486428e2aa30854020b9 caused lra to cycle
> on some SDmode reloads for power6.  As explained in more detail
> in the PR comments, the problem was a conflict between two target
> hooks: rs6000_secondary_memory_needed_mode required SDmode FPR
> reloads to use DDmode memory (rightly, since using SDmode memory
> wouldn't make progress) but rs6000_can_change_mode_class didn't
> allow FPRs to change from SDmode to DDmode.  Previously lra
> ignored that and changed the mode anyway.
> 
> >From what Segher says, it sounds like the "from_size < 8 || to_size < 8"
> check is mostly there for SF<->64-bit subregs, and that SDmode is stored
> in the way that target-independent code expects.

That is what it looks like to me, yes.

> This patch therefore
> allows SD<->DD changes.
> 
> I wondered about checking for SD<->64-bit changes instead, but that
> seemed like an unnecessary generalisation for this stage.

Yeah, good call.

> Bootstrapped and regression-tested on powerpc64le-unknown-linux-gnu,
> both with default settings and with --with-cpu=power6.  (For power6
> I reverted the lra patch to do the before testing, since the build
> hanged otherwise.)  The patch fixed the regressing testcases and
> introduced no other test changes.  Zdenek confirms that it also
> fixes powerpc-linux-gnu builds.  OK to install?

(Already committed).

Thanks for doing the patch!


Segher


> gcc/
>   PR target/94254
>   * config/rs6000/rs6000.c (rs6000_can_change_mode_class): Allow
>   FPRs to change between SDmode and DDmode.

Re: [RFC] Fix for pr64706 testcase faulre

[Jan Hubicka]

> 
> Why don't we represent the alias at the cgraph level?  That is,
> why do decls come into play at all here?  b prevails and it has a
> reference to a.c:a so we need to keep (and emit) that.  The only issue
> would be that we'd end up with two 'a's so we have to notice that
> and fixup somehow.
> 
> Isn't this in the end a similar issue as with extern inline bodies
> vs. the offline copy body?

The difference is that, for the first time, we need to load two
different bodies for one DECL (because first is reachable by alias in
a.c and other is reahable by the prevailing non-weak declaration).
Function bodies are bound to decls and not symbols and this needs to be done 
somehow.
> 
> > I would like to do this w/o need to stream in the body, because such 
> > situation
> > happens quite commonly local aliases: Until we are able to prove that all
> > referencs to a local aliase of a symbol are gone we need to assume that body
> > needs to be preserved and those are potentially quite frequent.
> > 
> > So basically only solution I think of is to make lto-stream-in live with the
> > fact that function decl was changed.  So we can stream original decl to the
> > beggining of the function section and during stream in do rewritting of
> > DECL_CONTEXT similar way as tree-inline would do.  We need to be careful 
> > about
> > keeping debug info in shape too (we are not producing inline copy, we are
> > merely unsharing the main decl).
> > 
> > What do you think?
> 
> I think the not prevailed but still needed cgraph node for 'a' should
> have its decl cleared ;)

I am not sure I understand what you mean here :)
> 
> > Note that I was considering case of simply not sharing decls with
> > aliases at stream time.  This does not work because by localizing the
> > decl one will not redirect references to the symbol correctly.
> 
> But all remaining references should be via 'b', not via 'a'.

Yes, they are, that is why I create a.static and b becomes alias of it.
Moving body of a.c:a to b would also work, but it is essentially the
same problem.  All we would need to do is to pick an "winning" alias,
make it the main definition and make aliases alias of it.
I think this would only introduce furhter problem, since signature of b
is not guaranteed to be same as signature of a.

Honza
> 
> > Other option would be to always produce a static symbol for every
> > symbol with an alias.  This again can be IMO only done by cloning it and
> > would be relatively expensive because at the stream out time we do have
> > considerable quantity of aliases especially with -fpic.
> > 
> > Honza
> > 
> > gcc/ChangeLog:
> > 
> > 2020-03-26  Jan Hubicka  
> > 
> > * cgraph.c (cgraph_node::make_local): Add force parameter.
> > * cgraph.h (symtab_node::clone_referring_skip_aliases): New
> > member function.
> > (symtab_node::stream_in_summary_p): Likewise.
> > * ipa-fnsummary.c (inline_read_section): Use
> > symtab_node::stream_in_summary_p.
> > * ipa-prop.c (ipa_read_node_info): Likewise.
> > * symtab.c (symtab_node::clone_referring): Fix formating.
> > (symtab_node::clone_referring_skip_aliases): New member function.
> > (symtab_node::dump_referring): Fix formating
> > (symtab_node::stream_in_summary_p): New member function.
> > 
> > gcc/lto/ChangeLog:
> > 
> > 2020-03-26  Jan Hubicka  
> > 
> > * lto-symtab.c (lto_symtab_symbol_p): New function.
> > (has_prevailing_alias_p): New function.
> > (lto_symtab_replace_node): Break out from ...; support alias
> > interposition
> > (lto_cgraph_replace_node): ... here.
> > (lto_varpool_replace_node): Use the common code.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 2020-03-26  Jan Hubicka  
> > 
> > * gcc.dg/lto/alias-resolution-1_0.c: New test.
> > * gcc.dg/lto/alias-resolution-1_1.c: New test.
> > 
> > diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> > index 6b780f80eb3..1f1319ed5fe 100644
> > --- a/gcc/cgraph.c
> > +++ b/gcc/cgraph.c
> > @@ -2465,9 +2465,10 @@ cgraph_node::call_for_symbol_thunks_and_aliases 
> > (bool (*callback)
> >  /* Worker to bring NODE local.  */
> >  
> >  bool
> > -cgraph_node::make_local (cgraph_node *node, void *)
> > +cgraph_node::make_local (cgraph_node *node, void *force)
> >  {
> > -  gcc_checking_assert (node->can_be_local_p ());
> > +  if (!force)
> > +gcc_checking_assert (node->can_be_local_p ());
> >if (DECL_COMDAT (node->decl) || DECL_EXTERNAL (node->decl))
> >  {
> >node->make_decl_local ();
> > @@ -2486,12 +2487,14 @@ cgraph_node::make_local (cgraph_node *node, void *)
> >return false;
> >  }
> >  
> > -/* Bring cgraph node local.  */
> > +/* Bring cgraph node local.  If FORCE is true skip checking if symbol can
> > +   become local.  */
> >  
> >  void
> > -cgraph_node::make_local (void)
> > +cgraph_node::make_local (bool force)
> >  {
> > -  call_for_symbol_thunks_and_aliases (cgraph_node::make_local, NULL, true);
> > +  call_for_symbol_thunks_and_aliases 

Re: [RS6000] PR94145, make PLT loads volatile

[Segher Boessenkool]

Hi!

On Mon, Mar 23, 2020 at 12:21:03PM +1030, Alan Modra wrote:
> On Wed, Mar 18, 2020 at 04:53:59PM -0500, Segher Boessenkool wrote:
> > Could you please send a new patch (could be the same patch even) that
> > is easier to review for me?
> 
> The PLT is volatile.  On PowerPC it is a bss style section which the
> dynamic loader initialises to point at resolver stubs (called glink on
> PowerPC64) to support lazy resolution of function addresses.  The
> first call to a given function goes via the dynamic loader symbol
> resolver, which updates the PLT entry for that function and calls the
> function.  The second call, if there is one and we don't have a
> multi-threaded race, will use the updated PLT entry and thus avoid
> the relatively slow symbol resolver path.

Okay, so it isn't volatile, we have the guarantee that it will stay the
same after we have called the function once (on this same execution
thread)?

> Calls via the PLT are like calls via a function pointer, except that
> no initialised function pointer is volatile like the PLT.  All
> initialised function pointers are resolved at program startup to point
> at the function or are left as NULL.  There is no support for lazy
> resolution of any user visible function pointer.
> 
> So why does any of this matter to gcc?  Well, normally the PLT call
> mechanism happens entirely behind gcc's back, but since we implemented
> inline PLT calls (effectively putting the PLT code stub that loads the
> PLT entry inline and making that code sequence scheduled), the load of
> the PLT entry is visible to gcc.  That load then is subject to gcc
> optimization, for example in
> 
> /* -S -mcpu=future -mpcrel -mlongcall -O2.  */
> int foo (int);
> void bar (void)
> {
>   while (foo(0))
> foo (99);
> }
> 
> we see the PLT load for foo being hoisted out of the loop and stashed
> in a call-saved register.  If that happens to be the first call to
> foo, then the stashed value is that for the resolver stub, and every
> call to foo in the loop will then go via the slow resolver path.  Not
> a good idea.  Also, if foo turns out to be a local function and the
> linker replaces the PLT calls with direct calls to foo then gcc has
> just wasted a call-saved register.

So you are saying that calling the PLT directly is always faster than
calling via a function pointer, even if that is the correct resolved
address?  That is the part I am worried about.

I think that is right, but I don't quite see it, and I don't know what
is done at runtime nearly well enough :-/

> This patch teaches gcc that the PLT loads are volatile.  The change
> doesn't affect other loads of function pointers and thus has no effect
> on normal indirect function calls.  Note that because the
> "optimization" this patch prevents can only occur over function calls,
> the only place gcc can stash PLT loads is in call-saved registers or
> in other memory.  I'm reasonably confident that this change will be
> neutral or positive for the "ld -z now" case where the PLT is not
> volatile, in code where there is any register pressure.

That is good enough for me :-)

> Even if gcc
> could be taught to recognise cases where the PLT is resolved, you'd
> need to discount use of registers to cache PLT loads by some factor
> involving the chance that those calls would be converted to direct
> calls..

>   PR target/94145
>   * config/rs6000/rs6000.c (rs6000_longcall_ref): Use unspec_volatile
>   for PLT16_LO and PLT_PCREL.
>   * config/rs6000/rs6000.md (UNSPEC_PLT16_LO, UNSPEC_PLT_PCREL): Remove.
>   (UNSPECV_PLT16_LO, UNSPECV_PLT_PCREL): Define.
>   (pltseq_plt16_lo_, pltseq_plt_pcrel): Use unspec_volatile.

Okay for trunk.  Thank you!


Segher

Re: [RFC] Fix for pr64706 testcase faulre

[Jan Hubicka]

> > Is it?  What does a non-weak local alias to a weak function mean?
> 
> I think we should continue to try to model ELF semantics re weak and 
> aliases.  If so, then yes, LTO gets it wrong and the above testcase should 
> not abort.  Weak doesn't enter the picture for creating aliases (the 
> aliases is created with the declaration, and we require an available 
> definition, and the alias is henceforth bound to _that_ very definition).  
> I.e. the 'a.c:b' name should continue to refer to the same code snippet 
> identified by the a.c:a name, not be redirected to the overriding b.c:a.
> 
> I'm wondering about the amount of code necessary to fix this.  I think 
> that points towards a wrong level of representation somewhere, and perhaps 
> _that_ should be changed instead of trying to work around it.
> 
> For instance, right now aliases are different from non-aliases, whereas in 
> reality there's no difference: there are simply names referring to code or 
> data snippets, and it so happens that for some snippets there are multiple 
> names, and it further just so happens that some names for a code snippet 
> become overridden/removed by other names for other code snippets, which 
> doesn't invalidate the fact that there still is another name for the first 
> snippet.
> 
> If it were modelled like that in cgraph/lto all the rest would naturally 
> follow.  (Of course you would need tracking of when some code/data 
> snippets can't be referred to by name anymore (and hence are useless), 
> because all names for them went away, or none of the existing names is 
> used anymore, but I don't think that materially would change much in our 
> internal data structures).

Yep, the trouble is caused by fact that GCC representation is not quite
what linker does. I.e. we bind function bodies with their declarations
and declarations with symbols.
We are bouit about assumption of 1to1 correspondence between function
bodies and their symbols.
This is not true because one body may have multiple aliases but also
it may define mutliple different symbols (alternative entry points &
labels).

Reorganizing this design is quite some work.

We need to change trees/GIMPLE to use symbols instead of DECLs when
referring to them.  So parameter of CALL_EXPR would not be decl but
symbol associated with it. 

We to move everyting that is property of symbol away from decl into
symbols (i.e. assembler name, visibility, section etc) and thus we would
need to change everything from frontends all the way to RTL.

I am gradually shuffling things in this direction (I plan to move
assembler names to symbols and gradually do that for visibilitie so we
can have cross-module referneces to labels and finally support static
initializers taking addresses of them), but the process is
slow and I think it makes sense to first fix correctness issues with
what we have rahter than waiting for major surgery to be finished.

Honza
> 
> 
> Ciao,
> Michael.

Re: [PATCH][RFC] API extension for binutils (type of symbols).

[Jeff Law via Gcc-patches]

On Mon, 2020-03-09 at 17:56 +0100, Martin Liška wrote:
> On 3/9/20 4:36 PM, H.J. Lu wrote:
> > We nee to support different variables, like TLS, data and bss variables.
> 
> Why do we need TLS? Right now, it's not supported by nm. Or am I wrong?
> 
> About BSS and DATA I agree that it would be handy. I can theoretically
> covered with code in get_variable_section/bss_initializer_p. But it's
> quite logic and I'm not sure we should simulate it.
> 
> @Honza/Richi: Do you have any opinion about that?
Or could we just fix the damn broken configure scripts?  Isn't that what's
driving all this nonsense?

The HP-UX bits for this in configure work correctly.I verified that months
ago and even have a sed script which will take a broken configure script and fix
it.

jeff

[og9] Really fix og9 "Fix hang when running oacc exec with CUDA 9.0 nvprof"

[Thomas Schwinge]

Hi!

On 2020-03-25T18:09:25+0100, I wrote:
> On 2018-02-22T12:23:25+0100, Tom de Vries  wrote:
>> when using cuda 9 nvprof with an openacc executable, the executable hangs.

> What Frederik has discovered today in the hard way... is that the og9
> version of this patch did get its code altered in a way so that it no
> longer resolves the problem it's meant to resolve -- the hang was back.
> On Git-mirror-based openacc-gcc-9-branch that's:
>
> commit 84af3c5a2fbb5023057e2ca319b0c22f5f7d4795
> Author: Julian Brown 
> AuthorDate: Tue Feb 26 16:00:54 2019 -0800
> Commit: Kwok Cheung Yeung 
> CommitDate: Fri May 31 13:40:07 2019 -0700
>
> Fix hang when running oacc exec with CUDA 9.0 nvprof
>
> 2018-09-20  Tom de Vries  
> Cesar Philippidis  
>
> libgomp/
> [...]
>
> ..., which got cherry-picked (automated, without any review) into current
> devel/omp/gcc-9 in commit f752d880a5abc591a25ad22fb892363f6520bcf1.

OK, I had confused myself here.  I wrongly blamed that commit to be
responsible for the hang being back, when in fact it's only the later og9
"OpenACC Profiling Interface (incomplete)" backport from trunk that
introduced the problem.  On Git-mirror-based openacc-gcc-9-branch that's:

commit 1246da4f164bcf2ec4430b89686a38c47e55b5f9
Author: tschwinge 
AuthorDate: Fri May 17 19:13:36 2019 +
Commit: Kwok Cheung Yeung 
CommitDate: Fri Jul 26 14:32:02 2019 -0700

OpenACC Profiling Interface (incomplete)

libgomp/
[...]

..., which got cherry-picked (automated, without any review) into current
devel/omp/gcc-9 in commit 9342531a7fc9f6e368e37bbd4ea9f4d01f43514e.

The confusing thing was that the og9 "Fix hang when running oacc exec
with CUDA 9.0 nvprof" commit appears *before* the og9 "OpenACC Profiling
Interface (incomplete)" backport that it relates to.

And, in addition to that, I pushed the wrong (incomplete) version of my
fix.

> Of course, it would've helped tremendously had the original og7 commit
> included a test case...  :'-/ (... by simply reproducing the nested calls
> that CUDA 9 nvprof seems to be doing.)
>
> Still without a test case, for now I have pushed the attached patch to
> devel/omp/gcc-9 in commit 9ae129017c7fc1fa638d6beedd3802b515ca692b 'Fix
> og9 "Fix hang when running oacc exec with CUDA 9.0 nvprof"'.

..., and now the attached patch to devel/omp/gcc-9 in commit
775f1686a3df68bd20370f1fabc6273883e2c5d2 'Really fix og9 "Fix hang when
running oacc exec with CUDA 9.0 nvprof"'.


Grüße
 Thomas


-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
>From 775f1686a3df68bd20370f1fabc6273883e2c5d2 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Thu, 26 Mar 2020 17:34:01 +0100
Subject: [PATCH] Really fix og9 "Fix hang when running oacc exec with CUDA 9.0
 nvprof"

In my yesterday's commit 9ae129017c7fc1fa638d6beedd3802b515ca692b 'Fix og9 "Fix
hang when running oacc exec with CUDA 9.0 nvprof"', I wrongly blamed the og9
"Fix hang when running oacc exec with CUDA 9.0 nvprof" to be responsible for
the hang being back, when in fact it's only the later og9 "OpenACC Profiling
Interface (incomplete)" backport from trunk that introduced the problem.

The confusing thing was that the og9 "Fix hang when running oacc exec with CUDA
9.0 nvprof" commit appears *before* the og9 "OpenACC Profiling Interface
(incomplete)" backport that it relates to.

And, in addition to that, I pushed the wrong (incomplete) version of my fix.

	libgomp/
	* oacc-init.c (acc_init_1): Move other 'acc_init_state' logic to
	where it belongs.
---
 libgomp/ChangeLog.omp |  5 +
 libgomp/oacc-init.c   | 12 
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp
index 75c45917998..922e00fbff5 100644
--- a/libgomp/ChangeLog.omp
+++ b/libgomp/ChangeLog.omp
@@ -1,3 +1,8 @@
+2020-03-26  Thomas Schwinge 
+
+	* oacc-init.c (acc_init_1): Move other 'acc_init_state' logic to
+	where it belongs.
+
 2020-03-25  Thomas Schwinge 
 
 	* oacc-init.c (acc_init_1): Move 'acc_init_state' logic to where
diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c
index 765fa2f3b95..40c14fa9bf2 100644
--- a/libgomp/oacc-init.c
+++ b/libgomp/oacc-init.c
@@ -317,10 +317,6 @@ acc_init_1 (acc_device_t d, acc_construct_t parent_construct, int implicit)
   gomp_init_device (acc_dev);
   gomp_mutex_unlock (_dev->lock);
 
-  gomp_mutex_lock (_init_state_lock);
-  acc_init_state = initialized;
-  gomp_mutex_unlock (_init_state_lock);
-
   if (profiling_p)
 {
   prof_info.event_type = acc_ev_device_init_end;
@@ -329,6 +325,14 @@ acc_init_1 (acc_device_t d, acc_construct_t parent_construct, int implicit)
 _info);
 }
 
+  /* We're setting 'initialized' *after* 'goacc_profiling_dispatch', so 

Re: [RFC] Fix for pr64706 testcase faulre

[Michael Matz]

Hello,

On Thu, 26 Mar 2020, Richard Biener wrote:

> > = b.c:
> > 
> > __attribute__((weak))
> > __attribute__((noinline))
> > 
> > int a()
> > {
> >   return 1;
> > }
> > 
> > __attribute__((noinline))
> > static int b() __attribute__((alias("a")));
> > void
> > test()
> > {
> >   if (b()!=1)
> > __builtin_abort ();
> > }
> > 
> > = b.c:
> > 
> > __attribute__((noinline))
> > int a();
> > 
> > int a()
> > {
> >   return 2;
> > }
> > extern void test ();
> > int
> > main()
> > {
> >   if (a() != 2)
> > __builtin_abort ();
> >   test();
> >   return 0;
> > }
> > 
> > Here LTO linking will replace weak symbol a() by definition from b.c and
> > rediect static alias b in a.c to this definition which is wrong.
> 
> Is it?  What does a non-weak local alias to a weak function mean?

I think we should continue to try to model ELF semantics re weak and 
aliases.  If so, then yes, LTO gets it wrong and the above testcase should 
not abort.  Weak doesn't enter the picture for creating aliases (the 
aliases is created with the declaration, and we require an available 
definition, and the alias is henceforth bound to _that_ very definition).  
I.e. the 'a.c:b' name should continue to refer to the same code snippet 
identified by the a.c:a name, not be redirected to the overriding b.c:a.

I'm wondering about the amount of code necessary to fix this.  I think 
that points towards a wrong level of representation somewhere, and perhaps 
_that_ should be changed instead of trying to work around it.

For instance, right now aliases are different from non-aliases, whereas in 
reality there's no difference: there are simply names referring to code or 
data snippets, and it so happens that for some snippets there are multiple 
names, and it further just so happens that some names for a code snippet 
become overridden/removed by other names for other code snippets, which 
doesn't invalidate the fact that there still is another name for the first 
snippet.

If it were modelled like that in cgraph/lto all the rest would naturally 
follow.  (Of course you would need tracking of when some code/data 
snippets can't be referred to by name anymore (and hence are useless), 
because all names for them went away, or none of the existing names is 
used anymore, but I don't think that materially would change much in our 
internal data structures).


Ciao,
Michael.

Re: [PATCH] rs6000: Don't split constant oprator when add, move to temp register for future optimization

[will schmidt via Gcc-patches]

On Thu, 2020-03-26 at 05:06 -0500, luoxhu--- via Gcc-patches wrote:
> From: Xionghu Luo 
> 
> Remove split code from add3 to allow a later pass to split.
> This allows later logic to hoist out constant load in add
> instructions.
> In loop, lis+ori could be hoisted out to improve performance compared
> with
> previous addis+addi (About 15% on typical case), weak point is
> one more register is used and one more instruction is
> generated.  i.e.:
> 
> addis 3,3,0x8765
> addi 3,3,0x4321
> 
> =>
> 
> lis 9,0x8765
> ori 9,9,0x4321
> add 3,3,9

LGTM.  :-)
I defer to Segher for his review & approval, etc.

Thanks,
-Will

> 
> gcc/ChangeLog:
> 
> 2020-03-26  Xiong Hu Luo  
> 
>   * config/rs6000/rs6000.md (add3): Remove split code, move constant
> to temp register before add.
> 
> gcc/testsuite/ChangeLog:
> 
> 2020-03-26  Xiong Hu Luo  
> 
>   * gcc.target/powerpc/add-const.c: New.
> ---
>  gcc/config/rs6000/rs6000.md  | 25 ++--
> 
>  gcc/testsuite/gcc.target/powerpc/add-const.c | 18 ++
>  2 files changed, 20 insertions(+), 23 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/add-const.c
> 
> diff --git a/gcc/config/rs6000/rs6000.md
> b/gcc/config/rs6000/rs6000.md
> index ad88b6783af..72f3f604e0d 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -1733,29 +1733,8 @@ (define_expand "add3"
> || rtx_equal_p (operands[0], operands[1]))
>? operands[0] : gen_reg_rtx (mode));
> 
> -  /* Adding a constant to r0 is not a valid insn, so use a
> different
> -  strategy in that case.  */
> -  if (reg_or_subregno (operands[1]) == 0 || reg_or_subregno
> (tmp) == 0)
> - {
> -   if (operands[0] == operands[1])
> - FAIL;
> -   rs6000_emit_move (operands[0], operands[2], mode);
> -   emit_insn (gen_add3 (operands[0], operands[1],
> operands[0]));
> -   DONE;
> - }
> -
> -  HOST_WIDE_INT val = INTVAL (operands[2]);
> -  HOST_WIDE_INT low = ((val & 0x) ^ 0x8000) - 0x8000;
> -  HOST_WIDE_INT rest = trunc_int_for_mode (val - low,
> mode);
> -
> -  if (mode == DImode && !satisfies_constraint_L (GEN_INT
> (rest)))
> - FAIL;
> -
> -  /* The ordering here is important for the prolog expander.
> -  When space is allocated from the stack, adding 'low' first may
> -  produce a temporary deallocation (which would be bad).  */
> -  emit_insn (gen_add3 (tmp, operands[1], GEN_INT (rest)));
> -  emit_insn (gen_add3 (operands[0], tmp, GEN_INT (low)));
> +  rs6000_emit_move (tmp, operands[2], mode);
> +  emit_insn (gen_add3 (operands[0], operands[1], tmp));
>DONE;
>  }
>  })
> diff --git a/gcc/testsuite/gcc.target/powerpc/add-const.c
> b/gcc/testsuite/gcc.target/powerpc/add-const.c
> new file mode 100644
> index 000..e1007247b32
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/add-const.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile { target { lp64 } } } */
> +/* { dg-options "-O3 -fno-unroll-loops" } */
> +
> +/* Ensure the lis,ori are generated, which indicates they have
> +   been hoisted outside of the loop.  */
> +
> +typedef unsigned long ulong;
> +ulong
> +foo (ulong n, ulong h)
> +{
> +  int i;
> +  for (i = 0; i < n; i++)
> +h = ((h + 8) | h) + 0x87654321;
> +  return h;
> +}
> +
> +/* { dg-final { scan-assembler-times {\mlis\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mori\M} 1 } } */

Re: [PATCH] rs6000: Save/restore r31 if frame_pointer_needed is true

[will schmidt via Gcc-patches]

On Wed, 2020-03-25 at 23:15 -0500, luoxhu--- via Gcc-patches wrote:
> From: Xionghu Luo 
> 

Hi,
No real issues noted in my review. Patch is straighforward, just a
couple cosmetic comments inline below.


> This P1 bug is exposed by FRE refactor of r263875.  Comparing the fre
> dump file shows no obvious change of the segment fault function
> proves
> it to be a target issue.
> frame_pointer_needed is set to true in reload pass
> setup_can_eliminate,
> but regs_ever_live[31] is false, so pro_and_epilogue doesn't
> save/restore
> r31 even it is used actually, causing CPU2006 465.tonto segment fault
> of
> loading from invalid addresses.

Could also use a statement here that also reflects what the patch does.

"Thus, mark the register as in-use if frame_pointer_needed is true and
reg is HARD_FRAME_POINTER_REGNUM."


> Bootstrap and regression tested pass on Power8-LE.  Backport to gcc-9
> required once approved.
> 
> gcc/ChangeLog
> 
> 2020-03-26  Xiong Hu Luo  
> 
>   PR target/91518
>   * config/rs6000/rs6000-logue.c (save_reg_p): Save r31 if
>   frame_pointer_needed.


We don't actually reference 'r31' in the code change.  Maybe preferred
to refer to it as HARD_FRAME_POINTER_REGNUM instead?


> ---
>  gcc/config/rs6000/rs6000-logue.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/gcc/config/rs6000/rs6000-logue.c
> b/gcc/config/rs6000/rs6000-logue.c
> index 4cbf228eb79..7b62ff03afd 100644
> --- a/gcc/config/rs6000/rs6000-logue.c
> +++ b/gcc/config/rs6000/rs6000-logue.c
> @@ -116,6 +116,9 @@ save_reg_p (int reg)
>   return true;
>  }
> 
> +  if (reg == HARD_FRAME_POINTER_REGNUM && frame_pointer_needed)
> +return true;
> +

Ok.  

>return !call_used_or_fixed_reg_p (reg) && df_regs_ever_live_p
> (reg);
>  }

Thanks
-Will

Re: C++ 'NON_LVALUE_EXPR' in OMP array section handling

[Sandra Loosemore]

On 3/26/20 8:27 AM, Thomas Schwinge wrote:

Hi!

Note that as this code is shared between OpenACC/OpenMP, this might
affect OpenMP, too, as far as I can tell.  (Subject updated.)  Jakub, can
you please have a look, too?

On 2020-03-25T23:02:38-0600, Sandra Loosemore  wrote:

The attached patch fixes a bug I found in the C++ front end's handling
of OpenACC data clauses.  The problem here is that the C++ front end
wraps the array bounds expressions in NON_LVALUE_EXPR tree nodes, and
the code here (which appears to have been copied from similar code in
the C front end) was failing to strip those before checking to see if
they were INTEGER_CST nodes, etc.


So, I had a quick look.  I'm confirming the 'NON_LVALUE_EXPR' (C++ only,
not seen for C) difference, and that 'STRIP_NOPS' gets rid of these.
However, I also in some code paths see, for example, 'integer_nonzerop'
calls, which internally do 'STRIP_ANY_LOCATION_WRAPPER'.

I don't know if 'STRIP_NOPS' is the correct "hammer" to use here, I don't
know what the usual convention is: explicitly remove (via 'STRIP_NOPS' as
you suggested, or something else), or have the enquiry functions do it
('STRIP_ANY_LOCATION_WRAPPER' as 'integer_nonzerop' is doing, for
example).

Empirical data doesn't mean too much here, of course, I'm not seeing a
lot of explicit 'STRIP_*' calls in 'gcc/cp/semantics.c'.  ;-)


I saw that STRIP_NOPS seem to be the preferred way to do things in e.g. 
fold-const.c.  I don't know if there's a reason to use some less general 
form of STRIP_* here?



Sadly, I have no test case for this because it was only triggering an
error in conjunction with some other OpenACC patches that are not yet on
trunk


So maybe the problem is actually that these "other OpenACC patches" are
not sufficiently sanitizing the input data they're doing checks on?


No.  In the current code on trunk, both places that are being patched 
continue with checks like


TREE_CODE (low_bound) == INTEGER_CST

etc.  So when they're wrapped in NON_LVALUE_EXPRs, it's basically 
failing to detect constants in array dimension specifiers and falling 
through to other code.  (And it's patches to the "other code" that were 
diagnosing the bogus error I saw.)


-Sandra

Re: [RFC] Fix for pr64706 testcase faulre

[Richard Biener]

On Thu, 26 Mar 2020, Jan Hubicka wrote:

> Hi,
> this patch started as an attempt to fix pr64706 that kept me walking in loops
> for some time (I failed to make a hack that would make the testcase work in 
> all
> basic settings of -O0/2 -flto set on the both files independently. All GCC
> releases crashes on some).
> 
> The testcase has ODR violation that breaks comdat groups and kind of works 
> with
> non-lto by accident, but it triggers longer problem that we do not handle
> aliases correctly.
> 
> In ELF an alias is just additional symbol at a given position in code segment,
> while for GCC we do not have code segment. Function bodies are assigned to
> delcarations and aliases are linked to those declarations. This makes 
> difference
> in the following testcase:
> 
> = b.c:
> 
> __attribute__((weak))
> __attribute__((noinline))
> 
> int a()
> {
>   return 1;
> }
> 
> __attribute__((noinline))
> static int b() __attribute__((alias("a")));
> void
> test()
> {
>   if (b()!=1)
> __builtin_abort ();
> }
> 
> = b.c:
> 
> __attribute__((noinline))
> int a();
> 
> int a()
> {
>   return 2;
> }
> extern void test ();
> int
> main()
> {
>   if (a() != 2)
> __builtin_abort ();
>   test();
>   return 0;
> }
> 
> Here LTO linking will replace weak symbol a() by definition from b.c and
> rediect static alias b in a.c to this definition which is wrong.

Is it?  What does a non-weak local alias to a weak function mean?

> This patch works towards fixing it by making lto-symtab to notice that there 
> is
> previaling alias (a.c:b in my testcase) of a prevailed symbol (a.c:a in my
> testcase that got previaled by b.c:a).  In this case the body of prevailed
> symbol (a.c:a) must be saved because it is still reachable by calling b.c:a.
> This is done by turning the prevailed symbol to a static symbol.
> 
> This has some fallout in streaming because we need to stream in summaries of
> such prevailed symbols which is not hard to do and is purpose of
> stream_in_summary_p.
> 
> Now however we have a problem if tree sharing decided to share delcaration of
> a.c:a and b.c:a (which it does in my testcase) because we have one decl
> and two bodies.  I use the code form profile merging to patch around but this
> code is not fully correct (it works in profile merging only because the
> body is discarded immediatly after).
> 
> The code creates new static decl, say a.static and moves the body of a.c to 
> it.
> This however breaks because when streaming in body of a.static we will prevail
> all references to a.c:a to b.c:a.  This is OK, for example when streaming in
> statement calling a(). It is wrong when streaming in _CONTEXT pointers.
> 
> So in turn my patch mostly works, but sometimes I get errors like
> /aux/hubicka/trunk-git/gcc/testsuite/gcc.dg/lto/alias-resolution-1_0.c:9:5: 
> error: label context is not the current function declaration
> 
> I do not see way around this with our current infrastructure.  One way would 
> be
> to read body of a.c:a and clone it to a.static. This is bit wasteful but it is
> also problem in case body of b.c:a is already read to memory.

Why don't we represent the alias at the cgraph level?  That is,
why do decls come into play at all here?  b prevails and it has a
reference to a.c:a so we need to keep (and emit) that.  The only issue
would be that we'd end up with two 'a's so we have to notice that
and fixup somehow.

Isn't this in the end a similar issue as with extern inline bodies
vs. the offline copy body?

> I would like to do this w/o need to stream in the body, because such situation
> happens quite commonly local aliases: Until we are able to prove that all
> referencs to a local aliase of a symbol are gone we need to assume that body
> needs to be preserved and those are potentially quite frequent.
> 
> So basically only solution I think of is to make lto-stream-in live with the
> fact that function decl was changed.  So we can stream original decl to the
> beggining of the function section and during stream in do rewritting of
> DECL_CONTEXT similar way as tree-inline would do.  We need to be careful about
> keeping debug info in shape too (we are not producing inline copy, we are
> merely unsharing the main decl).
> 
> What do you think?

I think the not prevailed but still needed cgraph node for 'a' should
have its decl cleared ;)

> Note that I was considering case of simply not sharing decls with
> aliases at stream time.  This does not work because by localizing the
> decl one will not redirect references to the symbol correctly.

But all remaining references should be via 'b', not via 'a'.

> Other option would be to always produce a static symbol for every
> symbol with an alias.  This again can be IMO only done by cloning it and
> would be relatively expensive because at the stream out time we do have
> considerable quantity of aliases especially with -fpic.
> 
> Honza
> 
> gcc/ChangeLog:
> 
> 2020-03-26  Jan Hubicka  
> 
>   * cgraph.c 

blinds and shades

[max xie]

Hi My Friend,

Hope you everything is well. 

Now the Coronavirus has spread very seriously, please take care,
If you need other support please let us know, thanks .
Our factory still working, If you need any blinds and shades  we will send the 
goods ASAP.

 With best regards,
Max Xie 
Chief Sales Officer | CSO 
China: 86-15706899111 
Stock code: 870727 
Your China best OEM supplier 
max...@sunfreeblinds.com 
www.sunfreeblinds.com 
Ningbo Liyang New Material Company Limited 
No 418 Qihang North Road, YinZhou Technical Economic Development 
Area,Ningbo,Zhejiang,China 315145

C++ 'NON_LVALUE_EXPR' in OMP array section handling (was: [PATCH, OpenACC] Bug fix for processing OpenACC data clauses in C++)

[Thomas Schwinge]

Hi!

Note that as this code is shared between OpenACC/OpenMP, this might
affect OpenMP, too, as far as I can tell.  (Subject updated.)  Jakub, can
you please have a look, too?

On 2020-03-25T23:02:38-0600, Sandra Loosemore  wrote:
> The attached patch fixes a bug I found in the C++ front end's handling
> of OpenACC data clauses.  The problem here is that the C++ front end
> wraps the array bounds expressions in NON_LVALUE_EXPR tree nodes, and
> the code here (which appears to have been copied from similar code in
> the C front end) was failing to strip those before checking to see if
> they were INTEGER_CST nodes, etc.

So, I had a quick look.  I'm confirming the 'NON_LVALUE_EXPR' (C++ only,
not seen for C) difference, and that 'STRIP_NOPS' gets rid of these.
However, I also in some code paths see, for example, 'integer_nonzerop'
calls, which internally do 'STRIP_ANY_LOCATION_WRAPPER'.

I don't know if 'STRIP_NOPS' is the correct "hammer" to use here, I don't
know what the usual convention is: explicitly remove (via 'STRIP_NOPS' as
you suggested, or something else), or have the enquiry functions do it
('STRIP_ANY_LOCATION_WRAPPER' as 'integer_nonzerop' is doing, for
example).

Empirical data doesn't mean too much here, of course, I'm not seeing a
lot of explicit 'STRIP_*' calls in 'gcc/cp/semantics.c'.  ;-)

> Sadly, I have no test case for this because it was only triggering an
> error in conjunction with some other OpenACC patches that are not yet on
> trunk

So maybe the problem is actually that these "other OpenACC patches" are
not sufficiently sanitizing the input data they're doing checks on?

> and the tree dumps don't show anything useful.

Well, the 'original' tree dumps do show (C++) vs. don't show (C) the
'NON_LVALUE_EXPR's.

> I confirmed that
> the problem exists on trunk without those other patches by examining
> things in the debugger.  This is the example I was using for my
> hand-testing:
>
> void f (float a[16][16], float b[16][16], float c[16][16])
> {
>int i, j, k;
>
> #pragma acc kernels copyin(a[0:16][0:16], b[0:16][0:16]) 
> copyout(c[0:16][0:16])
>{
>  for (i = 0; i < 16; i++) {
>for (j = 0; j < 16; j++) {
>  float t = 0;
>  for (k = 0; k < 16; k++)
>t += a[i][k] * b[k][j];
>  c[i][j] = t;
>}
>  }
>}
>
> }
>
> Is this patch OK to commit now, or in Stage 1?

First we need to figure out if this is an actual/current bug (which then
needs GCC PR filed, and later also backports to release branches), or
not.


Grüße
 Thomas


> commit 3d2cda1e758a54111af04e80ea3dbaa27b3387e7
> Author: Sandra Loosemore 
> Date:   Wed Mar 25 21:29:17 2020 -0700
>
> Bug fix for processing OpenACC data clauses in C++.
>
> The C++ front end wraps the values in OpenACC data clause array range
> specifications in NON_LVALUE_EXPR tree nodes.  The existing code was
> failing to strip these before checking for constant values.
>
> 2020-03-25  Sandra Loosemore 
>
>   gcc/cp/
>   * semantics.c (handle_omp_array_sections_1): Call STRIP_NOPS
>   on length and low_bound to unwrap NON_LVALUE_EXPRs.
>   (handle_omp_array_sections): Likewise.
>
> diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
> index 34ccb9f..8945939 100644
> --- a/gcc/cp/ChangeLog
> +++ b/gcc/cp/ChangeLog
> @@ -1,3 +1,9 @@
> +2020-03-25  Sandra Loosemore 
> +
> + * semantics.c (handle_omp_array_sections_1): Call STRIP_NOPS
> + on length and low_bound to unwrap NON_LVALUE_EXPRs.
> + (handle_omp_array_sections): Likewise.
> +
>  2020-03-25  Patrick Palka  
>
>   PR c++/94265
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index 2721a55..2efc077 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -4861,6 +4861,10 @@ handle_omp_array_sections_1 (tree c, tree t, vec 
> ,
>  length = mark_rvalue_use (length);
>/* We need to reduce to real constant-values for checks below.  */
>if (length)
> +STRIP_NOPS (length);
> +  if (low_bound)
> +STRIP_NOPS (low_bound);
> +  if (length)
>  length = fold_simple (length);
>if (low_bound)
>  low_bound = fold_simple (low_bound);
> @@ -5160,6 +5164,11 @@ handle_omp_array_sections (tree c, enum 
> c_omp_region_type ort)
> tree low_bound = TREE_PURPOSE (t);
> tree length = TREE_VALUE (t);
>
> +   if (length)
> + STRIP_NOPS (length);
> +   if (low_bound)
> + STRIP_NOPS (low_bound);
> +
> i--;
> if (low_bound
> && TREE_CODE (low_bound) == INTEGER_CST
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter

[RFC] Fix for pr64706 testcase faulre

[Jan Hubicka]

Hi,
this patch started as an attempt to fix pr64706 that kept me walking in loops
for some time (I failed to make a hack that would make the testcase work in all
basic settings of -O0/2 -flto set on the both files independently. All GCC
releases crashes on some).

The testcase has ODR violation that breaks comdat groups and kind of works with
non-lto by accident, but it triggers longer problem that we do not handle
aliases correctly.

In ELF an alias is just additional symbol at a given position in code segment,
while for GCC we do not have code segment. Function bodies are assigned to
delcarations and aliases are linked to those declarations. This makes difference
in the following testcase:

= b.c:

__attribute__((weak))
__attribute__((noinline))

int a()
{
  return 1;
}

__attribute__((noinline))
static int b() __attribute__((alias("a")));
void
test()
{
  if (b()!=1)
__builtin_abort ();
}

= b.c:

__attribute__((noinline))
int a();

int a()
{
  return 2;
}
extern void test ();
int
main()
{
  if (a() != 2)
__builtin_abort ();
  test();
  return 0;
}

Here LTO linking will replace weak symbol a() by definition from b.c and
rediect static alias b in a.c to this definition which is wrong.

This patch works towards fixing it by making lto-symtab to notice that there is
previaling alias (a.c:b in my testcase) of a prevailed symbol (a.c:a in my
testcase that got previaled by b.c:a).  In this case the body of prevailed
symbol (a.c:a) must be saved because it is still reachable by calling b.c:a.
This is done by turning the prevailed symbol to a static symbol.

This has some fallout in streaming because we need to stream in summaries of
such prevailed symbols which is not hard to do and is purpose of
stream_in_summary_p.

Now however we have a problem if tree sharing decided to share delcaration of
a.c:a and b.c:a (which it does in my testcase) because we have one decl
and two bodies.  I use the code form profile merging to patch around but this
code is not fully correct (it works in profile merging only because the
body is discarded immediatly after).

The code creates new static decl, say a.static and moves the body of a.c to it.
This however breaks because when streaming in body of a.static we will prevail
all references to a.c:a to b.c:a.  This is OK, for example when streaming in
statement calling a(). It is wrong when streaming in _CONTEXT pointers.

So in turn my patch mostly works, but sometimes I get errors like
/aux/hubicka/trunk-git/gcc/testsuite/gcc.dg/lto/alias-resolution-1_0.c:9:5: 
error: label context is not the current function declaration

I do not see way around this with our current infrastructure.  One way would be
to read body of a.c:a and clone it to a.static. This is bit wasteful but it is
also problem in case body of b.c:a is already read to memory.

I would like to do this w/o need to stream in the body, because such situation
happens quite commonly local aliases: Until we are able to prove that all
referencs to a local aliase of a symbol are gone we need to assume that body
needs to be preserved and those are potentially quite frequent.

So basically only solution I think of is to make lto-stream-in live with the
fact that function decl was changed.  So we can stream original decl to the
beggining of the function section and during stream in do rewritting of
DECL_CONTEXT similar way as tree-inline would do.  We need to be careful about
keeping debug info in shape too (we are not producing inline copy, we are
merely unsharing the main decl).

What do you think?

Note that I was considering case of simply not sharing decls with
aliases at stream time.  This does not work because by localizing the
decl one will not redirect references to the symbol correctly.

Other option would be to always produce a static symbol for every
symbol with an alias.  This again can be IMO only done by cloning it and
would be relatively expensive because at the stream out time we do have
considerable quantity of aliases especially with -fpic.

Honza

gcc/ChangeLog:

2020-03-26  Jan Hubicka  

* cgraph.c (cgraph_node::make_local): Add force parameter.
* cgraph.h (symtab_node::clone_referring_skip_aliases): New
member function.
(symtab_node::stream_in_summary_p): Likewise.
* ipa-fnsummary.c (inline_read_section): Use
symtab_node::stream_in_summary_p.
* ipa-prop.c (ipa_read_node_info): Likewise.
* symtab.c (symtab_node::clone_referring): Fix formating.
(symtab_node::clone_referring_skip_aliases): New member function.
(symtab_node::dump_referring): Fix formating
(symtab_node::stream_in_summary_p): New member function.

gcc/lto/ChangeLog:

2020-03-26  Jan Hubicka  

* lto-symtab.c (lto_symtab_symbol_p): New function.
(has_prevailing_alias_p): New function.
(lto_symtab_replace_node): Break out from ...; support alias
interposition
(lto_cgraph_replace_node): ... here.

[committed] wwwdocs: Fix the link to the 2020-03-12 update on GCC 9.

[Gerald Pfeifer]

Not sure where the original URL came from, but it does not work, so 
I looked this up in the mailing list archive and updated it.

Pushed.

Gerald
---
 htdocs/index.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/htdocs/index.html b/htdocs/index.html
index edeeb817..d14d4325 100644
--- a/htdocs/index.html
+++ b/htdocs/index.html
@@ -131,7 +131,7 @@ More news? Let ger...@pfeifer.com know!
 
   Status:
   
-  https://gcc.gnu.org/pipermail/gcc/2020-March/231811.html;>2020-03-12
+  https://gcc.gnu.org/pipermail/gcc/2020-March/38.html;>2020-03-12
   
   (regression fixes  docs only).
   
-- 
2.25.1

[committed] libstdc++: Add some C++20 additions to

[Jonathan Wakely via Gcc-patches]

* include/std/chrono (chrono::days, chrono::weeks, chrono::years)
(chrono::months, chrono::sys_days, chrono::local_t)
(chrono::local_time, chrono::local_seconds, chrono::local_days):
Define for C++20.
(chrono::time_point): Add missing static assert.
* testsuite/20_util/time_point/requirements/duration_neg.cc: New test.
* testsuite/std/time/clock/file/overview.cc: New test.
* testsuite/std/time/clock/file/members.cc: New test.
* testsuite/std/time/syn_c++20.cc: New test.

Tested powerpc64le-linux, committed to master.

commit 16948c54b7576fb4b27c59915eac71a0c6bf94f6
Author: Jonathan Wakely 
Date:   Thu Mar 26 14:00:12 2020 +

libstdc++: Add some C++20 additions to 

* include/std/chrono (chrono::days, chrono::weeks, chrono::years)
(chrono::months, chrono::sys_days, chrono::local_t)
(chrono::local_time, chrono::local_seconds, chrono::local_days):
Define for C++20.
(chrono::time_point): Add missing static assert.
* testsuite/20_util/time_point/requirements/duration_neg.cc: New 
test.
* testsuite/std/time/clock/file/overview.cc: New test.
* testsuite/std/time/clock/file/members.cc: New test.
* testsuite/std/time/syn_c++20.cc: New test.

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index b1fa5b83295..514926c5c05 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -709,22 +709,36 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 
 /// nanoseconds
-typedef duration<_GLIBCXX_CHRONO_INT64_T, nano>nanoseconds;
+using nanoseconds  = duration<_GLIBCXX_CHRONO_INT64_T, nano>;
 
 /// microseconds
-typedef duration<_GLIBCXX_CHRONO_INT64_T, micro>   microseconds;
+using microseconds = duration<_GLIBCXX_CHRONO_INT64_T, micro>;
 
 /// milliseconds
-typedef duration<_GLIBCXX_CHRONO_INT64_T, milli>   milliseconds;
+using milliseconds = duration<_GLIBCXX_CHRONO_INT64_T, milli>;
 
 /// seconds
-typedef duration<_GLIBCXX_CHRONO_INT64_T>  seconds;
+using seconds  = duration<_GLIBCXX_CHRONO_INT64_T>;
 
 /// minutes
-typedef duration<_GLIBCXX_CHRONO_INT64_T, ratio< 60>>   minutes;
+using minutes  = duration<_GLIBCXX_CHRONO_INT64_T, ratio< 60>>;
 
 /// hours
-typedef duration<_GLIBCXX_CHRONO_INT64_T, ratio<3600>>  hours;
+using hours= duration<_GLIBCXX_CHRONO_INT64_T, 
ratio<3600>>;
+
+#if __cplusplus > 201703L
+/// days
+using days = duration<_GLIBCXX_CHRONO_INT64_T, ratio<86400>>;
+
+/// weeks
+using weeks= duration<_GLIBCXX_CHRONO_INT64_T, 
ratio<604800>>;
+
+/// years
+using years= duration<_GLIBCXX_CHRONO_INT64_T, 
ratio<31556952>>;
+
+/// months
+using months   = duration<_GLIBCXX_CHRONO_INT64_T, ratio<2629746>>;
+#endif // C++20
 
 #undef _GLIBCXX_CHRONO_INT64_T
 
@@ -732,9 +746,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 template
   struct time_point
   {
-   typedef _Clock  clock;
-   typedef _Durduration;
-   typedef typename duration::rep  rep;
+   static_assert(__is_duration<_Dur>::value,
+   "duration must be a specialization of std::chrono::duration");
+
+   typedef _Clock  clock;
+   typedef _Durduration;
+   typedef typename duration::rep  rep;
typedef typename duration::period   period;
 
constexpr time_point() : __d(duration::zero())
@@ -790,7 +807,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   time_point<_Clock, _ToDur>>::type
   time_point_cast(const time_point<_Clock, _Dur>& __t)
   {
-   typedef time_point<_Clock, _ToDur>  __time_point;
+   typedef time_point<_Clock, _ToDur>  __time_point;
return __time_point(duration_cast<_ToDur>(__t.time_since_epoch()));
   }
 
@@ -837,7 +854,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {
typedef duration<_Rep2, _Period2>   __dur2;
typedef typename common_type<_Dur1,__dur2>::type__ct;
-   typedef time_point<_Clock, __ct>__time_point;
+   typedef time_point<_Clock, __ct>__time_point;
return __time_point(__lhs.time_since_epoch() + __rhs);
   }
 
@@ -851,7 +868,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {
typedef duration<_Rep1, _Period1>   __dur1;
typedef typename common_type<__dur1,_Dur2>::type__ct;
-   typedef time_point<_Clock, __ct>__time_point;
+   typedef 

Re: [stage1][PATCH] Provide warning for missing jobserver.

[Martin Liška]

On 3/26/20 1:42 PM, Jan Hubicka wrote:

Hi.

I'm suggesting to provide a warning when one uses -flto=jobserver
but we can't detect job server for some reason.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed in next stage1?
Thanks,
Martin

gcc/ChangeLog:

2020-03-26  Martin Liska  

PR driver/94330
* lto-wrapper.c (run_gcc): When using -flto=jobserver,
report warning when the jobserver is not detected.

This looks like a good idea to me (though I guess Richi needs to approve
it).  I would make message more informative so it is clear that
jobserver is supposed to be provided by GNU make and that we resort
to running in one thread.


Feel free to provide a better wording.



Morivation is that prople won't get confused trying to start GCC from
other kind of build systems and also when you cut the command out
the warning should be explicit enough to make you notice that you can
wait for very long time ;)


Yes, it will be handy for these situations. If possible, I would like to see
it landing in GCC 10. But it's Richi's turn.

Martin



Honza

---
  gcc/lto-wrapper.c | 12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)





diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 46a88b233f6..6263c164888 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1473,10 +1473,16 @@ run_gcc (unsigned argc, char *argv[])
auto_parallel = 0;
parallel = 0;
  }
-  else if (!jobserver && jobserver_active_p ())
+  else
  {
-  parallel = 1;
-  jobserver = 1;
+  bool active_jobserver = jobserver_active_p ();
+  if (jobserver && !active_jobserver)
+   warning (0, "jobserver is not available.");
+  else if (!jobserver && active_jobserver)
+   {
+ parallel = 1;
+ jobserver = 1;
+   }
  }
  
if (linker_output)

Re: [stage1][PATCH] Provide warning for missing jobserver.

[Jan Hubicka]

> Hi.
> 
> I'm suggesting to provide a warning when one uses -flto=jobserver
> but we can't detect job server for some reason.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed in next stage1?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2020-03-26  Martin Liska  
> 
>   PR driver/94330
>   * lto-wrapper.c (run_gcc): When using -flto=jobserver,
>   report warning when the jobserver is not detected.
This looks like a good idea to me (though I guess Richi needs to approve
it).  I would make message more informative so it is clear that
jobserver is supposed to be provided by GNU make and that we resort
to running in one thread.

Morivation is that prople won't get confused trying to start GCC from
other kind of build systems and also when you cut the command out
the warning should be explicit enough to make you notice that you can
wait for very long time ;)

Honza
> ---
>  gcc/lto-wrapper.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> 

> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> index 46a88b233f6..6263c164888 100644
> --- a/gcc/lto-wrapper.c
> +++ b/gcc/lto-wrapper.c
> @@ -1473,10 +1473,16 @@ run_gcc (unsigned argc, char *argv[])
>auto_parallel = 0;
>parallel = 0;
>  }
> -  else if (!jobserver && jobserver_active_p ())
> +  else
>  {
> -  parallel = 1;
> -  jobserver = 1;
> +  bool active_jobserver = jobserver_active_p ();
> +  if (jobserver && !active_jobserver)
> + warning (0, "jobserver is not available.");
> +  else if (!jobserver && active_jobserver)
> + {
> +   parallel = 1;
> +   jobserver = 1;
> + }
>  }
>  
>if (linker_output)
> 

Re: [PATCH v3] Fix PR90332 by extending half size vector mode

[Richard Biener via Gcc-patches]

On Thu, Mar 26, 2020 at 12:01 PM Kewen.Lin  wrote:
>
> Hi Richi,
>
> on 2020/3/25 下午4:25, Richard Biener wrote:
> > On Tue, Mar 24, 2020 at 9:30 AM Kewen.Lin  wrote:
> >>
> >> Hi,
> >>
> >> The new version with refactoring has been attached.
> >> Bootstrapped/regtested on powerpc64le-linux-gnu (LE) P8 and P9.
> >>
> >> Is it ok for trunk?
> >
> > Yes.
> >
>
> Thanks!  I'm sorry that I forgot to update the nelts with new element number
> for smaller vector for the path constructing with smaller vectors.
>
> The difference against the previous one is:
>
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -2251,12 +2251,13 @@ vector_vector_composition_type (tree vtype, 
> poly_uint64 nelts, tree *ptype)
>/* First check if vec_init optab supports construction from
>  vector pieces directly.  */
>scalar_mode elmode = SCALAR_TYPE_MODE (TREE_TYPE (vtype));
> +  poly_uint64 inelts = pbsize / GET_MODE_BITSIZE (elmode);
>machine_mode rmode;
> -  if (related_vector_mode (vmode, elmode, nelts).exists ()
> +  if (related_vector_mode (vmode, elmode, inelts).exists ()
>   && (convert_optab_handler (vec_init_optab, vmode, rmode)
>   != CODE_FOR_nothing))
> {
> - *ptype = build_vector_type (TREE_TYPE (vtype), nelts);
> + *ptype = build_vector_type (TREE_TYPE (vtype), inelts);
>   return vtype;
> }
>
> This new version has been bootstrapped/regtested on
> powerpc64le-linux-gnu (LE) P8 and x86_64-redhat-linux.
>
> May I install this new instead?

Yes.

Richard.

> BR,
> Kewen
> -
> gcc/ChangeLog
>
> 2020-MM-DD  Kewen Lin  
>
> PR tree-optimization/90332
> * gcc/tree-vect-stmts.c (vector_vector_composition_type): New 
> function.
> (get_group_load_store_type): Adjust to call 
> vector_vector_composition_type,
> extend it to construct with scalar types.
> (vectorizable_load): Likewise.

Re: [PATCH v3] coroutines: Implement n4849 recommended symmetric transfer.

[Nathan Sidwell]

On 3/25/20 5:40 PM, Iain Sandoe wrote:

Nathan Sidwell  wrote:


On 3/24/20 2:08 PM, Iain Sandoe wrote:


tree suspend = TREE_VEC_ELT (awaiter_calls, 1); /* await_suspend().  */
+  tree susp_type;
+  if (tree fndecl = cp_get_callee_fndecl_nofold (suspend))
+susp_type = TREE_TYPE (TREE_TYPE (fndecl));
+  else
+susp_type = TREE_TYPE (suspend);


Why, when there's a call of a named function, is it that TREE_TYPE (suspend) is 
insufficient? You mentioned TARGET_EXPR, but why is their type differenty?


it isn’t - the code imported from an earlier impl. was jumping through hoops 
not needed here - changed to:
"susp_type = TREE_TYPE (suspend);”


good, that makes sense, thanks!

Patch is good to commit now, thanks

nathan
--
Nathan Sidwell

Re: [PATCH v2] coroutines: Implement n4849 changes to exception handling.

[Nathan Sidwell]

On 3/25/20 3:46 PM, Iain Sandoe wrote:

Hi,

Iain Sandoe  wrote:


This is the first of two remaining changes needed to bring the GCC
implementation into line with the standard draft at n4849.


Here is a revised version with the “initial await resume called” variable
renamed to be more consistent, as we discussed off-list.




+  /* n4849 adds specific behaviour to treat exceptions thrown by the
+ await_resume () of the initial suspend expression.  In order to
+ implement this, we need to treat the initial_suspend expression
+ as if it were part of the user's authored function body.  This


(channelling Sandra) user-authored, I think?
 only need to exercise the initial

Otherwise fine -- no cognative dissonance on variable name vs meaning :)

ok, thanks

nathan

--
Nathan Sidwell

[Arm] Implement CDE intrinsics for MVE registers.

[Matthew Malcomson]

[Arm] Implement CDE intrinsics for MVE registers.

Implement CDE intrinsics on MVE registers.

Other than the basics required for adding intrinsics this patch consists
of three changes.

** We separate out the MVE types and casts from the arm_mve.h header.

This is so that the types can be used in arm_cde.h without the need to include
the entire arm_mve.h header.
The only type that arm_cde.h needs is `uint8x16_t`, so this separation could be
avoided by using a `typedef` in this file.
Since the introduced intrinsics are all defined to act on the full range of MVE
types, declaring all such types seems intuitive since it will provide their
declaration to the user too.

This arm_mve_types.h header not only includes the MVE types, but also
the conversion intrinsics between them.
Some of the conversion intrinsics are needed for arm_cde.h, but most are
not.  We include all conversion intrinsics to keep the definition of
such conversion functions all in one place, on the understanding that
extra conversion functions being defined when including `arm_cde.h` is
not a problem.

** We define the TARGET_RESOLVE_OVERLOADED_BUILTIN hook for the Arm backend.

This is needed to implement the polymorphism for the required intrinsics.
The intrinsics have no specialised version, and the resulting assembly
instruction for all different types should be exactly the same.
Due to this we have implemented these intrinsics via one builtin on one type.
All other calls to the intrinsic with different types are implicitly cast to
the one type that is defined, and hence are all expanded to the same RTL
pattern that is only defined for one machine mode.

** We seperate the initialisation of the CDE intrinsics from others.

This allows us to ensure that the CDE intrinsics acting on MVE registers
are only created when both CDE and MVE are available.
Only initialising these builtins when both features are available is
especially important since they require a type that is only initialised
when the target supports hard float.  Hence trying to initialise these
builtins on a soft float target would cause an ICE.

Testing done:
  Full bootstrap and regtest on arm-none-linux-gnueabihf
  Regression test on arm-none-eabi

NOTE: These tests ICE on arm-none-eabi.
This is due to bugzilla bug 94341, and not due to a problem in this patch.

NOTE2: This patch depends on the CDE intrinsic framework patch by
Dennis.
https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542008.html


Ok for trunk?

gcc/ChangeLog:

2020-03-26  Matthew Malcomson  

* config.gcc (arm_mve_types.h): New extra_header for arm.
* config/arm/arm-builtins.c (arm_resolve_overloaded_builtin): New.
(arm_init_cde_builtins): New.
(arm_init_acle_builtins): Remove initialisation of CDE builtins.
(arm_init_builtins): Call arm_init_cde_builtins when target
supports CDE.
* config/arm/arm-c.c (arm_resolve_overloaded_builtin): New declaration.
(arm_register_target_pragmas): Initialise resolve_overloaded_builtin
hook to the implementation for the arm backend.
* config/arm/arm.h (ARM_MVE_CDE_CONST_1): New.
(ARM_MVE_CDE_CONST_2): New.
(ARM_MVE_CDE_CONST_3): New.
* config/arm/arm_cde.h (__arm_vcx1q_u8): New.
(__arm_vcx1qa): New.
(__arm_vcx2q): New.
(__arm_vcx2q_u8): New.
(__arm_vcx2qa): New.
(__arm_vcx3q): New.
(__arm_vcx3q_u8): New.
(__arm_vcx3qa): New.
* config/arm/arm_cde_builtins.def (vcx1q, vcx1qa, vcx2q, vcx2qa, vcx3q,
vcx3qa): New builtins defined.
* config/arm/arm_mve.h: Move typedefs and conversion intrinsics
to arm_mve_types.h header.
* config/arm/arm_mve_types.h: New file.
* config/arm/mve.md (arm_vcx1qv16qi, arm_vcx1qav16qi, arm_vcx2qv16qi,
arm_vcx2qav16qi, arm_vcx3qv16qi, arm_vcx3qav16qi): New patterns.
* config/arm/predicates.md (const_int_mve_cde1_operand,
const_int_mve_cde2_operand, const_int_mve_cde3_operand): New.

gcc/testsuite/ChangeLog:

2020-03-26  Matthew Malcomson  
Dennis Zhang  

* gcc.target/arm/acle/cde-mve-error-1.c: New test.
* gcc.target/arm/acle/cde-mve-error-2.c: New test.
* gcc.target/arm/acle/cde-mve-error-3.c: New test.
* gcc.target/arm/acle/cde-mve-full-assembly.c: New test.
* gcc.target/arm/acle/cde-mve-tests.c: New test.
* lib/target-supports.exp (arm_v8_1m_main_cde_mve_fp): New check
effective.


cde_mve_regs.patch.gz
Description: application/gzip

Re: [Patch, fortran] PR94246 - [9/10 Regression] valgrind error for ./gfortran.dg/bessel_5.f90 since r9-1566-g87c789f1c0b2df41

[Tobias Burnus]

Dear Paul,

OK – thanks for the patch.

Tobias

PS: I assume that the spacing issue in the patch
is due to the mail program.

On 3/26/20 12:20 PM, Paul Richard Thomas via Fortran wrote:


This turned out to be relatively trivial, following a fair amount of
head scratching:-(

Regtests on FC31/x64_86 - OK for both branches?

Paul

2020-03-26  Paul Thomas  

 PR fortran/94246
 * expr.c (scalarize_intrinsic_call): Remove the error checking.
 Make a copy of the expression to be simplified and only replace
 the original if the simplification succeeds.

2020-03-26  Paul Thomas  

 PR fortran/94246
 * gfortran.dg/bessel_5_redux.f90 : New test.

diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index 79e00b4112a..1106341df91 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -2296,9 +2296,8 @@ scalarize_intrinsic_call (gfc_expr *e, bool init_flag)
gfc_constructor_base ctor;
gfc_constructor *args[5] = {};  /* Avoid uninitialized warnings.  */
gfc_constructor *ci, *new_ctor;
-  gfc_expr *expr, *old;
+  gfc_expr *expr, *old, *p;
int n, i, rank[5], array_arg;
-  int errors = 0;

if (e == NULL)
  return false;
@@ -2366,8 +2365,6 @@ scalarize_intrinsic_call (gfc_expr *e, bool init_flag)
n++;
  }

-  gfc_get_errors (NULL, );
-
/* Using the array argument as the master, step through the array
   calling the function for each element and advancing the array
   constructors together.  */
@@ -2401,8 +2398,12 @@ scalarize_intrinsic_call (gfc_expr *e, bool init_flag)
/* Simplify the function calls.  If the simplification fails, the
   error will be flagged up down-stream or the library will deal
   with it.  */
-  if (errors == 0)
-gfc_simplify_expr (new_ctor->expr, 0);
+  p = gfc_copy_expr (new_ctor->expr);
+
+  if (!gfc_simplify_expr (p, init_flag))
+gfc_free_expr (p);
+  else
+gfc_replace_expr (new_ctor->expr, p);

for (i = 0; i < n; i++)
  if (args[i])

-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter

[Patch, fortran] PR94246 - [9/10 Regression] valgrind error for ./gfortran.dg/bessel_5.f90 since r9-1566-g87c789f1c0b2df41

[Paul Richard Thomas via Gcc-patches]

This turned out to be relatively trivial, following a fair amount of
head scratching:-(

Regtests on FC31/x64_86 - OK for both branches?

Paul

2020-03-26  Paul Thomas  

PR fortran/94246
* expr.c (scalarize_intrinsic_call): Remove the error checking.
Make a copy of the expression to be simplified and only replace
the original if the simplification succeeds.

2020-03-26  Paul Thomas  

PR fortran/94246
* gfortran.dg/bessel_5_redux.f90 : New test.

diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index 79e00b4112a..1106341df91 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -2296,9 +2296,8 @@ scalarize_intrinsic_call (gfc_expr *e, bool init_flag)
   gfc_constructor_base ctor;
   gfc_constructor *args[5] = {};  /* Avoid uninitialized warnings.  */
   gfc_constructor *ci, *new_ctor;
-  gfc_expr *expr, *old;
+  gfc_expr *expr, *old, *p;
   int n, i, rank[5], array_arg;
-  int errors = 0;

   if (e == NULL)
 return false;
@@ -2366,8 +2365,6 @@ scalarize_intrinsic_call (gfc_expr *e, bool init_flag)
   n++;
 }

-  gfc_get_errors (NULL, );
-
   /* Using the array argument as the master, step through the array
  calling the function for each element and advancing the array
  constructors together.  */
@@ -2401,8 +2398,12 @@ scalarize_intrinsic_call (gfc_expr *e, bool init_flag)
   /* Simplify the function calls.  If the simplification fails, the
  error will be flagged up down-stream or the library will deal
  with it.  */
-  if (errors == 0)
-gfc_simplify_expr (new_ctor->expr, 0);
+  p = gfc_copy_expr (new_ctor->expr);
+
+  if (!gfc_simplify_expr (p, init_flag))
+gfc_free_expr (p);
+  else
+gfc_replace_expr (new_ctor->expr, p);

   for (i = 0; i < n; i++)
 if (args[i])
! { dg-do compile }
! { dg-options "-Wall" }
!
! Check fix for PR94246 in which the errors in line 63 caused a segfault
! because the cleanup was not done correctly without the -fno-range-check option.
!
! This is a copy of bessel_5.f90 with the error messages added.
!
! -Wall has been specified to disabled -pedantic, which warns about the
! negative order (GNU extension) to the order of the Bessel functions of
! first and second kind.
!

implicit none
integer :: i


! Difference to mpfr_jn <= 1 epsilon

if (any (abs (BESSEL_JN(2, 5, 2.457) - [(BESSEL_JN(i, 2.457), i = 2, 5)]) &
  > epsilon(0.0))) then
  print *, 'FAIL 1'
  STOP 1
end if


! Difference to mpfr_yn <= 4 epsilon

if (any (abs (BESSEL_YN(2, 5, 2.457) - [(BESSEL_YN(i, 2.457), i = 2, 5)]) &
 > epsilon(0.0)*4)) then
  STOP 2
end if


! Difference to mpfr_jn <= 1 epsilon

if (any (abs (BESSEL_JN(0, 10, 4.457) &
  - [ (BESSEL_JN(i, 4.457), i = 0, 10) ]) &
 > epsilon(0.0))) then
  STOP 3
end if


! Difference to mpfr_yn <= 192 epsilon

if (any (abs (BESSEL_YN(0, 10, 4.457) &
  - [ (BESSEL_YN(i, 4.457), i = 0, 10) ]) &
 > epsilon(0.0)*192)) then
  STOP 4
end if


! Difference to mpfr_jn: None.  (Special case: X = 0.0)

if (any (BESSEL_JN(0, 10, 0.0) /= [ (BESSEL_JN(i, 0.0), i = 0, 10) ])) &
then
  STOP 5
end if


! Difference to mpfr_yn: None.  (Special case: X = 0.0)

if (any (BESSEL_YN(0, 10, 0.0) /= [ (BESSEL_YN(i, 0.0), i = 0, 10) ])) & ! { dg-error "overflows|-INF" }
then
  STOP 6
end if


! Difference to mpfr_jn <= 1 epsilon

if (any (abs (BESSEL_JN(0, 10, 1.0) &
  - [ (BESSEL_JN(i, 1.0), i = 0, 10) ]) &
 > epsilon(0.0)*1)) then
 STOP 7
end if

! Difference to mpfr_yn <= 32 epsilon

if (any (abs (BESSEL_YN(0, 10, 1.0) &
  - [ (BESSEL_YN(i, 1.0), i = 0, 10) ]) &
 > epsilon(0.0)*32)) then
  STOP 8
end if

end

[PATCH v3] Fix PR90332 by extending half size vector mode

[Kewen.Lin via Gcc-patches]

Hi Richi,

on 2020/3/25 下午4:25, Richard Biener wrote:
> On Tue, Mar 24, 2020 at 9:30 AM Kewen.Lin  wrote:
>>
>> Hi,
>>
>> The new version with refactoring has been attached.
>> Bootstrapped/regtested on powerpc64le-linux-gnu (LE) P8 and P9.
>>
>> Is it ok for trunk?
> 
> Yes.
> 

Thanks!  I'm sorry that I forgot to update the nelts with new element number
for smaller vector for the path constructing with smaller vectors.

The difference against the previous one is:

--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -2251,12 +2251,13 @@ vector_vector_composition_type (tree vtype, poly_uint64 
nelts, tree *ptype)
   /* First check if vec_init optab supports construction from
 vector pieces directly.  */
   scalar_mode elmode = SCALAR_TYPE_MODE (TREE_TYPE (vtype));
+  poly_uint64 inelts = pbsize / GET_MODE_BITSIZE (elmode);
   machine_mode rmode;
-  if (related_vector_mode (vmode, elmode, nelts).exists ()
+  if (related_vector_mode (vmode, elmode, inelts).exists ()
  && (convert_optab_handler (vec_init_optab, vmode, rmode)
  != CODE_FOR_nothing))
{
- *ptype = build_vector_type (TREE_TYPE (vtype), nelts);
+ *ptype = build_vector_type (TREE_TYPE (vtype), inelts);
  return vtype;
}

This new version has been bootstrapped/regtested on 
powerpc64le-linux-gnu (LE) P8 and x86_64-redhat-linux.

May I install this new instead?

BR,
Kewen
-
gcc/ChangeLog

2020-MM-DD  Kewen Lin  

PR tree-optimization/90332
* gcc/tree-vect-stmts.c (vector_vector_composition_type): New function.
(get_group_load_store_type): Adjust to call 
vector_vector_composition_type,
extend it to construct with scalar types.
(vectorizable_load): Likewise.
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 2ca8e494680..12beef6978c 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -2220,6 +2220,62 @@ vect_get_store_rhs (stmt_vec_info stmt_info)
   gcc_unreachable ();
 }
 
+/* Function VECTOR_VECTOR_COMPOSITION_TYPE
+
+   This function returns a vector type which can be composed with NETLS pieces,
+   whose type is recorded in PTYPE.  VTYPE should be a vector type, and has the
+   same vector size as the return vector.  It checks target whether supports
+   pieces-size vector mode for construction firstly, if target fails to, check
+   pieces-size scalar mode for construction further.  It returns NULL_TREE if
+   fails to find the available composition.
+
+   For example, for (vtype=V16QI, nelts=4), we can probably get:
+ - V16QI with PTYPE V4QI.
+ - V4SI with PTYPE SI.
+ - NULL_TREE.  */
+
+static tree
+vector_vector_composition_type (tree vtype, poly_uint64 nelts, tree *ptype)
+{
+  gcc_assert (VECTOR_TYPE_P (vtype));
+  gcc_assert (known_gt (nelts, 0U));
+
+  machine_mode vmode = TYPE_MODE (vtype);
+  if (!VECTOR_MODE_P (vmode))
+return NULL_TREE;
+
+  poly_uint64 vbsize = GET_MODE_BITSIZE (vmode);
+  unsigned int pbsize;
+  if (constant_multiple_p (vbsize, nelts, ))
+{
+  /* First check if vec_init optab supports construction from
+vector pieces directly.  */
+  scalar_mode elmode = SCALAR_TYPE_MODE (TREE_TYPE (vtype));
+  poly_uint64 inelts = pbsize / GET_MODE_BITSIZE (elmode);
+  machine_mode rmode;
+  if (related_vector_mode (vmode, elmode, inelts).exists ()
+ && (convert_optab_handler (vec_init_optab, vmode, rmode)
+ != CODE_FOR_nothing))
+   {
+ *ptype = build_vector_type (TREE_TYPE (vtype), inelts);
+ return vtype;
+   }
+
+  /* Otherwise check if exists an integer type of the same piece size and
+if vec_init optab supports construction from it directly.  */
+  if (int_mode_for_size (pbsize, 0).exists ()
+ && related_vector_mode (vmode, elmode, nelts).exists ()
+ && (convert_optab_handler (vec_init_optab, rmode, elmode)
+ != CODE_FOR_nothing))
+   {
+ *ptype = build_nonstandard_integer_type (pbsize, 1);
+ return build_vector_type (*ptype, nelts);
+   }
+}
+
+  return NULL_TREE;
+}
+
 /* A subroutine of get_load_store_type, with a subset of the same
arguments.  Handle the case where STMT_INFO is part of a grouped load
or store.
@@ -2300,8 +2356,7 @@ get_group_load_store_type (stmt_vec_info stmt_info, tree 
vectype, bool slp,
 by simply loading half of the vector only.  Usually
 the construction with an upper zero half will be elided.  */
  dr_alignment_support alignment_support_scheme;
- scalar_mode elmode = SCALAR_TYPE_MODE (TREE_TYPE (vectype));
- machine_mode vmode;
+ tree half_vtype;
  if (overrun_p
  && !masked_p
  && (((alignment_support_scheme
@@ -2310,12 +2365,8 @@ get_group_load_store_type (stmt_vec_info stmt_info, tree 
vectype, bool slp,
  || alignment_support_scheme == 

[committed] arm: unified syntax for libgcc when built with -Os [PR94220]

[Richard Earnshaw (lists)]

The recent patch to convert all thumb1 code in libgcc to unified syntax
ommitted the conditional code that is used only when building the
library for minimal size.  This patch fixes this case.

I've also fixed the COND macro so that a single definition is always
used that is for unified syntax.  This eliminates a warning that is now
being seen from the assembler when compiling the ieee fp support code.

PR target/94220
* config/arm/lib1funcs.asm (COND): Use a single definition for
unified syntax.
(aeabi_uidivmod): Unified syntax when optimizing Thumb for size.
(aeabi_idivmod): Likewise.
(divsi3_skip_div0_test): Likewise.
---
 libgcc/config/arm/lib1funcs.S | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
index e8d2158f8d6..a094417e786 100644
--- a/libgcc/config/arm/lib1funcs.S
+++ b/libgcc/config/arm/lib1funcs.S
@@ -226,7 +226,6 @@ LSYM(Lend_fde):
 .endm
 #define do_pushpush
 #define do_pop pop
-#define COND(op1, op2, cond) op1 ## op2 ## cond
 /* Perform an arithmetic operation with a variable shift operand.  This
requires two instructions and a scratch register on Thumb-2.  */
 .macro shiftop name, dest, src1, src2, shiftop, shiftreg, tmp
@@ -241,12 +240,13 @@ LSYM(Lend_fde):
 .endm
 #define do_pushstmfd sp!,
 #define do_pop ldmfd sp!,
-#define COND(op1, op2, cond) op1 ## cond ## op2
 .macro shiftop name, dest, src1, src2, shiftop, shiftreg, tmp
\name \dest, \src1, \src2, \shiftop \shiftreg
 .endm
 #endif
 +#define COND(op1, op2, cond) op1 ## op2 ## cond
+
 #ifdef __ARM_EABI__
 .macro ARM_LDIV0 name signed
cmp r0, #0
@@ -494,7 +494,8 @@ pc  .reqr15
  /*
 */
 /* Bodies of the division and modulo routines. */
-/*
 */ 
+/*
 */
+
 .macro ARM_DIV_BODY dividend, divisor, result, curbit
  #if defined (__ARM_FEATURE_CLZ) && ! defined (__OPTIMIZE_SIZE__)
@@ -1136,8 +1137,8 @@ FUNC_START aeabi_uidivmod
push{r0, r1, lr}
bl  LSYM(udivsi3_skip_div0_test)
POP {r1, r2, r3}
-   mul r2, r0
-   sub r1, r1, r2
+   mulsr2, r0
+   subsr1, r1, r2
bx  r3
 # else
/* Both the quotient and remainder are calculated simultaneously
@@ -1151,7 +1152,7 @@ FUNC_START aeabi_uidivmod
 ARM_FUNC_START aeabi_uidivmod
cmp r1, #0
beq LSYM(Ldiv0)
-   mov r2, r0 +mov r2, r0
udivr0, r0, r1
mls r1, r0, r1, r2
RET
@@ -1235,29 +1236,29 @@ LSYM(Lover10):
beq LSYM(Ldiv0)
 LSYM(divsi3_skip_div0_test):
push{ work }
-   mov work, dividend
-   eor work, divisor   @ Save the sign of the result.
+   movswork, dividend
+   eorswork, divisor   @ Save the sign of the result.
mov ip, work
-   mov curbit, #1
-   mov result, #0
+   movscurbit, #1
+   movsresult, #0
cmp divisor, #0
bpl LSYM(Lover10)
-   neg divisor, divisor@ Loops below use unsigned.
+   negsdivisor, divisor@ Loops below use unsigned.
 LSYM(Lover10):
cmp dividend, #0
bpl LSYM(Lover11)
-   neg dividend, dividend
+   negsdividend, dividend
 LSYM(Lover11):
cmp dividend, divisor
blo LSYM(Lgot_result)
THUMB_DIV_MOD_BODY 0
 -  mov r0, result
+   movsr0, result
mov work, ip
cmp work, #0
bpl LSYM(Lover12)
-   neg r0, r0
+   negsr0, r0
 LSYM(Lover12):
pop { work }
RET
@@ -1348,8 +1349,8 @@ FUNC_START aeabi_idivmod
push{r0, r1, lr}
bl  LSYM(divsi3_skip_div0_test)
POP {r1, r2, r3}
-   mul r2, r0
-   sub r1, r1, r2
+   mulsr2, r0
+   subsr1, r1, r2
bx  r3
 # else
/* Both the quotient and remainder are calculated simultaneously
-- 
2.26.0

Re: [PATCH] tree: Fix -fcompare-debug issues due to protected_set_expr_location [PR94323]

[Richard Biener]

On Thu, 26 Mar 2020, Jakub Jelinek wrote:

> Hi!
> 
> On Thu, Mar 26, 2020 at 10:24:29AM +0100, Richard Biener wrote:
> > On Thu, 26 Mar 2020, Jakub Jelinek wrote:
> > 
> > > On Thu, Mar 26, 2020 at 10:14:35AM +0100, Richard Biener wrote:
> > > > Ick.  I fear we really need a better way of dealing with this
> > > > STATEMENT_LIST appearance with only -g ...
> > > 
> > > Yeah, it is very ugly and in some PRs I'm out of ideas what to do.
> > 
> > I think a more sustainable "fix" would be to simply not emit
> > the DEBUG_BEGIN_STMT when it would be the only reason to
> > emit a STATEMENT_LIST ...
> > 
> > Or always emit a STATEMENT_LIST node whenever there could be one
> > (hopefully not every stmt could be a stmt list and hopefully
> > we don't treat single-stmt STATEMENT_LIST specially).
> > 
> > That is, the fix should be to avoid this difference in the first
> > place, not try to deal with the difference later.
> 
> Or perhaps just a flag on the STATEMENT_LIST that would make it clear the
> STATEMENT_LIST wouldn't be there without -g.  Or different tree code like
> STATEMENT_LIST, except that it would be only this kind of container.

But then you still need to deal with those so the fixups would look
similar, it just shortens the checks a bit.

> Or disable -gstatement-frontiers by default and declare it -fcompare-debug
> incompatible.  Do any debug info consumers consume this?

I'm not aware of any.

Richard.

> In any case, I'd really like Alex' feedback on this.
>
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [PATCH] tree: Fix -fcompare-debug issues due to protected_set_expr_location [PR94323]

[Jakub Jelinek via Gcc-patches]

Hi!

On Thu, Mar 26, 2020 at 10:24:29AM +0100, Richard Biener wrote:
> On Thu, 26 Mar 2020, Jakub Jelinek wrote:
> 
> > On Thu, Mar 26, 2020 at 10:14:35AM +0100, Richard Biener wrote:
> > > Ick.  I fear we really need a better way of dealing with this
> > > STATEMENT_LIST appearance with only -g ...
> > 
> > Yeah, it is very ugly and in some PRs I'm out of ideas what to do.
> 
> I think a more sustainable "fix" would be to simply not emit
> the DEBUG_BEGIN_STMT when it would be the only reason to
> emit a STATEMENT_LIST ...
> 
> Or always emit a STATEMENT_LIST node whenever there could be one
> (hopefully not every stmt could be a stmt list and hopefully
> we don't treat single-stmt STATEMENT_LIST specially).
> 
> That is, the fix should be to avoid this difference in the first
> place, not try to deal with the difference later.

Or perhaps just a flag on the STATEMENT_LIST that would make it clear the
STATEMENT_LIST wouldn't be there without -g.  Or different tree code like
STATEMENT_LIST, except that it would be only this kind of container.
Or disable -gstatement-frontiers by default and declare it -fcompare-debug
incompatible.  Do any debug info consumers consume this?

In any case, I'd really like Alex' feedback on this.

Jakub

[PATCH] rs6000: Don't split constant oprator when add, move to temp register for future optimization

[luoxhu--- via Gcc-patches]

From: Xionghu Luo 

Remove split code from add3 to allow a later pass to split.
This allows later logic to hoist out constant load in add instructions.
In loop, lis+ori could be hoisted out to improve performance compared with
previous addis+addi (About 15% on typical case), weak point is
one more register is used and one more instruction is generated.  i.e.:

addis 3,3,0x8765
addi 3,3,0x4321

=>

lis 9,0x8765
ori 9,9,0x4321
add 3,3,9

gcc/ChangeLog:

2020-03-26  Xiong Hu Luo  

* config/rs6000/rs6000.md (add3): Remove split code, move constant
  to temp register before add.

gcc/testsuite/ChangeLog:

2020-03-26  Xiong Hu Luo  

* gcc.target/powerpc/add-const.c: New.
---
 gcc/config/rs6000/rs6000.md  | 25 ++--
 gcc/testsuite/gcc.target/powerpc/add-const.c | 18 ++
 2 files changed, 20 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/add-const.c

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index ad88b6783af..72f3f604e0d 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -1733,29 +1733,8 @@ (define_expand "add3"
  || rtx_equal_p (operands[0], operands[1]))
 ? operands[0] : gen_reg_rtx (mode));
 
-  /* Adding a constant to r0 is not a valid insn, so use a different
-strategy in that case.  */
-  if (reg_or_subregno (operands[1]) == 0 || reg_or_subregno (tmp) == 0)
-   {
- if (operands[0] == operands[1])
-   FAIL;
- rs6000_emit_move (operands[0], operands[2], mode);
- emit_insn (gen_add3 (operands[0], operands[1], operands[0]));
- DONE;
-   }
-
-  HOST_WIDE_INT val = INTVAL (operands[2]);
-  HOST_WIDE_INT low = ((val & 0x) ^ 0x8000) - 0x8000;
-  HOST_WIDE_INT rest = trunc_int_for_mode (val - low, mode);
-
-  if (mode == DImode && !satisfies_constraint_L (GEN_INT (rest)))
-   FAIL;
-
-  /* The ordering here is important for the prolog expander.
-When space is allocated from the stack, adding 'low' first may
-produce a temporary deallocation (which would be bad).  */
-  emit_insn (gen_add3 (tmp, operands[1], GEN_INT (rest)));
-  emit_insn (gen_add3 (operands[0], tmp, GEN_INT (low)));
+  rs6000_emit_move (tmp, operands[2], mode);
+  emit_insn (gen_add3 (operands[0], operands[1], tmp));
   DONE;
 }
 })
diff --git a/gcc/testsuite/gcc.target/powerpc/add-const.c 
b/gcc/testsuite/gcc.target/powerpc/add-const.c
new file mode 100644
index 000..e1007247b32
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/add-const.c
@@ -0,0 +1,18 @@
+/* { dg-do compile { target { lp64 } } } */
+/* { dg-options "-O3 -fno-unroll-loops" } */
+
+/* Ensure the lis,ori are generated, which indicates they have
+   been hoisted outside of the loop.  */
+
+typedef unsigned long ulong;
+ulong
+foo (ulong n, ulong h)
+{
+  int i;
+  for (i = 0; i < n; i++)
+h = ((h + 8) | h) + 0x87654321;
+  return h;
+}
+
+/* { dg-final { scan-assembler-times {\mlis\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mori\M} 1 } } */
-- 
2.21.0.777.g83232e3864

Re: [stage1] [PATCH] Make target_clones resolver fn static if possible.

[Jakub Jelinek via Gcc-patches]

On Thu, Mar 26, 2020 at 10:46:48AM +0100, Martin Liška wrote:
> 2020-03-26  Martin Liska  
> 
>   * gcc.target/i386/pr81213.c: Do not scan assembler
>   and add one missing PR entry.

Ok.

> ---
>  gcc/testsuite/gcc.target/i386/pr81213.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.target/i386/pr81213.c 
> b/gcc/testsuite/gcc.target/i386/pr81213.c
> index 334838631d0..6194e9ecccd 100644
> --- a/gcc/testsuite/gcc.target/i386/pr81213.c
> +++ b/gcc/testsuite/gcc.target/i386/pr81213.c
> @@ -1,3 +1,4 @@
> +/* PR ipa/81213.  */
>  /* PR ipa/81214.  */
>  /* { dg-do run } */
>  /* { dg-require-ifunc "" } */
> @@ -16,7 +17,3 @@ int main()
>  {
>return foo() + bar();
>  }
> -
> -/* { dg-final { scan-assembler "\t.globl\tfoo" } } */
> -/* { dg-final { scan-assembler "foo.resolver:" } } */
> -/* { dg-final { scan-assembler "foo\\, @gnu_indirect_function" } } */
> -- 
> 2.25.1
> 


Jakub

Re: [stage1] [PATCH] Make target_clones resolver fn static if possible.

[Martin Liška]

On 3/26/20 10:23 AM, Jakub Jelinek wrote:

On Mon, Mar 23, 2020 at 04:09:52PM +0100, Martin Liška wrote:

2020-03-17  Martin Liska  

PR target/93274 PR lto/94271
* gcc.target/i386/pr81213-2.c: New test.
* gcc.target/i386/pr81213.c: Add additional source.
* gcc.dg/lto/pr94271_0.c: New test.
* gcc.dg/lto/pr94271_1.c: New test.


I've noticed this test now has UNRESOLVED cases:
+UNRESOLVED: gcc.target/i386/pr81213.c scan-assembler \\t.globl\\tfoo
+UNRESOLVED: gcc.target/i386/pr81213.c scan-assembler foo.resolver:
+UNRESOLVED: gcc.target/i386/pr81213.c scan-assembler foo, 
@gnu_indirect_function

1) shall the test start with PR ipa/81214 reference when it is PR ipa/81213
test?


Probably yes, note that the test is relevant also to ipa/81214. So I'll add 
both PRs
references.


2) the UNRESOLVED cases can be fixed e.g. through:

--- gcc/testsuite/gcc.target/i386/pr81213.c.jj  2020-03-25 11:39:07.605865708 
+0100
+++ gcc/testsuite/gcc.target/i386/pr81213.c 2020-03-26 10:13:23.616527400 
+0100
@@ -1,6 +1,7 @@
  /* PR ipa/81214.  */
  /* { dg-do run } */
  /* { dg-require-ifunc "" } */
+/* { dg-options "-save-temps" } */
  /* { dg-additional-sources "pr81213-2.c" } */
  
  int bar();


3) but then one ends up with
FAIL: gcc.target/i386/pr81213.c scan-assembler \t.globl\tfoo


The asm scan does not make sense now as it's a run-time test
and we would see a linker error in case of 2 .globl foo symbols.

I'm suggesting a patch.
Martin



Do you want to change that to scan-assembler-not now that you don't want to
make foo public, or something else?


--- a/gcc/testsuite/gcc.target/i386/pr81213.c
+++ b/gcc/testsuite/gcc.target/i386/pr81213.c
@@ -1,6 +1,9 @@
  /* PR ipa/81214.  */
-/* { dg-do compile } */
+/* { dg-do run } */
  /* { dg-require-ifunc "" } */
+/* { dg-additional-sources "pr81213-2.c" } */
+
+int bar();
  
  __attribute__((target_clones("avx","arch=slm","arch=core-avx2","default")))

  static int
@@ -11,7 +14,7 @@ foo ()
  
  int main()

  {
-  return foo();
+  return foo() + bar();
  }
  
  /* { dg-final { scan-assembler "\t.globl\tfoo" } } */

--
2.25.1



Jakub



>From 6a5c17037c0a20bc687a0c08234928959f441971 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Thu, 26 Mar 2020 10:45:50 +0100
Subject: [PATCH] Fix UNRESOLVED test-case.

gcc/testsuite/ChangeLog:

2020-03-26  Martin Liska  

	* gcc.target/i386/pr81213.c: Do not scan assembler
	and add one missing PR entry.
---
 gcc/testsuite/gcc.target/i386/pr81213.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/gcc/testsuite/gcc.target/i386/pr81213.c b/gcc/testsuite/gcc.target/i386/pr81213.c
index 334838631d0..6194e9ecccd 100644
--- a/gcc/testsuite/gcc.target/i386/pr81213.c
+++ b/gcc/testsuite/gcc.target/i386/pr81213.c
@@ -1,3 +1,4 @@
+/* PR ipa/81213.  */
 /* PR ipa/81214.  */
 /* { dg-do run } */
 /* { dg-require-ifunc "" } */
@@ -16,7 +17,3 @@ int main()
 {
   return foo() + bar();
 }
-
-/* { dg-final { scan-assembler "\t.globl\tfoo" } } */
-/* { dg-final { scan-assembler "foo.resolver:" } } */
-/* { dg-final { scan-assembler "foo\\, @gnu_indirect_function" } } */
-- 
2.25.1

[stage1][PATCH] Provide warning for missing jobserver.

[Martin Liška]

Hi.

I'm suggesting to provide a warning when one uses -flto=jobserver
but we can't detect job server for some reason.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed in next stage1?
Thanks,
Martin

gcc/ChangeLog:

2020-03-26  Martin Liska  

PR driver/94330
* lto-wrapper.c (run_gcc): When using -flto=jobserver,
report warning when the jobserver is not detected.
---
 gcc/lto-wrapper.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)


diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 46a88b233f6..6263c164888 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1473,10 +1473,16 @@ run_gcc (unsigned argc, char *argv[])
   auto_parallel = 0;
   parallel = 0;
 }
-  else if (!jobserver && jobserver_active_p ())
+  else
 {
-  parallel = 1;
-  jobserver = 1;
+  bool active_jobserver = jobserver_active_p ();
+  if (jobserver && !active_jobserver)
+	warning (0, "jobserver is not available.");
+  else if (!jobserver && active_jobserver)
+	{
+	  parallel = 1;
+	  jobserver = 1;
+	}
 }
 
   if (linker_output)

[PATCH] document --param allow-store-data-races change

[Richard Biener]

I pushed the following update.

2020-03-26  Richard Biener  

* gcc-10/changes.html: Document --param allow-store-data-races
change.
---
 htdocs/gcc-10/changes.html | 9 +
 1 file changed, 9 insertions(+)

diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html
index 5e358a0c..3c3a7830 100644
--- a/htdocs/gcc-10/changes.html
+++ b/htdocs/gcc-10/changes.html
@@ -48,6 +48,15 @@ a work-in-progress.
 The automatic template instantiation at link time (-frepo)
 has been removed.
   
+  
+The --param allow-store-data-races internal parameter has
+been removed in favor of a new official option
+-fallow-store-data-races.  While default behavior is
+unchanged and the new option allows to correctly maintain a per
+compilation unit setting across link-time optimization, alteration
+of the default via --param allow-store-data-races will
+now be diagnosed and build systems have to be adjusted accordingly.
+  
 
 
 
-- 
2.16.4

Re: [PATCH] tree: Fix -fcompare-debug issues due to protected_set_expr_location [PR94323]

[Richard Biener]

On Thu, 26 Mar 2020, Jakub Jelinek wrote:

> On Thu, Mar 26, 2020 at 10:14:35AM +0100, Richard Biener wrote:
> > Ick.  I fear we really need a better way of dealing with this
> > STATEMENT_LIST appearance with only -g ...
> 
> Yeah, it is very ugly and in some PRs I'm out of ideas what to do.

I think a more sustainable "fix" would be to simply not emit
the DEBUG_BEGIN_STMT when it would be the only reason to
emit a STATEMENT_LIST ...

Or always emit a STATEMENT_LIST node whenever there could be one
(hopefully not every stmt could be a stmt list and hopefully
we don't treat single-stmt STATEMENT_LIST specially).

That is, the fix should be to avoid this difference in the first
place, not try to deal with the difference later.

Richard.

Re: [stage1] [PATCH] Make target_clones resolver fn static if possible.

[Jakub Jelinek via Gcc-patches]

On Mon, Mar 23, 2020 at 04:09:52PM +0100, Martin Liška wrote:
> 2020-03-17  Martin Liska  
> 
>   PR target/93274 PR lto/94271
>   * gcc.target/i386/pr81213-2.c: New test.
>   * gcc.target/i386/pr81213.c: Add additional source.
>   * gcc.dg/lto/pr94271_0.c: New test.
>   * gcc.dg/lto/pr94271_1.c: New test.

I've noticed this test now has UNRESOLVED cases:
+UNRESOLVED: gcc.target/i386/pr81213.c scan-assembler \\t.globl\\tfoo
+UNRESOLVED: gcc.target/i386/pr81213.c scan-assembler foo.resolver:
+UNRESOLVED: gcc.target/i386/pr81213.c scan-assembler foo, 
@gnu_indirect_function

1) shall the test start with PR ipa/81214 reference when it is PR ipa/81213
test?
2) the UNRESOLVED cases can be fixed e.g. through:

--- gcc/testsuite/gcc.target/i386/pr81213.c.jj  2020-03-25 11:39:07.605865708 
+0100
+++ gcc/testsuite/gcc.target/i386/pr81213.c 2020-03-26 10:13:23.616527400 
+0100
@@ -1,6 +1,7 @@
 /* PR ipa/81214.  */
 /* { dg-do run } */
 /* { dg-require-ifunc "" } */
+/* { dg-options "-save-temps" } */
 /* { dg-additional-sources "pr81213-2.c" } */
 
 int bar();

3) but then one ends up with
FAIL: gcc.target/i386/pr81213.c scan-assembler \t.globl\tfoo

Do you want to change that to scan-assembler-not now that you don't want to
make foo public, or something else?

> --- a/gcc/testsuite/gcc.target/i386/pr81213.c
> +++ b/gcc/testsuite/gcc.target/i386/pr81213.c
> @@ -1,6 +1,9 @@
>  /* PR ipa/81214.  */
> -/* { dg-do compile } */
> +/* { dg-do run } */
>  /* { dg-require-ifunc "" } */
> +/* { dg-additional-sources "pr81213-2.c" } */
> +
> +int bar();
>  
>  __attribute__((target_clones("avx","arch=slm","arch=core-avx2","default")))
>  static int
> @@ -11,7 +14,7 @@ foo ()
>  
>  int main()
>  {
> -  return foo();
> +  return foo() + bar();
>  }
>  
>  /* { dg-final { scan-assembler "\t.globl\tfoo" } } */
> -- 
> 2.25.1
> 

Jakub

Re: [PATCH] tree: Fix -fcompare-debug issues due to protected_set_expr_location [PR94323]

[Jakub Jelinek via Gcc-patches]

On Thu, Mar 26, 2020 at 10:14:35AM +0100, Richard Biener wrote:
> Ick.  I fear we really need a better way of dealing with this
> STATEMENT_LIST appearance with only -g ...

Yeah, it is very ugly and in some PRs I'm out of ideas what to do.

Jakub

Re: [PING PATCH coroutines] Do not strip cleanup_point when promote temporaries out of current stmt

[Iain Sandoe]

Hi Bin,

Bin.Cheng via Gcc-patches  wrote:


On Mon, Mar 16, 2020 at 5:34 PM Bin.Cheng  wrote:

On Wed, Mar 11, 2020 at 5:07 PM Iain Sandoe  wrote:

Bin.Cheng  wrote:


On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe  wrote:



With current trunk + Bin’s two approved patches.

I see no change in the testcase (lambda-09-capture-object.C) before /  
after

the patch
(it fails for me at -O0 only - in both cases).


If it helps, I could push a branch to users/iains/ on the FSF git  
server with this sequence.
Sorry for being slow replying.  This is weird, were you testing against  
trunk?


Yes.


So I suspect you were testing with local changes?


No local changes.

(and two different platforms - x86_64-darwin16, and x86_64-linux-gnu  
[cfarm122]).


* At least on Darwin (I have a trunk build from overnight) I see no change  
at r10-7390-g27f8c8c4c923.



 If so, please push the branch.

Ping.  Any updates?


* I am currently working first on trying to complete the n4849 update  
changes.


* This remains on my TODO.
  - I am trying to figure out how you can consistently get different results 
from me.


I noticed you committed fix to coro-torture.exp,


Hopefully, at least, the testsuite problem is fixed for you now?

thanks
Iain

Re: [PATCH] tree: Fix -fcompare-debug issues due to protected_set_expr_location [PR94323]

[Richard Biener]

On Thu, 26 Mar 2020, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase FAILs since recently when the C++ FE started calling
> protected_set_expr_location more often.
> With -g, it is called on a STATEMENT_LIST that contains a DEBUG_BEGIN_STMT
> and CLEANUP_POINT_EXPR, and as STATEMENT_LISTs have !CAN_HAVE_LOCATION_P,
> nothing is set.  Without -g, it is called instead on the CLEANUP_POINT_EXPR
> directly and changes its location.
> 
> The following patch recurses on the single non-DEBUG_BEGIN_STMT statement
> of a STATEMENT_LIST if any to make the two behave the same.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ick.  I fear we really need a better way of dealing with this
STATEMENT_LIST appearance with only -g ...

But OK for now.

Richard.

> 2020-03-26  Jakub Jelinek  
> 
>   PR debug/94323
>   * tree.c (protected_set_expr_location): Recurse on STATEMENT_LIST
>   that contains exactly one non-DEBUG_BEGIN_STMT statement.
> 
>   * g++.dg/debug/pr94323.C: New test.
> 
> --- gcc/tree.c.jj 2020-03-23 19:46:45.552448327 +0100
> +++ gcc/tree.c2020-03-25 17:22:12.438904778 +0100
> @@ -5146,6 +5146,33 @@ protected_set_expr_location (tree t, loc
>  {
>if (CAN_HAVE_LOCATION_P (t))
>  SET_EXPR_LOCATION (t, loc);
> +  else if (t && TREE_CODE (t) == STATEMENT_LIST)
> +{
> +  /* With -gstatement-frontiers we could have a STATEMENT_LIST with
> +  DEBUG_BEGIN_STMT(s) and only a single other stmt, which with
> +  -g wouldn't be present and we'd have that single other stmt
> +  directly instead.  */
> +  struct tree_statement_list_node *n = STATEMENT_LIST_HEAD (t);
> +  if (!n)
> + return;
> +  while (TREE_CODE (n->stmt) == DEBUG_BEGIN_STMT)
> + {
> +   n = n->next;
> +   if (!n)
> + return;
> + }
> +  tree t2 = n->stmt;
> +  do
> + {
> +   n = n->next;
> +   if (!n)
> + {
> +   protected_set_expr_location (t2, loc);
> +   return;
> + }
> + }
> +  while (TREE_CODE (n->stmt) == DEBUG_BEGIN_STMT);
> +}
>  }
>  
>  /* Data used when collecting DECLs and TYPEs for language data removal.  */
> --- gcc/testsuite/g++.dg/debug/pr94323.C.jj   2020-03-25 17:17:49.857819078 
> +0100
> +++ gcc/testsuite/g++.dg/debug/pr94323.C  2020-03-25 17:17:17.533300951 
> +0100
> @@ -0,0 +1,13 @@
> +// PR debug/94323
> +// { dg-do compile }
> +// { dg-options "-O2 -fcompare-debug" }
> +
> +volatile int a;
> +
> +void
> +foo ()
> +{
> +  ({
> + a;
> +   });
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

New Finnish PO file for 'gcc' (version 10.1-b20200322)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'gcc' has been submitted
by the Finnish team of translators.  The file is available at:

https://translationproject.org/latest/gcc/fi.po

(This file, 'gcc-10.1-b20200322.fi.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/gcc/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/gcc.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

Re: [OBVIOUS][PATCH] Skip test for non-x86 targets.

[Martin Liška]

There are 2 more nits needed for the test-case fix.
I'm going to install this version.

Martin
>From 3aece9a2f2eed08c91d5f52d5481701d01bc3a90 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Thu, 26 Mar 2020 10:07:06 +0100
Subject: [PATCH] Skip test for non-x86 targets.

gcc/testsuite/ChangeLog:

2020-03-26  Martin Liska  

	PR testsuite/94334
	* gcc.dg/lto/pr94271_0.c: Skip for non-x86 targets
	and add ifunc effective target.
	* gcc.target/i386/pr81213-2.c: Add ifunc effective target.
---
 gcc/testsuite/gcc.dg/lto/pr94271_0.c  | 2 ++
 gcc/testsuite/gcc.target/i386/pr81213-2.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/gcc/testsuite/gcc.dg/lto/pr94271_0.c b/gcc/testsuite/gcc.dg/lto/pr94271_0.c
index 2ce7d65411a..e3c3fa17ea0 100644
--- a/gcc/testsuite/gcc.dg/lto/pr94271_0.c
+++ b/gcc/testsuite/gcc.dg/lto/pr94271_0.c
@@ -1,5 +1,7 @@
 /* PR lto/94271 */
+/* { dg-skip-if "" { ! { i?86-*-* x86_64-*-* } } } */
 /* { dg-lto-do link } */
+/* { dg-require-ifunc "" } */
 
 int a;
 
diff --git a/gcc/testsuite/gcc.target/i386/pr81213-2.c b/gcc/testsuite/gcc.target/i386/pr81213-2.c
index a80622cb184..ec9138018ea 100644
--- a/gcc/testsuite/gcc.target/i386/pr81213-2.c
+++ b/gcc/testsuite/gcc.target/i386/pr81213-2.c
@@ -1,3 +1,4 @@
+/* { dg-require-ifunc "" } */
 __attribute__((target_clones("avx","arch=slm","arch=core-avx2","default")))
 static int
 foo ()
-- 
2.25.1

Re: [OBVIOUS][PATCH] Skip test for non-x86 targets.

[Rainer Orth]

Hi Martin,

> The test-case only works on x86 targets.
> I'm going to install the patch.

as noted in the PR, you also need

/* { dg-require-ifunc "" } */

Rainer

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

Re: [PATCH] gimplify: Fix -fcompare-debug differences caused by gimplify_body [PR94281]

[Richard Biener]

On Thu, 26 Mar 2020, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase FAILs, because gimplify_body adds a GIMPLE_NOP only
> when there are no statements in the function and with -g there is a
> DEBUG_BEGIN_STMT, so it doesn't add it and due to -fno-tree-dce that never
> gets removed afterwards.  Similarly, if the body seq after gimplification
> contains some DEBUG_BEGIN_STMTs plus a single gbind, then we could behave
> differently between -g0 and -g, by using that gbind as the body in the -g0
> case and not in the -g case.
> This patch fixes that by ignoring DEBUG_BEGIN_STMTs (other debug stmts can't
> appear at this point yet thankfully) during decisions and if we pick the
> single gbind and there are DEBUG_BEGIN_STMTs next to it, it moves them into
> the gbind.
> While debugging this, I found also a bug in the gimple_seq_last_nondebug_stmt
> function, for a seq that has a single non-DEBUG_BEGIN_STMT statement
> followed by one or more DEBUG_BEGIN_STMTs it would return NULL rather than
> the first statement.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

OK.

Thanks,
Richard.

> 2020-03-26  Jakub Jelinek  
> 
>   PR debug/94281
>   * gimple.h (gimple_seq_first_nondebug_stmt): New function.
>   (gimple_seq_last_nondebug_stmt): Don't return NULL if seq contains
>   a single non-debug stmt followed by one or more debug stmts.
>   * gimplify.c (gimplify_body): Use gimple_seq_first_nondebug_stmt
>   instead of gimple_seq_first_stmt, use gimple_seq_first_nondebug_stmt
>   and gimple_seq_last_nondebug_stmt instead of gimple_seq_first and
>   gimple_seq_last to check if outer_stmt gbind could be reused and
>   if yes and it is surrounded by any debug stmts, move them into the
>   gbind body.
> 
>   * g++.dg/debug/pr94281.C: New test.
> 
> --- gcc/gimple.h.jj   2020-03-25 14:34:07.696199561 +0100
> +++ gcc/gimple.h  2020-03-25 15:26:38.045052864 +0100
> @@ -4728,6 +4728,18 @@ is_gimple_debug (const gimple *gs)
>  }
>  
>  
> +/* Return the first nondebug statement in GIMPLE sequence S.  */
> +
> +static inline gimple *
> +gimple_seq_first_nondebug_stmt (gimple_seq s)
> +{
> +  gimple_seq_node n = gimple_seq_first (s);
> +  while (n && is_gimple_debug (n))
> +n = n->next;
> +  return n;
> +}
> +
> +
>  /* Return the last nondebug statement in GIMPLE sequence S.  */
>  
>  static inline gimple *
> @@ -4737,7 +4749,7 @@ gimple_seq_last_nondebug_stmt (gimple_se
>for (n = gimple_seq_last (s);
> n && is_gimple_debug (n);
> n = n->prev)
> -if (n->prev == s)
> +if (n == s)
>return NULL;
>return n;
>  }
> --- gcc/gimplify.c.jj 2020-03-25 14:34:07.722199170 +0100
> +++ gcc/gimplify.c2020-03-25 14:41:46.447348434 +0100
> @@ -14849,7 +14849,7 @@ gimplify_body (tree fndecl, bool do_parm
>/* Gimplify the function's body.  */
>seq = NULL;
>gimplify_stmt (_SAVED_TREE (fndecl), );
> -  outer_stmt = gimple_seq_first_stmt (seq);
> +  outer_stmt = gimple_seq_first_nondebug_stmt (seq);
>if (!outer_stmt)
>  {
>outer_stmt = gimple_build_nop ();
> @@ -14859,8 +14859,37 @@ gimplify_body (tree fndecl, bool do_parm
>/* The body must contain exactly one statement, a GIMPLE_BIND.  If this is
>   not the case, wrap everything in a GIMPLE_BIND to make it so.  */
>if (gimple_code (outer_stmt) == GIMPLE_BIND
> -  && gimple_seq_first (seq) == gimple_seq_last (seq))
> -outer_bind = as_a  (outer_stmt);
> +  && (gimple_seq_first_nondebug_stmt (seq)
> +   == gimple_seq_last_nondebug_stmt (seq)))
> +{
> +  outer_bind = as_a  (outer_stmt);
> +  if (gimple_seq_first_stmt (seq) != outer_stmt
> +   || gimple_seq_last_stmt (seq) != outer_stmt)
> + {
> +   /* If there are debug stmts before or after outer_stmt, move them
> +  inside of outer_bind body.  */
> +   gimple_stmt_iterator gsi = gsi_for_stmt (outer_stmt, );
> +   gimple_seq second_seq = NULL;
> +   if (gimple_seq_first_stmt (seq) != outer_stmt
> +   && gimple_seq_last_stmt (seq) != outer_stmt)
> + {
> +   second_seq = gsi_split_seq_after (gsi);
> +   gsi_remove (, false);
> + }
> +   else if (gimple_seq_first_stmt (seq) != outer_stmt)
> + gsi_remove (, false);
> +   else
> + {
> +   gsi_remove (, false);
> +   second_seq = seq;
> +   seq = NULL;
> + }
> +   gimple_seq_add_seq_without_update (,
> +  gimple_bind_body (outer_bind));
> +   gimple_seq_add_seq_without_update (, second_seq);
> +   gimple_bind_set_body (outer_bind, seq);
> + }
> +}
>else
>  outer_bind = gimple_build_bind (NULL_TREE, seq, NULL);
>  
> --- gcc/testsuite/g++.dg/debug/pr94281.C.jj   2020-03-25 14:47:11.241498952 
> +0100
> +++ gcc/testsuite/g++.dg/debug/pr94281.C  2020-03-25 14:45:32.637971196 
> +0100
> @@ -0,0 +1,11 

[OBVIOUS][PATCH] Skip test for non-x86 targets.

[Martin Liška]

Hi.

The test-case only works on x86 targets.
I'm going to install the patch.

Martin

gcc/testsuite/ChangeLog:

2020-03-26  Martin Liska  

PR testsuite/94334
* gcc.dg/lto/pr94271_0.c: Skip for non-x86 targets.
---
 gcc/testsuite/gcc.dg/lto/pr94271_0.c | 1 +
 1 file changed, 1 insertion(+)


diff --git a/gcc/testsuite/gcc.dg/lto/pr94271_0.c b/gcc/testsuite/gcc.dg/lto/pr94271_0.c
index 2ce7d65411a..2d0f1453afa 100644
--- a/gcc/testsuite/gcc.dg/lto/pr94271_0.c
+++ b/gcc/testsuite/gcc.dg/lto/pr94271_0.c
@@ -1,4 +1,5 @@
 /* PR lto/94271 */
+/* { dg-skip-if "" { ! { i?86-*-* x86_64-*-* } } } */
 /* { dg-lto-do link } */
 
 int a;

[PATCH] tree: Fix -fcompare-debug issues due to protected_set_expr_location [PR94323]

[Jakub Jelinek via Gcc-patches]

Hi!

The following testcase FAILs since recently when the C++ FE started calling
protected_set_expr_location more often.
With -g, it is called on a STATEMENT_LIST that contains a DEBUG_BEGIN_STMT
and CLEANUP_POINT_EXPR, and as STATEMENT_LISTs have !CAN_HAVE_LOCATION_P,
nothing is set.  Without -g, it is called instead on the CLEANUP_POINT_EXPR
directly and changes its location.

The following patch recurses on the single non-DEBUG_BEGIN_STMT statement
of a STATEMENT_LIST if any to make the two behave the same.

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

2020-03-26  Jakub Jelinek  

PR debug/94323
* tree.c (protected_set_expr_location): Recurse on STATEMENT_LIST
that contains exactly one non-DEBUG_BEGIN_STMT statement.

* g++.dg/debug/pr94323.C: New test.

--- gcc/tree.c.jj   2020-03-23 19:46:45.552448327 +0100
+++ gcc/tree.c  2020-03-25 17:22:12.438904778 +0100
@@ -5146,6 +5146,33 @@ protected_set_expr_location (tree t, loc
 {
   if (CAN_HAVE_LOCATION_P (t))
 SET_EXPR_LOCATION (t, loc);
+  else if (t && TREE_CODE (t) == STATEMENT_LIST)
+{
+  /* With -gstatement-frontiers we could have a STATEMENT_LIST with
+DEBUG_BEGIN_STMT(s) and only a single other stmt, which with
+-g wouldn't be present and we'd have that single other stmt
+directly instead.  */
+  struct tree_statement_list_node *n = STATEMENT_LIST_HEAD (t);
+  if (!n)
+   return;
+  while (TREE_CODE (n->stmt) == DEBUG_BEGIN_STMT)
+   {
+ n = n->next;
+ if (!n)
+   return;
+   }
+  tree t2 = n->stmt;
+  do
+   {
+ n = n->next;
+ if (!n)
+   {
+ protected_set_expr_location (t2, loc);
+ return;
+   }
+   }
+  while (TREE_CODE (n->stmt) == DEBUG_BEGIN_STMT);
+}
 }
 
 /* Data used when collecting DECLs and TYPEs for language data removal.  */
--- gcc/testsuite/g++.dg/debug/pr94323.C.jj 2020-03-25 17:17:49.857819078 
+0100
+++ gcc/testsuite/g++.dg/debug/pr94323.C2020-03-25 17:17:17.533300951 
+0100
@@ -0,0 +1,13 @@
+// PR debug/94323
+// { dg-do compile }
+// { dg-options "-O2 -fcompare-debug" }
+
+volatile int a;
+
+void
+foo ()
+{
+  ({
+ a;
+   });
+}

Jakub

[PATCH] gimplify: Fix -fcompare-debug differences caused by gimplify_body [PR94281]

[Jakub Jelinek via Gcc-patches]

Hi!

The following testcase FAILs, because gimplify_body adds a GIMPLE_NOP only
when there are no statements in the function and with -g there is a
DEBUG_BEGIN_STMT, so it doesn't add it and due to -fno-tree-dce that never
gets removed afterwards.  Similarly, if the body seq after gimplification
contains some DEBUG_BEGIN_STMTs plus a single gbind, then we could behave
differently between -g0 and -g, by using that gbind as the body in the -g0
case and not in the -g case.
This patch fixes that by ignoring DEBUG_BEGIN_STMTs (other debug stmts can't
appear at this point yet thankfully) during decisions and if we pick the
single gbind and there are DEBUG_BEGIN_STMTs next to it, it moves them into
the gbind.
While debugging this, I found also a bug in the gimple_seq_last_nondebug_stmt
function, for a seq that has a single non-DEBUG_BEGIN_STMT statement
followed by one or more DEBUG_BEGIN_STMTs it would return NULL rather than
the first statement.

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

2020-03-26  Jakub Jelinek  

PR debug/94281
* gimple.h (gimple_seq_first_nondebug_stmt): New function.
(gimple_seq_last_nondebug_stmt): Don't return NULL if seq contains
a single non-debug stmt followed by one or more debug stmts.
* gimplify.c (gimplify_body): Use gimple_seq_first_nondebug_stmt
instead of gimple_seq_first_stmt, use gimple_seq_first_nondebug_stmt
and gimple_seq_last_nondebug_stmt instead of gimple_seq_first and
gimple_seq_last to check if outer_stmt gbind could be reused and
if yes and it is surrounded by any debug stmts, move them into the
gbind body.

* g++.dg/debug/pr94281.C: New test.

--- gcc/gimple.h.jj 2020-03-25 14:34:07.696199561 +0100
+++ gcc/gimple.h2020-03-25 15:26:38.045052864 +0100
@@ -4728,6 +4728,18 @@ is_gimple_debug (const gimple *gs)
 }
 
 
+/* Return the first nondebug statement in GIMPLE sequence S.  */
+
+static inline gimple *
+gimple_seq_first_nondebug_stmt (gimple_seq s)
+{
+  gimple_seq_node n = gimple_seq_first (s);
+  while (n && is_gimple_debug (n))
+n = n->next;
+  return n;
+}
+
+
 /* Return the last nondebug statement in GIMPLE sequence S.  */
 
 static inline gimple *
@@ -4737,7 +4749,7 @@ gimple_seq_last_nondebug_stmt (gimple_se
   for (n = gimple_seq_last (s);
n && is_gimple_debug (n);
n = n->prev)
-if (n->prev == s)
+if (n == s)
   return NULL;
   return n;
 }
--- gcc/gimplify.c.jj   2020-03-25 14:34:07.722199170 +0100
+++ gcc/gimplify.c  2020-03-25 14:41:46.447348434 +0100
@@ -14849,7 +14849,7 @@ gimplify_body (tree fndecl, bool do_parm
   /* Gimplify the function's body.  */
   seq = NULL;
   gimplify_stmt (_SAVED_TREE (fndecl), );
-  outer_stmt = gimple_seq_first_stmt (seq);
+  outer_stmt = gimple_seq_first_nondebug_stmt (seq);
   if (!outer_stmt)
 {
   outer_stmt = gimple_build_nop ();
@@ -14859,8 +14859,37 @@ gimplify_body (tree fndecl, bool do_parm
   /* The body must contain exactly one statement, a GIMPLE_BIND.  If this is
  not the case, wrap everything in a GIMPLE_BIND to make it so.  */
   if (gimple_code (outer_stmt) == GIMPLE_BIND
-  && gimple_seq_first (seq) == gimple_seq_last (seq))
-outer_bind = as_a  (outer_stmt);
+  && (gimple_seq_first_nondebug_stmt (seq)
+ == gimple_seq_last_nondebug_stmt (seq)))
+{
+  outer_bind = as_a  (outer_stmt);
+  if (gimple_seq_first_stmt (seq) != outer_stmt
+ || gimple_seq_last_stmt (seq) != outer_stmt)
+   {
+ /* If there are debug stmts before or after outer_stmt, move them
+inside of outer_bind body.  */
+ gimple_stmt_iterator gsi = gsi_for_stmt (outer_stmt, );
+ gimple_seq second_seq = NULL;
+ if (gimple_seq_first_stmt (seq) != outer_stmt
+ && gimple_seq_last_stmt (seq) != outer_stmt)
+   {
+ second_seq = gsi_split_seq_after (gsi);
+ gsi_remove (, false);
+   }
+ else if (gimple_seq_first_stmt (seq) != outer_stmt)
+   gsi_remove (, false);
+ else
+   {
+ gsi_remove (, false);
+ second_seq = seq;
+ seq = NULL;
+   }
+ gimple_seq_add_seq_without_update (,
+gimple_bind_body (outer_bind));
+ gimple_seq_add_seq_without_update (, second_seq);
+ gimple_bind_set_body (outer_bind, seq);
+   }
+}
   else
 outer_bind = gimple_build_bind (NULL_TREE, seq, NULL);
 
--- gcc/testsuite/g++.dg/debug/pr94281.C.jj 2020-03-25 14:47:11.241498952 
+0100
+++ gcc/testsuite/g++.dg/debug/pr94281.C2020-03-25 14:45:32.637971196 
+0100
@@ -0,0 +1,11 @@
+// PR debug/94281
+// { dg-do compile }
+// { dg-options "-O1 -fno-tree-dce -fipa-icf -fno-tree-forwprop 
-fcompare-debug" }
+
+void fn1()
+{
+}
+void fn2()
+{
+  ;
+}

Jakub

[committed] wwwdocs: Switch link to libgfortran.h from ViewCVS to Git.

[Gerald Pfeifer]

This is the second such case in the tree.  Thanks to Frank Ch. Eigler 
for the updated link.

Pushed.

Gerald
---
 htdocs/gcc-4.8/changes.html | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/htdocs/gcc-4.8/changes.html b/htdocs/gcc-4.8/changes.html
index 83f7da6c..60cf4d42 100644
--- a/htdocs/gcc-4.8/changes.html
+++ b/htdocs/gcc-4.8/changes.html
@@ -435,8 +435,7 @@ int i = A().f();  // error, f() requires an lvalue object
   (dimension(..)) has been added. Note that currently
   gfortran's own array descriptor is used, which is different from the
   one defined in TS29113, see https://gcc.gnu.org/viewcvs/trunk/libgfortran/libgfortran.h?content-type=text%2Fplainview=co;>
-  gfortran's header file or use the https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libgfortran/libgfortran.h;>gfortran's
 header file or use the http://chasm-interop.sourceforge.net/;>Chasm Language
   Interoperability Tools.
 
-- 
2.25.1

RE: [RFC] Should widening_mul should consider block frequency?

[Yangfei (Felix)]

> -Original Message-
> From: Richard Biener [mailto:richard.guent...@gmail.com]
> Sent: Thursday, March 26, 2020 3:37 PM
> To: Yangfei (Felix) 
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [RFC] Should widening_mul should consider block frequency?
> 
> >
> > That's a good point.  I have attached the v2 patch.
> > Also did a spec2017 test on aarch64, no obvious impact witnessed with this.
> > Can you sponsor this patch please?  My networking does not work well
> > and I am having some trouble pushing it : - (
> 
> Pushed.  For the future can you please attach patches suitable for git am?

Sure, will do.  Thanks for the help : - )  

Felix

Re: [RFC] Should widening_mul should consider block frequency?

[Richard Biener via Gcc-patches]

On Thu, Mar 26, 2020 at 2:06 AM Yangfei (Felix)  wrote:
>
> Hi!
>
> > -Original Message-
> > From: Richard Biener [mailto:richard.guent...@gmail.com]
> > Sent: Tuesday, March 24, 2020 10:14 PM
> > To: Yangfei (Felix) 
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: [RFC] Should widening_mul should consider block frequency?
> >
> > > > As written in the PR I'd follow fma generation and restrict defs to the 
> > > > same
> > BB.
> > >
> > > Thanks for the suggestion.  That should be more consistent.
> > > Attached please find the adapted patch.
> > > Bootstrap and tested on both x86_64 and aarch64 Linux platform.
> >
> > OK with moving the BB check before the is_widening_mult_p call since it's 
> > way
> > cheaper.
>
> That's a good point.  I have attached the v2 patch.
> Also did a spec2017 test on aarch64, no obvious impact witnessed with this.
> Can you sponsor this patch please?  My networking does not work well and I am 
> having some trouble pushing it : - (

Pushed.  For the future can you please attach patches suitable for git am?

Thanks,
Richard.

> git commit msg:
>
> widening_mul: restrict ops to be defined in the same basic-block when 
> convert plusminus to widen
>
> In the testcase for PR94269, widening_mul moves two multiply instructions 
> from outside the loop to inside
> the loop, merging with two add instructions separately.  This increases 
> the cost of the loop.  Like FMA detection
> in the same pass, simply restrict ops to be defined in the same 
> basic-block to avoid possibly moving multiply
> to a different block with a higher execution frequency.
>
> 2020-03-26  Felix Yang  
>
> gcc/
> PR tree-optimization/94269
> * tree-ssa-math-opts.c (convert_plusminus_to_widen): Restrict this
> operation to single basic block.
>
> gcc/testsuite/
> PR tree-optimization/94269
> * gcc.dg/pr94269.c: New test.
>
> change log:
>
> gcc:
> +2020-03-26  Felix Yang  
> +
> +   PR tree-optimization/94269
> +   * tree-ssa-math-opts.c (convert_plusminus_to_widen): Restrict this
> +   operation to single basic block.
>
> gcc/testsuite:
> +2020-03-26  Felix Yang  
> +
> +   PR tree-optimization/94269
> +   * gcc.dg/pr94269.c: New test.
> +

Re: [PING PATCH coroutines] Do not strip cleanup_point when promote temporaries out of current stmt

[Bin.Cheng via Gcc-patches]

On Mon, Mar 16, 2020 at 5:34 PM Bin.Cheng  wrote:
>
> On Wed, Mar 11, 2020 at 5:07 PM Iain Sandoe  wrote:
> >
> > Bin.Cheng  wrote:
> >
> > > On Thu, Mar 5, 2020 at 10:18 PM Iain Sandoe  wrote:
> >
> > >> With current trunk + Bin’s two approved patches.
> > >>
> > >> I see no change in the testcase (lambda-09-capture-object.C) before / 
> > >> after
> > >> the patch
> > >>  (it fails for me at -O0 only - in both cases).
> > >
> >
> > > I tried exactly what you did, however, the result is different.
> >
> > I am a bit concerned that we get different results - yesterday I retested 
> > this on:
> >   Linux : x86_64-linux (cfarm gcc122)
> >   Darwin (x86_64-darwin16),
> >  with the same results on both platforms.
> >
> > 1) I applied the two testcases (note I have renamed 
> > lambda-09-capture-object.C => lambda-11-capture-object.C so that the test 
> > numbers are sequential).  However, I have not changed the test in any other 
> > way.
> >
> > Native configuration is x86_64-pc-linux-gnu
> >
> > === g++ tests ===
> >
> > Schedule of variations:
> > unix/-m32
> > unix/-m64
> >
> > Running target unix/-m32
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for 
> > target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for 
> > target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp 
> > as tool-and-target-specific interface file.
> > Running 
> > /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp
> >  ...
> > FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
> > FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal 
> > compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess 
> > errors)
> > Running 
> > /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp
> >  ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal 
> > compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for 
> > excess errors)
> >
> > Running target unix/-m64
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for 
> > target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for 
> > target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp 
> > as tool-and-target-specific interface file.
> > Running 
> > /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp
> >  ...
> > FAIL: g++.dg/coroutines/co-await-syntax-10.C (internal compiler error)
> > FAIL: g++.dg/coroutines/co-await-syntax-10.C (test for excess errors)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal 
> > compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess 
> > errors)
> > Running 
> > /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp
> >  ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal 
> > compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for 
> > excess errors)
> >
> > ^^ so, this shows that both tests fail (co-await-syntax-10.C is expected)
> >
> > 2) I apply Bin’s patch (Pickup more CO_AWAIT_EXPR expanding cases) (which 
> > is approved)
> >
> > Native configuration is x86_64-pc-linux-gnu
> >
> > === g++ tests ===
> >
> > Schedule of variations:
> > unix/-m32
> > unix/-m64
> >
> > Running target unix/-m32
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for 
> > target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for 
> > target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp 
> > as tool-and-target-specific interface file.
> > Running 
> > /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/coroutines.exp
> >  ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (internal 
> > compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C (test for excess 
> > errors)
> > Running 
> > /home/iains/gcc-master/src-patched/gcc/testsuite/g++.dg/coroutines/torture/coro-torture.exp
> >  ...
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (internal 
> > compiler error)
> > FAIL: g++.dg/coroutines/torture/lambda-11-capture-object.C   -O0  (test for 
> > excess errors)
> >
> > Running target unix/-m64
> > Using /usr/share/dejagnu/baseboards/unix.exp as board description file for 
> > target.
> > Using /usr/share/dejagnu/config/unix.exp as generic interface file for 
> > target.
> > Using /home/iains/gcc-master/src-patched/gcc/testsuite/config/default.exp 
> > as tool-and-target-specific interface file.
> > Running 
> >