Two fixes for pretty-print.c for mingw-w64

2018-08-13 Thread Liu Hao
The two patches attached have addressed two issues in the ANSI escape 
sequence translator I sent before.  Please review, and consider 
backporting these to gcc-8-branch.





commit 5e79ccaa625169fcaca5847feff0ee168cbad83f (HEAD -> makepkg)
Author: Liu Hao 
Date:   Tue Aug 14 12:22:08 2018 +0800

gcc/pretty-print.c: Make reverse video work for non-DBCS environments.

`COMMON_LVB_REVERSE_VIDEO` only works for DBCS environments (such
as CJK charsets). As it is unreliable, we swap the foreground and
background by hand if this flag is set.

Signed-off-by: Liu Hao 

commit a250716957205fee0de3553f318d04b9bfc1f35f
Author: Liu Hao 
Date:   Tue Aug 14 12:13:32 2018 +0800

gcc/pretty-print.c: Do not call `_close()` on the handle returned 
by `_get_osfhandle()`.


Microsoft documentation about `_get_osfhandle()` is misleading. It
shouldn't have mentioned that the handle can be closed in such a
way, as the handle is automatically closed when the associated
`FILE *` or FD is closed. Under no circumstance does it make any
sense to `_close()` it by hand.

The non-debug versions of MSVCRT and UCRTBASE tolerate such errors,
but with the debug version of UCRTBASE an assertion is triggered.

Reference: 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle

Signed-off-by: Liu Hao 




--
Best regards,
LH_Mouse
From a250716957205fee0de3553f318d04b9bfc1f35f Mon Sep 17 00:00:00 2001
From: Liu Hao 
Date: Tue, 14 Aug 2018 12:13:32 +0800
Subject: [PATCH 1/2] gcc/pretty-print.c: Do not call `_close()` on the handle
 returned by `_get_osfhandle()`.

Microsoft documentation about `_get_osfhandle()` is misleading. It
shouldn't have mentioned that the handle can be closed in such a
way, as the handle is automatically closed when the associated
`FILE *` or FD is closed. Under no circumstance does it make any
sense to `_close()` it by hand.

The non-debug versions of MSVCRT and UCRTBASE tolerate such errors,
but with the debug version of UCRTBASE an assertion is triggered.

Reference: 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle
Signed-off-by: Liu Hao 
---
 gcc/pretty-print.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 736af8f7735..31eb8893f2a 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -684,7 +684,6 @@ mingw_ansi_fputs (const char *str, FILE *fp)
 /* If it is not a console, write everything as-is.  */
 write_all (h, read, strlen (read));
 
-  _close ((intptr_t) h);
   return 1;
 }
 
-- 
2.18.0

From 5e79ccaa625169fcaca5847feff0ee168cbad83f Mon Sep 17 00:00:00 2001
From: Liu Hao 
Date: Tue, 14 Aug 2018 12:22:08 +0800
Subject: [PATCH 2/2] gcc/pretty-print.c: Make reverse video work for non-DBCS
 environments.

`COMMON_LVB_REVERSE_VIDEO` only works for DBCS environments (such
as CJK charsets). As it is unreliable, we swap the foreground and
background by hand if this flag is set.

Signed-off-by: Liu Hao 
---
 gcc/pretty-print.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 31eb8893f2a..02967d05f75 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -640,6 +640,16 @@ sgr_set_it:
{
  attrib_add |= sb.wAttributes & ~attrib_rm;
}
+  if (attrib_add & COMMON_LVB_REVERSE_VIDEO)
+   {
+ /* COMMON_LVB_REVERSE_VIDEO is only effective for DBCS.
+  * Swap foreground and background colors by hand.
+  */
+ attrib_add = (attrib_add & 0xFF00)
+   | ((attrib_add & 0x00F0) >> 4)
+   | ((attrib_add & 0x000F) << 4);
+ attrib_add &= ~COMMON_LVB_REVERSE_VIDEO;
+   }
   SetConsoleTextAttribute (h, attrib_add);
   break;
 }
-- 
2.18.0



[PATCH 2/6] detect unterminated const arrays in strlen calls (PR 86552)

2018-08-13 Thread Martin Sebor

[PATCH 2/6] detect unterminated const arrays in strlen calls (PR 86552)

The attached changes implement the detection of past-the-end reads
by strlen due to unterminated arguments.
PR tree-optimization/86552 - missing warning for reading past the end

gcc/ChangeLog:

	* builtins.c (warn_string_no_nul): New function.
	(expand_builtin_strlen): Warn for unterminated arrays.
	(fold_builtin_strlen): Add argument.  Warn for unterminated arrays.
	(fold_builtin_1): Adjust call to fold_builtin_strlen.
	* builtins.h (warn_string_no_nul): New function.

gcc/testsuite/ChangeLog:

	* gcc.dg/warn-strlen-no-nul.c: New test.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index a7aa4b2..78ced93 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -150,7 +150,7 @@ static tree stabilize_va_list_loc (location_t, tree, int);
 static rtx expand_builtin_expect (tree, rtx);
 static tree fold_builtin_constant_p (tree);
 static tree fold_builtin_classify_type (tree);
-static tree fold_builtin_strlen (location_t, tree, tree);
+static tree fold_builtin_strlen (location_t, tree, tree, tree);
 static tree fold_builtin_inf (location_t, tree, int);
 static tree rewrite_call_expr (location_t, tree, int, tree, int, ...);
 static bool validate_arg (const_tree, enum tree_code code);
@@ -550,6 +550,36 @@ string_length (const void *ptr, unsigned eltsize, unsigned maxelts)
   return n;
 }
 
+/* For a call expression EXP to a function that expects a string argument,
+   issue a diagnostic due to it being a called with an argument NONSTR
+   that is a character array with no terminating NUL.  */
+
+void
+warn_string_no_nul (location_t loc, tree exp, tree fndecl, tree nonstr)
+{
+  loc = expansion_point_location_if_in_system_header (loc);
+
+  bool warned;
+  if (exp)
+{
+  if (!fndecl)
+	fndecl = get_callee_fndecl (exp);
+  warned = warning_at (loc, OPT_Wstringop_overflow_,
+			   "%K%qD argument missing terminating nul",
+			   exp, fndecl);
+}
+  else
+{
+  gcc_assert (fndecl);
+  warned = warning_at (loc, OPT_Wstringop_overflow_,
+			   "%qD argument missing terminating nul",
+			   fndecl);
+}
+
+  if (warned && DECL_P (nonstr))
+inform (DECL_SOURCE_LOCATION (nonstr), "referenced argument declared here");
+}
+
 /* Compute the length of a null-terminated character string or wide
character string handling character sizes of 1, 2, and 4 bytes.
TREE_STRING_LENGTH is not the right way because it evaluates to
@@ -2874,7 +2904,6 @@ expand_builtin_strlen (tree exp, rtx target,
 
   struct expand_operand ops[4];
   rtx pat;
-  tree len;
   tree src = CALL_EXPR_ARG (exp, 0);
   rtx src_reg;
   rtx_insn *before_strlen;
@@ -2883,20 +2912,39 @@ expand_builtin_strlen (tree exp, rtx target,
   unsigned int align;
 
   /* If the length can be computed at compile-time, return it.  */
-  len = c_strlen (src, 0);
+  tree array;
+  tree len = c_strlen (src, 0, );
   if (len)
-return expand_expr (len, target, target_mode, EXPAND_NORMAL);
+{
+  if (array)
+	{
+	  /* Array refers to the non-nul terminated constant array
+	 whose length is attempted to be computed.  */
+	  warn_string_no_nul (EXPR_LOCATION (exp), exp, NULL_TREE, array);
+	  return NULL_RTX;
+	}
+  return expand_expr (len, target, target_mode, EXPAND_NORMAL);
+}
 
   /* If the length can be computed at compile-time and is constant
  integer, but there are side-effects in src, evaluate
  src for side-effects, then return len.
  E.g. x = strlen (i++ ? "xfoo" + 1 : "bar");
  can be optimized into: i++; x = 3;  */
-  len = c_strlen (src, 1);
-  if (len && TREE_CODE (len) == INTEGER_CST)
+  len = c_strlen (src, 1, );
+  if (len)
 {
-  expand_expr (src, const0_rtx, VOIDmode, EXPAND_NORMAL);
-  return expand_expr (len, target, target_mode, EXPAND_NORMAL);
+  if (array)
+	{
+	  warn_string_no_nul (EXPR_LOCATION (exp), exp, NULL_TREE, array);
+	  return NULL_RTX;
+	}
+
+  if (TREE_CODE (len) == INTEGER_CST)
+	{
+	  expand_expr (src, const0_rtx, VOIDmode, EXPAND_NORMAL);
+	  return expand_expr (len, target, target_mode, EXPAND_NORMAL);
+	}
 }
 
   align = get_pointer_alignment (src) / BITS_PER_UNIT;
@@ -8339,19 +8387,30 @@ fold_builtin_classify_type (tree arg)
   return build_int_cst (integer_type_node, type_to_class (TREE_TYPE (arg)));
 }
 
-/* Fold a call to __builtin_strlen with argument ARG.  */
+/* Fold a strlen call to FNDECL of TYPE, and with argument ARG.  */
 
 static tree
