Re: [PATCH] PR71482: Add -Wglobal-constructors

[Sean Gillespie]

On Fri, May 31, 2019 at 9:30 AM Martin Sebor  wrote:
>
> On 5/31/19 10:12 AM, Sean Gillespie wrote:
> > On Thu, May 30, 2019 at 4:28 PM Martin Sebor  wrote:
> >>
> >> On 5/28/19 3:30 PM, Sean Gillespie wrote:
> >>> This adds a new warning, -Wglobal-constructors, that warns whenever a
> >>> decl requires a global constructor or destructor. This new warning fires
> >>> whenever a thread_local or global variable is declared whose type has a
> >>> non-trivial constructor or destructor. When faced with both a constructor
> >>> and a destructor, the error message mentions the destructor and is only
> >>> fired once.
> >>>
> >>> This warning mirrors the Clang option -Wglobal-constructors, which warns
> >>> on the same thing. -Wglobal-constructors was present in Apple's GCC and
> >>> later made its way into Clang.
> >>>
> >>> Bootstrapped and regression-tested on x86-64 linux, new tests passing.
> >>
> >> I can't tell from the Clang online manual:
> >>
> >> Is the warning meant to trigger just for globals, or for all
> >> objects with static storage duration regardless of scope (i.e.,
> >> including namespace-scope objects and static locals and static
> >> class members)?
> >>
> >> "Requires a constructor to initialize" doesn't seem very clear
> >> to me.  Is the warning intended to trigger for objects that
> >> require dynamic initialization?  If so, then what about dynamic
> >> intialization of objects of trivial types, such as this:
> >>
> >> static int i = std::string ("foo").length ();
> >>
> >> or even
> >>
> >> static int j = strlen (getenv ("TMP"));
> >>
> >> If these aren't meant to be diagnosed then the description should
> >> make it clear (the first one involves a ctor, the second one does
> >> not).  But I would think that if the goal is to find sources of
> >> dynamic initialization then diagnosing the above would be useful.
> >> If so, the description should make it clear and tests verifying
> >> that it works should be added.
> >>
> >> Martin
> >
> > The answer to both of those is yes. Clang warns on both of these with
> > -Wglobal-constructors because the goal of -Wglobal-constructors is to
> > catch and warn against potential life-before-main behavior.
> >
> > Based on Marek's feedback I'm about to push another patch that also
> > warns on your above two examples and adds tests for them. The idea
> > is that any C++-level dynamically initialized globals get warned. Constexpr
> > constructors or things that otherwise can be statically initialized
> > are not warned.
>
> That makes sense.
>
> >
> > Regarding the doc description, how about something like this:
> >
> > "Warns whenever an object with static storage duration requires
> > dynamic initialiation,
> > through global constructors or otherwise."
>
> This makes it clearer -- i.e, global constructors refers to the ELF
> concept rather than to the C++ one.  What does the otherwise part
> refer to?  Some non-ELF mechanism?

Ah, I was thinking "otherwise" to mean the C++ sense of constructor,
and not the ELF one. I realize now that it's clearer without "for
otherwise", since then "global constructors" would refer to just the
ELF concept.

>
> >> PS Dynamic initialization can be optimized into static
> >> initialization, even when it involves a user-defined constructor.
> >> If the purpose of the warning is to find objects that are
> >> dynamically initialized in the sense of the C++ language then
> >> implementing it in the front-end is sufficient.  But if the goal
> >> is to detect constructors that will actually run at runtime (i.e.,
> >> have a startup cost and may have dependencies on one another) then
> >> it needs to be implemented much later.
> >>
> >
> > Clang sticks to the C++ language definition of dynamic initialization for 
> > the
> > purposes of this warning and I think we should as well. Without runtime 
> > checks
> > this is a best-effort warning, but it is capable of catching a bunch
> > of the common
> > ways that you can get constructors running on startup, which is the goal.
>
> It might be worth mentioning that in the manual, perhaps along with
> an example showing an instance of the warning that will most likely
> be a true positive vs a false positive.  E.g., something like this:
>
>const char* const tmp = getenv ("TMP");
>
> vs
>
>const int n = strlen ("123");   // initialized statically
>

Will do.

Sean

Re: [PATCH] Clean up non-conforming names

[Thomas Rodgers]

Revising previous version of this patch to pick another missed
uglification.

>From 543d6be586ad8c29ef0ceefd55a249c715bd2480 Mon Sep 17 00:00:00 2001
From: Thomas Rodgers 
Date: Fri, 31 May 2019 13:28:32 -0700
Subject: [PATCH] Clean up non-conforming names

	* include/pstl/algorithm_impl.h (__parallel_set_union_op):
	Uglfiy copy_range1 and copy_range2
	(__pattern_walk2_n): Rename local n to __n
	* include/pstl/parallel_backend_tbb.h (struct __binary_no_op):
	Rename parameter _T to _Tp.
---
 libstdc++-v3/include/pstl/algorithm_impl.h | 18 +-
 .../include/pstl/parallel_backend_tbb.h|  4 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/libstdc++-v3/include/pstl/algorithm_impl.h b/libstdc++-v3/include/pstl/algorithm_impl.h
index 3491e002c94..2216420d1c3 100644
--- a/libstdc++-v3/include/pstl/algorithm_impl.h
+++ b/libstdc++-v3/include/pstl/algorithm_impl.h
@@ -282,7 +282,7 @@ _RandomAccessIterator2
 __pattern_walk2_n(_ExecutionPolicy&& __exec, _RandomAccessIterator1 __first1, _Size __n, _RandomAccessIterator2 __first2,
   _Function __f, _IsVector __is_vector, /*parallel=*/std::true_type)
 {
-return __internal::__pattern_walk2(std::forward<_ExecutionPolicy>(__exec), __first1, __first1 + n, __first2, __f,
+return __internal::__pattern_walk2(std::forward<_ExecutionPolicy>(__exec), __first1, __first1 + __n, __first2, __f,
__is_vector, std::true_type());
 }
 
@@ -2855,22 +2855,22 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _
 const auto __n1 = __last1 - __first1;
 const auto __n2 = __last2 - __first2;
 
-auto copy_range1 = [__is_vector](_ForwardIterator1 __begin, _ForwardIterator1 __end, _OutputIterator __res) {
+auto __copy_range1 = [__is_vector](_ForwardIterator1 __begin, _ForwardIterator1 __end, _OutputIterator __res) {
 return __internal::__brick_copy(__begin, __end, __res, __is_vector);
 };
-auto copy_range2 = [__is_vector](_ForwardIterator2 __begin, _ForwardIterator2 __end, _OutputIterator __res) {
+auto __copy_range2 = [__is_vector](_ForwardIterator2 __begin, _ForwardIterator2 __end, _OutputIterator __res) {
 return __internal::__brick_copy(__begin, __end, __res, __is_vector);
 };
 
 // {1} {}: parallel copying just first sequence
 if (__n2 == 0)
 return __internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), __first1, __last1, __result,
- copy_range1, std::true_type());
+ __copy_range1, std::true_type());
 
 // {} {2}: parallel copying justmake  second sequence
 if (__n1 == 0)
 return __internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), __first2, __last2, __result,
- copy_range2, std::true_type());
+ __copy_range2, std::true_type());
 
 // testing  whether the sequences are intersected
 _ForwardIterator1 __left_bound_seq_1 = std::lower_bound(__first1, __last1, *__first2, __comp);
@@ -2882,7 +2882,7 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _
 std::forward<_ExecutionPolicy>(__exec),
 [=] {
 __internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), __first1, __last1, __result,
-  copy_range1, std::true_type());
+  __copy_range1, std::true_type());
 },
 [=] {
 __internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), __first2, __last2,
@@ -2901,7 +2901,7 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _
 std::forward<_ExecutionPolicy>(__exec),
 [=] {
 __internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), __first2, __last2, __result,
-  copy_range2, std::true_type());
+  __copy_range2, std::true_type());
 },
 [=] {
 __internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), __first1, __last1,
@@ -2920,7 +2920,7 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, _ForwardIterator1 __first1, _
 //do parallel copying of [first1; left_bound_seq_1)
 [=] {
 __internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), __first1, __left_bound_seq_1,
-  __res_or, copy_range1, std::true_type());
+  __res_or, __copy_range1, std::true_type());
 },
 [=, &__result] {
 __result = 

Re: [PATCH,RFC 0/3] Support for CTF in GCC

[Indu Bhagat]

On 05/29/2019 12:15 AM, Richard Biener wrote:

Of course.  We are merely discussing of where the triggering of processing
starts: debug hooks, or something like:

dwarf2out_early_finish() {
...
if (ctf)
  ctf_emit();
}

(and then in addition if the current DWARF info would be the source of CTF
info, or if it'd be whatever the compiler gives you as trees)

The thing is, with debug hooks you'd have to invent a scheme of stacking
hooks on top of each other (because we want to generate DWARF and CTF from
the same compilation).  That seems like a wasted effort when our wish is
for the hooks to go away alltogether.


When the debug hooks go away, the functionality can be folded in. Much like
above, the ctf proposed implementation will do :

ctf_early_global_decl (tree decl)
{
ctf_decl (decl);

real_debug_hooks->early_global_decl (decl);
}

These ctf_* debug hooks wrappers are as lean as shown above.

I do understand now that if debug hooks are destined to go away, all the
implementation which wraps debug hooks (go dump hooks, vms debug hooks,
and now the proposed ctf debug hooks) will need some merging. But to generate
CTF, I think working on type or decl instead of DWARF dies to is a better
implementation because if user wants only CTF, no DWARF trees need to be
created.

This way we keep DWARF and CTF generation independent of each other (as the
user may want either one of these or both).

The user currently can't have both DWARF and STABS either.  That things like
godump uses debug hooks is just (convenient?) abuse.

In the end frontends will not call sth like dwarf2out_decl but maybe
gen_subroutine_die () or gen_template_die ().  So how do you expect
the "wrapping" to work there?

I understand you want CTF for "actually emitted" decls so I propose you
instead hook into the symtab code which would end up calling the
early_global_decl debug hook.  But please don't add new debug hook
users.


OK.

Will I need to tap both the callsites of the early_global_decl () debug hook ? :
  1. symbol_table::finalize_compilation_unit () in cgraphunit.c
  2. rest_of_decl_compilation () in passes.c
Or is the last one for something specific to godump debug hooks and C++ ?

I guess the above will take care of the CTF generation bit. For emission,
something similar should be done because DWARF hooks will not be initialized if
DWARF debuginfo is not requested by the user. So I cannot have the CTF emission
code in the dwarf2out*finish () debug hooks as suggested earlier.

Curious to know how the current debug hook users like dbx debug hooks will be
taken care of in the future design ? Is it just the wrapping/stacking of debug
hooks that's problematic and not the clean instances like dbx debug hooks ?

PR c++/85254

[Ville Voutilainen]

Tested on Linux-PPC64, acked by Jason on irc. Applying to trunk
on Saturday.

2019-06-01  Ville Voutilainen  

gcc/cp

 PR c++/85254
* class.c (fixup_type_variants): Handle CLASSTYPE_FINAL.

testsuite/

 PR c++/85254
* g++.dg/ext/is_final.C: Amend.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index a2585a6..d6ac6ce 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -1907,6 +1907,7 @@ fixup_type_variants (tree t)
 	= TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t);
 
   TYPE_POLYMORPHIC_P (variants) = TYPE_POLYMORPHIC_P (t);
+  CLASSTYPE_FINAL (variants) = CLASSTYPE_FINAL (t);
 
   TYPE_BINFO (variants) = TYPE_BINFO (t);
 
diff --git a/gcc/testsuite/g++.dg/ext/is_final.C b/gcc/testsuite/g++.dg/ext/is_final.C
index b3875ad..20e5d62 100644
--- a/gcc/testsuite/g++.dg/ext/is_final.C
+++ b/gcc/testsuite/g++.dg/ext/is_final.C
@@ -43,3 +43,17 @@ static_assert( __is_final (Ff), "Ff is final" );
 static_assert( __is_final (Ff),   "Ff is final" );
 static_assert( __is_final (Ff),  "Ff is final" );
 
+// PR 85254
+
+template  struct final_trait_wrap{ typedef T type; };
+
+template  struct my_is_final
+{
+  static const bool value = __is_final(typename final_trait_wrap::type);
+};
+
+struct final1 final {};
+template  struct final2 final {};
+
+static_assert( my_is_final::value, "final1 is final" );
+static_assert( my_is_final>::value, "final2 is final" );

Re: Test for C++20 p0858 - ConstexprIterator requirements.

[Ville Voutilainen]

On Sat, 1 Jun 2019 at 01:24, Ed Smith-Rowland via libstdc++
 wrote:
>
> Greetings,
>
> Iterators for  and  are usabe in a constexpr context
> since C++2017.
>
> This just adds a compile test to make sure and check a box for C++20
> p0858 - ConstexprIterator requirements.


Those tests don't use the iterators in a constexpr context. To do
that, maybe do those std::copy operations
in a constexpr function and then initialize a constexpr variable with
the result of a call to that function?

[libstdc++,doc] doc/xml/manual/allocator.xml - move hoard.org back to http

[Gerald Pfeifer]

hoard.org is a bit strange in that https is broken (certificate 
problem), so switch that back to plain http.

Committed.

Gerald

2019-05-31  Gerald Pfeifer  

* doc/xml/manual/allocator.xml: Move hoard.org back to http.

Index: doc/xml/manual/allocator.xml
===
--- doc/xml/manual/allocator.xml(revision 271813)
+++ doc/xml/manual/allocator.xml(working copy)
@@ -500,7 +500,7 @@
   
   
http://www.w3.org/1999/xlink;
- xlink:href="https://www.hoard.org;>
+ xlink:href="http://hoard.org;>
   The Hoard Memory Allocator

   

Test for C++20 p0858 - ConstexprIterator requirements.

[Ed Smith-Rowland via gcc-patches]

Greetings,

Iterators for  and  are usabe in a constexpr context 
since C++2017.


This just adds a compile test to make sure and check a box for C++20 
p0858 - ConstexprIterator requirements.


Ed


2019-06-03  Edward Smith-Rowland  <3dw...@verizon.net>

Test for C++20 p0858 - ConstexprIterator requirements.
* testsuite/21_strings/basic_string_view/requirements/constexpr_iter.cc:
New test.
* testsuite/23_containers/array/requirements/constexpr_iter.cc:
New test.

Index: testsuite/21_strings/basic_string_view/requirements/constexpr_iter.cc
===
--- testsuite/21_strings/basic_string_view/requirements/constexpr_iter.cc   
(nonexistent)
+++ testsuite/21_strings/basic_string_view/requirements/constexpr_iter.cc   
(working copy)
@@ -0,0 +1,33 @@
+// { dg-do compile { target c++2a } }
+//
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+#include 
+#include 
+
+void
+test()
+{
+  constexpr std::string_view hw("Hello, World!");
+  bool ok = 'H' == *hw.begin();
+  auto ch = hw[4];
+  bool cok = 'W' == *(hw.cbegin() + 7);
+
+  std::array a2{{0,0,0,0,0,0,0,0,0,0,0,0,0}};
+  std::copy(hw.begin(), hw.end(), a2.begin());
+}
Index: testsuite/23_containers/array/requirements/constexpr_iter.cc
===
--- testsuite/23_containers/array/requirements/constexpr_iter.cc
(nonexistent)
+++ testsuite/23_containers/array/requirements/constexpr_iter.cc
(working copy)
@@ -0,0 +1,32 @@
+// { dg-do compile { target c++2a } }
+//
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+#include 
+
+void
+test()
+{
+  constexpr std::array a1{{1, 2, 3}};
+  bool ok = 1 == *a1.begin();
+  auto n = a1[0] * a1[1]* a1[2];
+  bool cok = 1 == *a1.cbegin();
+
+  std::array a2{{0, 0, 0}};
+  std::copy(a1.begin(), a1.end(), a2.begin());
+}

Re: [PATCH] Fix alignment option parser (PR90684)

[Jeff Law]

On 5/30/19 10:38 AM, Wilco Dijkstra wrote:
> Fix the alignment option parser to always allow up to 4 alignments.
> Now -falign-functions=16:8:8:8 no longer reports an error.
> 
> OK for commit (and backport to GCC9)?
> 
> ChangeLog:
> 2019-05-30  Wilco Dijkstra  
> 
> 
>   PR driver/90684
>   * gcc/opts.c (parse_and_check_align_values): Allow 4 alignment values.
Based on Martin L's comments in the BZ.  OK for the trunk.
jeff

Re: [Patch] Fix ix86_expand_sse_comi_round (PR Target/89750, PR Target/86444)

[Jeff Law]

On 5/30/19 2:53 AM, Hongtao Liu wrote:
> On Thu, May 30, 2019 at 3:23 AM Jeff Law  wrote:
>> On 5/9/19 10:54 PM, Hongtao Liu wrote:
>>> On Fri, May 10, 2019 at 3:55 AM Jeff Law  wrote:
 On 5/6/19 11:38 PM, Hongtao Liu wrote:
> Hi Uros and GCC:
>   This patch is to fix ix86_expand_sse_comi_round whose implementation
> was not correct.
>   New implentation aligns with _mm_cmp_round_s[sd]_mask.
>
> Bootstrap and regression tests for x86 is fine.
> Ok for trunk?
>
>
> ChangeLog:
> gcc/
>* config/i386/i386-expand.c (ix86_expand_sse_comi_round):
>Modified, original implementation isn't correct.
>
> gcc/testsuite
>* gcc.target/i386/avx512f-vcomisd-2.c: New runtime tests.
>* gcc.target/i386/avx512f-vcomisd-2.c: Likewise.
 So you'll have to bear with me, I'm not really familiar with this code,
 but in the absence of a maintainer I'll try to work through it.


> -- BR, Hongtao
>
>
> 0001-Fix-ix86_expand_sse_comi_round.patch
>
> Index: gcc/ChangeLog
> ===
> --- gcc/ChangeLog (revision 270933)
> +++ gcc/ChangeLog (working copy)
> @@ -1,3 +1,11 @@
> +2019-05-06  H.J. Lu  
> + Hongtao Liu  
> +
> + PR Target/89750
> + PR Target/86444
> + * config/i386/i386-expand.c (ix86_expand_sse_comi_round):
> + Modified, original implementation isn't correct.
> +
>  2019-05-06  Segher Boessenkool  
>
>   * config/rs6000/rs6000.md (FIRST_ALTIVEC_REGNO, LAST_ALTIVEC_REGNO)
> Index: gcc/config/i386/i386-expand.c
> ===
> --- gcc/config/i386/i386-expand.c (revision 270933)
> +++ gcc/config/i386/i386-expand.c (working copy)
> @@ -9853,18 +9853,24 @@
>const struct insn_data_d *insn_p = _data[icode];
>machine_mode mode0 = insn_p->operand[0].mode;
>machine_mode mode1 = insn_p->operand[1].mode;
> -  enum rtx_code comparison = UNEQ;
> -  bool need_ucomi = false;
>
>/* See avxintrin.h for values.  */
> -  enum rtx_code comi_comparisons[32] =
> +  static const enum rtx_code comparisons[32] =
 So I assume the comment refers to the _CMP_* #defines in avxintrin.h?

>>>   Yes.
>  {
> -  UNEQ, GT, GE, UNORDERED, LTGT, UNLE, UNLT, ORDERED, UNEQ, UNLT,
> -  UNLE, LT, LTGT, GE, GT, LT, UNEQ, GT, GE, UNORDERED, LTGT, UNLE,
> -  UNLT, ORDERED, UNEQ, UNLT, UNLE, LT, LTGT, GE, GT, LT
> +  EQ, LT, LE, UNORDERED, NE, UNGE, UNGT, ORDERED,
> +  EQ, UNLT, UNLE, UNORDERED, LTGT, GE, GT, ORDERED,
> +  EQ, LT, LE, UNORDERED, NE, UNGE, UNGT, ORDERED,
> +  EQ, UNLT, UNLE, UNORDERED, LTGT, GE, GT, ORDERED
>  };
 For CMP_EQ_UQ aren't we looking for an unordered comparison, so UNEQ
 seems right, but you're using EQ.  Can you double-check this?  If it's
 wrong, then please make sure we cover this case with a test.

>>> Avx512f-vcomis[sd]-2.c covers all 32 compare predicates.
>>> UNEQ and EQ behave differently when either operand is NAN, besides
>>> they're the same.
>>> Since NAN operands are handled separtely, so EQ/UNEQ makes no
>>> difference, That why this passes cover tests.
>>> I'll correct it.
>> Ah.  Thanks.  FWIW my approach was to walk through the _CMP_* defines in
>> avxintrin.h and map that to what I thought the comparison ought to be.
>> Then I reviewed my result against your patch.  I got a couple wrong, but
>> could easily see my mistake.  The only one I couldn't reconcile was the
>> CMP_EQ_UQ.  Knowing the NaNs are handled separately makes it clear.
>> Thanks gain.
>>
>>
>>


> @@ -9932,11 +10021,37 @@
>  }
>
>emit_insn (pat);
> +
> +  /* XXX: Set CCFPmode and check a different CCmode.  Does it work
> + correctly?  */
> +  if (GET_MODE (set_dst) != mode)
> +set_dst = gen_rtx_REG (mode, REGNO (set_dst));
 This looks worrisome, even without the cryptic comment.  I don't think
 you can just blindly change the mode like that.  Unless you happen to
 know that the only things you test in the new mode were set in precisely
 the same way as the old mode.

>>> Modified as:
>>> +  /* NB: Set CCFPmode and check a different CCmode.  */
>>> +  if (GET_MODE (set_dst) != mode)
>>> +set_dst = gen_rtx_REG (mode, FLAGS_REG);
>> That might actually be worse.  The mode carries semantic information
>> about where to find the various condition codes within the flags
>> register and which condition codes are valid.  The register number
>> determines which (of possibly many) flags registers we are querying.
>>
>> Thus if the mode of SET_DEST is not the same as MODE, then there is a
>> mismatch between the point where the condition codes were set and where
>> we want to use 

Re: [PATCH][RFC] Sanitize equals and hash functions in hash-tables.

[Jeff Law]

On 5/22/19 3:13 AM, Martin Liška wrote:
> On 5/21/19 1:51 PM, Richard Biener wrote:
>> On Tue, May 21, 2019 at 1:02 PM Martin Liška  wrote:
>>>
>>> On 5/21/19 11:38 AM, Richard Biener wrote:
 On Tue, May 21, 2019 at 12:07 AM Jeff Law  wrote:
>
> On 5/13/19 1:41 AM, Martin Liška wrote:
>> On 11/8/18 9:56 AM, Martin Liška wrote:
>>> On 11/7/18 11:23 PM, Jeff Law wrote:
 On 10/30/18 6:28 AM, Martin Liška wrote:
> On 10/30/18 11:03 AM, Jakub Jelinek wrote:
>> On Mon, Oct 29, 2018 at 04:14:21PM +0100, Martin Liška wrote:
>>> +hashtab_chk_error ()
>>> +{
>>> +  fprintf (stderr, "hash table checking failed: "
>>> +   "equal operator returns true for a pair "
>>> +   "of values with a different hash value");
>> BTW, either use internal_error here, or at least if using fprintf
>> terminate with \n, in your recent mail I saw:
>> ...different hash valueduring RTL pass: vartrack
>> ^^
> Sure, fixed in attached patch.
>
> Martin
>
>>> +  gcc_unreachable ();
>>> +}
>>   Jakub
>>
> 0001-Sanitize-equals-and-hash-functions-in-hash-tables.patch
>
> From 0d9c979c845580a98767b83c099053d36eb49bb9 Mon Sep 17 00:00:00 2001
> From: marxin 
> Date: Mon, 29 Oct 2018 09:38:21 +0100
> Subject: [PATCH] Sanitize equals and hash functions in hash-tables.
>
> ---
>  gcc/hash-table.h | 40 +++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/hash-table.h b/gcc/hash-table.h
> index bd83345c7b8..694eedfc4be 100644
> --- a/gcc/hash-table.h
> +++ b/gcc/hash-table.h
> @@ -503,6 +503,7 @@ private:
>
>value_type *alloc_entries (size_t n CXX_MEM_STAT_INFO) const;
>value_type *find_empty_slot_for_expand (hashval_t);
> +  void verify (const compare_type , hashval_t hash);
>bool too_empty_p (unsigned int);
>void expand ();
>static bool is_deleted (value_type )
> @@ -882,8 +883,12 @@ hash_table
>if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
>  expand ();
>
> -  m_searches++;
> +#if ENABLE_EXTRA_CHECKING
> +if (insert == INSERT)
> +  verify (comparable, hash);
> +#endif
>
> +  m_searches++;
>value_type *first_deleted_slot = NULL;
>hashval_t index = hash_table_mod1 (hash, m_size_prime_index);
>hashval_t hash2 = hash_table_mod2 (hash, m_size_prime_index);
> @@ -930,6 +935,39 @@ hash_table
>return _entries[index];
>  }
>
> +#if ENABLE_EXTRA_CHECKING
> +
> +/* Report a hash table checking error.  */
> +
> +ATTRIBUTE_NORETURN ATTRIBUTE_COLD
> +static void
> +hashtab_chk_error ()
> +{
> +  fprintf (stderr, "hash table checking failed: "
> + "equal operator returns true for a pair "
> + "of values with a different hash value\n");
> +  gcc_unreachable ();
> +}
 I think an internal_error here is probably still better than a simple
 fprintf, even if the fprintf is terminated with a \n :-)
>>> Fully agree with that, but I see a lot of build errors when using 
>>> internal_error.
>>>
 The question then becomes can we bootstrap with this stuff enabled and
 if not, are we likely to soon?  It'd be a shame to put it into
 EXTRA_CHECKING, but then not be able to really use EXTRA_CHECKING
 because we've got too many bugs to fix.
>>> Unfortunately it's blocked with these 2 PRs:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87845
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87847
>> Hi.
>>
>> I've just added one more PR:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90450
>>
>> I'm sending updated version of the patch that provides a disablement for 
>> the 3 PRs
>> with a new function disable_sanitize_eq_and_hash.
>>
>> With that I can bootstrap and finish tests. However, I've done that with 
>> a patch
>> limits maximal number of checks:
> So rather than call the disable_sanitize_eq_and_hash, can you have its
> state set up when you instantiate the object?  It's not a huge deal,
> just thinking about loud.
>
>
>
> So how do we want to go forward, particularly the EXTRA_EXTRA checking
> issue :-)

 There is at least one PR where we have a table where elements _in_ the
 table are never compared against each other but always against another
 object (I guess that's usual even), but the setup is in a way that the
 comparison 

[committed] Add OpenMP lastprivate conditional support for simd construct

[Jakub Jelinek]

Hi!

The following patch adds lastprivate(conditional:) support for simd
construct (at least when not combined with worksharing construct, that will
be done incrementally).

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2019-05-31  Jakub Jelinek  

* tree.h (OMP_CLAUSE__CONDTEMP__ITER): Define.
* gimplify.c (gimplify_scan_omp_clauses): Allow lastprivate conditional
on OMP_SIMD if not nested inside of worksharing loop that also has
lastprivate conditional clause for the same decl.
(gimplify_omp_for): Add _condtemp_ clauses to OMP_SIMD if needed.
* omp-low.c (scan_sharing_clauses): Handle OMP_CLAUSE__CONDTEMP_ also
on simd.
(lower_rec_input_clauses): Likewise.  Handle lastprivate conditional
on simd construct.
(lower_lastprivate_conditional_clauses): Handle lastprivate conditional
on simd construct.
(lower_lastprivate_clauses): Likewise.
(lower_omp_sections): Call lower_lastprivate_conditional_clauses before
calling lower_rec_input_clauses.
(lower_omp_for): Likewise.
(lower_omp_1): Use first rather than second OMP_CLAUSE__CONDTEMP_
clause on simd construct.
* omp-expand.c (expand_omp_simd): Initialize cond_var if
OMP_CLAUSE__CONDTEMP_ clause is present.

* c-c++-common/gomp/lastprivate-conditional-2.c (foo): Don't expect
a sorry on lastprivate conditional on simd construct.
* gcc.dg/vect/vect-simd-6.c: New test.
* gcc.dg/vect/vect-simd-7.c: New test.

--- gcc/tree.h.jj   2019-05-29 09:42:27.337398449 +0200
+++ gcc/tree.h  2019-05-31 15:22:18.098541471 +0200
@@ -1752,6 +1752,10 @@ class auto_suppress_location_wrappers
 #define OMP_CLAUSE__GRIDDIM__GROUP(NODE) \
   OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE__GRIDDIM_), 1)
 
+/* _CONDTEMP_ holding temporary with iteration count.  */
+#define OMP_CLAUSE__CONDTEMP__ITER(NODE) \
+  (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE__CONDTEMP_)->base.public_flag)
+
 /* SSA_NAME accessors.  */
 
 /* Whether SSA_NAME NODE is a virtual operand.  This simply caches the
--- gcc/gimplify.c.jj   2019-05-30 23:19:14.464931841 +0200
+++ gcc/gimplify.c  2019-05-31 18:52:21.487672269 +0200
@@ -8146,17 +8146,29 @@ gimplify_scan_omp_clauses (tree *list_p,
}
  if (OMP_CLAUSE_LASTPRIVATE_CONDITIONAL (c))
{
- if (code == OMP_FOR
- || code == OMP_SECTIONS
- || region_type == ORT_COMBINED_PARALLEL)
-   flags |= GOVD_LASTPRIVATE_CONDITIONAL;
- else
+ splay_tree_node n = NULL;
+ if (code == OMP_SIMD
+ && outer_ctx
+ && outer_ctx->region_type == ORT_WORKSHARE)
+   {
+ n = splay_tree_lookup (outer_ctx->variables,
+(splay_tree_key) decl);
+ if (n == NULL
+ && outer_ctx->outer_context
+ && (outer_ctx->outer_context->region_type
+ == ORT_COMBINED_PARALLEL))
+   n = splay_tree_lookup (outer_ctx->outer_context->variables,
+  (splay_tree_key) decl);
+   }
+ if (n && (n->value & GOVD_LASTPRIVATE_CONDITIONAL) != 0)
{
  sorry_at (OMP_CLAUSE_LOCATION (c),
"% modifier on % "
"clause not supported yet");
  OMP_CLAUSE_LASTPRIVATE_CONDITIONAL (c) = 0;
}
+ else
+   flags |= GOVD_LASTPRIVATE_CONDITIONAL;
}
  if (outer_ctx
  && (outer_ctx->region_type == ORT_COMBINED_PARALLEL
@@ -11559,6 +11571,28 @@ gimplify_omp_for (tree *expr_p, gimple_s
  omp_add_variable (ctx, var, GOVD_CONDTEMP | GOVD_SEEN);
}
 }
+  else if (TREE_CODE (orig_for_stmt) == OMP_SIMD)
+{
+  unsigned lastprivate_conditional = 0;
+  for (tree c = gimple_omp_for_clauses (gfor); c; c = OMP_CLAUSE_CHAIN (c))
+   if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LASTPRIVATE
+   && OMP_CLAUSE_LASTPRIVATE_CONDITIONAL (c))
+ ++lastprivate_conditional;
+  if (lastprivate_conditional)
+   {
+ struct omp_for_data fd;
+ omp_extract_for_data (gfor, , NULL);
+ tree type = unsigned_type_for (fd.iter_type);
+ while (lastprivate_conditional--)
+   {
+ tree c = build_omp_clause (UNKNOWN_LOCATION,
+OMP_CLAUSE__CONDTEMP_);
+ OMP_CLAUSE_DECL (c) = create_tmp_var (type);
+ OMP_CLAUSE_CHAIN (c) = gimple_omp_for_clauses (gfor);
+ gimple_omp_for_set_clauses (gfor, c);
+   }
+   }
+}
 
   if (ret != GS_ALL_DONE)
 return GS_ERROR;
--- gcc/omp-low.c.jj2019-05-31 11:52:20.491195088 +0200

Re: [MIPS] Fix for the wrong argument sequence in MSA builtin for FMADD/MADDV family.

[Jeff Law]

On 5/22/19 12:13 AM, Dragan Mladjenovic wrote:
> Hi all,
> 
> This patch makes the behavior of __builtin_msa_ fmadd/fmsub/maddv match the
> expected one. While not a recent regression, it would be nice if this can
> be back-ported in order to minimize the fragmentation.
> 
> Best regards,
> 
> Dragan
> 
> gcc/ChangeLog:
> 
> 2019-05-20  Prachi Godbole  
> Robert Suchanek  
> 
>   * gcc/config/mips/mips.c (mips_expand_builtin_insn): Swap the 1st
>   and 3rd operands of the fmadd/fmsub/maddv builtin.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-05-20  Dragan Mladjenovic  
> 
>   * gcc.target/mips/msa-fmadd.c: New.
THanks.  I've installed this one the trunk.

Out of an abundance of caution I have not installed it on any of the
release branches.  Feel free to ping me in a week or so and if there
haven't been any issues I'll back port it.

jeff

libgo patch committed: Drop unused C type reflection code

[Ian Lance Taylor]

This libgo patch drops unused C type reflection code.  In particular,
it drops __go_type_descriptors_equal, which is no longer used, and
will be made obsolete by https://golang.org/cl/179598.  Bootstrapped
and ran Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 271822)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-52176566485e20968394a5cb67a89ac676182594
+4150db0e4613043e38a146a971e5b0dcacad7c2a
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/Makefile.am
===
--- libgo/Makefile.am   (revision 271818)
+++ libgo/Makefile.am   (working copy)
@@ -469,7 +469,6 @@ runtime_files = \
runtime/go-setenv.c \
runtime/go-signal.c \
runtime/go-strslice.c \
-   runtime/go-typedesc-equal.c \
runtime/go-unsafe-pointer.c \
runtime/go-unsetenv.c \
runtime/go-unwind.c \
Index: libgo/runtime/go-type.h
===
--- libgo/runtime/go-type.h (revision 271669)
+++ libgo/runtime/go-type.h (working copy)
@@ -153,53 +153,6 @@ struct __go_uncommon_type
   struct __go_open_array __methods;
 };
 
-/* The type descriptor for a fixed array type.  */
-
-struct __go_array_type
-{
-  /* Starts like all type descriptors.  */
-  struct __go_type_descriptor __common;
-
-  /* The element type.  */
-  struct __go_type_descriptor *__element_type;
-
-  /* The type of a slice of the same element type.  */
-  struct __go_type_descriptor *__slice_type;
-
-  /* The length of the array.  */
-  uintptr_t __len;
-};
-
-/* The type descriptor for a slice.  */
-
-struct __go_slice_type
-{
-  /* Starts like all other type descriptors.  */
-  struct __go_type_descriptor __common;
-
-  /* The element type.  */
-  struct __go_type_descriptor *__element_type;
-};
-
-/* The direction of a channel.  */
-#define CHANNEL_RECV_DIR 1
-#define CHANNEL_SEND_DIR 2
-#define CHANNEL_BOTH_DIR (CHANNEL_RECV_DIR | CHANNEL_SEND_DIR)
-
-/* The type descriptor for a channel.  */
-
-struct __go_channel_type
-{
-  /* Starts like all other type descriptors.  */
-  struct __go_type_descriptor __common;
-
-  /* The element type.  */
-  const struct __go_type_descriptor *__element_type;
-
-  /* The direction.  */
-  uintptr_t __dir;
-};
-
 /* The type descriptor for a function.  */
 
 struct __go_func_type
