[PATCH] PR fortran/61765 -- Avoid ENTRY names in check of repeditive symbols

[Steve Kargl]

The attached patch has been tested on x86_64-*-freebsd.   There
were no regression.  The patch is less then obvious, but simple.
OK to commit?

2019-01-11  Steven G. Kargl  

PR fortran/61765
* resolve.c (gfc_verify_binding_labels): Break if-elseif-elseif 
structure into independent
if's with a return to simplify logic.  Avoid a check for ENTRY name 
with bind(c).

2019-01-11  Steven G. Kargl  

PR fortran/61765
* gfortran.dg/pr61765.f90: New test.

-- 
Steve
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(revision 267862)
+++ gcc/fortran/resolve.c	(working copy)
@@ -11789,11 +11789,12 @@ gfc_verify_binding_labels (gfc_symbol *sym)
 		 sym->binding_label, &sym->declared_at, &gsym->where);
   /* Clear the binding label to prevent checking multiple times.  */
   sym->binding_label = NULL;
-
+  return;
 }
-  else if (sym->attr.flavor == FL_VARIABLE && module
-	   && (strcmp (module, gsym->mod_name) != 0
-	   || strcmp (sym->name, gsym->sym_name) != 0))
+
+  if (sym->attr.flavor == FL_VARIABLE && module
+  && (strcmp (module, gsym->mod_name) != 0
+	  || strcmp (sym->name, gsym->sym_name) != 0))
 {
   /* This can only happen if the variable is defined in a module - if it
 	 isn't the same module, reject it.  */
@@ -11802,14 +11803,16 @@ gfc_verify_binding_labels (gfc_symbol *sym)
 		 sym->name, module, sym->binding_label,
 		 &sym->declared_at, &gsym->where, gsym->mod_name);
   sym->binding_label = NULL;
+  return;
 }
-  else if ((sym->attr.function || sym->attr.subroutine)
-	   && ((gsym->type != GSYM_SUBROUTINE && gsym->type != GSYM_FUNCTION)
-	   || (gsym->defined && sym->attr.if_source != IFSRC_IFBODY))
-	   && sym != gsym->ns->proc_name
-	   && (module != gsym->mod_name
-	   || strcmp (gsym->sym_name, sym->name) != 0
-	   || (module && strcmp (module, gsym->mod_name) != 0)))
+
+  if ((sym->attr.function || sym->attr.subroutine)
+  && ((gsym->type != GSYM_SUBROUTINE && gsym->type != GSYM_FUNCTION)
+	   || (gsym->defined && sym->attr.if_source != IFSRC_IFBODY))
+  && (sym != gsym->ns->proc_name && sym->attr.entry == 0)
+  && (module != gsym->mod_name
+	  || strcmp (gsym->sym_name, sym->name) != 0
+	  || (module && strcmp (module, gsym->mod_name) != 0)))
 {
   /* Print an error if the procedure is defined multiple times; we have to
 	 exclude references to the same procedure via module association or
Index: gcc/testsuite/gfortran.dg/pr61765.f90
===
--- gcc/testsuite/gfortran.dg/pr61765.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr61765.f90	(working copy)
@@ -0,0 +1,15 @@
+! { dg-do compile }
+   subroutine sub1(x)
+ integer, intent(in) :: x
+ entry sub1_c(x) bind(c)
+   end subroutine sub1
+
+   subroutine sub2_c(x) bind(c)
+ integer, intent(in) :: x
+ entry sub2(x)
+   end subroutine sub2_c
+
+   subroutine sub3_c(x) bind(c)
+ integer, intent(in) :: x
+ entry sub3_c_c(x) bind(c)
+   end subroutine sub3_c

Re: [PATCH] PR88777, Out-of-range offsets building glibc test-tgmath2.c

[Alan Modra]

On Fri, Jan 11, 2019 at 07:49:54PM +, Richard Sandiford wrote:
> FWIW, I think this is papering over a deeper issue,

Most certainly.  At the very least there's the fact that many places
in the compiler that call attr_min_value (via other functions) don't
bother checking for an INT_MAX return.  I also didn't try to analyse
why bb-reorder.c making wrong decisions due to confused copy_bb_p and
similar predicates led to hppa not replacing short branches with
longer ones.  So there are likely to be some final.c issues too.

-- 
Alan Modra
Australia Development Lab, IBM

[PATCH] PR libstdc++/88811 fix typo introduced in r266569

[Jonathan Wakely]

PR libstdc++/88811
PR libstdc++/83306
* src/filesystem/path.cc: Fix typo. If first path is empty, show []
before second path.
* testsuite/experimental/filesystem/filesystem_error/cons.cc: New
test.

Tested x86_64-linux, committed to trunk.


commit 23c3b47b453b8796bbdb75abede5305b1a5435f3
Author: Jonathan Wakely 
Date:   Sat Jan 12 00:02:07 2019 +

PR libstdc++/88811 fix typo introduced in r266569

PR libstdc++/88811
PR libstdc++/83306
* src/filesystem/path.cc: Fix typo. If first path is empty, show []
before second path.
* testsuite/experimental/filesystem/filesystem_error/cons.cc: New
test.

diff --git a/libstdc++-v3/src/filesystem/path.cc 
b/libstdc++-v3/src/filesystem/path.cc
index 048c5617668..92def10e33e 100644
--- a/libstdc++-v3/src/filesystem/path.cc
+++ b/libstdc++-v3/src/filesystem/path.cc
@@ -494,7 +494,7 @@ fs::filesystem_error::_M_gen_what()
   const std::string pstr2 = _M_path2.u8string();
   experimental::string_view s = this->system_error::what();
   const size_t len = 18 + s.length()
-+ (pstr1.length() ? pstr1.length() + 3 : 0)
++ (pstr1.length() || pstr2.length() ? pstr1.length() + 3 : 0)
 + (pstr2.length() ? pstr2.length() + 3 : 0);
   std::string w;
   w.reserve(len);
@@ -506,8 +506,10 @@ fs::filesystem_error::_M_gen_what()
   w += pstr1;
   w += ']';
 }
-  if (!pstr1.empty())
+  if (!pstr2.empty())
 {
+  if (pstr1.empty())
+   w += " []";
   w += " [";
   w += pstr2;
   w += ']';
diff --git 
a/libstdc++-v3/testsuite/experimental/filesystem/filesystem_error/cons.cc 
b/libstdc++-v3/testsuite/experimental/filesystem/filesystem_error/cons.cc
new file mode 100644
index 000..a08852df1e3
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/filesystem_error/cons.cc
@@ -0,0 +1,90 @@
+// Copyright (C) 2018-2019 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 copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++17 -lstdc++fs" }
+// { dg-do run { target c++11 } }
+// { dg-require-filesystem-ts "" }
+
+#include 
+#include 
+
+using std::experimental::filesystem::filesystem_error;
+using std::experimental::filesystem::path;
+
+bool contains(std::string what_str, std::string expected)
+{
+  return what_str.find(expected) != std::string::npos;
+}
+
+void
+test01()
+{
+  const char* const str = "error test";
+  const std::error_code ec = make_error_code(std::errc::is_a_directory);
+  const path p1 = "test/path/one";
+  const path p2 = "/test/path/two";
+
+  const filesystem_error e1(str, ec);
+  VERIFY( contains(e1.what(), str) );
+  VERIFY( !contains(e1.what(), "[]") ); // no "empty path" in the string
+  VERIFY( e1.path1().empty() );
+  VERIFY( e1.path2().empty() );
+  VERIFY( e1.code() == ec );
+
+  const filesystem_error e2(str, p1, ec);
+  VERIFY( e2.path1() == p1 );
+  VERIFY( e2.path2().empty() );
+  VERIFY( contains(e2.what(), str) );
+  VERIFY( contains(e2.what(), p1.string()) );
+  VERIFY( e2.code() == ec );
+
+  const filesystem_error e3(str, path{}, ec);
+  VERIFY( e3.path1().empty() );
+  VERIFY( e3.path2().empty() );
+  VERIFY( contains(e3.what(), str) );
+  VERIFY( e3.code() == ec );
+
+  const filesystem_error e4(str, p1, p2, ec);
+  VERIFY( e4.path1() == p1 );
+  VERIFY( e4.path2() == p2 );
+  VERIFY( contains(e4.what(), str) );
+  VERIFY( contains(e4.what(), p1.string()) );
+  VERIFY( contains(e4.what(), p2.string()) );
+  VERIFY( !contains(e4.what(), "[]") );
+  VERIFY( e4.code() == ec );
+
+  const filesystem_error e5(str, p1, path{}, ec);
+  VERIFY( e5.path1() == p1 );
+  VERIFY( e5.path2().empty() );
+  VERIFY( contains(e5.what(), str) );
+  VERIFY( contains(e5.what(), p1.string()) );
+  VERIFY( e5.code() == ec );
+
+  const filesystem_error e6(str, path{}, p2, ec);
+  VERIFY( e6.path1().empty() );
+  VERIFY( e6.path2() == p2 );
+  VERIFY( contains(e6.what(), str) );
+  VERIFY( contains(e6.what(), "[]") );
+  VERIFY( contains(e6.what(), p2.string()) );
+  VERIFY( e6.code() == ec );
+}
+
+int
+main()
+{
+  test01();
+}

Re: [PATCH] PR88777, Out-of-range offsets building glibc test-tgmath2.c

[Alan Modra]

On Fri, Jan 11, 2019 at 11:42:31AM -0700, Jeff Law wrote:
> On 1/10/19 12:19 AM, Alan Modra wrote:
> > bb-reorder is quite seriously broken if get_attr_min_length should
> > return INT_MAX, which it does for hppa on branches with r267666.
> Presumably you're referring to the overflows and such?

Yes.  Even get_uncond_jump_length would have been INT_MAX.  All of
the predicates deciding on whether to copy or reorder blocks were
therefore broken.

The following is fairly obvious and would stop some of the silliness,
but I guess now is not the time to propose this sort of patch.

* bb-reorder.c (copy_bb_p): Don't overflow size calculation.
(get_uncond_jump_length): Assert length less than INT_MAX and
non-negative.

diff --git a/gcc/bb-reorder.c b/gcc/bb-reorder.c
index e4ae8b89c09..c21d204627e 100644
--- a/gcc/bb-reorder.c
+++ b/gcc/bb-reorder.c
@@ -1357,8 +1357,8 @@ connect_traces (int n_traces, struct trace *traces)
 static bool
 copy_bb_p (const_basic_block bb, int code_may_grow)
 {
-  int size = 0;
-  int max_size = uncond_jump_length;
+  unsigned int size = 0;
+  unsigned int max_size = uncond_jump_length;
   rtx_insn *insn;
 
   if (EDGE_COUNT (bb->preds) < 2)
@@ -1376,7 +1376,11 @@ copy_bb_p (const_basic_block bb, int code_may_grow)
   FOR_BB_INSNS (bb, insn)
 {
   if (INSN_P (insn))
-   size += get_attr_min_length (insn);
+   {
+ size += get_attr_min_length (insn);
+ if (size > max_size)
+   break;
+   }
 }
 
   if (size <= max_size)
@@ -1385,7 +1389,7 @@ copy_bb_p (const_basic_block bb, int code_may_grow)
   if (dump_file)
 {
   fprintf (dump_file,
-  "Block %d can't be copied because its size = %d.\n",
+  "Block %d can't be copied because its size = %u.\n",
   bb->index, size);
 }
 
@@ -1397,7 +1401,7 @@ copy_bb_p (const_basic_block bb, int code_may_grow)
 int
 get_uncond_jump_length (void)
 {
-  int length;
+  unsigned int length;
 
   start_sequence ();
   rtx_code_label *label = emit_label (gen_label_rtx ());
@@ -1405,6 +1409,7 @@ get_uncond_jump_length (void)
   length = get_attr_min_length (jump);
   end_sequence ();
 
+  gcc_assert (length < INT_MAX);
   return length;
 }
 

-- 
Alan Modra
Australia Development Lab, IBM

[PATCH] P0972R0 zero(), min(), and max() should be noexcept

[Jonathan Wakely]

This paper has been included in the C++20 draft, but the changes to add
noexcept can be made unconditionally, to apply for C++11 too.

* include/std/chrono (duration_values::zero(), duration_values::min())
(duration_values::max()): Add noexcept.
(duration::zero(), duration::min(), duration::max()): Likewise.
(time_point::zero(), time_point::min(), time_point::max()): Likewise.
* testsuite/20_util/duration/requirements/noexcept.cc: New test.
* testsuite/20_util/time_point/requirements/noexcept.cc: New test.

Tested powerpc64le-linux, committed to trunk.

commit 98c18da3d74259e038a39c77e3d04494f9395dff
Author: Jonathan Wakely 
Date:   Fri Jan 11 22:45:27 2019 +

P0972R0  zero(), min(), and max() should be noexcept

This paper has been included in the C++20 draft, but the changes to add
noexcept can be made unconditionally, to apply for C++11 too.

* include/std/chrono (duration_values::zero(), 
duration_values::min())
(duration_values::max()): Add noexcept.
(duration::zero(), duration::min(), duration::max()): Likewise.
(time_point::zero(), time_point::min(), time_point::max()): 
Likewise.
* testsuite/20_util/duration/requirements/noexcept.cc: New test.
* testsuite/20_util/time_point/requirements/noexcept.cc: New test.

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 3ac0860df5e..9e63fa9c698 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -273,15 +273,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   struct duration_values
   {
static constexpr _Rep
-   zero()
+   zero() noexcept
{ return _Rep(0); }
 
static constexpr _Rep
-   max()
+   max() noexcept
{ return numeric_limits<_Rep>::max(); }
 
static constexpr _Rep
-   min()
+   min() noexcept
{ return numeric_limits<_Rep>::lowest(); }
   };
 
@@ -428,15 +428,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
// 20.11.5.4 special values
static constexpr duration
-   zero()
+   zero() noexcept
{ return duration(duration_values::zero()); }
 
static constexpr duration
-   min()
+   min() noexcept
{ return duration(duration_values::min()); }
 
static constexpr duration
-   max()
+   max() noexcept
{ return duration(duration_values::max()); }
 
   private:
@@ -666,11 +666,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
// special values
static constexpr time_point
-   min()
+   min() noexcept
{ return time_point(duration::min()); }
 
static constexpr time_point
-   max()
+   max() noexcept
{ return time_point(duration::max()); }
 
   private:
diff --git a/libstdc++-v3/testsuite/20_util/duration/requirements/noexcept.cc 
b/libstdc++-v3/testsuite/20_util/duration/requirements/noexcept.cc
new file mode 100644
index 000..03bad272ff4
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/duration/requirements/noexcept.cc
@@ -0,0 +1,39 @@
+// Copyright (C) 2019 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 copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-do compile { target c++11 } }
+
+// P0972R0  zero(), min(), and max() should be noexcept
+
+#include 
+
+using namespace std;
+
+using vals = chrono::duration_values;
+static_assert( noexcept(vals::zero()), "duration_values::zero()" );
+static_assert( noexcept(vals::min()), "duration_values::min()" );
+static_assert( noexcept(vals::max()), "duration_values::max()" );
+
+using dur1 = chrono::system_clock::duration;
+static_assert( noexcept(dur1::zero()), "duration::zero()" );
+static_assert( noexcept(dur1::min()), "duration::min()" );
+static_assert( noexcept(dur1::max()), "duration::max()" );
+
+using dur2 = chrono::duration>;
+static_assert( noexcept(dur2::zero()), "duration::zero()" );
+static_assert( noexcept(dur2::min()), "duration::min()" );
+static_assert( noexcept(dur2::max()), "duration::max()" );
diff --git a/libstdc++-v3/testsuite/20_util/time_point/requirements/noexcept.cc 
b/libstdc++-v3/testsuite/20_util/time_point/requirements/noexcept.cc
new file mode 100644
index 000..075cbbc6fee
--- /dev/null
+++ b/libstdc++-v3/testsuite/2

[PATCH] Document C++20 library status

[Jonathan Wakely]

* doc/xml/manual/intro.xml: Include new section.
* doc/xml/manual/status_cxx2017.xml: Document more
implementation-defined properties of the library.
* doc/xml/manual/status_cxx2020.xml: Document C++2a status.
* doc/html/*: Regenerate.

Committed to trunk.
commit d24c65771c84e98a2800b053e6594cf85b4a9a69
Author: Jonathan Wakely 
Date:   Fri Jan 11 22:10:09 2019 +

Document C++20 library status

* doc/xml/manual/intro.xml: Include new section.
* doc/xml/manual/status_cxx2017.xml: Document more
implementation-defined properties of the library.
* doc/xml/manual/status_cxx2020.xml: Document C++2a status.
* doc/html/*: Regenerate.

diff --git a/libstdc++-v3/doc/xml/manual/intro.xml 
b/libstdc++-v3/doc/xml/manual/intro.xml
index 7159b592c9f..28210cb0862 100644
--- a/libstdc++-v3/doc/xml/manual/intro.xml
+++ b/libstdc++-v3/doc/xml/manual/intro.xml
@@ -43,6 +43,10 @@
 http://www.w3.org/2001/XInclude"; parse="xml" 
href="status_cxx2017.xml">
 
 
+
+http://www.w3.org/2001/XInclude"; parse="xml" 
href="status_cxx2020.xml">
+
+
 
 http://www.w3.org/2001/XInclude"; parse="xml" 
href="status_cxxtr1.xml">
 
diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml 
b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
index f3793083375..0af8a02b00f 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
@@ -980,6 +980,14 @@ and test for __STDCPP_MATH_SPEC_FUNCS__ >= 
201003L.
   documents behaviour which is new in the 2017 standard.

 
+   
+  20.5.1.2 [headers]
+  Whether names from Annex K are declared by C++ headers depends on
+  whether the underlying C library supports Annex K and declares the
+  names. For the GNU C library, there is no Annex K support and so
+  none of its names are declared by C++ headers.
+   
+

   23.6.5 [optional.bad_optional_access]
   what() returns "bad optional access".
@@ -987,8 +995,7 @@ and test for __STDCPP_MATH_SPEC_FUNCS__ >= 
201003L.
 

   23.7.3 [variant.variant]
-  Whether variant supports over-aligned types
-  should be documented here.
+  variant supports over-aligned types.

 

@@ -998,15 +1005,21 @@ and test for __STDCPP_MATH_SPEC_FUNCS__ >= 
201003L.
 

   23.12.5.2 [memory.resource.pool.options]
-  The limits for maximum number of blocks and largest allocation size
-  supported by pool_options should be documented
-  here.
+  Let S equal numeric_limits::digits.
+  The limit for maximum number of blocks in a chunk is given by
+  
2N-1,
+  where N is min(19, 3 + 
S/2).
+  The largest allocation size that will be allocated from a pool is
+  
222
+  when S > 20,
+  otherwise 3072 when S > 
16,
+  otherwise 768.

 

   23.12.6.1 [memory.resource.monotonic.buffer.ctor]
-  The default next_buffer_size and growth factor should
-  be documented here.
+  The default next_buffer_size is 128 * 
sizeof(void*).
+  The default growth factor is 1.5.

 

@@ -1067,13 +1080,32 @@ and test for __STDCPP_MATH_SPEC_FUNCS__ >= 
201003L.
 

   30.10.5 [fs.filesystem.syn]
-  The clock used for file times is
-  std::chrono::system_clock.
+  The clock used for file times is an unspecified type
+  with a signed 64-bit representation, capable of representing
+  timestamps with nanosecond resolution. The clock's epoch is
+  unspecified, but is not the same as the system clock's epoch.

 

   30.10.7.1 [fs.path.generic]
   dot-dot in the root-directory refers to the root-directory itself.
+  On Windows, a drive specifier such as "C:" or
+  "z:" is treated as a root-name. On Cygwin, a path
+  that begins with two successive directory separators is a
+  root-name. Otherwise (for POSIX-like systems other than Cygwin),
+  the implementation-defined root-name is an unspecified string
+  which does not appear in any pathnames.
+   
+
+   
+  30.10.10.1 [fs.enum.path.format]
+  The character sequence is always interpreted in the native pathname
+  format.
+   
+
+   
+  30.10.15.4 [fs.op.file_size]
+  If !is_regular_file(p), an error is reported.

 
 
diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2020.xml 
b/libstdc++-v3/doc/xml/manual/status_cxx2020.xml
new file mode 100644
index 000..67353850ce5
--- /dev/null
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2020.xml
@@ -0,0 +1,980 @@
+http://docbook.org/ns/docbook"; version="5.0" 
+xml:id="status.iso.2020" xreflabel="Status C++ 2020">
+
+
+C++ 202a
+  
+ISO C++
+2020
+  
+
+
+
+In this implementation the -std=gnu++2a or
+-std=c++2a flag must be used to enable language
+and library
+features. See dialect
+options. The pre-defined symbol
+__cplusplus is used to check for the
+presence

[PATCH] P0357R3 reference_wrapper for incomplete types

[Jonathan Wakely]

This patch implements the C++2a proposal to allow incomplete types in
std::reference_wrapper, which was previously undefined.

The change cannot be implemented for earlier standards, because prior to
C++2a std::reference_wrapper has a weak result type, so must inspect the
template argument to see if it defines a nested result_type member. That
is deprecated (but still required) in C++17, and removed from C++2a.

The removal of the base class from reference_wrapper is a potential ABI
change, as it could alter the layout of a type which derives from
reference_wrapper and from an empty type with _Weak_result_type as
a base class.  Previously the repeated _Weak_result_type base class
would have prevented the empty base-class optimization, but if
reference_wrapper no longer derives from it, the empty class could be
placed at the same address as the reference_wrapper base.  In
practice, the only types which derive from _Weak_result_type or from
_Reference_wrapper_base_memfun or any of its base classes are non-empty
types defined in libstdc++: std::reference_wrapper, std::function, and
std::_Bind. As they are non-empty types, they are not eligible for EBO
anyway.

* include/bits/refwrap.h [__cplusplus > 201703L]
(_Refwrap_base_arg1, _Refwrap_base_arg2, _Reference_wrapper_base)
(_Reference_wrapper_base_memfun): Do not define for C++2a.
(reference_wrapper): Do not derive from _Reference_wrapper_base_memfun
for C++2a.
(reference_wrapper::operator()): Add static assertion.
* testsuite/20_util/reference_wrapper/incomplete.cc: New test.

Tested powerpc64le-linux, committed to trunk.

commit d41fa8998e6b9760b4946df464296a2472e5fc06
Author: Jonathan Wakely 
Date:   Fri Jan 11 22:56:01 2019 +

P0357R3 reference_wrapper for incomplete types

This patch implements the C++2a proposal to allow incomplete types in
std::reference_wrapper, which was previously undefined.

The change cannot be implemented for earlier standards, because prior to
C++2a std::reference_wrapper has a weak result type, so must inspect the
template argument to see if it defines a nested result_type member. That
is deprecated (but still required) in C++17, and removed from C++2a.

The removal of the base class from reference_wrapper is a potential ABI
change, as it could alter the layout of a type which derives from
reference_wrapper and from an empty type with _Weak_result_type as
a base class.  Previously the repeated _Weak_result_type base class
would have prevented the empty base-class optimization, but if
reference_wrapper no longer derives from it, the empty class could be
placed at the same address as the reference_wrapper base.  In
practice, the only types which derive from _Weak_result_type or from
_Reference_wrapper_base_memfun or any of its base classes are non-empty
types defined in libstdc++: std::reference_wrapper, std::function, and
std::_Bind. As they are non-empty types, they are not eligible for EBO
anyway.

* include/bits/refwrap.h [__cplusplus > 201703L]
(_Refwrap_base_arg1, _Refwrap_base_arg2, _Reference_wrapper_base)
(_Reference_wrapper_base_memfun): Do not define for C++2a.
(reference_wrapper): Do not derive from 
_Reference_wrapper_base_memfun
for C++2a.
(reference_wrapper::operator()): Add static assertion.
* testsuite/20_util/reference_wrapper/incomplete.cc: New test.

diff --git a/libstdc++-v3/include/bits/refwrap.h 
b/libstdc++-v3/include/bits/refwrap.h
index 5299b212510..6b4335a22ac 100644
--- a/libstdc++-v3/include/bits/refwrap.h
+++ b/libstdc++-v3/include/bits/refwrap.h
@@ -175,6 +175,7 @@ _GLIBCXX_MEM_FN_TRAITS(&& noexcept, false_type, true_type)
 : _Weak_result_type_memfun::type>
 { };
 
+#if __cplusplus <= 201703L
   // Detect nested argument_type.
   template>
 struct _Refwrap_base_arg1
@@ -279,6 +280,7 @@ _GLIBCXX_MEM_FN_TRAITS(&& noexcept, false_type, true_type)
 {
   using result_type = typename _Mem_fn_traits<_MemFunPtr>::__result_type;
 };
+#endif // ! C++20
 
   /**
*  @brief Primary class template for reference_wrapper.
@@ -287,7 +289,11 @@ _GLIBCXX_MEM_FN_TRAITS(&& noexcept, false_type, true_type)
*/
   template
 class reference_wrapper
+#if __cplusplus <= 201703L
+// In C++20 std::reference_wrapper allows T to be incomplete,
+// so checking for nested types could result in ODR violations.
 : public _Reference_wrapper_base_memfun::type>
+#endif
 {
   _Tp* _M_data;
 
@@ -327,6 +333,9 @@ _GLIBCXX_MEM_FN_TRAITS(&& noexcept, false_type, true_type)
typename result_of<_Tp&(_Args&&...)>::type
operator()(_Args&&... __args) const
{
+#if __cplusplus > 201703L
+ static_assert(sizeof(type), "type must be complete");
+#endif
  return std::__invoke(get(), std::forward<_Args>(__args)...);
}
 

Re: [PATCH] Define __cpp_lib_erase_if feature test macro

[Jonathan Wakely]

On 10/01/19 13:49 +, Jonathan Wakely wrote:

The C++2a draft specifies the value 201811L for this, but as an
extension we return the number of elements erased. This is expected to
be standardised, so the macro has the value 201900L until a proper value
is specified in the draft.

* include/bits/erase_if.h: Define __cpp_lib_erase_if.
* include/std/deque: Likewise.
* include/std/forward_list: Likewise.
* include/std/list: Likewise.
* include/std/string: Likewise.
* include/std/vector: Likewise.
* include/std/version: Likewise.
* testsuite/21_strings/basic_string/erasure.cc: Test macro.
* testsuite/23_containers/deque/erasure.cc: Likewise.
* testsuite/23_containers/forward_list/erasure.cc: Likewise.
* testsuite/23_containers/list/erasure.cc: Likewise.
* testsuite/23_containers/map/erasure.cc: Likewise.
* testsuite/23_containers/set/erasure.cc: Likewise.
* testsuite/23_containers/unordered_map/erasure.cc: Likewise.
* testsuite/23_containers/unordered_set/erasure.cc: Likewise.
* testsuite/23_containers/vector/erasure.cc: Likewise.

Tested x86_64-linux, committed to trunk.






diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index f090fba0308..e11ae3a688a 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -96,6 +96,7 @@
#define __cpp_lib_clamp 201603
#define __cpp_lib_constexpr_char_traits 201611
#define __cpp_lib_enable_shared_from_this 201603
+#define __cpp_lib_erase_if 201900L
#define __cpp_lib_filesystem 201703
#define __cpp_lib_gcd 201606
#define __cpp_lib_gcd_lcm 201606


I put this in the C++17 group, but it's a C++2a feature.

Fixed with this patch, committed to trunk.


commit e60c82dfba1812b17d427382239cec4d7341bc75
Author: Jonathan Wakely 
Date:   Fri Jan 11 22:29:21 2019 +

Fix location of __cpp_lib_erase_if macro

This macro should only be defined for C++2a, not C++17.

* include/std/version (__cpp_lib_erase_if): Move to C++20 group.

diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index 903b75b483a..e9a1f1251af 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -96,7 +96,6 @@
 #define __cpp_lib_clamp 201603
 #define __cpp_lib_constexpr_char_traits 201611
 #define __cpp_lib_enable_shared_from_this 201603
-#define __cpp_lib_erase_if 201900L
 #define __cpp_lib_filesystem 201703
 #define __cpp_lib_gcd 201606
 #define __cpp_lib_gcd_lcm 201606
@@ -142,6 +141,7 @@
 
 #if __cplusplus > 201703L
 // c++2a
+#define __cpp_lib_erase_if 201900L
 #ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
 # define __cpp_lib_is_constant_evaluated 201811L
 #endif

Re: [PATCH v3 00/10] AMD GCN Port v3

[Jeff Law]

On 1/7/19 4:58 AM, Richard Biener wrote:
> On Mon, Jan 7, 2019 at 11:48 AM Andrew Stubbs  wrote:
>>
>> New year ping!
>>
>> The last remaining middle-end patch was already applied, so it's only
>> the backend, config, and testsuite patches remaining to be committed.
>> And, it's mostly only the back end still requiring review.
>>
>> Hopefully I should still be able to get them reviewed and committed in
>> time for GCC 9, given that disruption to other targets should no longer
>> be an issue?
> 
> Yes, it's OK to add during stage4 once approved.
And I think the V3 patch is reasonable enough to go in now.  There's
some concerns that have been raised with the implementation, but I'm
comfortable with Andrew faulting in fixes if those concerns turn into
real issues.

Andrew, you're green-lighted for the trunk.

jeff

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Marek Polacek]

On Fri, Jan 11, 2019 at 04:23:23PM -0500, Jason Merrill wrote:
> On 1/11/19 4:21 PM, Marek Polacek wrote:
> > On Fri, Jan 11, 2019 at 01:55:09PM -0500, Jason Merrill wrote:
> > > On 1/11/19 11:09 AM, Marek Polacek wrote:
> > > > On Mon, Jan 07, 2019 at 04:59:14PM -0500, Jason Merrill wrote:
> > > > > On 1/7/19 4:29 PM, Marek Polacek wrote:
> > > > > > This patch fixes bogus -Wredundant-move warnings reported in 88692 
> > > > > > and 87882.
> > > > > > 
> > > > > > To quickly recap, this warning is supposed to warn for cases like
> > > > > > 
> > > > > > struct T { };
> > > > > > 
> > > > > > T fn(T t)
> > > > > > {
> > > > > >  return std::move (t);
> > > > > > }
> > > > > > 
> > > > > > where NRVO isn't applicable for T because it's a parameter, but it's
> > > > > > a local variable and we're returning, so C++11 says activate move
> > > > > > semantics, so the std::move is redundant.  But, as these testcases 
> > > > > > show,
> > > > > > we're failing to realize that that is not the case when returning 
> > > > > > *this,
> > > > > > which is disguised as an ordinary PARM_DECL, and 
> > > > > > treat_lvalue_as_rvalue_p
> > > > > > was fooled by that.
> > > > > 
> > > > > Hmm, the function isn't returning 'this', it's returning '*this'.  I 
> > > > > guess
> > > > > what's happening is that in order to pass *this to the reference 
> > > > > parameter
> > > > > of move, we end up converting it from pointer to reference by 
> > > > > NOP_EXPR, and
> > > > > the STRIP_NOPS in maybe_warn_pessimizing_move throws that away so 
> > > > > that it
> > > > > then thinks we're returning 'this'.  I expect the same thing could 
> > > > > happen
> > > > > with any parameter of pointer-to-class type.
> > > > 
> > > > You're right, I didn't realize that we warned even for parameters of 
> > > > pointer-to-class
> > > > types.  So why don't we disable the warning for PARM_DECLs with pointer 
> > > > types?
> > > 
> > > std::move is certainly redundant for parms of pointer type (or other 
> > > scalar
> > > type), so we might still want to warn about 'return std::move(this)'.  The
> > > problem here is that we're discarding the indirection, so we aren't 
> > > actually
> > > considering the returned expression.
> > 
> > We won't warn for 'return std::move(this)' in any case becase
> > maybe_warn_pessimizing_move returns for non-class types.  This is in line 
> > with
> > what clang does.  I'm not sure if we should change that; I'd rather not.
> > > Is the STRIP_NOPS really necessary?  It seems we shouldn't remove a 
> > > NOP_EXPR
> > > from pointer to reference.
> > 
> > I think we need that in order to make can_do_nrvo_p/treat_lvalue_as_rvalue_p
> > work.  Given the outermost NOP_EXPR will always be of reference type, how 
> > about
> > just returning if, after STRIP_NOPS, we don't see an ADDR_EXPR?  That fixes 
> > the
> > testcases I've been meaning to fix and doesn't regress anything.  I.e., 
> > this:
> > 
> > --- a/gcc/cp/typeck.c
> > +++ b/gcc/cp/typeck.c
> > @@ -9412,8 +9412,9 @@ maybe_warn_pessimizing_move (tree retval, tree 
> > functype)
> >  {
> >tree arg = CALL_EXPR_ARG (fn, 0);
> >STRIP_NOPS (arg);
> > - if (TREE_CODE (arg) == ADDR_EXPR)
> > -   arg = TREE_OPERAND (arg, 0);
> > + if (TREE_CODE (arg) != ADDR_EXPR)
> > +   return;
> > + arg = TREE_OPERAND (arg, 0);
> >arg = convert_from_reference (arg);
> >/* Warn if we could do copy elision were it not for the move.  */
> >if (can_do_nrvo_p (arg, functype))
> > 
> > I can test/post the complete patch.  Or am I again missing something 
> > obvious? :)
> 
> That looks good.  OK if it passes.

Which it did, so I'm installing the following.  Thanks!

2019-01-11  Marek Polacek  

PR c++/88692, c++/87882 - -Wredundant-move false positive with *this.
* typeck.c (maybe_warn_pessimizing_move): Return if ARG isn't
ADDR_EXPR.

* g++.dg/cpp0x/Wredundant-move5.C: New test.
* g++.dg/cpp0x/Wredundant-move6.C: New test.

diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index e399cd3fe45..43d2899a3c4 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9412,8 +9412,9 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
{
  tree arg = CALL_EXPR_ARG (fn, 0);
  STRIP_NOPS (arg);
- if (TREE_CODE (arg) == ADDR_EXPR)
-   arg = TREE_OPERAND (arg, 0);
+ if (TREE_CODE (arg) != ADDR_EXPR)
+   return;
+ arg = TREE_OPERAND (arg, 0);
  arg = convert_from_reference (arg);
  /* Warn if we could do copy elision were it not for the move.  */
  if (can_do_nrvo_p (arg, functype))
diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C 
gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C
new file mode 100644
index 000..0e2ec46d11e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C
@@ -0,0 +1,53 @@
+// PR c++/88692
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wredundant-move"

Go patch committed: Pad structs that end with a zero-sized field

[Ian Lance Taylor]

This patch by Cherry Zhang changes the Go frontend to pad structs
ending with a zero-sized field.  For a struct with zero-sized last
field, the address of the field falls out of the object boundary,
which confuses the garbage collector.  Pad an extra byte in this case.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 267789)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-960637781ca9546ea2db913e48afd7eccbdadfa9
+0d64279c01a37b2579c0c62ca4f2c3e3f81de07c
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 267789)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -13082,6 +13082,12 @@ Struct_construction_expression::do_get_b
  ++pv;
}
 }
