Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-26 23:16 GMT+02:00 Jakub Jelinek :
> On Tue, Jun 26, 2018 at 11:12:40PM +0200, Thomas Koenig wrote:
>> Hi Janus,
>>
>> with your patch, we would only warn about
>>
>> var .and. func()
>>
>> and not about
>>
>> func() .and. var.
>>
>> AFAIK, TRUTH_AND_EXPR does not guarantee that func() will
>> always be exectued, or if it does, I have not seen the
>> documentation; it just happens to match what we currently
>> see (which may be due to missed optimizatins in the back end,
>> or the lack of test cases exposing this).
>
> If you are talking about what the middle-end does, TRUTH_AND_EXPR
> always evaluates side-effects in both operands, while for
> TRUTH_ANDIF_EXPR it evaluates the second operand only if the first operand
> is not false.

thanks for the comments. Looking into gfc_conv_expr_op, I see:

case INTRINSIC_AND:
  code = TRUTH_ANDIF_EXPR;
  lop = 1;
  break;

case INTRINSIC_OR:
  code = TRUTH_ORIF_EXPR;
  lop = 1;
  break;

That seems to indicate that Fortran's logical .AND. operator is
translated into TRUTH_ANDIF_EXPR, right? Also the empirically observed
behavior matches Jakub's description of TRUTH_ANDIF_EXPR, where the
first operator is always evaluated and the second one only on demand.
Therefore I think my patch is correct in warning about an impure
function only if it occurs in the second operand of an and/or
operator.

Cheers,
Janus

Re: [PATCH 3/3][POPCOUNT] Remove unnecessary if condition in phiopt

[Kugan Vivekanandarajah]

Hi Richard,

Thanks for the review,

On 25 June 2018 at 20:20, Richard Biener  wrote:
> On Fri, Jun 22, 2018 at 11:16 AM Kugan Vivekanandarajah
>  wrote:
>>
>> gcc/ChangeLog:
>
> @@ -1516,6 +1521,114 @@ minmax_replacement (basic_block cond_bb,
> basic_block middle_bb,
>
>return true;
>  }
> +/* Convert
> +
> +   
> +   if (b_4(D) != 0)
> +   goto 
>
> vertical space before the comment.
Done.

>
> + edge e0 ATTRIBUTE_UNUSED, edge e1
> ATTRIBUTE_UNUSED,
>
> why pass them if they are unused?
Removed.

>
> +  if (stmt_count != 2)
> +return false;
>
> so what about the case when there is no conversion?
Done.

>
> +  /* Check that we have a popcount builtin.  */
> +  if (!is_gimple_call (popcount)
> +  || !gimple_call_builtin_p (popcount, BUILT_IN_NORMAL))
> +return false;
> +  tree fndecl = gimple_call_fndecl (popcount);
> +  if ((DECL_FUNCTION_CODE (fndecl) != BUILT_IN_POPCOUNT)
> +  && (DECL_FUNCTION_CODE (fndecl) != BUILT_IN_POPCOUNTL)
> +  && (DECL_FUNCTION_CODE (fndecl) != BUILT_IN_POPCOUNTLL))
> +return false;
>
> look at popcount handling in tree-vrp.c how to properly also handle
> IFN_POPCOUNT.
> (CASE_CFN_POPCOUNT)
Done.
>
> +  /* Cond_bb has a check for b_4 != 0 before calling the popcount
> + builtin.  */
> +  if (gimple_code (cond) != GIMPLE_COND
> +  || gimple_cond_code (cond) != NE_EXPR
> +  || TREE_CODE (gimple_cond_lhs (cond)) != SSA_NAME
> +  || rhs != gimple_cond_lhs (cond))
> +return false;
>
> The check for SSA_NAME is redundant.
> You fail to check that gimple_cond_rhs is zero.
Done.

>
> +  /* Remove the popcount builtin and cast stmt.  */
> +  gsi = gsi_for_stmt (popcount);
> +  gsi_remove (, true);
> +  gsi = gsi_for_stmt (cast);
> +  gsi_remove (, true);
> +
> +  /* And insert the popcount builtin and cast stmt before the cond_bb.  */
> +  gsi = gsi_last_bb (cond_bb);
> +  gsi_insert_before (, popcount, GSI_NEW_STMT);
> +  gsi_insert_before (, cast, GSI_NEW_STMT);
>
> use gsi_move_before ().  You need to reset flow sensitive info on the
> LHS of the popcount call as well as on the LHS of the cast.
Done.

>
> You fail to check the PHI operand on the false edge.  Consider
>
>  if (b != 0)
>res = __builtin_popcount (b);
>  else
>res = 1;
>
> You fail to check the PHI operand on the true edge.  Consider
>
>  res = 0;
>  if (b != 0)
>{
>   __builtin_popcount (b);
>   res = 2;
>}
>
> and using -fno-tree-dce and whatever you need to keep the
> popcount call in the IL.  A gimple testcase for phiopt will do.
>
> Your testcase relies on popcount detection.  Please write it
> using __builtin_popcount instead.  Write one with a cast and
> one without.
Added the testcases.

Is this OK now.

Thanks,
Kugan
>
> Thanks,
> Richard.
>
>
>> 2018-06-22  Kugan Vivekanandarajah  
>>
>> * tree-ssa-phiopt.c (cond_removal_in_popcount_pattern): New.
>> (tree_ssa_phiopt_worker): Call cond_removal_in_popcount_pattern.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-06-22  Kugan Vivekanandarajah  
>>
>> * gcc.dg/tree-ssa/popcount3.c: New test.
From 5b776871d99161653dbb7a7fc353268ab3be6880 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah 
Date: Fri, 22 Jun 2018 14:16:21 +1000
Subject: [PATCH 3/3] improve phiopt for builtin popcount

Change-Id: Iab8861cc15590cc2603be9967ca9477a223c90a8
---
 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-16.c |  12 +++
 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-17.c |  12 +++
 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-18.c |  14 
 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-19.c |  15 
 gcc/testsuite/gcc.dg/tree-ssa/popcount3.c  |  15 
 gcc/tree-ssa-phiopt.c  | 127 +
 6 files changed, 195 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-16.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-17.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-18.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-19.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/popcount3.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-16.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-16.c
new file mode 100644
index 000..e7a2711
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-16.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+int foo (int a)
+{
+  int c = 0;
+  if (a != 0)
+c = __builtin_popcount (a);
+  return c;
+}
+
+/* { dg-final { scan-tree-dump-not "if" "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-17.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-17.c
new file mode 100644
index 000..25ba096
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-17.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+int foo (unsigned int a)
+{
+  int c = 0;
+  if (a != 0)
+c = __builtin_popcount (a);
+  return c;
+}
+
+/* { dg-final { scan-tree-dump-not "if" "optimized" } } */
diff 

Re: [PATCH 2/3][POPCOUNT] Check if zero check is done before entering the loop

[Kugan Vivekanandarajah]

Hi Richard,

Thanks for the review.

On 25 June 2018 at 20:02, Richard Biener  wrote:
> On Fri, Jun 22, 2018 at 11:14 AM Kugan Vivekanandarajah
>  wrote:
>>
>> gcc/ChangeLog:
>
> The canonical way is calling simplify_using_initial_conditions on the
> may_be_zero condition.
>
> Richard.
>
>> 2018-06-22  Kugan Vivekanandarajah  
>>
>> * tree-ssa-loop-niter.c (number_of_iterations_popcount): If popcount
>> argument is checked for zero before entering loop, avoid checking again.
Do you like the attached patch which does this.

Thanks,
Kugan
From 78cb0ea3d058f1d1db73f259825b8bb07eb1ca30 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah 
Date: Fri, 22 Jun 2018 14:11:28 +1000
Subject: [PATCH 2/3] in niter dont check for zero when it is alrealy checked

Change-Id: Ie94a35a1a3c2d8bdffd3dc54a94684c032efc7e0
---
 gcc/tree-ssa-loop-niter.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index f5ffc0f..be0cff5 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -2596,10 +2596,15 @@ number_of_iterations_popcount (loop_p loop, edge exit,
 
   niter->niter = iter;
   niter->assumptions = boolean_true_node;
+
   if (adjust)
-niter->may_be_zero = fold_build2 (EQ_EXPR, boolean_type_node, src,
+{
+  tree may_be_zero = fold_build2 (EQ_EXPR, boolean_type_node, src,
   build_zero_cst
   (TREE_TYPE (src)));
+  niter->may_be_zero =
+	simplify_using_initial_conditions (loop, may_be_zero);
+}
   else
 niter->may_be_zero = boolean_false_node;
 
-- 
2.7.4

Re: [PATCH 1/3][POPCOUNT] Handle COND_EXPR in expression_expensive_p

[Kugan Vivekanandarajah]

Hi Richard,

Thanks for the review.

On 25 June 2018 at 20:01, Richard Biener  wrote:
> On Fri, Jun 22, 2018 at 11:13 AM Kugan Vivekanandarajah
>  wrote:
>>
>> [PATCH 1/3][POPCOUNT] Handle COND_EXPR in expression_expensive_p
>
> This says that COND_EXPR itself isn't expensive.  I think we should
> constrain that a bit.
> I think a good default would be to only allow a single COND_EXPR which
> you can achieve
> by adding a bool in_cond_expr_p = false argument to the function, pass
> in_cond_expr_p
> down and pass true down from the COND_EXPR handling itself.
>
> I'm not sure if we should require either COND_EXPR arm (operand 1 or
> 2) to be constant
> or !EXPR_P (then multiple COND_EXPRs might be OK).
>
> The main idea is to avoid evaluating many expressions but only
> choosing one in the end.
>
> The simplest patch achieving that is sth like
>
> +  if (code == COND_EXPR)
> +return (expression_expensive_p (TREE_OPERAND (expr, 0))
>   || (EXPR_P (TREE_OPERAND (expr, 1)) && EXPR_P
> (TREE_OPERAND (expr, 2)))
> +   || expression_expensive_p (TREE_OPERAND (expr, 1))
> +   || expression_expensive_p (TREE_OPERAND (expr, 2)));
>
> OK with that change.

Is || (EXPR_P (TREE_OPERAND (expr, 1)) || EXPR_P (TREE_OPERAND (expr,
2))) slightly better ?
Attaching  with the change. Is this OK?


Because, for pr81661.c, we now allow as not expensive

unit-size 
align:32 warn_if_not_align:0 symtab:0 alias-set 1
canonical-type 0x769455e8 precision:32 min  max 
pointer_to_this >

arg:0 

arg:0 
visited
def_stmt a.1_10 = a;
version:10>
arg:1 >
arg:1 

arg:0 

arg:0 

arg:0 
arg:0  arg:1 >
arg:1 
visited
def_stmt c.2_11 = c;
version:11>>
arg:1 
visited
def_stmt b.3_13 = b;
version:13>>
arg:1 

arg:0 

arg:0 

arg:0 

arg:0 
arg:0  arg:1
>>
arg:1 
arg:0 
arg:2 >>

Which also leads to an ICE in gimplify_modify_expr. I think this is a
latent issue and I am trying to find the source

the expr in gimple_modify_expr is

unit-size 
align:32 warn_if_not_align:0 symtab:0 alias-set 1
canonical-type 0x769455e8 precision:32 min  max 
pointer_to_this >
side-effects
arg:0 
used ignored SI (null):0:0 size  unit-size 
align:32 warn_if_not_align:0 context >
arg:1 

arg:0 

arg:0 

arg:0 

arg:0 

arg:0 
arg:0  arg:1
>
arg:1 
visited
def_stmt c.2_11 = c;
version:11>>>
arg:1 

arg:0 
visited
def_stmt b.3_13 = b;
version:13>>

and the *to_p is not SSA_NAME and VAR_DECL.

Thanks,
Kugan



>
> Richard.
>
>> gcc/ChangeLog:
>>
>> 2018-06-22  Kugan Vivekanandarajah  
>>
>> * tree-scalar-evolution.c (expression_expensive_p): Handle COND_EXPR.
From 8f59f05ad21ac218834547593a7f308b4f837b1e Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah 
Date: Fri, 22 Jun 2018 14:10:26 +1000
Subject: [PATCH 1/3] generate popcount when checked for zero

Change-Id: Iff93a1bd58d12e5e6951bc15ebf5db2ec2c85c2e
---
 gcc/tree-scalar-evolution.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/gcc/tree-scalar-evolution.c b/gcc/tree-scalar-evolution.c
index 4b0ec02..d992582 100644
--- a/gcc/tree-scalar-evolution.c
+++ b/gcc/tree-scalar-evolution.c
@@ -3508,6 +3508,13 @@ expression_expensive_p (tree expr)
   return false;
 }
 
+  if (code == COND_EXPR)
+return (expression_expensive_p (TREE_OPERAND (expr, 0))
+	|| (EXPR_P (TREE_OPERAND (expr, 1))
+		|| EXPR_P (TREE_OPERAND (expr, 2)))
+	|| expression_expensive_p (TREE_OPERAND (expr, 1))
+	|| expression_expensive_p (TREE_OPERAND (expr, 2)));
+
   switch (TREE_CODE_CLASS (code))
 {
 case tcc_binary:
-- 
2.7.4

C++ PATCH for c++/86320, memory-hog with std::array of pair

[Jason Merrill]

In this PR, we have a large std::array of pairs.  Since the C array is
wrapped in a class we don't go to build_vec_init, so we end up with
digest_init wanting to build up the element initializer for each
element of the array.

In the more general case, like 80272, we have a data structure
problem: we don't currently have a good way of expressing the same
dynamic initialization of many elements within a CONSTRUCTOR.
RANGE_EXPR probably ought to work, but will need more work at
genericize or gimplify time.

But in this case, the initialization for each element reduces to
constant 0, so we don't even need to add anything to the CONSTRUCTOR.
We just need to realize that if the initializer for one element is 0,
the others will be as well, and we don't need to iterate over the
whole array.

For the trunk, I also use a RANGE_EXPR to handle constant
initialization by a value other than 0.

#include 
#include 

void foo ()
{
  std::array, 1024 * 1024> arr {};
}

Tested x86_64-pc-linux-gnu, applying to trunk and 8.
commit 45b75c343a5ab44a039923febbfd6d9c11ad1621
Author: Jason Merrill 
Date:   Tue Jun 26 14:58:58 2018 -0400

PR c++/86320 - memory-hog with std::array of pair

* typeck2.c (process_init_constructor_array): Only compute a
constant initializer once.

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 43e236de41c..91aa5a62856 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1365,8 +1365,22 @@ process_init_constructor_array (tree type, tree init, int nested,
 	if (next)
 	  {
 	flags |= picflag_from_initializer (next);
-	CONSTRUCTOR_APPEND_ELT (v, size_int (i), next);
+	if (len > i+1
+		&& (initializer_constant_valid_p (next, TREE_TYPE (next))
+		== null_pointer_node))
+	  {
+		tree range = build2 (RANGE_EXPR, size_type_node,
+ build_int_cst (size_type_node, i),
+ build_int_cst (size_type_node, len - 1));
+		CONSTRUCTOR_APPEND_ELT (v, range, next);
+		break;
+	  }
+	else
+	  CONSTRUCTOR_APPEND_ELT (v, size_int (i), next);
 	  }
+	else
+	  /* Don't bother checking all the other elements.  */
+	  break;
   }
 
   CONSTRUCTOR_ELTS (init) = v;
commit 484d1a56ac83f53bf0a6064d9762acceb9d2f158
Author: Jason Merrill 
Date:   Tue Jun 26 14:58:58 2018 -0400

PR c++/86320 - memory-hog with std::array of pair

* typeck2.c (process_init_constructor_array): If zero-initialization
is fine for one element, we're done.

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 43e236de41c..5f738f038f0 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1367,6 +1367,9 @@ process_init_constructor_array (tree type, tree init, int nested,
 	flags |= picflag_from_initializer (next);
 	CONSTRUCTOR_APPEND_ELT (v, size_int (i), next);
 	  }
+	else
+	  /* Don't bother checking all the other elements.  */
+	  break;
   }
 
   CONSTRUCTOR_ELTS (init) = v;

C++ PATCH for c++/80290, memory-hog with nested std::pair

[Jason Merrill]

When the std::pair constructors got more complex to handle, it
aggravated a preexisting algorithmic problem in template overload
resolution:

As part of template argument deduction in a call, once we've deduced
all the template arguments we can but before we substitute them to
form an actual declaration, for any function parameters that don't
involve template parameters we need to check that it's possible to
convert the argument to the parameter type (wg21.link/cwg1391).

As a result, we end up calculating the conversion twice: once here,
and then again in add_function_candidate as part of normal overload
resolution.  Normally this isn't a big deal, but when the argument is
a multiply-nested initializer list, doubling the conversion processing
at each level leads to combinatorial explosion.

The patch for GCC 8 just disables the deduction-time check for a
multiply nested initializer list.  This is a small correctness
regression, but is unlikely to affect real code.

The patch for trunk avoids the duplication by remembering the
conversion we calculate at deduction time and then reusing it in
overload resolution rather than calculating it again.

Tested x86_64-pc-linux-gnu, applying to trunk and 8 (respectively).
commit 10b1142610715b75d526c3313897740033694ac9
Author: Jason Merrill 
Date:   Tue Jun 26 09:56:53 2018 -0400

PR c++/80290 - memory-hog with std::pair.

* pt.c (fn_type_unification): Add convs parameter.
(check_non_deducible_conversion): Remember conversion.
(check_non_deducible_conversions): New.  Do checks here.
(type_unification_real): Not here.  Remove flags parm.
* call.c (add_function_candidate): Make convs a parameter.
Don't recalculate the conversion if it's already set.
(add_template_candidate_real): Allocate convs here.
(good_conversion, conv_flags): New.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index aa0e696972a..209c1fd2f0e 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -192,7 +192,7 @@ static struct z_candidate *add_conv_candidate
 	 tree, tsubst_flags_t);
 static struct z_candidate *add_function_candidate
 	(struct z_candidate **, tree, tree, tree, const vec *, tree,
-	 tree, int, tsubst_flags_t);
+	 tree, int, conversion**, tsubst_flags_t);
 static conversion *implicit_conversion (tree, tree, tree, bool, int,
 	tsubst_flags_t);
 static conversion *reference_binding (tree, tree, tree, bool, int,
@@ -1929,6 +1929,23 @@ implicit_conversion (tree to, tree from, tree expr, bool c_cast_p,
   return NULL;
 }
 
+/* Like implicit_conversion, but return NULL if the conversion is bad.
+
+   This is not static so that check_non_deducible_conversion can call it within
+   add_template_candidate_real as part of overload resolution; it should not be
+   called outside of overload resolution.  */
+
+conversion *
+good_conversion (tree to, tree from, tree expr,
+		 int flags, tsubst_flags_t complain)
+{
+  conversion *c = implicit_conversion (to, from, expr, /*cast*/false,
+   flags, complain);
+  if (c && c->bad_p)
+c = NULL;
+  return c;
+}
+
 /* Add a new entry to the list of candidates.  Used by the add_*_candidate
functions.  ARGS will not be changed until a single candidate is
selected.  */
@@ -1975,6 +1992,37 @@ remaining_arguments (tree arg)
   return n;
 }
 
