Re: [PATCH][libstd++][PR92156]
On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar
wrote:
> Fixes all this.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92156
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91630
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415
>
> On Fri, Apr 17, 2020 at 10:45 PM kamlesh kumar
> wrote:
> >
> > This patch corrects the requirement of 4,5 and 6th constructor
> > As per https://en.cppreference.com/w/cpp/utility/any/any.
> >
> > ChangeLog:
> > 2020-04-17 Kamlesh Kumar
> >
> > PR libstdc++/92156
> > * include/std/any (ans::any(_ValueType &&):: Remove
> is_constructible.
> > (any::any(in_place_type_t<_ValueType>, _Args&&...)): Use decay_t.
> > (any::any(in_place_type_t<_ValueType>,initializer_list<_Up>,
> _Args&&...)):
> > Use decay_t.
> > * testsuite/20_util/any/misc/92156.cc: New Test.
> >
> > diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
> > index 6b7e68f0e63..fb212eb2231 100644
> > --- a/libstdc++-v3/include/std/any
> > +++ b/libstdc++-v3/include/std/any
> > @@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > /// Construct with a copy of @p __value as the contained object.
> > template ,
> >typename _Mgr = _Manager<_Tp>,
> > - __any_constructible_t<_Tp, _ValueType&&> = true,
> > - enable_if_t::value, bool> = true>
> > + enable_if_t::value &&
> > + !__is_in_place_type<_Tp>::value, bool> = true>
> >any(_ValueType&& __value)
> >: _M_manager(&_Mgr::_S_manage)
> >{
> > _Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value));
> >}
> >
> > -/// Construct with a copy of @p __value as the contained object.
> > -template ,
> > - typename _Mgr = _Manager<_Tp>,
> > - enable_if_t<__and_v,
> > - __not_>,
> > - __not_<__is_in_place_type<_Tp>>>,
> > - bool> = false>
> > - any(_ValueType&& __value)
> > - : _M_manager(&_Mgr::_S_manage)
> > - {
> > -_Mgr::_S_create(_M_storage, __value);
> > - }
> > -
> > /// Construct with an object created from @p __args as the
> contained object.
> > template > - typename _Tp = _Decay<_ValueType>,
> > + typename _Tp = decay_t<_ValueType>,
> >typename _Mgr = _Manager<_Tp>,
> >__any_constructible_t<_Tp, _Args&&...> = false>
> >explicit
> > @@ -214,7 +201,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > /// Construct with an object created from @p __il and @p __args as
> > /// the contained object.
> > template > - typename _Tp = _Decay<_ValueType>,
> > + typename _Tp = decay_t<_ValueType>,
> >typename _Mgr = _Manager<_Tp>,
> >__any_constructible_t<_Tp, initializer_list<_Up>,
> > _Args&&...> = false>
> > diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc
> b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc
> > new file mode 100644
> > index 000..c4f1ed55aee
> > --- /dev/null
> > +++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc
> > @@ -0,0 +1,34 @@
> > +// { dg-options "-std=gnu++17" }
> > +// { dg-do compile }
> > +
> > +// Copyright (C) 2014-2020 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
> > +//
[RFA] Require powerpc_vsx_ok in gcc.target/powerpc/pr71763.c
From: Douglas Rupp
Hello,
(submitting this on behalf of Doug Rupp, one of my colleagues)
We're getting an error when running this test on PowerPC VxWorks 7,
due to an unexpected warning:
| Excess errors:
| cc1: warning: '-mvsx' and '-mno-altivec' are incompatible
The warning comes from a combination of factors:
- The test itself uses -mvsx explicitly via the following directive:
// { dg-options "-O1 -mvsx" }
- Our toolchain was configured so as to make -mno-altivec
the default;
- These two options are mutually exclusive.
This commit adds a powerpc_vsx_ok dg-require-effective-target directive
to that test, and thus making it UNSUPPORTED instead.
Tested on PowerPC VxWorks 7. Also tested on PowerPC ELF as well,
a platform where we do not make -mno-altivec the default, to verify
that the test continues to run as usual in that case.
gcc/testsuite/
* gcc.target/powerpc/pr71763.c: Require powerpc_vsx_ok.
OK for master?
Thanks!
--
Joel
---
gcc/testsuite/gcc.target/powerpc/pr71763.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gcc/testsuite/gcc.target/powerpc/pr71763.c
b/gcc/testsuite/gcc.target/powerpc/pr71763.c
index b36ddfa26b0..b394393 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr71763.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr71763.c
@@ -1,5 +1,6 @@
// PR target/71763
// { dg-do compile }
+// { dg-require-effective-target powerpc_vsx_ok }
// { dg-options "-O1 -mvsx" }
int a, b;
--
2.17.1
[committed] libstdc++: Add comparison operators to types
Some more C++20 changes from P1614R2, "The Mothership has Landed".
* include/std/chrono (duration, time_point): Define operator<=> and
remove redundant operator!= for C++20.
* testsuite/20_util/duration/comparison_operators/three_way.cc: New
test.
* testsuite/20_util/time_point/comparison_operators/three_way.cc: New
test.
Tested powerpc64le-linux, committed to master.
commit 27c171775abb943d59e2b3fb6fbc0b3680fc7a39
Author: Jonathan Wakely
Date: Sat Apr 18 00:47:45 2020 +0100
libstdc++: Add comparison operators to types
Some more C++20 changes from P1614R2, "The Mothership has Landed".
* include/std/chrono (duration, time_point): Define operator<=> and
remove redundant operator!= for C++20.
* testsuite/20_util/duration/comparison_operators/three_way.cc: New
test.
* testsuite/20_util/time_point/comparison_operators/three_way.cc:
New
test.
diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 514926c5c05..6d78f32ac78 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -43,6 +43,7 @@
#include // for literals support.
#if __cplusplus > 201703L
# include
+# include
#endif
namespace std _GLIBCXX_VISIBILITY(default)
@@ -668,12 +669,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return __ct(__lhs).count() < __ct(__rhs).count();
}
+#if __cpp_lib_three_way_comparison
+template
+ requires three_way_comparable>
+ constexpr auto
+ operator<=>(const duration<_Rep1, _Period1>& __lhs,
+ const duration<_Rep2, _Period2>& __rhs)
+ {
+ using __ct = common_type_t,
+ duration<_Rep2, _Period2>>;
+ return __ct(__lhs).count() <=> __ct(__rhs).count();
+ }
+#else
template
constexpr bool
operator!=(const duration<_Rep1, _Period1>& __lhs,
const duration<_Rep2, _Period2>& __rhs)
{ return !(__lhs == __rhs); }
+#endif
template
@@ -903,11 +918,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
const time_point<_Clock, _Dur2>& __rhs)
{ return __lhs.time_since_epoch() == __rhs.time_since_epoch(); }
+#if __cpp_lib_three_way_comparison
+template _Dur2>
+ constexpr auto
+ operator<=>(const time_point<_Clock, _Dur1>& __lhs,
+ const time_point<_Clock, _Dur2>& __rhs)
+ { return __lhs.time_since_epoch() <=> __rhs.time_since_epoch(); }
+#else
template
constexpr bool
operator!=(const time_point<_Clock, _Dur1>& __lhs,
const time_point<_Clock, _Dur2>& __rhs)
{ return !(__lhs == __rhs); }
+#endif
template
constexpr bool
diff --git
a/libstdc++-v3/testsuite/20_util/duration/comparison_operators/three_way.cc
b/libstdc++-v3/testsuite/20_util/duration/comparison_operators/three_way.cc
new file mode 100644
index 000..12c20f82811
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/duration/comparison_operators/three_way.cc
@@ -0,0 +1,62 @@
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+// Copyright (C) 2020 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
+//
[committed] libstdc++: Fix testsuite utility's use of allocators
In C++20 the rebind and const_reference members of std::allocator are
gone, so this testsuite utility stopped working, causing
ext/pb_ds/regression/priority_queue_rand_debug.cc to FAIL.
* testsuite/util/native_type/native_priority_queue.hpp: Use
allocator_traits to rebind allocator.
Tested x86_64-linux, committed to master.
commit c9960294062dbda0847d26a1b5ee37a55210d69c
Author: Jonathan Wakely
Date: Sat Apr 18 00:10:54 2020 +0100
libstdc++: Fix testsuite utility's use of allocators
In C++20 the rebind and const_reference members of std::allocator are
gone, so this testsuite utility stopped working, causing
ext/pb_ds/regression/priority_queue_rand_debug.cc to FAIL.
* testsuite/util/native_type/native_priority_queue.hpp: Use
allocator_traits to rebind allocator.
diff --git a/libstdc++-v3/testsuite/util/native_type/native_priority_queue.hpp
b/libstdc++-v3/testsuite/util/native_type/native_priority_queue.hpp
index 1445892ebe0..1ceb446c0ee 100644
--- a/libstdc++-v3/testsuite/util/native_type/native_priority_queue.hpp
+++ b/libstdc++-v3/testsuite/util/native_type/native_priority_queue.hpp
@@ -55,20 +55,30 @@ namespace __gnu_pbds
struct base_seq
{
private:
- typedef typename _Alloc::template rebind value_rebind;
+#if __cplusplus >= 201103L
+ using value_alloc = typename std::allocator_traits<_Alloc>::template
+ rebind_alloc;
+#else
+ typedef typename _Alloc::template rebind::other value_alloc;
+#endif
public:
- typedef std::vector type;
+ typedef std::vector type;
};
template
struct base_seq
{
private:
- typedef typename _Alloc::template rebind value_rebind;
+#if __cplusplus >= 201103L
+ using value_alloc = typename std::allocator_traits<_Alloc>::template
+ rebind_alloc;
+#else
+ typedef typename _Alloc::template rebind::other value_alloc;
+#endif
public:
- typedef std::deque type;
+ typedef std::deque type;
};
} // namespace detail
@@ -89,11 +99,16 @@ namespace __gnu_pbds
{
private:
typedef PB_DS_BASE_C_DEC base_type;
- typedef typename _Alloc::template rebind value_rebind;
+#if __cplusplus >= 201103L
+ using value_ref = const Value_Type&;
+#else
+ typedef typename
+ _Alloc::template rebind::other::const_reference value_ref;
+#endif
public:
typedef Value_Type value_type;
- typedef typename value_rebind::other::const_reference const_reference;
+ typedef value_ref const_reference;
typedef native_pq_tag container_category;
typedef Cmp_Fn cmp_fn;
[committed] libstdc++: Add comparison operators to sequence containers
Some more C++20 changes from P1614R2, "The Mothership has Landed".
This implements <=> for sequence containers (and the __normal_iterator
and _Pointer_adapter class templates).
* include/bits/forward_list.h (forward_list): Define operator<=> and
remove redundant comparison operators for C++20.
* include/bits/stl_bvector.h (vector): Likewise.
* include/bits/stl_deque.h (deque): Likewise.
* include/bits/stl_iterator.h (__normal_iterator): Likewise.
* include/bits/stl_list.h (list): Likewise.
* include/bits/stl_vector.h (vector): Likewise.
* include/debug/deque (__gnu_debug::deque): Likewise.
* include/debug/forward_list (__gnu_debug::forward_list): Likewise.
* include/debug/list (__gnu_debug::list): Likewise.
* include/debug/safe_iterator.h (__gnu_debug::_Safe_iterator):
Likewise.
* include/debug/vector (__gnu_debug::vector): Likewise.
* include/ext/pointer.h (__gnu_cxx::_Pointer_adapter): Define
operator<=> for C++20.
* testsuite/23_containers/deque/operators/cmp_c++20.cc: New test.
* testsuite/23_containers/forward_list/cmp_c++20.cc: New test.
* testsuite/23_containers/list/cmp_c++20.cc: New test.
* testsuite/23_containers/vector/bool/cmp_c++20.cc: New test.
* testsuite/23_containers/vector/cmp_c++20.cc: New test.
Tested powerpc64le-linux, committed to master.
I have patches on the way for associative containers and ,
after which the only parts of P1614R2 missing are the unordered
containers and container adaptors, which are both easy.
commit bd2420f8faaf4bb33310e82f7dd45c5e33476c87
Author: Jonathan Wakely
Date: Fri Apr 17 23:41:04 2020 +0100
libstdc++: Add comparison operators to sequence containers
Some more C++20 changes from P1614R2, "The Mothership has Landed".
This implements <=> for sequence containers (and the __normal_iterator
and _Pointer_adapter class templates).
* include/bits/forward_list.h (forward_list): Define operator<=> and
remove redundant comparison operators for C++20.
* include/bits/stl_bvector.h (vector): Likewise.
* include/bits/stl_deque.h (deque): Likewise.
* include/bits/stl_iterator.h (__normal_iterator): Likewise.
* include/bits/stl_list.h (list): Likewise.
* include/bits/stl_vector.h (vector): Likewise.
* include/debug/deque (__gnu_debug::deque): Likewise.
* include/debug/forward_list (__gnu_debug::forward_list): Likewise.
* include/debug/list (__gnu_debug::list): Likewise.
* include/debug/safe_iterator.h (__gnu_debug::_Safe_iterator):
Likewise.
* include/debug/vector (__gnu_debug::vector): Likewise.
* include/ext/pointer.h (__gnu_cxx::_Pointer_adapter): Define
operator<=> for C++20.
* testsuite/23_containers/deque/operators/cmp_c++20.cc: New test.
* testsuite/23_containers/forward_list/cmp_c++20.cc: New test.
* testsuite/23_containers/list/cmp_c++20.cc: New test.
* testsuite/23_containers/vector/bool/cmp_c++20.cc: New test.
* testsuite/23_containers/vector/cmp_c++20.cc: New test.
diff --git a/libstdc++-v3/include/bits/forward_list.h
b/libstdc++-v3/include/bits/forward_list.h
index b926d45f205..49b2a973718 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -180,13 +180,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
operator==(const _Self& __x, const _Self& __y) noexcept
{ return __x._M_node == __y._M_node; }
-
+#if __cpp_impl_three_way_comparison < 201907L
/**
* @brief Forward list iterator inequality comparison.
*/
friend bool
operator!=(const _Self& __x, const _Self& __y) noexcept
{ return __x._M_node != __y._M_node; }
+#endif
_Self
_M_next() const noexcept
@@ -258,12 +259,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
operator==(const _Self& __x, const _Self& __y) noexcept
{ return __x._M_node == __y._M_node; }
+#if __cpp_impl_three_way_comparison < 201907L
/**
* @brief Forward list const_iterator inequality comparison.
*/
friend bool
operator!=(const _Self& __x, const _Self& __y) noexcept
{ return __x._M_node != __y._M_node; }
+#endif
_Self
_M_next() const noexcept
@@ -1426,6 +1429,28 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
operator==(const forward_list<_Tp, _Alloc>& __lx,
const forward_list<_Tp, _Alloc>& __ly);
+#if __cpp_lib_three_way_comparison
+ /**
+ * @brief Forward list ordering relation.
+ * @param __x A `forward_list`.
+ * @param __y A `forward_list` of the same type as `__x`.
+ * @return A value indicating whether `__x` is less than, equal to,
+ * greater than, or incomparable with
Re: [PATCH] wwwdocs: document my changes for gcc 10
On Thu, 16 Apr 2020, David Malcolm wrote: > Validates. The wording could probably use some work. I did not really spot anything, and rather found your writing very clear. > OK to push to the website repo? Yes, thank you. > +https://cwe.mitre.org/;>CWE weakness identifiers, which Here you can omit the trailing /, verified on my end. Quite an impressive list, by the way! Gerald
Re: [PATCH] wwwdocs: document my changes for gcc 10
On Thu, 2020-04-16 at 19:15 -0400, David Malcolm via Gcc-patches wrote: > Validates. The wording could probably use some work. > > OK to push to the website repo? OK. As are any small updates if you wanted to twiddle the wording. jeff >
Re: [PATCH] x86: Insert ENDBR if function will be called indirectly
On Fri, 2020-04-17 at 08:18 -0700, H.J. Lu wrote:
> On Wed, Apr 8, 2020 at 9:41 AM Jeff Law wrote:
> > On Wed, 2020-04-08 at 09:23 -0700, H.J. Lu wrote:
> > > On Wed, Apr 8, 2020 at 9:16 AM Jeff Law wrote:
> > > > On Tue, 2020-03-31 at 08:11 -0700, H.J. Lu via Gcc-patches wrote:
> > > > > Since constant_call_address_operand has
> > > > >
> > > > > ;; Test for a pc-relative call operand
> > > > > (define_predicate "constant_call_address_operand"
> > > > > (match_code "symbol_ref")
> > > > > {
> > > > > if (ix86_cmodel == CM_LARGE || ix86_cmodel == CM_LARGE_PIC
> > > > > || flag_force_indirect_call)
> > > > > return false;
> > > > > if (TARGET_DLLIMPORT_DECL_ATTRIBUTES && SYMBOL_REF_DLLIMPORT_P (op))
> > > > > return false;
> > > > > return true;
> > > > > })
> > > > >
> > > > > even if cgraph_node::get (cfun->decl)->only_called_directly_p ()
> > > > > returns
> > > > > false, the fuction may still be called indirectly. Copy the logic
> > > > > from
> > > > > constant_call_address_operand to rest_of_insert_endbranch to insert
> > > > > ENDBR
> > > > > at function entry if function will be called indirectly.
> > > > >
> > > > > gcc/
> > > > >
> > > > > PR target/94417
> > > > > * config/i386/i386-features.c (rest_of_insert_endbranch): Insert
> > > > > ENDBR at function entry if function will be called indirectly.
> > > > Can you just call constant_call_address_operand rather than copying its
> > > > contents?
> > >
> > > I wish I could. constant_call_address_operand uses SYMBOL_REF_DLLIMPORT_P
> > > (op)
> > > But I need to use DECL_DLLIMPORT_P (cfun->decl)).
> > Sigh. In that case I guess the patch is OK as-is.
> >
>
> I'd like to backport this wrong code fix to GCC 9/8 branches.
> Is it OK for GCC 9/8 branches?
Sure.
jeff
[committed] [PR rtl-optimization/90275] Another 90275 related cse.c fix
This isn't precisely the same issue that we were originally tracking with 90275, but it's closely related and we might as well stuff it in the same bucket. This time instead of having a NOP copy insn that we can completely ignore and ultimately remove, we have a NOP set within a multi-set PARALLEL. It ultimately triggers the same failure when the source of such a set is a hard register for the same reasons as we've already noted in the BZ and patches-to-date. For prior cases we've been able to mark the insn as a nop set and ignore it for the rest of cse_insn, ultimately removing it. That's not really an option here as there are other sets that we have to preserve. We might be able to fix this instance by splitting the multi-set insn, but I'm not keen to introduce splitting into cse. Furthermore, the target may not be able to split the insn. So I considered this is non-starter. What I finally settled on was to use the existing do_not_record machinery to ignore the nop set within the parallel (and only that set within the parallel). One might argue that we should always ignore a REG_UNUSED set. But I rejected that idea -- we could have cse-able divmod insns where the first had a REG_UNUSED note for a destination, but the second did not. One might also argue that we could have a nop set without a REG_UNUSED in a multi-set parallel and thus we could trigger yet another insert_regs ICE at some point. I tend to think this is a possibility. If we see this happen, we'll have to revisit. One might also argue that cse should, in general, be more tolerant of dead code and nop sets. I'd fully agree with that. But I loathe the idea of revamping that code and fear that we'd likely get it wrong along the way. Certainly not appropriate at the current time. Bootstrapped and regression tested on a variety of *-linux-gnu targets (including arm & aarch64). Also regression tested on all the usual *-elf targets in the tester. Installing on the trunk, jeff commit 3737ccc424c56a2cecff202dd79f88d28850eeb2 Author: Jeff Law Date: Fri Apr 17 15:38:13 2020 -0600 [committed] [PR rtl-optimization/90275] Another 90275 related cse.c fix This time instead of having a NOP copy insn that we can completely ignore and ultimately remove, we have a NOP set within a multi-set PARALLEL. It triggers, the same failure when the source of such a set is a hard register for the same reasons as we've already noted in the BZ and patches-to-date. For prior cases we've been able to mark the insn as a nop set and ignore it for the rest of cse_insn, ultimately removing it. That's not really an option here as there are other sets that we have to preserve. We might be able to fix this instance by splitting the multi-set insn, but I'm not keen to introduce splitting into cse. Furthermore, the target may not be able to split the insn. So I considered this is non-starter. What I finally settled on was to use the existing do_not_record machinery to ignore the nop set within the parallel (and only that set within the parallel). One might argue that we should always ignore a REG_UNUSED set. But I rejected that idea -- we could have cse-able divmod insns where the first had a REG_UNUSED note for a destination, but the second did not. One might also argue that we could have a nop set without a REG_UNUSED in a multi-set parallel and thus we could trigger yet another insert_regs ICE at some point. I tend to think this is a possibility. If we see this happen, we'll have to revisit. PR rtl-optimization/90275 * cse.c (cse_insn): Avoid recording nop sets in multi-set parallels when the destination has a REG_UNUSED note. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 93badc209ae..47f22327542 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2020-04-17 Jeff Law + + PR rtl-optimization/90275 + * cse.c (cse_insn): Avoid recording nop sets in multi-set parallels + when the destination has a REG_UNUSED note. + 2020-04-17 Tobias Burnus PR middle-end/94635 diff --git a/gcc/cse.c b/gcc/cse.c index f07bbdbebad..5aaba8d80e0 100644 --- a/gcc/cse.c +++ b/gcc/cse.c @@ -4715,8 +4715,20 @@ cse_insn (rtx_insn *insn) /* Compute SRC's hash code, and also notice if it should not be recorded at all. In that case, -prevent any further processing of this assignment. */ - do_not_record = 0; +prevent any further processing of this assignment. + +We set DO_NOT_RECORD if the destination has a REG_UNUSED note. +This avoids getting the source register into the tables, where it +may be invalidated later (via REG_QTY), then trigger an ICE upon +re-insertion. + +This is only a problem in multi-set insns. If it were a single +set the dead copy would have been removed.
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/17/20 12:19 AM, Jason Merrill wrote:
On 4/15/20 1:30 PM, Martin Sebor wrote:
On 4/13/20 8:43 PM, Jason Merrill wrote:
On 4/12/20 5:49 PM, Martin Sebor wrote:
On 4/10/20 8:52 AM, Jason Merrill wrote:
On 4/9/20 4:23 PM, Martin Sebor wrote:
On 4/9/20 1:32 PM, Jason Merrill wrote:
On 4/9/20 3:24 PM, Martin Sebor wrote:
On 4/9/20 1:03 PM, Jason Merrill wrote:
On 4/8/20 1:23 PM, Martin Sebor wrote:
On 4/7/20 3:36 PM, Marek Polacek wrote:
On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote:
On 4/7/20 1:50 PM, Marek Polacek wrote:
On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via
Gcc-patches wrote:
Among the numerous regressions introduced by the change
committed
to GCC 9 to allow string literals as template arguments is
a failure
to recognize the C++ nullptr and GCC's __null constants as
pointers.
For one, I didn't realize that nullptr, being a null
pointer constant,
doesn't have a pointer type, and two, I didn't think of
__null (which
is a special integer constant that NULL sometimes expands
to).
The attached patch adjusts the special handling of
trailing zero
initializers in reshape_init_array_1 to recognize both
kinds of
constants and avoid treating them as zeros of the array
integer
element type. This restores the expected diagnostics when
either
constant is used in the initializer list.
Martin
PR c++/94510 - nullptr_t implicitly cast to zero twice in
std::array
gcc/cp/ChangeLog:
PR c++/94510
* decl.c (reshape_init_array_1): Exclude mismatches
with all kinds
of pointers.
gcc/testsuite/ChangeLog:
PR c++/94510
* g++.dg/init/array57.C: New test.
* g++.dg/init/array58.C: New test.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index a127734af69..692c8ed73f4 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree
elt_type, tree max_index, reshape_iter *d,
TREE_CONSTANT (new_init) = false;
/* Pointers initialized to strings must be
treated as non-zero
- even if the string is empty. */
+ even if the string is empty. Handle all kinds of
pointers,
+ including std::nullptr and GCC's __nullptr, neither
of which
+ has a pointer type. */
tree init_type = TREE_TYPE (elt_init);
- if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P
(init_type)
+ bool init_is_ptr = (POINTER_TYPE_P (init_type)
+ || NULLPTR_TYPE_P (init_type)
+ || null_node_p (elt_init));
+ if (POINTER_TYPE_P (elt_type) != init_is_ptr
|| !type_initializer_zero_p (elt_type, elt_init))
last_nonzero = index;
It looks like this still won't handle e.g. pointers to
member functions,
e.g.
struct S { };
int arr[3] = { (void (S::*) ()) 0, 0, 0 };
would still be accepted. You could use
TYPE_PTR_OR_PTRMEM_P instead of
POINTER_TYPE_P to catch this case.
Good catch! That doesn't fail because unlike null data
member pointers
which are represented as -1, member function pointers are
represented
as a zero.
I had looked for an API that would answer the question: "is
this
expression a pointer?" without having to think of all the
different
kinds of them but all I could find was null_node_p(). Is
this a rare,
isolated case that having an API like that wouldn't be worth
having
or should I add one like in the attached update?
Martin
PR c++/94510 - nullptr_t implicitly cast to zero twice in
std::array
gcc/cp/ChangeLog:
PR c++/94510
* decl.c (reshape_init_array_1): Exclude mismatches with
all kinds
of pointers.
* gcc/cp/cp-tree.h (null_pointer_constant_p): New function.
(Drop the gcc/cp/.)
+/* Returns true if EXPR is a null pointer constant of any
type. */
+
+inline bool
+null_pointer_constant_p (tree expr)
+{
+ STRIP_ANY_LOCATION_WRAPPER (expr);
+ if (expr == null_node)
+ return true;
+ tree type = TREE_TYPE (expr);
+ if (NULLPTR_TYPE_P (type))
+ return true;
+ if (POINTER_TYPE_P (type))
+ return integer_zerop (expr);
+ return null_member_pointer_value_p (expr);
+}
+
We already have a null_ptr_cst_p so it would be sort of
confusing to have
this as well. But are you really interested in whether it's
a null pointer,
not just a pointer?
The goal of the code is to detect a mismatch in "pointerness"
between
an initializer expression and the type of the initialized
element, so
it needs to know if the expression is a pointer (non-nulls
pointers
are detected in type_initializer_zero_p). That means testing
a number
of IMO unintuitive conditions:
TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr))
|| NULLPTR_TYPE_P (TREE_TYPE (expr))
|| null_node_p (expr)
I don't know if this type of a query is common in the C++ FE
but unless
this is an isolated use case then besides fixing the bug I
thought it
would be nice to make it easier to get the test above right,
or at least
come close to it.
Since null_pointer_constant_p already exists (but isn't
suitable here
because it returns true for
Re: [RFC] split pseudos during loop unrolling in RTL unroller
On Fri, Apr 17, 2020 at 08:53:08AM +0200, Richard Biener wrote: > On Thu, Apr 16, 2020 at 7:46 PM Segher Boessenkool > wrote: > > > > On Thu, Apr 16, 2020 at 10:33:45AM +0200, Richard Biener wrote: > > > On Wed, Apr 15, 2020 at 11:23 PM Segher Boessenkool > > > > On a general note, we shouldn't depend on some pass that may or may not > > > > clean up the mess we make, when we could just avoid making a mess in the > > > > first place. > > > > > > True - but the issue at hand is not trivial given you have to care for > > > partial defs, uses outside of the loop (or across the backedge), etc. > > > So there's plenty of things to go "wrong" here. > > > > Certainly. But *all* RTL passes before RA should leave things in "web > > form" (compare to SSA form). The code in web.c is probably just fine; > > but we shouldn't have one web pass, *all* passes should leave things in > > a useful form! > > Yeah well, but RTL is not in SSA form "Webs" are not the *same* as SSA, in a few crucial ways; but they serve similar purposes: they both make code transformations easier to do. Both do this by having more different (independent) names. > and there's no RTL IL verification > in place to track degradation. Adding this isn't hard in principle. But currently it is violated all over the place, including in backend code. > And we even work in the opposite way > when expanding to RTL from SSA, coalescing as much as we can ... Which often is bad. A lot of what expand does is premature optimisation that a) is much too complicated, and b) results in worse code in the end. Expand should not try to do *anything* "interesting", it should "merely" translate to RTL. (And in a way that can be debugged sanely at all, if possible ;-) ) > > > > The web pass belongs immediately after expand; but ideally, even expand > > > > would not reuse pseudos anyway. > > > > > > But for example when lower-subreg decomposes things in a way turning > > > partial defs into full defs new opportunities to split the web arise. > > > > But that is only for the new registers it creates, or what am I missing? > > No idea, just made up the example that maintaing "SSA" RTL is > not automagic. Oh sure, but in most cases, it is. My point is that making web form only once (and messing it up after that) isn't great. > > > > Maybe it would be better as some utility routines, not a pass? > > > > > > Sure, but then when do we apply it? > > > > All of the time! Ideally every pass would leave things in a good shape. > > Usually it is cheapest as well as easiest to do things manually, but for > > some harder cases, such helper routines can be used. > > > > > Ideally scheduling would to > > > register renaming itself and thus not rely on the used pseudos > > > (I'm not sure if it tracks false dependences - I guess it must if it > > > isn't able to rename regs). That would be a much better place > > > for improvements? > > > > sched2 runs after RA, so it has nothing to do with webs? And sched1 > > doesn't do much relevant here (it doesn't move insns much). > > > > I don't see how this is directly related to register renaming either? > > If scheduling ignores "false" dependences (anti dependence with > full defs) then when it schedules across such defs needs to perform > renaming. Maybe I'm using bogus terms here. Your terminology is fine afaic, but I don't see what you mean, sorry. Maybe I don't know sched well enough... I thought it would just refuse to move something if that would result in broken code? Segher
[PATCH] c++: Fix ICE with { } as template argument [PR94592]
As an extension (there should be a CWG about this though), we support
braced-init-list as a template argument, but convert_nontype_argument
had trouble digesting them. We ICEd because of the double coercion we
perform for template arguments: convert_nontype_argument called from
finish_template_type got a { }, and since a class type was involved and
we were in a template, convert_like created an IMPLICIT_CONV_EXPR. Then
the second conversion of the same argument crashed in constexpr.c
because the IMPLICIT_CONV_EXPR had gotten wrapped in a TARGET_EXPR.
Another issue was that an IMPLICIT_CONV_EXPR leaked to constexpr.c when
building an aggregate init.
We should have instantiated the IMPLICIT_CONV_EXPR in the first call to
convert_nontype_argument, but we didn't, because the call to
is_nondependent_constant_expression returned false because it checks
!BRACE_ENCLOSED_INITIALIZER_P. Then non_dep was false even though the
expression didn't contain anything dependent and we didn't instantiate
it in convert_nontype_argument. I'm not sure what the point
of that BRACE_ENCLOSED_INITIALIZER_P. check was, but removing it doesn't
break anything and fixes these crashes.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
PR c++/94592
* constexpr.c (is_nondependent_constant_expression): Don't check
BRACE_ENCLOSED_INITIALIZER_P.
(is_nondependent_static_init_expression): Likewise.
* g++.dg/cpp2a/nontype-class34.C: New test.
* g++.dg/cpp2a/nontype-class35.C: New test.
---
gcc/cp/constexpr.c | 2 --
gcc/testsuite/g++.dg/cpp2a/nontype-class34.C | 16
gcc/testsuite/g++.dg/cpp2a/nontype-class35.C | 17 +
3 files changed, 33 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class34.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class35.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index c8e7d083f40..21a5bbd798c 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -8295,7 +8295,6 @@ bool
is_nondependent_constant_expression (tree t)
{
return (!type_unknown_p (t)
- && !BRACE_ENCLOSED_INITIALIZER_P (t)
&& is_constant_expression (t)
&& !instantiation_dependent_expression_p (t));
}
@@ -8307,7 +8306,6 @@ bool
is_nondependent_static_init_expression (tree t)
{
return (!type_unknown_p (t)
- && !BRACE_ENCLOSED_INITIALIZER_P (t)
&& is_static_init_expression (t)
&& !instantiation_dependent_expression_p (t));
}
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class34.C
b/gcc/testsuite/g++.dg/cpp2a/nontype-class34.C
new file mode 100644
index 000..2d3ba018618
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class34.C
@@ -0,0 +1,16 @@
+// PR c++/94592 - ICE with { } as template argument.
+// { dg-do compile { target c++2a } }
+
+struct A {
+ constexpr A() {}
+};
+
+template struct B { };
+
+template void bar () {
+B<{}> var;
+}
+
+void fu() {
+bar();
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class35.C
b/gcc/testsuite/g++.dg/cpp2a/nontype-class35.C
new file mode 100644
index 000..78cf0a39c81
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class35.C
@@ -0,0 +1,17 @@
+// PR c++/94592 - ICE with { } as template argument.
+// { dg-do compile { target c++2a } }
+
+struct A {
+int i;
+constexpr A(int n) : i(n) {}
+};
+
+template struct B { int i; constexpr B() : i(a.i) { } };
+
+template void bar () {
+B<{1}> var;
+}
+
+void fu() {
+bar();
+}
base-commit: a28edad3da5c59f09565d3d42e20be1a924986c4
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
Re: [patch, fortran] Fix PR PR93500
On Fri, Apr 17, 2020 at 10:33 AM Thomas Koenig wrote: > > Hi Fritz, > > > First, it appears if simplify_bound_dim returns _bad_expr (and a > > div/0 occurs) then this code will free _bad_expr. I'm not sure > > whether or not that can actually occur, but it is certainly incorrect, > > since _bad_expr points to static storage. The only other possible > > case is bounds[d] == NULL, in which case the free is a no-op. I > > suggest removing the free call. > > OK, removed. > > > That being said, it looks like the same error condition can occur with > > the lcobound intrinsic. > > I have adjusted the test case so it also checks for the coarray case; > this is handled correctly with a "Divion by zero" error without > changing the patch in that respect. > > So, anything else? If not, I will commit this within a few days. > Let the record show I am unsettled that the error path is different between simplify_bound and simplify_cobound, and that different errors occur for the program and subroutine case. There is probably a root cause somewhere else that should be fixed one day. However! That is not a problem for this PR, nor for stage 4, and is certainly no fault of this patch. Therefore the patch looks OK to me for now. Thank you for your work! --- Fritz Reese
Re: [PATCH] aarch64: Fix .cfi_window_save with pac-ret [PR94515]
On 4/17/20 1:55 PM, Szabolcs Nagy wrote:
The 04/17/2020 12:50, Jason Merrill wrote:
On 4/17/20 6:08 AM, Kyrylo Tkachov wrote:
Hi Szabolcs,
-Original Message-
From: Szabolcs Nagy
Sent: 09 April 2020 15:20
To: gcc-patches@gcc.gnu.org
Cc: Richard Earnshaw ; Richard Sandiford
; Kyrylo Tkachov
Subject: [PATCH] aarch64: Fix .cfi_window_save with pac-ret [PR94515]
On aarch64 -mbranch-protection=pac-ret reuses the dwarf
opcode for window_save to mean "toggle the return address
mangle state", but in the dwarf2cfi internal logic the
state was not properly recorded so the remember/restore
logic was broken.
This can cause the unwinder not to authenticate return
addresses that were signed (or vice versa) which means
a runtime crash on a pauth enabled system.
Currently only aarch64 pac-ret uses REG_CFA_TOGGLE_RA_MANGLE.
I think this is ok, but this code is in the midend so I've CC'ed Jason who,
from what I gather from MAINTAINERS, is maintainer for this file.
Thanks,
Kyrill
gcc/ChangeLog:
2020-04-XX Szabolcs Nagy
PR target/94515
* dwarf2cfi.c (dwarf2out_frame_debug): Record RA
mangle state in cur_row->window_save.
gcc/testsuite/ChangeLog:
2020-04-XX Szabolcs Nagy
PR target/94515
* g++.target/aarch64/pr94515.C: New test.
---
gcc/dwarf2cfi.c| 3 ++
gcc/testsuite/g++.target/aarch64/pr94515.C | 43 ++
2 files changed, 46 insertions(+)
create mode 100644 gcc/testsuite/g++.target/aarch64/pr94515.C
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index 229fbfacc30..22989a6c2fb 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -2145,6 +2145,9 @@ dwarf2out_frame_debug (rtx_insn *insn)
case REG_CFA_TOGGLE_RA_MANGLE:
/* This uses the same DWARF opcode as the next operation. */
dwarf2out_frame_debug_cfa_window_save (true);
+ /* Keep track of RA mangle state by toggling the window_save bit.
+ This is needed so .cfi_window_save is emitted correctly. */
+ cur_row->window_save = !cur_row->window_save;
It looks like passing 'true' to dwarf2out_frame_debug_cfa_window_save
prevents that function from messing with the window_safe flag. Does
changing the argument to 'false' get the behavior you want?
we want the state = !state toggling.
it might make more sense to do that in
dwarf2out_frame_debug_cfa_window_save(true)
or to inline that entire logic into the two
places where it is used (instead of
dispatching with a bool argument).
I think that inlining and dropping the parameter would be cleaner.
for the bug fix i'd like a minimal change
(so it can be backported), doing the fix in
dwarf2out_frame_debug_cfa_window_save
is fine with me, would you prefer that?
No, thanks. If you want to commit your patch as is for backporting and
then do the inlining in a separate commit, that works for me.
handled_one = true;
break;
diff --git a/gcc/testsuite/g++.target/aarch64/pr94515.C
b/gcc/testsuite/g++.target/aarch64/pr94515.C
new file mode 100644
index 000..7707c773a72
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/pr94515.C
@@ -0,0 +1,43 @@
+/* PR target/94515. Check .cfi_window_save with multiple return paths. */
+/* { dg-do run } */
+/* { dg-additional-options "-O2 -mbranch-protection=pac-ret --save-temps" }
*/
+
+volatile int zero = 0;
+
+__attribute__((noinline, target("branch-protection=none")))
+void unwind (void)
+{
+ if (zero == 0)
+throw 42;
+}
+
+__attribute__((noinline, noipa, target("branch-protection=pac-ret")))
+int test (int z)
+{
+ if (z) {
+asm volatile ("":::"x20","x21");
+unwind ();
+return 1;
+ } else {
+unwind ();
+return 2;
+ }
+}
+
+__attribute__((target("branch-protection=none")))
+int main ()
+{
+ try {
+test (zero);
+__builtin_abort ();
+ } catch (...) {
+return 0;
+ }
+ __builtin_abort ();
+}
+
+/* This check only works if there are two return paths in test and
+ cfi_window_save is used for both instead of cfi_remember_state
+ plus cfi_restore_state. This is currently the case with -O2. */
+
+/* { dg-final { scan-assembler-times {\t\.cfi_window_save\n} 4 } } */
--
2.17.1
Re: [PATCH] c++: spec_hasher::equal and PARM_DECLs [PR94632]
On 4/17/20 1:35 PM, Patrick Palka wrote:
In the testcase below, during specialization of c::d, we build two
identical specializations of the parameter type b -- one when
substituting into c::d's TYPE_ARG_TYPES and another when substituting into
c::d's DECL_ARGUMENTS.
We don't reuse the first specialization the second time around as a consequence
of the fix for PR c++/56247 which made PARM_DECLs always compare different from
one another during spec_hasher::equal. As a result, when looking up existing
specializations of 'b', spec_hasher::equal considers the template argument
decltype(e')::k to be different from decltype(e'')::k, where e' and e'' are the
result of two calls to tsubst_copy on the PARM_DECL e.
Since the two specializations are considered different due to the mentioned fix,
their TYPE_CANONICAL points to themselves even though they are otherwise
identical types, and this triggers an ICE in maybe_rebuild_function_decl_type
when comparing the TYPE_ARG_TYPES of c::d to its DECL_ARGUMENTS.
This patch fixes this issue at the spec_hasher::equal level by ignoring the
'comparing_specializations' flag in cp_tree_equal whenever the DECL_CONTEXTs of
the two parameters are identical. This seems to be a sufficient condition to be
able to correctly compare PARM_DECLs structurally. (This also subsumes the
CONSTRAINT_VAR_P check since constraint variables all have empty, and therefore
identical, DECL_CONTEXTs.)
OK.
I can think of two other ways to avoid this ICE. One solution would be to do
maybe_rebuild_function_decl_type only when we have instantiated the function,
and not when we are just specializing it during instantiation of an enclosing
class template. This would avoid the problematic same_type_p call altogether.
Another solution would be to set up a local_specialization_stack in
instantiate_class_template_1 so that we can reuse PARM_DECL specializations
during tsubst_copy. As a consequence, e' and e'' mentioned above would be
identical.
Hmm, I would think we would want the local_specialization stack closer
to the function substitution, not specific to the class template
instantiation.
This patch passes 'make check-c++', and a full bootstrap/regtest is in progress.
gcc/cp/ChangeLog:
PR c++/94632
* tree.c (cp_tree_equal) : Ignore
comparing_specializations if the parameters' contexts are identical.
gcc/testsuite/ChangeLog:
PR c++/94632
* g++.dg/template/canon-type-14.C: New test.
---
gcc/cp/tree.c | 5 +++--
gcc/testsuite/g++.dg/template/canon-type-14.C | 8
2 files changed, 11 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/canon-type-14.C
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 8e4934c0009..dc4f1f48d3c 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -3723,11 +3723,12 @@ cp_tree_equal (tree t1, tree t2)
up for expressions that involve 'this' in a member function
template. */
- if (comparing_specializations && !CONSTRAINT_VAR_P (t1))
+ if (comparing_specializations
+ && DECL_CONTEXT (t1) != DECL_CONTEXT (t2))
/* When comparing hash table entries, only an exact match is
good enough; we don't want to replace 'this' with the
version from another function. But be more flexible
- with local parameters in a requires-expression. */
+ with parameters with identical contexts. */
return false;
if (same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
diff --git a/gcc/testsuite/g++.dg/template/canon-type-14.C
b/gcc/testsuite/g++.dg/template/canon-type-14.C
new file mode 100644
index 000..fa05bdb9a74
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/canon-type-14.C
@@ -0,0 +1,8 @@
+// PR c++/94632
+// { dg-do compile { target c++11 } }
+
+template struct b;
+template class c {
+ template static void d(f e, b);
+};
+template class c;
Re: introduce target tmpnam and require it in tests relying on it
On 4/17/20 11:48 AM, Alexandre Oliva wrote: On Apr 9, 2020, Alexandre Oliva wrote: Some target C libraries that aren't recognized as freestanding don't have filesystem support, so calling tmpnam, fopen/open and remove/unlink fails to link. This patch introduces a tmpnam effective target to the testsuite, and requires it in the tests that call tmpnam. Tested on x86_64-linux-gnu, and with a cross to arm-eabi. Ok to install? for gcc/testsuite/ChangeLog * lib/target-supports.exp (check_effective_target_tmpnam): New. * gcc.c-torture/execute/fprintf-2.c: Require it. * gcc.c-torture/execute/printf-2.c: Likewise. * gcc.c-torture/execute/user-printf.c: Likewise. Ping? https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543672.html I'm okay with the changes to the tests. The target-supports.exp changes look reasonable to me as well but I can't approve them. Since you said it's for targets that don't have file I/O functions I wonder if the name would better reflect that if it were called, say, check_effective_target_fileio? I don't expect it's necessary to worry about handling errors in the .exp test. Martin
Re: [PATCH] c++: Abbreviated function template return type [PR92187]
On 4/17/20 9:40 AM, Patrick Palka wrote:
When updating an auto return type of an abbreviated function template in
splice_late_return_type, we should also propagate PLACEHOLDER_TYPE_CONSTRAINTS
(and cv-qualifiers) of the original auto node.
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to commit?
gcc/cp/ChangeLog:
PR c++/92187
* pt.c (splice_late_return_type): Propagate cv-qualifiers and
PLACEHOLDER_TYPE_CONSTRAINTS from the original auto node to the new one.
gcc/testsuite/ChangeLog:
PR c++/92187
* g++.dg/concepts/abbrev5.C: New test.
* g++.dg/concepts/abbrev6.C: New test.
---
gcc/cp/pt.c | 15 +++
gcc/testsuite/g++.dg/concepts/abbrev5.C | 15 +++
gcc/testsuite/g++.dg/concepts/abbrev6.C | 12
3 files changed, 38 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev5.C
create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev6.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 0a8ec3198d2..9e39f46a090 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -29032,10 +29032,17 @@ splice_late_return_type (tree type, tree
late_return_type)
{
tree idx = get_template_parm_index (*auto_node);
if (TEMPLATE_PARM_LEVEL (idx) <= processing_template_decl)
- /* In an abbreviated function template we didn't know we were dealing
- with a function template when we saw the auto return type, so update
- it to have the correct level. */
- *auto_node = make_auto_1 (TYPE_IDENTIFIER (*auto_node), true);
+ {
+ /* In an abbreviated function template we didn't know we were dealing
+with a function template when we saw the auto return type, so
update
+it to have the correct level. */
+ tree new_auto = make_auto_1 (TYPE_IDENTIFIER (*auto_node), false);
+ PLACEHOLDER_TYPE_CONSTRAINTS (new_auto)
+ = PLACEHOLDER_TYPE_CONSTRAINTS (*auto_node);
I wonder about trying to share code between here and tsubst level
lowering, but I guess this is short enough. OK
+ TYPE_CANONICAL (new_auto) = canonical_type_parameter (new_auto);
+ new_auto = cp_build_qualified_type (new_auto, TYPE_QUALS
(*auto_node));
+ *auto_node = new_auto;
+ }
}
return type;
}
diff --git a/gcc/testsuite/g++.dg/concepts/abbrev5.C
b/gcc/testsuite/g++.dg/concepts/abbrev5.C
new file mode 100644
index 000..de594b5c1df
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/abbrev5.C
@@ -0,0 +1,15 @@
+// PR c++/92187
+// { dg-do compile { target concepts } }
+
+template
+concept C = false;
+
+C auto f(auto)
+{
+ return 42; // { dg-error "deduced return type" }
+}
+
+void foo()
+{
+ f(0);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/abbrev6.C
b/gcc/testsuite/g++.dg/concepts/abbrev6.C
new file mode 100644
index 000..862675e5193
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/abbrev6.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target concepts } }
+
+const auto (auto)
+{
+ static int n;
+ return n;
+}
+
+void foo()
+{
+ f(5) = 0; // { dg-error "read-only" }
+}
[PATCH 1/1] PR94613: Fix vec_sel builtin for IBM Z
The vsel instruction is a bit-wise select instruction. Using an
IF_THEN_ELSE to express it in RTL is wrong and leads to wrong code being
generated in the combine pass.
With the patch the pattern is written using bit operations. However,
I've just noticed that the manual still demands a fixed point mode for
AND/IOR and friends although several targets emit bit ops on floating
point vectors (including i386, Power, and s390). So I assume this is a
safe thing to do?!
Bootstrapped and regression-tested on IBM z15.
gcc/ChangeLog:
2020-04-17 Andreas Krebbel
PR target/94613
* config/s390/s390-builtin-types.def: Add 3 new function modes.
* config/s390/s390-builtins.def: Add mode dependent low-level
builtin and map the overloaded builtins to these.
* config/s390/vx-builtins.md ("vec_selV_HW"): Rename to ...
("vsel
PR target/94613
* gcc.target/s390/zvector/pr94613.c: New test.
* gcc.target/s390/zvector/vec_sel-1.c: New test.
---
gcc/config/s390/s390-builtin-types.def| 3 +
gcc/config/s390/s390-builtins.def | 65 +++---
gcc/config/s390/vx-builtins.md| 27 ++-
.../gcc.target/s390/zvector/pr94613.c | 38
.../gcc.target/s390/zvector/vec_sel-1.c | 211 ++
5 files changed, 300 insertions(+), 44 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/s390/zvector/pr94613.c
create mode 100644 gcc/testsuite/gcc.target/s390/zvector/vec_sel-1.c
diff --git a/gcc/config/s390/s390-builtin-types.def
b/gcc/config/s390/s390-builtin-types.def
index 63b1c1ffd70..76ae8fed1ae 100644
--- a/gcc/config/s390/s390-builtin-types.def
+++ b/gcc/config/s390/s390-builtin-types.def
@@ -301,6 +301,7 @@ DEF_FN_TYPE_3 (BT_FN_UV16QI_UV2DI_UV2DI_UV16QI, BT_UV16QI,
BT_UV2DI, BT_UV2DI, B
DEF_FN_TYPE_3 (BT_FN_UV16QI_UV8HI_UV8HI_INTPTR, BT_UV16QI, BT_UV8HI, BT_UV8HI,
BT_INTPTR)
DEF_FN_TYPE_3 (BT_FN_UV2DI_UV2DI_ULONGLONG_INT, BT_UV2DI, BT_UV2DI,
BT_ULONGLONG, BT_INT)
DEF_FN_TYPE_3 (BT_FN_UV2DI_UV2DI_UV2DI_INT, BT_UV2DI, BT_UV2DI, BT_UV2DI,
BT_INT)
+DEF_FN_TYPE_3 (BT_FN_UV2DI_UV2DI_UV2DI_UV2DI, BT_UV2DI, BT_UV2DI, BT_UV2DI,
BT_UV2DI)
DEF_FN_TYPE_3 (BT_FN_UV2DI_UV4SI_UV4SI_UV2DI, BT_UV2DI, BT_UV4SI, BT_UV4SI,
BT_UV2DI)
DEF_FN_TYPE_3 (BT_FN_UV4SI_UV2DI_UV2DI_INTPTR, BT_UV4SI, BT_UV2DI, BT_UV2DI,
BT_INTPTR)
DEF_FN_TYPE_3 (BT_FN_UV4SI_UV4SI_UINT_INT, BT_UV4SI, BT_UV4SI, BT_UINT, BT_INT)
@@ -322,6 +323,7 @@ DEF_FN_TYPE_3 (BT_FN_V2DF_V2DF_DBL_INT, BT_V2DF, BT_V2DF,
BT_DBL, BT_INT)
DEF_FN_TYPE_3 (BT_FN_V2DF_V2DF_UCHAR_UCHAR, BT_V2DF, BT_V2DF, BT_UCHAR,
BT_UCHAR)
DEF_FN_TYPE_3 (BT_FN_V2DF_V2DF_UINT_UINT, BT_V2DF, BT_V2DF, BT_UINT, BT_UINT)
DEF_FN_TYPE_3 (BT_FN_V2DF_V2DF_V2DF_INT, BT_V2DF, BT_V2DF, BT_V2DF, BT_INT)
+DEF_FN_TYPE_3 (BT_FN_V2DF_V2DF_V2DF_UV2DI, BT_V2DF, BT_V2DF, BT_V2DF, BT_UV2DI)
DEF_FN_TYPE_3 (BT_FN_V2DF_V2DF_V2DF_V2DF, BT_V2DF, BT_V2DF, BT_V2DF, BT_V2DF)
DEF_FN_TYPE_3 (BT_FN_V2DI_UV2DI_UV2DI_INTPTR, BT_V2DI, BT_UV2DI, BT_UV2DI,
BT_INTPTR)
DEF_FN_TYPE_3 (BT_FN_V2DI_V2DF_INT_INTPTR, BT_V2DI, BT_V2DF, BT_INT, BT_INTPTR)
@@ -332,6 +334,7 @@ DEF_FN_TYPE_3 (BT_FN_V4SF_V2DF_INT_INT, BT_V4SF, BT_V2DF,
BT_INT, BT_INT)
DEF_FN_TYPE_3 (BT_FN_V4SF_V4SF_FLT_INT, BT_V4SF, BT_V4SF, BT_FLT, BT_INT)
DEF_FN_TYPE_3 (BT_FN_V4SF_V4SF_UCHAR_UCHAR, BT_V4SF, BT_V4SF, BT_UCHAR,
BT_UCHAR)
DEF_FN_TYPE_3 (BT_FN_V4SF_V4SF_V4SF_INT, BT_V4SF, BT_V4SF, BT_V4SF, BT_INT)
+DEF_FN_TYPE_3 (BT_FN_V4SF_V4SF_V4SF_UV4SI, BT_V4SF, BT_V4SF, BT_V4SF, BT_UV4SI)
DEF_FN_TYPE_3 (BT_FN_V4SF_V4SF_V4SF_V4SF, BT_V4SF, BT_V4SF, BT_V4SF, BT_V4SF)
DEF_FN_TYPE_3 (BT_FN_V4SI_UV4SI_UV4SI_INTPTR, BT_V4SI, BT_UV4SI, BT_UV4SI,
BT_INTPTR)
DEF_FN_TYPE_3 (BT_FN_V4SI_V2DI_V2DI_INTPTR, BT_V4SI, BT_V2DI, BT_V2DI,
BT_INTPTR)
diff --git a/gcc/config/s390/s390-builtins.def
b/gcc/config/s390/s390-builtins.def
index d05570cdeba..c69573df695 100644
--- a/gcc/config/s390/s390-builtins.def
+++ b/gcc/config/s390/s390-builtins.def
@@ -687,36 +687,41 @@ B_DEF (s390_vsceg,
vec_scatter_elementv2di,0,
/* First two operands are swapped in s390-c.c */
OB_DEF (s390_vec_sel, s390_vec_sel_b8_a,
s390_vec_sel_dbl_b, B_VX, BT_FN_OV4SI_OV4SI_OV4SI_OV4SI)
-OB_DEF_VAR (s390_vec_sel_b8_a, s390_vsel, 0,
0, BT_OV_BV16QI_BV16QI_BV16QI_UV16QI)
-OB_DEF_VAR (s390_vec_sel_b8_b, s390_vsel, 0,
0, BT_OV_BV16QI_BV16QI_BV16QI_BV16QI)
-OB_DEF_VAR (s390_vec_sel_s8_a, s390_vsel, 0,
0, BT_OV_V16QI_V16QI_V16QI_UV16QI)
-OB_DEF_VAR (s390_vec_sel_s8_b, s390_vsel, 0,
0, BT_OV_V16QI_V16QI_V16QI_BV16QI)
-OB_DEF_VAR (s390_vec_sel_u8_a, s390_vsel, 0,
0, BT_OV_UV16QI_UV16QI_UV16QI_UV16QI)
-OB_DEF_VAR (s390_vec_sel_u8_b, s390_vsel, 0,
Re: introduce target tmpnam and require it in tests relying on it
On Apr 9, 2020, Alexandre Oliva wrote: > Some target C libraries that aren't recognized as freestanding don't > have filesystem support, so calling tmpnam, fopen/open and > remove/unlink fails to link. > This patch introduces a tmpnam effective target to the testsuite, and > requires it in the tests that call tmpnam. > Tested on x86_64-linux-gnu, and with a cross to arm-eabi. > Ok to install? > for gcc/testsuite/ChangeLog > * lib/target-supports.exp (check_effective_target_tmpnam): New. > * gcc.c-torture/execute/fprintf-2.c: Require it. > * gcc.c-torture/execute/printf-2.c: Likewise. > * gcc.c-torture/execute/user-printf.c: Likewise. Ping? https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543672.html -- Alexandre Oliva, freedom fighterhe/himhttps://FSFLA.org/blogs/lxo/ Free Software Evangelist Stallman was right, but he's left :( GNU Toolchain Engineer Live long and free, and prosper ethically
Re: [PATCH] aarch64: Fix .cfi_window_save with pac-ret [PR94515]
The 04/17/2020 12:50, Jason Merrill wrote:
> On 4/17/20 6:08 AM, Kyrylo Tkachov wrote:
> > Hi Szabolcs,
> >
> > > -Original Message-
> > > From: Szabolcs Nagy
> > > Sent: 09 April 2020 15:20
> > > To: gcc-patches@gcc.gnu.org
> > > Cc: Richard Earnshaw ; Richard Sandiford
> > > ; Kyrylo Tkachov
> > > Subject: [PATCH] aarch64: Fix .cfi_window_save with pac-ret [PR94515]
> > >
> > > On aarch64 -mbranch-protection=pac-ret reuses the dwarf
> > > opcode for window_save to mean "toggle the return address
> > > mangle state", but in the dwarf2cfi internal logic the
> > > state was not properly recorded so the remember/restore
> > > logic was broken.
> > >
> > > This can cause the unwinder not to authenticate return
> > > addresses that were signed (or vice versa) which means
> > > a runtime crash on a pauth enabled system.
> > >
> > > Currently only aarch64 pac-ret uses REG_CFA_TOGGLE_RA_MANGLE.
> >
> > I think this is ok, but this code is in the midend so I've CC'ed Jason who,
> > from what I gather from MAINTAINERS, is maintainer for this file.
> > Thanks,
> > Kyrill
> >
> > >
> > > gcc/ChangeLog:
> > >
> > > 2020-04-XX Szabolcs Nagy
> > >
> > > PR target/94515
> > > * dwarf2cfi.c (dwarf2out_frame_debug): Record RA
> > > mangle state in cur_row->window_save.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > 2020-04-XX Szabolcs Nagy
> > >
> > > PR target/94515
> > > * g++.target/aarch64/pr94515.C: New test.
> > > ---
> > > gcc/dwarf2cfi.c| 3 ++
> > > gcc/testsuite/g++.target/aarch64/pr94515.C | 43 ++
> > > 2 files changed, 46 insertions(+)
> > > create mode 100644 gcc/testsuite/g++.target/aarch64/pr94515.C
> > >
> > > diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
> > > index 229fbfacc30..22989a6c2fb 100644
> > > --- a/gcc/dwarf2cfi.c
> > > +++ b/gcc/dwarf2cfi.c
> > > @@ -2145,6 +2145,9 @@ dwarf2out_frame_debug (rtx_insn *insn)
> > > case REG_CFA_TOGGLE_RA_MANGLE:
> > > /* This uses the same DWARF opcode as the next operation. */
> > > dwarf2out_frame_debug_cfa_window_save (true);
> > > + /* Keep track of RA mangle state by toggling the window_save bit.
> > > +This is needed so .cfi_window_save is emitted correctly. */
> > > + cur_row->window_save = !cur_row->window_save;
>
> It looks like passing 'true' to dwarf2out_frame_debug_cfa_window_save
> prevents that function from messing with the window_safe flag. Does
> changing the argument to 'false' get the behavior you want?
we want the state = !state toggling.
it might make more sense to do that in
dwarf2out_frame_debug_cfa_window_save(true)
or to inline that entire logic into the two
places where it is used (instead of
dispatching with a bool argument).
for the bug fix i'd like a minimal change
(so it can be backported), doing the fix in
dwarf2out_frame_debug_cfa_window_save
is fine with me, would you prefer that?
>
> > > handled_one = true;
> > > break;
> > >
> > > diff --git a/gcc/testsuite/g++.target/aarch64/pr94515.C
> > > b/gcc/testsuite/g++.target/aarch64/pr94515.C
> > > new file mode 100644
> > > index 000..7707c773a72
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.target/aarch64/pr94515.C
> > > @@ -0,0 +1,43 @@
> > > +/* PR target/94515. Check .cfi_window_save with multiple return paths.
> > > */
> > > +/* { dg-do run } */
> > > +/* { dg-additional-options "-O2 -mbranch-protection=pac-ret
> > > --save-temps" }
> > > */
> > > +
> > > +volatile int zero = 0;
> > > +
> > > +__attribute__((noinline, target("branch-protection=none")))
> > > +void unwind (void)
> > > +{
> > > + if (zero == 0)
> > > +throw 42;
> > > +}
> > > +
> > > +__attribute__((noinline, noipa, target("branch-protection=pac-ret")))
> > > +int test (int z)
> > > +{
> > > + if (z) {
> > > +asm volatile ("":::"x20","x21");
> > > +unwind ();
> > > +return 1;
> > > + } else {
> > > +unwind ();
> > > +return 2;
> > > + }
> > > +}
> > > +
> > > +__attribute__((target("branch-protection=none")))
> > > +int main ()
> > > +{
> > > + try {
> > > +test (zero);
> > > +__builtin_abort ();
> > > + } catch (...) {
> > > +return 0;
> > > + }
> > > + __builtin_abort ();
> > > +}
> > > +
> > > +/* This check only works if there are two return paths in test and
> > > + cfi_window_save is used for both instead of cfi_remember_state
> > > + plus cfi_restore_state. This is currently the case with -O2. */
> > > +
> > > +/* { dg-final { scan-assembler-times {\t\.cfi_window_save\n} 4 } } */
> > > --
> > > 2.17.1
> >
>
--
[PATCH] c++: spec_hasher::equal and PARM_DECLs [PR94632]
In the testcase below, during specialization of c::d, we build two
identical specializations of the parameter type b -- one when
substituting into c::d's TYPE_ARG_TYPES and another when substituting into
c::d's DECL_ARGUMENTS.
We don't reuse the first specialization the second time around as a consequence
of the fix for PR c++/56247 which made PARM_DECLs always compare different from
one another during spec_hasher::equal. As a result, when looking up existing
specializations of 'b', spec_hasher::equal considers the template argument
decltype(e')::k to be different from decltype(e'')::k, where e' and e'' are the
result of two calls to tsubst_copy on the PARM_DECL e.
Since the two specializations are considered different due to the mentioned fix,
their TYPE_CANONICAL points to themselves even though they are otherwise
identical types, and this triggers an ICE in maybe_rebuild_function_decl_type
when comparing the TYPE_ARG_TYPES of c::d to its DECL_ARGUMENTS.
This patch fixes this issue at the spec_hasher::equal level by ignoring the
'comparing_specializations' flag in cp_tree_equal whenever the DECL_CONTEXTs of
the two parameters are identical. This seems to be a sufficient condition to be
able to correctly compare PARM_DECLs structurally. (This also subsumes the
CONSTRAINT_VAR_P check since constraint variables all have empty, and therefore
identical, DECL_CONTEXTs.)
I can think of two other ways to avoid this ICE. One solution would be to do
maybe_rebuild_function_decl_type only when we have instantiated the function,
and not when we are just specializing it during instantiation of an enclosing
class template. This would avoid the problematic same_type_p call altogether.
Another solution would be to set up a local_specialization_stack in
instantiate_class_template_1 so that we can reuse PARM_DECL specializations
during tsubst_copy. As a consequence, e' and e'' mentioned above would be
identical. Do one of these three solutions seem right?
This patch passes 'make check-c++', and a full bootstrap/regtest is in progress.
gcc/cp/ChangeLog:
PR c++/94632
* tree.c (cp_tree_equal) : Ignore
comparing_specializations if the parameters' contexts are identical.
gcc/testsuite/ChangeLog:
PR c++/94632
* g++.dg/template/canon-type-14.C: New test.
---
gcc/cp/tree.c | 5 +++--
gcc/testsuite/g++.dg/template/canon-type-14.C | 8
2 files changed, 11 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/canon-type-14.C
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 8e4934c0009..dc4f1f48d3c 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -3723,11 +3723,12 @@ cp_tree_equal (tree t1, tree t2)
up for expressions that involve 'this' in a member function
template. */
- if (comparing_specializations && !CONSTRAINT_VAR_P (t1))
+ if (comparing_specializations
+ && DECL_CONTEXT (t1) != DECL_CONTEXT (t2))
/* When comparing hash table entries, only an exact match is
good enough; we don't want to replace 'this' with the
version from another function. But be more flexible
- with local parameters in a requires-expression. */
+ with parameters with identical contexts. */
return false;
if (same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
diff --git a/gcc/testsuite/g++.dg/template/canon-type-14.C
b/gcc/testsuite/g++.dg/template/canon-type-14.C
new file mode 100644
index 000..fa05bdb9a74
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/canon-type-14.C
@@ -0,0 +1,8 @@
+// PR c++/94632
+// { dg-do compile { target c++11 } }
+
+template struct b;
+template class c {
+ template static void d(f e, b);
+};
+template class c;
--
2.26.1.107.gefe3874640
[PATCH][x86][1/3]: Add -mzero-caller-saved-regs=[skip|used-gpr|all-gpr|used|all]
Hi,
This is a PING for an old patch proposed by H. J. Lu on Oct, 2018:
https://gcc.gnu.org/legacy-ml/gcc-patches/2018-10/msg02079.html
This is the first patch of the total 3 patches set, which provides the
following new feature:
-mzero-caller-saved-regs=[skip|used-gpr|all-gpr|used|all] command-line
option and zero_caller_saved_regs("skip|used|all") function attribue:
1. -mzero-caller-saved-regs=skip and zero_caller_saved_regs("skip")
Don't zero caller-saved registers upon function return.
2. -mzero-caller-saved-regs=used-gpr and zero_caller_saved_regs("used-gpr")
Zero used caller-saved integer registers upon function return.
3. -mzero-caller-saved-regs=all-gpr and zero_caller_saved_regs("all-gpr”)
Zero all caller-saved integer registers upon function return.
4. -mzero-caller-saved-regs=used and zero_caller_saved_regs("used")
Zero used caller-saved integer and vector registers upon function return.
5. -mzero-caller-saved-regs=all and zero_caller_saved_regs("all")
Zero all caller-saved integer and vector registers upon function return.
This feature is needed by Linux kernel security improvement. Please refer to
Kees Cook’s talk on Linux Plumber Conference 2019:
https://outflux.net/slides/2019/lpc/gcc-and-clang.pdf
Tested on x86-64 with bootstrapping GCC trunk, regression tests exposed
several new regressions, these new regressions are
fixed by 2 following patches I will send in next two emails.
Any comment?
thanks.
Qing
[PATCH][x86][2/3]: Add -mzero-caller-saved-regs=[skip|used-gpr|all-gpr|used|all]
Hi, This is the 2nd patch of the total 3 patches set for providing the new feature -mzero-caller-saved-regs for linux kernel security improvement. This patch is for resolving the new regressions triggered by the first patch. This patch is to Add ix86_any_return_p to check simple_return in a PARALLEL to support: (jump_insn 39 38 40 5 (parallel [ (simple_return) (unspec [ (const_int 0 [0]) ] UNSPEC_SIMPLE_RETURN) ]) "/tmp/x.c":105 -1 (nil) -> simple_return) Any comments? Qing 0002-x86-Add-ix86_any_return_p.patch Description: Binary data
[PATCH][x86][3/3]: Add -mzero-caller-saved-regs=[skip|used-gpr|all-gpr|used|all]
Hi, This is the 3rd patch of the total 3 patches set for providing the new feature -mzero-caller-saved-regs for linux kernel security improvement. This patch is to Update gcc.target/i386/ret-thunk-2[234].c Qing 0003-Update-gcc.target-i386-ret-thunk-2-234-.c.patch Description: Binary data
Re: [PATCH][libstd++][PR92156]
Fixes all this.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92156
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91630
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415
On Fri, Apr 17, 2020 at 10:45 PM kamlesh kumar wrote:
>
> This patch corrects the requirement of 4,5 and 6th constructor
> As per https://en.cppreference.com/w/cpp/utility/any/any.
>
> ChangeLog:
> 2020-04-17 Kamlesh Kumar
>
> PR libstdc++/92156
> * include/std/any (ans::any(_ValueType &&):: Remove is_constructible.
> (any::any(in_place_type_t<_ValueType>, _Args&&...)): Use decay_t.
> (any::any(in_place_type_t<_ValueType>,initializer_list<_Up>,
> _Args&&...)):
> Use decay_t.
> * testsuite/20_util/any/misc/92156.cc: New Test.
>
> diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
> index 6b7e68f0e63..fb212eb2231 100644
> --- a/libstdc++-v3/include/std/any
> +++ b/libstdc++-v3/include/std/any
> @@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> /// Construct with a copy of @p __value as the contained object.
> template ,
>typename _Mgr = _Manager<_Tp>,
> - __any_constructible_t<_Tp, _ValueType&&> = true,
> - enable_if_t::value, bool> = true>
> + enable_if_t::value &&
> + !__is_in_place_type<_Tp>::value, bool> = true>
>any(_ValueType&& __value)
>: _M_manager(&_Mgr::_S_manage)
>{
> _Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value));
>}
>
> -/// Construct with a copy of @p __value as the contained object.
> -template ,
> - typename _Mgr = _Manager<_Tp>,
> - enable_if_t<__and_v,
> - __not_>,
> - __not_<__is_in_place_type<_Tp>>>,
> - bool> = false>
> - any(_ValueType&& __value)
> - : _M_manager(&_Mgr::_S_manage)
> - {
> -_Mgr::_S_create(_M_storage, __value);
> - }
> -
> /// Construct with an object created from @p __args as the contained
> object.
> template - typename _Tp = _Decay<_ValueType>,
> + typename _Tp = decay_t<_ValueType>,
>typename _Mgr = _Manager<_Tp>,
>__any_constructible_t<_Tp, _Args&&...> = false>
>explicit
> @@ -214,7 +201,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> /// Construct with an object created from @p __il and @p __args as
> /// the contained object.
> template - typename _Tp = _Decay<_ValueType>,
> + typename _Tp = decay_t<_ValueType>,
>typename _Mgr = _Manager<_Tp>,
>__any_constructible_t<_Tp, initializer_list<_Up>,
> _Args&&...> = false>
> diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc
> b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc
> new file mode 100644
> index 000..c4f1ed55aee
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc
> @@ -0,0 +1,34 @@
> +// { dg-options "-std=gnu++17" }
> +// { dg-do compile }
> +
> +// Copyright (C) 2014-2020 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
> +//
[PATCH][libstd++][PR92156]
This patch corrects the requirement of 4,5 and 6th constructor
As per https://en.cppreference.com/w/cpp/utility/any/any.
ChangeLog:
2020-04-17 Kamlesh Kumar
PR libstdc++/92156
* include/std/any (ans::any(_ValueType &&):: Remove
is_constructible.
(any::any(in_place_type_t<_ValueType>, _Args&&...)): Use decay_t.
(any::any(in_place_type_t<_ValueType>,initializer_list<_Up>,
_Args&&...)):
Use decay_t.
* testsuite/20_util/any/misc/92156.cc: New Test.
diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
index 6b7e68f0e63..fb212eb2231 100644
--- a/libstdc++-v3/include/std/any
+++ b/libstdc++-v3/include/std/any
@@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// Construct with a copy of @p __value as the contained object.
template ,
typename _Mgr = _Manager<_Tp>,
- __any_constructible_t<_Tp, _ValueType&&> = true,
- enable_if_t::value, bool> = true>
+ enable_if_t::value &&
+ !__is_in_place_type<_Tp>::value, bool> = true>
any(_ValueType&& __value)
: _M_manager(&_Mgr::_S_manage)
{
_Mgr::_S_create(_M_storage, std::forward<_ValueType>(__value));
}
-/// Construct with a copy of @p __value as the contained object.
-template ,
- typename _Mgr = _Manager<_Tp>,
- enable_if_t<__and_v,
- __not_>,
- __not_<__is_in_place_type<_Tp>>>,
- bool> = false>
- any(_ValueType&& __value)
- : _M_manager(&_Mgr::_S_manage)
- {
-_Mgr::_S_create(_M_storage, __value);
- }
-
/// Construct with an object created from @p __args as the contained
object.
template ,
+ typename _Tp = decay_t<_ValueType>,
typename _Mgr = _Manager<_Tp>,
__any_constructible_t<_Tp, _Args&&...> = false>
explicit
@@ -214,7 +201,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// Construct with an object created from @p __il and @p __args as
/// the contained object.
template ,
+ typename _Tp = decay_t<_ValueType>,
typename _Mgr = _Manager<_Tp>,
__any_constructible_t<_Tp, initializer_list<_Up>,
_Args&&...> = false>
diff --git a/libstdc++-v3/testsuite/20_util/any/misc/92156.cc
b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc
new file mode 100644
index 000..c4f1ed55aee
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/any/misc/92156.cc
@@ -0,0 +1,34 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile }
+
+// Copyright (C) 2014-2020 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
+//
Re: [PATCH 0/19][GCC-8] aarch64: Backport outline atomics
Hi Andre, the patch series passed bootstrap and check with no new fails on Graviton2 aarch64-linux. Thanks, Sebastian On 4/16/20, 12:24 PM, "Pop, Sebastian" wrote: Thanks Andre for the back-port to gcc-8. Overall the patches look good to me. Could you please move the patch "[PATCH 13/19][GCC-8] Aarch64: Fix shrinkwrapping interactions with atomics (PR92692)" just after "[PATCH 8/19][GCC-8] aarch64: Implement TImode compare-and-swap" such that the change that breaks TSAN builds gets fixed right away instead of waiting to get the fix after 4 more patches? I would like to test the patches on Graviton2. Could you please send me the git format-patch version? It is hard to extract the patches from the mailing list without the patches getting scrambled. Thanks, Sebastian On 4/16/20, 7:24 AM, "Andre Vieira (lists)" wrote: Hi, This series backports all the patches and fixes regarding outline atomics to the gcc-8 branch. Bootstrapped the series for aarch64-linux-gnu and regression tested. Is this OK for gcc-8? Andre Vieira (19): aarch64: Add early clobber for aarch64_store_exclusive aarch64: Simplify LSE cas generation aarch64: Improve cas generation aarch64: Improve swp generation aarch64: Improve atomic-op lse generation aarch64: Remove early clobber from ATOMIC_LDOP scratch aarch64: Extend %R for integer registers aarch64: Implement TImode compare-and-swap aarch64: Tidy aarch64_split_compare_and_swap aarch64: Add out-of-line functions for LSE atomics Add visibility to libfunc constructors aarch64: Implement -moutline-atomics Aarch64: Fix shrinkwrapping interactions with atomics (PR92692) aarch64: Fix store-exclusive in load-operate LSE helpers aarch64: Configure for sys/auxv.h in libgcc for lse-init.c aarch64: Fix up aarch64_compare_and_swaphi pattern [PR94368] aarch64: Fix bootstrap with old binutils [PR93053] aarch64: Fix ICE due to aarch64_gen_compare_reg_maybe_ze [PR94435] re PR target/90724 (ICE with __sync_bool_compare_and_swap with -march=armv8.2-a+sve)
Re: [PATCH] c++: Hard error with tentative parse of declaration [PR88754]
On Fri, Apr 17, 2020 at 12:45:37PM -0400, Jason Merrill via Gcc-patches wrote: > On 4/16/20 7:33 PM, Patrick Palka wrote: > > In the testcase for this PR, we try to parse the statement > > > >A(value<0>()); > > > > first tentatively as a declaration (with a parenthesized declarator), and > > during > > this tentative parse we end up issuing a hard error from > > cp_parser_check_template_parameters about its invalidness as a declaration. > > > > Rather than issuing a hard error, it seems we should instead simulate an > > error > > since we're parsing tentatively. This would then allow cp_parser_statement > > to > > recover and successfully parse the statement as an expression-statement > > instead. > > > > Passes 'make check-c++', does this look OK to commit after > > bootstrap/regtesting? > > > > gcc/cp/ChangeLog: > > > > PR c++/88754 > > * parser. (cp_parser_check_template_parameters): Before issiung a hard s/\. /.c/;s/issiung/issuing/ > > error, first try simulating an error instead. Jakub
Re: [PATCH] aarch64: Fix .cfi_window_save with pac-ret [PR94515]
On 4/17/20 6:08 AM, Kyrylo Tkachov wrote:
Hi Szabolcs,
-Original Message-
From: Szabolcs Nagy
Sent: 09 April 2020 15:20
To: gcc-patches@gcc.gnu.org
Cc: Richard Earnshaw ; Richard Sandiford
; Kyrylo Tkachov
Subject: [PATCH] aarch64: Fix .cfi_window_save with pac-ret [PR94515]
On aarch64 -mbranch-protection=pac-ret reuses the dwarf
opcode for window_save to mean "toggle the return address
mangle state", but in the dwarf2cfi internal logic the
state was not properly recorded so the remember/restore
logic was broken.
This can cause the unwinder not to authenticate return
addresses that were signed (or vice versa) which means
a runtime crash on a pauth enabled system.
Currently only aarch64 pac-ret uses REG_CFA_TOGGLE_RA_MANGLE.
I think this is ok, but this code is in the midend so I've CC'ed Jason who,
from what I gather from MAINTAINERS, is maintainer for this file.
Thanks,
Kyrill
gcc/ChangeLog:
2020-04-XX Szabolcs Nagy
PR target/94515
* dwarf2cfi.c (dwarf2out_frame_debug): Record RA
mangle state in cur_row->window_save.
gcc/testsuite/ChangeLog:
2020-04-XX Szabolcs Nagy
PR target/94515
* g++.target/aarch64/pr94515.C: New test.
---
gcc/dwarf2cfi.c| 3 ++
gcc/testsuite/g++.target/aarch64/pr94515.C | 43 ++
2 files changed, 46 insertions(+)
create mode 100644 gcc/testsuite/g++.target/aarch64/pr94515.C
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index 229fbfacc30..22989a6c2fb 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -2145,6 +2145,9 @@ dwarf2out_frame_debug (rtx_insn *insn)
case REG_CFA_TOGGLE_RA_MANGLE:
/* This uses the same DWARF opcode as the next operation. */
dwarf2out_frame_debug_cfa_window_save (true);
+ /* Keep track of RA mangle state by toggling the window_save bit.
+ This is needed so .cfi_window_save is emitted correctly. */
+ cur_row->window_save = !cur_row->window_save;
It looks like passing 'true' to dwarf2out_frame_debug_cfa_window_save
prevents that function from messing with the window_safe flag. Does
changing the argument to 'false' get the behavior you want?
handled_one = true;
break;
diff --git a/gcc/testsuite/g++.target/aarch64/pr94515.C
b/gcc/testsuite/g++.target/aarch64/pr94515.C
new file mode 100644
index 000..7707c773a72
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/pr94515.C
@@ -0,0 +1,43 @@
+/* PR target/94515. Check .cfi_window_save with multiple return paths. */
+/* { dg-do run } */
+/* { dg-additional-options "-O2 -mbranch-protection=pac-ret --save-temps" }
*/
+
+volatile int zero = 0;
+
+__attribute__((noinline, target("branch-protection=none")))
+void unwind (void)
+{
+ if (zero == 0)
+throw 42;
+}
+
+__attribute__((noinline, noipa, target("branch-protection=pac-ret")))
+int test (int z)
+{
+ if (z) {
+asm volatile ("":::"x20","x21");
+unwind ();
+return 1;
+ } else {
+unwind ();
+return 2;
+ }
+}
+
+__attribute__((target("branch-protection=none")))
+int main ()
+{
+ try {
+test (zero);
+__builtin_abort ();
+ } catch (...) {
+return 0;
+ }
+ __builtin_abort ();
+}
+
+/* This check only works if there are two return paths in test and
+ cfi_window_save is used for both instead of cfi_remember_state
+ plus cfi_restore_state. This is currently the case with -O2. */
+
+/* { dg-final { scan-assembler-times {\t\.cfi_window_save\n} 4 } } */
--
2.17.1
Re: [PATCH] c++: Hard error with tentative parse of declaration [PR88754]
On 4/16/20 7:33 PM, Patrick Palka wrote:
In the testcase for this PR, we try to parse the statement
A(value<0>());
first tentatively as a declaration (with a parenthesized declarator), and during
this tentative parse we end up issuing a hard error from
cp_parser_check_template_parameters about its invalidness as a declaration.
Rather than issuing a hard error, it seems we should instead simulate an error
since we're parsing tentatively. This would then allow cp_parser_statement to
recover and successfully parse the statement as an expression-statement instead.
Passes 'make check-c++', does this look OK to commit after bootstrap/regtesting?
gcc/cp/ChangeLog:
PR c++/88754
* parser. (cp_parser_check_template_parameters): Before issiung a hard
error, first try simulating an error instead.
OK.
gcc/testsuite/ChangeLog:
PR c++/88754
* g++.dg/parse/ambig10.C: New test.
---
gcc/cp/parser.c | 4
gcc/testsuite/g++.dg/parse/ambig10.C | 20
2 files changed, 24 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/parse/ambig10.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index e037e7d8c8e..47e3f2bbd3d 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -28531,6 +28531,10 @@ cp_parser_check_template_parameters (cp_parser* parser,
if (!template_id_p
&& parser->num_template_parameter_lists == num_templates + 1)
return true;
+
+ if (cp_parser_simulate_error (parser))
+return false;
+
/* If there are more template classes than parameter lists, we have
something like:
diff --git a/gcc/testsuite/g++.dg/parse/ambig10.C b/gcc/testsuite/g++.dg/parse/ambig10.C
new file mode 100644
index 000..42b04b16923
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/ambig10.C
@@ -0,0 +1,20 @@
+// PR c++/88754
+// { dg-do compile }
+
+struct A
+{
+ A(int);
+ void foo();
+};
+
+template int value() { return N; }
+
+void bar()
+{
+ A(value<0>()).foo();
+ A(value<0>());
+ (A(value<0>())).foo();
+
+ A value<0>; // { dg-error "invalid declaration" }
+ A value<0>(); // { dg-error "invalid declaration" }
+}
Re: [PATCH] c++: Non-type-dependent variadic lambda init-capture [PR94483]
On 4/16/20 1:53 PM, Patrick Palka wrote:
In this PR (which I think is misclassified as ice-on-invalid instead of
ice-on-valid), we're ICEing on a use of an 'int... a' template parameter pack as
part of the variadic lambda init-capture [...z=a].
The unexpected thing about this variadic init-capture is that it is not
type-dependent, and so when we call do_auto_deduction from
lambda_capture_field_type it actually resolves its type to 'int' instead of
exiting early like it would do for a type-dependent variadic initializer. This
later confuses add_capture which, according to one of its comments, assumes that
'type' is always 'auto' for a variadic init-capture.
The simplest fix, and the approach that this patch takes, seems to be to avoid
doing auto deduction in lambda_capture_field_type when the initializer uses
parameter packs, so that we always return 'auto' even in the non-type-dependent
case.
Hmm, that will make 'z' more dependent than necessary, but I suppose its
packness will tend to make uses dependent anyway, so OK.
Passes 'make check-c++', does this look OK to commit after full
bootstrap/regtesting?
gcc/cp/ChangeLog:
PR c++/94483
* lambda.c (lambda_capture_field_type): Avoid doing auto deduction if
the explicit initializer has parameter packs.
gcc/testsuite/ChangeLog:
PR c++/94483
* g++.dg/cpp2a/lambda-pack-init5.C: New test.
---
gcc/cp/lambda.c| 5 -
gcc/testsuite/g++.dg/cpp2a/lambda-pack-init5.C | 18 ++
2 files changed, 22 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-pack-init5.C
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index 4f39f99756b..b55c2f85d27 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -223,7 +223,10 @@ lambda_capture_field_type (tree expr, bool explicit_init_p,
/* Add the reference now, so deduction doesn't lose
outermost CV qualifiers of EXPR. */
type = build_reference_type (type);
- type = do_auto_deduction (type, expr, auto_node);
+ if (uses_parameter_packs (expr))
+ /* Stick with 'auto' even if the type could be deduced. */;
+ else
+ type = do_auto_deduction (type, expr, auto_node);
}
else if (!is_this && type_dependent_expression_p (expr))
{
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-pack-init5.C
b/gcc/testsuite/g++.dg/cpp2a/lambda-pack-init5.C
new file mode 100644
index 000..492fc479e94
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-pack-init5.C
@@ -0,0 +1,18 @@
+// PR c++/94483
+// { dg-do compile { target c++2a } }
+
+template constexpr auto x1
+ = [...z = -a] (auto F) { return F(z...); };
+
+template constexpr auto x2
+ = [&...z = a] (auto F) { return F(z...); };
+
+template constexpr auto x3
+ = [z = -a] (auto F) { return F(z); }; // { dg-error "packs not expanded" }
+
+
+constexpr auto sum = [] (auto... xs) { return (xs + ... + 0); };
+const int y1 = 1, y2 = 2, y3 = 3;
+
+static_assert(x1<1,2,3>(sum) == -6);
+static_assert(x2(sum) == 6);
Re: [Patch][OpenMP] Fix 'omp exit data' for Fortran arrays (PR 94635)
On Fri, Apr 17, 2020 at 05:57:06PM +0200, Tobias Burnus wrote: > --- a/gcc/gimplify.c > +++ b/gcc/gimplify.c > @@ -8785,11 +8785,15 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq > *pre_p, >'exit data' - and in particular for 'delete:' - having an 'alloc:' >does not make sense. Likewise, for 'update' only transferring the >data itself is needed as the rest has been handled in previous > - directives. */ > - if ((code == OMP_TARGET_EXIT_DATA || code == OMP_TARGET_UPDATE) > - && (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER > - || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET)) > - remove = true; > + directives. However, for 'exit data', the array descriptor needs > + to be delete; hence, we turn the MAP_TO_PSET into a MAP_DELETE. */ > + if (code == OMP_TARGET_EXIT_DATA > + && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET) > + OMP_CLAUSE_SET_MAP_KIND (c, GOMP_MAP_DELETE); > + else if ((code == OMP_TARGET_EXIT_DATA || code == OMP_TARGET_UPDATE) > +&& (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER > +|| OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET)) > + remove = true; Wrong indentation of the last line, should be 1 tab + 4 spaces instead of 2 tabs. Otherwise LGTM, thanks. Jakub
Re: [PATCH] Fold (add -1; zero_ext; add +1) operations to zero_ext when not zero (PR37451, PR61837)
On Thu, Apr 16, 2020 at 08:21:40PM -0500, Segher Boessenkool wrote:
> On Wed, Apr 15, 2020 at 10:18:16AM +0100, Richard Sandiford wrote:
> > luoxhu--- via Gcc-patches writes:
> > > -count = simplify_gen_binary (PLUS, mode, count, const1_rtx);
> > > +{
> > > + /* Fold (add -1; zero_ext; add +1) operations to zero_ext based on
> > > addop0
> > > + is never zero, as gimple pass loop ch will do optimization to simplify
> > > + the loop to NO loop for loop condition is false. */
> >
> > IMO the code needs to prove this, rather than just assume that previous
> > passes have made it so.
>
> Well, it should gcc_assert it, probably.
>
> It is the left-hand side of a+b... it cannot be 0, because niter always
> is simplified!
Scratch that... it cannot be *constant* 0, but that isn't the issue here.
Segher
Re: [Patch][OpenMP] Fix 'omp exit data' for Fortran arrays (PR 94635)
Next try – with the proper patch instead of a full test case.
On 4/17/20 5:54 PM, Tobias Burnus wrote:
It turned out that doing
omp enter data map(alloc:FortranArray)
omp exit data map(delete:FortranArray)
left the array descriptor (fortranarray [as opposed to
fortranarray.data])
on the device. (cf. -fdump-tree-omplower in the PR.)
Mapping FortranArray again (e.g. "map(tofrom:FortranArray)")
then failed by returning garbage.
This patch now removes the descriptor with 'data exit',
which was passed as MAP_TO_PSET clause to the middle end,
but got removed. Instead, the clause is now turned into MAP_DELETE.
Build on x86-64-gnu-linux and tested without and with AMDGCN
as offloading device. OK for the trunk?
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
[OpenMP] Fix 'omp exit data' for Fortran arrays (PR 94635)
PR middle-end/94635
* gimplify.c (gimplify_scan_omp_clauses): Turn MAP_TO_PSET to
MAP_DELETE.
PR middle-end/94635
* testsuite/libgomp.fortran/target-enter-data-2.F90: New.
gcc/gimplify.c | 14 +---
.../libgomp.fortran/target-enter-data-2.F90| 40 ++
2 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 8cdfae26510..6fd8196f01c 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -8785,11 +8785,15 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
'exit data' - and in particular for 'delete:' - having an 'alloc:'
does not make sense. Likewise, for 'update' only transferring the
data itself is needed as the rest has been handled in previous
- directives. */
- if ((code == OMP_TARGET_EXIT_DATA || code == OMP_TARGET_UPDATE)
- && (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER
- || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET))
- remove = true;
+ directives. However, for 'exit data', the array descriptor needs
+ to be delete; hence, we turn the MAP_TO_PSET into a MAP_DELETE. */
+ if (code == OMP_TARGET_EXIT_DATA
+ && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET)
+ OMP_CLAUSE_SET_MAP_KIND (c, GOMP_MAP_DELETE);
+ else if ((code == OMP_TARGET_EXIT_DATA || code == OMP_TARGET_UPDATE)
+ && (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER
+ || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET))
+ remove = true;
if (remove)
break;
diff --git a/libgomp/testsuite/libgomp.fortran/target-enter-data-2.F90 b/libgomp/testsuite/libgomp.fortran/target-enter-data-2.F90
new file mode 100644
index 000..320d8bf419f
--- /dev/null
+++ b/libgomp/testsuite/libgomp.fortran/target-enter-data-2.F90
@@ -0,0 +1,40 @@
+! { dg-additional-options "-DMEM_SHARED" { target offload_device_shared_as } }
+!
+! PR middle-end/94635
+ implicit none
+ integer, parameter :: N = 20
+ integer, allocatable, dimension(:) :: my1DPtr
+ integer, dimension(N) :: my1DArr
+ integer :: i
+
+ allocate(my1DPtr(N))
+ my1DPtr = 43
+
+ !$omp target enter data map(alloc: my1DPtr)
+!$omp target
+ my1DPtr = [(i , i = 1, N)]
+!$omp end target
+
+!$omp target map(from: my1DArr)
+ my1DArr = my1DPtr
+!$omp end target
+ !$omp target exit data map(delete: my1DPtr)
+
+ if (any (my1DArr /= [(i, i = 1, N)])) stop 1
+#if MEM_SHARED
+ if (any (my1DArr /= my1DPtr)) stop 2
+#else
+ if (any (43 /= my1DPtr)) stop 3
+#endif
+
+ my1DPtr = [(2*N-i, i = 1, N)]
+ my1DArr = 42
+
+ !$omp target map(tofrom: my1DArr) map(tofrom: my1DPtr(:))
+my1DArr = my1DPtr
+my1DPtr = 20
+ !$omp end target
+
+ if (any (my1DArr /= [(2*N-i, i = 1, N)])) stop 4
+ if (any (20 /= my1DPtr)) stop 6
+end
[Patch][OpenMP] Fix 'omp exit data' for Fortran arrays (PR 94635)
It turned out that doing
omp enter data map(alloc:FortranArray)
omp exit data map(delete:FortranArray)
left the array descriptor (fortranarray [as opposed to fortranarray.data])
on the device. (cf. -fdump-tree-omplower in the PR.)
Mapping FortranArray again (e.g. "map(tofrom:FortranArray)")
then failed by returning garbage.
This patch now removes the descriptor with 'data exit',
which was passed as MAP_TO_PSET clause to the middle end,
but got removed. Instead, the clause is now turned into MAP_DELETE.
Build on x86-64-gnu-linux and tested without and with AMDGCN
as offloading device. OK for the trunk?
Tobias
-
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
!===--test_target_enter_exit_data_allocate_array_alloc_delete.F90 - alloc/delete--===!
!
! OpenMP API Version 4.5 Nov 2015
!
! This tests covers the target enter/exit data with the
! map(alloc/delete) modifiers respectively, for arrays that have the
! allocatable modifier and that are dynamically generated.
!
!!===--===!
#include "ompvv.F90"
#define N 20
PROGRAM tests_target_enter_exit_data_allocate_array_alloc
USE iso_fortran_env
USE ompvv_lib
USE omp_lib
implicit none
INTEGER, ALLOCATABLE, DIMENSION(:) :: my1DPtr
INTEGER, ALLOCATABLE, DIMENSION(:,:) :: my2DPtr
INTEGER, ALLOCATABLE, DIMENSION(:,:,:) :: my3DPtr
! Helper arrays
INTEGER, DIMENSION(N) :: my1DArr
INTEGER, DIMENSION(N,N) :: my2DArr
INTEGER, DIMENSION(N,N,N) :: my3DArr
! Helper functions
LOGICAL :: isSharedEnv
CHARACTER (len = 400) :: helperMsg
INTEGER :: errors, i
OMPVV_TEST_OFFLOADING
OMPVV_TEST_AND_SET_SHARED_ENVIRONMENT(isSharedEnv)
WRITE(helperMsg, *) "Omitting part of the test due to &
data environment"
OMPVV_WARNING_IF(isSharedEnv, helperMsg)
OMPVV_TEST_VERBOSE(test_allocate_array1D_map_alloc() .ne. 0)
OMPVV_TEST_VERBOSE(test_allocate_array2D_map_alloc() .ne. 0)
OMPVV_TEST_VERBOSE(test_allocate_array3D_map_alloc() .ne. 0)
OMPVV_REPORT_AND_RETURN()
CONTAINS
! 1D Array test
INTEGER FUNCTION test_allocate_array1D_map_alloc()
OMPVV_INFOMSG("Testing map alloc/delete allocated 1D array")
errors = 0
! Allocate the arrays
allocate(my1DPtr(N))
! initialize
my1DPtr(:) = 0
! Mapping the array
!$omp target enter data map(alloc: my1DPtr(:))
! Assign a value to the allocated space
!$omp target
my1DPtr(:) = (/ (i , i = 1,N) /)
!$omp end target
! Confirm mapping with target region
!$omp target map(from: my1DArr)
my1DArr = my1DPtr
!$omp end target
IF (.NOT. isSharedEnv) THEN
! Make sure data does not get transfered over the host
OMPVV_TEST_AND_SET_VERBOSE(errors, ANY(my1DPtr /= 0))
END IF
OMPVV_TEST_AND_SET_VERBOSE(errors, SUM(my1DArr) /= ((N*(N+1)/2)))
! Asign a host value
my1DPtr(:) = 10
!$omp target exit data map(delete: my1DPtr(:))
! Check if I can transfer data with a map(tofrom:)
!$omp target map(from: my1DArr) map(tofrom: my1DPtr(:))
my1DArr = my1DPtr
my1DPtr(:) = 20
!$omp end target
! test that the values are the expected ones
OMPVV_TEST_AND_SET_VERBOSE(errors, ANY(my1DPtr /= 20))
OMPVV_TEST_AND_SET_VERBOSE(errors, ANY(my1DArr /= 10))
deallocate(my1DPtr)
test_allocate_array1D_map_alloc = errors
END FUNCTION test_allocate_array1D_map_alloc
! 2D Array test
INTEGER FUNCTION test_allocate_array2D_map_alloc()
OMPVV_INFOMSG("Testing map alloc/delete allocated 2D array")
errors = 0
! Allocate the arrays
allocate(my2DPtr(N,N))
! initialize
my2DPtr(:,:) = 0
! Mapping the array
!$omp target enter data map(alloc: my2DPtr(:,:))
! Assign a value to the allocated space
!$omp target
my2DPtr(:,:) = RESHAPE((/ (i , i = 1,N**2) /), (/ N,N /))
!$omp end target
! Confirm mapping with target region
!$omp target map(from: my2DArr)
my2DArr = my2DPtr
!$omp end target
IF (.NOT. isSharedEnv) THEN
OMPVV_TEST_AND_SET_VERBOSE(errors, ANY(my2DPtr /= 0))
END IF
OMPVV_TEST_AND_SET_VERBOSE(errors, SUM(my2DArr) /= ((N**2*(N**2+1)/2)))
! Asign a host value
[committed] libstdc++: Add comparison operators for string and regex types
Some more C++20 changes from P1614R2, "The Mothership has Landed".
This adds three-way comparison support to std::char_traits,
std::basic_string, std::basic_string_view, and std::sub_match.
* include/bits/basic_string.h (basic_string): Define operator<=> and
remove redundant comparison operators for C++20.
* include/bits/char_traits.h (__gnu_cxx::char_traits, char_traits):
Add comparison_category members.
(__detail::__char_traits_cmp_cat): New helper to get comparison
category from char traits class.
* include/bits/regex.h (regex_traits::_RegexMask::operator!=): Do not
define for C++20.
(sub_match): Define operator<=> and remove redundant comparison
operators for C++20.
(match_results): Remove redundant operator!= for C++20.
* include/std/string_view (basic_string_view): Define operator<=> and
remove redundant comparison operators for C++20.
* testsuite/21_strings/basic_string/operators/char/cmp_c++20.cc: New
test.
* testsuite/21_strings/basic_string/operators/wchar_t/cmp_c++20.cc:
New test.
* testsuite/21_strings/basic_string_view/operations/copy/char/
constexpr.cc: Initialize variable.
* testsuite/21_strings/basic_string_view/operations/copy/wchar_t/
constexpr.cc: Likewise.
* testsuite/21_strings/basic_string_view/operators/char/2.cc: Add
dg-do directive and remove comments showing incorrect signatures.
* testsuite/21_strings/basic_string_view/operators/wchar_t/2.cc:
Likewise.
* testsuite/21_strings/basic_string_view/operators/char/cmp_c++20.cc:
New test.
* testsuite/21_strings/basic_string_view/operators/wchar_t/cmp_c++20.cc:
New test.
* testsuite/28_regex/sub_match/compare_c++20.cc: New test.
Tested powerpc64le-linux, committed to master.
commit 875d6cb3b4919b58ae5e6313db715bc4dd3ddd6c
Author: Jonathan Wakely
Date: Fri Apr 17 16:24:49 2020 +0100
libstdc++: Add comparison operators for string and regex types
Some more C++20 changes from P1614R2, "The Mothership has Landed".
This adds three-way comparison support to std::char_traits,
std::basic_string, std::basic_string_view, and std::sub_match.
* include/bits/basic_string.h (basic_string): Define operator<=> and
remove redundant comparison operators for C++20.
* include/bits/char_traits.h (__gnu_cxx::char_traits, char_traits):
Add comparison_category members.
(__detail::__char_traits_cmp_cat): New helper to get comparison
category from char traits class.
* include/bits/regex.h (regex_traits::_RegexMask::operator!=): Do
not
define for C++20.
(sub_match): Define operator<=> and remove redundant comparison
operators for C++20.
(match_results): Remove redundant operator!= for C++20.
* include/std/string_view (basic_string_view): Define operator<=>
and
remove redundant comparison operators for C++20.
* testsuite/21_strings/basic_string/operators/char/cmp_c++20.cc: New
test.
* testsuite/21_strings/basic_string/operators/wchar_t/cmp_c++20.cc:
New test.
* testsuite/21_strings/basic_string_view/operations/copy/char/
constexpr.cc: Initialize variable.
* testsuite/21_strings/basic_string_view/operations/copy/wchar_t/
constexpr.cc: Likewise.
* testsuite/21_strings/basic_string_view/operators/char/2.cc: Add
dg-do directive and remove comments showing incorrect signatures.
* testsuite/21_strings/basic_string_view/operators/wchar_t/2.cc:
Likewise.
*
testsuite/21_strings/basic_string_view/operators/char/cmp_c++20.cc:
New test.
*
testsuite/21_strings/basic_string_view/operators/wchar_t/cmp_c++20.cc:
New test.
* testsuite/28_regex/sub_match/compare_c++20.cc: New test.
diff --git a/libstdc++-v3/include/bits/basic_string.h
b/libstdc++-v3/include/bits/basic_string.h
index 0374f8e6805..bc0c256b65e 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -6164,18 +6164,6 @@ _GLIBCXX_END_NAMESPACE_CXX11
&& !std::char_traits<_CharT>::compare(__lhs.data(), __rhs.data(),
__lhs.size())); }
- /**
- * @brief Test equivalence of C string and string.
- * @param __lhs C string.
- * @param __rhs String.
- * @return True if @a __rhs.compare(@a __lhs) == 0. False otherwise.
- */
- template
-inline bool
-operator==(const _CharT* __lhs,
- const basic_string<_CharT, _Traits, _Alloc>& __rhs)
-{ return __rhs.compare(__lhs) == 0; }
-
/**
* @brief Test equivalence of string and C string.
*
[PATCH] vect: Tweak vect_better_loop_vinfo_p handling of variable VFs
This patch fixes a large lmbench performance regression with
128-bit SVE, compiled in length-agnostic mode.
vect_better_loop_vinfo_p (new in GCC 10) tries to estimate whether
a new loop_vinfo is cheaper than a previous one, with an in-built
preference for the old one. For variable VF it prefers the old
loop_vinfo if it is cheaper for at least one VF. However, we have
no idea how likely that VF is in practice.
Another extreme would be to do what most of the rest of the
vectoriser does, and rely solely on the constant estimated VF.
But as noted in the comment, this means that a one-unit cost
difference would be enough to pick the new loop_vinfo,
despite the target generally preferring the old loop_vinfo
where possible. The cost model just isn't accurate enough
for that to produce good results as things stand: there might
not be any practical benefit to the new loop_vinfo at the
estimated VF, and it would be significantly worse for higher VFs.
The patch instead goes for a hacky compromise: make sure that the new
loop_vinfo is also no worse than the old loop_vinfo at double the
estimated VF. For all but trivial loops, this ensures that the
new loop_vinfo is only chosen if it is better than the old one
by a non-trivial amount at the estimated VF. It also avoids
putting too much faith in the VF estimate.
I realise this isn't great, but it's supposed to be a conservative fix
suitable for stage 4. The only affected testcases are the ones for
pr89007-*.c, where Advanced SIMD is indeed preferred for 128-bit SVE
and is no worse for 256-bit SVE.
Part of the problem here is that if the new loop_vinfo is better,
we discard the old one and never consider using it even as an
epilogue loop. This means that if we choose Advanced SIMD over SVE,
we're much more likely to have left-over scalar elements.
Another is that the estimate provided by estimated_poly_value might have
different probabilities attached. E.g. when tuning for a particular core,
the estimate is probably accurate, but when tuning for generic code,
the estimate is more of a guess. Relying solely on the estimate is
probably correct for the former but not for the latter.
Hopefully those are things that we could tackle in GCC 11.
Tested on aarch64-linux-gnu, aarch64_be-elf and x86_64-linux-gnu.
OK to install?
Richard
2020-04-17 Richard Sandiford
gcc/
* tree-vect-loop.c (vect_better_loop_vinfo_p): If old_loop_vinfo
has a variable VF, prefer new_loop_vinfo if it is cheaper for the
estimated VF and is no worse at double the estimated VF.
gcc/testsuite/
* gcc.target/aarch64/sve/cost_model_8.c: New test.
* gcc.target/aarch64/sve/cost_model_9.c: Likewise.
* gcc.target/aarch64/sve/pr89007-1.c: Add -msve-vector-bits=512.
* gcc.target/aarch64/sve/pr89007-2.c: Likewise.
---
.../gcc.target/aarch64/sve/cost_model_8.c | 12 +++
.../gcc.target/aarch64/sve/cost_model_9.c | 13
.../gcc.target/aarch64/sve/pr89007-1.c| 2 +-
.../gcc.target/aarch64/sve/pr89007-2.c| 2 +-
gcc/tree-vect-loop.c | 31 ++-
5 files changed, 57 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/cost_model_8.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/cost_model_9.c
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 265bcfdc5af..b6c3faeae51 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -2414,7 +2414,36 @@ vect_better_loop_vinfo_p (loop_vec_info new_loop_vinfo,
poly_widest_int rel_old = (old_loop_vinfo->vec_inside_cost
* poly_widest_int (new_vf));
if (maybe_lt (rel_old, rel_new))
-return false;
+{
+ /* When old_loop_vinfo uses a variable vectorization factor,
+we know that it has a lower cost for at least one runtime VF.
+However, we don't know how likely that VF is.
+
+One option would be to compare the costs for the estimated VFs.
+The problem is that that can put too much pressure on the cost
+model. E.g. if the estimated VF is also the lowest possible VF,
+and if old_loop_vinfo is 1 unit worse than new_loop_vinfo
+for the estimated VF, we'd then choose new_loop_vinfo even
+though (a) new_loop_vinfo might not actually be better than
+old_loop_vinfo for that VF and (b) it would be significantly
+worse at larger VFs.
+
+Here we go for a hacky compromise: pick new_loop_vinfo if it is
+no more expensive than old_loop_vinfo even after doubling the
+estimated old_loop_vinfo VF. For all but trivial loops, this
+ensures that we only pick new_loop_vinfo if it is significantly
+better than old_loop_vinfo at the estimated VF. */
+ if (rel_new.is_constant ())
+ return false;
+
+ HOST_WIDE_INT new_estimated_vf = estimated_poly_value (new_vf);
+ HOST_WIDE_INT old_estimated_vf =
Re: [PATCH] x86: Insert ENDBR if function will be called indirectly
On Wed, Apr 8, 2020 at 9:41 AM Jeff Law wrote:
>
> On Wed, 2020-04-08 at 09:23 -0700, H.J. Lu wrote:
> > On Wed, Apr 8, 2020 at 9:16 AM Jeff Law wrote:
> > > On Tue, 2020-03-31 at 08:11 -0700, H.J. Lu via Gcc-patches wrote:
> > > > Since constant_call_address_operand has
> > > >
> > > > ;; Test for a pc-relative call operand
> > > > (define_predicate "constant_call_address_operand"
> > > > (match_code "symbol_ref")
> > > > {
> > > > if (ix86_cmodel == CM_LARGE || ix86_cmodel == CM_LARGE_PIC
> > > > || flag_force_indirect_call)
> > > > return false;
> > > > if (TARGET_DLLIMPORT_DECL_ATTRIBUTES && SYMBOL_REF_DLLIMPORT_P (op))
> > > > return false;
> > > > return true;
> > > > })
> > > >
> > > > even if cgraph_node::get (cfun->decl)->only_called_directly_p () returns
> > > > false, the fuction may still be called indirectly. Copy the logic from
> > > > constant_call_address_operand to rest_of_insert_endbranch to insert
> > > > ENDBR
> > > > at function entry if function will be called indirectly.
> > > >
> > > > gcc/
> > > >
> > > > PR target/94417
> > > > * config/i386/i386-features.c (rest_of_insert_endbranch): Insert
> > > > ENDBR at function entry if function will be called indirectly.
> > > Can you just call constant_call_address_operand rather than copying its
> > > contents?
> >
> > I wish I could. constant_call_address_operand uses SYMBOL_REF_DLLIMPORT_P
> > (op)
> > But I need to use DECL_DLLIMPORT_P (cfun->decl)).
> Sigh. In that case I guess the patch is OK as-is.
>
I'd like to backport this wrong code fix to GCC 9/8 branches.
Is it OK for GCC 9/8 branches?
Thanks.
--
H.J.
From f86e27e71fb963aaa95b1da31515f888f6e146f8 Mon Sep 17 00:00:00 2001
From: "H.J. Lu"
Date: Wed, 8 Apr 2020 09:47:35 -0700
Subject: [PATCH] x86: Insert ENDBR if function will be called indirectly
Since constant_call_address_operand has
;; Test for a pc-relative call operand
(define_predicate "constant_call_address_operand"
(match_code "symbol_ref")
{
if (ix86_cmodel == CM_LARGE || ix86_cmodel == CM_LARGE_PIC
|| flag_force_indirect_call)
return false;
if (TARGET_DLLIMPORT_DECL_ATTRIBUTES && SYMBOL_REF_DLLIMPORT_P (op))
return false;
return true;
})
even if cgraph_node::get (cfun->decl)->only_called_directly_p () returns
false, the fuction may still be called indirectly. Copy the logic from
constant_call_address_operand to rest_of_insert_endbranch to insert ENDBR
at function entry if function will be called indirectly.
NB: gcc.target/i386/pr94417-2.c is updated to expect 4 ENDBRs, instead
of 2, since only GCC 10 has the fix for PR target/89355 not to insert
ENDBR after NOTE_INSN_DELETED_LABEL.
gcc/
Backport from master
PR target/94417
* config/i386/i386.c (rest_of_insert_endbranch): Insert ENDBR at
function entry if function will be called indirectly.
gcc/testsuite/
PR target/94417
* gcc.target/i386/pr94417-1.c: New test.
* gcc.target/i386/pr94417-2.c: Likewise.
* gcc.target/i386/pr94417-3.c: Likewise.
(cherry picked from commit c5f379653964a1d2c7037b2de3e947a48370a198)
---
gcc/config/i386/i386.c| 7 ++-
gcc/testsuite/gcc.target/i386/pr94417-1.c | 20
gcc/testsuite/gcc.target/i386/pr94417-2.c | 21 +
gcc/testsuite/gcc.target/i386/pr94417-3.c | 19 +++
4 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/i386/pr94417-1.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr94417-2.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr94417-3.c
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 5d12d82db5b..3891d44ce85 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2535,7 +2535,12 @@ rest_of_insert_endbranch (void)
&& (!flag_manual_endbr
|| lookup_attribute ("cf_check",
DECL_ATTRIBUTES (cfun->decl)))
- && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+ && (!cgraph_node::get (cfun->decl)->only_called_directly_p ()
+ || ix86_cmodel == CM_LARGE
+ || ix86_cmodel == CM_LARGE_PIC
+ || flag_force_indirect_call
+ || (TARGET_DLLIMPORT_DECL_ATTRIBUTES
+ && DECL_DLLIMPORT_P (cfun->decl
{
/* Queue ENDBR insertion to x86_function_profiler. */
if (crtl->profile && flag_fentry)
diff --git a/gcc/testsuite/gcc.target/i386/pr94417-1.c b/gcc/testsuite/gcc.target/i386/pr94417-1.c
new file mode 100644
index 000..5bbe057fa8f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr94417-1.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-options "-O2 -fcf-protection -mcmodel=large" } */
+/* { dg-final { scan-assembler-times {\mendbr} 2 } } */
+
+extern void ext (void);
+
+__attribute((noclone, noinline))
+static
+void
+foo (void)
+{
+ ext ();
+}
+
+void
+bar (void)
+{
+ foo ();
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr94417-2.c
[committed] aarch64: Tweak SVE load/store costs
We were seeing performance regressions on 256-bit SVE with code like:
for (int i = 0; i < count; ++i)
#pragma GCC unroll 128
for (int j = 0; j < 128; ++j)
*dst++ = 1;
(derived from lmbench).
For 128-bit SVE, it's clearly better to use Advanced SIMD STPs here,
since they can store 256 bits at a time. We already do this for
-msve-vector-bits=128 because in that case Advanced SIMD comes first
in autovectorize_vector_modes.
If we handled full-loop predication well for this kind of loop,
the choice between Advanced SIMD and 256-bit SVE would be mostly
a wash, since both of them could store 256 bits at a time. However,
SVE would still have the extra prologue overhead of setting up the
predicate, so Advanced SIMD would still be the natural choice.
As things stand though, we don't handle full-loop predication well
for this kind of loop, so the 256-bit SVE code is significantly worse.
Something to fix for GCC 11 (hopefully). However, even though we
account for the overhead of predication in the cost model, the SVE
version (wrongly) appeared to need half the number of stores.
That was enough to drown out the predication overhead and meant
that we'd pick the SVE code over the Advanced SIMD code.
512-bit SVE has a clear advantage over Advanced SIMD, so we should
continue using SVE there.
This patch tries to account for this in the cost model. It's a bit
of a compromise; see the comment in the patch for more details.
Tested on aarch64-linux-gnu and aarch64_be-elf, pushed.
Richard
2020-04-17 Richard Sandiford
gcc/
* config/aarch64/aarch64.c (aarch64_advsimd_ldp_stp_p): New function.
(aarch64_sve_adjust_stmt_cost): Add a vectype parameter. Double the
cost of load and store insns if one loop iteration has enough scalar
elements to use an Advanced SIMD LDP or STP.
(aarch64_add_stmt_cost): Update call accordingly.
gcc/testsuite/
* gcc.target/aarch64/sve/cost_model_2.c: New test.
* gcc.target/aarch64/sve/cost_model_3.c: Likewise.
* gcc.target/aarch64/sve/cost_model_4.c: Likewise.
* gcc.target/aarch64/sve/cost_model_5.c: Likewise.
* gcc.target/aarch64/sve/cost_model_6.c: Likewise.
* gcc.target/aarch64/sve/cost_model_7.c: Likewise.
---
gcc/ChangeLog | 8 ++
gcc/config/aarch64/aarch64.c | 77 ++-
gcc/testsuite/ChangeLog | 9 +++
.../gcc.target/aarch64/sve/cost_model_2.c | 12 +++
.../gcc.target/aarch64/sve/cost_model_3.c | 13
.../gcc.target/aarch64/sve/cost_model_4.c | 12 +++
.../gcc.target/aarch64/sve/cost_model_5.c | 13
.../gcc.target/aarch64/sve/cost_model_6.c | 12 +++
.../gcc.target/aarch64/sve/cost_model_7.c | 12 +++
9 files changed, 164 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/cost_model_2.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/cost_model_3.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/cost_model_4.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/cost_model_5.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/cost_model_6.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/cost_model_7.c
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index d2aa482b2e0..de6a099cb57 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-04-17 Richard Sandiford
+
+ * config/aarch64/aarch64.c (aarch64_advsimd_ldp_stp_p): New function.
+ (aarch64_sve_adjust_stmt_cost): Add a vectype parameter. Double the
+ cost of load and store insns if one loop iteration has enough scalar
+ elements to use an Advanced SIMD LDP or STP.
+ (aarch64_add_stmt_cost): Update call accordingly.
+
2020-04-17 Richard Biener
PR other/94629
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d0a41c286cd..24c055df0dc 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13518,6 +13518,32 @@ aarch64_builtin_vectorization_cost (enum
vect_cost_for_stmt type_of_cost,
}
}
+/* Return true if creating multiple copies of STMT_INFO for Advanced SIMD
+ vectors would produce a series of LDP or STP operations. KIND is the
+ kind of statement that STMT_INFO represents. */
+static bool
+aarch64_advsimd_ldp_stp_p (enum vect_cost_for_stmt kind,
+ stmt_vec_info stmt_info)
+{
+ switch (kind)
+{
+case vector_load:
+case vector_store:
+case unaligned_load:
+case unaligned_store:
+ break;
+
+default:
+ return false;
+}
+
+ if (aarch64_tune_params.extra_tuning_flags
+ & AARCH64_EXTRA_TUNE_NO_LDP_STP_QREGS)
+return false;
+
+ return is_gimple_assign (stmt_info->stmt);
+}
+
/* Return true if STMT_INFO extends the result of a load. */
static bool
aarch64_extending_load_p (stmt_vec_info stmt_info)
@@ -13556,10 +13582,12 @@
Re: [patch, fortran] Fix PR PR93500
Hi Fritz,
First, it appears if simplify_bound_dim returns _bad_expr (and a
div/0 occurs) then this code will free _bad_expr. I'm not sure
whether or not that can actually occur, but it is certainly incorrect,
since _bad_expr points to static storage. The only other possible
case is bounds[d] == NULL, in which case the free is a no-op. I
suggest removing the free call.
OK, removed.
That being said, it looks like the same error condition can occur with
the lcobound intrinsic.
I have adjusted the test case so it also checks for the coarray case;
this is handled correctly with a "Divion by zero" error without
changing the patch in that respect.
So, anything else? If not, I will commit this within a few days.
Regards
Thomas
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 9b95200c241..650837e18c3 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -3986,6 +3986,9 @@ resolve_operator (gfc_expr *e)
op1 = e->value.op.op1;
op2 = e->value.op.op2;
+ if (op1 == NULL && op2 == NULL)
+return false;
+
dual_locus_error = false;
/* op1 and op2 cannot both be BOZ. */
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 807565b4e80..855fbe27d9f 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4251,7 +4251,11 @@ simplify_bound (gfc_expr *array, gfc_expr *dim, gfc_expr
*kind, int upper)
for (j = 0; j < d; j++)
gfc_free_expr (bounds[j]);
- return bounds[d];
+
+ if (gfc_seen_div0)
+ return _bad_expr;
+ else
+ return bounds[d];
}
}
diff --git a/gcc/testsuite/gfortran.dg/arith_divide_3.f90
b/gcc/testsuite/gfortran.dg/arith_divide_3.f90
new file mode 100644
index 000..95682dfdda7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/arith_divide_3.f90
@@ -0,0 +1,17 @@
+! { dg-do compile }
+! { dg-options "-fcoarray=single" }
+! PR 93500 - this used to cause an ICE
+
+program p
+ integer :: a(min(2,0)/0) ! { dg-error "Division by zero" }
+ integer, save :: c[min(2,0)/0,*] ! { dg-error "Division by zero|must have
constant shape" }
+ integer :: b = lbound(a) ! { dg-error "must be an array" }
+ print *,lcobound(c)
+end program p
+
+subroutine s
+ integer :: a(min(2,0)/0) ! { dg-error "Division by zero" }
+ integer, save :: c[min(2,0)/0,*] ! { dg-error "Division by zero" }
+ integer :: b = lbound(a)
+ print *,lcobound(c)
+end subroutine s
[PATCH] c++: Abbreviated function template return type [PR92187]
When updating an auto return type of an abbreviated function template in
splice_late_return_type, we should also propagate PLACEHOLDER_TYPE_CONSTRAINTS
(and cv-qualifiers) of the original auto node.
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK to commit?
gcc/cp/ChangeLog:
PR c++/92187
* pt.c (splice_late_return_type): Propagate cv-qualifiers and
PLACEHOLDER_TYPE_CONSTRAINTS from the original auto node to the new one.
gcc/testsuite/ChangeLog:
PR c++/92187
* g++.dg/concepts/abbrev5.C: New test.
* g++.dg/concepts/abbrev6.C: New test.
---
gcc/cp/pt.c | 15 +++
gcc/testsuite/g++.dg/concepts/abbrev5.C | 15 +++
gcc/testsuite/g++.dg/concepts/abbrev6.C | 12
3 files changed, 38 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev5.C
create mode 100644 gcc/testsuite/g++.dg/concepts/abbrev6.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 0a8ec3198d2..9e39f46a090 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -29032,10 +29032,17 @@ splice_late_return_type (tree type, tree
late_return_type)
{
tree idx = get_template_parm_index (*auto_node);
if (TEMPLATE_PARM_LEVEL (idx) <= processing_template_decl)
- /* In an abbreviated function template we didn't know we were dealing
- with a function template when we saw the auto return type, so update
- it to have the correct level. */
- *auto_node = make_auto_1 (TYPE_IDENTIFIER (*auto_node), true);
+ {
+ /* In an abbreviated function template we didn't know we were dealing
+with a function template when we saw the auto return type, so
update
+it to have the correct level. */
+ tree new_auto = make_auto_1 (TYPE_IDENTIFIER (*auto_node), false);
+ PLACEHOLDER_TYPE_CONSTRAINTS (new_auto)
+ = PLACEHOLDER_TYPE_CONSTRAINTS (*auto_node);
+ TYPE_CANONICAL (new_auto) = canonical_type_parameter (new_auto);
+ new_auto = cp_build_qualified_type (new_auto, TYPE_QUALS
(*auto_node));
+ *auto_node = new_auto;
+ }
}
return type;
}
diff --git a/gcc/testsuite/g++.dg/concepts/abbrev5.C
b/gcc/testsuite/g++.dg/concepts/abbrev5.C
new file mode 100644
index 000..de594b5c1df
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/abbrev5.C
@@ -0,0 +1,15 @@
+// PR c++/92187
+// { dg-do compile { target concepts } }
+
+template
+concept C = false;
+
+C auto f(auto)
+{
+ return 42; // { dg-error "deduced return type" }
+}
+
+void foo()
+{
+ f(0);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/abbrev6.C
b/gcc/testsuite/g++.dg/concepts/abbrev6.C
new file mode 100644
index 000..862675e5193
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/abbrev6.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target concepts } }
+
+const auto (auto)
+{
+ static int n;
+ return n;
+}
+
+void foo()
+{
+ f(5) = 0; // { dg-error "read-only" }
+}
--
2.26.1.107.gefe3874640
Re: [PATCH PR94577] [AArch64] ：Add an error message in large code model for ilp32
"duanbo (C)" writes:
> Thank you for your suggestions.
> I have modified accordingly and a full test has been carried, no new failure
> witnessed.
> Attached please find the new patch which has been adjusted to be suitable for
> git am.
> Does the v2 patch look better ?
>
> Thanks,
> Duan bo
>
> -Original Message-
> From: Wilco Dijkstra [mailto:wilco.dijks...@arm.com]
> Sent: Tuesday, April 14, 2020 4:40 AM
> To: GCC Patches ; duanbo (C)
> Subject: Re: [PATCH PR2] aarch64：Add an error message in large code model
> for ilp32
>
> Hi Duanbo,
>
>> This is a simple fix for pr94577.
>> The option -mabi=ilp32 should not be used in large code model. Like x86,
>> using -mx32 and -mcmodel=large together will result in an error message.
>> On aarch64, there is no error message for this option conflict.
>> A solution to this problem can be found in the attached patch.
>> Bootstrap and tested on aarch64 Linux platform. No new regression witnessed.
>> Any suggestion?
>
> Thanks for your patch, more than 4GB doesn't make any sense with ILP32 indeed.
> A few suggestions:
>
> It would be good to also update the documentation for -mcmodel=large to state
> it is incompatible with -fpic, -fPIC and -mabi=ilp32.
>
> The patch adds a another switch statement on mcmodel that ignores the
> previous processing done (which may changes the selected mcmodel). It would
> be safer and more concise to use one switch at the top level and in each case
> use an if statement to handle the special cases.
>
> A few minor nitpics:
>
> + PR target/94577
> + * gcc.target/aarch64/pr94577.c : New test
>
> Just like comments, there should be a '.' at the end of changelog entries.
>
> AFAICT the format isn't exactly specified, but the email header should be
> like:
>
> [PATCH][AArch64] PR94577: Add an error message in large code model for ilp32
>
> Sometimes the PR number is also placed in brackets.
>
> Cheers,
> Wilco
>
>
> From feb16a5e5d35d4f632e1be10ce0ac4f4c3505d22 Mon Sep 17 00:00:00 2001
> From: Duan bo
> Date: Wed, 15 Apr 2020 05:19:31 -0400
> Subject: [PATCH] aarch64: Add an error message in large code model for ilp32
> [PR94577]
>
> The option -mabi=ilp32 should not be used in large code model. An error
> message is added for the option conflict.
>
> 2020-04-15 Duan bo
>
> PR target/94577
> * config/aarch64/aarch64.c: Add an error message for option conflict.
> * doc/invoke.texi (-mcmodel=large): Mention that -mcmodel=large is
> incompatible with -fpic, -fPIC and -mabi=ilp32.
>
> 2020-04-15 Duan bo
>
> PR target/94577
> * gcc.target/aarch64/pr94577.c: New test.
> ---
> gcc/ChangeLog | 7
> gcc/config/aarch64/aarch64.c | 41 --
> gcc/doc/invoke.texi| 4 ++-
> gcc/testsuite/ChangeLog| 5 +++
> gcc/testsuite/gcc.target/aarch64/pr94577.c | 10 ++
> 5 files changed, 47 insertions(+), 20 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94577.c
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 3c6a45e8fe7..c2f1fcb7bff 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,10 @@
> +2020-04-15 Duan bo
> +
> + PR target/94577
> + * config/aarch64/aarch64.c: Add an error message for option conflict.
> + * doc/invoke.texi (-mcmodel=large): Mention that -mcmodel=large is
> + incompatible with -fpic, -fPIC and -mabi=ilp32.
> +
> 2020-04-14 Max Filippov
>
> PR target/94584
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 4af562a81ea..f8a38bd899a 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -14707,32 +14707,35 @@ aarch64_init_expanders (void)
> static void
> initialize_aarch64_code_model (struct gcc_options *opts)
> {
> - if (opts->x_flag_pic)
> + aarch64_cmodel = opts->x_aarch64_cmodel_var;
> + switch (opts->x_aarch64_cmodel_var)
> {
> - switch (opts->x_aarch64_cmodel_var)
> - {
> - case AARCH64_CMODEL_TINY:
> + case AARCH64_CMODEL_TINY:
> + if (opts->x_flag_pic)
Minor formatting nit, but: the case statement should be indented
by the same amount as the "{" for the switch statement. The code
after the case statement should be indented by two further columns.
(The formatting is wrong in the existing code too, which is probably
what confused things.)
> aarch64_cmodel = AARCH64_CMODEL_TINY_PIC;
> -break;
> - case AARCH64_CMODEL_SMALL:
> + break;
> + case AARCH64_CMODEL_SMALL:
> + if (opts->x_flag_pic)
> +{
> #ifdef HAVE_AS_SMALL_PIC_RELOCS
> -aarch64_cmodel = (flag_pic == 2
> - ? AARCH64_CMODEL_SMALL_PIC
> - : AARCH64_CMODEL_SMALL_SPIC);
> + aarch64_cmodel = (flag_pic == 2
> +? AARCH64_CMODEL_SMALL_PIC
> +:
Re: [RFC] split pseudos during loop unrolling in RTL unroller
On 4/17/20 1:53 AM, Richard Biener wrote: Yeah well, but RTL is not in SSA form and there's no RTL IL verification in place to track degradation. And we even work in the opposite way when expanding to RTL from SSA, coalescing as much as we can ... Which is itself problematic, introducing unnecessary antidependences at the start of RTL. We've seen performance issues with this on several occasions. Bill
c++: avoid testcase warning on arm
I've pushed this patch to avoid a warning introduced by 94426's change
to linkage of lambda.
The arm eabi wants to emit the vtable, other abis don't.
nathan
--
Nathan Sidwell
diff --git i/gcc/testsuite/ChangeLog w/gcc/testsuite/ChangeLog
index dd2cb04916f..b98c72cdd2a 100644
--- i/gcc/testsuite/ChangeLog
+++ w/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-04-17 Nathan Sidwell
+
+ PR c++/94608
+ * g++.dg/lto/pr83720_0.C: Add fn def to avoid warning on arm ABI.
+
2020-04-17 Jakub Jelinek
PR rtl-optimization/94618
diff --git i/gcc/testsuite/g++.dg/lto/pr83720_0.C w/gcc/testsuite/g++.dg/lto/pr83720_0.C
index 4e63c9be7cd..91f36caf2c0 100644
--- i/gcc/testsuite/g++.dg/lto/pr83720_0.C
+++ w/gcc/testsuite/g++.dg/lto/pr83720_0.C
@@ -48,7 +48,7 @@ public:
k(al);
} d([](b::ai) {
struct be {
-virtual void f();
+virtual void f(){}
};
struct bf;
b::c().aj("", ::f);
Re: [PATCH v2] i386: Fix up *testqi_ext_3 define_insn_and_split [PR94567]
On Fri, Apr 17, 2020 at 2:38 PM Jakub Jelinek wrote: > > On Fri, Apr 17, 2020 at 02:12:19PM +0200, Uros Bizjak wrote: > > Is it possible to perform widening only for !TARGET_PARTIAL_REG_STALL? > > I'm worried that highpart may not be cleared, so the effects of > > partial reg stall can be quite noticeable. > > So, like this then? Yes. Please note that there is no partial reg stall when widening from SImode to DImode, but this is already handled. > All I've added was another condition, interdiff: > diff -u gcc/config/i386/i386.md gcc/config/i386/i386.md > --- gcc/config/i386/i386.md 2020-04-16 14:21:41.686972979 +0200 > +++ gcc/config/i386/i386.md 2020-04-17 14:29:33.882690098 +0200 > @@ -8750,6 +8750,18 @@ > == GET_MODE_PRECISION (GET_MODE (operands[2])) > && !register_operand (operands[2], >GET_MODE (operands[2]))) > +/* And we shouldn't widen if > + TARGET_PARTIAL_REG_STALL. */ > +|| (TARGET_PARTIAL_REG_STALL > +&& (INTVAL (operands[3]) + INTVAL (operands[4]) > +>= (paradoxical_subreg_p (operands[2]) > +&& (GET_MODE_CLASS > + (GET_MODE (SUBREG_REG > (operands[2]))) > +== MODE_INT) > +? GET_MODE_PRECISION > +(GET_MODE (SUBREG_REG (operands[2]))) > +: GET_MODE_PRECISION > +(GET_MODE (operands[2]) > ? CCZmode : CCNOmode)" >"#" >"&& 1" > and nothing in the splitter would need to change because the condition > would ensure that for TARGET_PARTIAL_REG_STALL we require in all problematic > cases CCZmode. The only exception would be the pos + len == 8 > optimization which we wouldn't perform (of course with CCZmode we'd still > do), but that isn't a partial reg stall thing, that is an optimization to > use shorter testb instead of testw. > > 2020-04-17 Jakub Jelinek > Jeff Law > > PR target/94567 > * config/i386/i386.md (*testqi_ext_3): Use CCZmode rather than > CCNOmode in ix86_match_ccmode if len is equal to mode precision, > or pos + len >= 32, or pos + len is equal to operands[2] precision > and operands[2] is not a register operand. During splitting perform > SImode AND if operands[0] doesn't have CCZmode and pos + len is > equal to mode precision. > > * gcc.c-torture/execute/pr94567.c: New test. OK, as far as I can see. Thanks, Uros. > --- gcc/config/i386/i386.md.jj 2020-04-17 08:49:33.236921107 +0200 > +++ gcc/config/i386/i386.md 2020-04-17 14:29:33.882690098 +0200 > @@ -8730,10 +8730,38 @@ (define_insn_and_split "*testqi_ext_3" >&& INTVAL (operands[3]) > 32 >&& INTVAL (operands[3]) + INTVAL (operands[4]) == 64)) > && ix86_match_ccmode (insn, > -/* *testdi_1 requires CCZmode if the mask has bit > +/* If zero_extract mode precision is the same > + as len, the SF of the zero_extract > + comparison will be the most significant > + extracted bit, but this could be matched > + after splitting only for pos 0 len all bits > + trivial extractions. Require CCZmode. */ > +(GET_MODE_PRECISION (mode) > + == INTVAL (operands[3])) > +/* Otherwise, require CCZmode if we'd use a mask > + with the most significant bit set and can't > + widen it to wider mode. *testdi_1 also > + requires CCZmode if the mask has bit > 31 set and all bits above it clear. */ > -GET_MODE (operands[2]) == DImode > -&& INTVAL (operands[3]) + INTVAL (operands[4]) == 32 > +|| (INTVAL (operands[3]) + INTVAL (operands[4]) > +>= 32) > +/* We can't widen also if val is not a REG. */ > +|| (INTVAL (operands[3]) + INTVAL (operands[4]) > +== GET_MODE_PRECISION (GET_MODE (operands[2])) > +&& !register_operand (operands[2], > + GET_MODE (operands[2]))) > +/* And we shouldn't widen if > + TARGET_PARTIAL_REG_STALL. */ > +|| (TARGET_PARTIAL_REG_STALL > +&& (INTVAL (operands[3]) + INTVAL
[PATCH v2] i386: Fix up *testqi_ext_3 define_insn_and_split [PR94567]
On Fri, Apr 17, 2020 at 02:12:19PM +0200, Uros Bizjak wrote: > Is it possible to perform widening only for !TARGET_PARTIAL_REG_STALL? > I'm worried that highpart may not be cleared, so the effects of > partial reg stall can be quite noticeable. So, like this then? All I've added was another condition, interdiff: diff -u gcc/config/i386/i386.md gcc/config/i386/i386.md --- gcc/config/i386/i386.md 2020-04-16 14:21:41.686972979 +0200 +++ gcc/config/i386/i386.md 2020-04-17 14:29:33.882690098 +0200 @@ -8750,6 +8750,18 @@ == GET_MODE_PRECISION (GET_MODE (operands[2])) && !register_operand (operands[2], GET_MODE (operands[2]))) +/* And we shouldn't widen if + TARGET_PARTIAL_REG_STALL. */ +|| (TARGET_PARTIAL_REG_STALL +&& (INTVAL (operands[3]) + INTVAL (operands[4]) +>= (paradoxical_subreg_p (operands[2]) +&& (GET_MODE_CLASS + (GET_MODE (SUBREG_REG (operands[2]))) +== MODE_INT) +? GET_MODE_PRECISION +(GET_MODE (SUBREG_REG (operands[2]))) +: GET_MODE_PRECISION +(GET_MODE (operands[2]) ? CCZmode : CCNOmode)" "#" "&& 1" and nothing in the splitter would need to change because the condition would ensure that for TARGET_PARTIAL_REG_STALL we require in all problematic cases CCZmode. The only exception would be the pos + len == 8 optimization which we wouldn't perform (of course with CCZmode we'd still do), but that isn't a partial reg stall thing, that is an optimization to use shorter testb instead of testw. 2020-04-17 Jakub Jelinek Jeff Law PR target/94567 * config/i386/i386.md (*testqi_ext_3): Use CCZmode rather than CCNOmode in ix86_match_ccmode if len is equal to mode precision, or pos + len >= 32, or pos + len is equal to operands[2] precision and operands[2] is not a register operand. During splitting perform SImode AND if operands[0] doesn't have CCZmode and pos + len is equal to mode precision. * gcc.c-torture/execute/pr94567.c: New test. --- gcc/config/i386/i386.md.jj 2020-04-17 08:49:33.236921107 +0200 +++ gcc/config/i386/i386.md 2020-04-17 14:29:33.882690098 +0200 @@ -8730,10 +8730,38 @@ (define_insn_and_split "*testqi_ext_3" && INTVAL (operands[3]) > 32 && INTVAL (operands[3]) + INTVAL (operands[4]) == 64)) && ix86_match_ccmode (insn, -/* *testdi_1 requires CCZmode if the mask has bit +/* If zero_extract mode precision is the same + as len, the SF of the zero_extract + comparison will be the most significant + extracted bit, but this could be matched + after splitting only for pos 0 len all bits + trivial extractions. Require CCZmode. */ +(GET_MODE_PRECISION (mode) + == INTVAL (operands[3])) +/* Otherwise, require CCZmode if we'd use a mask + with the most significant bit set and can't + widen it to wider mode. *testdi_1 also + requires CCZmode if the mask has bit 31 set and all bits above it clear. */ -GET_MODE (operands[2]) == DImode -&& INTVAL (operands[3]) + INTVAL (operands[4]) == 32 +|| (INTVAL (operands[3]) + INTVAL (operands[4]) +>= 32) +/* We can't widen also if val is not a REG. */ +|| (INTVAL (operands[3]) + INTVAL (operands[4]) +== GET_MODE_PRECISION (GET_MODE (operands[2])) +&& !register_operand (operands[2], + GET_MODE (operands[2]))) +/* And we shouldn't widen if + TARGET_PARTIAL_REG_STALL. */ +|| (TARGET_PARTIAL_REG_STALL +&& (INTVAL (operands[3]) + INTVAL (operands[4]) +>= (paradoxical_subreg_p (operands[2]) +&& (GET_MODE_CLASS + (GET_MODE (SUBREG_REG (operands[2]))) +== MODE_INT) +? GET_MODE_PRECISION +
Re: [PATCH] i386: Fix up *testqi_ext_3 define_insn_and_split [PR94567]
On Fri, Apr 17, 2020 at 10:29 AM Jakub Jelinek wrote: > > Hi! > > As the testcase shows, there are unfortunately more problematic cases > in *testqi_ext_3 if the mode is not CCZmode, because the sign flag might > not behave the same between the insn with zero_extract and what we split it > into. > > The previous fix to the insn condition was because *testdi_1 for mask with > upper 32-bits clear and bit 31 set is implemented using SImode test and thus > SF is set depending on that bit 31 rather than on always cleared. > > But we can have other cases. On the zero_extract (which has mode), > we can have either the pos + len == precision of mode, or > pos + len < precision of mode cases. The former one copies the most > significant bit into SF, the latter will have SF always cleared. > > For the former case, either it is a zero_extract from a larger mode, but > then when we perform test in that larger mode, SF will be always clear and > thus mismatch from the zero_extract case (so we need to enforce CCZmode), > or it will be a zero_extract from same mode with pos 0 and len equal to > mode precision, such zero_extracts should have been really simplified > into their first operand. > > For the latter case, when SF is always clear on the define_insn with > zero_extract, we need to split into something that doesn't sometimes set > SF, i.e. it has to be a test with mask that doesn't have the most > significant bit set. In some cases it can be achieved through using test > in a wider mode (e.g. in the testcase, there is > (zero_extract:SI (reg:HI) (const_int 13) (const_int 3)) > which will always set SF to 0, but we split it into > (and:HI (reg:HI) (const_int -8)) > which will copy the MSB of (reg:HI) into SF, but we can do: > (and:SI (subreg:SI (reg:HI) 0) (const_int 0xfff8)) > which will keep SF always cleared), but there are various cases where we > can't (when already using DImode, or when SImode and we'd turned it into > the problematic *testdi_1 implemented using SImode test, or when > the val operand is a MEM (we don't want to read from memory more than > the user originally wanted), paradoxical subreg of MEM could be problematic > too if we through the narrowing end up with a MEM). > > So, the patch attempts to require CCZmode (and not CCNOmode) if it can't > really ensure the SF will have same meaning between the define_insn and what > we split it into, and if we decide we allow CCNOmode, it needs to avoid > performing narrowing and/or widen if pos + len would indicate we'd have MSB > set in the mask. > > I must say I'm not really sure about the performance effects of the widening > for partial register stall. Is it possible to perform widening only for !TARGET_PARTIAL_REG_STALL? I'm worried that highpart may not be cleared, so the effects of partial reg stall can be quite noticeable. Otherwise, thank you for your persistence! I'd limit the pattern to process only a couple of important and known good cases by now, and this way end the constant fuss with this pattern. Uros. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > Or do you want instead to require CCZmode in the define_insn condition so > that we don't do any widening? We'd still need to avoid some of the > narrowing and probably even in the condition look through the paradoxical > subregs to find out what mode we plan to do the test in. > > Note, Jeff has preapproved the patch in the PR, but I'd still like to > give Uros as the maintainer the chance to chime in. > > 2020-04-17 Jakub Jelinek > Jeff Law > > PR target/94567 > * config/i386/i386.md (*testqi_ext_3): Use CCZmode rather than > CCNOmode in ix86_match_ccmode if len is equal to mode precision, > or pos + len >= 32, or pos + len is equal to operands[2] precision > and operands[2] is not a register operand. During splitting perform > SImode AND if operands[0] doesn't have CCZmode and pos + len is > equal to mode precision. > > * gcc.c-torture/execute/pr94567.c: New test. > > --- gcc/config/i386/i386.md.jj 2020-04-16 14:14:47.045182166 +0200 > +++ gcc/config/i386/i386.md 2020-04-16 14:21:41.686972979 +0200 > @@ -8730,10 +8730,26 @@ (define_insn_and_split "*testqi_ext_3" >&& INTVAL (operands[3]) > 32 >&& INTVAL (operands[3]) + INTVAL (operands[4]) == 64)) > && ix86_match_ccmode (insn, > -/* *testdi_1 requires CCZmode if the mask has bit > +/* If zero_extract mode precision is the same > + as len, the SF of the zero_extract > + comparison will be the most significant > + extracted bit, but this could be matched > + after splitting only for pos 0 len all bits > + trivial extractions. Require CCZmode. */ > +(GET_MODE_PRECISION (mode) > +
RE: [PATCH] aarch64: Fix .cfi_window_save with pac-ret [PR94515]
Hi Szabolcs,
> -Original Message-
> From: Szabolcs Nagy
> Sent: 09 April 2020 15:20
> To: gcc-patches@gcc.gnu.org
> Cc: Richard Earnshaw ; Richard Sandiford
> ; Kyrylo Tkachov
> Subject: [PATCH] aarch64: Fix .cfi_window_save with pac-ret [PR94515]
>
> On aarch64 -mbranch-protection=pac-ret reuses the dwarf
> opcode for window_save to mean "toggle the return address
> mangle state", but in the dwarf2cfi internal logic the
> state was not properly recorded so the remember/restore
> logic was broken.
>
> This can cause the unwinder not to authenticate return
> addresses that were signed (or vice versa) which means
> a runtime crash on a pauth enabled system.
>
> Currently only aarch64 pac-ret uses REG_CFA_TOGGLE_RA_MANGLE.
I think this is ok, but this code is in the midend so I've CC'ed Jason who,
from what I gather from MAINTAINERS, is maintainer for this file.
Thanks,
Kyrill
>
> gcc/ChangeLog:
>
> 2020-04-XX Szabolcs Nagy
>
> PR target/94515
> * dwarf2cfi.c (dwarf2out_frame_debug): Record RA
> mangle state in cur_row->window_save.
>
> gcc/testsuite/ChangeLog:
>
> 2020-04-XX Szabolcs Nagy
>
> PR target/94515
> * g++.target/aarch64/pr94515.C: New test.
> ---
> gcc/dwarf2cfi.c| 3 ++
> gcc/testsuite/g++.target/aarch64/pr94515.C | 43 ++
> 2 files changed, 46 insertions(+)
> create mode 100644 gcc/testsuite/g++.target/aarch64/pr94515.C
>
> diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
> index 229fbfacc30..22989a6c2fb 100644
> --- a/gcc/dwarf2cfi.c
> +++ b/gcc/dwarf2cfi.c
> @@ -2145,6 +2145,9 @@ dwarf2out_frame_debug (rtx_insn *insn)
>case REG_CFA_TOGGLE_RA_MANGLE:
> /* This uses the same DWARF opcode as the next operation. */
> dwarf2out_frame_debug_cfa_window_save (true);
> + /* Keep track of RA mangle state by toggling the window_save bit.
> +This is needed so .cfi_window_save is emitted correctly. */
> + cur_row->window_save = !cur_row->window_save;
> handled_one = true;
> break;
>
> diff --git a/gcc/testsuite/g++.target/aarch64/pr94515.C
> b/gcc/testsuite/g++.target/aarch64/pr94515.C
> new file mode 100644
> index 000..7707c773a72
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/aarch64/pr94515.C
> @@ -0,0 +1,43 @@
> +/* PR target/94515. Check .cfi_window_save with multiple return paths. */
> +/* { dg-do run } */
> +/* { dg-additional-options "-O2 -mbranch-protection=pac-ret --save-temps" }
> */
> +
> +volatile int zero = 0;
> +
> +__attribute__((noinline, target("branch-protection=none")))
> +void unwind (void)
> +{
> + if (zero == 0)
> +throw 42;
> +}
> +
> +__attribute__((noinline, noipa, target("branch-protection=pac-ret")))
> +int test (int z)
> +{
> + if (z) {
> +asm volatile ("":::"x20","x21");
> +unwind ();
> +return 1;
> + } else {
> +unwind ();
> +return 2;
> + }
> +}
> +
> +__attribute__((target("branch-protection=none")))
> +int main ()
> +{
> + try {
> +test (zero);
> +__builtin_abort ();
> + } catch (...) {
> +return 0;
> + }
> + __builtin_abort ();
> +}
> +
> +/* This check only works if there are two return paths in test and
> + cfi_window_save is used for both instead of cfi_remember_state
> + plus cfi_restore_state. This is currently the case with -O2. */
> +
> +/* { dg-final { scan-assembler-times {\t\.cfi_window_save\n} 4 } } */
> --
> 2.17.1
RE: [PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal frames [PR94514]
Hi Szabolcs,
> -Original Message-
> From: Szabolcs Nagy
> Sent: 09 April 2020 15:20
> To: gcc-patches@gcc.gnu.org
> Cc: Richard Earnshaw ; Richard Sandiford
> ; Kyrylo Tkachov
> Subject: [PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal
> frames [PR94514]
>
> With -mbranch-protection=pac-ret the debug info toggles the
> signedness state of the return address so the unwinder knows when
> the return address needs pointer authentication.
>
> The unwind context flags were not updated according to the dwarf
> frame info.
>
> This causes unwinding across frames that were built without pac-ret
> to incorrectly authenticate the return address wich corrupts the
> return address on a system where PAuth is enabled.
>
> Note: This even affects systems where all code use pac-ret because
> unwinding across a signal frame the return address is not signed.
>
Ok, I'm guessing this needs backporting?
Thanks,
Kyrill
> gcc/testsuite/ChangeLog:
>
> 2020-04-XX Szabolcs Nagy
>
> PR target/94514
> * g++.target/aarch64/pr94514.C: New test.
> * gcc.target/aarch64/pr94514.c: New test.
>
> libgcc/ChangeLog:
>
> 2020-04-XX Szabolcs Nagy
>
> PR target/94514
> * config/aarch64/aarch64-unwind.h (aarch64_frob_update_context):
> Update context->flags accroding to the frame state.
> ---
> gcc/testsuite/g++.target/aarch64/pr94514.C | 26
> gcc/testsuite/gcc.target/aarch64/pr94514.c | 76 ++
> libgcc/config/aarch64/aarch64-unwind.h | 2 +
> 3 files changed, 104 insertions(+)
> create mode 100644 gcc/testsuite/g++.target/aarch64/pr94514.C
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94514.c
>
> diff --git a/gcc/testsuite/g++.target/aarch64/pr94514.C
> b/gcc/testsuite/g++.target/aarch64/pr94514.C
> new file mode 100644
> index 000..2a8c949ba30
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/aarch64/pr94514.C
> @@ -0,0 +1,26 @@
> +/* PR target/94514. Unwind across mixed pac-ret and non-pac-ret frames.
> */
> +/* { dg-do run } */
> +
> +__attribute__((noinline, target("branch-protection=pac-ret")))
> +static void do_throw (void)
> +{
> + throw 42;
> + __builtin_abort ();
> +}
> +
> +__attribute__((noinline, target("branch-protection=none")))
> +static void no_pac_ret (void)
> +{
> + do_throw ();
> + __builtin_abort ();
> +}
> +
> +int main ()
> +{
> + try {
> +no_pac_ret ();
> + } catch (...) {
> +return 0;
> + }
> + __builtin_abort ();
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr94514.c
> b/gcc/testsuite/gcc.target/aarch64/pr94514.c
> new file mode 100644
> index 000..bbbf5a6b0b3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr94514.c
> @@ -0,0 +1,76 @@
> +/* PR target/94514. Unwind across mixed pac-ret and non-pac-ret frames.
> */
> +/* { dg-do run } */
> +/* { dg-options "-fexceptions -O2" } */
> +
> +#include
> +#include
> +#include
> +
> +#define die() \
> + do { \
> +printf ("%s:%d: reached unexpectedly.\n", __FILE__, __LINE__); \
> +fflush (stdout); \
> +abort (); \
> + } while (0)
> +
> +static struct _Unwind_Exception exc;
> +
> +static _Unwind_Reason_Code
> +force_unwind_stop (int version, _Unwind_Action actions,
> + _Unwind_Exception_Class exc_class,
> + struct _Unwind_Exception *exc_obj,
> + struct _Unwind_Context *context,
> + void *stop_parameter)
> +{
> + printf ("%s: CFA: %p PC: %p actions: %d\n",
> + __func__,
> + (void *)_Unwind_GetCFA (context),
> + (void *)_Unwind_GetIP (context),
> + (int)actions);
> + if (actions & _UA_END_OF_STACK)
> +die ();
> + return _URC_NO_REASON;
> +}
> +
> +static void force_unwind (void)
> +{
> +#ifndef __USING_SJLJ_EXCEPTIONS__
> + _Unwind_ForcedUnwind (, force_unwind_stop, 0);
> +#else
> + _Unwind_SjLj_ForcedUnwind (, force_unwind_stop, 0);
> +#endif
> +}
> +
> +__attribute__((noinline, target("branch-protection=pac-ret")))
> +static void f2_pac_ret (void)
> +{
> + force_unwind ();
> + die ();
> +}
> +
> +__attribute__((noinline, target("branch-protection=none")))
> +static void f1_no_pac_ret (void)
> +{
> + f2_pac_ret ();
> + die ();
> +}
> +
> +__attribute__((noinline, target("branch-protection=pac-ret")))
> +static void f0_pac_ret (void)
> +{
> + f1_no_pac_ret ();
> + die ();
> +}
> +
> +static void cleanup_handler (void *p)
> +{
> + printf ("%s: Success.\n", __func__);
> + exit (0);
> +}
> +
> +int main ()
> +{
> + char dummy __attribute__((cleanup (cleanup_handler)));
> + f0_pac_ret ();
> + die ();
> +}
> diff --git a/libgcc/config/aarch64/aarch64-unwind.h
> b/libgcc/config/aarch64/aarch64-unwind.h
> index 4c8790bae67..ed84a96db41 100644
> --- a/libgcc/config/aarch64/aarch64-unwind.h
> +++ b/libgcc/config/aarch64/aarch64-unwind.h
> @@ -104,6 +104,8 @@ aarch64_frob_update_context (struct
> _Unwind_Context *context,
>if (fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset & 0x1)
Re: [PATCH] Initialize file_data->lto_section_header before lto_mode_identity_table call.
On Fri, Apr 17, 2020 at 10:44 AM Martin Liška wrote: > > Hi. > > It's quite obvious fix where we ask for a compression algorithm > to file_data->lto_section_header which is unfilled. > > Patch works with nvptx offloading compiler and lto.exp works fine > on x86_64-linux-gnu. > > Ready to be installed? OK. > Thanks, > Martin > > gcc/lto/ChangeLog: > > 2020-04-17 Martin Liska > > PR lto/94612 > * lto-common.c: Initialize file_data->lto_section_header > before lto_mode_identity_table call. It is needed because > it decompresses a LTO section. > --- > gcc/lto/lto-common.c | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) > >
[PATCH] Initialize file_data->lto_section_header before lto_mode_identity_table call.
Hi.
It's quite obvious fix where we ask for a compression algorithm
to file_data->lto_section_header which is unfilled.
Patch works with nvptx offloading compiler and lto.exp works fine
on x86_64-linux-gnu.
Ready to be installed?
Thanks,
Martin
gcc/lto/ChangeLog:
2020-04-17 Martin Liska
PR lto/94612
* lto-common.c: Initialize file_data->lto_section_header
before lto_mode_identity_table call. It is needed because
it decompresses a LTO section.
---
gcc/lto/lto-common.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/gcc/lto/lto-common.c b/gcc/lto/lto-common.c
index e073abce2e7..cee5f0e9935 100644
--- a/gcc/lto/lto-common.c
+++ b/gcc/lto/lto-common.c
@@ -2197,11 +2197,6 @@ lto_file_finalize (struct lto_file_decl_data *file_data, lto_file *file,
file_data->renaming_hash_table = lto_create_renaming_table ();
file_data->file_name = file->filename;
file_data->order = order;
-#ifdef ACCEL_COMPILER
- lto_input_mode_table (file_data);
-#else
- file_data->mode_table = lto_mode_identity_table;
-#endif
/* Read and verify LTO section. */
data = lto_get_summary_section_data (file_data, LTO_section_lto, );
@@ -2217,6 +2212,12 @@ lto_file_finalize (struct lto_file_decl_data *file_data, lto_file *file,
file_data->lto_section_header.minor_version,
file_data->file_name);
+#ifdef ACCEL_COMPILER
+ lto_input_mode_table (file_data);
+#else
+ file_data->mode_table = lto_mode_identity_table;
+#endif
+
data = lto_get_summary_section_data (file_data, LTO_section_decls, );
if (data == NULL)
{
[PATCH] i386: Fix up *testqi_ext_3 define_insn_and_split [PR94567]
Hi! As the testcase shows, there are unfortunately more problematic cases in *testqi_ext_3 if the mode is not CCZmode, because the sign flag might not behave the same between the insn with zero_extract and what we split it into. The previous fix to the insn condition was because *testdi_1 for mask with upper 32-bits clear and bit 31 set is implemented using SImode test and thus SF is set depending on that bit 31 rather than on always cleared. But we can have other cases. On the zero_extract (which has mode), we can have either the pos + len == precision of mode, or pos + len < precision of mode cases. The former one copies the most significant bit into SF, the latter will have SF always cleared. For the former case, either it is a zero_extract from a larger mode, but then when we perform test in that larger mode, SF will be always clear and thus mismatch from the zero_extract case (so we need to enforce CCZmode), or it will be a zero_extract from same mode with pos 0 and len equal to mode precision, such zero_extracts should have been really simplified into their first operand. For the latter case, when SF is always clear on the define_insn with zero_extract, we need to split into something that doesn't sometimes set SF, i.e. it has to be a test with mask that doesn't have the most significant bit set. In some cases it can be achieved through using test in a wider mode (e.g. in the testcase, there is (zero_extract:SI (reg:HI) (const_int 13) (const_int 3)) which will always set SF to 0, but we split it into (and:HI (reg:HI) (const_int -8)) which will copy the MSB of (reg:HI) into SF, but we can do: (and:SI (subreg:SI (reg:HI) 0) (const_int 0xfff8)) which will keep SF always cleared), but there are various cases where we can't (when already using DImode, or when SImode and we'd turned it into the problematic *testdi_1 implemented using SImode test, or when the val operand is a MEM (we don't want to read from memory more than the user originally wanted), paradoxical subreg of MEM could be problematic too if we through the narrowing end up with a MEM). So, the patch attempts to require CCZmode (and not CCNOmode) if it can't really ensure the SF will have same meaning between the define_insn and what we split it into, and if we decide we allow CCNOmode, it needs to avoid performing narrowing and/or widen if pos + len would indicate we'd have MSB set in the mask. I must say I'm not really sure about the performance effects of the widening for partial register stall. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Or do you want instead to require CCZmode in the define_insn condition so that we don't do any widening? We'd still need to avoid some of the narrowing and probably even in the condition look through the paradoxical subregs to find out what mode we plan to do the test in. Note, Jeff has preapproved the patch in the PR, but I'd still like to give Uros as the maintainer the chance to chime in. 2020-04-17 Jakub Jelinek Jeff Law PR target/94567 * config/i386/i386.md (*testqi_ext_3): Use CCZmode rather than CCNOmode in ix86_match_ccmode if len is equal to mode precision, or pos + len >= 32, or pos + len is equal to operands[2] precision and operands[2] is not a register operand. During splitting perform SImode AND if operands[0] doesn't have CCZmode and pos + len is equal to mode precision. * gcc.c-torture/execute/pr94567.c: New test. --- gcc/config/i386/i386.md.jj 2020-04-16 14:14:47.045182166 +0200 +++ gcc/config/i386/i386.md 2020-04-16 14:21:41.686972979 +0200 @@ -8730,10 +8730,26 @@ (define_insn_and_split "*testqi_ext_3" && INTVAL (operands[3]) > 32 && INTVAL (operands[3]) + INTVAL (operands[4]) == 64)) && ix86_match_ccmode (insn, -/* *testdi_1 requires CCZmode if the mask has bit +/* If zero_extract mode precision is the same + as len, the SF of the zero_extract + comparison will be the most significant + extracted bit, but this could be matched + after splitting only for pos 0 len all bits + trivial extractions. Require CCZmode. */ +(GET_MODE_PRECISION (mode) + == INTVAL (operands[3])) +/* Otherwise, require CCZmode if we'd use a mask + with the most significant bit set and can't + widen it to wider mode. *testdi_1 also + requires CCZmode if the mask has bit 31 set and all bits above it clear. */ -GET_MODE (operands[2]) == DImode -&& INTVAL (operands[3]) + INTVAL (operands[4]) == 32 +|| (INTVAL (operands[3]) + INTVAL
Re: [PATCH] Fix a few of the PVS studio reported bugs
On Fri, 17 Apr 2020, Jakub Jelinek wrote: > On Fri, Apr 17, 2020 at 09:39:20AM +0200, Richard Biener wrote: > > On Fri, 17 Apr 2020, Jakub Jelinek wrote: > > > > > On Fri, Apr 17, 2020 at 09:24:34AM +0200, Richard Biener wrote: > > > > --- a/gcc/optabs.c > > > > +++ b/gcc/optabs.c > > > > @@ -1050,7 +1050,7 @@ expand_binop_directly (enum insn_code icode, > > > > machine_mode mode, optab binoptab, > > > >commutative_p = commutative_optab_p (binoptab); > > > >if (commutative_p > > > >&& GET_MODE (xop0) != xmode0 && GET_MODE (xop1) != xmode1 > > > > - && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode1) > > > > + && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode0) > > > > > > Do we need 4 comparisons for that? Wouldn't > > > if (commutative_p > > > && xmode0 != xmode1 > > > && GET_MODE (xop0) == xmode1 > > > && GET_MODE (xop1) == xmode0) > > > std::swap (xop0, xop1); > > > be more readable and shorter? > > > > Not sure - xmode[01] are derived from insn_data while xop[01] are > > passed in as arguments so it seems the code wants to check the > > modes are not fine already, not whether swapping is a no-op > > because both are equal. > > I meant that the condition after your changes is actually functionally > equivalent to the one I've posted. > Because, if xmode0 == xmode1, then the condition will never be true, > as either GET_MODE (xop0) == xmode1 or GET_MODE (xop0) != xmode0 will > be false. And if xmode0 != xmode1, we have the two == comparisons, > so no need to check further that it is unequal to something else. OK, that's true. Not sure about being more readable though. Richard.
Re: [PATCH] Fix -fcompare-debug issue in delete_insn_and_edges [PR94618]
On Fri, 17 Apr 2020, Jakub Jelinek wrote:
> Hi!
>
> delete_insn_and_edges calls purge_dead_edges whenever deleting the last insn
> in a bb, whatever it is. If it called it only for mandatory last insns
> in the basic block (that may not be followed by DEBUG_INSNs, dunno if that
> is control_flow_insn_p or something more complex), that wouldn't be a
> problem, but as it calls it on any last insn and can actually do something
> in the bb, if such an insn is followed by one more more DEBUG_INSNs and
> nothing else in the same bb, we don't call purge_dead_edges with -g and do
> call it with -g0.
>
> On the testcase, there are two reg-to-reg moves with REG_EH_REGION notes
> (previously memory accesses but simplified and yet not optimized), and the
> second is followed by DEBUG_INSNs; the second move is delete_insn_and_edges
> and after removing it, for -g0 purge_dead_edges removes the REG_EH_REGION
> from the now last insn in the bb (the first reg-to-reg move), while
> for -g it isn't called and things diverge from that quickly on.
>
> Fixed by calling purdge_dead_edges even if we remove the last real insn
> followed only by DEBUG_INSNs in the same bb.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2020-04-17 Jakub Jelinek
>
> PR rtl-optimization/94618
> * cfgrtl.c (delete_insn_and_edges): Set purge not just when
> insn is the BB_END of its block, but also when it is only followed
> by DEBUG_INSNs in its block.
>
> * g++.dg/opt/pr94618.C: New test.
>
> --- gcc/cfgrtl.c.jj 2020-01-12 11:54:36.218416351 +0100
> +++ gcc/cfgrtl.c 2020-04-16 17:28:00.792849169 +0200
> @@ -230,10 +230,20 @@ delete_insn_and_edges (rtx_insn *insn)
> {
>bool purge = false;
>
> - if (INSN_P (insn)
> - && BLOCK_FOR_INSN (insn)
> - && BB_END (BLOCK_FOR_INSN (insn)) == insn)
> -purge = true;
> + if (INSN_P (insn) && BLOCK_FOR_INSN (insn))
> +{
> + basic_block bb = BLOCK_FOR_INSN (insn);
> + if (BB_END (bb) == insn)
> + purge = true;
> + else if (DEBUG_INSN_P (BB_END (bb)))
> + for (rtx_insn *dinsn = NEXT_INSN (insn);
> + DEBUG_INSN_P (dinsn); dinsn = NEXT_INSN (dinsn))
> + if (BB_END (bb) == dinsn)
> + {
> + purge = true;
> + break;
> + }
> +}
>delete_insn (insn);
>if (purge)
> return purge_dead_edges (BLOCK_FOR_INSN (insn));
> --- gcc/testsuite/g++.dg/opt/pr94618.C.jj 2020-04-16 17:32:45.891637523
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr94618.C2020-04-16 17:32:22.715979882
> +0200
> @@ -0,0 +1,25 @@
> +// PR rtl-optimization/94618
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O2 -fnon-call-exceptions -fcompare-debug" }
> +
> +struct S
> +{
> + int a, b, c;
> + int foo () noexcept { return a; }
> + int bar () noexcept { return b; }
> + void baz (int);
> + void qux () { if (c) for (int x = foo (); x != bar (); ) baz (x); }
> +};
> +
> +struct T
> +{
> + S s;
> + void foo ();
> +};
> +
> +void
> +T::foo ()
> +{
> + s.qux ();
> + s.qux ();
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH] Allow new/delete operator deletion only for replaceable.
On Fri, 17 Apr 2020 at 08:05, Jakub Jelinek wrote:
> One needs to use check-c++-all or GXX_TESTSUITE_STDS=98,11,14,17,2a make check
> or similar to get that though, because 11 isn't tested by default, only 98,
> 14 and 17 are ATM I think.
Right.
> Fixed thusly, committed to trunk as obvious.
Alternatively, leave it so it can be tested with C++11 but only
declare the sized delete when the compiler supports it:
#if __cpp_sized_deallocation
void operator delete[](void* ptr, __SIZE_TYPE__ sz) noexcept {
--count;
::operator delete(ptr, sz);
}
#endif
Re: [PATCH] Fix a few of the PVS studio reported bugs
On Fri, Apr 17, 2020 at 09:39:20AM +0200, Richard Biener wrote: > On Fri, 17 Apr 2020, Jakub Jelinek wrote: > > > On Fri, Apr 17, 2020 at 09:24:34AM +0200, Richard Biener wrote: > > > --- a/gcc/optabs.c > > > +++ b/gcc/optabs.c > > > @@ -1050,7 +1050,7 @@ expand_binop_directly (enum insn_code icode, > > > machine_mode mode, optab binoptab, > > >commutative_p = commutative_optab_p (binoptab); > > >if (commutative_p > > >&& GET_MODE (xop0) != xmode0 && GET_MODE (xop1) != xmode1 > > > - && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode1) > > > + && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode0) > > > > Do we need 4 comparisons for that? Wouldn't > > if (commutative_p > > && xmode0 != xmode1 > > && GET_MODE (xop0) == xmode1 > > && GET_MODE (xop1) == xmode0) > > std::swap (xop0, xop1); > > be more readable and shorter? > > Not sure - xmode[01] are derived from insn_data while xop[01] are > passed in as arguments so it seems the code wants to check the > modes are not fine already, not whether swapping is a no-op > because both are equal. I meant that the condition after your changes is actually functionally equivalent to the one I've posted. Because, if xmode0 == xmode1, then the condition will never be true, as either GET_MODE (xop0) == xmode1 or GET_MODE (xop0) != xmode0 will be false. And if xmode0 != xmode1, we have the two == comparisons, so no need to check further that it is unequal to something else. Jakub
Re: [PATCH] gcc/gcc.c: add outfiles spec
> On 15 Apr 2020, at 11:41, Rasmus Villemoes wrote:
> On a somewhat related note, we're currently carrying this locally in
> order to build (link) the vxworks kernel image itself using gcc:
>
> diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h
> index 0f604f1bcc4..06b4e2e6870 100644
> --- a/gcc/config/vxworks.h
> +++ b/gcc/config/vxworks.h
> @@ -112,7 +112,7 @@ along with GCC; see the file COPYING3. If not see
>will treat it as an unrecognized option. */
> #undef VXWORKS_LINK_SPEC
> #define VXWORKS_LINK_SPEC \
> -"%{!mrtp:-r} \
> +" \
> %{!shared:\
>%{mrtp:-q %{h*} \
> %{R*} %{!T*: %(link_start) } \
>
> and then module builds must pass -r [or perhaps -Wl,-r depending on how
> the above gets solved] explicitly. From the link above, it sounds like
> you are already passing -r explicitly and not relying on lack of -mrtp
> implying it, so would the above be acceptable for upstream (with proper
> changelog and whitespace cleanup of course)?
This is a sensitive area and, while I understand the rationale,
I think we'd first need to experiment quite a bit in-house to go
with such a change.
As I mentioned in the thread referenced a couple of messages up,
the issue is not so much our own uses of the toolchain but assumptions
by possible invocations through IDE and Makefiles part of the
VxWorks environment.
I'll discuss with the team here.
Olivier
[PATCH] Fix -fcompare-debug issue in delete_insn_and_edges [PR94618]
Hi!
delete_insn_and_edges calls purge_dead_edges whenever deleting the last insn
in a bb, whatever it is. If it called it only for mandatory last insns
in the basic block (that may not be followed by DEBUG_INSNs, dunno if that
is control_flow_insn_p or something more complex), that wouldn't be a
problem, but as it calls it on any last insn and can actually do something
in the bb, if such an insn is followed by one more more DEBUG_INSNs and
nothing else in the same bb, we don't call purge_dead_edges with -g and do
call it with -g0.
On the testcase, there are two reg-to-reg moves with REG_EH_REGION notes
(previously memory accesses but simplified and yet not optimized), and the
second is followed by DEBUG_INSNs; the second move is delete_insn_and_edges
and after removing it, for -g0 purge_dead_edges removes the REG_EH_REGION
from the now last insn in the bb (the first reg-to-reg move), while
for -g it isn't called and things diverge from that quickly on.
Fixed by calling purdge_dead_edges even if we remove the last real insn
followed only by DEBUG_INSNs in the same bb.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2020-04-17 Jakub Jelinek
PR rtl-optimization/94618
* cfgrtl.c (delete_insn_and_edges): Set purge not just when
insn is the BB_END of its block, but also when it is only followed
by DEBUG_INSNs in its block.
* g++.dg/opt/pr94618.C: New test.
--- gcc/cfgrtl.c.jj 2020-01-12 11:54:36.218416351 +0100
+++ gcc/cfgrtl.c2020-04-16 17:28:00.792849169 +0200
@@ -230,10 +230,20 @@ delete_insn_and_edges (rtx_insn *insn)
{
bool purge = false;
- if (INSN_P (insn)
- && BLOCK_FOR_INSN (insn)
- && BB_END (BLOCK_FOR_INSN (insn)) == insn)
-purge = true;
+ if (INSN_P (insn) && BLOCK_FOR_INSN (insn))
+{
+ basic_block bb = BLOCK_FOR_INSN (insn);
+ if (BB_END (bb) == insn)
+ purge = true;
+ else if (DEBUG_INSN_P (BB_END (bb)))
+ for (rtx_insn *dinsn = NEXT_INSN (insn);
+DEBUG_INSN_P (dinsn); dinsn = NEXT_INSN (dinsn))
+ if (BB_END (bb) == dinsn)
+ {
+ purge = true;
+ break;
+ }
+}
delete_insn (insn);
if (purge)
return purge_dead_edges (BLOCK_FOR_INSN (insn));
--- gcc/testsuite/g++.dg/opt/pr94618.C.jj 2020-04-16 17:32:45.891637523
+0200
+++ gcc/testsuite/g++.dg/opt/pr94618.C 2020-04-16 17:32:22.715979882 +0200
@@ -0,0 +1,25 @@
+// PR rtl-optimization/94618
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2 -fnon-call-exceptions -fcompare-debug" }
+
+struct S
+{
+ int a, b, c;
+ int foo () noexcept { return a; }
+ int bar () noexcept { return b; }
+ void baz (int);
+ void qux () { if (c) for (int x = foo (); x != bar (); ) baz (x); }
+};
+
+struct T
+{
+ S s;
+ void foo ();
+};
+
+void
+T::foo ()
+{
+ s.qux ();
+ s.qux ();
+}
Jakub
Re: [PATCH] Fix a few of the PVS studio reported bugs
On Fri, 17 Apr 2020, Jakub Jelinek wrote: > On Fri, Apr 17, 2020 at 09:24:34AM +0200, Richard Biener wrote: > > --- a/gcc/optabs.c > > +++ b/gcc/optabs.c > > @@ -1050,7 +1050,7 @@ expand_binop_directly (enum insn_code icode, > > machine_mode mode, optab binoptab, > >commutative_p = commutative_optab_p (binoptab); > >if (commutative_p > >&& GET_MODE (xop0) != xmode0 && GET_MODE (xop1) != xmode1 > > - && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode1) > > + && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode0) > > Do we need 4 comparisons for that? Wouldn't > if (commutative_p > && xmode0 != xmode1 > && GET_MODE (xop0) == xmode1 > && GET_MODE (xop1) == xmode0) > std::swap (xop0, xop1); > be more readable and shorter? Not sure - xmode[01] are derived from insn_data while xop[01] are passed in as arguments so it seems the code wants to check the modes are not fine already, not whether swapping is a no-op because both are equal. Thus I went for the obvious change at this point. Richard.
[committed] inliner: Don't ICE on NULL TYPE_DOMAIN [PR94621]
Hi!
When I've added the VLA tweak for OpenMP to avoid error_mark_nodes in the IL in
type, I forgot that TYPE_DOMAIN could be NULL. Furthermore, as an optimization,
this patch checks the hopefully cheapest condition that is very likely false
most of the time (enabled only during OpenMP handling) first.
Bootstrapped/regtested on x86_64-linux and i686-linux, acked by Richard in
the PR, committed to trunk so far.
2020-04-17 Jakub Jelinek
PR tree-optimization/94621
* tree-inline.c (remap_type_1): Don't dereference NULL TYPE_DOMAIN.
Move id->adjust_array_error_bounds check first in the condition.
* gcc.c-torture/compile/pr94621.c: New test.
--- gcc/tree-inline.c.jj2020-03-16 09:03:32.776171761 +0100
+++ gcc/tree-inline.c 2020-04-16 17:57:05.319059075 +0200
@@ -556,8 +556,9 @@ remap_type_1 (tree type, copy_body_data
/* For array bounds where we have decided not to copy over the bounds
variable which isn't used in OpenMP/OpenACC region, change them to
an uninitialized VAR_DECL temporary. */
- if (TYPE_MAX_VALUE (TYPE_DOMAIN (new_tree)) == error_mark_node
- && id->adjust_array_error_bounds
+ if (id->adjust_array_error_bounds
+ && TYPE_DOMAIN (new_tree)
+ && TYPE_MAX_VALUE (TYPE_DOMAIN (new_tree)) == error_mark_node
&& TYPE_MAX_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
{
tree v = create_tmp_var (TREE_TYPE (TYPE_DOMAIN (new_tree)));
--- gcc/testsuite/gcc.c-torture/compile/pr94621.c.jj2020-04-16
17:54:48.924077393 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr94621.c 2020-04-16
17:54:33.774301572 +0200
@@ -0,0 +1,16 @@
+/* PR tree-optimization/94621 */
+
+struct S { int c, e[]; };
+
+static inline int
+foo (struct S *m, int r, int c)
+{
+ int (*a)[][m->c] = (int (*)[][m->c])>e;
+ return (*a)[r][c];
+}
+
+void
+bar (struct S *a)
+{
+ foo (a, 0, 0);
+}
Jakub
Re: [PATCH] Fix a few of the PVS studio reported bugs
On Fri, Apr 17, 2020 at 09:24:34AM +0200, Richard Biener wrote: > --- a/gcc/optabs.c > +++ b/gcc/optabs.c > @@ -1050,7 +1050,7 @@ expand_binop_directly (enum insn_code icode, > machine_mode mode, optab binoptab, >commutative_p = commutative_optab_p (binoptab); >if (commutative_p >&& GET_MODE (xop0) != xmode0 && GET_MODE (xop1) != xmode1 > - && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode1) > + && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode0) Do we need 4 comparisons for that? Wouldn't if (commutative_p && xmode0 != xmode1 && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode0) std::swap (xop0, xop1); be more readable and shorter? Jakub
[PATCH] Fix a few of the PVS studio reported bugs
This fixes the obvious (to me) ones. Bootstrap & regtest running on x86_64-unknown-linux-gnu. Richard. 2020-04-17 Richard Biener PR other/94629 * cgraphclones.c (cgraph_node::create_clone): Remove duplicate initialization. * dwarf2out.c (dw_val_equal_p): Fix pasto in dw_val_class_vms_delta comparison. * optabs.c (expand_binop_directly): Fix pasto in commutation check. * tree-ssa-sccvn.c (vn_reference_lookup_pieces): Fix pasto in initialization. --- gcc/cgraphclones.c | 1 - gcc/dwarf2out.c | 2 +- gcc/optabs.c | 2 +- gcc/tree-ssa-sccvn.c | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index c73b8f810f0..832471da8df 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -403,7 +403,6 @@ cgraph_node::create_clone (tree new_decl, profile_count prof_count, new_node->tp_first_run = tp_first_run; new_node->tm_clone = tm_clone; new_node->icf_merged = icf_merged; - new_node->merged_comdat = merged_comdat; new_node->thunk = thunk; new_node->unit_id = unit_id; new_node->merged_comdat = merged_comdat; diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 7d270f2a1b5..787340e9279 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -1479,7 +1479,7 @@ dw_val_equal_p (dw_val_node *a, dw_val_node *b) case dw_val_class_vms_delta: return (!strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1) - && !strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1)); + && !strcmp (a->v.val_vms_delta.lbl2, b->v.val_vms_delta.lbl2)); case dw_val_class_discr_value: return (a->v.val_discr_value.pos == b->v.val_discr_value.pos diff --git a/gcc/optabs.c b/gcc/optabs.c index 8dd351286cd..1456e59ffa7 100644 --- a/gcc/optabs.c +++ b/gcc/optabs.c @@ -1050,7 +1050,7 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab, commutative_p = commutative_optab_p (binoptab); if (commutative_p && GET_MODE (xop0) != xmode0 && GET_MODE (xop1) != xmode1 - && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode1) + && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode0) std::swap (xop0, xop1); /* If we are optimizing, force expensive constants into a register. */ diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c index fd74809ca6e..238931d3f3e 100644 --- a/gcc/tree-ssa-sccvn.c +++ b/gcc/tree-ssa-sccvn.c @@ -3446,7 +3446,7 @@ vn_reference_lookup_pieces (tree vuse, alias_set_type set, = valueize_refs (shared_lookup_references); vr1.type = type; vr1.set = set; - vr1.set = base_set; + vr1.base_set = base_set; vr1.hashcode = vn_reference_compute_hash (); if ((cst = fully_constant_vn_reference_p ())) return cst; -- 2.13.7
Re: [PATCH] Do not use HAVE_DOS_BASED_FILE_SYSTEM for Cygwin.
On 4/16/20 6:10 PM, Maciej W. Rozycki wrote: On Thu, 16 Apr 2020, Martin Li?ka wrote: The patch is fix for Cygwin where we should not define HAVE_DOS_BASED_FILE_SYSTEM and use back slashes as a path component separator. [...] diff --git a/ltmain.sh b/ltmain.sh index 79f9ba89af5..8ad183010f0 100644 --- a/ltmain.sh +++ b/ltmain.sh @@ -3425,7 +3425,7 @@ int setenv (const char *, const char *, int); # define PATH_SEPARATOR ':' #endif -#if defined (_WIN32) || defined (__MSDOS__) || defined (__DJGPP__) || \ +#if (defined (_WIN32) && ! defined(__CYGWIN__)) || defined (__MSDOS__) || defined (__DJGPP__) || \ defined (__OS2__) # define HAVE_DOS_BASED_FILE_SYSTEM # define FOPEN_WB "wb" This part needs to go upstream so as to let us avoid local clutter. Also this does not fit in 80 columns and has to be reformatted. Maciej Hi. I've just installed the patch (including the 80 columns limit fix). Martin
Re: [PATCH] Allow new/delete operator deletion only for replaceable.
On Tue, Apr 14, 2020 at 12:54:17PM +0200, Martin Liška wrote:
> On 4/14/20 10:37 AM, Jakub Jelinek wrote:
> > On Tue, Apr 14, 2020 at 09:11:37AM +0200, Martin Liška wrote:
> > > +/* PR c++/94314. */
> > > +/* { dg-do run } */
> > > +/* { dg-options "-O2 -fdump-tree-cddce-details -std=c++14" } */
> > > +/* { dg-additional-options "-fdelete-null-pointer-checks" } */
> >
> > Any reason why you want to do it for -std=c++14 only?
>
> I need at least -std=c++11 for the noexcept keyword. I updated that.
With c++11 one gets:
Excess errors:
.../testsuite/g++.dg/pr94314-4.C:19:28: error: too many arguments to function
'void operator delete(void*)'
because C++ sized deallocation is a C++14 feature.
One needs to use check-c++-all or GXX_TESTSUITE_STDS=98,11,14,17,2a make check
or similar to get that though, because 11 isn't tested by default, only 98,
14 and 17 are ATM I think.
Fixed thusly, committed to trunk as obvious.
Note for next time, there shouldn't be any tests directly in g++.dg/
which has subdirectories, so optimization related tests should go into
g++.dg/opt/ etc.
2020-04-17 Jakub Jelinek
PR c++/94314
* g++.dg/pr94314-4.C: Require c++14 rather than c++11.
--- gcc/testsuite/g++.dg/pr94314-4.C.jj 2020-04-17 08:49:49.129682532 +0200
+++ gcc/testsuite/g++.dg/pr94314-4.C2020-04-17 08:57:24.836861755 +0200
@@ -1,5 +1,5 @@
/* PR c++/94314. */
-/* { dg-do run { target c++11 } } */
+/* { dg-do run { target c++14 } } */
/* { dg-options "-O2 -fdump-tree-cddce-details -fdelete-null-pointer-checks" }
*/
int count = 0;
Jakub
Re: [PATCH] Do not modify tab options in vimrc for .py files.
On 4/16/20 4:16 PM, Alexander Monakov wrote: On Thu, 16 Apr 2020, Martin Liška wrote: To be honest I have: autocmd Filetype python setlocal expandtab tabstop=4 shiftwidth=4 softtabstop=4 in my default vim config. But I'm wondering what's default for 'python' Filetype? Since October 2013 Vim ftplugin/python.vim has: " As suggested by PEP8. setlocal expandtab shiftwidth=4 softtabstop=4 tabstop=8 Fine! That's what I expected. So the default is correct. Please disregard my suggestion then, no need to add an 'else' branch there. Thank you for the searching of it. Martin Thanks. Alexander
Re: [RFC] split pseudos during loop unrolling in RTL unroller
On Thu, Apr 16, 2020 at 7:46 PM Segher Boessenkool wrote: > > On Thu, Apr 16, 2020 at 10:33:45AM +0200, Richard Biener wrote: > > On Wed, Apr 15, 2020 at 11:23 PM Segher Boessenkool > > > On a general note, we shouldn't depend on some pass that may or may not > > > clean up the mess we make, when we could just avoid making a mess in the > > > first place. > > > > True - but the issue at hand is not trivial given you have to care for > > partial defs, uses outside of the loop (or across the backedge), etc. > > So there's plenty of things to go "wrong" here. > > Certainly. But *all* RTL passes before RA should leave things in "web > form" (compare to SSA form). The code in web.c is probably just fine; > but we shouldn't have one web pass, *all* passes should leave things in > a useful form! Yeah well, but RTL is not in SSA form and there's no RTL IL verification in place to track degradation. And we even work in the opposite way when expanding to RTL from SSA, coalescing as much as we can ... > > > The web pass belongs immediately after expand; but ideally, even expand > > > would not reuse pseudos anyway. > > > > But for example when lower-subreg decomposes things in a way turning > > partial defs into full defs new opportunities to split the web arise. > > But that is only for the new registers it creates, or what am I missing? No idea, just made up the example that maintaing "SSA" RTL is not automagic. > > > Maybe it would be better as some utility routines, not a pass? > > > > Sure, but then when do we apply it? > > All of the time! Ideally every pass would leave things in a good shape. > Usually it is cheapest as well as easiest to do things manually, but for > some harder cases, such helper routines can be used. > > > Ideally scheduling would to > > register renaming itself and thus not rely on the used pseudos > > (I'm not sure if it tracks false dependences - I guess it must if it > > isn't able to rename regs). That would be a much better place > > for improvements? > > sched2 runs after RA, so it has nothing to do with webs? And sched1 > doesn't do much relevant here (it doesn't move insns much). > > I don't see how this is directly related to register renaming either? If scheduling ignores "false" dependences (anti dependence with full defs) then when it schedules across such defs needs to perform renaming. Maybe I'm using bogus terms here. Richard. > > Segher
Re: [PATCH] reject scalar array initialization with nullptr [PR94510]
On 4/15/20 1:30 PM, Martin Sebor wrote:
On 4/13/20 8:43 PM, Jason Merrill wrote:
On 4/12/20 5:49 PM, Martin Sebor wrote:
On 4/10/20 8:52 AM, Jason Merrill wrote:
On 4/9/20 4:23 PM, Martin Sebor wrote:
On 4/9/20 1:32 PM, Jason Merrill wrote:
On 4/9/20 3:24 PM, Martin Sebor wrote:
On 4/9/20 1:03 PM, Jason Merrill wrote:
On 4/8/20 1:23 PM, Martin Sebor wrote:
On 4/7/20 3:36 PM, Marek Polacek wrote:
On Tue, Apr 07, 2020 at 02:46:52PM -0600, Martin Sebor wrote:
On 4/7/20 1:50 PM, Marek Polacek wrote:
On Tue, Apr 07, 2020 at 12:50:48PM -0600, Martin Sebor via
Gcc-patches wrote:
Among the numerous regressions introduced by the change
committed
to GCC 9 to allow string literals as template arguments is
a failure
to recognize the C++ nullptr and GCC's __null constants as
pointers.
For one, I didn't realize that nullptr, being a null
pointer constant,
doesn't have a pointer type, and two, I didn't think of
__null (which
is a special integer constant that NULL sometimes expands to).
The attached patch adjusts the special handling of trailing
zero
initializers in reshape_init_array_1 to recognize both
kinds of
constants and avoid treating them as zeros of the array
integer
element type. This restores the expected diagnostics when
either
constant is used in the initializer list.
Martin
PR c++/94510 - nullptr_t implicitly cast to zero twice in
std::array
gcc/cp/ChangeLog:
PR c++/94510
* decl.c (reshape_init_array_1): Exclude mismatches
with all kinds
of pointers.
gcc/testsuite/ChangeLog:
PR c++/94510
* g++.dg/init/array57.C: New test.
* g++.dg/init/array58.C: New test.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index a127734af69..692c8ed73f4 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6041,9 +6041,14 @@ reshape_init_array_1 (tree elt_type,
tree max_index, reshape_iter *d,
TREE_CONSTANT (new_init) = false;
/* Pointers initialized to strings must be treated
as non-zero
- even if the string is empty. */
+ even if the string is empty. Handle all kinds of
pointers,
+ including std::nullptr and GCC's __nullptr, neither
of which
+ has a pointer type. */
tree init_type = TREE_TYPE (elt_init);
- if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P
(init_type)
+ bool init_is_ptr = (POINTER_TYPE_P (init_type)
+ || NULLPTR_TYPE_P (init_type)
+ || null_node_p (elt_init));
+ if (POINTER_TYPE_P (elt_type) != init_is_ptr
|| !type_initializer_zero_p (elt_type, elt_init))
last_nonzero = index;
It looks like this still won't handle e.g. pointers to
member functions,
e.g.
struct S { };
int arr[3] = { (void (S::*) ()) 0, 0, 0 };
would still be accepted. You could use TYPE_PTR_OR_PTRMEM_P
instead of
POINTER_TYPE_P to catch this case.
Good catch! That doesn't fail because unlike null data
member pointers
which are represented as -1, member function pointers are
represented
as a zero.
I had looked for an API that would answer the question: "is this
expression a pointer?" without having to think of all the
different
kinds of them but all I could find was null_node_p(). Is
this a rare,
isolated case that having an API like that wouldn't be worth
having
or should I add one like in the attached update?
Martin
PR c++/94510 - nullptr_t implicitly cast to zero twice in
std::array
gcc/cp/ChangeLog:
PR c++/94510
* decl.c (reshape_init_array_1): Exclude mismatches with
all kinds
of pointers.
* gcc/cp/cp-tree.h (null_pointer_constant_p): New function.
(Drop the gcc/cp/.)
+/* Returns true if EXPR is a null pointer constant of any
type. */
+
+inline bool
+null_pointer_constant_p (tree expr)
+{
+ STRIP_ANY_LOCATION_WRAPPER (expr);
+ if (expr == null_node)
+ return true;
+ tree type = TREE_TYPE (expr);
+ if (NULLPTR_TYPE_P (type))
+ return true;
+ if (POINTER_TYPE_P (type))
+ return integer_zerop (expr);
+ return null_member_pointer_value_p (expr);
+}
+
We already have a null_ptr_cst_p so it would be sort of
confusing to have
this as well. But are you really interested in whether it's a
null pointer,
not just a pointer?
The goal of the code is to detect a mismatch in "pointerness"
between
an initializer expression and the type of the initialized
element, so
it needs to know if the expression is a pointer (non-nulls
pointers
are detected in type_initializer_zero_p). That means testing a
number
of IMO unintuitive conditions:
TYPE_PTR_OR_PTRMEM_P (TREE_TYPE (expr))
|| NULLPTR_TYPE_P (TREE_TYPE (expr))
|| null_node_p (expr)
I don't know if this type of a query is common in the C++ FE
but unless
this is an isolated use case then besides fixing the bug I
thought it
would be nice to make it easier to get the test above right, or
at least
come close to it.
Since null_pointer_constant_p already exists (but isn't
suitable here
because it returns true for plain literal zeros)
Why is that