+  if (this->type_->struct_type()->has_padding())
+{
+  // Feed an extra value if there is a padding field.
+  Btype *fbtype = Type::lookup_integer_type("uint8")->get_backend(gogo);
+  init.push_back(gogo->backend()->zero_expression(fbtype));
+}
   return gogo->backend()->constructor_expression(btype, init, 
this->location());
 }
 
Index: gcc/go/gofrontend/types.cc
===
--- gcc/go/gofrontend/types.cc  (revision 267789)
+++ gcc/go/gofrontend/types.cc  (working copy)
@@ -24,8 +24,7 @@
 // backend.h.
 
 static void
-get_backend_struct_fields(Gogo* gogo, const Struct_field_list* fields,
- bool use_placeholder,
+get_backend_struct_fields(Gogo* gogo, Struct_type* type, bool use_placeholder,
  std::vector* bfields);
 
 static void
@@ -1162,8 +1161,7 @@ Type::get_backend_placeholder(Gogo* gogo
   // struct field.
   {
std::vector bfields;
-   get_backend_struct_fields(gogo, this->struct_type()->fields(),
- true, &bfields);
+   get_backend_struct_fields(gogo, this->struct_type(), true, &bfields);
bt = gogo->backend()->struct_type(bfields);
   }
   break;
@@ -6140,12 +6138,14 @@ Struct_type::interface_method_table(Inte
 // backend.h.
 
 static void
-get_backend_struct_fields(Gogo* gogo, const Struct_field_list* fields,
- bool use_placeholder,
+get_backend_struct_fields(Gogo* gogo, Struct_type* type, bool use_placeholder,
  std::vector* bfields)
 {
+  const Struct_field_list* fields = type->fields();
   bfields->resize(fields->size());
   size_t i = 0;
+  int64_t lastsize = 0;
+  bool saw_nonzero = false;
   for (Struct_field_list::const_iterator p = fields->begin();
p != fields->end();
++p, ++i)
@@ -6155,8 +6155,24 @@ get_backend_struct_fields(Gogo* gogo, co
 ? p->type()->get_backend_placeholder(gogo)
 : p->type()->get_backend(gogo));
   (*bfields)[i].location = p->location();
+  lastsize = gogo->backend()->type_size((*bfields)[i].btype);
+  if (lastsize != 0)
+saw_nonzero = true;
 }
   go_assert(i == fields->size());
+  if (saw_nonzero && lastsize == 0)
+{
+  // For nonzero-sized structs which end in a zero-sized thing, we add
+  // an extra byte of padding to the type. This padding ensures that
+  // taking the address of the zero-sized thing can't manufacture a
+  // pointer to the next object in the heap. See issue 9401.
+  size_t n = fields->size();
+  bfields->resize(n + 1);
+  (*bfields)[n].name = "_";
+  (*bfields)[n].btype = 
Type::lookup_integer_type("uint8")->get_backend(gogo);
+  (*bfields)[n].location = (*bfields)[n-1].location;
+  type->set_has_padding();
+}
 }
 
 // Get the backend representation for a struct type.
@@ -6165,7 +6181,7 @@ Btype*
 Struct_type::do_get_backend(Gogo* gogo)
 {
   std::vector bfields;
-  get_backend_struct_fields(gogo, this->fields_, false, &bfields);
+  get_backend_struct_fields(gogo, this, false, &bfields);
   return gogo->backend()->struct_type(bfields);
 }
 
@@ -10504,8 +10520,7 @@ Named_type::convert(Gogo* gogo)
 case TYPE_STRUCT:
   {
std::vector bfields;
-   get_backend_struct_fields(gogo, base->struct_type()->fields(),
- true, &bfields);
+   get_backend_struct_fields(gogo, base->struct_type(), true, &bfields);
if (!gogo->backend()->set_placeholder_struct_type(bt, bfields))
  bt = gogo->backend()->error_type();
   }
Index: gcc/go/gofrontend/types.h
===
--- gcc/go/gofrontend/types.h   (revision 267789)
+++ gcc/go/gofrontend/types.h   (working copy)
@@ -2432,7 +2432,7 @@ class 

Re: [PATCH] [RFC] PR target/52813 and target/11807

[Jeff Law]

On 1/8/19 5:03 AM, Richard Sandiford wrote:
> Bernd Edlinger  writes:
>> On 1/7/19 10:23 AM, Jakub Jelinek wrote:
>>> On Sun, Dec 16, 2018 at 06:13:57PM +0200, Dimitar Dimitrov wrote:
 -  /* Clobbering the STACK POINTER register is an error.  */
 +  /* Clobbered STACK POINTER register is not saved/restored by GCC,
 + which is often unexpected by users.  See PR52813.  */
if (overlaps_hard_reg_set_p (regset, Pmode, STACK_POINTER_REGNUM))
  {
 -  error ("Stack Pointer register clobbered by %qs in %", 
 regname);
 +  warning (0, "Stack Pointer register clobbered by %qs in %",
 + regname);
 +  warning (0, "GCC has always ignored Stack Pointer % 
 clobbers");
>>>
>>> Why do we write Stack Pointer rather than stack pointer?  That is really
>>> weird.  The second warning would be a note based on the first one, i.e.
>>> if (warning ()) note ();
>>> and better have some -W* option to silence the warning.
>>>
>>
>> Yes, thanks for this suggestion.
>>
>> Meanwhile I found out, that the stack clobber has only been ignored up to
>> gcc-5 (at least with lra targets, not really sure about reload targets).
>> From gcc-6 on, with the exception of PR arm/77904 which was a regression due
>> to the underlying lra change, but fixed later, and back-ported to gcc-6.3.0,
>> this works for all targets I tried so far.
>>
>> To me, it starts to look like a rather unique and useful feature, that I 
>> would
>> like to keep working.
> 
> Not sure what you mean by "unique".  But forcing a frame is a bit of
> a slippery concept.  Force it where?  For the asm only, or the whole
> function?  This depends on optimisation and hasn't been consistent
> across GCC versions, since it depends on the shrink-wrapping
> optimisation.  (There was a similar controversy a while ago about
> to what extent -fno-omit-frame-pointer should "force a frame".)
> 
> The effect on the redzone seems like something that should be specified
> explicitly rather than as an (accidental?) side effect of listing the
> sp in the clobber list.  Maybe this would be another use for the "asm
> attributes" proposal.  "noreturn" was another attribute suggested on
> IRC yesterday.
> 
> But either way, the general feeling seems to be that going straight to a
> hard error is too harsh, since there's quite a bit of existing code that
> has the clobber.  This patch implements the compromise discussed on IRC
> yesterday of making it a -Wdeprecated warning instead.
> 
> Tested on x86_64-linux-gnu and aarch64-linux-gnu.  OK to install?
> 
> Richard
> 
> Dimitar: sorry the run-around on this patch, and thanks for the
> submission.  It looks from all the controversy like it was a
> long-festering PR for a reason. :-/
> 
> 
> 2019-01-07  Richard Sandiford  
> 
> gcc/
>   PR inline-asm/52813
>   * doc/extend.texi: Document that listing the stack pointer in the
>   clobber list of an asm is a deprecated feature.
>   * common.opt (Wdeprecated): Moved from c-family/c.opt.
>   * cfgexpand.c (asm_clobber_reg_is_valid): Issue a -Wdeprecated
>   warning instead of an error for clobbers of the stack pointer.
>   Add a note explaining why.
> 
> gcc/c-family/
>   PR inline-asm/52813
>   * c.opt (Wdeprecated): Move documentation and variable to common.opt.
> 
> gcc/d/
>   PR inline-asm/52813
>   * lang.opt (Wdeprecated): Reference common.opt instead of c.opt.
> 
> gcc/testsuite/
>   PR inline-asm/52813
>   * gcc.target/i386/pr52813.c (test1): Turn the diagnostic into a
>   -Wdeprecated warning and expect a following note:.
OK.

FWIW the number of packages affected in Fedora was in single digits,
some of which have already been fixed.

But if folks want to go with a deprecated warning instead of straight to
an error, I won't complain.

jeff

Re: [PATCH, GCC] PR target/86487: fix the way 'uses_hard_regs_p' handles paradoxical subregs

[Jeff Law]

On 1/8/19 8:21 AM, Andre Vieira (lists) wrote:
> 
> 
> On 07/01/2019 22:50, Jeff Law wrote:
>> On 1/7/19 7:42 AM, Andre Vieira (lists) wrote:
>>> Hi,
>>>
>>> This patch fixes the way 'uses_hard_regs_p' handles paradoxical subregs.
>>>   The function is supposed to detect whether a register access of 'x'
>>> overlaps with 'set'.  For SUBREGs it should check whether any of the
>>> full multi-register overlaps with 'set'.  The former behavior used to
>>> grab the widest mode of the inner/outer registers of a SUBREG and the
>>> inner register, and check all registers from the inner-register onwards
>>> for the given width.  For normal SUBREGS this gives you the full
>>> register, for paradoxical SUBREGS however it may give you the wrong set
>>> of registers if the index is not the first of the multi-register set.
>>>
>>> The original error reported in PR target/86487 can no longer be
>>> reproduced with the given test, this was due to an unrelated code-gen
>>> change, regardless I believe this should still be fixed as it is simply
>>> wrong behavior by uses_hard_regs_p which may be triggered by a different
>>> test-case or by future changes to the compiler.  Also it is useful to
>>> point out that this isn't actually a 'target' issue as this code could
>>> potentially hit any other target using paradoxical SUBREGS.  Should I
>>> change the Bugzilla ticket to reflect this is actually a target agnostic
>>> issue in RTL?
>>>
>>> There is a gotcha here, I don't know what would happen if you hit the
>>> cases of get_hard_regno where it would return -1, quoting the comment
>>> above that function "If X is not a register or a subreg of a register,
>>> return -1." though I think if we are hitting this then things must have
>>> gone wrong before?
>>>
>>> Bootstrapped on aarch64, arm and x86, no regressions.
>>>
>>> Is this OK for trunk?
>>>
>>>
>>> gcc/ChangeLog:
>>> 2019-01-07 Andre Vieira  
>>>
>>>
>>>  PR target/86487
>>>  * lra-constraints.c(uses_hard_regs_p): Fix handling of
>>> paradoxical SUBREGS.
>> But doesn't wider_subreg_mode give us the wider of the two modes here
>> and we use that wider mode when we call overlaps_hard_reg_set_p which
>> should ultimately check all the registers in the paradoxical.
>>
>> I must be missing something here?!?
>>
>> jeff
>>
> 
> Hi Jeff,
> 
> It does give us the wider of the two modes, but we also then grab the
> "subreg" of the paradoxical subreg.  If you look at the first example
> case of the bugzilla ticket, for an older gcc (say gcc-8) and the
> options provided (using big-endian), it will generate the following subreg:
> (subreg:DI (reg:SI 122) 0)
> 
> This paradoxical subreg represents a register pair r0-r1, where because
> of big-endian and subgreg index 0, r1 is the value we care about and r0
> the one we say "it can be whatever" by using this paradoxical subreg.
> 
> When 'uses_hard_regs_p' sees this as a subreg, it sets 'mode' to the
> wider, i.e. DImode, but it also sets 'x' to the subreg i.e. 'reg:SI
> 122', for which get_hard_regno correctly returns 'r1'.  But if you now
> pass 'overlaps_hard_reg_set_p' DImode and 'r1', it will check whether
> 'set' contains either 'r1-r2', and not 'r1'r0'.
> 
> To reproduce this again I now applied this patch to GCC 8 and found an
> issue with it. 'REG_P (x)' returns false if x is a 'SUBREG'. So I will
> need to change the later check to also include 'SUBREG_P (x)', I guess I
> was testing with a too new version of gcc that didn't lead to the bogus
> register allocation...
> 
> Which really encourages me to add some sort of testcase, but I'd very
> much like to come up with a less flaky one, we basically need to force
> the generation of a paradoxical subreg 'x', where 'get_hard_regno
> (SUBREG_REG (x)) != get_hard_regno (x)'.  This will cause
> 'uses_hard_regs_p' to give you a wrong answer.
BTW, you might look at 87305 which is another problem with big endian
pseudos and paradoxical subregs that Vlad just fixed.

jeff

[C++ PATCH] PR c++/88312 - pack expansion of decltype.

[Jason Merrill]

The standard doesn't really talk about an expression depending on the number
of elements of a pack, but that's definitely an important form of template
argument dependence.

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

* pt.c (instantiation_dependent_r): A template non-type parameter
pack is instantiation-dependent.
---
 gcc/cp/pt.c | 2 ++
 gcc/testsuite/g++.dg/cpp0x/variadic-decltype1.C | 9 +
 gcc/cp/ChangeLog| 6 ++
 3 files changed, 17 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic-decltype1.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 84f59a2c356..f062a2b9707 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -25736,6 +25736,8 @@ instantiation_dependent_r (tree *tp, int *walk_subtrees,
 case TEMPLATE_PARM_INDEX:
   if (dependent_type_p (TREE_TYPE (*tp)))
return *tp;
+  if (TEMPLATE_PARM_PARAMETER_PACK (*tp))
+   return *tp;
   /* We'll check value-dependence separately.  */
   return NULL_TREE;
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-decltype1.C 
b/gcc/testsuite/g++.dg/cpp0x/variadic-decltype1.C
new file mode 100644
index 000..c87c6bad59f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic-decltype1.C
@@ -0,0 +1,9 @@
+// PR c++/88555
+// { dg-do compile { target c++11 } }
+
+template  struct T {};
+
+template 
+void test() {
+using Test = T;
+}
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 9208f1e4937..0eb669b3eac 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,9 @@
+2019-01-11  Jason Merrill  
+
+   PR c++/88312 - pack expansion of decltype.
+   * pt.c (instantiation_dependent_r): A template non-type parameter
+   pack is instantiation-dependent.
+
 2019-01-11  Jason Merrill  
 
PR c++/88613 - ICE with use of const var in lambda.

base-commit: 0c2ebbc4659d15c2a546b75323ba6af1ff1d073b
-- 
2.20.1

[C++ PATCH] PR c++/88613 - ICE with use of const var in lambda.

[Jason Merrill]

The issue here was that we were cp_folding a location wrapper around a
lambda capture proxy before it had been mark_rvalue_used.  I considered
adding mark_rvalue_use calls to build_new_op_1, but it seems appropriate to
have them in cp_fold_maybe_rvalue when we know we're trying to produce an
rvalue.

The change to mark_use is for a related issue: when we change the operand of
the location wrapper from VAR_DECL to INTEGER_CST, we need the TREE_CODE of
the location wrapper to change as well, from VIEW_CONVERT_EXPR to
NON_LVALUE_EXPR.

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

* expr.c (mark_use): Fix location wrapper handling.
* cp-gimplify.c (cp_fold_maybe_rvalue): Call mark_rvalue_use.
---
 gcc/cp/cp-gimplify.c|  2 ++
 gcc/cp/expr.c   | 17 +++--
 .../g++.dg/cpp0x/lambda/lambda-const10.C| 11 +++
 gcc/cp/ChangeLog|  6 ++
 4 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const10.C

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 726adac4f4e..121dfa41d8d 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2124,6 +2124,8 @@ cp_fold_maybe_rvalue (tree x, bool rval)
   while (true)
 {
   x = cp_fold (x);
+  if (rval)
+   x = mark_rvalue_use (x);
   if (rval && DECL_P (x)
  && !TYPE_REF_P (TREE_TYPE (x)))
{
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index 071c6fb9205..9160043ed11 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -187,10 +187,23 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
}
   break;
 
-CASE_CONVERT:
 case VIEW_CONVERT_EXPR:
   if (location_wrapper_p (expr))
-   loc = EXPR_LOCATION (expr);
+   {
+ loc = EXPR_LOCATION (expr);
+ tree op = TREE_OPERAND (expr, 0);
+ tree nop = RECUR (op);
+ if (nop == error_mark_node)
+   return error_mark_node;
+ TREE_OPERAND (expr, 0) = nop;
+ /* If we're replacing a DECL with a constant, we also need to change
+the TREE_CODE of the location wrapper.  */
+ if (op != nop && rvalue_p)
+   TREE_SET_CODE (expr, NON_LVALUE_EXPR);
+ return expr;
+   }
+  gcc_fallthrough();
+CASE_CONVERT:
   recurse_op[0] = true;
   break;
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const10.C 
b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const10.C
new file mode 100644
index 000..3112f08f5c2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const10.C
@@ -0,0 +1,11 @@
+// PR c++/88613
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -Wtautological-compare }
+
+void a() {
+  const int b = 5;
+  [=] {
+if (b != 5)
+  ;
+  }();
+}
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 4b9fa821d74..74e4dff494e 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,9 @@
+2019-01-11  Jason Merrill  
+
+   PR c++/88613 - ICE with use of const var in lambda.
+   * expr.c (mark_use): Fix location wrapper handling.
+   * cp-gimplify.c (cp_fold_maybe_rvalue): Call mark_rvalue_use.
+
 2019-01-11  Paolo Carlini  
 
* decl.c (start_decl): Improve error location.

base-commit: ead1f4b6412011a79e84db928f6a98dd7cf09a3d
-- 
2.20.1

Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI

[Steve Ellcey]

On Fri, 2019-01-11 at 14:45 +, Richard Sandiford wrote:
> 
> > +
> > +/* Return true for types that could be supported as SIMD return or
> > +   argument types.  */
> > +
> > +static bool supported_simd_type (tree t)
> > +{
> > +  return (FLOAT_TYPE_P (t) || INTEGRAL_TYPE_P (t));
> 
> We should also check that the size is 1, 2, 4 or 8 bytes.

I fixed this, I also allow for POINTER_P types which allowed me
to not do the POINTER_P check below which you asked about and
which I now think was a mistake (more comments below).

> > 
> > +   wmsg = G_("unsupported return type return type %qT for simd");
> 
> Typo: doubled "return type".

Fixed.
> 
> Maybe s/for simd/for % functions/ in both messages.
> Since that will blow the line limit...
> 
> > +  warning_at (DECL_SOURCE_LOCATION (node->decl), 0, wmsg,
> > ret_type);
> > +  return 0;
> > +}
> 
> ...it's probably worth just calling warning_at in each arm of the
> "if".
> We'll then get proper format checking in bootstraps.

I made this change.

> > +  for (t = DECL_ARGUMENTS (node->decl); t; t = DECL_CHAIN (t))
> > +{
> > +  arg_type = TREE_TYPE (t);
> > +  if (POINTER_TYPE_P (arg_type))
> > +   arg_type = TREE_TYPE (arg_type);
> > +  if (!currently_supported_simd_type (arg_type))
> > +   {
> > + if (supported_simd_type (arg_type))
> > +   wmsg = G_("GCC does not currently support argument type %qT
> > "
> > + "for simd");
> > + else
> > +   wmsg = G_("unsupported argument type %qT for simd");
> > + warning_at (DECL_SOURCE_LOCATION (node->decl), 0, wmsg,
> > arg_type);
> > + return 0;
> > +   }
> > +}
> 
> The ABI supports all argument types, so this should only check
> current_supported_simd_type and should always use the "GCC does
> not..."
> message.

Done.

> Could you explain why the POINTER_TYPE_P handling is correct?
> Think it's worth a comment.  Dropping it is also fine if that's
> easier.

I now think this was a mistake but after removing it I had to allow
for POINTER_P type in supported_simd_type to get the regression
tests to pass.  I think the current code is the correct behavour
and more closely matches x86 in terms of what is and is not vectorized.


> > +  if (clonei->simdlen == 0)
> > +{
> > +  if (SCALAR_INT_MODE_P (TYPE_MODE (base_type)))
> > +   clonei->simdlen = clonei->vecsize_int;
> > +  else
> > +   clonei->simdlen = clonei->vecsize_float;
> > +  clonei->simdlen /= GET_MODE_BITSIZE (SCALAR_TYPE_MODE (base_type));
> > +  return 1;
> > +}
> 
> I should have noticed this last time, but base_type is the CDT in the
> Intel ABI.  That isn't always right for the AArch64 ABI.
> 
> I think for now currently_supported_simd_type should take base_type
> as a second parameter and check that the given type has the same
> size.

I have not changed this, I am not quite sure what you mean.  What is
CDT?  Clone data type?  Are you saying I should use node->decl->type
instead of base_type?

> 
> > +  /* Restrict ourselves to vectors that fit in a single
> > register  */
> > +
> > +  gcc_assert (tree_fits_shwi_p (TYPE_SIZE (base_type)));
> > +  vsize = clonei->simdlen * tree_to_shwi (TYPE_SIZE (base_type));
> > +  if (vsize > 128)
> > +{
> > + warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
> > + "GCC does not currently support simdlen %d for
> > type %qT",
> > + clonei->simdlen, base_type);
> > + return 0;
> > +}
> 
> nit: block contents indented too far.

Fixed.

> > +  return 0;
> > +}
> 
> Doesn't this mean that we always silently fail for an explicit and
> correct simdlen?  If so, that suggests we might not have enough
> testsuite coverage :-)

Well, we were failing here and some tests were failing but then I
'fixed' the tests in my last patch to pass instead of fixing this
bug.  I have now changed it to 'return 1' and re-fixed the tests
that I incorrectly fixed last time.  So at least it wasn't a silent
fail (until I silenced it).

Here is the latest patch, any help you can give me on the base_type
issue would be appreciated.


2018-01-11  Steve Ellcey  

* config/aarch64/aarch64.c (cgraph.h): New include.
(intl.h): New include.
(supported_simd_type): New function.
(currently_supported_simd_type): Ditto.
(aarch64_simd_clone_compute_vecsize_and_simdlen): Ditto.
(aarch64_simd_clone_adjust): Ditto.
(aarch64_simd_clone_usable): Ditto.
(TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN): New macro.
(TARGET_SIMD_CLONE_ADJUST): Ditto.
(TARGET_SIMD_CLONE_USABLE): Ditto.
* config/i386/i386.c (ix86_simd_clone_adjust): Add definition check.
* omp-simd-clone.c (expand_simd_clones): Add targetm.simd_clone.adjust
call.


2018-01-11  Steve Ellcey  

* g++.dg/gomp/declare-simd-1.C: Add aarch64 specific
warning checks and assembler scans.
* g++.dg/gomp/declare-simd-3.C: Ditto.
* g++.dg/gomp/d

Re: [C++ Patch] Fix three additional locations

[Jason Merrill]

On 1/11/19 4:56 PM, Paolo Carlini wrote:

Hi,

On 11/01/19 22:22, Jason Merrill wrote:

On 1/11/19 4:13 PM, Paolo Carlini wrote:

Hi,

On 11/01/19 19:58, Jason Merrill wrote:

On 1/10/19 9:24 AM, Paolo Carlini wrote:

Hi again,

this one is also matter of consistency with, say, the precise 
location that we use for the error message at the beginning of 
check_methods. Indeed, the sequence of error messages of 
g++.dg/inherit/pure1.C reflect that. Tested x86_64-linux.


Thanks, Paolo.

PS: minor issues anyway, but I'm almost done with these low hanging 
fruits which I'm proposing to fix for 9 too


Hmm, wouldn't it be preferable to use the location of the 
initializer when the initializer is the problem?


I see what you mean and indeed yesterday I gave that some thought. In 
practice, we have the usual issue that currently constants don't have 
a location


They do now in a lot more cases, with location wrappers.  If not, we 
could fall back on the decl location with EXPR_LOC_OR_LOC.


Yes. And that's what we are in fact already doing in all the other uses 
of cp_expr_loc_or_loc in decl.c. Seems a good solution to me too. I'm 
finishing testing the below.


OK.

Jason

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Jeff Law]

On 1/6/19 4:34 PM, Martin Sebor wrote:
> Attached is an updated patch with the wording change to the manual
> discussed below and rebased on the top of today's trunk.
> 
> Martin
> 
> PS Thanks for the additional info, Iain.
> 
> On 1/5/19 10:53 AM, Iain Sandoe wrote:
>>
>>> On 5 Jan 2019, at 17:39, Martin Sebor  wrote:
>>>
>>> On 1/5/19 3:31 AM, Iain Sandoe wrote:
 Hi Martin,
> On 4 Jan 2019, at 22:30, Mike Stump  wrote:
>
> On Jan 4, 2019, at 2:03 PM, Martin Sebor  wrote:
>>
>> The improved handling of attribute positional arguments added
>> in r266195 introduced a regression on Darwin where attribute
>> format with the CFString archetype accepts CFString* parameter
>> types in positions where only char* would otherwise be allowed.
>
> You didn't ask Ok?  I'll assume you want a review...  The darwin bits and 
> the testsuite bits look fine.
>
>>
>> Index: gcc/doc/extend.texi
>> ===
>> --- gcc/doc/extend.texi(revision 267580)
>> +++ gcc/doc/extend.texi(working copy)
>> @@ -22368,10 +22368,12 @@ bit-fields.  See the Solaris man page for 
>> @code{cm
>>
>>   @node Darwin Format Checks
>>   @subsection Darwin Format Checks
>>   -Darwin targets support the @code{CFString} (or @code{__CFString__}) 
>> in the format
>>
>> -attribute context.  Declarations made with such attribution are parsed 
>> for correct syntax
>>
>> -and format argument types.  However, parsing of the format string 
>> itself is currently undefined
>>
>> -and is not carried out by this version of the compiler.
>> +In addition to the full set of archetypes, Darwin targets also support
>>
>> +the @code{CFString} (or @code{__CFString__}) archetype in the 
>> @code{format}
>>
>> +attribute.  Declarations with this archetype are parsed for correct 
>> syntax
>>
>> +and argument types.  However, parsing of the format string itself and
>>
>> +validating arguments against it in calls to such functions is currently
>>
>> +not performed.
>> Additionally, @code{CFStringRefs} (defined by the 
>> @code{CoreFoundation} headers) may
>>
>>   also be used as format arguments.  Note that the relevant headers are 
>> only likely to be
>>
>>
 I find “archetype(s)” to be an usual (and possibly unfamiliar to many) 
 word for this context.

 how about:
 s/archetype in/variant for the/
 and then
   s/with this archetype/with this variant/
 in the following sentence.
 However, just 0.02GBP .. (fixing the fails is more important than 
 bikeshedding the wording).

>>>
>>> Thanks for chiming in!  I used archetype because that's the term
>>> used in the attribute format specification to describe the first
>>> argument.  I do tend to agree that archetype alone may not be
>>> sufficiently familiar to all users.  I'm happy to add text to
>>> make that clear.  Would you find the following better?
>>>
>>>   In addition to the full set of format archetypes (attribute
>>>   format style arguments such as @code{printf}, @code{scanf},
>>>   @code{strftime}, and @code{strfmon}), Darwin targets also
>>>   support the @code{CFString} (or @code{__CFString__}) archetype…
>>
>> Yes, that makes is clearer
>>
>> (as an aside, I think that to many people the meaning of archetype - as 
>> ‘generic’  or ‘root example’
>>
>>    etc  tends to come to mind before the ‘template/mold’ meaning … however 
>> couldn’t think of
>>
>>   a better term that’s not already overloaded).
>>
>>> FWIW, I wanted to figure out how the CFString attribute made it
>>> possible to differentiate between printf and scanf (and the other)
>>> kinds of functions, for example, so I could add new tests for it,
>>> but I couldn't tell that from the manual.  So I'm trying to update
>>> the text to make it clear that although CFString is just like
>>> the sprintf and scanf format arguments/archetypes, beyond
>>> validating declarations that use it, the attribute serves no
>>> functional purpose, so the printf/scanf distinction is moot.
>>
>> The CFString container** is more general than our implementation, e.g. it 
>> should be able
>>
>> to contain a variety of string formats (e.g. UTF etc.).   AFAIR at the time 
>> I implemented
>>
>> it for FSF GCC (it was originally in the Apple Local branch), we didn’t have 
>> sufficient parsing
>>
>> support for such things (and the support in the Apple-local branch didn’t 
>> look applicable).
>>
>>
>> If we do more sophisitcated checks, we probably need to take cognisance of 
>> the fact that a
>>
>> fully-implemented CFString impl can have non-ascii payload.   I suspect 
>> (although honestly
>>
>> it’s been a while, I maybe mis-remember) that was the reason I didn’t try to 
>> implement the
>>
>> inspection at the time (if so, there’s probably a code 

Re: [C++ Patch] Fix three additional locations

[Paolo Carlini]

Hi,

On 11/01/19 22:22, Jason Merrill wrote:

On 1/11/19 4:13 PM, Paolo Carlini wrote:

Hi,

On 11/01/19 19:58, Jason Merrill wrote:

On 1/10/19 9:24 AM, Paolo Carlini wrote:

Hi again,

this one is also matter of consistency with, say, the precise 
location that we use for the error message at the beginning of 
check_methods. Indeed, the sequence of error messages of 
g++.dg/inherit/pure1.C reflect that. Tested x86_64-linux.


Thanks, Paolo.

PS: minor issues anyway, but I'm almost done with these low hanging 
fruits which I'm proposing to fix for 9 too


Hmm, wouldn't it be preferable to use the location of the 
initializer when the initializer is the problem?


I see what you mean and indeed yesterday I gave that some thought. In 
practice, we have the usual issue that currently constants don't have 
a location


They do now in a lot more cases, with location wrappers.  If not, we 
could fall back on the decl location with EXPR_LOC_OR_LOC.


Yes. And that's what we are in fact already doing in all the other uses 
of cp_expr_loc_or_loc in decl.c. Seems a good solution to me too. I'm 
finishing testing the below.


Thanks, Paolo.




Index: cp/decl.c
===
--- cp/decl.c   (revision 267858)
+++ cp/decl.c   (working copy)
@@ -7293,7 +7293,10 @@ cp_finish_decl (tree decl, tree init, bool init_co
synthesize_method (decl);
}
  else
-   error ("function %q#D is initialized like a variable", decl);
+   error_at (cp_expr_loc_or_loc (init,
+ DECL_SOURCE_LOCATION (decl)),
+ "function %q#D is initialized like a variable",
+ decl);
}
  /* else no initialization required.  */
}
Index: cp/decl2.c
===
--- cp/decl2.c  (revision 267858)
+++ cp/decl2.c  (working copy)
@@ -924,12 +924,14 @@ grokfield (const cp_declarator *declarator,
  else
{
  gcc_assert (TREE_CODE (TREE_TYPE (value)) == FUNCTION_TYPE);
+ location_t iloc
+   = cp_expr_loc_or_loc (init, DECL_SOURCE_LOCATION (value));
  if (friendp)
-   error ("initializer specified for friend function %qD",
-  value);
+   error_at (iloc, "initializer specified for friend "
+ "function %qD", value);
  else
-   error ("initializer specified for static member function %qD",
-  value);
+   error_at (iloc, "initializer specified for static "
+ "member function %qD", value);
}
}
   else if (TREE_CODE (value) == FIELD_DECL)
Index: testsuite/g++.dg/cpp0x/pr62101.C
===
--- testsuite/g++.dg/cpp0x/pr62101.C(revision 267858)
+++ testsuite/g++.dg/cpp0x/pr62101.C(working copy)
@@ -3,7 +3,7 @@
 
 struct X
 {
-  friend void g(X, int) = 0; // { dg-error "initializer specified for friend 
function" }
+  friend void g(X, int) = 0; // { dg-error "15:initializer specified for 
friend function" }
   friend void g(X, int) = default; // { dg-error "cannot be defaulted" }
   // { dg-prune-output "note" }
   friend void f(X, int) = delete;
Index: testsuite/g++.dg/inherit/pure1.C
===
--- testsuite/g++.dg/inherit/pure1.C(revision 267858)
+++ testsuite/g++.dg/inherit/pure1.C(working copy)
@@ -2,13 +2,13 @@
 // Origin: Volker Reichelt  
 // { dg-do compile }
 
-void foo0() = 0;   // { dg-error "like a variable" }
+void foo0() = 0;   // { dg-error "6:function .void foo0\\(\\). 
is initialized like a variable" }
 virtual void foo1() = 0;   // { dg-error "1:'virtual' outside class" }
-// { dg-error "like a variable" "" { target *-*-* } .-1 }
+// { dg-error "14:function .void foo1\\(\\). is initialized like a variable" 
"" { target *-*-* } .-1 }
 struct A
 {
-  void foo2() = 0; // { dg-error "non-virtual" }
-  static void foo3() = 0;  // { dg-error "static member" }
+  void foo2() = 0; // { dg-error "8:initializer specified for 
non-virtual method" }
+  static void foo3() = 0;  // { dg-error "15:initializer specified for 
static member function" }
   virtual static void foo4() = 0;  // { dg-error "both 'virtual' and 'static'" 
}
   virtual void foo5() = 0; // { dg-error "base class" }
 };
@@ -15,5 +15,6 @@ struct A
 
 struct B : A
 {
-  static void foo5() = 0;  // { dg-error "static member|declared" }
+  static void foo5() = 0;  // { dg-error "15:initializer specified for 
static member function" }
+// { dg-error "declared" "" { target *-*-* } .-1 }  
 };

Re: [PATCH] detect references to statics in inline function signatures (PR 88718)

[Joseph Myers]

On Fri, 11 Jan 2019, Martin Sebor wrote:

> gcc/testsuite/ChangeLog:
> 
>   PR c/88718
>   * gcc.dg/inline-40.c: New test.
>   * gcc.dg/inline-41.c: New test.

We already have tests inline-40.c and inline-41.c; these need to be 
renumbered accordingly.

> +  if (add)
> + {
> +   if (csi->function == func
> +   || csi->function)
> + continue;

Since func is non-NULL, this is equivalent to "if (csi->function)".

Don't you want to break, not continue, in the case where csi->function (as 
all the tentative entries come first)?  As-is, this looks like it would 
have quadratic cost if a file has many inline functions each referencing 
some file-scope static.  (The "Avoid adding another tentative record for 
this DECL if one already exists." also looks quadratic, but having very 
many functions in a file seems more likely than very many references to 
statics in a single declaration that is not a definition.)

> @@ -5165,6 +5259,11 @@ finish_decl (tree decl, location_t init_loc, tree
>   set_user_assembler_name (decl, asmspec);
>   }
>  
> +  /* Remove any tentative record of a non-static inline function
> +  referencing a static decl made a DECL that is not a definition.  */
> +  if (TREE_CODE (decl) == FUNCTION_DECL)
> + update_tentative_inline_static (decl);
> + 

You also need to do the update when the declaration wasn't a function 
declaration at all.  The present patch produces spurious warnings for:

static int i;
int a[sizeof i];
inline void f (void) {}

because the reference to i in the declaration of a gets treated as if it 
were in the definition of f.

Also, you need to distinguish the case of updating for a function 
definition, and updating for a declaration that's not a definition, for a 
function that was previously defined.  E.g.

inline void f (int a[sizeof (int)]) {}
static int i;
inline void f (int a[sizeof i]);

must not diagnose a reference to i in f (you have such tests with the 
declaration before the definition, but none that I can see with the 
declaration after the definition).  I think this means the caller of 
update_tentative_inline_static should specify whether to add or remove, 
rather than update_tentative_inline_static trying to deduce it from 
properties of a DECL (that don't say whether the particular declaration in 
question is a definition or not, just whether a definition has been seen).

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

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Jason Merrill]

