Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Bernd Edlinger]

On 09/29/16 22:38, Jason Merrill wrote:
> On Thu, Sep 29, 2016 at 3:58 PM, Bernd Edlinger
>  wrote:
>> Unfortunately, without that exception there is a false positive:
>>
>> In file included from ../../gcc-trunk/gcc/ada/gcc-interface/decl.c:30:0:
>> ../../gcc-trunk/gcc/ada/gcc-interface/decl.c: In function 'int
>> adjust_packed(tree, tree, int)':
>> ../../gcc-trunk/gcc/tree.h:1874:22: error: << on signed integer in
>> boolean context [-Werror=int-in-bool-context]
>> ? ((unsigned)1) << ((NODE)->type_common.align - 1) : 0)
>>   ~~^~
>
> Ah, this issue again: the shift isn't in boolean context, it's in
> integer context.  I think we want to be a lot more conservative about
> these warnings in the arms of a COND_EXPR.  In fact, I think the
> entire
>
>/* Distribute the conversion into the arms of a COND_EXPR.  */
>
> section is wrong now that we're doing delayed folding.
>

Could you take care of this ?


For the warning, I think I can suppress it just while
the recursing into the condition arms.

As in this updated patch.

Is it OK?


Bernd.
gcc:
2016-09-29  Bernd Edlinger  

* doc/invoke.texi: Update -Wint-in-bool-context.

c-family:
2016-09-29  Bernd Edlinger  

* c-common.c (c_common_truthvalue_conversion): Warn for suspicious
left shift in boolean context.

cp:
2016-09-29  Bernd Edlinger  

* parser.c (cp_parser_condition): Fix a warning.

testsuite:
2016-09-29  Bernd Edlinger  

* c-c++-common/Wint-in-bool-context.c: Update test.
Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c	(revision 240571)
+++ gcc/c-family/c-common.c	(working copy)
@@ -4655,6 +4655,11 @@ c_common_truthvalue_conversion (location_t locatio
 	return c_common_truthvalue_conversion (location,
 	   TREE_OPERAND (expr, 0));
 
+case LSHIFT_EXPR:
+  warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context,
+		  "<< in boolean context, did you mean '<' ?");
+  break;
+
 case COND_EXPR:
   if (warn_int_in_bool_context
 	  && !from_macro_definition_at (EXPR_LOCATION (expr)))
@@ -4676,6 +4681,8 @@ c_common_truthvalue_conversion (location_t locatio
 	{
 	  tree op1 = TREE_OPERAND (expr, 1);
 	  tree op2 = TREE_OPERAND (expr, 2);
+	  int w = warn_int_in_bool_context;
+	  warn_int_in_bool_context = 0;
 	  /* In C++ one of the arms might have void type if it is throw.  */
 	  if (!VOID_TYPE_P (TREE_TYPE (op1)))
 	op1 = c_common_truthvalue_conversion (location, op1);
@@ -4683,10 +4690,13 @@ c_common_truthvalue_conversion (location_t locatio
 	op2 = c_common_truthvalue_conversion (location, op2);
 	  expr = fold_build3_loc (location, COND_EXPR, truthvalue_type_node,
   TREE_OPERAND (expr, 0), op1, op2);
+	  warn_int_in_bool_context = w;
 	  goto ret;
 	}
   else
 	{
+	  int w = warn_int_in_bool_context;
+	  warn_int_in_bool_context = 0;
 	  /* Folding will happen later for C.  */
 	  expr = build3 (COND_EXPR, truthvalue_type_node,
 			 TREE_OPERAND (expr, 0),
@@ -4694,6 +4704,7 @@ c_common_truthvalue_conversion (location_t locatio
 			 TREE_OPERAND (expr, 1)),
 			 c_common_truthvalue_conversion (location,
 			 TREE_OPERAND (expr, 2)));
+	  warn_int_in_bool_context = w;
 	  goto ret;
 	}
 
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c	(revision 240571)
+++ gcc/cp/parser.c	(working copy)
@@ -11244,7 +11244,7 @@ cp_parser_condition (cp_parser* parser)
 	{
 	  tree pushed_scope;
 	  bool non_constant_p;
-	  bool flags = LOOKUP_ONLYCONVERTING;
+	  int flags = LOOKUP_ONLYCONVERTING;
 
 	  /* Create the declaration.  */
 	  decl = start_decl (declarator, _specifiers,
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi	(revision 240571)
+++ gcc/doc/invoke.texi	(working copy)
@@ -6028,7 +6028,8 @@ of the C++ standard.
 @opindex Wno-int-in-bool-context
 Warn for suspicious use of integer values where boolean values are expected,
 such as conditional expressions (?:) using non-boolean integer constants in
-boolean context, like @code{if (a <= b ? 2 : 3)}.
+boolean context, like @code{if (a <= b ? 2 : 3)}.  Or left shifting in
+boolean context, like @code{for (a = 0; 1 << a; a++);}.
 This warning is enabled by @option{-Wall}.
 
 @item -Wno-int-to-pointer-cast
Index: gcc/testsuite/c-c++-common/Wint-in-bool-context.c
===
--- gcc/testsuite/c-c++-common/Wint-in-bool-context.c	(revision 240571)
+++ gcc/testsuite/c-c++-common/Wint-in-bool-context.c	(working copy)
@@ -25,5 +25,7 @@ int foo (int a, int b)
   if (b ? 1+1 : 1) /* { dg-warning "boolean context" } */
 return 7;
 
+  for (a = 0; 1 << a; a++); 

Fwd: [PATCH] gcc: Fix sysroot relative paths for MinGW

[Tadek Kijkowski]

Can I have plain-text mode, please, gmail?

-- Forwarded message --
From: Tadek Kijkowski 
Date: 2016-09-30 5:16 GMT+02:00
Subject: Re: [PATCH] gcc: Fix sysroot relative paths for MinGW
To: Jeff Law 
CC: gcc-patches@gcc.gnu.org


2016-09-29 21:16 GMT+02:00 Jeff Law :
> On 09/23/2016 12:31 AM, Tadek Kijkowski wrote:
>>
>> Prevent paths relative to sysroot directory from being transformed to
>> Windows form with MSYS prefix.
>> See: http://www.mingw.org/wiki/Posix_path_conversion
>>
>> 2016-09-23  Tadek Kijkowski  
>>
>> * gcc/Makefile.in: Fix sysroot relative paths for MinGW
>>
>>
>> Index: gcc/Makefile.in
>> ===
>> --- gcc/Makefile.in(revision 240386)
>> +++ gcc/Makefile.in(working copy)
>> @@ -603,6 +603,18 @@
>>  # UNSORTED
>>  # 
>>
>> +# MSYS will zealously translate all paths to Windows form,
>> +# so /usr/include becomes c:/msysX/usr/include.
>> +# If sysroot is specified this is undesirable, so this function converts
>> +# /usr/include to //usr\include, which will become /usr/include
>> +# again when passed to gcc.
>> +ifneq ($(and @TARGET_SYSTEM_ROOT@,$(filter %-mingw32,$(host))),)
>> +sysroot_relative_path = $(if $(2),$$(echo '$(1)' | tr '/' '\\' | sed
>> 's,^\\,//,'),$(1))
>> +else
>> +sysroot_relative_path = $(1)
>> +endif
>
> I'd really like to see the documentation here improved.
>
> There's no mention of the second argument's purpose.  And one has to parse
> the ifneq line to understand what it's doing.  Those kinds of things should
> be made clear in a comment.
>

OK. I'll expand it.

>
>> +
>> +
>>  # Directory in which the compiler finds libraries etc.
>>  libsubdir =
>> $(libdir)/gcc/$(real_target_noncanonical)/$(version)$(accel_dir_suffix)
>>  # Directory in which the compiler finds executables
>> @@ -2751,14 +2763,14 @@
>>  PREPROCESSOR_DEFINES = \
>>-DGCC_INCLUDE_DIR=\"$(libsubdir)/include\" \
>>-DFIXED_INCLUDE_DIR=\"$(libsubdir)/include-fixed\" \
>> -  -DGPLUSPLUS_INCLUDE_DIR=\"$(gcc_gxx_include_dir)\" \
>> +  -DGPLUSPLUS_INCLUDE_DIR=\"$(call
>> sysroot_relative_path,$(gcc_gxx_include_dir),$(filter-out
>> 0,$(gcc_gxx_include_dir_add_sysroot)))\" \
>
> So why the $(filter-out 0, )?
>
> I'd really like to avoid being too clever here and write this code in the
> most straightforward way possible.
>

Hmm... that's partially leftover from the abandoned idea to pass
@TARGET_SYSTEM_ROOT@ as second parameter of sysroot_relative_path.
Sysroot is prepended to GPLUSPLUS_INCLUDE_DIR in runtime only if
$(gcc_gxx_include_dir) is 1.
Since sysroot_relative_path checks for non-empty string the easiest
way was to use filter-out. But I agree this way it's confusing.

How about if I change the sysroot_relative_path function to explicitly
check for 1? But still - since $(if) checks for empty string, it will
have to use filter or filter-out.

# MSYS will zealously translate all paths to Windows form, so
/usr/include becomes c:/msysX/usr/include.
# If sysroot is specified this is undesirable, so this function
converts /usr/include to //usr\include, which will become /usr/include
# again when passed to gcc.
# This function takes two parameters: first parameter is include
directory path, second parameter tells if the path is relative
# to TARGET_SYSTEM_ROOT.
# If TARGET_SYSTEM_ROOT is not configured or host is not MinGW32, this
function always expands to the unmodified first parameter
# if TARGET_SYSTEM_ROOT is configured and host is MinGW32, but second
parameter is not 1, this function again expands to the unmodified
first parameter
# otherwise, it expands to a shell expression which will transform the
first parameter as described above.
ifneq ($(and @TARGET_SYSTEM_ROOT@,$(filter %-mingw32,$(host))),)
sysroot_relative_path = $(if $(filter 1,$(2)),$$(echo '$(1)' | tr '/'
'\\' | sed 's,^\\,//,'),$(1))
else
sysroot_relative_path = $(1)
endif

...

PREPROCESSOR_DEFINES = \
  -DGCC_INCLUDE_DIR=\"$(libsubdir)/include\" \
  -DFIXED_INCLUDE_DIR=\"$(libsubdir)/include-fixed\" \
  -DGPLUSPLUS_INCLUDE_DIR=\"$(call
sysroot_relative_path,$(gcc_gxx_include_dir),$(gcc_gxx_include_dir_add_sysroot))\"
\
...
  -DNATIVE_SYSTEM_HEADER_DIR=\"$(call
sysroot_relative_path,$(NATIVE_SYSTEM_HEADER_DIR),1)\" \
...

What do you think?

N.B. I'd prefer to use backticks over "$()", but it could clash if
some include paths already contain backtick expressions.

Tadek

[PATCH] read-md.c: track column numbers

[David Malcolm]

This patch adds rudimentary column-number tracking to read-md.c, to
give more precise locations for messages for problems in .md files
(and in the RTL frontend I'm working on):

../../src/gcc/config/i386/i386.md:1204:22: error: unknown rtx code 
`define_mood_iterator'
../../src/gcc/config/i386/i386.md:1204:25: note: following context is `PTR'

(note the column numbers)

One wart is restoring the column when:
  unread_char ('\n')
It can handle a single one of these using m_last_line_colno.  More
than one could potentially be handled by using a vec, but that
complicates the dependencies, so I kept it simple by only handling one
of these (hence my use of the word "rudimentary" above).

Successfully bootstrapped on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
* genattrtab.c (make_internal_attr): Supply dummy column number to
file_location ctor.
(main): Likewise.
* genoutput.c (init_insn_for_nothing): Likewise.
* gensupport.c (add_define_attr): Likewise.
* read-md.c (message_at_1): Print column number.
(fatal_with_file_and_line): Likewise.
(rtx_reader::read_char): Track column numbers.
(rtx_reader::unread_char): Likewise.
(rtx_reader::rtx_reader): Initialize m_read_md_colno.
(rtx_reader::handle_include): Stash and restore m_read_md_colno.
(rtx_reader::handle_file): Initialize m_read_md_colno.
(rtx_reader::get_current_location): Supply column number to
file_location ctor.
* read-md.h (struct file_location): Add field "colno".
(file_location::file_location): Likewise.
(rtx_reader::get_colno): New accessor.
(rtx_reader::m_read_md_colno): New field.
(rtx_reader::m_last_line_colno): New field.
---
 gcc/genattrtab.c |  4 ++--
 gcc/genoutput.c  |  2 +-
 gcc/gensupport.c |  2 +-
 gcc/read-md.c| 33 -
 gcc/read-md.h| 16 +---
 5 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
index c8e166e..ccfcdfd 100644
--- a/gcc/genattrtab.c
+++ b/gcc/genattrtab.c
@@ -4657,7 +4657,7 @@ make_internal_attr (const char *name, rtx value, int 
special)
   attr->is_numeric = 1;
   attr->is_const = 0;
   attr->is_special = (special & ATTR_SPECIAL) != 0;
-  attr->default_val = get_attr_value (file_location ("", 0),
+  attr->default_val = get_attr_value (file_location ("", 0, 0),
  value, attr, -2);
 }
 
@@ -5279,7 +5279,7 @@ main (int argc, const char **argv)
   md_rtx_info info;
   info.def = rtx_alloc (DEFINE_ASM_ATTRIBUTES);
   XVEC (info.def, 0) = rtvec_alloc (0);
-  info.loc = file_location ("", 0);
+  info.loc = file_location ("", 0, 0);
   info.index = -1;
   gen_insn ();
 }
diff --git a/gcc/genoutput.c b/gcc/genoutput.c
index 5909258..f792cb4 100644
--- a/gcc/genoutput.c
+++ b/gcc/genoutput.c
@@ -980,7 +980,7 @@ init_insn_for_nothing (void)
   idata = XCNEW (struct data);
   new (idata) data ();
   idata->name = "*placeholder_for_nothing";
-  idata->loc = file_location ("", 0);
+  idata->loc = file_location ("", 0, 0);
   idata_end = >next;
 }
 
diff --git a/gcc/gensupport.c b/gcc/gensupport.c
index 1648c9c..cb74aea 100644
--- a/gcc/gensupport.c
+++ b/gcc/gensupport.c
@@ -446,7 +446,7 @@ add_define_attr (const char *name)
   XEXP (t1, 2) = rtx_alloc (CONST_STRING);
   XSTR (XEXP (t1, 2), 0) = "yes";
   e->data = t1;
-  e->loc = file_location ("built-in", -1);
+  e->loc = file_location ("built-in", -1, -1);
   e->next = define_attr_queue;
   define_attr_queue = e;
 
diff --git a/gcc/read-md.c b/gcc/read-md.c
index f069ba5..e158be5 100644
--- a/gcc/read-md.c
+++ b/gcc/read-md.c
@@ -218,7 +218,7 @@ print_c_condition (const char *cond)
 static void ATTRIBUTE_PRINTF(2,0)
 message_at_1 (file_location loc, const char *msg, va_list ap)
 {
-  fprintf (stderr, "%s:%d: ", loc.filename, loc.lineno);
+  fprintf (stderr, "%s:%d:%d: ", loc.filename, loc.lineno, loc.colno);
   vfprintf (stderr, msg, ap);
   fputc ('\n', stderr);
 }
@@ -274,8 +274,8 @@ fatal_with_file_and_line (const char *msg, ...)
 
   va_start (ap, msg);
 
-  fprintf (stderr, "%s:%d: error: ", rtx_reader_ptr->get_filename (),
-  rtx_reader_ptr->get_lineno ());
+  fprintf (stderr, "%s:%d:%d: error: ", rtx_reader_ptr->get_filename (),
+  rtx_reader_ptr->get_lineno (), rtx_reader_ptr->get_colno ());
   vfprintf (stderr, msg, ap);
   putc ('\n', stderr);
 
@@ -294,9 +294,9 @@ fatal_with_file_and_line (const char *msg, ...)
 }
   context[i] = '\0';
 
-  fprintf (stderr, "%s:%d: note: following context is `%s'\n",
+  fprintf (stderr, "%s:%d:%d: note: following context is `%s'\n",
   rtx_reader_ptr->get_filename (), rtx_reader_ptr->get_lineno (),
-  context);
+  rtx_reader_ptr->get_colno (), context);
 
   va_end (ap);
   exit (1);
@@ -384,7 +384,13 @@ rtx_reader::read_char (void)
 
   ch = getc (m_read_md_file);
   if 

[committed] testsuite cleanups for nios2-elf

[Sandra Loosemore]

I've checked in this patch to clean up some test failures I noticed on 
nios2-elf.


This bare-metal target defaults to -fno-delete-null-pointer-checks at 
the request of Altera, because address 0 is valid and is typically where 
the interrupt vector lives.  So tests for behavior that depends on 
-fdelete-null-pointer-checks have to specify that option explicitly. 
See r222777 for a bunch of other test cases that previously got this 
treatment.


Also, this target doesn't support -fpic or -rdynamic, so I've added some 
bits to skip a few new-ish test cases that depend on those options.


-Sandra

2016-09-29  Sandra Loosemore  

	gcc/testsuite/
	* c-c++-common/pr27336.c: Make dependency on 
	-fdelete-null-pointer-checks explicit.
	* g++.dg/cpp0x/constexpr-array-ptr10.C: Likewise.
	* g++.dg/cpp0x/constexpr-nullptr-1.C: Likewise.
	* g++.dg/lto/pr69589_0.C: Add nios2-*-elf to unsupported targets.
	* gcc.dg/pic-1.c: Require fpic target support.
	* gcc.dg/pic-2.c: Likewise.
Index: gcc/testsuite/c-c++-common/pr27336.c
===
--- gcc/testsuite/c-c++-common/pr27336.c	(revision 240596)
+++ gcc/testsuite/c-c++-common/pr27336.c	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-vrp1" } */
+/* { dg-options "-O2 -fdelete-null-pointer-checks -fdump-tree-vrp1" } */
 
 struct B { int x; };
 extern void g3(struct B *that)  __attribute__((nonnull));
Index: gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C
===
--- gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C	(revision 240596)
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-array-ptr10.C	(working copy)
@@ -8,7 +8,7 @@
 // that and involves all constexpr object pointers.
 
 // { dg-do compile { target c++11 } }
-// { dg-additional-options "-Wall -Wextra" }
+// { dg-additional-options "-Wall -Wextra -fdelete-null-pointer-checks" }
 
 namespace A {
 
Index: gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-1.C
===
--- gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-1.C	(revision 240596)
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-1.C	(working copy)
@@ -6,7 +6,7 @@
 // c++/67376 on gcc-patches for additional background.
 
 // { dg-do compile { target c++11 } }
-// { dg-options "-fdump-tree-optimized" }
+// { dg-options "-fdelete-null-pointer-checks -fdump-tree-optimized" }
 
 // Runtime assert.  Used for potentially invalid expressions.
 #define RA(e)  ((e) ? (void)0 : __builtin_abort ())
Index: gcc/testsuite/g++.dg/lto/pr69589_0.C
===
--- gcc/testsuite/g++.dg/lto/pr69589_0.C	(revision 240596)
+++ gcc/testsuite/g++.dg/lto/pr69589_0.C	(working copy)
@@ -1,7 +1,7 @@
 // { dg-lto-do link }
 // { dg-lto-options "-O2 -rdynamic" }
 // { dg-extra-ld-options "-r -nostdlib" }
-// { dg-skip-if "Skip targets without -rdynamic support" { arm*-none-eabi aarch64*-*-elf } { "*" } { "" } }
+// { dg-skip-if "Skip targets without -rdynamic support" { arm*-none-eabi aarch64*-*-elf nios2-*-elf } { "*" } { "" } }
 
 #pragma GCC visibility push(hidden)
 struct A { int [] (long); };
Index: gcc/testsuite/gcc.dg/pic-1.c
===
--- gcc/testsuite/gcc.dg/pic-1.c	(revision 240596)
+++ gcc/testsuite/gcc.dg/pic-1.c	(working copy)
@@ -1,4 +1,5 @@
 /* { dg-do compile { target { ! { *-*-darwin* hppa*-*-* } } } } */
+/* { dg-require-effective-target fpic } */
 /* { dg-options "-fpic" } */
 
 #if __PIC__ != 1
Index: gcc/testsuite/gcc.dg/pic-2.c
===
--- gcc/testsuite/gcc.dg/pic-2.c	(revision 240596)
+++ gcc/testsuite/gcc.dg/pic-2.c	(working copy)
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-require-effective-target fpic } */
 /* { dg-options "-fPIC" } */
 
 #if __PIC__ != 2

Re: [PATCH] fixincludes: fix fixincludes for MinGW

[Tadek Kijkowski]

The fixincl executable uses system function to call applyfix or to
direcly patch a header file, with parameters enclosed in single
quotes. The problem is that MinGW system function just calls cmd.exe,
which doesn't strip quotes from parameters and completely ignores
quotes for embedding spaces in parameters. The MinGW system function
also doesn't allow embedding newlines in executed command parameters.
As a result fixincludes doesn't work at all when trying to build a cross
compiler with mingw as host.

On MinGW host, this patch adds system_with_shell function, which
transforms command passed to system function is following way:
* Enclose entire command in double quotes
* Prepend shell executable name, taken from environment variable SHELL
* Replace each occurence of newline with '$'\n'' sequence which is
understood by bash and ksh
* Escape special characters

Second attempt:

Moved system_with_shell to fixlib.c. Escape some special characters
located between single quotes as enclosing single-quoted string in double
quotes nullifies single quotes. Fixed passing file name to apply_fix.
Fixed "test" tests and "shell" fixes, also for DJGPP.

Convert line all endings to unix in check.sh to avoid false positives.

Followed Jeff's and Bruce's advices:
Fixed formatting.
Call system_with_shell explicitly. If it's not needed it just expands
to "system".

On MinGW (mingw64 actually) all fixinclude tests pass.
On DJGPP all but two tests pass - if you manage to get it to compile at all.

2016-09-30  Tadek Kijkowski  

* check.tpl: Convert line endings to unix on test outputs
* fixfixes.c: Fixed passing file name to apply_fix when
SEPARATE_FIX_PROC is defined
* fixincl.c: Use system_with_shell, fixes for MinGW and DJGPP
* fixlib.c, fixlib.h: Added system_with_shell and fix_path_separators

Index: fixincludes/check.tpl
===
--- fixincludes/check.tpl (revision 240643)
+++ fixincludes/check.tpl (working copy)
@@ -123,6 +123,11 @@
 exec < ${TESTDIR}/LIST
 while read f
 do
+  if [ -n "$MSYSTEM" -o -n "$DJGPP" ]
+  then
+# On MinGW and DJGPP convert line endings to avoid false positives
+mv $f $f.dos; tr -d '\r' < $f.dos > $f; rm $f.dos
+  fi
   if [ ! -f ${TESTBASE}/$f ]
   then
 echo "Newly fixed header:  $f" >&2
Index: fixincludes/fixfixes.c
===
--- fixincludes/fixfixes.c (revision 240643)
+++ fixincludes/fixfixes.c (working copy)
@@ -790,7 +790,8 @@
   return EXIT_FAILURE;
 }

-  apply_fix (pFix, argv[1]);
+  /* Second parameter of apply_fix is file name */
+  apply_fix (pFix, argv[2]);
   fclose (stdout);
   fclose (stdin);
   unlink (argv[4]);
Index: fixincludes/fixincl.c
===
--- fixincludes/fixincl.c (revision 240643)
+++ fixincludes/fixincl.c (working copy)
@@ -74,9 +74,12 @@
 #endif /* DO_STATS */

 const char incl_quote_pat[] = "^[ \t]*#[ \t]*include[ \t]*\"[^/]";
-tSCC z_fork_err[] = "Error %d (%s) starting filter process for %s\n";
 regex_t incl_quote_re;

+#ifndef SEPARATE_FIX_PROC
+tSCC z_fork_err[] = "Error %d (%s) starting filter process for %s\n";
+#endif
+
 static void do_version (void) ATTRIBUTE_NORETURN;
 char *load_file (const char *);
 void run_compiles (void);
@@ -188,7 +191,7 @@
   puts (zBuf + 5);
   exit (strcmp (run_shell (zBuf), program_id));
 #else
-  exit (system (zBuf));
+  exit (system_with_shell (zBuf));
 #endif
 }

@@ -275,6 +278,11 @@
   /* NULL as the first argument to `tempnam' causes it to DTRT
  wrt the temporary directory where the file will be created.  */
   pz_temp_file = tempnam( NULL, "fxinc" );
+
+#if defined(__MINGW32__)
+  fix_path_separators (pz_temp_file);
+#endif
+
 # endif

   signal (SIGQUIT, SIG_IGN);
@@ -566,7 +574,27 @@
   free ((void *) pz_res);
   return res;
 }
