Re: [PATCH] PR libstdc++/88607 replace or remove unnecessary UTF-8 characters

2019-01-05 Thread Jonathan Wakely

On 03/01/19 22:44 +, Jonathan Wakely wrote:

On 03/01/19 22:07 +, Jonathan Wakely wrote:

On 03/01/19 20:38 +, Jonathan Wakely wrote:

There are a number of UTF-8 characters in comments which add no value
and can be replaced with ASCII equivalents, or removed entirely for the
section sign (U+00A7).

PR libstdc++/88607
* include/bits/forward_list.h: Replace UTF-8 "ligature fi" character.
* include/debug/forward_list: Likewise.
* include/experimental/bits/shared_ptr.h: Remove UTF-8 "section sign"
character.
* include/experimental/chrono: Likewise.
* include/experimental/functional: Likewise.
* include/experimental/ratio: Likewise.
* include/experimental/system_error: Likewise.
* include/experimental/tuple: Likewise.
* include/experimental/type_traits: Likewise.
* include/parallel/workstealing.h: Replace UTF-8 "en dash" character.
* include/parallel/multiseq_selection.h: Likewise.


This replaces some more non-ASCII characters.

Tested powerpc64-linux, committed to trunk.


The tests added by this patch would probably be a good idea, but I'm
not sure if -finput-charset=ascii works for all targets.


This is committed to trunk now. If some targets fail the test we can
deal with them as needed.



commit 53a31b41d06059fdaf1f09fd66f5d9e219246759
Author: Jonathan Wakely 
Date:   Thu Jan 3 22:18:14 2019 +

   PR libstdc++/88607 add tests using -finput-charset=ascii
   
   This verifies that the  header can be compiled with ASCII

   as the input character set.
   
   PR libstdc++/88607

   * testsuite/17_intro/headers/c++1998/charset.cc: New test.
   * testsuite/17_intro/headers/c++2011/charset.cc: New test.
   * testsuite/17_intro/headers/c++2014/charset.cc: New test.
   * testsuite/17_intro/headers/c++2017/charset.cc: New test.
   * testsuite/17_intro/headers/c++2020/charset.cc: New test.

diff --git a/libstdc++-v3/testsuite/17_intro/headers/c++1998/charset.cc 
b/libstdc++-v3/testsuite/17_intro/headers/c++1998/charset.cc
new file mode 100644
index 000..864c64ef831
--- /dev/null
+++ b/libstdc++-v3/testsuite/17_intro/headers/c++1998/charset.cc
@@ -0,0 +1,4 @@
+// { dg-options "-finput-charset=ascii" }
+// { dg-do compile }
+
+#include 
diff --git a/libstdc++-v3/testsuite/17_intro/headers/c++2011/charset.cc 
b/libstdc++-v3/testsuite/17_intro/headers/c++2011/charset.cc
new file mode 100644
index 000..864c64ef831
--- /dev/null
+++ b/libstdc++-v3/testsuite/17_intro/headers/c++2011/charset.cc
@@ -0,0 +1,4 @@
+// { dg-options "-finput-charset=ascii" }
+// { dg-do compile }
+
+#include 
diff --git a/libstdc++-v3/testsuite/17_intro/headers/c++2014/charset.cc 
b/libstdc++-v3/testsuite/17_intro/headers/c++2014/charset.cc
new file mode 100644
index 000..864c64ef831
--- /dev/null
+++ b/libstdc++-v3/testsuite/17_intro/headers/c++2014/charset.cc
@@ -0,0 +1,4 @@
+// { dg-options "-finput-charset=ascii" }
+// { dg-do compile }
+
+#include 
diff --git a/libstdc++-v3/testsuite/17_intro/headers/c++2017/charset.cc 
b/libstdc++-v3/testsuite/17_intro/headers/c++2017/charset.cc
new file mode 100644
index 000..864c64ef831
--- /dev/null
+++ b/libstdc++-v3/testsuite/17_intro/headers/c++2017/charset.cc
@@ -0,0 +1,4 @@
+// { dg-options "-finput-charset=ascii" }
+// { dg-do compile }
+
+#include 
diff --git a/libstdc++-v3/testsuite/17_intro/headers/c++2020/charset.cc 
b/libstdc++-v3/testsuite/17_intro/headers/c++2020/charset.cc
new file mode 100644
index 000..864c64ef831
--- /dev/null
+++ b/libstdc++-v3/testsuite/17_intro/headers/c++2020/charset.cc
@@ -0,0 +1,4 @@
+// { dg-options "-finput-charset=ascii" }
+// { dg-do compile }
+
+#include 




Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Jan Hubicka
> On Sat, Jan 05, 2019 at 03:50:06PM -0800, Steve Kargl wrote:
> > 
> > /gfortran.dg/pr79966.f90 -fno-diagnostics-show-caret 
> > -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -O -O2 
> > -fpeel-loops -finline-functions -fipa-cp-clone -fdump-ipa-inline-details -S 
> > -o pr79966.s
> > PASS: gfortran.dg/pr79966.f90   -O  (test for excess errors)
> > FAIL: gfortran.dg/pr79966.f90   -O   scan-ipa-dump inline "Inlined 
> > tp_sum/[0-9]+ into runtptests/[0-9]+"
> > testcase /safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/dg.exp completed in 0 
> > seconds
> > 
> > on x86_64-*-freebsd.
> > 
> 
> This Bug ipa/88711

Thanks, I will take a look tomorrow.
https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00218.html
fixes rather important and old bug in accounting runtime estimate for
functions. This affects decisions of inliner and ipa-cp, so the testcase
may need retuning.

I am working on retuning of the inlining parameters and discovered the
bug while looking into few anomalies. Funilly it means that I can start
my tests over.

Honza
> 
> -- 
> Steve


Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Steve Kargl
On Sat, Jan 05, 2019 at 03:50:06PM -0800, Steve Kargl wrote:
> 
> /gfortran.dg/pr79966.f90 -fno-diagnostics-show-caret 
> -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -O -O2 
> -fpeel-loops -finline-functions -fipa-cp-clone -fdump-ipa-inline-details -S 
> -o pr79966.s
> PASS: gfortran.dg/pr79966.f90   -O  (test for excess errors)
> FAIL: gfortran.dg/pr79966.f90   -O   scan-ipa-dump inline "Inlined 
> tp_sum/[0-9]+ into runtptests/[0-9]+"
> testcase /safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/dg.exp completed in 0 
> seconds
> 
> on x86_64-*-freebsd.
> 

This Bug ipa/88711

-- 
Steve


Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Steve Kargl
On Sat, Jan 05, 2019 at 11:48:15PM +0100, Jan Hubicka wrote:
> > > FAIL: gfortran.dg/class_alias.f90   -Os  execution test
> > 
> > I see this failure on trunk as well, but I don't think it was
> > introduced by my commit. Instead, I see I it starting with r267601:
> > 
> > https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=267601
> > 
> > Jan, could you please have a look?
> 
> Actually I have noticed about half an hour ago that i commited WIP
> variant of the patch.  It is fixed by this patch
> 
> I apologize for the breakage.
> 
> Index: ChangeLog
> ===
> --- ChangeLog (revision 267602)
> +++ ChangeLog (working copy)
> @@ -1,5 +1,15 @@
>  2019-01-05  Jan Hubicka  
>  
> + * doc/invoke.texi (max-inline-insns-small): New parameters.
> + * ipa-inline.c (want_early_inline_function_p): simplify.
> + (want_inline_small_function_p): Fix pasto from previous patch;
> + use max-inline-insns-small bound.
> + * params.def (max-inline-insns-small): New param.
> + * ipa-fnsummary.c (analyze_function_body): Initialize time/size
> + variables correctly.
> +

/gfortran.dg/pr79966.f90 -fno-diagnostics-show-caret 
-fno-diagnostics-show-line-numbers -fdiagnostics-color=never -O -O2 
-fpeel-loops -finline-functions -fipa-cp-clone -fdump-ipa-inline-details -S -o 
pr79966.s
PASS: gfortran.dg/pr79966.f90   -O  (test for excess errors)
FAIL: gfortran.dg/pr79966.f90   -O   scan-ipa-dump inline "Inlined 
tp_sum/[0-9]+ into runtptests/[0-9]+"
testcase /safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/dg.exp completed in 0 
seconds

on x86_64-*-freebsd.

-- 
steve






Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Jan Hubicka
> > FAIL: gfortran.dg/class_alias.f90   -Os  execution test
> 
> I see this failure on trunk as well, but I don't think it was
> introduced by my commit. Instead, I see I it starting with r267601:
> 
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=267601
> 
> Jan, could you please have a look?

Actually I have noticed about half an hour ago that i commited WIP
variant of the patch.  It is fixed by this patch

I apologize for the breakage.

Index: ChangeLog
===
--- ChangeLog   (revision 267602)
+++ ChangeLog   (working copy)
@@ -1,5 +1,15 @@
 2019-01-05  Jan Hubicka  
 
+   * doc/invoke.texi (max-inline-insns-small): New parameters.
+   * ipa-inline.c (want_early_inline_function_p): simplify.
+   (want_inline_small_function_p): Fix pasto from previous patch;
+   use max-inline-insns-small bound.
+   * params.def (max-inline-insns-small): New param.
+   * ipa-fnsummary.c (analyze_function_body): Initialize time/size
+   variables correctly.
+
+2019-01-05  Jan Hubicka  
+
* doc/invoke.texi: Document max-inline-insns-size,
uninlined-function-insns, uninlined-function-time,
uninlined-thunk-insns and uninlined-thunk-time.
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 267601)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2019-01-05  Jan Hubicka  
+
+   * gcc.dg/ipa/ipcp-2.c: Update bounds.
+
 2019-01-05  Dominique d'Humieres  
 
* gcc.dg/plugin/plugindir1.c: Adjust dg-prune-output for Darwin.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 267601)
+++ doc/invoke.texi (working copy)
@@ -11007,6 +11007,10 @@ by the compiler are investigated.  To th
 (more restrictive) limit compared to functions declared inline can
 be applied.
 
+@item max-inline-insns-small
+This is bound applied to calls which are considered relevant with
+@option{-finline-small-functions}.
+
 @item max-inline-insns-size
 This is bound applied to calls which are optimized for size. Small growth
 may be desirable to anticipate optimization oppurtunities exposed by inlining.