On 1/11/19 4:21 PM, Marek Polacek wrote:

On Fri, Jan 11, 2019 at 01:55:09PM -0500, Jason Merrill wrote:

On 1/11/19 11:09 AM, Marek Polacek wrote:

On Mon, Jan 07, 2019 at 04:59:14PM -0500, Jason Merrill wrote:

On 1/7/19 4:29 PM, Marek Polacek wrote:

This patch fixes bogus -Wredundant-move warnings reported in 88692 and 87882.

To quickly recap, this warning is supposed to warn for cases like

struct T { };

T fn(T t)
{
 return std::move (t);
}

where NRVO isn't applicable for T because it's a parameter, but it's
a local variable and we're returning, so C++11 says activate move
semantics, so the std::move is redundant.  But, as these testcases show,
we're failing to realize that that is not the case when returning *this,
which is disguised as an ordinary PARM_DECL, and treat_lvalue_as_rvalue_p
was fooled by that.


Hmm, the function isn't returning 'this', it's returning '*this'.  I guess
what's happening is that in order to pass *this to the reference parameter
of move, we end up converting it from pointer to reference by NOP_EXPR, and
the STRIP_NOPS in maybe_warn_pessimizing_move throws that away so that it
then thinks we're returning 'this'.  I expect the same thing could happen
with any parameter of pointer-to-class type.


You're right, I didn't realize that we warned even for parameters of 
pointer-to-class
types.  So why don't we disable the warning for PARM_DECLs with pointer types?


std::move is certainly redundant for parms of pointer type (or other scalar
type), so we might still want to warn about 'return std::move(this)'.  The
problem here is that we're discarding the indirection, so we aren't actually
considering the returned expression.


We won't warn for 'return std::move(this)' in any case becase
maybe_warn_pessimizing_move returns for non-class types.  This is in line with
what clang does.  I'm not sure if we should change that; I'd rather not.
  

Is the STRIP_NOPS really necessary?  It seems we shouldn't remove a NOP_EXPR
from pointer to reference.


I think we need that in order to make can_do_nrvo_p/treat_lvalue_as_rvalue_p
work.  Given the outermost NOP_EXPR will always be of reference type, how about
just returning if, after STRIP_NOPS, we don't see an ADDR_EXPR?  That fixes the
testcases I've been meaning to fix and doesn't regress anything.  I.e., this:

--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -9412,8 +9412,9 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
 {
   tree arg = CALL_EXPR_ARG (fn, 0);
   STRIP_NOPS (arg);
- if (TREE_CODE (arg) == ADDR_EXPR)
-   arg = TREE_OPERAND (arg, 0);
+ if (TREE_CODE (arg) != ADDR_EXPR)
+   return;
+ arg = TREE_OPERAND (arg, 0);
   arg = convert_from_reference (arg);
   /* Warn if we could do copy elision were it not for the move.  */
   if (can_do_nrvo_p (arg, functype))

I can test/post the complete patch.  Or am I again missing something obvious? :)


That looks good.  OK if it passes.

Jason

Re: [C++ Patch] Fix three additional locations

[Jason Merrill]

On 1/11/19 4:13 PM, Paolo Carlini wrote:

Hi,

On 11/01/19 19:58, Jason Merrill wrote:

On 1/10/19 9:24 AM, Paolo Carlini wrote:

Hi again,

this one is also matter of consistency with, say, the precise 
location that we use for the error message at the beginning of 
check_methods. Indeed, the sequence of error messages of 
g++.dg/inherit/pure1.C reflect that. Tested x86_64-linux.


Thanks, Paolo.

PS: minor issues anyway, but I'm almost done with these low hanging 
fruits which I'm proposing to fix for 9 too


Hmm, wouldn't it be preferable to use the location of the initializer 
when the initializer is the problem?


I see what you mean and indeed yesterday I gave that some thought. In 
practice, we have the usual issue that currently constants don't have a 
location


They do now in a lot more cases, with location wrappers.  If not, we 
could fall back on the decl location with EXPR_LOC_OR_LOC.  But I 
suppose indicating the decl is fine too.


Jason

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Marek Polacek]

On Fri, Jan 11, 2019 at 01:55:09PM -0500, Jason Merrill wrote:
> On 1/11/19 11:09 AM, Marek Polacek wrote:
> > On Mon, Jan 07, 2019 at 04:59:14PM -0500, Jason Merrill wrote:
> > > On 1/7/19 4:29 PM, Marek Polacek wrote:
> > > > This patch fixes bogus -Wredundant-move warnings reported in 88692 and 
> > > > 87882.
> > > > 
> > > > To quickly recap, this warning is supposed to warn for cases like
> > > > 
> > > > struct T { };
> > > > 
> > > > T fn(T t)
> > > > {
> > > > return std::move (t);
> > > > }
> > > > 
> > > > where NRVO isn't applicable for T because it's a parameter, but it's
> > > > a local variable and we're returning, so C++11 says activate move
> > > > semantics, so the std::move is redundant.  But, as these testcases show,
> > > > we're failing to realize that that is not the case when returning *this,
> > > > which is disguised as an ordinary PARM_DECL, and 
> > > > treat_lvalue_as_rvalue_p
> > > > was fooled by that.
> > > 
> > > Hmm, the function isn't returning 'this', it's returning '*this'.  I guess
> > > what's happening is that in order to pass *this to the reference parameter
> > > of move, we end up converting it from pointer to reference by NOP_EXPR, 
> > > and
> > > the STRIP_NOPS in maybe_warn_pessimizing_move throws that away so that it
> > > then thinks we're returning 'this'.  I expect the same thing could happen
> > > with any parameter of pointer-to-class type.
> > 
> > You're right, I didn't realize that we warned even for parameters of 
> > pointer-to-class
> > types.  So why don't we disable the warning for PARM_DECLs with pointer 
> > types?
> 
> std::move is certainly redundant for parms of pointer type (or other scalar
> type), so we might still want to warn about 'return std::move(this)'.  The
> problem here is that we're discarding the indirection, so we aren't actually
> considering the returned expression.

We won't warn for 'return std::move(this)' in any case becase
maybe_warn_pessimizing_move returns for non-class types.  This is in line with
what clang does.  I'm not sure if we should change that; I'd rather not.
 
> Is the STRIP_NOPS really necessary?  It seems we shouldn't remove a NOP_EXPR
> from pointer to reference.

I think we need that in order to make can_do_nrvo_p/treat_lvalue_as_rvalue_p
work.  Given the outermost NOP_EXPR will always be of reference type, how about
just returning if, after STRIP_NOPS, we don't see an ADDR_EXPR?  That fixes the
testcases I've been meaning to fix and doesn't regress anything.  I.e., this:

--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -9412,8 +9412,9 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
{
  tree arg = CALL_EXPR_ARG (fn, 0);
  STRIP_NOPS (arg);
- if (TREE_CODE (arg) == ADDR_EXPR)
-   arg = TREE_OPERAND (arg, 0);
+ if (TREE_CODE (arg) != ADDR_EXPR)
+   return;
+ arg = TREE_OPERAND (arg, 0);
  arg = convert_from_reference (arg);
  /* Warn if we could do copy elision were it not for the move.  */
  if (can_do_nrvo_p (arg, functype))

I can test/post the complete patch.  Or am I again missing something obvious? :)

Marek

Re: [C++ Patch] Fix three additional locations

[Paolo Carlini]

Hi,

On 11/01/19 19:58, Jason Merrill wrote:

On 1/10/19 9:24 AM, Paolo Carlini wrote:

Hi again,

this one is also matter of consistency with, say, the precise 
location that we use for the error message at the beginning of 
check_methods. Indeed, the sequence of error messages of 
g++.dg/inherit/pure1.C reflect that. Tested x86_64-linux.


Thanks, Paolo.

PS: minor issues anyway, but I'm almost done with these low hanging 
fruits which I'm proposing to fix for 9 too


Hmm, wouldn't it be preferable to use the location of the initializer 
when the initializer is the problem?


I see what you mean and indeed yesterday I gave that some thought. In 
practice, we have the usual issue that currently constants don't have a 
location thus the only - brittle - way to achieve that is relying on 
input_location. That appears to work for the cases in decl.c and decl2.c 
but, as far as I can see, nothing similar can be cooked up for the 
check_methods case (we don't have the initializer at all, input_location 
doesn't make any sense). On the other hand, front-ends like clang just 
consistently use the location of the decl, that's why I decided to go 
ahead and propose a simple solution using everywhere that location.


Looks like for the time being we can't make much progress, because, in 
most of the error messages related to the initializers, thanks to 
input_location we can point to the initializers and resolving the 
inconsistency with check_methods seems tough.


Paolo.

Re: V2 [PATCH] i386: Add pass_remove_partial_avx_dependency

[Jeff Law]

On 12/30/18 9:50 AM, H.J. Lu wrote:
> On Wed, Nov 28, 2018 at 12:21 PM Jan Hubicka  wrote:
>>
>>> On 11/28/18 12:48 PM, H.J. Lu wrote:
 On Mon, Nov 5, 2018 at 7:29 AM Jan Hubicka  wrote:
>
>> On 11/5/18 7:21 AM, Jan Hubicka wrote:

 Did you mean "the nearest common dominator"?
>>>
>>> If the nearest common dominator appears in the loop while all uses are
>>> out of loops, this will result in suboptimal xor placement.
>>> In this case you want to split edges out of the loop.
>>>
>>> In general this is what the LCM framework will do for you if the problem
>>> is modelled siimlar way as in mode_swtiching.  At entry function mode is
>>> "no zero register needed" and all conversions need mode "zero register
>>> needed".  Mode switching should then do the correct placement decisions
>>> (reaching minimal number of executions of xor).
>>>
>>> Jeff, whan is your optinion on the approach taken by the patch?
>>> It seems like a special case of more general issue, but I do not see
>>> very elegant way to solve it at least in the GCC 9 horisont, so if
>>> the placement is correct we can probalby go either with new pass or
>>> making this part of mode swithcing (which is anyway run by x86 backend)
>> So I haven't followed this discussion at all, but did touch on this
>> issue with some patch a month or two ago with a target patch that was
>> trying to avoid the partial stalls.
>>
>> My assumption is that we're trying to find one or more places to
>> initialize the upper half of an avx register so as to avoid partial
>> register stall at existing sites that set the upper half.
>>
>> This sounds like a classic PRE/LCM style problem (of which mode
>> switching is just another variant).   A common-dominator approach is
>> closer to a classic GCSE and is going to result is more initializations
>> at sub-optimal points than a PRE/LCM style.
>
> yes, it is usual code placement problem. It is special case because the
> zero register is not modified by the conversion (just we need to have
> zero somewhere).  So basically we do not have kills to the zero except
> for entry block.
>

 Do you have  testcase to show thatf the nearest common dominator
 in the loop, while all uses areout of loops, leads to suboptimal xor
 placement?
>>> I don't have a testcase, but it's all but certain nearest common
>>> dominator is going to be a suboptimal placement.  That's going to create
>>> paths where you're going to emit the xor when it's not used.
>>>
>>> The whole point of the LCM algorithms is they are optimal in terms of
>>> expression evaluations.
>>
>> i think testcase should be something like
>>
>> test()
>> {
>>   while (true)
>>   {
>>  if (cond1)
>>{
>>  do_one_conversion;
>>  return;
>>}
>>  if (cond2)
>>{
>>  do_other_conversion;
>>  return;
>>}
>>   }
>> }
> 
> We got
> 
> [hjl@gnu-cfl-1 pr87007]$ cat test2.i
> extern float f1[],f2[];
> extern int i1[],i2[];
> float
> foo (int k, int n[])
> {
>   if (k == 1)
> return 1;
> 
>   if (k == 4)
> return 5;
> 
>   for(int i = 0; i != k; i++){
> if(n[i] > 100)
>   f1[i] = i1[i];
> else
>   f2[i] = i2[i];
>   }
> 
>   return k;
Note the if-if construct within the loop of the pseudo code will
generate a notably different cfg than the if-else in your code (which
creates a simple diamond).   The pseudo code also exits in those cases
rather than continuing the loop.

The differences in the CFGs you wind up with are critically important.

For the first chunk of pseudocode the computationally optimal placement
point is just before the conversions.  THere's precisely one execution
of the xor at runtime no matter how many loop iterations occur.

For your example code the optimal placement point is just outside the
loop.  Essentially both arms of the conditional need the xor.  So
possible insertion points are just before the loop, just inside the loop
or in both arms.  Obviously the optimal point is just before the loop.

JEff

Re: [PATCH] avoid ICE when pretty-printing a VLA with an error bound (PR 85956, take 2)

[Jeff Law]

On 1/7/19 3:42 PM, Jakub Jelinek wrote:
> Hi!
> 
> On Thu, May 31, 2018 at 01:34:19PM -0400, Jason Merrill wrote:
>> Returning error_mark_node from omp_copy_decl and then continuing seems
>> like the problem, then.  Would it really be that hard to return an
>> uninitialized variable instead?
> 
> The following patch does that, but not from omp_copy_decl, but only in the
> caller for the array bounds (the rest would be still errors).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2019-01-07  Jakub Jelinek  
> 
>   PR middle-end/85956
>   PR lto/88733
>   * tree-inline.h (struct copy_body_data): Add adjust_array_error_bounds
>   field.
>   * tree-inline.c (remap_type_1): Formatting fix.  If TYPE_MAX_VALUE of
>   ARRAY_TYPE's TYPE_DOMAIN is newly error_mark_node, replace it with
>   a dummy "omp dummy var" variable if id->adjust_array_error_bounds.
>   * omp-low.c (new_omp_context): Set cb.adjust_array_error_bounds.
> fortran/
>   * trans-openmp.c: Include attribs.h.
>   (gfc_walk_alloc_comps, gfc_omp_clause_linear_ctor): Handle
>   VAR_DECL max bound with "omp dummy var" attribute like NULL or
>   error_mark_node - recompute number of elts independently.
> testsuite/
>   * c-c++-common/gomp/pr85956.c: New test.
>   * g++.dg/gomp/pr88733.C: New test.
OK.
jeff

Re: [PATCH] Document atomic fetch and nand

[Jeff Law]

On 1/8/19 11:35 PM, Sebastian Huber wrote:
> Copy code example for fetch and nand from "Legacy __sync Built-in
> Functions for Atomic Memory Access" to "Built-in Functions for Memory
> Model Aware Atomic Operations".
> 
> gcc/
> 
>   * doc/extend.texi (Built-in Functions for Memory Model Aware
>   Atomic Operations): Document atomic fetch and nand.
OK
jeff

Re: [doc patch] fix minor typos in -Wmemset-transposed-args

[Jeff Law]

On 1/3/19 9:13 PM, Martin Sebor wrote:
> The attached patch corrects a couple of minor typos in the description
> of -Wmemset-transposed-args.  It also tweaks the text a bit for better
> readability.
> 
> Martin
> 
> gcc-doc-memset-transposed-args.diff
> 
> gcc/ChangeLog:
> 
>   * invoke.texi (-Wmemset-transposed-args): Fix typos, adjust wording.
OK
jeff

Re: [doc patch] fix minor typos in -Wmemset-transposed-args

[Martin Sebor]

Ping: https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00139.html

On 1/3/19 9:13 PM, Martin Sebor wrote:

The attached patch corrects a couple of minor typos in the description
of -Wmemset-transposed-args.  It also tweaks the text a bit for better
readability.

Martin

[PATCH] detect references to statics in inline function signatures (PR 88718)

[Martin Sebor]

The attached patch extends the detection of references to static
variables in inline functions to the function signatures, including
their return type.  Since the declaration of a function need not be
yet available at the time the static is referenced (e.g., when it's
referenced in the functions return type), the patch introduces
the concept of "tentative records" of such references and either
completes the records when the full declaration of the function,
including its definition, is known to be inline, or removes it
when it isn't.

Martin
PR c/88718 - Strange inconsistency between old style and new style definitions of inline functions.

gcc/c/ChangeLog:

	PR c/88718
	* c-decl.c (reset_inline_statics): New function.
	(record_inline_static): Optimize.
	(check_inline_statics): Handle tentative records for inline
	declarations without definitions.
	Print static declaration location.
	(push_file_scope): Clear records of references to statics.
	(finish_decl): Add tentative records of references to statics.
	(finish_function): Same.
	* c-typeck.c (build_external_ref): Handle all references to statics.

gcc/testsuite/ChangeLog:

	PR c/88718
	* gcc.dg/inline-40.c: New test.
	* gcc.dg/inline-41.c: New test.

Index: gcc/c/c-decl.c
===
--- gcc/c/c-decl.c	(revision 267616)
+++ gcc/c/c-decl.c	(working copy)
@@ -826,14 +826,47 @@ c_finish_incomplete_decl (tree decl)
 }
 }
 