@@ -221,34 +174,6 @@ struct __go_func_type
   struct __go_open_array __out;
 };
 
-/* A method on an interface type.  */
-
-struct __go_interface_method
-{
-  /* The name of the method.  */
-  const struct String *__name;
-
-  /* This is NULL for an exported method, or the name of the package
- where it lives.  */
-  const struct String *__pkg_path;
-
-  /* The real type of the method.  */
-  struct __go_type_descriptor *__type;
-};
-
-/* An interface type.  */
-
-struct __go_interface_type
-{
-  /* Starts like all other type descriptors.  */
-  struct __go_type_descriptor __common;
-
-  /* Array of __go_interface_method .  The methods are sorted in the
- same order that they appear in the definition of the
- interface.  */
-  struct __go_open_array __methods;
-};
-
 /* A map type.  */
 
 struct __go_map_type
@@ -301,69 +226,4 @@ struct __go_ptr_type
   const struct __go_type_descriptor *__element_type;
 };
 
-/* A field in a structure.  */
-
-struct __go_struct_field
-{
-  /* The name of the field--NULL for an anonymous field.  */
-  const struct String *__name;
-
-  /* This is NULL for an exported method, or the name of the package
- where it lives.  */
-  const struct String *__pkg_path;
-
-  /* The type of the field.  */
-  const struct __go_type_descriptor *__type;
-
-  /* The field tag, or NULL.  */
-  const struct String *__tag;
-
-  /* The offset of the field in the struct.  */
-  uintptr_t __offset;
-};
-
-/* A struct type.  */
-
-struct __go_struct_type
-{
-  /* Starts like all other type descriptors.  */
-  struct __go_type_descriptor __common;
-
-  /* An array of struct __go_struct_field.  */
-  struct __go_open_array __fields;
-};
-
-/* Whether a type descriptor is a pointer.  */
-
-static inline _Bool
-__go_is_pointer_type (const struct __go_type_descriptor *td)
-{
-  return ((td->__code & GO_CODE_MASK) == GO_PTR
- || (td->__code & GO_CODE_MASK) == GO_UNSAFE_POINTER);
-}
-
-/* Call a type hash function, given the __hashfn value.  */
-
-static inline uintptr_t
-__go_call_hashfn (const FuncVal *hashfn, const void *p, uintptr_t seed,
- uintptr_t size)
-{
-  uintptr_t (*h) (const void *, uintptr_t, uintptr_t) = (void *) hashfn->fn;
-  return __builtin_call_with_static_chain (h (p, seed, size), hashfn);
-}
-
-/* Call a type equality function, given the __equalfn value.  */
-
-static inline _Bool

Re: [PATCH v2] Fix PR64242

[Jeff Law]

On 5/28/19 11:26 AM, Wilco Dijkstra wrote:
> Improve the fix for PR64242.  Various optimizations can change a memory 
> reference
> into a frame access.  Given there are multiple virtual frame pointers which 
> may
> be replaced by multiple hard frame pointers, there are no checks for writes 
> to the
> various frame pointers.  So updates to a frame pointer tends to generate 
> incorrect
> code.  Improve the previous fix to also add clobbers of several frame 
> pointers and
> add a scheduling barrier.  This should work in most cases until GCC supports a
> generic "don't optimize across this instruction" feature.
> 
> Bootstrap OK. Testcase passes on AArch64 and x86-64.  Inspected x86, Arm,
> Thumb-1 and Thumb-2 assembler which looks correct. 
> 
> ChangeLog:
> 2018-12-07  Wilco Dijkstra  
> 
> gcc/
>   PR middle-end/64242
>   * builtins.c (expand_builtin_longjmp): Add frame clobbers and schedule 
> block.
>   (expand_builtin_nonlocal_goto): Likewise.
> 
> testsuite/
>   PR middle-end/64242
>   * gcc.c-torture/execute/pr64242.c: Update test.
OK.  Though given history we might expect some targets to barf on the
test changes.  Please keep an eye out for such breakage.

jeff

Go patch committed: Optimize append of make

[Ian Lance Taylor]

This patch to the Go frontend by Cherry Zhang optimizes calling append
with a second argument of a call to make.  The gc compiler recognizes
append(s, make([]T, n)...), and generates code to directly zero the
tail instead of allocating a new slice and copying.  This patch lets
the Go frontend do basically the same.

The difficulty is that at the point we handle append, there may
already be temporaries introduced (e.g. in order_evaluations), which
makes it hard to find the append-of-make pattern.  The compiler could
"see through" the value of a temporary, but it is only safe to do if
the temporary is not assigned multiple times.  For this, we add
tracking of assignments and uses for temporaries.

This also helps in optimizing non-escape slice make.  We already
optimize non-escape slice make with constant len/cap to stack
allocation.  But it failed to handle things like f(make([]T, n))
(where the slice doesn't escape and n is constant), because of the
temporary.  With tracking of temporary assignments and uses, it can
handle this now as well.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 271821)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-e521123b23494148d534755e2f3d7806b42c96ad
+52176566485e20968394a5cb67a89ac676182594
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 271821)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -1052,6 +1052,7 @@ Temporary_reference_expression*
 Expression::make_temporary_reference(Temporary_statement* statement,
 Location location)
 {
+  statement->add_use();
   return new Temporary_reference_expression(statement, location);
 }
 