+#elif defined(__MINGW32__) || defined(__DJGPP__)
+static int
+test_test (tTestDesc* p_test, char* pz_test_file)
+{
+  tSCC cmd_fmt[] =
+#if defined(__DJGPP__)
+"file=%s; test %s >/dev/null 2>/dev/null";
 #else
+"file=%s; test %s > /dev/null 2>&1";
+#endif
+  int res;
+
+  char *cmd_buf = XNEWVEC (char, strlen(cmd_fmt) +
strlen(pz_test_file) + strlen(p_test->pz_test_text));
+
+  sprintf (cmd_buf, cmd_fmt, pz_test_file, p_test->pz_test_text);
+  res = system_with_shell (cmd_buf);
+
+  free (cmd_buf);
+  return res ? SKIP_FIX : APPLY_FIX;
+}
+#else
 /*
  *  IF we are in MS-DOS land, then whatever shell-type test is required
  *  will, by definition, fail
@@ -887,7 +915,7 @@
   else /* NOT an "internal" fix: */
 {
   size_t parg_size;
-#ifdef __MSDOS__
+#if defined(__MSDOS__) && !defined(__DJGPP__)
   /* Don't use the "src > dstX; rm -f dst; mv -f dstX dst" trick:
  dst is a temporary file anyway, so we know there's no other
  file by that name; and DOS's system(3) doesn't mind to
@@ -906,6 +934,8 @@
 

Re: [PATCH], Backport PR 77670 fix to GCC 6.3

[Segher Boessenkool]

On Thu, Sep 29, 2016 at 06:04:47PM -0400, Michael Meissner wrote:
> I tried building spec 2006 with the current gcc 6.x branch, and gamess, 
> soplex,
> and povray fail when I use -mcpu=power9 -mpower9-minmax.
> 
> The patch for the trunk applies to the GCC 6.x branch, and I was able to build
> all of Spec 2006 with the patched compiler.  I did a bootstrap build and make
> check with no regressions on a little endian Power8 system.  Can I apply this
> patch to the gcc 6.x branch?

Yes please.  Thanks,


Segher

Re: Implement P0001R1 - C++17 removal of register storage class specifier

[Joseph Myers]

This is missing documentation of the new -Wregister option in invoke.texi.

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

Re: [PATCH] fixincludes: fix fixincludes for MinGW

[Tadek Kijkowski]

Hold on. I have much improved version almost ready. It passes all the tests now.

2016-09-30 0:15 GMT+02:00 Bruce Korb :
> I usually try to catch emails with "fixincludes" in the title.
> Can I please get a copy of the original patch?  Thanks.
>
>
> On 09/29/16 11:44, Jeff Law wrote:
>>
>> On 09/22/2016 11:26 PM, Tadek Kijkowski wrote:
>>>
>>> The fixincl executable uses system function to call applyfix or to
>>> direcly patch a header file, with parameters enclosed in single
>>> quotes. This problem is that MinGW system function just calls cmd.exe,
>>> which doesn't strip quotes from parameters and completely ignores
>>> quotes for embedding spaces in parameters. The MinGW system function
>>> also doesn't allow for newlines in executed command parameters. As a
>>> result fixincludes doesn't wotk at on when trying to build a cross
>>> compiler with mingw as host.
>>>
>>> On MinGW host, this patch adds system_with_shell function, which
>>> transforms command passed to system function is following way:
>>> - Enclose entire command in double quotes
>>> - Prepend shell executable name, taken from environment variable SHELL
>>> - Replace each occurence of newline with '$'\n'' sequence which is
>>> understood by bash and ksh (it is assumed that all newlines are
>>> embedded in command parameters enclosed in single quotes)
>>>
>>> 2016-09-23 Tadek Kijkowski (tkijkow...@gmail.com)
>>>
>>> * fixincl.c: Added system_with_shell for MinGW host.
>>>
>>>
>>> Index: fixincludes/fixincl.c
>>> ===
>>> --- fixincludes/fixincl.c   (revision 240386)
>>> +++ fixincludes/fixincl.c   (working copy)
>>> @@ -170,7 +170,111 @@
>>>exit (EXIT_SUCCESS);
>>>  }
>>>
>>> +#ifdef __MINGW32__
>>>
>>> +/* Count number of \c needle character instances in string */
>>> +static int
>>> +count_chars ( char* str, char needle )
>>
>> Formatting it.  This should be:
>>
>> count_chars (char* str, char needle)
>>
>> Note the lack of horizontal whitespace after the open paren or before
>> the close paren.  Similarly for system_with_shell declaration below.
>>
>> Wouldn't this be better named "count_occurrences_of_char" or
>> "count_instances_of_char"?
>>
>>
>>
>>
>>> +
>>> +  /* Allocate enough memory to fit newly created command string */
>>> +  cmd_size = strlen (env_shell) + (sizeof (z_shell_start_args) - 1)
>>> +   + strlen (s) + newline_cnt * (sizeof(z_shell_newline) - 1
>>> - 1)
>>> +   + (sizeof (z_shell_end_args) - 1) + 1;
>>
>> Why use sizeof (foo) - 1 rather than just strlen (foo) here?  Note that
>> GCC can compute the string length as a compile time constant, so you're
>> not gaining any performance by using sizeof here and strlen seems clearer.
>>
>>
>>
>>> +
>>> +  long_cmd = XNEWVEC (char, cmd_size);
>>> +
>>> +  /* Start with ${SHELL} -c " */
>>> +  strcpy (long_cmd, env_shell);
>>> +  strcat (long_cmd, z_shell_start_args);
>>> +
>>> +  /* End pointer for appending pieces of command while replacing
>>> newlines */
>>> +  cmd_endp = long_cmd + strlen(long_cmd);
>>
>> Formatting nit.  Space between function name and its argument list.
>>
>>
>>> +  nl_scan = s;
>>> +  while (nl_scan != NULL)
>>> +{
>>> +  char* next_nl = strchr (nl_scan, '\n');
>>
>> Formatting nit.  char *next_nl.
>>
>>
>>> +  if (next_nl)
>>> +{
>>> +  /* Append part of command between newlines */
>>> +  size_t part_len = next_nl - nl_scan;
>>> +  memcpy(cmd_endp, nl_scan, part_len);
>>
>> Formatting nit, space between function name and its argument list.
>>
>>> +  cmd_endp += part_len;
>>> +
>>> +  /* Append newline replacement */
>>> +  memcpy(cmd_endp, z_shell_newline, sizeof(z_shell_newline));
>>
>> Smilarly, space between functio nname and its argument list.
>>
>>> +  cmd_endp += sizeof(z_shell_newline) - 1;
>>
>> Here too.
>>
>>> +
>>> +  free ( (void*) long_cmd);
>>
>> free (long_cmd);
>>
>> Can you fix those various (minor) issue and resubmit.  I think it'll be
>> OK for the trunk with those changes.
>>
>> jeff
>>
>>
>

Re: [PATCH] fixincludes: fix fixincludes for MinGW

[Bruce Korb]

OK, I found it.  Looks like my MUA is getting too aggressive with its filtering.
What Jeff said, plus I would prefer the tail end looking like:

+
+#else
+
+#define system_with_shell system // normal call
+
+#endif /* defined(__MINGW32__) */

and modifying the call to use "system_with_shell".
The point being that obvious clues are better than subtle switcheroos.

On Thu, Sep 29, 2016 at 11:44 AM, Jeff Law  wrote:

Re: [PATCH] fixincludes: fix fixincludes for MinGW

[Bruce Korb]

I usually try to catch emails with "fixincludes" in the title.
Can I please get a copy of the original patch?  Thanks.

On 09/29/16 11:44, Jeff Law wrote:

On 09/22/2016 11:26 PM, Tadek Kijkowski wrote:

The fixincl executable uses system function to call applyfix or to
direcly patch a header file, with parameters enclosed in single
quotes. This problem is that MinGW system function just calls cmd.exe,
which doesn't strip quotes from parameters and completely ignores
quotes for embedding spaces in parameters. The MinGW system function
also doesn't allow for newlines in executed command parameters. As a
result fixincludes doesn't wotk at on when trying to build a cross
compiler with mingw as host.

On MinGW host, this patch adds system_with_shell function, which
transforms command passed to system function is following way:
- Enclose entire command in double quotes
- Prepend shell executable name, taken from environment variable SHELL
- Replace each occurence of newline with '$'\n'' sequence which is
understood by bash and ksh (it is assumed that all newlines are
embedded in command parameters enclosed in single quotes)

2016-09-23 Tadek Kijkowski (tkijkow...@gmail.com)

* fixincl.c: Added system_with_shell for MinGW host.


Index: fixincludes/fixincl.c
===
--- fixincludes/fixincl.c   (revision 240386)
+++ fixincludes/fixincl.c   (working copy)
@@ -170,7 +170,111 @@
   exit (EXIT_SUCCESS);
 }

+#ifdef __MINGW32__

+/* Count number of \c needle character instances in string */
+static int
+count_chars ( char* str, char needle )

Formatting it.  This should be:

count_chars (char* str, char needle)

Note the lack of horizontal whitespace after the open paren or before
the close paren.  Similarly for system_with_shell declaration below.

Wouldn't this be better named "count_occurrences_of_char" or
"count_instances_of_char"?





+
+  /* Allocate enough memory to fit newly created command string */
+  cmd_size = strlen (env_shell) + (sizeof (z_shell_start_args) - 1)
+   + strlen (s) + newline_cnt * (sizeof(z_shell_newline) - 1
- 1)
+   + (sizeof (z_shell_end_args) - 1) + 1;

Why use sizeof (foo) - 1 rather than just strlen (foo) here?  Note that
GCC can compute the string length as a compile time constant, so you're
not gaining any performance by using sizeof here and strlen seems clearer.




+
+  long_cmd = XNEWVEC (char, cmd_size);
+
+  /* Start with ${SHELL} -c " */
+  strcpy (long_cmd, env_shell);
+  strcat (long_cmd, z_shell_start_args);
+
+  /* End pointer for appending pieces of command while replacing
newlines */
+  cmd_endp = long_cmd + strlen(long_cmd);

Formatting nit.  Space between function name and its argument list.



+  nl_scan = s;
+  while (nl_scan != NULL)
+{
+  char* next_nl = strchr (nl_scan, '\n');

Formatting nit.  char *next_nl.



+  if (next_nl)
+{
+  /* Append part of command between newlines */
+  size_t part_len = next_nl - nl_scan;
+  memcpy(cmd_endp, nl_scan, part_len);

Formatting nit, space between function name and its argument list.


+  cmd_endp += part_len;
+
+  /* Append newline replacement */
+  memcpy(cmd_endp, z_shell_newline, sizeof(z_shell_newline));

Smilarly, space between functio nname and its argument list.


+  cmd_endp += sizeof(z_shell_newline) - 1;

Here too.


+
+  free ( (void*) long_cmd);

free (long_cmd);

Can you fix those various (minor) issue and resubmit.  I think it'll be
OK for the trunk with those changes.

jeff

Re: [PATCH], Backport PR 77670 fix to GCC 6.3

[Michael Meissner]

I tried building spec 2006 with the current gcc 6.x branch, and gamess, soplex,
and povray fail when I use -mcpu=power9 -mpower9-minmax.

The patch for the trunk applies to the GCC 6.x branch, and I was able to build
all of Spec 2006 with the patched compiler.  I did a bootstrap build and make
check with no regressions on a little endian Power8 system.  Can I apply this
patch to the gcc 6.x branch?

2016-09-29  Michael Meissner  

Back port from trunk

2016-09-21  Michael Meissner  

PR target/77670
* config/rs6000/predicates.md (invert_fpmask_comparison_operator):
New predicate that matches the ISA 3.0 XSCMP{EQ,GT,GE}DP
instructions when you want to invert the test.
* config/rs6000/rs6000.md (fpmask): Use the arguments in the
correct order for XXSEL.
(movcc_invert_p9): Define the inverted test
for using XSCMP{EQ,GT,GE}DP.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/predicates.md
===
--- gcc/config/rs6000/predicates.md (revision 240634)
+++ gcc/config/rs6000/predicates.md (working copy)
@@ -1163,6 +1163,12 @@ (define_predicate "scc_rev_comparison_op
 (define_predicate "fpmask_comparison_operator"
   (match_code "eq,gt,ge"))
 
+;; Return 1 if OP is a comparison operator suitable for vector/scalar
+;; comparisons that generate a 0/-1 mask (i.e. the inverse of
+;; fpmask_comparison_operator).
+(define_predicate "invert_fpmask_comparison_operator"
+  (match_code "ne,unlt,unle"))
+
 ;; Return 1 if OP is a comparison operation that is valid for a branch
 ;; insn, which is true if the corresponding bit in the CC register is set.
 (define_predicate "branch_positive_comparison_operator"
Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 240634)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -4846,6 +4846,43 @@ (define_insn_and_split "*mov<
  [(set_attr "length" "8")
   (set_attr "type" "vecperm")])
 
+;; Handle inverting the fpmask comparisons.
+(define_insn_and_split "*movcc_invert_p9"
+  [(set (match_operand:SFDF 0 "vsx_register_operand" "=&,")
+   (if_then_else:SFDF
+(match_operator:CCFP 1 "invert_fpmask_comparison_operator"
+   [(match_operand:SFDF2 2 "vsx_register_operand" 
",")
+(match_operand:SFDF2 3 "vsx_register_operand" 
",")])
+(match_operand:SFDF 4 "vsx_register_operand" ",")
+(match_operand:SFDF 5 "vsx_register_operand" ",")))
+   (clobber (match_scratch:V2DI 6 "=0,"))]
+  "TARGET_P9_MINMAX"
+  "#"
+  "&& 1"
+  [(set (match_dup 6)
+   (if_then_else:V2DI (match_dup 9)
+  (match_dup 7)
+  (match_dup 8)))
+   (set (match_dup 0)
+   (if_then_else:SFDF (ne (match_dup 6)
+  (match_dup 8))
+  (match_dup 5)
+  (match_dup 4)))]
+{
+  rtx op1 = operands[1];
+  enum rtx_code cond = reverse_condition_maybe_unordered (GET_CODE (op1));
+
+  if (GET_CODE (operands[6]) == SCRATCH)
+operands[6] = gen_reg_rtx (V2DImode);
+
+  operands[7] = CONSTM1_RTX (V2DImode);
+  operands[8] = CONST0_RTX (V2DImode);
+
+  operands[9] = gen_rtx_fmt_ee (cond, CCFPmode, operands[2], operands[3]);
+}
+ [(set_attr "length" "8")
+  (set_attr "type" "vecperm")])
+
 (define_insn "*fpmask"
   [(set (match_operand:V2DI 0 "vsx_register_operand" "=wa")
(if_then_else:V2DI
@@ -4865,7 +4902,7 @@ (define_insn "*xxsel"
   (match_operand:SFDF 3 "vsx_register_operand" "")
   (match_operand:SFDF 4 "vsx_register_operand" 
"")))]
   "TARGET_P9_MINMAX"
-  "xxsel %x0,%x1,%x3,%x4"
+  "xxsel %x0,%x4,%x3,%x1"
   [(set_attr "type" "vecmove")])
 
 

RE: [PATCH] Fix PR tree-optimization/77654

[Doug Gilmore]

> From: Christophe Lyon [christophe.l...@linaro.org]
> Sent: Thursday, September 29, 2016 12:17 PM
> To: Matthew Fortune
> Cc: Doug Gilmore; Richard Biener; gcc-patches@gcc.gnu.org; rgue...@gcc.gnu.org
> Subject: Re: [PATCH] Fix PR tree-optimization/77654
> ...
> 
> Since this commit, I've noticed ICE on arm target:
> FAIL: gcc.dg/params/blocksort-part.c -O3 --param prefetch-latency=0
> (internal compiler error)
> FAIL: gcc.dg/params/blocksort-part.c -O3 --param prefetch-latency=0
> (test for excess errors)
> Excess errors:
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/params/blocksort-part.c:116:6:
> internal compiler error: in duplicate
> _ssa_name_ptr_info, at tree-ssanames.c:630
> 0xd5a972 duplicate_ssa_name_ptr_info(tree_node*, ptr_info_def*)
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssanames.c:630
> 0xcac0e0 issue_prefetch_ref
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-loop-prefetch.c:1168
> 0xcad89f issue_prefetches
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-loop-prefetch.c:1195
> 0xcad89f loop_prefetch_arrays
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-loop-prefetch.c:1928
> 0xcae722 tree_ssa_prefetch_arrays()
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-loop-prefetch.c:1992
> 
> 
> --target arm-none-linux-gnueabihf --with-cpu=cortex-a9
> --wihth-fpu=neon-fp16 --with-mode=arm
> 
> I'm not sure the cpu/fpu/mode settings are mandatory but at least in this case
> the compiler ICEs.
> 
> Christophe
I'll look into this.

Doug
> 
> 
> > (Fixed whitespace/tab issue in the code and incorrect file in changelog)
> >
> > I can't progress the bug status. Who does that normally?
> >
> > Matthew

Re: [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP

[Alan Modra]

On Thu, Sep 29, 2016 at 10:55:20AM -0400, David Edelsohn wrote:
> Alan,
> 
> This patch broke bootstrap on AIX.  sysv4.opt is not included on AIX.
> References to sysv4.opt-specific flags have to be protected in
> rs6000.c.

Sorry.  I have reverted the change to rs6000_opt_vars.  svn r240639.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] Extend -falign-FOO=N to N[,M[,N2[,M2]]]

[Denys Vlasenko]

On 09/29/2016 07:32 PM, Denys Vlasenko wrote:

+#ifdef SUBALIGN_LOG


We want to avoid adding new #defines; existing ones are being converted
to target hooks. I suspect the best way is to record whether an M value
was specified, and override it in the x86 option_override if required.
If that's infeasible for some reason we can revisit this.


+  /* Before -falign-foo=N,M,N2,M2 was introduced, x86 had a tweak.
+ -falign-functions=N with N > 8 was adding secondary alignment.
+ -falign-functions=10 was emitting this before every function:
+.p2align 4,,9
+.p2align 3
+ Now this behavior (and more) can be explicitly requested:
+ -falign-functions=16,10,8
+ Retain old behavior if N2 is missing: */
+  else if (a[0].log > SUBALIGN_LOG)
+a[1].log = SUBALIGN_LOG;


So this would live in i386.c, probably.


Thanks, I'm working on it...


...and I can't find a sane way to achieve it in the way you prefer.
Hooking it into ix86_override_options_after_change()?

Like this?

/* Implement TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE hook.  */

static void
ix86_override_options_after_change (void)
{
  ix86_default_align (_options);
}

static void
ix86_default_align (struct gcc_options *opts)
{
  struct align_flags *fa;

  /* -falign-foo without argument: supply one */
  if (opts->x_flag_align_functions && !opts->x_str_align_functions)
{
  opts->x_str_align_functions = 
processor_target_table[ix86_tune].align_func;
}

  /* Before -falign-foo=N,M,N2,M2 was introduced, x86 had a tweak.
 -falign-functions=N with N > 8 was adding secondary alignment.
 -falign-functions=10 was emitting this before every function:
.p2align 4,,9
.p2align 3
 Retain old behavior if N2 is missing: */

  fa = this_target_flag_state->x_align_functions;
  if (fa[1].log == 0 && fa[0].log > 3)
fa[1].log = 3;
}

Well, the problem here is that init_alignments() is called
much later than this hook. The

  fa = this_target_flag_state->x_align_functions;
  if (fa[1].log == 0 && fa[0].log > 3)
fa[1].log = 3;

will not trigger here: x_align_functions[0].log is not yet set!
Just set N2 to 2^3 unconditionally?

  fa = this_target_flag_state->x_align_functions;
  fa[1].log = 3;

This will make any alignment, even none: -falign-functions=1
have 8 byte subalign.

Help?

[PATCH, i386]: Check maximum ext_level before calling CPUID with 0x80000008

[Uros Bizjak]

Hello!

... otherwise we can call CPUID with out-of range level.  Also,
simplify xgetbv checks.

2016-09-29  Uros Bizjak  

* config/i386/driver-i386.c (host_detect_local_cpu): Check maximum
ext_level before calling CPUID with 0x8008.
Simplify xgetbv checks.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/driver-i386.c
===
--- config/i386/driver-i386.c   (revision 240626)
+++ config/i386/driver-i386.c   (working copy)
@@ -517,7 +517,7 @@ const char *host_detect_local_cpu (int argc, const
   /* Check cpuid level of extended features.  */
   __cpuid (0x8000, ext_level, ebx, ecx, edx);
 
-  if (ext_level > 0x8000)
+  if (ext_level >= 0x8001)
 {
   __cpuid (0x8001, eax, ebx, ecx, edx);
 
@@ -535,7 +535,10 @@ const char *host_detect_local_cpu (int argc, const
   has_3dnowp = edx & bit_3DNOWP;
   has_3dnow = edx & bit_3DNOW;
   has_mwaitx = ecx & bit_MWAITX;
+}
 
+  if (ext_level >= 0x8008)
+{
   __cpuid (0x8008, eax, ebx, ecx, edx);
   has_clzero = ebx & bit_CLZERO;
 }
@@ -548,14 +551,21 @@ const char *host_detect_local_cpu (int argc, const
 #define XSTATE_OPMASK  0x20
 #define XSTATE_ZMM 0x40
 #define XSTATE_HI_ZMM  0x80
+
+#define XCR_AVX_ENABLED_MASK \
+  (XSTATE_SSE | XSTATE_YMM)
+#define XCR_AVX512F_ENABLED_MASK \
+  (XSTATE_SSE | XSTATE_YMM | XSTATE_OPMASK | XSTATE_ZMM | XSTATE_HI_ZMM)
+
   if (has_osxsave)
 asm (".byte 0x0f; .byte 0x01; .byte 0xd0"
 : "=a" (eax), "=d" (edx)
 : "c" (XCR_XFEATURE_ENABLED_MASK));
+  else
+eax = 0;
 
-  /* Check if SSE and YMM states are supported.  */
-  if (!has_osxsave
-  || (eax & (XSTATE_SSE | XSTATE_YMM)) != (XSTATE_SSE | XSTATE_YMM))
+  /* Check if AVX registers are supported.  */
+  if ((eax & XCR_AVX_ENABLED_MASK) != XCR_AVX_ENABLED_MASK)
 {
   has_avx = 0;
   has_avx2 = 0;
@@ -569,10 +579,8 @@ const char *host_detect_local_cpu (int argc, const
   has_xsavec = 0;
 }
 
-  if (!has_osxsave
-  || (eax &
- (XSTATE_SSE | XSTATE_YMM | XSTATE_OPMASK | XSTATE_ZMM | 
XSTATE_HI_ZMM))
- != (XSTATE_SSE | XSTATE_YMM | XSTATE_OPMASK | XSTATE_ZMM | 
XSTATE_HI_ZMM))
+  /* Check if AVX512F registers are supported.  */
+  if ((eax & XCR_AVX512F_ENABLED_MASK) != XCR_AVX512F_ENABLED_MASK)
 {
   has_avx512f = 0;
   has_avx512er = 0;
@@ -603,7 +611,7 @@ const char *host_detect_local_cpu (int argc, const
   unsigned int name;
 
   /* Detect geode processor by its processor signature.  */
-  if (ext_level > 0x8001)
+  if (ext_level >= 0x8002)
__cpuid (0x8002, name, ebx, ecx, edx);
   else
name = 0;

Re: Implement P0001R1 - C++17 removal of register storage class specifier

[Jason Merrill]

OK.

On Thu, Sep 29, 2016 at 4:21 PM, Jakub Jelinek  wrote:
> Hi!
>
> This patch pedwarns in C++17 on register storage class specifier, unless
> it is diagnosed errorneous for other reasons and unless it is used in the
> GNU global or local register variable extension.  Even in C++17, users can
> use -Wno-register to suppress the pedwarn, and for C++98-C++14 users can on
> the other side use -Wregister to get warning about it.  Clang seems to
> warn/error out similarly on variables with register storage class under
> -Wregister too, is quiet about the uses in register int var __asm ("...")
> (global and local), though strangely doesn't complain about register
> keywords on function arguments, which this patch diagnoses.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-09-29  Jakub Jelinek  
>
> Implement P0001R1 - C++17 removal of register storage class specifier
> c-family/
> * c.opt (Wregister): New warning.
> * c-opts.c (c_common_post_options): Enable -Wregister by
> default for C++17.
> cp/
> * decl.c (cp_finish_decl): Diagnose register storage class
> on vars except when used in GNU global or local register variable
> extension.
> (grokdeclarator): Diagnose register storage class on parameters.
> * except.c (expand_start_catch_block): Set DECL_REGISTER only
> after cp_finish_decl call.
> testsuite/
> * c-c++-common/Wvarargs-2.c (foo1): Except new warning for C++17.
> * c-c++-common/vector-subscript-2.c (vf): Expect new error for
> C++17.
> * c-c++-common/vector-subscript-5.c (foo): Don't use register
> keyword if not __SSE2__.
> * c-c++-common/Wvarargs.c (foo1, foo3): Expect new warnings for
> C++17.
> * g++.dg/compat/struct-layout-1_generate.c (iterative_hash): Remove
> register keywords.
> * g++.dg/eh/pr29166.C: Add -Wno-register option.
> * g++.dg/warn/register-parm-1.C (erroneous_warning,
> no_erroneous_warning): Expect new warnings for C++17.
> * g++.dg/warn/register-var-2.C (f): Likewise.
> * g++.dg/parse/register1.C (f): Expect new error for C++17.
> * g++.dg/parse/linkage2.C (foo): Likewise.
> * g++.dg/torture/pr36826.C (CoinMin, CoinMax): Avoid register
> keyword on parameters for C++17.
> * g++.dg/cpp1z/register1.C: New test.
> * g++.dg/cpp1z/register2.C: New test.
> * g++.dg/cpp1z/register3.C: New test.
>
> --- gcc/c-family/c.opt.jj   2016-09-26 12:06:37.0 +0200
> +++ gcc/c-family/c.opt  2016-09-29 14:54:17.712047430 +0200
> @@ -842,6 +842,10 @@ Wredundant-decls
>  C ObjC C++ ObjC++ Var(warn_redundant_decls) Warning
>  Warn about multiple declarations of the same object.
>
> +Wregister
> +C++ ObjC++ Var(warn_register) Warning
> +Warn about uses of register storage specifier.
> +
>  Wreorder
>  C++ ObjC++ Var(warn_reorder) Warning LangEnabledBy(C++ ObjC++,Wall)
>  Warn when the compiler reorders code.
> --- gcc/c-family/c-opts.c.jj2016-08-19 11:04:38.0 +0200
> +++ gcc/c-family/c-opts.c   2016-09-29 15:06:06.213254295 +0200
> @@ -871,6 +871,10 @@ c_common_post_options (const char **pfil
>  warn_shift_negative_value = (extra_warnings
>  && (cxx_dialect >= cxx11 || flag_isoc99));
>
> +  /* -Wregister is enabled by default in C++17.  */
> +  if (!global_options_set.x_warn_register)
> +warn_register = cxx_dialect >= cxx1z;
> +
>/* Declone C++ 'structors if -Os.  */
>if (flag_declone_ctor_dtor == -1)
>  flag_declone_ctor_dtor = optimize_size;
> --- gcc/cp/decl.c.jj2016-09-23 19:37:41.0 +0200
> +++ gcc/cp/decl.c   2016-09-29 16:05:03.724470275 +0200
> @@ -6711,6 +6711,19 @@ cp_finish_decl (tree decl, tree init, bo
>if (type == error_mark_node)
>  return;
>
> +  /* Warn about register storage specifiers except when in GNU global
> + or local register variable extension.  */
> +  if (VAR_P (decl) && DECL_REGISTER (decl) && asmspec_tree == NULL_TREE)
> +{
> +  if (cxx_dialect >= cxx1z)
> +   pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wregister,
> +"ISO C++1z does not allow % storage "
> +"class specifier");
> +  else
> +   warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wregister,
> +   "% storage class specifier used");
> +}
> +
>/* If a name was specified, get the string.  */
>if (at_namespace_scope_p ())
>  asmspec_tree = maybe_apply_renaming_pragma (decl, asmspec_tree);
> @@ -11634,7 +11647,20 @@ grokdeclarator (const cp_declarator *dec
> and in case doing stupid register allocation.  */
>
>  if (storage_class == sc_register)
> -  DECL_REGISTER (decl) = 1;
> +  {
> +   DECL_REGISTER (decl) = 1;
> +   /* Warn about register storage specifiers on PARM_DECLs.  */

Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Jason Merrill]

On Thu, Sep 29, 2016 at 3:58 PM, Bernd Edlinger
 wrote:
> Unfortunately, without that exception there is a false positive:
>
> In file included from ../../gcc-trunk/gcc/ada/gcc-interface/decl.c:30:0:
> ../../gcc-trunk/gcc/ada/gcc-interface/decl.c: In function 'int
> adjust_packed(tree, tree, int)':
> ../../gcc-trunk/gcc/tree.h:1874:22: error: << on signed integer in
> boolean context [-Werror=int-in-bool-context]
>? ((unsigned)1) << ((NODE)->type_common.align - 1) : 0)
>  ~~^~

Ah, this issue again: the shift isn't in boolean context, it's in
integer context.  I think we want to be a lot more conservative about
these warnings in the arms of a COND_EXPR.  In fact, I think the
entire

  /* Distribute the conversion into the arms of a COND_EXPR.  */

section is wrong now that we're doing delayed folding.

Jason

Implement P0001R1 - C++17 removal of register storage class specifier

[Jakub Jelinek]

Hi!

This patch pedwarns in C++17 on register storage class specifier, unless
it is diagnosed errorneous for other reasons and unless it is used in the
GNU global or local register variable extension.  Even in C++17, users can
use -Wno-register to suppress the pedwarn, and for C++98-C++14 users can on
the other side use -Wregister to get warning about it.  Clang seems to
warn/error out similarly on variables with register storage class under
-Wregister too, is quiet about the uses in register int var __asm ("...")
(global and local), though strangely doesn't complain about register
keywords on function arguments, which this patch diagnoses.

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

2016-09-29  Jakub Jelinek  

Implement P0001R1 - C++17 removal of register storage class specifier
c-family/
* c.opt (Wregister): New warning.
* c-opts.c (c_common_post_options): Enable -Wregister by
default for C++17.
cp/
* decl.c (cp_finish_decl): Diagnose register storage class
on vars except when used in GNU global or local register variable
extension.
(grokdeclarator): Diagnose register storage class on parameters.
* except.c (expand_start_catch_block): Set DECL_REGISTER only
after cp_finish_decl call.
testsuite/
* c-c++-common/Wvarargs-2.c (foo1): Except new warning for C++17.
* c-c++-common/vector-subscript-2.c (vf): Expect new error for
C++17.
* c-c++-common/vector-subscript-5.c (foo): Don't use register
keyword if not __SSE2__.
* c-c++-common/Wvarargs.c (foo1, foo3): Expect new warnings for
C++17.
* g++.dg/compat/struct-layout-1_generate.c (iterative_hash): Remove
register keywords.
* g++.dg/eh/pr29166.C: Add -Wno-register option.
* g++.dg/warn/register-parm-1.C (erroneous_warning,
no_erroneous_warning): Expect new warnings for C++17.
* g++.dg/warn/register-var-2.C (f): Likewise.
* g++.dg/parse/register1.C (f): Expect new error for C++17.
* g++.dg/parse/linkage2.C (foo): Likewise.
* g++.dg/torture/pr36826.C (CoinMin, CoinMax): Avoid register
keyword on parameters for C++17.
* g++.dg/cpp1z/register1.C: New test.
* g++.dg/cpp1z/register2.C: New test.
* g++.dg/cpp1z/register3.C: New test.

--- gcc/c-family/c.opt.jj   2016-09-26 12:06:37.0 +0200
+++ gcc/c-family/c.opt  2016-09-29 14:54:17.712047430 +0200
@@ -842,6 +842,10 @@ Wredundant-decls
 C ObjC C++ ObjC++ Var(warn_redundant_decls) Warning
 Warn about multiple declarations of the same object.
 
+Wregister
+C++ ObjC++ Var(warn_register) Warning
+Warn about uses of register storage specifier.
+
 Wreorder
 C++ ObjC++ Var(warn_reorder) Warning LangEnabledBy(C++ ObjC++,Wall)
 Warn when the compiler reorders code.
--- gcc/c-family/c-opts.c.jj2016-08-19 11:04:38.0 +0200
+++ gcc/c-family/c-opts.c   2016-09-29 15:06:06.213254295 +0200
@@ -871,6 +871,10 @@ c_common_post_options (const char **pfil
 warn_shift_negative_value = (extra_warnings
 && (cxx_dialect >= cxx11 || flag_isoc99));
 
+  /* -Wregister is enabled by default in C++17.  */
+  if (!global_options_set.x_warn_register)
+warn_register = cxx_dialect >= cxx1z;
+
   /* Declone C++ 'structors if -Os.  */
   if (flag_declone_ctor_dtor == -1)
 flag_declone_ctor_dtor = optimize_size;
--- gcc/cp/decl.c.jj2016-09-23 19:37:41.0 +0200
+++ gcc/cp/decl.c   2016-09-29 16:05:03.724470275 +0200
@@ -6711,6 +6711,19 @@ cp_finish_decl (tree decl, tree init, bo
   if (type == error_mark_node)
 return;
 
+  /* Warn about register storage specifiers except when in GNU global
+ or local register variable extension.  */
+  if (VAR_P (decl) && DECL_REGISTER (decl) && asmspec_tree == NULL_TREE)
+{
+  if (cxx_dialect >= cxx1z)
+   pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wregister,
+"ISO C++1z does not allow % storage "
+"class specifier");
+  else
+   warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wregister,
+   "% storage class specifier used");
+}
+
   /* If a name was specified, get the string.  */
   if (at_namespace_scope_p ())
 asmspec_tree = maybe_apply_renaming_pragma (decl, asmspec_tree);
@@ -11634,7 +11647,20 @@ grokdeclarator (const cp_declarator *dec
and in case doing stupid register allocation.  */
 
 if (storage_class == sc_register)
-  DECL_REGISTER (decl) = 1;
+  {
+   DECL_REGISTER (decl) = 1;
+   /* Warn about register storage specifiers on PARM_DECLs.  */
+   if (TREE_CODE (decl) == PARM_DECL)
+ {
+   if (cxx_dialect >= cxx1z)
+ pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wregister,
+  "ISO C++1z does not allow % storage "
+  "class specifier");
+ 

[PATCH] Fix -Wimplicit-fallthrough -C, handle some more comment styles and comments in between FALLTHRU comment and label

[Jakub Jelinek]

Hi!

The following patch does a few things:
1) fixes -Wimplicit-fallthrough -C
   (with -C the PREV_FALLTHROUGH flag is on the CPP_COMMENT token, we need
to propagate it to the C/C++ token's flags in the FEs)
2) it accepts a comment in between /* FALLTHRU */ comment and the
   case/default keyword or user label, people often write:
 ...
 /* FALLTHRU */

 /* Rationale or description of what the following code does.  */
 case ...:
   and forcing users to move their comments after the labels or after the
   first label might be too annoying
3) it adds support for some common FALLTHRU comment styles that appeared
   in GCC sources, or in Linux kernel etc., e.g.:

   /*lint -fallthrough */

   /* ... falls through ... */

   /* else fall-through */

   /* Intentional fall through.  */

   /* FALLTHRU - some explanation why.  */
4) it attempts to precisely document what is supported in the documentation
5) some libcpp fixes in the fallthrough_comment_p routine, in particular
   some even previously documented coment styles could be rejected in C++
   style comments if there wasn't any whitespace after them
6) testcase covering various forms

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

2016-09-29  Jakub Jelinek  

* c-lex.c (c_lex_with_flags) : For CPP_COMMENT
token with PREV_FALLTHROUGH, skip all following CPP_PADDING and
CPP_COMMENT tokens and set add_flags to PREV_FALLTHROUGH afterwards.

* doc/invoke.texi (-Wimplicit-fallthrough): Document the accepted
FALLTHRU comment styles.

* lex.c (fallthrough_comment_p): Extend to handle more common FALLTHRU
comment styles.
(_cpp_lex_direct): Allow arbitrary comments in between
fallthrough_comment_p comment and following token.

* c-c++-common/Wimplicit-fallthrough-22.c: New test.
* c-c++-common/Wimplicit-fallthrough-23.c: New test.

--- gcc/c-family/c-lex.c.jj 2016-09-27 09:46:07.0 +0200
+++ gcc/c-family/c-lex.c2016-09-29 12:11:30.633532650 +0200
@@ -598,7 +598,18 @@ c_lex_with_flags (tree *value, location_
 
 /* CPP_COMMENT will appear when compiling with -C and should be
ignored.  */
- case CPP_COMMENT:
+case CPP_COMMENT:
+  if (tok->flags & PREV_FALLTHROUGH)
+   {
+ do
+   {
+ tok = cpp_get_token_with_location (parse_in, loc);
+ type = tok->type;
+   }
+ while (type == CPP_PADDING || type == CPP_COMMENT);
+ add_flags = PREV_FALLTHROUGH;
+ goto retry_after_at;
+   }
goto retry;
 
 default:
--- gcc/doc/invoke.texi.jj  2016-09-27 09:46:07.0 +0200
+++ gcc/doc/invoke.texi 2016-09-29 13:19:41.046697347 +0200
@@ -4156,10 +4156,28 @@ C++17 provides a standard way to suppres
 warning using @code{[[fallthrough]];} instead of the GNU attribute.  In C++11
 or C++14 users can use @code{[[gnu::fallthrough]];}, which is a GNU extension.
 Instead of the these attributes, it is also possible to add a "falls through"
-comment to silence the warning.  GCC accepts a wide range of such comments,
-for example all of "Falls through.", "fallthru", "FALLS-THROUGH" work.  This
-comment needs to consist of two words merely, optionally followed by periods
-or whitespaces.
+comment to silence the warning.  The whole body of the C or C++ style comment
+should match one of the following regular expressions:
+
+@itemize @bullet
+
+@item @code{-fallthrough}
+
+@item @code{@@fallthrough@@}
+
+@item @code{lint -fallthrough ?}
+
+@item @code{[ \t.!]*(ELSE |INTENTIONAL(LY)? )?FALL(S | |-)?THR(OUGH|U)[ 
\t.!]*(-[^\n\r]*)?}
+
+@item @code{[ \t.!]*(Else |Intentional(ly)? )?Fall((s | |-)[Tt]|t)hr(ough|u)[ 
\t.!]*(-[^\n\r]*)?}
+
+@item @code{[ \t.!]*([Ee]lse |[Ii]ntentional(ly)? )?fall(s | |-)?thr(ough|u)[ 
\t.!]*(-[^\n\r]*)?}
+
+@end itemize
+
+and the comment needs to be followed after optional whitespace and other 
comments
+by @code{case} or @code{default} keywords or by a user label that preceeds some
+@code{case} or @code{default} label.
 
 @smallexample
 @group
--- libcpp/lex.c.jj 2016-09-26 12:06:49.0 +0200
+++ libcpp/lex.c2016-09-29 13:54:12.703757398 +0200
@@ -2059,41 +2059,102 @@ fallthrough_comment_p (cpp_reader *pfile
   from += 1 + len;
 }
   /* Whole comment contents (regex):
- [ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*
- [ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*
- [ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*
+ lint -fallthrough ?
+   */
+  else if (*from == 'l')
+{
+  size_t len = sizeof "int -fallthrough" - 1;
+  if ((size_t) (pfile->buffer->cur - from - 1) < len)
+   return false;
+  if (memcmp (from + 1, "int -fallthrough", len))
+return false;
+  from += 1 + len;
+  if (*from == ' ')
+from++;
+}
+  /* Whole comment contents (regex):
+ [ \t.!]*(ELSE |INTENTIONAL(LY)? )?FALL(S | |-)?THR(OUGH|U)[ 

Re: PATCH to add more FALLTHRU markers

[Jakub Jelinek]

On Thu, Sep 29, 2016 at 06:21:13PM +0200, Marek Polacek wrote:
> On Tue, Sep 27, 2016 at 09:58:20PM +0200, Jakub Jelinek wrote:
> > On Tue, Sep 27, 2016 at 09:29:10PM +0200, Florian Weimer wrote:
> > > Not sure if I read this code correctly, but if we fall through from
> > > V32HImode, and we have TARGET_SSE2 set, we execute this code:
> > > 
> > >   tmp = "p";
> > >   ssesuffix = TARGET_AVX512VL ? "q" : "";
> > > 
> > > And not gcc_unreachable (), as is probably intended.
> > 
> > It really doesn't matter.
> > The instruction uses
> > (define_mode_iterator VI12_AVX_AVX512F
> >   [ (V64QI "TARGET_AVX512F") (V32QI "TARGET_AVX") V16QI
> > (V32HI "TARGET_AVX512F") (V16HI "TARGET_AVX") V8HI])
> > iterator (and, after all, ix86_hard_regno_mode_ok should ensure the same),
> > which means V64QI/V32HI will only show up for TARGET_AVX512F, V32QI/V16HI
> > only for TARGET_AVX (which implies TARGET_SSE2), and the slightly
> > nonsensical
> > gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
> > before the switch (the  || TARGET_AVX512VL is pointless, because
> > TARGET_AVX512VL implies TARGET_SSE2 as well as TARGET_AVX2).
> > So, I'd go perhaps for (untested) following patch, first diff -up, followed
> > by diff -upb:
> 
> Looks good, are you going to test/commit it?  Or should I?

Forgot to test it, will do tomorrow.

Jakub

Use version namespace in normal mode

[François Dumont]

Hi

I think _GLIBCXX_BEGIN_NAMESPACE_ALGO should default to 
_GLIBCXX_BEGIN_NAMESPACE_VERSION when parallel mode is not active. Same 
for _GLIBCXX_BEGIN_NAMESPACE_CONTAINER, no ?


* include/bits/c++config (_GLIBCXX_BEGIN_NAMESPACE_ALGO)
(_GLIBCXX_END_NAMESPACE_ALGO): Default to respectively
_GLIBCXX_BEGIN_NAMESPACE_VERSION and _GLIBCXX_END_NAMESPACE_VERSION
when parallel mode is not active.
(_GLIBCXX_BEGIN_NAMESPACE_CONTAINER, _GLIBCXX_END_NAMESPACE_CONTAINER):
Likewise.

Ok to commit after normal check ? Should I rebuild library with 
versioned namespace activated ?


François

diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 656ef78..ebabcd5 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -358,6 +358,10 @@ namespace std
 	 namespace _GLIBCXX_STD_C { _GLIBCXX_BEGIN_NAMESPACE_VERSION
 # define _GLIBCXX_END_NAMESPACE_CONTAINER \
 	 _GLIBCXX_END_NAMESPACE_VERSION }
+#else
+# define _GLIBCXX_STD_C std
+# define _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _GLIBCXX_BEGIN_NAMESPACE_VERSION
+# define _GLIBCXX_END_NAMESPACE_CONTAINER _GLIBCXX_END_NAMESPACE_VERSION
 #endif
 
 #ifdef _GLIBCXX_PARALLEL
@@ -366,30 +370,10 @@ namespace std
 	 namespace _GLIBCXX_STD_A { _GLIBCXX_BEGIN_NAMESPACE_VERSION
 # define _GLIBCXX_END_NAMESPACE_ALGO \
 	 _GLIBCXX_END_NAMESPACE_VERSION }
-#endif
-
-#ifndef _GLIBCXX_STD_A
+#else
 # define _GLIBCXX_STD_A std
-#endif
-
-#ifndef _GLIBCXX_STD_C
-# define _GLIBCXX_STD_C std
-#endif
-
-#ifndef _GLIBCXX_BEGIN_NAMESPACE_ALGO
-# define _GLIBCXX_BEGIN_NAMESPACE_ALGO
-#endif
-
-#ifndef _GLIBCXX_END_NAMESPACE_ALGO
-# define _GLIBCXX_END_NAMESPACE_ALGO
-#endif
-
-#ifndef _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
-# define _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
-#endif
-
-#ifndef _GLIBCXX_END_NAMESPACE_CONTAINER
-# define _GLIBCXX_END_NAMESPACE_CONTAINER
+# define _GLIBCXX_BEGIN_NAMESPACE_ALGO _GLIBCXX_BEGIN_NAMESPACE_VERSION
+# define _GLIBCXX_END_NAMESPACE_ALGO _GLIBCXX_END_NAMESPACE_VERSION
 #endif
 
 // GLIBCXX_ABI Deprecated

Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Bernd Edlinger]

On 09/29/16 20:52, Bernd Edlinger wrote:
> On 09/29/16 20:03, Jason Merrill wrote:
>>
>> What do you think about dropping the TYPE_UNSIGNED exception as well?
>> I don't see what difference that makes.
>>
>
>
> If I drop that exception, then I could also drop the check for
> INTEGER_TYPE and the whole if, because I think other types can not
> happen, but if they are allowed they are as well bogus here.
>
> I can try a bootstrap and see if there are false positives.
>
> But I can do that as well in a follow-up patch, this should probably
> be done step by step, especially when it may trigger some false
> positives.
>
> I think I could also add more stuff, like unary + or - ?
> or maybe also binary +, -, * and / ?
>
> We already discussed making this a multi-level option,
> and maybe enabling the higher level explicitly in the
> boot-strap.
>
> As long as the warning continues to find more bugs than false
> positives, it is probably worth extending it to more cases.
>
> However unsigned integer shift are not undefined if they overflow.
>
> It is possible that this warning will then trigger also on valid
> code that does loop termination with unsigned int left shifting.
> I dont have a real example, but maybe  like this hypothetical C-code:
>
>   unsigned int x=1, bits=0;
>   while (x << bits) bits++;
>   printf("bits=%d\n", bits);
>
>
> Is it OK for everybody to warn for this on -Wall, or maybe only
> when -Wextra or for instance -Wint-in-bool-context=2 is used ?
>
>

Unfortunately, without that exception there is a false positive:

In file included from ../../gcc-trunk/gcc/ada/gcc-interface/decl.c:30:0:
../../gcc-trunk/gcc/ada/gcc-interface/decl.c: In function 'int 
adjust_packed(tree, tree, int)':
../../gcc-trunk/gcc/tree.h:1874:22: error: << on signed integer in 
boolean context [-Werror=int-in-bool-context]
   ? ((unsigned)1) << ((NODE)->type_common.align - 1) : 0)
 ~~^~
../../gcc-trunk/gcc/ada/gcc-interface/decl.c:6928:7: note: in expansion 
of macro 'TYPE_ALIGN'
if (TYPE_ALIGN (record_type)
^~


But that did not happen with this version:

Index: c-common.c
===
--- c-common.c  (revision 240571)
+++ c-common.c  (working copy)
@@ -4655,6 +4655,14 @@ c_common_truthvalue_conversion (location_t locatio
return c_common_truthvalue_conversion (location,
   TREE_OPERAND (expr, 0));

+case LSHIFT_EXPR:
+  /* Warn on signed integer left shift.  */
+  if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE
+ && !TYPE_UNSIGNED (TREE_TYPE (expr)))
+   warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context,
+   "<< on signed integer in boolean context");
+  break;
+
  case COND_EXPR:
if (warn_int_in_bool_context
  && !from_macro_definition_at (EXPR_LOCATION (expr)))


Is that version OK for you?


Bernd.

Re: [PATCH 2/2] [ARC] [libgcc] Fix defines

[Andrew Burgess]

* Claudiu Zissulescu  [2016-07-08 13:41:23 
+0200]:

> Don't use CPU macros, use CPU feature macros.
> 
> libgcc/
> 2016-05-26  Claudiu Zissulescu  
> 
>   * config/arc/lib1funcs.S (__mulsi3): Use feature defines instead
>   of checking for cpus.
>   (__umulsidi3, __umulsi3_highpart, __udivmodsi4, __divsi3)
>   (__modsi3, __clzsi2): Likewise.
> ---
>  libgcc/config/arc/lib1funcs.S | 45 
> +++
>  1 file changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/libgcc/config/arc/lib1funcs.S b/libgcc/config/arc/lib1funcs.S
> index 9bb25e0..422fd95 100644
> --- a/libgcc/config/arc/lib1funcs.S
> +++ b/libgcc/config/arc/lib1funcs.S
> @@ -188,18 +189,18 @@ SYM(__umulsi3_highpart):
>  */
>  #include "ieee-754/arc-ieee-754.h"
>  
> -#ifdef __ARC700__
> +#ifdef __ARC_MPY__
>   mov_s   r12,DBL0L
>   mpyuDBL0L,r12,DBL0H
>   j_s.d   [blink]
> - mpyhu   DBL0H,r12,DBL0H
> + MPYHU   DBL0H,r12,DBL0H

Is there a reason that instruction should be uppercase?

This otherwise looks fine to me.

Thanks,
Andrew

Re: [PATCH 1/2] [ARC] [libgcc] Add support for QuarkSE processor.

[Andrew Burgess]

* Claudiu Zissulescu  [2016-07-08 13:41:22 
+0200]:

> libgcc/
> 2016-05-26  Claudiu Zissulescu  
> 
>   * config/arc/dp-hack.h (ARC_OPTFPE): Define.
>   (__ARC_NORM__): Use instead ARC_OPTFPE.
>   * config/arc/fp-hack.h: Likewise.
>   * config/arc/lib1funcs.S (ARC_OPTFPE): Define.
>   (__ARC_MPY__): Use it insetead of __ARC700__ and __HS__.

There's significant whitespace changes in lib1funcs.S that's not
mentioned in the ChangeLog, and is in parts of the file not touched by
the actual functional changes.

I'd personally prefer to see the whitespace changes pushed as their
own commit as they make (for me) the diff harder to read.

Otherwise this all looks fine.

Thanks,
Andrew



> ---
>  libgcc/config/arc/dp-hack.h   |  12 +++--
>  libgcc/config/arc/fp-hack.h   |   8 +--
>  libgcc/config/arc/lib1funcs.S | 120 
> ++
>  3 files changed, 74 insertions(+), 66 deletions(-)
> 
> diff --git a/libgcc/config/arc/dp-hack.h b/libgcc/config/arc/dp-hack.h
> index 3c727b1..1f7f213 100644
> --- a/libgcc/config/arc/dp-hack.h
> +++ b/libgcc/config/arc/dp-hack.h
> @@ -30,21 +30,23 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If not, see
>  
>  #define FINE_GRAINED_LIBRARIES
>  #define ARC_DP_DEBUG 1
> -#if !defined (__ARC_NORM__) || ARC_DP_DEBUG
> +#define ARC_OPTFPE (defined (__ARC700__) || defined (__ARC_FPX_QUARK__))
> +
> +#if !ARC_OPTFPE || ARC_DP_DEBUG
>  #define L_pack_df
>  #define L_unpack_df
>  #define L_make_df
>  #define L_thenan_df
>  #define L_sf_to_df
>  #endif
> -#ifndef __ARC_NORM__
> +#if !ARC_OPTFPE
>  #define L_addsub_df
>  #elif ARC_DP_DEBUG
>  #define L_addsub_df
>  #define __adddf3 __adddf3_c
>  #define __subdf3 __subdf3_c
>  #endif
> -#ifndef __ARC_NORM__
> +#if !ARC_OPTFPE
>  #define L_mul_df
>  #define L_div_df
>  #elif (!defined (__ARC700__) && !defined (__ARC_MUL64__) \
> @@ -59,7 +61,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If not, see
>  #define L_div_df
>  #define __divdf3 __divdf3_c
>  #endif
> -#ifndef __ARC_NORM__
> +#if !ARC_OPTFPE
>  #define L_df_to_sf
>  #define L_si_to_df
>  #define L_df_to_si
> @@ -77,7 +79,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If not, see
>  #define L_usi_to_df
>  #define __floatunsidf __floatunsidf_c
>  #endif
> -#ifndef __ARC_NORM__
> +#if !ARC_OPTFPE
>  #define L_fpcmp_parts_df
>  #define L_compare_df
>  #define L_eq_df
> diff --git a/libgcc/config/arc/fp-hack.h b/libgcc/config/arc/fp-hack.h
> index 30b547a..5144bb9 100644
> --- a/libgcc/config/arc/fp-hack.h
> +++ b/libgcc/config/arc/fp-hack.h
> @@ -30,13 +30,15 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If not, see
>  
>  #define ARC_FP_DEBUG 1
>  #define FINE_GRAINED_LIBRARIES
> -#if !defined (__ARC_NORM__) || ARC_FP_DEBUG
> +#define ARC_OPTFPE (defined (__ARC700__) || defined (__ARC_FPX_QUARK__))
> +
> +#if !ARC_OPTFPE || ARC_FP_DEBUG
>  #define L_pack_sf
>  #define L_unpack_sf
>  #define L_make_sf
>  #define L_thenan_sf
>  #endif
> -#ifndef __ARC_NORM__
> +#if !ARC_OPTFPE
>  #define L_addsub_sf
>  #define L_mul_sf
>  #define L_div_sf
> @@ -61,7 +63,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If not, see
>  #define L_usi_to_sf
>  #define __floatunsisf __floatunsisf_c
>  #endif
> -#ifndef __ARC_NORM__
> +#if !ARC_OPTFPE
>  #define L_fpcmp_parts_sf
>  #define L_compare_sf
>  #define L_eq_sf
> diff --git a/libgcc/config/arc/lib1funcs.S b/libgcc/config/arc/lib1funcs.S
> index 1c8961c..9bb25e0 100644
> --- a/libgcc/config/arc/lib1funcs.S
> +++ b/libgcc/config/arc/lib1funcs.S
> @@ -32,29 +32,29 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If not, see
> This exception does not however invalidate any other reasons why
> the executable file might be covered by the GNU General Public License.  
> */
>  
> - 
> +
>   /* ANSI concatenation macros.  */
> - 
> +
>   #define CONCAT1(a, b) CONCAT2(a, b)
>   #define CONCAT2(a, b) a ## b
> - 
> +
>   /* Use the right prefix for global labels.  */
> - 
> +
>   #define SYM(x) CONCAT1 (__USER_LABEL_PREFIX__, x)
> - 
> +
>  #ifndef WORKING_ASSEMBLER
>  #define abs_l abs
>  #define asl_l asl
>  #define mov_l mov
>  #endif
> - 
> +
>  #define FUNC(X) .type SYM(X),@function
>  #define HIDDEN_FUNC(X)   FUNC(X)` .hidden X
>  #define ENDFUNC0(X) .Lfe_##X: .size X,.Lfe_##X-X
>  #define ENDFUNC(X)  ENDFUNC0(X)
>  
> - 
> - 
> +
> +
>  #ifdef  L_mulsi3
>   .section .text
>   .align 4
> @@ -64,10 +64,10 @@ SYM(__mulsi3):
>  
>  /* This the simple version.
>  
> -  while (a) 
> +  while (a)
>  {
>if (a & 1)
> -r += b;
> + r += b;
>a >>= 1;
>b <<= 1;
>  }
> @@ -132,7 +132,7 @@ SYM(__mulsi3):
>   add2.cs r0,r0,r1
>   lsr.f r2,r2
>   add3.cs r0,r0,r1
> - bne.d .Loop 
> + bne.d .Loop
>   add3 r1,r3,r1
>   j_s [blink]
>   

Re: [PATCH] Disable compact casesi patterns for arcv2

[Andrew Burgess]

* Claudiu Zissulescu  [2016-09-29 10:41:02 
+0200]:

> Here it is.  The previous version had more mods which should be in a 
> different patch.
> 
> Please let me know if you still have issues with it,
> Claudiu
> 
> gcc/
> 2016-05-09  Claudiu Zissulescu  
> 
>   * common/config/arc/arc-common.c (arc_option_optimization_table):
>   Remove compact casesi option.
>   * config/arc/arc.c (arc_override_options): Use compact casesi
>   option only for pre-ARCv2 cores.
> ---
>  gcc/common/config/arc/arc-common.c | 1 -
>  gcc/config/arc/arc.c   | 7 +++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/common/config/arc/arc-common.c 
> b/gcc/common/config/arc/arc-common.c
> index f5b9c6d..5b687fb 100644
> --- a/gcc/common/config/arc/arc-common.c
> +++ b/gcc/common/config/arc/arc-common.c
> @@ -56,7 +56,6 @@ static const struct default_options 
> arc_option_optimization_table[] =
>  { OPT_LEVELS_ALL, OPT_mbbit_peephole, NULL, 1 },
>  { OPT_LEVELS_SIZE, OPT_mq_class, NULL, 1 },
>  { OPT_LEVELS_SIZE, OPT_mcase_vector_pcrel, NULL, 1 },
> -{ OPT_LEVELS_SIZE, OPT_mcompact_casesi, NULL, 1 },
>  { OPT_LEVELS_NONE, 0, NULL, 0 }
>};
>  
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index 2b25b0b..825bccf 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -858,6 +858,13 @@ arc_override_options (void)
>if (arc_size_opt_level == 3)
>  optimize_size = 1;
>  
> +  /* Compact casesi is not a valid option for ARCv2 family, disable
> + it.  */
> +  if (TARGET_V2)
> +TARGET_COMPACT_CASESI = 0;
> +  else if (optimize_size == 1)
> +TARGET_COMPACT_CASESI = 1;
> +
>if (flag_pic)
>  target_flags |= MASK_NO_SDATA_SET;

I wonder if we should warn for the TARGET_V2 case?  Currently if the
option is supplied on an ARCv2 (-mcompact-casesi) then the option is
silently ignored.  This might confuse some users.

In the non TARGET_V2 case I notice that the option is _always_
enabled, with no option of disabling the option.  If we add a check of
global_options_set then we can make this smarter, default on, but can
still be tuned off if a user ever wants to.  The alternative would be
to entirely remove the TARGET_COMPACT_CASESI flag altogether?

While I was thinking about this I wrote the code below, it probably
needs polishing, but gives an idea of what I have in mind.  What do
you think?

Thanks,
Andrew

---

diff --git a/gcc/common/config/arc/arc-common.c 
b/gcc/common/config/arc/arc-common.c
index f5b9c6d..5b687fb 100644
--- a/gcc/common/config/arc/arc-common.c
+++ b/gcc/common/config/arc/arc-common.c
@@ -56,7 +56,6 @@ static const struct default_options 
arc_option_optimization_table[] =
 { OPT_LEVELS_ALL, OPT_mbbit_peephole, NULL, 1 },
 { OPT_LEVELS_SIZE, OPT_mq_class, NULL, 1 },
 { OPT_LEVELS_SIZE, OPT_mcase_vector_pcrel, NULL, 1 },
-{ OPT_LEVELS_SIZE, OPT_mcompact_casesi, NULL, 1 },
 { OPT_LEVELS_NONE, 0, NULL, 0 }
   };
 
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index 2b25b0b..65a5c10 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -858,6 +858,18 @@ arc_override_options (void)
   if (arc_size_opt_level == 3)
 optimize_size = 1;
 
+  /* Compact casesi is not a valid option for ARCv2 family.  */
+  if (TARGET_V2
+  && global_options_set.x_TARGET_COMPACT_CASESI
+  && global_options.x_TARGET_COMPACT_CASESI)
+{
+  warning (0, "compact-casesi is not applicable to arc-v2");
+  TARGET_COMPACT_CASESI = 0;
+}
+  else if (optimize_size == 1
+  && !global_options_set.x_TARGET_COMPACT_CASESI)
+TARGET_COMPACT_CASESI = 1;
+
   if (flag_pic)
 target_flags |= MASK_NO_SDATA_SET;
 

Re: [PATCH] Fix PR tree-optimization/77654

[Christophe Lyon]

On 23 September 2016 at 17:55, Matthew Fortune
 wrote:
> Doug Gilmore  writes:
>> > From: Richard Biener [rguent...@suse.de]
>> > Sent: Thursday, September 22, 2016 12:43 AM
>> > To: Doug Gilmore
>> > Cc: gcc-patches@gcc.gnu.org; rgue...@gcc.gnu.org
>> > Subject: RE: [PATCH] Fix PR tree-optimization/77654
>> >
>> > On Wed, 21 Sep 2016, Doug Gilmore wrote:
>> >
>> > ...
>> > > Sorry I that missed point.  I glossed your comment "addr_base should
>> > > always be a pointer", causing me to go off into the weeds.
>> > >
>> > > New patch attached.
>> >
>> > Ok if successfully bootstrapped / tested.
>> >
>> > Thanks,
>> > Richard.
>> The change bootstrapped on X86_64 and the several "make check" errors
>> also appeared in latest archived mail message to gcc-testresults.
>
> Committed as r240439.
>

Since this commit, I've noticed ICE on arm target:
FAIL: gcc.dg/params/blocksort-part.c -O3 --param prefetch-latency=0
(internal compiler error)
FAIL: gcc.dg/params/blocksort-part.c -O3 --param prefetch-latency=0
(test for excess errors)
Excess errors:
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/params/blocksort-part.c:116:6:
internal compiler error: in duplicate
_ssa_name_ptr_info, at tree-ssanames.c:630
0xd5a972 duplicate_ssa_name_ptr_info(tree_node*, ptr_info_def*)
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssanames.c:630
0xcac0e0 issue_prefetch_ref
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-loop-prefetch.c:1168
0xcad89f issue_prefetches
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-loop-prefetch.c:1195
0xcad89f loop_prefetch_arrays
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-loop-prefetch.c:1928
0xcae722 tree_ssa_prefetch_arrays()
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-ssa-loop-prefetch.c:1992


--target arm-none-linux-gnueabihf --with-cpu=cortex-a9
--wihth-fpu=neon-fp16 --with-mode=arm

I'm not sure the cpu/fpu/mode settings are mandatory but at least in this case
the compiler ICEs.

Christophe


> (Fixed whitespace/tab issue in the code and incorrect file in changelog)
>
> I can't progress the bug status. Who does that normally?
>
> Matthew

Re: [ARM] Fix new constraints and attributes of SI/HI data movement patterns

[Christophe Lyon]

On 29 September 2016 at 14:45, Kyrill Tkachov
 wrote:
>
> On 29/09/16 09:45, Matthew Wahab wrote:
>>
>> The patch at https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01975.html
>> added constraints and "arch" attributes to some data movement patterns,
>> to fix wrongly generating MOVW instructions when not supported by the
>> targets. This was needed to fix a broken build but also resulted in MOVW
>> instructions not being generated when they should be.
>>
>> This patch fixes the code-gen problems by removing the new attributes
>> from *arm_movsi_vfp and *thumb2_movsi_vfp since they are unnecessary.
>> It changes the rest of the added arch attributes from "t2", which
>> specifies a Thumb-2 target, to the weaker "v6t2" which specifies a
>> target that supports Thumb-2.
>>
>> Tested for arm-none-linux-gnueabihf with native bootstrap and make check
>> on ARMv8-A and for arm-none-eabi with cross-compiled check-gcc on an
>> ARMv8.2-A emulator.
>>
>> There is one unrelated failure in gcc.target/arm/fp16-aapcs-3.c, which
>> is a recently added test for the FP16 ARM alternative format. This has
>> dg-require-effective-target arm_fp16_ok
>> which is true for +fp16 because that implies mfp16-format=ieee. The test
>> should instead be requiring
>> dg-require-effective-target arm_fp16_alternative_ok
>> I'll send a patch to fix this.
>>
>> Ok for trunk?
>
>
> Ok.
> Thanks for the fixup,
> Kyrill
>

I started validations before you committed, and the results
came back after. I noticed no difference.

Thanks

>
>
>> Matthew
>>
>> gcc/
>> 2016-09-29  Matthew Wahab  
>>
>> * config/arm/arm.md (*arm_movsi_insn): Replace "t2" arch attribute
>> with "v6t2".  Move "arch" attribute above "pool_range".
>> * config/arm/vfp.md (*arm_movhi_vfp): Likewise.
>> (*thumb2_movhi_vfp): Likewise.
>> (*arm_movhi_fp16): Likewise.
>> (*thumb2_movhi_fp16): Likewise.
>> (*arm_movsi_vfp): Remove "arch" attribute.
>> (*thumb2_movsi_vfp): Likewise.
>
>

[PATCH, i386 testsuite]: Simplify feature bit tests in feature check headers

[Uros Bizjak]

Hello!

There is no point in writing e.g. "if ((ecx & bit_OSXSAVE) ==
(bit_OSXSAVE))", for single-bit constants. Simplify this to "if (ecx &
bit_OSXSAVE)".

2016-09-29  Uros Bizjak  

* gcc.target/i386/adx-check.h (main): Simplify feature bit tests.
* gcc.target/i386/avx2-check.h (main): Ditto.
* gcc.target/i386/avx512bw-check.h (main): Ditto.
* gcc.target/i386/avx512cd-check.h (main): Ditto.
* gcc.target/i386/avx512dq-check.h (main): Ditto.
* gcc.target/i386/avx512er-check.h (main): Ditto.
* gcc.target/i386/avx512f-check.h (main): Ditto.
* gcc.target/i386/avx512ifma-check.h (main): Ditto.
* gcc.target/i386/avx512vbmi-check.h (main): Ditto.
* gcc.target/i386/avx512vl-check.h (main): Ditto.

Tested on x86_64-linux-gnu {,-m32}  and committed to mainline SVN.

Uros.
Index: gcc.target/i386/adx-check.h
===
--- gcc.target/i386/adx-check.h (revision 240626)
+++ gcc.target/i386/adx-check.h (working copy)
@@ -23,7 +23,7 @@ main ()
 
   __cpuid_count (7, 0, eax, ebx, ecx, edx);
 
-  if ((ebx & bit_ADX) == bit_ADX)
+  if (ebx & bit_ADX)
 {
   do_test ();
 #ifdef DEBUG
Index: gcc.target/i386/avx2-check.h
===
--- gcc.target/i386/avx2-check.h(revision 240626)
+++ gcc.target/i386/avx2-check.h(working copy)
@@ -19,7 +19,7 @@ main ()
 return 0;
 
   /* Run AVX2 test only if host has AVX2 support.  */
-  if ((ecx & bit_OSXSAVE) == (bit_OSXSAVE))
+  if (ecx & bit_OSXSAVE)
 {
   if (__get_cpuid_max (0, NULL) < 7)
return 0;
@@ -26,7 +26,7 @@ main ()
 
   __cpuid_count (7, 0, eax, ebx, ecx, edx);
 
-  if ((avx_os_support ()) && ((ebx & bit_AVX2) == bit_AVX2))
+  if ((ebx & bit_AVX2) && avx_os_support ())
{
  do_test ();
 #ifdef DEBUG
Index: gcc.target/i386/avx512bw-check.h
===
--- gcc.target/i386/avx512bw-check.h(revision 240626)
+++ gcc.target/i386/avx512bw-check.h(working copy)
@@ -19,7 +19,7 @@ main ()
 return 0;
 
   /* Run AVX512BW test only if host has AVX512BW support.  */
-  if ((ecx & bit_OSXSAVE) == (bit_OSXSAVE))
+  if (ecx & bit_OSXSAVE)
 {
   if (__get_cpuid_max (0, NULL) < 7)
return 0;
@@ -26,7 +26,7 @@ main ()
 
   __cpuid_count (7, 0, eax, ebx, ecx, edx);
 
-  if ((avx512f_os_support ()) && ((ebx & bit_AVX512BW) == bit_AVX512BW))
+  if ((ebx & bit_AVX512BW) && avx512f_os_support ())
{
  do_test ();
 #ifdef DEBUG
Index: gcc.target/i386/avx512cd-check.h
===
--- gcc.target/i386/avx512cd-check.h(revision 240626)
+++ gcc.target/i386/avx512cd-check.h(working copy)
@@ -18,7 +18,7 @@ main ()
   if (!__get_cpuid (1, , , , ))
 return 0;
 
-  if ((ecx & bit_OSXSAVE) == (bit_OSXSAVE))
+  if (ecx & bit_OSXSAVE)
 {
   if (__get_cpuid_max (0, NULL) < 7)
return 0;
@@ -25,7 +25,7 @@ main ()
 
   __cpuid_count (7, 0, eax, ebx, ecx, edx);
 
-  if ((avx512f_os_support ()) && ((ebx & (bit_AVX512CD)) == 
(bit_AVX512CD)))
+  if ((ebx & bit_AVX512CD) && avx512f_os_support ())
{
  do_test ();
 #ifdef DEBUG
Index: gcc.target/i386/avx512dq-check.h
===
--- gcc.target/i386/avx512dq-check.h(revision 240626)
+++ gcc.target/i386/avx512dq-check.h(working copy)
@@ -19,7 +19,7 @@ main ()
 return 0;
 
   /* Run AVX512DQ test only if host has AVX512DQ support.  */
-  if ((ecx & bit_OSXSAVE) == (bit_OSXSAVE))
+  if (ecx & bit_OSXSAVE)
 {
   if (__get_cpuid_max (0, NULL) < 7)
return 0;
@@ -26,7 +26,7 @@ main ()
 
   __cpuid_count (7, 0, eax, ebx, ecx, edx);
 
-  if ((avx512f_os_support ()) && ((ebx & bit_AVX512DQ) == bit_AVX512DQ))
+  if ((ebx & bit_AVX512DQ) && avx512f_os_support ())
{
  do_test ();
 #ifdef DEBUG
Index: gcc.target/i386/avx512er-check.h
===
--- gcc.target/i386/avx512er-check.h(revision 240626)
+++ gcc.target/i386/avx512er-check.h(working copy)
@@ -18,7 +18,7 @@ main ()
   if (!__get_cpuid (1, , , , ))
 return 0;
 
-  if ((ecx & bit_OSXSAVE) == (bit_OSXSAVE))
+  if (ecx & bit_OSXSAVE)
 {
   if (__get_cpuid_max (0, NULL) < 7)
return 0;
@@ -25,7 +25,7 @@ main ()
 
   __cpuid_count (7, 0, eax, ebx, ecx, edx);
 
-  if ((avx512f_os_support ()) && ((ebx & bit_AVX512ER) == bit_AVX512ER))
+  if ((ebx & bit_AVX512ER) && avx512f_os_support ())
{
  do_test ();
 #ifdef DEBUG
Index: gcc.target/i386/avx512f-check.h
===
--- gcc.target/i386/avx512f-check.h (revision 240626)
+++ gcc.target/i386/avx512f-check.h (working copy)
@@ -19,7 +19,7 @@ main ()
 

Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Bernd Edlinger]

On 09/29/16 20:03, Jason Merrill wrote:
> On Wed, Sep 28, 2016 at 12:09 PM, Bernd Edlinger
>  wrote:
>> On 09/28/16 16:41, Jason Merrill wrote:
>>> On Tue, Sep 27, 2016 at 11:10 AM, Bernd Edlinger
>>>  wrote:
 On 09/27/16 16:42, Jason Merrill wrote:
> On Tue, Sep 27, 2016 at 10:28 AM, Bernd Edlinger
>  wrote:
>> On 09/27/16 16:10, Florian Weimer wrote:
>>> * Bernd Edlinger:
>>>
> “0 << 0” is used in a similar context, to create a zero constant for a
> multi-bit subfield of an integer.
>
> This example comes from GDB, in bfd/elf64-alpha.c:
>
> |   insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);
>

 Of course that is not a boolean context, and will not get a warning.

 Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)".

 Maybe 1 and 0 come from macro expansion
>>>
>>> But what's the intent of treating 1 << 0 and 0 << 0 differently in the
>>> patch, then?
>>
>> I am not sure if it was a good idea.
>>
>> I saw, we had code of the form
>> bool flag = 1 << 2;
>>
>> another value LOOKUP_PROTECT is  1 << 0, and
>> bool flag = 1 << 0;
>>
>> would at least not overflow the allowed value range of a boolean.
>
> Assigning a bit mask to a bool variable is still probably not what was
> intended, even if it doesn't change the value.

 That works for me too.
 I can simply remove that exception.
>>>
>>> Sounds good.
>>
>> Great.  Is that an "OK with that change"?
>
> What do you think about dropping the TYPE_UNSIGNED exception as well?
> I don't see what difference that makes.
>


If I drop that exception, then I could also drop the check for
INTEGER_TYPE and the whole if, because I think other types can not
happen, but if they are allowed they are as well bogus here.

I can try a bootstrap and see if there are false positives.

But I can do that as well in a follow-up patch, this should probably
be done step by step, especially when it may trigger some false
positives.

I think I could also add more stuff, like unary + or - ?
or maybe also binary +, -, * and / ?

We already discussed making this a multi-level option,
and maybe enabling the higher level explicitly in the
boot-strap.

As long as the warning continues to find more bugs than false
positives, it is probably worth extending it to more cases.

However unsigned integer shift are not undefined if they overflow.

It is possible that this warning will then trigger also on valid
code that does loop termination with unsigned int left shifting.
I dont have a real example, but maybe  like this hypothetical C-code:

  unsigned int x=1, bits=0;
  while (x << bits) bits++;
  printf("bits=%d\n", bits);


Is it OK for everybody to warn for this on -Wall, or maybe only
when -Wextra or for instance -Wint-in-bool-context=2 is used ?



Bernd.

Re: [PATCH, i386]: Fix PR 77756, _-get_cpuid() returns wrong values for level 7 (extended features)

[Uros Bizjak]

On Wed, Sep 28, 2016 at 11:32 PM, Uros Bizjak  wrote:

> 2016-09-28  Uros Bizjak  

I have reverted my previous patch and committed the following:

2016-09-29  Uros Bizjak  

PR target/77756
* config/i386/cpuid.h (__get_cpuid_count): New.
(__get_cpuid): Rename __level to __leaf.

testsuite/ChangeLog:

2016-09-29  Uros Bizjak  

PR target/77756
* gcc.target/i386/pr77756.c: New test.

Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline, will be backported to release branches.

Uros.
Index: config/i386/cpuid.h
===
--- config/i386/cpuid.h (revision 240626)
+++ config/i386/cpuid.h (working copy)
@@ -229,31 +229,37 @@
   return __eax;
 }
 
-/* Return cpuid data for requested cpuid level, as found in returned
+/* Return cpuid data for requested cpuid leaf, as found in returned
eax, ebx, ecx and edx registers.  The function checks if cpuid is
supported and returns 1 for valid cpuid information or 0 for
-   unsupported cpuid level.  All pointers are required to be non-null.  */
+   unsupported cpuid leaf.  All pointers are required to be non-null.  */
 
 static __inline int
-__get_cpuid (unsigned int __level,
+__get_cpuid (unsigned int __leaf,
 unsigned int *__eax, unsigned int *__ebx,
 unsigned int *__ecx, unsigned int *__edx)
 {
-  unsigned int __ext = __level & 0x8000;
+  unsigned int __ext = __leaf & 0x8000;
 
-  if (__get_cpuid_max (__ext, 0) < __level)
+  if (__get_cpuid_max (__ext, 0) < __leaf)
 return 0;
 
-  if (__ext)
-__cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
-  else
-{
-  if (__level >= 13)
-   __cpuid_count (__level, 1, *__eax, *__ebx, *__ecx, *__edx);
-  else if (__level >= 7)
-   __cpuid_count (__level, 0, *__eax, *__ebx, *__ecx, *__edx);
-  else
-   __cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
-}
+  __cpuid (__leaf, *__eax, *__ebx, *__ecx, *__edx);
   return 1;
 }
+
+/* Same as above, but sub-leaf can be specified.  */
+
+static __inline int
+__get_cpuid_count (unsigned int __leaf, unsigned int __subleaf,
+  unsigned int *__eax, unsigned int *__ebx,
+  unsigned int *__ecx, unsigned int *__edx)
+{
+  unsigned int __ext = __leaf & 0x8000;
+
+  if (__get_cpuid_max (__ext, 0) < __leaf)
+return 0;
+
+  __cpuid_count (__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
+  return 1;
+}
Index: testsuite/gcc.target/i386/pr77756.c
===
--- testsuite/gcc.target/i386/pr77756.c (revision 240626)
+++ testsuite/gcc.target/i386/pr77756.c (working copy)
@@ -11,7 +11,7 @@
 {
   unsigned int eax, ebx, ecx, edx;
 
-  if (!__get_cpuid (7, , , , ))
+  if (!__get_cpuid_count (7, 0, , , , ))
__builtin_abort ();
 
   if (!(ebx & bit_AVX2))

Re: [PATCH] fixincludes: fix fixincludes for MinGW

[Jeff Law]

On 09/22/2016 11:26 PM, Tadek Kijkowski wrote:

The fixincl executable uses system function to call applyfix or to
direcly patch a header file, with parameters enclosed in single
quotes. This problem is that MinGW system function just calls cmd.exe,
which doesn't strip quotes from parameters and completely ignores
quotes for embedding spaces in parameters. The MinGW system function
also doesn't allow for newlines in executed command parameters. As a
result fixincludes doesn't wotk at on when trying to build a cross
compiler with mingw as host.

On MinGW host, this patch adds system_with_shell function, which
transforms command passed to system function is following way:
- Enclose entire command in double quotes
- Prepend shell executable name, taken from environment variable SHELL
- Replace each occurence of newline with '$'\n'' sequence which is
understood by bash and ksh (it is assumed that all newlines are
embedded in command parameters enclosed in single quotes)

2016-09-23 Tadek Kijkowski (tkijkow...@gmail.com)

* fixincl.c: Added system_with_shell for MinGW host.


Index: fixincludes/fixincl.c
===
--- fixincludes/fixincl.c   (revision 240386)
+++ fixincludes/fixincl.c   (working copy)
@@ -170,7 +170,111 @@
   exit (EXIT_SUCCESS);
 }

+#ifdef __MINGW32__

+/* Count number of \c needle character instances in string */
+static int
+count_chars ( char* str, char needle )

Formatting it.  This should be:

count_chars (char* str, char needle)

Note the lack of horizontal whitespace after the open paren or before 
the close paren.  Similarly for system_with_shell declaration below.


Wouldn't this be better named "count_occurrences_of_char" or 
"count_instances_of_char"?






+
+  /* Allocate enough memory to fit newly created command string */
+  cmd_size = strlen (env_shell) + (sizeof (z_shell_start_args) - 1)
+   + strlen (s) + newline_cnt * (sizeof(z_shell_newline) - 1 - 1)
+   + (sizeof (z_shell_end_args) - 1) + 1;
Why use sizeof (foo) - 1 rather than just strlen (foo) here?  Note that 
GCC can compute the string length as a compile time constant, so you're 
not gaining any performance by using sizeof here and strlen seems clearer.





+
+  long_cmd = XNEWVEC (char, cmd_size);
+
+  /* Start with ${SHELL} -c " */
+  strcpy (long_cmd, env_shell);
+  strcat (long_cmd, z_shell_start_args);
+
+  /* End pointer for appending pieces of command while replacing newlines */
+  cmd_endp = long_cmd + strlen(long_cmd);

Formatting nit.  Space between function name and its argument list.



+  nl_scan = s;
+  while (nl_scan != NULL)
+{
+  char* next_nl = strchr (nl_scan, '\n');

Formatting nit.  char *next_nl.



+  if (next_nl)
+{
+  /* Append part of command between newlines */
+  size_t part_len = next_nl - nl_scan;
+  memcpy(cmd_endp, nl_scan, part_len);

Formatting nit, space between function name and its argument list.


+  cmd_endp += part_len;
+
+  /* Append newline replacement */
+  memcpy(cmd_endp, z_shell_newline, sizeof(z_shell_newline));

Smilarly, space between functio nname and its argument list.


+  cmd_endp += sizeof(z_shell_newline) - 1;

Here too.


+
+  free ( (void*) long_cmd);

free (long_cmd);

Can you fix those various (minor) issue and resubmit.  I think it'll be 
OK for the trunk with those changes.


jeff

Re: gcc build problem (i386.c) -- missing declaration

[Louis Krupp]

My target was gfortran.

In any case, someone else fixed this problem.

Louis


  On Thu, 29 Sep 2016 11:10:15 -0700 Jeff Law  wrote  
 > On 09/22/2016 04:52 PM, Louis Krupp wrote: 
 > > As of revision 240383 , i386.c isn't compiling.  The errors are: 
 > > 
 > > In file included from ../../gcc_trunk/gcc/target-def.h:106:0, 
 > >  from ../../gcc_trunk/gcc/config/i386/i386.c:81: 
 > > ./target-hooks-def.h:92:38: error: ‘hook_uint_uintp_false’ was not 
 > > declared in this scope 
 > >  #define TARGET_ASM_ELF_FLAGS_NUMERIC hook_uint_uintp_false 
 > >   ^ 
 > > ./target-hooks-def.h:2205:5: note: in expansion of macro 
 > > ‘TARGET_ASM_ELF_FLAGS_NUMERIC’ 
 > >  TARGET_ASM_ELF_FLAGS_NUMERIC, \ 
 > >  ^~~~ 
 > > ./target-hooks-def.h:1792:5: note: in expansion of macro ‘TARGET_ASM_OUT’ 
 > >  TARGET_ASM_OUT, \ 
 > >  ^~ 
 > > ../../gcc_trunk/gcc/config/i386/i386.c:50811:29: note: in expansion of 
 > > macro ‘TARGET_INITIALIZER’ 
 > >  struct gcc_target targetm = TARGET_INITIALIZER; 
 > > 
 > > The problem seems to be that hook_uint_uintp_false() was added to hooks.c 
 > > but not to hooks.h.  I have things working on my copy with this change: 
 > > 
 > > --- hooks.h (revision 240383) 
 > > +++ hooks.h (working copy) 
 > > @@ -95,6 +95,7 @@ extern tree hook_tree_tree_int_treep_bool_null (tr 
 > > 
 > >  extern unsigned hook_uint_void_0 (void); 
 > >  extern unsigned int hook_uint_mode_0 (machine_mode); 
 > > +extern bool hook_uint_uintp_false (unsigned int, unsigned int *); 
 > > 
 > >  extern bool default_can_output_mi_thunk_no_vcall (const_tree, 
 > > HOST_WIDE_INT, 
 > >   HOST_WIDE_INT, 
 > > const_tree); 
 > > 
 > > If I'm not missing something, and if this is a genuine build problem, and 
 > > if this change looks good, I can commit it unless someone else is in the 
 > > process of doing that.  I just need someone's approval.  (I should add 
 > > that I'm on the Fortran commit-after-approval list, but I'm not on a 
 > > general gcc list as far as I know.) 
 > > 
 > Unfortunately you didn't indicate the target you used.  Just saying  
 > "i386.c" isn't helpful as significant amounts of code in i386.c are  
 > conditionalized based on the target triplet (ie something like  
 > x86_64-pc-linux-gnu or i686-pc-kfreebsd-gnu. 
 >  
 > What specific target were you trying to build? 
 >  
 > Jeff 
 >  
 > 

Re: gcc build problem (i386.c) -- missing declaration

[Jeff Law]

On 09/22/2016 04:52 PM, Louis Krupp wrote:

As of revision 240383 , i386.c isn't compiling.  The errors are:

In file included from ../../gcc_trunk/gcc/target-def.h:106:0,
 from ../../gcc_trunk/gcc/config/i386/i386.c:81:
./target-hooks-def.h:92:38: error: ‘hook_uint_uintp_false’ was not declared in 
this scope
 #define TARGET_ASM_ELF_FLAGS_NUMERIC hook_uint_uintp_false
  ^
./target-hooks-def.h:2205:5: note: in expansion of macro 
‘TARGET_ASM_ELF_FLAGS_NUMERIC’
 TARGET_ASM_ELF_FLAGS_NUMERIC, \
 ^~~~
./target-hooks-def.h:1792:5: note: in expansion of macro ‘TARGET_ASM_OUT’
 TARGET_ASM_OUT, \
 ^~
../../gcc_trunk/gcc/config/i386/i386.c:50811:29: note: in expansion of macro 
‘TARGET_INITIALIZER’
 struct gcc_target targetm = TARGET_INITIALIZER;

The problem seems to be that hook_uint_uintp_false() was added to hooks.c but 
not to hooks.h.  I have things working on my copy with this change:

--- hooks.h (revision 240383)
+++ hooks.h (working copy)
@@ -95,6 +95,7 @@ extern tree hook_tree_tree_int_treep_bool_null (tr

 extern unsigned hook_uint_void_0 (void);
 extern unsigned int hook_uint_mode_0 (machine_mode);
+extern bool hook_uint_uintp_false (unsigned int, unsigned int *);

 extern bool default_can_output_mi_thunk_no_vcall (const_tree, HOST_WIDE_INT,
  HOST_WIDE_INT, const_tree);

If I'm not missing something, and if this is a genuine build problem, and if 
this change looks good, I can commit it unless someone else is in the process 
of doing that.  I just need someone's approval.  (I should add that I'm on the 
Fortran commit-after-approval list, but I'm not on a general gcc list as far as 
I know.)

Unfortunately you didn't indicate the target you used.  Just saying 
"i386.c" isn't helpful as significant amounts of code in i386.c are 
conditionalized based on the target triplet (ie something like 
x86_64-pc-linux-gnu or i686-pc-kfreebsd-gnu.


What specific target were you trying to build?

Jeff

Re: C++ PATCH for c++/68703 (dependent vector length)

[Jason Merrill]

On Wed, Sep 28, 2016 at 2:43 PM, David Edelsohn  wrote:
> Hi, Jason
>
> This patch added testcases g++.dg/ext/vector32.C and vector32a.C, but
> there is no gcc/testsuite/ChangeLog entry.
>
> The testcases are failing on AIX with a strange ICE:
>
> src/gcc/testsuite/g++.dg/ext/vector32.C: In function 'int main()':
> src/gcc/testsuite/g++.dg/ext/vector32.C:18:1: internal compiler error:
> in get, at cgraph.h:395
>
> Do you want me to open a new PR for this?

Just add it to the PR, I think, the fix wasn't complete.

Jason

Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Jason Merrill]

On Wed, Sep 28, 2016 at 12:09 PM, Bernd Edlinger
 wrote:
> On 09/28/16 16:41, Jason Merrill wrote:
>> On Tue, Sep 27, 2016 at 11:10 AM, Bernd Edlinger
>>  wrote:
>>> On 09/27/16 16:42, Jason Merrill wrote:
 On Tue, Sep 27, 2016 at 10:28 AM, Bernd Edlinger
  wrote:
> On 09/27/16 16:10, Florian Weimer wrote:
>> * Bernd Edlinger:
>>
 “0 << 0” is used in a similar context, to create a zero constant for a
 multi-bit subfield of an integer.

 This example comes from GDB, in bfd/elf64-alpha.c:

 |   insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);

>>>
>>> Of course that is not a boolean context, and will not get a warning.
>>>
>>> Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)".
>>>
>>> Maybe 1 and 0 come from macro expansion
>>
>> But what's the intent of treating 1 << 0 and 0 << 0 differently in the
>> patch, then?
>
> I am not sure if it was a good idea.
>
> I saw, we had code of the form
> bool flag = 1 << 2;
>
> another value LOOKUP_PROTECT is  1 << 0, and
> bool flag = 1 << 0;
>
> would at least not overflow the allowed value range of a boolean.

 Assigning a bit mask to a bool variable is still probably not what was
 intended, even if it doesn't change the value.
>>>
>>> That works for me too.
>>> I can simply remove that exception.
>>
>> Sounds good.
>
> Great.  Is that an "OK with that change"?

What do you think about dropping the TYPE_UNSIGNED exception as well?
I don't see what difference that makes.

Jason

Re: [PATCH] Fix PR55152

[Andrew Pinski]

On Thu, Sep 29, 2016 at 8:23 PM, Richard Biener  wrote:
> On Thu, 29 Sep 2016, Richard Biener wrote:
>
>> On Wed, 28 Sep 2016, Joseph Myers wrote:
>>
>> > On Wed, 28 Sep 2016, Richard Biener wrote:
>> >
>> > > Index: gcc/testsuite/gcc.dg/pr55152.c
>> > > ===
>> > > --- gcc/testsuite/gcc.dg/pr55152.c(revision 0)
>> > > +++ gcc/testsuite/gcc.dg/pr55152.c(working copy)
>> > > @@ -0,0 +1,13 @@
>> > > +/* { dg-do compile } */
>> > > +/* { dg-options "-O -ffinite-math-only -fstrict-overflow 
>> > > -fdump-tree-optimized" } */
>> > > +
>> > > +double g (double a)
>> > > +{
>> > > +  return (a>=-a)?a:-a;
>> >
>> > You should need -fno-signed-zeros for this (that is, for the
>> > transformation to MAX_EXPR), not -ffinite-math-only.  For a == -0, that
>> > function should return -0, but abs would produce +0.
>>
>> This means that tree-ssa-phiopt.c has a bug:
>>
>> static bool
>> minmax_replacement (basic_block cond_bb, basic_block middle_bb,
>> edge e0, edge e1, gimple *phi,
>> tree arg0, tree arg1)
>> {
>> ...
>>   /* The optimization may be unsafe due to NaNs.  */
>>   if (HONOR_NANS (type))
>> return false;
>>
>> and it should check HONOR_SIGNED_ZEROS as well.
>>
>> I'll fix that as a followup.
>
> Committed as follows.
>
> Bootstrapped / tested on x86_64-unknown-linux-gnu.
>
> Richard.
>
> 2016-09-29  Richard Biener  
>
> PR middle-end/55152
> * match.pd: Add max(a,-a) -> abs(a) pattern.


Hmm, shouldn't we also do "min(a, -a) -> - abs(a)" ?

Thanks,
Andrew Pinski

> * tree-ssa-phiopt.c (minmax_replacement): Disable for
> HONOR_SIGNED_ZEROS types.
>
> * gcc.dg/pr55152.c: New testcase.
> * gcc.dg/tree-ssa/phi-opt-5.c: Adjust.
>
> Index: gcc/match.pd
> ===
> --- gcc/match.pd(revision 240565)
> +++ gcc/match.pd(working copy)
> @@ -1271,6 +1284,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  (simplify
>   (max:c (min:c @0 @1) @1)
>   @1)
> +/* max(a,-a) -> abs(a).  */
> +(simplify
> + (max:c @0 (negate @0))
> + (if (TREE_CODE (type) != COMPLEX_TYPE
> +  && (! ANY_INTEGRAL_TYPE_P (type)
> + || TYPE_OVERFLOW_UNDEFINED (type)))
> +  (abs @0)))
>  (simplify
>   (min @0 @1)
>   (switch
> Index: gcc/tree-ssa-phiopt.c
> ===
> --- gcc/tree-ssa-phiopt.c   (revision 240565)
> +++ gcc/tree-ssa-phiopt.c   (working copy)
> @@ -1075,7 +1075,7 @@ minmax_replacement (basic_block cond_bb,
>type = TREE_TYPE (PHI_RESULT (phi));
>
>/* The optimization may be unsafe due to NaNs.  */
> -  if (HONOR_NANS (type))
> +  if (HONOR_NANS (type) || HONOR_SIGNED_ZEROS (type))
>  return false;
>
>cond = as_a  (last_stmt (cond_bb));
> Index: gcc/testsuite/gcc.dg/pr55152.c
> ===
> --- gcc/testsuite/gcc.dg/pr55152.c  (revision 0)
> +++ gcc/testsuite/gcc.dg/pr55152.c  (working copy)
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -ffinite-math-only -fno-signed-zeros -fstrict-overflow 
> -fdump-tree-optimized" } */
> +
> +double g (double a)
> +{
> +  return (a>=-a)?a:-a;
> +}
> +int f(int a)
> +{
> +  return (a>=-a)?a:-a;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "ABS_EXPR" 2 "optimized" } } */
> Index: gcc/testsuite/gcc.dg/tree-ssa/phi-opt-5.c
> ===
> --- gcc/testsuite/gcc.dg/tree-ssa/phi-opt-5.c   (revision 240612)
> +++ gcc/testsuite/gcc.dg/tree-ssa/phi-opt-5.c   (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O1 -ffinite-math-only -fdump-tree-phiopt1" } */
> +/* { dg-options "-O1 -ffinite-math-only -fno-signed-zeros 
> -fdump-tree-phiopt1" } */
>
>  float repl1 (float varx)
>  {

Re: PATCH to fix g++.dg/cpp0x/fallthrough2.C

[Jason Merrill]

OK.

On Thu, Sep 29, 2016 at 9:26 AM, Marek Polacek  wrote:
> On Thu, Sep 29, 2016 at 03:20:26PM +0200, Jakub Jelinek wrote:
>> Shouldn't that be { target c++14_down } instead?
>
> Didn't know about that.  Thus:
>
> 2016-09-29  Marek Polacek  
>
> * g++.dg/cpp0x/fallthrough2.C: Use the c++14_down target.
>
> diff --git gcc/testsuite/g++.dg/cpp0x/fallthrough2.C 
> gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
> index 71c4a4f..075885a 100644
> --- gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
> +++ gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
> @@ -11,7 +11,7 @@ f (int i)
>  {
>  case 1:
>bar (1);
> -  [[fallthrough]]; // { dg-warning ".fallthrough. is a C\\+\\+17 
> feature" "" { target { ! c++1z } }  }
> +  [[fallthrough]]; // { dg-warning ".fallthrough. is a C\\+\\+17 
> feature" "" { target { c++14_down } }  }
>  case 3:
>bar (1);
>[[gnu::fallthrough, gnu::fallthrough]]; // { dg-warning ".fallthrough. 
> attribute specified multiple times" }
>
> Marek

Re: [PATCH] Machine-readable RTL dumps: print_rtx_function

[Jeff Law]

On 09/29/2016 11:25 AM, Bernd Schmidt wrote:

On 09/29/2016 07:47 PM, David Malcolm wrote:

This patch adds a new function, print_rtx_function, intended for use
for generating function dumps suitable for parsing by the RTL frontend,
but also intended to be human-readable, and human-authorable.



 (note 1 0 4 (nil) NOTE_INSN_DELETED)
 (note 4 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
 (insn 2 4 3 2 (set (mem/c:SI (plus:DI (reg/f:DI 82
virtual-stack-vars)
 (const_int -4 [0xfffc])) [1 i+0
S4 A32])
 (reg:SI 5 di [ i ])) t.c:2 -1
  (nil))


I think it might be a good idea to get rid of redundant information like
insn numbers for such a dump format. But that can be left for followup
patches.
I would make the same suggestion.  The insn # and backend pattern name 
(if any) should be omitted in machine-readable dump format.  I'm fine 
with that as a follow-up as well.


jeff

Re: [PATCH][RTL ifcvt] Transform (X == CST) ? -CST : Y into (X == CST) ? -X : Y when conditional negation is available

[Jeff Law]

On 09/28/2016 10:33 AM, Kyrill Tkachov wrote:

FWIW, the simpler form of the transformation is already done just
prior to leaving SSA form in tree-ssa-uncprop.c.  So there may not be
a ton of opportunities for the simpler form in the RTL optimizers.



Indeed there weren't that many places, but they were some. They helped
in avoiding materialising the floating point 0.0
in particular in some cases.
Yea, that makes perfect sense.  I'm pretty sure we don't do the 
transformation on any floating point constants.





My only concern here is this transformation isn't significantly
different than the simpler form, which is apparently causing some kind
of combine problem.

I'd like to understand the combine problem better before giving final
approval to this patch.
So it's performance/codesize issue, not a correctness issue.  For some 
reason I thought it was a correctness issue and thus needed 
investigation prior to this patch going forward.







Sure, I'm attaching the ifcvt patch that does the aforementioned simple
transform.
The testcase it regresses is:
int
foo (float a, float b, float c, int d)
{
  return a > 5 && a < 10 ? 6 : 0;
}

As far as I got is:
The relevant pre-combine (aarch64) RTL without the patch is:
(insn 21 19 45 2 (set (reg:SI 87)
(zero_extend:SI (subreg:QI (reg:SI 86) 0))) fbad.c:4 90
{*zero_extendqisi2_aarch64}
 (expr_list:REG_DEAD (reg:SI 86)
(nil)))
(insn 47 45 48 2 (set (reg:CC 66 cc)
(compare:CC (reg:SI 87)
(const_int 0 [0]))) fbad.c:4 392 {cmpsi}
 (expr_list:REG_DEAD (reg:SI 87)
(nil)))
(insn 48 47 30 2 (set (reg:SI 76 [  ])
(if_then_else:SI (ne (reg:CC 66 cc)
(const_int 0 [0]))
(reg:SI 89)
(const_int 0 [0]))) fbad.c:4 444 {*cmovsi_insn}
 (expr_list:REG_DEAD (reg:SI 89)
(expr_list:REG_DEAD (reg:CC 66 cc)
(nil


and with the patch it is:
(insn 21 19 45 2 (set (reg:SI 87)
(zero_extend:SI (subreg:QI (reg:SI 86) 0))) fbad.c:4 90
{*zero_extendqisi2_aarch64}
 (expr_list:REG_DEAD (reg:SI 86)
(nil)))
(insn 46 45 47 2 (set (reg:CC 66 cc)
(compare:CC (reg:SI 87)
(const_int 0 [0]))) fbad.c:4 392 {cmpsi}
 (nil))
(insn 47 46 30 2 (set (reg:SI 76 [  ])
(if_then_else:SI (eq (reg:CC 66 cc)
(const_int 0 [0]))
(reg:SI 87)
(reg:SI 89))) fbad.c:4 444 {*cmovsi_insn}
 (expr_list:REG_DEAD (reg:SI 89)
(expr_list:REG_DEAD (reg:SI 87)
(expr_list:REG_DEAD (reg:CC 66 cc)
(nil)

Combine is trying to combine 21 -> 47 in the first case (21 -> 46 in the
second) and without the
transformation ends up looking into the uses of CC as well, the
IF_THEN_ELSE in insn 48.
But I think it only does that if reg 87 is dead in insn 47.

Yea, I see what you're saying.


In the end what happens is it fails to merge the comparison and the use
of the comparison
and ends up emitting extra code.
So before:
fmovs2, 5.0e+0
fmovs1, 1.0e+1
movw0, 6
fcmpes0, s2
fccmpes0, s1, 0, gt
cselw0, w0, wzr, mi //conditional select
ret

after:
foo:
fmovs2, 5.0e+0
fmovs1, 1.0e+1
movw1, 6
fcmpes0, s2
fccmpes0, s1, 0, gt
csetw0, mi // conditional set
cmpw0, 0
cselw0, w0, w1, eq
ret

I think it has to do with the logic to set and use added_sets_2 in
try_combine
but I got lost trying to debug it, so I sort of attributed it to
"extending live ranges
of registers is bad"
Well, extending the live range restricts what combine can do in some 
cases.  Essentially combine has to preserve the assignment to (reg 87). 
It'll end up trying to match a more complex parallel pattern in that 
case -- which likely fails to be recognized as a valid pattern.


These can sometimes be addressed with 3->2 splitters.

Anyway, since the combine issue is strictly a performance question 
rather than a correctness issue, your patch is OK for the trunk.


Jeff

Re: [PATCH 2/2] Extend -falign-FOO=N to N[,M[,N2[,M2]]]

[Denys Vlasenko]

On 09/29/2016 04:45 PM, Bernd Schmidt wrote:

On 09/28/2016 02:57 PM, Denys Vlasenko wrote:

No change from past behavior: Tested that "-falign-functions" uses
an arch-dependent alignment. Tested that "-O2" uses an
arch-dependent alignment. Tested that "-O2 -falign-functions=N"
uses explicitly given alignment.


I suspect we may want some testcases to cover this as well as the new
functionality. Look for existing ones that can maybe be adapted.


I added one -  testsuite/gcc.target/i386/falign-functions.c



Index: gcc/common/config/i386/i386-common.c
===
--- gcc/common/config/i386/i386-common.c(revision 239860)
+++ gcc/common/config/i386/i386-common.c(working copy)
@@ -998,36 +998,17 @@ ix86_handle_option (struct gcc_options *opts,
 }
   return true;

-
-  /* Comes from final.c -- no real reason to change it.  */
-#define MAX_CODE_ALIGN 16
-
 case OPT_malign_loops_:
   warning_at (loc, 0, "-malign-loops is obsolete, use -falign-loops");
-  if (value > MAX_CODE_ALIGN)
-error_at (loc, "-malign-loops=%d is not between 0 and %d",
-  value, MAX_CODE_ALIGN);
-  else
-opts->x_align_loops = 1 << value;
   return true;


That does seem to be a functional change. I'll defer to Uros.


It would be awkward to translate -malign-loops=%d et al
to comma-separated string format.
Since this warning is there for some 15 years already,
anyone who actually cares should have converted to new options
long ago. With patch, -malign-loops=%d will still emit a warning,
but be ignored. At worst, this would result in not aligning
code as requested via obsolete options.



Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c(revision 239860)
+++ gcc/config/arm/arm.c(working copy)
@@ -2899,9 +2899,10 @@ static GTY(()) tree init_optimize;
 static void
 arm_override_options_after_change_1 (struct gcc_options *opts)
 {
-  if (opts->x_align_functions <= 0)
+  if (opts->x_flag_align_functions && !opts->x_str_align_functions)


Are these conditions really equivalent? It looks like zero was
the default even when no -falign-functions was specified.
 Or is that overriden by init_alignments?


 {
-  if (opts->x_align_loops == 0)
+  /* -falign-foo without argument: supply one */
+  if (opts->x_flag_align_loops && !opts->x_str_align_loops)


Same here.


The execution flow for option parsing is somewhat convoluted, no doubt.

I found it experimentally that these are locations where
default alignment parameters are set when -falign-functions
is given with no arguments (or when it is implied by -O2).



+@gccoptlist{-faggressive-loop-optimizations @gol
+-falign-functions[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol
+-falign-jumps[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol
+-falign-labels[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol
+-falign-loops[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol


I think it would be best not to use the same name for different arguments. 
Maybe call the second set n2, m2 (everywhere).


 @item -falign-functions
 @itemx -falign-functions=@var{n}
+@itemx -falign-functions=@var{n},@var{m}
+@itemx -falign-functions=@var{n},@var{m},@var{n},@var{m}


Three args is also valid, isn't it (here and for the other options)?


Yes.
 

 @item -falign-labels
 @itemx -falign-labels=@var{n}
+@itemx -falign-labels=@var{n},@var{m}
+@itemx -falign-labels=@var{n},@var{m},@var{n},@var{m}
 @opindex falign-labels
+If @var{m} is not specified, it defaults to @var{n}.
 Align all branch targets to a power-of-two boundary, skipping up to
-@var{n} bytes like @option{-falign-functions}.  This option can easily
+@var{m}-1 bytes like @option{-falign-functions}.


Maybe just write "Align all branch targets to a power-of-two boundary. The options 
are exactly as described for @option{-falign-functions}."

Why m-1, by the way, Wouldn't it be simpler to just specify the maximum number 
of bytes skipped?


Two reasons.

(1) Because of the defaults. What M defaults to in -falign-labels=N? It's easy
to remember that missing M defaults to N.
The rule "M defaults to N-1" would be more awkward to remember.

(2) This parameter can be seen not as "maximum number of bytes skipped", but as
"minimum number of bytes available before boundary".

IOW: -falign-functions=N,M is
"align to N so as to ensure that at least M [without -1!] bytes can be fetched
before N-byte boundary".
Example: -falign-functions=16 == -falign-functions=16,16 ==
"align to 16 so as to ensure that at least [or in this case, always] 16 bytes
are available before 16-byte boundary".

This is actually a more useful point of view:
in choosing M, you need to decide what is the typical instruction length.
For example, http://lkml.iu.edu/hypermail/linux/kernel/1505.2/03292.html

> defconfig vmlinux (w/o FRAME_POINTER) has 42141 functions.
> 6923 of them have 1st insn 5 or more bytes long,

Re: [PATCH] Machine-readable RTL dumps: print_rtx_function

[Bernd Schmidt]

On 09/29/2016 07:47 PM, David Malcolm wrote:

This patch adds a new function, print_rtx_function, intended for use
for generating function dumps suitable for parsing by the RTL frontend,
but also intended to be human-readable, and human-authorable.



 (note 1 0 4 (nil) NOTE_INSN_DELETED)
 (note 4 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
 (insn 2 4 3 2 (set (mem/c:SI (plus:DI (reg/f:DI 82 virtual-stack-vars)
 (const_int -4 [0xfffc])) [1 i+0 S4 A32])
 (reg:SI 5 di [ i ])) t.c:2 -1
  (nil))


I think it might be a good idea to get rid of redundant information like 
insn numbers for such a dump format. But that can be left for followup 
patches.



OK for trunk?


OK.


Bernd

Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

[Jeff Law]

On 09/28/2016 10:49 AM, Bernd Schmidt wrote:

On 09/28/2016 06:36 PM, Jeff Law wrote:

A "p" prefix for pseudos might still be a good idea, but there's still
the issue of a real "p0" register name causing confusion.

So how do you think we should deal with distinguishing between the
different registers that may appear in a dump file?


I think the main problem we were trying to solve is making sure we can
make future-proof dumps. So that would argue for allowing h0, v0, p0
syntax in the reader, but not printing them out that way by default.

Correct, I'm mostly concerned with future proofing.

I'm certainly OK with a flag to create dumps in a form that's easier to 
parse, but leaving things as-is by default.





Also, if we don't already, allow hard regs to be specified by name in
the reader, and maybe even require it for virtuals.

Works for me.

jeff

[PATCH] Machine-readable RTL dumps: print_rtx_function

[David Malcolm]

This patch adds a new function, print_rtx_function, intended for use
for generating function dumps suitable for parsing by the RTL frontend,
but also intended to be human-readable, and human-authorable.

I put the function in its own file (rather than print-rtl.c) to avoid
having to recompile all the gen* files etc every time the format is
tweaked.

For the RTL frontend, it's not enough for a dump to simply record
the insn chain: we need to also capture the CFG and various other state.

It's possible to reconstruct the CFG based on jump instructions when we're
in "cfgrtl" mode (https://gcc.gnu.org/wiki/cfglayout_mode) but not when
in "cfglayout" mode - in the latter,  unconditional jumps are represented
purely by edges in the CFG, not via jump instructions.  So it seems best
to directly capture the CFG.  I plan to keep the CFG-reconstruction code,
so that the CFG information can be optional, to make it easier to
write interesting test cases by hand (for cfgrtl passes).

Some other state needs to be captured.  I know that we need to at least
capture crtl->return_rtx.  The reason for this is that in the normal case
of expanding gimple, this is set inside assign_parms via non-trivial
target-specific logic.  We don't want to rerun assign_parms when loading
a dump as it could insert new insns.  In theory we could rerun parts of
this logic, but it seems simplest to have the dump record the decisions
that were made by assign_parms.  (if crtl->return_rtx is not set, then
df subtly changes behavior, e.g on x86_64 "ax" is not recognized as
used at the exit block).

Nothing in the tree actually uses the new dump function; you (currently)
have to call it manually from the debugger.  We could start using this
function in various pass dumps - though that would obviously
run the risk of affecting dump-scanning in the DejaGnu test suite, so
I'm leaving that as followup work.

The new dump format looks a lot like the existing insn chain dumps:
Here's an example dump (from x86_64).  I put the insn chain at the
top, with the other data coming after it, since GCC developers are
used to seeing the insn chain.  The dump uses indentation and
comments to make it easier for humans to grok.

 (function "times_two"
   (insn-chain
 (note 1 0 4 (nil) NOTE_INSN_DELETED)
 (note 4 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
 (insn 2 4 3 2 (set (mem/c:SI (plus:DI (reg/f:DI 82 virtual-stack-vars)
 (const_int -4 [0xfffc])) [1 i+0 S4 A32])
 (reg:SI 5 di [ i ])) t.c:2 -1
  (nil))
 (note 3 2 6 2 NOTE_INSN_FUNCTION_BEG)
 (insn 6 3 7 2 (set (reg:SI 89)
 (mem/c:SI (plus:DI (reg/f:DI 82 virtual-stack-vars)
 (const_int -4 [0xfffc])) [1 i+0 S4 A32])) 
t.c:3 -1
  (nil))
 (insn 7 6 10 2 (parallel [
 (set (reg:SI 87 [ _2 ])
 (ashift:SI (reg:SI 89)
 (const_int 1 [0x1])))
 (clobber (reg:CC 17 flags))
 ]) t.c:3 -1
  (expr_list:REG_EQUAL (ashift:SI (mem/c:SI (plus:DI (reg/f:DI 82 
virtual-stack-vars)
 (const_int -4 [0xfffc])) [1 i+0 S4 
A32])
 (const_int 1 [0x1]))
 (nil)))
 (insn 10 7 14 2 (set (reg:SI 88 [  ])
 (reg:SI 87 [ _2 ])) t.c:3 -1
  (nil))
 (insn 14 10 15 2 (set (reg/i:SI 0 ax)
 (reg:SI 88 [  ])) t.c:4 -1
  (nil))
 (insn 15 14 0 2 (use (reg/i:SI 0 ax)) t.c:4 -1
  (nil))
   ) ;; insn-chain
   (cfg
 (bb 0
   (edge 0 2 (flags 0x1))
 ) ;; bb
 (bb 2
   (edge 2 1 (flags 0x1))
 ) ;; bb
 (bb 1
 ) ;; bb
   ) ;; cfg
   (crtl
 (return_rtx
   (reg/i:SI 0 ax)
 ) ;; return_rtx
   ) ;; crtl
 ) ;; function "times_two"

I anticipate the format gaining extra fields, but I want to nail
down the basic ideas and structure a little before I rewrite the
RTL frontend to use it, hence this patch.

Successfully bootstrapped on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
* Makefile.in (OBJS): Add print-rtl-function.o.
* print-rtl-function.c: New file.
* print-rtl.h (print_rtx_function): New decl.
---
 gcc/Makefile.in  |   1 +
 gcc/print-rtl-function.c | 131 +++
 gcc/print-rtl.h  |   2 +
 3 files changed, 134 insertions(+)
 create mode 100644 gcc/print-rtl-function.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index ff12908..15c48bc 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1405,6 +1405,7 @@ OBJS = \
postreload.o \
predict.o \
print-rtl.o \
+   print-rtl-function.o \
print-tree.o \
profile.o \
real.o \
diff --git a/gcc/print-rtl-function.c b/gcc/print-rtl-function.c
new file mode 100644
index 000..c4b99c0
--- /dev/null
+++ b/gcc/print-rtl-function.c
@@ 

Re: PATCH to add more FALLTHRU markers

[Marek Polacek]

On Tue, Sep 27, 2016 at 07:57:17PM +0100, Richard Sandiford wrote:
> Marek Polacek  writes:
> > Currently Makefile.in contains -Wno-error for several of the insn-* files, 
> > but
> > after further investigation I think with this patch we won't need them 
> > anymore.
> > I'm not removing it until I bootstrap gcc on more arches, though.  
> > Meanwhile,
> > this patch at least makes the code more robust.
> >
> > Richard S., can you look at the genattrtab.c bit?  Thanks,
> 
> genattrab.c part is OK, thanks.

Thanks.  I'm going to commit this part separately.

Marek

Re: PATCH to add more FALLTHRU markers

[Marek Polacek]

On Tue, Sep 27, 2016 at 09:58:20PM +0200, Jakub Jelinek wrote:
> On Tue, Sep 27, 2016 at 09:29:10PM +0200, Florian Weimer wrote:
> > Not sure if I read this code correctly, but if we fall through from
> > V32HImode, and we have TARGET_SSE2 set, we execute this code:
> > 
> >   tmp = "p";
> >   ssesuffix = TARGET_AVX512VL ? "q" : "";
> > 
> > And not gcc_unreachable (), as is probably intended.
> 
> It really doesn't matter.
> The instruction uses
> (define_mode_iterator VI12_AVX_AVX512F
>   [ (V64QI "TARGET_AVX512F") (V32QI "TARGET_AVX") V16QI
> (V32HI "TARGET_AVX512F") (V16HI "TARGET_AVX") V8HI])
> iterator (and, after all, ix86_hard_regno_mode_ok should ensure the same),
> which means V64QI/V32HI will only show up for TARGET_AVX512F, V32QI/V16HI
> only for TARGET_AVX (which implies TARGET_SSE2), and the slightly
> nonsensical
> gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
> before the switch (the  || TARGET_AVX512VL is pointless, because
> TARGET_AVX512VL implies TARGET_SSE2 as well as TARGET_AVX2).
> So, I'd go perhaps for (untested) following patch, first diff -up, followed
> by diff -upb:

Looks good, are you going to test/commit it?  Or should I?

Marek

Fix missing -Wimplicit-fallthrough warning

[Marek Polacek]

Here, a missing -Wimplicit-fallthrough warning was caused by a misplaced
FALLTHROUGH_LABEL_P check.  As it is now, for FALLTHROUGH_LABEL_P we'd
never gotten around to
 1933 /* So that next warn_implicit_fallthrough_r will start 
looking for
 1934a new sequence starting with this label.  */
 1935 gsi_prev (gsi_p);

The fix is to move the check to should_warn_for_implicit_fallthrough.

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

2016-09-29  Marek Polacek  

* gimplify.c (should_warn_for_implicit_fallthrough): Check for
FALLTHROUGH_LABEL_P here...
(warn_implicit_fallthrough_r): ...not here.

* c-c++-common/Wimplicit-fallthrough-22.c: New test.

diff --git gcc/gimplify.c gcc/gimplify.c
index 66bb8be..e077a7e 100644
--- gcc/gimplify.c
+++ gcc/gimplify.c
@@ -1817,6 +1817,10 @@ should_warn_for_implicit_fallthrough 
(gimple_stmt_iterator *gsi_p, tree label)
 {
   gimple_stmt_iterator gsi = *gsi_p;
 
+  /* Don't warn if the label is marked with a "falls through" comment.  */
+  if (FALLTHROUGH_LABEL_P (label))
+return false;
+
   /* Don't warn for a non-case label followed by a statement:
case 0:
 foo ();
@@ -1903,7 +1907,6 @@ warn_implicit_fallthrough_r (gimple_stmt_iterator *gsi_p, 
bool *handled_ops_p,
if (gimple_code (next) == GIMPLE_LABEL
&& gimple_has_location (next)
&& (label = gimple_label_label (as_a  (next)))
-   && !FALLTHROUGH_LABEL_P (label)
&& prev != NULL)
  {
struct label_entry *l;
diff --git gcc/testsuite/c-c++-common/Wimplicit-fallthrough-22.c 
gcc/testsuite/c-c++-common/Wimplicit-fallthrough-22.c
index e69de29..7a81e47 100644
--- gcc/testsuite/c-c++-common/Wimplicit-fallthrough-22.c
+++ gcc/testsuite/c-c++-common/Wimplicit-fallthrough-22.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-Wimplicit-fallthrough" } */
+
+void bar (int);
+
+void
+foo (int i)
+{
+  switch (i)
+{
+case 1:
+  bar (1);
+  /* FALLTHROUGH */
+case 2:
+  bar (2); /* { dg-warning "statement may fall through" } */
+case 3:
+  bar (3); /* { dg-warning "statement may fall through" } */
+case 4:
+  bar (4);
+default:
+  break;
+}
+}

Marek

Fix missing FALLTHRU comments

[Marek Polacek]

My upcoming fix revealed more places that were missing a fall through marker.

Bootstrapped/regtested on x86_64-linux, ppc64-linux, and aarch64-linux-gnu with
my fix, applying to trunk.

2016-09-29  Marek Polacek  

* rtti.c (involves_incomplete_p): Add fall through comment.

* dwarf2out.c (loc_descriptor): Add fall through comment.
(add_const_value_attribute): Likewise.

diff --git gcc/cp/rtti.c gcc/cp/rtti.c
index 75aeb0b..a882761 100644
--- gcc/cp/rtti.c
+++ gcc/cp/rtti.c
@@ -855,7 +855,7 @@ involves_incomplete_p (tree type)
 case UNION_TYPE:
   if (!COMPLETE_TYPE_P (type))
return true;
-
+  /* Fall through.  */
 default:
   /* All other types do not involve incomplete class types.  */
   return false;
diff --git gcc/dwarf2out.c gcc/dwarf2out.c
index 972da16..90e68e2 100644
--- gcc/dwarf2out.c
+++ gcc/dwarf2out.c
@@ -14554,6 +14554,7 @@ loc_descriptor (rtx rtl, machine_mode mode,
 case SYMBOL_REF:
   if (!const_ok_for_output (rtl))
break;
+  /* FALLTHROUGH */
 case LABEL_REF:
   if (mode != VOIDmode && GET_MODE_SIZE (mode) == DWARF2_ADDR_SIZE
  && (dwarf_version >= 4 || !dwarf_strict))
@@ -17201,6 +17202,7 @@ add_const_value_attribute (dw_die_ref die, rtx rtl)
 case SYMBOL_REF:
   if (!const_ok_for_output (rtl))
return false;
+  /* FALLTHROUGH */
 case LABEL_REF:
   if (dwarf_version >= 4 || !dwarf_strict)
goto rtl_addr;

Marek

Re: [PATCH][v4] GIMPLE store merging pass

[Kyrill Tkachov]

On 29/09/16 16:37, Pat Haugen wrote:

On 09/28/2016 10:59 AM, Bill Schmidt wrote:

Bill, could you or someone else with access to Power benchmarking try this 
patch out on some benchmarks

that you usually use? The new pass in this patch is on by default and can be 
turned off by -fno-store-merging
if needed.  Jakub indicated that his last attempt at this work caused 
regressions on powerpc so I'd like to
see if this patch is okay in that regard.

Hi Kyrill,

Thanks for the heads-up!  I will have someone on my team look at this as soon 
as possible.


I tried the patch on SPEC2006 on powerpc64le and the results were neutral, no 
noticeable improvements or degradations.


Thanks for checking.
Much appreciated
Kyrill


-Pat

Re: [PATCH][v4] GIMPLE store merging pass

[Pat Haugen]

On 09/28/2016 10:59 AM, Bill Schmidt wrote:
>> Bill, could you or someone else with access to Power benchmarking try this 
>> patch out on some benchmarks
>> > that you usually use? The new pass in this patch is on by default and can 
>> > be turned off by -fno-store-merging
>> > if needed.  Jakub indicated that his last attempt at this work caused 
>> > regressions on powerpc so I'd like to
>> > see if this patch is okay in that regard.
> Hi Kyrill, 
> 
> Thanks for the heads-up!  I will have someone on my team look at this as soon 
> as possible.
> 

I tried the patch on SPEC2006 on powerpc64le and the results were neutral, no 
noticeable improvements or degradations.

-Pat

Re: Fix LTO_STREAMER_DEBUG build

[Richard Biener]

On September 29, 2016 4:54:36 PM GMT+02:00, Thomas Schwinge 
 wrote:
>Hi!
>
>While working on something else, I found LTO_STREAMER_DEBUG broken.
>Enabling (#define) gcc/lto-stream.h:LTO_STREAMER_DEBUG, some further
>checking is done on the LTO streamer (writer/reader), and some
>additional
>debugging hooks are provided.  In the end, I didn't use this facility
>for
>my debugging, but here is anyway the patch to un-break it.  Not yet
>thoroughly tested, will do that later -- OK for trunk then?

Works for me though I'd expect the checking code to be broken (I foubd it 
hardly useful)

Richard.

>commit b70b2465f9c901ccf783049ab10dc896a5bb0f67
>Author: Thomas Schwinge 
>Date:   Thu Sep 29 14:30:11 2016 +0200
>
>Fix LTO_STREAMER_DEBUG build
>---
> gcc/lto-streamer.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>diff --git gcc/lto-streamer.c gcc/lto-streamer.c
>index bfde1fe..a44a916 100644
>--- gcc/lto-streamer.c
>+++ gcc/lto-streamer.c
>@@ -267,23 +267,23 @@ struct tree_hash_entry
> 
> struct tree_entry_hasher : nofree_ptr_hash 
> {
>-  static inline hashval_t hash (const value_type *);
>-  static inline bool equal (const value_type *, const compare_type *);
>+  static inline hashval_t hash (const tree_hash_entry *);
>+  static inline bool equal (const tree_hash_entry *, const
>tree_hash_entry *);
> };
> 
> inline hashval_t
>-tree_entry_hasher::hash (const value_type *e)
>+tree_entry_hasher::hash (const tree_hash_entry *e)
> {
>   return htab_hash_pointer (e->key);
> }
> 
> inline bool
>-tree_entry_hasher::equal (const value_type *e1, const compare_type
>*e2)
>+tree_entry_hasher::equal (const tree_hash_entry *e1, const
>tree_hash_entry *e2)
> {
>   return (e1->key == e2->key);
> }
> 
>-static hash_table *tree_htab;
>+static hash_table *tree_htab;
> #endif
> 
> /* Initialization common to the LTO reader and writer.  */
>@@ -299,7 +299,7 @@ lto_streamer_init (void)
> streamer_check_handled_ts_structures ();
> 
> #ifdef LTO_STREAMER_DEBUG
>-  tree_htab = new hash_table (31);
>+  tree_htab = new hash_table (31);
> #endif
> }
> 
>
>
>Grüße
> Thomas

Re: [PATCH][v4] GIMPLE store merging pass

[Kyrill Tkachov]

Hi Richard,

Thanks for the detailed comments, I'll be working on addressing them.
Below are answers to some of your questions:

On 29/09/16 11:45, Richard Biener wrote:


+
+  /* If we're inserting a non-bytesized width or not at a byte boundary
+ use an intermediate wide_int to perform the bit-insertion correctly.  */
+  if (sub_byte_op_p
+  || (TREE_CODE (expr) == INTEGER_CST
+ && mode_for_size (bitlen, MODE_INT, 0) == BLKmode))
I wonder when we have BLKmode here ...


+{
+  unsigned int byte_size = last_byte - first_byte + 1;
+
+  /* The functions native_encode_expr/native_interpret_expr uses the
+TYPE_MODE of the type to determine the number of bytes to write/read
+so if we want to process a number of bytes that does not have a
+TYPE_MODE of equal size we need to use a type that has a valid mode
+for it.  */
+
+  machine_mode mode
+   = smallest_mode_for_size (byte_size * BITS_PER_UNIT, MODE_INT);
+  tree dest_int_type
+   = build_nonstandard_integer_type (GET_MODE_BITSIZE (mode), UNSIGNED);
+  byte_size = GET_MODE_SIZE (mode);

... how we ever get non-BLKmode here.


smallest_mode_for_size is guaranteed to never return BLKmode.
It returns a mode that contains at least the requested number of bits.
mode_for_size returns BLKmode if no mode fits the exact number of bits.




+  /* The region from the byte array that we're inserting into.  */
+  tree ptr_wide_int
+   = native_interpret_expr (dest_int_type, ptr + first_byte,
+total_bytes);
+
+  gcc_assert (ptr_wide_int);
+  wide_int dest_wide_int
+   = wi::to_wide (ptr_wide_int, TYPE_PRECISION (dest_int_type));
+  wide_int expr_wide_int
+   = wi::to_wide (tmp_int, byte_size * BITS_PER_UNIT);
+  if (BYTES_BIG_ENDIAN)
+   {
+ unsigned int insert_pos
+   = byte_size * BITS_PER_UNIT - bitlen - (bitpos % BITS_PER_UNIT);
+ dest_wide_int
+   = wi::insert (dest_wide_int, expr_wide_int, insert_pos, bitlen);
+   }
+  else
+   dest_wide_int = wi::insert (dest_wide_int, expr_wide_int,
+   bitpos % BITS_PER_UNIT, bitlen);
+
+  tree res = wide_int_to_tree (dest_int_type, dest_wide_int);
+  native_encode_expr (res, ptr + first_byte, total_bytes, 0);
+

OTOH this whole dance looks as complicated and way more expensive than
using native_encode_expr into a temporary buffern and then a
manually implemented "bit-merging" of it at ptr + first_byte + bitpos.
AFAICS that operation is even endianess agnostic.


I did try implementing that, but it kept blowing up in big-endian.
I found it to be very fiddly. I can try again, but we'll see how it goes...




+
+bool
+pass_store_merging::terminate_and_process_all_chains (basic_block bb)
+{
+  hash_map::iterator iter
+= m_stores.begin ();
+  bool ret = false;
+  for (; iter != m_stores.end (); ++iter)
+ret |= terminate_and_release_chain ((*iter).first);
+
+  if (ret)
+gimple_purge_dead_eh_edges (bb);

Why do you need this?  I don't see how merging stores should effect EH
edges at all -- unless you are removing EH side-effects (ISTR you
are not merging cross-BB).


I was seeing a testsuite failure in the C++ testsuite,
an ICE about EH edges. However, when I rerun the testsuite today
without the gimple_purge_dead_eh_edges call I don't see any fallout...
I have been rebasing the patch against trunk regularly so maybe something
change in the underlying trunk since then...




+
+bool
+imm_store_chain_info::coalesce_immediate_stores ()
+{
+  /* Anything less can't be processed.  */
+  if (m_store_info.length () < 2)
+return false;
+
+  if (dump_file && (dump_flags & TDF_DETAILS))
+fprintf (dump_file, "Attempting to coalesce %u stores in chain.\n",
+m_store_info.length ());
+
+  store_immediate_info *info;
+  unsigned int i;
+
+  /* Order the stores by the bitposition they write to.  */
+  m_store_info.qsort (sort_by_bitpos);
+
+  info = m_store_info[0];
+  merged_store_group *merged_store = new merged_store_group (info);
+  m_merged_store_groups.safe_push (merged_store);
+
+  FOR_EACH_VEC_ELT (m_store_info, i, info)
+{
+  if (dump_file)
+   {
+ fprintf (dump_file, "Store %u:\nbitsize:" HOST_WIDE_INT_PRINT_DEC
+ " bitpos:" HOST_WIDE_INT_PRINT_DEC " val:\n",
+  i, info->bitsize, info->bitpos);
+ print_generic_expr (dump_file, info->val, 0);
+ fprintf (dump_file, "\n\n");
+   }
+
+  if (i == 0)
+   continue;
+
+  /* |---store 1---|
+  |---store 2---|
+   Overlapping stores.  */
+  unsigned HOST_WIDE_INT start = info->bitpos;
+  if (IN_RANGE (start, merged_store->start,
+   merged_store->start + merged_store->width - 1))
+   {
+ merged_store->merge_overlapping (info);
+   

Re: [v3 PATCH] Make optional::reset noexcept, make optional::value work in constant expressions.

[Ville Voutilainen]

On 29 September 2016 at 17:43, Jonathan Wakely  wrote:
>> +int main()
>> +{
>> +  static_assert(test());
>> +}
>
>
> Can this second test use { dg-do compile } instead?

Yes it can, and it should (we shouldn't run what we don't need to run,
the suite is
heavy-weight enough already), well spotted. I'll fix that before committing.

[PATCH v2] add -fprolog-pad=N option to c-family

[Torsten Duwe]

In case anybody missed it, the Linux kernel side to make use
of this has also been finished meanwhile. Of course it can not
be accepted without compiler support; and this feature patch
is much more versatile than just Linux kernel live patching
on a single architecture.

Changes since the previous version
(which in turn was based on Maxim's suggestion):

* Document the feature in *.texi

* Automatically disable IPA-RA, like normal profiling does.
  You never know in advance what the code patched in at run time will do.
  Any optimisation here is potentially wrong.

* record a prolog_nop_pad_size value specified on the command line
  in each function's attributes, so that it survives an LTO pipe.

Signed-off-by: Torsten Duwe 

diff --git a/gcc/attribs.c b/gcc/attribs.c
index 16996e9..a5cf2aa 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -365,6 +365,21 @@ decl_attributes (tree *node, tree attributes, int flags)
   if (!attributes_initialized)
 init_attributes ();
 
+  /* If we're building NOP pads because of a command line arg, note the size
+ for LTO builds, unless the attribute has already been overridden. */
+  if (TREE_CODE (*node) == FUNCTION_DECL && prolog_nop_pad_size > 0)
+{
+  tree pp_attr = lookup_attribute ("prolog_pad", attributes);
+  if (! pp_attr )
+   {
+ tree pp_size = build_int_cstu (integer_type_node, 
prolog_nop_pad_size);
+
+ attributes = tree_cons (get_identifier ("prolog_pad"),
+ tree_cons ( NULL_TREE, pp_size, NULL_TREE),
+ attributes);
+   }
+}
+
   /* If this is a function and the user used #pragma GCC optimize, add the
  options to the attribute((optimize(...))) list.  */
   if (TREE_CODE (*node) == FUNCTION_DECL && current_optimize_pragma)
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index f2846bb..278a99e 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -393,6 +393,7 @@ static tree handle_designated_init_attribute (tree *, tree, 
tree, int, bool *);
 static tree handle_bnd_variable_size_attribute (tree *, tree, tree, int, bool 
*);
 static tree handle_bnd_legacy (tree *, tree, tree, int, bool *);
 static tree handle_bnd_instrument (tree *, tree, tree, int, bool *);
+static tree handle_prolog_pad_attribute (tree *, tree, tree, int, bool *);
 
 static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
 static bool nonnull_check_p (tree, unsigned HOST_WIDE_INT);
@@ -834,6 +835,8 @@ const struct attribute_spec c_common_attribute_table[] =
  handle_bnd_legacy, false },
   { "bnd_instrument", 0, 0, true, false, false,
  handle_bnd_instrument, false },
+  { "prolog_pad",1, 1, false, true, true,
+ handle_prolog_pad_attribute, false },
   { NULL, 0, 0, false, false, false, NULL, false }
 };
 
@@ -9687,6 +9690,14 @@ handle_designated_init_attribute (tree *node, tree name, 
tree, int,
   return NULL_TREE;
 }
 
+static tree
+handle_prolog_pad_attribute (tree *, tree, tree, int,
+bool *)
+{
+  /* Nothing to be done here. */
+  return NULL_TREE;
+}
+
 
 /* Check for valid arguments being passed to a function with FNTYPE.
There are NARGS arguments in the array ARGARRAY.  LOC should be used for
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index fec58bc..0220faa 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -536,6 +536,10 @@ c_common_handle_option (size_t scode, const char *arg, int 
value,
   cpp_opts->ext_numeric_literals = value;
   break;
 
+case OPT_fprolog_pad_:
+  prolog_nop_pad_size = value;
+  break;
+
 case OPT_idirafter:
   add_path (xstrdup (arg), AFTER, 0, true);
   break;
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 88038a0..804c654 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1427,6 +1427,10 @@ fpreprocessed
 C ObjC C++ ObjC++
 Treat the input file as already preprocessed.
 
+fprolog-pad=
+C ObjC C++ ObjC++ RejectNegative Joined UInteger Var(prolog_nop_pad_size) 
Init(0)
+Pad NOPs before each function prolog
+
 ftrack-macro-expansion
 C ObjC C++ ObjC++ JoinedOrMissing RejectNegative UInteger
 ; converted into ftrack-macro-expansion=
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2ed9285..463a5a5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10387,6 +10387,16 @@ of the function name, it is considered to be a match.  
For C99 and C++
 extended identifiers, the function name must be given in UTF-8, not
 using universal character names.
 
+@item -fprolog-pad=N
+@opindex fprolog-pad
+Generate a pad of N nops right at the beginning
+of each function, which can be used to patch in any desired
+instrumentation at run time, provided that the code segment
+is writeable. For run time identification, the starting addresses
+of these 

Re: [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP

[David Edelsohn]

Alan,

This patch broke bootstrap on AIX.  sysv4.opt is not included on AIX.
References to sysv4.opt-specific flags have to be protected in
rs6000.c.

In file included from
/gsa/yktgsa-p4/06/gnu/aix/7.1/power/include/c++/4.8.4/cstddef:42:0,
 from /gsa/yktgsa/home/e/d/edelsohn/install/include/gmp.h:51,
 from /nasfarm/edelsohn/src/src/gcc/system.h:681,
 from /nasfarm/edelsohn/src/src/gcc/config/rs6000/rs6000.c:22:
/nasfarm/edelsohn/src/src/gcc/config/rs6000/rs6000.c:37133:35: error:
'struct gcc_options' has no member named 'x_rs6000_gnu_attr'
 offsetof (struct gcc_options, x_rs6000_gnu_attr),
   ^
/nasfarm/edelsohn/src/src/gcc/config/rs6000/rs6000.c:37134:40: error:
'struct cl_target_option' has no member named 'x_rs6000_gnu_attr'
 offsetof (struct cl_target_option, x_rs6000_gnu_attr), },

Thanks, David

Fix LTO_STREAMER_DEBUG build

[Thomas Schwinge]

Hi!

While working on something else, I found LTO_STREAMER_DEBUG broken.
Enabling (#define) gcc/lto-stream.h:LTO_STREAMER_DEBUG, some further
checking is done on the LTO streamer (writer/reader), and some additional
debugging hooks are provided.  In the end, I didn't use this facility for
my debugging, but here is anyway the patch to un-break it.  Not yet
thoroughly tested, will do that later -- OK for trunk then?

commit b70b2465f9c901ccf783049ab10dc896a5bb0f67
Author: Thomas Schwinge 
Date:   Thu Sep 29 14:30:11 2016 +0200

Fix LTO_STREAMER_DEBUG build
---
 gcc/lto-streamer.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git gcc/lto-streamer.c gcc/lto-streamer.c
index bfde1fe..a44a916 100644
--- gcc/lto-streamer.c
+++ gcc/lto-streamer.c
@@ -267,23 +267,23 @@ struct tree_hash_entry
 
 struct tree_entry_hasher : nofree_ptr_hash 
 {
-  static inline hashval_t hash (const value_type *);
-  static inline bool equal (const value_type *, const compare_type *);
+  static inline hashval_t hash (const tree_hash_entry *);
+  static inline bool equal (const tree_hash_entry *, const tree_hash_entry *);
 };
 
 inline hashval_t
-tree_entry_hasher::hash (const value_type *e)
+tree_entry_hasher::hash (const tree_hash_entry *e)
 {
   return htab_hash_pointer (e->key);
 }
 
 inline bool
-tree_entry_hasher::equal (const value_type *e1, const compare_type *e2)
+tree_entry_hasher::equal (const tree_hash_entry *e1, const tree_hash_entry *e2)
 {
   return (e1->key == e2->key);
 }
 
-static hash_table *tree_htab;
+static hash_table *tree_htab;
 #endif
 
 /* Initialization common to the LTO reader and writer.  */
@@ -299,7 +299,7 @@ lto_streamer_init (void)
 streamer_check_handled_ts_structures ();
 
 #ifdef LTO_STREAMER_DEBUG
-  tree_htab = new hash_table (31);
+  tree_htab = new hash_table (31);
 #endif
 }
 


Grüße
 Thomas

Explicitly list all tree codes in gcc/tree-streamer.c:record_common_node (was: [PR lto/77458] Avoid ICE in offloading with differing _FloatN, _FloatNx types)

[Thomas Schwinge]

Hi Richard!

On Mon, 19 Sep 2016 13:25:01 +0200, Richard Biener  
wrote:
> On Mon, Sep 19, 2016 at 1:19 PM, Thomas Schwinge
>  wrote:
> > On Mon, 19 Sep 2016 10:18:35 +0200, Richard Biener 
> >  wrote:
> >> On Fri, Sep 16, 2016 at 3:32 PM, Thomas Schwinge
> >>  wrote:
> >> > --- gcc/tree-streamer.c
> >> > +++ gcc/tree-streamer.c
> >> > @@ -278,9 +278,23 @@ record_common_node (struct streamer_tree_cache_d 
> >> > *cache, tree node)
> >> >streamer_tree_cache_append (cache, node, cache->nodes.length ());
> >> >
> >> >if (POINTER_TYPE_P (node)
> >> > -  || TREE_CODE (node) == COMPLEX_TYPE
> >> >|| TREE_CODE (node) == ARRAY_TYPE)
> >> >  record_common_node (cache, TREE_TYPE (node));
> >> > +  else if (TREE_CODE (node) == COMPLEX_TYPE)
> >> > [...]
> >> >else if (TREE_CODE (node) == RECORD_TYPE)

> [looks to me we miss handling of vector type components alltogether,
> maybe there are no global vector type trees ...]

Looks like it, yes.  Would a patch like the following be reasonable,
which explicitly lists/handles all expected tree codes, or is something
like that not feasible?  (That's a subset of tree codes I gathered by a
partial run of the GCC testsuite, and libgomp testsuite; not claiming
this is complete.)

commit f28dd9618be8a26c6a75ee089f1755e4e0281106
Author: Thomas Schwinge 
Date:   Thu Sep 29 16:35:19 2016 +0200

Explicitly list all tree codes in gcc/tree-streamer.c:record_common_node
---
 gcc/tree-streamer.c | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git gcc/tree-streamer.c gcc/tree-streamer.c
index 6ada89a..8567a81 100644
--- gcc/tree-streamer.c
+++ gcc/tree-streamer.c
@@ -303,17 +303,32 @@ record_common_node (struct streamer_tree_cache_d *cache, 
tree node)
  in the cache as hash value.  */
   streamer_tree_cache_append (cache, node, cache->nodes.length ());
 
-  if (POINTER_TYPE_P (node)
-  || TREE_CODE (node) == ARRAY_TYPE)
-record_common_node (cache, TREE_TYPE (node));
-  else if (TREE_CODE (node) == COMPLEX_TYPE)
+  switch (TREE_CODE (node))
 {
+case ERROR_MARK:
+case FIELD_DECL:
+case FIXED_POINT_TYPE:
+case IDENTIFIER_NODE:
+case INTEGER_CST:
+case INTEGER_TYPE:
+case POINTER_BOUNDS_TYPE:
+case REAL_TYPE:
+case TREE_LIST:
+case VOID_CST:
+case VOID_TYPE:
+  /* No recursion.  */
+  break;
+case POINTER_TYPE:
+case REFERENCE_TYPE:
+case ARRAY_TYPE:
+  record_common_node (cache, TREE_TYPE (node));
+  break;
+case COMPLEX_TYPE:
   /* Verify that a complex type's component type (node_type) has been
 handled already (and we thus don't need to recurse here).  */
   verify_common_node_recorded (cache, TREE_TYPE (node));
-}
-  else if (TREE_CODE (node) == RECORD_TYPE)
-{
+  break;
+case RECORD_TYPE:
   /* The FIELD_DECLs of structures should be shared, so that every
 COMPONENT_REF uses the same tree node when referencing a field.
 Pointer equality between FIELD_DECLs is used by the alias
@@ -322,6 +337,9 @@ record_common_node (struct streamer_tree_cache_d *cache, 
tree node)
 nonoverlapping_component_refs_of_decl_p).  */
   for (tree f = TYPE_FIELDS (node); f; f = TREE_CHAIN (f))
record_common_node (cache, f);
+  break;
+default:
+  gcc_unreachable ();
 }
 }
 


Grüße
 Thomas

Re: [PATCH 2/2] Extend -falign-FOO=N to N[,M[,N2[,M2]]]

[Bernd Schmidt]

On 09/28/2016 02:57 PM, Denys Vlasenko wrote:


This change makes it possible to align functions to 64-byte boundaries *if*
this does not introduce huge amount of padding.


I'm still somewhat undecided, but it seems like a decent enough way to 
expose these possibilities to the user, so unless someone objects, I 
think I'll approve the final version. But first, there are a few more 
things to address (you might want to read the patch submission 
guidelines on our web pages).



Testing:


We require that every patch is bootstrapped and the regression testsuite 
run on at least one primary target. Since you're modifying several 
targets it would be good to at least build them as well. Patch 
submission mails need to state that such testing happened and on which 
targets.



Tested that with -falign-functions=N (tried 8, 15, 16, 17...) the alignment
directives are the same before and after the patch.
Tested that -falign-functions=N,N (two equal parameters) works exactly
like -falign-functions=N.

No change from past behavior:
Tested that "-falign-functions" uses an arch-dependent alignment.
Tested that "-O2" uses an arch-dependent alignment.
Tested that "-O2 -falign-functions=N" uses explicitly given alignment.


I suspect we may want some testcases to cover this as well as the new 
functionality. Look for existing ones that can maybe be adapted.



Index: gcc/common/config/i386/i386-common.c
===
--- gcc/common/config/i386/i386-common.c(revision 239860)
+++ gcc/common/config/i386/i386-common.c(working copy)
@@ -998,36 +998,17 @@ ix86_handle_option (struct gcc_options *opts,
}
   return true;

-
-  /* Comes from final.c -- no real reason to change it.  */
-#define MAX_CODE_ALIGN 16
-
 case OPT_malign_loops_:
   warning_at (loc, 0, "-malign-loops is obsolete, use -falign-loops");
-  if (value > MAX_CODE_ALIGN)
-   error_at (loc, "-malign-loops=%d is not between 0 and %d",
- value, MAX_CODE_ALIGN);
-  else
-   opts->x_align_loops = 1 << value;
   return true;


That does seem to be a functional change. I'll defer to Uros.


Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c(revision 239860)
+++ gcc/config/arm/arm.c(working copy)
@@ -2899,9 +2899,10 @@ static GTY(()) tree init_optimize;
 static void
 arm_override_options_after_change_1 (struct gcc_options *opts)
 {
-  if (opts->x_align_functions <= 0)
+  if (opts->x_flag_align_functions && !opts->x_str_align_functions)


Are these conditions really equivalent? It looks like zero was the 
default even when no -falign-functions was specified. Or is that 
overriden by init_alignments?



 {
-  if (opts->x_align_loops == 0)
+  /* -falign-foo without argument: supply one */
+  if (opts->x_flag_align_loops && !opts->x_str_align_loops)


Same here.


+@gccoptlist{-faggressive-loop-optimizations @gol
+-falign-functions[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol
+-falign-jumps[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol
+-falign-labels[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol
+-falign-loops[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol


I think it would be best not to use the same name for different 
arguments. Maybe call the second set n2, m2 (everywhere).



 @item -falign-functions
 @itemx -falign-functions=@var{n}
+@itemx -falign-functions=@var{n},@var{m}
+@itemx -falign-functions=@var{n},@var{m},@var{n},@var{m}


Three args is also valid, isn't it (here and for the other options)?


+If @var{m} is not specified, it defaults to @var{n}.


I'd move these notes after the point where M is explained, in all places 
where they occur.



 @item -falign-labels
 @itemx -falign-labels=@var{n}
+@itemx -falign-labels=@var{n},@var{m}
+@itemx -falign-labels=@var{n},@var{m},@var{n},@var{m}
 @opindex falign-labels
+If @var{m} is not specified, it defaults to @var{n}.
 Align all branch targets to a power-of-two boundary, skipping up to
-@var{n} bytes like @option{-falign-functions}.  This option can easily
+@var{m}-1 bytes like @option{-falign-functions}.


Maybe just write "Align all branch targets to a power-of-two boundary. 
The options are exactly as described for @option{-falign-functions}."


Why m-1, by the way, Wouldn't it be simpler to just specify the maximum 
number of bytes skipped?



===
--- gcc/toplev.c(revision 239860)
+++ gcc/toplev.c(working copy)
@@ -1177,29 +1177,78 @@ target_supports_section_anchors_p (void)
   return true;
 }

-/* Default the align_* variables to 1 if they're still unset, and
-   set up the align_*_log variables.  */
+static const char *
+read_uint (const char *flag, const char *name, int *np)


Every function should have a comment in front of it that documents the 
arguments and the return value (argument names in caps). This is as 

Re: [v3 PATCH] Make optional::reset noexcept, make optional::value work in constant expressions.

[Jonathan Wakely]

On 29/09/16 17:22 +0300, Ville Voutilainen wrote:

--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/observers/6.cc
@@ -0,0 +1,39 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do run }
+
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a moved_to of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+#include 
+#include 
+
+struct Y
+{
+  constexpr int test() & {return 7;}
+};
+
+constexpr int
+test()
+{
+  std::optional opt{Y{}};
+  return opt.value().test();
+}
+
+int main()
+{
+  static_assert(test());
+}


Can this second test use { dg-do compile } instead?

OK for trunk either way.

[PATCH, Fortran] PR fortran/77764 - ICE in is_anonymous_component

[Fritz Reese]

ICE in [1] is due to failure to null-guard map components in
gfc_compare_union_types. Attached is [obvious] fix - will commit soon
without complaints.

---
Fritz Reese

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77764

2016-09-29  Fritz Reese  

Fix ICE for maps with zero components.

PR fortran/77764
* gcc/fortran/interface.c (gfc_compare_union_types): Null-guard map
components.

PR fortran/77764
* gcc/testsuite/gfortran.dg/dec_union_8.f90: New testcase.
diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index 9d4e5e9..55923a3 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -526,6 +526,7 @@ int
 gfc_compare_union_types (gfc_symbol *un1, gfc_symbol *un2)
 {
   gfc_component *map1, *map2, *cmp1, *cmp2;
+  gfc_symbol *map1_t, *map2_t;
 
   if (un1->attr.flavor != FL_UNION || un2->attr.flavor != FL_UNION)
 return 0;
@@ -541,16 +542,26 @@ gfc_compare_union_types (gfc_symbol *un1, gfc_symbol *un2)
  we compare the maps sequentially. */
   for (;;)
   {
-cmp1 = map1->ts.u.derived->components;
-cmp2 = map2->ts.u.derived->components;
+map1_t = map1->ts.u.derived;
+map2_t = map2->ts.u.derived;
+
+cmp1 = map1_t->components;
+cmp2 = map2_t->components;
+
+/* Protect against null components.  */
+if (map1_t->attr.zero_comp != map2_t->attr.zero_comp)
+  return 0;
+
+if (map1_t->attr.zero_comp)
+  return 1;
+
 for (;;)
 {
   /* No two fields will ever point to the same map type unless they are
  the same component, because one map field is created with its type
  declaration. Therefore don't worry about recursion here. */
   /* TODO: worry about recursion into parent types of the unions? */
-  if (compare_components (cmp1, cmp2,
-map1->ts.u.derived, map2->ts.u.derived) == 0)
+  if (compare_components (cmp1, cmp2, map1_t, map2_t) == 0)
 return 0;
 
   cmp1 = cmp1->next;
diff --git a/gcc/testsuite/gfortran.dg/dec_union_8.f90 b/gcc/testsuite/gfortran.dg/dec_union_8.f90
new file mode 100644
index 000..2d856fc
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/dec_union_8.f90
@@ -0,0 +1,21 @@
+! { dg-do compile }
+! { dg-options "-fdec-structure" }
+!
+! PR fortran/77764
+!
+! Test an ICE due to a map with zero components.
+!
+
+program p
+
+structure /s1/
+  union
+map
+end map
+map
+  real :: a = 2.0
+end map
+  end union
+end structure
+
+end

[v3 PATCH] Make optional::reset noexcept, make optional::value work in constant expressions.

[Ville Voutilainen]

These tidbits were reported by Eric Fiselier.
They are fairly simple conformance issues.

Tested on Linux-x64.

2016-09-29  Ville Voutilainen  

Make optional::reset noexcept, make optional::value
work in constant expressions.
* include/std/optional (_M_get): Make constexpr.
(reset): Make noexcept.
* testsuite/20_util/optional/assignment/7.cc: New.
* testsuite/20_util/optional/observers/6.cc: New.
diff --git a/libstdc++-v3/include/std/optional 
b/libstdc++-v3/include/std/optional
index b14faf1..21210ab 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -378,7 +378,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   constexpr bool _M_is_engaged() const noexcept
   { return this->_M_engaged; }
 
-  _Tp&
+  constexpr _Tp&
   _M_get() noexcept
   { return _M_payload; }
 
@@ -777,7 +777,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
? std::move(this->_M_get())
: static_cast<_Tp>(std::forward<_Up>(__u));
}
-  void reset() { this->_M_reset(); }
+  void reset() noexcept { this->_M_reset(); }
 };
 
   template
diff --git a/libstdc++-v3/testsuite/20_util/optional/assignment/7.cc 
b/libstdc++-v3/testsuite/20_util/optional/assignment/7.cc
new file mode 100644
index 000..d392b40
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/assignment/7.cc
@@ -0,0 +1,31 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do run }
+
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a moved_to of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+#include 
+#include 
+
+int main()
+{
+  std::optional o{666};
+  VERIFY(o && *o == 666);
+  o.reset();
+  VERIFY(!o);
+  static_assert(noexcept(std::declval().reset()));
+}
diff --git a/libstdc++-v3/testsuite/20_util/optional/observers/6.cc 
b/libstdc++-v3/testsuite/20_util/optional/observers/6.cc
new file mode 100644
index 000..f156a66
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/observers/6.cc
@@ -0,0 +1,39 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do run }
+
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a moved_to of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+#include 
+#include 
+
+struct Y
+{
+  constexpr int test() & {return 7;}
+};
+
+constexpr int
+test()
+{
+  std::optional opt{Y{}};
+  return opt.value().test();
+}
+
+int main()
+{
+  static_assert(test());
+}

[PATCH, Fortran] PR fortran/77782 - ICE in gfc_get_union_type

[Fritz Reese]

ICE in [1] is due to an incomplete fix for PR fortran/77327 (r239819,
see [2],[3]). Specifically in interface.c (gfc_compare_derived_types)
I overlooked the case where FL_UNION type symbols could be compared as
equal to FL_STRUCTURE type symbols, which is _never_ correct. The
faulty logic causes a UNION to be considered equal to a STRUCTURE,
thus the union receives the structure's backend declaration. Obviously
everything goes haywire from there.

Attached is the [obvious] fix. Will commit to trunk soon, barring any
concerns from others.

---
Fritz Reese

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77782
[2] https://gcc.gnu.org/ml/fortran/2016-08/msg00144.html
[3] https://gcc.gnu.org/ml/fortran/2016-08/msg00145.html

2016-09-29  Fritz Reese  

Fix ICE caused by union types comparing equal to structure types.

PR fortran/77782
* gcc/fortran/interface.c (gfc_compare_derived_types): Use
gfc_compare_union_types to compare union types.

PR fortran/77782
* gcc/testsuite/gfortran.dg/dec_structure_16.f90: New testcase.
diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index 9a19fa7..9d4e5e9 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -589,6 +589,10 @@ gfc_compare_derived_types (gfc_symbol *derived1, gfc_symbol *derived2)
 
   gcc_assert (derived1 && derived2);
 
+  /* Compare UNION types specially.  */
+  if (derived1->attr.flavor == FL_UNION || derived2->attr.flavor == FL_UNION)
+return gfc_compare_union_types (derived1, derived2);
+
   /* Special case for comparing derived types across namespaces.  If the
  true names and module names are the same and the module name is
  nonnull, then they are equal.  */
diff --git a/gcc/testsuite/gfortran.dg/dec_structure_16.f90 b/gcc/testsuite/gfortran.dg/dec_structure_16.f90
new file mode 100644
index 000..f9b2671
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/dec_structure_16.f90
@@ -0,0 +1,35 @@
+! { dg-do compile }
+! { dg-options "-fdec-structure" }
+!
+! PR fortran/77782
+!
+! Test an ICE where a union might be considered equal to a structure,
+! causing the union's backend_decl to be replaced with that of the structure.
+!
+
+program p
+
+structure /s1/
+  union
+map
+  integer(4) a
+  end map
+map
+  real(4) b
+end map
+  end union
+end structure
+
+structure /s2/
+  union ! regression: if this union == s1, we ICE in gfc_get_union_type
+map
+  integer(2) x, y
+  integer(4) z
+end map
+  end union
+end structure
+
+record /s1/ r1
+r1.a = 0
+
+end

Re: [PATCH, RFC] gcov: dump in a static dtor instead of in an atexit handler

[Martin Liška]

On 09/29/2016 03:00 PM, Nathan Sidwell wrote:
> On 09/29/16 08:54, Nathan Sidwell wrote:
>> On 09/29/16 08:49, Martin Liška wrote:
>>> Ideally we should have a macro for each target telling whether it supports
>>> priorities or not.
>>> However, we probably don't have? I would suggest to make the test 
>>> conditional
>>> just for main
>>> targets which support priorities?
>>
>> or a dg_effective_target test.  Probably overkill if there's exactly one 
>> target
>> impacted.
> 
> already there : effective_target_init_priority

Nice that we have it. Looks it's going to be very first usage of the effective 
target.
I'm suggesting patch which also changes debugging messages to make it more 
readable.

Ready for trunk?
Thanks,
Martin
>From cd66f6f9b9c68267698720dcf350b140d86f4201 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 29 Sep 2016 15:39:08 +0200
Subject: [PATCH] Use effective target for pr16855

gcc/testsuite/ChangeLog:

2016-09-29  Martin Liska  

	* g++.dg/gcov/pr16855.C: Add init_priority as an effective
	target.
---
 gcc/testsuite/g++.dg/gcov/pr16855.C | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/gcc/testsuite/g++.dg/gcov/pr16855.C b/gcc/testsuite/g++.dg/gcov/pr16855.C
index 91801d4..8167176 100644
--- a/gcc/testsuite/g++.dg/gcov/pr16855.C
+++ b/gcc/testsuite/g++.dg/gcov/pr16855.C
@@ -1,13 +1,15 @@
 /* { dg-options "-fprofile-arcs -ftest-coverage" } */
 /* { dg-do run { target native } } */
+/* { dg-require-effective-target init_priority  } */
 
 #include 
+#include 
 
 int a;
 
 void foo()
 {
-  a = 123;		  /* count(1) */
+  fprintf (stderr, "atexit handler foo()\n"); 		  /* count(1) */
 }
 
 #include 
@@ -15,10 +17,10 @@ using namespace std;
 class Test {
 public:
 	Test(void){
-	cout<< "In Test ctor" << endl;			  /* count(1) */
+	fprintf (stderr, "In Test ctor\n");		  /* count(1) */
 	}
 	~Test(void){
-	cout<< "In Test dtor" << endl;			  /* count(1) */
+	fprintf (stderr, "In Test dtor\n");		  /* count(1) */
 	}
 }T1;
 
@@ -27,8 +29,7 @@ void uncalled(void){
 }
 int main(void){
 atexit ();
-// Test T2;
-cout<< "In main" << endl;  /* count(1) */
+fprintf (stderr, "In main\n");  /* count(1) */
 return 0;
 }
 
@@ -36,12 +37,12 @@ return 0;
 
 __attribute__((constructor))
 static void construct_navigationBarImages() {
-  fprintf (stderr,  "((construct_navigationBarImages))"); /* count(1) */
+  fprintf (stderr,  "((construct_navigationBarImages))\n"); /* count(1) */
 }
 
 __attribute__((destructor))
 static void destroy_navigationBarImages() {
-  fprintf (stderr,  "((destroy_navigationBarImages))");	  /* count(1) */
+  fprintf (stderr,  "((destroy_navigationBarImages))\n");  /* count(1) */
 }
 
 /* { dg-final { run-gcov branches { -b pr16855.C } } } */
-- 
2.9.2

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Jonathan Wakely]

On 29/09/16 14:37 +0100, Andre Vieira (lists) wrote:

Hi Jonathan,

On 27/09/16 16:11, Jonathan Wakely wrote:


The test might not be very good, but tests some small integer values
and some other values where accuracy is lost for one or other of the
alternative implementations mentioned above. If this FAILs for some
32-bit targets we might need to adjust the tolerances or the
dg-options.


On arm-none-eabi I'm seeing a failure for the long double type and inputs:
{ 1e-2l, 1e-4l, 1e-4l, 0.0150004999375l }

The abs(frac) is higher than the toler: 1.73455e-16 vs 1e-16. Is that a
reasonable difference? Should we raise toler3 to 1e-15?

The last line is also too high:
 { 2147483647.l, 2147483647.l, 2147483647.l, 3719550785.027307813987l }
Yields a frac of: 1.28198e-16

Those are the only ones that pass the 1e-16 threshold.


Yes, it makes sense to raise it, although Ed's reworking the code and
tests anyway.

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Andre Vieira (lists)]

Hi Jonathan,

On 27/09/16 16:11, Jonathan Wakely wrote:
> 
> The test might not be very good, but tests some small integer values
> and some other values where accuracy is lost for one or other of the
> alternative implementations mentioned above. If this FAILs for some
> 32-bit targets we might need to adjust the tolerances or the
> dg-options.

On arm-none-eabi I'm seeing a failure for the long double type and inputs:
{ 1e-2l, 1e-4l, 1e-4l, 0.0150004999375l }

The abs(frac) is higher than the toler: 1.73455e-16 vs 1e-16. Is that a
reasonable difference? Should we raise toler3 to 1e-15?

The last line is also too high:
  { 2147483647.l, 2147483647.l, 2147483647.l, 3719550785.027307813987l }
Yields a frac of: 1.28198e-16

Those are the only ones that pass the 1e-16 threshold.

Cheers,
Andre

Re: PATCH to fix g++.dg/cpp0x/fallthrough2.C

[Marek Polacek]

On Thu, Sep 29, 2016 at 03:20:26PM +0200, Jakub Jelinek wrote:
> Shouldn't that be { target c++14_down } instead?

Didn't know about that.  Thus:

2016-09-29  Marek Polacek  

* g++.dg/cpp0x/fallthrough2.C: Use the c++14_down target.

diff --git gcc/testsuite/g++.dg/cpp0x/fallthrough2.C 
gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
index 71c4a4f..075885a 100644
--- gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
+++ gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
@@ -11,7 +11,7 @@ f (int i)
 {
 case 1:
   bar (1);
-  [[fallthrough]]; // { dg-warning ".fallthrough. is a C\\+\\+17 feature" 
"" { target { ! c++1z } }  }
+  [[fallthrough]]; // { dg-warning ".fallthrough. is a C\\+\\+17 feature" 
"" { target { c++14_down } }  }
 case 3:
   bar (1);
   [[gnu::fallthrough, gnu::fallthrough]]; // { dg-warning ".fallthrough. 
attribute specified multiple times" }

Marek

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Jonathan Wakely]

On 29/09/16 13:12 +0100, Jonathan Wakely wrote:

On 29/09/16 14:02 +0200, Rainer Orth wrote:

Hi Jonathan,


If only there was some way the Solaris team could contact us so we
could coordinate and stop adding more and more hacks to mess with each
others headers. But I assume they don't have access to the www or
email, because the only other explanation is too rude to say on a
public list.


Presumably they now provide all the declarations for C++11, so we
don't need to declare them. Moving the new hypot overload outside the


still this doesn't need into , but into libstdc++ configury
IMNSHO.


I agree. Presumably they wanted their updated headers to work with
existing GCC releases, and not have to wait until we modified our
configury to cooperate with their changes.

But if they simply communicated with us we could plan for that kind of
thing in advance, and ensure we don't keep trying to fix each others
headers from a distance.


_GLIBCXX_USE_C99_MATH_TR1 block should mean it's declared. That will
break when Solaris adds C++17 support, so we'd better add some macro
to guard the new hypot overloads, so they can be disabled again. Let's
call it __CORRECT_ISO_CPP17_MATH_H_PROTO (and maybe add a coment like
"Dear Solaris team, ...").


... which assumes they do read this ;-)


Indeed.

Since they seem to be adding declarations to  and only in the
global namespace, and the 3-argument form doesn't exist in the global
namespace, maybe we can assume they aren't going to add it to
.



Does this work?


It does indeed, at least running the single testcase with runtest now
passes.


Thanks, I'll commit that soon.


Herre's what I committed, without a CORRECT_ISO_CPP17_MATH_H_PROTO
macro check (which I'd put in the wrong place anyway :-)

Tested powerpc64le-linux, committed to trunk.


commit e253b8464a024a062dc01f0eaad66b10e3132374
Author: Jonathan Wakely 
Date:   Thu Sep 29 13:41:55 2016 +0100

Define C++17 std::hypot without _GLIBCXX_USE_C99_MATH_TR1

	* include/c_global/cmath (hypot, __hypot3): Move C++17 overloads
	outside _GLIBCXX_USE_C99_MATH_TR1 condition.

diff --git a/libstdc++-v3/include/c_global/cmath b/libstdc++-v3/include/c_global/cmath
index fffa0e7..0e7c4ad 100644
--- a/libstdc++-v3/include/c_global/cmath
+++ b/libstdc++-v3/include/c_global/cmath
@@ -1455,46 +1455,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   return hypot(__type(__x), __type(__y));
 }
 
-#if __cplusplus > 201402L
-#define __cpp_lib_hypot 201603
-  // [c.math.hypot3], three-dimensional hypotenuse
-
-  template
-inline _Tp
-__hypot3(_Tp __x, _Tp __y, _Tp __z)
-{
-  __x = std::abs(__x);
-  __y = std::abs(__y);
-  __z = std::abs(__z);
-  if (_Tp __a = __x < __y ? __y < __z ? __z : __y : __x < __z ? __z : __x)
-	return __a * std::sqrt((__x / __a) * (__x / __a)
-			   + (__y / __a) * (__y / __a)
-			   + (__z / __a) * (__z / __a));
-  else
-	return {};
-}
-
-  inline float
-  hypot(float __x, float __y, float __z)
-  { return std::__hypot3(__x, __y, __z); }
-
-  inline double
-  hypot(double __x, double __y, double __z)
-  { return std::__hypot3(__x, __y, __z); }
-
-  inline long double
-  hypot(long double __x, long double __y, long double __z)
-  { return std::__hypot3(__x, __y, __z); }
-
-  template
-typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type
-hypot(_Tp __x, _Up __y, _Vp __z)
-{
-  using __type = typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type;
-  return std::__hypot3<__type>(__x, __y, __z);
-}
-#endif // C++17
-
 #ifndef __CORRECT_ISO_CPP11_MATH_H_PROTO
   constexpr int
   ilogb(float __x)
@@ -1830,6 +1790,53 @@ _GLIBCXX_END_NAMESPACE_VERSION
 
 #endif // C++11
 
+#if __cplusplus > 201402L
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  // [c.math.hypot3], three-dimensional hypotenuse
+#define __cpp_lib_hypot 201603
+
+  template
+inline _Tp
+__hypot3(_Tp __x, _Tp __y, _Tp __z)
+{
+  __x = std::abs(__x);
+  __y = std::abs(__y);
+  __z = std::abs(__z);
+  if (_Tp __a = __x < __y ? __y < __z ? __z : __y : __x < __z ? __z : __x)
+	return __a * std::sqrt((__x / __a) * (__x / __a)
+			   + (__y / __a) * (__y / __a)
+			   + (__z / __a) * (__z / __a));
+  else
+	return {};
+}
+
+  inline float
+  hypot(float __x, float __y, float __z)
+  { return std::__hypot3(__x, __y, __z); }
+
+  inline double
+  hypot(double __x, double __y, double __z)
+  { return std::__hypot3(__x, __y, __z); }
+
+  inline long double
+  hypot(long double __x, long double __y, long double __z)
+  { return std::__hypot3(__x, __y, __z); }
+
+  template
+typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type
+hypot(_Tp __x, _Up __y, _Vp __z)
+{
+  using __type = typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type;
+  return std::__hypot3<__type>(__x, __y, __z);
+}
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace
+#endif // C++17
+
+
 #if 

Re: PATCH to fix g++.dg/cpp0x/fallthrough2.C

[Jakub Jelinek]

On Thu, Sep 29, 2016 at 03:13:57PM +0200, Marek Polacek wrote:
> This test failed with make check-c++1z because in C++1z we don't get the
> expected message.
> 
> Bootstrapped/regtested on x86_64-linux, applying to trunk.
> 
> 2016-09-29  Marek Polacek  
> 
>   * g++.dg/cpp0x/fallthrough2.C: Only expect the warning in C++11 and
>   C++14.
> 
> diff --git gcc/testsuite/g++.dg/cpp0x/fallthrough2.C 
> gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
> index b6964e1..71c4a4f 100644
> --- gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
> +++ gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
> @@ -11,7 +11,7 @@ f (int i)
>  {
>  case 1:
>bar (1);
> -  [[fallthrough]]; // { dg-warning ".fallthrough. is a C\\+\\+17 
> feature" }
> +  [[fallthrough]]; // { dg-warning ".fallthrough. is a C\\+\\+17 
> feature" "" { target { ! c++1z } }  }

Shouldn't that be { target c++14_down } instead?

Jakub

Re: [PR lto/77458] Avoid ICE in offloading with differing _FloatN, _FloatNx types

[Thomas Schwinge]

Hi Richard!

On Mon, 19 Sep 2016 13:25:01 +0200, Richard Biener  
wrote:
> On Mon, Sep 19, 2016 at 1:19 PM, Thomas Schwinge
>  wrote:
> > On Mon, 19 Sep 2016 10:18:35 +0200, Richard Biener 
> >  wrote:
> >> On Fri, Sep 16, 2016 at 3:32 PM, Thomas Schwinge
> >>  wrote:
> >> > --- gcc/tree-streamer.c
> >> > +++ gcc/tree-streamer.c
> >> > @@ -278,9 +278,23 @@ record_common_node (struct streamer_tree_cache_d 
> >> > *cache, tree node)
> >> >streamer_tree_cache_append (cache, node, cache->nodes.length ());
> >> >
> >> >if (POINTER_TYPE_P (node)
> >> > -  || TREE_CODE (node) == COMPLEX_TYPE
> >> >|| TREE_CODE (node) == ARRAY_TYPE)
> >> >  record_common_node (cache, TREE_TYPE (node));
> >> > +  else if (TREE_CODE (node) == COMPLEX_TYPE)
> >> > +{
> >> > +  /* Assert that complex types' component types have already been 
> >> > handled
> >> > +(and we thus don't need to recurse here).  See PR lto/77458.  */
> >> > +[...]

> >> So I very much like to go forward with this kind of change as well

> > [patch]

> Ok with [changes]

Like this?  (I'll then continue to replicate this for other tree codes.)

commit b304f88f7226d93c8e13584a916cc713a989a635
Author: Thomas Schwinge 
Date:   Wed Sep 28 12:36:59 2016 +0200

[PR lto/77458] Avoid ICE in offloading with differing _FloatN, _FloatNx 
types

gcc/
PR lto/77458
* tree-core.h (enum tree_index): Put the complex types after their
component types.
* tree-streamer.c (verify_common_node_recorded): New function.
(preload_common_nodes) : Use it.
---
 gcc/tree-core.h | 31 +--
 gcc/tree-streamer.c | 33 -
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git gcc/tree-core.h gcc/tree-core.h
index 353a625..35b67c9 100644
--- gcc/tree-core.h
+++ gcc/tree-core.h
@@ -553,20 +553,6 @@ enum tree_index {
   TI_BOOLEAN_FALSE,
   TI_BOOLEAN_TRUE,
 
-  TI_COMPLEX_INTEGER_TYPE,
-  TI_COMPLEX_FLOAT_TYPE,
-  TI_COMPLEX_DOUBLE_TYPE,
-  TI_COMPLEX_LONG_DOUBLE_TYPE,
-
-  TI_COMPLEX_FLOAT16_TYPE,
-  TI_COMPLEX_FLOATN_NX_TYPE_FIRST = TI_COMPLEX_FLOAT16_TYPE,
-  TI_COMPLEX_FLOAT32_TYPE,
-  TI_COMPLEX_FLOAT64_TYPE,
-  TI_COMPLEX_FLOAT128_TYPE,
-  TI_COMPLEX_FLOAT32X_TYPE,
-  TI_COMPLEX_FLOAT64X_TYPE,
-  TI_COMPLEX_FLOAT128X_TYPE,
-
   TI_FLOAT_TYPE,
   TI_DOUBLE_TYPE,
   TI_LONG_DOUBLE_TYPE,
@@ -596,6 +582,23 @@ enum tree_index {
 - TI_FLOATN_NX_TYPE_FIRST  \
 + 1)
 
+  /* Put the complex types after their component types, so that in (sequential)
+ tree streaming we can assert that their component types have already been
+ handled (see tree-streamer.c:record_common_node).  */
+  TI_COMPLEX_INTEGER_TYPE,
+  TI_COMPLEX_FLOAT_TYPE,
+  TI_COMPLEX_DOUBLE_TYPE,
+  TI_COMPLEX_LONG_DOUBLE_TYPE,
+
+  TI_COMPLEX_FLOAT16_TYPE,
+  TI_COMPLEX_FLOATN_NX_TYPE_FIRST = TI_COMPLEX_FLOAT16_TYPE,
+  TI_COMPLEX_FLOAT32_TYPE,
+  TI_COMPLEX_FLOAT64_TYPE,
+  TI_COMPLEX_FLOAT128_TYPE,
+  TI_COMPLEX_FLOAT32X_TYPE,
+  TI_COMPLEX_FLOAT64X_TYPE,
+  TI_COMPLEX_FLOAT128X_TYPE,
+
   TI_FLOAT_PTR_TYPE,
   TI_DOUBLE_PTR_TYPE,
   TI_LONG_DOUBLE_PTR_TYPE,
diff --git gcc/tree-streamer.c gcc/tree-streamer.c
index 7ea7096..6ada89a 100644
--- gcc/tree-streamer.c
+++ gcc/tree-streamer.c
@@ -248,6 +248,32 @@ streamer_tree_cache_lookup (struct streamer_tree_cache_d 
*cache, tree t,
 }
 
 
+/* Verify that NODE is in CACHE.  */
+
+static void
+verify_common_node_recorded (struct streamer_tree_cache_d *cache, tree node)
+{
+  /* Restrict this to flag_checking only because in general violating it is
+ harmless plus we never know what happens on all targets/frontend/flag(!)
+ combinations.  */
+  if (!flag_checking)
+return;
+
+  bool found = false;
+  if (cache->node_map)
+gcc_assert (streamer_tree_cache_lookup (cache, node, NULL));
+  else
+{
+  gcc_assert (cache->nodes.exists ());
+  /* Linear search...  */
+  for (unsigned i = 0; !found && i < cache->nodes.length (); ++i)
+   if (cache->nodes[i] == node)
+ found = true;
+  gcc_assert (found);
+}
+}
+
+
 /* Record NODE in CACHE.  */
 
 static void
@@ -278,9 +304,14 @@ record_common_node (struct streamer_tree_cache_d *cache, 
tree node)
   streamer_tree_cache_append (cache, node, cache->nodes.length ());
 
   if (POINTER_TYPE_P (node)
-  || TREE_CODE (node) == COMPLEX_TYPE
   || TREE_CODE (node) == ARRAY_TYPE)
 record_common_node (cache, TREE_TYPE (node));
+  else if (TREE_CODE (node) == COMPLEX_TYPE)
+{
+  /* Verify that a complex type's component type (node_type) has been
+handled already (and we thus don't need to recurse here).  */
+  verify_common_node_recorded (cache, TREE_TYPE (node));
+}
   else if (TREE_CODE (node) == RECORD_TYPE)
   

Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Dominique d'Humières]

I have bootstrapped revision r240599 with the patch 
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01315.html without any problem. 
However compiling the following Fortran tests

FAIL: gfortran.dg/binding_label_tests_16.f03  -g  (internal compiler error)
FAIL: gfortran.dg/module_commons_3.f90-g  (internal compiler error)
FAIL: gfortran.dg/module_equivalence_1.f90-g  (internal compiler error)
FAIL: gfortran.dg/use_11.f90  -g  (internal compiler error)
FAIL: gfortran.dg/use_only_1.f90  -g  (internal compiler error)
FAIL: gfortran.dg/widechar_5.f90  -g  (internal compiler error)

FAIL: libgomp.fortran/udr15.f90   -g  (internal compiler error)

are giving an ICE with -g of the kind

internal compiler error: in dwarf2out_imported_module_or_decl, at 
dwarf2out.c:24070

corresponding to

  gcc_assert (scope_die->die_child);

The Ada test gnat.dg/debug7.adb is also failing with

FAIL: gnat.dg/debug7.adb (test for excess errors)
Excess errors:
gnat1: incorrect object file extension

Dominique

> Le 28 sept. 2016 à 17:44, Jason Merrill  a écrit :
> 
> On Thu, Aug 18, 2016 at 5:33 AM, Pierre-Marie de Rodat
>  wrote:
>> A check in dwarf2out_imported_module_or_decl prevents valid strict
>> DWARF2 constructs such as DW_TAG_imported_declaration from being emitted
>> in dwarf2out_imported_module_or_decl_1.
>> 
>> The latter already protects the emission of newer DWARF tags with
>> appropriate checks, so the one in the former is redundant and
>> pessimistic.  This function is already called from places like
>> process_scope_var, which are not protected anyway.
>> 
>> This patch removes the check in dwarf2out_imported_module_or_decl so
>> that tags like DW_TAG_imported_declaration are emitted even in strict
>> DWARF2 mode.
>> 
>> Bootstrapped and regtested on x86_64-linux, no regression.
> 
> This check was added for bug 41405, a Darwin bootstrap problem.
> Dominique, can you verify that the compiler still bootstraps with this
> change?  OK if so.
> 
> Jason

PATCH to fix g++.dg/cpp0x/fallthrough2.C

[Marek Polacek]

This test failed with make check-c++1z because in C++1z we don't get the
expected message.

Bootstrapped/regtested on x86_64-linux, applying to trunk.

2016-09-29  Marek Polacek  

* g++.dg/cpp0x/fallthrough2.C: Only expect the warning in C++11 and
C++14.

diff --git gcc/testsuite/g++.dg/cpp0x/fallthrough2.C 
gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
index b6964e1..71c4a4f 100644
--- gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
+++ gcc/testsuite/g++.dg/cpp0x/fallthrough2.C
@@ -11,7 +11,7 @@ f (int i)
 {
 case 1:
   bar (1);
-  [[fallthrough]]; // { dg-warning ".fallthrough. is a C\\+\\+17 feature" }
+  [[fallthrough]]; // { dg-warning ".fallthrough. is a C\\+\\+17 feature" 
"" { target { ! c++1z } }  }
 case 3:
   bar (1);
   [[gnu::fallthrough, gnu::fallthrough]]; // { dg-warning ".fallthrough. 
attribute specified multiple times" }

Marek

Re: [PATCH, RFC] gcov: dump in a static dtor instead of in an atexit handler

[Nathan Sidwell]

On 09/29/16 08:54, Nathan Sidwell wrote:

On 09/29/16 08:49, Martin Liška wrote:

Ideally we should have a macro for each target telling whether it supports
priorities or not.
However, we probably don't have? I would suggest to make the test conditional
just for main
targets which support priorities?


or a dg_effective_target test.  Probably overkill if there's exactly one target
impacted.


already there : effective_target_init_priority

Re: [PATCH, RFC] gcov: dump in a static dtor instead of in an atexit handler

[Nathan Sidwell]

On 09/29/16 08:49, Martin Liška wrote:

Ideally we should have a macro for each target telling whether it supports 
priorities or not.
However, we probably don't have? I would suggest to make the test conditional 
just for main
targets which support priorities?


or a dg_effective_target test.  Probably overkill if there's exactly one target 
impacted.


nathan

Re: [PATCH] print-rtx.c: add 'h', v' and 'p' prefixes to regnos

[David Malcolm]

On Wed, 2016-09-28 at 18:49 +0200, Bernd Schmidt wrote:
> On 09/28/2016 06:36 PM, Jeff Law wrote:
> > > A "p" prefix for pseudos might still be a good idea, but there's
> > > still
> > > the issue of a real "p0" register name causing confusion.
> > So how do you think we should deal with distinguishing between the
> > different registers that may appear in a dump file?
> 
> I think the main problem we were trying to solve is making sure we
> can 
> make future-proof dumps. So that would argue for allowing h0, v0, p0 
> syntax in the reader, but not printing them out that way by default.
> 
> Also, if we don't already, allow hard regs to be specified by name in
> the reader, and maybe even require it for virtuals.

Yes: the problems I'm trying to solve here are:
(a) making dumps more resilient against changing .md files, and
(b) where possible, allowing target-independent dump fragments where
everything is a pseudo

The issue I ran into was parsing the "r" format code, for a regno,
where print_rtx can print various things after the actual number.  My
hope with the prefix patch was to give the parser more of a hint as to
the category of reg (and to perhaps make dumps easier for humans to
read).

But it looks like the "r" format code is only ever used by REG, which
means there's always a closing parenthesis at the end of the material
emitted for the "r" format code.  So given that I *think* that the
parser already has all it needs, and that the patch is redundant.

So my plan for the reader is:
- read the number emitted by "r"
- see if there's a name after the number.  If there is, assume a hard
or virtual reg, and try to parse the name:
  - if the name is recognized, use the target's current number for that
name (future-proofing virtuals against .md changes)
  - if the name is not recognized, issue a fatal error (it's probably a
typo, or maybe a backport from a future version of gcc, or a target
incompatibility)
  - if there's no name, assume it's a pseudo.  Remap all pseudos in a
postprocessing phase to ensure that they *are* pseudos (even if the .md
has gained some hard regs in the meantime), leaving the numbering
untouched if possible (the common case).

...and to drop the regno-prefixing idea from this patch.

Hopefully this sounds sane.

(I'm also working on a function-dumping patch, which will cover CFG
information and various "crtl" information and other state that can't
be reconstructed and hence ought to be in the dump).

Dave

Re: [PATCH, RFC] gcov: dump in a static dtor instead of in an atexit handler

[Martin Liška]

On 09/29/2016 11:00 AM, Rainer Orth wrote:
> Hi Martin,
> 
>> On 08/12/2016 04:08 PM, Martin Liška wrote:
>>> On 08/10/2016 02:53 PM, Nathan Sidwell wrote:
 On 08/10/16 06:43, Martin Liška wrote:
> Hello.
>
> There are multiple PRs (mentioned in ChangeLog) which suffer from
> missing capability of gcov
> to save counters for functions with constructor/destructor
> attributes. I done that by simply
> replacing atexit handler (gcov_exit) with a new static destructor
> (__gcov_exit), which has
> priority 99 (I did the same for __gcov_init). However, I'm not sure
> whether it's possible
> that a ctor defined in a source file can be potentially executed before
> __gcov_init (w/ the default
> priority)?
>
> Patch survives:
> make check -k -j10 RUNTESTFLAGS="gcov.exp"
> make check -k -j10 RUNTESTFLAGS="tree-prof.exp"
>
> I've just also verified that a DSO gcov dump works as before. I'm
> attaching a test-case which
> tests both static ctors/dtors, as well as C++ ctors/dtors.

 Does a coverage bootstrap (--enable-coverage) still succeed?
>>>
>>> Well, looks results are more unstable than I thought.
>>> Even running 'make -j1' in objdir/x86_64-pc-linux-gnu/libgcc repeatedly
>>> generates different results.
>>> I'll dig in after weekend.
>>>
>>> Martin
> [...]
 I think this is a good idea, but we should document the changed
 behavior. (I don't think the current behaviour's documented).
>>
>> I'm adding a new hunk that documents the behavior.
>>
>> Is the patch ready to be installed?
> 
> the testcase FAILs on Solaris 12 (both SPARC and x86):
> 
> +FAIL: g++.dg/gcov/pr16855.C  -std=gnu++11  gcov: 1 failures in line counts, 
> 0 i
> n branch percentages, 0 in return percentages, 0 in intermediate format
> +FAIL: g++.dg/gcov/pr16855.C  -std=gnu++11  line 21: is #:should be 1
> +FAIL: g++.dg/gcov/pr16855.C  -std=gnu++14  gcov: 1 failures in line counts, 
> 0 i
> n branch percentages, 0 in return percentages, 0 in intermediate format
> +FAIL: g++.dg/gcov/pr16855.C  -std=gnu++14  line 21: is #:should be 1
> +FAIL: g++.dg/gcov/pr16855.C  -std=gnu++98  gcov: 1 failures in line counts, 
> 0 i
> n branch percentages, 0 in return percentages, 0 in intermediate format
> +FAIL: g++.dg/gcov/pr16855.C  -std=gnu++98  line 21: is #:should be 1
> 
> I haven't looked closer yet, but notice that you require constructor
> priority support which isn't available everywhere (it is on Solaris 12,
> but not before).
> 
>   Rainer
> 

Hello.

Sorry for the test-breakage. The issue is really connected to fact that current 
trunk relies
on support of dtor priority. The former implementation called the function 
__gcov_exit via atexit.
If I understand correctly, fully support of static ctors/dtors, C++ 
ctors/dtors, with combination
of atexit cannot be done on a target w/o ctor/dtor priorities.

Ideally we should have a macro for each target telling whether it supports 
priorities or not.
However, we probably don't have? I would suggest to make the test conditional 
just for main
targets which support priorities?

Thoughts?

Re: [ARM] Fix new constraints and attributes of SI/HI data movement patterns

[Kyrill Tkachov]

On 29/09/16 09:45, Matthew Wahab wrote:

The patch at https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01975.html
added constraints and "arch" attributes to some data movement patterns,
to fix wrongly generating MOVW instructions when not supported by the
targets. This was needed to fix a broken build but also resulted in MOVW
instructions not being generated when they should be.

This patch fixes the code-gen problems by removing the new attributes
from *arm_movsi_vfp and *thumb2_movsi_vfp since they are unnecessary.
It changes the rest of the added arch attributes from "t2", which
specifies a Thumb-2 target, to the weaker "v6t2" which specifies a
target that supports Thumb-2.

Tested for arm-none-linux-gnueabihf with native bootstrap and make check
on ARMv8-A and for arm-none-eabi with cross-compiled check-gcc on an
ARMv8.2-A emulator.

There is one unrelated failure in gcc.target/arm/fp16-aapcs-3.c, which
is a recently added test for the FP16 ARM alternative format. This has
dg-require-effective-target arm_fp16_ok
which is true for +fp16 because that implies mfp16-format=ieee. The test
should instead be requiring
dg-require-effective-target arm_fp16_alternative_ok
I'll send a patch to fix this.

Ok for trunk?


Ok.
Thanks for the fixup,
Kyrill



Matthew

gcc/
2016-09-29  Matthew Wahab  

* config/arm/arm.md (*arm_movsi_insn): Replace "t2" arch attribute
with "v6t2".  Move "arch" attribute above "pool_range".
* config/arm/vfp.md (*arm_movhi_vfp): Likewise.
(*thumb2_movhi_vfp): Likewise.
(*arm_movhi_fp16): Likewise.
(*thumb2_movhi_fp16): Likewise.
(*arm_movsi_vfp): Remove "arch" attribute.
(*thumb2_movsi_vfp): Likewise.

[PATCH][3/2] Fix PR77768

[Richard Biener]

Another one - this time with the testcase from the other PR.

Bootstrapped / tested on x86_64-unknown-linux-gnu, applied.

Richard.

2016-09-29  Richard Biener  

PR tree-optimization/77768
* tree-ssa-pre.c (eliminate_dom_walker::before_dom_children):
Handle stores to readonly memory when removing redundant stores.

* gcc.dg/torture/pr77768.c: New testcase.

Index: gcc/tree-ssa-pre.c
===
--- gcc/tree-ssa-pre.c  (revision 240609)
+++ gcc/tree-ssa-pre.c  (working copy)
@@ -4443,9 +4443,11 @@ eliminate_dom_walker::before_dom_childre
  && operand_equal_p (val, rhs, 0))
{
  /* We can only remove the later store if the former aliases
-at least all accesses the later one does.  */
+at least all accesses the later one does or if the store
+was to readonly memory storing the same value.  */
  alias_set_type set = get_alias_set (lhs);
- if (vnresult->set == set
+ if (! vnresult
+ || vnresult->set == set
  || alias_set_subset_of (set, vnresult->set))
{
  if (dump_file && (dump_flags & TDF_DETAILS))
Index: gcc/testsuite/gcc.dg/torture/pr77768.c
===
--- gcc/testsuite/gcc.dg/torture/pr77768.c  (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr77768.c  (working copy)
@@ -0,0 +1,13 @@
+/* { dg-do run } */
+
+static const int a;
+int b;
+int *c, *d;
+int main()
+{
+  c = (int *)
+  c == d ?: __builtin_exit(0); 
+  for (; b; b++ >= (*d = a))
+;
+  return 0;
+}

Re: [PATCH] Fix PR55152

[Richard Biener]

On Thu, 29 Sep 2016, Richard Biener wrote:

> On Wed, 28 Sep 2016, Joseph Myers wrote:
> 
> > On Wed, 28 Sep 2016, Richard Biener wrote:
> > 
> > > Index: gcc/testsuite/gcc.dg/pr55152.c
> > > ===
> > > --- gcc/testsuite/gcc.dg/pr55152.c(revision 0)
> > > +++ gcc/testsuite/gcc.dg/pr55152.c(working copy)
> > > @@ -0,0 +1,13 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O -ffinite-math-only -fstrict-overflow 
> > > -fdump-tree-optimized" } */
> > > +
> > > +double g (double a)
> > > +{
> > > +  return (a>=-a)?a:-a;
> > 
> > You should need -fno-signed-zeros for this (that is, for the 
> > transformation to MAX_EXPR), not -ffinite-math-only.  For a == -0, that 
> > function should return -0, but abs would produce +0.
> 
> This means that tree-ssa-phiopt.c has a bug:
> 
> static bool
> minmax_replacement (basic_block cond_bb, basic_block middle_bb,
> edge e0, edge e1, gimple *phi,
> tree arg0, tree arg1)
> {
> ...
>   /* The optimization may be unsafe due to NaNs.  */
>   if (HONOR_NANS (type))
> return false;
> 
> and it should check HONOR_SIGNED_ZEROS as well.
> 
> I'll fix that as a followup.

Committed as follows.

Bootstrapped / tested on x86_64-unknown-linux-gnu.

Richard.

2016-09-29  Richard Biener  

PR middle-end/55152
* match.pd: Add max(a,-a) -> abs(a) pattern.
* tree-ssa-phiopt.c (minmax_replacement): Disable for
HONOR_SIGNED_ZEROS types.

* gcc.dg/pr55152.c: New testcase.
* gcc.dg/tree-ssa/phi-opt-5.c: Adjust.

Index: gcc/match.pd
===
--- gcc/match.pd(revision 240565)
+++ gcc/match.pd(working copy)
@@ -1271,6 +1284,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (simplify
  (max:c (min:c @0 @1) @1)
  @1)
+/* max(a,-a) -> abs(a).  */
+(simplify
+ (max:c @0 (negate @0))
+ (if (TREE_CODE (type) != COMPLEX_TYPE
+  && (! ANY_INTEGRAL_TYPE_P (type)
+ || TYPE_OVERFLOW_UNDEFINED (type)))
+  (abs @0)))
 (simplify
  (min @0 @1)
  (switch
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c   (revision 240565)
+++ gcc/tree-ssa-phiopt.c   (working copy)
@@ -1075,7 +1075,7 @@ minmax_replacement (basic_block cond_bb,
   type = TREE_TYPE (PHI_RESULT (phi));
 
   /* The optimization may be unsafe due to NaNs.  */
-  if (HONOR_NANS (type))
+  if (HONOR_NANS (type) || HONOR_SIGNED_ZEROS (type))
 return false;
 
   cond = as_a  (last_stmt (cond_bb));
Index: gcc/testsuite/gcc.dg/pr55152.c
===
--- gcc/testsuite/gcc.dg/pr55152.c  (revision 0)
+++ gcc/testsuite/gcc.dg/pr55152.c  (working copy)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O -ffinite-math-only -fno-signed-zeros -fstrict-overflow 
-fdump-tree-optimized" } */
+
+double g (double a)
+{
+  return (a>=-a)?a:-a;
+}
+int f(int a)
+{
+  return (a>=-a)?a:-a;
+}
+
+/* { dg-final { scan-tree-dump-times "ABS_EXPR" 2 "optimized" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/phi-opt-5.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/phi-opt-5.c   (revision 240612)
+++ gcc/testsuite/gcc.dg/tree-ssa/phi-opt-5.c   (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O1 -ffinite-math-only -fdump-tree-phiopt1" } */
+/* { dg-options "-O1 -ffinite-math-only -fno-signed-zeros -fdump-tree-phiopt1" 
} */
 
 float repl1 (float varx)
 {

Re: [PATCH v3] Optimize strchr to strlen

[Oleg Endo]

On Fri, 2016-09-23 at 23:10 +0900, Oleg Endo wrote:
> On Fri, 2016-09-23 at 14:07 +, Wilco Dijkstra wrote:
> > 
> > After discussion (https://gcc.gnu.org/ml/gcc-patches/2016-09/msg007
> > 18
> > .html)
> > here is the latest version of the strchr patch.  This uses a gimple
> > statement for
> > the pointer addition so the gsi_prev always points at the strlen
> > call.
> > 
> > Optimize strchr (s, 0) to s + strlen (s).  strchr (s, 0) appears a
> > common
> > idiom for finding the end of a string, however it is not a very
> > efficient
> > way of doing so.  Strlen is a much simpler operation which is
> > significantly
> > faster (eg. on x86 strlen is 50% faster for strings of 8 bytes and
> > about
> > twice as fast as strchr on strings of 1KB).
> > 
> > OK for commit?

> Please add PR tree-optimization/61056 to the changelog so that it
> gets linked in bugzilla.

Notice that the "PR AAA/BBB" markers from the changelog should also be
included in the SVN commit message.  Otherwise the bugzilla commit hook
doesn't notice it, because it looks at the commit messages and not at
the contents of the ChangeLog files.

Cheers,
Oleg

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Jonathan Wakely]

On 29/09/16 14:02 +0200, Rainer Orth wrote:

Hi Jonathan,


If only there was some way the Solaris team could contact us so we
could coordinate and stop adding more and more hacks to mess with each
others headers. But I assume they don't have access to the www or
email, because the only other explanation is too rude to say on a
public list.


Presumably they now provide all the declarations for C++11, so we
don't need to declare them. Moving the new hypot overload outside the


still this doesn't need into , but into libstdc++ configury
IMNSHO.


I agree. Presumably they wanted their updated headers to work with
existing GCC releases, and not have to wait until we modified our
configury to cooperate with their changes.

But if they simply communicated with us we could plan for that kind of
thing in advance, and ensure we don't keep trying to fix each others
headers from a distance.


_GLIBCXX_USE_C99_MATH_TR1 block should mean it's declared. That will
break when Solaris adds C++17 support, so we'd better add some macro
to guard the new hypot overloads, so they can be disabled again. Let's
call it __CORRECT_ISO_CPP17_MATH_H_PROTO (and maybe add a coment like
"Dear Solaris team, ...").


... which assumes they do read this ;-)


Indeed.

Since they seem to be adding declarations to  and only in the
global namespace, and the 3-argument form doesn't exist in the global
namespace, maybe we can assume they aren't going to add it to
.



Does this work?


It does indeed, at least running the single testcase with runtest now
passes.


Thanks, I'll commit that soon.

[accaf, Fortran, patch, v1] Generate caf-reference chains only from the first coarray reference on, and more.

[Andre Vehreschild]

Hi all,

attached patch fixes an addressing issue for coarrays *in* derived types.
Before the patch the caf runtime reference chain was generated from the start
of the symbol to the last reference *and* the reference chain upto the coarray
in the derived type was used to call the caf_*_by_ref () functions. The patch
fixes this by skipping the generation of unnecessary caf runtime references. 

The second part fixes finding the token for coarrayed arrays. The new semantic
is, that each allocatable array has the coarray token in its .token member,
which the allocate_array now makes use of.

Bootstrapped and regtested ok on x86_64-linux/F23. Ok for trunk?

Regards,
Andre
-- 
Andre Vehreschild * Email: vehre ad gmx dot de 
gcc/fortran/ChangeLog:

2016-09-29  Andre Vehreschild  

* trans-array.c (gfc_array_allocate): Use the token from coarray's
.token member.
* trans-intrinsic.c (conv_expr_ref_to_caf_ref): Only generate
caf-reference chains from the first coarray references on.
* trans-types.c (gfc_get_derived_type): Switch on mandatory .token
member generation for allocatable arrays in coarrays in derived types.

gcc/testsuite/ChangeLog:

2016-09-29  Andre Vehreschild  

* gfortran.dg/coarray_allocate_10.f08: New test.
* gfortran.dg/coindexed_1.f90: Above fixes allow execution.


diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 0b97760..50312fe 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -5406,7 +5406,6 @@ gfc_array_allocate (gfc_se * se, gfc_expr * expr, tree status, tree errmsg,
   gfc_expr **lower;
   gfc_expr **upper;
   gfc_ref *ref, *prev_ref = NULL, *coref;
-  gfc_se caf_se;
   bool allocatable, coarray, dimension, alloc_w_e3_arr_spec = false;
 
   ref = expr->ref;
@@ -5531,7 +5530,6 @@ gfc_array_allocate (gfc_se * se, gfc_expr * expr, tree status, tree errmsg,
 	}
 }
 
-  gfc_init_se (_se, NULL);
   gfc_start_block ();
 
   /* Allocate memory to store the data.  */
@@ -5543,9 +5541,7 @@ gfc_array_allocate (gfc_se * se, gfc_expr * expr, tree status, tree errmsg,
 
   if (coarray && flag_coarray == GFC_FCOARRAY_LIB)
 {
-  tmp = gfc_get_tree_for_caf_expr (expr);
-  gfc_get_caf_token_offset (_se, , NULL, tmp, NULL_TREE, expr);
-  gfc_add_block_to_block (, _se.pre);
+  token = gfc_conv_descriptor_token (se->expr);
   token = gfc_build_addr_expr (NULL_TREE, token);
 }
 
@@ -5557,7 +5553,6 @@ gfc_array_allocate (gfc_se * se, gfc_expr * expr, tree status, tree errmsg,
   else
 gfc_allocate_using_malloc (, pointer, size, status);
 
-  gfc_add_block_to_block (, _se.post);
   if (dimension)
 {
   cond = gfc_unlikely (fold_build2_loc (input_location, NE_EXPR,
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 954f7b3..a499c32 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -1110,7 +1110,7 @@ compute_component_offset (tree field, tree type)
 static tree
 conv_expr_ref_to_caf_ref (stmtblock_t *block, gfc_expr *expr)
 {
-  gfc_ref *ref = expr->ref;
+  gfc_ref *ref = expr->ref, *last_comp_ref;
   tree caf_ref = NULL_TREE, prev_caf_ref = NULL_TREE, reference_type, tmp, tmp2,
   field, last_type, inner_struct, mode, mode_rhs, dim_array, dim, dim_type,
   start, end, stride, vector, nvec;
@@ -1127,8 +1127,29 @@ conv_expr_ref_to_caf_ref (stmtblock_t *block, gfc_expr *expr)
 
   /* Prevent uninit-warning.  */
   reference_type = NULL_TREE;
-  last_type = gfc_typenode_for_spec (>symtree->n.sym->ts);
-  last_type_n = expr->symtree->n.sym->ts.type;
+
+  /* Skip refs upto the first coarray-ref.  */
+  last_comp_ref = NULL;
+  while (ref && (ref->type != REF_ARRAY || ref->u.ar.codimen == 0))
+{
+  /* Remember the type of components skipped.  */
+  if (ref->type == REF_COMPONENT)
+	last_comp_ref = ref;
+  ref = ref->next;
+}
+  /* When a component was skipped, get the type information of the last
+ component ref, else get the type from the symbol.  */
+  if (last_comp_ref)
+{
+  last_type = gfc_typenode_for_spec (_comp_ref->u.c.component->ts);
+  last_type_n = last_comp_ref->u.c.component->ts.type;
+}
+  else
+{
+  last_type = gfc_typenode_for_spec (>symtree->n.sym->ts);
+  last_type_n = expr->symtree->n.sym->ts.type;
+}
+
   while (ref)
 {
   if (ref->type == REF_ARRAY && ref->u.ar.codimen > 0
diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c
index 27a6bab..05122d9 100644
--- a/gcc/fortran/trans-types.c
+++ b/gcc/fortran/trans-types.c
@@ -2565,7 +2565,8 @@ gfc_get_derived_type (gfc_symbol * derived, bool in_coarray)
   if ((!c->attr.pointer && !c->attr.proc_pointer)
 	  || c->ts.u.derived->backend_decl == NULL)
 	c->ts.u.derived->backend_decl = gfc_get_derived_type (c->ts.u.derived,
-			  in_coarray);
+			  in_coarray
+			|| c->attr.codimension);
 

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Rainer Orth]

Hi Jonathan,

>>If only there was some way the Solaris team could contact us so we
>>could coordinate and stop adding more and more hacks to mess with each
>>others headers. But I assume they don't have access to the www or
>>email, because the only other explanation is too rude to say on a
>>public list.
>
> Presumably they now provide all the declarations for C++11, so we
> don't need to declare them. Moving the new hypot overload outside the

still this doesn't need into , but into libstdc++ configury
IMNSHO.

> _GLIBCXX_USE_C99_MATH_TR1 block should mean it's declared. That will
> break when Solaris adds C++17 support, so we'd better add some macro
> to guard the new hypot overloads, so they can be disabled again. Let's
> call it __CORRECT_ISO_CPP17_MATH_H_PROTO (and maybe add a coment like
> "Dear Solaris team, ...").

... which assumes they do read this ;-)

> Does this work?

It does indeed, at least running the single testcase with runtest now
passes.

Thanks.
Rainer

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

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Jonathan Wakely]

On 29/09/16 11:47 +0100, Jonathan Wakely wrote:

On 29/09/16 12:39 +0200, Rainer Orth wrote:

Hi Jonathan,


That would suggest Solaris uses include/c_std/cmath (where I forgot to
add the new overloads) rather than include/c_global/cmath ... is that
right?


Alternatively it's using c_global/cmath but _GLIBCXX_USE_C99_MATH_TR1
is not defined, as the new overloads are inside that block. I thought
that was defined for Solaris though, which is why we have the
__CORRECT_ISO_CPP11_MATH_H_PROTO there in that file.


it is indeed, at least initially.  What I see in preprocessed input is
this:

ro@galeras 208 > grep _GLIBCXX_USE_C99_MATH testsuite/normal4/hypot.ii
#define _GLIBCXX_USE_C99_MATH _GLIBCXX11_USE_C99_MATH
#define _GLIBCXX_USE_C99_MATH_TR1 1
#undef _GLIBCXX_USE_C99_MATH
#undef _GLIBCXX_USE_C99_MATH_TR1

It turns out the #undef's are from :

#if __cplusplus >= 201103L
#undef  _GLIBCXX_USE_C99_MATH
#undef  _GLIBCXX_USE_C99_MATH_TR1
#endif

No idea what this nonsense is trying to accomplish!  It's already in
Solaris 11.3, however.


Wow.

If only there was some way the Solaris team could contact us so we
could coordinate and stop adding more and more hacks to mess with each
others headers. But I assume they don't have access to the www or
email, because the only other explanation is too rude to say on a
public list.


Presumably they now provide all the declarations for C++11, so we
don't need to declare them. Moving the new hypot overload outside the
_GLIBCXX_USE_C99_MATH_TR1 block should mean it's declared. That will
break when Solaris adds C++17 support, so we'd better add some macro
to guard the new hypot overloads, so they can be disabled again. Let's
call it __CORRECT_ISO_CPP17_MATH_H_PROTO (and maybe add a coment like
"Dear Solaris team, ...").

Does this work?

diff --git a/libstdc++-v3/include/c_global/cmath b/libstdc++-v3/include/c_global/cmath
index fffa0e7..86ffd34 100644
--- a/libstdc++-v3/include/c_global/cmath
+++ b/libstdc++-v3/include/c_global/cmath
@@ -1455,46 +1455,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   return hypot(__type(__x), __type(__y));
 }
 
-#if __cplusplus > 201402L
-#define __cpp_lib_hypot 201603
-  // [c.math.hypot3], three-dimensional hypotenuse
-
-  template
-inline _Tp
-__hypot3(_Tp __x, _Tp __y, _Tp __z)
-{
-  __x = std::abs(__x);
-  __y = std::abs(__y);
-  __z = std::abs(__z);
-  if (_Tp __a = __x < __y ? __y < __z ? __z : __y : __x < __z ? __z : __x)
-	return __a * std::sqrt((__x / __a) * (__x / __a)
-			   + (__y / __a) * (__y / __a)
-			   + (__z / __a) * (__z / __a));
-  else
-	return {};
-}
-
-  inline float
-  hypot(float __x, float __y, float __z)
-  { return std::__hypot3(__x, __y, __z); }
-
-  inline double
-  hypot(double __x, double __y, double __z)
-  { return std::__hypot3(__x, __y, __z); }
-
-  inline long double
-  hypot(long double __x, long double __y, long double __z)
-  { return std::__hypot3(__x, __y, __z); }
-
-  template
-typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type
-hypot(_Tp __x, _Up __y, _Vp __z)
-{
-  using __type = typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type;
-  return std::__hypot3<__type>(__x, __y, __z);
-}
-#endif // C++17
-
 #ifndef __CORRECT_ISO_CPP11_MATH_H_PROTO
   constexpr int
   ilogb(float __x)
@@ -1830,6 +1790,55 @@ _GLIBCXX_END_NAMESPACE_VERSION
 
 #endif // C++11
 
+#if __cplusplus > 201402L
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+#ifndef __CORRECT_ISO_CPP17_MATH_H_PROTO
+  // [c.math.hypot3], three-dimensional hypotenuse
+#define __cpp_lib_hypot 201603
+
+  template
+inline _Tp
+__hypot3(_Tp __x, _Tp __y, _Tp __z)
+{
+  __x = std::abs(__x);
+  __y = std::abs(__y);
+  __z = std::abs(__z);
+  if (_Tp __a = __x < __y ? __y < __z ? __z : __y : __x < __z ? __z : __x)
+	return __a * std::sqrt((__x / __a) * (__x / __a)
+			   + (__y / __a) * (__y / __a)
+			   + (__z / __a) * (__z / __a));
+  else
+	return {};
+}
+
+  inline float
+  hypot(float __x, float __y, float __z)
+  { return std::__hypot3(__x, __y, __z); }
+
+  inline double
+  hypot(double __x, double __y, double __z)
+  { return std::__hypot3(__x, __y, __z); }
+
+  inline long double
+  hypot(long double __x, long double __y, long double __z)
+  { return std::__hypot3(__x, __y, __z); }
+
+  template
+typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type
+hypot(_Tp __x, _Up __y, _Vp __z)
+{
+  using __type = typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type;
+  return std::__hypot3<__type>(__x, __y, __z);
+#endif
+}
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace
+#endif // C++17
+
+
 #if _GLIBCXX_USE_STD_SPEC_FUNCS
 #  include 
 #endif

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Rainer Orth]

Hi Jonathan,

>>It turns out the #undef's are from :
>>
>>#if __cplusplus >= 201103L
>>#undef  _GLIBCXX_USE_C99_MATH
>>#undef  _GLIBCXX_USE_C99_MATH_TR1
>>#endif
>>
>>No idea what this nonsense is trying to accomplish!  It's already in
>>Solaris 11.3, however.
>
> Wow.
>
> If only there was some way the Solaris team could contact us so we
> could coordinate and stop adding more and more hacks to mess with each
> others headers. But I assume they don't have access to the www or
> email, because the only other explanation is too rude to say on a
> public list.

it very much depends who you are dealing with: some are quite good at
coordinating with other/external groups, while others, well, don't get
me started...

I'll see what I can dig up here: my goal has been to get rid of the need
for fixincludes on newer Solaris versions, but progress has been slow ;-(

We'll probably have to undo this via fixincludes since at least on
Solaris 11.x it's already in the wild.  Maybe this can be stopped before
Solaris 12 ships, though...

>>> Once again I wish we had a Solaris machine in the compile farm, or it
>>> was possible to install a Solaris VM and get OS updates without paying
>>> Oracle for the privilege.
>>
>>That's easily doable: Solaris is free for development use; you get
>>access to the release (11, 11.1, 11.2, 11.3, ...) versions, just not to
>>patches/updates.
>
> Yes, but because the libc headers get changed by patches and updates,
> I found it was useless to install it in a VM. Because I had outdated
> headers I couldn't test how our build system interacts with a fully
> updated version of Solaris.

True: there have certainly issues like this in the past; that's why I'm
trying to keep even older Solaris versions up to date WRT patches.

>>They do have a program for free academic use of
>>Solaris, including updates, these days: https://academy.oracle.com/.
>
> That might be useful, I'll see if I qualify. Thanks.

Another option might be to get Oracle grant a similar exception to the
compile farm.  I'll ask around if there's any chance of this.

Rainer

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

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Jonathan Wakely]

On 29/09/16 12:39 +0200, Rainer Orth wrote:

Hi Jonathan,


That would suggest Solaris uses include/c_std/cmath (where I forgot to
add the new overloads) rather than include/c_global/cmath ... is that
right?


Alternatively it's using c_global/cmath but _GLIBCXX_USE_C99_MATH_TR1
is not defined, as the new overloads are inside that block. I thought
that was defined for Solaris though, which is why we have the
__CORRECT_ISO_CPP11_MATH_H_PROTO there in that file.


it is indeed, at least initially.  What I see in preprocessed input is
this:

ro@galeras 208 > grep _GLIBCXX_USE_C99_MATH testsuite/normal4/hypot.ii
#define _GLIBCXX_USE_C99_MATH _GLIBCXX11_USE_C99_MATH
#define _GLIBCXX_USE_C99_MATH_TR1 1
#undef _GLIBCXX_USE_C99_MATH
#undef _GLIBCXX_USE_C99_MATH_TR1

It turns out the #undef's are from :

#if __cplusplus >= 201103L
#undef  _GLIBCXX_USE_C99_MATH
#undef  _GLIBCXX_USE_C99_MATH_TR1
#endif

No idea what this nonsense is trying to accomplish!  It's already in
Solaris 11.3, however.


Wow.

If only there was some way the Solaris team could contact us so we
could coordinate and stop adding more and more hacks to mess with each
others headers. But I assume they don't have access to the www or
email, because the only other explanation is too rude to say on a
public list.



Once again I wish we had a Solaris machine in the compile farm, or it
was possible to install a Solaris VM and get OS updates without paying
Oracle for the privilege.


That's easily doable: Solaris is free for development use; you get
access to the release (11, 11.1, 11.2, 11.3, ...) versions, just not to
patches/updates.


Yes, but because the libc headers get changed by patches and updates,
I found it was useless to install it in a VM. Because I had outdated
headers I couldn't test how our build system interacts with a fully
updated version of Solaris.


They do have a program for free academic use of
Solaris, including updates, these days: https://academy.oracle.com/.


That might be useful, I'll see if I qualify. Thanks.

Re: [PATCH][v4] GIMPLE store merging pass

[Richard Biener]

On Wed, 28 Sep 2016, Kyrill Tkachov wrote:

> Hi all,
> 
> This is v4 of the pass. It addresses feedback by Bernhard, including typo
> fixes and
> skipping of debug statements.
> Also, I've extended it to handle the case from PR 23684 and included that
> testcase
> in the patch.  Merging now triggers more often.
> I've also added purging of dead EH edges that was missing from the previous
> versions.
> 
> Bootstrapped and tested on aarch64-none-linux-gnu, x86_64-unknown-linux-gnu,
> arm-none-linux-gnueabihf.
> Also tested on aarch64 big-endian.
> 
> I saw no regressions on my x86_64 machine on SPEC2006. I think the changes in
> individual benchmarks were
> in the noise though I think the x86_64 expanders could be improved to split
> expensive movabsq instructions
> into two movl ones (I think).
> 
> Bill, could you or someone else with access to Power benchmarking try this
> patch out on some benchmarks
> that you usually use? The new pass in this patch is on by default and can be
> turned off by -fno-store-merging
> if needed.  Jakub indicated that his last attempt at this work caused
> regressions on powerpc so I'd like to
> see if this patch is okay in that regard.

Comments below.

> Thanks,
> Kyrill
> 
> 2016-09-28  Kyrylo Tkachov  
> 
> PR middle-end/22141
> * Makefile.in (OBJS): Add gimple-ssa-store-merging.o.
> * common.opt (fstore-merging): New Optimization option.
> * opts.c (default_options_table): Add entry for
> OPT_ftree_store_merging.
> * params.def (PARAM_STORE_MERGING_ALLOW_UNALIGNED): Define.
> * passes.def: Insert pass_tree_store_merging.
> * tree-pass.h (make_pass_store_merging): Declare extern
> prototype.
> * gimple-ssa-store-merging.c: New file.
> * doc/invoke.texi (Optimization Options): Document
> -fstore-merging.
> 
> 2016-09-28  Kyrylo Tkachov  
> Jakub Jelinek  
> Andrew Pinski  
> 
> PR middle-end/22141
> PR rtl-optimization/23684
> * gcc.c-torture/execute/pr22141-1.c: New test.
> * gcc.c-torture/execute/pr22141-2.c: Likewise.
> * gcc.target/aarch64/ldp_stp_1.c: Adjust for -fstore-merging.
> * gcc.target/aarch64/ldp_stp_4.c: Likewise.
> * gcc.dg/store_merging_1.c: New test.
> * gcc.dg/store_merging_2.c: Likewise.
> * gcc.dg/store_merging_3.c: Likewise.
> * gcc.dg/store_merging_4.c: Likewise.
> * gcc.dg/store_merging_5.c: Likewise.
> * gcc.dg/store_merging_6.c: Likewise.
> * gcc.dg/store_merging_7.c: Likewise.
> * gcc.target/i386/pr22141.c: Likewise.
> * gcc.target/i386/pr34012.c: Add -fno-store-merging to dg-options.
> 
> commit 74e3e742e446f6a892c8ee755f640756efc816cd
> Author: Kyrylo Tkachov 
> Date:   Tue Jul 12 12:30:47 2016 +0100
> 
> GIMPLE store merging pass
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 9e08fbf..d83b2f1 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1300,6 +1300,7 @@ OBJS = \
>   gimple-ssa-isolate-paths.o \
>   gimple-ssa-nonnull-compare.o \
>   gimple-ssa-split-paths.o \
> + gimple-ssa-store-merging.o \
>   gimple-ssa-strength-reduction.o \
>   gimple-ssa-sprintf.o \
>   gimple-streamer-in.o \
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 168735e..4d73852 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1460,6 +1460,10 @@ fstrict-volatile-bitfields
>  Common Report Var(flag_strict_volatile_bitfields) Init(-1) Optimization
>  Force bitfield accesses to match their type width.
>  
> +fstore-merging
> +Common Var(flag_store_merging) Optimization
> +Use the tree store merging pass.
> +
>  fguess-branch-probability
>  Common Report Var(flag_guess_branch_prob) Optimization
>  Enable guessing of branch probabilities.
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 6767462..83acd3b 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -403,7 +403,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fsingle-precision-constant -fsplit-ivs-in-unroller @gol
>  -fsplit-paths @gol
>  -fsplit-wide-types -fssa-backprop -fssa-phiopt @gol
> --fstdarg-opt -fstrict-aliasing @gol
> +-fstdarg-opt -fstore-merging -fstrict-aliasing @gol
>  -fstrict-overflow -fthread-jumps -ftracer -ftree-bit-ccp @gol
>  -ftree-builtin-call-dce -ftree-ccp -ftree-ch @gol
>  -ftree-coalesce-vars -ftree-copy-prop -ftree-dce -ftree-dominator-opts @gol
> @@ -414,8 +414,8 @@ Objective-C and Objective-C++ Dialects}.
>  -ftree-loop-vectorize @gol
>  -ftree-parallelize-loops=@var{n} -ftree-pre -ftree-partial-pre -ftree-pta 
> @gol
>  -ftree-reassoc -ftree-sink -ftree-slsr -ftree-sra @gol
> --ftree-switch-conversion -ftree-tail-merge -ftree-ter @gol
> --ftree-vectorize -ftree-vrp -funconstrained-commons @gol
> +-ftree-switch-conversion -ftree-tail-merge @gol
> +-ftree-ter -ftree-vectorize -ftree-vrp -funconstrained-commons @gol
>  -funit-at-a-time 

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Rainer Orth]

Hi Jonathan,

>>That would suggest Solaris uses include/c_std/cmath (where I forgot to
>>add the new overloads) rather than include/c_global/cmath ... is that
>>right?
>
> Alternatively it's using c_global/cmath but _GLIBCXX_USE_C99_MATH_TR1
> is not defined, as the new overloads are inside that block. I thought
> that was defined for Solaris though, which is why we have the
> __CORRECT_ISO_CPP11_MATH_H_PROTO there in that file.

it is indeed, at least initially.  What I see in preprocessed input is
this:

ro@galeras 208 > grep _GLIBCXX_USE_C99_MATH testsuite/normal4/hypot.ii
#define _GLIBCXX_USE_C99_MATH _GLIBCXX11_USE_C99_MATH
#define _GLIBCXX_USE_C99_MATH_TR1 1
#undef _GLIBCXX_USE_C99_MATH
#undef _GLIBCXX_USE_C99_MATH_TR1

It turns out the #undef's are from :

#if __cplusplus >= 201103L
#undef  _GLIBCXX_USE_C99_MATH
#undef  _GLIBCXX_USE_C99_MATH_TR1
#endif

No idea what this nonsense is trying to accomplish!  It's already in
Solaris 11.3, however.

> Once again I wish we had a Solaris machine in the compile farm, or it
> was possible to install a Solaris VM and get OS updates without paying
> Oracle for the privilege.

That's easily doable: Solaris is free for development use; you get
access to the release (11, 11.1, 11.2, 11.3, ...) versions, just not to
patches/updates.  They do have a program for free academic use of
Solaris, including updates, these days: https://academy.oracle.com/.

Rainer

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

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Rainer Orth]

Hi Jonathan,

> That would suggest Solaris uses include/c_std/cmath (where I forgot to
> add the new overloads) rather than include/c_global/cmath ... is that
> right?

no, include/cmath points to the c_global version.

Rainer

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

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Jonathan Wakely]

On 29/09/16 10:10 +0100, Jonathan Wakely wrote:

On 29/09/16 10:54 +0200, Rainer Orth wrote:

Hi Jonathan,


This adds the new 3D std::hypot() functions. This implementation seems
to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or
hypot(hypot(x, y), z), and should be a bit more accurate at very large
or very small values due to reducing the arguments by the largest one.
Improvements welcome though, as this is not my forte.

The test might not be very good, but tests some small integer values
and some other values where accuracy is lost for one or other of the
alternative implementations mentioned above. If this FAILs for some
32-bit targets we might need to adjust the tolerances or the
dg-options.

* doc/xml/manual/status_cxx2017.xml: Update status.
* include/c_global/cmath (hypot): Add three-dimensional overloads.
* testsuite/26_numerics/headers/cmath/hypot.cc: New.

Tested powerpc64le-linux and x86_64-linux, committed to trunk.


the new test currently FAILs on Solaris 12 (both SPARC and x86):

+FAIL: 26_numerics/headers/cmath/hypot.cc (test for excess errors)
+WARNING: 26_numerics/headers/cmath/hypot.cc compilation failed to produce execu
table

Excess errors:
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38:
 error: no matching function for call to 'hypot(double, double, double)'
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38:
 error: no matching function for call to 'hypot(double, double, double)'
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38:
 error: template argument 2 is invalid

and many more.


That would suggest Solaris uses include/c_std/cmath (where I forgot to
add the new overloads) rather than include/c_global/cmath ... is that
right?


Alternatively it's using c_global/cmath but _GLIBCXX_USE_C99_MATH_TR1
is not defined, as the new overloads are inside that block. I thought
that was defined for Solaris though, which is why we have the
__CORRECT_ISO_CPP11_MATH_H_PROTO there in that file.

Once again I wish we had a Solaris machine in the compile farm, or it
was possible to install a Solaris VM and get OS updates without paying
Oracle for the privilege.

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Jonathan Wakely]

On 29/09/16 10:54 +0200, Rainer Orth wrote:

Hi Jonathan,


This adds the new 3D std::hypot() functions. This implementation seems
to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or
hypot(hypot(x, y), z), and should be a bit more accurate at very large
or very small values due to reducing the arguments by the largest one.
Improvements welcome though, as this is not my forte.

The test might not be very good, but tests some small integer values
and some other values where accuracy is lost for one or other of the
alternative implementations mentioned above. If this FAILs for some
32-bit targets we might need to adjust the tolerances or the
dg-options.

* doc/xml/manual/status_cxx2017.xml: Update status.
* include/c_global/cmath (hypot): Add three-dimensional overloads.
* testsuite/26_numerics/headers/cmath/hypot.cc: New.

Tested powerpc64le-linux and x86_64-linux, committed to trunk.


the new test currently FAILs on Solaris 12 (both SPARC and x86):

+FAIL: 26_numerics/headers/cmath/hypot.cc (test for excess errors)
+WARNING: 26_numerics/headers/cmath/hypot.cc compilation failed to produce execu
table

Excess errors:
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38:
 error: no matching function for call to 'hypot(double, double, double)'
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38:
 error: no matching function for call to 'hypot(double, double, double)'
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38:
 error: template argument 2 is invalid

and many more.


That would suggest Solaris uses include/c_std/cmath (where I forgot to
add the new overloads) rather than include/c_global/cmath ... is that
right?

Re: [PATCH, RFC] gcov: dump in a static dtor instead of in an atexit handler

[Rainer Orth]

Hi Martin,

> On 08/12/2016 04:08 PM, Martin Liška wrote:
>> On 08/10/2016 02:53 PM, Nathan Sidwell wrote:
>>> On 08/10/16 06:43, Martin Liška wrote:
 Hello.

 There are multiple PRs (mentioned in ChangeLog) which suffer from
 missing capability of gcov
 to save counters for functions with constructor/destructor
 attributes. I done that by simply
 replacing atexit handler (gcov_exit) with a new static destructor
 (__gcov_exit), which has
 priority 99 (I did the same for __gcov_init). However, I'm not sure
 whether it's possible
 that a ctor defined in a source file can be potentially executed before
 __gcov_init (w/ the default
 priority)?

 Patch survives:
 make check -k -j10 RUNTESTFLAGS="gcov.exp"
 make check -k -j10 RUNTESTFLAGS="tree-prof.exp"

 I've just also verified that a DSO gcov dump works as before. I'm
 attaching a test-case which
 tests both static ctors/dtors, as well as C++ ctors/dtors.
>>>
>>> Does a coverage bootstrap (--enable-coverage) still succeed?
>> 
>> Well, looks results are more unstable than I thought.
>> Even running 'make -j1' in objdir/x86_64-pc-linux-gnu/libgcc repeatedly
>> generates different results.
>> I'll dig in after weekend.
>> 
>> Martin
[...]
>>> I think this is a good idea, but we should document the changed
>>> behavior. (I don't think the current behaviour's documented).
>
> I'm adding a new hunk that documents the behavior.
>
> Is the patch ready to be installed?

the testcase FAILs on Solaris 12 (both SPARC and x86):

+FAIL: g++.dg/gcov/pr16855.C  -std=gnu++11  gcov: 1 failures in line counts, 0 i
n branch percentages, 0 in return percentages, 0 in intermediate format
+FAIL: g++.dg/gcov/pr16855.C  -std=gnu++11  line 21: is #:should be 1
+FAIL: g++.dg/gcov/pr16855.C  -std=gnu++14  gcov: 1 failures in line counts, 0 i
n branch percentages, 0 in return percentages, 0 in intermediate format
+FAIL: g++.dg/gcov/pr16855.C  -std=gnu++14  line 21: is #:should be 1
+FAIL: g++.dg/gcov/pr16855.C  -std=gnu++98  gcov: 1 failures in line counts, 0 i
n branch percentages, 0 in return percentages, 0 in intermediate format
+FAIL: g++.dg/gcov/pr16855.C  -std=gnu++98  line 21: is #:should be 1

I haven't looked closer yet, but notice that you require constructor
priority support which isn't available everywhere (it is on Solaris 12,
but not before).

Rainer

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

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Rainer Orth]

Hi Jonathan,

> This adds the new 3D std::hypot() functions. This implementation seems
> to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or
> hypot(hypot(x, y), z), and should be a bit more accurate at very large
> or very small values due to reducing the arguments by the largest one.
> Improvements welcome though, as this is not my forte.
>
> The test might not be very good, but tests some small integer values
> and some other values where accuracy is lost for one or other of the
> alternative implementations mentioned above. If this FAILs for some
> 32-bit targets we might need to adjust the tolerances or the
> dg-options.
>
>   * doc/xml/manual/status_cxx2017.xml: Update status.
>   * include/c_global/cmath (hypot): Add three-dimensional overloads.
>   * testsuite/26_numerics/headers/cmath/hypot.cc: New.
>
> Tested powerpc64le-linux and x86_64-linux, committed to trunk.

the new test currently FAILs on Solaris 12 (both SPARC and x86):

+FAIL: 26_numerics/headers/cmath/hypot.cc (test for excess errors)
+WARNING: 26_numerics/headers/cmath/hypot.cc compilation failed to produce execu
table

Excess errors:
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38:
 error: no matching function for call to 'hypot(double, double, double)'
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38:
 error: no matching function for call to 'hypot(double, double, double)'
/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38:
 error: template argument 2 is invalid

and many more.

Rainer

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

[PATCH] Disable compact casesi patterns for arcv2

[Claudiu Zissulescu]

Here it is.  The previous version had more mods which should be in a different 
patch.

Please let me know if you still have issues with it,
Claudiu

gcc/
2016-05-09  Claudiu Zissulescu  

* common/config/arc/arc-common.c (arc_option_optimization_table):
Remove compact casesi option.
* config/arc/arc.c (arc_override_options): Use compact casesi
option only for pre-ARCv2 cores.
---
 gcc/common/config/arc/arc-common.c | 1 -
 gcc/config/arc/arc.c   | 7 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/gcc/common/config/arc/arc-common.c 
b/gcc/common/config/arc/arc-common.c
index f5b9c6d..5b687fb 100644
--- a/gcc/common/config/arc/arc-common.c
+++ b/gcc/common/config/arc/arc-common.c
@@ -56,7 +56,6 @@ static const struct default_options 
arc_option_optimization_table[] =
 { OPT_LEVELS_ALL, OPT_mbbit_peephole, NULL, 1 },
 { OPT_LEVELS_SIZE, OPT_mq_class, NULL, 1 },
 { OPT_LEVELS_SIZE, OPT_mcase_vector_pcrel, NULL, 1 },
-{ OPT_LEVELS_SIZE, OPT_mcompact_casesi, NULL, 1 },
 { OPT_LEVELS_NONE, 0, NULL, 0 }
   };
 
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index 2b25b0b..825bccf 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -858,6 +858,13 @@ arc_override_options (void)
   if (arc_size_opt_level == 3)
 optimize_size = 1;
 
+  /* Compact casesi is not a valid option for ARCv2 family, disable
+ it.  */
+  if (TARGET_V2)
+TARGET_COMPACT_CASESI = 0;
+  else if (optimize_size == 1)
+TARGET_COMPACT_CASESI = 1;
+
   if (flag_pic)
 target_flags |= MASK_NO_SDATA_SET;
 
-- 
1.9.1

[ARM] Fix new constraints and attributes of SI/HI data movement patterns

[Matthew Wahab]

The patch at https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01975.html
added constraints and "arch" attributes to some data movement patterns,
to fix wrongly generating MOVW instructions when not supported by the
targets. This was needed to fix a broken build but also resulted in MOVW
instructions not being generated when they should be.

This patch fixes the code-gen problems by removing the new attributes
from *arm_movsi_vfp and *thumb2_movsi_vfp since they are unnecessary.
It changes the rest of the added arch attributes from "t2", which
specifies a Thumb-2 target, to the weaker "v6t2" which specifies a
target that supports Thumb-2.

Tested for arm-none-linux-gnueabihf with native bootstrap and make check
on ARMv8-A and for arm-none-eabi with cross-compiled check-gcc on an
ARMv8.2-A emulator.

There is one unrelated failure in gcc.target/arm/fp16-aapcs-3.c, which
is a recently added test for the FP16 ARM alternative format. This has
dg-require-effective-target arm_fp16_ok
which is true for +fp16 because that implies mfp16-format=ieee. The test
should instead be requiring
dg-require-effective-target arm_fp16_alternative_ok
I'll send a patch to fix this.

Ok for trunk?
Matthew

gcc/
2016-09-29  Matthew Wahab  

* config/arm/arm.md (*arm_movsi_insn): Replace "t2" arch attribute
with "v6t2".  Move "arch" attribute above "pool_range".
* config/arm/vfp.md (*arm_movhi_vfp): Likewise.
(*thumb2_movhi_vfp): Likewise.
(*arm_movhi_fp16): Likewise.
(*thumb2_movhi_fp16): Likewise.
(*arm_movsi_vfp): Remove "arch" attribute.
(*thumb2_movsi_vfp): Likewise.
>From 7ef04c9cc749f1705ea657874c9db43e8a7d5320 Mon Sep 17 00:00:00 2001
From: Matthew Wahab 
Date: Wed, 28 Sep 2016 12:08:22 +0100
Subject: [PATCH] [ARM] Fix new constraints and attributes of SI/HI data
 movement patterns

The patch at https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01975.html
added constraints and "arch" attributes to some data movement patterns,
to fix wrongly generating MOVW instructions when not supported by the
targets. This was needed to fix a broken build but also resulted in MOVW
instructions not being generated when they should be.

This patch fixes the code-gen problems by removing the new attributes
from *arm_movsi_insn, *arm_movsi_vfp and *thumb2_movsi_vfp since they
are unnecessary. It changes the rest of the added arch attributes from
"t2", which specifies a Thumb-2 target, to the weaker "v6t2" which
specifies a target that supports Thumb-2.

Tested for arm-none-linux-gnueabihf with native bootstrap and make check
on ARMv8-A and for arm-none-eabi with cross-compiled check-gcc on an
ARMv8.2-A emulator.

There is one unrelated failure in gcc.target/arm/fp16-aapcs-3.c, which
is a recently added test for the FP16 ARM alternative format. This has
dg-require-effective-target arm_fp16_ok
which is true for +fp16 because that implies mfp16-format=ieee. The test
should instead be requiring
dg-require-effective-target arm_fp16_alternative_ok

gcc/
2016-09-29  Matthew Wahab  

	* config/arm/arm.md (*arm_movsi_insn): Replace "t2" arch attribute
	with "v6t2".  Move "arch" attribute above "pool_range".
	* config/arm/vfp.md (*arm_movhi_vfp): Likewise.
	(*thumb2_movhi_vfp): Likewise.
	(*arm_movhi_fp16): Likewise.
	(*thumb2_movhi_fp16): Likewise.
	(*arm_movsi_vfp): Remove "arch" attribute.
	(*thumb2_movsi_vfp): Likewise.
---
 gcc/config/arm/arm.md |  2 +-
 gcc/config/arm/vfp.md | 10 --
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 999292b..396aab7 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -6064,8 +6064,8 @@
str%?\\t%1, %0"
   [(set_attr "type" "mov_reg,mov_imm,mvn_imm,mov_imm,load1,store1")
(set_attr "predicable" "yes")
+   (set_attr "arch" "*,*,*,v6t2,*,*")
(set_attr "pool_range" "*,*,*,*,4096,*")
-   (set_attr "arch" "*,*,*,t2,*,*")
(set_attr "neg_pool_range" "*,*,*,*,4084,*")]
 )
 
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index 21eaf48..f39e590 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -65,7 +65,7 @@
 (const_string "f_mcr")
 (const_string "f_mrc")
 (const_string "fmov")])
-  (set_attr "arch" "*, *, t2, *, *, *, *, *")
+  (set_attr "arch" "*, *, v6t2, *, *, *, *, *")
   (set_attr "pool_range" "*, *, *, *, 256, *, *, *")
   (set_attr "neg_pool_range" "*, *, *, *, 244, *, *, *")
   (set_attr "length" "4")]
@@ -109,7 +109,7 @@
   (set_attr "type"
"mov_reg, mov_imm, mov_imm, mov_imm, store1, load1,\
 f_mcr, f_mrc, fmov")
-  (set_attr "arch" "*, *, *, t2, *, *, *, *, *")
+  (set_attr "arch" "*, *, *, v6t2, *, *, *, *, *")
   (set_attr "pool_range" "*, *, *, *, *, 4094, *, *, *")
   (set_attr "neg_pool_range" "*, *, *, *, *, 250, *, *, *")
   (set_attr "length" "2, 4, 2, 4, 4, 4, 4, 4, 4")]
@@ -161,7 +161,7 @@
 (const_string "f_mcr")

Re: [PATCH v3] Optimize strchr to strlen

[Richard Biener]

On Wed, Sep 28, 2016 at 5:45 PM, Jason Merrill  wrote:
> OK.
>
> On Wed, Sep 28, 2016 at 11:43 AM, Wilco Dijkstra  
> wrote:
>> Jason Merrill wrote:
>>> I think this broke g++.dg/ext/builtin10.C.
>>
>> That's odd. It appears if you add a fold in gimple-fold.c, it no longer 
>> calls the
>> folding code in builtins.c. No idea what the idea behind that is (especially 
>> since
>> there are other builtins that appear in both files), but this simple patch 
>> fixes it:
>>
>> If strchr can't be folded in gimple-fold.c, break so folding code in 
>> builtins.c is
>> also called.
>>
>> OK for commit?

Please move the remaining folding of strchr to gimple-fold.c then.
All of the non-constant "folding" in builtins.c should move there,
constant folding parts should move to fold-const-call.c.

Thanks,
Richard.

>> 2016-09-28  Wilco Dijkstra  
>>
>> * gimple-fold.c (gimple_fold_builtin): After failing to fold
>> strchr, also try the generic folding.
>> --
>> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
>> index 
>> ddf4cf0ae68ef6708377fdb1a2b45575d90da799..b6802e81fd1a7fd0b309cb9aa0f984f7bacb6596
>>  100644
>> --- a/gcc/gimple-fold.c
>> +++ b/gcc/gimple-fold.c
>> @@ -2948,7 +2948,10 @@ gimple_fold_builtin (gimple_stmt_iterator *gsi)
>>  case BUILT_IN_STRNCAT:
>>return gimple_fold_builtin_strncat (gsi);
>>  case BUILT_IN_STRCHR:
>> -  return gimple_fold_builtin_strchr (gsi);
>> +  if (gimple_fold_builtin_strchr (gsi))
>> +   return true;
>> +  /* Perform additional folding in builtin.c.  */
>> +  break;
>>  case BUILT_IN_FPUTS:
>>return gimple_fold_builtin_fputs (gsi, gimple_call_arg (stmt, 0),
>> gimple_call_arg (stmt, 1), false);
>>

Re: [PATCH PR77718]

[Richard Biener]

On Wed, Sep 28, 2016 at 5:51 PM, Aaron Sawdey
 wrote:
> This patch that Bernd put in PR77718 seems to be fine. Bootstrap and
> regtest done on powerpc64le, no new failures. Ok for trunk?

Ok.

Richard.

> 2016-09-28  Bernd Schmidt  
>
> * builtins.c (expand_builtin_memcmp): don't swap args unless
> result is only being compared with zero.
>
> Index: builtins.c
> ===
> --- builtins.c  (revision 240511)
> +++ builtins.c  (working copy)
> @@ -3707,11 +3707,13 @@ expand_builtin_memcmp (tree exp, rtx tar
>
>by_pieces_constfn constfn = NULL;
>
> -  const char *src_str = c_getstr (arg1);
> -  if (src_str == NULL)
> -src_str = c_getstr (arg2);
> -  else
> -std::swap (arg1_rtx, arg2_rtx);
> +  const char *src_str = c_getstr (arg2);
> +  if (result_eq && src_str == NULL)
> +{
> +  src_str = c_getstr (arg1);
> +  if (src_str != NULL)
> +   std::swap (arg1_rtx, arg2_rtx);
> +}
>
>/* If SRC is a string constant and block move would be done
>   by pieces, we can avoid loading the string from memory
>
> --
> Aaron Sawdey, Ph.D.  acsaw...@linux.vnet.ibm.com
> 050-2/C113  (507) 253-7520 home: 507/263-0782
> IBM Linux Technology Center - PPC Toolchain
>

Re: [PATCH] Detect whether target can use -fprofile-update=atomic

[Rainer Orth]

Hi Martin,

> 2016-09-06  Martin Liska  
>
>   * gcc.dg/profile-update-warning.c: New test.
[...]
> diff --git a/gcc/testsuite/gcc.dg/profile-update-warning.c 
> b/gcc/testsuite/gcc.dg/profile-update-warning.c
> new file mode 100644
> index 000..0614fad
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/profile-update-warning.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile { target { i?86-*-* x86_64-*-* } } } */
> +/* { dg-options "-fprofile-update=atomic -fprofile-generate -march=i386 
> -m32" } */
> +
> +int main(int argc, char *argv[])
> +{
> +  return 0;
> +} /* { dg-warning "target does not support atomic profile update, single 
> mode is selected" } */

this test FAILs on 32-bit-default x86 configurations like
i386-pc-solaris2.* and i686-pc-linux-gnu for the 64-bit multilib:

FAIL: gcc.dg/profile-update-warning.c  (test for warnings, line 7)
FAIL: gcc.dg/profile-update-warning.c (test for excess errors)

Excess errors:
cc1: error: CPU you selected does not support x86-64 instruction set
cc1: error: CPU you selected does not support x86-64 instruction set

What happens here is that -m64 is added after -march=i386 -m32, causing
the error above.  This doesn't happen for 64-bit-default targets: in the
64-bit case there's just the -m32 from the testcase, for the 32-bit
multilib just another -m32 is added, so the 32-bit case ist tested
twice.

Fixed like this, tested on i386-pc-solaris2.12 and x86_64-pc-linux-gnu,
installed.

Rainer


2016-09-28  Rainer Orth  

* gcc.dg/profile-update-warning.c: Restrict to ia32.
(dg-options): Remove -m32.

# HG changeset patch
# Parent  b421ce4362a3675d9fc4aa98f23e3bfc85b5c4c6
Fix 64-bit gcc.dg/profile-update-warning.c

diff --git a/gcc/testsuite/gcc.dg/profile-update-warning.c b/gcc/testsuite/gcc.dg/profile-update-warning.c
--- a/gcc/testsuite/gcc.dg/profile-update-warning.c
+++ b/gcc/testsuite/gcc.dg/profile-update-warning.c
@@ -1,5 +1,5 @@
-/* { dg-do compile { target { i?86-*-* x86_64-*-* } } } */
-/* { dg-options "-fprofile-update=atomic -fprofile-generate -march=i386 -m32" } */
+/* { dg-do compile { target { { i?86-*-* x86_64-*-* } && ia32 } } } */
+/* { dg-options "-fprofile-update=atomic -fprofile-generate -march=i386" } */
 
 int main(int argc, char *argv[])
 {

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

Re: [PATCH 2/2] Extend -falign-FOO=N to N[,M[,N2[,M2]]]

[Florian Weimer]

* Denys Vlasenko:

> Example syntax is -falign-functions=64,9: "align to 64 by skipping up to
> 9 bytes (not inclusive)". IOW: "after a call insn, CPU will always be able
> to fetch at least 9 bytes of insns".

Is it possible to set this using the optimize function attribute?

For example, we could use this to reduce alignment for compat
functions in glibc.

Re: [PATCH][v4] GIMPLE store merging pass

[Kyrill Tkachov]

On 29/09/16 08:36, Richard Biener wrote:

On Wed, 28 Sep 2016, Pat Haugen wrote:


On 09/28/2016 10:54 AM, Kyrill Tkachov wrote:

+fstore-merging
+Common Var(flag_store_merging) Optimization
+Use the tree store merging pass.
+

Did you purposely leave off "Report" for this option? I noticed the option didn't show up 
in the "options enabled:" section of the .s file when -fverbose-asm is specified.

Report should be added.  Also please don't mention "tree" -- this is
user facing documentation which shouldn't talk about GCC internals.
Thus for example "Merge adjacent stores." would be more appropriate here.


Thanks, I'll update this.

Kyrill


Richard.

Re: [PATCH][v4] GIMPLE store merging pass

[Richard Biener]

On Wed, 28 Sep 2016, Pat Haugen wrote:

> On 09/28/2016 10:54 AM, Kyrill Tkachov wrote:
> > +fstore-merging
> > +Common Var(flag_store_merging) Optimization
> > +Use the tree store merging pass.
> > +
> 
> Did you purposely leave off "Report" for this option? I noticed the option 
> didn't show up in the "options enabled:" section of the .s file when 
> -fverbose-asm is specified.

Report should be added.  Also please don't mention "tree" -- this is
user facing documentation which shouldn't talk about GCC internals.
Thus for example "Merge adjacent stores." would be more appropriate here.

Richard.

Re: [PATCH] Fix PR55152

[Richard Biener]

On Wed, 28 Sep 2016, Joseph Myers wrote:

> On Wed, 28 Sep 2016, Richard Biener wrote:
> 
> > Index: gcc/testsuite/gcc.dg/pr55152.c
> > ===
> > --- gcc/testsuite/gcc.dg/pr55152.c  (revision 0)
> > +++ gcc/testsuite/gcc.dg/pr55152.c  (working copy)
> > @@ -0,0 +1,13 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O -ffinite-math-only -fstrict-overflow 
> > -fdump-tree-optimized" } */
> > +
> > +double g (double a)
> > +{
> > +  return (a>=-a)?a:-a;
> 
> You should need -fno-signed-zeros for this (that is, for the 
> transformation to MAX_EXPR), not -ffinite-math-only.  For a == -0, that 
> function should return -0, but abs would produce +0.

This means that tree-ssa-phiopt.c has a bug:

static bool
minmax_replacement (basic_block cond_bb, basic_block middle_bb,
edge e0, edge e1, gimple *phi,
tree arg0, tree arg1)
{
...
  /* The optimization may be unsafe due to NaNs.  */
  if (HONOR_NANS (type))
return false;

and it should check HONOR_SIGNED_ZEROS as well.

I'll fix that as a followup.

Thanks,
Richard.