-/* Record that inline function FUNC contains a reference (location
-   LOC) to static DECL (file-scope or function-local according to
-   TYPE).  */
+/* Free records of references to static variables gathered so far.  */
 
+static void
+reset_inline_statics (void)
+{
+  if (!c_inline_statics)
+return;
+
+  for (c_inline_static *csi = c_inline_statics, *next = csi->next;
+   csi; csi = next)
+ggc_free (csi);
+
+  c_inline_statics = NULL;
+}
+
+/* Record that inline function FUNC either does contain or may contain
+   a reference (location LOC) to static DECL (file-scope or function-local
+   according to TYPE).  For a null FUNC, a tentative record is created that
+   reflects a reference in the function signature and that is either updated
+   or removed when the function declaration is complete.  */
+
 void
 record_inline_static (location_t loc, tree func, tree decl,
 		  enum c_inline_static_type type)
 {
+  gcc_assert (decl);
+
+  if (c_inline_statics)
+{
+  /* Avoid adding another tentative record for this DECL if one
+	 already exists.  */
+  for (c_inline_static *csi = c_inline_statics; csi; csi = csi->next)
+	{
+	  if (c_inline_statics->function)
+	break;
+
+	  if (decl == c_inline_statics->static_decl)
+	return;
+	}
+}
+
   c_inline_static *csi = ggc_alloc ();
   csi->location = loc;
   csi->function = func;
@@ -843,6 +876,50 @@ record_inline_static (location_t loc, tree func, t
   c_inline_statics = csi;
 }
 
+/* Update tentative records of references to static declarations in
+   an inline declaration of function FUNC, or remove them if FUNC
+   isn't declared inline.  A tentative record is one whose FUNCTION
+   is null.  */
+
+static void
+update_tentative_inline_static (tree func)
+{
+  gcc_assert (func);
+
+  c_inline_static *csi = c_inline_statics;
+  if (!csi)
+  return;
+
+  /* True to associate FUNC with the tentative records, false to remove
+ them.  */
+  bool add = (DECL_DECLARED_INLINE_P (func)
+	  && DECL_EXTERNAL (func)
+	  && DECL_INITIAL (func));
+
+  for (c_inline_static *csi = c_inline_statics, *last = csi;
+   csi; csi = csi->next)
+{
+  if (add)
+	{
+	  if (csi->function == func
+	  || csi->function)
+	continue;
+
+	  csi->function = func;
+	}
+  else
+	{
+	  if (csi->function)
+	break;
+
+	  if (last == c_inline_statics)
+	c_inline_statics = last = csi->next;
+	  else
+	last->next = csi->next;
+	}
+}
+}
+
 /* Check for references to static declarations in inline functions at
the end of the translation unit and diagnose them if the functions
are still inline definitions.  */
@@ -853,22 +930,36 @@ check_inline_statics (void)
   struct c_inline_static *csi;
   for (csi = c_inline_statics; csi; csi = csi->next)
 {
-  if (DECL_EXTERNAL (csi->function))
-	switch (csi->type)
-	  {
-	  case csi_internal:
-	pedwarn (csi->location, 0,
-		 "%qD is static but used in inline function %qD "
-		 "which is not static", csi->static_decl, csi->function);
-	break;
-	  case csi_modifiable:
-	pedwarn (csi->location, 0,
-		 "%q+D is static but declared in inline function %qD "
-		 "which is not static", csi->static_decl, csi->function);
-	break;
-	  default:
-	gcc_unreachable ();
-	  }
+  /* FUNCTION is unset for a declaration whose signature references
+	 a static variable and for which a definition wasn't provided.  */
+  if (!csi->function)
+	continue;
+
+  if (!DECL_EXTERNAL (csi->function))
+	continue;
+
+  

Re: [PATCH] Check requested alignment in SET_{DECL,TYPE}_ALIGN is pow2_or_zerop before aligning on targets with partial int modes

[Jeff Law]

On 12/30/18 4:51 AM, Jozef Lawrynowicz wrote:
> There have been some ICEs in the past for msp430-elf with the large
> memory model (20-bit pointers), caused by SET_{DECL,TYPE}_ALIGN being called
> with an argument which resolves to 20 i.e. POINTER_SIZE,
> or the default value of TARGET_VTABLE_ENTRY_ALIGN (which is POINTER_SIZE).
> 
> The attached patch adds an assertion that SET_{DECL,TYPE}_ALIGN is called with
> a value which is a power of 2, or 0, for targets which support a partial int
> mode. This should catch issues in the future with non-power of 2
> alignments being set, which could propagate into serious problems later in the
> compilation process.
> 
> If the filtering to only perform this check for targets supporting a partial
> int mode is unnecessary, I can remove that so CHECK_POW2_OR_ZEROP always
> expands to check_pow2_or_zerop.
> 
> Successfully bootstrapped and regtested on x86_64-pc-linux-gnu and
> msp430-elf/-mlarge.
> 
> Ok for trunk, or does this have to wait for GCC10 Stage 1?
> 
Let's defer to gcc-10.

I'm not sure it's worth trying to conditionalize this on the partial
integer modes.  Why not just do it all the time?  I'd be surprised if
it's that expensive.

jeff

Re: [PATCH] PR88777, Out-of-range offsets building glibc test-tgmath2.c

[Richard Sandiford]

Jeff Law  writes:
> On 1/10/19 12:19 AM, Alan Modra wrote:
>> bb-reorder is quite seriously broken if get_attr_min_length should
>> return INT_MAX, which it does for hppa on branches with r267666.
> Presumably you're referring to the overflows and such?
>
>
>> 
>> I went the wrong way with my min_attr_value r267666 change.  It seems
>> that it isn't important that a "can't calculate" value is returned
>> from recursive calls, but rather that it returns the minimum among
>> those the function can calculate, ie. a conservative minimum length.
>> That's what this patch does, going back to the behaviour of
>> min_attr_value prior to r267666, with an added comment to stop foolish
>> patches in future.
>> 
>> Bootstrapped and regression tested powerpc64le-linux.  OK?
>> 
>>  PR 88777
>>  PR 88614
>>  * genattrtab.c (min_fn): Don't translate values.
>>  (min_attr_value): Return INT_MAX when the value can't be calculated.
>>  Return minimum among any values that can be calculated.
>>  (max_attr_value): Adjust.
> OK.  Given you're likely done for the day I'm going to go ahead and
> install it momentarily to make my tester happy again :-)

FWIW, I think this is papering over a deeper issue, but I guess it's
OK to install anyway to unbreak builds.

Was meaning to have a look today but got sidetracked onto other things,
sorry.

Thanks,
Richard

one more patch for PR87305

[Vladimir Makarov]

  This patch adds code for little endian case of

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87305

  The patch was successfully bootstrapped and tested on x86-64.

  Committed as rev. 267854.


Index: ChangeLog
===
--- ChangeLog	(revision 267853)
+++ ChangeLog	(working copy)
@@ -1,3 +1,10 @@
+2019-01-11  Vladimir Makarov  
+
+	PR rtl-optimization/87305
+	* lra-assigns.c
+	(setup_live_pseudos_and_spill_after_risky_transforms): Add code
+	for little endian pseudos used as paradoxical subreg.
+
 2019-01-11  Jakub Jelinek  
 
 	PR tree-optimization/88693
Index: lra-assigns.c
===
--- lra-assigns.c	(revision 267849)
+++ lra-assigns.c	(working copy)
@@ -1174,10 +1174,14 @@ setup_live_pseudos_and_spill_after_risky
 		  - hard_regno_nregs (hard_regno, PSEUDO_REGNO_MODE (i)));
 	enum reg_class rclass = lra_get_allocno_class (i);
 
-	if (WORDS_BIG_ENDIAN
-	&& (hard_regno - nregs_diff < 0
-		|| !TEST_HARD_REG_BIT (reg_class_contents[rclass],
-   hard_regno - nregs_diff)))
+	if ((WORDS_BIG_ENDIAN
+	 && (hard_regno - nregs_diff < 0
+		 || !TEST_HARD_REG_BIT (reg_class_contents[rclass],
+	hard_regno - nregs_diff)))
+	|| (!WORDS_BIG_ENDIAN
+		&& (hard_regno + nregs_diff >= FIRST_PSEUDO_REGISTER
+		|| !TEST_HARD_REG_BIT (reg_class_contents[rclass],
+	   hard_regno + nregs_diff
 	  {
 	/* Hard registers of paradoxical sub-registers are out of
 	   range of pseudo register class.  Spill the pseudo.  */

Re: [RFC] moving assemble_start_function / assemble_end_function to output_mi_thunk

[Max Filippov]

On Fri, Jan 11, 2019 at 10:50 AM Jeff Law  wrote:
> I think this needs to defer to gcc-10

Ok, will resend when in stage1.

-- 
Thanks.
-- Max

Re: [PATCH] Optimize away x86 mem stores of what the mem contains already (PR rtl-optimization/79593)

[Jeff Law]

On 1/7/19 3:51 PM, Jakub Jelinek wrote:
> Hi!
> 
> As mentioned in that PR, we have a SI->DImode zero extension and RA happens
> to choose to zero extend from a SImode memory slot which is the low part of
> the DImode memory slot into which the zero extension is to be stored. 
> Unfortunately, the RTL DSE part really doesn't have infrastructure to
> remember and, if needed, invalidate loads, it just remembers stores, so
> handling this generically is quite unlikely at least for GCC9.
> 
> This patch just handles that through a peephole2 (other option would be to
> handle it in the define_split for the zero extension, but the peephole2 is
> likely to catch more things).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2019-01-07  Jakub Jelinek  
> 
>   PR rtl-optimization/79593
>   * config/i386/i386.md (reg = mem; mem = reg): New define_peephole2.
Note I'd hacked up a POC in DSE and it caught something like 10
instances across a bootstrap.  In the end I don't think this matters
much in practice (peephole vs optimizing at the split point).

Jeff

Re: [C++ Patch] Fix three additional locations

[Jason Merrill]

On 1/10/19 9:24 AM, Paolo Carlini wrote:

Hi again,

this one is also matter of consistency with, say, the precise location 
that we use for the error message at the beginning of check_methods. 
Indeed, the sequence of error messages of g++.dg/inherit/pure1.C reflect 
that. Tested x86_64-linux.


Thanks, Paolo.

PS: minor issues anyway, but I'm almost done with these low hanging 
fruits which I'm proposing to fix for 9 too


Hmm, wouldn't it be preferable to use the location of the initializer 
when the initializer is the problem?


Jason

Re: PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default"

[Jason Merrill]

On 1/11/19 1:36 PM, Tobias Burnus wrote:

Dear Jason, dear all,

Jason Merrill wrote on 5 Dec 2018:

You can get at the destructor with CLASSTYPE_DESTRUCTOR rather than walking 
through TYPE_FIELDS. I'd also check DECL_DEFAULTED_IN_CLASS_P.
I'd also do this in maybe_emit_vtables rather than here, so that it only 
happens once per class.


Updated patch. I also added a test case which checks that the destructor
is not generated.

[ Original patch: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01824.html ]

Background again:
If a class contains any 'virtual ... = 0', it's an abstract class and for an
abstract class, the destructor not added to the vtable.

For a normal
  virtual ~class() { }
that's not a problem as the class::~class() destructor will be generated 
during
the parsing of the function.

But for
  virtual ~class() = default;
the destructor will be generated via mark_used via the vtable.

If one now declares a derived class and uses it, the class::~class() is 
generated
in that translation unit.  Unless, #pragma interface/implementation is used.

In that case, the 'default' destructor will never be generated.

Build and regtested on x86_64-gnu-linux.
OK? Or do you have more suggested changes?



+  && DECL_DEFAULTED_IN_CLASS_P(CLASSTYPE_DESTRUCTOR(ctype))
+  && DECL_DEFAULTED_FN (CLASSTYPE_DESTRUCTOR(ctype)))


Checking DECL_DEFAULTED_FN again is redundant, it's checked by 
DECL_DEFAULTED_IN_CLASS_P.  OK with that last condition removed.


Jason

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Jason Merrill]

On 1/11/19 11:09 AM, Marek Polacek wrote:

On Mon, Jan 07, 2019 at 04:59:14PM -0500, Jason Merrill wrote:

On 1/7/19 4:29 PM, Marek Polacek wrote:

This patch fixes bogus -Wredundant-move warnings reported in 88692 and 87882.

To quickly recap, this warning is supposed to warn for cases like

struct T { };

T fn(T t)
{
return std::move (t);
}

where NRVO isn't applicable for T because it's a parameter, but it's
a local variable and we're returning, so C++11 says activate move
semantics, so the std::move is redundant.  But, as these testcases show,
we're failing to realize that that is not the case when returning *this,
which is disguised as an ordinary PARM_DECL, and treat_lvalue_as_rvalue_p
was fooled by that.


Hmm, the function isn't returning 'this', it's returning '*this'.  I guess
what's happening is that in order to pass *this to the reference parameter
of move, we end up converting it from pointer to reference by NOP_EXPR, and
the STRIP_NOPS in maybe_warn_pessimizing_move throws that away so that it
then thinks we're returning 'this'.  I expect the same thing could happen
with any parameter of pointer-to-class type.


You're right, I didn't realize that we warned even for parameters of 
pointer-to-class
types.  So why don't we disable the warning for PARM_DECLs with pointer types?


std::move is certainly redundant for parms of pointer type (or other 
scalar type), so we might still want to warn about 'return 
std::move(this)'.  The problem here is that we're discarding the 
indirection, so we aren't actually considering the returned expression.


Is the STRIP_NOPS really necessary?  It seems we shouldn't remove a 
NOP_EXPR from pointer to reference.


Jason

Re: [RFC] moving assemble_start_function / assemble_end_function to output_mi_thunk

[Jeff Law]

On 1/8/19 3:38 PM, Max Filippov wrote:
> Hello,
> 
> I'm implementing MI thunk generation for the xtensa target and I've got
> an issue that when my code generates a constant it is missing in the
> resulting assembly. This happens because a constant pool output happens
> inside the assemble_start_function, which is called before the thunk
> function body has a chance to be generated. The following patch moves
> assemble_start_function / assemble_end_function pair to the backend for
> all targets that define TARGET_ASM_OUTPUT_MI_THUNK to allow calling
> assemble_start_function after the function body is ready.
> 
> Is it OK, or should I try to fix it differently?
> 
> ---8<---
> From bad901880a3f9fc69726aa082e2b2c674bacca94 Mon Sep 17 00:00:00 2001
> From: Max Filippov 
> Date: Mon, 7 Jan 2019 18:22:12 -0800
> Subject: [PATCH] gcc: move assemble_start_function / assemble_end_function to
>  output_mi_thunk
> 
> Let backends call assemble_start_function after they have generated
> thunk function body so that a constant pool could be output if it is
> required.
> 
> gcc/
> 2019-01-08  Max Filippov  
> 
>   * cgraphunit.c (cgraph_node::expand_thunk): Remove
>   assemble_start_function and assemble_end_function calls.
>   * config/alpha/alpha.c (alpha_output_mi_thunk_osf): Call
>   assemble_start_function and assemble_end_function.
>   * config/arc/arc.c (arc_output_mi_thunk): Likewise.
>   * config/arm/arm.c (arm_output_mi_thunk): Likewise.
>   * config/bfin/bfin.c (bfin_output_mi_thunk): Likewise.
>   * config/c6x/c6x.c (c6x_output_mi_thunk): Likewise.
>   * config/cris/cris.c (cris_asm_output_mi_thunk): Likewise.
>   * config/csky/csky.c (csky_output_mi_thunk): Likewise.
>   * config/epiphany/epiphany.c (epiphany_output_mi_thunk): Likewise.
>   * config/frv/frv.c (frv_asm_output_mi_thunk): Likewise.
>   * config/i386/i386.c (x86_output_mi_thunk): Likewise.
>   * config/ia64/ia64.c (ia64_output_mi_thunk): Likewise.
>   * config/m68k/m68k.c (m68k_output_mi_thunk): Likewise.
>   * config/microblaze/microblaze.c (microblaze_asm_output_mi_thunk):
>   Likewise.
>   * config/mips/mips.c (mips_output_mi_thunk): Likewise.
>   * config/mmix/mmix.c (mmix_asm_output_mi_thunk): Likewise.
>   * config/mn10300/mn10300.c (mn10300_asm_output_mi_thunk): Likewise.
>   * config/nds32/nds32.c (nds32_asm_output_mi_thunk): Likewise.
>   * config/nios2/nios2.c (nios2_asm_output_mi_thunk): Likewise.
>   * config/or1k/or1k.c (or1k_output_mi_thunk): Likewise.
>   * config/pa/pa.c (pa_asm_output_mi_thunk): Likewise.
>   * config/riscv/riscv.c (riscv_output_mi_thunk): Likewise.
>   * config/rs6000/rs6000.c (rs6000_output_mi_thunk): Likewise.
>   * config/s390/s390.c (s390_output_mi_thunk): Likewise.
>   * config/sh/sh.c (sh_output_mi_thunk): Likewise.
>   * config/sparc/sparc.c (sparc_output_mi_thunk): Likewise.
>   * config/spu/spu.c (spu_output_mi_thunk): Likewise.
>   * config/stormy16/stormy16.c (xstormy16_asm_output_mi_thunk):
>   Likewise.
>   * config/tilegx/tilegx.c (tilegx_output_mi_thunk): Likewise.
>   * config/tilepro/tilepro.c (tilepro_asm_output_mi_thunk): Likewise.
>   * config/vax/vax.c (vax_output_mi_thunk): Likewise.
I think this needs to defer to gcc-10

jeff

Re: [PATCH 2/2][GCC][ARM] Implement hint intrinsics for ARM

[Sudakshina Das]

Hi Srinath

On 10/01/19 19:20, Srinath Parvathaneni wrote:
> Hi All,
> 
> This patch implements the ACLE hint intrinsics (nop,yield,wfe,wfi,sev
> and sevl), for all ARM targets.
> 
> The intrinsics specification will be published on the Arm website [1].
> 
> [1]
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0053c/IHI0053C_acle_2_0.pdf
> 
> Bootstrapped on arm-none-linux-gnueabihf, regression tested on
> arm-none-eabi with no regressions and
> ran the added tests for arm, thumb-1 and thumb-2 modes.
> 
> Ok for trunk? If ok, could someone commit the patch on my behalf, I
> don't have commit rights.
> 
> Thanks,
> Srinath
> 
> gcc/ChangeLog:
> 
> 2019-01-10  Srinath Parvathaneni  
> 
>   * config/arm/arm-builtins.c (NOP_QUALIFIERS): New qualifier.
>   (arm_expand_builtin_args): New case.
>   * config/arm/arm.md (yield): New pattern name.
>   (wfe): Likewise.
>   (wfi): Likewise.
>   (sev): Likewise.
>   (sevl): Likewise.
>   * config/arm/arm_acle.h (__nop ): New inline function.
>   (__yield): Likewise.
>   (__sev): Likewise.
>   (__sevl): Likewise.
>   (__wfi): Likewise.
>   (__wfe): Likewise.
>   * config/arm/arm_acle_builtins.def (VAR1):
>   (nop): New builtin definitions.
>   (yield): Likewise.
>   (sev): Likewise.
>   (sevl): Likewise.
>   (wfi): Likewise.
>   (wfe): Likewise.
>   * config/arm/unspecs.md (unspecv):
>   (VUNSPEC_YIELD): New volatile unspec.
>   (VUNSPEC_SEV): Likewise.
>   (VUNSPEC_SEVL): Likewise.
>   (VUNSPEC_WFI): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-01-10  Srinath Parvathaneni  
> 
>   * gcc.target/arm/acle/nop.c: New test.
>   * gcc.target/arm/acle/sev-1.c: Likewise.
>   * gcc.target/arm/acle/sev-2.c: Likewise.
>   * gcc.target/arm/acle/sev-3.c: Likewise.
>   * gcc.target/arm/acle/sevl-1.c: Likewise.
>   * gcc.target/arm/acle/sevl-2.c: Likewise.
>   * gcc.target/arm/acle/sevl-3.c: Likewise.
>   * gcc.target/arm/acle/wfe-1.c: Likewise.
>   * gcc.target/arm/acle/wfe-2.c: Likewise.
>   * gcc.target/arm/acle/wfe-3.c: Likewise.
>   * gcc.target/arm/acle/wfi-1.c: Likewise.
>   * gcc.target/arm/acle/wfi-2.c: Likewise.
>   * gcc.target/arm/acle/wfi-3.c: Likewise.
>   * gcc.target/arm/acle/yield-1.c: Likewise.
>   * gcc.target/arm/acle/yield-2.c: Likewise.
>   * gcc.target/arm/acle/yield-3.c: Likewise.
> 

Thanks for doing this and I am not a maintainer. I do have a few questions:

...

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 
f6196e9316898e3258e08d8f2ece8fe9640676ca..36b24cfdfa6c61d952a5c704f54d37f2b0fdd34e
 
100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -8906,6 +8906,76 @@
 (set_attr "type" "mov_reg")]
  )

+(define_insn "yield"
+  [(unspec_volatile [(const_int 0)] VUNSPEC_YIELD)]
+  ""
+{
+  if (TARGET_ARM)
+return ".inst\t0xe320f001\t//yield";
+  else if(TARGET_THUMB2)

There should be a space after the if. Likewise for all the other 
instructions.

+return ".inst\t0xf3af8001\t//yield";
+  else /* TARGET_THUMB1 */
+return ".inst\t0xbf10\t//yield";
+}
+  [(set_attr "type" "coproc")]

Can you please explain the coproc attribute. Also I think maybe you can 
use the "length" attribute here. Likewise for all the other instructions.

Finally, for the tests why not combine the tests like the AArch64 patch 
where all the intrinsics were tested in the same file with common 
testing options? You could have only three new files for all the testing?

Thanks
Sudi

+)
+

> 
> 
> 

Re: [PATCH] PR88777, Out-of-range offsets building glibc test-tgmath2.c

[Jeff Law]

On 1/10/19 12:19 AM, Alan Modra wrote:
> bb-reorder is quite seriously broken if get_attr_min_length should
> return INT_MAX, which it does for hppa on branches with r267666.
Presumably you're referring to the overflows and such?


> 
> I went the wrong way with my min_attr_value r267666 change.  It seems
> that it isn't important that a "can't calculate" value is returned
> from recursive calls, but rather that it returns the minimum among
> those the function can calculate, ie. a conservative minimum length.
> That's what this patch does, going back to the behaviour of
> min_attr_value prior to r267666, with an added comment to stop foolish
> patches in future.
> 
> Bootstrapped and regression tested powerpc64le-linux.  OK?
> 
>   PR 88777
>   PR 88614
>   * genattrtab.c (min_fn): Don't translate values.
>   (min_attr_value): Return INT_MAX when the value can't be calculated.
>   Return minimum among any values that can be calculated.
>   (max_attr_value): Adjust.
OK.  Given you're likely done for the day I'm going to go ahead and
install it momentarily to make my tester happy again :-)

jeff

Re: [PATCH] Fix PR88775

[Jeff Law]

On 1/10/19 6:44 AM, Richard Biener wrote:
> 
> I am testing the following patch teaching VRP predicate analysis about
> 
>   __x.5_4 = (long unsigned int) "hello";
>   __y.6_5 = (long unsigned int) _3;
>   if (__x.5_4 != __y.6_5)
> 
> so that we know sth about the relation of the converted entities.
> This appearantly (didn't back out other stuff) helps PR88775
> after Jakubs changes to libstdc++ (before his changes a related
> VN patch helped which meanwhile shows miscompiling
> 20_util/function_objects/comparisons_pointer.cc...).
> 
> I now see DOM performing all required optimization thanks to it
> using the EVRP machinery.
> 
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> 
> OK for trunk?
> 
> Any idea how we can have a reliable testcase for the std::string
> optimization?
> 
> Thanks,
> Richard.
> 
> From 78a345845651565daac295f8dfbfc64cf5e8ccf3 Mon Sep 17 00:00:00 2001
> From: Richard Guenther 
> Date: Thu, 10 Jan 2019 14:34:22 +0100
> Subject: [PATCH] fix-pr88775-2
> 
> 2019-01-10  Richard Biener  
> 
>   PR tree-optimization/88775
>   * tree-vrp.c (register_edge_assert_for_2): Register asserts
>   from (T) a CMP (T) b.
Seems reasonable to me.  It looks like a slight generalization of code
I've seen elsewhere, but can't find at the moment.

Jeff

Re: PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default"

[Tobias Burnus]

Dear Jason, dear all,

Jason Merrill wrote on 5 Dec 2018:
> You can get at the destructor with CLASSTYPE_DESTRUCTOR rather than walking 
> through TYPE_FIELDS. I'd also check DECL_DEFAULTED_IN_CLASS_P.
> I'd also do this in maybe_emit_vtables rather than here, so that it only 
> happens once per class. 

Updated patch. I also added a test case which checks that the destructor
is not generated.

[ Original patch: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01824.html ]

Background again:
   If a class contains any 'virtual ... = 0', it's an abstract class and for an
   abstract class, the destructor not added to the vtable.

   For a normal
 virtual ~class() { }
   that's not a problem as the class::~class() destructor will be generated 
during
   the parsing of the function.

   But for
 virtual ~class() = default;
   the destructor will be generated via mark_used via the vtable.

   If one now declares a derived class and uses it, the class::~class() is 
generated
   in that translation unit.  Unless, #pragma interface/implementation is used.

   In that case, the 'default' destructor will never be generated.

Build and regtested on x86_64-gnu-linux.
OK? Or do you have more suggested changes?

Tobias
2019-01-11  Tobias Burnus  

	PR C++/88114
	* decl2.c (maybe_emit_vtables): If needed, generate code for
	the destructor of an abstract class.
	(mark_used): Update comment for older function-name change.

	PR C++/88114
	* g++.dg/cpp0x/defaulted61.C: New.
	* g++.dg/cpp0x/defaulted62.C: New.

diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index dbab95fbc96..2e7ecdaa01c 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -2220,6 +2220,17 @@ maybe_emit_vtables (tree ctype)
 	}
 }
 