@@ -8286,23 +8287,87 @@ Builtin_call_expression::flatten_append(
   Temporary_statement* l2tmp = NULL;
   Expression_list* add = NULL;
   Expression* len2;
+  Call_expression* makecall = NULL;
   if (this->is_varargs())
 {
   go_assert(args->size() == 2);
 
-  // s2tmp := s2
-  s2tmp = Statement::make_temporary(NULL, args->back(), loc);
-  inserter->insert(s2tmp);
-
-  // l2tmp := len(s2tmp)
-  lenref = Expression::make_func_reference(lenfn, NULL, loc);
-  call_args = new Expression_list();
-  call_args->push_back(Expression::make_temporary_reference(s2tmp, loc));
-  len = Expression::make_call(lenref, call_args, false, loc);
-  gogo->lower_expression(function, inserter, );
-  gogo->flatten_expression(function, inserter, );
-  l2tmp = Statement::make_temporary(int_type, len, loc);
-  inserter->insert(l2tmp);
+  std::pair p =
+Expression::find_makeslice_call(args->back());
+  makecall = p.first;
+  if (makecall != NULL)
+{
+  // We are handling
+  //   append(s, make([]T, len[, cap])...))
+  // which has already been lowered to
+  //   append(s, runtime.makeslice(T, len, cap)).
+  // We will optimize this to directly zeroing the tail,
+  // instead of allocating a new slice then copy.
+
+  // Retrieve the length. Cannot reference s2 as we will remove
+  // the makeslice call.
+  Expression* len_arg = makecall->args()->at(1);
+  len_arg = Expression::make_cast(int_type, len_arg, loc);
+  l2tmp = Statement::make_temporary(int_type, len_arg, loc);
+  inserter->insert(l2tmp);
+
+  Expression* cap_arg = makecall->args()->at(2);
+  cap_arg = Expression::make_cast(int_type, cap_arg, loc);
+  Temporary_statement* c2tmp =
+Statement::make_temporary(int_type, cap_arg, loc);
+  inserter->insert(c2tmp);
+
+  // Check bad len/cap here.
+  // if len2 < 0 { panicmakeslicelen(); }
+  len2 = Expression::make_temporary_reference(l2tmp, loc);
+  Expression* zero = Expression::make_integer_ul(0, int_type, loc);
+  Expression* cond = Expression::make_binary(OPERATOR_LT, len2,
+ zero, loc);
+  Expression* arg =
+
Expression::make_integer_ul(RUNTIME_ERROR_MAKE_SLICE_LEN_OUT_OF_BOUNDS,
+NULL, loc);
+  Expression* call = Runtime::make_call(Runtime::RUNTIME_ERROR,
+loc, 1, arg);
+  cond = Expression::make_conditional(cond, call, zero->copy(), loc);
+  gogo->lower_expression(function, inserter, );
+  gogo->flatten_expression(function, inserter, );
+  Statement* s = Statement::make_statement(cond, false);
+  inserter->insert(s);
+
+  // if cap2 

Re: [PATCH] correct the representation of ADDR_EXPR involving pointer to array [PR 90694]

[Jeff Law]

On 5/31/19 1:56 PM, Martin Sebor wrote:
> Given a poiner to array p, tree dumps for expressions like &(*p)[2]
> actually show &*p[2].  That's not right -- the parentheses are
> important to differentiate indexing into the array the pointer
> points to from indexing into the pointer.
> 
> The attached patch adjusts the tree pretty printer to add the parens
> when the pointer points to an array.
> 
> Tested on x86_64-linux.
> 
> Martin
> 
> gcc-90694.diff
> 
> PR middle-end/90694 - incorrect representation of ADDR_EXPR involving a 
> pointer to array
> 
> gcc/ChangeLog:
> 
>   PR middle-end/90694
>   * tree-pretty-print.c (dump_generic_node): Add parentheses.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR middle-end/90694
>   * gcc.dg/tree-ssa/dump-5.c: New test.
OK.  I'm going to assume that the gimple parser already does the right
thing since it's supposed to already handle C expressions correctly.

Jeff

Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Jeff Law]

On 5/22/19 3:34 PM, Martin Sebor wrote:
> -Wreturn-local-addr detects a subset of instances of returning
> the address of a local object from a function but the warning
> doesn't try to handle alloca or VLAs, or some non-trivial cases
> of ordinary automatic variables[1].
> 
> The attached patch extends the implementation of the warning to
> detect those.  It still doesn't detect instances where the address
> is the result of a built-in such strcpy[2].
> 
> Tested on x86_64-linux.
> 
> Martin
> 
> [1] For example, this is only diagnosed with the patch:
> 
>   void* f (int i)
>   {
> struct S { int a[2]; } s[2];
> return >a[i];
>   }
> 
> [2] The following is not diagnosed even with the patch:
> 
>   void sink (void*);
> 
>   void* f (int i)
>   {
> char a[6];
> char *p = __builtin_strcpy (a, "123");
> sink (p);
> return p;
>   }
> 
> I would expect detecting to be possible and useful.  Maybe as
> a follow-up.
> 
> gcc-71924.diff
> 
> PR middle-end/71924 - missing -Wreturn-local-addr returning alloca result
> PR middle-end/90549 - missing -Wreturn-local-addr maybe returning an address 
> of a local array plus offset
> 
> gcc/ChangeLog:
> 
>   PR c/71924
>   * gimple-ssa-isolate-paths.c (is_addr_local): New function.
>   (warn_return_addr_local_phi_arg, warn_return_addr_local): Same.
>   (find_implicit_erroneous_behavior): Call warn_return_addr_local_phi_arg.
>   (find_explicit_erroneous_behavior): Call warn_return_addr_local.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c/71924
>   * gcc.dg/Wreturn-local-addr-2.c: New test.
>   * gcc.dg/Walloca-4.c: Prune expected warnings.
>   * gcc.dg/pr41551.c: Same.
>   * gcc.dg/pr59523.c: Same.
>   * gcc.dg/tree-ssa/pr88775-2.c: Same.
>   * gcc.dg/winline-7.c: Same.
> 
> diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c
> index 33fe352bb23..2933ecf502e 100644
> --- a/gcc/gimple-ssa-isolate-paths.c
> +++ b/gcc/gimple-ssa-isolate-paths.c
> @@ -341,6 +341,135 @@ stmt_uses_0_or_null_in_undefined_way (gimple *stmt)
>return false;
>  }
>  
> +/* Return true if EXPR is a expression of pointer type that refers
> +   to the address of a variable with automatic storage duration.
> +   If so, set *PLOC to the location of the object or the call that
> +   allocated it (for alloca and VLAs).  When PMAYBE is non-null,
> +   also consider PHI statements and set *PMAYBE when some but not
> +   all arguments of such statements refer to local variables, and
> +   to clear it otherwise.  */
> +
> +static bool
> +is_addr_local (tree exp, location_t *ploc, bool *pmaybe = NULL,
> +hash_set *visited = NULL)
> +{
> +  if (TREE_CODE (exp) == SSA_NAME)
> +{
> +  gimple *def_stmt = SSA_NAME_DEF_STMT (exp);
> +  enum gimple_code code = gimple_code (def_stmt);
> +
> +  if (is_gimple_assign (def_stmt))
> + {
> +   tree type = TREE_TYPE (gimple_assign_lhs (def_stmt));
> +   if (POINTER_TYPE_P (type))
> + {
> +   tree ptr = gimple_assign_rhs1 (def_stmt);
> +   return is_addr_local (ptr, ploc, pmaybe, visited);
> + }
> +   return false;
> + }
So this is going to recurse on the rhs1 of something like
POINTER_PLUS_EXPR, that's a good thing :-)   But isn't it non-selective
about the codes where we recurse?

Consider

  ptr = (cond) ? res1 : res2

I think we'll end up recursing on the condition rather than looking at
res1 and res2.


I suspect there are a very limited number of expression codes that
appear on the RHS where we'd want to recurse on one or both operands.

POINTER_PLUS_EXPR, NOP_EXPR, maybe COND_EXPR (where you have to recurse
on both and logically and the result), BIT_AND (maybe we masked off some
bits in an address).  That's probably about it :-)

Are there any other codes you've seen or think would be useful in
practice to recurse through?  I'd rather list them explicitly rather
than just recurse down through every rhs1 we encounter.



> +
> +  if (code == GIMPLE_PHI && pmaybe)
> + {
> +   unsigned count = 0;
> +   gphi *phi_stmt = as_a  (def_stmt);
> +
> +   unsigned nargs = gimple_phi_num_args (phi_stmt);
> +   for (unsigned i = 0; i < nargs; ++i)
> + {
> +   if (!visited->add (phi_stmt))
> + {
> +   tree arg = gimple_phi_arg_def (phi_stmt, i);
> +   if (is_addr_local (arg, ploc, pmaybe, visited))
> + ++count;
> + }
> + }
So imagine

p = phi (x1, x1, x2)

Where both x1 and x2 would pass is_addr_local.  I think this code would
incorrectly set pmaybe.

We process the first instance of x1, find it is local and bump count.
When we encounter the second instance, it's in our map and we do
nothing.  THen we process x2 and bump count again.  So count would be 2.
 But count is going to be less than nargs so we'll set *pmaybe to true.

ISTM you'd have to record the result for each phi argument and query
that to 

[PATCH] Clean up non-conforming names

[Thomas Rodgers]

* include/pstl/algorithm_impl.h (__parallel_set_union_op):
Uglfiy copy_range1 and copy_range2
* include/pstl/parallel_backend_tbb.h (struct __binary_no_op):
Rename parameter _T to _Tp.
---
 libstdc++-v3/include/pstl/algorithm_impl.h   | 16 
 libstdc++-v3/include/pstl/parallel_backend_tbb.h |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/libstdc++-v3/include/pstl/algorithm_impl.h 
b/libstdc++-v3/include/pstl/algorithm_impl.h
index 3491e002c94..a224b2b3633 100644
--- a/libstdc++-v3/include/pstl/algorithm_impl.h
+++ b/libstdc++-v3/include/pstl/algorithm_impl.h
@@ -2855,22 +2855,22 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, 
_ForwardIterator1 __first1, _
 const auto __n1 = __last1 - __first1;
 const auto __n2 = __last2 - __first2;
 
-auto copy_range1 = [__is_vector](_ForwardIterator1 __begin, 
_ForwardIterator1 __end, _OutputIterator __res) {
+auto __copy_range1 = [__is_vector](_ForwardIterator1 __begin, 
_ForwardIterator1 __end, _OutputIterator __res) {
 return __internal::__brick_copy(__begin, __end, __res, __is_vector);
 };
-auto copy_range2 = [__is_vector](_ForwardIterator2 __begin, 
_ForwardIterator2 __end, _OutputIterator __res) {
+auto __copy_range2 = [__is_vector](_ForwardIterator2 __begin, 
_ForwardIterator2 __end, _OutputIterator __res) {
 return __internal::__brick_copy(__begin, __end, __res, __is_vector);
 };
 
 // {1} {}: parallel copying just first sequence
 if (__n2 == 0)
 return 
__internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), 
__first1, __last1, __result,
- copy_range1, 
std::true_type());
+ __copy_range1, 
std::true_type());
 
 // {} {2}: parallel copying justmake  second sequence
 if (__n1 == 0)
 return 
__internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), 
__first2, __last2, __result,
- copy_range2, 
std::true_type());
+ __copy_range2, 
std::true_type());
 
 // testing  whether the sequences are intersected
 _ForwardIterator1 __left_bound_seq_1 = std::lower_bound(__first1, __last1, 
*__first2, __comp);
@@ -2882,7 +2882,7 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, 
_ForwardIterator1 __first1, _
 std::forward<_ExecutionPolicy>(__exec),
 [=] {
 
__internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), 
__first1, __last1, __result,
-  copy_range1, 
std::true_type());
+  __copy_range1, 
std::true_type());
 },
 [=] {
 
__internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), 
__first2, __last2,
@@ -2901,7 +2901,7 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, 
_ForwardIterator1 __first1, _
 std::forward<_ExecutionPolicy>(__exec),
 [=] {
 
__internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), 
__first2, __last2, __result,
-  copy_range2, 
std::true_type());
+  __copy_range2, 
std::true_type());
 },
 [=] {
 
__internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), 
__first1, __last1,
@@ -2920,7 +2920,7 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, 
_ForwardIterator1 __first1, _
 //do parallel copying of [first1; left_bound_seq_1)
 [=] {
 
__internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), 
__first1, __left_bound_seq_1,
-  __res_or, copy_range1, 
std::true_type());
+  __res_or, __copy_range1, 
std::true_type());
 },
 [=, &__result] {
 __result = __internal::__parallel_set_op(
@@ -2942,7 +2942,7 @@ __parallel_set_union_op(_ExecutionPolicy&& __exec, 
_ForwardIterator1 __first1, _
 //do parallel copying of [first2; left_bound_seq_2)
 [=] {
 
__internal::__pattern_walk2_brick(std::forward<_ExecutionPolicy>(__exec), 
__first2, __left_bound_seq_2,
-  __res_or, copy_range2, 
std::true_type());
+  __res_or, __copy_range2, 
std::true_type());
 },
 [=, &__result] {
 __result = __internal::__parallel_set_op(
diff --git a/libstdc++-v3/include/pstl/parallel_backend_tbb.h 
b/libstdc++-v3/include/pstl/parallel_backend_tbb.h
index c6f5bb8d5ab..9c05ade0532 100644
--- 

Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers

[Alex Henrie]

On Fri, May 31, 2019 at 1:38 AM Richard Biener  wrote:
>
> On Thu, 30 May 2019, Alex Henrie wrote:
>
> > In Wine we need a way to (without warnings) put ms_hook_prologue into
> > a macro that is applied to functions, function pointers, and function
> > pointer typedefs. It sounds like you're saying that you will not
> > accept a patch that silences or splits off warnings about using
> > ms_hook_prologue with function pointers and function pointer typedefs.
> > So how do you think Wine's problem should be solved?
>
> I think ms_hook_prologue should be allowed to apply to function types
> and function decls.  If you say it should apply to function pointers
> then I suppose you want to have it apply to the pointed to function
> of the function pointer - but that's not possible without an indirection
> via a function pointer typedef IIRC.

No, if ms_hook_prologue is applied to a function pointer, it shouldn't
do anything except maybe trigger some optimization of the code around
the indirect function call.

> I also have the following which _may_ motivate that attributes
> currently not applying to function types (because they only
> affect function definitions) should also apply there:
>
> typedef int  (myfun)  (int *) __attribute__((nonnull(1)));
> myfun x;
> int x(int *p) { return p != (int*)0; }
>
> this applies nonnull to the function definition of 'x' but
> I put the attribute on the typedef.  I didn't manage to
> do without the myfun x; declaration.

That is a great example and another compelling reason to allow
"fndecl" attributes in more places.

> > It seems to me that any information about the target of a function
> > pointer, even the flatten attribute or the ms_hook_prologue attribute,
> > provides information that could be useful for optimizing the code
> > around the indirect function call. That sounds like a compelling
> > argument for allowing these attributes in more places without
> > warnings.
>
> Sure.  Can you write down the three cases after macro expansion
> here to clarify what you need?  Esp. say what the attribute should
> apply to.  Just silencing the warning without actually achieving
> what you want would be bad I think ;)

Essentially, the following needs to compile without warnings:

#define WINAPI __attribute__((__stdcall__)) \
   __attribute__((__ms_hook_prologue__))

typedef unsigned int (WINAPI *APPLICATION_RECOVERY_CALLBACK)(void*);

void WINAPI foo()
{
APPLICATION_RECOVERY_CALLBACK bar;
unsigned int (WINAPI *baz)(void*);
}

-Alex

Go patch committed: Fix int-to-string conversion with large constant

[Ian Lance Taylor]

This patch to the Go frontend by Cherry Zhang fixes int-to-string
conversion with large integer constants.

Currently, the type conversion code thinks the int-to-string
conversion is constant if the integer operand is constant, but it
actually generates a call to runtime.intstring if the integer does not
fit in a "ushort", which makes it not suitable in constant context,
such as static initializer.

This patch makes it handle all constant integer input as constants,
generating a constant string.

This fixes https://golang.org/issue/32347.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

I am unable to attach the actual patch, as GMail rejects it for some
unknown reason.  To see the patch, please see
https://golang.org/cl/179777 or
https://gcc.gnu.org/viewcvs/gcc?view=revision=271821

Ian

[PATCH] correct the representation of ADDR_EXPR involving pointer to array [PR 90694]

[Martin Sebor]

Given a poiner to array p, tree dumps for expressions like &(*p)[2]
actually show &*p[2].  That's not right -- the parentheses are
important to differentiate indexing into the array the pointer
points to from indexing into the pointer.

The attached patch adjusts the tree pretty printer to add the parens
when the pointer points to an array.

Tested on x86_64-linux.

Martin
PR middle-end/90694 - incorrect representation of ADDR_EXPR involving a pointer to array

gcc/ChangeLog:

	PR middle-end/90694
	* tree-pretty-print.c (dump_generic_node): Add parentheses.

gcc/testsuite/ChangeLog:

	PR middle-end/90694
	* gcc.dg/tree-ssa/dump-5.c: New test.

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/dump-5.c b/gcc/testsuite/gcc.dg/tree-ssa/dump-5.c
new file mode 100644
index 000..6807b5e9ef4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/dump-5.c
@@ -0,0 +1,15 @@
+/* PR middle-end/90694 - incorrect representation of ADDR_EXPR involving
+   a pointer to array
+   { dg-do compile }
+   { dg-options "-fdump-tree-original" } */
+
+typedef char A8[8];
+
+unsigned f (A8 *pa)
+{
+  return __builtin_strlen (&(*pa)[2]);
+}
+
+/* Veriy the expression is correct in the dump:
+  { dg-final { scan-tree-dump-not "\\\&\\\*pa\\\[2\\\]" "original" } }
+  { dg-final { scan-tree-dump "\\\&\\\(\\\*pa\\\)\\\[2\\\]" "original" } } */
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 6645a646617..30d1d65e6bc 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -1676,9 +1676,17 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
 	  {
 	if (TREE_CODE (TREE_OPERAND (node, 0)) != ADDR_EXPR)
 	  {
+		/* Enclose pointers to arrays in parentheses.  */
+		tree op0 = TREE_OPERAND (node, 0);
+		tree op0type = TREE_TYPE (op0);
+		if (POINTER_TYPE_P (op0type)
+		&& TREE_CODE (TREE_TYPE (op0type)) == ARRAY_TYPE)
+		  pp_left_paren (pp);
 		pp_star (pp);
-		dump_generic_node (pp, TREE_OPERAND (node, 0),
-   spc, flags, false);
+		dump_generic_node (pp, op0, spc, flags, false);
+		if (POINTER_TYPE_P (op0type)
+		&& TREE_CODE (TREE_TYPE (op0type)) == ARRAY_TYPE)
+		  pp_right_paren (pp);
 	  }
 	else
 	  dump_generic_node (pp,

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

[Jeff Law]

On 1/11/19 1:10 PM, Martin Sebor wrote:
> The attached patch extends the detection of references to static
> variables in inline functions to the function signatures, including
> their return type.  Since the declaration of a function need not be
> yet available at the time the static is referenced (e.g., when it's
> referenced in the functions return type), the patch introduces
> the concept of "tentative records" of such references and either
> completes the records when the full declaration of the function,
> including its definition, is known to be inline, or removes it
> when it isn't.
> 
> Martin
> 
> gcc-88718.diff
> 
> PR c/88718 - Strange inconsistency between old style and new style 
> definitions of inline functions.
> 
> gcc/c/ChangeLog:
> 
>   PR c/88718
>   * c-decl.c (reset_inline_statics): New function.
>   (record_inline_static): Optimize.
>   (check_inline_statics): Handle tentative records for inline
>   declarations without definitions.
>   Print static declaration location.
>   (push_file_scope): Clear records of references to statics.
>   (finish_decl): Add tentative records of references to statics.
>   (finish_function): Same.
>   * c-typeck.c (build_external_ref): Handle all references to statics.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c/88718
>   * gcc.dg/inline-40.c: New test.
>   * gcc.dg/inline-41.c: New test.
> 
> Index: gcc/c/c-decl.c
> ===
> --- gcc/c/c-decl.c(revision 267616)
> +++ gcc/c/c-decl.c(working copy)
> @@ -826,14 +826,47 @@ c_finish_incomplete_decl (tree decl)
>  }
>  }
>  
> -/* Record that inline function FUNC contains a reference (location
> -   LOC) to static DECL (file-scope or function-local according to
> -   TYPE).  */
> +/* Free records of references to static variables gathered so far.  */
>  
> +static void
> +reset_inline_statics (void)
> +{
> +  if (!c_inline_statics)
> +return;
> +
> +  for (c_inline_static *csi = c_inline_statics, *next = csi->next;
> +   csi; csi = next)
> +ggc_free (csi);
> +
> +  c_inline_statics = NULL;
> +}
> +
> +/* Record that inline function FUNC either does contain or may contain
> +   a reference (location LOC) to static DECL (file-scope or function-local
> +   according to TYPE).  For a null FUNC, a tentative record is created that
> +   reflects a reference in the function signature and that is either updated
> +   or removed when the function declaration is complete.  */
> +
>  void
>  record_inline_static (location_t loc, tree func, tree decl,
> enum c_inline_static_type type)
>  {
> +  gcc_assert (decl);
> +
> +  if (c_inline_statics)
> +{
> +  /* Avoid adding another tentative record for this DECL if one
> +  already exists.  */
> +  for (c_inline_static *csi = c_inline_statics; csi; csi = csi->next)
> + {
> +   if (c_inline_statics->function)
> + break;
> +
> +   if (decl == c_inline_statics->static_decl)
> + return;
> + }
> +}
> +
I'm going to assume this isn't called enough for the linear search to
matter from a compile-time standpoint.

OK for the trunk.

Jeff

Re: [PATCH,RFC 0/3] Support for CTF in GCC

[Indu Bhagat]

On 05/24/2019 02:26 AM, Richard Biener wrote:

Currently, it does look like CTF for possibly to-be-omitted symbols will be
generated... I assume even DWARF needs to handle this case. Can you point me to
how DWARF does this ?

It emits the debug information.  DWARF outputs a representation of the source,
not only emitted objects.  We prune some "unused" bits if the user prefers us
to do that but we do not omit information on types or decls that are used in
the source but later eventually optimized away.


It seems to me that linker support to garbage collect
unused entries would be the way to go forward (probably easy for the
declarations
but not so for the types)?

Hmm, garbage collecting unused types in linker - Let me get back to you on
this. It does not look easy. Decl should be doable though.

For example DWARF has something like type units that can be refered
to via hashes.  GCC can output those into separate sections and I can
envision outputting separate debug (CTF) sections for each declaration.
The linker could then merge sections for declarations that survived
and pick up all referenced type sections.  Restrictions on ordering
for CTF may make this a bit difficult though, essentially forcing a
separate intermediate "unlinked" format and the linker regenerating
the final one.  OTOH CTF probably simply concatenates data from
different CUs?


Yes, I cannot see this happening with CTF easily without some format changes.

At link-time, there needs to be de-duplication of CTF types across CUs. This
linker component needs work at this time, although we do have a working
prototype.

Regarding the type units in DWARF, are the shared/common types duplicated
across the type units ? If not duplicated, how are the referenced types
maintained/denoted across type units ?

Thanks!
Indu

Re: [PATCH] fixincludes breaks mingw64 build

[Jonathan Wakely]

On 31/05/19 17:18 +0200, Jonas Jelten wrote:

On 31/05/2019 16.38, Jonathan Wakely wrote:

On 31/05/19 15:35 +0200, Jonas Jelten wrote:

Hi!

I'm trying to build the x86_64-w64-mingw64 crosscompiler on gentoo.
It breaks because a fixincludes-fix is applied at a place where it should not 
be applied.

This broke the cross-gcc build for gcc-8.3.0 and 9.1.0 with error messages that 
made it tricky to figure out what was
really going on.


[snip]

N.B. The gcc-bugs mailing list is for automated email from Bugzilla,
not for reporting bugs. Sending the patch to gcc-patches (and CCing a
relevant maintainer) is correct, but there's no need (and no point) to
also send it to gcc-bugs.





Alright, thanks for the hint. I was just following the instructions in 
gcc-9.1.0/fixincludes/README:

"""
Please also send relevant information to gcc-b...@gcc.gnu.org, 
gcc-patches@gcc.gnu.org and,
please, to me: bk...@gnu.org.
"""


Oh, indeed! 


Bruce, should that suggest submitting something to Bugzilla instead?
Mails sent directly to gcc-bugs tend to get lost among all the
automated email from Bugzilla.

[C++ PATCH, PING^3] PR60531 - Wrong error about unresolved overloaded function

[Harald van Dijk]

another ping

On 12/05/2019 17:57, Harald van Dijk wrote:

ping again

On 26/04/2019 19:58, Harald van Dijk wrote:

ping

On 13/04/2019 10:01, Harald van Dijk wrote:

Hi,

For PR60531, GCC wrongly rejects function templates with explicitly
specified template arguments as overloaded. They are resolved by
resolve_nondeduced_context, which is normally called by
cp_default_conversion through decay_conversion, but the latter have
extra effects making them unusable here. Calling the former directly
does work.

Bootstrapped on x86_64-pc-linux-gnu on top of r270264 with
--enable-languages=all; make check shows no regressions.

Does this look okay?

This is my first code contribution to GCC, please let me know if
anything is missing. I have not signed any copyright disclaimer or
copyright assignment;  says that is not necessary
for small changes, which I trust this is. If it is needed after all,
please let me know what specifically will be required.

Cheers,
Harald van Dijk

 PR c++/60531
 * typeck.c (cp_build_binary_op): See if overload can be resolved.
 (cp_build_unary_op): Ditto.

 * g++.dg/template/operator15.C: New test.

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 03b14024738..e1ffe88ce2c 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4384,10 +4384,6 @@ cp_build_binary_op (const op_location_t ,
    /* True if both operands have arithmetic type.  */
    bool arithmetic_types_p;

-  /* Apply default conversions.  */
-  op0 = orig_op0;
-  op1 = orig_op1;
-
    /* Remember whether we're doing / or %.  */
    bool doing_div_or_mod = false;

@@ -4397,6 +4393,10 @@ cp_build_binary_op (const op_location_t ,
    /* Tree holding instrumentation expression.  */
    tree instrument_expr = NULL_TREE;

+  /* Apply default conversions.  */
+  op0 = resolve_nondeduced_context (orig_op0, complain);
+  op1 = resolve_nondeduced_context (orig_op1, complain);
+
    if (code == TRUTH_AND_EXPR || code == TRUTH_ANDIF_EXPR
    || code == TRUTH_OR_EXPR || code == TRUTH_ORIF_EXPR
    || code == TRUTH_XOR_EXPR)
@@ -6204,11 +6204,13 @@ cp_build_unary_op (enum tree_code code, tree xarg, bool 
noconvert,
    if (!arg || error_operand_p (arg))
  return error_mark_node;

+  arg = resolve_nondeduced_context (arg, complain);
+
    if ((invalid_op_diag
 = targetm.invalid_unary_op ((code == UNARY_PLUS_EXPR
  ? CONVERT_EXPR
  : code),
-   TREE_TYPE (xarg
+   TREE_TYPE (arg
  {
    if (complain & tf_error)
  error (invalid_op_diag);
diff --git a/gcc/testsuite/g++.dg/template/operator15.C 
b/gcc/testsuite/g++.dg/template/operator15.C
new file mode 100644
index 000..755442266bb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/operator15.C
@@ -0,0 +1,6 @@
+// PR c++/60531
+
+template < class T > T foo ();
+
+bool b1 = foo == foo;
+int (*fp1)() = +foo;

Re: [PATCH] PR tree-optimization/90681 Fix ICE in vect_slp_analyze_node_operations_1

[Richard Sandiford]

Alejandro Martinez Vicente  writes:
> Hi,
>
> This patch fixes bug 90681.  It was caused by trying to SLP vectorize a non
> grouped load.  We've fixed it by tweaking a bit the implementation: mark
> masked loads as not vectorizable, but support them as an special case.  Then
> the detect them in the test for normal non-grouped loads that was already
> there.
>
> The bug reproducer now works and the performance test we added is still happy.
>
> Alejandro
>
> gcc/ChangeLog:
>
> 2019-05-31  Alejandro Martinez  
>
>   PR tree-optimization/90681
>   * internal-fn.c (mask_load_direct): Mark as non-vectorizable again.
>   * tree-vect-slp.c (vect_build_slp_tree_1): Add masked loads as a
>   special case for SLP, but fail on non-grouped loads.
>
>
> 2019-05-31  Alejandro Martinez  
>
> gcc/testsuite/
>
>   PR tree-optimization/90681
>   * gfortran.dg/vect/pr90681.f: Bug reproducer.

OK, thanks.

Richard

> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
> index 3051a7a..04081f3 100644
> --- a/gcc/internal-fn.c
> +++ b/gcc/internal-fn.c
> @@ -100,7 +100,7 @@ init_internal_fns ()
>  /* Create static initializers for the information returned by
> direct_internal_fn.  */
>  #define not_direct { -2, -2, false }
> -#define mask_load_direct { -1, 2, true }
> +#define mask_load_direct { -1, 2, false }
>  #define load_lanes_direct { -1, -1, false }
>  #define mask_load_lanes_direct { -1, -1, false }
>  #define gather_load_direct { -1, -1, false }
> diff --git a/gcc/testsuite/gfortran.dg/vect/pr90681.f 
> b/gcc/testsuite/gfortran.dg/vect/pr90681.f
> new file mode 100644
> index 000..03d3987
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/vect/pr90681.f
> @@ -0,0 +1,13 @@
> +C { dg-do compile }
> +C { dg-additional-options "-march=armv8.2-a+sve" { target { aarch64*-*-* } } 
> }
> +  SUBROUTINE HMU (H1)
> +  COMMON DD(107)
> +  DIMENSION H1(NORBS,*)
> +DO 70 J1 = IA,I1
> +   H1(I1,J1) = 0
> +   JO1 = J1
> +   IF (JO1.EQ.1) THEN
> +   H1(I1,J1) = DD(NI)
> +   END IF
> +   70   CONTINUE
> +  END
> diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
> index 884db33..23a8a20 100644
> --- a/gcc/tree-vect-slp.c
> +++ b/gcc/tree-vect-slp.c
> @@ -661,6 +661,7 @@ vect_build_slp_tree_1 (unsigned char *swap,
>machine_mode optab_op2_mode;
>machine_mode vec_mode;
>stmt_vec_info first_load = NULL, prev_first_load = NULL;
> +  bool load_p = false;
>  
>/* For every stmt in NODE find its def stmt/s.  */
>stmt_vec_info stmt_info;
> @@ -714,7 +715,10 @@ vect_build_slp_tree_1 (unsigned char *swap,
>if (gcall *call_stmt = dyn_cast  (stmt))
>   {
> rhs_code = CALL_EXPR;
> -   if ((gimple_call_internal_p (call_stmt)
> +
> +   if (gimple_call_internal_p (stmt, IFN_MASK_LOAD))
> + load_p = true;
> +   else if ((gimple_call_internal_p (call_stmt)
>  && (!vectorizable_internal_fn_p
>  (gimple_call_internal_fn (call_stmt
> || gimple_call_tail_p (call_stmt)
> @@ -732,7 +736,10 @@ vect_build_slp_tree_1 (unsigned char *swap,
>   }
>   }
>else
> - rhs_code = gimple_assign_rhs_code (stmt);
> + {
> +   rhs_code = gimple_assign_rhs_code (stmt);
> +   load_p = TREE_CODE_CLASS (rhs_code) == tcc_reference;
> + }
>  
>/* Check the operation.  */
>if (i == 0)
> @@ -899,7 +906,7 @@ vect_build_slp_tree_1 (unsigned char *swap,
>  } /* Grouped access.  */
>else
>   {
> -   if (TREE_CODE_CLASS (rhs_code) == tcc_reference)
> +   if (load_p)
>   {
> /* Not grouped load.  */
> if (dump_enabled_p ())

Re: [PATCH] PR c/43673 - Incorrect warning in dfp printf.

[Jeff Law]

On 2/25/19 6:13 PM, luo...@linux.ibm.com wrote:
> From: Xiong Hu Luo 
> 
> dfp printf/scanf of Ha/HA, Da/DA and DDa/DDA is not set properly, cause
> incorrect warning happens:
> "use of 'D' length modifier with 'a' type character".
> 
> Regression-tested on powerpc64le-linux, OK for trunk and gcc-8?
> 
> gcc/c-family/ChangeLog:
> 
> 2019-02-25  Xiong Hu Luo  
> 
>   PR c/43673
>   * c-format.c (print_char_table, scanf_char_table): Replace BADLEN with
>   TEX_D32, TEX_D64 or TEX_D128.
> 
> gcc/testsuit/ChangeLog:
> 
> 2019-02-25  Xiong Hu Luo  
> 
>   PR c/43673
>   * gcc.dg/format-dfp-printf-1.c: New test.
>   * gcc.dg/format-dfp-scanf-1.c: Likewise.
THanks.  Installed on the trunk.

Sorry for the delays and miscommunications.
jeff

Re: *Ping* Re: [PATCH] PR c/43673 - Incorrect warning in dfp printf.

[Jeff Law]

On 5/23/19 4:26 PM, Joseph Myers wrote:
> On Tue, 21 May 2019, Jeff Law wrote:
> 
>> On 5/20/19 6:56 PM, luoxhu wrote:
>>> Ping for GCC-10.
>> I thought this was a NAK in its current form.
>>
>> See Ryan's c#1 in the BZ.
> 
> I don't see that as relevant to this bug report.
> 
> That comment is about the question of how GCC can know whether libc's 
> printf supports this feature at all (since in principle the warnings are 
> meant to relate to the features libc actually supports, unless you make a 
> particular format language explicit by specifying gnu_printf rather than 
> printf in the format attribute - though in practice it's only for MinGW 
> that GCC actually knows about a different set of formats supported by 
> default).  That comment suggests a possible answer (testing predefined 
> macros after the implicit preinclusion of any implicitly preincluded 
> header; note that the macro in question is now __STDC_IEC_60559_DFP__, in 
> TS 18661-2 and C2X) - although, while glibc has supported stdc-predef.h 
> for some time, current libdfp does not provide such a header.
> 
> This bug report is about an issue that, in the case where GCC is accepting 
> DFP printf formats, the set of such formats accepted is incomplete.  A fix 
> for it should be independent of any fix for the other (harder) issue.  (If 
> there is a printf implementation that does in fact support the same subset 
> of DFP formats supported by GCC's printf checking, but not the ones that 
> are the subject of this bug report, that would complicate things.)
> 
So your position is that runtime support isn't a factor in whether or
not we issue diagnostics for this stuff?

I guess that makes sense.  While we do have some checks for what the
runtime& target support, they're geared more type compatibility &
promotions.  Targets can add additional things via TARGET_FORMAT_TYPES
and friends.

Jeff

Re: [PATCH] libiberty: Don't use VLAs if __STDC_NO_VLA__ is non-zero

[Jeff Law]

On 5/30/19 2:50 PM, Michael Forney wrote:
> VLAs are optional in C11, and an implementation that does not support
> them will define __STDC_NO_VLA__ to 1.
> 
> 2019-05-30  Michael Forney  
> 
>   * cp-demangle.c: Don't define CP_DYNAMIC_ARRAYS if __STDC_NO_VLA__
>   is non-zero.
THanks.  I've installed this on the trunk.  Though I must admit some
surprise, what compiler are you using that defines STDC_NO_VLA? :-)

jeff

Re: [PATCH] fix more -Wformat-diag issues

[Jeff Law]

On 5/31/19 9:56 AM, Martin Sebor wrote:
> On 5/30/19 5:49 PM, Jeff Law wrote:
>> So in several places there's a comment which indicates that debugging
>> dumps and the like do not follow conventions.  Presumably you've tried
>> to keep a narrow scope on the diagnostic push/pops.  I'm also concerned
>> that the comments you mention that we trigger an ICE.
>>
>> So while I'll ack this patch, I would like to know more about the ICE
>> that's triggered in the checker and what the plans are for fixing it.
> 
> Sorry, I didn't word the comment (copied below) very clearly.
> What I meant to say is that the calls to error() in these files
> that don't follow the convention are ultimately followed by
> an ICE triggered either by an assert (as in cfgloop.c) or a call
> to internal_error (cgraph.h).  The diagnostics themselves don't
> cause an ICE.
OK.  Thanks for the clarification.

> 
> In a comment on one of the i18n bugs raised for these strings
> Richard suggests these error calls should probably replaced by
> direct calls to the pretty printer.  That would let us avoid
> suppressing the warnings and also presumably make it clear to
> translators the format strings aren't meant to be translated.
> It seemed like too big of a change for this patch so I simply
> suppressed the warnings but I agree it's worth considering at
> some point.
Agreed.

> 
> I'll adjust the comment before I check in the patch (I'm hoping
> to commit it at the same time as the checker itself once it's
> approved).
Your call on when to commit :-)

Jeff

Re: [PATCH] Don't insert ENDBR after NOTE_INSN_DELETED_LABEL

[H.J. Lu]

On Fri, May 31, 2019 at 10:43 AM Jakub Jelinek  wrote:
>
> On Fri, May 31, 2019 at 09:53:42AM -0700, H.J. Lu wrote:
> > 2019-05-31  H.J. Lu  
> >   Hongtao Liu  
> >
> > gcc/
> >
> >   PR target/89355
> >   * config/i386/i386.c (ix86_init_large_pic_reg): Don't set
> >   LABEL_PRESERVE_P.
>
> Why this change?  It is both not needed and undesirable, the
> large pic reg label is used magically, not as a normal label, so
> LABEL_PRESERVE_P on it is desirable.

Removed.

> Otherwise LGTM.
>
> Jakub

This is the patch I am checking in.

Thanks.

-- 
H.J.
From 57c4c30a5eda286800fd6c4e0101b82d8cfb604f Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Thu, 14 Feb 2019 09:12:57 -0800
Subject: [PATCH] i386: Don't insert ENDBR after NOTE_INSN_DELETED_LABEL

NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
but was not used for other purposes than taking its address which cannot
be used as target for indirect jumps.

Tested on Linux/x86-64 with -fcf-protection.

For x86-64 libc.so on glibc master branch (commit f43b8dd55588c3),

Before: 2961 endbr64
After:  2943 endbr64

2019-05-31  H.J. Lu  
	Hongtao Liu  

gcc/

	PR target/89355
	* config/i386/i386-features.c (rest_of_insert_endbranch): Remove
	NOTE_INSN_DELETED_LABEL check.

gcc/testsuite/

	PR target/89355
	* gcc.target/i386/cet-label-3.c: New test.
	* gcc.target/i386/cet-label-4.c: Likewise.
	* gcc.target/i386/cet-label-5.c: Likewise.
---
 gcc/config/i386/i386-features.c |  5 +
 gcc/testsuite/gcc.target/i386/cet-label-3.c | 23 +
 gcc/testsuite/gcc.target/i386/cet-label-4.c | 12 +++
 gcc/testsuite/gcc.target/i386/cet-label-5.c | 13 
 4 files changed, 49 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-5.c

diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index 60a120f4df7..c8de5261240 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1911,10 +1911,7 @@ rest_of_insert_endbranch (void)
 	  continue;
 	}
 
-	  if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
-	  || (NOTE_P (insn)
-		  && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
-	/* TODO.  Check /s bit also.  */
+	  if (LABEL_P (insn) && LABEL_PRESERVE_P (insn))
 	{
 	  cet_eb = gen_nop_endbr ();
 	  emit_insn_after (cet_eb, insn);
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-3.c b/gcc/testsuite/gcc.target/i386/cet-label-3.c
new file mode 100644
index 000..9f427a866f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-3.c
@@ -0,0 +1,23 @@
+/* PR target/89355  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection" } */
+/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } } } } */
+int
+test (int* val)
+{
+  int status = 99;
+
+  if (!val)
+{
+  status = 22;
+  goto end;
+}
+
+  extern int x;
+  *val = x;
+
+  status = 0;
+end:
+  return status;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-4.c b/gcc/testsuite/gcc.target/i386/cet-label-4.c
new file mode 100644
index 000..d743d2bf202
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-4.c
@@ -0,0 +1,12 @@
+/* PR target/89355  */
+/* { dg-do compile { target { fpic && lp64 } } } */
+/* { dg-options "-O2 -fcf-protection -fPIC -mcmodel=large" } */
+/* { dg-final { scan-assembler-times "endbr64" 1 } } */
+
+extern int val;
+
+int
+test (void)
+{
+  return val;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-5.c b/gcc/testsuite/gcc.target/i386/cet-label-5.c
new file mode 100644
index 000..862c79b058d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-5.c
@@ -0,0 +1,13 @@
+/* PR target/89355  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -Wno-return-local-addr" } */
+/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } } } } */
+
+void *
+func (void)
+{
+  return &
+bar:
+  return 0;
+}
-- 
2.20.1

libgo patch committed: Cheaper context switch on x86_64

[Ian Lance Taylor]

This libgo patch by Cherry Zhang implements cheaper goroutine context
switches on x86_64 GNU/Linux.

Currently, goroutine switches are implemented with libc
getcontext/setcontext functions, which saves/restores the machine
register states and also the signal context.  This does more than what
we need, and performs an expensive syscall.

This patch implements a simplified version of getcontext/setcontext,
in assembly, that only saves/restores the necessary part, i.e. the
callee-save registers, and the PC, SP.  A simplified version of
makecontext, written in C, is also added.  Currently this is only
implemented on x86_64 GNU/Linux.

Bootstrapped and tested on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 271784)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-4dc60d989293d070702024e7dea52b9849f74775
+8402f6ac021ba20163ab4fcdb10ab7bb642de6dc
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/Makefile.am
===
--- libgo/Makefile.am   (revision 271669)
+++ libgo/Makefile.am   (working copy)
@@ -481,6 +481,7 @@ runtime_files = \
runtime/runtime_c.c \
runtime/stack.c \
runtime/yield.c \
+   runtime/go-context.S \
$(rtems_task_variable_add_file) \
$(runtime_getncpu_file)
 
Index: libgo/configure.ac
===
--- libgo/configure.ac  (revision 271669)
+++ libgo/configure.ac  (working copy)
@@ -26,6 +26,7 @@ m4_rename([_AC_ARG_VAR_PRECIOUS],[glibgo
 m4_define([_AC_ARG_VAR_PRECIOUS],[])
 AC_PROG_CC
 AC_PROG_GO
+AM_PROG_AS
 m4_rename_force([glibgo_PRECIOUS],[_AC_ARG_VAR_PRECIOUS])
 
 AC_SUBST(CFLAGS)
Index: libgo/runtime/go-context.S
===
--- libgo/runtime/go-context.S  (nonexistent)
+++ libgo/runtime/go-context.S  (working copy)
@@ -0,0 +1,69 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This provides a simplified version of getcontext and
+// setcontext. They are like the corresponding functions
+// in libc, but we only save/restore the callee-save
+// registers and PC, SP. Unlike the libc functions, we
+// don't save/restore the signal masks and floating point
+// environment.
+
+#if defined(__x86_64__) && defined(__linux__) && !defined(__CET__)
+
+#define RBP_OFF(0*8)
+#define RBX_OFF(1*8)
+#define R12_OFF(2*8)
+#define R13_OFF(3*8)
+#define R14_OFF(4*8)
+#define R15_OFF(5*8)
+#define SP_OFF (6*8)
+#define PC_OFF (7*8)
+
+.globl __go_getcontext
+.text
+__go_getcontext:
+   movq%rbx, RBX_OFF(%rdi)
+   movq%rbp, RBP_OFF(%rdi)
+   movq%r12, R12_OFF(%rdi)
+   movq%r13, R13_OFF(%rdi)
+   movq%r14, R14_OFF(%rdi)
+   movq%r15, R15_OFF(%rdi)
+
+   movq(%rsp), %rax// return PC
+   movq%rax, PC_OFF(%rdi)
+   leaq8(%rsp), %rax   // the SP before pushing return PC
+   movq%rax, SP_OFF(%rdi)
+
+   ret
+
+.globl __go_setcontext
+.text
+__go_setcontext:
+   movqRBX_OFF(%rdi), %rbx
+   movqRBP_OFF(%rdi), %rbp
+   movqR12_OFF(%rdi), %r12
+   movqR13_OFF(%rdi), %r13
+   movqR14_OFF(%rdi), %r14
+   movqR15_OFF(%rdi), %r15
+   movqSP_OFF(%rdi), %rsp
+   movqPC_OFF(%rdi), %rdx
+
+   jmp *%rdx
+
+.globl __go_makecontext
+.text
+__go_makecontext:
+   addq%rcx, %rdx
+
+   // Align the SP, and push a dummy return address.
+   andq$~0xfULL, %rdx
+   subq$8, %rdx
+   movq$0, (%rdx)
+
+   movq%rdx, SP_OFF(%rdi)
+   movq%rsi, PC_OFF(%rdi)
+
+   ret
+
+#endif
Index: libgo/runtime/proc.c
===
--- libgo/runtime/proc.c(revision 271669)
+++ libgo/runtime/proc.c(working copy)
@@ -75,7 +75,7 @@ initcontext(void)
 }
 
 static inline void
-fixcontext(ucontext_t *c __attribute__ ((unused)))
+fixcontext(__go_context_t *c __attribute__ ((unused)))
 {
 }
 
@@ -182,18 +182,18 @@ fixcontext(ucontext_t* c)
 // Go, and Go has no simple way to align a field to such a boundary.
 // So we make the field larger in runtime2.go and pick an appropriate
 // offset within the field here.
-static ucontext_t*
+static __go_context_t*
 ucontext_arg(uintptr_t* go_ucontext)
 {
uintptr_t p = (uintptr_t)go_ucontext;
-   size_t align = __alignof__(ucontext_t);
+   size_t align = __alignof__(__go_context_t);
if(align > 16) {
// We only ensured space for up to a 16 byte alignment
// in 

Re: [PATCH] Don't insert ENDBR after NOTE_INSN_DELETED_LABEL

[Jakub Jelinek]

On Fri, May 31, 2019 at 09:53:42AM -0700, H.J. Lu wrote:
> 2019-05-31  H.J. Lu  
>   Hongtao Liu  
> 
> gcc/
> 
>   PR target/89355
>   * config/i386/i386.c (ix86_init_large_pic_reg): Don't set
>   LABEL_PRESERVE_P.

Why this change?  It is both not needed and undesirable, the
large pic reg label is used magically, not as a normal label, so
LABEL_PRESERVE_P on it is desirable.

Otherwise LGTM.

Jakub

[C++ PATCH] decls with error_node type

[Nathan Sidwell]

I discovered we could occasionally create TYPE_DECLs with 
error_mark_node type, and then successfully push them into the symbol table!


I don't think we should be doing this -- it means TREE_TYPE of a 
TYPE_DECL might not be a type, with resulting ICE fallout.  (When we 
cons up a VAR_DECL to cope with an undefined lookup we give it int type, 
to avoid this kind of thing.)


This patch stops grokdeclarator doing this, by immediately returning 
error_mark_node.  And added an assert to push_template_decl_real, where 
I noticed the problem occurring.


The fallout is some error cascade on symbol lookups that now fail. 
error33.C was particularly screwy:


  typedef void (A::T) ();

would pushing a T into the current scope.

Jason, ok?

nathan
--
Nathan Sidwell
2019-05-31  Nathan Sidwell  

	* decl.c (grokdeclarator): Don't create decls with error_mark_node
	type.
	* pt.c (push_template_decl_real): Assert decl's type not
	error_mark_node.

	* g++.dg/concepts/pr84423-1.C: Add more expected errors.
	* g++.dg/cpp0x/auto39.C: Likewise.
	* g++.dg/parse/error32.C: Likewise.
	* g++.dg/parse/error33.C: Likewise.

Index: gcc/cp/decl.c
===
--- gcc/cp/decl.c	(revision 271811)
+++ gcc/cp/decl.c	(working copy)
@@ -12082,11 +12082,9 @@ grokdeclarator (const cp_declarator *dec
   if (type_uses_auto (type))
 	{
-	  if (alias_p)
-	error_at (declspecs->locations[ds_type_spec],
-		  "% not allowed in alias declaration");
-	  else
-	error_at (declspecs->locations[ds_type_spec],
-		  "typedef declared %");
-	  type = error_mark_node;
+	  error_at (declspecs->locations[ds_type_spec],
+		alias_p
+		? G_("% not allowed in alias declaration")
+		: G_("typedef declared %"));
+	  return error_mark_node;
 	}
 
@@ -12097,5 +12095,5 @@ grokdeclarator (const cp_declarator *dec
 	{
 	  error ("typedef name may not be a nested-name-specifier");
-	  type = error_mark_node;
+	  return error_mark_node;
 	}
 
Index: gcc/cp/pt.c
===
--- gcc/cp/pt.c	(revision 271811)
+++ gcc/cp/pt.c	(working copy)
@@ -5450,4 +5450,6 @@ push_template_decl_real (tree decl, bool
 return error_mark_node;
 
+  gcc_checking_assert (TREE_TYPE (decl) != error_mark_node);
+
   /* See if this is a partial specialization.  */
   is_partial = ((DECL_IMPLICIT_TYPEDEF_P (decl)
Index: gcc/testsuite/g++.dg/concepts/pr84423-1.C
===
--- gcc/testsuite/g++.dg/concepts/pr84423-1.C	(revision 271811)
+++ gcc/testsuite/g++.dg/concepts/pr84423-1.C	(working copy)
@@ -6,3 +6,3 @@ template using A = auto;  // {
 template class> struct B {};
 
-B b;
+B b;  // { dg-error "not declared|invalid" }
Index: gcc/testsuite/g++.dg/cpp0x/auto39.C
===
--- gcc/testsuite/g++.dg/cpp0x/auto39.C	(revision 271811)
+++ gcc/testsuite/g++.dg/cpp0x/auto39.C	(working copy)
@@ -4,3 +4,3 @@
 typedef auto T; // { dg-error "9:typedef declared 'auto'" }
 
-void foo() { T(); }
+void foo() { T(); } // { dg-error "not declared" }
Index: gcc/testsuite/g++.dg/parse/error32.C
===
--- gcc/testsuite/g++.dg/parse/error32.C	(revision 271811)
+++ gcc/testsuite/g++.dg/parse/error32.C	(working copy)
@@ -8,5 +8,5 @@ typedef void (A::T)(); /* { dg-error "ty
 void foo()
 {
-  T t;
+  T t; /* { dg-error "was not declared" } */
   t; /* { dg-error "was not declared" } */
 }
Index: gcc/testsuite/g++.dg/parse/error33.C
===
--- gcc/testsuite/g++.dg/parse/error33.C	(revision 271811)
+++ gcc/testsuite/g++.dg/parse/error33.C	(working copy)
@@ -9,8 +9,9 @@ struct A
 typedef void (A::T)(); /* { dg-error "typedef name may not be a nested" } */
 
-void bar(T); /* { dg-message "note: declared here" } */
+void bar(T); /* { dg-error "declared void" } */
+// { dg-error "not declared" "" { target *-*-* } .-1 }
 
 void baz()
 {
-  bar(::foo); /* { dg-error "too many arguments" } */
+  bar(::foo); /* { dg-error "not declared" } */
 }

PING^2: [PATCH] i386: Properly encode xmm16-xmm31/ymm16-ymm31 for vector move

[H.J. Lu]

On Tue, May 21, 2019 at 2:43 PM H.J. Lu  wrote:
>
> On Fri, Feb 22, 2019 at 8:25 AM H.J. Lu  wrote:
> >
> > Hi Jan, Uros,
> >
> > This patch fixes the wrong code bug:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89229
> >
> > Tested on AVX2 and AVX512 with and without --with-arch=native.
> >
> > OK for trunk?
> >
> > Thanks.
> >
> > H.J.
> > --
> > i386 backend has
> >
> > INT_MODE (OI, 32);
> > INT_MODE (XI, 64);
> >
> > So, XI_MODE represents 64 INTEGER bytes = 64 * 8 = 512 bit operation,
> > in case of const_1, all 512 bits set.
> >
> > We can load zeros with narrower instruction, (e.g. 256 bit by inherent
> > zeroing of highpart in case of 128 bit xor), so TImode in this case.
> >
> > Some targets prefer V4SF mode, so they will emit float xorps for zeroing.
> >
> > sse.md has
> >
> > (define_insn "mov_internal"
> >   [(set (match_operand:VMOVE 0 "nonimmediate_operand"
> >  "=v,v ,v ,m")
> > (match_operand:VMOVE 1 "nonimmediate_or_sse_const_operand"
> >  " C,BC,vm,v"))]
> > 
> >   /* There is no evex-encoded vmov* for sizes smaller than 64-bytes
> >  in avx512f, so we need to use workarounds, to access sse registers
> >  16-31, which are evex-only. In avx512vl we don't need workarounds. 
> >  */
> >   if (TARGET_AVX512F &&  < 64 && !TARGET_AVX512VL
> >   && (EXT_REX_SSE_REG_P (operands[0])
> >   || EXT_REX_SSE_REG_P (operands[1])))
> > {
> >   if (memory_operand (operands[0], mode))
> > {
> >   if ( == 32)
> > return "vextract64x4\t{$0x0, %g1, %0|%0, %g1, 
> > 0x0}";
> >   else if ( == 16)
> > return "vextract32x4\t{$0x0, %g1, %0|%0, %g1, 
> > 0x0}";
> >   else
> > gcc_unreachable ();
> > }
> > ...
> >
> > However, since ix86_hard_regno_mode_ok has
> >
> >  /* TODO check for QI/HI scalars.  */
> >   /* AVX512VL allows sse regs16+ for 128/256 bit modes.  */
> >   if (TARGET_AVX512VL
> >   && (mode == OImode
> >   || mode == TImode
> >   || VALID_AVX256_REG_MODE (mode)
> >   || VALID_AVX512VL_128_REG_MODE (mode)))
> > return true;
> >
> >   /* xmm16-xmm31 are only available for AVX-512.  */
> >   if (EXT_REX_SSE_REGNO_P (regno))
> > return false;
> >
> >   if (TARGET_AVX512F &&  < 64 && !TARGET_AVX512VL
> >   && (EXT_REX_SSE_REG_P (operands[0])
> >   || EXT_REX_SSE_REG_P (operands[1])))
> >
> > is a dead code.
> >
> > Also for
> >
> > long long *p;
> > volatile __m256i yy;
> >
> > void
> > foo (void)
> > {
> >_mm256_store_epi64 (p, yy);
> > }
> >
> > with AVX512VL, we should generate
> >
> > vmovdqa %ymm0, (%rax)
> >
> > not
> >
> > vmovdqa64   %ymm0, (%rax)
> >
> > All TYPE_SSEMOV vector moves are consolidated to ix86_output_ssemov:
> >
> > 1. If xmm16-xmm31/ymm16-ymm31 registers aren't used, SSE/AVX vector
> > moves will be generated.
> > 2. If xmm16-xmm31/ymm16-ymm31 registers are used:
> >a. With AVX512VL, AVX512VL vector moves will be generated.
> >b. Without AVX512VL, xmm16-xmm31/ymm16-ymm31 register to register
> >   move will be done with zmm register move.
> >
> > ext_sse_reg_operand is removed since it is no longer needed.
> >
> > Tested on AVX2 and AVX512 with and without --with-arch=native.
> >
> > gcc/
> >
> > PR target/89229
> > PR target/89346
> > * config/i386/i386-protos.h (ix86_output_ssemov): New prototype.
> > * config/i386/i386.c (ix86_get_ssemov): New function.
> > (ix86_output_ssemov): Likewise.
> > * config/i386/i386.md (*movxi_internal_avx512f): Call
> > ix86_output_ssemov for TYPE_SSEMOV.
> > (*movoi_internal_avx): Call ix86_output_ssemov for TYPE_SSEMOV.
> > Remove ext_sse_reg_operand and TARGET_AVX512VL check.
> > (*movti_internal): Likewise.
> > (*movdi_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> > Remove ext_sse_reg_operand check.
> > (*movsi_internal): Likewise.
> > (*movtf_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> > (*movdf_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> > Remove TARGET_AVX512F, TARGET_PREFER_AVX256, TARGET_AVX512VL
> > and ext_sse_reg_operand check.
> > (*movsf_internal_avx): Call ix86_output_ssemov for TYPE_SSEMOV.
> > Remove TARGET_PREFER_AVX256, TARGET_AVX512VL and
> > ext_sse_reg_operand check.
> > * config/i386/mmx.md (MMXMODE:*mov_internal): Call
> > ix86_output_ssemov for TYPE_SSEMOV.  Remove ext_sse_reg_operand
> > check.
> > * config/i386/sse.md (VMOVE:mov_internal): Call
> > ix86_output_ssemov for TYPE_SSEMOV.  Remove TARGET_AVX512VL
> > check.
> > * config/i386/predicates.md (ext_sse_reg_operand): Removed.
> >
> > gcc/testsuite/
> >
> > PR target/89229

[PATCH] Don't insert ENDBR after NOTE_INSN_DELETED_LABEL

[H.J. Lu]

On Tue, May 28, 2019 at 10:33 AM Jeff Law  wrote:
>
> On 5/28/19 9:48 AM, Jakub Jelinek wrote:
> > On Tue, May 28, 2019 at 08:30:59AM -0700, H.J. Lu wrote:
> >>> We shouldn't generate ENDBR in that case, nothing can goto to bar 
> >>> (otherwise
> >>> it would remain a normal label, not a deleted label).
> >>>
> >>
> >> But return value of func () may be used with indirect jump.
> >
> > No, it may be used say to print that address, but computed goto can't be
> > used to jump from one function to a different function, see
> > https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
> > "You may not use this mechanism to jump to code in a different function.
> > If you do that, totally unpredictable things happen."
> Right.  We disallowed this case some time ago IIRC.  It's essentially
> undefined behavior.  I would even claim that in a CET world that we
> *want* a CET fault if something tried to use the deleted label as a jump
> target and thus an ENDBR is undesirable.
>
>
> >
> > NOTE_INSN_DELETED_LABEL is not guaranteed to be followed by any sensible
> > code, the only reason it is kept is that there is or might be something
> > referencing the label and so you want to emit the label somewhere in the
> > function, but don't care much where in the function.
> Exactly.
>
> Jeff

Here is the updated patch not to insert ENDBR after NOTE_INSN_DELETED_LABEL.

Tested on Linux/x86-64 with -fcf-protection.

OK for trunk?

Thanks.

-- 
H.J.
From 221484c1c6871d6fa93d4338df52d03d76a436dc Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Thu, 14 Feb 2019 09:12:57 -0800
Subject: [PATCH] i386: Don't insert ENDBR after NOTE_INSN_DELETED_LABEL

NOTE_INSN_DELETED_LABEL is used to mark what used to be a 'code_label',
but was not used for other purposes than taking its address which cannot
be used as target for indirect jumps.

Since LABEL_PRESERVE_P is true only if the label address was taken, don't
set LABEL_PRESERVE_P on label used to initialize PIC register.

Tested on Linux/x86-64 with -fcf-protection.

For x86-64 libc.so on glibc master branch (commit f43b8dd55588c3),

Before: 2961 endbr64
After:  2943 endbr64

2019-05-31  H.J. Lu  
	Hongtao Liu  

gcc/

	PR target/89355
	* config/i386/i386-features.c (rest_of_insert_endbranch): Remove
	NOTE_INSN_DELETED_LABEL check.
	* config/i386/i386.c (ix86_init_large_pic_reg): Don't set
	LABEL_PRESERVE_P.

gcc/testsuite/

	PR target/89355
	* gcc.target/i386/cet-label-3.c: New test.
	* gcc.target/i386/cet-label-4.c: Likewise.
	* gcc.target/i386/cet-label-5.c: Likewise.
---
 gcc/config/i386/i386-features.c |  5 +
 gcc/config/i386/i386.c  |  1 -
 gcc/testsuite/gcc.target/i386/cet-label-3.c | 23 +
 gcc/testsuite/gcc.target/i386/cet-label-4.c | 12 +++
 gcc/testsuite/gcc.target/i386/cet-label-5.c | 13 
 5 files changed, 49 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-5.c

diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index 60a120f4df7..c8de5261240 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1911,10 +1911,7 @@ rest_of_insert_endbranch (void)
 	  continue;
 	}
 
-	  if ((LABEL_P (insn) && LABEL_PRESERVE_P (insn))
-	  || (NOTE_P (insn)
-		  && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))
-	/* TODO.  Check /s bit also.  */
+	  if (LABEL_P (insn) && LABEL_PRESERVE_P (insn))
 	{
 	  cet_eb = gen_nop_endbr ();
 	  emit_insn_after (cet_eb, insn);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 79fcb5c4e57..bb4df4760b6 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1651,7 +1651,6 @@ ix86_init_large_pic_reg (unsigned int tmp_regno)
   gcc_assert (Pmode == DImode);
   label = gen_label_rtx ();
   emit_label (label);
-  LABEL_PRESERVE_P (label) = 1;
   tmp_reg = gen_rtx_REG (Pmode, tmp_regno);
   gcc_assert (REGNO (pic_offset_table_rtx) != tmp_regno);
   emit_insn (gen_set_rip_rex64 (pic_offset_table_rtx,
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-3.c b/gcc/testsuite/gcc.target/i386/cet-label-3.c
new file mode 100644
index 000..9f427a866f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-3.c
@@ -0,0 +1,23 @@
+/* PR target/89355  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection" } */
+/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } } } } */
+int
+test (int* val)
+{
+  int status = 99;
+
+  if (!val)
+{
+  status = 22;
+  goto end;
+}
+
+  extern int x;
+  *val = x;
+
+  status = 0;
+end:
+  return status;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-4.c b/gcc/testsuite/gcc.target/i386/cet-label-4.c
new file mode 100644
index 

[PATCH] PR tree-optimization/90681 Fix ICE in vect_slp_analyze_node_operations_1

[Alejandro Martinez Vicente]

Hi,

This patch fixes bug 90681.  It was caused by trying to SLP vectorize a non
grouped load.  We've fixed it by tweaking a bit the implementation: mark
masked loads as not vectorizable, but support them as an special case.  Then
the detect them in the test for normal non-grouped loads that was already
there.

The bug reproducer now works and the performance test we added is still happy.

Alejandro

gcc/ChangeLog:

2019-05-31  Alejandro Martinez  

PR tree-optimization/90681
* internal-fn.c (mask_load_direct): Mark as non-vectorizable again.
* tree-vect-slp.c (vect_build_slp_tree_1): Add masked loads as a
special case for SLP, but fail on non-grouped loads.


2019-05-31  Alejandro Martinez  

gcc/testsuite/

PR tree-optimization/90681
* gfortran.dg/vect/pr90681.f: Bug reproducer.


fix.patch
Description: fix.patch

Re: [PATCH] PR71482: Add -Wglobal-constructors

[Martin Sebor]

On 5/31/19 10:12 AM, Sean Gillespie wrote:

On Thu, May 30, 2019 at 4:28 PM Martin Sebor  wrote:


On 5/28/19 3:30 PM, Sean Gillespie wrote:

This adds a new warning, -Wglobal-constructors, that warns whenever a
decl requires a global constructor or destructor. This new warning fires
whenever a thread_local or global variable is declared whose type has a
non-trivial constructor or destructor. When faced with both a constructor
and a destructor, the error message mentions the destructor and is only
fired once.

This warning mirrors the Clang option -Wglobal-constructors, which warns
on the same thing. -Wglobal-constructors was present in Apple's GCC and
later made its way into Clang.

Bootstrapped and regression-tested on x86-64 linux, new tests passing.


I can't tell from the Clang online manual:

Is the warning meant to trigger just for globals, or for all
objects with static storage duration regardless of scope (i.e.,
including namespace-scope objects and static locals and static
class members)?

"Requires a constructor to initialize" doesn't seem very clear
to me.  Is the warning intended to trigger for objects that
require dynamic initialization?  If so, then what about dynamic
intialization of objects of trivial types, such as this:

static int i = std::string ("foo").length ();

or even

static int j = strlen (getenv ("TMP"));

If these aren't meant to be diagnosed then the description should
make it clear (the first one involves a ctor, the second one does
not).  But I would think that if the goal is to find sources of
dynamic initialization then diagnosing the above would be useful.
If so, the description should make it clear and tests verifying
that it works should be added.

Martin


The answer to both of those is yes. Clang warns on both of these with
-Wglobal-constructors because the goal of -Wglobal-constructors is to
catch and warn against potential life-before-main behavior.

Based on Marek's feedback I'm about to push another patch that also
warns on your above two examples and adds tests for them. The idea
is that any C++-level dynamically initialized globals get warned. Constexpr
constructors or things that otherwise can be statically initialized
are not warned.


That makes sense.



Regarding the doc description, how about something like this:

"Warns whenever an object with static storage duration requires
dynamic initialiation,
through global constructors or otherwise."


This makes it clearer -- i.e, global constructors refers to the ELF
concept rather than to the C++ one.  What does the otherwise part
refer to?  Some non-ELF mechanism?


PS Dynamic initialization can be optimized into static
initialization, even when it involves a user-defined constructor.
If the purpose of the warning is to find objects that are
dynamically initialized in the sense of the C++ language then
implementing it in the front-end is sufficient.  But if the goal
is to detect constructors that will actually run at runtime (i.e.,
have a startup cost and may have dependencies on one another) then
it needs to be implemented much later.



Clang sticks to the C++ language definition of dynamic initialization for the
purposes of this warning and I think we should as well. Without runtime checks
this is a best-effort warning, but it is capable of catching a bunch
of the common
ways that you can get constructors running on startup, which is the goal.


It might be worth mentioning that in the manual, perhaps along with
an example showing an instance of the warning that will most likely
be a true positive vs a false positive.  E.g., something like this:

  const char* const tmp = getenv ("TMP");

vs

  const int n = strlen ("123");   // initialized statically

Martin

Re: New .md construct: define_insn_and_rewrite

[Richard Sandiford]

Hans-Peter Nilsson  writes:
> On Tue, 14 May 2019, Richard Sandiford wrote:
>> Several SVE patterns need define_insn_and_splits that generate the
>> same insn_code, but with different operands.  That's probably a
>> niche requirement, but it's cropping up often enough on the ACLE
>> branch that I think it would be good to have a syntactic sugar for it.
>>
>> This patch therefore adds a new construct called define_insn_and_rewrite.
>
>> It's basically a define_insn_and_split with an implicit split pattern,
>> obtained by copying the insn pattern and replacing match_operands with
>> match_dups and match_operators with match_op_dups.
>
> I think that sentence should be in the documentation, in one of
> the introductory sentences.
>
>> Any comments?
>
>>  Suggestions for better names?
>
> I'll pass on this, for now. ;-)
> But, IMHO it lacks an allusion to define_insn_and_split.
>
>> Index: gcc/doc/md.texi
>
>> +@end smallexample
>
> Can you please add the equivalent define_insn_and_split
> "expansion" to the example?  It'd help understanding the concept
> graphically at a glance.

OK, here's what I installed.

Richard


2019-05-31  Richard Sandiford  

gcc/
* doc/md.texi: Document define_insn_and_rewrite.
* rtl.def (DEFINE_INSN_AND_REWRITE): New rtx code.
* gensupport.c (queue_elem): Update comment.
(replace_operands_with_dups): New function.
(gen_rewrite_sequence): Likewise.
(process_rtx): Handle DEFINE_INSN_AND_REWRITE.
* read-rtl.c (apply_subst_iterator): Likewise.
(add_condition_to_rtx, named_rtx_p): Likewise.
(rtx_reader::read_rtx_operand): Likewise.
* config/aarch64/aarch64-sve.md
(while_ult_cc): Rename to...
(*while_ult_cc): ...this and use
define_insn_and_rewrite.
(*cond__any): Turn into define_insn_and_rewrites.
Remove separate define_split.

Index: gcc/doc/md.texi
===
--- gcc/doc/md.texi 2019-05-31 17:26:54.367202007 +0100
+++ gcc/doc/md.texi 2019-05-31 17:27:03.711176173 +0100
@@ -8498,6 +8498,119 @@ functionality as two separate @code{defi
 patterns.  It exists for compactness, and as a maintenance tool to prevent
 having to ensure the two patterns' templates match.
 
+@findex define_insn_and_rewrite
+It is sometimes useful to have a @code{define_insn_and_split}
+that replaces specific operands of an instruction but leaves the
+rest of the instruction pattern unchanged.  You can do this directly
+with a @code{define_insn_and_split}, but it requires a
+@var{new-insn-pattern-1} that repeats most of the original @var{insn-pattern}.
+There is also the complication that an implicit @code{parallel} in
+@var{insn-pattern} must become an explicit @code{parallel} in
+@var{new-insn-pattern-1}, which is easy to overlook.
+A simpler alternative is to use @code{define_insn_and_rewrite}, which
+is a form of @code{define_insn_and_split} that automatically generates
+@var{new-insn-pattern-1} by replacing each @code{match_operand}
+in @var{insn-pattern} with a corresponding @code{match_dup}, and each
+@code{match_operator} in the pattern with a corresponding @code{match_op_dup}.
+The arguments are otherwise identical to @code{define_insn_and_split}:
+
+@smallexample
+(define_insn_and_rewrite
+  [@var{insn-pattern}]
+  "@var{condition}"
+  "@var{output-template}"
+  "@var{split-condition}"
+  "@var{preparation-statements}"
+  [@var{insn-attributes}])
+@end smallexample
+
+The @code{match_dup}s and @code{match_op_dup}s in the new
+instruction pattern use any new operand values that the
+@var{preparation-statements} store in the @code{operands} array,
+as for a normal @code{define_insn_and_split}.  @var{preparation-statements}
+can also emit additional instructions before the new instruction.
+They can even emit an entirely different sequence of instructions and
+use @code{DONE} to avoid emitting a new form of the original
+instruction.
+
+The split in a @code{define_insn_and_rewrite} is only intended
+to apply to existing instructions that match @var{insn-pattern}.
+@var{split-condition} must therefore start with @code{&&},
+so that the split condition applies on top of @var{condition}.
+
+Here is an example from the AArch64 SVE port, in which operand 1 is
+known to be equivalent to an all-true constant and isn't used by the
+output template:
+
+@smallexample
+(define_insn_and_rewrite "*while_ult_cc"
+  [(set (reg:CC CC_REGNUM)
+(compare:CC
+  (unspec:SI [(match_operand:PRED_ALL 1)
+  (unspec:PRED_ALL
+[(match_operand:GPI 2 "aarch64_reg_or_zero" "rZ")
+ (match_operand:GPI 3 "aarch64_reg_or_zero" "rZ")]
+UNSPEC_WHILE_LO)]
+ UNSPEC_PTEST_PTRUE)
+  (const_int 0)))
+   (set (match_operand:PRED_ALL 0 "register_operand" "=Upa")
+(unspec:PRED_ALL [(match_dup 2)
+  

Re: [PATCH] PR71482: Add -Wglobal-constructors

[Marek Polacek]

On Fri, May 31, 2019 at 06:25:03PM +0200, Jakub Jelinek wrote:
> On Fri, May 31, 2019 at 12:16:58PM -0400, Marek Polacek wrote:
> > > The warning is not meant to diagnose these.  But I do agree that the
> > > documentation for the new warning should be improved.
> > 
> > I was partially wrong: if i/j are at file scope, then the warning triggers.
> > But if i/j are in a function, then it doesn't warn.
> 
> That is correct, because the block scope statics aren't actually initialized
> from a global constructor, but with a guard variable whenever a function is
> called the first time.  So on
> int bar ();
> struct S { S (); ~S (); };
> 
> void
> foo ()
> {
>   static int a = bar ();
>   static S s;
> }
> this warning shouldn't warn at all.  It might be useful to have another
> warning for these, but it should be a different warning.

Right, agreed -- and the patch already does the right thing.

Marek

Re: [PATCH] PR71482: Add -Wglobal-constructors

[Jakub Jelinek]

On Fri, May 31, 2019 at 12:16:58PM -0400, Marek Polacek wrote:
> > The warning is not meant to diagnose these.  But I do agree that the
> > documentation for the new warning should be improved.
> 
> I was partially wrong: if i/j are at file scope, then the warning triggers.
> But if i/j are in a function, then it doesn't warn.

That is correct, because the block scope statics aren't actually initialized
from a global constructor, but with a guard variable whenever a function is
called the first time.  So on
int bar ();
struct S { S (); ~S (); };

void
foo ()
{
  static int a = bar ();
  static S s;
}
this warning shouldn't warn at all.  It might be useful to have another
warning for these, but it should be a different warning.

Jakub

Re: [PATCH] PR71482: Add -Wglobal-constructors

[Marek Polacek]

On Fri, May 31, 2019 at 11:50:47AM -0400, Marek Polacek wrote:
> On Thu, May 30, 2019 at 05:28:47PM -0600, Martin Sebor wrote:
> > On 5/28/19 3:30 PM, Sean Gillespie wrote:
> > > This adds a new warning, -Wglobal-constructors, that warns whenever a
> > > decl requires a global constructor or destructor. This new warning fires
> > > whenever a thread_local or global variable is declared whose type has a
> > > non-trivial constructor or destructor. When faced with both a constructor
> > > and a destructor, the error message mentions the destructor and is only
> > > fired once.
> > > 
> > > This warning mirrors the Clang option -Wglobal-constructors, which warns
> > > on the same thing. -Wglobal-constructors was present in Apple's GCC and
> > > later made its way into Clang.
> > > 
> > > Bootstrapped and regression-tested on x86-64 linux, new tests passing.
> > 
> > I can't tell from the Clang online manual:
> > 
> > Is the warning meant to trigger just for globals, or for all
> > objects with static storage duration regardless of scope (i.e.,
> > including namespace-scope objects and static locals and static
> > class members)?
> > 
> > "Requires a constructor to initialize" doesn't seem very clear
> > to me.  Is the warning intended to trigger for objects that
> > require dynamic initialization?  If so, then what about dynamic
> > intialization of objects of trivial types, such as this:
> > 
> >   static int i = std::string ("foo").length ();
> > 
> > or even
> > 
> >   static int j = strlen (getenv ("TMP"));
> 
> The warning is not meant to diagnose these.  But I do agree that the
> documentation for the new warning should be improved.

I was partially wrong: if i/j are at file scope, then the warning triggers.
But if i/j are in a function, then it doesn't warn.

My apologies.

Marek

Re: [PATCH] PR71482: Add -Wglobal-constructors

[Sean Gillespie]

On Thu, May 30, 2019 at 4:28 PM Martin Sebor  wrote:
>
> On 5/28/19 3:30 PM, Sean Gillespie wrote:
> > This adds a new warning, -Wglobal-constructors, that warns whenever a
> > decl requires a global constructor or destructor. This new warning fires
> > whenever a thread_local or global variable is declared whose type has a
> > non-trivial constructor or destructor. When faced with both a constructor
> > and a destructor, the error message mentions the destructor and is only
> > fired once.
> >
> > This warning mirrors the Clang option -Wglobal-constructors, which warns
> > on the same thing. -Wglobal-constructors was present in Apple's GCC and
> > later made its way into Clang.
> >
> > Bootstrapped and regression-tested on x86-64 linux, new tests passing.
>
> I can't tell from the Clang online manual:
>
> Is the warning meant to trigger just for globals, or for all
> objects with static storage duration regardless of scope (i.e.,
> including namespace-scope objects and static locals and static
> class members)?
>
> "Requires a constructor to initialize" doesn't seem very clear
> to me.  Is the warning intended to trigger for objects that
> require dynamic initialization?  If so, then what about dynamic
> intialization of objects of trivial types, such as this:
>
>static int i = std::string ("foo").length ();
>
> or even
>
>static int j = strlen (getenv ("TMP"));
>
> If these aren't meant to be diagnosed then the description should
> make it clear (the first one involves a ctor, the second one does
> not).  But I would think that if the goal is to find sources of
> dynamic initialization then diagnosing the above would be useful.
> If so, the description should make it clear and tests verifying
> that it works should be added.
>
> Martin

The answer to both of those is yes. Clang warns on both of these with
-Wglobal-constructors because the goal of -Wglobal-constructors is to
catch and warn against potential life-before-main behavior.

Based on Marek's feedback I'm about to push another patch that also
warns on your above two examples and adds tests for them. The idea
is that any C++-level dynamically initialized globals get warned. Constexpr
constructors or things that otherwise can be statically initialized
are not warned.

Regarding the doc description, how about something like this:

"Warns whenever an object with static storage duration requires
dynamic initialiation,
through global constructors or otherwise."

>
> PS Dynamic initialization can be optimized into static
> initialization, even when it involves a user-defined constructor.
> If the purpose of the warning is to find objects that are
> dynamically initialized in the sense of the C++ language then
> implementing it in the front-end is sufficient.  But if the goal
> is to detect constructors that will actually run at runtime (i.e.,
> have a startup cost and may have dependencies on one another) then
> it needs to be implemented much later.
>

Clang sticks to the C++ language definition of dynamic initialization for the
purposes of this warning and I think we should as well. Without runtime checks
this is a best-effort warning, but it is capable of catching a bunch
of the common
ways that you can get constructors running on startup, which is the goal.

Sean Gillespie

Re: [patch] Fix segfault caused by spurious constant overflow

[Eric Botcazou]

> Hmm, ISTR we had such mitigations in place (or have) elsewhere keying
> on the most significant bit set instead of power-of-two.

fold_plusminus_mult_expr only factors out for power-of-two:

  if (exact_log2 (absu_hwi (int11)) > 0 && int01 % int11 == 0
  /* The remainder should not be a constant, otherwise we
 end up folding i * 4 + 2 to (i * 2 + 1) * 2 which has
 increased the number of multiplications necessary.  */
  && TREE_CODE (arg10) != INTEGER_CST)

> But your case
> likely recurses and runs into the extract_multiv limiting to eventually
> stop, even for (N + 4) * 8, right?

Yes, it oscillates between extract_multiv and fold_plusminus_mult_expr until 
reaching the maximal depth.

> If so shouldn't we prevent this even for !TYPE_OVERFLOW_WRAPS?  Also
> 
> + && !(tree_fits_shwi_p (c)
> +  && exact_log2 (absu_hwi (tree_to_shwi (c))) > 0))

The code only distributes for TYPE_OVERFLOW_WRAPS though:

  /* The last case is if we are a multiply.  In that case, we can
 apply the distributive law to commute the multiply and addition
 if the multiplication of the constants doesn't overflow
 and overflow is defined.  With undefined overflow
 op0 * c might overflow, while (op0 + orig_op1) * c doesn't.  */
  if (code == MULT_EXPR && TYPE_OVERFLOW_WRAPS (ctype))
return fold_build2 (tcode, ctype,
fold_build2 (code, ctype,
 fold_convert (ctype, op0),
 fold_convert (ctype, c)),
op1);

> is better written as
> 
>&& exact_log2 (wi::to_wide (c)) > 0
> 
> not sure why the sizetype constant for you fits in a signed HWI
> or you need to compute its absolute value.  Eventually you
> need to use wi::abs(wide_int::from (wi::to_wide (c), TYPE_PRECISION
> (TREE_TYPE (c)), SIGNED))
> or so.

This is just mirrored on what fold_plusminus_mult_expr does.

-- 
Eric Botcazou

Re: [PATCH][AArch64] Increase default function alignment

[Wilco Dijkstra]

Hi Steve,

> I have no objection to the change but could the commit message and/or
> comments be expanded to explain the ':12' part of this value.  I
> couldn't find an explanation for it in the code and I don't understand
> what it does.

See 
https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-falign-functions

Basically the 12 guarantees there are at least 12 useable bytes in the 
16-byte aligned block. This significantly lowers the overhead of padding
compared to always forcing 16-byte alignment. The Neoverse N1 tuning
already uses this feature.

Wilco

Re: [PATCH] fix more -Wformat-diag issues

[Martin Sebor]

On 5/30/19 5:49 PM, Jeff Law wrote:

On 5/22/19 10:34 AM, Martin Sebor wrote:

Incorporating the feedback I got on the -Wformat-diag checker
provided an opportunity to tighten up existing and implement
a small number of few additional rules based on GCC Coding
Conventions (https://gcc.gnu.org/codingconventions.html).
The checker now also warns for incorrect uses of the following:

  *  bitfield (rather than bit-field)
  *  builtin (rather than built-in)
  *  command line (rather than command-line)
  *  enumeral (rather than enumerated)
  *  floating point (rather than floating-point)
  *  non-zero (rather than nonzero)

In addition, it has become better at finding unquoted qualifiers
(like const in const-qualified or "const %qT" rather than %"), and detects some additional abbreviations (e.g., "stmt"
instead of "statement").

These improvements exposed a number of additional issues in our
sources that the attached patch corrects.

As before, I have tested the patch on x86_64-linux and adjusted
the fallout in the test suite.  More cleanup will likely be needed
on other targets but based on the prior changes I don't expect it
to be extensive.

I will post the patch with the checker implementation separately.

Martin

PS As discussed in the thread below, some of the instances of
added hyphenation in "floating-point" aren't strictly necessary
and the  wording might need to be tweaked a bit to make it so:
   https://gcc.gnu.org/ml/gcc/2019-05/msg00168.html
I'll handle it in a subsequent commit if it's viewed important.

gcc-wformat-diag.diff

gcc/c/ChangeLog:

* c-decl.c (start_decl): Adjust quoting and hyphenation
in diagnostics.
(finish_decl): Same.
(finish_enum): Same.
(start_function): Same.
(declspecs_add_type): Same.
* c-parser.c (warn_for_abs): Same.
* c-typeck.c (build_binary_op): Same.

gcc/c-family/ChangeLog:

* c-attribs.c (handle_mode_attribute): Adjust quoting and hyphenation.
(handle_alias_ifunc_attribute): Same.
(handle_copy_attribute): Same.
(handle_weakref_attribute): Same.
(handle_nonnull_attribute): Same.
* c-warn.c (warn_for_sign_compare): Same.
(warn_for_restrict): Same.
* c.opt: Same.

* c-common.h (GCC_DIAG_STYLE): Adjust.
 (GCC_DIAG_RAW_STYLE): New macro.

gcc/cp/ChangeLog:

* call.c (build_conditional_expr_1): Adjust quoting and hyphenation.
(convert_like_real): Same.
(convert_arg_to_ellipsis): Same.
* constexpr.c (diag_array_subscript): Same.
* constraint.cc (diagnose_trait_expression): Same.
* cvt.c (ocp_convert): Same.
* decl.c (start_decl): Same.
(check_for_uninitialized_const_var): Same.
(grokfndecl): Same.
(check_special_function_return_type): Same.
(finish_enum_value_list): Same.
(start_preparsed_function): Same.
* parser.c (cp_parser_decl_specifier_seq): Same.
* typeck.c (cp_build_binary_op): Same.
(build_static_cast_1): Same.

* cp-tree.h (GCC_DIAG_STYLE): Adjust.
 (GCC_DIAG_RAW_STYLE): New macro.

gcc/lto/ChangeLog:

* lto-common.c (lto_file_finalize): Adjust quoting and hyphenation.

gcc/objc/ChangeLog:

* objc-act.c (objc_build_setter_call): Adjust quoting and hyphenation.
* objc-encoding.c (encode_gnu_bitfield): Same.

gcc/ChangeLog:

* config/i386/i386-features.c (ix86_get_function_versions_dispatcher):
Adjust quoting and hyphenation.
* convert.c (convert_to_real_1): Same.
* gcc.c (driver_wrong_lang_callback): Same.
(driver::handle_unrecognized_options): Same.
* gimple-ssa-nonnull-compare.c (do_warn_nonnull_compare): Same.
* opts-common.c (cmdline_handle_error): Same.
(read_cmdline_option): Same.
* opts-global.c (complain_wrong_lang): Same.
(print_ignored_options): Same.
(handle_common_deferred_options): Same.
* pretty-print.h: Same.
* print-rtl.c (debug_bb_n_slim): Same.
* sched-rgn.c (make_pass_sched_fusion): Same.
* tree-cfg.c (verify_gimple_assign_unary): Same.
(verify_gimple_label): Same.
* tree-ssa-operands.c (verify_ssa_operands): Same.
* varasm.c (do_assemble_alias): Same.
(assemble_alias): Same.

* diagnostic-core.h (GCC_DIAG_STYLE): Adjust.
 (GCC_DIAG_RAW_STYLE): New macro.

* cfghooks.c: Disable -Wformat-diags.
* cfgloop.c: Same.
* cfgrtl.c: Same.
* cgraph.c: Same.
* diagnostic-show-locus.c: Same.
* diagnostic.c: Same.
* gimple-pretty-print.c: Same.
* graph.c: Same.
* symtab.c: Same.
* tree-eh.c Same.
* tree-pretty-print.c: Same.
* tree-ssa.c: Same.

* configure: Regenerate.
* configure.ac (ACX_PROG_CXX_WARNING_OPTS): Add -Wno-error=format-diag.
 (ACX_PROG_CC_WARNING_OPTS): Same.

Re: Simplify more EXACT_DIV_EXPR comparisons

[Aldy Hernandez]

Sure. No problem. Thanks for looking at this.

Aldy

On Fri, May 31, 2019, 17:48 Marc Glisse  wrote:

> On Fri, 31 May 2019, Aldy Hernandez wrote:
>
> > I've never been too happy with the too large due to cast warnings. For
> that
> > matter, it seems like a lot of the unbounded alloca warning variants were
> > artifacts of the way we couldn't get precise ranges after vrp asserts had
> > disappeared and we were trying to guess at what the actual range in the
> > original code was. It's fragile at best.
>
> Yes, very fragile.
>
> > I haven't been paying too much attention to walloca because the ranger
> gets
> > considerably better context ranges in the ranger walloca version, and we
> > are getting correct warnings for a variety of things we couldn't before.
> So
> > I was hoping to ignore this until we all agreed on what range, vrp etc
> will
> > look like going forward.
>
> Seems sensible.
>
> > That being said, I could take a closer look at this xfail on Monday if
> > y'all would like. But I don't currently have strong opinions either way.
> I
> > guess it'll all change in the next few months.
>
> As long as you are ok with one Walloca testcase being xfailed until the
> VRP work lands, I don't think there is a need to spend time on it now.
>
> --
> Marc Glisse
>

Re: [PATCH] PR71482: Add -Wglobal-constructors

[Marek Polacek]

On Thu, May 30, 2019 at 05:28:47PM -0600, Martin Sebor wrote:
> On 5/28/19 3:30 PM, Sean Gillespie wrote:
> > This adds a new warning, -Wglobal-constructors, that warns whenever a
> > decl requires a global constructor or destructor. This new warning fires
> > whenever a thread_local or global variable is declared whose type has a
> > non-trivial constructor or destructor. When faced with both a constructor
> > and a destructor, the error message mentions the destructor and is only
> > fired once.
> > 
> > This warning mirrors the Clang option -Wglobal-constructors, which warns
> > on the same thing. -Wglobal-constructors was present in Apple's GCC and
> > later made its way into Clang.
> > 
> > Bootstrapped and regression-tested on x86-64 linux, new tests passing.
> 
> I can't tell from the Clang online manual:
> 
> Is the warning meant to trigger just for globals, or for all
> objects with static storage duration regardless of scope (i.e.,
> including namespace-scope objects and static locals and static
> class members)?
> 
> "Requires a constructor to initialize" doesn't seem very clear
> to me.  Is the warning intended to trigger for objects that
> require dynamic initialization?  If so, then what about dynamic
> intialization of objects of trivial types, such as this:
> 
>   static int i = std::string ("foo").length ();
> 
> or even
> 
>   static int j = strlen (getenv ("TMP"));

The warning is not meant to diagnose these.  But I do agree that the
documentation for the new warning should be improved.

> If these aren't meant to be diagnosed then the description should
> make it clear (the first one involves a ctor, the second one does
> not).  But I would think that if the goal is to find sources of
> dynamic initialization then diagnosing the above would be useful.
> If so, the description should make it clear and tests verifying
> that it works should be added.
> 
> Martin
> 
> PS Dynamic initialization can be optimized into static
> initialization, even when it involves a user-defined constructor.
> If the purpose of the warning is to find objects that are
> dynamically initialized in the sense of the C++ language then
> implementing it in the front-end is sufficient.  But if the goal

My sense is that this is what we're doing here.  So the patch seems reasonable
to me.  I suspect we'll have to tweak the warning a little bit, but overall it
looks fine to me.  As always, there's room to expand and improve, but the
warning looks useful to me as-is.

Marek

Re: Simplify more EXACT_DIV_EXPR comparisons

[Marc Glisse]

On Fri, 31 May 2019, Aldy Hernandez wrote:


I've never been too happy with the too large due to cast warnings. For that
matter, it seems like a lot of the unbounded alloca warning variants were
artifacts of the way we couldn't get precise ranges after vrp asserts had
disappeared and we were trying to guess at what the actual range in the
original code was. It's fragile at best.


Yes, very fragile.


I haven't been paying too much attention to walloca because the ranger gets
considerably better context ranges in the ranger walloca version, and we
are getting correct warnings for a variety of things we couldn't before. So
I was hoping to ignore this until we all agreed on what range, vrp etc will
look like going forward.


Seems sensible.


That being said, I could take a closer look at this xfail on Monday if
y'all would like. But I don't currently have strong opinions either way. I
guess it'll all change in the next few months.


As long as you are ok with one Walloca testcase being xfailed until the 
VRP work lands, I don't think there is a need to spend time on it now.


--
Marc Glisse

Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Jeff Law]

On 5/30/19 4:56 PM, Martin Sebor wrote:
> On 5/30/19 10:15 AM, Jeff Law wrote:
>> On 5/30/19 9:34 AM, Martin Sebor wrote:
>>
> If the alias oracle can be used to give the same results without
> excessive false positives then I think it would be fine to make
> use of it.  Is that something you consider a prerequisite for
> this change or should I look into it as a followup?
 I think we should explore it a bit before making a final decision.  It
 may guide us for other work in this space (like detecting escaping
 locals).   I think a dirty prototype to see if it's even in the right
 ballpark would make sense.
>>>
>>> Okay, let me look into it.
>> Sounds good.  Again, go with a quick prototype to see if it's likely
>> feasible.  The tests you've added should dramatically help evaluating if
>> the oracle is up to the task.
> 
> So to expand on what I said on the phone when we spoke: the problem
> I quickly ran into with the prototype is that I wasn't able to find
> a way to identify pointers to alloca/VLA storage.
Your analysis matches my very quick read of the aliasing code.  It may
be the case that the Steensgaard patent got in the way here.


> 
> In the the points-to solution for the pointer being returned they
> both have the vars_contains_escaped_heap flag set.  That seems like
> an omission that shouldn't be hard to fix, but on its own, I don't
> think it would be sufficient.
RIght.  In theory the result of an alloca call shouldn't alias anything
in the heap -- but there were some implementations of alloca that were
built on top of malloc (ugh).  That flag may be catering to that case.

But in the case of a __builtin_alloca that shouldn't apply.  Hmm.  That
ultimately might be a bug.



> 
> In the IL a VLA is represented as a pointer to an array, but when
> returning a pointer into a VLA (at some offset so it's an SSA_NAME),
> the pointer's point-to solution doesn't include the VLA pointer or
> (AFAICS) make it possible to tell even that it is a VLA.  For example
> here:
> 
>   f (int n)
>   {
>     int * p;
>     int[0:D.1912] * a.1;
>     sizetype _1;
>     void * saved_stack.3_3;
>     sizetype _6;
> 
>      [local count: 1073741824]:
>     saved_stack.3_3 = __builtin_stack_save ();
>     _1 = (sizetype) n_2(D);
>     _6 = _1 * 4;
>     a.1_8 = __builtin_alloca_with_align (_6, 32);
>     p_9 = a.1_8 + _6;
>     __builtin_stack_restore (saved_stack.3_3);
>     return p_9;
>   }
> 
> p_9's solution's is:
> 
>   p_9, points-to vars: { D.1925 } (escaped, escaped heap)
> 
> I couldn't find out how to determine that D.1925 is a VLA (or even
> what it is).  It's not among the function's local variables that
> FOR_EACH_LOCAL_DECL iterates over.
It's possible that decl was created internally as part of the alias
oracle's analysis.

See make_heapvar in tree-ssa-structalias.c
> 
> By replacing the VLA in the test case with a call to malloc I get
> this for the returned p_7:
> 
>   p_7, points-to NULL, points-to vars: { D.1916 } (escaped, escaped heap)
> 
> Again, I see no way to quickly tell that this pointer points into
> the block returned from malloc.
If there's a way to make that determination it'd have to be on the
variable since the pt_solution flag bits don't carry a storage class
directly.

You might try to get a handle on those decls and dump them to see if
there's anything easily identifiable.  But it may be easier to dig into
the code that creates them.  A real quick scan of the aliasing code also
shows the "variable_info" structure.  It's private to the aliasing code,
but might guide you at things to look at.

Regardless, I don't see an immediate path forward  using the oracle to
identify objects in the stack for your patch.  WHich is unfortunate.




Jeff

Re: [PATCH][AArch64] Increase default function alignment

[Steve Ellcey]

On Fri, 2019-05-31 at 11:52 +, Wilco Dijkstra wrote:
> With -mcpu=generic the function alignment is currently 8, however almost all
> supported cores prefer 16 or higher, so increase the default to 16:12.
> This gives ~0.2% performance increase on SPECINT2017, while codesize is 0.12%
> larger.
> 
> ChangeLog:
> 2019-05-31  Wilco Dijkstra  
> 
>   * config/aarch64/aarch64.c (generic_tunings): Set function
> alignment to 16.
> 
> --
> 
> diff --git a/gcc/config/aarch64/aarch64.c
> b/gcc/config/aarch64/aarch64.c
> index
> 0023cb37bbae5afe9387840c1bb6b43586d4fac2..ed1422af6aab5e3c6eeea37ec57
> e69b64092a0ab 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -693,7 +693,7 @@ static const struct tune_params generic_tunings =
>4, /* memmov_cost  */
>2, /* issue_rate  */
>(AARCH64_FUSE_AES_AESMC), /* fusible_ops  */
> -  "8",   /* function_align.  */
> +  "16:12",   /* function_align.  */
>"4",   /* jump_align.  */
>"8",   /* loop_align.  */
>2, /* int_reassoc_width.  */

I have no objection to the change but could the commit message and/or
comments be expanded to explain the ':12' part of this value.  I
couldn't find an explanation for it in the code and I don't understand
what it does.

Steve Ellcey
sell...@marvell.com

Re: [PATCH] fixincludes breaks mingw64 build

[Jonas Jelten]

On 31/05/2019 16.38, Jonathan Wakely wrote:
> On 31/05/19 15:35 +0200, Jonas Jelten wrote:
>> Hi!
>>
>> I'm trying to build the x86_64-w64-mingw64 crosscompiler on gentoo.
>> It breaks because a fixincludes-fix is applied at a place where it should 
>> not be applied.
>>
>> This broke the cross-gcc build for gcc-8.3.0 and 9.1.0 with error messages 
>> that made it tricky to figure out what was
>> really going on.
> 
> [snip]
> 
> N.B. The gcc-bugs mailing list is for automated email from Bugzilla,
> not for reporting bugs. Sending the patch to gcc-patches (and CCing a
> relevant maintainer) is correct, but there's no need (and no point) to
> also send it to gcc-bugs.
> 
> 


Alright, thanks for the hint. I was just following the instructions in 
gcc-9.1.0/fixincludes/README:

"""
Please also send relevant information to gcc-b...@gcc.gnu.org, 
gcc-patches@gcc.gnu.org and,
please, to me: bk...@gnu.org.
"""

-- JJ

Re: [PATCH] A jump threading opportunity for condition branch

[Jeff Law]

On 5/31/19 1:24 AM, Richard Biener wrote:
> On Thu, 30 May 2019, Jeff Law wrote:
> 
>> On 5/30/19 12:41 AM, Richard Biener wrote:
>>> On May 29, 2019 10:18:01 PM GMT+02:00, Jeff Law  wrote:
 On 5/23/19 6:11 AM, Richard Biener wrote:
> On Thu, 23 May 2019, Jiufu Guo wrote:
>
>> Hi,
>>
>> Richard Biener  writes:
>>
>>> On Tue, 21 May 2019, Jiufu Guo wrote:

 +}
 +
 +  if (TREE_CODE_CLASS (gimple_assign_rhs_code (def)) !=
 tcc_comparison)
 +return false;
 +
 +  /* Check if phi's incoming value is defined in the incoming
 basic_block.  */
 +  edge e = gimple_phi_arg_edge (phi, index);
 +  if (def->bb != e->src)
 +return false;
>>> why does this matter?
>>>
>> Through preparing pathes and duplicating block, this transform can
 also
>> help to combine a cmp in previous block and a gcond in current
 block.
>> "if (def->bb != e->src)" make sure the cmp is define in the incoming
>> block of the current; and then combining "cmp with gcond" is safe. 
 If
>> the cmp is defined far from the incoming block, it would be hard to
>> achieve the combining, and the transform may not needed.
> We're in SSA form so the "combining" doesn't really care where the
> definition comes from.
 Combining doesn't care, but we need to make sure the copy of the
 conditional ends up in the right block since it wouldn't necessarily be
 associated with def->bb anymore.  But I'd expect the sinking pass to
 make this a non-issue in practice anyway.

>
 +
 +  if (!single_succ_p (def->bb))
 +return false;
>>> Or this?  The actual threading will ensure this will hold true.
>>>
>> Yes, other thread code check this and ensure it to be true, like
>> function thread_through_normal_block. Since this new function is
 invoked
>> outside thread_through_normal_block, so, checking single_succ_p is
 also
>> needed for this case.
> I mean threading will isolate the path making this trivially true.
> It's also no requirement for combining, in fact due to the single-use
> check the definition can be sinked across the edge already (if
> the edges dest didn't have multiple predecessors which this threading
> will fix as well).
 I don't think so.  The CMP source block could end with a call and have
 an abnormal edge (for example).  We can't put the copied conditional
 before the call and putting it after the call essentially means
 creating
 a new block.

 The CMP source block could also end with a conditional.  Where do we
 put
 the one we want to copy into the CMP source block in that case? :-)

 This is something else we'd want to check if we ever allowed the the
 CMP
 defining block to not be the immediate predecessor of the conditional
 jump block.  If we did that we'd need to validate that the block where
 we're going to insert the copy of the jump has a single successor.
>>>
>>> But were just isolating a path here. The actual combine job is left to 
>>> followup cleanups. 
>> Absolutely agreed.  My point was that there's some additional stuff we'd
>> have to verify does the right thing if we wanted to allow the CMP to be
>> somewhere other than in the immediate predecessor of the conditional
>> jump block.
> 
> For correctness?  No.  For the CMP to be forwarded?  No.  For optimality
> maybe - forwarding a binary operation always incurs register pressure
> increase.
For correctness of the patch.  Conceptually I have _no_ issues with
having the CMP in a different block than an immediate predecessor of the
conditional jump block.  But the patch does certain code which would
need to be audited with that change in mind.

> 
> Btw, as you already said sinking should have sinked the CMP to the
> predecessor (since we have a single use in the PHI).
> 
> So I hardly see the point of making this difference.
:-)

jeff

[PATCH] Add noexcept to tuple<> and simplify tuple noexcept-specifiers

[Jonathan Wakely]

* include/std/tuple (tuple<>): Add noexcept to allocator-extended
constructors.
(tuple::__nothrow_default_constructible()): New helper
function.
(tuple::tuple(), explicit tuple::tuple()): Use helper.

Tested powerpc64le-linux, committed to trunk.

commit d099e922a9b5ca48bed5a1fa3fc5977dbdb0f070
Author: Jonathan Wakely 
Date:   Fri May 31 11:52:47 2019 +0100

Add noexcept to tuple<> and simplify tuple noexcept-specifiers

* include/std/tuple (tuple<>): Add noexcept to allocator-extended
constructors.
(tuple::__nothrow_default_constructible()): New helper
function.
(tuple::tuple(), explicit tuple::tuple()): Use 
helper.

diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index b81157c097b..64516b1dece 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -898,9 +898,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   tuple() = default;
   // No-op allocator constructors.
   template
-   tuple(allocator_arg_t, const _Alloc&) { }
+   tuple(allocator_arg_t, const _Alloc&) noexcept { }
   template
-   tuple(allocator_arg_t, const _Alloc&, const tuple&) { }
+   tuple(allocator_arg_t, const _Alloc&, const tuple&) noexcept { }
 };
 
   /// Partial specialization, 2-element tuple.
@@ -925,11 +925,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
 
   template
-static constexpr bool __nothrow_constructible()
-{
-  return __and_,
-   is_nothrow_constructible<_T2, _U2>>::value;
-}
+   static constexpr bool __nothrow_constructible()
+   {
+ return __and_,
+   is_nothrow_constructible<_T2, _U2>>::value;
+   }
+
+  static constexpr bool __nothrow_default_constructible()
+  {
+   return __and_,
+ is_nothrow_default_constructible<_T2>>::value;
+  }
 
 public:
   template >
::value, bool>::type = true>
constexpr tuple()
-   noexcept(__and_,
-   is_nothrow_default_constructible<_T2>>::value)
+   noexcept(__nothrow_default_constructible())
: _Inherited() { }
 
   template >>>
   ::value, bool>::type = false>
explicit constexpr tuple()
-   noexcept(__and_,
-   is_nothrow_default_constructible<_T2>>::value)
+   noexcept(__nothrow_default_constructible())
: _Inherited() { }
 
   // Shortcut for the cases where constructors taking _T1, _T2
@@ -1108,7 +1112,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  && _TCC<_Dummy>::template
_ImplicitlyConvertibleTuple<_T1, _T2>(),
bool>::type=true>
-
tuple(allocator_arg_t __tag, const _Alloc& __a,
  const _T1& __a1, const _T2& __a2)
: _Inherited(__tag, __a, __a1, __a2) { }

Re: undefined behavior in value_range::equiv_add()?

[Jeff Law]

On 5/31/19 3:00 AM, Richard Biener wrote:
> On Fri, May 31, 2019 at 2:27 AM Jeff Law  wrote:
>>
>> On 5/29/19 10:20 AM, Aldy Hernandez wrote:
>>> On 5/29/19 12:12 PM, Jeff Law wrote:
 On 5/29/19 9:58 AM, Aldy Hernandez wrote:
> On 5/29/19 9:24 AM, Richard Biener wrote:
>> On Wed, May 29, 2019 at 2:18 PM Aldy Hernandez 
>> wrote:
>>>
>>> As per the API, and the original documentation to value_range,
>>> VR_UNDEFINED and VR_VARYING should never have equivalences.  However,
>>> equiv_add is tacking on equivalences blindly, and there are various
>>> regressions that happen if I fix this oversight.
>>>
>>> void
>>> value_range::equiv_add (const_tree var,
>>>   const value_range *var_vr,
>>>   bitmap_obstack *obstack)
>>> {
>>>  if (!m_equiv)
>>>m_equiv = BITMAP_ALLOC (obstack);
>>>  unsigned ver = SSA_NAME_VERSION (var);
>>>  bitmap_set_bit (m_equiv, ver);
>>>  if (var_vr && var_vr->m_equiv)
>>>bitmap_ior_into (m_equiv, var_vr->m_equiv);
>>> }
>>>
>>> Is this a bug in the documentation / API, or is equiv_add incorrect
>>> and
>>> we should fix the fall-out elsewhere?
>>
>> I think this must have been crept in during the classification.  If you
>> go back to say GCC 7 you shouldn't see value-ranges with
>> UNDEFINED/VARYING state in the lattice that have equivalences.
>>
>> It may not be easy to avoid with the new classy interface but we're
>> certainly not tacking on them "blindly".  At least we're not supposed
>> to.  As usual the intermediate state might be "broken" but
>> intermediateness is not sth the new class "likes".
>
> It looks like extract_range_from_stmt (by virtue of
> vrp_visit_assignment_or_call and then extract_range_from_ssa_name)
> returns one of these intermediate ranges.  It would seem to me that an
> outward looking API method like vr_values::extract_range_from_stmt
> shouldn't be returning inconsistent ranges.  Or are there no guarantees
> for value_ranges from within all of vr_values?
 ISTM that if we have an implementation constraint that says a VR_VARYING
 or VR_UNDEFINED range can't have equivalences, then we need to honor
 that at the minimum for anything returned by an external API.  Returning
 an inconsistent state is bad.  I'd even state that we should try damn
 hard to avoid it in internal APIs as well.
>>>
>>> Agreed * 2.
>>>

>
> Perhaps I should give a little background.  As part of your
> value_range_base re-factoring last year, you mentioned that you didn't
> split out intersect like you did union because of time or oversight.  I
> have code to implement intersect (attached), for which I've noticed that
> I must leave equivalences intact, even when transitioning to
> VR_UNDEFINED:
>
> [from the attached patch]
> +  /* If THIS is varying we want to pick up equivalences from OTHER.
> + Just special-case this here rather than trying to fixup after the
> + fact.  */
> +  if (this->varying_p ())
> +this->deep_copy (other);
> +  else if (this->undefined_p ())
> +/* ?? Leave any equivalences already present in an undefined.
> +   This is technically not allowed, but we may get an in-flight
> +   value_range in an intermediate state.  */
 Where/when does this happen?
>>>
>>> The above snippet is not currently in mainline.  It's in the patch I'm
>>> proposing to clean up intersect.  It's just that while cleaning up
>>> intersect I noticed that if we keep to the value_range API, we end up
>>> clobbering an equivalence to a VR_UNDEFINED that we depend up further up
>>> the call chain.
>>>
>>> The reason it doesn't happen in mainline is because intersect_helper
>>> bails early on an undefined, thus leaving the problematic equivalence
>>> intact.
>>>
>>> You can see it in mainline though, with the following testcase:
>>>
>>> int f(int x)
>>> {
>>>   if (x != 0 && x != 1)
>>> return -2;
>>>
>>>   return !x;
>>> }
>>>
>>> Break in evrp_range_analyzer::record_ranges_from_stmt() and see that the
>>> call to extract_range_from_stmt() returns a VR of undefined *WITH*
>>> equivalences:
>>>
>>>   vr_values->extract_range_from_stmt (stmt, _edge, , );
>>>
>>> This VR is later fed to update_value_range() and ultimately intersect(),
>>> which in mainline, leaves the equivalences alone, but IMO should respect
>>> that API and nuke them.
>> So I think this is coming from extract_range_from_ssa_name:
>>
>>> void
>>> vr_values::extract_range_from_ssa_name (value_range *vr, tree var)
>>> {
>>>   value_range *var_vr = get_value_range (var);
>>>
>>>   if (!var_vr->varying_p ())
>>> vr->deep_copy (var_vr);
>>>   else
>>> vr->set (var);
>>>
>>>   vr->equiv_add (var, get_value_range (var), _equiv_obstack);
>>> }
>>
>> Where we 

Re: [PATCH] Make debug(edge) more verbose.

[Jeff Law]

On 5/31/19 3:23 AM, Martin Liška wrote:
> Hi.
> 
> With the patch one can see more info during debugging:
> 
> (gdb) pp
>  6)>
>  5 [11.0% (guessed)]  count:105119324 (estimated locally) 
> (FALLTHRU,CAN_FALLTHRU,LOOP_EXIT)
> 
> Ready for trunk?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2019-05-31  Martin Liska  
> 
>   * cfg.c (debug): Use TDF_DETAILS for debug and
>   print edge info only once.
> ---
>  gcc/cfg.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> 
OK
jeff

Re: [PATCH] fixincludes breaks mingw64 build

[Jonathan Wakely]

On 31/05/19 15:35 +0200, Jonas Jelten wrote:

Hi!

I'm trying to build the x86_64-w64-mingw64 crosscompiler on gentoo.
It breaks because a fixincludes-fix is applied at a place where it should not 
be applied.

This broke the cross-gcc build for gcc-8.3.0 and 9.1.0 with error messages that 
made it tricky to figure out what was
really going on.


[snip]

N.B. The gcc-bugs mailing list is for automated email from Bugzilla,
not for reporting bugs. Sending the patch to gcc-patches (and CCing a
relevant maintainer) is correct, but there's no need (and no point) to
also send it to gcc-bugs.

Re: RFC: [PATCH] Remove using-declarations that add std names to __gnu_cxx

[Jonathan Wakely]

On 31/05/19 11:36 +0100, Jonathan Wakely wrote:

On 29/05/19 21:00 +0100, Jonathan Wakely wrote:

These using-declarations appear to have been added for simplicity when
moving the non-standard extensions from namespace std to namespace
__gnu_cxx. Dumping all these names into namespace __gnu_cxx allows
uses like __gnu_cxx::size_t and __gnu_cxx::pair, which serve no useful
purpose, but allows creating unnecessarily unportable code.

This patch removes most of the using-declarations from namespace scope,
then either qualifies names as needed or adds using-declarations at
block scope or typedefs at class scope.

* include/backward/hashtable.h (size_t, ptrdiff_t)
(forward_iterator_tag, input_iterator_tag, _Construct, _Destroy)
(distance, vector, pair, __iterator_category): Remove
using-declarations that add these names to namespace __gnu_cxx.
* include/ext/bitmap_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/debug_allocator.h (size_t): Likewise.
* include/ext/functional (size_t, unary_function, binary_function)
(mem_fun1_t, const_mem_fun1_t, mem_fun1_ref_t, const_mem_fun1_ref_t):
Likewise.
* include/ext/malloc_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/memory (ptrdiff_t, pair, __iterator_category): Likewise.
* include/ext/mt_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/new_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/numeric (iota): Fix outdated comment.
* include/ext/pool_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/rb_tree (_Rb_tree, allocator): Likewise.
* include/ext/rope (size_t, ptrdiff_t, allocator, _Destroy): Likewise.
* include/ext/ropeimpl.h (size_t, printf, basic_ostream)
(__throw_length_error, _Destroy, std::__uninitialized_fill_n_a):
Likewise.
* include/ext/slist (size_t, ptrdiff_t, _Construct, _Destroy)
(allocator, __true_type, __false_type): Likewise.

Does anybody think we should keep __gnu_cxx::size_t,
__gnu_cxx::input_iterator_tag, __gnu_cxx::vector, __gnu_cxx::pair etc.
or should I go ahead and commit this?


Committed to trunk.


And this fix is needed to fix an AIX bootstrap failure.

Committed to trunk.


commit e49b5946d09205e57a60a84d764b77a1d2d09a83
Author: Jonathan Wakely 
Date:   Fri May 31 14:58:15 2019 +0100

Fix breakage due to removing __gnu_cxx::size_t declaration

Restore the using-declaration but locally in the source file, not in the
header.

* src/c++98/bitmap_allocator.cc: Add using-declaration for size_t.

diff --git a/libstdc++-v3/src/c++98/bitmap_allocator.cc b/libstdc++-v3/src/c++98/bitmap_allocator.cc
index d22bf4e118f..ffaea1cbac9 100644
--- a/libstdc++-v3/src/c++98/bitmap_allocator.cc
+++ b/libstdc++-v3/src/c++98/bitmap_allocator.cc
@@ -28,6 +28,8 @@ namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
+  using std::size_t;
+
   namespace __detail
   {
 template class __mini_vector<

[PATCH] Fix typo in index comporasion of CONSTRUCTOR.

[Martin Liška]

Hi.

I've been working on an overhaul of IPA ICF and I noticed that issue.

Looks to me obvious.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-05-31  Martin Liska  

* fold-const.c (operand_equal_p): Fix typo as compare_tree_int
returns 0 when operands are equal.
---
 gcc/fold-const.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 42289013cc1..3e066a2ecc4 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -3508,10 +3508,10 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 		 positives for GENERIC.  */
 		  || (c0->index
 		  && (TREE_CODE (c0->index) != INTEGER_CST 
-			  || !compare_tree_int (c0->index, i)))
+			  || compare_tree_int (c0->index, i)))
 		  || (c1->index
 		  && (TREE_CODE (c1->index) != INTEGER_CST 
-			  || !compare_tree_int (c1->index, i
+			  || compare_tree_int (c1->index, i
 		return 0;
 	}
 	  return 1;

[PATCH] fixincludes breaks mingw64 build

[Jonas Jelten]

Hi!

I'm trying to build the x86_64-w64-mingw64 crosscompiler on gentoo.
 It breaks because a fixincludes-fix is applied at a place where it should not 
be applied.

This broke the cross-gcc build for gcc-8.3.0 and 9.1.0 with error messages that 
made it tricky to figure out what was
really going on.


This code snippet of mingw's stdio.h:


#ifdef _WIN64
  _CRTIMP FILE *__cdecl __iob_func(void);
#define _iob  __iob_func()
#else
[.]
#if (!defined(NO_OLDNAMES) || defined(__GNUC__))
  __MINGW_EXTENSION typedef __int64 fpos_t;
#define _FPOSOFF(fp) ((long)(fp))


= is modified by fixincludes to be:


#ifdef _WIN64
  _CRTIMP FILE *__cdecl __iob_func(void);
#define _iob  __iob

#  if defined(__STDC__) || defined(__cplusplus)
 extern int snprintf(char *, size_t, const char *, ...);
 extern int vsnprintf(char *, size_t, const char *, __gnuc_va_list);
#  else /* not __STDC__) || __cplusplus */
 extern int snprintf();
 extern int vsnprintf();
#  endif /* __STDC__) || __cplusplus */
_func()
#else
[]
#if (!defined(NO_OLDNAMES) || defined(__GNUC__))
  __MINGW_EXTENSION typedef __int64 fpos_t;
#define _FPOSOFF(fp) ((long)(fp))


which leads to this compiler error:

In file included from /usr/x86_64-w64-mingw64/sys-include/_mingw.h:12,
 from /usr/x86_64-w64-mingw64/sys-include/crtdefs.h:10,
 from /usr/x86_64-w64-mingw64/sys-include/stddef.h:7,
 from 
/tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/build/gcc/include/stddef.h:1,
 from 
/tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/gcc-8.3.0/libgcc/../gcc/tsystem.h:44,
 from 
/tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/gcc-8.3.0/libgcc/libgcc2.c:27:
/tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/build/gcc/include-fixed/stdio.h:
 In function ‘_func’:
/tmp/portage/cross-x86_64-w64-mingw64/gcc-8.3.0-r1/work/build/gcc/include-fixed/stdio.h:136:3:
 error: expected
declaration specifiers before ‘__extension__’
   __MINGW_EXTENSION typedef __int64 fpos_t;
   ^

and many many more follow-up errors.



A simple fix seems to be to check for a line ending in the select-regex:

Author: Jonas Jelten 
Date:   Wed May 31 15:10:52 2019 +0200

fixincludes: check for lineending to not break mingw64-build

--- a/fixincludes/inclhack.def  2019-05-31 15:01:31.841235934 +0200
+++ b/fixincludes/inclhack.def  2019-05-31 15:09:31.068347492 +0200
@@ -2163,7 +2163,7 @@
 fix = {
 hackname = hpux10_stdio_declarations;
 files= stdio.h;
-select   = "^#[ \t]*define _iob[ \t]*__iob";
+select   = "^#[ \t]*define _iob[ \t]*__iob[ \t]*\n";
 bypass   = "^[ \t]*extern[ \t]*int[ \t]*vsnprintf[ \t]*\\(";
 c_fix = format;
 c_fix_arg = "%0\n\n"
--- a/fixincludes/fixincl.x 2019-05-31 15:13:20.430156243 +0200
+++ b/fixincludes/fixincl.x 2019-05-31 15:13:24.953156662 +0200
@@ -4272,7 +4272,7 @@
  *  content selection pattern - do fix if pattern found
  */
 tSCC zHpux10_Stdio_DeclarationsSelect0[] =
-   "^#[ \t]*define _iob[ \t]*__iob";
+   "^#[ \t]*define _iob[ \t]*__iob[ \t]*\n";

 /*
  *  content bypass pattern - skip fix if pattern found



Cheers

-- Jonas

[C++PATCH] Lambda names are anonymous

[Nathan Sidwell]

Lambda types have names that are anonymous for all intents and purposes, 
but sometimes need to be distinguished from other unnamed types.  We 
currently create lambda names specially.  This patch changes that to use 
the anon-name machinery, and then set another spare bit on that 
identifier to mark it specially.


The 'LAMBDA_TYPE_P' macro can accept any node (not just types), but that 
(a) is different to the TYPE_ANON_P and other such macros, and (b) 
nearly all use cases fall into:

i) we know we have a type,
ii) we're doing LAMBDA_TYPE_P (DECL_CONTEXT (thing))

This suggests replacing LAMBDA_TYPE_P with:
1) TYPE_LAMBDA_P for cases we have a type
2) DECL_LAMBDA_SCOPE_P for the context case (mirroring DECL_CLASS_SCOPE_P)

I'll do that in a later patch.

nathan

--
Nathan Sidwell
2019-05-31  Nathan Sidwell  

	* cp-tree.h (IDENTIFIER_LAMBDA_P): New.
	(TYPE_ANON_P): New.
	(LAMBDA_TYPE_P, TYPE_UNNAMED_P):  Likewise.
	(LAMBDANAME_PREFIX, LAMBDANAME_FORMAT): Delete.
	(make_lambda_name): Don't declare.
	* error.c (dump_aggr_type): Check for lambdas before other
	anonymous names.
	* lambda.c (begin_lambda_type): Use make_anon_name.
	* cp-lang.c (cxx_dwarf_name): Lambda names smell anonymous.
	* mangle.c (write_local_name): Likewise.
	* name-lookup.c (lambda_cnt, make_lambda_name): Delete.

Index: gcc/cp/cp-lang.c
===
--- gcc/cp/cp-lang.c	(revision 271809)
+++ gcc/cp/cp-lang.c	(working copy)
@@ -110,6 +110,5 @@ cxx_dwarf_name (tree t, int verbosity)
   gcc_assert (DECL_P (t));
 
-  if (DECL_NAME (t)
-  && (IDENTIFIER_ANON_P (DECL_NAME (t)) || LAMBDA_TYPE_P (t)))
+  if (DECL_NAME (t) && IDENTIFIER_ANON_P (DECL_NAME (t)))
 return NULL;
   if (verbosity >= 2)
Index: gcc/cp/cp-tree.h
===
--- gcc/cp/cp-tree.h	(revision 271809)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -1298,7 +1298,14 @@ struct GTY (()) tree_trait_expr {
 };
 
+/* Identifiers used for lambda types are almost anonymous.  Use this
+   spare flag to distinguish them (they also have the anonymous flag).  */
+#define IDENTIFIER_LAMBDA_P(NODE) \
+  (IDENTIFIER_NODE_CHECK(NODE)->base.protected_flag)
+
 /* Based off of TYPE_UNNAMED_P.  */
-#define LAMBDA_TYPE_P(NODE) \
-  (CLASS_TYPE_P (NODE) && CLASSTYPE_LAMBDA_EXPR (NODE))
+#define LAMBDA_TYPE_P(NODE)			\
+  (TREE_CODE (NODE) == RECORD_TYPE		\
+   && TYPE_LINKAGE_IDENTIFIER (NODE)\
+   && IDENTIFIER_LAMBDA_P (TYPE_LINKAGE_IDENTIFIER (NODE)))
 
 /* Test if FUNCTION_DECL is a lambda function.  */
@@ -1936,7 +1943,13 @@ enum languages { lang_c, lang_cplusplus
 #define TYPE_NAME_LENGTH(NODE) (IDENTIFIER_LENGTH (TYPE_IDENTIFIER (NODE)))
 
-/* Nonzero if NODE has no name for linkage purposes.  */
-#define TYPE_UNNAMED_P(NODE) \
-  (OVERLOAD_TYPE_P (NODE) && IDENTIFIER_ANON_P (TYPE_LINKAGE_IDENTIFIER (NODE)))
+/* Any kind of anonymous type.  */
+#define TYPE_ANON_P(NODE)	\
+  (TYPE_LINKAGE_IDENTIFIER (NODE)\
+   && IDENTIFIER_ANON_P (TYPE_LINKAGE_IDENTIFIER (NODE)))
+
+/* Nonzero if NODE, a TYPE, has no name for linkage purposes.  */
+#define TYPE_UNNAMED_P(NODE)	\
+  (TYPE_ANON_P (NODE)		\
+   && !IDENTIFIER_LAMBDA_P (TYPE_LINKAGE_IDENTIFIER (NODE)))
 
 /* The _DECL for this _TYPE.  */
@@ -5358,7 +5371,4 @@ extern GTY(()) vec *keyed_c
 #endif	/* NO_DOT_IN_LABEL */
 
-#define LAMBDANAME_PREFIX "__lambda"
-#define LAMBDANAME_FORMAT LAMBDANAME_PREFIX "%d"
-
 #define UDLIT_OP_ANSI_PREFIX "operator\"\""
 #define UDLIT_OP_ANSI_FORMAT UDLIT_OP_ANSI_PREFIX "%s"
@@ -6366,5 +6376,4 @@ extern bool note_iteration_stmt_body_sta
 extern void note_iteration_stmt_body_end	(bool);
 extern void determine_local_discriminator	(tree);
-extern tree make_lambda_name			(void);
 extern int decls_match(tree, tree, bool = true);
 extern bool maybe_version_functions		(tree, tree, bool);
Index: gcc/cp/error.c
===
--- gcc/cp/error.c	(revision 271809)
+++ gcc/cp/error.c	(working copy)
@@ -739,12 +739,5 @@ dump_aggr_type (cxx_pretty_printer *pp,
 }
 
-  if (!name || IDENTIFIER_ANON_P (name))
-{
-  if (flags & TFF_CLASS_KEY_OR_ENUM)
-	pp_string (pp, M_(""));
-  else
-	pp_printf (pp, M_(""), variety);
-}
-  else if (LAMBDA_TYPE_P (t))
+  if (LAMBDA_TYPE_P (t))
 {
   /* A lambda's "type" is essentially its signature.  */
@@ -756,6 +749,14 @@ dump_aggr_type (cxx_pretty_printer *pp,
   pp_greater (pp);
 }
+  else if (!name || IDENTIFIER_ANON_P (name))
+{
+  if (flags & TFF_CLASS_KEY_OR_ENUM)
+	pp_string (pp, M_(""));
+  else
+	pp_printf (pp, M_(""), variety);
+}
   else
 pp_cxx_tree_identifier (pp, name);
+
   if (tmplate)
 dump_template_parms (pp, TYPE_TEMPLATE_INFO (t),
Index: gcc/cp/lambda.c
===
--- gcc/cp/lambda.c	(revision 271809)
+++ gcc/cp/lambda.c	(working copy)
@@ -129,20 +129,13 @@ tree
 

Re: [PATCH][RFC] Sanitize equals and hash functions in hash-tables.

[Martin Liška]

On 5/31/19 2:50 PM, Richard Biener wrote:
> On Wed, May 22, 2019 at 11:13 AM Martin Liška  wrote:
>>
>> On 5/21/19 1:51 PM, Richard Biener wrote:
>>> On Tue, May 21, 2019 at 1:02 PM Martin Liška  wrote:

 On 5/21/19 11:38 AM, Richard Biener wrote:
> On Tue, May 21, 2019 at 12:07 AM Jeff Law  wrote:
>>
>> On 5/13/19 1:41 AM, Martin Liška wrote:
>>> On 11/8/18 9:56 AM, Martin Liška wrote:
 On 11/7/18 11:23 PM, Jeff Law wrote:
> On 10/30/18 6:28 AM, Martin Liška wrote:
>> On 10/30/18 11:03 AM, Jakub Jelinek wrote:
>>> On Mon, Oct 29, 2018 at 04:14:21PM +0100, Martin Liška wrote:
 +hashtab_chk_error ()
 +{
 +  fprintf (stderr, "hash table checking failed: "
 +   "equal operator returns true for a pair "
 +   "of values with a different hash value");
>>> BTW, either use internal_error here, or at least if using fprintf
>>> terminate with \n, in your recent mail I saw:
>>> ...different hash valueduring RTL pass: vartrack
>>> ^^
>> Sure, fixed in attached patch.
>>
>> Martin
>>
 +  gcc_unreachable ();
 +}
>>>   Jakub
>>>
>> 0001-Sanitize-equals-and-hash-functions-in-hash-tables.patch
>>
>> From 0d9c979c845580a98767b83c099053d36eb49bb9 Mon Sep 17 00:00:00 
>> 2001
>> From: marxin 
>> Date: Mon, 29 Oct 2018 09:38:21 +0100
>> Subject: [PATCH] Sanitize equals and hash functions in hash-tables.
>>
>> ---
>>  gcc/hash-table.h | 40 +++-
>>  1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/hash-table.h b/gcc/hash-table.h
>> index bd83345c7b8..694eedfc4be 100644
>> --- a/gcc/hash-table.h
>> +++ b/gcc/hash-table.h
>> @@ -503,6 +503,7 @@ private:
>>
>>value_type *alloc_entries (size_t n CXX_MEM_STAT_INFO) const;
>>value_type *find_empty_slot_for_expand (hashval_t);
>> +  void verify (const compare_type , hashval_t hash);
>>bool too_empty_p (unsigned int);
>>void expand ();
>>static bool is_deleted (value_type )
>> @@ -882,8 +883,12 @@ hash_table
>>if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
>>  expand ();
>>
>> -  m_searches++;
>> +#if ENABLE_EXTRA_CHECKING
>> +if (insert == INSERT)
>> +  verify (comparable, hash);
>> +#endif
>>
>> +  m_searches++;
>>value_type *first_deleted_slot = NULL;
>>hashval_t index = hash_table_mod1 (hash, m_size_prime_index);
>>hashval_t hash2 = hash_table_mod2 (hash, m_size_prime_index);
>> @@ -930,6 +935,39 @@ hash_table
>>return _entries[index];
>>  }
>>
>> +#if ENABLE_EXTRA_CHECKING
>> +
>> +/* Report a hash table checking error.  */
>> +
>> +ATTRIBUTE_NORETURN ATTRIBUTE_COLD
>> +static void
>> +hashtab_chk_error ()
>> +{
>> +  fprintf (stderr, "hash table checking failed: "
>> + "equal operator returns true for a pair "
>> + "of values with a different hash value\n");
>> +  gcc_unreachable ();
>> +}
> I think an internal_error here is probably still better than a simple
> fprintf, even if the fprintf is terminated with a \n :-)
 Fully agree with that, but I see a lot of build errors when using 
 internal_error.

> The question then becomes can we bootstrap with this stuff enabled and
> if not, are we likely to soon?  It'd be a shame to put it into
> EXTRA_CHECKING, but then not be able to really use EXTRA_CHECKING
> because we've got too many bugs to fix.
 Unfortunately it's blocked with these 2 PRs:
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87845
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87847
>>> Hi.
>>>
>>> I've just added one more PR:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90450
>>>
>>> I'm sending updated version of the patch that provides a disablement 
>>> for the 3 PRs
>>> with a new function disable_sanitize_eq_and_hash.
>>>
>>> With that I can bootstrap and finish tests. However, I've done that 
>>> with a patch
>>> limits maximal number of checks:
>> So rather than call the disable_sanitize_eq_and_hash, can you have its
>> state set up when you instantiate the object?  It's not a huge deal,
>> just thinking about loud.
>>
>>
>>
>> So how do we want to go forward, particularly the EXTRA_EXTRA checking
>> issue :-)
>
> There is at least one PR where we have a table where 

Re: [PATCH][RFC] Sanitize equals and hash functions in hash-tables.

[Richard Biener]

On Wed, May 22, 2019 at 11:13 AM Martin Liška  wrote:
>
> On 5/21/19 1:51 PM, Richard Biener wrote:
> > On Tue, May 21, 2019 at 1:02 PM Martin Liška  wrote:
> >>
> >> On 5/21/19 11:38 AM, Richard Biener wrote:
> >>> On Tue, May 21, 2019 at 12:07 AM Jeff Law  wrote:
> 
>  On 5/13/19 1:41 AM, Martin Liška wrote:
> > On 11/8/18 9:56 AM, Martin Liška wrote:
> >> On 11/7/18 11:23 PM, Jeff Law wrote:
> >>> On 10/30/18 6:28 AM, Martin Liška wrote:
>  On 10/30/18 11:03 AM, Jakub Jelinek wrote:
> > On Mon, Oct 29, 2018 at 04:14:21PM +0100, Martin Liška wrote:
> >> +hashtab_chk_error ()
> >> +{
> >> +  fprintf (stderr, "hash table checking failed: "
> >> +   "equal operator returns true for a pair "
> >> +   "of values with a different hash value");
> > BTW, either use internal_error here, or at least if using fprintf
> > terminate with \n, in your recent mail I saw:
> > ...different hash valueduring RTL pass: vartrack
> > ^^
>  Sure, fixed in attached patch.
> 
>  Martin
> 
> >> +  gcc_unreachable ();
> >> +}
> >   Jakub
> >
>  0001-Sanitize-equals-and-hash-functions-in-hash-tables.patch
> 
>  From 0d9c979c845580a98767b83c099053d36eb49bb9 Mon Sep 17 00:00:00 
>  2001
>  From: marxin 
>  Date: Mon, 29 Oct 2018 09:38:21 +0100
>  Subject: [PATCH] Sanitize equals and hash functions in hash-tables.
> 
>  ---
>   gcc/hash-table.h | 40 +++-
>   1 file changed, 39 insertions(+), 1 deletion(-)
> 
>  diff --git a/gcc/hash-table.h b/gcc/hash-table.h
>  index bd83345c7b8..694eedfc4be 100644
>  --- a/gcc/hash-table.h
>  +++ b/gcc/hash-table.h
>  @@ -503,6 +503,7 @@ private:
> 
> value_type *alloc_entries (size_t n CXX_MEM_STAT_INFO) const;
> value_type *find_empty_slot_for_expand (hashval_t);
>  +  void verify (const compare_type , hashval_t hash);
> bool too_empty_p (unsigned int);
> void expand ();
> static bool is_deleted (value_type )
>  @@ -882,8 +883,12 @@ hash_table
> if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
>   expand ();
> 
>  -  m_searches++;
>  +#if ENABLE_EXTRA_CHECKING
>  +if (insert == INSERT)
>  +  verify (comparable, hash);
>  +#endif
> 
>  +  m_searches++;
> value_type *first_deleted_slot = NULL;
> hashval_t index = hash_table_mod1 (hash, m_size_prime_index);
> hashval_t hash2 = hash_table_mod2 (hash, m_size_prime_index);
>  @@ -930,6 +935,39 @@ hash_table
> return _entries[index];
>   }
> 
>  +#if ENABLE_EXTRA_CHECKING
>  +
>  +/* Report a hash table checking error.  */
>  +
>  +ATTRIBUTE_NORETURN ATTRIBUTE_COLD
>  +static void
>  +hashtab_chk_error ()
>  +{
>  +  fprintf (stderr, "hash table checking failed: "
>  + "equal operator returns true for a pair "
>  + "of values with a different hash value\n");
>  +  gcc_unreachable ();
>  +}
> >>> I think an internal_error here is probably still better than a simple
> >>> fprintf, even if the fprintf is terminated with a \n :-)
> >> Fully agree with that, but I see a lot of build errors when using 
> >> internal_error.
> >>
> >>> The question then becomes can we bootstrap with this stuff enabled and
> >>> if not, are we likely to soon?  It'd be a shame to put it into
> >>> EXTRA_CHECKING, but then not be able to really use EXTRA_CHECKING
> >>> because we've got too many bugs to fix.
> >> Unfortunately it's blocked with these 2 PRs:
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87845
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87847
> > Hi.
> >
> > I've just added one more PR:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90450
> >
> > I'm sending updated version of the patch that provides a disablement 
> > for the 3 PRs
> > with a new function disable_sanitize_eq_and_hash.
> >
> > With that I can bootstrap and finish tests. However, I've done that 
> > with a patch
> > limits maximal number of checks:
>  So rather than call the disable_sanitize_eq_and_hash, can you have its
>  state set up when you instantiate the object?  It's not a huge deal,
>  just thinking about loud.
> 
> 
> 
>  So how do we want to go forward, particularly the EXTRA_EXTRA checking
>  issue :-)
> >>>
> >>> There is at least one PR where we have a table where elements _in_ the
> >>> table are never 

Re: Do not ask alias subset query when access patch can not extend

[Jan Hubicka]

> > +/* Return true if TYPE is a composite type (i.e. we may apply one of 
> > handled
> > +   components on it).  */
> > +
> > +static bool
> > +access_patch_may_continue_p (tree type)
> 
> 'path', but I think type_has_components_p would be a better fit?

Thanks, looks better indeed!
> 
> > +{
> > +  return AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type)
> > +|| TREE_CODE (type) == COMPLEX_TYPE;
> > +}
> > +
> >  /* Determine if the two component references REF1 and REF2 which are
> > based on access types TYPE1 and TYPE2 and of which at least one is based
> > on an indirect reference may alias.  REF2 is the only one that can
> > @@ -965,6 +1028,7 @@ aliasing_component_refs_p (tree ref1,
> >   if there is no B2 in the tail of path1 and no B1 on the
> >   tail of path2.  */
> >if (compare_type_sizes (TREE_TYPE (ref2), type1) >= 0
> > +  && (access_patch_may_continue_p (TREE_TYPE (ref2)))
> 
> extra parens around the call here and below.
> 
> OK with all these issues fixed.

Thanks. That is leftover of a debug code to compare old and new oracle.
I will be more careful next time.

Honza
> 
> Richard.
> 
> >&& (base1_alias_set == ref2_alias_set
> >|| alias_set_subset_of (base1_alias_set, ref2_alias_set)))
> >  {
> > @@ -974,6 +1038,7 @@ aliasing_component_refs_p (tree ref1,
> >/* If this is ptr vs. decl then we know there is no ptr ... decl path.  
> > */
> >if (!ref2_is_decl
> >&& compare_type_sizes (TREE_TYPE (ref1), type2) >= 0
> > +  && (access_patch_may_continue_p (TREE_TYPE (ref1)))
> >&& (base2_alias_set == ref1_alias_set
> >   || alias_set_subset_of (base2_alias_set, ref1_alias_set)))
> >  {
> > 
> 
> -- 
> Richard Biener 
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Re: Teach same_types_for_tbaa to structurally compare arrays, pointers and vectors

[Richard Biener]

On Wed, 29 May 2019, Jan Hubicka wrote:

> Hi,
> this is a variant of testcase I have comitted. Once Martin implements SRA
> part, we could add next variant that drops -fno-tree-sra.
> 
> It seems odd that constant propagation only happens in fre3.
> I woud expect fre1 to discover this already.
> The IL before fre1 and 3 differs only by:
> 
>  test ()
>  {
>struct foo foo;
>struct bar * barptr.0_1;
>struct foo * fooptr.1_2;
> -  struct bar * barptr.2_3;
> -  int _8;
> +  int _7;
>  
> -   :
> +   [local count: 1073741824]:
>foo.val = 0;
>barptr.0_1 = barptr;
>barptr.0_1->val2 = 123;
>fooptr.1_2 = fooptr;
>*fooptr.1_2 = foo;
> -  barptr.2_3 = barptr;
> -  _8 = barptr.2_3->val2;
> +  _7 = barptr.0_1->val2;
>foo ={v} {CLOBBER};
> -  return _8;
> +  return _7;
>  
>  }
> 
> Why VN is not able to optimize the barptr access and lookup through
> it at once?  It looks that could potentially save some need to re-run
> GVN since it is common to store pointers to memory and use them multiple
> times to access other pointers.

This is because in the first pass we substitute the value of
barptr.2_3 (barptr.0_1) when looking up barptr.2_3->val2 and
since that happens in VNs IL we run into
ao_ref_init_from_vn_reference which does not re-build
a GENERIC tree for the access path but leaves us with NULL
ao_ref.ref -- I suppose we could put the original ref tree
in there, too, even if the pieces are "valueized", it's just
a more imprecise representation of the ref.

That helps this testcase.

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Richard.

2019-05-31  Richard Biener  

* tree-ssa-sccvn.c (ao_ref_init_from_vn_reference): Get original
full reference tree and record in ref->ref.
(vn_reference_lookup_3): Pass in original ref to
ao_ref_init_from_vn_reference.
(vn_reference_lookup): Likewise.
* tree-ssa-sccvn.h (ao_ref_init_from_vn_reference): Adjust prototype.

* gcc.dg/tree-ssa/alias-access-path-1.c: Scan fre1.

Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 271803)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -995,7 +995,7 @@ copy_reference_ops_from_ref (tree ref, v
 bool
 ao_ref_init_from_vn_reference (ao_ref *ref,
   alias_set_type set, tree type,
-  vec ops)
+  vec ops, tree orig_ref)
 {
   vn_reference_op_t op;
   unsigned i;
@@ -1149,7 +1149,7 @@ ao_ref_init_from_vn_reference (ao_ref *r
   if (base == NULL_TREE)
 return false;
 
-  ref->ref = NULL_TREE;
+  ref->ref = orig_ref;
   ref->base = base;
   ref->ref_alias_set = set;
   if (base_alias_set != -1)
@@ -1976,7 +1976,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree
{
  lhs_ref_ok = ao_ref_init_from_vn_reference (_ref,
  get_alias_set (lhs),
- TREE_TYPE (lhs), lhs_ops);
+ TREE_TYPE (lhs), lhs_ops,
+ lhs);
  if (lhs_ref_ok
  && !refs_may_alias_p_1 (ref, _ref, true))
{
@@ -2718,7 +2719,7 @@ vn_reference_lookup (tree op, tree vuse,
  Otherwise preserve the full reference for advanced TBAA.  */
   if (!valuezied_anything
  || !ao_ref_init_from_vn_reference (, vr1.set, vr1.type,
-vr1.operands))
+vr1.operands, op))
ao_ref_init (, op);
   if (! tbaa_p)
r.ref_alias_set = r.base_alias_set = 0;
Index: gcc/tree-ssa-sccvn.h
===
--- gcc/tree-ssa-sccvn.h(revision 271803)
+++ gcc/tree-ssa-sccvn.h(working copy)
@@ -229,7 +229,7 @@ vn_nary_op_t vn_nary_op_insert (tree, tr
 vn_nary_op_t vn_nary_op_insert_pieces (unsigned int, enum tree_code,
   tree, tree *, tree, unsigned int);
 bool ao_ref_init_from_vn_reference (ao_ref *, alias_set_type, tree,
-   vec );
+   vec, tree = NULL_TREE);
 vec vn_reference_operands_for_lookup (tree);
 tree vn_reference_lookup_pieces (tree, alias_set_type, tree,
 vec ,
Index: gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c (revision 271803)
+++ gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c (working copy)
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-fre3 -fno-tree-sra" } */
+/* { dg-options "-O2 -fdump-tree-fre1 -fno-tree-sra" } */
+
 struct foo
 {
   int val;
@@ -18,4 +19,4 @@ test ()
   return barptr->val2;
 }
 
-/* { 

[PATCH][AArch64] Increase default function alignment

[Wilco Dijkstra]

With -mcpu=generic the function alignment is currently 8, however almost all
supported cores prefer 16 or higher, so increase the default to 16:12.
This gives ~0.2% performance increase on SPECINT2017, while codesize is 0.12%
larger.

ChangeLog:
2019-05-31  Wilco Dijkstra  

* config/aarch64/aarch64.c (generic_tunings): Set function alignment to 
16.

--

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
0023cb37bbae5afe9387840c1bb6b43586d4fac2..ed1422af6aab5e3c6eeea37ec57e69b64092a0ab
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -693,7 +693,7 @@ static const struct tune_params generic_tunings =
   4, /* memmov_cost  */
   2, /* issue_rate  */
   (AARCH64_FUSE_AES_AESMC), /* fusible_ops  */
-  "8", /* function_align.  */
+  "16:12", /* function_align.  */
   "4", /* jump_align.  */
   "8", /* loop_align.  */
   2,   /* int_reassoc_width.  */

Re: Hashtable Small size optimization

[Jonathan Wakely]

Re https://gcc.gnu.org/ml/gcc-patches/2018-10/msg00903.html


On 15/10/18 22:46 +0200, François Dumont wrote:

I started considering PR libstdc++/68303.

First thing was to write a dedicated performance test case, it is the 
unordered_small_size.cc I'd like to add with this patch.


Great, more performance tests are always good.

The first runs show a major difference between tr1 and std 
implementations, tr1 being much better:


std::tr1::unordered_set without hash code cached: 1st insert    
   9r    9u    1s  14725920mem    0pf
std::tr1::unordered_set with hash code cached: 1st insert       
8r    9u    0s  14719680mem    0pf
std::unordered_set without hash code cached: 1st insert      
17r   17u    0s  16640080mem    0pf
std::unordered_set with hash code cached: 1st insert      14r   
14u    0s  16638656mem    0pf


I had a look in gdb to find out why and the answer was quite obvious. 
For 20 insertions tr1 implementation bucket count goes through [11, 
23] whereas for std it is [2, 5, 11, 23], so 2 more expensive rehash.


As unordered containers are dedicated to rather important number of 
elements I propose to review the rehash policy with this patch so that 
std also starts at 11 on the 1st insertion. After the patch figures 
are:


std::tr1::unordered_set without hash code cached: 1st insert    
   9r    9u    0s  14725920mem    0pf
std::tr1::unordered_set with hash code cached: 1st insert       
8r    8u    0s  14719680mem    0pf
std::unordered_set without hash code cached: 1st insert      
15r   15u    0s  16640128mem    0pf
std::unordered_set with hash code cached: 1st insert      12r   
12u    0s  16638688mem    0pf


OK, that seems worthwhile then.

Moreover I noticed that performance tests are built with -O2, is that 
intentional ? The std implementation uses more abstractions than tr1, 
looks like building with -O3 optimizes away most of those abstractions 
making tr1 and std implementation much closer:


Yes, I think it's intentional that -O2 is used, because I think
that's the most commonly-used optimisation level. We don't want to 

std::tr1::unordered_set without hash code cached: 1st insert    
   2r    1u    1s  14725952mem    0pf
std::tr1::unordered_set with hash code cached: 1st insert       
2r    1u    0s  14719536mem    0pf
std::unordered_set without hash code cached: 1st insert       
2r    2u    0s  16640064mem    0pf
std::unordered_set with hash code cached: 1st insert       2r    
2u    0s  16638608mem    0pf


Hmm, interesting. I wonder if we can determine what is not being
optimized at -O2, and either tweak the code or improve the compiler.

Note that this patch also rework the alternative rehash policy based 
on powers of 2 so that it also starts with a larger number of bucket 
(16) and respects LWG2156.


Last I had to wider the memory column so that alignment is preserved 
even when memory diff is negative.


Tested under Linux x86_64.

Ok to commit ?


Does this patch still apply cleanly? I think it would be good to
commit it now, early in stage 1.

Re: [PATCH][RFC] final-value replacement from DCE

[Richard Biener]

On Wed, 29 May 2019, Jakub Jelinek wrote:

> On Wed, May 29, 2019 at 09:57:50AM -0600, Jeff Law wrote:
> > > FAIL: gcc.dg/builtin-object-size-1.c execution test
> > > FAIL: gcc.dg/builtin-object-size-5.c scan-assembler-not abort
> 
> I admit I haven't looked at the details here, but wonder if the optimization
> couldn't be done only in the DCE passes post IPA, otherwise we risk
> behavior changes for __builtin_object_size.

We can do that - the first CD-DCE pass is in the loop pipeline though,
_after_ final value replacement.  Looking at the testsuite fallout
it's also clear that doing loop-header copying before final-value
replacement results in better code for some testcases.

So I'm trying turning the first DCE after loop-header copying into
a CD-DCE run, not doing final value replacement before IPA.

The following does that independently, bootstrapped & tested
on x86_64-unknown-linux-gnu.  It will leave

FAIL: gcc.dg/tree-ssa/pr68619-4.c scan-tree-dump optimized "PHI <.*, 39"

because the testcase is totally unclear on who is supposed to
propagate 39 and why.  With CD-DCE there's one PRE opportunity
less because, well, a value is no longer partially redundant.

I hope I catched all dce/cddce dump issues and it just seemed to
me that unifying dce and cd-dce may be a useful cleanup
and just have

  NEXT_PASS (pass_dce, true /* perform control-dependent DCE */)

but not for today...

Not going to apply this separately but only eventually together
with the rest.

Richard.

2019-05-31  Richard Biener  

PR tree-optimization/68619
* passes.def (pass_dce after CH): Turn into pass_cd_dce.

* g++.dg/tree-ssa/copyprop-1.C: Adjust dump scanned.
* gcc.dg/tree-ssa/20030709-2.c: Likewise.
* gcc.dg/tree-ssa/20030808-1.c: Likewise.
* gcc.dg/tree-ssa/20040729-1.c: Likewise.
* gcc.dg/tree-ssa/loop-36.c: Likewise.
* gcc.dg/tree-ssa/ssa-dce-1.c: Likewise.
* gcc.dg/tree-ssa/ssa-dce-2.c:  Likewise.

Index: gcc/passes.def
===
--- gcc/passes.def  (revision 271802)
+++ gcc/passes.def  (working copy)
@@ -231,7 +231,7 @@ along with GCC; see the file COPYING3.
   NEXT_PASS (pass_isolate_erroneous_paths);
   NEXT_PASS (pass_dse);
   NEXT_PASS (pass_reassoc, true /* insert_powi_p */);
-  NEXT_PASS (pass_dce);
+  NEXT_PASS (pass_cd_dce);
   NEXT_PASS (pass_forwprop);
   NEXT_PASS (pass_phiopt, false /* early_p */);
   NEXT_PASS (pass_ccp, true /* nonzero_p */);
Index: gcc/testsuite/g++.dg/tree-ssa/copyprop-1.C
===
--- gcc/testsuite/g++.dg/tree-ssa/copyprop-1.C  (revision 271802)
+++ gcc/testsuite/g++.dg/tree-ssa/copyprop-1.C  (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O -fdump-tree-dce3" } */
+/* { dg-options "-O -fdump-tree-cddce2" } */
 
 /* Verify that we can eliminate the useless conversions to/from
const qualified pointer types
@@ -27,4 +27,4 @@ int foo(Object)
 
 /* Remaining should be two loads.  */
 
-/* { dg-final { scan-tree-dump-times " = \[^\n\]*;" 2 "dce3" } } */
+/* { dg-final { scan-tree-dump-times " = \[^\n\]*;" 2 "cddce2" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/20030709-2.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/20030709-2.c  (revision 271802)
+++ gcc/testsuite/gcc.dg/tree-ssa/20030709-2.c  (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O -fdump-tree-dce3" } */
+/* { dg-options "-O -fdump-tree-cddce2" } */
   
 struct rtx_def;
 typedef struct rtx_def *rtx;
@@ -42,13 +42,13 @@ get_alias_set (t)
 
 /* There should be precisely one load of ->decl.rtl.  If there is
more than, then the dominator optimizations failed.  */
-/* { dg-final { scan-tree-dump-times "->decl\\.rtl" 1 "dce3"} } */
+/* { dg-final { scan-tree-dump-times "->decl\\.rtl" 1 "cddce2"} } */
   
 /* There should be no loads of .rtmem since the complex return statement
is just "return 0".  */
-/* { dg-final { scan-tree-dump-times ".rtmem" 0 "dce3"} } */
+/* { dg-final { scan-tree-dump-times ".rtmem" 0 "cddce2"} } */
   
 /* There should be one IF statement (the complex return statement should
collapse down to a simple return 0 without any conditionals).  */
-/* { dg-final { scan-tree-dump-times "if " 1 "dce3"} } */
+/* { dg-final { scan-tree-dump-times "if " 1 "cddce2"} } */
 
Index: gcc/testsuite/gcc.dg/tree-ssa/20030808-1.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/20030808-1.c  (revision 271802)
+++ gcc/testsuite/gcc.dg/tree-ssa/20030808-1.c  (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O1 -fdump-tree-cddce3" } */
+/* { dg-options "-O1 -fdump-tree-cddce4" } */
   
 extern void abort (void);
 
@@ -33,8 +33,8 @@ delete_dead_jumptables ()
 /* There should be no loads of ->code.  If any 

Re: [patch] Fix segfault caused by spurious constant overflow

[Richard Biener]

On Fri, May 31, 2019 at 11:56 AM Eric Botcazou  wrote:
>
> Hi,
>
> this is a regression present for 32-bit targets on mainline and 9 branch only,
> but the underlying issue dates back from the removal of the TYPE_IS_SIZETYPE
> machinery.  Since the removal of this machinery, there is an internal conflict
> for sizetype: it's an unsigned type so with wrap-around semantics on overflow
> but overflow nevertheless needs to be tracked for it in order to avoid buffer
> overflows for large objects.
>
> The constant folder contains various tricks to that effect and the Ada front-
> end even more so, because VLAs are first-class citizens in the language and
> their lower bound is not necessarily 0.  In particular, you need to be able to
> distinguish large constants in sizetype from small negative constants turned
> into even larger constants, because the latter appear in the length of e.g.:
>
>   type Arr is array (2 .. N) of Long_Long_Integer;
>
> This works when the expressions haven't been too much mangled by the constant
> folder and here we have a counter-example for 32-bit targets.  Starting with:
>
>   (N + 4294967295) * 8
>
> which is the sizetype expression of the size of Arr in bytes, extract_muldiv_1
> distributes the multiplication:
>
>   N * 8 + 4294967288
>
> and, immediately after, fold_plusminus_mult_expr factors it back:
>
>   (N + 536870911) * 8
>
> At this point, there is no way for gigi to tell that it's the expression of a
> simple array with no overflow in sight because the 536870911 constant is large
> but not large enough, so gigi flags it as overflowing for small values of N.
>
> I don't see any other way out than disabling the back-and-forth mathematical
> game played by the constant folder in this case, which very likely brings no
> benefit in practice, hence the proposed fix.
>
> Tested on x86_64-suse-linux, OK for the mainline and 9 branch?

Hmm, ISTR we had such mitigations in place (or have) elsewhere keying
on the most significant bit set instead of power-of-two.  But your case
likely recurses and runs into the extract_multiv limiting to eventually
stop, even for (N + 4) * 8, right?  If so shouldn't we prevent this
even for !TYPE_OVERFLOW_WRAPS?  Also

+ && !(tree_fits_shwi_p (c)
+  && exact_log2 (absu_hwi (tree_to_shwi (c))) > 0))

is better written as

   && exact_log2 (wi::to_wide (c)) > 0

not sure why the sizetype constant for you fits in a signed HWI
or you need to compute its absolute value.  Eventually you
need to use wi::abs(wide_int::from (wi::to_wide (c), TYPE_PRECISION
(TREE_TYPE (c)), SIGNED))
or so.

Thanks,
Richard.

>
> 2019-05-31  Eric Botcazou  
>
> * fold-const.c (extract_muldiv_1) : Do not distribute a
> multiplication by a power-of-two value.
>
>
> 2019-05-31  Eric Botcazou  
>
> * gnat.dg/specs/discr6.ads: New test.
>
> --
> Eric Botcazou

Re: [PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap

[Richard Biener]

On Fri, 31 May 2019, Jakub Jelinek wrote:

> On Fri, May 31, 2019 at 11:43:29AM +0200, Richard Biener wrote:
> > Hmm.  Is it enough to set TREE_THIS_NOTRAP on the ARRAY_REF instead?
> 
> That works too.
> 
> > At least it's documented the ARRAY_REF then isn't the issue.
> > As for conditional vs. unconditional I'm not so sure but automatic
> > vars are excepmted from that anyways?
> 
> Only if they are guaranteed not to have out of bound accesses.
> 
> Anyway, I'll go with this, those ARRAY_REFs indeed should never trap.

Thanks - a lot simpler indeed ;)

Richard.

> 2019-05-31  Jakub Jelinek  
> 
>   * omp-low.c (lower_rec_simd_input_clauses): Set TREE_THIS_NOTRAP on
>   ivar and lvar.
> 
>   * gcc.dg/vect/vect-cond-12.c: New test.
> 
> --- gcc/omp-low.c.jj  2019-05-30 23:19:14.469931759 +0200
> +++ gcc/omp-low.c 2019-05-31 11:52:20.491195088 +0200
> @@ -3728,6 +3728,8 @@ lower_rec_simd_input_clauses (tree new_v
>NULL_TREE, NULL_TREE);
>lvar = build4 (ARRAY_REF, TREE_TYPE (new_var), avar, sctx->lane,
>NULL_TREE, NULL_TREE);
> +  TREE_THIS_NOTRAP (ivar) = 1;
> +  TREE_THIS_NOTRAP (lvar) = 1;
>  }
>if (DECL_P (new_var))
>  {
> --- gcc/testsuite/gcc.dg/vect/vect-cond-12.c.jj   2019-05-31 
> 11:25:33.203577504 +0200
> +++ gcc/testsuite/gcc.dg/vect/vect-cond-12.c  2019-05-31 11:26:58.616174115 
> +0200
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-fopenmp-simd" } */
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target 
> vect_condition } } } */
> +
> +int x;
> +
> +void
> +foo (int *a)
> +{
> +  #pragma omp simd lastprivate (x)
> +  for (int i = 0; i < 1024; ++i)
> +if (a[i])
> +  x = i;
> +}
> 
> 
>   Jakub
> 

-- 
Richard Biener 
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Re: Do not ask alias subset query when access patch can not extend

[Richard Biener]

On Fri, 31 May 2019, Jan Hubicka wrote:

> Hi
> in most common cases we have access paths that ends by read/write of a
> non-composite type.  When aliasing_component_refs_p finds no match in
> the acccess path it asks alias oracle if the one alias set is subset of
> other to figure out whether one access path can be extended to the
> other.
> 
> This is unnecessarily conservatie especially for pointers where we say tru
> for
>  foo->ptr
>  bar->ptr
> where foo and bar are two structures containing pointers of different type.
> 
> This patch bypasses the alis oracle check for non-composite types.  Along with
> yesterday patch and patch to handle pointers in types_same_for_tbaa_p we now
> get:
> 
>   refs_may_alias_p: 3019081 disambiguations, 3318862 queries
>   ref_maybe_used_by_call_p: 7112 disambiguations, 3044693 queries
>   call_may_clobber_ref_p: 817 disambiguations, 817 queries
>   aliasing_component_ref_p: 2259 disambiguations, 17881 queries
>   TBAA oracle: 1421561 disambiguations 2922798 queries
>552636 are in alias set 0
>569791 queries asked about the same object
>0 queries asked about the same alias set
>0 access volatile
>260727 are dependent in the DAG
>118083 are aritificially in conflict with void *
> 
> Compared to:
> 
>   refs_may_alias_p: 3013678 disambiguations, 3314059 queries
>   ref_maybe_used_by_call_p: 7112 disambiguations, 3039278 queries
>   call_may_clobber_ref_p: 817 disambiguations, 817 queries
>   aliasing_component_ref_p: 636 disambiguations, 15844 queries
>   TBAA oracle: 1417999 disambiguations 2915696 queries
>552182 are in alias set 0
>569795 queries asked about the same object
>0 queries asked about the same alias set
>0 access volatile
>259437 are dependent in the DAG
>116283 are aritificially in conflict with void *
> 
> (my original prototype patch did about 14000 disambiguations out of 4
> querries while passing testuite, so we are slowly getting closer).
> 
> I have a testcase but it depends on same_type_for_tbaa not giving up on
> pointers, so I plan to send it as a followup.
> 
> Bootstrapped/regtested x86_64-linux (all languages), OK?
> 
>   * tree-ssa-alias.c (access_patch_may_continue_p): New function.
>   (aliasing_component_refs_p): Use it.
> 
> Index: tree-ssa-alias.c
> ===
> --- tree-ssa-alias.c  (revision 271747)
> @@ -822,6 +842,16 @@ same_type_for_tbaa (tree type1, tree typ
>return 0;
>  }
>  
> +/* Return true if TYPE is a composite type (i.e. we may apply one of handled
> +   components on it).  */
> +
> +static bool
> +access_patch_may_continue_p (tree type)

'path', but I think type_has_components_p would be a better fit?

> +{
> +  return AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type)
> +  || TREE_CODE (type) == COMPLEX_TYPE;
> +}
> +
>  /* Determine if the two component references REF1 and REF2 which are
> based on access types TYPE1 and TYPE2 and of which at least one is based
> on an indirect reference may alias.  REF2 is the only one that can
> @@ -965,6 +1028,7 @@ aliasing_component_refs_p (tree ref1,
>   if there is no B2 in the tail of path1 and no B1 on the
>   tail of path2.  */
>if (compare_type_sizes (TREE_TYPE (ref2), type1) >= 0
> +  && (access_patch_may_continue_p (TREE_TYPE (ref2)))

extra parens around the call here and below.

OK with all these issues fixed.

Richard.

>&& (base1_alias_set == ref2_alias_set
>|| alias_set_subset_of (base1_alias_set, ref2_alias_set)))
>  {
> @@ -974,6 +1038,7 @@ aliasing_component_refs_p (tree ref1,
>/* If this is ptr vs. decl then we know there is no ptr ... decl path.  */
>if (!ref2_is_decl
>&& compare_type_sizes (TREE_TYPE (ref1), type2) >= 0
> +  && (access_patch_may_continue_p (TREE_TYPE (ref1)))
>&& (base2_alias_set == ref1_alias_set
> || alias_set_subset_of (base2_alias_set, ref1_alias_set)))
>  {
> 

-- 
Richard Biener 
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Re: Hashtable comment cleanups & renamings

[Jonathan Wakely]

On 27/05/19 22:11 +0200, François Dumont wrote:
I had miss some occurences of __bucket_hint to replace with 
__bucket_count_hint so here is a new version.


Ok to commit with the simple ChangeLog entry below ?


OK for trunk, thanks.

Re: RFC: [PATCH] Remove using-declarations that add std names to __gnu_cxx

[Jonathan Wakely]

On 29/05/19 21:00 +0100, Jonathan Wakely wrote:

These using-declarations appear to have been added for simplicity when
moving the non-standard extensions from namespace std to namespace
__gnu_cxx. Dumping all these names into namespace __gnu_cxx allows
uses like __gnu_cxx::size_t and __gnu_cxx::pair, which serve no useful
purpose, but allows creating unnecessarily unportable code.

This patch removes most of the using-declarations from namespace scope,
then either qualifies names as needed or adds using-declarations at
block scope or typedefs at class scope.

* include/backward/hashtable.h (size_t, ptrdiff_t)
(forward_iterator_tag, input_iterator_tag, _Construct, _Destroy)
(distance, vector, pair, __iterator_category): Remove
using-declarations that add these names to namespace __gnu_cxx.
* include/ext/bitmap_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/debug_allocator.h (size_t): Likewise.
* include/ext/functional (size_t, unary_function, binary_function)
(mem_fun1_t, const_mem_fun1_t, mem_fun1_ref_t, const_mem_fun1_ref_t):
Likewise.
* include/ext/malloc_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/memory (ptrdiff_t, pair, __iterator_category): Likewise.
* include/ext/mt_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/new_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/numeric (iota): Fix outdated comment.
* include/ext/pool_allocator.h (size_t, ptrdiff_t): Likewise.
* include/ext/rb_tree (_Rb_tree, allocator): Likewise.
* include/ext/rope (size_t, ptrdiff_t, allocator, _Destroy): Likewise.
* include/ext/ropeimpl.h (size_t, printf, basic_ostream)
(__throw_length_error, _Destroy, std::__uninitialized_fill_n_a):
Likewise.
* include/ext/slist (size_t, ptrdiff_t, _Construct, _Destroy)
(allocator, __true_type, __false_type): Likewise.

Does anybody think we should keep __gnu_cxx::size_t,
__gnu_cxx::input_iterator_tag, __gnu_cxx::vector, __gnu_cxx::pair etc.
or should I go ahead and commit this?


Committed to trunk.

[PATCH] PR libstdc++/90682 allow set_terminate(0) and set_unexpected(0)

[Jonathan Wakely]

Make these functions restore the default handlers when passed a null
pointer. This is consistent with std::pmr::set_default_resource(0), and
also matches the current behaviour of libc++.

In order to avoid duplicating the preprocessor condition from
eh_term_handler.cc more that into a new eh_term_handler.h header and
define a macro that can be used in both eh_term_handler.cc and
eh_terminate.cc.

PR libstdc++/90682
* libsupc++/eh_term_handler.cc: Include eh_term_handler.h to get
definition of _GLIBCXX_DEFAULT_TERM_HANDLER.
* libsupc++/eh_term_handler.h: New header defining
_GLIBCXX_DEFAULT_TERM_HANDLER.
* libsupc++/eh_terminate.cc: Include eh_term_handler.h.
(set_terminate): Restore default handler when argument is null.
(set_unexpected): Likewise.
* testsuite/18_support/set_terminate.cc: New test.
* testsuite/18_support/set_unexpected.cc: New test.

Tested x86_64-linux, committed to trunk.


commit 4a0eb6b39bc2668baebe17f0120187b22f726851
Author: Jonathan Wakely 
Date:   Fri May 31 00:26:44 2019 +0100

PR libstdc++/90682 allow set_terminate(0) and set_unexpected(0)

Make these functions restore the default handlers when passed a null
pointer. This is consistent with std::pmr::set_default_resource(0), and
also matches the current behaviour of libc++.

In order to avoid duplicating the preprocessor condition from
eh_term_handler.cc more that into a new eh_term_handler.h header and
define a macro that can be used in both eh_term_handler.cc and
eh_terminate.cc.

PR libstdc++/90682
* libsupc++/eh_term_handler.cc: Include eh_term_handler.h to get
definition of _GLIBCXX_DEFAULT_TERM_HANDLER.
* libsupc++/eh_term_handler.h: New header defining
_GLIBCXX_DEFAULT_TERM_HANDLER.
* libsupc++/eh_terminate.cc: Include eh_term_handler.h.
(set_terminate): Restore default handler when argument is null.
(set_unexpected): Likewise.
* testsuite/18_support/set_terminate.cc: New test.
* testsuite/18_support/set_unexpected.cc: New test.

diff --git a/libstdc++-v3/libsupc++/eh_term_handler.cc 
b/libstdc++-v3/libsupc++/eh_term_handler.cc
index 6a368c0699e..4b330bc9177 100644
--- a/libstdc++-v3/libsupc++/eh_term_handler.cc
+++ b/libstdc++-v3/libsupc++/eh_term_handler.cc
@@ -24,21 +24,8 @@
 
 #include 
 #include "unwind-cxx.h"
+#include "eh_term_handler.h"
 
-/* We default to the talkative, informative handler in a normal hosted
-   library.  This pulls in the demangler, the dyn-string utilities, and
-   elements of the I/O library.  For a low-memory environment, you can return
-   to the earlier "silent death" handler by configuring GCC with
-   --disable-libstdcxx-verbose and rebuilding the library.
-   In a freestanding environment, we default to this latter approach.  */
-
-#if _GLIBCXX_HOSTED && _GLIBCXX_VERBOSE && __cpp_exceptions
 /* The current installed user handler.  */
 std::terminate_handler __cxxabiv1::__terminate_handler =
-   __gnu_cxx::__verbose_terminate_handler;
-#else
-# include 
-/* The current installed user handler.  */
-std::terminate_handler __cxxabiv1::__terminate_handler = std::abort;
-#endif
-
+   _GLIBCXX_DEFAULT_TERM_HANDLER;
diff --git a/libstdc++-v3/libsupc++/eh_term_handler.h 
b/libstdc++-v3/libsupc++/eh_term_handler.h
new file mode 100644
index 000..e4774bdf9c5
--- /dev/null
+++ b/libstdc++-v3/libsupc++/eh_term_handler.h
@@ -0,0 +1,39 @@
+// -*- C++ -*- default std::terminate handler
+// Copyright (C) 2002-2019 Free Software Foundation, Inc.
+//
+// This file is part of GCC.
+//
+// GCC is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// GCC is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// Under Section 7 of GPL version 3, you are granted additional
+// permissions described in the GCC Runtime Library Exception, version
+// 3.1, as published by the Free Software Foundation.
+
+// You should have received a copy of the GNU General Public License and
+// a copy of the GCC Runtime Library Exception along with this program;
+// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+// .
+
+#include 
+
+/* We default to the talkative, informative handler in a normal hosted
+   library.  This pulls in the demangler, the dyn-string utilities, and
+   elements of the I/O library.  For a low-memory environment, you can return
+   to the earlier "silent death" handler by configuring GCC with
+   --disable-libstdcxx-verbose and rebuilding the 

Re: [PATCH] PR libstdc++/71579 assert that type traits are not misused with an incomplete type

[Jonathan Wakely]

On 31/05/19 08:58 +0300, Antony Polukhin wrote:

On Thu, May 30, 2019, 19:38 Jonathan Wakely  wrote:
<...>


I've attached a relative diff, to be applied on top of yours, with my
suggested tweaks. Do you see any issues with it?

(If you're happy with those tweaks I can go ahead and apply this,
there's no need for an updated patch from you).



Looks good. Please apply!


Here's what I've tested and committed to trunk.

I decided to add a more detailed changelog too.



commit c877256d8a2b0f9383c1cd93c0b2907496c715db
Author: Jonathan Wakely 
Date:   Fri May 31 09:23:09 2019 +0100

PR libstdc++/71579 assert that type traits are not misused with incomplete types

This patch adds static asserts for type traits misuse with incomplete
classes and unions. This gives a nice readable error message instead
of an UB and odr-violations.

Some features of the patch:
* each type trait has it's own static_assert inside. This gives better
diagnostics than the approach with putting the assert into a helper
structure and using it in each trait.
* the result of completeness check is not memorized by the compiler.
This gives no false positive after the first failed check.
* some of the compiler builtins already implement the check. But not
all of them! So the asserts are in all the type_traits that may
benefit from the check. This also makes the behavior of libstdc++ more
consistent across different (non GCC) compilers.
* std::is_base_of does not have the assert as it works well in many
cases with incomplete types

2019-05-31  Antony Polukhin  

PR libstdc++/71579
* include/std/type_traits __type_identity, __is_complete_or_unbounded):
New helpers for checking preconditions in traits.
(is_trivial, is_trivially_copyable, is_standard_layout, is_pod)
(is_literal_type, is_empty, is_polymorphic, is_final, is_abstract)
(is_destructible, is_nothrow_destructible, is_constructible)
(is_default_constructible, is_copy_constructible)
(is_move_constructible, is_nothrow_default_constructible)
(is_nothrow_constructible, is_nothrow_copy_constructible)
(is_nothrow_move_constructible, is_copy_assignable, is_move_assignable)
(is_nothrow_assignable, is_nothrow_copy_assignable)
(is_nothrow_move_assignable, is_trivially_constructible)
(is_trivially_copy_constructible, is_trivially_move_constructible)
is_trivially_assignable, is_trivially_copy_assignable)
(is_trivially_move_assignable, is_trivially_destructible)
(alignment_of, is_swappable, is_nothrow_swappable, is_invocable)
(is_invocable_r, is_nothrow_invocable)
(has_unique_object_representations, is_aggregate): Add static_asserts
to make sure that type traits are not misused with incomplete types.
(__is_constructible_impl, __is_nothrow_default_constructible_impl)
(__is_nothrow_constructible_impl, __is_nothrow_assignable_impl): New
base characteristics without assertions that can be reused in other
traits.
* testsuite/20_util/is_complete_or_unbounded/memoization.cc: New test.
* testsuite/20_util/is_complete_or_unbounded/memoization_neg.cc: New
test.
* testsuite/20_util/is_complete_or_unbounded/value.cc: New test.
* testsuite/20_util/is_abstract/incomplete_neg.cc: New test.
* testsuite/20_util/is_aggregate/incomplete_neg.cc: New test.
* testsuite/20_util/is_class/value.cc: Check incomplete type.
* testsuite/20_util/is_function/value.cc: Likewise.
* testsuite/20_util/is_move_constructible/incomplete_neg.cc: New test.
* testsuite/20_util/is_nothrow_move_assignable/incomplete_neg.cc: New
test.
* testsuite/20_util/is_polymorphic/incomplete_neg.cc: New test.
* testsuite/20_util/is_reference/value.cc: Check incomplete types.
* testsuite/20_util/is_unbounded_array/value.cc: Likewise.
* testsuite/20_util/is_union/value.cc: Likewise.
* testsuite/20_util/is_void/value.cc: Likewise.
* testsuite/util/testsuite_tr1.h: Add incomplete union type.

diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index 3a622eb61e0..78a113af415 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -91,6 +91,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
 struct conditional;
 
+  template 
+struct __type_identity {
+  using type = _Type;
+};
+
   template
 struct __or_;
 
@@ -177,6 +182,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #endif // C++17
 
+  // Forward declarations
+  template
+struct is_reference;
+  template
+struct is_function;
+  template
+struct 

Re: [PATCH] PR libstdc++/85494 use rdseed and rand_s in std::random_device

[Jonathan Wakely]

On 31/05/19 10:47 +0100, Jonathan Wakely wrote:

On 31/05/19 10:46 +0100, Jonathan Wakely wrote:

On 31/05/19 09:26 +, Szabolcs Nagy wrote:

On 29/05/2019 23:03, Jonathan Wakely wrote:

This fixes a test that started to fail in mingw.

Tested x86_64-linux, powerpc64le-linux and x86_64-w64-ming32.

Committed to trunk.


note that the updated test fails on baremetal

FAIL: 26_numerics/random/random_device/cons/token.cc execution test

26_numerics/random/random_device/cons/token.cc:67: void test03(): Assertion 
'count != 0' failed.

(if i add "default" to the token list in test03 then it passes)


Hmm, then I messed something up. If the "default" token works then one
of the others should too (I would expect that to be "mt19937" for bare
metal).

I'll take a look, thanks.



Doh:

--- a/libstdc++-v3/testsuite/26_numerics/random/random_device/cons/token.cc
+++ b/libstdc++-v3/testsuite/26_numerics/random/random_device/cons/token.cc
@@ -51,7 +51,7 @@ test03()
{
 // At least one of these tokens should be valid.
 const std::string tokens[] = {
-"rdseed", "rdrand", "rand_s", "/dev/urandom", "/dev/random", "mt19337"
+"rdseed", "rdrand", "rand_s", "/dev/urandom", "/dev/random", "mt19937"
 };
 int count = 0;
 for (const std::string& token : tokens)



Here's what I've committed to fix the COW string failures and the
baremetal failure. Tested powerpc64le-linux and x86_64-linux, old and
new ABIs.


commit a7d8558fdd74dc96dc0bd73c4a663b658f4155ba
Author: Jonathan Wakely 
Date:   Fri May 31 08:47:47 2019 +0100

Fix random_device to work with COW strings again

Instead of duplicating the initialization functions that take string,
add a new member taking a raw pointer that can be used to convert the
constructor token from the old string to the new.

Also fix "mt19337" typos in a testcase.

* include/bits/random.h (random_device::_M_init(const char*, size_t)):
Add new private member function.
* src/c++11/cow-string-inst.cc (random_device::_M_init(const string&))
(random_device::_M_init_pretr1(const string&)): Call new private
member with string data.
* src/c++11/random.cc (random_device::_M_init(const char*, size_t)):
Define.
* testsuite/26_numerics/random/random_device/cons/default-cow.cc: New
test using COW strings.
* testsuite/26_numerics/random/random_device/cons/default.cc: Generate
a value from the device.
* testsuite/26_numerics/random/random_device/cons/token.cc: Likewise.
Fix typo in token string.

diff --git a/libstdc++-v3/include/bits/random.h b/libstdc++-v3/include/bits/random.h
index 9c959d6dc84..e63dbcf5a25 100644
--- a/libstdc++-v3/include/bits/random.h
+++ b/libstdc++-v3/include/bits/random.h
@@ -1648,6 +1648,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 result_type _M_getval_pretr1();
 double _M_getentropy() const noexcept;
 
+void _M_init(const char*, size_t); // not exported from the shared library
+
 union
 {
   struct
diff --git a/libstdc++-v3/src/c++11/cow-string-inst.cc b/libstdc++-v3/src/c++11/cow-string-inst.cc
index c36f297438e..107a45750f2 100644
--- a/libstdc++-v3/src/c++11/cow-string-inst.cc
+++ b/libstdc++-v3/src/c++11/cow-string-inst.cc
@@ -35,61 +35,15 @@
 
 #ifdef  _GLIBCXX_USE_C99_STDINT_TR1
 #include 
-#if defined __i386__ || defined __x86_64__
-# include 
-#endif
-#include 
 
 namespace std _GLIBCXX_VISIBILITY(default)
 {
   void
   random_device::_M_init(const std::string& token)
-  {
-const char *fname = token.c_str();
-
-if (token == "default")
-  {
-#if (defined __i386__ || defined __x86_64__) && defined _GLIBCXX_X86_RDRAND
-	unsigned int eax, ebx, ecx, edx;
-	// Check availability of cpuid and, for now at least, also the
-	// CPU signature for Intel's
-	if (__get_cpuid_max(0, ) > 0 && ebx == signature_INTEL_ebx)
-	  {
-	__cpuid(1, eax, ebx, ecx, edx);
-	if (ecx & bit_RDRND)
-	  {
-		_M_file = nullptr;
-		return;
-	  }
-	  }
-#endif
-
-	fname = "/dev/urandom";
-  }
-else if (token != "/dev/urandom" && token != "/dev/random")
-fail:
-  std::__throw_runtime_error(__N("random_device::"
- "random_device(const std::string&)"));
-
-_M_file = static_cast(std::fopen(fname, "rb"));
-if (!_M_file)
-  goto fail;
-  }
+  { _M_init(token.c_str(), token.length()); }
 
   void
   random_device::_M_init_pretr1(const std::string& token)
-  {
-unsigned long __seed = 5489UL;
-if (token != "mt19937")
-  {
-	const char* __nptr = token.c_str();
-	char* __endptr;
-	__seed = std::strtoul(__nptr, &__endptr, 0);
-	if (*__nptr == '\0' || *__endptr != '\0')
-	  std::__throw_runtime_error(__N("random_device::random_device"
-	 "(const std::string&)"));
-  }
-_M_mt.seed(__seed);
-  }
+  { _M_init(token.c_str(), token.length()); }
 } // namespace
 #endif
diff --git a/libstdc++-v3/src/c++11/random.cc 

[PATCH][OBVIOUS] Add pretty print for const_tree.

[Martin Liška]

Hi.

One obvious pretty printer change.

Martin

gcc/ChangeLog:

2019-05-31  Martin Liska  

* gdbhooks.py: Add const_tree to TreePrinter.
---
 gcc/gdbhooks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/gcc/gdbhooks.py b/gcc/gdbhooks.py
index 7b1a7be0002..39f5c4772f9 100644
--- a/gcc/gdbhooks.py
+++ b/gcc/gdbhooks.py
@@ -540,7 +540,7 @@ class GdbPrettyPrinters(gdb.printing.PrettyPrinter):
 
 def build_pretty_printer():
 pp = GdbPrettyPrinters('gcc')
-pp.add_printer_for_types(['tree'],
+pp.add_printer_for_types(['tree', 'const_tree'],
  'tree', TreePrinter)
 pp.add_printer_for_types(['cgraph_node *', 'varpool_node *', 'symtab_node *'],
  'symtab_node', SymtabNodePrinter)

Re: [PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap

[Jakub Jelinek]

On Fri, May 31, 2019 at 11:43:29AM +0200, Richard Biener wrote:
> Hmm.  Is it enough to set TREE_THIS_NOTRAP on the ARRAY_REF instead?

That works too.

> At least it's documented the ARRAY_REF then isn't the issue.
> As for conditional vs. unconditional I'm not so sure but automatic
> vars are excepmted from that anyways?

Only if they are guaranteed not to have out of bound accesses.

Anyway, I'll go with this, those ARRAY_REFs indeed should never trap.

2019-05-31  Jakub Jelinek  

* omp-low.c (lower_rec_simd_input_clauses): Set TREE_THIS_NOTRAP on
ivar and lvar.

* gcc.dg/vect/vect-cond-12.c: New test.

--- gcc/omp-low.c.jj2019-05-30 23:19:14.469931759 +0200
+++ gcc/omp-low.c   2019-05-31 11:52:20.491195088 +0200
@@ -3728,6 +3728,8 @@ lower_rec_simd_input_clauses (tree new_v
 NULL_TREE, NULL_TREE);
   lvar = build4 (ARRAY_REF, TREE_TYPE (new_var), avar, sctx->lane,
 NULL_TREE, NULL_TREE);
+  TREE_THIS_NOTRAP (ivar) = 1;
+  TREE_THIS_NOTRAP (lvar) = 1;
 }
   if (DECL_P (new_var))
 {
--- gcc/testsuite/gcc.dg/vect/vect-cond-12.c.jj 2019-05-31 11:25:33.203577504 
+0200
+++ gcc/testsuite/gcc.dg/vect/vect-cond-12.c2019-05-31 11:26:58.616174115 
+0200
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fopenmp-simd" } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target 
vect_condition } } } */
+
+int x;
+
+void
+foo (int *a)
+{
+  #pragma omp simd lastprivate (x)
+  for (int i = 0; i < 1024; ++i)
+if (a[i])
+  x = i;
+}


Jakub

[patch] Fix segfault caused by spurious constant overflow

[Eric Botcazou]

Hi,

this is a regression present for 32-bit targets on mainline and 9 branch only, 
but the underlying issue dates back from the removal of the TYPE_IS_SIZETYPE 
machinery.  Since the removal of this machinery, there is an internal conflict 
for sizetype: it's an unsigned type so with wrap-around semantics on overflow 
but overflow nevertheless needs to be tracked for it in order to avoid buffer 
overflows for large objects.

The constant folder contains various tricks to that effect and the Ada front-
end even more so, because VLAs are first-class citizens in the language and 
their lower bound is not necessarily 0.  In particular, you need to be able to 
distinguish large constants in sizetype from small negative constants turned 
into even larger constants, because the latter appear in the length of e.g.:

  type Arr is array (2 .. N) of Long_Long_Integer;

This works when the expressions haven't been too much mangled by the constant 
folder and here we have a counter-example for 32-bit targets.  Starting with:

  (N + 4294967295) * 8

which is the sizetype expression of the size of Arr in bytes, extract_muldiv_1 
distributes the multiplication:

  N * 8 + 4294967288

and, immediately after, fold_plusminus_mult_expr factors it back:

  (N + 536870911) * 8

At this point, there is no way for gigi to tell that it's the expression of a 
simple array with no overflow in sight because the 536870911 constant is large 
but not large enough, so gigi flags it as overflowing for small values of N.

I don't see any other way out than disabling the back-and-forth mathematical 
game played by the constant folder in this case, which very likely brings no 
benefit in practice, hence the proposed fix.

Tested on x86_64-suse-linux, OK for the mainline and 9 branch?


2019-05-31  Eric Botcazou  

* fold-const.c (extract_muldiv_1) : Do not distribute a
multiplication by a power-of-two value.


2019-05-31  Eric Botcazou  

* gnat.dg/specs/discr6.ads: New test.

-- 
Eric BotcazouIndex: fold-const.c
===
--- fold-const.c	(revision 271694)
+++ fold-const.c	(working copy)
@@ -6475,8 +6475,13 @@ extract_muldiv_1 (tree t, tree c, enum t
 	 apply the distributive law to commute the multiply and addition
 	 if the multiplication of the constants doesn't overflow
 	 and overflow is defined.  With undefined overflow
-	 op0 * c might overflow, while (op0 + orig_op1) * c doesn't.  */
-  if (code == MULT_EXPR && TYPE_OVERFLOW_WRAPS (ctype))
+	 op0 * c might overflow, while (op0 + orig_op1) * c doesn't.
+	 But fold_plusminus_mult_expr would factor back any power-of-two
+	 value so do not distribute in the first place in this case.  */
+  if (code == MULT_EXPR
+	  && TYPE_OVERFLOW_WRAPS (ctype)
+	  && !(tree_fits_shwi_p (c)
+	   && exact_log2 (absu_hwi (tree_to_shwi (c))) > 0))
 	return fold_build2 (tcode, ctype,
 			fold_build2 (code, ctype,
 	 fold_convert (ctype, op0),
-- { dg-do compile }

package Discr6 is

  subtype Index_T is Integer range 0 .. 15;

  type Arr is array (Index_T range <> ) of Long_Long_Integer;

  type Rec2 (Size : Index_T := 2) is record
A : Arr (2 .. Size);
  end record;

  type Rec3 (D : Boolean := False) is record
R : Rec2;
case D is
  when False=> null;
  when True => I : Integer;
end case;
  end record;

end Discr6;

Re: [PATCH] PR libstdc++/85494 use rdseed and rand_s in std::random_device

[Jonathan Wakely]

On 31/05/19 10:46 +0100, Jonathan Wakely wrote:

On 31/05/19 09:26 +, Szabolcs Nagy wrote:

On 29/05/2019 23:03, Jonathan Wakely wrote:

This fixes a test that started to fail in mingw.

Tested x86_64-linux, powerpc64le-linux and x86_64-w64-ming32.

Committed to trunk.


note that the updated test fails on baremetal

FAIL: 26_numerics/random/random_device/cons/token.cc execution test

26_numerics/random/random_device/cons/token.cc:67: void test03(): Assertion 
'count != 0' failed.

(if i add "default" to the token list in test03 then it passes)


Hmm, then I messed something up. If the "default" token works then one
of the others should too (I would expect that to be "mt19937" for bare
metal).

I'll take a look, thanks.



Doh:

--- a/libstdc++-v3/testsuite/26_numerics/random/random_device/cons/token.cc
+++ b/libstdc++-v3/testsuite/26_numerics/random/random_device/cons/token.cc
@@ -51,7 +51,7 @@ test03()
{
  // At least one of these tokens should be valid.
  const std::string tokens[] = {
-"rdseed", "rdrand", "rand_s", "/dev/urandom", "/dev/random", "mt19337"
+"rdseed", "rdrand", "rand_s", "/dev/urandom", "/dev/random", "mt19937"
  };
  int count = 0;
  for (const std::string& token : tokens)

Re: [PATCH] PR libstdc++/85494 use rdseed and rand_s in std::random_device

[Jonathan Wakely]

On 31/05/19 09:26 +, Szabolcs Nagy wrote:

On 29/05/2019 23:03, Jonathan Wakely wrote:

This fixes a test that started to fail in mingw.

Tested x86_64-linux, powerpc64le-linux and x86_64-w64-ming32.

Committed to trunk.


note that the updated test fails on baremetal

FAIL: 26_numerics/random/random_device/cons/token.cc execution test

26_numerics/random/random_device/cons/token.cc:67: void test03(): Assertion 
'count != 0' failed.

(if i add "default" to the token list in test03 then it passes)


Hmm, then I messed something up. If the "default" token works then one
of the others should too (I would expect that to be "mt19937" for bare
metal).

I'll take a look, thanks.

Re: Make aliasing_component_refs_p to work harder when same_type_for_tbaa returns -1

[Jan Hubicka]

> 
>   * gcc.dg/lto/alias-access-path-2.0.c: New testcase.
> 
>   * tree-ssa-alias.c (aliasing_component_refs_p): Do not give up
>   immediately after same_types_for_tbaa_p returns -1 and continue
>   looking for possible exact match; if matching types are arrays
>   watch for partial overlaps.
>   (indirect_ref_may_alias_decl_p): Watch for partial array overlaps.
>   (indirect_refs_may_alias_p): Do type based disambiguation first;
>   update comment.
> Index: testsuite/g++.dg/lto/pr88130_0.C
> ===
> --- testsuite/g++.dg/lto/pr88130_0.C  (nonexistent)
> +++ testsuite/g++.dg/lto/pr88130_0.C  (working copy)

Hi,
I have just noticed that I included unrelated testcase by accident,
please ignore it.

Honza

Do not ask alias subset query when access patch can not extend

[Jan Hubicka]

Hi
in most common cases we have access paths that ends by read/write of a
non-composite type.  When aliasing_component_refs_p finds no match in
the acccess path it asks alias oracle if the one alias set is subset of
other to figure out whether one access path can be extended to the
other.

This is unnecessarily conservatie especially for pointers where we say tru
for
 foo->ptr
 bar->ptr
where foo and bar are two structures containing pointers of different type.

This patch bypasses the alis oracle check for non-composite types.  Along with
yesterday patch and patch to handle pointers in types_same_for_tbaa_p we now
get:

  refs_may_alias_p: 3019081 disambiguations, 3318862 queries
  ref_maybe_used_by_call_p: 7112 disambiguations, 3044693 queries
  call_may_clobber_ref_p: 817 disambiguations, 817 queries
  aliasing_component_ref_p: 2259 disambiguations, 17881 queries
  TBAA oracle: 1421561 disambiguations 2922798 queries
   552636 are in alias set 0
   569791 queries asked about the same object
   0 queries asked about the same alias set
   0 access volatile
   260727 are dependent in the DAG
   118083 are aritificially in conflict with void *

Compared to:

  refs_may_alias_p: 3013678 disambiguations, 3314059 queries
  ref_maybe_used_by_call_p: 7112 disambiguations, 3039278 queries
  call_may_clobber_ref_p: 817 disambiguations, 817 queries
  aliasing_component_ref_p: 636 disambiguations, 15844 queries
  TBAA oracle: 1417999 disambiguations 2915696 queries
   552182 are in alias set 0
   569795 queries asked about the same object
   0 queries asked about the same alias set
   0 access volatile
   259437 are dependent in the DAG
   116283 are aritificially in conflict with void *

(my original prototype patch did about 14000 disambiguations out of 4
querries while passing testuite, so we are slowly getting closer).

I have a testcase but it depends on same_type_for_tbaa not giving up on
pointers, so I plan to send it as a followup.

Bootstrapped/regtested x86_64-linux (all languages), OK?

* tree-ssa-alias.c (access_patch_may_continue_p): New function.
(aliasing_component_refs_p): Use it.

Index: tree-ssa-alias.c
===
--- tree-ssa-alias.c(revision 271747)
@@ -822,6 +842,16 @@ same_type_for_tbaa (tree type1, tree typ
   return 0;
 }
 
+/* Return true if TYPE is a composite type (i.e. we may apply one of handled
+   components on it).  */
+
+static bool
+access_patch_may_continue_p (tree type)
+{
+  return AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type)
+|| TREE_CODE (type) == COMPLEX_TYPE;
+}
+
 /* Determine if the two component references REF1 and REF2 which are
based on access types TYPE1 and TYPE2 and of which at least one is based
on an indirect reference may alias.  REF2 is the only one that can
@@ -965,6 +1028,7 @@ aliasing_component_refs_p (tree ref1,
  if there is no B2 in the tail of path1 and no B1 on the
  tail of path2.  */
   if (compare_type_sizes (TREE_TYPE (ref2), type1) >= 0
+  && (access_patch_may_continue_p (TREE_TYPE (ref2)))
   && (base1_alias_set == ref2_alias_set
   || alias_set_subset_of (base1_alias_set, ref2_alias_set)))
 {
@@ -974,6 +1038,7 @@ aliasing_component_refs_p (tree ref1,
   /* If this is ptr vs. decl then we know there is no ptr ... decl path.  */
   if (!ref2_is_decl
   && compare_type_sizes (TREE_TYPE (ref1), type2) >= 0
+  && (access_patch_may_continue_p (TREE_TYPE (ref1)))
   && (base2_alias_set == ref1_alias_set
  || alias_set_subset_of (base2_alias_set, ref1_alias_set)))
 {

Re: [PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap

[Richard Biener]

On Fri, 31 May 2019, Jakub Jelinek wrote:

> Hi!
> 
> As a preparation for lastprivate(conditional:) on #pragma omp simd, I need
> if-conversion to handle "omp simd array" accesses.  These are safe, no
> matter whether written or read, each simd lane accesses their own element,
> after the vectorization it is all just a single vector read or store or RMW
> cycle, it will never trap (it is an automatic variable), it will never be
> out of bounds (the index is guaranteed to be within the bounds of the array,
> as the array is sized to the vectorization factor and index is the simd lane
> number within that vectorization factor).
> 
> Tested on x86_64-linux with vect.exp, ok for trunk if it passes full
> bootstrap/regtest?

Hmm.  Is it enough to set TREE_THIS_NOTRAP on the ARRAY_REF instead?
At least it's documented the ARRAY_REF then isn't the issue.
As for conditional vs. unconditional I'm not so sure but automatic
vars are excepmted from that anyways?

Richard.

> 2019-05-31  Jakub Jelinek  
> 
>   * tree-if-conv.c: Include attribs.h.
>   (ifcvt_memrefs_wont_trap): Return true for ARRAY_REF access to
>   "omp simd array" variable with .GOMP_SIMD_LANE as index.
> 
>   * gcc.dg/vect/vect-cond-12.c: New test.
> 
> --- gcc/tree-if-conv.c.jj 2019-05-14 21:37:32.932400472 +0200
> +++ gcc/tree-if-conv.c2019-05-31 11:13:13.558732900 +0200
> @@ -120,6 +120,7 @@ along with GCC; see the file COPYING3.
>  #include "fold-const.h"
>  #include "tree-ssa-sccvn.h"
>  #include "tree-cfgcleanup.h"
> +#include "attribs.h"
>  
>  /* Only handle PHIs with no more arguments unless we are asked to by
> simd pragma.  */
> @@ -897,6 +898,32 @@ ifcvt_memrefs_wont_trap (gimple *stmt, v
>if (DR_W_UNCONDITIONALLY (*master_dr))
>  return true;
>  
> +  /* OpenMP simd lane accesses to omp simd array variables are always
> + within bounds and can be handled as if it was unconditional.  */
> +  tree base_var = get_base_address (base);
> +  if (VAR_P (base_var)
> +  && lookup_attribute ("omp simd array", DECL_ATTRIBUTES (base_var)))
> +{
> +  tree ref = DR_REF (a);
> +  struct loop *loop = loop_containing_stmt (stmt);
> +  if (loop->simduid
> +   && TREE_CODE (ref) == ARRAY_REF
> +   && TREE_CODE (TREE_OPERAND (ref, 1)) == SSA_NAME)
> + {
> +   gimple *def = SSA_NAME_DEF_STMT (TREE_OPERAND (ref, 1));
> +   if (is_gimple_call (def)
> +   && gimple_call_internal_p (def)
> +   && (gimple_call_internal_fn (def) == IFN_GOMP_SIMD_LANE))
> + {
> +   tree arg = gimple_call_arg (def, 0);
> +   gcc_assert (TREE_CODE (arg) == SSA_NAME);
> +   arg = SSA_NAME_VAR (arg);
> +   if (arg == loop->simduid)
> + return true;
> + }
> + }
> +}
> +
>/* If a is unconditionally accessed then ...
>  
>   Even a is conditional access, we can treat it as an unconditional
> --- gcc/testsuite/gcc.dg/vect/vect-cond-12.c.jj   2019-05-31 
> 11:25:33.203577504 +0200
> +++ gcc/testsuite/gcc.dg/vect/vect-cond-12.c  2019-05-31 11:26:58.616174115 
> +0200
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-fopenmp-simd" } */
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target 
> vect_condition } } } */
> +
> +int x;
> +
> +void
> +foo (int *a)
> +{
> +  #pragma omp simd lastprivate (x)
> +  for (int i = 0; i < 1024; ++i)
> +if (a[i])
> +  x = i;
> +}
> 
>   Jakub
> 

-- 
Richard Biener 
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Re: [RFC][PR88838][SVE] Use 32-bit WHILELO in LP64 mode

[Richard Sandiford]

Kugan Vivekanandarajah  writes:
> @@ -609,8 +615,14 @@ vect_set_loop_masks_directly (struct loop *loop, 
> loop_vec_info loop_vinfo,
>  
>/* Get the mask value for the next iteration of the loop.  */
>next_mask = make_temp_ssa_name (mask_type, NULL, "next_mask");
> -  gcall *call = vect_gen_while (next_mask, test_index, this_test_limit);
> -  gsi_insert_before (test_gsi, call, GSI_SAME_STMT);
> +  tree test_index_cmp_type = make_ssa_name (compare_type);
> +  gimple *conv_stmt = gimple_build_assign (test_index_cmp_type,
> +NOP_EXPR,
> +test_index);
> +  gsi_insert_before (test_gsi, conv_stmt, GSI_NEW_STMT);

We only need to convert once, so this should happen before the
FOR_EACH_VEC_ELT_REVERSE loop.  Would be better as:

  gimple_seq test_seq = NULL;
  test_index = gimple_convert (_seq, compare_type, text_index);
  gimple_insert_seq_before (test_gsi, test_seq, GSI_SAME_STMT);

so that we don't generate unnecessary converts.

> +  gcall *call = vect_gen_while (next_mask, test_index_cmp_type,
> + this_test_limit);
> +  gsi_insert_after (test_gsi, call, GSI_SAME_STMT);
>  
>vect_set_loop_mask (loop, mask, init_mask, next_mask);
>  }
> @@ -637,12 +649,12 @@ vect_set_loop_condition_masked (struct loop *loop, 
> loop_vec_info loop_vinfo,
>  
>tree compare_type = LOOP_VINFO_MASK_COMPARE_TYPE (loop_vinfo);
>unsigned int compare_precision = TYPE_PRECISION (compare_type);
> -  unsigned HOST_WIDE_INT max_vf = vect_max_vf (loop_vinfo);
>tree orig_niters = niters;
>  
>/* Type of the initial value of NITERS.  */
>tree ni_actual_type = TREE_TYPE (niters);
>unsigned int ni_actual_precision = TYPE_PRECISION (ni_actual_type);
> +  tree niters_skip = LOOP_VINFO_MASK_SKIP_NITERS (loop_vinfo);
>  
>/* Convert NITERS to the same size as the compare.  */
>if (compare_precision > ni_actual_precision
> @@ -661,33 +673,7 @@ vect_set_loop_condition_masked (struct loop *loop, 
> loop_vec_info loop_vinfo,
>else
>  niters = gimple_convert (_seq, compare_type, niters);
>  
> -  /* Convert skip_niters to the right type.  */
> -  tree niters_skip = LOOP_VINFO_MASK_SKIP_NITERS (loop_vinfo);
> -
> -  /* Now calculate the value that the induction variable must be able
> - to hit in order to ensure that we end the loop with an all-false mask.
> - This involves adding the maximum number of inactive trailing scalar
> - iterations.  */
> -  widest_int iv_limit;
> -  bool known_max_iters = max_loop_iterations (loop, _limit);
> -  if (known_max_iters)
> -{
> -  if (niters_skip)
> - {
> -   /* Add the maximum number of skipped iterations to the
> -  maximum iteration count.  */
> -   if (TREE_CODE (niters_skip) == INTEGER_CST)
> - iv_limit += wi::to_widest (niters_skip);
> -   else
> - iv_limit += max_vf - 1;
> - }
> -  /* IV_LIMIT is the maximum number of latch iterations, which is also
> -  the maximum in-range IV value.  Round this value down to the previous
> -  vector alignment boundary and then add an extra full iteration.  */
> -  poly_uint64 vf = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
> -  iv_limit = (iv_limit & -(int) known_alignment (vf)) + max_vf;
> -}
> -
> +  widest_int iv_limit = vect_iv_limit_for_full_masking (loop_vinfo);
>/* Get the vectorization factor in tree form.  */
>tree vf = build_int_cst (compare_type,
>  LOOP_VINFO_VECT_FACTOR (loop_vinfo));
> @@ -717,7 +703,7 @@ vect_set_loop_condition_masked (struct loop *loop, 
> loop_vec_info loop_vinfo,
>   /* See whether zero-based IV would ever generate all-false masks
>  before wrapping around.  */
>   bool might_wrap_p
> -   = (!known_max_iters
> +   = (iv_limit == UINT_MAX

Shouldn't this be == -1?

>|| (wi::min_precision (iv_limit * rgm->max_nscalars_per_iter,
>   UNSIGNED)
>> compare_precision));
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 4942c69..1240037 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -1029,7 +1029,10 @@ static bool
>  vect_verify_full_masking (loop_vec_info loop_vinfo)
>  {
>struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
> +  tree niters_type = TREE_TYPE (LOOP_VINFO_NITERS (loop_vinfo));
>unsigned int min_ni_width;
> +  unsigned int max_nscalars_per_iter
> += vect_get_max_nscalars_per_iter (loop_vinfo);
>  
>/* Use a normal loop if there are no statements that need masking.
>   This only happens in rare degenerate cases: it means that the loop
> @@ -1048,7 +1051,7 @@ vect_verify_full_masking (loop_vec_info loop_vinfo)
>  max_ni = wi::smin (max_ni, max_back_edges + 1);
>  
>/* Account for rgroup masks, in which each bit is replicated N times.  */
> -  max_ni *= 

[PATCH] Handle "omp simd array" accesses in ifcvt_memrefs_wont_trap

[Jakub Jelinek]

Hi!

As a preparation for lastprivate(conditional:) on #pragma omp simd, I need
if-conversion to handle "omp simd array" accesses.  These are safe, no
matter whether written or read, each simd lane accesses their own element,
after the vectorization it is all just a single vector read or store or RMW
cycle, it will never trap (it is an automatic variable), it will never be
out of bounds (the index is guaranteed to be within the bounds of the array,
as the array is sized to the vectorization factor and index is the simd lane
number within that vectorization factor).

Tested on x86_64-linux with vect.exp, ok for trunk if it passes full
bootstrap/regtest?

2019-05-31  Jakub Jelinek  

* tree-if-conv.c: Include attribs.h.
(ifcvt_memrefs_wont_trap): Return true for ARRAY_REF access to
"omp simd array" variable with .GOMP_SIMD_LANE as index.

* gcc.dg/vect/vect-cond-12.c: New test.

--- gcc/tree-if-conv.c.jj   2019-05-14 21:37:32.932400472 +0200
+++ gcc/tree-if-conv.c  2019-05-31 11:13:13.558732900 +0200
@@ -120,6 +120,7 @@ along with GCC; see the file COPYING3.
 #include "fold-const.h"
 #include "tree-ssa-sccvn.h"
 #include "tree-cfgcleanup.h"
+#include "attribs.h"
 
 /* Only handle PHIs with no more arguments unless we are asked to by
simd pragma.  */
@@ -897,6 +898,32 @@ ifcvt_memrefs_wont_trap (gimple *stmt, v
   if (DR_W_UNCONDITIONALLY (*master_dr))
 return true;
 
+  /* OpenMP simd lane accesses to omp simd array variables are always
+ within bounds and can be handled as if it was unconditional.  */
+  tree base_var = get_base_address (base);
+  if (VAR_P (base_var)
+  && lookup_attribute ("omp simd array", DECL_ATTRIBUTES (base_var)))
+{
+  tree ref = DR_REF (a);
+  struct loop *loop = loop_containing_stmt (stmt);
+  if (loop->simduid
+ && TREE_CODE (ref) == ARRAY_REF
+ && TREE_CODE (TREE_OPERAND (ref, 1)) == SSA_NAME)
+   {
+ gimple *def = SSA_NAME_DEF_STMT (TREE_OPERAND (ref, 1));
+ if (is_gimple_call (def)
+ && gimple_call_internal_p (def)
+ && (gimple_call_internal_fn (def) == IFN_GOMP_SIMD_LANE))
+   {
+ tree arg = gimple_call_arg (def, 0);
+ gcc_assert (TREE_CODE (arg) == SSA_NAME);
+ arg = SSA_NAME_VAR (arg);
+ if (arg == loop->simduid)
+   return true;
+   }
+   }
+}
+
   /* If a is unconditionally accessed then ...
 
  Even a is conditional access, we can treat it as an unconditional
--- gcc/testsuite/gcc.dg/vect/vect-cond-12.c.jj 2019-05-31 11:25:33.203577504 
+0200
+++ gcc/testsuite/gcc.dg/vect/vect-cond-12.c2019-05-31 11:26:58.616174115 
+0200
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fopenmp-simd" } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target 
vect_condition } } } */
+
+int x;
+
+void
+foo (int *a)
+{
+  #pragma omp simd lastprivate (x)
+  for (int i = 0; i < 1024; ++i)
+if (a[i])
+  x = i;
+}

Jakub

Re: [PATCH] PR libstdc++/85494 use rdseed and rand_s in std::random_device

[Szabolcs Nagy]

On 29/05/2019 23:03, Jonathan Wakely wrote:
> This fixes a test that started to fail in mingw.
> 
> Tested x86_64-linux, powerpc64le-linux and x86_64-w64-ming32.
> 
> Committed to trunk.

note that the updated test fails on baremetal

FAIL: 26_numerics/random/random_device/cons/token.cc execution test

26_numerics/random/random_device/cons/token.cc:67: void test03(): Assertion 
'count != 0' failed.

(if i add "default" to the token list in test03 then it passes)

[PATCH] Make debug(edge) more verbose.

[Martin Liška]

Hi.

With the patch one can see more info during debugging:

(gdb) pp
 6)>
 5 [11.0% (guessed)]  count:105119324 (estimated locally) 
(FALLTHRU,CAN_FALLTHRU,LOOP_EXIT)

Ready for trunk?
Thanks,
Martin

gcc/ChangeLog:

2019-05-31  Martin Liska  

* cfg.c (debug): Use TDF_DETAILS for debug and
print edge info only once.
---
 gcc/cfg.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)


diff --git a/gcc/cfg.c b/gcc/cfg.c
index 94e68c83e45..983115ee40a 100644
--- a/gcc/cfg.c
+++ b/gcc/cfg.c
@@ -546,9 +546,10 @@ dump_edge_info (FILE *file, edge e, dump_flags_t flags, int do_succ)
 DEBUG_FUNCTION void
 debug (edge_def )
 {
-  /* FIXME (crowl): Is this desireable?  */
-  dump_edge_info (stderr, , TDF_NONE, false);
-  dump_edge_info (stderr, , TDF_NONE, true);
+  fprintf (stderr, " %d)>\n",
+	   ref.src->index, ref.dest->index);
+  dump_edge_info (stderr, , TDF_DETAILS, false);
+  fprintf (stderr, "\n");
 }
 
 DEBUG_FUNCTION void

[C++ Patch] Fix cp_parser_unqualified_id typo

[Paolo Carlini]

Hi,

noticed this typo: we are passing the location to build_nt as third 
argument instead of passing it to cp_expr as second argument. Tested 
x86_64-linux.


By the way, we have the option of using build_min_nt_loc instead of 
build_nt in all these places for BIT_NOT_EXPR in 
cp_parser_unqualified_id, Certainly it has the advantage that the 
location information survives in the tree node when cp_expr is 
eventually converted to a plain tree... (below tests fine) What do you 
think?


Thanks, Paolo.



2019-05-31  Paolo Carlini  

  * parser.c (cp_parser_unqualified_id): Fix typo, properly call
  build_nt.
Index: parser.c
===
--- parser.c(revision 271785)
+++ parser.c(working copy)
@@ -6063,7 +6063,7 @@ cp_parser_unqualified_id (cp_parser* parser,
   "%<~auto%> only available with "
   "%<-std=c++14%> or %<-std=gnu++14%>");
cp_lexer_consume_token (parser->lexer);
-   return cp_expr (build_nt (BIT_NOT_EXPR, make_auto (), loc));
+   return cp_expr (build_nt (BIT_NOT_EXPR, make_auto ()), loc);
  }
 
/* If there was an explicit qualification (S::~T), first look
Index: parser.c
===
--- parser.c(revision 271761)
+++ parser.c(working copy)
@@ -6052,7 +6052,7 @@ cp_parser_unqualified_id (cp_parser* parser,
&& constructor_name_p (token->u.value, scope
  {
cp_lexer_consume_token (parser->lexer);
-   return cp_expr (build_nt (BIT_NOT_EXPR, scope), loc);
+   return build_min_nt_loc (loc, BIT_NOT_EXPR, scope);
  }
 
/* ~auto means the destructor of whatever the object is.  */
@@ -6063,7 +6063,7 @@ cp_parser_unqualified_id (cp_parser* parser,
   "%<~auto%> only available with "
   "%<-std=c++14%> or %<-std=gnu++14%>");
cp_lexer_consume_token (parser->lexer);
-   return cp_expr (build_nt (BIT_NOT_EXPR, make_auto (), loc));
+   return build_min_nt_loc (loc, BIT_NOT_EXPR, make_auto ());
  }
 
/* If there was an explicit qualification (S::~T), first look
@@ -6153,8 +6153,8 @@ cp_parser_unqualified_id (cp_parser* parser,
   time.  */
type_decl = cp_parser_identifier (parser);
if (type_decl != error_mark_node)
- type_decl = build_nt (BIT_NOT_EXPR, type_decl);
-   return cp_expr (type_decl, loc);
+ type_decl = build_min_nt_loc (loc, BIT_NOT_EXPR, type_decl);
+   return type_decl;
  }
  }
/* If an error occurred, assume that the name of the
@@ -6162,7 +6162,7 @@ cp_parser_unqualified_id (cp_parser* parser,
   class.  That allows us to keep parsing after running
   into ill-formed destructor names.  */
if (type_decl == error_mark_node && scope)
- return build_nt (BIT_NOT_EXPR, scope);
+ return build_min_nt_loc (loc, BIT_NOT_EXPR, scope);
else if (type_decl == error_mark_node)
  return error_mark_node;
 
@@ -6189,7 +6189,7 @@ cp_parser_unqualified_id (cp_parser* parser,
"typedef-name %qD used as destructor declarator",
type_decl);
 
-   return cp_expr (build_nt (BIT_NOT_EXPR, TREE_TYPE (type_decl), loc));
+   return build_min_nt_loc (loc, BIT_NOT_EXPR, TREE_TYPE (type_decl));
   }
 
 case CPP_KEYWORD:

Re: [PATCHv2] debug: make -feliminate-unused-debug-symbols the default [PR debug/86964]

[Richard Biener]

On Fri, May 31, 2019 at 10:39 AM Thomas De Schampheleire
 wrote:
>
>
>
> On Fri, May 31, 2019, 10:23 Richard Biener  wrote:
>>
>> On Wed, May 29, 2019 at 12:10 PM Thomas De Schampheleire
>>  wrote:
>> >
>> > Hi Richard,
>>
>> Sorry for the delay - I have bootstrapped/tested the patch and
>> installed it on trunk.
>> If there will be no complaints I plan to backport it for GCC 9.2 (you
>> may need to
>> remind me in a few weeks).
>
>
> Thanks.
>
> I think I read somewhere that 7.4 is the last 7.x release, is that correct?
>
> But what about 8.x? If there will be a new release I think these patches 
> should be backported too, no? After all, it's part of a regression.

I'm not opposed to that but would like to see feedback first - too bad
we missed making this the default for GCC 9.1.

> In what location can you see such info about which branches and releases are 
> expected, latest release in a stream, etc?

There's only the toplevel https://gcc.gnu.org/ page which lists active
branches.  That the next release from the 7 branch will be
the last is not documented anywhere.

Richard.

> Best regards
> Thomas

Re: value_range_base::{non_zero_p, set_zero, set_non_zero}

[Aldy Hernandez]

Thanks. I will adjust accordingly.

On Fri, May 31, 2019, 02:26 Martin Sebor  wrote:

> On 5/30/19 12:58 AM, Aldy Hernandez wrote:
> > Hi.
> >
> > We have zero_p in the API, but we don't have non_zero_p.  Instead we use
> > a non-API function range_is_nonnull.  I've fixed this.
> >
> > I have also gotten rid of the duplicity of using the non-API
> > range_is_null in favor of value_range_base::zero_p().
> >
> > Furthermore, there's value_range*::set_null and
> > value_range*::set_nonnull().  It's inconsistent to use null/nonnull as
> > well as zero/non_zero throughout.  I've moved everything to *zero.
>
> With the -Wformat-diag cleanup still fresh in my memory, I can't
> help but point out that the GCC spelling convention calls for
> "nonzero" vs "non-zero" or "non zero".
>
> Naming the function set_nonzero() would be in line with both
> the convention and established practice (over 2000 instances)
> and set_non_zero would not be (only 22 instances of non_zero
> in GCC sources).
>
> This, of course, is in contrast to things like bit-field and
> built-in where the convention calls for the hyphen but where
> in code we seem to prefer "bitfield" nonetheless ;-) (Names
> like get_bit_field_ref_def and bit_field_size being
> the exceptions).
>
> Martin
>
> >
> > Finally, it seems to me that the derived value_range versions of
> > set_*zero/null are a bit confusing in that they clear equivalences
> > behind the scenes.  There's no intuitive reason why setting a range of
> > [0,0] versus [5,10] should clear equivalences.  I've made the
> > equivalence nuking explicit in the handful of places where we do this,
> > and thus reduced the need for separate value_range versions.
> >
> > I believe with these changes, as well as the pending intersect patch,
> > we've cleaned up the remaining value_range uses where we actually wanted
> > to use value_range_base.  Or at least the remaining "value_range tem"
> > business.
> >
> > OK?
> >
> > Aldy
>
>

Re: Simplify more EXACT_DIV_EXPR comparisons

[Aldy Hernandez]

I've never been too happy with the too large due to cast warnings. For that
matter, it seems like a lot of the unbounded alloca warning variants were
artifacts of the way we couldn't get precise ranges after vrp asserts had
disappeared and we were trying to guess at what the actual range in the
original code was. It's fragile at best.

I haven't been paying too much attention to walloca because the ranger gets
considerably better context ranges in the ranger walloca version, and we
are getting correct warnings for a variety of things we couldn't before. So
I was hoping to ignore this until we all agreed on what range, vrp etc will
look like going forward.

That being said, I could take a closer look at this xfail on Monday if
y'all would like. But I don't currently have strong opinions either way. I
guess it'll all change in the next few months.

Aldy

On Thu, May 30, 2019, 18:26 Jeff Law  wrote:

> On 5/29/19 3:27 PM, Marc Glisse wrote:
> > On Mon, 20 May 2019, Richard Biener wrote:
> >
> >> On Mon, May 20, 2019 at 10:16 AM Marc Glisse 
> >> wrote:
> >>>
> >>> On Mon, 20 May 2019, Richard Biener wrote:
> >>>
>  On Sun, May 19, 2019 at 6:16 PM Marc Glisse 
>  wrote:
> >
> > Hello,
> >
> > 2 pieces:
> >
> > - the first one handles the case where the denominator is negative.
> It
> > doesn't happen often with exact_div, so I don't handle it
> > everywhere, but
> > this one looked trivial
> >
> > - handle the case where a pointer difference is cast to an unsigned
> > type
> > before being compared to a constant (I hit this in std::vector).
> > With some
> > range info we could probably handle some non-constant cases as
> well...
> >
> > The second piece breaks Walloca-13.c (-Walloca-larger-than=100 -O2)
> >
> > void f (void*);
> > void g (int *p, int *q)
> > {
> >__SIZE_TYPE__ n = (__SIZE_TYPE__)(p - q);
> >if (n < 100)
> >  f (__builtin_alloca (n));
> > }
> >
> > At the time of walloca2, we have
> >
> >_1 = p_5(D) - q_6(D);
> ># RANGE [-2305843009213693952, 2305843009213693951]
> >_2 = _1 /[ex] 4;
> ># RANGE ~[2305843009213693952, 16140901064495857663]
> >n_7 = (long unsigned intD.10) _2;
> >_11 = (long unsigned intD.10) _1;
> >if (_11 <= 396)
> > [...]
> >_3 = allocaD.1059 (n_7);
> >
> > and warn.
> 
>  That's indeed to complicated relation of _11 to n_7 for
>  VRP predicate discovery.
> 
> > However, DOM3 later produces
> >
> >_1 = p_5(D) - q_6(D);
> >_11 = (long unsigned intD.10) _1;
> >if (_11 <= 396)
> 
>  while _11 vs. _1 works fine.
> 
> > [...]
> ># RANGE [0, 99] NONZERO 127
> >_2 = _1 /[ex] 4;
> ># RANGE [0, 99] NONZERO 127
> >n_7 = (long unsigned intD.10) _2;
> >_3 = allocaD.1059 (n_7);
> >
> > so I am tempted to say that the walloca2 pass is too early, xfail the
> > testcase and file an issue...
> 
>  Hmm, there's a DOM pass before walloca2 already and moving
>  walloca2 after loop opts doesn't look like the best thing to do?
>  I suppose it's not DOM but sinking that does the important transform
>  here?  That is,
> 
>  Index: gcc/passes.def
>  ===
>  --- gcc/passes.def  (revision 271395)
>  +++ gcc/passes.def  (working copy)
>  @@ -241,9 +241,9 @@ along with GCC; see the file COPYING3.
>    NEXT_PASS (pass_optimize_bswap);
>    NEXT_PASS (pass_laddress);
>    NEXT_PASS (pass_lim);
>  -  NEXT_PASS (pass_walloca, false);
>    NEXT_PASS (pass_pre);
>    NEXT_PASS (pass_sink_code);
>  +  NEXT_PASS (pass_walloca, false);
>    NEXT_PASS (pass_sancov);
>    NEXT_PASS (pass_asan);
>    NEXT_PASS (pass_tsan);
> 
>  fixes it?
> >>>
> >>> I will check, but I don't think walloca uses any kind of on-demand
> >>> VRP, so
> >>> we still need some pass to update the ranges after sinking, which
> >>> doesn't
> >>> seem to happen until the next DOM pass.
> >>
> >> Oh, ok...  Aldy, why's this a separate pass anyways?  I think similar
> >> other warnigns are emitted from RTL expansion?  So maybe we can
> >> indeed move the pass towards warn_restrict or late_warn_uninit.
> >
> > I tried moving it after 'sink' and that didn't help. Moving it next to
> > warn_restrict works for this test but breaks 2 others that currently
> > "work" by accident (+ one where the message changes between "unbounded"
> > and "too large", it isn't clear what the difference is between those
> > messages).
> So "unbounded" means there was no controlling predicate that would allow
> an upper bound on the size of the alloca to be determined.
>
> "too large" breaks down into two cases.  One where we know it's too
> large, the 

Re: undefined behavior in value_range::equiv_add()?

[Richard Biener]

On Fri, May 31, 2019 at 2:27 AM Jeff Law  wrote:
>
> On 5/29/19 10:20 AM, Aldy Hernandez wrote:
> > On 5/29/19 12:12 PM, Jeff Law wrote:
> >> On 5/29/19 9:58 AM, Aldy Hernandez wrote:
> >>> On 5/29/19 9:24 AM, Richard Biener wrote:
>  On Wed, May 29, 2019 at 2:18 PM Aldy Hernandez 
>  wrote:
> >
> > As per the API, and the original documentation to value_range,
> > VR_UNDEFINED and VR_VARYING should never have equivalences.  However,
> > equiv_add is tacking on equivalences blindly, and there are various
> > regressions that happen if I fix this oversight.
> >
> > void
> > value_range::equiv_add (const_tree var,
> >   const value_range *var_vr,
> >   bitmap_obstack *obstack)
> > {
> >  if (!m_equiv)
> >m_equiv = BITMAP_ALLOC (obstack);
> >  unsigned ver = SSA_NAME_VERSION (var);
> >  bitmap_set_bit (m_equiv, ver);
> >  if (var_vr && var_vr->m_equiv)
> >bitmap_ior_into (m_equiv, var_vr->m_equiv);
> > }
> >
> > Is this a bug in the documentation / API, or is equiv_add incorrect
> > and
> > we should fix the fall-out elsewhere?
> 
>  I think this must have been crept in during the classification.  If you
>  go back to say GCC 7 you shouldn't see value-ranges with
>  UNDEFINED/VARYING state in the lattice that have equivalences.
> 
>  It may not be easy to avoid with the new classy interface but we're
>  certainly not tacking on them "blindly".  At least we're not supposed
>  to.  As usual the intermediate state might be "broken" but
>  intermediateness is not sth the new class "likes".
> >>>
> >>> It looks like extract_range_from_stmt (by virtue of
> >>> vrp_visit_assignment_or_call and then extract_range_from_ssa_name)
> >>> returns one of these intermediate ranges.  It would seem to me that an
> >>> outward looking API method like vr_values::extract_range_from_stmt
> >>> shouldn't be returning inconsistent ranges.  Or are there no guarantees
> >>> for value_ranges from within all of vr_values?
> >> ISTM that if we have an implementation constraint that says a VR_VARYING
> >> or VR_UNDEFINED range can't have equivalences, then we need to honor
> >> that at the minimum for anything returned by an external API.  Returning
> >> an inconsistent state is bad.  I'd even state that we should try damn
> >> hard to avoid it in internal APIs as well.
> >
> > Agreed * 2.
> >
> >>
> >>>
> >>> Perhaps I should give a little background.  As part of your
> >>> value_range_base re-factoring last year, you mentioned that you didn't
> >>> split out intersect like you did union because of time or oversight.  I
> >>> have code to implement intersect (attached), for which I've noticed that
> >>> I must leave equivalences intact, even when transitioning to
> >>> VR_UNDEFINED:
> >>>
> >>> [from the attached patch]
> >>> +  /* If THIS is varying we want to pick up equivalences from OTHER.
> >>> + Just special-case this here rather than trying to fixup after the
> >>> + fact.  */
> >>> +  if (this->varying_p ())
> >>> +this->deep_copy (other);
> >>> +  else if (this->undefined_p ())
> >>> +/* ?? Leave any equivalences already present in an undefined.
> >>> +   This is technically not allowed, but we may get an in-flight
> >>> +   value_range in an intermediate state.  */
> >> Where/when does this happen?
> >
> > The above snippet is not currently in mainline.  It's in the patch I'm
> > proposing to clean up intersect.  It's just that while cleaning up
> > intersect I noticed that if we keep to the value_range API, we end up
> > clobbering an equivalence to a VR_UNDEFINED that we depend up further up
> > the call chain.
> >
> > The reason it doesn't happen in mainline is because intersect_helper
> > bails early on an undefined, thus leaving the problematic equivalence
> > intact.
> >
> > You can see it in mainline though, with the following testcase:
> >
> > int f(int x)
> > {
> >   if (x != 0 && x != 1)
> > return -2;
> >
> >   return !x;
> > }
> >
> > Break in evrp_range_analyzer::record_ranges_from_stmt() and see that the
> > call to extract_range_from_stmt() returns a VR of undefined *WITH*
> > equivalences:
> >
> >   vr_values->extract_range_from_stmt (stmt, _edge, , );
> >
> > This VR is later fed to update_value_range() and ultimately intersect(),
> > which in mainline, leaves the equivalences alone, but IMO should respect
> > that API and nuke them.
> So I think this is coming from extract_range_from_ssa_name:
>
> > void
> > vr_values::extract_range_from_ssa_name (value_range *vr, tree var)
> > {
> >   value_range *var_vr = get_value_range (var);
> >
> >   if (!var_vr->varying_p ())
> > vr->deep_copy (var_vr);
> >   else
> > vr->set (var);
> >
> >   vr->equiv_add (var, get_value_range (var), _equiv_obstack);
> > }
>
> Where we blindly add VAR to the equiv bitmap in the range.
>

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Richard Biener]

On Thu, May 30, 2019 at 10:46 PM David Malcolm  wrote:
>
> On Thu, 2019-05-30 at 11:23 -0500, Qing Zhao wrote:
> > Hi,
> >
> > PR 90581 (provide an option to adjust the maximum depth of nested
> > #include)
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
> >
> > is to add a new cpp option -fmax-inlcude-depth to set the maximum
> > depth of nested #include.
> >
> > '-fmax-include-depth=DEPTH'
> >  Set the maximum depth of the nested include.  The default value
> > is
> >  200.
> >
> > Please check the attached patch.
> > I have done bootstrap and regression test on X86, no any issue.
> >
> > thanks a lot.
> >
> > Qing.
> >
> Thanks for working on this.  It's looking promising, but I agree that a
> param would be better than an option.

Not sure - this is for language limits and we do have existing
like -ftemplate-backtrace-limit and -ftemplate-depth.

> One idea that occurred to me looking at the patch...
>
> > index 3ee8bc4..480c282 100644
> > --- a/libcpp/directives.c
> > +++ b/libcpp/directives.c
> > @@ -831,7 +831,7 @@ do_include_common (cpp_reader *pfile, enum include_type 
> > type)
> >  }
> >
> >/* Prevent #include recursion.  */
> > -  if (pfile->line_table->depth >= CPP_STACK_MAX)
> > +  if (pfile->line_table->depth >= CPP_OPTION (pfile, max_include_depth))
> >  cpp_error (pfile, CPP_DL_ERROR, "#include nested too deeply");
>
> ...a nice usability tweak here would be to give a hint about the new
> param, to give the user an idea on how to increase the limit.
>
> Maybe something like:
>
> cpp_error (pfile, CPP_DL_ERROR,
>   "%<#include%> nested too deeply (depth %i);"
>   " use %<--param max-include-depth=LIMIT%> to support deeper 
> nesting",
>   pfile->line_table->depth);
>
> (though probably that would be better as a followup "note")
>
> Dave

Re: Simplify more EXACT_DIV_EXPR comparisons

[Richard Biener]

On Wed, May 29, 2019 at 11:28 PM Marc Glisse  wrote:
>
> On Mon, 20 May 2019, Richard Biener wrote:
>
> > On Mon, May 20, 2019 at 10:16 AM Marc Glisse  wrote:
> >>
> >> On Mon, 20 May 2019, Richard Biener wrote:
> >>
> >>> On Sun, May 19, 2019 at 6:16 PM Marc Glisse  wrote:
> 
>  Hello,
> 
>  2 pieces:
> 
>  - the first one handles the case where the denominator is negative. It
>  doesn't happen often with exact_div, so I don't handle it everywhere, but
>  this one looked trivial
> 
>  - handle the case where a pointer difference is cast to an unsigned type
>  before being compared to a constant (I hit this in std::vector). With 
>  some
>  range info we could probably handle some non-constant cases as well...
> 
>  The second piece breaks Walloca-13.c (-Walloca-larger-than=100 -O2)
> 
>  void f (void*);
>  void g (int *p, int *q)
>  {
> __SIZE_TYPE__ n = (__SIZE_TYPE__)(p - q);
> if (n < 100)
>   f (__builtin_alloca (n));
>  }
> 
>  At the time of walloca2, we have
> 
> _1 = p_5(D) - q_6(D);
> # RANGE [-2305843009213693952, 2305843009213693951]
> _2 = _1 /[ex] 4;
> # RANGE ~[2305843009213693952, 16140901064495857663]
> n_7 = (long unsigned intD.10) _2;
> _11 = (long unsigned intD.10) _1;
> if (_11 <= 396)
>  [...]
> _3 = allocaD.1059 (n_7);
> 
>  and warn.
> >>>
> >>> That's indeed to complicated relation of _11 to n_7 for
> >>> VRP predicate discovery.
> >>>
>  However, DOM3 later produces
> 
> _1 = p_5(D) - q_6(D);
> _11 = (long unsigned intD.10) _1;
> if (_11 <= 396)
> >>>
> >>> while _11 vs. _1 works fine.
> >>>
>  [...]
> # RANGE [0, 99] NONZERO 127
> _2 = _1 /[ex] 4;
> # RANGE [0, 99] NONZERO 127
> n_7 = (long unsigned intD.10) _2;
> _3 = allocaD.1059 (n_7);
> 
>  so I am tempted to say that the walloca2 pass is too early, xfail the
>  testcase and file an issue...
> >>>
> >>> Hmm, there's a DOM pass before walloca2 already and moving
> >>> walloca2 after loop opts doesn't look like the best thing to do?
> >>> I suppose it's not DOM but sinking that does the important transform
> >>> here?  That is,
> >>>
> >>> Index: gcc/passes.def
> >>> ===
> >>> --- gcc/passes.def  (revision 271395)
> >>> +++ gcc/passes.def  (working copy)
> >>> @@ -241,9 +241,9 @@ along with GCC; see the file COPYING3.
> >>>   NEXT_PASS (pass_optimize_bswap);
> >>>   NEXT_PASS (pass_laddress);
> >>>   NEXT_PASS (pass_lim);
> >>> -  NEXT_PASS (pass_walloca, false);
> >>>   NEXT_PASS (pass_pre);
> >>>   NEXT_PASS (pass_sink_code);
> >>> +  NEXT_PASS (pass_walloca, false);
> >>>   NEXT_PASS (pass_sancov);
> >>>   NEXT_PASS (pass_asan);
> >>>   NEXT_PASS (pass_tsan);
> >>>
> >>> fixes it?
> >>
> >> I will check, but I don't think walloca uses any kind of on-demand VRP, so
> >> we still need some pass to update the ranges after sinking, which doesn't
> >> seem to happen until the next DOM pass.
> >
> > Oh, ok...  Aldy, why's this a separate pass anyways?  I think similar
> > other warnigns are emitted from RTL expansion?  So maybe we can
> > indeed move the pass towards warn_restrict or late_warn_uninit.
>
> I tried moving it after 'sink' and that didn't help. Moving it next to
> warn_restrict works for this test but breaks 2 others that currently
> "work" by accident (+ one where the message changes between "unbounded"
> and "too large", it isn't clear what the difference is between those
> messages).
>
> My suggestion, in addition to the original patch, is
>
> * gcc.dg/Walloca-13.c: Xfail.
>
> --- Walloca-13.c(revision 271742)
> +++ Walloca-13.c(working copy)
> @@ -1,12 +1,12 @@
>   /* { dg-do compile } */
>   /* { dg-require-effective-target alloca } */
>   /* { dg-options "-Walloca-larger-than=100 -O2" } */
>
>   void f (void*);
>
>   void g (int *p, int *q)
>   {
> __SIZE_TYPE__ n = (__SIZE_TYPE__)(p - q);
> if (n < 100)
> -f (__builtin_alloca (n));
> +f (__builtin_alloca (n)); // { dg-bogus "may be too large due to 
> conversion" "" { xfail { *-*-* } } }
>   }
>
> Is that ok?

It works for me.  Any pass ordering changes should probably be done
as followup anyways since they might expose other issues (as you've seen...)

Richard.

> > I also see that the Og pass pipeline misses the second walloca pass
> > completely (and also the warn_restrict pass).
> >
> > Given code sinkings obvious effects on SSA value-range representation
> > it may make sense to add another instance of that pass earlier.
>
> --
> Marc Glisse

Re: [PATCHv2] debug: make -feliminate-unused-debug-symbols the default [PR debug/86964]

[Thomas De Schampheleire]

On Fri, May 31, 2019, 10:23 Richard Biener 
wrote:

> On Wed, May 29, 2019 at 12:10 PM Thomas De Schampheleire
>  wrote:
> >
> > Hi Richard,
>
> Sorry for the delay - I have bootstrapped/tested the patch and
> installed it on trunk.
> If there will be no complaints I plan to backport it for GCC 9.2 (you
> may need to
> remind me in a few weeks).
>

Thanks.

I think I read somewhere that 7.4 is the last 7.x release, is that correct?

But what about 8.x? If there will be a new release I think these patches
should be backported too, no? After all, it's part of a regression.

In what location can you see such info about which branches and releases
are expected, latest release in a stream, etc?

Best regards
Thomas

Re: apply unary op to both sides of (vec_cond x cst1 cst2)

[Richard Biener]

On Thu, May 30, 2019 at 12:02 AM Marc Glisse  wrote:
>
> On Mon, 20 May 2019, Richard Biener wrote:
>
> > On Sun, May 19, 2019 at 6:22 PM Marc Glisse  wrote:
> >>
> >> Hello,
> >>
> >> I noticed this one with BIT_NOT_EXPR a while ago. C++ testcase because I
> >> haven't looked at gimplefe yet...
> >
> > OK.  I wonder if there's any issue with -ftrapv and negate/abs of
> > INT_MIN containing vectors (we don't implement -ftrapv optabs for
> > vectors of course...).
> > We at least avoid folding those cases I think and sanitizers then may
> > see those unconditionally?
>
> The new version of the patch checks that both sides can indeed be folded
> to constants before doing the transformation. We should have the same
> for functions like sqrt, but fold_const_call does not seem to handle
> vectors :-(

Looks good to me.  Btw, no reason to not extend fold_const_call to handle
the vector case ;)

Richard.

> @@ -2911,20 +2913,35 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  @2)
> (if (!VOID_TYPE_P (TREE_TYPE (@1)) || VOID_TYPE_P (type))
>  @1)))
>   (simplify
>(vec_cond VECTOR_CST@0 @1 @2)
>(if (integer_all_onesp (@0))
> @1
> (if (integer_zerop (@0))
>  @2)))
>
> +/* Sink unary operations to constant branches, but only if we do fold it to
> +   constants.  */
> +(for op (negate bit_not abs absu)
> + (simplify
> +  (op (vec_cond @0 VECTOR_CST@1 VECTOR_CST@2))
> +  (with
> +   {
> + tree cst1, cst2;
> + cst1 = const_unop (op, type, @1);
> + if (cst1)
> +   cst2 = const_unop (op, type, @2);
> +   }
> +   (if (cst1 && cst2)
> +(vec_cond @0 { cst1; } { cst2; })
> +
>   /* Simplification moved from fold_cond_expr_with_comparison.  It may also
>  be extended.  */
>   /* This pattern implements two kinds simplification:
>
>  Case 1)
>  (cond (cmp (convert1? x) c1) (convert2? x) c2) -> (minmax (x c)) if:
>1) Conversions are type widening from smaller type.
>2) Const c1 equals to c2 after canonicalizing comparison.
>3) Comparison has tree code LT, LE, GT or GE.
>  This specific pattern is needed when (cmp (convert x) c) may not
>
>
> >> 2019-05-20  Marc Glisse  
> >>
> >> gcc/
> >> * match.pd (~(vec?cst1:cst2)); New transformation.
> >>
> >> gcc/testsuite/
> >> * g++.dg/tree-ssa/cprop-vcond.C
>
> --
> Marc Glisse

Re: [PATCHv2] debug: make -feliminate-unused-debug-symbols the default [PR debug/86964]

[Richard Biener]

On Wed, May 29, 2019 at 12:10 PM Thomas De Schampheleire
 wrote:
>
> Hi Richard,

Sorry for the delay - I have bootstrapped/tested the patch and
installed it on trunk.
If there will be no complaints I plan to backport it for GCC 9.2 (you
may need to
remind me in a few weeks).

Thanks again,
Richard.

> El mar., 21 may. 2019 a las 16:57, Thomas De Schampheleire
> () escribió:
> >
> > From: Thomas De Schampheleire 
> >
> > In addition to making -feliminate-unused-debug-symbols work for the DWARF
> > format (see [1]), make this option the default. This behavior was the case
> > before, e.g. under gcc 4.9.x.
> > [1] https://gcc.gnu.org/viewcvs/gcc?view=revision=269925
> >
> > This change requires some updates to test cases, which expected the previous
> > default of not eliminating unused debug symbols.
> >
> > gcc/ChangeLog:
> >
> > 2019-05-21  Thomas De Schampheleire  
> >
> > PR debug/86964
> > * common.opt (feliminate-unused-debug-symbols): Enable by default.
> > * doc/invoke.texi (Debugging Options): Document new default of
> > -feliminate-unused-debug-symbols and remove restriction to 'stabs'.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2019-05-21  Thomas De Schampheleire  
> >
> > PR debug/86964
> > * g++.dg/debug/dwarf2/fesd-any.C: Use
> > -fno-eliminate-unused-debug-symbols.
> > * g++.dg/debug/dwarf2/fesd-baseonly.C: Likewise.
> > * g++.dg/debug/dwarf2/fesd-none.C: Likewise.
> > * g++.dg/debug/dwarf2/fesd-reduced.C: Likewise.
> > * g++.dg/debug/dwarf2/fesd-sys.C: Likewise.
> > * g++.dg/debug/dwarf2/inline-var-1.C: Likewise.
> > * g++.dg/debug/enum-2.C: Likewise.
> > * gcc.dg/debug/dwarf2/fesd-any.c: Likewise.
> > * gcc.dg/debug/dwarf2/fesd-baseonly.c: Likewise.
> > * gcc.dg/debug/dwarf2/fesd-none.c: Likewise.
> > * gcc.dg/debug/dwarf2/fesd-reduced.c: Likewise.
> > * gcc.dg/debug/dwarf2/fesd-sys.c: Likewise.
> > ---
> >  gcc/common.opt| 2 +-
> >  gcc/doc/invoke.texi   | 9 +
> >  gcc/testsuite/g++.dg/debug/dwarf2/fesd-any.C  | 2 +-
> >  gcc/testsuite/g++.dg/debug/dwarf2/fesd-baseonly.C | 2 +-
> >  gcc/testsuite/g++.dg/debug/dwarf2/fesd-none.C | 2 +-
> >  gcc/testsuite/g++.dg/debug/dwarf2/fesd-reduced.C  | 2 +-
> >  gcc/testsuite/g++.dg/debug/dwarf2/fesd-sys.C  | 2 +-
> >  gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C  | 2 +-
> >  gcc/testsuite/g++.dg/debug/enum-2.C   | 1 +
> >  gcc/testsuite/gcc.dg/debug/dwarf2/fesd-any.c  | 2 +-
> >  gcc/testsuite/gcc.dg/debug/dwarf2/fesd-baseonly.c | 2 +-
> >  gcc/testsuite/gcc.dg/debug/dwarf2/fesd-none.c | 2 +-
> >  gcc/testsuite/gcc.dg/debug/dwarf2/fesd-reduced.c  | 2 +-
> >  gcc/testsuite/gcc.dg/debug/dwarf2/fesd-sys.c  | 2 +-
> >  14 files changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index d342c4f3749..0e72fd08ec4 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -1379,7 +1379,7 @@ Common Report Var(flag_ipa_sra) Init(0) Optimization
> >  Perform interprocedural reduction of aggregates.
> >
> >  feliminate-unused-debug-symbols
> > -Common Report Var(flag_debug_only_used_symbols)
> > +Common Report Var(flag_debug_only_used_symbols) Init(1)
> >  Perform unused symbol elimination in debug info.
> >
> >  feliminate-unused-debug-types
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index 5e3e8873d35..06c8c60f19e 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -388,7 +388,7 @@ Objective-C and Objective-C++ Dialects}.
> >  -fno-eliminate-unused-debug-types @gol
> >  -femit-struct-debug-baseonly  -femit-struct-debug-reduced @gol
> >  -femit-struct-debug-detailed@r{[}=@var{spec-list}@r{]} @gol
> > --feliminate-unused-debug-symbols  -femit-class-debug-always @gol
> > +-fno-eliminate-unused-debug-symbols  -femit-class-debug-always @gol
> >  -fno-merge-debug-strings  -fno-dwarf2-cfi-asm @gol
> >  -fvar-tracking  -fvar-tracking-assignments}
> >
> > @@ -7827,10 +7827,11 @@ confusion with @option{-gdwarf-@var{level}}.
> >  Instead use an additional @option{-g@var{level}} option to change the
> >  debug level for DWARF.
> >
> > -@item -feliminate-unused-debug-symbols
> > +@item -fno-eliminate-unused-debug-symbols
> >  @opindex feliminate-unused-debug-symbols
> > -Produce debugging information in stabs format (if that is supported),
> > -for only symbols that are actually used.
> > +@opindex fno-eliminate-unused-debug-symbols
> > +By default, no debug information is produced for symbols that are not 
> > actually
> > +used. Use this option if you want debug information for all symbols.
> >
> >  @item -femit-class-debug-always
> >  @opindex femit-class-debug-always
> > diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/fesd-any.C 
> > b/gcc/testsuite/g++.dg/debug/dwarf2/fesd-any.C
> > index a4a0b50ee50..5868ebc9c85 100644
> > --- 

Re: [PATCH] Fix ICE in ssa_create_duplicates (PR tree-optimization/90671)

[Richard Biener]

On Thu, 30 May 2019, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs on the trunk, because both gsi_split_seq_after
> and gsi_insert_seq_after have assertions that the split is not after
> gsi_end_p iterator or insertion is not after such an iterator.
> 
> My understanding of Alex' change is that it wants to hide any added
> debug stmts temporarily from create_block_for_threading duplication, so that
> it remains just in one of the blocks.  Now, template_last_to_copy is
> initialized using last_bb (template_block), if that block is initially
> completely empty, template_last_to_copy will be gsi_end_p, gsi_stmt on it
> NULL, so I think in that case we can't split any sequence anywhere, we
> simply want to hide the whole sequence from the block duplication and put it
> in afterwards.
> 
> The following patch does that.  Bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk?

OK.

Richard.
 
> 2019-05-30  Jakub Jelinek  
> 
>   PR tree-optimization/90671
>   * tree-ssa-threadupdate.c (ssa_create_duplicates): If
>   template_block used to be empty on the first call, don't use
>   gsi_split_seq_after and gsi_insert_seq_after, but remember whole
>   seq with bb_seq and set it with set_bb_seq.
> 
>   * gcc.dg/torture/pr90671.c: New test.
> 
> --- gcc/tree-ssa-threadupdate.c.jj2019-05-23 12:57:15.522512319 +0200
> +++ gcc/tree-ssa-threadupdate.c   2019-05-30 10:15:44.718468219 +0200
> @@ -1142,12 +1142,25 @@ ssa_create_duplicates (struct redirectio
>gimple_seq seq = NULL;
>if (gsi_stmt (local_info->template_last_to_copy)
> != gsi_stmt (gsi_last_bb (local_info->template_block)))
> - seq = gsi_split_seq_after (local_info->template_last_to_copy);
> + {
> +   if (gsi_end_p (local_info->template_last_to_copy))
> + {
> +   seq = bb_seq (local_info->template_block);
> +   set_bb_seq (local_info->template_block, NULL);
> + }
> +   else
> + seq = gsi_split_seq_after (local_info->template_last_to_copy);
> + }
>create_block_for_threading (local_info->template_block, rd, 0,
> _info->duplicate_blocks);
>if (seq)
> - gsi_insert_seq_after (_info->template_last_to_copy,
> -   seq, GSI_SAME_STMT);
> + {
> +   if (gsi_end_p (local_info->template_last_to_copy))
> + set_bb_seq (local_info->template_block, seq);
> +   else
> + gsi_insert_seq_after (_info->template_last_to_copy,
> +   seq, GSI_SAME_STMT);
> + }
>  
>/* Go ahead and wire up outgoing edges and update PHIs for the 
> duplicate
>block.   */
> --- gcc/testsuite/gcc.dg/torture/pr90671.c.jj 2019-05-30 10:20:13.686068207 
> +0200
> +++ gcc/testsuite/gcc.dg/torture/pr90671.c2019-05-30 10:19:50.815442342 
> +0200
> @@ -0,0 +1,16 @@
> +/* PR tree-optimization/90671 */
> +/* { dg-do compile } */
> +/* { dg-additional-options "-w -g" } */
> +
> +int a;
> +
> +int
> +main ()
> +{
> +  int b, c;
> +  for (c = 0; c < 2; c++)
> +while (a)
> +  if (b)
> + break;
> +  return 0;
> +}
> 
>   Jakub
> 

-- 
Richard Biener 
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers

[Richard Biener]

On Thu, 30 May 2019, Alex Henrie wrote:

> On Thu, May 30, 2019 at 6:59 PM Joseph Myers  wrote:
> >
> > On Thu, 30 May 2019, Alex Henrie wrote:
> >
> > > At this point I think I'm convinced that any attribute that applies to
> > > a function should also be allowed on a function pointer without any
> > > warnings. We can start by turning off the warnings for the "fndecl"
> > > attributes and then clean up the other attributes as time goes on.
> >
> > This is inherently a property of the attribute in question.  The issue is
> > not whether it applies to function pointers; it's whether it applies to
> > function types.
> >
> > For example, the "section" or "alias" attributes are attributes that apply
> > to a declaration, but not a type.  Because they apply to variables as well
> > as functions, they are meaningful on function pointers - but the meaning
> > is *not* the same as applying them to the pointed-to function.
> >
> > The "flatten" attribute, however, seems only meaningful for functions, not
> > variables, not function types and not function pointers.
> >
> > We should try to work out for each attribute exactly what construct it
> > appertains to - which for many but not all function attributes is indeed
> > the type of the function rather than the function itself.  Then move to
> > making such attributes work on types.  But for attributes such as
> > "flatten" that logically appertain to the declaration not its type, we
> > should continue to diagnose them on function pointers or types.
> 
> In Wine we need a way to (without warnings) put ms_hook_prologue into
> a macro that is applied to functions, function pointers, and function
> pointer typedefs. It sounds like you're saying that you will not
> accept a patch that silences or splits off warnings about using
> ms_hook_prologue with function pointers and function pointer typedefs.
> So how do you think Wine's problem should be solved?

I think ms_hook_prologue should be allowed to apply to function types
and function decls.  If you say it should apply to function pointers
then I suppose you want to have it apply to the pointed to function
of the function pointer - but that's not possible without an indirection
via a function pointer typedef IIRC.

I also have the following which _may_ motivate that attributes
currently not applying to function types (because they only
affect function definitions) should also apply there:

typedef int  (myfun)  (int *) __attribute__((nonnull(1)));
myfun x;
int x(int *p) { return p != (int*)0; }

this applies nonnull to the function definition of 'x' but
I put the attribute on the typedef.  I didn't manage to
do without the myfun x; declaration.

> It seems to me that any information about the target of a function
> pointer, even the flatten attribute or the ms_hook_prologue attribute,
> provides information that could be useful for optimizing the code
> around the indirect function call. That sounds like a compelling
> argument for allowing these attributes in more places without
> warnings.

Sure.  Can you write down the three cases after macro expansion
here to clarify what you need?  Esp. say what the attribute should
apply to.  Just silencing the warning without actually achieving
what you want would be bad I think ;)

Richard.