Index: testsuite/gcc.dg/ipa/ipcp-2.c
===
--- testsuite/gcc.dg/ipa/ipcp-2.c   (revision 267601)
+++ testsuite/gcc.dg/ipa/ipcp-2.c   (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O3 -fipa-cp -fipa-cp-clone -fdump-ipa-cp -fno-early-inlining 
--param ipa-cp-eval-threshold=80"  } */
+/* { dg-options "-O3 -fipa-cp -fipa-cp-clone -fdump-ipa-cp -fno-early-inlining 
--param ipa-cp-eval-threshold=200"  } */
 /* { dg-add-options bind_pic_locally } */
 
 extern int get_stuff (int);
Index: ipa-inline.c
===
--- ipa-inline.c(revision 267601)
+++ ipa-inline.c(working copy)
@@ -637,8 +637,7 @@ want_early_inline_function_p (struct cgr
 
   if (growth <= PARAM_VALUE (PARAM_MAX_INLINE_INSNS_SIZE))
;
-  else if (!e->maybe_hot_p ()
-  && growth > 0)
+  else if (!e->maybe_hot_p ())
{
  if (dump_enabled_p ())
dump_printf_loc (MSG_MISSED_OPTIMIZATION, e->call_stmt,
@@ -791,7 +790,7 @@ want_inline_small_function_p (struct cgr
   ipa_hints hints = estimate_edge_hints (e);
   int big_speedup = -1; /* compute this lazily */
 
-  if (growth <= PARAM_VALUE (PARAM_VALUE (PARAM_MAX_INLINE_INSNS_SIZE)))
+  if (growth <= PARAM_VALUE (PARAM_MAX_INLINE_INSNS_SIZE))
;
   /* Apply MAX_INLINE_INSNS_SINGLE limit.  Do not do so when
 hints suggests that inlining given function is very profitable.  */
@@ -809,9 +808,8 @@ want_inline_small_function_p (struct cgr
  want_inline = false;
}
   else if (!DECL_DECLARED_INLINE_P (callee->decl)
-  && (in_lto_p
-  && growth >= PARAM_VALUE (PARAM_EARLY_INLINING_INSNS))
-  && !opt_for_fn (e->caller->decl, flag_inline_functions))
+  && !opt_for_fn (e->caller->decl, flag_inline_functions)
+  && growth >= PARAM_VALUE (PARAM_MAX_INLINE_INSNS_SMALL))
{
  /* growth_likely_positive is expensive, always test it last.  */
   if (growth >= MAX_INLINE_INSNS_SINGLE
Index: params.def
===
--- params.def  (revision 267601)
+++ params.def  (working copy)
@@ -83,6 +83,11 @@ DEFPARAM (PARAM_MAX_INLINE_INSNS_AUTO,
  "The maximum number of instructions when automatically inlining.",
  30, 0, 0)
 
+DEFPARAM (PARAM_MAX_INLINE_INSNS_SMALL,
+ "max-inline-insns-small",
+ "The maximum number of instructions when automatically inlining small 
functions.",
+ 0, 0, 0)
+
 DEFPARAM (PARAM_MAX_INLINE_INSNS_SIZE,
  "max-

[PATCH][jit] Add thread-local globals to the libgccjit frontend

2019-01-05 Thread Marc Nieper-Wißkirchen
This patch adds thread-local globals to the libgccjit frontend. The
library user can mark a global as being thread-local by calling
`gcc_jit_lvalue_set_bool_thread_local'. It is implemented by calling
`set_decl_tls_model (inner, decl_default_tls_model (inner))', where
`inner' is the GENERIC tree corresponding to the global.

ChangeLog

2019-01-05  Marc Nieper-Wißkirchen  

* docs/topics/compatibility.rst: Add LIBGCCJIT_ABI_11.
* docs/topics/expressions.rst (Global variables): Add
documentation of gcc_jit_lvalue_set_bool_thread_local.
* docs/_build/texinfo/libgccjit.texi: Regenerate.
* jit-playback.c: Include "varasm.h".
Within namespace gcc::jit::playback...
(context::new_global) Add "thread_local_p" param and use it
to set DECL_TLS_MODEL.
* jit-playback.h: Within namespace gcc::jit::playback...
(context::new_global: Add "thread_local_p" param.
* jit-recording.c: Within namespace gcc::jit::recording...
(global::replay_into): Provide m_thread_local to call to
new_global.
(global::write_reproducer): Call write_reproducer_thread_local.
(global::write_reproducer_thread_local): New method.
* jit-recording.h: Within namespace gcc::jit::recording...
(lvalue::dyn_cast_global): New virtual function.
(global::m_thread_local): New field.
* libgccjit.c (gcc_jit_lvalue_set_bool_thread_local): New
function.
* libgccjit.h
(LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local): New
macro.
(gcc_jit_lvalue_set_bool_thread_local): New function.
* libgccjit.map (LIBGCCJIT_ABI_11): New.
(gcc_jit_lvalue_set_bool_thread_local): Add.

Testing

The result of `make check-jit' is (the failing test in
`test-sum-squares.c` is also failing without this patch on my
machine):

Native configuration is x86_64-pc-linux-gnu

=== jit tests ===

Schedule of variations:
unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file
for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/mnieper/gcc/src/gcc/testsuite/config/default.exp as
tool-and-target-specific interface file.
Running /home/mnieper/gcc/src/gcc/testsuite/jit.dg/jit.exp ...
FAIL:  test-combination.c.exe iteration 1 of 5:
verify_code_sum_of_squares: dump_vrp1: actual: "
FAIL: test-combination.c.exe killed: 20233 exp6 0 0 CHILDKILLED SIGABRT SIGABRT
FAIL:  test-sum-of-squares.c.exe iteration 1 of 5: verify_code:
dump_vrp1: actual: "
FAIL: test-sum-of-squares.c.exe killed: 22698 exp6 0 0 CHILDKILLED
SIGABRT SIGABRT
FAIL:  test-threads.c.exe: verify_code_sum_of_squares: dump_vrp1: actual: "
FAIL: test-threads.c.exe killed: 22840 exp6 0 0 CHILDKILLED SIGABRT SIGABRT

=== jit Summary ===

# of expected passes 6506
# of unexpected failures 6

Patch

diff --git a/gcc/jit/docs/topics/compatibility.rst
b/gcc/jit/docs/topics/compatibility.rst
index 38d338b1f..10926537d 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -171,3 +171,9 @@ entrypoints:

 ``LIBGCCJIT_ABI_10`` covers the addition of
 :func:`gcc_jit_context_new_rvalue_from_vector`
+
+``LIBGCCJIT_ABI_11``
+
+
+``LIBGCCJIT_ABI_11`` covers the addition of
+:func:`gcc_jit_lvalue_set_bool_thread_local`
diff --git a/gcc/jit/docs/topics/expressions.rst
b/gcc/jit/docs/topics/expressions.rst
index 9dee2d87a..984d02cc8 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -576,6 +576,21 @@ Global variables
   referring to it.  Analogous to using an "extern" global from a
   header file.

+.. function:: void\
+  gcc_jit_lvalue_set_bool_thread_local (gcc_jit_lvalue *global,\
+int thread_local_p)
+
+   Given a :c:type:`gcc_jit_lvalue *` for a global created through
+   :c:func:`gcc_jit_context_new_global`, mark/clear the global as being
+   thread-local. Analogous to a global with thread storage duration in C11.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_11`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+  #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local
+
 Working with pointers, structs and unions
 -

diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 86f588db9..6c00a98c0 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "opt-suggestions.h"
 #include "gcc.h"
 #include "diagnostic.h"
+#include "varasm.h"

 #include 

@@ -461,7 +462,8 @@ playback::context::
 new_global (location *loc,
  enum gcc_jit_global_kind kind,
  type *type,
- const char *name)
+ const char *name,
+ bool thread_local_p)
 {
   gcc_assert (type);
   gcc_assert (name);
@@ -487,6 +489,8 @@ new_global (location *loc,
   DECL_EXTERNAL (inner) = 1;
   break;
 }
+  if (thread_local_p)
+set_decl_tls_model (inner, decl_default_tls_model (inner));

   if (loc)
 set_tr

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

2019-01-05 Thread Dominique d'Humières
Hi Martin,

The patch on top of r267591 fixes pr88638 without regression.

Note

 FAIL: c-c++-common/attributes-4.c  -std=gnu++14 (test for excess errors)
 FAIL: c-c++-common/attributes-4.c  -std=gnu++17 (test for excess errors)
 FAIL: c-c++-common/attributes-4.c  -std=gnu++98 (test for excess errors)

Thanks for the fix.

Dominique



[PATCH] Define new filesystem::__file_clock type

2019-01-05 Thread Jonathan Wakely

In C++17 the clock used for filesystem::file_time_type is unspecified,
allowing it to be chrono::system_clock. The C++2a draft requires it to
be a distinct type, with additional member functions to convert to/from
other clocks (either the system clock or UTC). In order to avoid an ABI
change later, this patch defines a new distinct type now, which will be
used for std::chrono::file_clock later.

* include/bits/fs_fwd.h (__file_clock): Define new clock.
(file_time_type): Redefine in terms of __file_clock.
* src/filesystem/ops-common.h (file_time): Add FIXME comment about
overflow.
* src/filesystem/std-ops.cc (is_set(perm_options, perm_options)): Give
internal linkage.
(internal_file_lock): New helper type for accessing __file_clock.
(do_copy_file): Use internal_file_lock to convert system time to
file_time_type.
(last_write_time(const path&, error_code&)): Likewise.
(last_write_time(const path&, file_time_type, error_code&)): Likewise.

Tested powerpc64-linux, committed to trunk.

commit c91a86730ed8df76325dd0649d8837e6feb24aab
Author: Jonathan Wakely 
Date:   Sat Jan 5 12:52:40 2019 +

Define new filesystem::__file_clock type

In C++17 the clock used for filesystem::file_time_type is unspecified,
allowing it to be chrono::system_clock. The C++2a draft requires it to
be a distinct type, with additional member functions to convert to/from
other clocks (either the system clock or UTC). In order to avoid an ABI
change later, this patch defines a new distinct type now, which will be
used for std::chrono::file_clock later.

* include/bits/fs_fwd.h (__file_clock): Define new clock.
(file_time_type): Redefine in terms of __file_clock.
* src/filesystem/ops-common.h (file_time): Add FIXME comment about
overflow.
* src/filesystem/std-ops.cc (is_set(perm_options, perm_options)): 
Give
internal linkage.
(internal_file_lock): New helper type for accessing __file_clock.
(do_copy_file): Use internal_file_lock to convert system time to
file_time_type.
(last_write_time(const path&, error_code&)): Likewise.
(last_write_time(const path&, file_time_type, error_code&)): 
Likewise.

diff --git a/libstdc++-v3/include/bits/fs_fwd.h 
b/libstdc++-v3/include/bits/fs_fwd.h
index 3bcf2dd650c..a0e3d73e2a3 100644
--- a/libstdc++-v3/include/bits/fs_fwd.h
+++ b/libstdc++-v3/include/bits/fs_fwd.h
@@ -294,7 +294,49 @@ _GLIBCXX_END_NAMESPACE_CXX11
   operator^=(directory_options& __x, directory_options __y) noexcept
   { return __x = __x ^ __y; }
 
-  using file_time_type = std::chrono::system_clock::time_point;
+  struct __file_clock
+  {
+using duration  = chrono::nanoseconds;
+using rep   = duration::rep;
+using period= duration::period;
+using time_point= chrono::time_point<__file_clock>;
+static constexpr bool is_steady = false;
+
+static time_point
+now() noexcept
+{ return _S_from_sys(chrono::system_clock::now()); }
+
+  private:
+using __sys_clock = chrono::system_clock;
+
+// This clock's (unspecified) epoch is 2174-01-01 00:00:00 UTC.
+// A signed 64-bit duration with nanosecond resolution gives roughly
+// +/- 292 years, which covers the 1901-2446 date range for ext4.
+static constexpr chrono::seconds _S_epoch_diff{6437664000};
+
+  protected:
+// For internal use only
+template
+  static
+  chrono::time_point<__file_clock, _Dur>
+  _S_from_sys(const chrono::time_point<__sys_clock, _Dur>& __t) noexcept
+  {
+   using __file_time = chrono::time_point<__file_clock, _Dur>;
+   return __file_time{__t.time_since_epoch()} - _S_epoch_diff;
+  }
+
+// For internal use only
+template
+  static
+  chrono::time_point<__sys_clock, _Dur>
+  _S_to_sys(const chrono::time_point<__file_clock, _Dur>& __t) noexcept
+  {
+   using __sys_time = chrono::time_point<__sys_clock, _Dur>;
+   return __sys_time{__t.time_since_epoch()} + _S_epoch_diff;
+  }
+  };
+
+  using file_time_type = __file_clock::time_point;
 
   // operational functions
 
diff --git a/libstdc++-v3/src/filesystem/ops-common.h 
b/libstdc++-v3/src/filesystem/ops-common.h
index dcd61cc26cd..1c0d650f444 100644
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -158,6 +158,24 @@ namespace __gnu_posix
 nanoseconds ns{};
 #endif
 
+// FIXME
+// There are possible timespec values which will overflow
+// chrono::system_clock::time_point but would not overflow
+// __file_clock::time_point, due to its different epoch.
+//
+// By checking for overflow of the intermediate system_clock::duration
+// type, we report an error for values which are actually representable
+//

Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Steve Kargl
On Sat, Jan 05, 2019 at 11:45:42AM -0800, Steve Kargl wrote:
> Execution timeout is: 300
> spawn [open ...]
> STOP 2
> FAIL: gfortran.dg/class_alias.f90   -Os  execution test
> 
> STOP 2 occurs at the end of testcase, so I suspect finalization is messed up.

This is bizarre.  Changing the code where STOP 2 occurs to

  if (var_p%x .ne. 2) then
 print *, var_p%x
 STOP 2
  end if

Compiling with gfortran gives

% gfcx -o z -Os class_alias.f90 && ./z
   2
STOP 2

Something is broken with -Os and your patch.  The original dump
contains

  if (var_p._data->x != 2)
{
  {
struct __st_parameter_dt dt_parm.5;

dt_parm.5.common.filename = &"class_alias.f90"[1]{lb: 1 sz: 1};
dt_parm.5.common.line = 92;
dt_parm.5.common.flags = 128;
dt_parm.5.common.unit = 6;
_gfortran_st_write (&dt_parm.5);
_gfortran_transfer_integer_write (&dt_parm.5, &var_p._data->x, 4);
_gfortran_st_write_done (&dt_parm.5);
  }
  _gfortran_stop_numeric (2, 0);
}

which looks correct.  The -fdump-tree-optimized looks like var_p has been
sort of skipped. 

  _11 = var_a._data;
  _12 = _11->x;
  if (_12 != 2)
goto ; [0.04%]
  else
goto ; [99.96%]

   [local count: 429153]:
  _gfortran_stop_numeric (1, 0);   

This is the 'if (var_a%x /= 2) STOP 1' chunk.  We jump to ,
which is the body of the if-endif block that contains STOP 2.

   [local count: 428982]:
  dt_parm.5.common.filename = &"class_alias.f90"[1]{lb: 1 sz: 1};
  dt_parm.5.common.line = 92;
  MEM[(integer(kind=4) *)&dt_parm.5] = 25769803904;
  _gfortran_st_write (&dt_parm.5);
  _14 = &MEM[(struct test *)_6].x;
  _gfortran_transfer_integer_write (&dt_parm.5, _14, 4);
  _gfortran_st_write_done (&dt_parm.5);
  dt_parm.5 ={v} {CLOBBER};
  _gfortran_stop_numeric (2, 0);

I would expect the basic block  to look like

   [local count: x]
  _13 = var_p._data;
  _14 = _13->x;
  if (_14 != 2)
 goto 
  else
 goto 

Now, what is  above would be the contents of  and
 would then jump to the deallocation statements.

-- 
Steve


Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Steve Kargl
On Sat, Jan 05, 2019 at 11:39:08AM -0800, Steve Kargl wrote:
> On Sat, Jan 05, 2019 at 03:35:21PM +0100, Janus Weil wrote:
> > Am Sa., 5. Jan. 2019 um 14:37 Uhr schrieb Thomas Koenig 
> > :
> > >
> > > Hi Janus,
> > >
> > > > the attached patch fixes PR 88009, an ICE-on-invalid regression caused
> > > > by one of my earlier commits. Apart from adding some extra checks to
> > > > avoid ICEs, it also uses the 'artificial' attribute to suppress bogus
> > > > errors (see comment #3) and does some minor cleanup in
> > > > resolve_fl_variable.
> > > >
> > > > Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> > >
> > >
> > > the patch looks good, OK for trunk.
> > 
> > thanks for the quick review, Thomas! Committed as r267598.
> > 
> 
> I still need to verify, but it appears that this commit 
> is causing a regression on x86_64-*-freebsd.
> 
> FAIL: gfortran.dg/class_alias.f90   -Os  execution test
> 
> Reverting parts of commits and rebuilding and testing takes
> about an hour or more, verification won't be available for
> bit.
> 

More info from build log.


Executing on host: /safe/sgk/gcc/objx/gcc/testsuite/gfortran/../../gfortran 
-B/safe/sgk/gcc/objx/gcc/testsuite/gfortran/../../ 
-B/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/ 
/safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/class_alias.f90
-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers 
-fdiagnostics-color=never-Os  -fdump-tree-original  
-B/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libatomic/.libs 
-B/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs  -lm  -o 
./class_alias.exe(timeout = 300)
spawn -ignore SIGHUP /safe/sgk/gcc/objx/gcc/testsuite/gfortran/../../gfortran 
-B/safe/sgk/gcc/objx/gcc/testsuite/gfortran/../../ 
-B/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/ 
/safe/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/class_alias.f90 
-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers 
-fdiagnostics-color=never -Os -fdump-tree-original 
-B/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libatomic/.libs 
-B/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs 
-L/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs -lm -o 
./class_alias.exe
PASS: gfortran.dg/class_alias.f90   -Os  (test for excess errors)
Setting LD_LIBRARY_PATH to 
.:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libatomic/.libs:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs:/safe/sgk/gcc/objx/gcc:.:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libgfortran/.libs:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libatomic/.libs:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs:/safe/sgk/gcc/objx/x86_64-unknown-freebsd13.0/./libquadmath/.libs:/safe/sgk/gcc/objx/gcc
Execution timeout is: 300
spawn [open ...]
STOP 2
FAIL: gfortran.dg/class_alias.f90   -Os  execution test

STOP 2 occurs at the end of testcase, so I suspect finalization is messed up.
-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow


Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Steve Kargl
On Sat, Jan 05, 2019 at 03:35:21PM +0100, Janus Weil wrote:
> Am Sa., 5. Jan. 2019 um 14:37 Uhr schrieb Thomas Koenig 
> :
> >
> > Hi Janus,
> >
> > > the attached patch fixes PR 88009, an ICE-on-invalid regression caused
> > > by one of my earlier commits. Apart from adding some extra checks to
> > > avoid ICEs, it also uses the 'artificial' attribute to suppress bogus
> > > errors (see comment #3) and does some minor cleanup in
> > > resolve_fl_variable.
> > >
> > > Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> >
> >
> > the patch looks good, OK for trunk.
> 
> thanks for the quick review, Thomas! Committed as r267598.
> 

I still need to verify, but it appears that this commit 
is causing a regression on x86_64-*-freebsd.

FAIL: gfortran.dg/class_alias.f90   -Os  execution test

Reverting parts of commits and rebuilding and testing takes
about an hour or more, verification won't be available for
bit.

-- 
Steve


Re: [Committed] Fix the gcc.dg/plugin/plugindir*.c tests for darwin

2019-01-05 Thread Dominique d'Humières
> Committed on trunk as obvious

Committed as revision r267599. Patch attached.

Dominique



patch-plug
Description: Binary data


Add new --param knobs for inliner

2019-01-05 Thread Jan Hubicka
Hi,
this patch adds new parameters to ipa-inline. max-inline-insns-size is
useful to increase inlining limits for programs with large abstraction
penalty.

uinlined-* should be useful for architecutures with greater function
call overhead than modern x86 chips (which is good portion of them,
especially s390 as I learnt on Cauldron). It would be nice to benchmark
effect of those and tune default in config/* files. I think this is a
reasonable way to deal with architecutral differences without making
inliner hard to tune in long term.

Bootstrapped/regtested x86_64-linux, plan to commit it soon.

Honza

* doc/invoke.texi: Document max-inline-insns-size,
uninlined-function-insns, uninlined-function-time,
uninlined-thunk-insns and uninlined-thunk-time.
* params.def: Add max-inline-insns-size,
uninlined-function-insns, uninlined-function-time,
uninlined-thunk-insns and uninlined-thunk-time.
* ipa-fnsummary.c (compute_fn_summary, analyze_function_body): Use
new parameters.
* ipa-inline.c (can_inline_edge_by_limits_p,
want_inline_small_function_p): Use new parameters.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 267585)
+++ doc/invoke.texi (working copy)
@@ -11007,6 +11007,23 @@ by the compiler are investigated.  To th
 (more restrictive) limit compared to functions declared inline can
 be applied.
 
+@item max-inline-insns-size
+This is bound applied to calls which are optimized for size. Small growth
+may be desirable to anticipate optimization oppurtunities exposed by inlining.
+
+@item uninlined-function-insns
+Number of instructions accounted by inliner for function overhead such as
+function prologue and epilogue.
+
+@item uninlined-function-time
+Extra time accounted by inliner for function overhead such as time needed to
+execute function prologue and epilogue
+
+@item uninlined-thunk-insns
+@item uninlined-thunk-time
+Same as @option{--param uninlined-function-insns} and
+@option{--param uninlined-function-time} but applied to function thunks
+
 @item inline-min-speedup
 When estimated performance improvement of caller + callee runtime exceeds this
 threshold (in percent), the function can be inlined regardless of the limit on
Index: ipa-fnsummary.c
===
--- ipa-fnsummary.c (revision 267600)
+++ ipa-fnsummary.c (working copy)
@@ -2034,7 +2081,10 @@ analyze_function_body (struct cgraph_nod
   info->account_size_time (0, 0, bb_predicate, bb_predicate);
 
   bb_predicate = predicate::not_inlined ();
-  info->account_size_time (2 * ipa_fn_summary::size_scale, 0, bb_predicate,
+  info->account_size_time (PARAM_VALUE (PARAM_UNINLINED_FUNCTION_INSNS)
+  * ipa_fn_summary::size_scale,
+  PARAM_VALUE (PARAM_UNINLINED_FUNCTION_TIME),
+  bb_predicate,
   bb_predicate);
 
   if (fbi.info)
@@ -2418,7 +2468,11 @@ compute_fn_summary (struct cgraph_node *
   node->local.can_change_signature = false;
   es->call_stmt_size = eni_size_weights.call_cost;
   es->call_stmt_time = eni_time_weights.call_cost;
-  info->account_size_time (ipa_fn_summary::size_scale * 2, 2, t, t);
+  info->account_size_time (ipa_fn_summary::size_scale
+  * PARAM_VALUE
+(PARAM_UNINLINED_FUNCTION_THUNK_INSNS),
+  PARAM_VALUE
+(PARAM_UNINLINED_FUNCTION_THUNK_TIME), t, t);
   t = predicate::not_inlined ();
   info->account_size_time (2 * ipa_fn_summary::size_scale, 0, t, t);
   ipa_update_overall_fn_summary (node);
Index: ipa-inline.c
===
--- ipa-inline.c(revision 267585)
+++ ipa-inline.c(working copy)
@@ -523,7 +523,7 @@ can_inline_edge_by_limits_p (struct cgra
   > opt_for_fn (caller->decl, optimize_size))
{
  int growth = estimate_edge_growth (e);
- if (growth > 0
+ if (growth > PARAM_VALUE (PARAM_MAX_INLINE_INSNS_SIZE)
  && (!DECL_DECLARED_INLINE_P (callee->decl)
  && growth >= MAX (MAX_INLINE_INSNS_SINGLE,
MAX_INLINE_INSNS_AUTO)))
@@ -635,7 +635,7 @@ want_early_inline_function_p (struct cgr
   int growth = estimate_edge_growth (e);
   int n;
 
-  if (growth <= 0)
+  if (growth <= PARAM_VALUE (PARAM_MAX_INLINE_INSNS_SIZE))
;
   else if (!e->maybe_hot_p ()
   && growth > 0)
@@ -791,7 +791,7 @@ want_inline_small_function_p (struct cgr
   ipa_hints hints = estimate_edge_hints (e);
   int big_speedup = -1; /* compute this lazily */
 
-  if (growth <= 0)
+  if (growth <= PARAM_VALUE (PARAM_VALUE (PARAM_MAX_INLINE_INSNS_SIZE)))
;
   

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

2019-01-05 Thread Iain Sandoe


> 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 comment to that effect).

> Out of curiosity, is the attribute used for function call
> validation by compilers other than GCC?  I couldn't find
> anything online.

It used to be used for the platform GCC, when that was the “system compiler" 
(last edition
 apple 4.2.1) and I therefore assume it’s used by clang - but actually haven’t 
double-checked
at the time we added it - it was relevant.

Let’s revisit this in 10, and see if we can’t sync up with the platform 
expectations from the clang impl.

thanks for taking care of this,

Iain

** it’s actually a simple ObjectiveC object that happens to be supported 

Fix accounting of time in ipa-fnsummary

2019-01-05 Thread Jan Hubicka
Hi,
this patch fixes the way we account time.  analyze_function_body
uses frequency of BB execution and expected time to execute the stmt
to compute overall time it takes to invoke function, but it manages to
account time which is not scaled by frequency, yet.

Bootstrapped/regtested x86_64-linux, comitted.

2019-01-05  Jan Hubicka  

* ipa-fnsummary.c (analyze_function_body): Fix accounting of time.

Index: ipa-fnsummary.c
===
--- ipa-fnsummary.c (revision 267585)
+++ ipa-fnsummary.c (working copy)
@@ -2234,12 +2234,12 @@ analyze_function_body (struct cgraph_nod
{
  predicate ip = bb_predicate & predicate::not_inlined ();
  info->account_size_time (this_size * prob,
-  (this_time * prob) / 2, ip,
+  (final_time * prob) / 2, ip,
   p);
}
  if (prob != 2)
info->account_size_time (this_size * (2 - prob),
-(this_time * (2 - prob) / 2),
+(final_time * (2 - prob) / 2),
 bb_predicate,
 p);
}


Re: [PATCH] PR fortran/69101 -- IEEE_SELECTED_REAL_KIND part I

2019-01-05 Thread Steve Kargl
On Sat, Jan 05, 2019 at 09:22:34AM -0800, Steve Kargl wrote:
> On Sat, Jan 05, 2019 at 01:23:14PM +0100, Thomas Koenig wrote:
> > 
> > Maybe it would be better to put the checking and argument reordering
> > into its own routine (something like gfc_check_minloc_maxloc in
> > check.c) so all three arguments would be present, with an
> > expression possibly containing NULL, for the later routines.
> > This could add some clarity and save code duplication later,
> > when the second part of the patch is done.
> > 
> > However, I do not feel strongly about that. If you feel that your
> > current approach is better, that is fine by me.
> > 
> 
> Thanks, Thomas.  I tried a few things and finally landed on
> the keyword vs positional handling and argument checking in
> simplify_ieee_selected_real_kind.  The actual heavy lifting
> comes from gfc_simplify_selected_real_kind where the constant
> expressions are extracted and gfortran tries to determine a
> valid kind.  I see if I can moving the keyword/positional
> and checking into a separate function.
> 

Well, that was quick.  Moving code around is problematic.  The
hijacking of the interface checking causes gfortran to see from
1 to 3 actual arguments.  A simple example is ISRK(r=3.14) (yes,
an invalid call).  This has exact 1 actual argument, namely,
arg=expr->value.function.actual.  So, gfortran see arg->next=NULL
and arg->next->next obvious cannot exist.  The re-ordering would
require allocation of next and next->next to get the number of
actual arguments to match P,R,RADIX, so that we can extract 
pointers to the expressions.  Not worth the effort at the moment.

Of I might change my mind when I try to tackle the hijacking of

  function rknd(n) result r
use ieee_arithmetic
integer n
r = ieee_selected_real_kind(p=n)
  end function rknd

-- 
Steve


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

2019-01-05 Thread Martin Sebor

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...

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.

Out of curiosity, is the attribute used for function call
validation by compilers other than GCC?  I couldn't find
anything online.

Thanks again!
Martin


[Committed] Fix the gcc.dg/plugin/plugindir*.c tests for darwin

2019-01-05 Thread Dominique d'Humières
Committed on trunk as obvious

* gcc.dg/plugin/plugindir1.c: Adjust dg-prune-output for Darwin.
* gcc.dg/plugin/plugindir2.c: Likewise.
* gcc.dg/plugin/plugindir3.c: Likewise.
* gcc.dg/plugin/plugindir4.c: Likewise.

Dominique



Re: [PATCH] PR fortran/69101 -- IEEE_SELECTED_REAL_KIND part I

2019-01-05 Thread Steve Kargl
On Sat, Jan 05, 2019 at 01:23:14PM +0100, Thomas Koenig wrote:
> 
> Maybe it would be better to put the checking and argument reordering
> into its own routine (something like gfc_check_minloc_maxloc in
> check.c) so all three arguments would be present, with an
> expression possibly containing NULL, for the later routines.
> This could add some clarity and save code duplication later,
> when the second part of the patch is done.
> 
> However, I do not feel strongly about that. If you feel that your
> current approach is better, that is fine by me.
> 

Thanks, Thomas.  I tried a few things and finally landed on
the keyword vs positional handling and argument checking in
simplify_ieee_selected_real_kind.  The actual heavy lifting
comes from gfc_simplify_selected_real_kind where the constant
expressions are extracted and gfortran tries to determine a
valid kind.  I see if I can moving the keyword/positional
and checking into a separate function.

Long term I think we need to put all functions into the table
of intrinsic subprograms, so that handling these functions is
almost identical to sin(), cos(), etc.  This would then give
the keyword vs positional handling automatically.  ieee module
procedures would be marked in gfc_intrinsic_sym with a bool or
a bit in a bitfield, ie,

   gfc_intrinsic_sym *isym;
   isym->ieee = true;   ! Member of ieee_arthmetic.
or
   isym->ieee = 1;  ! Member of ieee_arithmetic.

'isym->ieee = true' then indicates that gfortran needs to check
if the ieee_arithmetic has been used and is in scope when a procedure
is seen in the Fortran code.  If yes, we have the module procedure.
If no, we have a possibly user-defined procedure that shadows a routine
from an intrinsic module.

-- 
Steve


Re: [PATCH, C++,rebased] Fix PR c++/88261

2019-01-05 Thread Bernd Edlinger
On 1/4/19 10:22 PM, Jason Merrill wrote:
> Hmm, I'm uncomfortable with starting to pass in the decl just for the sake of 
> deciding whether this diagnostic should be a pedwarn or error. In general, 
> because of copy elision, we can't know at this point what we're initializing, 
> so I'd rather not pretend we can.  Instead, maybe add a 
> LOOKUP_ALLOW_FLEXARY_INIT flag that you can add to the flags argument in the 
> call from store_init_value?
> 

Okay, I reworked the patch, to pass a bit in the flags, it was a bit more 
complicated
than anticipated, because it is necessary to pass the flag thru 
process_init_constructor
and friends to the recursive invocation of digest_init_r.  It turned out that
digest_nsdmi_init did not need to change, since it is always wrong to use 
flexarray init
there.  I added a new test case (flexary32.C) to exercises a few cases where 
non static
direct member intializers are allowed to use flexarrays (in static members) and 
where that
would be wrong (in automatic members).  So  that seems to work.

New version of the patch is attached.

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


Thanks
Bernd.
gcc/cp:
2019-01-05  Bernd Edlinger  

	PR c++/88261
	PR c++/69696
	* cp-tree.h (LOOKUP_ALLOW_FLEXARRAY_INIT): New flag value.
	* typeck2.c (digest_init_r): Raise an error for non-static
	initialization of a flexible array member.
	(process_init_constructor, massage_init_elt,
	process_init_constructor_array, process_init_constructor_record,
	process_init_constructor_union, process_init_constructor): Add the
	flags parameter and pass it thru.
	(store_init_value): Pass LOOKUP_ALLOW_FLEXARRAY_INIT parameter to
	digest_init_flags for static decls.

gcc/testsuite:
2019-01-05  Bernd Edlinger  

	PR c++/88261
	PR c++/69696
	* gcc.dg/array-6.c: Move from here ...
	* c-c++-common/array-6.c: ... to here and add some more test coverage.
	* g++.dg/ext/flexary32.C: New test.
	* g++.dg/ext/flexary3.C: Adjust test.
	* g++.dg/ext/flexary12.C: Likewise.
	* g++.dg/ext/flexary13.C: Likewise.
	* g++.dg/ext/flexary15.C: Likewise.
	* g++.dg/warn/Wplacement-new-size-1.C: Likewise.
	* g++.dg/warn/Wplacement-new-size-2.C: Likewise.
	* g++.dg/warn/Wplacement-new-size-6.C: Likewise.


Index: gcc/cp/cp-tree.h
Index: gcc/cp/cp-tree.h
===
--- gcc/cp/cp-tree.h	(revision 267569)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -5454,6 +5454,8 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, T
 #define LOOKUP_NO_NON_INTEGRAL (LOOKUP_NO_RVAL_BIND << 1)
 /* Used for delegating constructors in order to diagnose self-delegation.  */
 #define LOOKUP_DELEGATING_CONS (LOOKUP_NO_NON_INTEGRAL << 1)
+/* Allow initialization of a flexible array members.  */
+#define LOOKUP_ALLOW_FLEXARRAY_INIT (LOOKUP_DELEGATING_CONS << 1)
 
 #define LOOKUP_NAMESPACES_ONLY(F)  \
   (((F) & LOOKUP_PREFER_NAMESPACES) && !((F) & LOOKUP_PREFER_TYPES))
Index: gcc/cp/typeck2.c
===
--- gcc/cp/typeck2.c	(revision 267569)
+++ gcc/cp/typeck2.c	(working copy)
@@ -35,7 +35,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gcc-rich-location.h"
 
 static tree
-process_init_constructor (tree type, tree init, int nested,
+process_init_constructor (tree type, tree init, int nested, int flags,
 			  tsubst_flags_t complain);
 
 
@@ -817,8 +817,12 @@ store_init_value (tree decl, tree init, vec *v = CONSTRUCTOR_ELTS (init);
@@ -1365,7 +1383,8 @@ static int
 	ce->index = error_mark_node;
   gcc_assert (ce->value);
   ce->value
-	= massage_init_elt (TREE_TYPE (type), ce->value, nested, complain);
+	= massage_init_elt (TREE_TYPE (type), ce->value, nested, flags,
+			complain);
 
   gcc_checking_assert
 	(ce->value == error_mark_node
@@ -1373,7 +1392,7 @@ static int
 	 (strip_array_types (TREE_TYPE (type)),
 	  strip_array_types (TREE_TYPE (ce->value);
 
-  flags |= picflag_from_initializer (ce->value);
+  picflags |= picflag_from_initializer (ce->value);
 }
 
   /* No more initializers. If the array is unbounded, we are done. Otherwise,
@@ -1389,7 +1408,8 @@ static int
 	   we can't rely on the back end to do it for us, so make the
 	   initialization explicit by list-initializing from T{}.  */
 	next = build_constructor (init_list_type_node, NULL);
-	next = massage_init_elt (TREE_TYPE (type), next, nested, complain);
+	next = massage_init_elt (TREE_TYPE (type), next, nested, flags,
+ complain);
 	if (initializer_zerop (next))
 	  /* The default zero-initialization is fine for us; don't
 		 add anything to the CONSTRUCTOR.  */
@@ -1406,7 +1426,7 @@ static int
 
 	if (next)
 	  {
-	flags |= picflag_from_initializer (next);
+	picflags |= picflag_from_initializer (next);
 	if (len > i+1
 		&& (initializer_constant_valid_p (next, TREE_TYPE (next))
 		== null_pointer_node))
@@ -1426,7 +1446,7 @@ static int
   }
 
  

[PATCH v3] GCC: Fix Loongson3 LLSC Errata

2019-01-05 Thread YunQiang Su
From: Paul Hua 

In some older Loongson3 processors there is a LL/SC errata that can
cause the CPU to deadlock occasionally.  The details are very
complicated. We find a way to work around this errata by
 a) adding a sync before ll/lld instruction,
 b) adding a sync before branch target that between ll and sc.
The assembler do the jobs 'a', gcc do the jobs 'b'.

This patch add an option '-mfix-loongson3-llsc' option, and it will also
call 'as' with the same option. So if 'as' doesn't support this option,
an error will happen.

The macro '__mips_fix_loongson3_llsc' is predefined, when this option is
enabled, as some program, like linux kernel will need to aware this
option is used.

This patch also add a configure options
--with-mips-fix-loongson3-llsc=[yes|no] to enable fix-loongson3-llsc
by config.

v1 -> v2:
  * Predefine __mips_fix_loongson3_llsc.
  * Add to ASM_SPECS.
v2 -> v3:
  * Add the missing patch to doc/*.texi back.

gcc/
* config.gcc (supported_defaults): Add fix-loongson3-llsc
(with_fix_loongson3_llsc): Add validation.
(all_defaults): Add fix-loongson3-llsc.
* config/mips/mips.c (mips_process_sync_loop): Add sync before
branch target that between ll and sc.
* config/mips/mips.h
(OPTION_DEFAULT_SPECS): Add a default for fix-loongson3-llsc.
(ASM_SPECS): Add a default for fix-loongson3-llsc.
(TARGET_CPU_CPP_BUILTINS): Predefine __mips_fix_loongson3_llsc.
gcc/config/mips/mips.opt: New option.
* doc/install.texi (--with-fix-loongson3-llsc):Document the new
option.
* doc/invoke.texi (-mfix-loongson3-llsc):Document the new option.

gcc/testsuite/
* gcc.target/mips/fix-loongson3-llsc.c: New test.
* gcc.target/mips/mips.exp (option): Add fix-loongson3-llsc.
---
 gcc/config.gcc| 19 +--
 gcc/config/mips/mips.c| 13 +++--
 gcc/config/mips/mips.h|  7 ++-
 gcc/config/mips/mips.opt  |  4 
 gcc/doc/install.texi  |  4 
 gcc/doc/invoke.texi   |  8 
 .../gcc.target/mips/fix-loongson3-llsc.c  | 10 ++
 gcc/testsuite/gcc.target/mips/mips.exp|  1 +
 8 files changed, 61 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/mips/fix-loongson3-llsc.c

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 3f5a37dc8..4e7609345 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4283,7 +4283,7 @@ case "${target}" in
;;
 
mips*-*-*)
-   supported_defaults="abi arch arch_32 arch_64 float fpu nan 
fp_32 odd_spreg_32 tune tune_32 tune_64 divide llsc mips-plt synci lxc1-sxc1 
madd4"
+   supported_defaults="abi arch arch_32 arch_64 float fpu nan 
fp_32 odd_spreg_32 tune tune_32 tune_64 divide llsc mips-plt synci lxc1-sxc1 
madd4 fix-loongson3-llsc"
 
case ${with_float} in
"" | soft | hard)
@@ -4436,6 +4436,21 @@ case "${target}" in
exit 1
;;
esac
+
+   case ${with_fix_loongson3_llsc} in
+   yes)
+   with_fix_loongson3_llsc=fix-loongson3-llsc
+   ;;
+   no)
+   with_fix_loongson3_llsc=no-fix-loongson3-llsc
+   ;;
+   "")
+   ;;
+   *)
+   echo "Unknown fix-loongson3-llsc type used in 
--with-fix-loongson3-llsc" 1>&2
+   exit 1
+   ;;
+   esac
;;
 
nds32*-*-*)
@@ -4955,7 +4970,7 @@ case ${target} in
 esac
 
 t=
-all_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64 
schedule float mode fpu nan fp_32 odd_spreg_32 divide llsc mips-plt synci tls 
lxc1-sxc1 madd4"
+all_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64 
schedule float mode fpu nan fp_32 odd_spreg_32 divide llsc mips-plt synci tls 
lxc1-sxc1 madd4 fix-loongson3-llsc"
 for option in $all_defaults
 do
eval "val=\$with_"`echo $option | sed s/-/_/g`
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 95dc9468e..9cafc1629 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -14127,7 +14127,7 @@ mips_process_sync_loop (rtx_insn *insn, rtx *operands)
   mips_multi_start ();
 
   /* Output the release side of the memory barrier.  */
-  if (need_atomic_barrier_p (model, true))
+  if (need_atomic_barrier_p (model, true) && !FIX_LOONGSON3_LLSC)
 {
   if (required_oldval == 0 && TARGET_OCTEON)
{
@@ -14148,6 +14148,10 @@ mips_process_sync_loop (rtx_insn *insn, rtx *operands)
   /* Output the branch-back label.  */
   mips_multi_add_label ("1:");
 
+  /* Loongson3 target need sync before ll/lld.  */
+  if (need_atomic_barrier_p (model,  true) && F

Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Janus Weil
Am Sa., 5. Jan. 2019 um 14:37 Uhr schrieb Thomas Koenig :
>
> Hi Janus,
>
> > the attached patch fixes PR 88009, an ICE-on-invalid regression caused
> > by one of my earlier commits. Apart from adding some extra checks to
> > avoid ICEs, it also uses the 'artificial' attribute to suppress bogus
> > errors (see comment #3) and does some minor cleanup in
> > resolve_fl_variable.
> >
> > Regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
>
> the patch looks good, OK for trunk.

thanks for the quick review, Thomas! Committed as r267598.

Cheers,
Janus


Re: [Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Thomas Koenig

Hi Janus,


the attached patch fixes PR 88009, an ICE-on-invalid regression caused
by one of my earlier commits. Apart from adding some extra checks to
avoid ICEs, it also uses the 'artificial' attribute to suppress bogus
errors (see comment #3) and does some minor cleanup in
resolve_fl_variable.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?



the patch looks good, OK for trunk.

Regards

Thomas


[Patch, Fortran] PR 88009: [9 Regression] ICE in find_intrinsic_vtab, at fortran/class.c:2761

2019-01-05 Thread Janus Weil
Hi all,

the attached patch fixes PR 88009, an ICE-on-invalid regression caused
by one of my earlier commits. Apart from adding some extra checks to
avoid ICEs, it also uses the 'artificial' attribute to suppress bogus
errors (see comment #3) and does some minor cleanup in
resolve_fl_variable.

Regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2019-01-05  Janus Weil  

PR fortran/88009
* class.c (gfc_find_derived_vtab): Mark the _final component as
artificial.
(find_intrinsic_vtab): Ditto. Also add an extra check to avoid
dereferencing a null pointer and adjust indentation.
* resolve.c (resolve_fl_variable): Add extra check to avoid
dereferencing a null pointer. Move variable declarations to local scope.
(resolve_fl_procedure): Add extra check to avoid dereferencing a null
pointer.
* symbol.c (check_conflict): Suppress errors for artificial symbols.


2019-01-05  Janus Weil  

PR fortran/88009
* gfortran.dg/blockdata_10.f90: New test case.
diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c
index e55ab25882f..77f0fca9385 100644
--- a/gcc/fortran/class.c
+++ b/gcc/fortran/class.c
@@ -2466,6 +2466,7 @@ gfc_find_derived_vtab (gfc_symbol *derived)
 		goto cleanup;
 	  c->attr.proc_pointer = 1;
 	  c->attr.access = ACCESS_PRIVATE;
+	  c->attr.artificial = 1;
 	  c->tb = XCNEW (gfc_typebound_proc);
 	  c->tb->ppc = 1;
 	  generate_finalization_wrapper (derived, ns, tname, c);
@@ -2762,9 +2763,9 @@ find_intrinsic_vtab (gfc_typespec *ts)
 	  /* This is elemental so that arrays are automatically
 		 treated correctly by the scalarizer.  */
 	  copy->attr.elemental = 1;
-	  if (ns->proc_name->attr.flavor == FL_MODULE)
+	  if (ns->proc_name && ns->proc_name->attr.flavor == FL_MODULE)
 		copy->module = ns->proc_name->name;
-		  gfc_set_sym_referenced (copy);
+	  gfc_set_sym_referenced (copy);
 	  /* Set up formal arguments.  */
 	  gfc_get_symbol ("src", sub_ns, &src);
 	  src->ts.type = ts->type;
@@ -2798,6 +2799,7 @@ find_intrinsic_vtab (gfc_typespec *ts)
 		goto cleanup;
 	  c->attr.proc_pointer = 1;
 	  c->attr.access = ACCESS_PRIVATE;
+	  c->attr.artificial = 1;
 	  c->tb = XCNEW (gfc_typebound_proc);
 	  c->tb->ppc = 1;
 	  c->initializer = gfc_get_null_expr (NULL);
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index a498d19e411..beafe8da8bc 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -12274,13 +12274,8 @@ deferred_requirements (gfc_symbol *sym)
 static bool
 resolve_fl_variable (gfc_symbol *sym, int mp_flag)
 {
-  int no_init_flag, automatic_flag;
-  gfc_expr *e;
-  const char *auto_save_msg;
-  bool saved_specification_expr;
-
-  auto_save_msg = "Automatic object %qs at %L cannot have the "
-		  "SAVE attribute";
+  const char *auto_save_msg = "Automatic object %qs at %L cannot have the "
+			  "SAVE attribute";
 
   if (!resolve_fl_var_and_proc (sym, mp_flag))
 return false;
@@ -12288,7 +12283,7 @@ resolve_fl_variable (gfc_symbol *sym, int mp_flag)
   /* Set this flag to check that variables are parameters of all entries.
  This check is effected by the call to gfc_resolve_expr through
  is_non_constant_shape_array.  */
-  saved_specification_expr = specification_expr;
+  bool saved_specification_expr = specification_expr;
   specification_expr = true;
 
   if (sym->ns->proc_name
@@ -12315,6 +12310,8 @@ resolve_fl_variable (gfc_symbol *sym, int mp_flag)
 {
   /* Make sure that character string variables with assumed length are
 	 dummy arguments.  */
+  gfc_expr *e = NULL;
+
   if (sym->ts.u.cl)
 	e = sym->ts.u.cl->length;
   else
@@ -12364,7 +12361,7 @@ resolve_fl_variable (gfc_symbol *sym, int mp_flag)
 apply_default_init_local (sym); /* Try to apply a default initialization.  */
 
   /* Determine if the symbol may not have an initializer.  */
-  no_init_flag = automatic_flag = 0;
+  int no_init_flag = 0, automatic_flag = 0;
   if (sym->attr.allocatable || sym->attr.external || sym->attr.dummy
   || sym->attr.intrinsic || sym->attr.result)
 no_init_flag = 1;
@@ -12494,7 +12491,7 @@ resolve_fl_procedure (gfc_symbol *sym, int mp_flag)
  module procedures are excluded by 2.2.3.3 - i.e., they are not
  externally accessible and can access all the objects accessible in
  the host.  */
-  if (!(sym->ns->parent
+  if (!(sym->ns->parent && sym->ns->parent->proc_name
 	&& sym->ns->parent->proc_name->attr.flavor == FL_MODULE)
   && gfc_check_symbol_access (sym))
 {
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index a88e7c01df7..cd52c73031b 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -440,6 +440,9 @@ check_conflict (symbol_attribute *attr, const char *name, locus *where)
   const char *a1, *a2;
   int standard;
 
+  if (attr->artificial)
+return true;
+
   if (where == NULL)
 where = &gfc_current_locus;
 
diff --git a/gcc/testsuit

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

2019-01-05 Thread Jakub Jelinek
On Sat, Jan 05, 2019 at 01:17:21PM +, Bernd Edlinger wrote:
> Well, yes it works, but this can only optimize invalid code like 
> strlen((char*)L"wide").

There is nothing invalid on it.  Furthermore, the strlen pass doesn't
optimize just strlen, it does many optimizations that rely on knowing how
many non-zero bytes there are and if followed by zero byte.
Initialization from a multi-byte string literal is just an initialization
like any other, from array of ints, or characters, whatever.

Jakub


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

2019-01-05 Thread Bernd Edlinger
On 1/5/19 1:31 PM, Jakub Jelinek wrote:
> On Sat, Jan 05, 2019 at 12:20:24PM +, Bernd Edlinger wrote:
>> co-incidentally my "Make strlen range computations more conservative" patch
>> contained a fix on the same spot, I just did not have a test case for it:
>>
>> @@ -3184,7 +3146,10 @@ get_min_string_length (tree rhs, bool *f
>>&& TREE_READONLY (rhs))
>>  rhs = DECL_INITIAL (rhs);
>>
>> -  if (rhs && TREE_CODE (rhs) == STRING_CST)
>> +  if (rhs && TREE_CODE (rhs) == STRING_CST
>> +  && tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (rhs == 1
>> +  && TREE_STRING_LENGTH (rhs) > 0
>> +  && TREE_STRING_POINTER (rhs) [TREE_STRING_LENGTH (rhs) - 1] == '\0')
>>  {
>>*full_string_p = true;
>>return strlen (TREE_STRING_POINTER (rhs));
>>
>>
>> additionally to your patch this tests the string is in fact a single-byte 
>> string.
>> since strlen returns garbage otherwise.
> 
> Multi-byte STRING_CSTs seem to be stored in the target byte order, e.g.
> L"abcde" in a x86_64-linux -> powerpc64-linux cross is still
> "\000\000\000a\000\000\000b\000\000\000c\000\000\000d\000\000\000\000"
> so I think strlen is exactly what we want.  The tree-ssa-strlen.c pass
> doesn't track string lengths in whatever units it has, it tracks number of
> non-zero bytes followed by zero byte known at certain address, or, if there
> is no zero byte known, the minimum amount of non-zero bytes known at certain
> address.
> 

Well, yes it works, but this can only optimize invalid code like 
strlen((char*)L"wide").

However, optimizing invalid stuff away does both defeat warnings in later 
passes,
and is also not helpful for debugging the resulting code in general.

> And, your patch has also the bad effect that it won't return any length for
> the strings that aren't zero terminated.  We do want to return 9 for the
> "abcdefghi" string without zero terminator at the end, just need to set
> *full_string_p to false.  And, for "abcd\000fghi" string without zero
> termination at the end, we do want to return 4 and set *full_string_p to
> true.
> 

Right, I did not consider the nul in the middle.


Bernd.


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

2019-01-05 Thread Jakub Jelinek
On Sat, Jan 05, 2019 at 12:20:24PM +, Bernd Edlinger wrote:
> co-incidentally my "Make strlen range computations more conservative" patch
> contained a fix on the same spot, I just did not have a test case for it:
> 
> @@ -3184,7 +3146,10 @@ get_min_string_length (tree rhs, bool *f
>&& TREE_READONLY (rhs))
>  rhs = DECL_INITIAL (rhs);
> 
> -  if (rhs && TREE_CODE (rhs) == STRING_CST)
> +  if (rhs && TREE_CODE (rhs) == STRING_CST
> +  && tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (rhs == 1
> +  && TREE_STRING_LENGTH (rhs) > 0
> +  && TREE_STRING_POINTER (rhs) [TREE_STRING_LENGTH (rhs) - 1] == '\0')
>  {
>*full_string_p = true;
>return strlen (TREE_STRING_POINTER (rhs));
> 
> 
> additionally to your patch this tests the string is in fact a single-byte 
> string.
> since strlen returns garbage otherwise.

Multi-byte STRING_CSTs seem to be stored in the target byte order, e.g.
L"abcde" in a x86_64-linux -> powerpc64-linux cross is still
"\000\000\000a\000\000\000b\000\000\000c\000\000\000d\000\000\000\000"
so I think strlen is exactly what we want.  The tree-ssa-strlen.c pass
doesn't track string lengths in whatever units it has, it tracks number of
non-zero bytes followed by zero byte known at certain address, or, if there
is no zero byte known, the minimum amount of non-zero bytes known at certain
address.

And, your patch has also the bad effect that it won't return any length for
the strings that aren't zero terminated.  We do want to return 9 for the
"abcdefghi" string without zero terminator at the end, just need to set
*full_string_p to false.  And, for "abcd\000fghi" string without zero
termination at the end, we do want to return 4 and set *full_string_p to
true.

Jakub


Re: [PATCH] PR fortran/69101 -- IEEE_SELECTED_REAL_KIND part I

2019-01-05 Thread Thomas Koenig

Hi Steve,

thanks for taking this on!

Maybe it would be better to put the checking and argument reordering
into its own routine (something like gfc_check_minloc_maxloc in
check.c) so all three arguments would be present, with an
expression possibly containing NULL, for the later routines.
This could add some clarity and save code duplication later,
when the second part of the patch is done.

However, I do not feel strongly about that. If you feel that your
current approach is better, that is fine by me.

Regards

Thomas


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

2019-01-05 Thread Bernd Edlinger
Hi,

co-incidentally my "Make strlen range computations more conservative" patch
contained a fix on the same spot, I just did not have a test case for it:

@@ -3184,7 +3146,10 @@ get_min_string_length (tree rhs, bool *f
   && TREE_READONLY (rhs))
 rhs = DECL_INITIAL (rhs);

-  if (rhs && TREE_CODE (rhs) == STRING_CST)
+  if (rhs && TREE_CODE (rhs) == STRING_CST
+  && tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (rhs == 1
+  && TREE_STRING_LENGTH (rhs) > 0
+  && TREE_STRING_POINTER (rhs) [TREE_STRING_LENGTH (rhs) - 1] == '\0')
 {
   *full_string_p = true;
   return strlen (TREE_STRING_POINTER (rhs));


additionally to your patch this tests the string is in fact a single-byte 
string.
since strlen returns garbage otherwise.


Bernd.


[PATCH] Add a two value VR comparison to two consecutive PHI args optimization (PR tree-optimization/88676, take 2)

2019-01-05 Thread Jakub Jelinek
On Sat, Jan 05, 2019 at 09:51:31AM +0100, Richard Biener wrote:
> >+  if (TYPE_PRECISION (TREE_TYPE (lhs)) == TYPE_PRECISION (TREE_TYPE
> >(arg0)))
> >+{
> >+  if (TREE_CODE (TREE_TYPE (arg0)) == BOOLEAN_TYPE
> >+  || !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)))
> >+type = TREE_TYPE (lhs);
> >+  else
> >+type = TREE_TYPE (arg0);
> 
> This misses some comments. Presumably you want to avoid arithmetic with bool 
> types (or generally non-mode precision types!?).

Just for bool because the weird semantics it has.  And do as few conversions
as possible.

> >+}
> >+  else if (TYPE_PRECISION (TREE_TYPE (lhs)) > TYPE_PRECISION
> >(TREE_TYPE (arg0)))
> >+type = TREE_TYPE (lhs);
> >+  else
> >+type = TREE_TYPE (arg0);
> >+  if (!TYPE_OVERFLOW_WRAPS (type))
> >+type = unsigned_type_for (type);
> 
> Would doing this unconditionally simplify things? I think "optimizing" the - 
> fwrapv case isn't worth it. 

I've originally meant to do it only if really needed but then got lazy.

> >+  tree m = fold_convert (type, wide_int_to_tree (TREE_TYPE (lhs),
> >min));
> >+  enum tree_code code;
> >+  if (tree_int_cst_lt (arg0, arg1))
> >+{
> >+  code = PLUS_EXPR;
> >+  m = int_const_binop (MINUS_EXPR, fold_convert (type, arg0), m);
> >+}
> >+  else
> >+{
> >+  code = MINUS_EXPR;
> >+  m = int_const_binop (PLUS_EXPR, fold_convert (type, arg0), m);
> >+}
> 
> Delay wide-int to tree conversion until here. 

But by doing these computations on wide-int properly it appears it isn't
that hard to do.

Essentially, after conversion to type, lhs is known to be in [min, min+1]
range and we either want to perform lhs + a or a - lhs.
If it is in signed type, we need to verify it doesn't overflow, so if
a is non-negative, make sure (min+1) + a doesn't overflow and if a
is negative, make sure min + a doesn't overflow.  Similarly, for
a - lhs case, if min (and min+1) are non-negative, make sure a - (min+1)
doesn't overflow, and if min is negative (min+1 in that case could be
negative or 0), make sure a - min doesn't overflow.

Ok for trunk if it passes bootstrap/regtest?

I guess I should add more testcases that test these corner cases,
the boundary cases of overflowing/not overflowing and different precisions
of the two types.

2019-01-05  Jakub Jelinek  

PR tree-optimization/88676
* tree-ssa-phiopt.c (two_value_replacement): New function.
(tree_ssa_phiopt_worker): Call it.

* gcc.dg/tree-ssa/pr88676.c: New test.
* gcc.dg/pr88676.c: New test.
* gcc.dg/tree-ssa/pr15826.c: Just verify there is no goto,
allow &.

--- gcc/tree-ssa-phiopt.c.jj2019-01-04 18:53:11.315322371 +0100
+++ gcc/tree-ssa-phiopt.c   2019-01-05 13:04:38.185082743 +0100
@@ -48,6 +48,8 @@ along with GCC; see the file COPYING3.
 #include "case-cfn-macros.h"
 
 static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
+static bool two_value_replacement (basic_block, basic_block, edge, gphi *,
+  tree, tree);
 static bool conditional_replacement (basic_block, basic_block,
 edge, edge, gphi *, tree, tree);
 static gphi *factor_out_conditional_conversion (edge, edge, gphi *, tree, tree,
@@ -332,8 +334,11 @@ tree_ssa_phiopt_worker (bool do_store_el
}
 
  /* Do the replacement of conditional if it can be done.  */
- if (!early_p
- && conditional_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
+ if (two_value_replacement (bb, bb1, e2, phi, arg0, arg1))
+   cfgchanged = true;
+ else if (!early_p
+  && conditional_replacement (bb, bb1, e1, e2, phi,
+  arg0, arg1))
cfgchanged = true;
  else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
cfgchanged = true;
@@ -572,6 +577,142 @@ factor_out_conditional_conversion (edge
   return newphi;
 }
 
+/* Optimize
+   # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
+   if (x_5 op cstN) # where op is == or != and N is 1 or 2
+ goto bb3;
+   else
+ goto bb4;
+   bb3:
+   bb4:
+   # r_6 = PHI # where cst3 == cst4 + 1 or cst4 == cst3 + 1
+
+   to r_6 = x_5 + (min (cst3, cst4) - cst1) or
+   r_6 = (min (cst3, cst4) + cst1) - x_5 depending on op, N and which
+   of cst3 and cst4 is smaller.  */
+
+static bool
+two_value_replacement (basic_block cond_bb, basic_block middle_bb,
+  edge e1, gphi *phi, tree arg0, tree arg1)
+{
+  /* Only look for adjacent integer constants.  */
+  if (!INTEGRAL_TYPE_P (TREE_TYPE (arg0))
+  || !INTEGRAL_TYPE_P (TREE_TYPE (arg1))
+  || TREE_CODE (arg0) != INTEGER_CST
+  || TREE_CODE (arg1) != INTEGER_CST
+  || (tree_int_cst_lt (arg0, arg1)
+ ? wi::to_widest (arg0) + 1 != wi::to_widest (arg1)
+ : wi::to_widest (arg1) + 1 != wi::to_widest (arg1)))
+return false;
+
+  if (!empty_block_p (middle_bb))
+   

Re: [nvptx] vector length patch series

2019-01-05 Thread Tom de Vries
On 22-12-18 03:13, Tom de Vries wrote:
> If you have a test-case where this is indeed failing without the
> proposed hook implementation, then please try to remove the hardcoding
> of vector_length > 32 from the test-source and instead set it using
> -fopenacc-dim. AFAIU, the proposed hook does not handle that case, so
> you should be able to make it fail.

Filed as PR88706 - "[og8, nvptx, openacc] Inconsistencies when vector
length set using vector_length clause or fopenacc-dim" (
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88706 ).

Thanks,
- Tom


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

2019-01-05 Thread Iain Sandoe
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).

Iain



Re: [PATCH 2/6, OpenACC, libgomp] Async re-work, oacc-* parts (revised, v4)

2019-01-05 Thread Chung-Lin Tang

Hi Thomas,
this is the current version of the oacc-* parts of the Async Re-work patch.

I have reverted away from the earlier mentioned attempt of using lockless
techniques to manage the asyncqueues; it is really hard to do in a 100% correct
manner, unless we only use something like simple lists to manage them,
which probably makes lookup unacceptably slow.

For now, I have changed to use the conventional locking and success/fail return
codes for the synchronize/serialize hooks. I hope this is enough to pass
and get committed.

Thanks,
Chung-Lin

Index: oacc-async.c
===
--- oacc-async.c(revision 267507)
+++ oacc-async.c(working copy)
@@ -27,10 +27,99 @@
.  */
 
 #include 
+#include 
 #include "openacc.h"
 #include "libgomp.h"
 #include "oacc-int.h"
 
+static struct goacc_thread *
+get_goacc_thread (void)
+{
+  struct goacc_thread *thr = goacc_thread ();
+
+  if (!thr || !thr->dev)
+gomp_fatal ("no device active");
+
+  return thr;
+}
+
+static struct gomp_device_descr *
+get_goacc_thread_device (void)
+{
+  struct goacc_thread *thr = goacc_thread ();
+
+  if (!thr || !thr->dev)
+gomp_fatal ("no device active");
+
+  return thr->dev;
+}
+
+attribute_hidden struct goacc_asyncqueue *
+lookup_goacc_asyncqueue (struct goacc_thread *thr, bool create, int async)
+{
+  /* The special value acc_async_noval (-1) maps to the thread-specific
+ default async stream.  */
+  if (async == acc_async_noval)
+async = thr->default_async;
+
+  if (async == acc_async_sync)
+return NULL;
+
+  if (async < 0)
+gomp_fatal ("bad async %d", async);
+
+  struct goacc_asyncqueue *ret_aq = NULL;
+  struct gomp_device_descr *dev = thr->dev;
+
+  gomp_mutex_lock (&dev->openacc.async.lock);
+
+  if (!create
+  && (async >= dev->openacc.async.nasyncqueue
+ || !dev->openacc.async.asyncqueue[async]))
+goto end;
+
+  if (async >= dev->openacc.async.nasyncqueue)
+{
+  int diff = async + 1 - dev->openacc.async.nasyncqueue;
+  dev->openacc.async.asyncqueue
+   = gomp_realloc (dev->openacc.async.asyncqueue,
+   sizeof (goacc_aq) * (async + 1));
+  memset (dev->openacc.async.asyncqueue + dev->openacc.async.nasyncqueue,
+ 0, sizeof (goacc_aq) * diff);
+  dev->openacc.async.nasyncqueue = async + 1;
+}
+
+  if (!dev->openacc.async.asyncqueue[async])
+{
+  dev->openacc.async.asyncqueue[async] = dev->openacc.async.construct_func 
();
+
+  if (!dev->openacc.async.asyncqueue[async])
+   {
+ gomp_mutex_unlock (&dev->openacc.async.lock);
+ gomp_fatal ("async %d creation failed", async);
+   }
+  
+  /* Link new async queue into active list.  */
+  goacc_aq_list n = gomp_malloc (sizeof (struct goacc_asyncqueue_list));
+  n->aq = dev->openacc.async.asyncqueue[async];
+  n->next = dev->openacc.async.active;
+  dev->openacc.async.active = n;
+}
+
+  ret_aq = dev->openacc.async.asyncqueue[async];
+
+ end:
+  gomp_mutex_unlock (&dev->openacc.async.lock);
+  return ret_aq;
+}
+
+attribute_hidden struct goacc_asyncqueue *
+get_goacc_asyncqueue (int async)
+{
+  struct goacc_thread *thr = get_goacc_thread ();
+  return lookup_goacc_asyncqueue (thr, true, async);
+}
+
 int
 acc_async_test (int async)
 {
@@ -42,18 +131,25 @@ acc_async_test (int async)
   if (!thr || !thr->dev)
 gomp_fatal ("no device active");
 
-  return thr->dev->openacc.async_test_func (async);
+  goacc_aq aq = lookup_goacc_asyncqueue (thr, true, async);
+  return thr->dev->openacc.async.test_func (aq);
 }
 
 int
 acc_async_test_all (void)
 {
-  struct goacc_thread *thr = goacc_thread ();
+  struct goacc_thread *thr = get_goacc_thread ();
 
-  if (!thr || !thr->dev)
-gomp_fatal ("no device active");
-
-  return thr->dev->openacc.async_test_all_func ();
+  int ret = 1;
+  gomp_mutex_lock (&thr->dev->openacc.async.lock);
+  for (goacc_aq_list l = thr->dev->openacc.async.active; l; l = l->next)
+if (!thr->dev->openacc.async.test_func (l->aq))
+  {
+   ret = 0;
+   break;
+  }
+  gomp_mutex_unlock (&thr->dev->openacc.async.lock);
+  return ret;
 }
 
 void
@@ -62,12 +158,10 @@ acc_wait (int async)
   if (!async_valid_p (async))
 gomp_fatal ("invalid async argument: %d", async);
 
-  struct goacc_thread *thr = goacc_thread ();
+  struct goacc_thread *thr = get_goacc_thread ();
 
-  if (!thr || !thr->dev)
-gomp_fatal ("no device active");
-
-  thr->dev->openacc.async_wait_func (async);
+  goacc_aq aq = lookup_goacc_asyncqueue (thr, true, async);
+  thr->dev->openacc.async.synchronize_func (aq);
 }
 
 /* acc_async_wait is an OpenACC 1.0 compatibility name for acc_wait.  */
@@ -84,23 +178,34 @@ acc_async_wait (int async)
 void
 acc_wait_async (int async1, int async2)
 {
-  struct goacc_thread *thr = goacc_thread ();
+  struct goacc_thread *thr = get_goacc_thread ();
 
-  if (!thr || !thr->dev)
-   

Re: [PATCH] Fix expansion ICE with call returning VL structure (PR middle-end/82564, PR target/88620)

2019-01-05 Thread Richard Biener
On January 3, 2019 11:55:16 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>The following testcases ICE on x86_64 and other targets.
>
>The problem is that a function with nested functions returning VLA
>structures is inlined (or versioned) so that the variable sizes become
>constant, but we still have a MEM_REF[(struct S *) &D.1234] = fn ();
>where the D.1234 variable has fixed non-BLKmode size, but struct S is
>still a VLA type.  Furthermore, D.1234 isn't marked as addressable,
>because it is only accessed through such MEM_REFs.  Because of that
>mem_ref_refers_to_non_mem_p is true and we take a different
>expand_assignment path from normal VLA return expansion and try to
>create
>the VLA structure temporary, which ICEs obviously.
>
>The following patch fixes that by using a MEM temporary for this rare
>case.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>2019-01-03  Jakub Jelinek  
>
>   PR middle-end/82564
>   PR target/88620
>   * expr.c (expand_assignment): For calls returning VLA structures
>   if to_rtx is not a MEM, force it into a stack temporary.
>
>   * gcc.dg/nested-func-12.c: New test.
>   * gcc.c-torture/compile/pr82564.c: New test.
>
>--- gcc/expr.c.jj  2019-01-01 12:37:16.751981612 +0100
>+++ gcc/expr.c 2019-01-03 19:17:25.882346422 +0100
>@@ -5254,6 +5254,21 @@ expand_assignment (tree to, tree from, b
> emit_move_insn (XEXP (to_rtx, 1), read_complex_part (temp,
>true));
>   }
>   }
>+  /* For calls to functions returning variable length structures,
>if TO_RTX
>+   is not a MEM, go through a MEM because we must not create
>temporaries
>+   of the VLA type.  */
>+  else if (!MEM_P (to_rtx)
>+ && TREE_CODE (from) == CALL_EXPR
>+ && COMPLETE_TYPE_P (TREE_TYPE (from))
>+ && TREE_CODE (TYPE_SIZE (TREE_TYPE (from))) != INTEGER_CST)
>+  {
>+rtx temp = assign_stack_temp (GET_MODE (to_rtx),
>+  GET_MODE_SIZE (GET_MODE (to_rtx)));
>+result = store_field (temp, bitsize, bitpos, bitregion_start,
>+  bitregion_end, mode1, from, get_alias_set (to),
>+  nontemporal, reversep);
>+emit_move_insn (to_rtx, temp);
>+  }
>   else
>   {
> if (MEM_P (to_rtx))
>--- gcc/testsuite/gcc.dg/nested-func-12.c.jj   2019-01-03
>19:25:46.466084209 +0100
>+++ gcc/testsuite/gcc.dg/nested-func-12.c  2019-01-03 19:25:38.181220942
>+0100
>@@ -0,0 +1,48 @@
>+/* PR target/88620 */
>+/* { dg-do run } */
>+/* { dg-options "-Ofast --param ipa-cp-eval-threshold=0
>-fno-guess-branch-probability -fno-inline-small-functions" } */
>+/* { dg-require-effective-target alloca } */
>+
>+void
>+foo (int n)
>+{
>+  struct S { int a[n]; };
>+
>+  struct S
>+  fn (void)
>+  {
>+struct S s;
>+s.a[0] = 42;
>+return s;
>+  }
>+
>+  auto struct S
>+  fn2 (void)
>+  {
>+return fn ();
>+  }
>+
>+  struct S x;
>+  fn ();
>+  fn2 ();
>+  x = fn ();
>+
>+  if (x.a[0] != 42)
>+__builtin_abort ();
>+
>+  if (fn ().a[0] != 42)
>+__builtin_abort ();
>+
>+  __typeof__ (fn ()) *p = &x;
>+  if (p->a[0] != 42)
>+__builtin_abort ();
>+
>+  if (fn2 ().a[0] != 42)
>+__builtin_abort ();
>+}
>+
>+int
>+main (void)
>+{
>+  foo (1);
>+}
>--- gcc/testsuite/gcc.c-torture/compile/pr82564.c.jj   2019-01-03
>19:22:36.763215290 +0100
>+++ gcc/testsuite/gcc.c-torture/compile/pr82564.c  2019-01-03
>19:24:44.607105204 +0100
>@@ -0,0 +1,15 @@
>+/* PR middle-end/82564 */
>+/* { dg-require-effective-target alloca } */
>+
>+int
>+main ()
>+{
>+  int t = 8, i;
>+  typedef struct { char v[t]; } B; 
>+  B a, b;
>+  B __attribute__ ((noinline)) f () { return b; }
>+  for (i = 0; i < 8; i++)
>+b.v[i] = i;
>+  a = f ();
>+  return 0;
>+}
>
>   Jakub



Re: [PATCH] Tweak dwarf2out const_ok_for_output* (PR debug/88635)

2019-01-05 Thread Richard Biener
On January 3, 2019 11:36:00 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>The following testcase FAILs on 8.x branch, went latent later on on the
>trunk and I suppose Alex' i386.c ix86_const_not_ok_for_debug_p change
>would
>have prevented it too.
>
>The problem is in what the comment in const_ok_for_output* says, we
>don't do
>sufficient checking of what kind of expressions we accept inside of
>CONST
>and we could have there something that the assembler doesn't really
>like.
>In particular, in the testcase we ended up with
>(const:P (minus:P (const_int -2) (unspec [(symbol_ref ...)]
>UNSPEC_GOTOFF)))
>and as doesn't like -2-.LC0@gotoff.
>
>We already have some hacks in there, like punt on NOT or NEG.  The
>following
>patch extends it, by requiring that the CONST doesn't contain e.g. .LC0
>+ .LC1
>or any kind of labels in the second operand of MINUS (which is like
>negation).  In theory, we could allow .LC0 - .LC1 if both labels are in
>the
>same section, but the patch tries to be safe.
>
>After writing the patch, I've noticed Alex made
>ix86_const_not_ok_for_debug_p
>changes, but I think they aren't enough, they will still fail on e.g.
>all the cases
>where there are SYMBOL_REFs rather than UNSPECs involved, and on the
>other
>side it disallows 16+.LC0@gotoff which happens quite often.
>
>If const_ok_for_output rejects the whole CONST, there is already code
>to try
>to split it up into smaller expressions, so e.g. that .LC0 - .LC1 would
>be
>emitted then as DW_OP_addr .LC0 DW_OP_addr .LC1 DW_OP_minus, this patch
>extends it so that it can handle the UNSPECs that way as well, so the
>above
>would be DW_OP_const1s 0xff DW_OP_addr .LC0@gotoff DW_OP_minus.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
>without the i386.c part after a while for 8.x?

OK. 

Richard. 

>2019-01-03  Jakub Jelinek  
>
>   PR debug/88635
>   * dwarf2out.c (const_ok_for_output_1): Reject MINUS that contains
>   SYMBOL_REF, CODE_LABEL or UNSPEC in subexpressions of second argument.
>   Reject PLUS that contains SYMBOL_REF, CODE_LABEL or UNSPEC in
>   subexpressions of both operands.
>   (mem_loc_descriptor): Handle UNSPEC if target hook acks it and all the
>   subrtxes are CONSTANT_P.
>   * config/i386/i386.c (ix86_const_not_ok_for_debug_p): Revert
>   2018-11-09 changes.
>
>   * gcc.dg/debug/dwarf2/pr88635.c: New test.
>
>--- gcc/dwarf2out.c.jj 2019-01-03 12:04:45.720730572 +0100
>+++ gcc/dwarf2out.c2019-01-03 14:35:02.897782400 +0100
>@@ -14464,6 +14464,41 @@ const_ok_for_output_1 (rtx rtl)
> case NOT:
> case NEG:
>   return false;
>+case PLUS:
>+  {
>+  /* Make sure SYMBOL_REFs/UNSPECs are at most in one of the
>+ operands.  */
>+  subrtx_var_iterator::array_type array;
>+  bool first = false;
>+  FOR_EACH_SUBRTX_VAR (iter, array, XEXP (rtl, 0), ALL)
>+if (SYMBOL_REF_P (*iter)
>+|| LABEL_P (*iter)
>+|| GET_CODE (*iter) == UNSPEC)
>+  {
>+first = true;
>+break;
>+  }
>+  if (!first)
>+return true;
>+  FOR_EACH_SUBRTX_VAR (iter, array, XEXP (rtl, 1), ALL)
>+if (SYMBOL_REF_P (*iter)
>+|| LABEL_P (*iter)
>+|| GET_CODE (*iter) == UNSPEC)
>+  return false;
>+  return true;
>+  }
>+case MINUS:
>+  {
>+  /* Disallow negation of SYMBOL_REFs or UNSPECs when they
>+ appear in the second operand of MINUS.  */
>+  subrtx_var_iterator::array_type array;
>+  FOR_EACH_SUBRTX_VAR (iter, array, XEXP (rtl, 1), ALL)
>+if (SYMBOL_REF_P (*iter)
>+|| LABEL_P (*iter)
>+|| GET_CODE (*iter) == UNSPEC)
>+  return false;
>+  return true;
>+  }
> default:
>   return true;
> }
>@@ -15607,6 +15642,7 @@ mem_loc_descriptor (rtx rtl, machine_mod
>pool.  */
> case CONST:
> case SYMBOL_REF:
>+case UNSPEC:
>   if (!is_a  (mode, &int_mode)
> || (GET_MODE_SIZE (int_mode) > DWARF2_ADDR_SIZE
> #ifdef POINTERS_EXTEND_UNSIGNED
>@@ -15614,6 +15650,30 @@ mem_loc_descriptor (rtx rtl, machine_mod
> #endif
> ))
>   break;
>+
>+  if (GET_CODE (rtl) == UNSPEC)
>+  {
>+/* If delegitimize_address couldn't do anything with the UNSPEC, we
>+   can't express it in the debug info.  This can happen e.g. with
>some
>+   TLS UNSPECs.  Allow UNSPECs formerly from CONST that the backend
>+   approves.  */
>+bool not_ok = false;
>+subrtx_var_iterator::array_type array;
>+FOR_EACH_SUBRTX_VAR (iter, array, rtl, ALL)
>+  if ((*iter != rtl && !CONSTANT_P (*iter))
>+  || !const_ok_for_output_1 (*iter))
>+{
>+  not_ok = true;
>+  break;
>+}
>+
>+if (not_ok)
>+  break;
>+
>+rtl = gen_rtx_CONST (GET_MODE (rtl), rtl);
>+goto symref;
>+  }
>+
>   if (G

Re: [PATCH] Add a two value VR comparison to two consecutive PHI args optimization (PR tree-optimization/88676)

2019-01-05 Thread Richard Biener
On January 4, 2019 11:49:45 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>The following patch adds an optimization, if we know certain SSA_NAME
>has two possible values and have GIMPLE_COND EQ_EXPR/NE_EXPR of that
>SSA_NAME with one of the two values and PHI which has two adjacent
>constants, we can optimize it into addition or subtraction.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
>The pr15826.c change is just a small change on what is present in
>GIMPLE
>with this patch, previously (from the bugzilla apparently only on some
>targets) we had a (_Bool) cast and now we have & 1, which effectively
>results in exactly the same expansion.
>
>2019-01-04  Jakub Jelinek  
>
>   PR tree-optimization/88676
>   * tree-ssa-phiopt.c (two_value_replacement): New function.
>   (tree_ssa_phiopt_worker): Call it.
>
>   * gcc.dg/tree-ssa/pr88676.c: New test.
>   * gcc.dg/pr88676.c: New test.
>   * gcc.dg/tree-ssa/pr15826.c: Just verify there is no goto,
>   allow &.
>
>--- gcc/tree-ssa-phiopt.c.jj   2019-01-01 12:37:17.716965779 +0100
>+++ gcc/tree-ssa-phiopt.c  2019-01-04 13:55:26.293746657 +0100
>@@ -48,6 +48,8 @@ along with GCC; see the file COPYING3.
> #include "case-cfn-macros.h"
> 
> static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
>+static bool two_value_replacement (basic_block, basic_block, edge,
>gphi *,
>+ tree, tree);
> static bool conditional_replacement (basic_block, basic_block,
>edge, edge, gphi *, tree, tree);
>static gphi *factor_out_conditional_conversion (edge, edge, gphi *,
>tree, tree,
>@@ -332,8 +334,11 @@ tree_ssa_phiopt_worker (bool do_store_el
>   }
> 
> /* Do the replacement of conditional if it can be done.  */
>-if (!early_p
>-&& conditional_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
>+if (two_value_replacement (bb, bb1, e2, phi, arg0, arg1))
>+  cfgchanged = true;
>+else if (!early_p
>+ && conditional_replacement (bb, bb1, e1, e2, phi,
>+ arg0, arg1))
>   cfgchanged = true;
> else if (abs_replacement (bb, bb1, e1, e2, phi, arg0, arg1))
>   cfgchanged = true;
>@@ -572,6 +577,118 @@ factor_out_conditional_conversion (edge
>   return newphi;
> }
> 
>+/* Optimize
>+   # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
>+   if (x_5 op cstN) # where op is == or != and N is 1 or 2
>+ goto bb3;
>+   else
>+ goto bb4;
>+   bb3:
>+   bb4:
>+   # r_6 = PHI # where cst3 == cst4 + 1 or cst4 ==
>cst3 + 1
>+
>+   to r_6 = x_5 + (min (cst3, cst4) - cst1) or
>+   r_6 = (min (cst3, cst4) + cst1) - x_5 depending on op, N and which
>+   of cst3 and cst4 is smaller.  */
>+
>+static bool
>+two_value_replacement (basic_block cond_bb, basic_block middle_bb,
>+ edge e1, gphi *phi, tree arg0, tree arg1)
>+{
>+  /* Only look for adjacent integer constants.  */
>+  if (!INTEGRAL_TYPE_P (TREE_TYPE (arg0))
>+  || !INTEGRAL_TYPE_P (TREE_TYPE (arg1))
>+  || TREE_CODE (arg0) != INTEGER_CST
>+  || TREE_CODE (arg1) != INTEGER_CST
>+  || (tree_int_cst_lt (arg0, arg1)
>+? wi::to_widest (arg0) + 1 != wi::to_widest (arg1)
>+: wi::to_widest (arg1) + 1 != wi::to_widest (arg1)))
>+return false;
>+
>+  if (!empty_block_p (middle_bb))
>+return false;
>+
>+  gimple *stmt = last_stmt (cond_bb);
>+  tree lhs = gimple_cond_lhs (stmt);
>+  tree rhs = gimple_cond_rhs (stmt);
>+
>+  if (TREE_CODE (lhs) != SSA_NAME
>+  || !INTEGRAL_TYPE_P (TREE_TYPE (lhs))
>+  || TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE
>+  || TREE_CODE (rhs) != INTEGER_CST)
>+return false;
>+
>+  switch (gimple_cond_code (stmt))
>+{
>+case EQ_EXPR:
>+case NE_EXPR:
>+  break;
>+default:
>+  return false;
>+}
>+
>+  wide_int min, max;
>+  if (get_range_info (lhs, &min, &max) != VR_RANGE
>+  || min + 1 != max
>+  || (wi::to_wide (rhs) != min
>+&& wi::to_wide (rhs) != max))
>+return false;
>+
>+  /* We need to know which is the true edge and which is the false
>+ edge so that we know when to invert the condition below.  */
>+  edge true_edge, false_edge;
>+  extract_true_false_edges_from_block (cond_bb, &true_edge,
>&false_edge);
>+  if ((gimple_cond_code (stmt) == EQ_EXPR)
>+  ^ (wi::to_wide (rhs) == max)
>+  ^ (e1 == false_edge))
>+std::swap (arg0, arg1);
>+
>+  tree type;
>+  if (TYPE_PRECISION (TREE_TYPE (lhs)) == TYPE_PRECISION (TREE_TYPE
>(arg0)))
>+{
>+  if (TREE_CODE (TREE_TYPE (arg0)) == BOOLEAN_TYPE
>+|| !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)))
>+  type = TREE_TYPE (lhs);
>+  else
>+  type = TREE_TYPE (arg0);

This misses some comments. Presumably you want to avoid arithmetic with bool 
types (or generally non-mode precision types!?).

>+}
>+  else if (TYPE_PRECISION (TREE_TYPE (lhs)) > TYPE_PRECISION
>(