+  /* For abstract classes, the destructor has been removed from the
+ vtable (in class.c's build_vtbl_initializer).  For a compiler-
+ generated destructor, it hence might not have been generated in
+ this translation unit - and with '#pragma interface' it might
+ never get generated.  */
+  if (CLASSTYPE_PURE_VIRTUALS (ctype)
+  && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (ctype)
+  && DECL_DEFAULTED_IN_CLASS_P(CLASSTYPE_DESTRUCTOR(ctype))
+  && DECL_DEFAULTED_FN (CLASSTYPE_DESTRUCTOR(ctype)))
+note_vague_linkage_fn (CLASSTYPE_DESTRUCTOR(ctype));
+
   /* Since we're writing out the vtable here, also write the debug
  info.  */
   note_debug_info_needed (ctype);
@@ -5497,7 +5508,7 @@ mark_used (tree decl, tsubst_flags_t complain)
 	 within the body of a function so as to avoid collecting live data
 	 on the stack (such as overload resolution candidates).
 
- We could just let cp_write_global_declarations handle synthesizing
+ We could just let c_parse_final_cleanups handle synthesizing
  this function by adding it to deferred_fns, but doing
  it at the use site produces better error messages.  */
   ++function_depth;
diff --git a/gcc/testsuite/g++.dg/cpp0x/defaulted61.C b/gcc/testsuite/g++.dg/cpp0x/defaulted61.C
new file mode 100644
index 000..e7e0a486292
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/defaulted61.C
@@ -0,0 +1,22 @@
+// { dg-do compile { target c++11 } }
+// { dg-final { scan-assembler "_ZN3OneD0Ev" } }
+
+// PR C++/88114
+// Destructor of an abstract class was never generated
+// when compiling the class - nor later due to the
+// '#pragma interface'
+
+#pragma implementation
+#pragma interface
+
+class One
+{
+ public:
+  virtual ~One() = default;
+  void some_fn();
+  virtual void later() = 0;
+ private:
+  int m_int;
+};
+
+void One::some_fn() { }
diff --git a/gcc/testsuite/g++.dg/cpp0x/defaulted62.C b/gcc/testsuite/g++.dg/cpp0x/defaulted62.C
new file mode 100644
index 000..d8dab608816
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/defaulted62.C
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++11 } }
+// { dg-final { scan-assembler-not "_ZN3OneD0Ev" } }
+
+// PR C++/88114
+// Destructor of an abstract class was never generated
+// when compiling the class - nor later due to the
+// '#pragma interface'
+// -> g++.dg/cpp0x/defaulted61.C
+
+// HERE, in g++.dg/cpp0x/defaulted62.C:
+// As we have commented the pragmas, it should NOT be created
+// #pragma implementation
+// #pragma interface
+
+class One
+{
+ public:
+  virtual ~One() = default;
+  void some_fn();
+  virtual void later() = 0;
+ private:
+  int m_int;
+};
+
+void One::some_fn() { }

Re: [PATCH] Fix ICU miscompilation due to tree-ssa-strlen.c bug (PR tree-optimization/88693)

[Jeff Law]

On 1/4/19 3:54 PM, Jakub Jelinek wrote:
> Hi!
> 
> The following patch fixes ICU miscompilation, where an initial part of a
> buffer is initializer from non-zero terminated string literal (in ICU
> actually from an array of non-zero chars that the FE newly turns into
> non-zero terminated string literal), but the code recently added to
> tree-ssa-strlen.c doesn't consider the possibility of string literals that
> aren't zero terminated.
> 
> STRING_CSTs actually are always zero terminated, but if TREE_STRING_LENGTH
> doesn't include the terminating character, it is just bytes.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
> 
> I was considering using strnlen, but if all STRING_CSTs are actually zero
> terminated, it will mean it reads are most one further byte and on many
> targets strlen is more optimized than strnlen.
> 
> 2019-01-04  Jakub Jelinek  
> 
>   PR tree-optimization/88693
>   * tree-ssa-strlen.c (get_min_string_length): Don't set *full_string_p
>   for STRING_CSTs that don't contain any NUL characters in the first
>   TREE_STRING_LENGTH bytes.
> 
>   * gcc.c-torture/execute/pr88693.c: New test.
OK
jeff

Re: [PATCH 1/2][GCC][AArch64] Implement hint intrinsics for AArch64

[Sudakshina Das]

Hi Srinath

On 10/01/19 19:20, Srinath Parvathaneni wrote:
> Hi All,
> 
> This patch implements the ACLE hint intrinsics (nop, yield, wfe, wfi,
> sev and sevl), for AArch64.
> 
> The instructions are documented in the ArmARM[1] and the intrinsics
> specification will be
> published on the Arm website [2].
> 
> [1]
> https://developer.arm.com/docs/ddi0487/latest/arm-architecture-reference-manual-armv8-for-armv8-a-architecture-profile
> [2]
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0053c/IHI0053C_acle_2_0.pdf
> 
> Bootstrapped on aarch64-none-linux-gnu and regression tested on
> aarch64-none-elf with no regressions.
> 
> Ok for trunk? If ok, could someone commit the patch on my behalf, I
> don't have commit rights.
> 
> Thanks,
> Srinath
> 
> gcc/ChangeLog:
> 
> 2019-01-10  Srinath Parvathaneni  
> 
>   * config/aarch64/aarch64.md (yield): New pattern name.
>   (wfe): Likewise.
>   (wfi): Likewise.
>   (sev): Likewise.
>   (sevl): Likewise.
>   (UNSPECV_YIELD): New volatile unspec.
>   (UNSPECV_WFE): Likewise.
>   (UNSPECV_WFI): Likewise.
>   (UNSPECV_SEV): Likewise.
>   (UNSPECV_SEVL): Likewise.
>   * config/aarch64/aarch64-builtins.c (aarch64_builtins):
>   AARCH64_SYSHINTOP_BUILTIN_NOP: New builtin.
>   AARCH64_SYSHINTOP_BUILTIN_YIELD: Likewise.
>   AARCH64_SYSHINTOP_BUILTIN_WFE: Likewise.
>   AARCH64_SYSHINTOP_BUILTIN_WFI: Likewise.
>   AARCH64_SYSHINTOP_BUILTIN_SEV: Likewise.
>   AARCH64_SYSHINTOP_BUILTIN_SEVL: Likewise.
>   (aarch64_init_syshintop_builtins): New function.
>   (aarch64_init_builtins): New call statement.
>   (aarch64_expand_builtin): New case.
>   * config/aarch64/arm_acle.h (__nop ): New inline function.
>   (__yield): Likewise.
>   (__sev): Likewise.
>   (__sevl): Likewise.
>   (__wfi): Likewise.
>   (__wfe): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-01-10  Srinath Parvathaneni  
> 
>   * gcc.target/aarch64/acle/hint-1.c: New test.
>   * gcc.target/aarch64/acle/hint-2.c: Likewise.
> 
> 

Thank you for doing this and I am not a maintainer. I have some comments 
bellow:

diff --git a/gcc/config/aarch64/aarch64-builtins.c 
b/gcc/config/aarch64/aarch64-builtins.c
index 
8cced94567008e28b1761ec8771589a3925f2904..d5424f98df1f5c8f206cbded097bdd2dfcd1ca8e
 
100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -399,6 +399,13 @@ enum aarch64_builtins
AARCH64_PAUTH_BUILTIN_AUTIA1716,
AARCH64_PAUTH_BUILTIN_PACIA1716,
AARCH64_PAUTH_BUILTIN_XPACLRI,
+  /* System Hint Operation Builtins for AArch64.  */
+  AARCH64_SYSHINTOP_BUILTIN_NOP,
+  AARCH64_SYSHINTOP_BUILTIN_YIELD,
+  AARCH64_SYSHINTOP_BUILTIN_WFE,
+  AARCH64_SYSHINTOP_BUILTIN_WFI,
+  AARCH64_SYSHINTOP_BUILTIN_SEV,
+  AARCH64_SYSHINTOP_BUILTIN_SEVL,
AARCH64_BUILTIN_MAX
  };

Is there any reason for the naming? They don't seem to be part of any 
extensions? IMHO AARCH64_BUILTIN_NOP, etc looks cleaner and follows 
other builtins which are not part of any extensions.

...
@@ -1395,6 +1436,29 @@ aarch64_expand_builtin (tree exp,
}

return target;
+case AARCH64_SYSHINTOP_BUILTIN_NOP:
+  emit_insn (GEN_FCN (CODE_FOR_nop) ());
+  return gen_reg_rtx (VOIDmode);
+

Needs a newline before the new case.

...
+(define_insn "yield"
+  [(unspec_volatile [(const_int 0)] UNSPECV_YIELD)]
+  ""
+  "yield"
+  [(set_attr "type" "coproc")]
+)

I don't believe setting the type to coproc in AArch64 is correct. 
Likewise for the other instructions.

...
+/* Test the nop ACLE hint intrinsic */
+/* { dg-do compile } */
+/* { dg-additional-options "-O0" } */
+/* { dg-options "-march=armv8-a" } */
+
+#include "arm_acle.h"
+
+void
+test_hint (void)
+{
+ __nop ();
+}
+
+/* { dg-final { scan-assembler-times "\tnop" 3 } } */

Just curious, why are there 3 nops here?

Thanks
Sudi

> 
> 
> 
> 

Re: [wwwdocs] Start of gcc-9/porting_to.html, some C/C++ changes.html changes

[Jeff Law]

On 1/11/19 6:34 AM, Jakub Jelinek wrote:
> Hi!
> 
> The following patch
> 1) mentions (partial) OpenMP 5.0 support and __builtin_convertvector in
>c-family section of gcc-9/changes.html
> 2) mentions C++2a progress in similar way how we've done it in
>gcc-8/changes.html (the list doesn't include all features, feel free
>to suggest what should go out of that list or be added and with what
>wording, some of the cxx-status.html feature names don't look usable
>in the list to me
> 3) lists std::is_constant_evaluated as implemented in cxx-status.html
> 4) adds beginning of gcc-9/porting_to.html , so far just mentions there
>the lifetime of automatic compound literals
> 
> Ok for wwwdocs?
OK.  We need a place to start dumping this stuff, even if we do heavy
editing later.

jeff

Re: [PATCH][RFC] Extend locations where to seach for Fortran pre-include.

[Joseph Myers]

On Fri, 11 Jan 2019, Martin Liška wrote:

> +/* Same as add_prefix, but prepending target_sysroot_hdrs_suffix to prefix.  
> */

Actually, it should be prepending target_system_root, but followed by 
target_sysroot_hdrs_suffix rather than target_sysroot_suffix.  That is, 
this function should be following add_sysrooted_prefix more closely.

> +  if (target_sysroot_hdrs_suffix)

So this should be "if (target_system_root)" - it needs to be sysrooted 
even if there is no sysroot headers suffix.

> +{
> +  char *sysroot_no_trailing_dir_separator
> + = xstrdup (target_sysroot_hdrs_suffix);
> +  size_t sysroot_len = strlen (target_sysroot_hdrs_suffix);

And again this would use target_system_root.

> +  if (sysroot_len > 0
> +   && target_sysroot_hdrs_suffix[sysroot_len - 1] == DIR_SEPARATOR)
> + sysroot_no_trailing_dir_separator[sysroot_len - 1] = '\0';

Likewise.

> +  if (target_sysroot_suffix)
> + prefix = concat (sysroot_no_trailing_dir_separator,
> +  target_sysroot_suffix, prefix, NULL);

While this would use target_sysroot_hdrs_suffix.

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

Re: [PATCH] Fix x86 PLUGIN_HEADERS

[Jeff Law]

On 1/11/19 9:42 AM, Jakub Jelinek wrote:
> Hi!
> 
> Since r264052 i386.h includes insn-attr-common.h, which unfortunately isn't
> installed, because all we install is:
> $(PLUGIN_HEADERS) $$(cd $(srcdir); echo *.h *.def)
> and thus plugins compiled against installed tree will not work on x86
> if they need to include anything that includes tm.h.
> 
> Below are two possible patches to resolve this.
> 
> From the generated headers, we already install
> insn-addr.h insn-codes.h insn-constants.h insn-flags.h insn-modes-inline.h 
> insn-modes.h insn-notes.def 
> so the missing insn-attr.h insn-attr-common.h looks like an omission to me,
> so the first patch makes sure we install even those two headers on all
> targets.
> 
> The other option is install insn-attr-common.h only on x86 where i386.h
> includes it and don't install insn-attr.h at all, which is what the second
> patch does.
> 
> Tested on x86_64-linux, ok for trunk (which patch)?
OK.  Jeff

[PATCH] Fix x86 PLUGIN_HEADERS

[Jakub Jelinek]

Hi!

Since r264052 i386.h includes insn-attr-common.h, which unfortunately isn't
installed, because all we install is:
$(PLUGIN_HEADERS) $$(cd $(srcdir); echo *.h *.def)
and thus plugins compiled against installed tree will not work on x86
if they need to include anything that includes tm.h.

Below are two possible patches to resolve this.

>From the generated headers, we already install
insn-addr.h insn-codes.h insn-constants.h insn-flags.h insn-modes-inline.h 
insn-modes.h insn-notes.def 
so the missing insn-attr.h insn-attr-common.h looks like an omission to me,
so the first patch makes sure we install even those two headers on all
targets.

The other option is install insn-attr-common.h only on x86 where i386.h
includes it and don't install insn-attr.h at all, which is what the second
patch does.

Tested on x86_64-linux, ok for trunk (which patch)?

2019-01-11  Jakub Jelinek  

* Makefile.in (PLUGIN_HEADERS): Add $(INSN_ATTR_H).

--- gcc/Makefile.in.jj  2019-01-10 11:43:17.05900 +0100
+++ gcc/Makefile.in 2019-01-11 17:31:36.858281784 +0100
@@ -3509,7 +3509,7 @@ PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $
   tree-ssa-loop-niter.h tree-ssa-ter.h tree-ssa-threadedge.h \
   tree-ssa-threadupdate.h inchash.h wide-int.h signop.h hash-map.h \
   hash-set.h dominance.h cfg.h cfgrtl.h cfganal.h cfgbuild.h cfgcleanup.h \
-  lcm.h cfgloopmanip.h file-prefix-map.h builtins.def \
+  lcm.h cfgloopmanip.h file-prefix-map.h builtins.def $(INSN_ATTR_H) \
   pass-instances.def params.list
 
 # generate the 'build fragment' b-header-vars

Jakub
2019-01-11  Jakub Jelinek  

* config/i386/t-i386 (TM_H): Add insn-attr-common.h.

--- gcc/config/i386/t-i386.jj   2019-01-01 12:37:32.919716339 +0100
+++ gcc/config/i386/t-i386  2019-01-11 17:36:37.535389125 +0100
@@ -17,7 +17,7 @@
 # .
 
 OPTIONS_H_EXTRA += $(srcdir)/config/i386/stringop.def
-TM_H += $(srcdir)/config/i386/x86-tune.def
+TM_H += $(srcdir)/config/i386/x86-tune.def insn-attr-common.h
 PASSES_EXTRA += $(srcdir)/config/i386/i386-passes.def
 
 i386-c.o: $(srcdir)/config/i386/i386-c.c

Re: patch to fix PR87305

[Vladimir Makarov]

On 01/11/2019 04:58 AM, Richard Sandiford wrote:

Hi Vlad,

I think for !WORDS_BIG_ENDIAN the equivalent problem to:

|| !TEST_HARD_REG_BIT (reg_class_contents[rclass],
   hard_regno - nregs_diff)))

would be:

|| !TEST_HARD_REG_BIT (reg_class_contents[rclass],
   hard_regno + biggest_nregs - 1)))

Where does that little-endian check happen?  I wasn't sure why
"biggest_mode goes outside the class" problems only occured here
for big-endian.


Yes, you are right.  Although probability of the same problem for little 
endian is much smaller, it is still present.  I am working on it.


It would be nice to avoid wrong assignments in IRA too but it is too 
much work.  LRA will fix wrong assignments (by spilling and probably 
reassignment afterwards) but sometimes with worse result.

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Marek Polacek]

On Mon, Jan 07, 2019 at 04:59:14PM -0500, Jason Merrill wrote:
> On 1/7/19 4:29 PM, Marek Polacek wrote:
> > This patch fixes bogus -Wredundant-move warnings reported in 88692 and 
> > 87882.
> > 
> > To quickly recap, this warning is supposed to warn for cases like
> > 
> > struct T { };
> > 
> > T fn(T t)
> > {
> >return std::move (t);
> > }
> > 
> > where NRVO isn't applicable for T because it's a parameter, but it's
> > a local variable and we're returning, so C++11 says activate move
> > semantics, so the std::move is redundant.  But, as these testcases show,
> > we're failing to realize that that is not the case when returning *this,
> > which is disguised as an ordinary PARM_DECL, and treat_lvalue_as_rvalue_p
> > was fooled by that.
> 
> Hmm, the function isn't returning 'this', it's returning '*this'.  I guess
> what's happening is that in order to pass *this to the reference parameter
> of move, we end up converting it from pointer to reference by NOP_EXPR, and
> the STRIP_NOPS in maybe_warn_pessimizing_move throws that away so that it
> then thinks we're returning 'this'.  I expect the same thing could happen
> with any parameter of pointer-to-class type.

You're right, I didn't realize that we warned even for parameters of 
pointer-to-class
types.  So why don't we disable the warning for PARM_DECLs with pointer types?

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

2019-01-11  Marek Polacek  

PR c++/88692 - -Wredundant-move false positive with *this.
* typeck.c (maybe_warn_pessimizing_move): Don't issue Wredundant-move
warnings for variables of pointer types.

* g++.dg/cpp0x/Wredundant-move5.C: New test.
* g++.dg/cpp0x/Wredundant-move6.C: New test.

diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index e399cd3fe45..2b26e49f676 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9426,7 +9426,8 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
}
  /* Warn if the move is redundant.  It is redundant when we would
 do maybe-rvalue overload resolution even without std::move.  */
- else if (treat_lvalue_as_rvalue_p (arg, /*parm_ok*/true))
+ else if (!POINTER_TYPE_P (TREE_TYPE (arg))
+  && treat_lvalue_as_rvalue_p (arg, /*parm_ok*/true))
{
  auto_diagnostic_group d;
  if (warning_at (loc, OPT_Wredundant_move,
diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C 
gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C
new file mode 100644
index 000..0e2ec46d11e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C
@@ -0,0 +1,53 @@
+// PR c++/88692
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wredundant-move" }
+
+// Define std::move.
+namespace std {
+  template
+struct remove_reference
+{ typedef _Tp   type; };
+
+  template
+struct remove_reference<_Tp&>
+{ typedef _Tp   type; };
+
+  template
+struct remove_reference<_Tp&&>
+{ typedef _Tp   type; };
+
+  template
+constexpr typename std::remove_reference<_Tp>::type&&
+move(_Tp&& __t) noexcept
+{ return static_cast::type&&>(__t); }
+}
+
+struct X {
+X f() && {
+return std::move(*this); // { dg-bogus "redundant move in return 
statement" }
+}
+
+X f2() & {
+return std::move(*this); // { dg-bogus "redundant move in return 
statement" }
+}
+
+X f3() {
+return std::move(*this); // { dg-bogus "redundant move in return 
statement" }
+}
+};
+
+struct S { int i; int j; };
+
+struct Y {
+  S f1 (S s) {
+return std::move (s); // { dg-warning "redundant move in return statement" 
}
+  }
+
+  S f2 (S* s) {
+return std::move (*s); // { dg-bogus "redundant move in return statement" }
+  }
+
+  S f3 (S** s) {
+return std::move (**s); // { dg-bogus "redundant move in return statement" 
}
+  }
+};
diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move6.C 
gcc/testsuite/g++.dg/cpp0x/Wredundant-move6.C
new file mode 100644
index 000..5808a78638e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move6.C
@@ -0,0 +1,43 @@
+// PR c++/87882
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wredundant-move" }
+
+// Define std::move.
+namespace std {
+  template
+struct remove_reference
+{ typedef _Tp   type; };
+
+  template
+struct remove_reference<_Tp&>
+{ typedef _Tp   type; };
+
+  template
+struct remove_reference<_Tp&&>
+{ typedef _Tp   type; };
+
+  template
+constexpr typename std::remove_reference<_Tp>::type&&
+move(_Tp&& __t) noexcept
+{ return static_cast::type&&>(__t); }
+}
+
+struct Foo {
+   Foo Bar() {
+ return std::move(*this); // { dg-bogus "redundant move in return 
statement" }
+   }
+   Foo Baz() {
+ return *this;
+   }
+  int i;
+};
+
+void Move(Foo & f)
+{
+  f = Foo{}.Bar();
+}
+
+void NoMove(Foo & f)
+{
+  f = Foo{}.Baz();
+}

Re: [PATCH] Define __cpp_lib_is_constant_evaluated

[Jonathan Wakely]

On 11/01/19 16:51 +0100, Jakub Jelinek wrote:

Hi!

Seems __cpp_lib_is_constant_evaluated should be defined to 201811L
if std::is_constant_evaluated() is implemented.

Ok for trunk?


OK, thanks.


2019-01-11  Jakub Jelinek  

* include/std/type_traits (__cpp_lib_is_constant_evaluated): Define.
* include/std/version (__cpp_lib_is_constant_evaluated): Define.

--- libstdc++-v3/include/std/type_traits.jj 2019-01-01 12:45:50.842546655 
+0100
+++ libstdc++-v3/include/std/type_traits2019-01-11 16:44:39.546916488 
+0100
@@ -3030,6 +3030,9 @@ template 
using unwrap_ref_decay_t = typename unwrap_ref_decay<_Tp>::type;

#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+
+#define __cpp_lib_is_constant_evaluated 201811L
+
  constexpr inline bool
  is_constant_evaluated() noexcept
  { return __builtin_is_constant_evaluated(); }
--- libstdc++-v3/include/std/version.jj 2019-01-10 16:42:18.081062695 +0100
+++ libstdc++-v3/include/std/version2019-01-11 16:48:35.944119391 +0100
@@ -142,6 +142,9 @@

#if __cplusplus > 201703L
// c++2a
+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+# define __cpp_lib_is_constant_evaluated 201811L
+#endif
#define __cpp_lib_list_remove_return_type 201806L
#endif // C++2a
#endif // C++17

Jakub

[wwwdocs] Add __cpp_* feature macros to C++20 entries that have those in projects/cxx_status.html

[Jakub Jelinek]

Hi!

I've noticed we don't have any feature test macros in the table for C++20,
even when a couple of the features have them defined.

Ok for wwwdocs?

--- htdocs/projects/cxx-status.html.jj  2019-01-11 14:24:06.524979265 +0100
+++ htdocs/projects/cxx-status.html 2019-01-11 16:41:16.650157887 +0100
@@ -148,7 +148,7 @@
http://wg21.link/P0905r1";>P0905R1
http://wg21.link/p1120r0";>P1120R0
No 
-   
+   __cpp_impl_three_way_comparison >= 201711 
 
 
Access checking on specializations 
@@ -220,7 +220,7 @@
Class Types in Non-Type Template Parameters 
   http://wg21.link/p0732r2";>P0732R2
9 
-   
+   __cpp_nontype_template_parameter_class >= 201806 
 
 
Atomic Compare-and-Exchange with Padding Bits 
@@ -232,7 +232,7 @@
Efficient sized delete for variable sized classes 
   http://wg21.link/p0722r3";>P0722R3
9 
-   
+   __cpp_impl_destroying_delete >= 201806 
 
 
Allowing Virtual Function Calls in Constant Expressions 
@@ -257,7 +257,7 @@
explicit(bool) 
   http://wg21.link/p0892r2";>P0892R2
9 
-   
+   __cpp_conditional_explicit >= 201806 
 
 
Signed integers are two's complement 
@@ -269,7 +269,7 @@
char8_t 
   http://wg21.link/p0482r6";>P0482R6
No 
-   
+   __cpp_char8_t >= 201811 
 
 
Immediate functions (consteval) 

Jakub

[PATCH] Define __cpp_lib_is_constant_evaluated

[Jakub Jelinek]

Hi!

Seems __cpp_lib_is_constant_evaluated should be defined to 201811L
if std::is_constant_evaluated() is implemented.

Ok for trunk?

2019-01-11  Jakub Jelinek  

* include/std/type_traits (__cpp_lib_is_constant_evaluated): Define.
* include/std/version (__cpp_lib_is_constant_evaluated): Define.

--- libstdc++-v3/include/std/type_traits.jj 2019-01-01 12:45:50.842546655 
+0100
+++ libstdc++-v3/include/std/type_traits2019-01-11 16:44:39.546916488 
+0100
@@ -3030,6 +3030,9 @@ template 
 using unwrap_ref_decay_t = typename unwrap_ref_decay<_Tp>::type;
 
 #ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+
+#define __cpp_lib_is_constant_evaluated 201811L
+
   constexpr inline bool
   is_constant_evaluated() noexcept
   { return __builtin_is_constant_evaluated(); }
--- libstdc++-v3/include/std/version.jj 2019-01-10 16:42:18.081062695 +0100
+++ libstdc++-v3/include/std/version2019-01-11 16:48:35.944119391 +0100
@@ -142,6 +142,9 @@
 
 #if __cplusplus > 201703L
 // c++2a
+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+# define __cpp_lib_is_constant_evaluated 201811L
+#endif
 #define __cpp_lib_list_remove_return_type 201806L
 #endif // C++2a
 #endif // C++17

Jakub

Re: [PATCH][RFC] Extend locations where to seach for Fortran pre-include.

[Martin Liška]

On 1/9/19 6:15 PM, Joseph Myers wrote:
> On Wed, 9 Jan 2019, Martin Liška wrote:
> 
>> Hi.
>>
>> May I please ping that Joseph. The provided patch should be applicable and
>> I would need help with the proper locations.
> 
> I'm not clear if there is a specific question here, or a patch needing 
> review.  In  I 
> noted a specific issue: TOOL_INCLUDE_DIR should be a non-sysrooted prefix.
> 
> Also, could you clarify what, in the patch 
>  (if that's 
> still the current version),
> 
>> +  add_sysrooted_hdrs_prefix (&prefixes, argv[2], NULL, 0, 0, false);
> 
> is doing (that is, examples of what sort of prefixes it's adding)?
> 

Hi.

I'm sending updated version of the patch that addresses the 2 notes
mentioned by Joseph.

Martin
>From 531180219809153592100a9eb92ca07b1bf51bda Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 20 Nov 2018 15:09:16 +0100
Subject: [PATCH] Extend locations where to seach for Fortran pre-include.

---
 gcc/Makefile.in   |   4 +-
 gcc/config/gnu-user.h |   2 +-
 gcc/gcc.c | 104 ++
 3 files changed, 88 insertions(+), 22 deletions(-)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 2fa9083d1b3..095156bd537 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2172,7 +2172,9 @@ DRIVER_DEFINES = \
   @TARGET_SYSTEM_ROOT_DEFINE@ \
   $(VALGRIND_DRIVER_DEFINES) \
   $(if $(SHLIB),$(if $(filter yes,@enable_shared@),-DENABLE_SHARED_LIBGCC)) \
-  -DCONFIGURE_SPECS="\"@CONFIGURE_SPECS@\""
+  -DCONFIGURE_SPECS="\"@CONFIGURE_SPECS@\"" \
+  -DTOOL_INCLUDE_DIR=\"$(gcc_tooldir)/include\" \
+  -DNATIVE_SYSTEM_HEADER_DIR=\"$(NATIVE_SYSTEM_HEADER_DIR)\"
 
 CFLAGS-gcc.o += $(DRIVER_DEFINES) -DBASEVER=$(BASEVER_s)
 gcc.o: $(BASEVER)
diff --git a/gcc/config/gnu-user.h b/gcc/config/gnu-user.h
index ba146921655..055a4f0afec 100644
--- a/gcc/config/gnu-user.h
+++ b/gcc/config/gnu-user.h
@@ -151,4 +151,4 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 
 #undef TARGET_F951_OPTIONS
 #define TARGET_F951_OPTIONS "%{!nostdinc:\
-  %:fortran-preinclude-file(-fpre-include= math-vector-fortran.h)}"
+  %:fortran-preinclude-file(-fpre-include= math-vector-fortran.h finclude%s/)}"
diff --git a/gcc/gcc.c b/gcc/gcc.c
index bcd04df1691..79e1e7d5f6e 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -2976,6 +2976,45 @@ add_sysrooted_prefix (struct path_prefix *pprefix, const char *prefix,
   add_prefix (pprefix, prefix, component, priority,
 	  require_machine_suffix, os_multilib);
 }