+/* [over.match.copy]: When initializing a temporary object (12.2) to be bound
+   to the first parameter of a constructor where the parameter is of type
+   "reference to possibly cv-qualified T" and the constructor is called with a
+   single argument in the context of direct-initialization of an object of type
+   "cv2 T", explicit conversion functions are also considered.
+
+   So set LOOKUP_COPY_PARM to let reference_binding know that
+   it's being called in that context.  */
+
+int
+conv_flags (int i, int nargs, tree fn, tree arg, int flags)
+{
+  int lflags = flags;
+  tree t;
+  if (i == 0 && nargs == 1 && DECL_CONSTRUCTOR_P (fn)
+  && (t = FUNCTION_FIRST_USER_PARMTYPE (fn))
+  && (same_type_ignoring_top_level_qualifiers_p
+	  (non_reference (TREE_VALUE (t)), DECL_CONTEXT (fn
+{
+  if (!(flags & LOOKUP_ONLYCONVERTING))
+	lflags |= LOOKUP_COPY_PARM;
+  if ((flags & LOOKUP_LIST_INIT_CTOR)
+	  && BRACE_ENCLOSED_INITIALIZER_P (arg))
+	lflags |= LOOKUP_NO_CONVERSION;
+}
+  else
+lflags |= LOOKUP_ONLYCONVERTING;
+
+  return lflags;
+}
+
 /* Create an overload candidate for the function or method FN called
with the argument list FIRST_ARG/ARGS and add it to CANDIDATES.
FLAGS is passed on to implicit_conversion.
@@ -1989,11 +2037,11 @@ add_function_candidate (struct z_candidate **candidates,
 			tree fn, tree ctype, tree first_arg,
 			const vec *args, tree access_path,
 			tree conversion_path, int flags,
+			conversion **convs,
 			tsubst_flags_t complain)
 {
   tree parmlist = TYPE_ARG_TYPES (TREE_TYPE (fn));
   int i, len;
-  conversion **convs;
   tree 

[RFC PATCH] diagnose built-in declarations without prototype (PR 83656)

[Martin Sebor]

With the exception of built-ins with the ellipsis (like sprintf),
GCC silently accepts declarations of built-in functions without
prototypes as well as calls to such functions with any numbers
or types of arguments, compatible or otherwise.  Calls with
arguments whose number and types match exactly those of
the built-in are considered by the middle-end for optimization.
Other calls (compatible or not, irrespective of whether their
number matches the number expected by the function) are then
made to the library functions.

Attached is a small fix to -Wbuiltin-declaration-mismatch to
have it diagnose built-in declarations without a prototype.
The warning is enabled by default so it causes a fair number
of tests to fail because of declarations like 'void abort();'
The breakdown of the built-ins behind the test failures is
below.

Before I take the time to clean up the test suite let me post
what I have in case going this route is not acceptable.  As
an alternative, I could try to avoid some of these warnings,
e.g., by diagnosing incompatible calls instead but I think
it's even less worthwhile for built-ins than trying to do
it for ordinary functions with -Wstrict-prototypes.  There
is, in my view, no justification today for standard functions
to be declared without a prototype.  (I could also make
the warning depend on language mode and -Wpedantic if that
were more preferable.)

Martin

About 115 tests fail due to incompatible declarations of
the built-in functions below (the number shows the number
of warnings for each functions):

428   abort
 58   exit
 36   memcpy
 17   memmove
 15   realloc
 14   cabs
  5   strncpy
  4   strcmp
  3   alloca
  2   rindex
  1   aligned_alloc
PR c/83656 - missing -Wbuiltin-declaration-mismatch on declaration without prototype

gcc/c/ChangeLog:

	PR c/83656
	* c-decl.c (diagnose_mismatched_decls): Diagnose declarations of
	built-in functions without a prototype.

gcc/testsuite/ChangeLog:

	PR c/83656
	* gcc.dg/Wbuiltin-declaration-mismatch-5.c: New test.

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 6c9e667..7008176 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -2088,15 +2088,29 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
 	 can't validate the argument list) the built-in definition is
 	 overridden, but optionally warn this was a bad choice of name.  */
   if (DECL_BUILT_IN (olddecl)
-	  && !C_DECL_DECLARED_BUILTIN (olddecl)
-	  && (!TREE_PUBLIC (newdecl)
-	  || (DECL_INITIAL (newdecl)
-		  && !prototype_p (TREE_TYPE (newdecl)
+	  && !C_DECL_DECLARED_BUILTIN (olddecl))
 	{
-	  warning (OPT_Wshadow, "declaration of %q+D shadows "
-		   "a built-in function", newdecl);
-	  /* Discard the old built-in function.  */
-	  return false;
+	  if (!TREE_PUBLIC (newdecl)
+	  || (DECL_INITIAL (newdecl)
+		  && !prototype_p (TREE_TYPE (newdecl
+	{
+	  warning_at (DECL_SOURCE_LOCATION (newdecl),
+			  OPT_Wshadow, "declaration of %qD shadows "
+			  "a built-in function", newdecl);
+	  /* Discard the old built-in function.  */
+	  return false;
+	}
+
+	  if (!prototype_p (TREE_TYPE (newdecl)))
+	{
+	  warning_at (DECL_SOURCE_LOCATION (newdecl),
+			  OPT_Wbuiltin_declaration_mismatch,
+			  "declaration of built-in function %qD without "
+			  "a prototype; expected %qT",
+			  newdecl, TREE_TYPE (olddecl));
+	  /* Discard the old built-in function.  */
+	  return false;
+	}
 	}
 
   if (DECL_INITIAL (newdecl))
diff --git a/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-5.c b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-5.c
new file mode 100644
index 000..51a634d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-5.c
@@ -0,0 +1,10 @@
+/* PR c/83656 - missing -Wbuiltin-declaration-mismatch on declaration
+   without prototype
+   { dg-do compile }
+   { dg-options "-Wbuiltin-declaration-mismatch" } */
+
+void* memcpy ();   /* { dg-warning "declaration of built-in function .memcpy. without a prototype; expected .void \\\*\\\(void \\\*, const void \\\*, \(long \)?unsigned int\\\)." } */
+
+int strcmp ();   /* { dg-warning "declaration of built-in function .strcmp. without a prototype; expected .int\\\(const char* \\\*, const char \\\*\\\)." } */
+
+int strcpy ();   /* { dg-warning "conflicting types for built-in function .strcpy.; expected .char \\\*\\\(char \\\*, const char \\\*\\\)." } */

Re: Limit Debug mode impact: overload __niter_base

[Jonathan Wakely]

On 26/06/18 17:03 +0100, Jonathan Wakely wrote:

On 18/06/18 23:01 +0200, François Dumont wrote:

Hi

    I abandon the idea of providing Debug algos, it would be too 
much code to add and maintain. However I haven't quit on reducing 
Debug mode performance impact.


    So this patch make use of the existing std::__niter_base to get 
rid of the debug layer around __gnu_debug::vector<>::iterator so 
that __builtin_memmove replacement can take place.


    As _Safe_iterator<> do not have a constructor taking a pointer I 
change algos implementation so that we do not try to instantiate the 
iterator type ourselves but rather rely on its operators + or -.


    The small drawback is that for std::equal algo where we can't 
use the __glibcxx_can_increment we need to keep the debug layer to 
make sure we don't reach past-the-end iterator. So I had to remove 
usage of __niter_base when in Debug mode, doing so it also disable 
potential usage of __builtin_memcmp when calling std::equal on 
std::__cxx1998::vector<> iterators. A rather rare situation I think.


We don't need to give up all checks in std::equal, we can do this:

@@ -1044,7 +1085,13 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
   typename iterator_traits<_II1>::value_type,
   typename iterator_traits<_II2>::value_type>)
  __glibcxx_requires_valid_range(__first1, __last1);
-
+#ifdef _GLIBCXX_DEBUG
+  typedef typename iterator_traits<_II1>::iterator_category _Cat1;
+  typedef typename iterator_traits<_II2>::iterator_category _Cat2;
+  if (!__are_same<_Cat1, input_iterator_tag>::__value
+ && __are_same<_Cat2, random_access_iterator_tag>::__value)
+   __glibcxx_requires_can_increment_range(__first1, __last1, __first2);
+#endif
  return std::__equal_aux(std::__niter_base(__first1),
 std::__niter_base(__last1),
 std::__niter_base(__first2));



    Note that I don't know how to test that __builtin_memmove has 
been indeed used. So I've been through some debug sessions to check 
that.


The attached patch (not fully tested) seems to be a much simpler way
to achieve the same thing. Instead of modifying all the helper
structs, just define a new function to re-wrap the result into the
desired iterator type.



diff --git a/libstdc++-v3/include/debug/stl_iterator.h 
b/libstdc++-v3/include/debug/stl_iterator.h
index a6a2a76..eca7203 100644
--- a/libstdc++-v3/include/debug/stl_iterator.h
+++ b/libstdc++-v3/include/debug/stl_iterator.h
@@ -120,4 +120,19 @@ namespace __gnu_debug
#endif
}

+#if __cplusplus >= 201103L
+namespace std
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+template
+_Iterator
+__niter_base(const __gnu_debug::_Safe_iterator<
+__gnu_cxx::__normal_iterator<_Iterator, _Container>,
+_Sequence>&);
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+#endif


Why is this overload only defined for C++11 and later? I defined it
unconditionally in the attached patch.

What do you think?


Here's a complete patch that passes all tests in normal mode and
causes no regressions in debug mode (we already have some debug test
failing).

I wondered whether we need another overload of __wrap_iter for
handling move_iterator, but I think the first overload works OK.


commit 8c22777c71589de7351b34ed4e94f3d3d2a216ee
Author: Jonathan Wakely 
Date:   Tue Jun 26 18:07:26 2018 +0100

Unwrap debug mode vector iterators to use optimized algorithms

By defining __niter_base for iterators from debug mode vectors more
algorithms can use the optimized implementations using memcpy or memset.
The new function __wrap_iter converts back to the original iterator
type, handling the case where it's a debug mode iterator and needs a
pointer to the container.

2018-06-27  Fran??ois Dumont  
Jonathan Wakely  

* include/bits/stl_algobase.h (__wrap_iter<_To, _From>): Define new
function to convert unwrapped iterator back to result type.
[_GLIBCXX_DEBUG] (__wrap_iter<_Iter,_Seq, _From>): Overload for
converting to _Safe_iterator.
[_GLIBCXX_DEBUG] (__wrap_iter<_Iter,_Seq>): Overload for converting
from reverse_iterator to reverse_iterator<_Safe_iterator>.
(__copy_move_a2, __copy_move_backward_a2, fill_n): Use __wrap_iter.
[_GLIBCXX_DEBUG] (equal<_II1, _II2>(_II1, _II1, _II2)): Use
__glibcxx_requires_can_increment_range to check for valid ranges.
* include/debug/stl_iterator.h (__niter_base): Declare overload for
iterators from debug mode vector.
* include/debug/vector (__niter_base): Define.

diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 022a3f1598b..31b2801d749 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -415,13 +415,52 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 

Re: [PATCH] PR libstdc++/86138 prevent implicit instantiation of COW empty rep

[Jonathan Wakely]

On 22/06/18 00:28 +0100, Jonathan Wakely wrote:

The explicit instantiation declarations for std::basic_string are
disabled for C++17 (and later) so that basic_string symbols get
implicitly instantiated in every translation unit that needs them.  On
targets that don't support STB_GNU_UNIQUE this leads to multiple copies
of the empty rep symbol for COW strings. In order to detect whether a
COW string needs to deallocate its storage it compares the address with
the empty rep.  When there are multiple copies of the empty rep object
the address is not unique, and so string destructors try to delete the
empty rep, which crashes.

In order to guarantee uniqueness of the _S_empty_rep_storage symbol this
patch adds an explicit instantiation declaration for just that symbol.
This means the other symbols are still implicitly instantiated in C++17
code, but for the empty rep the definition in the library gets used.

Separately, there is no need for C++17 code to implicitly instantiate
the I/O functions for strings, so this also restores the explicit
instantiation declarations for those functions.

PR libstdc++/86138
* include/bits/basic_string.tcc:
[__cplusplus > 201402 && !_GLIBCXX_USE_CXX11_ABI]
(basic_string::_Rep::_S_empty_rep_storage)
(basic_string::_Rep::_S_empty_rep_storage): Add explicit
instantiation declarations.
[__cplusplus > 201402] (operator>>, operator<<, getline): Re-enable
explicit instantiation declarations.
* testsuite/21_strings/basic_string/cons/char/86138.cc: New.
* testsuite/21_strings/basic_string/cons/wchar_t/86138.cc: New.

Tested x86_64-linux, committed to trunk.

If this passes testing on Cygwin I'll also backport it to gcc-7 and
gcc-8, as the explicit instantiation declarations are disabled for
C++17 on those branches.


The new tests are failing with _GLIBCXX_ASSERTIONS or _GLIBCXX_DEBUG
defined, because enabling assertions suppresses the explicit
instantiation declarations. However the COW empty reps and the I/O
functions don't contain any assertions, so don't need to be implicitly
instantiated.

Tested x86_64-linux, committed to trunk.



commit 22bd831a1bc38ba3e114c6166e96817fe03f006b
Author: Jonathan Wakely 
Date:   Tue Jun 26 22:14:29 2018 +0100

Declare some explicit instantiations for strings in Debug Mode

The empty reps and the I/O functions do not need to be implicitly
instantiated to enable assertions, so declare the explicit
instantiations when _GLIBCXX_EXTERN_TEMPLATE == -1 (i.e. when
_GLIBCXX_ASSERTIONS is defined).

PR libstdc++/86138
* include/bits/basic_string.tcc: [_GLIBCXX_EXTERN_TEMPLATE < 0]
Declare explicit instantiations of COW empty reps and I/O functions.

diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index 9fbea84c4af..04b68ca0202 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -1597,13 +1597,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // Inhibit implicit instantiations for required instantiations,
   // which are defined via explicit instantiations elsewhere.
-#if _GLIBCXX_EXTERN_TEMPLATE > 0
+#if _GLIBCXX_EXTERN_TEMPLATE
   // The explicit instantiations definitions in src/c++11/string-inst.cc
   // are compiled as C++14, so the new C++17 members aren't instantiated.
   // Until those definitions are compiled as C++17 suppress the declaration,
   // so C++17 code will implicitly instantiate std::string and std::wstring
   // as needed.
-# if __cplusplus <= 201402L
+# if __cplusplus <= 201402L && _GLIBCXX_EXTERN_TEMPLATE > 0
   extern template class basic_string;
 # elif ! _GLIBCXX_USE_CXX11_ABI
   // Still need to prevent implicit instantiation of the COW empty rep,
@@ -1626,7 +1626,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 getline(basic_istream&, string&);
 
 #ifdef _GLIBCXX_USE_WCHAR_T
-# if __cplusplus <= 201402L
+# if __cplusplus <= 201402L && _GLIBCXX_EXTERN_TEMPLATE > 0
   extern template class basic_string;
 # elif ! _GLIBCXX_USE_CXX11_ABI
   extern template basic_string::size_type
@@ -1646,7 +1646,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 basic_istream&
 getline(basic_istream&, wstring&);
 #endif // _GLIBCXX_USE_WCHAR_T
-#endif // _GLIBCXX_EXTERN_TEMPLATE > 0
+#endif // _GLIBCXX_EXTERN_TEMPLATE
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std

Re: [C++ Patch] More location fixes to grokdeclarator

[David Malcolm]

On Tue, 2018-06-26 at 01:44 +0200, Paolo Carlini wrote:
> Hi,
> 
> this includes straightforward tweaks to check_concept_fn and quite a
> bit 
> of additional work on grokdeclarator: most of it is also rather 
> straightforward. In a few places there is the subtlety that we want
> to 
> handle together ds_storage_class and ds_thread, whichever location
> is 
> the smallest but != UNKNOWN_LOCATION (UNKNWON_LOCATION meaning that
> the 
> issue is with the other one) or use the biggest location when say 
> ds_virtual and ds_storage_class conflict, because - I believe - we
> want 
> to point to the place where we give up. Thus I added the
> min_location 
> and max_location helpers. 

Note that directly comparing location_t values can be problematic: (one
value might be an ad-hoc location, and the other not; one might be a
macro expansion, etc).

You might want to use linemap_compare_locations or
linemap_location_before_p for this.

Hope this is helpful
Dave

Re: [PATCH] tighten up -Wbuiltin-declaration-mismatch (PR 86125)

[Martin Sebor]

Attached is an updated patch to tighten up the warning and also
prevent ICEs in the middle-end like in PR 86308 or PR 86202.

I took Richard's suggestion to add the POINTER_TYPE_P() check
to detect pointer/integer conflicts.  That also avoids the ICEs
above.

I also dealt with the fileptr_type_node problem so that file
I/O built-ins can be declared to take any object pointer type
as an argument, and that argument has to be the same for all
them.

I'm not too happy about the interaction with -Wextra but short
of enabling the stricter checks even without it or introducing
multiple levels for -Wbuiltin-declaration-mismatch I don't see
a good alternative.

Martin
PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched return type
PR middle-end/86308 - ICE in verify_gimple calling index() with an invalid declaration
PR middle-end/86202 - ICE in get_range_info calling an invalid memcpy() declaration

gcc/c/ChangeLog:

	PR c/86125
	PR middle-end/86202
	PR middle-end/86308
	* c-decl.c (match_builtin_function_types): Add arguments.
	(diagnose_mismatched_decls): Diagnose mismatched declarations
	of built-ins more strictly.
	* doc/invoke.texi (-Wbuiltin-declaration-mismatch): Update.

gcc/testsuite/ChangeLog:

	PR c/86125
	PR middle-end/86202
	PR middle-end/86308
	* gcc.dg/Wbuiltin-declaration-mismatch.c: New test.
	* gcc.dg/Wbuiltin-declaration-mismatch-2.c: New test.
	* gcc.dg/Wbuiltin-declaration-mismatch-3.c: New test.
	* gcc.dg/Wbuiltin-declaration-mismatch-4.c: New test.
	* gcc.dg/builtins-69.c: New test.

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index af16cfd..6c9e667 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1628,43 +1628,82 @@ c_bind (location_t loc, tree decl, bool is_global)
   bind (DECL_NAME (decl), decl, scope, false, nested, loc);
 }
 
+
 /* Subroutine of compare_decls.  Allow harmless mismatches in return
and argument types provided that the type modes match.  This function
-   return a unified type given a suitable match, and 0 otherwise.  */
+   returns a unified type given a suitable match, and 0 otherwise.  */
 
 static tree
-match_builtin_function_types (tree newtype, tree oldtype)
+match_builtin_function_types (tree newtype, tree oldtype,
+			  tree *strict, unsigned *argno)
 {
-  tree newrettype, oldrettype;
-  tree newargs, oldargs;
-  tree trytype, tryargs;
-
   /* Accept the return type of the new declaration if same modes.  */
-  oldrettype = TREE_TYPE (oldtype);
-  newrettype = TREE_TYPE (newtype);
+  tree oldrettype = TREE_TYPE (oldtype);
+  tree newrettype = TREE_TYPE (newtype);
+
+  *argno = 0;
+  *strict = NULL_TREE;
 
   if (TYPE_MODE (oldrettype) != TYPE_MODE (newrettype))
 return NULL_TREE;
 
-  oldargs = TYPE_ARG_TYPES (oldtype);
-  newargs = TYPE_ARG_TYPES (newtype);
-  tryargs = newargs;
+  if (!comptypes (oldrettype, newrettype))
+*strict = oldrettype;
+
+  tree oldargs = TYPE_ARG_TYPES (oldtype);
+  tree newargs = TYPE_ARG_TYPES (newtype);
+  tree tryargs = newargs;
 
-  while (oldargs || newargs)
+  for (unsigned i = 1; oldargs || newargs; ++i)
 {
   if (!oldargs
 	  || !newargs
 	  || !TREE_VALUE (oldargs)
-	  || !TREE_VALUE (newargs)
-	  || TYPE_MODE (TREE_VALUE (oldargs))
-	 != TYPE_MODE (TREE_VALUE (newargs)))
+	  || !TREE_VALUE (newargs))
 	return NULL_TREE;
 
+  tree oldtype = TREE_VALUE (oldargs);
+  tree newtype = TREE_VALUE (newargs);
+
+  /* Fail for types with incompatible modes/sizes.  */
+  if (TYPE_MODE (TREE_VALUE (oldargs))
+	  != TYPE_MODE (TREE_VALUE (newargs)))
+	return NULL_TREE;
+
+  /* Fail for function and object pointer mismatches.  */
+  if (FUNCTION_POINTER_TYPE_P (oldtype) != FUNCTION_POINTER_TYPE_P (newtype)
+	  || POINTER_TYPE_P (oldtype) != POINTER_TYPE_P (newtype))
+	return NULL_TREE;
+
+  if (oldtype == fileptr_type_node)
+	{
+	  /* Store the first FILE* argument type seen (whatever it is),
+	 and expect any subsequent declarations of file I/O built-ins
+	 to refer to it rather than to fileptr_type_node which is just
+	 void*.  */
+	  static tree last_fileptr_type;
+	  if (last_fileptr_type)
+	{
+	  if (!comptypes (last_fileptr_type, newtype))
+		{
+		  *argno = i;
+		  *strict = last_fileptr_type;
+		}
+	}
+	  else
+	last_fileptr_type = newtype;
+	}
+  else if (!*strict && !comptypes (oldtype, newtype))
+	{
+	  *argno = i;
+	  *strict = oldtype;
+	}
+
   oldargs = TREE_CHAIN (oldargs);
   newargs = TREE_CHAIN (newargs);
 }
 
-  trytype = build_function_type (newrettype, tryargs);
+  tree trytype = build_function_type (newrettype, tryargs);
 
   /* Allow declaration to change transaction_safe attribute.  */
   tree oldattrs = TYPE_ATTRIBUTES (oldtype);
@@ -1874,9 +1913,18 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
   if (TREE_CODE (olddecl) == FUNCTION_DECL
 	  && DECL_BUILT_IN (olddecl) && !C_DECL_DECLARED_BUILTIN (olddecl))
 	{
-	  /* Accept harmless mismatch in function types.
-	 This is for 

Re: [PATCH][AARCH64] PR target/84521 Fix frame pointer corruption with -fomit-frame-pointer with __builtin_setjmp

[Eric Botcazou]

> The AArch64 parts are OK. I've been holding off approving the patch while
> I wait for Eric to reply on the x86_64 fails with your new testcase.

The test is not portable in any case since it uses the "optimize" attribute so 
I'd just make it specific to Aarch64 and install the patch.

-- 
Eric Botcazou

Re: [PATCH v2] Fix LRA to handle multi-word eliminable registers

[Vladimir Makarov]

On 06/23/2018 08:38 AM, Dimitar Dimitrov wrote:

For some targets, Pmode != UNITS_PER_WORD. Take this into account
when marking hard registers as being used.

I tested C and C++ testsuits for x86_64 with and without this
patch. There was no regression, i.e. gcc.sum and g++.sum matched
exactly.

Changes since patch series v1:
   - Cleanup to use add_to_hard_reg_set.
   - Also fix check_pseudos_live_through_calls.
   - Decouple PRU tests so that LRA patch is now standalone.

gcc/ChangeLog:

2018-06-23  Dimitar Dimitrov  

* lra-eliminations.c (update_reg_eliminate): Mark all spanning hard
registers for Pmode..
* lra-lives.c (check_pseudos_live_through_calls): Mark all spanning
hard registers for the clobbered pseudo.
I am not sure about necessity of a change in 
check_pseudos_live_through_calls.  But I guess reload treats partially 
clobbered regs in analogous way as the change.  Plus the change makes 
LRA code safer anyway.


So please go ahead and commit the patch into the trunk.

Thank you.

Cc: Vladimir Makarov 
Cc: Peter Bergner 
Cc: Kenneth Zadeck 
Cc: Seongbae Park 
Cc: Jeff Law 
Signed-off-by: Dimitar Dimitrov 
---
  gcc/lra-eliminations.c | 4 ++--
  gcc/lra-lives.c| 3 ++-
  2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/gcc/lra-eliminations.c b/gcc/lra-eliminations.c
index 21d8d5f8018..f5f104020b3 100644
--- a/gcc/lra-eliminations.c
+++ b/gcc/lra-eliminations.c
@@ -1264,13 +1264,13 @@ update_reg_eliminate (bitmap insns_with_changed_offsets)
CLEAR_HARD_REG_SET (temp_hard_reg_set);
for (ep = reg_eliminate; ep < _eliminate[NUM_ELIMINABLE_REGS]; ep++)
  if (elimination_map[ep->from] == NULL)
-  SET_HARD_REG_BIT (temp_hard_reg_set, ep->from);
+  add_to_hard_reg_set (_hard_reg_set, Pmode, ep->from);
  else if (elimination_map[ep->from] == ep)
{
/* Prevent the hard register into which we eliminate from
   the usage for pseudos.  */
  if (ep->from != ep->to)
- SET_HARD_REG_BIT (temp_hard_reg_set, ep->to);
+ add_to_hard_reg_set (_hard_reg_set, Pmode, ep->to);
if (maybe_ne (ep->previous_offset, ep->offset))
  {
bitmap_ior_into (insns_with_changed_offsets,
diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c
index 588bc09cb8e..920fd02b997 100644
--- a/gcc/lra-lives.c
+++ b/gcc/lra-lives.c
@@ -581,7 +581,8 @@ check_pseudos_live_through_calls (int regno,
for (hr = 0; hr < FIRST_PSEUDO_REGISTER; hr++)
  if (targetm.hard_regno_call_part_clobbered (hr,
PSEUDO_REGNO_MODE (regno)))
-  SET_HARD_REG_BIT (lra_reg_info[regno].conflict_hard_regs, hr);
+  add_to_hard_reg_set (_reg_info[regno].conflict_hard_regs,
+  PSEUDO_REGNO_MODE (regno), hr);
lra_reg_info[regno].call_p = true;
if (! sparseset_bit_p (pseudos_live_through_setjumps, regno))
  return;

Re: [PATCH][GCC][AArch64] Add SIMD to REG pattern for movhf without armv8.2-a support (PR85769)

[James Greenhalgh]

On Wed, Jun 20, 2018 at 05:15:37AM -0500, Tamar Christina wrote:
> Hi Kyrill,
> 
> Many thanks for the review!
> 
> The 06/19/2018 16:47, Kyrill Tkachov wrote:
> > Hi Tamar,
> > 
> > On 19/06/18 15:07, Tamar Christina wrote:
> > > Hi All,
> > >
> > > This fixes a regression where we don't have an instruction for pre 
> > > Armv8.2-a
> > > to do a move of an fp16 value from a GP reg to a SIMD reg.
> > >
> > > This patch adds that pattern to movhf_aarch64 using a dup and only 
> > > selectes it
> > > using a very low priority.
> > >
> > > This fixes an ICE at -O0.
> > >
> > > Regtested on aarch64-none-elf and no issues.
> > > Bootstrapped on aarch64-none-linux-gnu and no issues.
> > >
> > > Ok for master?

OK,

Thanks,
James

> > >
> > > gcc/
> > > 2018-06-19  Tamar Christina  
> > >
> > > PR target/85769
> > > * config/aarch64/aarch64.md (*movhf_aarch64): Add dup v0.4h 
> > > pattern.
> > >
> > > gcc/testsuite/
> > > 2018-06-19  Tamar Christina  
> > >
> > > PR target/85769
> > > * gcc.target/aarch64/f16_mov_immediate_3.c: New.
> > > Thanks,
> > > Tamar
> > >
> > > -- 
> 

Re: [PATCH] [aarch64] Remove obsolete comment about X30

[James Greenhalgh]

On Wed, Jun 20, 2018 at 04:41:18AM -0500, Siddhesh Poyarekar wrote:
> On 06/19/2018 09:11 PM, James Greenhalgh wrote:
> > On Mon, Jun 18, 2018 at 08:43:04AM -0500, Siddhesh Poyarekar wrote:
> >> r217431 changed X30 as caller-saved in CALL_USE_REGISTERS because of
> >> which this comment about X30 not being marked as call-clobbered is no
> >> longer accurate.
> > 
> > Is the second paragraph is still relevant to how we define EPILOGUE_USES?
> 
> It is, but it is essentially a repetition of the comment directly above 
> EPILOGUE_USES.
> 
> > Possibly I'd rewrite the comment to explain the behaviour around calls and
> > how they interact with x30.
> 
> How about this then:

OK,

Thanks,
James

Re: [PATCH v2] Add HXT Phecda core support

[James Greenhalgh]

On Fri, Jun 22, 2018 at 02:52:33AM -0500, Hongbo Zhang wrote:
> HXT semiconductor's CPU core Phecda, as a variant of Qualcomm qdf24xx,
> reuses the same tuning structure and pipeline with it.

OK.

Thanks,
James

> 2018-06-19  Hongbo Zhang  
> 
>   * config/aarch64/aarch64-cores.def (AARCH64_CORE): Add phecda core.
>   * config/aarch64/aarch64-tune.md: Regenerate.
>   * doc/invoke.texi: Add phecda core.
> ---
> v2 change: description in change log updated.
> 
>  gcc/config/aarch64/aarch64-cores.def | 3 +++
>  gcc/config/aarch64/aarch64-tune.md   | 2 +-
>  gcc/doc/invoke.texi  | 2 +-
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 940b846..43ef9ac 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -14667,7 +14667,7 @@ performance of the code.  Permissible values for this 
> option are:
>  @samp{generic}, @samp{cortex-a35}, @samp{cortex-a53}, @samp{cortex-a55},
>  @samp{cortex-a57}, @samp{cortex-a72}, @samp{cortex-a73}, @samp{cortex-a75},
>  @samp{exynos-m1}, @samp{falkor}, @samp{qdf24xx}, @samp{saphira},
> -@samp{xgene1}, @samp{vulcan}, @samp{thunderx},
> +@samp{phecda}, @samp{xgene1}, @samp{vulcan}, @samp{thunderx},
>  @samp{thunderxt88}, @samp{thunderxt88p1}, @samp{thunderxt81},
>  @samp{thunderxt83}, @samp{thunderx2t99}, @samp{cortex-a57.cortex-a53},
>  @samp{cortex-a72.cortex-a53}, @samp{cortex-a73.cortex-a35},

This list is getting less and less useful over time!

James

Re: [PATCH][AARCH64] PR target/84521 Fix frame pointer corruption with -fomit-frame-pointer with __builtin_setjmp

[James Greenhalgh]

On Mon, Jun 25, 2018 at 04:24:14AM -0500, Sudakshina Das wrote:
> PING!
> 
> On 14/06/18 12:10, Sudakshina Das wrote:
> > Hi Eric
> > 
> > On 07/06/18 16:33, Eric Botcazou wrote:
> >>> Sorry this fell off my radar. I have reg-tested it on x86 and tried it
> >>> on the sparc machine from the gcc farm but I think I couldn't finished
> >>> the run and now its showing to he unreachable.
> >>
> >> The patch is a no-op for SPARC because it defines the nonlocal_goto 
> >> pattern.
> >>
> >> But I would nevertheless strongly suggest _not_ fiddling with the 
> >> generic code
> >> like that and just defining the nonlocal_goto pattern for Aarch64 
> >> instead.
> >>
> > 
> > Thank you for the suggestion, I have edited the patch accordingly and
> > defined the nonlocal_goto pattern for AArch64. This has also helped take
> > care of the issue with __builtin_longjmp that Wilco had mentioned in his
> > comment on the PR (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521#c19).
> > 
> > I have also modified the test case according to Wilco's comment to add 
> > an extra jump buffer. This test case passes with AArch64 but fails on
> > x86 trunk as follows (It may fail on other targets as well):
> > 
> > FAIL: gcc.c-torture/execute/pr84521.c   -O1  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O2  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O3 -fomit-frame-pointer
> > -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O3 -g  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -Os  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O2 -flto -fno-use-linker-plugin
> > -flto-partition=none  execution test
> > FAIL: gcc.c-torture/execute/pr84521.c   -O2 -flto -fuse-linker-plugin
> > -fno-fat-lto-objects  execution test
> > 
> > Testing: Bootstrapped and regtested on aarch64-none-linux-gnu.
> > 
> > Is this ok for trunk?

The AArch64 parts are OK. I've been holding off approving the patch while
I wait for Eric to reply on the x86_64 fails with your new testcase.

Thanks,
James



> > 
> > Sudi
> > 
> > *** gcc/ChangeLog ***
> > 
> > 2018-06-14  Sudakshina Das  
> > 
> >  PR target/84521
> >  * config/aarch64/aarch64.h (DONT_USE_BUILTIN_SETJMP): Update comment.
> >  * config/aarch64/aarch64.c (aarch64_needs_frame_chain): Add
> >  cfun->has_nonlocal_label to force frame chain.
> >  (aarch64_builtin_setjmp_frame_value): New.
> >  (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Define.
> >  * config/aarch64/aarch64.md (nonlocal_goto): New.
> > 
> > *** gcc/testsuite/ChangeLog ***
> > 
> > 2018-06-14  Sudakshina Das  
> > 
> >  PR target/84521
> >  * gcc.c-torture/execute/pr84521.c: New test.
> 

Re: [patch, fortran] Handling of .and. and .or. expressions

[Jakub Jelinek]

On Tue, Jun 26, 2018 at 11:12:40PM +0200, Thomas Koenig wrote:
> Hi Janus,
> 
> with your patch, we would only warn about
> 
> var .and. func()
> 
> and not about
> 
> func() .and. var.
> 
> AFAIK, TRUTH_AND_EXPR does not guarantee that func() will
> always be exectued, or if it does, I have not seen the
> documentation; it just happens to match what we currently
> see (which may be due to missed optimizatins in the back end,
> or the lack of test cases exposing this).

If you are talking about what the middle-end does, TRUTH_AND_EXPR
always evaluates side-effects in both operands, while for
TRUTH_ANDIF_EXPR it evaluates the second operand only if the first operand
is not false.

Jakub

Re: [line-map patch] Better packing of maps

[Nathan Sidwell]

On 06/26/2018 03:31 PM, David Malcolm wrote:


Which assert are you referring to?


Hm, I think I may have been confused by the unsigned arithmetic check in:
  if (start_location <= set->highest_line
  || start_location > LINEMAPS_MACRO_LOWEST_LOCATION (set))
/* We ran out of macro map space.   */
return NULL;

It looks to me that the second condition can only be true with the first 
being false when the macro expansion is >= 2^32 tokens (give or take).


However, AFAICT, linemap_add doesn't check whether ordinary maps have 
run into the macro range:

  if (set->highest_location < LINE_MAP_MAX_LOCATION_WITH_COLS)
   {...}
  else
start_location = set->highest_location + 1;


So if I'm reading the patch correctly, it introduces a new limit on how
big the macro maps can be: 0x1000, hence about 268 million macro
argument expansions, I believe, before it will stop tracking macro
expansions.


You may be correct.  The comment in line-map.h is unclear as to whether 
the  LINE_MAP_MAX_SOURCE_LOCATION is a barrier for macros or not, it can 
be read either way.



Has this been smoketested on some very large C++ TUs?


No.  The code bases available to me aren't gcc-trunk ready (I don't know 
if they contain single TUs so large).


nathan

--
Nathan Sidwell

Re: [patch, fortran] Handling of .and. and .or. expressions

[Thomas Koenig]

Hi Janus,

with your patch, we would only warn about

var .and. func()

and not about

func() .and. var.

AFAIK, TRUTH_AND_EXPR does not guarantee that func() will
always be exectued, or if it does, I have not seen the
documentation; it just happens to match what we currently
see (which may be due to missed optimizatins in the back end,
or the lack of test cases exposing this).

So, I do not think that we should only warn about the
first case here.

Regards

Thomas

Re: [PATCH] Adjust subprogram DIE re-usal

[Jeff Law]

On 06/26/2018 06:43 AM, Richard Biener wrote:
> 
> A patch from Honza not LTO streaming DECL_ORIGINAL_TYPE ends up
> ICEing during LTO bootstrap because we end up not re-using the
> DIE we create late during LTRANS for a subprogram because its
> parent is a namespace rather than a CU DIE (or a module).
> 
> I'm wondering of the general reason why we enforce (inconsistently)
> "We always want the DIE for this function that has the *_pc attributes to 
> be under comp_unit_die so the debugger can find it."
> We indeed generate a specification DIE rooted at the CU in addition to the
> declaration DIE inside the namespace for sth as simple as
> 
> namespace Foo { void foo () {} }
> 
> anyhow - the comment also says "We also need to do this [re-use the DIE]
> for abstract instances of inlines, since the spec requires the out-of-line
> copy to have the same parent.".  Not sure what condition this part of
> the comment applies to.
> 
> So my fix is to move the || get_AT (old_die, DW_AT_abstract_origin)
> check I added for early LTO debug to a global override - forcing
> DIE re-use for any DIE with an abstract origin set.  That is, all
> concrete instances are fine where they are.  That also avoids
> double-indirection DW_AT_specification -> DW_AT_abstract_origin -> DIE.
> 
> But as said, I wonder about the overall condition, esp. the
> DW_TAG_module special-casing (but not at the same time
> special-casing DW_TAG_namespace or DW_TAG_partial_unit).
> 
> LTO bootstrap is in progress on x86_64-unknown-linux-gnu.
> 
> OK if that succeeds?
> 
> Thanks,
> Richard.
> 
> 2018-06-26  Richard Biener  
> 
>   * dwarf2out.c (gen_subprogram_die): Always re-use DIEs with an
>   DW_AT_abstract_origin attribute.
Explicitly deferring to Jason here.

jeff

[PATCH] Hide alt_dump_file within dumpfile.c

[David Malcolm]

On Mon, 2018-06-25 at 15:34 +0200, Richard Biener wrote:
> On Wed, Jun 20, 2018 at 6:34 PM David Malcolm 
> wrote:
> > 
> > Here's v3 of the patch (one big patch this time, rather than a
> > kit).
> > 
> > Like the v2 patch kit, this patch reuses the existing dump API,
> > rather than inventing its own.
> > 
> > Specifically, it uses the dump_* functions in dumpfile.h that don't
> > take a FILE *, the ones that implicitly write to dump_file and/or
> > alt_dump_file.  I needed a name for them, so I've taken to calling
> > them the "structured dump API" (better name ideas welcome).
> > 
> > v3 eliminates v2's optinfo_guard class, instead using "dump_*_loc"
> > calls as delimiters when consolidating "dump_*" calls.  There's a
> > new dump_context class which has responsibility for consolidating
> > them into optimization records.
> > 
> > The dump_*_loc calls now capture more than just a location_t: they
> > capture the profile_count and the location in GCC's own sources
> > where
> > the dump is being emitted from.
> > 
> > This works by introducing a new "dump_location_t" class as the
> > argument of those dump_*_loc calls.  The dump_location_t can
> > be constructed from a gimple * or from an rtx_insn *, so that
> > rather than writing:
> > 
> >   dump_printf_loc (MSG_NOTE, gimple_location (stmt),
> >"some message: %i", 42);
> > 
> > you can write:
> > 
> >   dump_printf_loc (MSG_NOTE, stmt,
> >"some message: %i", 42);
> > 
> > and the dump_location_t constructor will grab the location_t and
> > profile_count of stmt, and the location of the "dump_printf_loc"
> > callsite (and gracefully handle "stmt" being NULL).
> > 
> > Earlier versions of the patch captured the location of the
> > dump_*_loc call via preprocessor hacks, or didn't work properly;
> > this version of the patch works more cleanly: internally,
> > dump_location_t is split into two new classes:
> >   * dump_user_location_t: the location_t and profile_count within
> > the *user's code*, and
> >   * dump_impl_location_t: the __builtin_FILE/LINE/FUNCTION within
> > the *implementation* code (i.e. GCC or a plugin), captured
> > "automagically" via default params
> > 
> > These classes are sometimes used elsewhere in the code.  For
> > example, "vect_location" becomes a dump_user_location_t
> > (location_t and profile_count), so that in e.g:
> > 
> >   vect_location = find_loop_location (loop);
> > 
> > it's capturing the location_t and profile_count, and then when
> > it's used here:
> > 
> >   dump_printf_loc (MSG_NOTE, vect_location, "foo");
> > 
> > the dump_location_t is constructed from the vect_location
> > plus the dump_impl_location_t at that callsite.
> > 
> > In contrast, loop-unroll.c's report_unroll's "locus" param
> > becomes a dump_location_t: we're interested in where it was
> > called from, not in the locations of the various dump_*_loc calls
> > within it.
> > 
> > Previous versions of the patch captured a gimple *, and needed
> > GTY markers; in this patch, the dump_user_location_t is now just a
> > location_t and a profile_count.
> > 
> > The v2 patch added an overload for dump_printf_loc so that you
> > could pass in either a location_t, or the new type; this version
> > of the patch eliminates that: they all now take dump_location_t.
> > 
> > Doing so required adding support for rtx_insn *, so that one can
> > write this kind of thing in RTL passes:
> > 
> >   dump_printf_loc (MSG_NOTE, insn, "foo");
> > 
> > One knock-on effect is that get_loop_location now returns a
> > dump_user_location_t rather than a location_t, so that it has
> > hotness information.
> > 
> > Richi: would you like me to split out this location-handling
> > code into a separate patch?  (It's kind of redundant without
> > adding the remarks and optimization records work, but if that's
> > easier I can do it)
> 
> I think that would be easier because it doesn't require the JSON
> stuff and so I'll happily approve it.
> 
> Thus - trying to review that bits (and sorry for the delay).
> 
> +  location_t srcloc = loc.get_location_t ();
> +
>if (dump_file && (dump_kind & pflags))
>  {
> -  dump_loc (dump_kind, dump_file, loc);
> +  dump_loc (dump_kind, dump_file, srcloc);
>print_gimple_stmt (dump_file, gs, spc, dump_flags |
> extra_dump_flags);
>  }
> 
>if (alt_dump_file && (dump_kind & alt_flags))
>  {
> -  dump_loc (dump_kind, alt_dump_file, loc);
> +  dump_loc (dump_kind, alt_dump_file, srcloc);
>print_gimple_stmt (alt_dump_file, gs, spc, dump_flags |
> extra_dump_flags);
>  }
> +
> +  if (optinfo_enabled_p ())
> +{
> +  optinfo  = begin_next_optinfo (loc);
> +  info.handle_dump_file_kind (dump_kind);
> +  info.add_stmt (gs, extra_dump_flags);
> +}
> 
> seeing this in multiple places.  I seem to remember that
> dump_file / alt_dump_file was suposed to handle dumping
> into two locations - a dump file and optinfo (or stdout).  

[PATCH] contrib: introduce Vim addon directory, add match.pd syntax plugin

[Alexander Monakov]

Hi,

This adds Vim syntax highlighting rules for match.pd and packages them together
with gcc-rtl.vim and gimple.vim, creating contrib/vim-gcc-dev subtree that can
be installed as a common Vim plugin.

Thanks.
Alexander

* vim-gcc-dev/README: New file.
* vim-gcc-dev/ftdetect/gcc-dev.vim: New file.
* vim-gcc-dev/syntax/gcc-match.vim: New file.
* gimple.vim: Move under vim-gcc-dev/syntax/.
* gcc-rtl.vim: Likewise.

diff --git a/contrib/vim-gcc-dev/README b/contrib/vim-gcc-dev/README
new file mode 100644
index 000..29bbf48492f
--- /dev/null
+++ b/contrib/vim-gcc-dev/README
@@ -0,0 +1,13 @@
+This directory serves as a simple Vim addon for GCC development.  It can be
+symlinked or copied into Vim plugin directory as any other plugin.  For
+example, if using vim-pathogen plugin manager:
+
+ln -s /path/to/gcc/contrib/vim-gcc-dev ~/.vim/bundle/
+
+This adds syntax highlighting rules for the match.pd file and GIMPLE/RTL dumps.
+
+You can also use RTL syntax rules for GCC machine desciption files by adding
+
+autocmd BufRead *.md  setf gcc-rtl
+
+to your ~/.vimrc file.
diff --git a/contrib/vim-gcc-dev/ftdetect/gcc-dev.vim 
b/contrib/vim-gcc-dev/ftdetect/gcc-dev.vim
new file mode 100644
index 000..ed6989aeacb
--- /dev/null
+++ b/contrib/vim-gcc-dev/ftdetect/gcc-dev.vim
@@ -0,0 +1,20 @@
+" Vim file type detection rules for GCC development
+"
+" Copyright (C) 2018 Free Software Foundation, Inc.
+"
+" This script is free software; you can redistribute it and/or modify
+" it under the terms of the GNU General Public License as published by
+" the Free Software Foundation; either version 3, or (at your option)
+" any later version
+
+augroup filetypedetect
+
+  au BufRead match.pdsetf gcc-match
+
+  " Match RTL dump file names such as test.c.234r.pass-name
+  au BufRead *.[1-3][0-9][0-9]r.*setf gcc-rtl
+
+  " Match GIMPLE and IPA dump file names
+  au BufRead *.[0-2][0-9][0-9][ti].* setf gimple
+
+augroup END
diff --git a/contrib/vim-gcc-dev/syntax/gcc-match.vim 
b/contrib/vim-gcc-dev/syntax/gcc-match.vim
new file mode 100644
index 000..356b07a15b2
--- /dev/null
+++ b/contrib/vim-gcc-dev/syntax/gcc-match.vim
@@ -0,0 +1,71 @@
+" Vim syntax highlighting rules for GCC match-and-simplify language.
+"
+" Copyright (C) 2018 Free Software Foundation, Inc.
+"
+" This script is free software; you can redistribute it and/or modify
+" it under the terms of the GNU General Public License as published by
+" the Free Software Foundation; either version 3, or (at your option)
+" any later version
+
+if exists("b:current_syntax")
+finish
+endif
+
+" Some keywords have a question mark, e.g. 'convert?'
+setl isk=@,48-57,_,?
+
+syn keyword pdTodo contained TODO FIXME XXX
+
+syn keyword pdCtrl match simplify
+syn keyword pdCtrl define_predicates define_operator_list
+syn keyword pdCtrl if switch for with
+
+syn keyword pdType type
+
+syn keyword pdOp view_convert view_convert?
+   \ convert convert? convert1 convert2 convert1? convert2?
+   \ realpart imagpart
+   \ cond vec_cond vec_perm
+   \ pointer_plus pointer_diff
+   \ plus minus mult mult_highpart
+   \ trunc_div ceil_div floor_div round_div
+   \ trunc_mod ceil_mod floor_mod round_mod
+   \ rdiv exact_div
+   \ fix_trunc float negate min max abs absu
+   \ lshift rshift lrotate rrotate
+   \ bit_ior bit_xor bit_and bit_not
+   \ truth_andif truth_orif truth_and
+   \ truth_or truth_xor truth_not
+   \ lt le gt ge eq ne unordered ordered
+   \ unlt unle ungt unge uneq ltgt
+   \ addr_space_convert fixed_convert
+   \ bit_insert complex conj
+   \ reduc_max reduc_min reduc_plus
+   \ dot_prod widen_sum sad fma
+   \ widen_mult widen_mult_plus widen_mult_minus widen_lshift
+   \ vec_widen_mult_hi vec_widen_mult_lo
+   \ vec_widen_mult_even vec_widen_mult_odd
+   \ vec_unpack_hi vec_unpack_lo
+   \ vec_unpack_float_hi vec_unpack_float_lo
+   \ vec_pack_trunc vec_pack_sat vec_pack_fix_trunc
+   \ vec_widen_lshift_hi vec_widen_lshift_lo
+
+" Match commutative/single-use specifiers: :C, :c, :s, :cs, etc.
+syn match pdOpSpec  ":[CcSs]\+\>"
+
+syn match pdCapture "@@\?[a-zA-Z0-9_]\+"
+
+syn region pdComment start="/\*" end="\*/" contains=pdTodo
+
+syn region pdPreProc start="^\s*#" skip="\\$" end="$" keepend
+
+hi def link pdCtrlStatement
+hi def link pdTypeIdentifier
+hi def link pdOp  Constant
+hi def link pdOpSpec  Operator
+hi def link pdCapture Special
+hi def link pdComment Comment
+hi def link pdTodoTodo
+hi def link pdPreProc PreProc
+
+let b:current_syntax = "gcc-match"
diff --git a/contrib/gcc-rtl.vim b/contrib/vim-gcc-dev/syntax/gcc-rtl.vim
similarity 

Re: [PATCH] fixincludes: Add missing hunk to tests/base/ioLib.h

[Jeff Law]

On 06/26/2018 05:59 AM, Rasmus Villemoes wrote:
> When adding the vxworks_iolib_include_unistd hack I failed to add the
> appropriate hunk to the tests/base/ioLib.h file, causing "make
> check-fixincludes" to fail.
> 
> OK for trunk?
> 
> 2018-06-26  Rasmus Villemoes  
> 
> fixincludes/
> 
>   * tests/base/ioLib.h [VXWORKS_IOLIB_INCLUDE_UNISTD_CHECK]: Add
>   missing hunk.
OK.
jeff

Re: [patch] Remap goto_locus on edges during inlining

[Jeff Law]

On 06/26/2018 02:54 AM, Eric Botcazou wrote:
> Hi,
> 
> this makes sure the goto_locus present (or not) on edges is properly remapped 
> during inlining.
> 
> Tested on x86-64/Linux, OK for the mainline?
> 
> 
> 2018-06-26  Eric Botcazou  
> 
>   * tree-inline.c (remap_location): New function extracted from...
>   (copy_edges_for_bb): Add ID parameter.  Remap goto_locus.
>   (copy_phis_for_bb): ...here.  Call remap_location.
>   (copy_cfg_body): Adjust call to copy_edges_for_bb.
OK.
jeff

Re: [PATCH, rs6000] Backport Fix tests that are failing in gcc.target/powerpc/bfp with -m32

[Kelvin Nilsen]

Hi Segher,

This patch, as revised in response to your suggestions, was committed to trunk 
on 4/17/2018.

Is this ok for backporting to gcc8, gcc7, and gcc6?

Thanks.


On 4/13/18 3:15 PM, Kelvin Nilsen wrote:
> Twelve failures have been occuring in the bfp test directory during -m32
> regression testing.
> 
> The cause of these failures was two-fold:
> 
> 1. Patches added subsequent to development of the tests caused new error
> messages
> to be emitted that are different than the error messages expected in the
> dejagnu patterns.
> These new patches also changed which built-in functions are legal when
> compiling with the
> -m32 command-line option.
> 
> 2. The implementation of overloaded built-in functions maps overloaded
> function names to
> non-overloaded names.  Depending on the stage at which an error is
> recognized, error
> messages may refer either to the overloaded built-in function name or
> the non-overloaded
> name.
> 
> This patch:
> 
> 1. Changes the expected error messages in certain test programs.
> 
> 2. Disables certain test programs from being exercised on 32-bit targets.
> 
> 3. Adds a "note" error message to explain the mapping from overloaded
> built-in functions
> to non-overloaded built-in functions.
> 
> 
> This patch has bootstrapped and tested without regressions on both
> powerpc64le-unknown-linux (P8) and on powerpc-linux (P7 big-endian, with
> both -m32
> and -m64 target options).
> 
> Is this ok for trunk?
> 
> gcc/ChangeLog:
> 
> 2018-04-13  Kelvin Nilsen  
> 
>     * config/rs6000/rs6000-protos.h (rs6000_builtin_is_supported_p):
>     New prototype.
>     * config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
>     Add note to error message to explain internal mapping of overloaded
>     built-in function name to non-overloaded built-in function name.
>     * config/rs6000/rs6000.c (rs6000_builtin_is_supported_p): New
>     function.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-04-13  Kelvin Nilsen  
> 
>     * gcc.target/powerpc/bfp/scalar-extract-sig-5.c: Simplify to
>     prevent cascading of errors and change expected error message.
>     * gcc.target/powerpc/bfp/scalar-test-neg-4.c: Restrict this test
>     to 64-bit targets.
>     * gcc.target/powerpc/bfp/scalar-test-data-class-8.c: Likewise.
>     * gcc.target/powerpc/bfp/scalar-test-data-class-9.c: Likewise.
>     * gcc.target/powerpc/bfp/scalar-test-data-class-10.c: Likewise.
>     * gcc.target/powerpc/bfp/scalar-insert-exp-11.c: Change expected
>     error message.
>     * gcc.target/powerpc/bfp/scalar-extract-exp-5.c: Likewise.
> 
> Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c
> ===
> --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c   
> (revision 259316)
> +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c   
> (working copy)
> @@ -8,10 +8,10 @@
>     error because the builtin requires 64 bits.  */
>  #include 
>  
> -unsigned __int128 /* { dg-error "'__int128' is not supported on this
> target" } */
> +unsigned long long int
>  get_significand (__ieee128 *p)
>  {
>    __ieee128 source = *p;
>  
> -  return __builtin_vec_scalar_extract_sig (source); /* { dg-error
> "builtin function '__builtin_vec_scalar_extract_sig' not supported in
> this compiler configuration" } */
> +  return (long long int) __builtin_vec_scalar_extract_sig (source); /*
> { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */
>  }
> Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-4.c
> ===
> --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-4.c   
> (revision 259316)
> +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-4.c    (working
> copy)
> @@ -1,5 +1,6 @@
>  /* { dg-do compile { target { powerpc*-*-* } } } */
>  /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" }
> { "-mcpu=power9" } } */
> +/* { dg-require-effective-target lp64 } */
>  /* { dg-require-effective-target powerpc_p9vector_ok } */
>  /* { dg-options "-mcpu=power9" } */
>  
> @@ -11,6 +12,8 @@
>  {
>    __ieee128 source = *p;
>  
> +  /* IEEE 128-bit floating point operations are only supported
> + on 64-bit targets.  */
>    return scalar_test_neg (source);
>  }
>  
> Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-8.c
> ===
> --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-8.c   
> (revision 259316)
> +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-8.c   
> (working copy)
> @@ -1,5 +1,6 @@
>  /* { dg-do compile { target { powerpc*-*-* } } } */
>  /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" }
> { "-mcpu=power9" } } */
> +/* { dg-require-effective-target lp64 } */
>  /* { dg-require-effective-target powerpc_p9vector_ok } */
>  /* { dg-options "-mcpu=power9" } */
>  
> @@ -11,6 +12,8 @@
>  {
>   

Re: [line-map patch] Better packing of maps

[David Malcolm]

On Tue, 2018-06-26 at 14:28 -0400, Nathan Sidwell wrote:
> I've been wandering around the line-map machinery on the modules
> branch. 
>   One thing I noticed was the padding of the line_map type hierarchy
> was 
> unfortunate.  That made me sad.  This patch fixes that.
> 
> Rather than keep the the reason field in line_map, I move it to 
> line_map_ordinary.  That allows line_map to be a 4 byte struct,
> rather 
> than 5 bytes data and 3 padding.  We can use the source_location it 
> contains to distinguish between ordinary and macro maps, by moving
> the 
> LINE_MAP_MAX_LOCATION constant into the header file.  While one 
> interpretation of the comments is that the macro and ordinary line
> maps 
> can grow arbitrarily, until they meet, the actual code presumes the 
> boundary is fixed, so this is not a new restriction. (see the assert
> in 
> linemap_enter_macro for instance)

Which assert are you referring to?

I'm not convinced that there already is a fixed boundary between the
ordinary maps and macro maps (though that's not necessarily an argument
against the patch); my reading of the existing linemap_enter_macro is
that it handles the two maps meeting, but doesn't set any restrictions
on where that meeting point is.

Previously there was an upper bound on the expansion of ordinary
locations:
  const source_location LINE_MAP_MAX_SOURCE_LOCATION = 0x7000;
but I believe that macro maps could in theory expand below this limit,
and this check in linemap_line_start is meant to prevent collisions
when this happens:

  /* Locations of ordinary tokens are always lower than locations of
 macro tokens.  */
  if (r >= LINEMAPS_MACRO_LOWEST_LOCATION (set))
return 0;

So if I'm reading the patch correctly, it introduces a new limit on how
big the macro maps can be: 0x1000, hence about 268 million macro
argument expansions, I believe, before it will stop tracking macro
expansions.

Has this been smoketested on some very large C++ TUs?

> Reordering the fields in the ordinary and macro map objects
> continues 
> the reduction in padding.  On a 64-bit target the ordinary map goes
> from 
> 32 to 24 bytes.

> booted & tested on x86_64-linux.  I'll commit in a few days, unless 
> there are objections (it is part of libcpp, but has an explicit 
> maintainer listed).
> 
> nathan

It would be nice to extend:
  gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c
  gcc/testsuite/gcc.dg/plugin/location-overflow-test-*.c
to get some test coverage of what happens when the two kinds of map
collide.

Hope this is constructive
Dave

Re: [PATCH] libstdc++/testsuite/experimental/algorithm link errors

[Jonathan Wakely]

On 26/06/18 15:09 -0400, David Edelsohn wrote:

The recent addition of

testsuite/experimental/algorithm/sample-2.cc
testsuite/experimental/algorithm/shuffle.cc

introduced link errors on AIX.  AIX (and Solaris) require additional
options for TLS in some situations.

This patch adds the dejagnu TLS directives.

Was this dependency intended?  Is this patch okay?


Yes, it's intended: those two tests depend on the per-thread random
number engine in , and so the tests need the same
directives as testsuite/experimental/random/randint.cc

OK for trunk, thanks.

[PATCH] libstdc++/testsuite/experimental/algorithm link errors

[David Edelsohn]

The recent addition of

testsuite/experimental/algorithm/sample-2.cc
testsuite/experimental/algorithm/shuffle.cc

introduced link errors on AIX.  AIX (and Solaris) require additional
options for TLS in some situations.

This patch adds the dejagnu TLS directives.

Was this dependency intended?  Is this patch okay?

Thanks, David

* testsuite/experimental/algorithm/sample-2.cc: Add TLS directives.
* testsuite/experimental/algorithm/shuffle.cc: Likewise.

Index: experimental/algorithm/sample-2.cc
===
--- experimental/algorithm/sample-2.cc  (revision 262161)
+++ experimental/algorithm/sample-2.cc  (working copy)
@@ -16,6 +16,8 @@
 // .

 // { dg-do run { target c++14 } }
+// { dg-require-effective-target tls_runtime }
+// { dg-add-options tls }

 #include 
 #include 

Index: experimental/algorithm/shuffle.cc
===
--- experimental/algorithm/shuffle.cc   (revision 262161)
+++ experimental/algorithm/shuffle.cc   (working copy)
@@ -1,4 +1,6 @@
 // { dg-do run { target c++14 } }
+// { dg-require-effective-target tls_runtime }
+// { dg-add-options tls }

 // Derived from: 2010-03-19  Paolo Carlini  

[PATCH, rs6000] Obvious patch to fix erroneous comment

[Kelvin Nilsen]

In recently committed patch to correct code generation for the vec_packsu 
(vector unsigned long long, vector unsigned long long) built-in function, I 
accidentally left a comment in place that was not relevant to the final patch 
that was committed.

This patch fixes that comment.  After regression testing, I have committed this 
patch as obvious.

gcc/testsuite/ChangeLog:

2018-06-26  Kelvin Nilsen  

* gcc.target/powerpc/builtins-1.c: Correct a comment.

Index: gcc/testsuite/gcc.target/powerpc/builtins-1.c
===
--- gcc/testsuite/gcc.target/powerpc/builtins-1.c   (revision 262149)
+++ gcc/testsuite/gcc.target/powerpc/builtins-1.c   (working copy)
@@ -288,7 +288,7 @@ int main ()
vec_mul mulld | mullw, mulhwu
vec_nor xxlnor
vec_or  xxlor
-   vec_packsu  vpkudus (matches twice due to -dp option)
+   vec_packsu  vpkudus
vec_perm vperm
vec_round xvrdpi
vec_sel xxsel

[line-map patch] Better packing of maps

[Nathan Sidwell]

I've been wandering around the line-map machinery on the modules branch. 
 One thing I noticed was the padding of the line_map type hierarchy was 
unfortunate.  That made me sad.  This patch fixes that.


Rather than keep the the reason field in line_map, I move it to 
line_map_ordinary.  That allows line_map to be a 4 byte struct, rather 
than 5 bytes data and 3 padding.  We can use the source_location it 
contains to distinguish between ordinary and macro maps, by moving the 
LINE_MAP_MAX_LOCATION constant into the header file.  While one 
interpretation of the comments is that the macro and ordinary line maps 
can grow arbitrarily, until they meet, the actual code presumes the 
boundary is fixed, so this is not a new restriction. (see the assert in 
linemap_enter_macro for instance)


Reordering the fields in the ordinary and macro map objects continues 
the reduction in padding.  On a 64-bit target the ordinary map goes from 
32 to 24 bytes.


booted & tested on x86_64-linux.  I'll commit in a few days, unless 
there are objections (it is part of libcpp, but has an explicit 
maintainer listed).


nathan
--
Nathan Sidwell
2018-06-26  Nathan Sidwell  

	Reorg line_map data structures for better packing.
	* include/line-map.h (enum lc_reason): Add LC_HWM.
	(LINE_MAP_MAX_LOCATION): Define here.
	(struct line_map): Move reason field to line_map_ordinary.  Adjust
	GTY tagging.
	(struct line_map_ordinary): Reorder fields for less padding.
	(struct line_map_macro): Likewise.
	(MAP_ORDINARY_P): New.
	(linemap_check_ordinary, linemap_check_macro): Adjust.
	* line-map.c (LINE_MAP_MAX_SOURCE_LOCATION): Delete.
	(new_linemap): Take start_location, not reason.  Adjust.
	(linemap_add, linemap_enter_macro): Adjust.
	(linemap_line_start): Likewise.
	(linemap_macro_expansion_map_p): Use MAP_ORDINARY_P.
	(linemap_macro_loc_to_spelling_point): Likewise.
	(linemap_macro_loc_to_def_point): Likewise.
	(linemap_dump): Likewise.

Index: include/line-map.h
===
--- include/line-map.h	(revision 262147)
+++ include/line-map.h	(working copy)
@@ -74,8 +74,9 @@ enum lc_reason
   LC_LEAVE,
   LC_RENAME,
   LC_RENAME_VERBATIM,
-  LC_ENTER_MACRO
+  LC_ENTER_MACRO,
   /* FIXME: add support for stringize and paste.  */
+  LC_HWM /* High Water Mark.  */
 };
 
 /* The typedef "source_location" is a key within the location database,
@@ -168,7 +169,7 @@ enum lc_reason
  |   Beyond this point, ordinary linemaps have 0 bits per column:
  |   each increment of the value corresponds to a new source line.
  |
-  0x7000 | LINE_MAP_MAX_SOURCE_LOCATION
+  0x7000 | LINE_MAP_MAX_LOCATION
  |   Beyond the point, we give up on ordinary maps; attempts to
  |   create locations in them lead to UNKNOWN_LOCATION (0).
  |
@@ -307,6 +308,9 @@ const source_location LINE_MAP_MAX_LOCAT
  gcc.dg/plugin/location-overflow-test-*.c.  */
 const source_location LINE_MAP_MAX_LOCATION_WITH_COLS = 0x6000;
 
+/* Highest possible source location encoded within an ordinary map.  */
+const source_location LINE_MAP_MAX_LOCATION = 0x7000;
+
 /* A range of source locations.
 
Ranges are closed:
@@ -377,11 +381,13 @@ typedef size_t (*line_map_round_alloc_si
location of the expansion point of PLUS. That location is mapped in
the map that is active right before the location of the invocation
of PLUS.  */
-struct GTY((tag ("0"), desc ("%h.reason == LC_ENTER_MACRO ? 2 : 1"))) line_map {
+
+/* This contains GTY mark-up to support precompiled headers.
+   line_map is an abstract class, only derived objects exist.  */
+struct GTY((tag ("0"), desc ("MAP_ORDINARY_P (&%h) ? 1 : 2"))) line_map {
   source_location start_location;
 
-  /* The reason for creation of this line map.  */
-  ENUM_BITFIELD (lc_reason) reason : CHAR_BIT;
+  /* Size and alignment is (usually) 4 bytes.  */
 };
 
 /* An ordinary line map encodes physical source locations. Those
@@ -397,13 +403,12 @@ struct GTY((tag ("0"), desc ("%h.reason
 
The highest possible source location is MAX_SOURCE_LOCATION.  */
 struct GTY((tag ("1"))) line_map_ordinary : public line_map {
-  const char *to_file;
-  linenum_type to_line;
+  /* Base class is 4 bytes.  */
 
-  /* An index into the set that gives the line mapping at whose end
- the current one was included.  File(s) at the bottom of the
- include stack have this set to -1.  */
-  int included_from;
+  /* 4 bytes of integers, each 1 byte for easy extraction/insertion.  */
+
+  /* The reason for creation of this line map.  */
+  ENUM_BITFIELD (lc_reason) reason : 8;
 
   /* SYSP is one for a system header, two for a C system header file
  that therefore needs to be extern "C" protected in C++, and zero
@@ -429,6 +434,18 @@ struct GTY((tag ("1"))) line_map_ordinar
  | |(e.g. 7)   |   (e.g. 5)|
  

Re: [PATCH] Fix bootstrap failure in vect_analyze_loop

[Jeff Law]

On 06/26/2018 02:06 AM, Richard Biener wrote:
> On Tue, Jun 26, 2018 at 2:21 AM David Malcolm  wrote:
>>
>> I ran into this bootstrap failure (with r262092):
>>
>> ../../../src/gcc/tree-vect-loop.c: In function ‘_loop_vec_info* 
>> vect_analyze_loop(loop*, loop_vec_info, vec_info_shared*)’:
>> ../../../src/gcc/tree-vect-loop.c:1946:25: error: ‘n_stmts’ may be used 
>> uninitialized in this function [-Werror=maybe-uninitialize ]
>>ok = vect_analyze_slp (loop_vinfo, *n_stmts);
>> ~^~
>> ../../../src/gcc/tree-vect-loop.c:2318:12: note: ‘n_stmts’ was declared here
>>unsigned n_stmts;
>> ^~~
>> cc1plus: all warnings being treated as errors
>>
>> It looks like it's inlining vect_analyze_loop_2 into vect_analyze_loop,
>> passing _stmts in by pointer.
>>
>> Normally, vect_get_datarefs_in_loop writes:
>>   *n_stmts = 0;
>> when
>>   if (!LOOP_VINFO_DATAREFS (loop_vinfo).exists ())
>> but not in the "else" path, and then, after lots of complex logic:
>>
>>   ok = vect_analyze_slp (loop_vinfo, *n_stmts);
>>
>> it uses the value in vect_analyze_loop_2, passed in via _stmts.
>>
>> So it's not at all clear to me (or the compiler) that the value is
>> initialized in all paths, and an initialization to zero seems a
>> reasonable fix.
>>
>> OK for trunk, assuming the bootstrap succeeds this time?
> 
> I already applied the very same fix, sorry for the breakage.
And I'll confirm the tester is happy again.
jeff

Re: Limit Debug mode impact: overload __niter_base

[Jonathan Wakely]

On 18/06/18 23:01 +0200, François Dumont wrote:

Hi

    I abandon the idea of providing Debug algos, it would be too much 
code to add and maintain. However I haven't quit on reducing Debug 
mode performance impact.


    So this patch make use of the existing std::__niter_base to get 
rid of the debug layer around __gnu_debug::vector<>::iterator so that 
__builtin_memmove replacement can take place.


    As _Safe_iterator<> do not have a constructor taking a pointer I 
change algos implementation so that we do not try to instantiate the 
iterator type ourselves but rather rely on its operators + or -.


    The small drawback is that for std::equal algo where we can't use 
the __glibcxx_can_increment we need to keep the debug layer to make 
sure we don't reach past-the-end iterator. So I had to remove usage of 
__niter_base when in Debug mode, doing so it also disable potential 
usage of __builtin_memcmp when calling std::equal on 
std::__cxx1998::vector<> iterators. A rather rare situation I think.


    Note that I don't know how to test that __builtin_memmove has been 
indeed used. So I've been through some debug sessions to check that.


The attached patch (not fully tested) seems to be a much simpler way
to achieve the same thing. Instead of modifying all the helper
structs, just define a new function to re-wrap the result into the
desired iterator type.



diff --git a/libstdc++-v3/include/debug/stl_iterator.h 
b/libstdc++-v3/include/debug/stl_iterator.h
index a6a2a76..eca7203 100644
--- a/libstdc++-v3/include/debug/stl_iterator.h
+++ b/libstdc++-v3/include/debug/stl_iterator.h
@@ -120,4 +120,19 @@ namespace __gnu_debug
#endif
}

+#if __cplusplus >= 201103L
+namespace std
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+template
+_Iterator
+__niter_base(const __gnu_debug::_Safe_iterator<
+__gnu_cxx::__normal_iterator<_Iterator, _Container>,
+_Sequence>&);
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+#endif


Why is this overload only defined for C++11 and later? I defined it
unconditionally in the attached patch.

What do you think?


diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 022a3f1598b..91d673e6c81 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -415,13 +415,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 __copy_move_a2(istreambuf_iterator<_CharT, char_traits<_CharT> >,
 		   istreambuf_iterator<_CharT, char_traits<_CharT> >, _CharT*);
 
+#ifdef _GLIBCXX_DEBUG
+  template
+inline __gnu_debug::_Safe_iterator<_Base, _Seq>
+__wrap_iter(__gnu_debug::_Safe_iterator<_Base, _Seq> __safe, _From __from)
+{
+  return __gnu_debug::_Safe_iterator<_Base, _Seq>(_Base(__from),
+		   __safe._M_sequence);
+}
+#endif
+
+  template
+inline _To
+__wrap_iter(_To, _From __from)
+{ return _To(__from); }
+
   template
 inline _OI
 __copy_move_a2(_II __first, _II __last, _OI __result)
 {
-  return _OI(std::__copy_move_a<_IsMove>(std::__niter_base(__first),
-	 std::__niter_base(__last),
-	 std::__niter_base(__result)));
+  return std::__wrap_iter(__result,
+	  std::__copy_move_a<_IsMove>(std::__niter_base(__first),
+  std::__niter_base(__last),
+  std::__niter_base(__result)));
 }
 
   /**
@@ -593,9 +609,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 inline _BI2
 __copy_move_backward_a2(_BI1 __first, _BI1 __last, _BI2 __result)
 {
-  return _BI2(std::__copy_move_backward_a<_IsMove>
-		  (std::__niter_base(__first), std::__niter_base(__last),
-		   std::__niter_base(__result)));
+  return std::__wrap_iter(__result,
+	  std::__copy_move_backward_a<_IsMove>(std::__niter_base(__first),
+	   std::__niter_base(__last),
+	   std::__niter_base(__result)));
 }
 
   /**
@@ -785,7 +802,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __glibcxx_function_requires(_OutputIteratorConcept<_OI, _Tp>)
   __glibcxx_requires_can_increment(__first, __n);
 
-  return _OI(std::__fill_n_a(std::__niter_base(__first), __n, __value));
+  return std::__wrap_iter(__first,
+	  std::__fill_n_a(std::__niter_base(__first), __n, __value));
 }
 
   template
diff --git a/libstdc++-v3/include/debug/stl_iterator.h b/libstdc++-v3/include/debug/stl_iterator.h
index a6a2a762766..431293bdec9 100644
--- a/libstdc++-v3/include/debug/stl_iterator.h
+++ b/libstdc++-v3/include/debug/stl_iterator.h
@@ -118,6 +118,19 @@ namespace __gnu_debug
 -> decltype(std::make_move_iterator(__base(__it.base(
 { return std::make_move_iterator(__base(__it.base())); }
 #endif
-}
+} // namespace __gnu_debug
+
+namespace std
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+template
+_Iterator
+__niter_base(const __gnu_debug::_Safe_iterator<
+		 __gnu_cxx::__normal_iterator<_Iterator, _Container>,
+		 _Sequence>&);
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace std
 
 #endif
diff --git 

Re: [PATCH, rs6000] don't use unaligned vsx for memset of less than 32 bytes

[Segher Boessenkool]

Hi!

On Mon, Jun 25, 2018 at 10:41:32AM -0500, Aaron Sawdey wrote:
> In gcc 8 I added support for unaligned vsx in the builtin expansion of
> memset(x,0,y). Turns out that for memset of less than 32 bytes, this
> doesn't really help much, and it also runs into an egregious load-hit-
> store case in CPU2006 components gcc and hmmer.
> 
> This patch reverts to the previous (gcc 7) behavior for memset of 16-31 
> bytes, which is to use vsx stores only if the target is 16 byte
> aligned. For 32 bytes or more, unaligned vsx stores will still be used.
>   Performance testing of the memset expansion shows that not much is
> given up by using scalar stores for 16-31 bytes, and CPU2006 runs show
> the performance regression is fixed.
> 
> Regstrap passes on powerpc64le, ok for trunk and backport to 8?

Yes, okay for both.  Thanks!


Segher


> 2018-06-25  Aaron Sawdey  
> 
>   * config/rs6000/rs6000-string.c (expand_block_clear): Don't use
>   unaligned vsx for 16B memset.

[PATCH] -fopt-info: add indentation via DUMP_VECT_SCOPE

[David Malcolm]

This patch adds a concept of nested "scopes" to dumpfile.c's dump_*_loc
calls, and wires it up to the DUMP_VECT_SCOPE macro in tree-vectorizer.h,
so that the nested structure is shown in -fopt-info by indentation.

For example, this converts -fopt-info-all e.g. from:

test.c:8:3: note: === analyzing loop ===
test.c:8:3: note: === analyze_loop_nest ===
test.c:8:3: note: === vect_analyze_loop_form ===
test.c:8:3: note: === get_loop_niters ===
test.c:8:3: note: symbolic number of iterations is (unsigned int) n_9(D)
test.c:8:3: note: not vectorized: loop contains function calls or data 
references that cannot be analyzed
test.c:8:3: note: vectorized 0 loops in function

to:

test.c:8:3: note: === analyzing loop ===
test.c:8:3: note:  === analyze_loop_nest ===
test.c:8:3: note:   === vect_analyze_loop_form ===
test.c:8:3: note:=== get_loop_niters ===
test.c:8:3: note:   symbolic number of iterations is (unsigned int) n_9(D)
test.c:8:3: note:   not vectorized: loop contains function calls or data 
references that cannot be analyzed
test.c:8:3: note: vectorized 0 loops in function

showing that the "symbolic number of iterations" message is within
the "=== analyze_loop_nest ===" (and not within the
"=== vect_analyze_loop_form ===").

This is also enabling work for followups involving optimization records
(allowing the records to directly capture the nested structure of the
dump messages).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
* dumpfile.c (dump_loc): Add indentation based on scope depth.
(dump_scope_depth): New variable.
(get_dump_scope_depth): New function.
(dump_begin_scope): New function.
(dump_end_scope): New function.
* dumpfile.h (get_dump_scope_depth): New declaration.
(dump_begin_scope): New declaration.
(dump_end_scope): New declaration.
(class auto_dump_scope): New class.
(AUTO_DUMP_SCOPE): New macro.
* tree-vectorizer.h (DUMP_VECT_SCOPE): Reimplement in terms of
AUTO_DUMP_SCOPE.
---
 gcc/dumpfile.c| 35 +++
 gcc/dumpfile.h| 39 +++
 gcc/tree-vectorizer.h | 15 ---
 3 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 122e420..190b52d 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -419,6 +419,8 @@ dump_loc (dump_flags_t dump_kind, FILE *dfile, 
source_location loc)
  DECL_SOURCE_FILE (current_function_decl),
  DECL_SOURCE_LINE (current_function_decl),
  DECL_SOURCE_COLUMN (current_function_decl));
+  /* Indentation based on scope depth.  */
+  fprintf (dfile, "%*s", get_dump_scope_depth (), "");
 }
 }
 
@@ -539,6 +541,39 @@ template void dump_dec (dump_flags_t, const poly_uint64 &);
 template void dump_dec (dump_flags_t, const poly_offset_int &);
 template void dump_dec (dump_flags_t, const poly_widest_int &);
 
+/* The current dump scope-nesting depth.  */
+
+static int dump_scope_depth;
+
+/* Get the current dump scope-nesting depth.
+   For use by dump_*_loc (for showing nesting via indentation).  */
+
+unsigned int
+get_dump_scope_depth ()
+{
+  return dump_scope_depth;
+}
+
+/* Push a nested dump scope.
+   Print "=== NAME ===\n" to the dumpfile, if any, and to the -fopt-info
+   destination, if any.
+   Increment the scope depth.  */
+
+void
+dump_begin_scope (const char *name, const dump_location_t )
+{
+  dump_printf_loc (MSG_NOTE, loc, "=== %s ===\n", name);
+  dump_scope_depth++;
+}
+
+/* Pop a nested dump scope.  */
+
+void
+dump_end_scope ()
+{
+  dump_scope_depth--;
+}
+
 /* Start a dump for PHASE. Store user-supplied dump flags in
*FLAG_PTR.  Return the number of streams opened.  Set globals
DUMP_FILE, and ALT_DUMP_FILE to point to the opened streams, and
diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
index 90d8930..89d5c11 100644
--- a/gcc/dumpfile.h
+++ b/gcc/dumpfile.h
@@ -456,6 +456,45 @@ dump_enabled_p (void)
   return (dump_file || alt_dump_file);
 }
 
+/* Managing nested scopes, so that dumps can express the call chain
+   leading to a dump message.  */
+
+extern unsigned int get_dump_scope_depth ();
+extern void dump_begin_scope (const char *name, const dump_location_t );
+extern void dump_end_scope ();
+
+/* Implementation detail of the AUTO_DUMP_SCOPE macro below.
+
+   A RAII-style class intended to make it easy to emit dump
+   information about entering and exiting a collection of nested
+   function calls.  */
+
+class auto_dump_scope
+{
+ public:
+  auto_dump_scope (const char *name, dump_location_t loc)
+  {
+if (dump_enabled_p ())
+  dump_begin_scope (name, loc);
+  }
+  ~auto_dump_scope ()
+  {
+if (dump_enabled_p ())
+  dump_end_scope ();
+  }
+};
+
+/* A macro for calling:
+ dump_begin_scope (NAME, LOC);
+   via an RAII object, thus printing "=== MSG ===\n" to the dumpfile etc,
+   and 

[committed] Introduce dump_location_t

[David Malcolm]

On Mon, 2018-06-25 at 15:34 +0200, Richard Biener wrote:
> On Wed, Jun 20, 2018 at 6:34 PM David Malcolm 
> wrote:
> > 
> > Here's v3 of the patch (one big patch this time, rather than a
> > kit).
> > 
> > Like the v2 patch kit, this patch reuses the existing dump API,
> > rather than inventing its own.
> > 
> > Specifically, it uses the dump_* functions in dumpfile.h that don't
> > take a FILE *, the ones that implicitly write to dump_file and/or
> > alt_dump_file.  I needed a name for them, so I've taken to calling
> > them the "structured dump API" (better name ideas welcome).
> > 
> > v3 eliminates v2's optinfo_guard class, instead using "dump_*_loc"
> > calls as delimiters when consolidating "dump_*" calls.  There's a
> > new dump_context class which has responsibility for consolidating
> > them into optimization records.
> > 
> > The dump_*_loc calls now capture more than just a location_t: they
> > capture the profile_count and the location in GCC's own sources
> > where
> > the dump is being emitted from.
> > 
> > This works by introducing a new "dump_location_t" class as the
> > argument of those dump_*_loc calls.  The dump_location_t can
> > be constructed from a gimple * or from an rtx_insn *, so that
> > rather than writing:
> > 
> >   dump_printf_loc (MSG_NOTE, gimple_location (stmt),
> >"some message: %i", 42);
> > 
> > you can write:
> > 
> >   dump_printf_loc (MSG_NOTE, stmt,
> >"some message: %i", 42);
> > 
> > and the dump_location_t constructor will grab the location_t and
> > profile_count of stmt, and the location of the "dump_printf_loc"
> > callsite (and gracefully handle "stmt" being NULL).
> > 
> > Earlier versions of the patch captured the location of the
> > dump_*_loc call via preprocessor hacks, or didn't work properly;
> > this version of the patch works more cleanly: internally,
> > dump_location_t is split into two new classes:
> >   * dump_user_location_t: the location_t and profile_count within
> > the *user's code*, and
> >   * dump_impl_location_t: the __builtin_FILE/LINE/FUNCTION within
> > the *implementation* code (i.e. GCC or a plugin), captured
> > "automagically" via default params
> > 
> > These classes are sometimes used elsewhere in the code.  For
> > example, "vect_location" becomes a dump_user_location_t
> > (location_t and profile_count), so that in e.g:
> > 
> >   vect_location = find_loop_location (loop);
> > 
> > it's capturing the location_t and profile_count, and then when
> > it's used here:
> > 
> >   dump_printf_loc (MSG_NOTE, vect_location, "foo");
> > 
> > the dump_location_t is constructed from the vect_location
> > plus the dump_impl_location_t at that callsite.
> > 
> > In contrast, loop-unroll.c's report_unroll's "locus" param
> > becomes a dump_location_t: we're interested in where it was
> > called from, not in the locations of the various dump_*_loc calls
> > within it.
> > 
> > Previous versions of the patch captured a gimple *, and needed
> > GTY markers; in this patch, the dump_user_location_t is now just a
> > location_t and a profile_count.
> > 
> > The v2 patch added an overload for dump_printf_loc so that you
> > could pass in either a location_t, or the new type; this version
> > of the patch eliminates that: they all now take dump_location_t.
> > 
> > Doing so required adding support for rtx_insn *, so that one can
> > write this kind of thing in RTL passes:
> > 
> >   dump_printf_loc (MSG_NOTE, insn, "foo");
> > 
> > One knock-on effect is that get_loop_location now returns a
> > dump_user_location_t rather than a location_t, so that it has
> > hotness information.
> > 
> > Richi: would you like me to split out this location-handling
> > code into a separate patch?  (It's kind of redundant without
> > adding the remarks and optimization records work, but if that's
> > easier I can do it)
> 
> I think that would be easier because it doesn't require the JSON
> stuff and so I'll happily approve it.
> 
> Thus - trying to review that bits (and sorry for the delay).
> 
> +  location_t srcloc = loc.get_location_t ();
> +
>if (dump_file && (dump_kind & pflags))
>  {
> -  dump_loc (dump_kind, dump_file, loc);
> +  dump_loc (dump_kind, dump_file, srcloc);
>print_gimple_stmt (dump_file, gs, spc, dump_flags |
> extra_dump_flags);
>  }
> 
>if (alt_dump_file && (dump_kind & alt_flags))
>  {
> -  dump_loc (dump_kind, alt_dump_file, loc);
> +  dump_loc (dump_kind, alt_dump_file, srcloc);
>print_gimple_stmt (alt_dump_file, gs, spc, dump_flags |
> extra_dump_flags);
>  }
> +
> +  if (optinfo_enabled_p ())
> +{
> +  optinfo  = begin_next_optinfo (loc);
> +  info.handle_dump_file_kind (dump_kind);
> +  info.add_stmt (gs, extra_dump_flags);
> +}
> 
> seeing this in multiple places.  I seem to remember that
> dump_file / alt_dump_file was suposed to handle dumping
> into two locations - a dump file and optinfo (or stdout).  

Re: [PATCH v3] Change default to -fno-math-errno

[Wilco Dijkstra]

Joseph Myers wrote:
> On Thu, 21 Jun 2018, Jeff Law wrote:
>
> > I think all this implies that the setting of -fno-math-errno by default
> > really depends on the math library in use since it's the library that
> > has to arrange for either errno to get set or for an exception to be raised.
>
> If the library does not set errno, clearly -fno-math-errno by default is 
> appropriate (and is the default on Darwin).

Various librarys no longer set errno nowadays, for example BSD, MUSL, Bionic
etc. For GLIBC 3.0 I'd propose to drop setting of errno as well since very few
applications check errno after math functions that may set it (C89 support
could be added using wrappers that set errno if required).

> If the library does set errno, but the user doesn't care about the errno 
> setting, -fno-math-errno is useful to that user; the question here is 
> whether it's also an appropriate default to assume the user doesn't care 
> about errno setting and so require them to pass -fmath-errno if they do.  
> There are separate cases here for if the library does or does not set 
> exceptions (the latter including most soft-float cases).

For C99 and higher an application could check math_errhandling and report
an error if the libary does not support the required error handling. However
error checks are feasible even if the floating point runtime does not to set
exceptions since the return value will be NaN or Inf (of course in the case
of non-IEEE754 FP all bets are off).

Note errno is not guaranteed to not be set even if there is no error, so you
need to check the return value first before checking errno.

> Then there are various built-in functions in GCC that use 
> ATTR_MATHFN_FPROUNDING when they should use ATTR_MATHFN_FPROUNDING_ERRNO - 
> because there are legitimate error cases for those functions for which 
> errno setting is appropriate.  sin, cos, tan (domain errors for infinite 
> arguments) are obvious examples.  (We have bug 64101 for erf, which is a 
> case where the glibc model of when to set errno for underflow does *not* 
> ever involve setting errno for this function, but some other 
> implementations might have other models for when setting errno is 
> appropriate.)

That looks incorrect indeed but that's mostly a problem with -fmath-errno as it
would result in GCC assuming the function is const/pure when in fact it isn't.
Does ATTR_MATHFN_FPROUNDING imply that errno is dead after the call?

Wilco

Re: std::vector default default and move constructors

[Jonathan Wakely]

On 02/06/18 14:00 +0200, François Dumont wrote:

Hi

    Here is this patch again, I consider all your remarks and also 
made some changes considering feedback on rbtree patch.





+   _Vector_impl(_Tp_alloc_type const& __a) _GLIBCXX_NOEXCEPT
+   : _Tp_alloc_type(__a)
+   { }
+
+#if __cplusplus >= 201103L
+   // Not defaulted to avoid noexcept qualification dependency on the
+   // _Tp_alloc_type move constructor one.


Could you please rephrase this comment as:

   // Not defaulted, to enforce noexcept(true) even when
   // !is_nothrow_move_constructible<_Tp_alloc_type>.

I prefer this wording, because most allocators don't have a move
constructor at all (just a copy constructor) so talking about its move
constructor is misleading.


+   _Vector_impl(_Vector_impl&& __x) noexcept
+   : _Tp_alloc_type(std::move(__x)), _Vector_impl_data(std::move(__x))
+   { }
+
+   _Vector_impl(_Tp_alloc_type&& __a) noexcept
+   : _Tp_alloc_type(std::move(__a))
+   { }
+
+   _Vector_impl(_Tp_alloc_type&& __a, _Vector_impl&& __rv) noexcept
+   : _Tp_alloc_type(std::move(__a)), _Vector_impl_data(std::move(__rv))
+   { }
+#endif

#if _GLIBCXX_SANITIZE_STD_ALLOCATOR && _GLIBCXX_SANITIZE_VECTOR
template
@@ -235,38 +259,42 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER

  _Tp_alloc_type&
  _M_get_Tp_allocator() _GLIBCXX_NOEXCEPT
-  { return *static_cast<_Tp_alloc_type*>(>_M_impl); }
+  { return this->_M_impl; }

  const _Tp_alloc_type&
  _M_get_Tp_allocator() const _GLIBCXX_NOEXCEPT
-  { return *static_cast(>_M_impl); }
+  { return this->_M_impl; }

  allocator_type
  get_allocator() const _GLIBCXX_NOEXCEPT
  { return allocator_type(_M_get_Tp_allocator()); }

-  _Vector_base()
-  : _M_impl() { }
+#if __cplusplus >= 201103L
+  _Vector_base() = default;
+#else
+  _Vector_base() { }
+#endif

  _Vector_base(const allocator_type& __a) _GLIBCXX_NOEXCEPT
  : _M_impl(__a) { }


Please add "// Kept for ABI compatibility" before this #if:


+#if !_GLIBCXX_INLINE_VERSION
  _Vector_base(size_t __n)
  : _M_impl()
  { _M_create_storage(__n); }
+#endif

  _Vector_base(size_t __n, const allocator_type& __a)
  : _M_impl(__a)
  { _M_create_storage(__n); }

#if __cplusplus >= 201103L
+  _Vector_base(_Vector_base&&) = default;
+


And here too:


+# if !_GLIBCXX_INLINE_VERSION
  _Vector_base(_Tp_alloc_type&& __a) noexcept
  : _M_impl(std::move(__a)) { }



OK for trunk with those three comment changes.

Thanks for your patience waiting for the review.

Documentation: -gsplit-dwarf missing from Option Summary

[Stephan Bergmann]

Cannot identify what sorting is used for that list (if any), so putting 
it at the end (and -gsplit-dwarf also follows -fvar-tracking-assignments 
in the "Options for Debugging Your Program" section).



Author: Stephan Bergmann 
Date:   Tue Jun 26 15:04:54 2018 +0200

Documentation: -gsplit-dwarf missing from Option Summary

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index f231da3cde2..3f74b9ac336 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -360,7 +360,7 @@ Objective-C and Objective-C++ Dialects}.
 -femit-struct-debug-detailed@r{[}=@var{spec-list}@r{]} @gol
 -feliminate-unused-debug-symbols  -femit-class-debug-always @gol
 -fno-merge-debug-strings  -fno-dwarf2-cfi-asm @gol
--fvar-tracking  -fvar-tracking-assignments}
+-fvar-tracking  -fvar-tracking-assignments -gsplit-dwarf}

 @item Optimization Options
 @xref{Optimize Options,,Options that Control Optimization}.

Re: [RFC PATCH 0/6] some vxworks/powerpc patches

[Rasmus Villemoes]

On 2018-06-19 18:45, Olivier Hainque wrote:
> Hi Rasmus,
> 
> 
> 1/6:
>>  vxworks: add target/h/wrn/coreip to the set of system include paths
> 
> As we discussed on the message dedicated to this patch, this
> one is ok.
> 
> For the rest, I still have concerns for part of the patches. Some
> would be ok modulo minor changes.
> 
> Before we get there: are you familiar with the "munch" facility
> of VxWorks ?
> 
> That can offer alternatives to some of the things you were after.

Yes, but doing regexp match on nm output feels wrong (though I am aware
that that is how things have been done for decades), and I really prefer
using the modern methods, which also allows e.g. use of constructor
priorities.

> 3/6:
>>  vxworks: enable use of .init_array/.fini_array for cdtors
> 
> Assuming we agree on the general approach, this part would
> be fine modulo adding parens around the expression. It doesn't
> do what you think it does in the current form.

Ah, of course. I'll add parens for the next roll.

> I'm a bit worried that forcing .init_array for constructors
> might not work for RTPs, at least on some versions of VxWorks.
> 
> We probably would need to fallback to ".ctors" as the section
> base name for not so recent versions. I'm not clear on all the
> ramifications there.

Hm, but if somebody built a compiler with --enable-initfini-array today
and passed -mrtp to it, they would already get .init_array instead of
.ctors, no? I.e., initfini-array.h would be included by tm.h after
vxworks.h, and the logic there would unconditionally override
vxworks.h's setting of TARGET_ASM_CONSTRUCTOR. So this "forcing" seems
to happen already for such a compiler, regardless of my patch?

> Some of the crt related patches are too potentially damaging
> for other modes of possible uses of a VxWorks toolchain, with
> possibly other versions of VxWorks. 

OK.

> I understand it works for you, that --enable-initfini-array is
> a new mode of use and that users would have the choice of not
> configuring with this.
> 
> I still think that we don't want to bind that configuration choice
> to something too specific.

OK.

> The more we enforce from that config choice, the trickier it will
> get to be able to understand/explain what it means. The least we enforce,
> the easier it will be to maintain and understand, and the more flexible
> it will be for users.
> 
> I'll propose alternatives to the crt parts that I'd rather not get
> in (below).
> 
> 6/6:
>>  vxworks: don't define vxworks_asm_out_constructor when using .init_array
> 
> Would be ok together with the previous one, and as long as we don't
> uncover fallouts for RTPs (meaning, we might have to revisit this in the
> future).

See above, I don't see how that could happen. Patch 6/6 is essentially a
no-op, except that it makes the compiler binary a tiny bit smaller.

> 2/6:
>>  libgcc: add crt{begin,end} for powerpc-wrs-vxworks target
> 
> That one would be fine as well. Even if we don't immediately add them
> to link closures from the config files (see below), I see how handy it
> can be to be able to reference them from user specs for instance.

Yes, I hope that building and installing crtbegin/end should be
harmless; it just requires that crtstuff.c actually compiles cleanly.

> 4 and 5/6
>>  powerpc/vxworks: add {START,END}FILE_SPEC for HAVE_INITFINI_ARRAY_SUPPORT
>>  powerpc/vxworks: [hack] add crti.o file
> 
> I really think those two should stay out. As I mentioned in a previous 
> message,
> there are many possible uses of a toolchain and this (unconditional addition
> of crtfiles and references to _ctors) would undoubtedly cause trouble in one
> case or another.
> 
> Also, the reference to _ctors tie to strong assumptions about how the
> environment arranges for .init_array to work, so really blurs the 
> responsibility
> assignments.
> 
> We might be able to come up with a proper set of options controlling what
> crt file to include in such or such circumstance but we're not quite there 
> yet.
> 
> In the meantime, I think you could get things to work with a custom
> userland spec and local crti.o (or alike, on your end so you can name and
> locate it as you wish).

Yes, these two are the most hackish ones, and I don't much like them
myself. So I'm happy to leave them out of mainline, and just use them
(or something equivalent) in-house.

> You could have, for example a spec file like this (sketch, to give an idea):
> 
> *startfile:
> + vx_ctors-begin.o crtbegin.o%s
> 
> where vx_ctors-begin.c would hold something like:
> 
>   void (* volatile _ctors[])()
> __attribute__((section (".init_array"))) = {};
> 
> with possible adjustments to priority sections and synchronization
> with your linker script.
> 
> You can handle the sentinel addition likewise, with a '*endfile'
> spec, or leave this to your linker script.

Using a custom spec file, custom linker script and something like the
vx_ctors_begin.o will probably be the long-term solution for us. We'd be
really 

[PATCH] Adjust subprogram DIE re-usal

[Richard Biener]

A patch from Honza not LTO streaming DECL_ORIGINAL_TYPE ends up
ICEing during LTO bootstrap because we end up not re-using the
DIE we create late during LTRANS for a subprogram because its
parent is a namespace rather than a CU DIE (or a module).

I'm wondering of the general reason why we enforce (inconsistently)
"We always want the DIE for this function that has the *_pc attributes to 
be under comp_unit_die so the debugger can find it."
We indeed generate a specification DIE rooted at the CU in addition to the
declaration DIE inside the namespace for sth as simple as

namespace Foo { void foo () {} }

anyhow - the comment also says "We also need to do this [re-use the DIE]
for abstract instances of inlines, since the spec requires the out-of-line
copy to have the same parent.".  Not sure what condition this part of
the comment applies to.

So my fix is to move the || get_AT (old_die, DW_AT_abstract_origin)
check I added for early LTO debug to a global override - forcing
DIE re-use for any DIE with an abstract origin set.  That is, all
concrete instances are fine where they are.  That also avoids
double-indirection DW_AT_specification -> DW_AT_abstract_origin -> DIE.

But as said, I wonder about the overall condition, esp. the
DW_TAG_module special-casing (but not at the same time
special-casing DW_TAG_namespace or DW_TAG_partial_unit).

LTO bootstrap is in progress on x86_64-unknown-linux-gnu.

OK if that succeeds?

Thanks,
Richard.

2018-06-26  Richard Biener  

* dwarf2out.c (gen_subprogram_die): Always re-use DIEs with an
DW_AT_abstract_origin attribute.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 262132)
+++ gcc/dwarf2out.c (working copy)
@@ -22780,26 +22780,25 @@ gen_subprogram_die (tree decl, dw_die_re
 apply; we just use the old DIE.  */
   expanded_location s = expand_location (DECL_SOURCE_LOCATION (decl));
   struct dwarf_file_data * file_index = lookup_filename (s.file);
-  if ((is_cu_die (old_die->die_parent)
-  /* This condition fixes the inconsistency/ICE with the
- following Fortran test (or some derivative thereof) while
- building libgfortran:
+  if (((is_cu_die (old_die->die_parent)
+   /* This condition fixes the inconsistency/ICE with the
+  following Fortran test (or some derivative thereof) while
+  building libgfortran:
 
-module some_m
-contains
-   logical function funky (FLAG)
- funky = .true.
-  end function
-end module
-  */
-  || (old_die->die_parent
-  && old_die->die_parent->die_tag == DW_TAG_module)
-  || context_die == NULL)
+ module some_m
+ contains
+logical function funky (FLAG)
+  funky = .true.
+   end function
+ end module
+*/
+   || (old_die->die_parent
+   && old_die->die_parent->die_tag == DW_TAG_module)
+   || context_die == NULL)
   && (DECL_ARTIFICIAL (decl)
   /* The location attributes may be in the abstract origin
  which in the case of LTO might be not available to
  look at.  */
-  || get_AT (old_die, DW_AT_abstract_origin)
   || (get_AT_file (old_die, DW_AT_decl_file) == file_index
   && (get_AT_unsigned (old_die, DW_AT_decl_line)
   == (unsigned) s.line)
@@ -22807,6 +22806,10 @@ gen_subprogram_die (tree decl, dw_die_re
   || s.column == 0
   || (get_AT_unsigned (old_die, DW_AT_decl_column)
   == (unsigned) s.column)
+ /* With LTO if there's an abstract instance for
+the old DIE, this is a concrete instance and
+thus re-use the DIE.  */
+ || get_AT (old_die, DW_AT_abstract_origin))
{
  subr_die = old_die;
 

Re: [PATCH], PowerPC long double transition patches, v2, Patch #3 (use correct way to get the IEEE 128-bit complex type)

[Joseph Myers]

On Sat, 23 Jun 2018, Segher Boessenkool wrote:

> On Thu, Jun 21, 2018 at 10:43:16PM +, Joseph Myers wrote:
> > On Thu, 21 Jun 2018, Segher Boessenkool wrote:
> > 
> > > The comment doesn't make much sense.  If long double is IBM, the long
> > > double complex mode is ICmode, not KCmode.  "To get the IEEE128 complex
> > > type", perhaps?  And can't you do _Complex __ieee128 or such?
> > 
> > You can do _Complex _Float128, in C only, not C++ (which doesn't have the 
> > _Float128 keyword).  _Complex can be used with keywords for type names, 
> > but not with a typedef name (built-in or otherwise, see discussion in bug 
> > 32187).
> 
> __ieee128 is a type, not a typedef:
> 
> lang_hooks.types.register_builtin_type (ieee128_float_type_node, "__ieee128");

A type registered with register_builtin_type is effectively a built-in 
typedef name and is handled in the front ends through the same code paths 
as typedef names (as opposed to types made up of type-specifier keywords, 
which are handled by custom logic in the front ends that maps multisets of 
such keywords to the corresponding types).

It would be possible, as discussed in bug 32187, to handle such types 
differently (so allowing _Complex together with __ieee128 while still not 
allowing it together with a user-defined typedef), but that's not the 
current situation.

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

Re: [PATCH] Default special members of regex types and add noexcept

[Jonathan Wakely]

On 26/06/18 09:19 +0200, Stephan Bergmann wrote:

On 08/06/18 13:01, Jonathan Wakely wrote:

Nothing very exciting, just adding noexcept and defaulting some
members.

* include/bits/regex.h (sub_match): Add noexcept to default
constructor and length observer.
(match_results): Add noexcept to default constructor and observers
with no preconditions. Define destructor as defaulted.
(operator==, operator!=, swap): Add noexcept.
(regex_iterator): Add default member initializers and define default
constructor and destructor as defaulted. Add noexcept to equality
and dereference operators.

Tested powerpc64le-linux, committed to trunk.


Looks like that forgot to also add noexcept to the definition of


   regex_iterator<_Bi_iter, _Ch_type, _Rx_traits>::
   operator==(const regex_iterator& __rhs) const


in libstdc++-v3/include/bits/regex.tcc?


Indeed I did, thanks.

Tested x86_64-linux, committed to trunk.


commit a3371eeaf2434550934a95f4f41a2a9ae31ead81
Author: Jonathan Wakely 
Date:   Tue Jun 26 13:01:56 2018 +0100

Add missing noexcept on definition to match declaration

* include/bits/regex.tcc (regex_iterator::operator==): Add missing
noexcept.

diff --git a/libstdc++-v3/include/bits/regex.tcc b/libstdc++-v3/include/bits/regex.tcc
index b92edb9ab29..dcf660902bc 100644
--- a/libstdc++-v3/include/bits/regex.tcc
+++ b/libstdc++-v3/include/bits/regex.tcc
@@ -500,7 +500,7 @@ namespace __detail
 	   typename _Rx_traits>
 bool
 regex_iterator<_Bi_iter, _Ch_type, _Rx_traits>::
-operator==(const regex_iterator& __rhs) const
+operator==(const regex_iterator& __rhs) const noexcept
 {
   if (_M_pregex == nullptr && __rhs._M_pregex == nullptr)
 	return true;

[PATCH] fixincludes: Add missing hunk to tests/base/ioLib.h

[Rasmus Villemoes]

When adding the vxworks_iolib_include_unistd hack I failed to add the
appropriate hunk to the tests/base/ioLib.h file, causing "make
check-fixincludes" to fail.

OK for trunk?

2018-06-26  Rasmus Villemoes  

fixincludes/

* tests/base/ioLib.h [VXWORKS_IOLIB_INCLUDE_UNISTD_CHECK]: Add
  missing hunk.
---
 fixincludes/tests/base/ioLib.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/fixincludes/tests/base/ioLib.h b/fixincludes/tests/base/ioLib.h
index d570c895d57..a3ff6c1eb86 100644
--- a/fixincludes/tests/base/ioLib.h
+++ b/fixincludes/tests/base/ioLib.h
@@ -17,3 +17,8 @@ extern int ioctl ( int asdf1234, int jkl , int qwerty ) ;
 #if defined( VXWORKS_WRITE_CONST_CHECK )
 extern int  write (int, const char*, size_t);
 #endif  /* VXWORKS_WRITE_CONST_CHECK */
+
+
+#if defined( VXWORKS_IOLIB_INCLUDE_UNISTD_CHECK )
+#include 
+#endif  /* VXWORKS_IOLIB_INCLUDE_UNISTD_CHECK */
-- 
2.16.4

Re: [PATCH] Fix x86 setcc + movzbl peephole2s (PR target/86314)

[Uros Bizjak]

On Tue, Jun 26, 2018 at 12:52 PM, Jakub Jelinek  wrote:
> Hi!
>
> These peephole2s assume that when matching insns like:
>   [(parallel [(set (reg FLAGS_REG) (match_operand 0))
>   (match_operand 4)])
> that operands[4] must be a set of a register with operands[0]
> as SET_SRC, but that isn't the case e.g. for:
> (insn 9 8 10 2 (parallel [
> (set (reg:CCC 17 flags)
> (compare:CCC (unspec_volatile:DI [
> (mem/v:DI (reg/v/f:DI 5 di [orig:94 p ] [94]) [-1 
>  S8 A64])
> (const_int 5 [0x5])
> ] UNSPECV_XCHG)
> (const_int 0 [0])))
> (set (zero_extract:DI (mem/v:DI (reg/v/f:DI 5 di [orig:94 p ] 
> [94]) [-1  S8 A64])
> (const_int 1 [0x1])
> (reg:DI 0 ax [96]))
> (const_int 1 [0x1]))
> ]) "pr86314.C":7 5268 {atomic_bit_test_and_setdi_1}
>  (expr_list:REG_DEAD (reg/v/f:DI 5 di [orig:94 p ] [94])
> (expr_list:REG_DEAD (reg:DI 0 ax [96])
> (nil
>
> As we want to clear operands[3] before this instruction, we need to
> verify that it isn't used in operands[0] (we check that) and that
> operands[4] does not set it (also checked), but also need to check that
> operands[4] doesn't use it (which we don't do).  In the usual case
> when SET_DEST of operands[4] is a REG and SET_SRC is operands[0] it
> makes no difference, but if SET_DEST sets something that uses operands[3]
> in it, or if SET_SRC is different from operands[0] and uses operands[3],
> then this results in a wrong peephole2 transformation.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk and release branches?
>
> 2018-06-26  Jakub Jelinek  
>
> PR target/86314
> * config/i386/i386.md (setcc + movzbl to xor + setcc peephole2s):
> Check reg_overlap_mentioned_p in addition to reg_set_p with the same
> operands.
>
> * gcc.dg/pr86314.c: New test.

OK everywhere.

Thanks,
Uros.

> --- gcc/config/i386/i386.md.jj  2018-06-25 14:51:24.878990827 +0200
> +++ gcc/config/i386/i386.md 2018-06-26 10:01:43.042907384 +0200
> @@ -12761,6 +12761,7 @@ (define_peephole2
>"(peep2_reg_dead_p (3, operands[1])
>  || operands_match_p (operands[1], operands[3]))
> && ! reg_overlap_mentioned_p (operands[3], operands[0])
> +   && ! reg_overlap_mentioned_p (operands[3], operands[4])
> && ! reg_set_p (operands[3], operands[4])
> && peep2_regno_dead_p (0, FLAGS_REG)"
>[(parallel [(set (match_dup 5) (match_dup 0))
> @@ -12786,6 +12787,7 @@ (define_peephole2
>  || operands_match_p (operands[2], operands[4]))
> && ! reg_overlap_mentioned_p (operands[4], operands[0])
> && ! reg_overlap_mentioned_p (operands[4], operands[1])
> +   && ! reg_overlap_mentioned_p (operands[4], operands[5])
> && ! reg_set_p (operands[4], operands[5])
> && refers_to_regno_p (FLAGS_REG, operands[1], (rtx *)NULL)
> && peep2_regno_dead_p (0, FLAGS_REG)"
> @@ -12835,6 +12837,7 @@ (define_peephole2
>"(peep2_reg_dead_p (3, operands[1])
>  || operands_match_p (operands[1], operands[3]))
> && ! reg_overlap_mentioned_p (operands[3], operands[0])
> +   && ! reg_overlap_mentioned_p (operands[3], operands[4])
> && ! reg_set_p (operands[3], operands[4])
> && peep2_regno_dead_p (0, FLAGS_REG)"
>[(parallel [(set (match_dup 5) (match_dup 0))
> @@ -12861,6 +12864,7 @@ (define_peephole2
>  || operands_match_p (operands[2], operands[4]))
> && ! reg_overlap_mentioned_p (operands[4], operands[0])
> && ! reg_overlap_mentioned_p (operands[4], operands[1])
> +   && ! reg_overlap_mentioned_p (operands[4], operands[5])
> && ! reg_set_p (operands[4], operands[5])
> && refers_to_regno_p (FLAGS_REG, operands[1], (rtx *)NULL)
> && peep2_regno_dead_p (0, FLAGS_REG)"
> --- gcc/testsuite/gcc.dg/pr86314.c.jj   2018-06-26 10:25:35.190160477 +0200
> +++ gcc/testsuite/gcc.dg/pr86314.c  2018-06-26 10:24:42.310077331 +0200
> @@ -0,0 +1,20 @@
> +// PR target/86314
> +// { dg-do run { target sync_int_long } }
> +// { dg-options "-O2" }
> +
> +__attribute__((noinline, noclone)) unsigned long
> +foo (unsigned long *p)
> +{
> +  unsigned long m = 1UL << ((*p & 1) ? 1 : 0);
> +  unsigned long n = __atomic_fetch_or (p, m, __ATOMIC_SEQ_CST);
> +  return (n & m) == 0;
> +}
> +
> +int
> +main ()
> +{
> +  unsigned long v = 1;
> +  if (foo () != 1)
> +__builtin_abort ();
> +  return 0;
> +}
>
> Jakub

[PATCH] Fix x86 setcc + movzbl peephole2s (PR target/86314)

[Jakub Jelinek]

Hi!

These peephole2s assume that when matching insns like:
  [(parallel [(set (reg FLAGS_REG) (match_operand 0))
  (match_operand 4)])
that operands[4] must be a set of a register with operands[0]
as SET_SRC, but that isn't the case e.g. for:
(insn 9 8 10 2 (parallel [
(set (reg:CCC 17 flags)
(compare:CCC (unspec_volatile:DI [
(mem/v:DI (reg/v/f:DI 5 di [orig:94 p ] [94]) [-1  
S8 A64])
(const_int 5 [0x5])
] UNSPECV_XCHG)
(const_int 0 [0])))
(set (zero_extract:DI (mem/v:DI (reg/v/f:DI 5 di [orig:94 p ] [94]) 
[-1  S8 A64])
(const_int 1 [0x1])
(reg:DI 0 ax [96]))
(const_int 1 [0x1]))
]) "pr86314.C":7 5268 {atomic_bit_test_and_setdi_1}
 (expr_list:REG_DEAD (reg/v/f:DI 5 di [orig:94 p ] [94])
(expr_list:REG_DEAD (reg:DI 0 ax [96])
(nil

As we want to clear operands[3] before this instruction, we need to
verify that it isn't used in operands[0] (we check that) and that
operands[4] does not set it (also checked), but also need to check that
operands[4] doesn't use it (which we don't do).  In the usual case
when SET_DEST of operands[4] is a REG and SET_SRC is operands[0] it
makes no difference, but if SET_DEST sets something that uses operands[3]
in it, or if SET_SRC is different from operands[0] and uses operands[3],
then this results in a wrong peephole2 transformation.

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

2018-06-26  Jakub Jelinek  

PR target/86314
* config/i386/i386.md (setcc + movzbl to xor + setcc peephole2s):
Check reg_overlap_mentioned_p in addition to reg_set_p with the same
operands.

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

--- gcc/config/i386/i386.md.jj  2018-06-25 14:51:24.878990827 +0200
+++ gcc/config/i386/i386.md 2018-06-26 10:01:43.042907384 +0200
@@ -12761,6 +12761,7 @@ (define_peephole2
   "(peep2_reg_dead_p (3, operands[1])
 || operands_match_p (operands[1], operands[3]))
&& ! reg_overlap_mentioned_p (operands[3], operands[0])
+   && ! reg_overlap_mentioned_p (operands[3], operands[4])
&& ! reg_set_p (operands[3], operands[4])
&& peep2_regno_dead_p (0, FLAGS_REG)"
   [(parallel [(set (match_dup 5) (match_dup 0))
@@ -12786,6 +12787,7 @@ (define_peephole2
 || operands_match_p (operands[2], operands[4]))
&& ! reg_overlap_mentioned_p (operands[4], operands[0])
&& ! reg_overlap_mentioned_p (operands[4], operands[1])
+   && ! reg_overlap_mentioned_p (operands[4], operands[5])
&& ! reg_set_p (operands[4], operands[5])
&& refers_to_regno_p (FLAGS_REG, operands[1], (rtx *)NULL)
&& peep2_regno_dead_p (0, FLAGS_REG)"
@@ -12835,6 +12837,7 @@ (define_peephole2
   "(peep2_reg_dead_p (3, operands[1])
 || operands_match_p (operands[1], operands[3]))
&& ! reg_overlap_mentioned_p (operands[3], operands[0])
+   && ! reg_overlap_mentioned_p (operands[3], operands[4])
&& ! reg_set_p (operands[3], operands[4])
&& peep2_regno_dead_p (0, FLAGS_REG)"
   [(parallel [(set (match_dup 5) (match_dup 0))
@@ -12861,6 +12864,7 @@ (define_peephole2
 || operands_match_p (operands[2], operands[4]))
&& ! reg_overlap_mentioned_p (operands[4], operands[0])
&& ! reg_overlap_mentioned_p (operands[4], operands[1])
+   && ! reg_overlap_mentioned_p (operands[4], operands[5])
&& ! reg_set_p (operands[4], operands[5])
&& refers_to_regno_p (FLAGS_REG, operands[1], (rtx *)NULL)
&& peep2_regno_dead_p (0, FLAGS_REG)"
--- gcc/testsuite/gcc.dg/pr86314.c.jj   2018-06-26 10:25:35.190160477 +0200
+++ gcc/testsuite/gcc.dg/pr86314.c  2018-06-26 10:24:42.310077331 +0200
@@ -0,0 +1,20 @@
+// PR target/86314
+// { dg-do run { target sync_int_long } }
+// { dg-options "-O2" }
+
+__attribute__((noinline, noclone)) unsigned long
+foo (unsigned long *p)
+{
+  unsigned long m = 1UL << ((*p & 1) ? 1 : 0);
+  unsigned long n = __atomic_fetch_or (p, m, __ATOMIC_SEQ_CST);
+  return (n & m) == 0;
+}
+
+int
+main ()
+{
+  unsigned long v = 1;
+  if (foo () != 1)
+__builtin_abort ();
+  return 0;
+}

Jakub

Re: [PATCH, PR86257, i386/debug] Fix insn prefix in tls_global_dynamic_64_

[Jakub Jelinek]

On Tue, Jun 26, 2018 at 11:20:32AM +0200, Rainer Orth wrote:
> > Committed (after moving the testcase to gcc.target/i386).
> 
> the new testcase FAILs on 32-bit Solaris/x86
> 
> FAIL: gcc.target/i386/pr86257.c scan-assembler data16[ \\t]*leaq
> 
> and, according to gcc-testresults, also on i586-unknown-freebsd10.4 and
> x86_64-pc-linux-gnu.

Yeah, it failed for me with -m32 or -mx32, and also fails with
-mtls-dialect=gnu2.  So I've installed following on top of your patch as
obvious.

2018-06-26  Jakub Jelinek  

PR debug/86257
* gcc.target/i386/pr86257.c: Add -mtls-dialect=gnu to dg-options.

--- gcc/testsuite/gcc.target/i386/pr86257.c.jj  2018-06-25 14:51:20.481986807 
+0200
+++ gcc/testsuite/gcc.target/i386/pr86257.c 2018-06-26 12:32:10.284048124 
+0200
@@ -1,7 +1,7 @@
 /* { dg-require-effective-target lp64 } */
 /* { dg-require-effective-target fpic } */
 /* { dg-require-effective-target tls } */
-/* { dg-options "-g -fPIC" } */
+/* { dg-options "-g -fPIC -mtls-dialect=gnu" } */
 
 __thread int i;
 

Jakub

[PATCH, committed] Add myself to MAINTAINERS file

[Rasmus Villemoes]

From: villemoes 

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@262133 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 ChangeLog   | 4 
 MAINTAINERS | 1 +
 2 files changed, 5 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 7d1dd1faaca..5010ba97135 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2018-06-26  Rasmus Villemoes  
+
+   * MAINTAINERS (write after approval): Add myself.
+
 2018-06-19  Nick Clifton  
 
* zlib/configure.ac: Restore old behaviour of only enabling
diff --git a/MAINTAINERS b/MAINTAINERS
index 928f5cb37ae..f50f61ffb0e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -599,6 +599,7 @@ Andre Vehreschild   
 Alex Velenko   
 Ilya Verbin
 Andre Vieira   
+Rasmus Villemoes   
 Kugan Vivekanandarajah 
 Ville Voutilainen  
 Nenad Vukicevic
-- 
2.16.4

Re: [PATCH, PR86257, i386/debug] Fix insn prefix in tls_global_dynamic_64_

[Rainer Orth]

Hi Tom,

> On 06/24/2018 11:56 PM, Jan Hubicka wrote:
>>> Hi,
>>>
>>> [ The analysis of this PR was done at https://stackoverflow.com/a/33557963 ,
>>> November 2015. ]
>>>
>>> This tls sequence:
>>> ...
>>> 0x00 .byte 0x66
>>> 0x01 leaq  x@tlsgd(%rip),%rdi
>>> 0x08 .word 0x
>>> 0x0a rex64
>>> 0x0b call __tls_get_addr@plt
>>> ...
>>> starts with an insn prefix, produced using a .byte.
>>>
>>> When using a .loc before the sequence, it's associated with the next 
>>> assembly
>>> instruction, which is the leaq, so the .loc will end up pointing inside the
>>> sequence rather than to the start of the sequence.  And when the linker
>>> relaxes the sequence, the .loc may end up pointing inside an insn.  This 
>>> will
>>> cause an executable containing such a misplaced .loc to crash when gdb
>>> continues from the associated breakpoint.
>> 
>> Hmm, I am not sure why .byte should be non-instruction and .data should
>> be instruction,
>> but I assume gas simply behaves this way.
>> 
>
> Well, I suppose when encountering .byte/.value/.long/.quad, gas has no
> way of knowing whether it's dealing with data or instructions, even in
> the text section. An even if it would know it's dealing with
> instructions, it wouldn't know where an instruction begins or ends. So
> to me the behaviour seems reasonable.
>
> If we'd implemented something like this in gas:
> ...
> .insn
> .byte 0x66
> .endinsn
> ...
> we could fix this more generically.
>
> Or maybe we'd want this, which allows us to express that the .byte is a
> prefix to an existing insn:
> ...
> .insn
> .byte 0x66
> leaq  x@tlsgd(%rip),%rdi
> .endinsn
> ...
>
>> Don't we have also other cases wehre .byte is used to output instructions?
>> 
>> Patch is OK (and probably should be backported after some soaking in 
>> mainline)
>> 
>
> Committed (after moving the testcase to gcc.target/i386).

the new testcase FAILs on 32-bit Solaris/x86

FAIL: gcc.target/i386/pr86257.c scan-assembler data16[ \\t]*leaq

and, according to gcc-testresults, also on i586-unknown-freebsd10.4 and
x86_64-pc-linux-gnu.

It's by design 64-bit only and should document that.  Done as follows,
tested on i386-pc-solaris2.11 (both multilibs), installed on mainline.

Rainer

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


2018-06-26  Rainer Orth  

* gcc.target/i386/pr86257.c: Require 64-bit.

# HG changeset patch
# Parent  68b96f5f97c10194b0d75108f730254c229bbf33
Require 64-bit in gcc.target/i386/pr86257.c

	gcc/testsuite:
	* gcc.target/i386/pr86257.c: Require 64-bit.

diff --git a/gcc/testsuite/gcc.target/i386/pr86257.c b/gcc/testsuite/gcc.target/i386/pr86257.c
--- a/gcc/testsuite/gcc.target/i386/pr86257.c
+++ b/gcc/testsuite/gcc.target/i386/pr86257.c
@@ -1,3 +1,4 @@
+/* { dg-require-effective-target lp64 } */
 /* { dg-require-effective-target fpic } */
 /* { dg-require-effective-target tls } */
 /* { dg-options "-g -fPIC" } */

Re: [PATCH] C++: Fix PR86083

[Andreas Krebbel]

On 06/26/2018 11:17 AM, Rainer Orth wrote:
> Hi Andreas,
> 
>> When turning a user-defined numerical literal into an operator
>> invocation the literal needs to be translated to the execution
>> character set.
>>
>> Bootstrapped and regtested on s390x. x86_64 still running.
>> Ok to apply if x86_64 is clean?
> 
> the new testcase FAILs on Solaris
> 
> FAIL: g++.dg/pr86082.C   (test for excess errors)
> 
> Excess errors:
> cc1plus: error: conversion from UTF-8 to IBM1047 not supported by iconv
> 
> and, according to gcc-testresults, on powerpc-ibm-aix7.2.0.0.
> 
> Fixed as follows, tested on i386-pc-solaris2.11, installed on mainline.

Thanks for fixing it!

-Andreas-

Re: [PATCH] C++: Fix PR86083

[Rainer Orth]

Hi Andreas,

> When turning a user-defined numerical literal into an operator
> invocation the literal needs to be translated to the execution
> character set.
>
> Bootstrapped and regtested on s390x. x86_64 still running.
> Ok to apply if x86_64 is clean?

the new testcase FAILs on Solaris

FAIL: g++.dg/pr86082.C   (test for excess errors)

Excess errors:
cc1plus: error: conversion from UTF-8 to IBM1047 not supported by iconv

and, according to gcc-testresults, on powerpc-ibm-aix7.2.0.0.

Fixed as follows, tested on i386-pc-solaris2.11, installed on mainline.

Rainer

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


2018-06-26  Rainer Orth  

* g++.dg/pr86082.C: Require IBM1047 support.

# HG changeset patch
# Parent  eeab29b53ad0ae8a16bb9e8d4d0f934e9d5ebebe
Require IBM1047 support in g++.dg/pr86082.C

diff --git a/gcc/testsuite/g++.dg/pr86082.C b/gcc/testsuite/g++.dg/pr86082.C
--- a/gcc/testsuite/g++.dg/pr86082.C
+++ b/gcc/testsuite/g++.dg/pr86082.C
@@ -1,4 +1,5 @@
 /* { dg-do link } */
+/* { dg-require-iconv "IBM1047" } */
 /* { dg-options "-fexec-charset=IBM1047 -std=c++11" } */
 
 /* When turning 123_test into an operator invocation the literal 123

[patch] Remap goto_locus on edges during inlining

[Eric Botcazou]

Hi,

this makes sure the goto_locus present (or not) on edges is properly remapped 
during inlining.

Tested on x86-64/Linux, OK for the mainline?


2018-06-26  Eric Botcazou  

* tree-inline.c (remap_location): New function extracted from...
(copy_edges_for_bb): Add ID parameter.  Remap goto_locus.
(copy_phis_for_bb): ...here.  Call remap_location.
(copy_cfg_body): Adjust call to copy_edges_for_bb.

-- 
Eric BotcazouIndex: tree-inline.c
===
--- tree-inline.c	(revision 262129)
+++ tree-inline.c	(working copy)
@@ -718,6 +718,7 @@ remap_block (tree *block, copy_body_data
 }
 
 /* Copy the whole block tree and root it in id->block.  */
+
 static tree
 remap_blocks (tree block, copy_body_data *id)
 {
@@ -738,6 +739,7 @@ remap_blocks (tree block, copy_body_data
 }
 
 /* Remap the block tree rooted at BLOCK to nothing.  */
+
 static void
 remap_blocks_to_null (tree block, copy_body_data *id)
 {
@@ -747,6 +749,27 @@ remap_blocks_to_null (tree block, copy_b
 remap_blocks_to_null (t, id);
 }
 
+/* Remap the location info pointed to by LOCUS.  */
+
+static location_t
+remap_location (location_t locus, copy_body_data *id)
+{
+  if (LOCATION_BLOCK (locus))
+{
+  tree *n = id->decl_map->get (LOCATION_BLOCK (locus));
+  gcc_assert (n);
+  if (*n)
+	return set_block (locus, *n);
+}
+
+  locus = LOCATION_LOCUS (locus);
+
+  if (locus != UNKNOWN_LOCATION && id->block)
+return set_block (locus, id->block);
+
+  return locus;
+}
+
 static void
 copy_statement_list (tree *tp)
 {
@@ -2145,7 +2168,8 @@ update_ssa_across_abnormal_edges (basic_
 
 static bool
 copy_edges_for_bb (basic_block bb, profile_count num, profile_count den,
-		   basic_block ret_bb, basic_block abnormal_goto_dest)
+		   basic_block ret_bb, basic_block abnormal_goto_dest,
+		   copy_body_data *id)
 {
   basic_block new_bb = (basic_block) bb->aux;
   edge_iterator ei;
@@ -2160,6 +2184,7 @@ copy_edges_for_bb (basic_block bb, profi
   {
 	edge new_edge;
 	int flags = old_edge->flags;
+	location_t locus = old_edge->goto_locus;
 
 	/* Return edges do get a FALLTHRU flag when they get inlined.  */
 	if (old_edge->dest->index == EXIT_BLOCK
@@ -2167,8 +2192,10 @@ copy_edges_for_bb (basic_block bb, profi
 	&& old_edge->dest->aux != EXIT_BLOCK_PTR_FOR_FN (cfun))
 	  flags |= EDGE_FALLTHRU;
 
-	new_edge = make_edge (new_bb, (basic_block) old_edge->dest->aux, flags);
+	new_edge
+	  = make_edge (new_bb, (basic_block) old_edge->dest->aux, flags);
 	new_edge->probability = old_edge->probability;
+	new_edge->goto_locus = remap_location (locus, id);
   }
 
   if (bb->index == ENTRY_BLOCK || bb->index == EXIT_BLOCK)
@@ -2365,16 +2392,7 @@ copy_phis_for_bb (basic_block bb, copy_b
 		  inserted = true;
 		}
 		  locus = gimple_phi_arg_location_from_edge (phi, old_edge);
-		  if (LOCATION_BLOCK (locus))
-		{
-		  tree *n;
-		  n = id->decl_map->get (LOCATION_BLOCK (locus));
-		  gcc_assert (n);
-		  locus = set_block (locus, *n);
-		}
-		  else
-		locus = LOCATION_LOCUS (locus);
-
+		  locus = remap_location (locus, id);
 		  add_phi_arg (new_phi, new_arg, new_edge, locus);
 		}
 	}
@@ -2705,7 +2723,7 @@ copy_cfg_body (copy_body_data * id,
 if (!id->blocks_to_copy
 	|| (bb->index > 0 && bitmap_bit_p (id->blocks_to_copy, bb->index)))
   need_debug_cleanup |= copy_edges_for_bb (bb, num, den, exit_block_map,
-	   abnormal_goto_dest);
+	   abnormal_goto_dest, id);
 
   if (new_entry)
 {

Re: [PATCH] [PR86064] split single cross-partition range with nonzero locviews

[Richard Biener]

On Tue, Jun 26, 2018 at 10:21 AM Alexandre Oliva  wrote:
>
> On Jun 22, 2018, Jeff Law  wrote:
>
> > On 06/12/2018 08:02 PM, Alexandre Oliva wrote:
> >>
> >> We didn't split cross-partition ranges in loclists to output a
> >> whole-function location expression, but with nonzero locviews, we
> >> force loclists, and then we have to split to avoid cross-partition
> >> list entries.
> >>
> >> From: Alexandre Oliva 
> >> for  gcc/ChangeLog
> >>
> >> PR debug/86064
> >> * dwarf2out.c (loc_list_has_views): Adjust comments.
> >> (dw_loc_list): Split single cross-partition range with
> >> nonzero locview.
> >>
> >> for  gcc/testsuite/ChangeLog
> >>
> >> PR debug/86064
> >> * gcc.dg/pr86064.c: New.
>
> > OK.
>
> Thanks.  For the gcc 8 branch too?  (pending retesting)

Yes.

Richard.

> --
> Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
> Be the change, be Free! FSF Latin America board member
> GNU Toolchain EngineerFree Software Evangelist

Re: [PATCH] [PR86064] split single cross-partition range with nonzero locviews

[Alexandre Oliva]

On Jun 22, 2018, Jeff Law  wrote:

> On 06/12/2018 08:02 PM, Alexandre Oliva wrote:
>> 
>> We didn't split cross-partition ranges in loclists to output a
>> whole-function location expression, but with nonzero locviews, we
>> force loclists, and then we have to split to avoid cross-partition
>> list entries.
>> 
>> From: Alexandre Oliva 
>> for  gcc/ChangeLog
>> 
>> PR debug/86064
>> * dwarf2out.c (loc_list_has_views): Adjust comments.
>> (dw_loc_list): Split single cross-partition range with
>> nonzero locview.
>> 
>> for  gcc/testsuite/ChangeLog
>> 
>> PR debug/86064
>> * gcc.dg/pr86064.c: New.

> OK.

Thanks.  For the gcc 8 branch too?  (pending retesting)

-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain EngineerFree Software Evangelist

Re: [PATCH] Fix bootstrap failure in vect_analyze_loop

[Richard Biener]

On Tue, Jun 26, 2018 at 2:21 AM David Malcolm  wrote:
>
> I ran into this bootstrap failure (with r262092):
>
> ../../../src/gcc/tree-vect-loop.c: In function ‘_loop_vec_info* 
> vect_analyze_loop(loop*, loop_vec_info, vec_info_shared*)’:
> ../../../src/gcc/tree-vect-loop.c:1946:25: error: ‘n_stmts’ may be used 
> uninitialized in this function [-Werror=maybe-uninitialize ]
>ok = vect_analyze_slp (loop_vinfo, *n_stmts);
> ~^~
> ../../../src/gcc/tree-vect-loop.c:2318:12: note: ‘n_stmts’ was declared here
>unsigned n_stmts;
> ^~~
> cc1plus: all warnings being treated as errors
>
> It looks like it's inlining vect_analyze_loop_2 into vect_analyze_loop,
> passing _stmts in by pointer.
>
> Normally, vect_get_datarefs_in_loop writes:
>   *n_stmts = 0;
> when
>   if (!LOOP_VINFO_DATAREFS (loop_vinfo).exists ())
> but not in the "else" path, and then, after lots of complex logic:
>
>   ok = vect_analyze_slp (loop_vinfo, *n_stmts);
>
> it uses the value in vect_analyze_loop_2, passed in via _stmts.
>
> So it's not at all clear to me (or the compiler) that the value is
> initialized in all paths, and an initialization to zero seems a
> reasonable fix.
>
> OK for trunk, assuming the bootstrap succeeds this time?

I already applied the very same fix, sorry for the breakage.

Richard.

> gcc/ChangeLog:
> * tree-vect-loop.c (vect_analyze_loop): Initialize n_stmts.
> ---
>  gcc/tree-vect-loop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index dacc881..2b3ced2 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -2315,7 +2315,7 @@ vect_analyze_loop (struct loop *loop, loop_vec_info 
> orig_loop_vinfo,
>return NULL;
>  }
>
> -  unsigned n_stmts;
> +  unsigned n_stmts = 0;
>poly_uint64 autodetected_vector_size = 0;
>while (1)
>  {
> --
> 1.8.5.3
>

Re: [PATCH] Default special members of regex types and add noexcept

[Stephan Bergmann]

On 08/06/18 13:01, Jonathan Wakely wrote:

Nothing very exciting, just adding noexcept and defaulting some
members.

 * include/bits/regex.h (sub_match): Add noexcept to default
 constructor and length observer.
 (match_results): Add noexcept to default constructor and observers
 with no preconditions. Define destructor as defaulted.
 (operator==, operator!=, swap): Add noexcept.
 (regex_iterator): Add default member initializers and define default
 constructor and destructor as defaulted. Add noexcept to equality
 and dereference operators.

Tested powerpc64le-linux, committed to trunk.


Looks like that forgot to also add noexcept to the definition of


regex_iterator<_Bi_iter, _Ch_type, _Rx_traits>::
operator==(const regex_iterator& __rhs) const


in libstdc++-v3/include/bits/regex.tcc?

Re: [PATCH] Remove -Wunsafe-loop-optimizations option (PR middle-end/86095)

[Richard Biener]

On Mon, 25 Jun 2018, NightStrike wrote:

> On Fri, Jun 15, 2018 at 3:08 PM, Jakub Jelinek  wrote:
> > Hi!
> >
> > As mentioned in the PR, all traces of this warning option except these
> > were removed earlier, so the warning option does nothing.
> 
> This is unfortunate.  As noted here:
> 
> https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01057.html
> 
> The warning is (or, it used to be) good at finding code areas to
> rewrite, and it's MUCH easier to use than the giant walls of text from
> -fopt-info.  Is it possible to recreate this functionality in some
> other way (or has that already been done, and I missed it)?

I guess it would be straight forward to queue an optinfo record
about ->assumptions being present on loops.  It's just that
passes tend to not use the more "complex" number_of_iterations_exit ()
interface but rather the overly simplistic interface giving up
in more complex situations.

-fopt-info-missed has the issue that existing passes were converted
to the opt-info dumping machinery without much thought on what
exactly is worth reporting under "missed"...

Richard.