-fold_builtin_strlen (location_t loc, tree type, tree arg)
+fold_builtin_strlen (location_t loc, tree fndecl, tree type, tree arg)
 {
   if (!validate_arg (arg, POINTER_TYPE))
 return NULL_TREE;
   else
 {
-  tree len = c_strlen (arg, 0);
-
+  tree nonstr = NULL_TREE;
+  tree len = c_strlen (arg, 0, );
   if (len)
-	return fold_convert_loc (loc, type, len);
+	{
+	  if (loc == UNKNOWN_LOCATION && EXPR_HAS_LOCATION (arg))
+	loc = EXPR_LOCATION (arg);
+
+	  /* To avoid warning multiple times about 

Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

2018-08-13 Thread David Malcolm
On Tue, 2018-08-14 at 11:02 +1200, Jason Merrill wrote:
> 
> 
> On Tue, Aug 14, 2018, 10:24 AM Marek Polacek 
> wrote:
> > On Mon, Aug 13, 2018 at 10:14:21PM +1200, Jason Merrill wrote:
> > > >> >> > --- gcc/cp/decl.c
> > > >> >> > +++ gcc/cp/decl.c
> > > >> >> > @@ -9581,7 +9581,7 @@ compute_array_index_type (tree
> > name, tree size, tsubst_flags_t complain)
> > > >> >> >  {
> > > >> >> >tree folded = cp_fully_fold (size);
> > > >> >> >if (TREE_CODE (folded) == INTEGER_CST)
> > > >> >> > -   pedwarn (location_of (size), OPT_Wpedantic,
> > > >> >> > +   pedwarn (input_location, OPT_Wpedantic,
> > > >> >>
> > > >> >> It should work to use location_of (osize) here.
> > > >> >
> > > >> > I dropped this hunk altogether.  Because location_of will
> > use
> > > >> > DECL_SOURCE_LOCATION for DECLs, the error message will point
> > to the declaration
> > > >> > itself, not the use.  I don't really care either way.
> > > >>
> > > >> We want the message to point to the use, which location_of
> > (osize)
> > > >> will provide, since it should still have a location wrapper
> > around a
> > > >> DECL.
> > > >
> > > > location_of (osize) is actually the same as location_of (size)
> > so that didn't
> > > > change anything.
> > > 
> > > Hunh, that's strange.  Why isn't osize the unfolded expression? 
> > Where
> > > is the location wrapper getting stripped?
> > 
> > I actually see that it didn't have the location wrapper at the
> > start.
> > The array bound is parsed in cp_parser_direct_new_declarator, and
> > we
> > never called maybe_wrap_with_location to add the wrapper.  I don't
> > know
> > where that's supposed to happen.

Currently we only call maybe_wrap_with_location in a few select places
in the C++ FE: for the arguments of call sites, and a few other places.
 I hope to overhaul this at some point in gcc 9 stage 1, but there may
be a case for adding more special-cases.

> > This quick hack works
> > 
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -8681,6 +8681,7 @@ cp_parser_direct_new_declarator (cp_parser*
> > parser)
> >cp_parser_require (parser, CPP_CLOSE_SQUARE,
> > RT_CLOSE_SQUARE);
> > 
> >/* Add this bound to the declarator.  */
> > +  expression = maybe_wrap_with_location (expression, token-
> > >location);
> >declarator = make_array_declarator (declarator, expression);
> > 
> >/* If the next token is not a `[', then there are no more
> > 
> > but that feels too ad-hoc and beyond the scope of this fix.
> 
> David, does this look right to you?

The usage of token->location struck me as potentially wrong.  There
have been plenty of places in our frontends where we've erroneously
used a token's location for an expression, rather than that of the
whole expression (due to grafting in location ranges in the gcc 6
timeframe); this jumped out at me as maybe one of these cases.

If I'm reading it right, the code is parsing this production:

   direct-new-declarator:
 [ expression ]
 direct-new-declarator [constant-expression]

What's an example of a string that matches this part of the grammar? 
Is it something like the highlighted part of:

  foo = new char[-42];
^

?

We parse the expression here:

  expression = cp_parser_expression (parser);

cp_parser_expression returns a cp_expr, which would implicitly wrap the
location if we have, say a integer constant.  In the example above,
this would capture the location of the constant:

  foo = new char[-42];
 ^~~

However, we throw away the "on the side" location information since
"expression" is a tree, rather than a cp_expr.

token->location is purely the location of the opening '[' token.  That
sounds like it's also throwing away location information.

What is it that's being parsed, and what should the location be? 

If this is for issuing a diagnostic like:

  error: size of array is negative

then what should the user see?  Presumably something like:

  error: size of array is negative
   foo = new char[-42];
  ^~~

or:

  error: size of array is negative
   foo = new char[-42];
 ~^~~~

or somesuch?  For the former, then converting expression to a cp_expr
and adding:

  expression = expression.maybe_add_location_wrapper ();

might work.

I have a patch to overhaul all of this which I'll try to finish and
post (but I'm knee-deep in something else right now).

Hope this is constructive
Dave


> > > > The code below uses input_location which is why I went with
> > > > it in the first place.  So, should I change this to
> > input_location?
> > > 
> > > I suppose so.
> > 
> > Here's the version with input_location.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> Ok.
> 
> > 2018-08-13  Marek Polacek  
> > 
> > PR c++/57891
> > * call.c (struct conversion): Add
> > check_narrowing_const_only.
> > (build_converted_constant_expr): Set check_narrowing and
> > 

[PING][PATCH] Make function clone name numbering independent.

2018-08-13 Thread Michael Ploujnikov
Ping and I've updated the patch since last time as follows:

  - unittest scans assembly rather than the constprop dump because its
forward changed
  - unittests should handle different hosts where any of
NO_DOT_IN_LABEL, NO_DOLLAR_IN_LABEL or __USER_LABEL_PREFIX__ may
be defined
  - not 100% it's safe to change DECL_NAME to DECL_ASSEMBLER_NAME in
cgraph_node::create_virtual_clone, but I've attempted to reduce
some code duplication
  - lto-partition.c: privatize_symbol_name_1 *does* need numbered
names
  - but cold sections don't
  - Expecting an IDENTIFIER_NODE in clone_function_name_1 avoids
unreliable string pointer use as pointed out in the first review
  - renamed clone_function_name_1 and clone_function_name to
numbered_clone_function_name_1 and numbered_clone_function_name to
clarify purpose and discourage future unintended uses

Also bootstrapped and regtested.

- Michael

On 2018-08-02 03:05 PM, Michael Ploujnikov wrote:
> On 2018-08-01 06:37 AM, Richard Biener wrote:
>> On Tue, Jul 31, 2018 at 7:40 PM Michael Ploujnikov
>>  wrote:
>>>
>>> On 2018-07-26 01:27 PM, Michael Ploujnikov wrote:
 On 2018-07-24 09:57 AM, Michael Ploujnikov wrote:
> On 2018-07-20 06:05 AM, Richard Biener wrote:
>>>  /* Return a new assembler name for a clone with SUFFIX of a decl named
>>> NAME.  */
>>> @@ -521,14 +521,13 @@ tree
>>>  clone_function_name_1 (const char *name, const char *suffix)
>>
>> pass this function the counter to use
>>
>>>  {
>>>size_t len = strlen (name);
>>> -  char *tmp_name, *prefix;
>>> +  char *prefix;
>>>
>>>prefix = XALLOCAVEC (char, len + strlen (suffix) + 2);
>>>memcpy (prefix, name, len);
>>>strcpy (prefix + len + 1, suffix);
>>>prefix[len] = symbol_table::symbol_suffix_separator ();
>>> -  ASM_FORMAT_PRIVATE_NAME (tmp_name, prefix, clone_fn_id_num++);
>>
>> and keep using ASM_FORMAT_PRIVATE_NAME here.  You need to change
>> the lto/lto-partition.c caller (just use zero as counter).
>>
>>> -  return get_identifier (tmp_name);
>>> +  return get_identifier (prefix);
>>>  }
>>>
>>>  /* Return a new assembler name for a clone of DECL with SUFFIX.  */
>>> @@ -537,7 +536,17 @@ tree
>>>  clone_function_name (tree decl, const char *suffix)
>>>  {
>>>tree name = DECL_ASSEMBLER_NAME (decl);
>>> -  return clone_function_name_1 (IDENTIFIER_POINTER (name), suffix);
>>> +  const char *decl_name = IDENTIFIER_POINTER (name);
>>> +  char *numbered_name;
>>> +  unsigned int *suffix_counter;
>>> +  if (!clone_fn_ids) {
>>> +/* Initialize the per-function counter hash table if this is the 
>>> first call */
>>> +clone_fn_ids = hash_map::create_ggc (64);
>>> +  }
>>
>> I still do not like throwing memory at the problem in this way for the
>> little benefit
>> this change provides :/
>>
>> So no approval from me at this point...
>>
>> Richard.
>
> Can you give me an idea of the memory constraints that are involved?
>
> The highest memory usage increase that I could find was when compiling
> a source file (from Linux) with roughly 10,000 functions. It showed a 2kB
> increase over the before-patch use of 6936kB which is barely 0.03%.
>
> Using a single counter can result in more confusing namespacing when
> you have .bar.clone.4 despite there only being 3 clones of .bar.
>
> From a practical point of view this change is helpful to anyone
> diffing binary output such as forensic analysts, Debian Reproducible
> Builds or even someone validating compiler output (before and after an 
> input
> source patch). The extra changes that this patch alleviates are a
> distraction and could even be misleading. For example, applying a
> source patch to the same Linux source produces the following binary
> diff before my change:
>
> --- /tmp/output.o.objdump
> +++ /tmp/patched-output.o.objdump
> @@ -1,5 +1,5 @@
>
> -/tmp/uverbs_cmd/output.o: file format elf32-i386
> +/tmp/uverbs_cmd/patched-output.o: file format elf32-i386
>
>
>  Disassembly of section .text.get_order:
> @@ -265,12 +265,12 @@
> 3:   e9 fc ff ff ff  jmp4 
>  4: R_386_PC32   .text.put_uobj_read
>
> -Disassembly of section .text.trace_kmalloc.constprop.3:
> +Disassembly of section .text.trace_kmalloc.constprop.4:
>
> - :
> + :
> 0:   83 3d 04 00 00 00 00cmpl   $0x0,0x4
>  2: R_386_32 __tracepoint_kmalloc
> -   7:   74 34   je 3d 
> 
> +   7:   74 34   je 3d 
> 
> 9:   55  push   %ebp
> a:   89 cd   mov%ecx,%ebp
> 

Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

2018-08-13 Thread Jason Merrill
On Tue, Aug 14, 2018, 10:24 AM Marek Polacek  wrote:

> On Mon, Aug 13, 2018 at 10:14:21PM +1200, Jason Merrill wrote:
> > >> >> > --- gcc/cp/decl.c
> > >> >> > +++ gcc/cp/decl.c
> > >> >> > @@ -9581,7 +9581,7 @@ compute_array_index_type (tree name, tree
> size, tsubst_flags_t complain)
> > >> >> >  {
> > >> >> >tree folded = cp_fully_fold (size);
> > >> >> >if (TREE_CODE (folded) == INTEGER_CST)
> > >> >> > -   pedwarn (location_of (size), OPT_Wpedantic,
> > >> >> > +   pedwarn (input_location, OPT_Wpedantic,
> > >> >>
> > >> >> It should work to use location_of (osize) here.
> > >> >
> > >> > I dropped this hunk altogether.  Because location_of will use
> > >> > DECL_SOURCE_LOCATION for DECLs, the error message will point to the
> declaration
> > >> > itself, not the use.  I don't really care either way.
> > >>
> > >> We want the message to point to the use, which location_of (osize)
> > >> will provide, since it should still have a location wrapper around a
> > >> DECL.
> > >
> > > location_of (osize) is actually the same as location_of (size) so that
> didn't
> > > change anything.
> >
> > Hunh, that's strange.  Why isn't osize the unfolded expression?  Where
> > is the location wrapper getting stripped?
>
> I actually see that it didn't have the location wrapper at the start.
> The array bound is parsed in cp_parser_direct_new_declarator, and we
> never called maybe_wrap_with_location to add the wrapper.  I don't know
> where that's supposed to happen.
>
> This quick hack works
>
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -8681,6 +8681,7 @@ cp_parser_direct_new_declarator (cp_parser* parser)
>cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
>
>/* Add this bound to the declarator.  */
> +  expression = maybe_wrap_with_location (expression, token->location);
>declarator = make_array_declarator (declarator, expression);
>
>/* If the next token is not a `[', then there are no more
>
> but that feels too ad-hoc and beyond the scope of this fix.
>

David, does this look right to you?

> > The code below uses input_location which is why I went with
> > > it in the first place.  So, should I change this to input_location?
> >
> > I suppose so.
>
> Here's the version with input_location.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>

Ok.

2018-08-13  Marek Polacek  
>
> PR c++/57891
> * call.c (struct conversion): Add check_narrowing_const_only.
> (build_converted_constant_expr): Set check_narrowing and
> check_narrowing_const_only.  Give error if expr is error node.
> (convert_like_real): Pass it to check_narrowing.
> * cp-tree.h (check_narrowing): Add a default parameter.
> * decl.c (compute_array_index_type): Use input_location instead of
> location_of.
> * pt.c (convert_nontype_argument): Return NULL_TREE if tf_error.
> * typeck2.c (check_narrowing): Don't warn for
> instantiation-dependent
> expressions.  Call maybe_constant_value instead of
> fold_non_dependent_expr.  Don't mention { } in diagnostic.  Only
> check
> narrowing for constants if CONST_ONLY.
>
> * g++.dg/cpp0x/Wnarrowing6.C: New test.
> * g++.dg/cpp0x/Wnarrowing7.C: New test.
> * g++.dg/cpp0x/Wnarrowing8.C: New test.
> * g++.dg/cpp0x/Wnarrowing9.C: New test.
> * g++.dg/cpp0x/Wnarrowing10.C: New test.
>
> * g++.dg/cpp0x/constexpr-47969.C: Adjust dg-error.
> * g++.dg/cpp0x/constexpr-ex2.C: Likewise.
> * g++.dg/cpp0x/constexpr-targ.C: Likewise.
> * g++.dg/cpp0x/scoped_enum2.C: Likewise.
> * g++.dg/ext/stmtexpr15.C: Likewise.
> * g++.dg/gomp/pr47963.C: Likewise.
> * g++.dg/init/new37.C: Likewise.
> * g++.dg/init/new43.C: Likewise.
> * g++.dg/other/fold1.C: Likewise.
> * g++.dg/parse/array-size2.C: Likewise.
> * g++.dg/template/dependent-name3.C: Likewise.
> * g++.dg/cpp0x/constexpr-data2.C: Add dg-error.
> * g++.dg/other/vrp1.C: Likewise.
> * g++.dg/template/char1.C: Likewise.
>
> diff --git gcc/cp/call.c gcc/cp/call.c
> index 209c1fd2f0e..62654a9e407 100644
> --- gcc/cp/call.c
> +++ gcc/cp/call.c
> @@ -107,6 +107,9 @@ struct conversion {
>   binding a reference directly or decaying to a pointer.  */
>BOOL_BITFIELD rvaluedness_matches_p: 1;
>BOOL_BITFIELD check_narrowing: 1;
> +  /* Whether check_narrowing should only check TREE_CONSTANTs; used
> + in build_converted_constant_expr.  */
> +  BOOL_BITFIELD check_narrowing_const_only: 1;
>/* The type of the expression resulting from the conversion.  */
>tree type;
>union {
> @@ -4152,9 +4155,18 @@ build_converted_constant_expr (tree type, tree
> expr, tsubst_flags_t complain)
>  }
>
>if (conv)
> -expr = convert_like (conv, expr, complain);
> +{
> +  conv->check_narrowing = true;
> +  

Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

2018-08-13 Thread Marek Polacek
On Mon, Aug 13, 2018 at 10:14:21PM +1200, Jason Merrill wrote:
> >> >> > --- gcc/cp/decl.c
> >> >> > +++ gcc/cp/decl.c
> >> >> > @@ -9581,7 +9581,7 @@ compute_array_index_type (tree name, tree size, 
> >> >> > tsubst_flags_t complain)
> >> >> >  {
> >> >> >tree folded = cp_fully_fold (size);
> >> >> >if (TREE_CODE (folded) == INTEGER_CST)
> >> >> > -   pedwarn (location_of (size), OPT_Wpedantic,
> >> >> > +   pedwarn (input_location, OPT_Wpedantic,
> >> >>
> >> >> It should work to use location_of (osize) here.
> >> >
> >> > I dropped this hunk altogether.  Because location_of will use
> >> > DECL_SOURCE_LOCATION for DECLs, the error message will point to the 
> >> > declaration
> >> > itself, not the use.  I don't really care either way.
> >>
> >> We want the message to point to the use, which location_of (osize)
> >> will provide, since it should still have a location wrapper around a
> >> DECL.
> >
> > location_of (osize) is actually the same as location_of (size) so that 
> > didn't
> > change anything.
> 
> Hunh, that's strange.  Why isn't osize the unfolded expression?  Where
> is the location wrapper getting stripped?

I actually see that it didn't have the location wrapper at the start.
The array bound is parsed in cp_parser_direct_new_declarator, and we
never called maybe_wrap_with_location to add the wrapper.  I don't know
where that's supposed to happen.

This quick hack works

--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -8681,6 +8681,7 @@ cp_parser_direct_new_declarator (cp_parser* parser)
   cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);

   /* Add this bound to the declarator.  */
+  expression = maybe_wrap_with_location (expression, token->location);
   declarator = make_array_declarator (declarator, expression);

   /* If the next token is not a `[', then there are no more

but that feels too ad-hoc and beyond the scope of this fix.

> > The code below uses input_location which is why I went with
> > it in the first place.  So, should I change this to input_location?
> 
> I suppose so.

Here's the version with input_location.

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

2018-08-13  Marek Polacek  

PR c++/57891
* call.c (struct conversion): Add check_narrowing_const_only.
(build_converted_constant_expr): Set check_narrowing and
check_narrowing_const_only.  Give error if expr is error node.
(convert_like_real): Pass it to check_narrowing.
* cp-tree.h (check_narrowing): Add a default parameter.
* decl.c (compute_array_index_type): Use input_location instead of
location_of.
* pt.c (convert_nontype_argument): Return NULL_TREE if tf_error.
* typeck2.c (check_narrowing): Don't warn for instantiation-dependent
expressions.  Call maybe_constant_value instead of
fold_non_dependent_expr.  Don't mention { } in diagnostic.  Only check
narrowing for constants if CONST_ONLY.

* g++.dg/cpp0x/Wnarrowing6.C: New test.
* g++.dg/cpp0x/Wnarrowing7.C: New test.
* g++.dg/cpp0x/Wnarrowing8.C: New test.
* g++.dg/cpp0x/Wnarrowing9.C: New test.
* g++.dg/cpp0x/Wnarrowing10.C: New test.

* g++.dg/cpp0x/constexpr-47969.C: Adjust dg-error.
* g++.dg/cpp0x/constexpr-ex2.C: Likewise.
* g++.dg/cpp0x/constexpr-targ.C: Likewise.
* g++.dg/cpp0x/scoped_enum2.C: Likewise.
* g++.dg/ext/stmtexpr15.C: Likewise.
* g++.dg/gomp/pr47963.C: Likewise.
* g++.dg/init/new37.C: Likewise.
* g++.dg/init/new43.C: Likewise.
* g++.dg/other/fold1.C: Likewise.
* g++.dg/parse/array-size2.C: Likewise.
* g++.dg/template/dependent-name3.C: Likewise.
* g++.dg/cpp0x/constexpr-data2.C: Add dg-error.
* g++.dg/other/vrp1.C: Likewise.
* g++.dg/template/char1.C: Likewise.

diff --git gcc/cp/call.c gcc/cp/call.c
index 209c1fd2f0e..62654a9e407 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -107,6 +107,9 @@ struct conversion {
  binding a reference directly or decaying to a pointer.  */
   BOOL_BITFIELD rvaluedness_matches_p: 1;
   BOOL_BITFIELD check_narrowing: 1;
+  /* Whether check_narrowing should only check TREE_CONSTANTs; used
+ in build_converted_constant_expr.  */
+  BOOL_BITFIELD check_narrowing_const_only: 1;
   /* The type of the expression resulting from the conversion.  */
   tree type;
   union {
@@ -4152,9 +4155,18 @@ build_converted_constant_expr (tree type, tree expr, 
tsubst_flags_t complain)
 }
 
   if (conv)
-expr = convert_like (conv, expr, complain);
+{
+  conv->check_narrowing = true;
+  conv->check_narrowing_const_only = true;
+  expr = convert_like (conv, expr, complain);
+}
   else
-expr = error_mark_node;
+{
+  if (complain & tf_error)
+   error_at (loc, "could not convert %qE from %qH to %qI", expr,
+ TREE_TYPE (expr), type);
+  expr = 

[PATCH 6/6] detect unterminated const arrays in strnlen calls (PR 86552)

2018-08-13 Thread Martin Sebor

The attached changes implement the detection of past-the-end reads
by strncpy due to unterminated arguments and excessive bounds.

PR tree-optimization/86552 - missing warning for reading past the end of non-string arrays

gcc/ChangeLog:
	* builtins.c (expand_builtin_strnlen): Detect, avoid expanding,
	and diagnose unterminated arrays.

gcc/testsuite/ChangeLog:
	* gcc.dg/warn-strnlen-no-nul.c: New.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 2f493d3..46df2ea 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -582,11 +582,16 @@ warn_string_no_nul (location_t loc, tree exp, tree fndecl, tree nonstr)
 
 /* If EXP refers to an unterminated constant character array return
the declaration of the object of which the array is a member or
-   element.  Otherwise return null.  */
+   element and if SIZE is not null, set *SIZE to the size of
+   the unterminated array and set *EXACT if the size is exact or
+   clear it otherwise.  Otherwise return null.  */
 
 tree
-unterminated_array (tree exp)
+unterminated_array (tree exp, tree *size /* = NULL */, bool *exact /* = NULL */)
 {
+  /* Offset from the beginning of the array or null.  */
+  tree off = NULL_TREE;
+
   if (TREE_CODE (exp) == SSA_NAME)
 {
   gimple *stmt = SSA_NAME_DEF_STMT (exp);
@@ -595,18 +600,43 @@ unterminated_array (tree exp)
 
   tree rhs1 = gimple_assign_rhs1 (stmt);
   tree_code code = gimple_assign_rhs_code (stmt);
-  if (code == ADDR_EXPR
-	  && TREE_CODE (TREE_OPERAND (rhs1, 0)) == ARRAY_REF)
-	rhs1 = rhs1;
-  else if (code != POINTER_PLUS_EXPR)
+  if ((code == ADDR_EXPR
+	   && TREE_CODE (TREE_OPERAND (rhs1, 0)) == ARRAY_REF)
+	  || code == POINTER_PLUS_EXPR)
+	{
+	  /* Store the index or offset.  */
+	  off = gimple_assign_rhs2 (stmt);
+	  exp = rhs1;
+	}
+  else
 	return NULL_TREE;
-
-  exp = rhs1;
 }
 
   tree nonstr;
-  if (c_strlen (exp, 1, ) && nonstr)
-return nonstr;
+  tree len = c_strlen (exp, 1, );
+  if (len && nonstr)
+{
+  if (size)
+	{
+	  if (off)
+	{
+	  if (TREE_CODE (off) == INTEGER_CST)
+		{
+		  /* Subtract the offset from the size of the array.  */
+		  *exact = true;
+		  off = fold_convert (ssizetype, off);
+		  len = fold_build2 (MINUS_EXPR, ssizetype, len, off);
+		}
+	  else
+		*exact = false;
+	}
+	  else
+	*exact = true;
+
+	  *size = len;
+	}
+  return nonstr;
+}
 
   return NULL_TREE;
 }
@@ -3068,7 +3098,8 @@ expand_builtin_strnlen (tree exp, rtx target, machine_mode target_mode)
   tree maxobjsize = max_object_size ();
   tree func = get_callee_fndecl (exp);
 
-  tree len = c_strlen (src, 0);
+  tree nonstr;
+  tree len = c_strlen (src, 0, );
 
   if (TREE_CODE (bound) == INTEGER_CST)
 {
@@ -3080,8 +3111,41 @@ expand_builtin_strnlen (tree exp, rtx target, machine_mode target_mode)
 			 exp, func, bound, maxobjsize))
 	  TREE_NO_WARNING (exp) = true;
 
+  bool exact = true;
   if (!len || TREE_CODE (len) != INTEGER_CST)
-	return NULL_RTX;
+	{
+	  /* Clear EXACT if LEN may be less than SRC suggests,
+	 such as in
+	   strnlen ([i], sizeof a)
+	 where the value of i is unknown.  Unless i's value is
+	 zero, the call is unsafe because the bound is greater. */
+	  nonstr = unterminated_array (src, , );
+	  if (!nonstr)
+	return NULL_RTX;
+	}
+
+  if (nonstr
+	  && !TREE_NO_WARNING (exp)
+	  && (tree_int_cst_lt (len, bound)
+	  || !exact))
+	{
+	  location_t warnloc
+	= expansion_point_location_if_in_system_header (loc);
+
+	  if (warning_at (warnloc, OPT_Wstringop_overflow_,
+			  exact
+			  ? G_("%K%qD specified bound %E exceeds the size %E "
+			   "of unterminated array")
+			  : G_("%K%qD specified bound %E may exceed the size "
+			   "of at most %E of unterminated array"),
+			  exp, func, bound, len))
+	{
+	  inform (DECL_SOURCE_LOCATION (nonstr),
+		  "referenced argument declared here");
+	  TREE_NO_WARNING (exp) = true;
+	  return NULL_RTX;
+	}
+	}
 
   len = fold_convert_loc (loc, size_type_node, len);
   len = fold_build2_loc (loc, MIN_EXPR, size_type_node, len, bound);
@@ -3107,6 +3171,18 @@ expand_builtin_strnlen (tree exp, rtx target, machine_mode target_mode)
   if (!len || TREE_CODE (len) != INTEGER_CST)
 return NULL_RTX;
 
+  if (!TREE_NO_WARNING (exp)
+  && wi::ltu_p (wi::to_wide (len), min)
+  && warning_at (loc, OPT_Wstringop_overflow_,
+		 "%K%qD specified bound [%wu, %wu] "
+		 "exceeds the size %E of unterminated array",
+		 exp, func, min.to_uhwi (), max.to_uhwi (), len))
+{
+  inform (DECL_SOURCE_LOCATION (nonstr),
+	  "referenced argument declared here");
+  TREE_NO_WARNING (exp) = true;
+}
+
   if (wi::gtu_p (min, wi::to_wide (len)))
 return expand_expr (len, target, target_mode, EXPAND_NORMAL);
 
diff --git a/gcc/builtins.h b/gcc/builtins.h
index f722dd8..c55fa6b 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -103,7 +103,7 @@ extern bool target_char_cst_p 

[PATCH 5/6] detect unterminated const arrays in stpcpy calls (PR 86552)

2018-08-13 Thread Martin Sebor

The attached changes implement the detection of past-the-end reads
by stpcpy due to unterminated arguments.

PR tree-optimization/86552 - missing warning for reading past the end of non-string arrays

gcc/ChangeLog:

	* builtins.c (unterminated_array): Handle ARRAY_REF.
	(expand_builtin_stpcpy_1): Detect unterminated char arrays.
	* builtins.h (unterminated_array): Declare extern.
	* gimple-fold.c (gimple_fold_builtin_stpcpy): Detect unterminated
	  arrays.
	(gimple_fold_builtin_sprintf): Propagate NO_WARNING to transformed
	calls.

gcc/testsuite/ChangeLog:

	* gcc.dg/warn-stpcpy-no-nul.c: New test.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index a77f25c..2f493d3 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -584,7 +584,7 @@ warn_string_no_nul (location_t loc, tree exp, tree fndecl, tree nonstr)
the declaration of the object of which the array is a member or
element.  Otherwise return null.  */
 
-static tree
+tree
 unterminated_array (tree exp)
 {
   if (TREE_CODE (exp) == SSA_NAME)
@@ -595,7 +595,10 @@ unterminated_array (tree exp)
 
   tree rhs1 = gimple_assign_rhs1 (stmt);
   tree_code code = gimple_assign_rhs_code (stmt);
-  if (code != POINTER_PLUS_EXPR)
+  if (code == ADDR_EXPR
+	  && TREE_CODE (TREE_OPERAND (rhs1, 0)) == ARRAY_REF)
+	rhs1 = rhs1;
+  else if (code != POINTER_PLUS_EXPR)
 	return NULL_TREE;
 
   exp = rhs1;
@@ -4004,9 +4007,14 @@ expand_builtin_stpcpy_1 (tree exp, rtx target, machine_mode mode)
 	 compile-time, not an expression containing a string.  This is
 	 because the latter will potentially produce pessimized code
 	 when used to produce the return value.  */
-  if (! c_getstr (src) || ! (len = c_strlen (src, 0)))
+  tree nonstr = NULL_TREE;
+  if (!c_getstr (src, NULL, )
+	  || !(len = c_strlen (src, 0, )))
 	return expand_movstr (dst, src, target, /*endp=*/2);
 
+  if (nonstr && !TREE_NO_WARNING (exp))
+	warn_string_no_nul (EXPR_LOCATION (exp), exp, NULL_TREE, nonstr);
+
   lenp1 = size_binop_loc (loc, PLUS_EXPR, len, ssize_int (1));
   ret = expand_builtin_mempcpy_args (dst, src, lenp1,
 	 target, exp, /*endp=*/2);
diff --git a/gcc/builtins.h b/gcc/builtins.h
index 73b0b0b..f722dd8 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -103,6 +103,7 @@ extern bool target_char_cst_p (tree t, char *p);
 
 extern internal_fn associated_internal_fn (tree);
 extern internal_fn replacement_internal_fn (gcall *);
+extern tree unterminated_array (tree);
 extern void warn_string_no_nul (location_t, tree, tree, tree);
 extern tree max_object_size ();
 
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 3fb8d85..70cb4ef 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -2787,7 +2787,7 @@ gimple_fold_builtin_stpcpy (gimple_stmt_iterator *gsi)
   location_t loc = gimple_location (stmt);
   tree dest = gimple_call_arg (stmt, 0);
   tree src = gimple_call_arg (stmt, 1);
-  tree fn, len, lenp1;
+  tree fn, lenp1;
 
   /* If the result is unused, replace stpcpy with strcpy.  */
   if (gimple_call_lhs (stmt) == NULL_TREE)
@@ -2800,10 +2800,25 @@ gimple_fold_builtin_stpcpy (gimple_stmt_iterator *gsi)
   return true;
 }
 
-  len = c_strlen (src, 1);
+  /* Set to non-null if ARG refers to an unterminated array.  */
+  tree nonstr;
+  tree len = c_strlen (src, 1, );
   if (!len
   || TREE_CODE (len) != INTEGER_CST)
-return false;
+{
+  nonstr = unterminated_array (src);
+  if (!nonstr)
+	return false;
+}
+
+  if (nonstr)
+{
+  /* Avoid folding calls with unterminated arrays.  */
+  if (!gimple_no_warning_p (stmt))
+	warn_string_no_nul (loc, NULL_TREE, gimple_call_fndecl (stmt), nonstr);
+  gimple_set_no_warning (stmt, true);
+  return false;
+}
 
   if (optimize_function_for_size_p (cfun)
   /* If length is zero it's small enough.  */
@@ -3066,6 +3081,12 @@ gimple_fold_builtin_sprintf (gimple_stmt_iterator *gsi)
 	 'format' is known to contain no % formats.  */
   gimple_seq stmts = NULL;
   gimple *repl = gimple_build_call (fn, 2, dest, fmt);
+
+  /* Propagate the NO_WARNING bit to avoid issuing the same
+	 warning more than once.  */
+  if (gimple_no_warning_p (stmt))
+	gimple_set_no_warning (repl, true);
+
   gimple_seq_add_stmt_without_update (, repl);
   if (gimple_call_lhs (stmt))
 	{
@@ -3114,6 +3135,12 @@ gimple_fold_builtin_sprintf (gimple_stmt_iterator *gsi)
   /* Convert sprintf (str1, "%s", str2) into strcpy (str1, str2).  */
   gimple_seq stmts = NULL;
   gimple *repl = gimple_build_call (fn, 2, dest, orig);
+
+  /* Propagate the NO_WARNING bit to avoid issuing the same
+	 warning more than once.  */
+  if (gimple_no_warning_p (stmt))
+	gimple_set_no_warning (repl, true);
+
   gimple_seq_add_stmt_without_update (, repl);
   if (gimple_call_lhs (stmt))
 	{
diff --git a/gcc/testsuite/gcc.dg/warn-stpcpy-no-nul.c b/gcc/testsuite/gcc.dg/warn-stpcpy-no-nul.c
new file mode 100644
index 

[PATCH 4/6] detect unterminated const arrays in sprintf calls (PR 86552)

2018-08-13 Thread Martin Sebor

The attached changes implement the detection of past-the-end reads
by the sprintf family of functions due to unterminated arguments to
%s directives.
PR tree-optimization/86552 - missing warning for reading past the end of non-string arrays

gcc/ChangeLog:

	* gimple-ssa-sprintf.c (struct fmtresult): Add new member and
	initialize it.
	(get_string_length): Detect unterminated arrays.
	(format_string): Same.
	(format_directive): Warn about unterminated arrays.

gcc/testsuite/ChangeLog:

	* gcc.dg/warn-sprintf-no-nul.c: New test.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index c652c55..95ab692 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -648,7 +648,7 @@ struct fmtresult
   /* Construct a FMTRESULT object with all counters initialized
  to MIN.  KNOWNRANGE is set when MIN is valid.  */
   fmtresult (unsigned HOST_WIDE_INT min = HOST_WIDE_INT_MAX)
-  : argmin (), argmax (),
+  : argmin (), argmax (), nonstr (),
 knownrange (min < HOST_WIDE_INT_MAX),
 nullp ()
   {
@@ -662,7 +662,7 @@ struct fmtresult
  KNOWNRANGE is set when both MIN and MAX are valid.   */
   fmtresult (unsigned HOST_WIDE_INT min, unsigned HOST_WIDE_INT max,
 	 unsigned HOST_WIDE_INT likely = HOST_WIDE_INT_MAX)
-  : argmin (), argmax (),
+  : argmin (), argmax (), nonstr (),
 knownrange (min < HOST_WIDE_INT_MAX && max < HOST_WIDE_INT_MAX),
 nullp ()
   {
@@ -689,6 +689,10 @@ struct fmtresult
  results in on output for an argument in the range above.  */
   result_range range;
 
+  /* Non-nul when the argument of a string directive is not a nul
+ terminated string.  */
+  tree nonstr;
+
   /* True when the range above is obtained from a known value of
  a directive's argument or its bounds and not the result of
  heuristics that depend on warning levels.  */
@@ -2129,10 +2133,12 @@ get_string_length (tree str)
   if (!str)
 return fmtresult ();
 
-  if (tree slen = c_strlen (str, 1))
+  tree arr;
+  if (tree slen = c_strlen (str, 1, ))
 {
   /* Simply return the length of the string.  */
   fmtresult res (tree_to_shwi (slen));
+  res.nonstr = arr;
   return res;
 }
 
@@ -2140,9 +2146,11 @@ get_string_length (tree str)
  by STR.  Strings of unknown lengths are bounded by the sizes of
  arrays that subexpressions of STR may refer to.  Pointers that
  aren't known to point any such arrays result in LENRANGE[1] set
- to SIZE_MAX.  */
+ to SIZE_MAX.  NONSTR is set to the declaration of the constant
+ array that is known not to be nul-terminated.  */
   tree lenrange[2];
-  bool flexarray = get_range_strlen (str, lenrange);
+  tree nonstr;
+  bool flexarray = get_range_strlen (str, lenrange, false, );
 
   if (lenrange [0] || lenrange [1])
 {
@@ -2165,6 +2173,7 @@ get_string_length (tree str)
 	max = HOST_WIDE_INT_M1U;
 
   fmtresult res (min, max);
+  res.nonstr = nonstr;
 
   /* Set RES.KNOWNRANGE to true if and only if all strings referenced
 	 by STR are known to be bounded (though not necessarily by their
@@ -2422,6 +2431,11 @@ format_string (const directive , tree arg, vr_values *)
   res.range.unlikely = res.range.max;
 }
 
+  /* If the argument isn't a nul-terminated string and the number
+ of bytes on output isn't bounded by precision, set NONSTR.  */
+  if (slen.nonstr && slen.range.min < (unsigned HOST_WIDE_INT)dir.prec[0])
+res.nonstr = slen.nonstr;
+
   /* Bump up the byte counters if WIDTH is greater.  */
   return res.adjust_for_width_or_precision (dir.width);
 }
@@ -2988,6 +3002,18 @@ format_directive (const sprintf_dom_walker::call_info ,
 			  fmtres.range.min, fmtres.range.max);
 }
 
+  if (!warned && fmtres.nonstr)
+{
+  warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (),
+			"%<%.*s%> directive argument is not a nul-terminated "
+			"string",
+			dirlen,
+			target_to_host (hostdir, sizeof hostdir, dir.beg));
+  if (warned && DECL_P (fmtres.nonstr))
+	inform (DECL_SOURCE_LOCATION (fmtres.nonstr),
+		"referenced argument declared here");
+}
+
   if (warned && fmtres.range.min < fmtres.range.likely
   && fmtres.range.likely < fmtres.range.max)
 inform_n (info.fmtloc, fmtres.range.likely,
@@ -4033,6 +4059,8 @@ sprintf_dom_walker::handle_gimple_call (gimple_stmt_iterator *gsi)
   format_result res = format_result ();
 
   bool success = compute_format_length (info, );
+  if (res.warned)
+gimple_set_no_warning (info.callstmt, true);
 
   /* When optimizing and the printf return value optimization is enabled,
  attempt to substitute the computed result for the return value of
diff --git a/gcc/testsuite/gcc.dg/warn-sprintf-no-nul.c b/gcc/testsuite/gcc.dg/warn-sprintf-no-nul.c
new file mode 100644
index 000..b331bb5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/warn-sprintf-no-nul.c
@@ -0,0 +1,90 @@
+/* PR tree-optimization/86552 - missing warning for reading past the end
+   of non-string arrays
+   Exercise 

[PATCH 3/6] detect unterminated const arrays in strcpy calls (PR 86552)

2018-08-13 Thread Martin Sebor

The attached changes implement the detection of past-the-end reads
by strcpy due to unterminated arguments.
PR tree-optimization/86552 - missing warning for reading past the end of non-string arrays

gcc/ChangeLog:

	* builtins.c (unterminated_array): New.
	(expand_builtin_strcpy): Adjust.
	(expand_builtin_strcpy_args): Detect unterminated arrays.
	* gimple-fold.c (get_maxval_strlen): Add argument.  Detect
	unterminated arrays.
	* gimple-fold.h (get_maxval_strlen): Add argument.
	(gimple_fold_builtin_strcpy): Detec unterminated arrays.

gcc/testsuite/ChangeLog:

	* gcc.dg/warn-strcpy-no-nul.c: New test.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 78ced93..a77f25c 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -132,7 +132,7 @@ static rtx expand_builtin_mempcpy (tree, rtx);
 static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx, tree, int);
 static rtx expand_builtin_strcat (tree, rtx);
 static rtx expand_builtin_strcpy (tree, rtx);
-static rtx expand_builtin_strcpy_args (tree, tree, rtx);
+static rtx expand_builtin_strcpy_args (tree, tree, tree, rtx);
 static rtx expand_builtin_stpcpy (tree, rtx, machine_mode);
 static rtx expand_builtin_stpncpy (tree, rtx);
 static rtx expand_builtin_strncat (tree, rtx);
@@ -580,6 +580,34 @@ warn_string_no_nul (location_t loc, tree exp, tree fndecl, tree nonstr)
 inform (DECL_SOURCE_LOCATION (nonstr), "referenced argument declared here");
 }
 
+/* If EXP refers to an unterminated constant character array return
+   the declaration of the object of which the array is a member or
+   element.  Otherwise return null.  */
+
+static tree
+unterminated_array (tree exp)
+{
+  if (TREE_CODE (exp) == SSA_NAME)
+{
+  gimple *stmt = SSA_NAME_DEF_STMT (exp);
+  if (!is_gimple_assign (stmt))
+	return NULL_TREE;
+
+  tree rhs1 = gimple_assign_rhs1 (stmt);
+  tree_code code = gimple_assign_rhs_code (stmt);
+  if (code != POINTER_PLUS_EXPR)
+	return NULL_TREE;
+
+  exp = rhs1;
+}
+
+  tree nonstr;
+  if (c_strlen (exp, 1, ) && nonstr)
+return nonstr;
+
+  return NULL_TREE;
+}
+
 /* Compute the length of a null-terminated character string or wide
character string handling character sizes of 1, 2, and 4 bytes.
TREE_STRING_LENGTH is not the right way because it evaluates to
@@ -3902,7 +3930,7 @@ expand_builtin_strcpy (tree exp, rtx target)
 		src, destsize);
 }
 
-  if (rtx ret = expand_builtin_strcpy_args (dest, src, target))
+  if (rtx ret = expand_builtin_strcpy_args (exp, dest, src, target))
 {
   /* Check to see if the argument was declared attribute nonstring
 	 and if so, issue a warning since at this point it's not known
@@ -3922,8 +3950,17 @@ expand_builtin_strcpy (tree exp, rtx target)
expand_builtin_strcpy.  */
 
 static rtx
-expand_builtin_strcpy_args (tree dest, tree src, rtx target)
+expand_builtin_strcpy_args (tree exp, tree dest, tree src, rtx target)
 {
+  /* Detect strcpy calls with unterminated arrays..  */
+  if (tree nonstr = unterminated_array (src))
+{
+  /* NONSTR refers to the non-nul terminated constant array.  */
+  if (!TREE_NO_WARNING (exp))
+	warn_string_no_nul (EXPR_LOCATION (exp), exp, NULL_TREE, nonstr);
+  return NULL_RTX;
+}
+
   return expand_movstr (dest, src, target, /*endp=*/0);
 }
 
@@ -3983,7 +4020,7 @@ expand_builtin_stpcpy_1 (tree exp, rtx target, machine_mode mode)
 
 	  if (CONST_INT_P (len_rtx))
 	{
-	  ret = expand_builtin_strcpy_args (dst, src, target);
+	  ret = expand_builtin_strcpy_args (exp, dst, src, target);
 
 	  if (ret)
 		{
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 5c88e33..3fb8d85 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -1591,21 +1591,30 @@ get_range_strlen (tree arg, tree minmaxlen[2], bool strict /* = false */,
 }
 
 tree
-get_maxval_strlen (tree arg, int type)
+get_maxval_strlen (tree arg, int type, tree *nonstr /* = NULL */)
 {
   bitmap visited = NULL;
   tree len[2] = { NULL_TREE, NULL_TREE };
 
   bool dummy;
   /* Set to non-null if ARG refers to an unterminated array.  */
-  tree nonstr = NULL_TREE;
-  if (!get_range_strlen (arg, len, , type, 0, , ))
+  tree mynonstr = NULL_TREE;
+  if (!get_range_strlen (arg, len, , type, 0, , ))
 len[1] = NULL_TREE;
   if (visited)
 BITMAP_FREE (visited);
 
+  if (nonstr)
+{
+  /* For callers prepared to handle unterminated arrays set
+   *NONSTR to point to the declaration of the array and return
+   the maximum length/size. */
+  *nonstr = mynonstr;
+  return len[1];
+}
+
   /* Fail if the constant array isn't nul-terminated.  */
-  return nonstr ? NULL_TREE : len[1];
+  return mynonstr ? NULL_TREE : len[1];
 }
 
 
@@ -1648,10 +1657,21 @@ gimple_fold_builtin_strcpy (gimple_stmt_iterator *gsi,
   if (!fn)
 return false;
 
-  tree len = get_maxval_strlen (src, 0);
+  /* Set to non-null if ARG refers to an unterminated array.  */
+  tree nonstr;
+  tree len = get_maxval_strlen (src, 0, );
   if 

[PATCH 1/6] prevent folding of unterminated const arrays in memchr calls (PR 86711, 86714)

2018-08-13 Thread Martin Sebor

The attached changes implement the detection of nul-terminated
constant arrays and incorrect or early folding of such arrays.
This resolves PR 86711 - wrong folding of memchr, and prevents
PR 86714 - tree-ssa-forwprop.c confused by too long initializer.
No warnings are issued.
PR tree-optimization/86714 - tree-ssa-forwprop.c confused by too long initializer
PR tree-optimization/86711 - wrong folding of memchr

gcc/ChangeLog:

	PR tree-optimization/86714
	PR tree-optimization/86711
	* builtins.h (c_strlen): Add argument.
	* builtins.c (c_strlen): Add argument and use it.
	* expr.c (string_constant): Add arguments.  Detect missing nul
	terminator and outermost declaration it's missing in.
	* expr.h (string_constant): Add argument.
	* fold-const.c (c_getstr): Change argument to tree*, rename
	other arguments.
	* fold-const-call.c (fold_const_call): Avoid folding calls with
	unterminated arrays.
	* gimple-fold.c (get_range_strlen): Add argument.
	(get_maxval_strlen): Adjust.
	* gimple-fold.h (get_range_strlen): Add argument.

gcc/testsuite/ChangeLog:

	PR tree-optimization/86714
	PR tree-optimization/86711
	* gcc.c-torture/execute/memchr-1.c: New test.
	* gcc.c-torture/execute/pr86714.c: New test.
	* gcc/testsuite/gcc.dg/strlenopt-56.c: New test.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 39611de..a7aa4b2 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -567,37 +567,55 @@ string_length (const void *ptr, unsigned eltsize, unsigned maxelts)
accesses.  Note that this implies the result is not going to be emitted
into the instruction stream.
 
+   When NONSTR is non-null and the string is not properly nul-terminated,
+   set *NONSTR to the declaration of the outermost constant object whose
+   initializer (or one of its elements) is not nul-terminated.
+
The value returned is of type `ssizetype'.
 
Unfortunately, string_constant can't access the values of const char
arrays with initializers, so neither can we do so here.  */
 
 tree
-c_strlen (tree src, int only_value)
+c_strlen (tree src, int only_value, tree *nonstr /* = NULL */)
 {
   STRIP_NOPS (src);
+
+  /* Used to detect non-nul-terminated strings in subexpressions
+ of a conditional expression.  When NONSTR is null, point it
+ arbitrarily at one of the elements for simplicity.  */
+  tree nstrs[] = { NULL_TREE, NULL_TREE };
+  if (!nonstr)
+nonstr = nstrs;
+
   if (TREE_CODE (src) == COND_EXPR
   && (only_value || !TREE_SIDE_EFFECTS (TREE_OPERAND (src, 0
 {
-  tree len1, len2;
-
-  len1 = c_strlen (TREE_OPERAND (src, 1), only_value);
-  len2 = c_strlen (TREE_OPERAND (src, 2), only_value);
+  tree len1 = c_strlen (TREE_OPERAND (src, 1), only_value, nstrs);
+  tree len2 = c_strlen (TREE_OPERAND (src, 2), only_value, nstrs + 1);
   if (tree_int_cst_equal (len1, len2))
-	return len1;
+	{
+	  *nonstr = nstrs[0] ? nstrs[0] : nstrs[1];
+	  return len1;
+	}
 }
 
   if (TREE_CODE (src) == COMPOUND_EXPR
   && (only_value || !TREE_SIDE_EFFECTS (TREE_OPERAND (src, 0
-return c_strlen (TREE_OPERAND (src, 1), only_value);
+return c_strlen (TREE_OPERAND (src, 1), only_value, nonstr);
 
   location_t loc = EXPR_LOC_OR_LOC (src, input_location);
 
   /* Offset from the beginning of the string in bytes.  */
   tree byteoff;
-  src = string_constant (src, );
-  if (src == 0)
-return NULL_TREE;
+  src = string_constant (src, , nonstr);
+  if (!src)
+{
+  /* On failure set *NONSTR to the first non-null NSTRS element
+	 if one is non-null, or to null.  */
+  *nonstr = nstrs[0] ? nstrs[0] : nstrs[1];
+  return NULL_TREE;
+}
 
   /* Determine the size of the string element.  */
   unsigned eltsize
@@ -641,21 +659,25 @@ c_strlen (tree src, int only_value)
   if (!maxelts)
 	return ssize_int (0);
 
-  /* We don't know the starting offset, but we do know that the string
-	 has no internal zero bytes.  If the offset falls within the bounds
-	 of the string subtract the offset from the length of the string,
-	 and return that.  Otherwise the length is zero.  Take care to
-	 use SAVE_EXPR in case the OFFSET has side-effects.  */
-  tree offsave = TREE_SIDE_EFFECTS (byteoff) ? save_expr (byteoff) : byteoff;
+  /* We don't know the starting offset, but we do know that
+	 the string has no internal NUL characters.  If the byte
+	 offset falls within the bounds of the string subtract
+	 the offset from the length of the string in bytes, and
+	 return the result.  Otherwise the length is zero.  Take
+	 care to use SAVE_EXPR in case the OFFSET has side-effects.  */
+  tree offsave
+	= TREE_SIDE_EFFECTS (byteoff) ? save_expr (byteoff) : byteoff;
   offsave = fold_convert (ssizetype, offsave);
   tree condexp = fold_build2_loc (loc, LE_EXPR, boolean_type_node, offsave,
   build_int_cst (ssizetype, len * eltsize));
-  tree lenexp = size_diffop_loc (loc, ssize_int (strelts * eltsize), offsave);
+  tree lenexp = size_diffop_loc 

[PATCH 0/6] improve handling of char arrays with missing nul (PR 86552, 86711, 86714)

2018-08-13 Thread Martin Sebor

To make reviewing the changes easier I've split up the patch
into a series:

1. Detection of nul-terminated constant arrays to prevent early
   folding.  This resolves PR 86711 - wrong folding of memchr,
   and prevents PR 86714 - tree-ssa-forwprop.c confused by too
   long initializer, but doesn't warn.

2. Warn for reads past unterminated constant character arrays.
   This adds warnings for string functions called with such arrays
   to resolve PR 86552 - missing warning for reading past the end
   of non-string arrays.  Now that GCC transforms braced-initializer
   lists into STRING_CSTs (even those with no nul), the warning is
   capable of diagnosing even those.

   2.1 strlen
   2.2 strcpy
   2.3 sprintf
   2.4 stpcpy
   2.5 strnlen

There are many more string functions where unterminated (constant
or otherwise) should be diagnosed.  I plan to continue to work on
those (with the constant ones first)  but I want to post this
updated patch for review now, mainly so that the wrong code bug
(PR 86711) can be resolved and the basic detection infrastructure
agreed on.

An open question in my mind is what should GCC do with such calls
after issuing a warning: replace them with traps?  Fold them into
constants?  Or continue to pass them through to the corresponding
library functions?

Martin

On 07/25/2018 05:38 PM, Martin Sebor wrote:

Ping: https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01124.html

The fix for bug 86532 has been checked in so this enhancement
can now be applied on top of it (with only minor adjustments).

On 07/19/2018 02:08 PM, Martin Sebor wrote:

In the discussion of my patch for pr86532 Bernd noted that
GCC silently accepts constant character arrays with no
terminating nul as arguments to strlen (and other string
functions).

The attached patch is a first step in detecting these kinds
of bugs in strlen calls by issuing -Wstringop-overflow.
The next step is to modify all other handlers of built-in
functions to detect the same problem (not part of this patch).
Yet another step is to detect these problems in arguments
initialized using the non-string form:

  const char a[] = { 'a', 'b', 'c' };

This patch is meant to apply on top of the one for bug 86532
(I tested it with an earlier version of that patch so there
is code in the context that does not appear in the latest
version of the other diff).

Martin







[PATCH, Darwin] Remove unnecessary target hook.

2018-08-13 Thread Iain Sandoe
Hi

For Darwin when we switch between text sections a linker-visible symbol is
required to preserve the linker’s  “atom model”.  Some time ago we implemented
TARGET_ASM_FUNCTION_SWITCHED_TEXT_SECTIONS to provide this.

A suitable symbol is now emitted directly from final.c so the target hook 
version
needs to be removed to avoid multiple symbols at the same address.

OK for trunk?
open branches?

Iain

gcc:
* config/darwin.c
 (darwin_function_switched_text_sections): Delete.
* gcc/config/darwin.h
 (TARGET_ASM_FUNCTION_SWITCHED_TEXT_SECTIONS): Likewise.
---
 gcc/config/darwin.c | 15 ---
 gcc/config/darwin.h |  4 
 2 files changed, 19 deletions(-)

diff --git a/gcc/config/darwin.c b/gcc/config/darwin.c
index 3a08aea07b..a31cb08b60 100644
--- a/gcc/config/darwin.c
+++ b/gcc/config/darwin.c
@@ -3711,19 +3711,4 @@ default_function_sections:
: text_section;
 }
 
-/* When a function is partitioned between sections, we need to insert a label
-   at the start of each new chunk - so that it may become a valid 'atom' for
-   eh and debug purposes.  Without this the linker will emit warnings if one 
-   tries to add line location information (since the switched fragment will 
-   be anonymous).  */
-
-void
-darwin_function_switched_text_sections (FILE *fp, tree decl, bool new_is_cold)
-{
-  /* Make sure we pick up all the relevant quotes etc.  */
-  assemble_name_raw (fp, new_is_cold ? "__cold_sect_of_" : "__hot_sect_of_");
-  assemble_name_raw (fp, IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
-  fputs (":\n", fp);
-}
-
 #include "gt-darwin.h"
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index 980ad9b405..a65ae441a0 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -708,10 +708,6 @@ extern GTY(()) section * 
darwin_sections[NUM_DARWIN_SECTIONS];
 #undef TARGET_ASM_FUNCTION_SECTION
 #define TARGET_ASM_FUNCTION_SECTION darwin_function_section
 
-#undef TARGET_ASM_FUNCTION_SWITCHED_TEXT_SECTIONS
-#define TARGET_ASM_FUNCTION_SWITCHED_TEXT_SECTIONS \
-   darwin_function_switched_text_sections
-
 #undef TARGET_ASM_SELECT_RTX_SECTION
 #define TARGET_ASM_SELECT_RTX_SECTION machopic_select_rtx_section
 #undef  TARGET_ASM_UNIQUE_SECTION
-- 
2.17.1




Re: [PATCH, OpenACC] Add support for gang local storage allocation in shared memory

2018-08-13 Thread Julian Brown
On Mon, 13 Aug 2018 11:42:26 -0700
Cesar Philippidis  wrote:

> On 08/13/2018 09:21 AM, Julian Brown wrote:
> 
> > diff --git
> > a/libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c
> > b/libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c new file
> > mode 100644 index 000..2fa708a --- /dev/null
> > +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c
> > @@ -0,0 +1,106 @@
> > +/* { dg-xfail-run-if "gangprivate
> > failure" { openacc_nvidia_accel_selected } { "-O0" } { "" } } */  
> 
> As a quick comment, I like the approach that you've taken with this
> patch, but the og8 patch only applies the gangprivate attribute in the
> c/c++ FE. I'd have to review the notes, but I seem to recall that
> excluding that clause in fortran was deliberate. Chung-Lin, do you
> recall the rationale behind that?
> 
> With that aside, is the above xfail still necessary? It seems to xpass
> for me on nvptx. However, I see this regression on the host:
> 
> FAIL: libgomp.oacc-c/../libgomp.oacc-c-c++-common/loop-gwv-2.c
> -DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1  -O2  execution test
> 
> There could be other regressions, but I only tested the new tests
> introduced by the patch so far.

Oops, this was the version of the patch I meant to post (and the one I
tested). The XFAIL on loop-gwv-2.c isn't necessary, plus that test
needed some other fixes to make it pass for NVPTX (it was written for
GCN to start with).

Everything else is the same. I'll see what I can come up with for a
Fortran test.

Thanks,

Julian
commit 7834b2f0dffec3e56e510c04e1663424b778fdfb
Author: Julian Brown 
Date:   Thu Aug 9 20:27:04 2018 -0700

[OpenACC] Add support for gang local storage allocation in shared memory

2018-08-10  Julian Brown  
	Chung-Lin Tang  

	gcc/
	* config/nvptx/nvptx.c (tree-hash-traits.h): Include.
	(gangprivate_shared_size): New global variable.
	(gangprivate_shared_align): Likewise.
	(gangprivate_shared_sym): Likewise.
	(gangprivate_shared_hmap): Likewise.
	(nvptx_option_override): Initialize gangprivate_shared_sym,
	gangprivate_shared_align.
	(nvptx_file_end): Output gangprivate_shared_sym.
	(nvptx_goacc_expand_accel_var): New function.
	(nvptx_set_current_function): New function.
	(TARGET_SET_CURRENT_FUNCTION): Define hook.
	(TARGET_GOACC_EXPAND_ACCEL): Likewise.
	* doc/tm.texi (TARGET_GOACC_EXPAND_ACCEL_VAR): Document new hook.
	* doc/tm.texi.in (TARGET_GOACC_EXPAND_ACCEL_VAR): Likewise.
	* expr.c (expand_expr_real_1): Remap decls marked with the
	"oacc gangprivate" atttribute.
	* omp-low.c (omp_context): Add oacc_partitioning_level and oacc_decls
	fields.
	(new_omp_context): Initialize oacc_decls in new omp_context.
	(delete_omp_context): Delete oacc_decls in old omp_context.
	(lower_oacc_head_tail): Record partitioning-level count in omp context.
	(oacc_record_private_var_clauses, oacc_record_vars_in_bind)
	(mark_oacc_gangprivate): New functions.
	(lower_omp_for): Call oacc_record_private_var_clauses with "for"
	clauses.  Call mark_oacc_gangprivate for gang-partitioned loops.
	(lower_omp_target): Call oacc_record_private_var_clauses with "target"
	clauses.
	Call mark_oacc_gangprivate for offloaded target regions.
	(lower_omp_1): Call vars_in_bind for GIMPLE_BIND within OMP regions.
	* target.def (expand_accel_var): New hook.

	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/gang-private-1.c: New test.
	* testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c: New test.
	* testsuite/libgomp.oacc-c/pr85465.c: New test.

diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index c0b0a2e..14eb842 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -73,6 +73,7 @@
 #include "cfgloop.h"
 #include "fold-const.h"
 #include "intl.h"
+#include "tree-hash-traits.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -137,6 +138,12 @@ static unsigned worker_red_size;
 static unsigned worker_red_align;
 static GTY(()) rtx worker_red_sym;
 
+/* Shared memory block for gang-private variables.  */
+static unsigned gangprivate_shared_size;
+static unsigned gangprivate_shared_align;
+static GTY(()) rtx gangprivate_shared_sym;
+static hash_map gangprivate_shared_hmap;
+
 /* Global lock variable, needed for 128bit worker & gang reductions.  */
 static GTY(()) tree global_lock_var;
 
@@ -210,6 +217,10 @@ nvptx_option_override (void)
   SET_SYMBOL_DATA_AREA (worker_red_sym, DATA_AREA_SHARED);
   worker_red_align = GET_MODE_ALIGNMENT (SImode) / BITS_PER_UNIT;
 
+  gangprivate_shared_sym = gen_rtx_SYMBOL_REF (Pmode, "__gangprivate_shared");
+  SET_SYMBOL_DATA_AREA (gangprivate_shared_sym, DATA_AREA_SHARED);
+  gangprivate_shared_align = GET_MODE_ALIGNMENT (SImode) / BITS_PER_UNIT;
+
   diagnose_openacc_conflict (TARGET_GOMP, "-mgomp");
   diagnose_openacc_conflict (TARGET_SOFT_STACK, "-msoft-stack");
   

Re: [PATCH] Use getentropy() for seeding PRNG

2018-08-13 Thread Janne Blomqvist
On Mon, Aug 13, 2018 at 5:36 PM, Fritz Reese  wrote:

> On Fri, Aug 3, 2018 at 9:19 AM Janne Blomqvist
>  wrote:
> >
> > The getentropy function, found on Linux, OpenBSD, and recently also
> > FreeBSD, can be used to get random bytes to initialize the PRNG.  It
> > is similar to the traditional way of reading from /dev/urandom, but
> > being a system call rather than a special file, it doesn't suffer from
> > problems like running out of file descriptors, or failure when running
> > in a container where /dev/urandom is not available.
> >
> > Regtested on x86_64-pc-linux-gnu, Ok for trunk?
>
> Actually, getentropy() is similar to reading from /dev/random, where
> getrandom() is similar to reading from /dev/urandom.


No, getentropy is similar to getrandom with the flags argument == 0. Which
is similar to reading /dev/urandom, except that just after boot if enough
entropy hasn't yet been gathered, it may block instead of returning some
not-quite-random data. But once it has been initialized, it will never
block again.

I agree that reading from /dev/random is overkill, but this patch isn't
doing the equivalent of that.


> Since the
> original behavior of getosrandom() is to read from /dev/urandom, I
> think it is better to use getrandom() for consistent semantics.
>
> Furthermore, getentropy() may block to achieve an appropriate degree
> of randomness, since it is intended for secure use.


The only time this might happen is just after boot, after that the entropy
never drains (in contrast to /dev/random). So unless you're planning to
write an init daemon in Fortran, this shouldn't matter.



-- 
Janne Blomqvist


Re: [PATCH] Use getentropy() for seeding PRNG

2018-08-13 Thread Jakub Jelinek
On Mon, Aug 13, 2018 at 01:12:12PM +0300, Janne Blomqvist wrote:
> PING

LGTM.

> > diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/
> > random.c
> > index 234c5ff95fd..229fa6995c0 100644
> > --- a/libgfortran/intrinsics/random.c
> > +++ b/libgfortran/intrinsics/random.c
> > @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen)
> >  rand_s ([i]);
> >return buflen;
> >  #else
> > -  /*
> > - TODO: When glibc adds a wrapper for the getrandom() system call
> > - on Linux, one could use that.
> > -
> > - TODO: One could use getentropy() on OpenBSD.  */
> > +#ifdef HAVE_GETENTROPY
> > +  if (getentropy (buf, buflen) == 0)
> > +return 0;
> > +#endif
> >int flags = O_RDONLY;
> >  #ifdef O_CLOEXEC
> >flags |= O_CLOEXEC;
> >
> >
> >
> > Just to be sure, I regtested this slightly modified patch as well. Ok for
> > trunk?
> >
> > --
> > Janne Blomqvist
> >

Jakub


[Patch, Fortran] PR 86935: Bad locus in ASSOCIATE statement

2018-08-13 Thread Janus Weil
Hi all,

this simple patch improves some of the diagnostics for invalid
ASSOCIATE statements:

https://github.com/janusw/gcc/commit/2f484479c741abddc8ac473cb0c1b9010397e006

In particular it gives a more precise locus and improved wording,
which is achieved basically by splitting a 'gfc_match' into two
separate ones. Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


[PATCH] rs6000: Fix pr56605.c

2018-08-13 Thread Segher Boessenkool
After the combine 2-2 changes, this testcase does not have a ZERO_EXTEND
in the intermediate code, but an AND instead.

This adjusts the testcase.  Committing.


2018-08-13  Segher Boessenkool  

gcc/testcuite/
* gcc.target/powerpc/pr56605.c: The generated code can have an AND
instead of a ZERO_EXTEND.

---
 gcc/testsuite/gcc.target/powerpc/pr56605.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/pr56605.c 
b/gcc/testsuite/gcc.target/powerpc/pr56605.c
index dc87640..304d6d6 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr56605.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr56605.c
@@ -12,5 +12,5 @@ void foo (short* __restrict sb, int* __restrict ia)
 ia[i] = (int) sb[i];
 }
 
-/* { dg-final { scan-rtl-dump-times "\\\(compare:CC \\\(zero_extend:DI 
\\\(reg:SI" 1 "combine" } } */
+/* { dg-final { scan-rtl-dump-times "\\\(compare:CC \\\((?:and|zero_extend):DI 
\\\(reg:\[SD\]I" 1 "combine" } } */
 
-- 
1.8.3.1



Re: [PATCH, OpenACC] Add support for gang local storage allocation in shared memory

2018-08-13 Thread Cesar Philippidis
On 08/13/2018 11:42 AM, Cesar Philippidis wrote:
> On 08/13/2018 09:21 AM, Julian Brown wrote:
> 
>> diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c 
>> b/libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c
>> new file mode 100644
>> index 000..2fa708a
>> --- /dev/null
>> +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c
>> @@ -0,0 +1,106 @@
>> +/* { dg-xfail-run-if "gangprivate failure" { openacc_nvidia_accel_selected 
>> } { "-O0" } { "" } } */
> 
> As a quick comment, I like the approach that you've taken with this
> patch, but the og8 patch only applies the gangprivate attribute in the
> c/c++ FE. I'd have to review the notes, but I seem to recall that
> excluding that clause in fortran was deliberate. Chung-Lin, do you
> recall the rationale behind that?

I found this in an old email:

  The older version of fortran that OpenACC supports doesn't have a
  concept of lexically scoped blocks like c/c++, so this isn't relevant
  except for explicit gang private variables.

So in other words, this is safe for fortran. It probably could use a
fortran test, because that functionality wasn't explicitly exercised in
og7/og8.

Cesar


[PATCH] PR libstdc++/45093 avoid warnings for _M_destroy_node

2018-08-13 Thread Jonathan Wakely

PR libstdc++/45093
* include/bits/stl_tree.h (_Rb_tree::_M_destroy_node(_Link_type)):
Combine definitions to avoid --detect-odr-violations warning.

Tested x86_64-linux, committed to trunk.


commit 99bf8add336b2307c09f122c8913ce9798a4cc6a
Author: Jonathan Wakely 
Date:   Mon Aug 13 13:58:49 2018 +0100

PR libstdc++/45093 avoid warnings for _M_destroy_node

PR libstdc++/45093
* include/bits/stl_tree.h (_Rb_tree::_M_destroy_node(_Link_type)):
Combine definitions to avoid --detect-odr-violations warning.

diff --git a/libstdc++-v3/include/bits/stl_tree.h 
b/libstdc++-v3/include/bits/stl_tree.h
index 0544f99f9ab..09e8d758873 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -626,10 +626,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_construct_node(__tmp, __x);
return __tmp;
   }
-
-  void
-  _M_destroy_node(_Link_type __p)
-  { get_allocator().destroy(__p->_M_valptr()); }
 #else
   template
void
@@ -658,14 +654,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  _M_construct_node(__tmp, std::forward<_Args>(__args)...);
  return __tmp;
}
+#endif
 
   void
-  _M_destroy_node(_Link_type __p) noexcept
+  _M_destroy_node(_Link_type __p) _GLIBCXX_NOEXCEPT
   {
+#if __cplusplus < 201103L
+   get_allocator().destroy(__p->_M_valptr());
+#else
_Alloc_traits::destroy(_M_get_Node_allocator(), __p->_M_valptr());
__p->~_Rb_tree_node<_Val>();
-  }
 #endif
+  }
 
   void
   _M_drop_node(_Link_type __p) _GLIBCXX_NOEXCEPT


[PATCH] Minor optimisations in operator new(size_t, align_val_t)

2018-08-13 Thread Jonathan Wakely

Thanks to Lars for the suggestions.

* libsupc++/new_opa.cc (operator new(size_t, align_val_t)): Use
__is_pow2 to check for valid alignment. Avoid branching when rounding
size to multiple of alignment.

Tested x86_64-linux, committed to trunk.


commit 766770a4f8692ef5fda2d1f987054864efb1c3a8
Author: Jonathan Wakely 
Date:   Mon Aug 13 13:56:04 2018 +0100

Minor optimisations in operator new(size_t, align_val_t)

* libsupc++/new_opa.cc (operator new(size_t, align_val_t)): Use
__is_pow2 to check for valid alignment. Avoid branching when 
rounding
size to multiple of alignment.

diff --git a/libstdc++-v3/libsupc++/new_opa.cc 
b/libstdc++-v3/libsupc++/new_opa.cc
index aa3e5dc4ce5..abb7451fafe 100644
--- a/libstdc++-v3/libsupc++/new_opa.cc
+++ b/libstdc++-v3/libsupc++/new_opa.cc
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "new"
 
 #if !_GLIBCXX_HAVE_ALIGNED_ALLOC && !_GLIBCXX_HAVE__ALIGNED_MALLOC \
@@ -105,7 +106,7 @@ operator new (std::size_t sz, std::align_val_t al)
 
   /* Alignment must be a power of two.  */
   /* XXX This should be checked by the compiler (PR 86878).  */
-  if (__builtin_expect (align & (align - 1), false))
+  if (__builtin_expect (!std::__ispow2(align), false))
 _GLIBCXX_THROW_OR_ABORT(bad_alloc());
 
   /* malloc (0) is unpredictable; avoid it.  */
@@ -120,8 +121,7 @@ operator new (std::size_t sz, std::align_val_t al)
 align = sizeof(void*);
 # endif
   /* C11: the value of size shall be an integral multiple of alignment.  */
-  if (std::size_t rem = sz & (align - 1))
-sz += align - rem;
+  sz = (sz + align - 1) & ~(align - 1);
 #endif
 
   void *p;


[PATCH] Add and to freestanding headers

2018-08-13 Thread Jonathan Wakely

* include/Makefile.am: Install  and  for freestanding.
* include/Makefile.in: Regenerate.
* testsuite/17_intro/freestanding.cc: Check for  and .

Tested x86_64-linux, committed to trunk.


commit 4c758384cab0b274b0d7626411483c5b366b308d
Author: Jonathan Wakely 
Date:   Mon Aug 13 13:39:48 2018 +0100

Add  and  to freestanding headers

* include/Makefile.am: Install  and  for freestanding.
* include/Makefile.in: Regenerate.
* testsuite/17_intro/freestanding.cc: Check for  and .

diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
index 70db3cb6260..271695806ff 100644
--- a/libstdc++-v3/include/Makefile.am
+++ b/libstdc++-v3/include/Makefile.am
@@ -1359,10 +1359,10 @@ install-data-local: install-freestanding-headers
 endif
 
 # This is a subset of the full install-headers rule.  We only need ,
-# , , , , , , ,
-# , , , , ,
-# , , , and any files which they include (and
-# which we provide).
+# , , , , , , ,
+# , , , , , ,
+# , , , , and any files which they include
+# (and which we provide).
 # , , , and  are installed by
 # libsupc++, so only the others and the sub-includes are copied here.
 install-freestanding-headers:
@@ -1375,7 +1375,7 @@ install-freestanding-headers:
  ${glibcxx_srcdir}/$(CPU_DEFINES_SRCDIR)/cpu_defines.h; do \
  $(INSTALL_DATA) $${file} $(DESTDIR)${host_installdir}; done
$(mkinstalldirs) $(DESTDIR)${gxx_include_dir}/${std_builddir}
-   for file in limits type_traits atomic; do \
+   for file in limits type_traits atomic bit version; do \
  $(INSTALL_DATA) ${std_builddir}/$${file} 
$(DESTDIR)${gxx_include_dir}/${std_builddir}; done
$(mkinstalldirs) $(DESTDIR)${gxx_include_dir}/${c_base_builddir}
for file in ciso646 cstddef cfloat climits cstdint cstdlib \
diff --git a/libstdc++-v3/testsuite/17_intro/freestanding.cc 
b/libstdc++-v3/testsuite/17_intro/freestanding.cc
index e5126ce5926..4c8498707c4 100644
--- a/libstdc++-v3/testsuite/17_intro/freestanding.cc
+++ b/libstdc++-v3/testsuite/17_intro/freestanding.cc
@@ -30,6 +30,10 @@
 #include 
 #include 
 
+// C++2a headers:
+#include 
+#include 
+
 int main()
 {
   std::exception e;
@@ -47,5 +51,10 @@ int main()
 
   std::initializer_list ilisti __attribute__((unused));
 
+#if __cplusplus > 201703L
+  static_assert( std::ispow2(256u) );
+  static_assert( __cpp_lib_void_t >= 201411L );
+#endif
+
   return 0;
 }


Re: [PATCH] libstdc++-v3: Have aligned_alloc() on Newlib

2018-08-13 Thread Jonathan Wakely

On 13/08/18 13:04 +0100, Jonathan Wakely wrote:

On 13/08/18 12:55 +0100, Szabolcs Nagy wrote:

On 09/08/18 10:08, Jonathan Wakely wrote:

On 09/08/18 06:56 +0200, Sebastian Huber wrote:

On 08/08/18 16:33, Jonathan Wakely wrote:

On 08/08/18 16:22 +0200, Ulrich Weigand wrote:

Jonathan Wakely wrote:


Aha, so newlib was using memalign previously:

@@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
 #else
 extern "C" void *memalign(std::size_t boundary, std::size_t size);
 #endif
-#define aligned_alloc memalign


Yes, exactly ... this commit introduced the regression.


OK, I've regressed the branches then - I'll fix that.


This should fix it. I'll finish testing and commit it.

Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
gcc-7-branch and gcc-8-branch, because changing newlib from using
memalign to aligned_alloc is safe.


Should I check in my patch in addition to your patch?


Yes please, on trunk and 7 and 8. It's better to use the standard
aligned_alloc if available.



but the newlib aligned_alloc is broken on baremetal targets,
it is implemented using posix_memalign which is not provided
by the newlib malloc implementation (except on cygwin)


Ouch, OK, let's revert it. Using memalign is fine.

The original problem that I think Sebastian was trying to solve should
be fixed by r263409 anyway (and was backported to all branches).


I've committed this to trunk and will do so on the branches too.


commit 6fe6d2c2256a73bca9ca3754761bd724b654db13
Author: Jonathan Wakely 
Date:   Mon Aug 13 14:05:46 2018 +0100

Revert "libstdc++-v3: Have aligned_alloc() on Newlib"

This reverts commit r263461 / 2e920cd849b3cf0a72df4f172e27676a3e70b73f
because aligned_alloc is not defined for baremetal newlib targets, see
https://gcc.gnu.org/ml/libstdc++/2018-08/msg00065.html

Revert
2018-08-10  Sebastian Huber  

PR target/85904
* configure.ac: Define HAVE_ALIGNED_ALLOC if building for
Newlib.
* configure: Regenerate.

diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index e15228dde5e..332af3706d3 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -330,7 +330,6 @@ else
 AC_DEFINE(HAVE_TANF)
 AC_DEFINE(HAVE_TANHF)
 
-AC_DEFINE(HAVE_ALIGNED_ALLOC)
 AC_DEFINE(HAVE_ICONV)
 AC_DEFINE(HAVE_MEMALIGN)
   else


C++ PATCH for c++/86499, non-local lambda with a capture-default

2018-08-13 Thread Marek Polacek
This PR complains about us accepting a lambda with a capture-default
at file scope, which seems to be forbidden.  This patch disallows
such a use.  clang gives an error, so I did the same, but maybe we
only want a pedwarn.

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

2018-08-13  Marek Polacek  

PR c++/86499
* parser.c (cp_parser_lambda_introducer): Give error if a non-local
lambda has a capture-default.

* g++.dg/cpp0x/lambda/lambda-non-local.C: New test.
* g++.dg/cpp0x/lambda/lambda-this10.C: Adjust dg-error.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 8cfcd150705..6a760136f99 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -10280,6 +10280,9 @@ cp_parser_lambda_introducer (cp_parser* parser, tree 
lambda_expr)
 {
   cp_lexer_consume_token (parser->lexer);
   first = false;
+
+  if (!(at_function_scope_p () || at_class_scope_p ()))
+   error ("non-local lambda expression cannot have a capture-default");
 }
 
   while (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_SQUARE))
diff --git gcc/testsuite/g++.dg/cpp0x/lambda/lambda-non-local.C 
gcc/testsuite/g++.dg/cpp0x/lambda/lambda-non-local.C
index e69de29bb2d..005f8f3888b 100644
--- gcc/testsuite/g++.dg/cpp0x/lambda/lambda-non-local.C
+++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-non-local.C
@@ -0,0 +1,10 @@
+// PR c++/86499
+// { dg-do compile { target c++11 } }
+
+auto l1 = [=]{}; // { dg-error "non-local lambda expression cannot have a 
capture-default" }
+auto l2 = [&]{}; // { dg-error "non-local lambda expression cannot have a 
capture-default" }
+
+namespace {
+  auto l3 = [=]{}; // { dg-error "non-local lambda expression cannot have a 
capture-default" }
+  auto l4 = [&]{}; // { dg-error "non-local lambda expression cannot have a 
capture-default" }
+}
diff --git gcc/testsuite/g++.dg/cpp0x/lambda/lambda-this10.C 
gcc/testsuite/g++.dg/cpp0x/lambda/lambda-this10.C
index b4b8e7201aa..e8e94771ef0 100644
--- gcc/testsuite/g++.dg/cpp0x/lambda/lambda-this10.C
+++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-this10.C
@@ -1,4 +1,4 @@
 // PR c++/54383
 // { dg-do compile { target c++11 } }
 
-auto foo = [&](int a) { return a > this->b; }; // { dg-error "this" }
+auto foo = [&](int a) { return a > this->b; }; // { dg-error "non-local|this" }


Re: [PATCH, OpenACC] Add support for gang local storage allocation in shared memory

2018-08-13 Thread Cesar Philippidis
On 08/13/2018 09:21 AM, Julian Brown wrote:

> diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c 
> b/libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c
> new file mode 100644
> index 000..2fa708a
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c
> @@ -0,0 +1,106 @@
> +/* { dg-xfail-run-if "gangprivate failure" { openacc_nvidia_accel_selected } 
> { "-O0" } { "" } } */

As a quick comment, I like the approach that you've taken with this
patch, but the og8 patch only applies the gangprivate attribute in the
c/c++ FE. I'd have to review the notes, but I seem to recall that
excluding that clause in fortran was deliberate. Chung-Lin, do you
recall the rationale behind that?

With that aside, is the above xfail still necessary? It seems to xpass
for me on nvptx. However, I see this regression on the host:

FAIL: libgomp.oacc-c/../libgomp.oacc-c-c++-common/loop-gwv-2.c
-DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1  -O2  execution test

There could be other regressions, but I only tested the new tests
introduced by the patch so far.

Cesar


[PATCH][debug] Add debug and earlydebug dumps

2018-08-13 Thread Tom de Vries
Hi,

With the introduction of early debug, we've added a phase in the compiler which
produces information which is not visible, unless we run the compiler in the
debugger and call debug_dwarf from dwarf2out_early_finish or some such.

This patch adds dumping of "early" and "final" debug info, into .earlydebug
and .debug dump files, such that we can follow f.i. the upper bound of a vla
type from early debug:
...
  DW_AT_upper_bound: location descriptor:
(0x7f0d645b7550) DW_OP_GNU_variable_value , 0
...
to final debug:
...
  DW_AT_upper_bound: location descriptor:
(0x7f0d645b7550) DW_OP_fbreg 18446744073709551592, 0
(0x7f0d645b7a00) DW_OP_deref 8, 0
...
to -dA annotated assembly file:
...
.uleb128 0x3# DW_AT_upper_bound
.byte   0x91# DW_OP_fbreg
.sleb128 -24
.byte   0x6 # DW_OP_deref
...

The .debug file shows the same information as the annotated assembly, but in
the same format as the "early" debug info.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

[debug] Add debug and earlydebug dumps

2018-08-13  Tom de Vries  

* cgraph.h (debuginfo_early_init, debuginfo_init, debuginfo_fini)
(debuginfo_start, debuginfo_stop, debuginfo_early_start)
(debuginfo_early_stop): Declare.
* cgraphunit.c (debuginfo_early_init, debuginfo_init, debuginfo_fini)
(debuginfo_start, debuginfo_stop, debuginfo_early_start)
(debuginfo_early_stop): New function.
(symbol_table::finalize_compilation_unit): Call debuginfo_early_start
and debuginfo_early_stop.
* dwarf2out.c (print_dw_val, print_loc_descr, print_die): Handle
flag_dump_noaddr and flag_dump_unnumbered.
(dwarf2out_finish, dwarf2out_early_finish): Dump dwarf.
* toplev.c (compile_file): Call debuginfo_start and debuginfo_stop.
(general_init): Call debuginfo_early_init.
(finalize): Call debuginfo_fini.
(do_compile): Call debuginfo_init.

* lto.c (lto_main):  Call debuginfo_early_start and
debuginfo_early_stop.

* gcc.c-torture/unsorted/dump-noaddr.x: Skip earlydebug and debug dumps.

---
 gcc/cgraph.h   |  8 +++
 gcc/cgraphunit.c   | 66 ++
 gcc/dwarf2out.c| 46 ---
 gcc/lto/lto.c  |  2 +
 gcc/testsuite/gcc.c-torture/unsorted/dump-noaddr.x |  7 +++
 gcc/toplev.c   |  5 ++
 6 files changed, 126 insertions(+), 8 deletions(-)

diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index a8b1b4cb3c3..2b00f0165fa 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -25,6 +25,14 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-ref.h"
 #include "plugin-api.h"
 
+extern void debuginfo_early_init (void);
+extern void debuginfo_init (void);
+extern void debuginfo_fini (void);
+extern void debuginfo_start (void);
+extern void debuginfo_stop (void);
+extern void debuginfo_early_start (void);
+extern void debuginfo_early_stop (void);
+
 class ipa_opt_pass_d;
 typedef ipa_opt_pass_d *ipa_opt_pass;
 
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 462e247328e..6649942c4fb 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -2636,6 +2636,70 @@ symbol_table::compile (void)
 }
 }
 
+static int debuginfo_early_dump_nr;
+static FILE *debuginfo_early_dump_file;
+static dump_flags_t debuginfo_early_dump_flags;
+
+static int debuginfo_dump_nr;
+static FILE *debuginfo_dump_file;
+static dump_flags_t debuginfo_dump_flags;
+
+void
+debuginfo_early_init (void)
+{
+  gcc::dump_manager *dumps = g->get_dumps ();
+  debuginfo_early_dump_nr = dumps->dump_register (".earlydebug", "earlydebug",
+ "earlydebug", DK_tree,
+ OPTGROUP_NONE,
+ false);
+  debuginfo_dump_nr = dumps->dump_register (".debug", "debug",
+"debug", DK_tree,
+OPTGROUP_NONE,
+false);
+}
+
+void
+debuginfo_init (void)
+{
+  gcc::dump_manager *dumps = g->get_dumps ();
+  debuginfo_dump_file = dump_begin (debuginfo_dump_nr, NULL);
+  debuginfo_dump_flags = dumps->get_dump_file_info (debuginfo_dump_nr)->pflags;
+  debuginfo_early_dump_file = dump_begin (debuginfo_early_dump_nr, NULL);
+  debuginfo_early_dump_flags = dumps->get_dump_file_info 
(debuginfo_early_dump_nr)->pflags;
+}
+
+void
+debuginfo_fini (void)
+{
+  if (debuginfo_dump_file)
+dump_end (debuginfo_dump_nr, debuginfo_dump_file);
+  if (debuginfo_early_dump_file)
+dump_end (debuginfo_early_dump_nr, debuginfo_early_dump_file);
+}
+
+void
+debuginfo_start (void)
+{
+  set_dump_file (debuginfo_dump_file);
+}
+
+void
+debuginfo_stop 

Re: [aarch64}: added variable issue rate feature for falkor

2018-08-13 Thread Kai Tietz
I repost updated patch containing ChangeLog entry.

Regards,
Kai
	Jim Wilson 
	Kai Tietz 

	* config/aarch64.c (aarch64_sched_reorder): Implement
	TARGET_SCHED_REORDER hook.
	(aarch64_variable_issue): Implement TARGET_SCHED_VARIABLE_ISSUE
	hook.
	(TARGET_SCHED_REORDER): Define.
	(TARGET_SCHED_VARIABLE_ISSUE): Likewise.
	* config/aarch64/falkor.md (falkor_variable_issue): New.

Index: aarch64/aarch64.c
===
--- aarch64.orig/aarch64.c
+++ aarch64/aarch64.c
@@ -914,7 +914,7 @@ static const struct tune_params qdf24xx_
   _branch_cost,
   _approx_modes,
   4, /* memmov_cost  */
-  4, /* issue_rate  */
+  8, /* issue_rate  */
   (AARCH64_FUSE_MOV_MOVK | AARCH64_FUSE_ADRP_ADD
| AARCH64_FUSE_MOVK_MOVK), /* fuseable_ops  */
   "16",	/* function_align.  */
@@ -17551,6 +17551,105 @@ aarch64_run_selftests (void)
 
 #endif /* #if CHECKING_P */
 
+/* The number of micro ops left over after issuing the last instruction in a
+   cycle.  This is subtracted from the next cycle before we start issuing insns.
+   This is initialized to 0 at the start of every basic block.  */
+static int leftover_uops = 0;
+
+/* Implement TARGET_SCHED_REORDER.  */
+
+static int
+aarch64_sched_reorder (FILE *file, int verbose,
+		   rtx_insn **ready ATTRIBUTE_UNUSED,
+		   int *n_readyp ATTRIBUTE_UNUSED,
+		   int clock)
+{
+  int can_issue_more = aarch64_sched_issue_rate ();
+
+  if ((enum attr_tune) aarch64_tune == TUNE_FALKOR)
+{
+  /* The start of a basic block.  */
+  if (clock == 0)
+	{
+	  if (leftover_uops && file && (verbose > 3))
+	fprintf (file, ";;\tLeftover uops ignored at bb start.\n");
+
+	  leftover_uops = 0;
+	}
+
+  /* Account for issue slots left over from previous cycle.  This value
+	 can be larger than the number of issue slots per cycle, so we need
+	 to check it here before scheduling any instructions.  */
+  else if (leftover_uops)
+	{
+	  can_issue_more -= leftover_uops;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tUse %d issue slots for leftover uops.\n",
+		   leftover_uops);
+	  fprintf (file, ";;\t%d issue slots left.\n", can_issue_more);
+	}
+
+	  leftover_uops = 0;
+
+	  if (can_issue_more < 0)
+	{
+	  leftover_uops = 0 - can_issue_more;
+	  can_issue_more = 0;
+
+	  if (file && (verbose > 3))
+		{
+		  fprintf (file, ";;skipping issue cycle.\n");
+		  fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+		}
+	}
+	}
+}
+
+  return can_issue_more;
+}
+
+/* Implement TARGET_SCHED_VARIABLE_ISSUE.  */
+
+static int
+aarch64_variable_issue (FILE *file, int verbose,
+			rtx_insn *insn, int more)
+{
+  if (GET_CODE (PATTERN (insn)) != USE
+  && GET_CODE (PATTERN (insn)) != CLOBBER)
+{
+  if ((enum attr_tune) aarch64_tune != TUNE_FALKOR)
+	more -= 1;
+  else
+	{
+	  int issue_slots = get_attr_falkor_variable_issue (insn);
+	  more -= issue_slots;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tInsn takes %d issue slots.\n", issue_slots);
+	  fprintf (file, ";;\t%d issue slots left.\n", more);
+	}
+
+	  /* We schedule an instruction first, and then subtract issue slots,
+	 which means the result can be negative.  We carry the extra over
+	 to the next cycle.  */
+
+	  if (more < 0)
+	{
+	  leftover_uops = 0 - more;
+	  more = 0;
+
+	  if (file && (verbose > 3))
+		fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+	}
+	}
+}
+
+  return more;
+}
+
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST aarch64_address_cost
 
@@ -17779,6 +17878,12 @@ aarch64_libgcc_floating_mode_supported_p
 #undef TARGET_SCHED_ISSUE_RATE
 #define TARGET_SCHED_ISSUE_RATE aarch64_sched_issue_rate
 
+#undef TARGET_SCHED_REORDER
+#define TARGET_SCHED_REORDER aarch64_sched_reorder
+
+#undef TARGET_SCHED_VARIABLE_ISSUE
+#define TARGET_SCHED_VARIABLE_ISSUE aarch64_variable_issue
+
 #undef TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD
 #define TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD \
   aarch64_sched_first_cycle_multipass_dfa_lookahead
Index: aarch64/falkor.md
===
--- aarch64.orig/falkor.md
+++ aarch64/falkor.md
@@ -685,3 +685,24 @@
 (define_bypass 3
   "falkor_afp_5_vxvy_mul,falkor_afp_5_vxvy_mla,falkor_afp_5_vxvy_vxvy_mul,falkor_afp_5_vxvy_vxvy_mla,falkor_afp_6_vxvy_mul,falkor_afp_6_vxvy_mla,falkor_afp_6_vxvy_vxvy_mul,falkor_afp_6_vxvy_vxvy_mla,falkor_fpdt_5_vxvy_mul,falkor_fpdt_5_vxvy_mla,falkor_fpdt_6_vxvy_mul,falkor_fpdt_6_vxvy_mla"
   "falkor_afp_5_vxvy_mla,falkor_afp_5_vxvy_vxvy_mla,falkor_afp_6_vxvy_mla,falkor_afp_6_vxvy_vxvy_mla,falkor_fpdt_5_vxvy_mla,falkor_fpdt_6_vxvy_mla")
+
+
+(define_attr "falkor_variable_issue" ""
+  (cond [
+;; A64 Instructions
+	 (eq_attr "type" 

Re: [aarch64}: added variable issue rate feature for falkor

2018-08-13 Thread Kai Tietz
2018-08-13 18:23 GMT+02:00 Segher Boessenkool :
> I cannot okay patches for aarch64.  But I did not notice other typoes,
> if that is what you're asking.

Yes. Thanks.

So, is patch ok with updated ChangeLog:

Jim Wilson 
Kai Tietz 

* config/aarch64.c (aarch64_sched_reorder): Implement
TARGET_SCHED_REORDER hook.
(aarch64_variable_issue): Implement TARGET_SCHED_VARIABLE_ISSUE
hook.
(TARGET_SCHED_REORDER): Define.
(TARGET_SCHED_VARIABLE_ISSUE): Likewise.
* config/aarch64/falkor.md (falkor_variable_issue): New.

Kai


RE: [PATCH][GCC][AArch64] Limit movmem copies to TImode copies.

2018-08-13 Thread Tamar Christina
Hi Thomas,

Thanks for the review.

I’ll correct the typo before committing if I have no other changes required by 
a maintainer.

Regards,
Tamar.

From: Thomas Preudhomme 
Sent: Monday, August 13, 2018 14:37
To: Tamar Christina 
Cc: gcc-patches@gcc.gnu.org; nd ; James Greenhalgh 
; Richard Earnshaw ; Marcus 
Shawcroft 
Subject: Re: [PATCH][GCC][AArch64] Limit movmem copies to TImode copies.

Hi Tamar,

Thanks for your patch.

Just one comment about your ChangeLog entry for the testsuiet change: shouldn't 
it mention that it is a new testcase? The patch you attached seems to create 
the file.

Best regards,

Thomas

On Mon, 13 Aug 2018 at 10:33, Tamar Christina 
mailto:tamar.christ...@arm.com>> wrote:
Hi All,

On AArch64 we have integer modes larger than TImode, and while we can generate
moves for these they're not as efficient.

So instead make sure we limit the maximum we can copy to TImode.  This means
copying a 16 byte struct will issue 1 TImode copy, which will be done using a
single STP as we expect but an CImode sized copy won't issue CImode operations.

Bootstrapped and regtested on aarch4-none-linux-gnu and no issues.
Crosstested aarch4_be-none-elf and no issues.

Ok for trunk?

Thanks,
Tamar

gcc/
2018-08-13  Tamar Christina  
mailto:tamar.christ...@arm.com>>

* config/aarch64/aarch64.c (aarch64_expand_movmem): Set TImode max.

gcc/testsuite/
2018-08-13  Tamar Christina  
mailto:tamar.christ...@arm.com>>

* gcc.target/aarch64/large_struct_copy_2.c: Add assembler scan.

--


Re: [aarch64}: added variable issue rate feature for falkor

2018-08-13 Thread Segher Boessenkool
On Mon, Aug 13, 2018 at 06:13:58PM +0200, Kai Tietz wrote:
> 2018-08-13 17:51 GMT+02:00 Segher Boessenkool :
> > On Mon, Aug 13, 2018 at 10:16:20AM +0200, Kai Tietz wrote:
> >> * config/aarch64.c (aarch64_sched_reorder): Implementing
> >> TARGET_SHED_REORDER hook.
> >> (aarch64_variable_issue): Implemented TARGET_SHED_VARIABLE_ISSUE
> >> hook.
> >> (TARGET_SHED_REORDER): Defined.
> >> (TARGET_SHED_VARIABLE_ISSUE): Likewise.
> >> * config/aarch64/falor.md (falkor_variable_issue): New.
> >
> > SCHED, not SHED :-)  And s/falor/falkor/ .
> 
> :) Thanks, otherwise ok?

I cannot okay patches for aarch64.  But I did not notice other typoes,
if that is what you're asking.


Segher


[PATCH, OpenACC] Add support for gang local storage allocation in shared memory

2018-08-13 Thread Julian Brown
This patch adds support for placing gang-private variables in NVPTX
per-CU shared memory. This is done by marking up addressable variables
declared at the appropriate parallelism level with an attribute ("oacc
gangprivate") in omp-low.c.

Target-dependent code in the NVPTX backend then modifies the symbol
associated with the variable at expand time via a new target hook
(TARGET_GOACC_EXPAND_ACCEL_VAR) in order to place it in shared memory,
which is faster to access than the ".local" memory that would otherwise
be used for such variables. This has (theoretical, at least)
consequences on program semantics, in that the shared memory is also
statically-allocated rather than obeying stack discipline -- but you
can't have recursive routine calls in OpenACC anyway, so that's no big
deal.

Other targets can use the same attribute in different ways, as
appropriate.

OK for trunk?

Thanks,

Julian

2018-08-10  Julian Brown  
Chung-Lin Tang  

gcc/
* config/nvptx/nvptx.c (tree-hash-traits.h): Include.
(gangprivate_shared_size): New global variable.
(gangprivate_shared_align): Likewise.
(gangprivate_shared_sym): Likewise.
(gangprivate_shared_hmap): Likewise.
(nvptx_option_override): Initialize gangprivate_shared_sym,
gangprivate_shared_align.
(nvptx_file_end): Output gangprivate_shared_sym.
(nvptx_goacc_expand_accel_var): New function.
(nvptx_set_current_function): New function.
(TARGET_SET_CURRENT_FUNCTION): Define hook.
(TARGET_GOACC_EXPAND_ACCEL): Likewise.
* doc/tm.texi (TARGET_GOACC_EXPAND_ACCEL_VAR): Document new hook.
* doc/tm.texi.in (TARGET_GOACC_EXPAND_ACCEL_VAR): Likewise.
* expr.c (expand_expr_real_1): Remap decls marked with the
"oacc gangprivate" atttribute.
* omp-low.c (omp_context): Add oacc_partitioning_level and oacc_decls
fields.
(new_omp_context): Initialize oacc_decls in new omp_context.
(delete_omp_context): Delete oacc_decls in old omp_context.
(lower_oacc_head_tail): Record partitioning-level count in omp context.
(oacc_record_private_var_clauses, oacc_record_vars_in_bind)
(mark_oacc_gangprivate): New functions.
(lower_omp_for): Call oacc_record_private_var_clauses with "for"
clauses.  Call mark_oacc_gangprivate for gang-partitioned loops.
(lower_omp_target): Call oacc_record_private_var_clauses with "target"
clauses.
Call mark_oacc_gangprivate for offloaded target regions.
(lower_omp_1): Call vars_in_bind for GIMPLE_BIND within OMP regions.
* target.def (expand_accel_var): New hook.

libgomp/
* testsuite/libgomp.oacc-c-c++-common/gang-private-1.c: New test.
* testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c: New test.
* testsuite/libgomp.oacc-c/pr85465.c: New test.
commit 9637e7ea887e100f35d99b8d12101f9f8a9b94e3
Author: Julian Brown 
Date:   Thu Aug 9 20:27:04 2018 -0700

[OpenACC] Add support for gang local storage allocation in shared memory

2018-08-10  Julian Brown  
	Chung-Lin Tang  

	gcc/
	* config/nvptx/nvptx.c (tree-hash-traits.h): Include.
	(gangprivate_shared_size): New global variable.
	(gangprivate_shared_align): Likewise.
	(gangprivate_shared_sym): Likewise.
	(gangprivate_shared_hmap): Likewise.
	(nvptx_option_override): Initialize gangprivate_shared_sym,
	gangprivate_shared_align.
	(nvptx_file_end): Output gangprivate_shared_sym.
	(nvptx_goacc_expand_accel_var): New function.
	(nvptx_set_current_function): New function.
	(TARGET_SET_CURRENT_FUNCTION): Define hook.
	(TARGET_GOACC_EXPAND_ACCEL): Likewise.
	* doc/tm.texi (TARGET_GOACC_EXPAND_ACCEL_VAR): Document new hook.
	* doc/tm.texi.in (TARGET_GOACC_EXPAND_ACCEL_VAR): Likewise.
	* expr.c (expand_expr_real_1): Remap decls marked with the
	"oacc gangprivate" atttribute.
	* omp-low.c (omp_context): Add oacc_partitioning_level and oacc_decls
	fields.
	(new_omp_context): Initialize oacc_decls in new omp_context.
	(delete_omp_context): Delete oacc_decls in old omp_context.
	(lower_oacc_head_tail): Record partitioning-level count in omp context.
	(oacc_record_private_var_clauses, oacc_record_vars_in_bind)
	(mark_oacc_gangprivate): New functions.
	(lower_omp_for): Call oacc_record_private_var_clauses with "for"
	clauses.  Call mark_oacc_gangprivate for gang-partitioned loops.
	(lower_omp_target): Call oacc_record_private_var_clauses with "target"
	clauses.
	Call mark_oacc_gangprivate for offloaded target regions.
	(lower_omp_1): Call vars_in_bind for GIMPLE_BIND within OMP regions.
	* target.def (expand_accel_var): New hook.

	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/gang-private-1.c: New test.
	* testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c: New test.
	* 

Re: [aarch64}: added variable issue rate feature for falkor

2018-08-13 Thread Kai Tietz
2018-08-13 17:51 GMT+02:00 Segher Boessenkool :
> Hi!
>
> On Mon, Aug 13, 2018 at 10:16:20AM +0200, Kai Tietz wrote:
>> * config/aarch64.c (aarch64_sched_reorder): Implementing
>> TARGET_SHED_REORDER hook.
>> (aarch64_variable_issue): Implemented TARGET_SHED_VARIABLE_ISSUE
>> hook.
>> (TARGET_SHED_REORDER): Defined.
>> (TARGET_SHED_VARIABLE_ISSUE): Likewise.
>> * config/aarch64/falor.md (falkor_variable_issue): New.
>
> SCHED, not SHED :-)  And s/falor/falkor/ .
>
>
> Segher

:) Thanks, otherwise ok?

Kai


Re: C++ PATCH to implement C++20 P0806R2, Deprecate implicit capture of this via [=]

2018-08-13 Thread Marek Polacek
On Mon, Aug 13, 2018 at 06:27:53PM +1200, Jason Merrill wrote:
> On Sun, Aug 12, 2018 at 3:53 PM, Marek Polacek  wrote:
> > -  var = add_capture (lambda,
> > -id,
> > -initializer,
> > -/*by_reference_p=*/
> > -   (this_capture_p
> > -|| (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda)
> > -== CPLD_REFERENCE)),
> > -   /*explicit_init_p=*/false);
> > +  var = add_capture (lambda, id, initializer,
> > +/*by_reference_p=*/
> > +(this_capture_p
> > + || (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda)
> > + == CPLD_REFERENCE)),
> > +/*explicit_init_p=*/false);
> 
> This reformatting seems unnecessary, and I prefer to avoid unnecessary
> reformatting to avoid noise in 'git blame'. The rest of the patch is
> OK.

Thanks, this is what I committed.

2018-08-13  Marek Polacek  

P0806R2 - Deprecate implicit capture of this via [=]
* lambda.c (add_default_capture): Formatting fixes.  Warn about
deprecated implicit capture of this via [=].

* g++.dg/cpp2a/lambda-this1.C: New test.
* g++.dg/cpp2a/lambda-this2.C: New test.
* g++.dg/cpp2a/lambda-this3.C: New test.

--- gcc/cp/lambda.c
+++ gcc/cp/lambda.c
@@ -695,14 +695,10 @@ tree
 add_default_capture (tree lambda_stack, tree id, tree initializer)
 {
   bool this_capture_p = (id == this_identifier);
-
   tree var = NULL_TREE;
-
   tree saved_class_type = current_class_type;
 
-  tree node;
-
-  for (node = lambda_stack;
+  for (tree node = lambda_stack;
node;
node = TREE_CHAIN (node))
 {
@@ -720,6 +716,19 @@ add_default_capture (tree lambda_stack, tree id, tree 
initializer)
 == CPLD_REFERENCE)),
/*explicit_init_p=*/false);
   initializer = convert_from_reference (var);
+
+  /* Warn about deprecated implicit capture of this via [=].  */
+  if (cxx_dialect >= cxx2a
+ && this_capture_p
+ && LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda) == CPLD_COPY
+ && !in_system_header_at (LAMBDA_EXPR_LOCATION (lambda)))
+   {
+ if (warning_at (LAMBDA_EXPR_LOCATION (lambda), OPT_Wdeprecated,
+ "implicit capture of %qE via %<[=]%> is deprecated "
+ "in C++20", this_identifier))
+   inform (LAMBDA_EXPR_LOCATION (lambda), "add explicit % or "
+   "%<*this%> capture");
+   }
 }
 
   current_class_type = saved_class_type;
--- gcc/testsuite/g++.dg/cpp2a/lambda-this1.C
+++ gcc/testsuite/g++.dg/cpp2a/lambda-this1.C
@@ -0,0 +1,51 @@
+// P0806R2
+// { dg-do compile }
+// { dg-options "-std=c++2a" }
+
+struct X {
+  int x;
+  void foo (int n) {
+auto a1 = [=] { x = n; }; // { dg-warning "implicit capture" }
+auto a2 = [=, this] { x = n; };
+auto a3 = [=, *this]() mutable { x = n; };
+auto a4 = [&] { x = n; };
+auto a5 = [&, this] { x = n; };
+auto a6 = [&, *this]() mutable { x = n; };
+
+auto a7 = [=] { // { dg-warning "implicit capture" }
+  auto a = [=] { // { dg-warning "implicit capture" }
+auto a2 = [=] { x = n; }; // { dg-warning "implicit capture" }
+  };
+};
+
+auto a8 = [=, this] {
+  auto a = [=, this] {
+auto a2 = [=, this] { x = n; };
+  };
+};
+
+auto a9 = [=, *this]() mutable {
+  auto a = [=, *this]() mutable {
+auto a2 = [=, *this]() mutable { x = n; };
+  };
+};
+
+auto a10 = [&] {
+  auto a = [&] {
+auto a2 = [&] { x = n; };
+  };
+};
+
+auto a11 = [&, this] {
+  auto a = [&, this] {
+auto a2 = [&, this] { x = n; };
+  };
+};
+
+auto a12 = [&, *this]() mutable {
+  auto a = [&, *this]() mutable {
+auto a2 = [&, *this]() mutable { x = n; };
+  };
+};
+  }
+};
--- gcc/testsuite/g++.dg/cpp2a/lambda-this2.C
+++ gcc/testsuite/g++.dg/cpp2a/lambda-this2.C
@@ -0,0 +1,51 @@
+// P0806R2
+// { dg-do compile }
+// { dg-options "-std=c++2a -Wno-deprecated" }
+
+struct X {
+  int x;
+  void foo (int n) {
+auto a1 = [=] { x = n; }; // { dg-bogus "implicit capture" }
+auto a2 = [=, this] { x = n; };
+auto a3 = [=, *this]() mutable { x = n; };
+auto a4 = [&] { x = n; };
+auto a5 = [&, this] { x = n; };
+auto a6 = [&, *this]() mutable { x = n; };
+
+auto a7 = [=] { // { dg-bogus "implicit capture" }
+  auto a = [=] { // { dg-bogus "implicit capture" }
+auto a2 = [=] { x = n; }; // { dg-bogus "implicit capture" }
+  };
+};
+
+auto a8 = [=, this] {
+  auto a = [=, this] {
+auto a2 = [=, this] { x = n; };
+  };
+};
+
+auto a9 = [=, *this]() mutable {
+  auto a = [=, *this]() mutable {
+auto a2 

Re: [aarch64}: added variable issue rate feature for falkor

2018-08-13 Thread Segher Boessenkool
Hi!

On Mon, Aug 13, 2018 at 10:16:20AM +0200, Kai Tietz wrote:
> * config/aarch64.c (aarch64_sched_reorder): Implementing
> TARGET_SHED_REORDER hook.
> (aarch64_variable_issue): Implemented TARGET_SHED_VARIABLE_ISSUE
> hook.
> (TARGET_SHED_REORDER): Defined.
> (TARGET_SHED_VARIABLE_ISSUE): Likewise.
> * config/aarch64/falor.md (falkor_variable_issue): New.

SCHED, not SHED :-)  And s/falor/falkor/ .


Segher


Re: [PATCH,nvptx] Use CUDA driver API to select default runtime launch, geometry

2018-08-13 Thread Cesar Philippidis
On 08/13/2018 08:08 AM, Tom de Vries wrote:
> On 08/13/2018 04:54 PM, Cesar Philippidis wrote:
>> Going
>> forward, how would you like to proceed with the nvptx BE vector length
>> changes.
> 
> Do you have a branch available on github containing the patch series
> you've submitted?

Yes, https://github.com/cesarjp/gcc/tree/trunk-og8-vl-private

Beware that I'm constantly rebasing that branch to keep my patches up to
date. All of the commit subject lines prefixed with [nvptx] touch the
nvptx BE. The [OpenACC] patches are either involve platform-independent
code or libgomp.

Cesar


Re: [PATCH,nvptx] Use CUDA driver API to select default runtime launch, geometry

2018-08-13 Thread Tom de Vries
On 08/13/2018 04:54 PM, Cesar Philippidis wrote:
> Going
> forward, how would you like to proceed with the nvptx BE vector length
> changes.

Do you have a branch available on github containing the patch series
you've submitted?

Thanks,
- Tom


Re: [PATCH,nvptx] Use CUDA driver API to select default runtime launch, geometry

2018-08-13 Thread Cesar Philippidis
On 08/13/2018 05:04 AM, Tom de Vries wrote:
> On 08/10/2018 08:39 PM, Cesar Philippidis wrote:
>> is that I modified the default value for vectors as follows
>>
>> +int vectors = default_dim_p[GOMP_DIM_VECTOR]
>> +  ? 0 : dims[GOMP_DIM_VECTOR];
>>
>> Technically, trunk only supports warp-sized vectors, but the fallback
>> code is already checking for the presence of vectors as so
>>
>> +if (default_dim_p[GOMP_DIM_VECTOR])
>> +  dims[GOMP_DIM_VECTOR]
>> += MIN (dims[GOMP_DIM_VECTOR],
>> +   (targ_fn->max_threads_per_block / warp_size
>> +* warp_size));
>>
> 
> That code handles the case that the default vector size is bigger than
> the function being launched allows, independent from whether that
> default is calculated by the runtime, or set by GOMP_OPENACC_DIM.
> 
> The GOMP_OPENACC_DIM part is forward compatible, given that currently
> the compiler doesn't allow the runtime to choose the vector length, and
> AFAICT that will remain the same after application of the submitted set
> of vector_length patches.
> 
>> therefore, I had the cuOccupancyMaxPotentialBlockSize code path behave
>> the same.
> 
> They don't behave the same. What you add here is ignoring
> GOMP_OPENACC_DIM[GOMP_DIM_VECTOR], not handling it. That requires a comment.

I meant, same in the sense that it inspects for a pre-defined value of
vector length; not the application of vector length. I should have been
more clear.

> Furthermore, by assigning dims[GOMP_DIM_VECTOR] at the start you break
> the pattern of the code, which:
> - first applies GOMP_OPENACC_DIM
> - then further fills in defaults as required
> - then applies defaults
> I've rewritten this bit to fit the pattern. This result is not pretty,
> but it'll do for now.  Changing the pattern may make things better
> structured, but this is something we can do in a follow up patch, and
> want to do for all dimensions at once, not just for vector, otherwise
> the code will become too convoluted.
> 
> Btw, I've also noticed that we don't handle a too high
> GOMP_OPENACC_DIM[GOMP_DIM_WORKER], I've added a TODO comment for this.

That's why I set vectors to dims[GOMP_DIM_VECTOR] when set. However, I
do agree that this is a task for a follow up patch.

> Committed as attached.

Thank you Tom!

Looking at my patch queue, there's only one more non-vector length
related patch in there - Remove use of CUDA unified memory in libgomp
. Going
forward, how would you like to proceed with the nvptx BE vector length
changes.

Cesar


Re: [PATCH] Use getentropy() for seeding PRNG

2018-08-13 Thread Fritz Reese
On Fri, Aug 3, 2018 at 9:19 AM Janne Blomqvist
 wrote:
>
> The getentropy function, found on Linux, OpenBSD, and recently also
> FreeBSD, can be used to get random bytes to initialize the PRNG.  It
> is similar to the traditional way of reading from /dev/urandom, but
> being a system call rather than a special file, it doesn't suffer from
> problems like running out of file descriptors, or failure when running
> in a container where /dev/urandom is not available.
>
> Regtested on x86_64-pc-linux-gnu, Ok for trunk?

Actually, getentropy() is similar to reading from /dev/random, where
getrandom() is similar to reading from /dev/urandom. Since the
original behavior of getosrandom() is to read from /dev/urandom, I
think it is better to use getrandom() for consistent semantics.

Furthermore, getentropy() may block to achieve an appropriate degree
of randomness, since it is intended for secure use. Of course such
block time would hardly be noticeable for a one-time read of a
thousand bits or so... but on principle I think we should provide a
quick cheesy seed by default, and the user may provide his own seed if
he wants expensive secure bits.

Just my opinion. I am personally OK with the [second version of the]
patch | sed s/getentropy/getrandom/g.

Fritz


Re: [aarch64}: added variable issue rate feature for falkor

2018-08-13 Thread Kai Tietz
2018-08-13 15:43 GMT+02:00 Bernhard Reutner-Fischer :
> On 13 August 2018 15:12:30 CEST, Bernhard Reutner-Fischer 
>  wrote:
>>On 13 August 2018 10:16:20 CEST, Kai Tietz 
>>wrote:
>>>Hello,
>>>
>>>this patch implements variable issue rate feature for falkor cpu.
>>>Additionally this patch adjusts the issue rate for falkor 8 as this
>>>value reflects more cpu's specification.
>>>
>>>This patch was tested against SPEC 2017 & 2016 and showed in general
>>>some improvements without any regressions for thos tests.
>>>
>>>ChangeLog:
>>>
>>>Jim Wilson 
>>>Kai Tietz 
>>>
>>>* config/aarch64.c (aarch64_sched_reorder): Implementing
>>>TARGET_SHED_REORDER hook.
>>
>>Present tense in ChangeLog please.
>>
>>>   (aarch64_variable_issue): Implemented
>>TARGET_SHED_VARIABLE_ISSUE
>>>hook.
>>>(TARGET_SHED_REORDER): Defined.
>>>(TARGET_SHED_VARIABLE_ISSUE): Likewise.
>>>* config/aarch64/falor.md (falkor_variable_issue): New.
>>>
>>>Ok for apply?
>>
>>s/subtrated/subtracted/
>>
>>
>>>
>>>Regards,
>>>Kai
>>>
>>>PS: I might be in need to update my key-files for stronger bitness.
>>>Whom I can ask for doing this?
>>
>>If you still can login:
>>https://gcc.gnu.org/ml/gcc-patches/2015-12/msg02105.html
>
> And if it's "just" you had a DSA key then add a new one with temporarily 
> accepting the old one:
> ssh -oPubkeyAcceptedKeyTypes=+ssh-dss 
>
>>HTH,
>

Thanks for you help. I corrected changelog text accordingly. Sadly I
don't have the old key anymore. So I would need an overseer to replace
my old key with an new one.

Kai


Re: [PING] [PATCH] Fix wrong code with truncated string literals (PR 86711/86714)

2018-08-13 Thread Martin Sebor

As I said below, the patch for PR 86552, 86711, 86714 that
was first posted on July 19 fixes both of these issues and
also diagnoses calls with unterminated strings:

  https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00155.html

On 08/12/2018 03:06 AM, Bernd Edlinger wrote:

Hi,

I'd like to ping for this patch: 
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01800.html

I will add a new BZ entry for the (minor) regression this patch
introduces in gcc.dg/strlenopt-49.c and assign it to myself.


Thanks
Bernd.

On 07/29/18 12:56, Bernd Edlinger wrote:

Hi!

This fixes two wrong code bugs where string_constant
returns over length string constants.  Initializers
like that are rejected in C++, but valid in C.

I have xfailed strlenopt-49.c, which tests this feature.
Personally I don't think that it is worth the effort to
optimize something that is per se invalid in C++.

The hunk in builtins.c is unrelated, but I would like
to use tree_to_shwi here, to be in line with the
tree_fits_shwi_p check above, and the rest of that
function which uses signed HWI throughout.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.




Re: [RFC][PATCH][mid-end] Optimize immediate choice in comparisons.

2018-08-13 Thread Richard Sandiford
Vlad Lazar  writes:
> diff --git a/gcc/expmed.h b/gcc/expmed.h
> index 
> 2890d9c9bbd034f01030dd551d544bf73e73b784..86a32a643fdd0fc9f396bd2c7904244bd484df16
>  100644
> --- a/gcc/expmed.h
> +++ b/gcc/expmed.h
> @@ -702,6 +702,10 @@ extern rtx emit_store_flag (rtx, enum rtx_code, rtx, 
> rtx, machine_mode,
>   extern rtx emit_store_flag_force (rtx, enum rtx_code, rtx, rtx,
> machine_mode, int, int);
>   
> +extern void canonicalize_comparison (machine_mode, enum rtx_code *, rtx *);
> +
> +extern enum rtx_code canonicalized_cmp_code (enum rtx_code);

It would probably be better to make the second function static (local
to expmed.c), since it's only used internally by canonicalize_comparison.
Switching the order of the two functions in expmed.c would avoid the need
for a forward declaration.

> @@ -6153,6 +6156,97 @@ emit_store_flag_force (rtx target, enum rtx_code code, 
> rtx op0, rtx op1,
>   
> return target;
>   }
> +
> +/* Choose the more appropiate immediate in comparisons.  The purpose of this

Maybe worth saying "scalar integer comparisons", since the function
doesn't handle vector or floating-point comparisons.

> +   is to end up with an immediate which can be loaded into a register in 
> fewer
> +   moves, if possible.
> +
> +   For each integer comparison there exists an equivalent choice:
> + i)   a >  b or a >= b + 1
> + ii)  a <= b or a <  b + 1
> + iii) a >= b or a >  b - 1
> + iv)  a <  b or a <= b - 1
> +
> +   The function is called in the gimple expanders which handle GIMPLE_ASSIGN
> +   and GIMPLE_COND.  It assumes that the first operand of the comparison is a
> +   register and the second is a constant.

This last paragraph doesn't seem accurate any more.  Probably best to
drop it, since there's no real need to say who the callers are if we
describe the interface.

> +   mode is the mode of the first operand.
> +   code points to the comparison code.
> +   imm points to the rtx containing the immediate.  */

GCC's convention is to put parameter names in caps in comments,
so "MODE is...", etc.

On the IMM line, it might be worth adding "*IMM must satisfy
CONST_SCALAR_INT_P on entry and continues to satisfy CONST_SCALAR_INT_P
on exit."

> +void canonicalize_comparison (machine_mode mode, enum rtx_code *code, rtx 
> *imm)
> +{
> +  int to_add = 0;
> +
> +  if (!SCALAR_INT_MODE_P (mode))
> +return;
> +
> +  /* Extract the immediate value from the rtx.  */
> +  wide_int imm_val = wi::shwi (INTVAL (*imm), mode);

This should be:

  rtx_mode_t imm_val (*imm, mode);

so that it copes with all CONST_SCALAR_INT_P, not just CONST_INT.

> +  if (*code == GT || *code == GTU || *code == LE || *code == LEU)
> +to_add = 1;
> +
> +  if (*code == GE || *code == GEU || *code == LT || *code == LTU)
> +to_add = -1;

Might be better to have:

  if (*code == GT || *code == GTU || *code == LE || *code == LEU)
to_add = 1;
  else if (*code == GE || *code == GEU || *code == LT || *code == LTU)
to_add = -1;
  else
return;

so that we exit early for other comparisons, rather than doing wasted work.

> +  /* Check for overflow/underflow in the case of signed values and
> + wrapping around in the case of unsigned values.  If any occur
> + cancel the optimization.  */
> +  wide_int max_val = wi::max_value (mode, SIGNED);
> +  wide_int min_val = wi::min_value (mode, SIGNED);
> +  if ((wi::cmps (imm_val, max_val) == 0 && to_add == 1)
> +  || (wi::cmps (imm_val, min_val) == 0 && to_add == -1))
> +return;

This needs to use the SIGNED/UNSIGNED choice appropriate for the
comparison (SIGNED for GT, UNSIGNED for GTU, etc.).  There doesn't
seem to be an existing function that says whether an rtx comparison
operation is signed or not (bit of a surprise), but there is
unsigned_condition, which returns the unsigned form of an rtx
comparison operator.  It might be worth adding something like:

  /* Return true if integer comparison operator CODE interprets its operands
 as unsigned.  */

  inline bool
  unsigned_condition_p (rtx_code code)
  {
return unsigned_condition (code) == code;
  }

and using that to choose between SIGNED and UNSIGNED.

Rather than using wi::cmp, a more direct way of checking for overflow
is to change this:

> +  /* Generate a mov instruction for both cases and see whether the change
> + was profitable.  */
> +  wide_int imm_modif = wi::add (imm_val, to_add);

to use the overflow form of wi::add, i.e.:

  wide_int imm_modif = wi::add (imm_val, to_add, sign, );

and return if "overflow" is set.

> +
> +  rtx reg = gen_reg_rtx (mode);

gen_reg_rtx creates a new pseudo register that essentially stays
around until we've finished compiling the function.  It's better to use:

  gen_rtx_REG (mode, LAST_VIRTUAL_REGISTER + 1);

for cost calculations, so that we don't waste pseudo register numbers.

> +  rtx new_imm = GEN_INT (imm_modif.to_shwi ());

This should be:

  rtx new_imm = immed_wide_int_const 

Re: [aarch64}: added variable issue rate feature for falkor

2018-08-13 Thread Bernhard Reutner-Fischer
On 13 August 2018 15:12:30 CEST, Bernhard Reutner-Fischer 
 wrote:
>On 13 August 2018 10:16:20 CEST, Kai Tietz 
>wrote:
>>Hello,
>>
>>this patch implements variable issue rate feature for falkor cpu.
>>Additionally this patch adjusts the issue rate for falkor 8 as this
>>value reflects more cpu's specification.
>>
>>This patch was tested against SPEC 2017 & 2016 and showed in general
>>some improvements without any regressions for thos tests.
>>
>>ChangeLog:
>>
>>Jim Wilson 
>>Kai Tietz 
>>
>>* config/aarch64.c (aarch64_sched_reorder): Implementing
>>TARGET_SHED_REORDER hook.
>
>Present tense in ChangeLog please.
>
>>   (aarch64_variable_issue): Implemented
>TARGET_SHED_VARIABLE_ISSUE
>>hook.
>>(TARGET_SHED_REORDER): Defined.
>>(TARGET_SHED_VARIABLE_ISSUE): Likewise.
>>* config/aarch64/falor.md (falkor_variable_issue): New.
>>
>>Ok for apply?
>
>s/subtrated/subtracted/
>
>
>>
>>Regards,
>>Kai
>>
>>PS: I might be in need to update my key-files for stronger bitness.
>>Whom I can ask for doing this?
>
>If you still can login:
>https://gcc.gnu.org/ml/gcc-patches/2015-12/msg02105.html

And if it's "just" you had a DSA key then add a new one with temporarily 
accepting the old one:
ssh -oPubkeyAcceptedKeyTypes=+ssh-dss 

>HTH,



Re: [PATCH][GCC][AArch64] Limit movmem copies to TImode copies.

2018-08-13 Thread Thomas Preudhomme
Hi Tamar,

Thanks for your patch.

Just one comment about your ChangeLog entry for the testsuiet change:
shouldn't it mention that it is a new testcase? The patch you attached
seems to create the file.

Best regards,

Thomas

On Mon, 13 Aug 2018 at 10:33, Tamar Christina 
wrote:

> Hi All,
>
> On AArch64 we have integer modes larger than TImode, and while we can
> generate
> moves for these they're not as efficient.
>
> So instead make sure we limit the maximum we can copy to TImode.  This
> means
> copying a 16 byte struct will issue 1 TImode copy, which will be done
> using a
> single STP as we expect but an CImode sized copy won't issue CImode
> operations.
>
> Bootstrapped and regtested on aarch4-none-linux-gnu and no issues.
> Crosstested aarch4_be-none-elf and no issues.
>
> Ok for trunk?
>
> Thanks,
> Tamar
>
> gcc/
> 2018-08-13  Tamar Christina  
>
> * config/aarch64/aarch64.c (aarch64_expand_movmem): Set TImode max.
>
> gcc/testsuite/
> 2018-08-13  Tamar Christina  
>
> * gcc.target/aarch64/large_struct_copy_2.c: Add assembler scan.
>
> --
>


[PATCH] Remove AIX 4.x, 5.x configuration

2018-08-13 Thread David Edelsohn
AIX 4.3, 5.1, 5.2, and 5.3 no longer are supported by IBM.  This patch
removes the GCC config.gcc stanzas and configuration files
corresponding to those AIX releases.

Thanks, David

* config.gcc [rs6000-ibm-aix4.x]: Delete.
[rs6000-ibm-aix5.1]: Delete.
[rs6000-ibm-aix5.2]: Delete.
[rs6000-ibm-aix5.3]: Delete.
* config/rs6000/aix43.h: Delete.
* config/rs6000/aix51.h: Delete.
* config/rs6000/aix52.h: Delete.
* config/rs6000/t-aix43: Delete.

Index: config.gcc
===
--- config.gcc  (revision 263505)
+++ config.gcc  (working copy)
@@ -2635,42 +2635,6 @@
extra_options="${extra_options} rs6000/sysv4.opt"
use_gcc_stdint=wrap
;;
-rs6000-ibm-aix4.[3456789]* | powerpc-ibm-aix4.[3456789]*)
-   tm_file="rs6000/biarch64.h ${tm_file} rs6000/aix.h
rs6000/aix43.h rs6000/xcoff.h rs6000/aix-stdint.h"
-   tmake_file="rs6000/t-aix43 t-slibgcc"
-   extra_options="${extra_options} rs6000/aix64.opt"
-   use_collect2=yes
-   thread_file='aix'
-   use_gcc_stdint=provide
-   extra_headers=
-   ;;
-rs6000-ibm-aix5.1.* | powerpc-ibm-aix5.1.*)
-   tm_file="rs6000/biarch64.h ${tm_file} rs6000/aix.h
rs6000/aix51.h rs6000/xcoff.h rs6000/aix-stdint.h"
-   extra_options="${extra_options} rs6000/aix64.opt"
-   tmake_file="rs6000/t-aix43 t-slibgcc"
-   use_collect2=yes
-   thread_file='aix'
-   use_gcc_stdint=wrap
-   extra_headers=
-   ;;
-rs6000-ibm-aix5.2.* | powerpc-ibm-aix5.2.*)
-   tm_file="${tm_file} rs6000/aix.h rs6000/aix52.h rs6000/xcoff.h
rs6000/aix-stdint.h"
-   tmake_file="rs6000/t-aix52 t-slibgcc"
-   extra_options="${extra_options} rs6000/aix64.opt"
-   use_collect2=yes
-   thread_file='aix'
-   use_gcc_stdint=wrap
-   extra_headers=
-   ;;
-rs6000-ibm-aix5.3.* | powerpc-ibm-aix5.3.*)
-   tm_file="${tm_file} rs6000/aix.h rs6000/aix53.h rs6000/xcoff.h
rs6000/aix-stdint.h"
-   tmake_file="rs6000/t-aix52 t-slibgcc"
-   extra_options="${extra_options} rs6000/aix64.opt"
-   use_collect2=yes
-   thread_file='aix'
-   use_gcc_stdint=wrap
-   extra_headers=altivec.h
-   ;;
 rs6000-ibm-aix6.* | powerpc-ibm-aix6.*)
tm_file="${tm_file} rs6000/aix.h rs6000/aix61.h rs6000/xcoff.h
rs6000/aix-stdint.h"
tmake_file="rs6000/t-aix52 t-slibgcc"


Re: [aarch64}: added variable issue rate feature for falkor

2018-08-13 Thread Bernhard Reutner-Fischer
On 13 August 2018 10:16:20 CEST, Kai Tietz  wrote:
>Hello,
>
>this patch implements variable issue rate feature for falkor cpu.
>Additionally this patch adjusts the issue rate for falkor 8 as this
>value reflects more cpu's specification.
>
>This patch was tested against SPEC 2017 & 2016 and showed in general
>some improvements without any regressions for thos tests.
>
>ChangeLog:
>
>Jim Wilson 
>Kai Tietz 
>
>* config/aarch64.c (aarch64_sched_reorder): Implementing
>TARGET_SHED_REORDER hook.

Present tense in ChangeLog please.

>   (aarch64_variable_issue): Implemented TARGET_SHED_VARIABLE_ISSUE
>hook.
>(TARGET_SHED_REORDER): Defined.
>(TARGET_SHED_VARIABLE_ISSUE): Likewise.
>* config/aarch64/falor.md (falkor_variable_issue): New.
>
>Ok for apply?

s/subtrated/subtracted/


>
>Regards,
>Kai
>
>PS: I might be in need to update my key-files for stronger bitness.
>Whom I can ask for doing this?

If you still can login:
https://gcc.gnu.org/ml/gcc-patches/2015-12/msg02105.html

HTH,



Re: [PATCH] Come up with TARGET_GET_VALID_OPTION_VALUES option hook (PR driver/83193).

2018-08-13 Thread Martin Liška
On 08/13/2018 02:54 PM, Ramana Radhakrishnan wrote:
> On Mon, Aug 13, 2018 at 1:49 PM, Martin Liška  wrote:
>> PING^1
>>
>> On 07/24/2018 02:05 PM, Martin Liška wrote:
>>> Hi.
>>>
>>> I'm sending updated version of the patch. It comes up with a new target 
>>> common hook
>>> that provide option completion list. It's used both in --help=target and 
>>> with --completion
>>> option. I implemented support for -match and -mtune for i386 target.
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> 
> Err I don't maintain the x86 backend to review this effectively. In an
> ideal world you would have split this into 2 patches 1 for the common
> parts and 1 for x86 and CC'd the relevant x86 maintainers. I'm not
> clear what you are looking for here from me :-/

Sorry, I was not clear. I would like to hear from ARM's folks that interface
design is fine for them? I know a global reviewer will have to approve that.

Martin

> 
> Ramana
>>>
>>> Martin
>>>
>>



[PATCH] Fix merging of 2 predictors (PR tree-optimization/86925).

2018-08-13 Thread Martin Liška
Hi.

Following patch handles and issue seen in Linux kernel. It's about
__builtin_expects seen in a PHI node.

Another issue I saw is compilation with -frounding-math. In that case
we should use non-rounding math for folding of probability value in
__builtin_with_probability.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/ChangeLog:

2018-08-13  Martin Liska  

PR tree-optimization/86925
* predict.c (expr_expected_value_1): When taking
later predictor, assign also probability.
Use fold_build2_initializer_loc in order to fold
the expression in -frounding-math.

gcc/testsuite/ChangeLog:

2018-08-13  Martin Liska  

PR tree-optimization/86925
* gcc.dg/predict-20.c: New test.
* gcc.dg/predict-21.c: New test.
---
 gcc/predict.c | 11 ---
 gcc/testsuite/gcc.dg/predict-20.c | 23 +++
 gcc/testsuite/gcc.dg/predict-21.c | 13 +
 3 files changed, 44 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/predict-20.c
 create mode 100644 gcc/testsuite/gcc.dg/predict-21.c


diff --git a/gcc/predict.c b/gcc/predict.c
index 3fbe3b704b3..8c8e79153fc 100644
--- a/gcc/predict.c
+++ b/gcc/predict.c
@@ -2332,13 +2332,17 @@ expr_expected_value_1 (tree type, tree op0, enum tree_code code,
 	  if (arg == PHI_RESULT (def))
 		continue;
 
+	  HOST_WIDE_INT probability2;
 	  new_val = expr_expected_value (arg, visited, ,
-	 probability);
+	 );
 
 	  /* It is difficult to combine value predictors.  Simply assume
 		 that later predictor is weaker and take its prediction.  */
 	  if (*predictor < predictor2)
-		*predictor = predictor2;
+		{
+		  *predictor = predictor2;
+		  *probability = probability2;
+		}
 	  if (!new_val)
 		return NULL;
 	  if (!val)
@@ -2423,7 +2427,8 @@ expr_expected_value_1 (tree type, tree op0, enum tree_code code,
 		  tree base = build_int_cst (integer_type_node,
 	 REG_BR_PROB_BASE);
 		  base = build_real_from_int_cst (t, base);
-		  tree r = fold_build2 (MULT_EXPR, t, prob, base);
+		  tree r = fold_build2_initializer_loc (UNKNOWN_LOCATION,
+			MULT_EXPR, t, prob, base);
 		  HOST_WIDE_INT probi
 		= real_to_integer (TREE_REAL_CST_PTR (r));
 		  if (probi >= 0 && probi <= REG_BR_PROB_BASE)
diff --git a/gcc/testsuite/gcc.dg/predict-20.c b/gcc/testsuite/gcc.dg/predict-20.c
new file mode 100644
index 000..31d01835b80
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/predict-20.c
@@ -0,0 +1,23 @@
+/* PR tree-optimization/86925 */
+
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate" } */
+
+int a, b;
+
+void
+c ()
+{
+  for (;;)
+{
+  if (__builtin_expect (b < 0, 0))
+	break;
+  if (__builtin_expect (a, 1))
+	break;
+}
+  int d = b < 0;
+  if (__builtin_expect (d, 0))
+asm("");
+}
+
+/* { dg-final { scan-tree-dump-times "__builtin_expect heuristics of edge" 3 "profile_estimate"} } */
diff --git a/gcc/testsuite/gcc.dg/predict-21.c b/gcc/testsuite/gcc.dg/predict-21.c
new file mode 100644
index 000..5949e5f4c19
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/predict-21.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-profile_estimate -frounding-math" } */
+
+extern int global;
+
+void foo (int base)
+{
+  for (int i = 0; __builtin_expect_with_probability (i < base, 1, 0.05f); i++)
+global++;
+}
+
+/* { dg-final { scan-tree-dump "first match heuristics: 5.00%" "profile_estimate"} } */
+/* { dg-final { scan-tree-dump "__builtin_expect_with_probability heuristics of edge .*->.*: 5.00%" "profile_estimate"} } */



Re: [PATCH] Come up with TARGET_GET_VALID_OPTION_VALUES option hook (PR driver/83193).

2018-08-13 Thread Ramana Radhakrishnan
On Mon, Aug 13, 2018 at 1:49 PM, Martin Liška  wrote:
> PING^1
>
> On 07/24/2018 02:05 PM, Martin Liška wrote:
>> Hi.
>>
>> I'm sending updated version of the patch. It comes up with a new target 
>> common hook
>> that provide option completion list. It's used both in --help=target and 
>> with --completion
>> option. I implemented support for -match and -mtune for i386 target.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.


Err I don't maintain the x86 backend to review this effectively. In an
ideal world you would have split this into 2 patches 1 for the common
parts and 1 for x86 and CC'd the relevant x86 maintainers. I'm not
clear what you are looking for here from me :-/

Ramana
>>
>> Martin
>>
>


Re: [PATCH v2 4/4] vxworks: don't define vxworks_asm_out_constructor when using .init_array

2018-08-13 Thread Olivier Hainque
Hello Rasmus,

> On 28 Jun 2018, at 10:43, Rasmus Villemoes  wrote:
> 
>   * config/vxworks.h: Guard vxworks_asm_out_constructor and
> vxworks_asm_out_destructor by !HAVE_INITFINI_ARRAY_SUPPORT
>   * config/vxworks.c: Likewise.

ok as well, also modulo a ChangeLog formatting nit: log continuations
beyond the first line start below the '*' of the first line, so:

* config/vxworks.h: Guard vxworks_asm_out_constructor and
vxworks_asm_out_destructor by !HAVE_INITFINI_ARRAY_SUPPORT.

Thanks,

Olivier



Re: [PATCH] Come up with TARGET_GET_VALID_OPTION_VALUES option hook (PR driver/83193).

2018-08-13 Thread Martin Liška
PING^1

On 07/24/2018 02:05 PM, Martin Liška wrote:
> Hi.
> 
> I'm sending updated version of the patch. It comes up with a new target 
> common hook
> that provide option completion list. It's used both in --help=target and with 
> --completion
> option. I implemented support for -match and -mtune for i386 target.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Martin
> 



[PATCH] Add test for memcpy expansion with hint.

2018-08-13 Thread Martin Liška
Hi.

i386 target uses hint-based expansion of memcpy-like and memset-like functions.
I would like to add a test in order to cover the functionality.

Ready for trunk?
Martin

gcc/ChangeLog:

2018-08-13  Martin Liska  

* config/i386/i386.c (ix86_expand_set_or_movmem): Dump
selected expansion strategy.

gcc/testsuite/ChangeLog:

2018-08-13  Martin Liska  

* gcc.dg/tree-prof/val-prof-10.c: New test.
---
 gcc/config/i386/i386.c   |  5 
 gcc/testsuite/gcc.dg/tree-prof/val-prof-10.c | 31 
 2 files changed, 36 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-prof/val-prof-10.c


diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 7554fd1f659..db1c4031a3f 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -27632,6 +27632,11 @@ ix86_expand_set_or_movmem (rtx dst, rtx src, rtx count_exp, rtx val_exp,
 		issetmem,
 		issetmem && val_exp == const0_rtx, have_as,
 		_check, , false);
+
+  if (dump_file)
+fprintf (dump_file, "Selected stringop expansion strategy: %s\n",
+	 stringop_alg_names[alg]);
+
   if (alg == libcall)
 return false;
   gcc_assert (alg != no_stringop);
diff --git a/gcc/testsuite/gcc.dg/tree-prof/val-prof-10.c b/gcc/testsuite/gcc.dg/tree-prof/val-prof-10.c
new file mode 100644
index 000..57854b5911b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-prof/val-prof-10.c
@@ -0,0 +1,31 @@
+/* { dg-options "-O2 -fdump-rtl-expand -mtune=core2" } */
+/* { dg-skip-if "" { ! { i?86-*-* x86_64-*-* } } } */
+
+long buffer1[128], buffer2[128];
+char *x;
+
+void foo(long *r)
+{
+  x = (char *)r;
+  asm volatile("" ::: "memory");
+}
+
+void
+__attribute__((noinline))
+compute()
+{
+  volatile int n = 24;
+  __builtin_memcpy (buffer1, buffer2, n);
+  foo ([0]);
+}
+
+int
+main()
+{
+  for (unsigned i = 0; i < 1; i++)
+compute ();
+
+  return 0;
+}
+
+/* { dg-final-use-not-autofdo { scan-rtl-dump "Selected stringop expansion strategy: rep_byte" "expand" } } */



Re: [PATCH] libstdc++-v3: Have aligned_alloc() on Newlib

2018-08-13 Thread Jonathan Wakely

On 13/08/18 12:55 +0100, Szabolcs Nagy wrote:

On 09/08/18 10:08, Jonathan Wakely wrote:

On 09/08/18 06:56 +0200, Sebastian Huber wrote:

On 08/08/18 16:33, Jonathan Wakely wrote:

On 08/08/18 16:22 +0200, Ulrich Weigand wrote:

Jonathan Wakely wrote:


Aha, so newlib was using memalign previously:

@@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
 #else
 extern "C" void *memalign(std::size_t boundary, std::size_t size);
 #endif
-#define aligned_alloc memalign


Yes, exactly ... this commit introduced the regression.


OK, I've regressed the branches then - I'll fix that.


This should fix it. I'll finish testing and commit it.

Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
gcc-7-branch and gcc-8-branch, because changing newlib from using
memalign to aligned_alloc is safe.


Should I check in my patch in addition to your patch?


Yes please, on trunk and 7 and 8. It's better to use the standard
aligned_alloc if available.



but the newlib aligned_alloc is broken on baremetal targets,
it is implemented using posix_memalign which is not provided
by the newlib malloc implementation (except on cygwin)


Ouch, OK, let's revert it. Using memalign is fine.

The original problem that I think Sebastian was trying to solve should
be fixed by r263409 anyway (and was backported to all branches).




Re: [PATCH,nvptx] Use CUDA driver API to select default runtime launch, geometry

2018-08-13 Thread Tom de Vries
On 08/10/2018 08:39 PM, Cesar Philippidis wrote:
> is that I modified the default value for vectors as follows
> 
> + int vectors = default_dim_p[GOMP_DIM_VECTOR]
> +   ? 0 : dims[GOMP_DIM_VECTOR];
> 
> Technically, trunk only supports warp-sized vectors, but the fallback
> code is already checking for the presence of vectors as so
> 
> + if (default_dim_p[GOMP_DIM_VECTOR])
> +   dims[GOMP_DIM_VECTOR]
> + = MIN (dims[GOMP_DIM_VECTOR],
> +(targ_fn->max_threads_per_block / warp_size
> + * warp_size));
> 

That code handles the case that the default vector size is bigger than
the function being launched allows, independent from whether that
default is calculated by the runtime, or set by GOMP_OPENACC_DIM.

The GOMP_OPENACC_DIM part is forward compatible, given that currently
the compiler doesn't allow the runtime to choose the vector length, and
AFAICT that will remain the same after application of the submitted set
of vector_length patches.

> therefore, I had the cuOccupancyMaxPotentialBlockSize code path behave
> the same.

They don't behave the same. What you add here is ignoring
GOMP_OPENACC_DIM[GOMP_DIM_VECTOR], not handling it. That requires a comment.

Furthermore, by assigning dims[GOMP_DIM_VECTOR] at the start you break
the pattern of the code, which:
- first applies GOMP_OPENACC_DIM
- then further fills in defaults as required
- then applies defaults
I've rewritten this bit to fit the pattern. This result is not pretty,
but it'll do for now.  Changing the pattern may make things better
structured, but this is something we can do in a follow up patch, and
want to do for all dimensions at once, not just for vector, otherwise
the code will become too convoluted.

Btw, I've also noticed that we don't handle a too high
GOMP_OPENACC_DIM[GOMP_DIM_WORKER], I've added a TODO comment for this.

> If you want, I can resubmit a patch without that change though> 
> 0001-nvptx-Use-CUDA-driver-API-to-select-default-runtime-.patch
> 
> 
> [nvptx] Use CUDA driver API to select default runtime launch geometry
> 
>   PR target/85590
> 
>   libgomp/
>   plugin/cuda/cuda.h (CUoccupancyB2DSize): New typedef.

You forgot to add '* ' in front of all files.

>   (cuOccupancyMaxPotentialBlockSizeWithFlags): Declare.

You've added cuOccupancyMaxPotentialBlockSize, not
cuOccupancyMaxPotentialBlockSizeWithFlags.

>   plugin/cuda-lib.def (cuOccupancyMaxPotentialBlockSize): New
>   CUDA_ONE_CALL_MAYBE_NULL.
>   plugin/plugin-nvptx.c (CUDA_VERSION < 6050): Define
>   CUoccupancyB2DSize and declare
>   cuOccupancyMaxPotentialBlockSizeWithFlags.

Same here.

>   (nvptx_exec): Use cuOccupancyMaxPotentialBlockSize to set the
>   default num_gangs and num_workers when the driver supports it.
> 
> ---
>  libgomp/plugin/cuda-lib.def   |  1 +
>  libgomp/plugin/cuda/cuda.h|  3 ++
>  libgomp/plugin/plugin-nvptx.c | 77 +--
>  3 files changed, 69 insertions(+), 12 deletions(-)
> 
> diff --git a/libgomp/plugin/cuda-lib.def b/libgomp/plugin/cuda-lib.def
> index 29028b504a0..b2a4c2154eb 100644
> --- a/libgomp/plugin/cuda-lib.def
> +++ b/libgomp/plugin/cuda-lib.def
> @@ -41,6 +41,7 @@ CUDA_ONE_CALL (cuModuleGetGlobal)
>  CUDA_ONE_CALL (cuModuleLoad)
>  CUDA_ONE_CALL (cuModuleLoadData)
>  CUDA_ONE_CALL (cuModuleUnload)
> +CUDA_ONE_CALL_MAYBE_NULL (cuOccupancyMaxPotentialBlockSize)
>  CUDA_ONE_CALL (cuStreamCreate)
>  CUDA_ONE_CALL (cuStreamDestroy)
>  CUDA_ONE_CALL (cuStreamQuery)
> diff --git a/libgomp/plugin/cuda/cuda.h b/libgomp/plugin/cuda/cuda.h
> index 4799825bda2..b4c1b29c5d8 100644
> --- a/libgomp/plugin/cuda/cuda.h
> +++ b/libgomp/plugin/cuda/cuda.h
> @@ -44,6 +44,7 @@ typedef void *CUevent;
>  typedef void *CUfunction;
>  typedef void *CUlinkState;
>  typedef void *CUmodule;
> +typedef size_t (*CUoccupancyB2DSize)(int);
>  typedef void *CUstream;
>  
>  typedef enum {
> @@ -170,6 +171,8 @@ CUresult cuModuleGetGlobal (CUdeviceptr *, size_t *, 
> CUmodule, const char *);
>  CUresult cuModuleLoad (CUmodule *, const char *);
>  CUresult cuModuleLoadData (CUmodule *, const void *);
>  CUresult cuModuleUnload (CUmodule);
> +CUresult cuOccupancyMaxPotentialBlockSize(int *, int *, CUfunction,
> +   CUoccupancyB2DSize, size_t, int);
>  CUresult cuStreamCreate (CUstream *, unsigned);
>  #define cuStreamDestroy cuStreamDestroy_v2
>  CUresult cuStreamDestroy (CUstream);
> diff --git a/libgomp/plugin/plugin-nvptx.c b/libgomp/plugin/plugin-nvptx.c
> index 6799a264976..9a4bc11e5fe 100644
> --- a/libgomp/plugin/plugin-nvptx.c
> +++ b/libgomp/plugin/plugin-nvptx.c
> @@ -61,9 +61,12 @@ CUresult cuLinkAddData (CUlinkState, CUjitInputType, void 
> *, size_t,
>   const char *, unsigned, CUjit_option *, void **);
>  CUresult cuLinkCreate (unsigned, CUjit_option *, void **, CUlinkState *);
>  #else
> +typedef size_t 

Re: [PATCH] libstdc++-v3: Have aligned_alloc() on Newlib

2018-08-13 Thread Szabolcs Nagy

On 09/08/18 10:08, Jonathan Wakely wrote:

On 09/08/18 06:56 +0200, Sebastian Huber wrote:

On 08/08/18 16:33, Jonathan Wakely wrote:

On 08/08/18 16:22 +0200, Ulrich Weigand wrote:

Jonathan Wakely wrote:


Aha, so newlib was using memalign previously:

@@ -53,20 +54,24 @@ aligned_alloc (std::size_t al, std::size_t sz)
 #else
 extern "C" void *memalign(std::size_t boundary, std::size_t size);
 #endif
-#define aligned_alloc memalign


Yes, exactly ... this commit introduced the regression.


OK, I've regressed the branches then - I'll fix that.


This should fix it. I'll finish testing and commit it.

Sebastian, your patch to define HAVE_ALIGNED_ALLOC is OK for
gcc-7-branch and gcc-8-branch, because changing newlib from using
memalign to aligned_alloc is safe.


Should I check in my patch in addition to your patch?


Yes please, on trunk and 7 and 8. It's better to use the standard
aligned_alloc if available.



but the newlib aligned_alloc is broken on baremetal targets,
it is implemented using posix_memalign which is not provided
by the newlib malloc implementation (except on cygwin)

in libstdc++ test log i see

spawn /B/gcc/xg++ -shared-libgcc -B/B/gcc -nostdinc++ -L/B/aarch64-none-elf/libstdc++-v3/src -L/B/aarch64-none-elf/libstdc++-v3/src/.libs 
-L/B/aarch64-none-elf/libstdc++-v3/libsupc++/.libs -B/P/aarch64-none-elf/bin/ -B/P/aarch64-none-elf/lib/ -isystem /P/aarch64-none-elf/include 
-isystem /P/aarch64-none-elf/sys-include -B/B/aarch64-none-elf/./libstdc++-v3/src/.libs -fmessage-length=0 -fno-show-column -ffunction-sections 
-fdata-sections -g -ffunction-sections -fdata-sections -O2 -DLOCALEDIR="." -nostdinc++ 
-I/B/aarch64-none-elf/libstdc++-v3/include/aarch64-none-elf -I/B/aarch64-none-elf/libstdc++-v3/include -I/S/gcc/libstdc++-v3/libsupc++ 
-I/S/gcc/libstdc++-v3/include/backward -I/S/gcc/libstdc++-v3/testsuite/util 
/S/gcc/libstdc++-v3/testsuite/18_support/aligned_alloc/aligned_alloc.cc -std=gnu++17 -fno-diagnostics-show-caret -fdiagnostics-color=never 
./libtestc++.a -L/B/aarch64-none-elf/libstdc++-v3/src/filesystem/.libs -specs=aem-ve.specs -lm -mcmodel=small -o ./aligned_alloc.exe

/P/aarch64-none-elf/bin/ld: 
/P/aarch64-none-elf/lib/libg.a(lib_a-aligned_alloc.o): in function 
`aligned_alloc':
/S/newlib-cygwin/newlib/libc/stdlib/aligned_alloc.c:35: undefined reference to 
`posix_memalign'
/P/aarch64-none-elf/bin/ld: /S/newlib-cygwin/newlib/libc/stdlib/aligned_alloc.c:35:(.text.aligned_alloc+0x14): relocation truncated to fit: 
R_AARCH64_CALL26 against undefined symbol `posix_memalign'

collect2: error: ld returned 1 exit status


FAIL: 18_support/aligned_alloc/aligned_alloc.cc (test for excess errors)
FAIL: 18_support/new_aligned.cc (test for excess errors)
FAIL: 20_util/allocator/overaligned.cc (test for excess errors)
FAIL: 20_util/memory_resource/2.cc (test for excess errors)
FAIL: 20_util/monotonic_buffer_resource/allocate.cc (test for excess errors)
FAIL: 20_util/monotonic_buffer_resource/deallocate.cc (test for excess errors)
FAIL: 20_util/monotonic_buffer_resource/release.cc (test for excess errors)
FAIL: 20_util/monotonic_buffer_resource/upstream_resource.cc (test for excess 
errors)
FAIL: 20_util/polymorphic_allocator/resource.cc (test for excess errors)
FAIL: 20_util/polymorphic_allocator/select.cc (test for excess errors)
FAIL: ext/bitmap_allocator/overaligned.cc (test for excess errors)
FAIL: ext/mt_allocator/overaligned.cc (test for excess errors)
FAIL: ext/new_allocator/overaligned.cc (test for excess errors)
FAIL: ext/pool_allocator/overaligned.cc (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new1.C   (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new2.C   (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new5.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new6.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new6.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/cpp1z/aligned-new8.C   (test for excess errors)
UNRESOLVED: 18_support/aligned_alloc/aligned_alloc.cc compilation failed to 
produce executable
UNRESOLVED: 18_support/new_aligned.cc compilation failed to produce executable
UNRESOLVED: 20_util/allocator/overaligned.cc compilation failed to produce 
executable
UNRESOLVED: 20_util/memory_resource/2.cc compilation failed to produce 
executable
UNRESOLVED: 20_util/monotonic_buffer_resource/allocate.cc compilation failed to 
produce executable
UNRESOLVED: 20_util/monotonic_buffer_resource/deallocate.cc compilation failed 
to produce executable
UNRESOLVED: 20_util/monotonic_buffer_resource/release.cc compilation failed to 
produce executable
UNRESOLVED: 20_util/monotonic_buffer_resource/upstream_resource.cc compilation 
failed to produce executable
UNRESOLVED: 20_util/polymorphic_allocator/resource.cc compilation failed to 
produce executable
UNRESOLVED: 

Re: [PATCH v2 3/4] vxworks: enable use of .init_array/.fini_array for cdtors

2018-08-13 Thread Olivier Hainque
Hi Rasmus,

> On 13 Aug 2018, at 10:24, Rasmus Villemoes  wrote:
> 
>> Ok, modulo ChangeLog reformatting:
> 
> Thanks for spotting that. I have a script that fixes the whitespace
> issue automatically, but it doesn't catch missing leading * in entries.
> 
> Do you want me to send an updated and rebased version of these patches
> (including changelog fixups)?

No, no need to repost just for the ChangeLog
format update.

Thanks,

Olivier



Re: [PATCH] S/390: Factor out constant pool ref decomposition

2018-08-13 Thread Andreas Krebbel
On 08/13/2018 10:09 AM, Ilya Leoshkevich wrote:
> 2018-07-27  Ilya Leoshkevich  
> 
>   * config/s390/s390.c (s390_decompose_constant_pool_ref):
> New function.
>   (s390_decompose_address): Factor out constant pool ref
> decomposition.

Applied. Thanks!

Andreas



Re: [PATCH] convert braced initializers to strings (PR 71625)

2018-08-13 Thread Jason Merrill

On 08/09/2018 12:17 PM, Martin Sebor wrote:

Using build_string() rather than build_string_literal() needed
a tweak in digest_init_r().  It didn't break anything but since
the array type may not have a domain yet, neither will the
string.  It looks like that may get adjusted later on but I've
temporarily guarded the code with #if 1.  If the change is
fine I'll remove the #if before committing. 


The digest_init_r change seems to follow from allowing STRING_CST of 
signed char, so yes, it's fine.



+ && TREE_CODE (init) == CONSTRUCTOR
+ && TREE_TYPE (init) == init_list_type_node)


This is BRACE_ENCLOSED_INITIALIZER_P.

OK with that change.

Jason


Re: C++ PATCH for c++/57891, narrowing conversions in non-type template arguments

2018-08-13 Thread Jason Merrill
On Sun, Aug 12, 2018 at 2:13 AM, Marek Polacek  wrote:
> On Sat, Aug 11, 2018 at 11:32:24PM +1200, Jason Merrill wrote:
>> On Fri, Aug 10, 2018 at 8:59 AM, Marek Polacek  wrote:
>> > On Mon, Aug 06, 2018 at 12:02:31AM +1000, Jason Merrill wrote:
>> >> > OK -- see the patch below.  Now, I'm not crazy about adding another bit
>> >> > to struct conversion, but reusing any of the other bits didn't seem
>> >> > safe/possible.  Maybe it'll come in handy when dealing with this problem
>> >> > for function templates.
>> >>
>> >> Looks good.
>> >>
>> >> >> >> @@ -6669,9 +6669,12 @@ convert_nontype_argument (tree type, tree 
>> >> >> >> expr, tsubst_flags_t complain)
>> >> >> >>   /* C++17: A template-argument for a non-type 
>> >> >> >> template-parameter shall
>> >> >> >>  be a converted constant expression (8.20) of the type 
>> >> >> >> of the
>> >> >> >>  template-parameter.  */
>> >> >> >> + int errs = errorcount;
>> >> >> >>   expr = build_converted_constant_expr (type, expr, 
>> >> >> >> complain);
>> >> >> >>   if (expr == error_mark_node)
>> >> >> >> -   return error_mark_node;
>> >> >> >> +   /* Make sure we return NULL_TREE only if we have really 
>> >> >> >> issued
>> >> >> >> +  an error, as described above.  */
>> >> >> >> +   return errorcount > errs ? NULL_TREE : error_mark_node;
>> >> >> >
>> >> >> > Is this still needed?
>> >> >>
>> >> >> Earlier you wrote,
>> >> >>
>> >> >> > Checking complain doesn't work because that doesn't
>> >> >> > say if we have really issued an error.  If we have not, and we return
>> >> >> > NULL_TREE anyway, we hit this assert:
>> >> >> >  8517   gcc_assert (!(complain & tf_error) || seen_error ());
>> >> >>
>> >> >> If (complain & tf_error), we shouldn't see error_mark_node without an
>> >> >> error having been issued.  Why is build_converted_constant_expr
>> >> >> returning error_mark_node without giving an error when (complain &
>> >> >> tf_error)?
>> >> >
>> >> > This can happen on invalid code; e.g. tests nontype13.C and crash87.C.
>> >> > What happens there is that we're trying to convert a METHOD_TYPE to 
>> >> > bool,
>> >> > which doesn't work: in standard_conversion we go into the
>> >> > else if (tcode == BOOLEAN_TYPE)
>> >> > block, which returns NULL, implicit_conversion also then returns NULL, 
>> >> > yet
>> >> > no error has been issued.
>> >>
>> >> Yes, that's normal.  The problem is in build_converted_constant_expr:
>> >>
>> >>   if (conv)
>> >> expr = convert_like (conv, expr, complain);
>> >>   else
>> >> expr = error_mark_node;
>> >>
>> >> Here when setting expr to error_mark_node I forgot to give an error if
>> >> (complain & tf_error).
>> >
>> > Done.  A few testcases needed adjusting but nothing surprising.
>> >
>> >> >> >> +  /* If we're in a template and we know the constant value, we 
>> >> >> >> can
>> >> >> >> +warn.  Otherwise wait for instantiation.  */
>> >> >> >> +  || (processing_template_decl && !TREE_CONSTANT (init)))
>> >> >> >
>> >> >> > I don't think we want this condition.  If the value is non-constant
>> >> >> > but also not dependent, it'll never be constant, so we can go ahead
>> >> >> > and complain.
>> >> >
>> >> > (This to be investigated later.)
>> >>
>> >> Why later?
>> >
>> > So this was this wrong error:
>> > https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00159.html
>> > which was confusing.  But I noticed it's because we instantiate 'size' 
>> > twice:
>> > in compute_array_index_type:
>> >   size = instantiate_non_dependent_expr_sfinae (size, complain);
>> >   size = build_converted_constant_expr (size_type_node, size, complain);
>> >   size = maybe_constant_value (size);
>> > and then in check_narrowing:
>> >   init = fold_non_dependent_expr (init, complain);
>> >
>> > (in this case, size is a call to constexpr user-defined conversion).
>> >
>> > But check_narrowing now returns early if 
>> > instantiation_dependent_expression_p,
>> > so I thought perhaps we could simply call maybe_constant_value which fixes 
>> > this
>> > problem and doesn't regress anything.  Does that seem like a sensible thing
>> > to do?
>>
>> Hmm, that'll have problems if we pass an unfolded template expression
>> to check_narrowing, but probably we don't want to do that anyway.  So
>> yes, it seems reasonable.
>
> Ok.
>
>> >> > --- gcc/cp/decl.c
>> >> > +++ gcc/cp/decl.c
>> >> > @@ -9581,7 +9581,7 @@ compute_array_index_type (tree name, tree size, 
>> >> > tsubst_flags_t complain)
>> >> >  {
>> >> >tree folded = cp_fully_fold (size);
>> >> >if (TREE_CODE (folded) == INTEGER_CST)
>> >> > -   pedwarn (location_of (size), OPT_Wpedantic,
>> >> > +   pedwarn (input_location, OPT_Wpedantic,
>> >>
>> >> It should work to use location_of (osize) here.
>> >
>> > I dropped this hunk altogether.  Because location_of will use
>> > DECL_SOURCE_LOCATION for DECLs, the error message will point to the 
>> > 

Re: [PATCH] Use getentropy() for seeding PRNG

2018-08-13 Thread Janne Blomqvist
PING

On Fri, Aug 3, 2018 at 5:05 PM, Janne Blomqvist 
wrote:

> On Fri, Aug 3, 2018 at 4:28 PM, Jakub Jelinek  wrote:
>
>> On Fri, Aug 03, 2018 at 04:19:03PM +0300, Janne Blomqvist wrote:
>> > --- a/libgfortran/intrinsics/random.c
>> > +++ b/libgfortran/intrinsics/random.c
>> > @@ -309,12 +309,9 @@ getosrandom (void *buf, size_t buflen)
>> >for (size_t i = 0; i < buflen / sizeof (unsigned int); i++)
>> >  rand_s ([i]);
>> >return buflen;
>> > +#elif defined(HAVE_GETENTROPY)
>> > +  return getentropy (buf, buflen);
>> >  #else
>>
>> I wonder if it wouldn't be better to use getentropy only if it is
>> successful
>> and fall back to whatever you were doing before.
>>
>> E.g. on Linux, I believe getentropy in glibc just uses the getrandom
>> syscall, which has only been introduced in Linux kernel 3.17.
>>
>
> Yes, that is my understanding as well.
>
>
>> So, if somebody is running glibc 2.25 or later on kernel < 3.17, it will
>> fail, even though reads from /dev/urandom could work.
>>
>
> Hmm, fair enough. So replace the random.c part of the patch with
>
> diff --git a/libgfortran/intrinsics/random.c b/libgfortran/intrinsics/
> random.c
> index 234c5ff95fd..229fa6995c0 100644
> --- a/libgfortran/intrinsics/random.c
> +++ b/libgfortran/intrinsics/random.c
> @@ -310,11 +310,10 @@ getosrandom (void *buf, size_t buflen)
>  rand_s ([i]);
>return buflen;
>  #else
> -  /*
> - TODO: When glibc adds a wrapper for the getrandom() system call
> - on Linux, one could use that.
> -
> - TODO: One could use getentropy() on OpenBSD.  */
> +#ifdef HAVE_GETENTROPY
> +  if (getentropy (buf, buflen) == 0)
> +return 0;
> +#endif
>int flags = O_RDONLY;
>  #ifdef O_CLOEXEC
>flags |= O_CLOEXEC;
>
>
>
> Just to be sure, I regtested this slightly modified patch as well. Ok for
> trunk?
>
> --
> Janne Blomqvist
>



-- 
Janne Blomqvist


[PATCH][GCC][AArch64] Limit movmem copies to TImode copies.

2018-08-13 Thread Tamar Christina
Hi All,

On AArch64 we have integer modes larger than TImode, and while we can generate
moves for these they're not as efficient.

So instead make sure we limit the maximum we can copy to TImode.  This means
copying a 16 byte struct will issue 1 TImode copy, which will be done using a
single STP as we expect but an CImode sized copy won't issue CImode operations.

Bootstrapped and regtested on aarch4-none-linux-gnu and no issues.
Crosstested aarch4_be-none-elf and no issues.

Ok for trunk?

Thanks,
Tamar

gcc/
2018-08-13  Tamar Christina  

* config/aarch64/aarch64.c (aarch64_expand_movmem): Set TImode max.

gcc/testsuite/
2018-08-13  Tamar Christina  

* gcc.target/aarch64/large_struct_copy_2.c: Add assembler scan.

-- 
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1e8d8104c066a265120ab776f7ab5a959d3512b6..cdd8bca98f8c50a804986510144db9ecf911bf1e 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -15927,13 +15927,17 @@ aarch64_expand_movmem (rtx *operands)
   /* Convert n to bits to make the rest of the code simpler.  */
   n = n * BITS_PER_UNIT;
 
+  /* Maximum amount to copy in one go.  The AArch64 back-end has integer modes
+ larger than TImode, but we should not use them for loads/stores here.  */
+  const int copy_limit = GET_MODE_BITSIZE (TImode);
+
   while (n > 0)
 {
   /* Find the largest mode in which to do the copy in without over reading
 	 or writing.  */
   opt_scalar_int_mode mode_iter;
   FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_INT)
-	if (GET_MODE_BITSIZE (mode_iter.require ()) <= n)
+	if (GET_MODE_BITSIZE (mode_iter.require ()) <= MIN (n, copy_limit))
 	  cur_mode = mode_iter.require ();
 
   gcc_assert (cur_mode != BLKmode);
diff --git a/gcc/testsuite/gcc.target/aarch64/large_struct_copy_2.c b/gcc/testsuite/gcc.target/aarch64/large_struct_copy_2.c
new file mode 100644
index ..8380ce008e7ffd30b6d21d89dc5ff3a9fd395e9a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/large_struct_copy_2.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+struct S0 {
+  signed f1;
+  long f2;
+  unsigned f3;
+  unsigned f4;
+  unsigned f5;
+} a;
+struct S2 {
+  long f0;
+  int f2;
+  struct S0 f3;
+};
+
+void fn1 () {
+  struct S2 b = {0, 1, 7, 4073709551611, 4, 8, 7};
+  a = b.f3;
+}
+
+/* { dg-final { scan-assembler-times {ldp\s+x[0-9]+} 2 } } */
+/* { dg-final { scan-assembler-times {stp\s+x[0-9]+} 2 } } */
+/* { dg-final { scan-assembler-not {ld[1-3]} } } */



RE: [PATCH][GCC][Arm] Fix subreg crash in different way by enabling the FP16 pattern unconditionally.

2018-08-13 Thread Tamar Christina
Ping

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org 
> On Behalf Of Tamar Christina
> Sent: Tuesday, July 31, 2018 10:47
> To: gcc-patches@gcc.gnu.org
> Cc: nd ; Ramana Radhakrishnan
> ; Richard Earnshaw
> ; ni...@redhat.com; Kyrylo Tkachov
> ; Thomas Preudhomme
> 
> Subject: RE: [PATCH][GCC][Arm] Fix subreg crash in different way by
> enabling the FP16 pattern unconditionally.
> 
> Ping 
> 
> > -Original Message-
> > From: Thomas Preudhomme 
> > Sent: Thursday, July 26, 2018 12:29
> > To: Tamar Christina 
> > Cc: gcc-patches@gcc.gnu.org; nd ; Ramana Radhakrishnan
> > ; Richard Earnshaw
> > ; ni...@redhat.com; Kyrylo Tkachov
> > 
> > Subject: Re: [PATCH][GCC][Arm] Fix subreg crash in different way by
> > enabling the FP16 pattern unconditionally.
> >
> > On Thu, 26 Jul 2018 at 12:01, Tamar Christina
> > 
> > wrote:
> > >
> > > Hi Thomas,
> > >
> > > > -Original Message-
> > > > From: Thomas Preudhomme 
> > > > Sent: Thursday, July 26, 2018 09:29
> > > > To: Tamar Christina 
> > > > Cc: gcc-patches@gcc.gnu.org; nd ; Ramana
> > Radhakrishnan
> > > > ; Richard Earnshaw
> > > > ; ni...@redhat.com; Kyrylo Tkachov
> > > > 
> > > > Subject: Re: [PATCH][GCC][Arm] Fix subreg crash in different way
> > > > by enabling the FP16 pattern unconditionally.
> > > >
> > > > Hi Tamar,
> > > >
> > > > On Wed, 25 Jul 2018 at 16:28, Tamar Christina
> > > > 
> > > > wrote:
> > > > >
> > > > > Hi Thomas,
> > > > >
> > > > > Thanks for the review!
> > > > >
> > > > > > >
> > > > > > > I don't believe the TARGET_FP16 guard to be needed, because
> > > > > > > the pattern doesn't actually generate code and requires
> > > > > > > another pattern for that, and a reg to reg move should
> > > > > > > always be possible anyway. So allowing the force to register
> > > > > > > here is safe and it allows the compiler to generate a
> > > > > > > correct error instead of ICEing in an
> > > > infinite loop.
> > > > > >
> > > > > > How about subreg to subreg move? Doesn't that expand to more
> > > > > > insns (subreg to reg and reg to subreg)? Couldn't you improve
> > > > > > the logic to check that there is actually a mode change so
> > > > > > that if there isn't (like moving from one subreg to another)
> > > > > > just expand to
> > a single move?
> > > > > >
> > > > >
> > > > > Yes, but that is not a new issue. My patch is simply removing
> > > > > the
> > > > > TARGET_FP16 restrictions and merging two patterns that should be
> > > > > one
> > > > using an iterator and nothing more.
> > > > >
> > > > > The redundant mov is already there and a different issue than
> > > > > the ICE I'm
> > > > trying to fix.
> > > >
> > > > It's there for movv4hf and movv6hf but your patch extends this
> > > > problem to movv2sf and movv4sf as well.
> > >
> > > I don't understand how it can. My patch just replaces one pattern
> > > for V4HF and one for V8HF with one pattern operating on VH.
> > >
> > > ;; Vector modes for 16-bit floating-point support.
> > > (define_mode_iterator VH [V8HF V4HF])
> > >
> > > My pattern has absolutely no effect on V2SF and V4SF or any of the
> > > other
> > modes.
> >
> > My bad, I was looking at VF.
> >
> > >
> > > >
> > > > >
> > > > > None of the code inside the expander is needed at all, the code
> > > > > really only has an effect on subreg to subreg moves, as
> > > > > `force_reg` doesn't do
> > > > anything when it's argument is already a reg.
> > > > >
> > > > > The comment in the expander (which was already there) is wrong.
> > > > > The
> > > > > *reason* the ICE is fixed isn't because of the `force_reg`. It's
> > > > > because of the mere presence of the expander itself. The
> > > > > expander matches the standard mov$a optab and so this prevents
> > > > emit_move_insn_1 from doing the move by subwords as it finds a
> > > > pattern that's able to do the move.
> > > >
> > > > Could you then fix the comment in your patch as well? I hadn't
> > > > understood the force_reg was not key here. You might want to
> > > > update the following sentence from your patch description if you
> > > > are going to include it in your commit message:
> > >
> > > I'll update the comment in the patch. The cover letter won't be
> > > included in the commit, But it does accurately reflect the current
> > > state of affairs. The patch will do the force_reg, It's just not the
> > > reason it
> > works.
> >
> > Understood.
> >
> > >
> > > >
> > > > The way this is worked around in the back-end is that we have move
> > > > patterns in neon.md that usually just force the register instead
> > > > of checking with the back-end.
> > > >
> > > > "The way this is worked around (..) that just force the register"
> > > > is what led me to believe the force_reg was important.
> > > >
> > > > >
> > > > > The expander however always falls through and doesn’t stop RTL
> > > > > generation. You could remove all the code in there and have it
> > > > > properly match the *neon_mov instructions which will do the
> > > > 

Re: [PATCH v2 3/4] vxworks: enable use of .init_array/.fini_array for cdtors

2018-08-13 Thread Rasmus Villemoes
On 2018-08-10 17:59, Olivier Hainque wrote:
> Hello Rasmus,
> 
>> On 28 Jun 2018, at 10:43, Rasmus Villemoes  wrote:
>>
>> Assume that if the user passed --enable-initfini-array when building
>> gcc, the rest of the toolchain (including the link spec and linker
>> script) is set up appropriately.
>> 2018-06-04  Rasmus Villemoes  
>>
>> gcc/
>>  config/vxworks.c: Set targetm.have_ctors_dtors if 
>> HAVE_INITFINI_ARRAY_SUPPORT.
>>  config/vxworks.h: Set SUPPORTS_INIT_PRIORITY if HAVE_INITFINI_ARRAY_SUPPORT.
> 
> Ok, modulo ChangeLog reformatting:
> 
>   * config/vxworks.c: Set targetm.have_ctors_dtors if
>   HAVE_INITFINI_ARRAY_SUPPORT.
>   * config/vxworks.h: Set SUPPORTS_INIT_PRIORITY if
>   HAVE_INITFINI_ARRAY_SUPPORT.

Thanks for spotting that. I have a script that fixes the whitespace
issue automatically, but it doesn't catch missing leading * in entries.

Do you want me to send an updated and rebased version of these patches
(including changelog fixups)?

Thanks,
Rasmus


[aarch64}: added variable issue rate feature for falkor

2018-08-13 Thread Kai Tietz
Hello,

this patch implements variable issue rate feature for falkor cpu.
Additionally this patch adjusts the issue rate for falkor 8 as this
value reflects more cpu's specification.

This patch was tested against SPEC 2017 & 2016 and showed in general
some improvements without any regressions for thos tests.

ChangeLog:

Jim Wilson 
Kai Tietz 

* config/aarch64.c (aarch64_sched_reorder): Implementing
TARGET_SHED_REORDER hook.
(aarch64_variable_issue): Implemented TARGET_SHED_VARIABLE_ISSUE
hook.
(TARGET_SHED_REORDER): Defined.
(TARGET_SHED_VARIABLE_ISSUE): Likewise.
* config/aarch64/falor.md (falkor_variable_issue): New.

Ok for apply?

Regards,
Kai

PS: I might be in need to update my key-files for stronger bitness.
Whom I can ask for doing this?
	Jim Wilson 
	Kai Tietz 

	* config/aarch64.c (aarch64_sched_reorder): Implementing
	TARGET_SHED_REORDER hook.
	(aarch64_variable_issue): Implemented TARGET_SHED_VARIABLE_ISSUE
	hook.
	(TARGET_SHED_REORDER): Defined.
	(TARGET_SHED_VARIABLE_ISSUE): Likewise.
	* config/aarch64/falor.md (falkor_variable_issue): New.

Index: aarch64/aarch64.c
===
--- aarch64.orig/aarch64.c
+++ aarch64/aarch64.c
@@ -914,7 +914,7 @@ static const struct tune_params qdf24xx_
   _branch_cost,
   _approx_modes,
   4, /* memmov_cost  */
-  4, /* issue_rate  */
+  8, /* issue_rate  */
   (AARCH64_FUSE_MOV_MOVK | AARCH64_FUSE_ADRP_ADD
| AARCH64_FUSE_MOVK_MOVK), /* fuseable_ops  */
   "16",	/* function_align.  */
@@ -17551,6 +17551,105 @@ aarch64_run_selftests (void)
 
 #endif /* #if CHECKING_P */
 
+/* The number of micro ops left over after issuing the last instruction in a
+   cycle.  This is subtrated from the next cycle before we start issuing insns.
+   This is initialized to 0 at the start of every basic block.  */
+static int leftover_uops = 0;
+
+/* Implement TARGET_SCHED_REORDER.  */
+
+static int
+aarch64_sched_reorder (FILE *file, int verbose,
+		   rtx_insn **ready ATTRIBUTE_UNUSED,
+		   int *n_readyp ATTRIBUTE_UNUSED,
+		   int clock)
+{
+  int can_issue_more = aarch64_sched_issue_rate ();
+
+  if ((enum attr_tune) aarch64_tune == TUNE_FALKOR)
+{
+  /* The start of a basic block.  */
+  if (clock == 0)
+	{
+	  if (leftover_uops && file && (verbose > 3))
+	fprintf (file, ";;\tLeftover uops ignored at bb start.\n");
+
+	  leftover_uops = 0;
+	}
+
+  /* Account for issue slots left over from previous cycle.  This value
+	 can be larger than the number of issue slots per cycle, so we need
+	 to check it here before scheduling any instructions.  */
+  else if (leftover_uops)
+	{
+	  can_issue_more -= leftover_uops;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tUse %d issue slots for leftover uops.\n",
+		   leftover_uops);
+	  fprintf (file, ";;\t%d issue slots left.\n", can_issue_more);
+	}
+
+	  leftover_uops = 0;
+
+	  if (can_issue_more < 0)
+	{
+	  leftover_uops = 0 - can_issue_more;
+	  can_issue_more = 0;
+
+	  if (file && (verbose > 3))
+		{
+		  fprintf (file, ";;skipping issue cycle.\n");
+		  fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+		}
+	}
+	}
+}
+
+  return can_issue_more;
+}
+
+/* Implement TARGET_SCHED_VARIABLE_ISSUE.  */
+
+static int
+aarch64_variable_issue (FILE *file, int verbose,
+			rtx_insn *insn, int more)
+{
+  if (GET_CODE (PATTERN (insn)) != USE
+  && GET_CODE (PATTERN (insn)) != CLOBBER)
+{
+  if ((enum attr_tune) aarch64_tune != TUNE_FALKOR)
+	more -= 1;
+  else
+	{
+	  int issue_slots = get_attr_falkor_variable_issue (insn);
+	  more -= issue_slots;
+
+	  if (file && (verbose > 3))
+	{
+	  fprintf (file, ";;\tInsn takes %d issue slots.\n", issue_slots);
+	  fprintf (file, ";;\t%d issue slots left.\n", more);
+	}
+
+	  /* We schedule an instruction first, and then subtract issue slots,
+	 which means the result can be negative.  We carry the extra over
+	 to the next cycle.  */
+
+	  if (more < 0)
+	{
+	  leftover_uops = 0 - more;
+	  more = 0;
+
+	  if (file && (verbose > 3))
+		fprintf (file, ";;\t%d uops left over.\n", leftover_uops);
+	}
+	}
+}
+
+  return more;
+}
+
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST aarch64_address_cost
 
@@ -17779,6 +17878,12 @@ aarch64_libgcc_floating_mode_supported_p
 #undef TARGET_SCHED_ISSUE_RATE
 #define TARGET_SCHED_ISSUE_RATE aarch64_sched_issue_rate
 
+#undef TARGET_SCHED_REORDER
+#define TARGET_SCHED_REORDER aarch64_sched_reorder
+
+#undef TARGET_SCHED_VARIABLE_ISSUE
+#define TARGET_SCHED_VARIABLE_ISSUE aarch64_variable_issue
+
 #undef TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD
 #define TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD \
   aarch64_sched_first_cycle_multipass_dfa_lookahead
Index: aarch64/falkor.md
===

[PATCH] S/390: Factor out constant pool ref decomposition

2018-08-13 Thread Ilya Leoshkevich
gcc/ChangeLog:

2018-07-27  Ilya Leoshkevich  

* config/s390/s390.c (s390_decompose_constant_pool_ref):
New function.
(s390_decompose_address): Factor out constant pool ref
decomposition.
---
 gcc/config/s390/s390.c | 155 ++---
 1 file changed, 67 insertions(+), 88 deletions(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index f5bf336b521..9d46fd88646 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -2754,6 +2754,67 @@ s390_short_displacement (rtx disp)
   return false;
 }
 
+/* Attempts to split `ref', which should be either UNSPEC_LTREF or
+   UNSPEC_LTREL_BASE, into (base + `disp').  In case pool base is not known,
+   caller-provided `pool_base' is used.  If successful, also determines the
+   following characteristics of `ref': `is_ptr' - whether it can be an
+   LA argument, `is_base_ptr' - whether the resulting base is a well-known
+   base register (stack/frame pointer, etc), `is_pool_ptr` - whether it is
+   considered a literal pool pointer for purposes of avoiding two different
+   literal pool pointers per insn during or after reload (`B' constraint).  */
+static bool
+s390_decompose_constant_pool_ref (rtx *ref, rtx *disp, bool *is_ptr,
+ bool *is_base_ptr, bool *is_pool_ptr,
+ rtx pool_base)
+{
+  if (!*ref)
+return true;
+
+  if (GET_CODE (*ref) == UNSPEC)
+switch (XINT (*ref, 1))
+  {
+  case UNSPEC_LTREF:
+   if (!*disp)
+ *disp = gen_rtx_UNSPEC (Pmode,
+ gen_rtvec (1, XVECEXP (*ref, 0, 0)),
+ UNSPEC_LTREL_OFFSET);
+   else
+ return false;
+
+   *ref = XVECEXP (*ref, 0, 1);
+   break;
+
+  case UNSPEC_LTREL_BASE:
+   if (XVECLEN (*ref, 0) == 1)
+ *ref = pool_base, *is_pool_ptr = true;
+   else
+ *ref = XVECEXP (*ref, 0, 1);
+   break;
+
+  default:
+   return false;
+  }
+
+  if (!REG_P (*ref) || GET_MODE (*ref) != Pmode)
+return false;
+
+  if (REGNO (*ref) == STACK_POINTER_REGNUM
+  || REGNO (*ref) == FRAME_POINTER_REGNUM
+  || ((reload_completed || reload_in_progress)
+ && frame_pointer_needed
+ && REGNO (*ref) == HARD_FRAME_POINTER_REGNUM)
+  || REGNO (*ref) == ARG_POINTER_REGNUM
+  || (flag_pic
+ && REGNO (*ref) == PIC_OFFSET_TABLE_REGNUM))
+*is_ptr = *is_base_ptr = true;
+
+  if ((reload_completed || reload_in_progress)
+  && *ref == cfun->machine->base_reg)
+*is_ptr = *is_base_ptr = *is_pool_ptr = true;
+
+  return true;
+}
+
 /* Decompose a RTL expression ADDR for a memory address into
its components, returned in OUT.
 
@@ -2865,96 +2926,14 @@ s390_decompose_address (rtx addr, struct s390_address 
*out)
 }
 
   /* Validate base register.  */
-  if (base)
-{
-  if (GET_CODE (base) == UNSPEC)
-   switch (XINT (base, 1))
- {
- case UNSPEC_LTREF:
-   if (!disp)
- disp = gen_rtx_UNSPEC (Pmode,
-gen_rtvec (1, XVECEXP (base, 0, 0)),
-UNSPEC_LTREL_OFFSET);
-   else
- return false;
-
-   base = XVECEXP (base, 0, 1);
-   break;
-
- case UNSPEC_LTREL_BASE:
-   if (XVECLEN (base, 0) == 1)
- base = fake_pool_base, literal_pool = true;
-   else
- base = XVECEXP (base, 0, 1);
-   break;
-
- default:
-   return false;
- }
-
-  if (!REG_P (base) || GET_MODE (base) != Pmode)
-   return false;
-
-  if (REGNO (base) == STACK_POINTER_REGNUM
- || REGNO (base) == FRAME_POINTER_REGNUM
- || ((reload_completed || reload_in_progress)
- && frame_pointer_needed
- && REGNO (base) == HARD_FRAME_POINTER_REGNUM)
- || REGNO (base) == ARG_POINTER_REGNUM
- || (flag_pic
- && REGNO (base) == PIC_OFFSET_TABLE_REGNUM))
-   pointer = base_ptr = true;
-
-  if ((reload_completed || reload_in_progress)
- && base == cfun->machine->base_reg)
-   pointer = base_ptr = literal_pool = true;
-}
+  if (!s390_decompose_constant_pool_ref (, , , _ptr,
+_pool, fake_pool_base))
+return false;
 
   /* Validate index register.  */
-  if (indx)
-{
-  if (GET_CODE (indx) == UNSPEC)
-   switch (XINT (indx, 1))
- {
- case UNSPEC_LTREF:
-   if (!disp)
- disp = gen_rtx_UNSPEC (Pmode,
-gen_rtvec (1, XVECEXP (indx, 0, 0)),
-UNSPEC_LTREL_OFFSET);
-   else
- return false;
-
-   indx = XVECEXP (indx, 0, 1);
-   break;
-
- case UNSPEC_LTREL_BASE:
-   if (XVECLEN (indx, 0) == 1)
- indx = fake_pool_base, 

Re: libgo: fix spurious test failure in libgo/runtime/pprof

2018-08-13 Thread Andreas Schwab
On Aug 09 2018, Ian Lance Taylor  wrote:

> On Thu, Aug 9, 2018 at 8:03 AM, Andreas Schwab  wrote:
>> TestMemoryProfiler uses a too restrictive pattern that fails to match
>> backtraces that contain two tabs after the function address.  That can
>> happen when the formatted addresses are of different length:
>
> Your patch seems to be whitespace only

Nope, look closer.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


Re: C++ PATCH for c++/86915, ICE with auto[]

2018-08-13 Thread Jason Merrill
OK.

On Sun, Aug 12, 2018 at 4:53 AM, Marek Polacek  wrote:
> This fixes ICE-on-invalid with an array of auto.  The problem was that
> create_array_type_for_decl got null name, so the error call crashed.  We
> need to handle this case as elsewhere in the function.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-08-11  Marek Polacek  
>
> PR c++/86915
> * decl.c (create_array_type_for_decl): Handle null name.
>
> * g++.dg/diagnostic/auto1.C: New test.
>
> --- gcc/cp/decl.c
> +++ gcc/cp/decl.c
> @@ -9838,7 +9838,10 @@ create_array_type_for_decl (tree name, tree type, tree 
> size)
>   type-specifier, the program is ill-formed.  */
>if (type_uses_auto (type))
>  {
> -  error ("%qD declared as array of %qT", name, type);
> +  if (name)
> +   error ("%qD declared as array of %qT", name, type);
> +  else
> +   error ("creating array of %qT", type);
>return error_mark_node;
>  }
>
> --- gcc/testsuite/g++.dg/diagnostic/auto1.C
> +++ gcc/testsuite/g++.dg/diagnostic/auto1.C
> @@ -0,0 +1,4 @@
> +// PR c++/86915
> +// { dg-do compile { target c++17 } }
> +
> +template struct S; // { dg-error "creating array of .auto." }


Re: C++ PATCH to implement C++20 P0806R2, Deprecate implicit capture of this via [=]

2018-08-13 Thread Jason Merrill
On Sun, Aug 12, 2018 at 3:53 PM, Marek Polacek  wrote:
> -  var = add_capture (lambda,
> -id,
> -initializer,
> -/*by_reference_p=*/
> -   (this_capture_p
> -|| (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda)
> -== CPLD_REFERENCE)),
> -   /*explicit_init_p=*/false);
> +  var = add_capture (lambda, id, initializer,
> +/*by_reference_p=*/
> +(this_capture_p
> + || (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda)
> + == CPLD_REFERENCE)),
> +/*explicit_init_p=*/false);

This reformatting seems unnecessary, and I prefer to avoid unnecessary
reformatting to avoid noise in 'git blame'. The rest of the patch is
OK.

Jason