+
+/* Same as add_prefix, but prepending target_sysroot_hdrs_suffix to prefix.  */
+
+static void
+add_sysrooted_hdrs_prefix (struct path_prefix *pprefix, const char *prefix,
+			   const char *component,
+			   /* enum prefix_priority */ int priority,
+			   int require_machine_suffix, int os_multilib)
+{
+  if (!IS_ABSOLUTE_PATH (prefix))
+fatal_error (input_location, "system path %qs is not absolute", prefix);
+
+  if (target_sysroot_hdrs_suffix)
+{
+  char *sysroot_no_trailing_dir_separator
+	= xstrdup (target_sysroot_hdrs_suffix);
+  size_t sysroot_len = strlen (target_sysroot_hdrs_suffix);
+
+  if (sysroot_len > 0
+	  && target_sysroot_hdrs_suffix[sysroot_len - 1] == DIR_SEPARATOR)
+	sysroot_no_trailing_dir_separator[sysroot_len - 1] = '\0';
+
+  if (target_sysroot_suffix)
+	prefix = concat (sysroot_no_trailing_dir_separator,
+			 target_sysroot_suffix, prefix, NULL);
+  else
+	prefix = concat (sysroot_no_trailing_dir_separator, prefix, NULL);
+
+  free (sysroot_no_trailing_dir_separator);
+
+  /* We have to override this because GCC's notion of sysroot
+	 moves along with GCC.  */
+  component = "GCC";
+}
+
+  add_prefix (pprefix, prefix, component, priority,
+	  require_machine_suffix, os_multilib);
+}
+
 
 /* Execute the command specified by the arguments on the current line of spec.
When using pipes, this includes several piped-together commands
@@ -9896,20 +9935,61 @@ debug_level_greater_than_spec_func (int argc, const char **argv)
   return NULL;
 }
 
-/* The function takes 2 arguments: OPTION name and file name.
+static void
+path_prefix_reset (path_prefix *prefix)
+{
+  struct prefix_list *iter, *next;
+  iter = prefix->plist;
+  while (iter)
+{
+  next = iter->next;
+  free (const_cast  (iter->prefix));
+  XDELETE (iter);
+  iter = next;
+}
+  prefix->plist = 0;
+  prefix->max_len = 0;
+}
+
+/* The function takes 3 arguments: OPTION name, file name and location
+   where we search for Fortran modules.
When the FILE is found by find_file, return OPTION=path_to_file.  */
 
 static const char *
 find_fortran_preinclude_file (int argc, const char **argv)
 {
-  if (argc != 2)
+  char *result = NULL;
+  if (argc != 3)
 return NULL;
 
+  struct path_prefix prefixes = { 0, 0, "preinclude" };
+
+  /* Search first for 'finclude' folder location for a head

[PATCH] PR libstdc++/88802 define std::hash for C++17

[Jonathan Wakely]

PR libstdc++/88802
* include/bits/functional_hash.h (hash): Define
specialization for C++17 (P0513R0, LWG 2817).
* testsuite/20_util/hash/nullptr.cc: New test.

Tested x86_64-linux, committed to trunk.

commit 3ade1f14e79b8bbb4939244e9125f9043de6
Author: Jonathan Wakely 
Date:   Fri Jan 11 14:26:34 2019 +

PR libstdc++/88802 define std::hash for C++17

PR libstdc++/88802
* include/bits/functional_hash.h (hash): Define
specialization for C++17 (P0513R0, LWG 2817).
* testsuite/20_util/hash/nullptr.cc: New test.

diff --git a/libstdc++-v3/include/bits/functional_hash.h 
b/libstdc++-v3/include/bits/functional_hash.h
index 2245fad69a7..32dc00c5a6f 100644
--- a/libstdc++-v3/include/bits/functional_hash.h
+++ b/libstdc++-v3/include/bits/functional_hash.h
@@ -254,6 +254,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   operator()(long double __val) const noexcept;
 };
 
+#if __cplusplus >= 201703L
+  template<>
+struct hash : public __hash_base
+{
+  size_t
+  operator()(nullptr_t) const noexcept
+  { return 0; }
+};
+#endif
+
   // @} group hashes
 
   // Hint about performance of hash functor. If not fast the hash-based
diff --git a/libstdc++-v3/testsuite/20_util/hash/nullptr.cc 
b/libstdc++-v3/testsuite/20_util/hash/nullptr.cc
new file mode 100644
index 000..366db679c36
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/hash/nullptr.cc
@@ -0,0 +1,37 @@
+// Copyright (C) 2019 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 copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++17" }
+// { dg-do run { target c++17 } }
+
+#include 
+#include 
+
+void
+test01()
+{
+  auto h1 = std::hash{}(nullptr);
+  static_assert(std::is_same_v);
+  auto h2 = std::hash{}(nullptr);
+  VERIFY( h1 == h2 );
+}
+
+int
+main()
+{
+  test01();
+}

Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI

[Richard Sandiford]

Steve Ellcey  writes:
> Here is an updated version of the GCC patch to enable SIMD functions on
> Aarch64.  There are a number of changes from the last patch.
>
> I reduced the shared code changes, there is still one change in shared code
> (omp-simd-clone.c) to call targetm.simd_clone.adjust from expand_simd_clones
> but it now uses the same argument as the existing call.  This new call allows
> Aarch64 to add the aarch64_vector_pcs attribute to SIMD clone definitions
> which in turn ensures they use the correct ABI.  Previously this target
> function was only called on declarations, not definitions.  This change 
> affects
> the x86 target so I modified ix86_simd_clone_adjust to return and do nothing
> when called with a definition.  This means there is no change in behaviour
> on x86.  I did a build and GCC testsuite run on x86 to verify this.
>
> Most of the changes from the previous patch are in the
> aarch64_simd_clone_compute_vecsize_and_simdlen function.
>
> The previous version was heavily based on the x86 function, this one has
> changes to address the issues that were raised in the earlier patch
> and so it no longer looks like the x86 version.  I use types instead of modes
> to check for what we can/cannot vectorize and I (try to) differentiate
> between vectors that we are not currently handling (but could later) and
> those that won't ever be handled.
>
> I have also added a testsuite patch to fix regressions in the gcc.dg/gomp
> and g++.dg/gomp tests.  There are no regressions with this patch applied.
>
> Steve Ellcey
> sell...@marvell.com
>
> 2018-12-19  Steve Ellcey  
>
>   * config/aarch64/aarch64.c (cgraph.h): New include.
>   (supported_simd_type): New function.
>   (currently_supported_simd_type): Ditto.
>   (aarch64_simd_clone_compute_vecsize_and_simdlen): Ditto.
>   (aarch64_simd_clone_adjust): Ditto.
>   (aarch64_simd_clone_usable): Ditto.
>   (TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN): New macro.
>   (TARGET_SIMD_CLONE_ADJUST): Ditto.
>   (TARGET_SIMD_CLONE_USABLE): Ditto.
>   * config/i386/i386.c (ix86_simd_clone_adjust): Add definition check.
>   * omp-simd-clone.c (expand_simd_clones): Add targetm.simd_clone.adjust
>   call.
>
> 2018-12-19  Steve Ellcey  
>
>   * g++.dg/gomp/declare-simd-1.C: Add aarch64 specific
>   warning checks and assembler scans.
>   * g++.dg/gomp/declare-simd-3.C: Ditto.
>   * g++.dg/gomp/declare-simd-4.C: Ditto.
>   * g++.dg/gomp/declare-simd-7.C: Ditto.
>   * gcc.dg/gomp/declare-simd-1.c: Ditto.
>   * gcc.dg/gomp/declare-simd-3.c: Ditto.

Sorry, hadn't realised this was still unreviewed.

> @@ -18064,6 +18066,138 @@ aarch64_estimated_poly_value (poly_int64 val)
>return val.coeffs[0] + val.coeffs[1] * over_128 / 128;
>  }
>  
> +
> +/* Return true for types that could be supported as SIMD return or
> +   argument types.  */
> +
> +static bool supported_simd_type (tree t)
> +{
> +  return (FLOAT_TYPE_P (t) || INTEGRAL_TYPE_P (t));

We should also check that the size is 1, 2, 4 or 8 bytes.

> +}
> +
> +/* Return true for types that currently are supported as SIMD return
> +   or argument types.  */
> +
> +static bool currently_supported_simd_type (tree t)
> +{
> +  if (COMPLEX_FLOAT_TYPE_P (t))
> +return false;
> +
> +  return supported_simd_type (t);
> +}
> +
> +/* Implement TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN.  */
> +
> +static int
> +aarch64_simd_clone_compute_vecsize_and_simdlen (struct cgraph_node *node,
> + struct cgraph_simd_clone *clonei,
> + tree base_type,
> + int num ATTRIBUTE_UNUSED)
> +{
> +  const char *wmsg;
> +  int vsize;
> +  tree t, ret_type, arg_type;
> +
> +  if (!TARGET_SIMD)
> +return 0;
> +
> +  if (clonei->simdlen
> +  && (clonei->simdlen < 2
> +   || clonei->simdlen > 1024
> +   || (clonei->simdlen & (clonei->simdlen - 1)) != 0))
> +{
> +  warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
> +   "unsupported simdlen %d", clonei->simdlen);
> +  return 0;
> +}
> +
> +  ret_type = TREE_TYPE (TREE_TYPE (node->decl));
> +  if (TREE_CODE (ret_type) != VOID_TYPE
> +  && !currently_supported_simd_type (ret_type))
> +{
> +  if (supported_simd_type (ret_type))
> + wmsg = G_("GCC does not currently support return type %qT for simd");
> +  else
> + wmsg = G_("unsupported return type return type %qT for simd");

Typo: doubled "return type".

Maybe s/for simd/for % functions/ in both messages.
Since that will blow the line limit...

> +  warning_at (DECL_SOURCE_LOCATION (node->decl), 0, wmsg, ret_type);
> +  return 0;
> +}

...it's probably worth just calling warning_at in each arm of the "if".
We'll then get proper format checking in bootstraps.

> +  for (t = DECL_ARGUMENTS (node->decl); t; t = DECL_CHAIN (t))
> +{
> +  arg_type = TREE_TYPE (t);
> + 

[committed][PATCH][GCC][testsuite][Arm] fix testism, add required option after require.

[Tamar Christina]

Hi All,

The test declared the fp16 requirement, but didn't add the options causing it to
fail when the target doesn't have it on by default.

Bootstrapped Regtested on arm-none-Linux-gnueabihf and no issues.

committed under the gcc obvious rules.

Thanks,
Tamar

gcc/testsuite/ChangeLog:

2019-01-11  Tamar Christina  

* gcc.target/aarch64/advsimd-intrinsics/vector-complex_f16.c: Require 
neon
and add options.

-- 
diff --git a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vector-complex_f16.c b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vector-complex_f16.c
index 99754b67e4b4f62561a2c094a59bb70d6af4f31a..53442492b3b7e71b87741a7bb1fcc791f311024f 100644
--- a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vector-complex_f16.c
+++ b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vector-complex_f16.c
@@ -1,7 +1,8 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_v8_3a_complex_neon_ok } */
-/* { dg-require-effective-target arm_v8_2a_fp16_scalar_ok } */
+/* { dg-require-effective-target arm_v8_2a_fp16_neon_ok } */
 /* { dg-add-options arm_v8_3a_complex_neon } */
+/* { dg-add-options arm_v8_2a_fp16_neon } */
 /* { dg-additional-options "-O2 -march=armv8.3-a+fp16 -save-temps" } */
 
 #include 

Re: [EXT] Re: [Patch 4/4][Aarch64] v2: Implement Aarch64 SIMD ABI

[Steve Ellcey]

On Fri, 2019-01-11 at 12:30 +0100, Jakub Jelinek wrote:
> 
> > Yeah, like you say, this was originally posted in stage 1 and is the
> > last patch in the series.  Not committing it would leave the work
> > incomplete in GCC 9.  The problem is that we're now in stage 4 rather
> > than stage 3.
> > 
> > I don't think I'm neutral enough to make the call.  Richard, Jakub?
> 
> I'm ok with accepting this late as an exception, if it can be committed
> reasonably soon (within a week or at most two).
> 
>   Jakub

OK, I will make the comment change and check it in.  Note that this
does not give us the complete implementation yet.  Patch 3/4 was OK'ed
by Richard last week but I hadn't checked that in either.  So I will
check both these in later today unless I haer otherwise.

That still leaves Patch 2/4 which is Aarch64 specific.

https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01421.html

Jakub had some comments on the test changes which I fixed but I did
not get any feedback on the actual code changes so I am not sure if 
that is OK or not.

STeve Ellcey
sell...@marvell.com

[wwwdocs] Start of gcc-9/porting_to.html, some C/C++ changes.html changes

[Jakub Jelinek]

Hi!

The following patch
1) mentions (partial) OpenMP 5.0 support and __builtin_convertvector in
   c-family section of gcc-9/changes.html
2) mentions C++2a progress in similar way how we've done it in
   gcc-8/changes.html (the list doesn't include all features, feel free
   to suggest what should go out of that list or be added and with what
   wording, some of the cxx-status.html feature names don't look usable
   in the list to me
3) lists std::is_constant_evaluated as implemented in cxx-status.html
4) adds beginning of gcc-9/porting_to.html , so far just mentions there
   the lifetime of automatic compound literals

Ok for wwwdocs?

--- htdocs/gcc-9/changes.html.jj2019-01-11 13:09:40.469122800 +0100
+++ htdocs/gcc-9/changes.html   2019-01-11 14:22:42.944333728 +0100
@@ -65,7 +65,21 @@ a work-in-progress.
 
 
 
-
+C family
+
+  Version 5.0 of the https://www.openmp.org/specifications/";
+>OpenMP specification is now partially supported in the C and C++
+  compilers.  For details which features of OpenMP 5.0 are and which are
+  not supported in the GCC 9 release see
+  https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00628.html";>this 
mail.
+  
+
+  New extensions:
+  
+__builtin_convertvector built-in for vector conversions
+  has been added. 
+  
+
 
 C++
 
@@ -79,6 +93,22 @@ a work-in-progress.
   dangling pointer, such as returning or assigning from a temporary
   list. 
   
+
+  
+The C++ front end has experimental support for some of the upcoming C++2a
+draft features with the -std=c++2a or 
-std=gnu++2a
+flags, including range-based for statements with initializer,
+less eager instantiation of constexpr functions, default constructible and
+assignable stateless lambdas, lambdas in unevaluated contexts, language
+support for empty objects, allow pack expansion in lambda init-capture,
+likely and unlikely attributes, class types in non-type template
+parameters, allowing virtual function calls in constant expressions,
+explicit(bool), signed integers are two's complement,
+std::is_constant_evaluated, nested inline namespaces, etc.
+For a full list of new features,
+see the C++
+status page.
+  
 
 
 Runtime Library (libstdc++)
--- htdocs/projects/cxx-status.html.jj  2019-01-11 13:09:41.475106482 +0100
+++ htdocs/projects/cxx-status.html 2019-01-11 14:24:06.524979265 +0100
@@ -280,7 +280,7 @@
 
std::is_constant_evaluated 
   http://wg21.link/p0595r2";>P0595R2
-   No 
+   9 

 
 
--- htdocs/gcc-9/porting_to.html.jj 2019-01-11 13:29:23.291922307 +0100
+++ htdocs/gcc-9/porting_to.html2019-01-11 13:23:03.831090097 +0100
@@ -0,0 +1,72 @@
+
+
+
+
+Porting to GCC 9
+https://gcc.gnu.org/gcc.css"; />
+
+
+
+Porting to GCC 9
+
+
+The GCC 9 release series differs from previous GCC releases in
+a number of ways. Some of these are a result
+of bug fixing, and some old behaviors have been intentionally changed
+to support new standards, or relaxed in standards-conforming ways to
+facilitate compilation or run-time performance.
+
+
+
+Some of these changes are user visible and can cause grief when
+porting to GCC 9. This document is an effort to identify common issues
+and provide solutions. Let us know if you have suggestions for improvements!
+
+
+
+
+
+C language issues
+
+Block scope compound literal's lifetime
+
+
+  The C standard says that compound literals which occur inside of the body
+  of a function have automatic storage duration associated with the
+  enclosing block.  Older GCC releases were putting such compound literals
+  into the scope of the whole function, so their lifetime actually ended
+  at the end of containing function.  This has been fixed in GCC 9.
+  Code that relied on this extended lifetime needs to be fixed, move the
+  compound literals to whatever scope they need to accessible in.
+
+  
+  struct S { int a, b; };
+  int foo(void) {
+// The following line no longer compiles
+struct S *p = &({ (struct S) { 1, 2 }; });
+struct S *q;
+{
+  q = &(struct S) { 3, 4 };
+}
+// This is invalid use after lifetime of the compound literal
+// ended.
+return q->b;
+  }
+  
+
+
+
+
+ 
+
+
+
+

Jakub

Re: [PATCH] Fix up initializer_each_zero_or_onep (PR middle-end/88758)

[Martin Liška]

On 1/9/19 9:43 AM, Richard Biener wrote:
> On Tue, 8 Jan 2019, Jakub Jelinek wrote:
> 
>> Hi!
>>
>> As mentioned in the PR, if a VECTOR_CST is not VECTOR_CST_STEPPED_P,
>> it is sufficient to recurse just on all the encoded elt, because if
>> the vector has more elts than encoded, all the remaining ones are equal to
>> the last one.
>> But, if it is stepped, there is the possibility that while the penultimate
>> and last encoded elt could be zero or one, the first non-encoded one could
>> be two or minus one.
>>
>> The patch as committed is doing:
>> unsigned HOST_WIDE_INT nelts = vector_cst_encoded_nelts (expr);
>> if (VECTOR_CST_STEPPED_P (expr)
>> && !TYPE_VECTOR_SUBPARTS (TREE_TYPE (expr)).is_constant (&nelts))
>>   return false;
>> so if it is stepped, it updates nelts to the subparts count and thus
>> attempts to verify all elts rather than just the encoded ones.  But that
>> fails because VECTOR_CST_ENCODED_ELT can't really access the non-encoded
>> ones.  The following patch fixes it by using vector_cst_elt there instead,
>> for the encoded elt it just returns VECTOR_CST_ENCODED_ELT immediately and
>> for the next value it will likely fail the predicate.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> OK.
> 
> Richard.
> 
>> 2019-01-08  Jelinek  
>>
>>  PR middle-end/88758
>>  * tree.c (initializer_each_zero_or_onep) : Use
>>  vector_cst_elt instead of VECTOR_CST_ENCODED_ELT.
>>
>> --- gcc/tree.c.jj2019-01-07 17:59:22.883951743 +0100
>> +++ gcc/tree.c   2019-01-08 18:15:01.956087119 +0100
>> @@ -11255,7 +11255,7 @@ initializer_each_zero_or_onep (const_tre
>>  
>>  for (unsigned int i = 0; i < nelts; ++i)
>>{
>> -tree elt = VECTOR_CST_ENCODED_ELT (expr, i);
>> +tree elt = vector_cst_elt (expr, i);
>>  if (!initializer_each_zero_or_onep (elt))
>>return false;
>>}
>>
>>  Jakub
>>
>>
> 

I'm adding a test-case for that, tested on x86_64-linux-gnu.
Verified that ICEs when the fix is removed. I'm going to install the patch.

Martin
>From 4c7b3356fff88dc443777f53ebb9e15e451a5d51 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Fri, 11 Jan 2019 14:17:46 +0100
Subject: [PATCH] Add a testcase (PR middle-end/88758).

gcc/testsuite/ChangeLog:

2019-01-11  Martin Liska  

	PR middle-end/88758
	* g++.dg/lto/pr88758_0.C: New test.
	* g++.dg/lto/pr88758_1.C: New test.
---
 gcc/testsuite/g++.dg/lto/pr88758_0.C | 7 +++
 gcc/testsuite/g++.dg/lto/pr88758_1.C | 9 +
 2 files changed, 16 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr88758_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr88758_1.C

diff --git a/gcc/testsuite/g++.dg/lto/pr88758_0.C b/gcc/testsuite/g++.dg/lto/pr88758_0.C
new file mode 100644
index 000..eccbf63c358
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr88758_0.C
@@ -0,0 +1,7 @@
+// { dg-lto-do link }
+// { dg-require-effective-target fpic }
+// { dg-require-effective-target shared }
+// { dg-lto-options { { -O3 -fPIC -flto -shared } } }
+
+void PreEvaluate(void);
+int main() { PreEvaluate(); return 0; }
diff --git a/gcc/testsuite/g++.dg/lto/pr88758_1.C b/gcc/testsuite/g++.dg/lto/pr88758_1.C
new file mode 100644
index 000..64ff57aeeb2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr88758_1.C
@@ -0,0 +1,9 @@
+extern int a[];
+int b;
+int c;
+
+void PreEvaluate(void) {
+  b = 0;
+  for (; b < 8; b++)
+a[b] = c * (b > 0 ? b - 1 : 0);
+}
-- 
2.20.1

Re: [PATCH] Improve RTL DSE with -fstack-protector* (PR rtl-optimization/88796)

[Jakub Jelinek]

On Fri, Jan 11, 2019 at 01:53:21PM +0100, Richard Biener wrote:
> >The canary slot in the stack frame is written in the prologue using
> >MEM_VOLATILE_P store, so we never consider those to be DSEd and is only
> >read
> >in the epilogue, so it shouldn't alias any other stores.
> >Similarly, __stack_chk_guard variable or say the TLS ssp slot or
> >whatever
> >else is used to hold the random pointer-sized value really shouldn't be
> >changed in -fstack-protector* instrumented functions, as that would
> >mean
> >they remembered one value in the prologue and would fail comparison in
> >the
> >epilogue if it changed in between.  So, I believe we can safely ignore
> >the
> >whole stack_pointer_test instruction in RTL DSE.
> >
> >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Isn't it enough to have the decl marked DECL_NONALIASED?  Alias analysis
> should not consider any address aliasing this (well, any with a mem_expr I
> guess).

No.  RTL DSE gives up completely in all MEM_VOLATILE_P reads.
  if ((MEM_ALIAS_SET (mem) == ALIAS_SET_MEMORY_BARRIER)
  || (MEM_VOLATILE_P (mem)))
{
  if (dump_file && (dump_flags & TDF_DETAILS))
fprintf (dump_file, " adding wild read, volatile or barrier.\n");
  add_wild_read (bb_info);
  insn_info->cannot_delete = true;
  return;
}
so it doesn't make into the alias oracle in any way, no idea why this has
been added in that form, seems to be a big hammer to me, but it is like that
(we obviously shouldn't try to replace_read those, but otherwise, I'd say
that whether a volatile or non-volatile read kills some store or not doesn't
really depend on whether it is volatile or not, but on the address;
I guess stage4 isn't the right time to change that though, it is this way
since r123530 when dse.c has been added).

Furthermore, the MEM_EXPR isn't always a DECL on which DECL_NONALIASED could be
applied, e.g. on x86_64-linux it is a MEM_REF built for the TLS memory slot.
Those were killing all the stores too.

Jakub

Re: [PATCH] Improve RTL DSE with -fstack-protector* (PR rtl-optimization/88796)

[Richard Biener]

On January 10, 2019 11:38:55 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>As mentioned in the PR, RTL DSE doesn't do much with
>-fstack-protector*,
>because the stack canary test in the epilogue of instrumented functions
>is a MEM_VOLATILE_P read out of the crtl->stack_protect_guard ssp
>canary
>slot in the stack frame and either a MEM_VOLATILE_P read of
>__stack_chk_guard variable, or corresponding some other location (e.g.
>TLS
>memory on x86).
>
>The canary slot in the stack frame is written in the prologue using
>MEM_VOLATILE_P store, so we never consider those to be DSEd and is only
>read
>in the epilogue, so it shouldn't alias any other stores.
>Similarly, __stack_chk_guard variable or say the TLS ssp slot or
>whatever
>else is used to hold the random pointer-sized value really shouldn't be
>changed in -fstack-protector* instrumented functions, as that would
>mean
>they remembered one value in the prologue and would fail comparison in
>the
>epilogue if it changed in between.  So, I believe we can safely ignore
>the
>whole stack_pointer_test instruction in RTL DSE.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Isn't it enough to have the decl marked DECL_NONALIASED? Alias analysis should 
not consider any address aliasing this (well, any with a mem_expr I guess). 

Richard. 

>2019-01-10  Jakub Jelinek  
>
>   PR rtl-optimization/88796
>   * emit-rtl.h (struct rtl_data): Add stack_protect_guard_decl field.
>   * cfgexpand.c (stack_protect_prologue): Initialize
>   crtl->stack_protect_guard_decl.
>   * function.c (stack_protect_epilogue): Use it instead of calling
>   targetm.stack_protect_guard again.
>   * dse.c (check_mem_read_rtx): Ignore MEM_VOLATILE_P reads from
>   MEMs with MEM_EXPR equal to crtl->stack_protect_guard or
>   crtl->stack_protect_guard_decl.
>   * config/i386/i386.c (ix86_stack_protect_guard): Set
>TREE_THIS_VOLATILE
>   on the returned MEM_EXPR.
>
>   * gcc.target/i386/pr88796.c: New test.
>
>--- gcc/emit-rtl.h.jj  2019-01-10 11:43:14.390377646 +0100
>+++ gcc/emit-rtl.h 2019-01-10 21:38:38.682055891 +0100
>@@ -87,6 +87,10 @@ struct GTY(()) rtl_data {
>  Used for detecting stack clobbers.  */
>   tree stack_protect_guard;
> 
>+  /* The __stack_chk_guard variable or expression holding the stack
>+ protector canary value.  */
>+  tree stack_protect_guard_decl;
>+
>/* List (chain of INSN_LIST) of labels heading the current handlers for
>  nonlocal gotos.  */
>   rtx_insn_list *x_nonlocal_goto_handler_labels;
>--- gcc/cfgexpand.c.jj 2019-01-07 09:50:26.774650762 +0100
>+++ gcc/cfgexpand.c2019-01-10 21:40:08.714589919 +0100
>@@ -6219,6 +6219,7 @@ stack_protect_prologue (void)
>   tree guard_decl = targetm.stack_protect_guard ();
>   rtx x, y;
> 
>+  crtl->stack_protect_guard_decl = guard_decl;
>   x = expand_normal (crtl->stack_protect_guard);
> 
>   if (targetm.have_stack_protect_combined_set () && guard_decl)
>--- gcc/function.c.jj  2019-01-10 16:43:54.802481070 +0100
>+++ gcc/function.c 2019-01-10 21:40:49.326928642 +0100
>@@ -4902,7 +4902,7 @@ init_function_start (tree subr)
> void
> stack_protect_epilogue (void)
> {
>-  tree guard_decl = targetm.stack_protect_guard ();
>+  tree guard_decl = crtl->stack_protect_guard_decl;
>   rtx_code_label *label = gen_label_rtx ();
>   rtx x, y;
>   rtx_insn *seq = NULL;
>--- gcc/dse.c.jj   2019-01-10 11:43:12.345411240 +0100
>+++ gcc/dse.c  2019-01-10 21:48:07.224799798 +0100
>@@ -2072,8 +2072,29 @@ check_mem_read_rtx (rtx *loc, bb_info_t
>   insn_info = bb_info->last_insn;
> 
>   if ((MEM_ALIAS_SET (mem) == ALIAS_SET_MEMORY_BARRIER)
>-  || (MEM_VOLATILE_P (mem)))
>+  || MEM_VOLATILE_P (mem))
> {
>+  if (crtl->stack_protect_guard
>+&& (MEM_EXPR (mem) == crtl->stack_protect_guard
>+|| (crtl->stack_protect_guard_decl
>+&& MEM_EXPR (mem) == crtl->stack_protect_guard_decl))
>+&& MEM_VOLATILE_P (mem))
>+  {
>+/* This is either the stack protector canary on the stack,
>+   which ought to be written by a MEM_VOLATILE_P store and
>+   thus shouldn't be deleted and is read at the very end of
>+   function, but shouldn't conflict with any other store.
>+   Or it is __stack_chk_guard variable or TLS or whatever else
>+   MEM holding the canary value, which really shouldn't be
>+   ever modified in -fstack-protector* protected functions,
>+   otherwise the prologue store wouldn't match the epilogue
>+   check.  */
>+if (dump_file && (dump_flags & TDF_DETAILS))
>+  fprintf (dump_file, " stack protector canary read ignored.\n");
>+insn_info->cannot_delete = true;
>+return;
>+  }
>+
>   if (dump_file && (dump_flags & TDF_DETAILS))
>   fprintf (dump_file, " adding wild read, volatile or barrier.\n");
>   add_wild_read (bb_info);
>--- gcc/config/i386/i386.c.jj  2019-01-10 11:43:1

Re: [PATCH] Fix misplaced combine totals dumping (PR bootstrap/88714)

[Richard Biener]

On January 10, 2019 11:25:59 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>r191883 seems to have introduced a pasto:
>--- trunk/gcc/passes.c 2012/10/01 00:17:52 191882
>+++ trunk/gcc/passes.c 2012/10/01 05:43:06 191883
>@@ -231,27 +231,23 @@
>   timevar_push (TV_DUMP);
>if (profile_arc_flag || flag_test_coverage ||
>flag_branch_probabilities)
> {
>-  dump_file = dump_begin (pass_profile.pass.static_pass_number,
>NULL);
>+  dump_start (pass_profile.pass.static_pass_number, NULL);
>   end_branch_prob ();
>-  if (dump_file)
>-  dump_end (pass_profile.pass.static_pass_number, dump_file);
>+  dump_finish (pass_profile.pass.static_pass_number);
> }
> 
>   if (optimize > 0)
> {
>-  dump_file = dump_begin (pass_combine.pass.static_pass_number,
>NULL);
>-  if (dump_file)
>-  {
>-dump_combine_total_stats (dump_file);
>-  dump_end (pass_combine.pass.static_pass_number, dump_file);
>-  }
>+  dump_start (pass_profile.pass.static_pass_number, NULL);
>+  print_combine_total_stats ();
>+  dump_finish (pass_combine.pass.static_pass_number);
> }
>
>where dump_finish was used with correct pass_combine, but dump_start
>was
>pastoed from the previous if and contained pass_profile instead.
>Next r193821 noticed this, but instead of fixing the dump_start
>argument
>changed dump_finish argument to match.
>
>So, in the end, the combiner statistics was emitted in profile_estimate
>dump
>and on the PR88714 issue suggested there is a difference already in the
>profile_estimate dump, when actually the IL changed only during pre and
>of
>course everything after it, including the combiner.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>2019-01-10  Jakub Jelinek  
>
>   PR bootstrap/88714
>   * passes.c (finish_optimization_passes): Call
>print_combine_total_stats
>   inside of pass_combine_1 dump rather than pass_profile_1.
>
>--- gcc/passes.c.jj2019-01-01 12:37:15.494002253 +0100
>+++ gcc/passes.c   2019-01-10 16:30:43.295424173 +0100
>@@ -361,9 +361,9 @@ finish_optimization_passes (void)
> 
>   if (optimize > 0)
> {
>-  dumps->dump_start (pass_profile_1->static_pass_number, NULL);
>+  dumps->dump_start (pass_combine_1->static_pass_number, NULL);
>   print_combine_total_stats ();
>-  dumps->dump_finish (pass_profile_1->static_pass_number);
>+  dumps->dump_finish (pass_combine_1->static_pass_number);
> }
> 
>   /* Do whatever is necessary to finish printing the graphs.  */
>
>   Jakub

Re: [PATCH] [RFC] PR target/52813 and target/11807

[Segher Boessenkool]

On Thu, Jan 10, 2019 at 09:56:28PM +, Richard Sandiford wrote:
> Jakub Jelinek  writes:
> > On Thu, Jan 10, 2019 at 09:23:27PM +, Richard Sandiford wrote:
> >> > "noreturn"...  What would that mean, *exactly*?  It cannot execute any
> >> > code the compiler can see, so such asm is better off as real asm anyway
> >> > (not inline asm).
> >> 
> >> "Exactly" is a strong word, and this wasn't my proposal, but...
> >> I think it would act like a noreturn call to an unknown function.
> >> Output operands wouldn't make sense, and arguably clobbers wouldn't
> >> either.
> >
> > "noreturn" asm can be done already now, just use
> > asm volatile ("..." ...);
> > __builtin_unreachable ();
> >
> > I think there is no need to add a new syntax for that.

Ah yes, the volatile makes this work.  Cool.  And as Richard says such
asm shouldn't have outputs anyway, so it will always be volatile.

> ISTR the point was that the PowerPC ABI places requirements on functions
> with noreturn calls

I don't think any of our ABIs do that?  Is this about the back chain?

> and the attribute would help GCC do the right thing
> in those circumstances.  So "noreturn" would imply a call that doesn't
> return, rather than just an infinite loop.


Segher

Re: [PATCH] [RFC] PR target/52813 and target/11807

[Richard Sandiford]

Segher Boessenkool  writes:
>> I think it would act like a noreturn call to an unknown function.
>
> Except it won't behave like a call otherwise (on Power all calls force a
> stack frame, for example; and on all targets noreturn calls do the same
> currently I think?)

I agree no current asm would behave like a call (e.g. causing leaf_p
to be false, and more).  The point of adding the attribute would be to
change that.

Richard

Re: [PATCH] [RFC] PR target/52813 and target/11807

[Segher Boessenkool]

On Thu, Jan 10, 2019 at 09:23:27PM +, Richard Sandiford wrote:
> Segher Boessenkool  writes:
> > On Tue, Jan 08, 2019 at 12:03:06PM +, Richard Sandiford wrote:
> >> Bernd Edlinger  writes:
> >> > Meanwhile I found out, that the stack clobber has only been ignored up to
> >> > gcc-5 (at least with lra targets, not really sure about reload targets).
> >> > From gcc-6 on, with the exception of PR arm/77904 which was a regression 
> >> > due
> >> > to the underlying lra change, but fixed later, and back-ported to 
> >> > gcc-6.3.0,
> >> > this works for all targets I tried so far.
> >> >
> >> > To me, it starts to look like a rather unique and useful feature, that I 
> >> > would
> >> > like to keep working.
> >> 
> >> Not sure what you mean by "unique".  But forcing a frame is a bit of
> >> a slippery concept.  Force it where?  For the asm only, or the whole
> >> function?  This depends on optimisation and hasn't been consistent
> >> across GCC versions, since it depends on the shrink-wrapping
> >> optimisation.  (There was a similar controversy a while ago about
> >> to what extent -fno-omit-frame-pointer should "force a frame".)
> >
> > It's not forcing a frame currently: it's just setting frame_pointer_needed.
> > Whatever happens from that is the target's business.
> 
> Do you mean the asm clobber or -fno-omit-frame-pointer?  If the option,
> then yeah, and that was exactly what was controversial :-)

I meant the asm clobber.  LRA sets frame_pointer_needed to 1 because the
stack pointer is clobbered, on many targets anyway:

  /* If we modify the source of an elimination rule, disable
 it.  Do the same if it is the destination and not the
 hard frame register.  */
  for (ep = reg_eliminate;
   ep < ®_eliminate[NUM_ELIMINABLE_REGS];
   ep++)
if (ep->from_rtx == XEXP (x, 0)
|| (ep->to_rtx == XEXP (x, 0)
&& ep->to_rtx != hard_frame_pointer_rtx))
  setup_can_eliminate (ep, false);

and setup_can_eliminate has

  if (! value
  && ep->from == FRAME_POINTER_REGNUM && ep->to == STACK_POINTER_REGNUM)
frame_pointer_needed = 1;

> >> The effect on the redzone seems like something that should be specified
> >> explicitly rather than as an (accidental?) side effect of listing the
> >> sp in the clobber list.  Maybe this would be another use for the "asm
> >> attributes" proposal.  "noreturn" was another attribute suggested on
> >> IRC yesterday.
> >
> > Redzone is target-dependent.
> 
> Right.  Target-dependent asm attributes wouldn't be a problem though.

It's harder to document, which means it is harder to get right (and get
people to _use_ it correctly), as well.

> Most other things about an asm are target-dependent anyway.

Very true.

> > "noreturn"...  What would that mean, *exactly*?  It cannot execute any
> > code the compiler can see, so such asm is better off as real asm anyway
> > (not inline asm).
> 
> "Exactly" is a strong word, and this wasn't my proposal, but...

"Exactly", as in, "please do enough hand-waving to cover all available
space" ;-)

There are many details that aren't quite obvious, but they do matter for
how usable and useful this extension would be.

> I think it would act like a noreturn call to an unknown function.

Except it won't behave like a call otherwise (on Power all calls force a
stack frame, for example; and on all targets noreturn calls do the same
currently I think?)

> Output operands wouldn't make sense, and arguably clobbers wouldn't
> either.

Yeah.


Segher

Re: [PATCH] Fix misplaced combine totals dumping (PR bootstrap/88714)

[Segher Boessenkool]

On Thu, Jan 10, 2019 at 11:25:59PM +0100, Jakub Jelinek wrote:
> So, in the end, the combiner statistics was emitted in profile_estimate dump
> and on the PR88714 issue suggested there is a difference already in the
> profile_estimate dump, when actually the IL changed only during pre and of
> course everything after it, including the combiner.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Seems obvious to me?  Okay for trunk as far as combine is concerned :-)


Segher


> 2019-01-10  Jakub Jelinek  
> 
>   PR bootstrap/88714
>   * passes.c (finish_optimization_passes): Call print_combine_total_stats
>   inside of pass_combine_1 dump rather than pass_profile_1.
> 
> --- gcc/passes.c.jj   2019-01-01 12:37:15.494002253 +0100
> +++ gcc/passes.c  2019-01-10 16:30:43.295424173 +0100
> @@ -361,9 +361,9 @@ finish_optimization_passes (void)
>  
>if (optimize > 0)
>  {
> -  dumps->dump_start (pass_profile_1->static_pass_number, NULL);
> +  dumps->dump_start (pass_combine_1->static_pass_number, NULL);
>print_combine_total_stats ();
> -  dumps->dump_finish (pass_profile_1->static_pass_number);
> +  dumps->dump_finish (pass_combine_1->static_pass_number);
>  }
>  
>/* Do whatever is necessary to finish printing the graphs.  */
> 

[committed][nvptx] Don't allow vector_length 64 with num_workers 16

[Tom de Vries]

Hi,

When using a compiler build with:
...
+#define PTX_DEFAULT_VECTOR_LENGTH PTX_CTA_SIZE
...

consider a test-case:
...
int
main (void)
{
  #pragma acc parallel vector_length (64)
#pragma acc loop worker
for (unsigned int i = 0; i < 32; i++)
  #pragma acc loop vector
  for (unsigned int j = 0; j < 64; j++)
;

  return 0;
}
...

If num_workers is 16, either because:
- we add a "num_workers (16)" clause on the parallel directive, or
- we set "GOMP_OPENACC_DIM=:16:", or
- the libgomp plugin chooses 16 num_workers
we run into an illegal instruction at runtime, because a bar.sync instruction
tries to use a barrier 16.  The instruction is illegal, because ptx supports
only 16 barriers per CTA, and the valid range is 0..15.

The problem is that with a warp-multiple vector length, we use a code generation
scheme with a per-worker barrier.  And because barrier zero is reserved for
per-cta barrier, only the remaining 15 barriers can be used as per-worker
barrier, and consequently we can't use num_workers larger than 15.

This problem occurs only for vector_length 64.  For vector_length 32, we use a
different code generation scheme, and for vector_length >= 96, the maximum
num_workers is not big enough not to trigger this problem.

Also, this problem only occurs for num_workers 16.  As explained above,
num_workers 15 is safe to use, and 16 is already the maximum num_workers for
vector_length 64.

This patch fixes the problem in both the compiler (handling "num_workers (16)")
and in the libgomp nvptx plugin (with and without "GOMP_OPENACC_DIM=:16:").

Committed to trunk.

Thanks,
- Tom

[nvptx] Don't allow vector_length 64 with num_workers 16

2019-01-10  Tom de Vries  

* config/nvptx/nvptx.c (PTX_CTA_NUM_BARRIERS, PTX_PER_CTA_BARRIER)
(PTX_NUM_PER_CTA_BARRIER, PTX_FIRST_PER_WORKER_BARRIER)
(PTX_NUM_PER_WORKER_BARRIERS): Define.
(nvptx_apply_dim_limits): Prevent vector_length 64 and
num_workers 16.

* plugin/plugin-nvptx.c (nvptx_exec): Prevent vector_length 64 and
num_workers 16.

---
 gcc/config/nvptx/nvptx.c  | 13 +
 libgomp/plugin/plugin-nvptx.c | 22 ++
 2 files changed, 35 insertions(+)

diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 643f5e86ccc..b37010ff58e 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -87,8 +87,14 @@
2.x.  */
 #define PTX_CTA_SIZE 1024
 
+#define PTX_CTA_NUM_BARRIERS 16
 #define PTX_WARP_SIZE 32
 
+#define PTX_PER_CTA_BARRIER 0
+#define PTX_NUM_PER_CTA_BARRIERS 1
+#define PTX_FIRST_PER_WORKER_BARRIER (PTX_NUM_PER_CTA_BARRIERS)
+#define PTX_NUM_PER_WORKER_BARRIERS (PTX_CTA_NUM_BARRIERS - 
PTX_NUM_PER_CTA_BARRIERS)
+
 #define PTX_DEFAULT_VECTOR_LENGTH PTX_WARP_SIZE
 #define PTX_MAX_VECTOR_LENGTH PTX_WARP_SIZE
 #define PTX_WORKER_LENGTH 32
@@ -5496,6 +5502,13 @@ nvptx_apply_dim_limits (int dims[])
   if (dims[GOMP_DIM_WORKER] > 0 &&  dims[GOMP_DIM_VECTOR] > 0
   && dims[GOMP_DIM_WORKER] * dims[GOMP_DIM_VECTOR] > PTX_CTA_SIZE)
 dims[GOMP_DIM_VECTOR] = PTX_WARP_SIZE;
+
+  /* If we need a per-worker barrier ... .  */
+  if (dims[GOMP_DIM_WORKER] > 0 &&  dims[GOMP_DIM_VECTOR] > 0
+  && dims[GOMP_DIM_VECTOR] > PTX_WARP_SIZE)
+/* Don't use more barriers than available.  */
+dims[GOMP_DIM_WORKER] = MIN (dims[GOMP_DIM_WORKER],
+PTX_NUM_PER_WORKER_BARRIERS);
 }
 
 /* Return true if FNDECL contains calls to vector-partitionable routines.  */
diff --git a/libgomp/plugin/plugin-nvptx.c b/libgomp/plugin/plugin-nvptx.c
index 60553bdf3bd..c80da64c422 100644
--- a/libgomp/plugin/plugin-nvptx.c
+++ b/libgomp/plugin/plugin-nvptx.c
@@ -1273,6 +1273,10 @@ nvptx_exec (void (*fn), size_t mapnum, void **hostaddrs, 
void **devaddrs,
  : dims[GOMP_DIM_VECTOR]);
workers = blocks / actual_vectors;
workers = MAX (workers, 1);
+   /* If we need a per-worker barrier ... .  */
+   if (actual_vectors > 32)
+ /* Don't use more barriers than available.  */
+ workers = MIN (workers, 15);
  }
 
for (i = 0; i != GOMP_DIM_MAX; i++)
@@ -1303,6 +1307,24 @@ nvptx_exec (void (*fn), size_t mapnum, void **hostaddrs, 
void **devaddrs,
 suggest_workers, suggest_workers);
 }
 
+  /* Check if the accelerator has sufficient barrier resources to
+ launch the offloaded kernel.  */
+  if (dims[GOMP_DIM_WORKER] > 15 && dims[GOMP_DIM_VECTOR] > 32)
+{
+  const char *msg
+   = ("The Nvidia accelerator has insufficient barrier resources to launch"
+  " '%s' with num_workers = %d and vector_length = %d"
+  "; "
+  "recompile the program with 'num_workers = x' on that offloaded"
+  " region or '-fopenacc-dim=:x:' where x <= 15"
+  "; "
+  "or, recompile the program with 'vector_length = 32' on that"

[committed][nvptx] Move PTX_CTA_SIZE up

[Tom de Vries]

Hi,

Move the defition of PTX_CTA_SIZE up in nvptx.c.

Committed to trunk.

Thanks,
- Tom

[nvptx] Move PTX_CTA_SIZE up

2019-01-11  Tom de Vries  

* config/nvptx/nvptx.c (PTX_CTA_SIZE): Move up.

---
 gcc/config/nvptx/nvptx.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 7fdc285b6f8..643f5e86ccc 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -82,17 +82,18 @@
 #define WORKAROUND_PTXJIT_BUG_2 1
 #define WORKAROUND_PTXJIT_BUG_3 1
 
+/* The PTX concept CTA (Concurrent Thread Array) maps on the CUDA concept 
thread
+   block, which has had a maximum number of threads of 1024 since CUDA version
+   2.x.  */
+#define PTX_CTA_SIZE 1024
+
 #define PTX_WARP_SIZE 32
+
 #define PTX_DEFAULT_VECTOR_LENGTH PTX_WARP_SIZE
 #define PTX_MAX_VECTOR_LENGTH PTX_WARP_SIZE
 #define PTX_WORKER_LENGTH 32
 #define PTX_DEFAULT_RUNTIME_DIM 0 /* Defer to runtime.  */
 
-/* The PTX concept CTA (Concurrent Thread Array) maps on the CUDA concept 
thread
-   block, which has had a maximum number of threads of 1024 since CUDA version
-   2.x.  */
-#define PTX_CTA_SIZE 1024
-
 /* The various PTX memory areas an object might reside in.  */
 enum nvptx_data_area
 {

[committed][libgomp, testsuite, openacc] Remove -foffload=-w in reduction-[1-5].c

[Tom de Vries]

Hi,

Before the commit "[libgomp, testsuite, openacc] Don't use const int for
dimensions", the "const int" construct was used to set launch dimensions in
reductions-[1-5].c.  In the case of -xc -O0, the const int is implemented as a
variable by the C front-end.  Consequently, the nvptx back-end generated
warnings that vector_length was overridden to be hard-coded, rather than left to
be set at runtime.  The test-cases silenced these warnings by switching off all
warnings in the accelerator compiler using "-foffload=-w".

Given that no warnings occur anymore, remove the "-foffload=-w" setting.

Committed to trunk.

Thanks,
- Tom

[libgomp, testsuite, openacc] Remove -foffload=-w in reduction-[1-5].c

2019-01-11  Tom de Vries  

* testsuite/libgomp.oacc-c-c++-common/reduction-1.c: Remove
-foffload=-w.
* testsuite/libgomp.oacc-c-c++-common/reduction-2.c: Same.
* testsuite/libgomp.oacc-c-c++-common/reduction-3.c: Same.
* testsuite/libgomp.oacc-c-c++-common/reduction-4.c: Same.
* testsuite/libgomp.oacc-c-c++-common/reduction-5.c: Same.

---
 libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c | 3 ---
 libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-2.c | 3 ---
 libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-3.c | 3 ---
 libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-4.c | 3 ---
 libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-5.c | 3 ---
 5 files changed, 15 deletions(-)

diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c
index 3fe9ae65309..4e76ccd5b5c 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-1.c
@@ -1,8 +1,5 @@
 /* { dg-do run } */
 
-/* Ignore vector_length warnings for offloaded (nvptx) targets.  */
-/* { dg-additional-options "-foffload=-w" } */
-
 /* Integer reductions.  */
 
 #include 
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-2.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-2.c
index 83986abf7e0..96e6c079ef3 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-2.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-2.c
@@ -1,8 +1,5 @@
 /* { dg-do run } */
 
-/* Ignore vector_length warnings for offloaded (nvptx) targets.  */
-/* { dg-additional-options "-foffload=-w" } */
-
 /* float reductions.  */
 
 #include 
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-3.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-3.c
index 3ac0f996a86..dc18426d748 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-3.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-3.c
@@ -1,8 +1,5 @@
 /* { dg-do run } */
 
-/* Ignore vector_length warnings for offloaded (nvptx) targets.  */
-/* { dg-additional-options "-foffload=-w" } */
-
 /* double reductions.  */
 
 #include 
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-4.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-4.c
index b8fa954cfee..d152674be45 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-4.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-4.c
@@ -1,8 +1,5 @@
 /* { dg-do run { target { ! { hppa*-*-hpux* } } } } */
 
-/* Ignore vector_length warnings for offloaded (nvptx) targets.  */
-/* { dg-additional-options "-foffload=-w" } */
-
 /* complex reductions.  */
 
 #include 
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-5.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-5.c
index 215d919a103..6f5d29316a0 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-5.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/reduction-5.c
@@ -1,9 +1,6 @@
 /* { dg-do run } */
 /* { dg-additional-options "-w" } */
 
-/* Ignore vector_length warnings for offloaded (nvptx) targets.  */
-/* { dg-additional-options "-foffload=-w" } */
-
 /* Multiple reductions.  */
 
 #include 

[committed][nvptx, testsuite, openacc, libgomp] Add insufficient-resources.c

[Tom de Vries]

Hi,

Add a test-case that tests the "insufficient resources" fatal in the nvptx
libgomp plugin.

Committed to trunk.

Thanks,
- Tom

[nvptx, testsuite, openacc, libgomp] Add insufficient-resources.c

2019-01-11  Tom de Vries  

* testsuite/libgomp.oacc-c-c++-common/insufficient-resources.c: New
test.

---
 .../insufficient-resources.c| 21 +
 1 file changed, 21 insertions(+)

diff --git 
a/libgomp/testsuite/libgomp.oacc-c-c++-common/insufficient-resources.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/insufficient-resources.c
new file mode 100644
index 000..b59cd420785
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/insufficient-resources.c
@@ -0,0 +1,21 @@
+/* { dg-set-target-env-var "GOMP_OPENACC_DIM" ":33:" } */
+/* { dg-shouldfail "" { openacc_nvidia_accel_selected } } */
+
+#include 
+#include 
+
+int
+main (void)
+{
+#pragma acc parallel vector_length (32)
+  {
+#pragma acc loop worker
+for (unsigned int i = 0; i < 32; i++)
+#pragma acc loop vector
+  for (unsigned int j = 0; j < 64; j++)
+   ;
+  }
+
+  return 0;
+}
+/* { dg-output "The Nvidia accelerator has insufficient resources" { target 
openacc_nvidia_accel_selected } } */

[PATCH] PR libstdc++/88125 remove duplicate entry in linker script

[Jonathan Wakely]

PR libstdc++/88125
* config/abi/pre/gnu.ver (GLIBCXX_3.4.6): Remove unused duplicate
pattern for std::basic_stringbuf::str().

Tested x86_64-linux, committed to trunk.


commit 561a3110e71ba2dbe91681b6abe10cbf3dfb4288
Author: Jonathan Wakely 
Date:   Fri Jan 11 11:16:21 2019 +

PR libstdc++/88125 remove duplicate entry in linker script

PR libstdc++/88125
* config/abi/pre/gnu.ver (GLIBCXX_3.4.6): Remove unused duplicate
pattern for std::basic_stringbuf::str().

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver 
b/libstdc++-v3/config/abi/pre/gnu.ver
index d3431d2c78e..dc934dd8509 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1165,10 +1165,6 @@ GLIBCXX_3.4.6 {
 
 _ZNSt15basic_stringbufI[cw]St11char_traitsI[cw]ESaI[cw]EE9showmanycEv;
 
-#ifdef HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT
-_ZNKSt15basic_stringbufIwSt11char_traitsIwESaIwEE3strEv;
-#endif
-
 _ZN9__gnu_cxx6__poolILb1EE13_M_initializeEv;
 
 } GLIBCXX_3.4.5;

Re: [Patch 4/4][Aarch64] v2: Implement Aarch64 SIMD ABI

[Jakub Jelinek]

On Fri, Jan 11, 2019 at 11:22:59AM +, Richard Sandiford wrote:
> Steve Ellcey  writes:
> > If this looks good to you can I go ahead and check it in?  I know
> > we are in Stage 3 now, but my recollection is that patches that were
> > initially submitted during Stage 1 could go ahead once approved.
> 
> Yeah, like you say, this was originally posted in stage 1 and is the
> last patch in the series.  Not committing it would leave the work
> incomplete in GCC 9.  The problem is that we're now in stage 4 rather
> than stage 3.
> 
> I don't think I'm neutral enough to make the call.  Richard, Jakub?

I'm ok with accepting this late as an exception, if it can be committed
reasonably soon (within a week or at most two).

Jakub

Re: [Patch 4/4][Aarch64] v2: Implement Aarch64 SIMD ABI

[Richard Sandiford]

Steve Ellcey  writes:
> OK, I fixed the issues in your last email.  I initially found one
> regression while testing.  In lra_create_live_ranges_1 I had removed
> the 'call_p = false' statement but did not replaced it with anything.
> This resulted in no regressions on aarch64 but caused a single
> regression on x86 (gcc.target/i386/pr87759.c).  I replaced the
> line with 'call_insn = NULL' and the regression went away so I
> have clean bootstraps and no regressions on aarch64 and x86 now.

Looks good to me bar the parameter description below.

> If this looks good to you can I go ahead and check it in?  I know
> we are in Stage 3 now, but my recollection is that patches that were
> initially submitted during Stage 1 could go ahead once approved.

Yeah, like you say, this was originally posted in stage 1 and is the
last patch in the series.  Not committing it would leave the work
incomplete in GCC 9.  The problem is that we're now in stage 4 rather
than stage 3.

I don't think I'm neutral enough to make the call.  Richard, Jakub?

> diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c
> index a00ec38..b77b675 100644
> --- a/gcc/lra-lives.c
> +++ b/gcc/lra-lives.c
> @@ -576,25 +576,39 @@ lra_setup_reload_pseudo_preferenced_hard_reg (int regno,
>  
>  /* Check that REGNO living through calls and setjumps, set up conflict
> regs using LAST_CALL_USED_REG_SET, and clear corresponding bits in
> -   PSEUDOS_LIVE_THROUGH_CALLS and PSEUDOS_LIVE_THROUGH_SETJUMPS.  */
> +   PSEUDOS_LIVE_THROUGH_CALLS and PSEUDOS_LIVE_THROUGH_SETJUMPS.
> +   CALL_INSN may be the specific call we want to check that REGNO lives
> +   through or a call that is guaranteed to clobber REGNO if any call
> +   in the current block clobbers REGNO.  */

I think it would be more accurate to say:

   CALL_INSN is a call that is representative of all calls in the region
   described by the PSEUDOS_LIVE_THROUGH_* sets, in terms of the registers
   that it preserves and clobbers.  */

> +
>  static inline void
>  check_pseudos_live_through_calls (int regno,
> -   HARD_REG_SET last_call_used_reg_set)
> +   HARD_REG_SET last_call_used_reg_set,
> +   rtx_insn *call_insn)
>  {
>int hr;
> +  rtx_insn *old_call_insn;
>  
>if (! sparseset_bit_p (pseudos_live_through_calls, regno))
>  return;
> +
> +  gcc_assert (call_insn && CALL_P (call_insn));
> +  old_call_insn = lra_reg_info[regno].call_insn;
> +  if (!old_call_insn
> +  || (targetm.return_call_with_max_clobbers
> +   && targetm.return_call_with_max_clobbers (old_call_insn, call_insn)
> +  == call_insn))
> +lra_reg_info[regno].call_insn = call_insn;
> +
>sparseset_clear_bit (pseudos_live_through_calls, regno);
>IOR_HARD_REG_SET (lra_reg_info[regno].conflict_hard_regs,
>   last_call_used_reg_set);
>  
>for (hr = 0; HARD_REGISTER_NUM_P (hr); hr++)
> -if (targetm.hard_regno_call_part_clobbered (hr,
> +if (targetm.hard_regno_call_part_clobbered (call_insn, hr,
>   PSEUDO_REGNO_MODE (regno)))
>add_to_hard_reg_set (&lra_reg_info[regno].conflict_hard_regs,
>  PSEUDO_REGNO_MODE (regno), hr);
> -  lra_reg_info[regno].call_p = true;
>if (! sparseset_bit_p (pseudos_live_through_setjumps, regno))
>  return;
>sparseset_clear_bit (pseudos_live_through_setjumps, regno);

BTW, I think we could save some compile time by moving the "for" loop
into the new "if", since otherwise call_insn should have no new
information.  But that was true before as well (since we could have
skipped the loop if lra_reg_info[regno].call_p was already true),
so it's really a separate issue.

Thanks,
Richard

Re: [PATCH] PR fortran/35031 -- Check F2018:C1246

[Paul Richard Thomas]

Hi Steve,

Yes, that's good for trunk.

Thanks

Paul

On Fri, 11 Jan 2019 at 01:16, Steve Kargl
 wrote:
>
> An entry-name obtains the elemental attribute from its containing
> procedure.  F2018:C1546 prohibits an procedure from having a BIND(C)
> attribute.  BIND(C) can appear on the entry-stmt line, so gfortran
> needs to check for a conflict.  The attached patch does this check.
> Tested on x86_64-*-freebsd.  Ok to commit?
>
> 2019-01-10  Steven G. Kargl  
>
> PR fortran/35031
> * decl.c (gfc_match_entry): Check for F2018:C1546.  Fix nearby
> mis-indentation.
>
> 2019-01-10  Steven G. Kargl  
>
> PR fortran/35031
> * gfortran.dg/pr35031.f90: new test.
>
> --
> Steve



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

Re: [v3 PATCH] Implement LWG 2221, No formatted output operator for nullptr

[Jonathan Wakely]

On 11/01/19 10:07 +0100, Rainer Orth wrote:

Hi Jonathan,


this patch broke Solaris bootstrap:

ld: fatal: libstdc++-symbols.ver-sun: 7117: symbol 'std::basic_ostream >::operator<<(decltype(nullptr))': symbol version conflict
ld: fatal: libstdc++-symbols.ver-sun: 7119: symbol 'std::basic_ostream >::operator<<(decltype(nullptr))': symbol version 
conflict

ld: fatal: libstdc++-symbols.ver-sun: 7117: symbol '_ZNSolsEDn': symbol version 
conflict
ld: fatal: libstdc++-symbols.ver-sun: 7119: symbol 
'_ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn': symbol version conflict

Again, there were two matches for those two symbols:

 GLIBCXX_3.4
   ##_ZNSolsE*[^Dg] (glob)
   _ZNSolsEDn;
 GLIBCXX_3.4.26
   ##_ZNSolsEDn (glob)
   _ZNSolsEDn;

 GLIBCXX_3.4
   ##_ZNSt13basic_ostreamIwSt11char_traitsIwEElsE*[^Dg] (glob)
   _ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn;
 GLIBCXX_3.4.26
   ##_ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn (glob)
   _ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn;

ISTM that the patterns were backwards.  The following patch fixes this
and allowed i386-pc-solaris2.11 bootstrap to complete without
regressions relative to the last successful one.


I think what I should have done is change [^g] to [^gn]. That
preserves the original behaviour (don't match the ppc64 long double
symbols) but also excludes the new symbols, which end in 'n'.

Maybe the attached patch would be better though. It matches every
basic_ostream::operator<<(T) for any scalar T except 'g', and adds a
second pattern to match basic_ostream::operator<<(T*) for various T.
But neither of those matches the new operator<<(nullptr_t) overload.


it allowed me to link libstdc++.so, too.  For my patch I'd only been
going from the ld errors and the matching patterns in the generated
libstdc++.map-sun, not knowing the background here.


THanks for checking it. I've committed the patch now.


FWIW I did run my symbol checker script, but it gets lots of false
positives because it doesn't understand the #if preprocessor
conditions, so it sees lots of false positive duplicates. I need to
make it smarter for it to be useful here.


Indeed: the variation possible here can be a total PITA ;-)


I've managed to cobble together a pipeline with sed and cpp that
allows me to test the linker script properly. It found some conflicts
that still remain but presumably aren't present on Solaris because
HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT is not defined.

There's one remaining conflict, which https://gcc.gnu.org/PR88125
covers. 


I'll commit a patch for those shortly.

RE: [PATCH 9/9][GCC][Arm] Add ACLE intrinsics for complex mutliplication and addition

[Tamar Christina]

Hi Christoph,

The arm one is a testism, I have a validated patch that I will commit soon.
The aarch64 one is a big-endian lane ordering issue, I had completely forgotten 
to test big-endian,
Patch for that is going through validation now.

Will submit the aarch64 one soon, sorry for the mess, splitting of the patches 
from the remainder of the series
had some casualties.. These should be the last.

Thanks and happy new years to you too!

Kind Regards,
Tamar

-Original Message-
From: Christophe Lyon  
Sent: Friday, January 11, 2019 10:02 AM
To: Tamar Christina 
Cc: Kyrill Tkachov ; gcc-patches@gcc.gnu.org; nd 
; Ramana Radhakrishnan ; Richard 
Earnshaw ; ni...@redhat.com
Subject: Re: [PATCH 9/9][GCC][Arm] Add ACLE intrinsics for complex 
mutliplication and addition

Hi Tamar,


On Thu, 10 Jan 2019 at 16:41, Tamar Christina  wrote:
>
> Hi Christoph,
>
> It was introduced in a small refactoring after which I only retested the 
> testcases I added,which don't trigger the issue.
>
> In any case it's a trivial fix and I'll submit a patch in a bit.
>
> Tamar
>
> 
> From: Christophe Lyon 
> Sent: Thursday, January 10, 2019 3:35:18 PM
> To: Tamar Christina
> Cc: Kyrill Tkachov; gcc-patches@gcc.gnu.org; nd; Ramana Radhakrishnan; 
> Richard Earnshaw; ni...@redhat.com
> Subject: Re: [PATCH 9/9][GCC][Arm] Add ACLE intrinsics for complex 
> mutliplication and addition
>
> Hi Tamar,
>
>
> On Thu, 10 Jan 2019 at 04:44, Tamar Christina  wrote:
> >
> > Hi Kyrill,
> >
> > Committed with a the addition of a few trivial defines and iterators 
> > that were missing due to The patch being split.
> >
> > Thanks,
> > Tamar
> >
> > -Original Message-
> > From: Kyrill Tkachov 
> > Sent: Friday, December 21, 2018 11:40 AM
> > To: Tamar Christina ; 
> > gcc-patches@gcc.gnu.org
> > Cc: nd ; Ramana Radhakrishnan 
> > ; Richard Earnshaw 
> > ; ni...@redhat.com
> > Subject: Re: [PATCH 9/9][GCC][Arm] Add ACLE intrinsics for complex 
> > mutliplication and addition
> >
> > Hi Tamar,
> >
> > On 11/12/18 15:46, Tamar Christina wrote:
> > > Hi All,
> > >
> > > This patch adds NEON intrinsics and tests for the Armv8.3-a 
> > > complex multiplication and add instructions with a rotate along the 
> > > Argand plane.
> > >
> > > The instructions are documented in the ArmARM[1] and the 
> > > intrinsics specification will be published on the Arm website [2].
> > >
> > > The Lane versions of these instructions are special in that they always 
> > > select a pair.
> > > using index 0 means selecting lane 0 and 1.  Because of this the 
> > > range check for the intrinsics require special handling.
> > >
> > > On Arm, in order to implement some of the lane intrinsics we're 
> > > using the structure of the register file.  The lane variant of 
> > > these instructions always select a D register, but the data itself 
> > > can be stored in Q registers.  This means that for single 
> > > precision complex numbers you are only allowed to select D[0] but using 
> > > the register file layout you can get the range 0-1 for lane indices by 
> > > selecting between Dn[0] and Dn+1[0].
> > >
> > > Same reasoning applies for half float complex numbers, except 
> > > there your D register indexes can be 0 or 1, so you have a total range of 
> > > 4 elements (for a V8HF).
> > >
> > >
> > > [1]
> > > https://developer.arm.com/docs/ddi0487/latest/arm-architecture-ref
> > > eren ce-manual-armv8-for-armv8-a-architecture-profile
> > > [2] https://developer.arm.com/docs/101028/latest
> > >
> > > Bootstrapped Regtested on arm-none-gnueabihf and no issues.
> > >
> > > Ok for trunk?
> > >
> >
> > Ok.
> > Thanks,
> > Kyrill
> >
> > > Thanks,
> > > Tamar
> > >
> > > gcc/ChangeLog:
> > >
> > > 2018-12-11  Tamar Christina  
> > >
> > > * config/arm/arm-builtins.c
> > > (enum arm_type_qualifiers): Add qualifier_lane_pair_index.
> > > (MAC_LANE_PAIR_QUALIFIERS): New.
> > > (arm_expand_builtin_args): Use it.
> > > (arm_expand_builtin_1): Likewise.
> > > * config/arm/arm-protos.h (neon_vcmla_lane_prepare_operands): New.
> > > * config/arm/arm.c (neon_vcmla_lane_prepare_operands): New.
> > > * config/arm/arm-c.c (arm_cpu_builtins): Add 
> > > __ARM_FEATURE_COMPLEX.
> > > * config/arm/arm_neon.h:
> > > (vcadd_rot90_f16): New.
> > > (vcaddq_rot90_f16): New.
> > > (vcadd_rot270_f16): New.
> > > (vcaddq_rot270_f16): New.
> > > (vcmla_f16): New.
> > > (vcmlaq_f16): New.
> > > (vcmla_lane_f16): New.
> > > (vcmla_laneq_f16): New.
> > > (vcmlaq_lane_f16): New.
> > > (vcmlaq_laneq_f16): New.
> > > (vcmla_rot90_f16): New.
> > > (vcmlaq_rot90_f16): New.
> > > (vcmla_rot90_lane_f16): New.
> > > (vcmla_rot90_laneq_f16): New.
> > > (vcmlaq_rot90_lane_f16): New.
> > > (vcmlaq_rot90_laneq_f16): New.
> > > (vcmla_rot180_f16): New.
> > >

[committed] Add testcase for already fixed (or latent) PR rtl-optimization/88296

[Jakub Jelinek]

Hi!

As mentioned in the PR, this is a LRA hang introduced with the
r266422 IRA costs change and which went away with r266862 IRA costs change.

Tested on x86_64-linux and i686-linux plus tested with older cc1 from end of
November where it timeouts, committed to trunk as obvious.

2019-01-11  Jakub Jelinek  

PR rtl-optimization/88296
* gcc.target/i386/pr88296.c: New test.

--- gcc/testsuite/gcc.target/i386/pr88296.c.jj  2019-01-11 11:13:43.016941274 
+0100
+++ gcc/testsuite/gcc.target/i386/pr88296.c 2019-01-11 11:14:14.558421497 
+0100
@@ -0,0 +1,66 @@
+/* PR rtl-optimization/88296 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2 -march=core2 -fnon-call-exceptions -fschedule-insns 
-fselective-scheduling -funroll-all-loops -fno-caller-saves 
-fno-guess-branch-probability -fno-ivopts -fno-rerun-cse-after-loop 
-fno-split-wide-types -fno-tree-bit-ccp -fno-tree-coalesce-vars --param 
max-completely-peeled-insns=13" } */
+
+int a, b, c;
+void bar (int, int);
+
+void
+foo (__int128 x, unsigned int y, char z)
+{
+  __int128 *d = &x;
+  unsigned short int e = 0;
+  unsigned char f = 0;
+  char g = 1;
+  (void) d;
+  bar (0, 0);
+  while (e < 1)
+{
+  __int128 h = 1;
+  ++c;
+  e = x;
+  g /= e;
+  if (g != 0)
+{
+  int i;
+  if (g == 0)
+i = a;
+  else
+{
+  ++g;
+  i = g;
+}
+  if (i == 0)
+{
+  a -= b + (!!g ? y : 0);
+  break;
+}
+  for (g = 0; g < 3; ++g)
+{
+  unsigned char *j = &f;
+  x /= h;
+  h /= x;
+  for (i = 0; i < 2; ++i)
+{
+  z <<= ((!!g && !!c) ? (b + 1) : (1 / *j));
+  y = ~e;
+  g &= y;
+  x |= 1;
+  b *= i;
+  ++c;
+}
+  f /= b > 1;
+  e /= f;
+  x += b << 32;/* { dg-warning "left shift count" } */
+}
+  if (g << g == 0)
+{
+  e *= b + 1;
+  a = 32;
+  f += (e << a) + (y && 1);
+  x = z;
+  c += x / 3;
+}
+}
+}
+}

Jakub

Re: [PATCH 9/9][GCC][Arm] Add ACLE intrinsics for complex mutliplication and addition

[Christophe Lyon]

Hi Tamar,


On Thu, 10 Jan 2019 at 16:41, Tamar Christina  wrote:
>
> Hi Christoph,
>
> It was introduced in a small refactoring after which I only retested the 
> testcases I added,which don't trigger the issue.
>
> In any case it's a trivial fix and I'll submit a patch in a bit.
>
> Tamar
>
> 
> From: Christophe Lyon 
> Sent: Thursday, January 10, 2019 3:35:18 PM
> To: Tamar Christina
> Cc: Kyrill Tkachov; gcc-patches@gcc.gnu.org; nd; Ramana Radhakrishnan; 
> Richard Earnshaw; ni...@redhat.com
> Subject: Re: [PATCH 9/9][GCC][Arm] Add ACLE intrinsics for complex 
> mutliplication and addition
>
> Hi Tamar,
>
>
> On Thu, 10 Jan 2019 at 04:44, Tamar Christina  wrote:
> >
> > Hi Kyrill,
> >
> > Committed with a the addition of a few trivial defines and iterators that 
> > were missing due to
> > The patch being split.
> >
> > Thanks,
> > Tamar
> >
> > -Original Message-
> > From: Kyrill Tkachov 
> > Sent: Friday, December 21, 2018 11:40 AM
> > To: Tamar Christina ; gcc-patches@gcc.gnu.org
> > Cc: nd ; Ramana Radhakrishnan ; 
> > Richard Earnshaw ; ni...@redhat.com
> > Subject: Re: [PATCH 9/9][GCC][Arm] Add ACLE intrinsics for complex 
> > mutliplication and addition
> >
> > Hi Tamar,
> >
> > On 11/12/18 15:46, Tamar Christina wrote:
> > > Hi All,
> > >
> > > This patch adds NEON intrinsics and tests for the Armv8.3-a complex
> > > multiplication and add instructions with a rotate along the Argand plane.
> > >
> > > The instructions are documented in the ArmARM[1] and the intrinsics
> > > specification will be published on the Arm website [2].
> > >
> > > The Lane versions of these instructions are special in that they always 
> > > select a pair.
> > > using index 0 means selecting lane 0 and 1.  Because of this the range
> > > check for the intrinsics require special handling.
> > >
> > > On Arm, in order to implement some of the lane intrinsics we're using
> > > the structure of the register file.  The lane variant of these
> > > instructions always select a D register, but the data itself can be
> > > stored in Q registers.  This means that for single precision complex
> > > numbers you are only allowed to select D[0] but using the register file 
> > > layout you can get the range 0-1 for lane indices by selecting between 
> > > Dn[0] and Dn+1[0].
> > >
> > > Same reasoning applies for half float complex numbers, except there
> > > your D register indexes can be 0 or 1, so you have a total range of 4 
> > > elements (for a V8HF).
> > >
> > >
> > > [1]
> > > https://developer.arm.com/docs/ddi0487/latest/arm-architecture-referen
> > > ce-manual-armv8-for-armv8-a-architecture-profile
> > > [2] https://developer.arm.com/docs/101028/latest
> > >
> > > Bootstrapped Regtested on arm-none-gnueabihf and no issues.
> > >
> > > Ok for trunk?
> > >
> >
> > Ok.
> > Thanks,
> > Kyrill
> >
> > > Thanks,
> > > Tamar
> > >
> > > gcc/ChangeLog:
> > >
> > > 2018-12-11  Tamar Christina  
> > >
> > > * config/arm/arm-builtins.c
> > > (enum arm_type_qualifiers): Add qualifier_lane_pair_index.
> > > (MAC_LANE_PAIR_QUALIFIERS): New.
> > > (arm_expand_builtin_args): Use it.
> > > (arm_expand_builtin_1): Likewise.
> > > * config/arm/arm-protos.h (neon_vcmla_lane_prepare_operands): New.
> > > * config/arm/arm.c (neon_vcmla_lane_prepare_operands): New.
> > > * config/arm/arm-c.c (arm_cpu_builtins): Add 
> > > __ARM_FEATURE_COMPLEX.
> > > * config/arm/arm_neon.h:
> > > (vcadd_rot90_f16): New.
> > > (vcaddq_rot90_f16): New.
> > > (vcadd_rot270_f16): New.
> > > (vcaddq_rot270_f16): New.
> > > (vcmla_f16): New.
> > > (vcmlaq_f16): New.
> > > (vcmla_lane_f16): New.
> > > (vcmla_laneq_f16): New.
> > > (vcmlaq_lane_f16): New.
> > > (vcmlaq_laneq_f16): New.
> > > (vcmla_rot90_f16): New.
> > > (vcmlaq_rot90_f16): New.
> > > (vcmla_rot90_lane_f16): New.
> > > (vcmla_rot90_laneq_f16): New.
> > > (vcmlaq_rot90_lane_f16): New.
> > > (vcmlaq_rot90_laneq_f16): New.
> > > (vcmla_rot180_f16): New.
> > > (vcmlaq_rot180_f16): New.
> > > (vcmla_rot180_lane_f16): New.
> > > (vcmla_rot180_laneq_f16): New.
> > > (vcmlaq_rot180_lane_f16): New.
> > > (vcmlaq_rot180_laneq_f16): New.
> > > (vcmla_rot270_f16): New.
> > > (vcmlaq_rot270_f16): New.
> > > (vcmla_rot270_lane_f16): New.
> > > (vcmla_rot270_laneq_f16): New.
> > > (vcmlaq_rot270_lane_f16): New.
> > > (vcmlaq_rot270_laneq_f16): New.
> > > (vcadd_rot90_f32): New.
> > > (vcaddq_rot90_f32): New.
> > > (vcadd_rot270_f32): New.
> > > (vcaddq_rot270_f32): New.
> > > (vcmla_f32): New.
> > > (vcmlaq_f32): New.
> > > (vcmla_lane_f32): New.
> > > (vcmla_laneq_f32): New.
> > > (vcmlaq_lane_f32): New.
> >

Re: patch to fix PR87305

[Richard Sandiford]

Hi Vlad,

Vladimir Makarov  writes:
> The following patch fixes
>
>    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87305
>
> The patch was bootstrapped and tested on x86-64 and ppc64 (be).
>
> Committed as rev. 267823.
>
> Index: ChangeLog
> ===
> --- ChangeLog (revision 267822)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,12 @@
> +2019-01-10  Vladimir Makarov  
> +
> + PR rtl-optimization/87305
> + * lra-assigns.c
> + (setup_live_pseudos_and_spill_after_risky_transforms): Check
> + allocation for big endian pseudos used as paradoxical subregs and
> + spill them if it is wrong.
> + * lra-constraints.c (lra_constraints): Add a comment.
> +
>  2019-01-10  Richard Biener  
>  
>   PR tree-optimization/88792
> Index: lra-assigns.c
> ===
> --- lra-assigns.c (revision 267822)
> +++ lra-assigns.c (working copy)
> @@ -1146,12 +1146,12 @@ static void
>  setup_live_pseudos_and_spill_after_risky_transforms (bitmap
>spilled_pseudo_bitmap)
>  {
> -  int p, i, j, n, regno, hard_regno;
> +  int p, i, j, n, regno, hard_regno, biggest_nregs, nregs_diff;
>unsigned int k, conflict_regno;
>poly_int64 offset;
>int val;
>HARD_REG_SET conflict_set;
> -  machine_mode mode;
> +  machine_mode mode, biggest_mode;
>lra_live_range_t r;
>bitmap_iterator bi;
>int max_regno = max_reg_num ();
> @@ -1166,8 +1166,26 @@ setup_live_pseudos_and_spill_after_risky
>for (n = 0, i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
>  if ((pic_offset_table_rtx == NULL_RTX
>|| i != (int) REGNO (pic_offset_table_rtx))
> - && reg_renumber[i] >= 0 && lra_reg_info[i].nrefs > 0)
> -  sorted_pseudos[n++] = i;
> + && (hard_regno = reg_renumber[i]) >= 0 && lra_reg_info[i].nrefs > 0)
> +  {
> + biggest_mode = lra_reg_info[i].biggest_mode;
> + biggest_nregs = hard_regno_nregs (hard_regno, biggest_mode);
> + nregs_diff = (biggest_nregs
> +   - hard_regno_nregs (hard_regno, PSEUDO_REGNO_MODE (i)));
> + enum reg_class rclass = lra_get_allocno_class (i);
> +
> + if (WORDS_BIG_ENDIAN
> + && (hard_regno - nregs_diff < 0
> + || !TEST_HARD_REG_BIT (reg_class_contents[rclass],
> +hard_regno - nregs_diff)))
> +   {
> + /* Hard registers of paradoxical sub-registers are out of
> +range of pseudo register class.  Spill the pseudo.  */
> + reg_renumber[i] = -1;
> + continue;
> +   }

I think for !WORDS_BIG_ENDIAN the equivalent problem to:

|| !TEST_HARD_REG_BIT (reg_class_contents[rclass],
   hard_regno - nregs_diff)))

would be:

|| !TEST_HARD_REG_BIT (reg_class_contents[rclass],
   hard_regno + biggest_nregs - 1)))

Where does that little-endian check happen?  I wasn't sure why
"biggest_mode goes outside the class" problems only occured here
for big-endian.

Thanks,
Richard

Patch ping (was Re: [PATCH] Fix ICU miscompilation due to tree-ssa-strlen.c bug (PR tree-optimization/88693))

[Jakub Jelinek]

On Fri, Jan 04, 2019 at 11:54:46PM +0100, Jakub Jelinek wrote:
> The following patch fixes ICU miscompilation, where an initial part of a
> buffer is initializer from non-zero terminated string literal (in ICU
> actually from an array of non-zero chars that the FE newly turns into
> non-zero terminated string literal), but the code recently added to
> tree-ssa-strlen.c doesn't consider the possibility of string literals that
> aren't zero terminated.
> 
> STRING_CSTs actually are always zero terminated, but if TREE_STRING_LENGTH
> doesn't include the terminating character, it is just bytes.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
> 
> I was considering using strnlen, but if all STRING_CSTs are actually zero
> terminated, it will mean it reads are most one further byte and on many
> targets strlen is more optimized than strnlen.

I mean hosts.

> 2019-01-04  Jakub Jelinek  
> 
>   PR tree-optimization/88693
>   * tree-ssa-strlen.c (get_min_string_length): Don't set *full_string_p
>   for STRING_CSTs that don't contain any NUL characters in the first
>   TREE_STRING_LENGTH bytes.
> 
>   * gcc.c-torture/execute/pr88693.c: New test.

I'd like to ping this patch.  Thanks.

Jakub

Re: [v3 PATCH] Implement LWG 2221, No formatted output operator for nullptr

[Rainer Orth]

Hi Jonathan,

>>this patch broke Solaris bootstrap:
>>
>>ld: fatal: libstdc++-symbols.ver-sun: 7117: symbol 'std::basic_ostream>std::char_traits >::operator<<(decltype(nullptr))': symbol version 
>>conflict
>>ld: fatal: libstdc++-symbols.ver-sun: 7119: symbol 
>>'std::basic_ostream 
>>>::operator<<(decltype(nullptr))': symbol version conflict
>>
>>ld: fatal: libstdc++-symbols.ver-sun: 7117: symbol '_ZNSolsEDn': symbol 
>>version conflict
>>ld: fatal: libstdc++-symbols.ver-sun: 7119: symbol 
>>'_ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn': symbol version conflict
>>
>>Again, there were two matches for those two symbols:
>>
>>  GLIBCXX_3.4
>>##_ZNSolsE*[^Dg] (glob)
>>_ZNSolsEDn;
>>  GLIBCXX_3.4.26
>>##_ZNSolsEDn (glob)
>>_ZNSolsEDn;
>>
>>  GLIBCXX_3.4
>>##_ZNSt13basic_ostreamIwSt11char_traitsIwEElsE*[^Dg] (glob)
>>_ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn;
>>  GLIBCXX_3.4.26
>>##_ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn (glob)
>>_ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn;
>>
>>ISTM that the patterns were backwards.  The following patch fixes this
>>and allowed i386-pc-solaris2.11 bootstrap to complete without
>>regressions relative to the last successful one.
>
> I think what I should have done is change [^g] to [^gn]. That
> preserves the original behaviour (don't match the ppc64 long double
> symbols) but also excludes the new symbols, which end in 'n'.
>
> Maybe the attached patch would be better though. It matches every
> basic_ostream::operator<<(T) for any scalar T except 'g', and adds a
> second pattern to match basic_ostream::operator<<(T*) for various T.
> But neither of those matches the new operator<<(nullptr_t) overload.

it allowed me to link libstdc++.so, too.  For my patch I'd only been
going from the ld errors and the matching patterns in the generated
libstdc++.map-sun, not knowing the background here.

> FWIW I did run my symbol checker script, but it gets lots of false
> positives because it doesn't understand the #if preprocessor
> conditions, so it sees lots of false positive duplicates. I need to
> make it smarter for it to be useful here.

Indeed: the variation possible here can be a total PITA ;-)

Thanks.
Rainer

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