Re: [PATCH 1/9] PowerPC: Map long double built-in functions if IEEE 128-bit long double.
On Thu, Oct 01, 2020 at 11:05:04PM +, Joseph Myers wrote: > On Thu, 24 Sep 2020, Michael Meissner via Gcc-patches wrote: > > > To map the math functions, typically this patch changes l to > > f128. > > However there are some exceptions that are handled with this patch. > > glibc 2.32 added __*ieee128 names for the *f128 functions, to allow the > long double functions to be called in a namespace-clean way (when the > defined feature test macros do not enable the TS 18661-3 function names). > So I think GCC should also prefer to map to those names where possible, > rather than the *f128 names. Good idea. Thanks. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Attempt to resolve PR83562
[Please CC me as I am not subscribed to this list.]
[This patch is only a draft and has not been tested at all.]
Some details have been discussed in [1]. mingw-w64 has got an implementation
[2] [3] for static libraries, but it takes a
destructor using the `__thiscall` convention. As both functions have C linkage,
they should agree with each other and should
behave simliarily.
Considerations:
0) This is going to be an ABI breakage for i?86. I am not sure whether people
would expect the old, broken behavior.
1) GCC doesn't provide `__cxa_atexit()`, but it would suffer from the same
problem. At the moment GCC calls `atexit()` to
register destructors so it appears to work.
2) There is an explicit reference to `__cxa_atexit` in 'gcc/cp/decl.c' and it
probably be changed, unless it is not used by
1?86-w64-mingw32 targets.
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83562
[2]
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-crt/crt/cxa_thread_atexit.c
[3]
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/f3e0fbb40cbc9f8821db8bd8a0c4dae8ff671e9f/
[4] https://github.com/msys2/MINGW-packages/issues/7071
--
Best regards,
LH_Mouse
From ac325bdcd6e3fa522f8b59d436cd4b3ab663de5c Mon Sep 17 00:00:00 2001
From: Liu Hao
Date: Thu, 8 Oct 2020 10:26:13 +0800
Subject: [PATCH] libsupc++: Make the destructor parameter to
`__cxa_thread_atexit()` use the `__thiscall` calling convention for
i686-w64-mingw32
The mingw-w64 implementations of `__cxa_thread_atexit()` and `__cxa_atexit()` have been
using `__thiscall` since two years ago. Using the default calling convention (which is
`__cdecl`) causes crashes as explained in PR83562.
Calling conventions have no effect on x86_64-w64-mingw32.
Reference: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83562
Reference: https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-crt/crt/cxa_thread_atexit.c
Reference: https://sourceforge.net/p/mingw-w64/mingw-w64/ci/f3e0fbb40cbc9f8821db8bd8a0c4dae8ff671e9f/
Reference: https://github.com/msys2/MINGW-packages/issues/7071
Signed-off-by: Liu Hao
---
libstdc++-v3/libsupc++/atexit_thread.cc | 14 ++
libstdc++-v3/libsupc++/cxxabi.h | 8
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/libstdc++-v3/libsupc++/atexit_thread.cc b/libstdc++-v3/libsupc++/atexit_thread.cc
index e97644f8cd4..4f7f982ac6b 100644
--- a/libstdc++-v3/libsupc++/atexit_thread.cc
+++ b/libstdc++-v3/libsupc++/atexit_thread.cc
@@ -30,16 +30,21 @@
#include
#endif
+// Simplify it a little for this file.
+#ifndef _GLIBCXX_CDTOR_CALLABI
+# define _GLIBCXX_CDTOR_CALLABI
+#endif
+
#if _GLIBCXX_HAVE___CXA_THREAD_ATEXIT
// Libc provides __cxa_thread_atexit definition.
#elif _GLIBCXX_HAVE___CXA_THREAD_ATEXIT_IMPL
-extern "C" int __cxa_thread_atexit_impl (void (*func) (void *),
+extern "C" int __cxa_thread_atexit_impl (void (_GLIBCXX_CDTOR_CALLABI *func) (void *),
void *arg, void *d);
extern "C" int
-__cxxabiv1::__cxa_thread_atexit (void (*dtor)(void *),
+__cxxabiv1::__cxa_thread_atexit (void (_GLIBCXX_CDTOR_CALLABI *dtor)(void *),
void *obj, void *dso_handle)
_GLIBCXX_NOTHROW
{
@@ -52,7 +57,7 @@ namespace {
// One element in a singly-linked stack of cleanups.
struct elt
{
-void (*destructor)(void *);
+void (_GLIBCXX_CDTOR_CALLABI *destructor)(void *);
void *object;
elt *next;
#ifdef _GLIBCXX_THREAD_ATEXIT_WIN32
@@ -116,7 +121,8 @@ namespace {
}
extern "C" int
-__cxxabiv1::__cxa_thread_atexit (void (*dtor)(void *), void *obj, void */*dso_handle*/)
+__cxxabiv1::__cxa_thread_atexit (void (_GLIBCXX_CDTOR_CALLABI *dtor)(void *),
+ void *obj, void */*dso_handle*/)
_GLIBCXX_NOTHROW
{
// Do this initialization once.
diff --git a/libstdc++-v3/libsupc++/cxxabi.h b/libstdc++-v3/libsupc++/cxxabi.h
index 000713ecdf8..3d6217a6fac 100644
--- a/libstdc++-v3/libsupc++/cxxabi.h
+++ b/libstdc++-v3/libsupc++/cxxabi.h
@@ -125,14 +125,22 @@ namespace __cxxabiv1
// DSO destruction.
int
+#ifdef _GLIBCXX_CDTOR_CALLABI
+ __cxa_atexit(void (_GLIBCXX_CDTOR_CALLABI *)(void*), void*, void*) _GLIBCXX_NOTHROW;
+#else
__cxa_atexit(void (*)(void*), void*, void*) _GLIBCXX_NOTHROW;
+#endif
void
__cxa_finalize(void*);
// TLS destruction.
int
+#ifdef _GLIBCXX_CDTOR_CALLABI
+ __cxa_thread_atexit(void (_GLIBCXX_CDTOR_CALLABI *)(void*), void*, void *) _GLIBCXX_NOTHROW;
+#else
__cxa_thread_atexit(void (*)(void*), void*, void *) _GLIBCXX_NOTHROW;
+#endif
// Pure virtual functions.
void
--
2.20.1
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v7] genemit.c (main): split insn-emit.c for compiling parallelly
Ping …...
Jojo
在 2020年9月27日 +0800 AM10:34，Jojo R ，写道：
> Hi,
>
> Has this patch been merged ?
>
> Jojo
> 在 2020年9月15日 +0800 PM5:16，Jojo R ，写道：
> > gcc/ChangeLog:
> >
> > * genemit.c (main): Print 'split line'.
> > * Makefile.in (insn-emit.c): Define split count and file
> >
> > ---
> > gcc/Makefile.in | 19 +
> > gcc/genemit.c | 104 +---
> > 2 files changed, 83 insertions(+), 40 deletions(-)
> >
> > diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> > index 79e854aa938..a7fcc7d5949 100644
> > --- a/gcc/Makefile.in
> > +++ b/gcc/Makefile.in
> > @@ -1258,6 +1258,21 @@ ANALYZER_OBJS = \
> > # We put the *-match.o and insn-*.o files first so that a parallel make
> > # will build them sooner, because they are large and otherwise tend to be
> > # the last objects to finish building.
> > +
> > +# target overrides
> > +-include $(tmake_file)
> > +
> > +INSN-GENERATED-SPLIT-NUM ?= 0
> > +
> > +insn-generated-split-num = $(shell i=1; j=`expr
> > $(INSN-GENERATED-SPLIT-NUM) + 1`; \
> > + while test $$i -le $$j; do \
> > + echo $$i; i=`expr $$i + 1`; \
> > + done)
> > +
> > +insn-emit-split-c := $(foreach o, $(shell for i in
> > $(insn-generated-split-num); do echo $$i; done), insn-emit$(o).c)
> > +insn-emit-split-obj = $(patsubst %.c,%.o, $(insn-emit-split-c))
> > +$(insn-emit-split-c): insn-emit.c
> > +
> > OBJS = \
> > gimple-match.o \
> > generic-match.o \
> > @@ -1265,6 +1280,7 @@ OBJS = \
> > insn-automata.o \
> > insn-dfatab.o \
> > insn-emit.o \
> > + $(insn-emit-split-obj) \
> > insn-extract.o \
> > insn-latencytab.o \
> > insn-modes.o \
> > @@ -2365,6 +2381,9 @@ $(simple_generated_c:insn-%.c=s-%): s-%:
> > build/gen%$(build_exeext)
> > $(RUN_GEN) build/gen$*$(build_exeext) $(md_file) \
> > $(filter insn-conditions.md,$^) > tmp-$*.c
> > $(SHELL) $(srcdir)/../move-if-change tmp-$*.c insn-$*.c
> > + $*v=$$(echo $$(csplit insn-$*.c /parallel\ compilation/ -k -s
> > {$(INSN-GENERATED-SPLIT-NUM)} -f insn-$* -b "%d.c" 2>&1));\
> > + [ ! "$$$*v" ] || grep "match not found" <<< $$$*v
> > + [ -s insn-$*0.c ] || (for i in $(insn-generated-split-num); do touch
> > insn-$*$$i.c; done && echo "" > insn-$*.c)
> > $(STAMP) s-$*
> >
> > # gencheck doesn't read the machine description, and the file produced
> > diff --git a/gcc/genemit.c b/gcc/genemit.c
> > index 84d07d388ee..54a0d909d9d 100644
> > --- a/gcc/genemit.c
> > +++ b/gcc/genemit.c
> > @@ -847,24 +847,13 @@ handle_overloaded_gen (overloaded_name *oname)
> > }
> > }
> >
> > -int
> > -main (int argc, const char **argv)
> > -{
> > - progname = "genemit";
> > -
> > - if (!init_rtx_reader_args (argc, argv))
> > - return (FATAL_EXIT_CODE);
> > -
> > -#define DEF_INTERNAL_OPTAB_FN(NAME, FLAGS, OPTAB, TYPE) \
> > - nofail_optabs[OPTAB##_optab] = true;
> > -#include "internal-fn.def"
> > -
> > - /* Assign sequential codes to all entries in the machine description
> > - in parallel with the tables in insn-output.c. */
> > -
> > - printf ("/* Generated automatically by the program `genemit'\n\
> > -from the machine description file `md'. */\n\n");
> > +/* Print include header. */
> >
> > +static void
> > +printf_include (void)
> > +{
> > + printf ("/* Generated automatically by the program `genemit'\n"
> > + "from the machine description file `md'. */\n\n");
> > printf ("#define IN_TARGET_CODE 1\n");
> > printf ("#include \"config.h\"\n");
> > printf ("#include \"system.h\"\n");
> > @@ -900,35 +889,70 @@ from the machine description file `md'. */\n\n");
> > printf ("#include \"tm-constrs.h\"\n");
> > printf ("#include \"ggc.h\"\n");
> > printf ("#include \"target.h\"\n\n");
> > +}
> >
> > - /* Read the machine description. */
> > +/* Generate the `gen_...' function from GET_CODE(). */
> >
> > - md_rtx_info info;
> > - while (read_md_rtx ())
> > - switch (GET_CODE (info.def))
> > - {
> > - case DEFINE_INSN:
> > - gen_insn ();
> > - break;
> > +static void
> > +gen_md_rtx (md_rtx_info *info)
> > +{
> > + switch (GET_CODE (info->def))
> > + {
> > + case DEFINE_INSN:
> > + gen_insn (info);
> > + break;
> >
> > - case DEFINE_EXPAND:
> > - printf ("/* %s:%d */\n", info.loc.filename, info.loc.lineno);
> > - gen_expand ();
> > - break;
> > + case DEFINE_EXPAND:
> > + printf ("/* %s:%d */\n", info->loc.filename, info->loc.lineno);
> > + gen_expand (info);
> > + break;
> >
> > - case DEFINE_SPLIT:
> > - printf ("/* %s:%d */\n", info.loc.filename, info.loc.lineno);
> > - gen_split ();
> > - break;
> > + case DEFINE_SPLIT:
> > + printf ("/* %s:%d */\n", info->loc.filename, info->loc.lineno);
> > + gen_split (info);
> > + break;
> >
> > - case DEFINE_PEEPHOLE2:
> > - printf ("/* %s:%d */\n", info.loc.filename, info.loc.lineno);
> > - gen_split ();
> > - break;
> > + case DEFINE_PEEPHOLE2:
> > + printf ("/* %s:%d */\n", info->loc.filename, info->loc.lineno);
> > + gen_split (info);
> > + break;
> >
> > - default:
> > - break;
> > - }
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +int
> > +main (int argc, const char **argv)
[PATCH] libstdc++: Implement C++20 features for
From: Thomas Rodgers New ctors and ::view() accessor for - * basic_stingbuf * basic_istringstream * basic_ostringstream * basic_stringstreamm New ::get_allocator() accessor for basic_stringbuf. libstdc++-v3/ChangeLog: * acinclude.m4 (glibcxx_SUBDIRS): Add src/c++20. * config/abi/pre/gnu.ver: Update GLIBCXX_3.4.29 for the addition of - basic_stringbuf::basic_stringbuf(allocator const&), basic_stringbuf::basic_stringbuf(openmode, allocator const&), basic_stringbuf::basic_stringbuf(basic_string&&, openmode), basic_stringbuf::basic_stringbuf(basic_stringbuf&&, allocator const&), basic_stringbuf::get_allocator(), basic_stringbuf::view(), basic_istringstream::basic_istringstream(basic_string&&, openmode), basic_istringstream::basic_istringstream(openmode, allocator const&), basic_istringstream::view(), basic_ostringstream::basic_ostringstream(basic_string&&, openmode), basic_ostringstream::basic_ostringstream(openmode, allocator const&), basic_ostringstream::view(), basic_stringstream::basic_stringstream(basic_string&&, openmode), basic_stringstream::basic_stringstream(openmode, allocator const&), basic_stringstream::view(). * configure: Regenerate. * include/std/sstream: (basic_stringbuf::basic_stringbuf(allocator const&)): New constructor. (basic_stringbuf::basic_stringbuf(openmode, allocator const&)): Likewise. (basic_stringbuf::basic_stringbuf(basic_string&&, openmode)): Likewise. (basic_stringbuf::basic_stringbuf(basic_stringbuf&&, allocator const&)): Likewise. (basic_stringbuf::get_allocator()): New method. (basic_stringbuf::view()): Likewise. (basic_istringstream::basic_istringstream(basic_string&&, openmode)): New constructor. (basic_istringstream::basic_istringstream(openmode, allocator const&)): Likewise (basic_istringstream::view()): New method. (basic_ostringstream::basic_ostringstream(basic_string&&, openmode)): New constructor. (basic_ostringstream::basic_ostringstream(openmode, allocator const&)): Likewise (basic_ostringstream::view()): New method. (basic_stringstream::basic_stringstream(basic_string&&, openmode)): New constructor. (basic_stringstream::basic_stringstream(openmode, allocator const&)): Likewise (basic_stringstream::view()): New method. * src/Makefile.in: Add c++20 directory. * src/Makefile.am: Regenerate. * src/c++20/Makefile.am: Add makefile for new sub-directory. * src/c++20/Makefile.in: Generate. * src/c++20/sstream-inst.cc: New file defining explicit instantiations for basic_stringbuf, basic_istringstream, basic_ostringstream, and basic_stringstream member functions added in C++20. * testsuite/27_io/basic_stringbuf/cons/char/2.cc: New test. * testsuite/27_io/basic_stringbuf/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringbuf/view/char/2.cc: Likewise. * testsuite/27_io/basic_stringbuf/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_istringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_istringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_istringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_istringstream/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_stringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_stringstream/view/wchar_t/2.cc: Likewise. --- libstdc++-v3/acinclude.m4 | 2 +- libstdc++-v3/config/abi/pre/gnu.ver | 45 ++ libstdc++-v3/configure| 16 +- libstdc++-v3/include/std/sstream | 196 + libstdc++-v3/src/Makefile.am | 12 +- libstdc++-v3/src/Makefile.in | 14 +- libstdc++-v3/src/c++20/Makefile.am| 105 +++ libstdc++-v3/src/c++20/Makefile.in| 735 ++ libstdc++-v3/src/c++20/sstream-inst.cc| 111 +++ .../27_io/basic_istringstream/cons/char/1.cc | 85 ++ .../basic_istringstream/cons/wchar_t/1.cc | 85 ++ .../27_io/basic_istringstream/view/char/1.cc | 35 + .../basic_istringstream/view/wchar_t/1.cc | 35 + .../27_io/basic_ostringstream/cons/char/1.cc | 85 ++ .../basic_ostringstream/cons/wchar_t/1.cc | 85 ++
Re: [PATCH, rs6000] rename BU_P10_MISC_2 define to BU_P10_POWERPC64_MISC_2
Hi! On Wed, Oct 07, 2020 at 12:44:04PM -0500, will schmidt wrote: > Rename our BU_P10_MISC_2 built-in define macro to be > BU_P10_POWERPC64_MISC_2. This more accurately reflects > that the macro includes the RS6000_BTM_POWERPC64 entry > that is not present in the other BU_P10_MISC macros, > and matches the style we used for the P7 equivalent. > > Should be entirely cosmetic, no codegen changes. > A regtest is underway just in case. This is okay for trunk. Thank you! > * gcc/config/rs6000/rs6000-builtin.def (BU_P10_MISC_2): Rename > to BU_P10_POWERPC64_MISC_2. > (CFUGED,CNTLZDM,CNTTZDM,PDEPD,PEXTD): Call renamed macro. (space after comma?) Segher
Re: [PATCH] rs6000: Fix extraneous characters in the documentation
Hi! On Wed, Aug 19, 2020 at 04:03:31PM -0300, Tulio Magno Quites Machado Filho via Gcc-patches wrote: > Replace them with a whitespace in order to avoid artifacts in the HTML > document. Multiple whitespaces in texinfo are supposed to work correctly, and are good to have for various reasons (more readable, and agrees to our coding conventions, and it agrees to texinfo's output, too!) But as Will says, there seem the be non-breaking space here, instead? 000: 3e20 2d62 656c 6f77 2069 7320 6465 6669 > -below is defi 010: 6e65 6420 746f 2062 6520 6120 6e6f 726d ned to be a norm 020: 616c 2076 6563 746f 7220 756e 7369 676e al vector unsign 030: 6564 2063 6861 7220 7479 7065 2ec2 a020 ed char type... 040: 5468 6520 7569 6e74 322c 2075 696e 7434 The uint2, uint4 050: 0a . 000: 3e20 2b62 656c 6f77 2069 7320 6465 6669 > +below is defi 010: 6e65 6420 746f 2062 6520 6120 6e6f 726d ned to be a norm 020: 616c 2076 6563 746f 7220 756e 7369 676e al vector unsign 030: 6564 2063 6861 7220 7479 7065 2e20 2054 ed char type. T 040: 6865 2075 696e 7432 2c20 7569 6e74 340a he uint2, uint4. Yeah, utf8 c2 a0, aka u+00a0. Replacing that with an actual space is of course okay (and trivial and obvious). Thanks! (Bonus points if you do all of doc/*.texi :-) ) Segher
[committed] libstdc++: Fix divide by zero in default template argument
libstdc++-v3/ChangeLog:
* include/bits/random.h (__detail::_Mod): Avoid divide by zero.
*
testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc:
New test.
Tested powerpc64le-linux. Committed to trunk.
commit 6ae17a3b6835b30102607d45ac89c7a668e2c8d4
Author: Jonathan Wakely
Date: Thu Oct 8 00:34:56 2020
libstdc++: Fix divide by zero in default template argument
libstdc++-v3/ChangeLog:
* include/bits/random.h (__detail::_Mod): Avoid divide by zero.
*
testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc:
New test.
diff --git a/libstdc++-v3/include/bits/random.h
b/libstdc++-v3/include/bits/random.h
index 4a6558c966a..920f3d91513 100644
--- a/libstdc++-v3/include/bits/random.h
+++ b/libstdc++-v3/include/bits/random.h
@@ -109,7 +109,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template= __m - 1),
- bool __schrage_ok = __m % __a < __m / __a>
+ bool __schrage_ok = __a != 0 && __m % __a < __m / __a>
struct _Mod
{
typedef typename _Select_uint_least_thttp://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++11 } }
+
+#include
+
+unsigned
+test01()
+{
+ std::linear_congruential_engine l;
+ return l(); // this used to result in divide by zero
+}
[committed] libstdc++: Fix non-reserved names in headers
My recent changes to std::exception_ptr moved some members to be inline
in the header but didn't replace the variable names with reserved names.
The "tmp" variable must be fixed. The "other" parameter is actually a
reserved name because of std::allocator::rebind::other but should
be fixed anyway.
There are also some bad uses of "ForwardIterator" in .
There's also a "il" parameter in a std::seed_seq constructor in
which is only reserved since C++14.
libstdc++-v3/ChangeLog:
* include/bits/random.h (seed_seq(initializer_list)): Rename
parameter to use reserved name.
* include/bits/ranges_algo.h (shift_left, shift_right): Rename
template parameters to use reserved name.
* libsupc++/exception_ptr.h (exception_ptr): Likewise for
parameters and local variables.
* testsuite/17_intro/names.cc: Check "il". Do not check "d" and
"y" in C++20 mode.
Tested powerpc64le-linux. Committed to trunk.
commit 23f75da95f5e8e09e9fcbd5b0d2885e6c44739aa
Author: Jonathan Wakely
Date: Thu Oct 8 00:05:53 2020
libstdc++: Fix non-reserved names in headers
My recent changes to std::exception_ptr moved some members to be inline
in the header but didn't replace the variable names with reserved names.
The "tmp" variable must be fixed. The "other" parameter is actually a
reserved name because of std::allocator::rebind::other but should
be fixed anyway.
There are also some bad uses of "ForwardIterator" in .
There's also a "il" parameter in a std::seed_seq constructor in
which is only reserved since C++14.
libstdc++-v3/ChangeLog:
* include/bits/random.h (seed_seq(initializer_list)): Rename
parameter to use reserved name.
* include/bits/ranges_algo.h (shift_left, shift_right): Rename
template parameters to use reserved name.
* libsupc++/exception_ptr.h (exception_ptr): Likewise for
parameters and local variables.
* testsuite/17_intro/names.cc: Check "il". Do not check "d" and
"y" in C++20 mode.
diff --git a/libstdc++-v3/include/bits/random.h
b/libstdc++-v3/include/bits/random.h
index 19307fbc3ca..4a6558c966a 100644
--- a/libstdc++-v3/include/bits/random.h
+++ b/libstdc++-v3/include/bits/random.h
@@ -6063,7 +6063,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ }
template
- seed_seq(std::initializer_list<_IntType> il);
+ seed_seq(std::initializer_list<_IntType> __il);
template
seed_seq(_InputIterator __begin, _InputIterator __end);
diff --git a/libstdc++-v3/include/bits/ranges_algo.h
b/libstdc++-v3/include/bits/ranges_algo.h
index 61673e38fa6..f1a4cc24c0d 100644
--- a/libstdc++-v3/include/bits/ranges_algo.h
+++ b/libstdc++-v3/include/bits/ranges_algo.h
@@ -3696,10 +3696,10 @@ namespace ranges
} // namespace ranges
#define __cpp_lib_shift 201806L
- template
-constexpr ForwardIterator
-shift_left(ForwardIterator __first, ForwardIterator __last,
- typename iterator_traits::difference_type __n)
+ template
+constexpr _ForwardIterator
+shift_left(_ForwardIterator __first, _ForwardIterator __last,
+ typename iterator_traits<_ForwardIterator>::difference_type __n)
{
__glibcxx_assert(__n >= 0);
if (__n == 0)
@@ -3711,16 +3711,17 @@ namespace ranges
return std::move(std::move(__mid), std::move(__last),
std::move(__first));
}
- template
-constexpr ForwardIterator
-shift_right(ForwardIterator __first, ForwardIterator __last,
- typename iterator_traits::difference_type __n)
+ template
+constexpr _ForwardIterator
+shift_right(_ForwardIterator __first, _ForwardIterator __last,
+ typename iterator_traits<_ForwardIterator>::difference_type __n)
{
__glibcxx_assert(__n >= 0);
if (__n == 0)
return __first;
- using _Cat = typename
iterator_traits::iterator_category;
+ using _Cat
+ = typename iterator_traits<_ForwardIterator>::iterator_category;
if constexpr (derived_from<_Cat, bidirectional_iterator_tag>)
{
auto __mid = ranges::next(__last, -__n, __first);
diff --git a/libstdc++-v3/libsupc++/exception_ptr.h
b/libstdc++-v3/libsupc++/exception_ptr.h
index a053122d48e..4497d0e8581 100644
--- a/libstdc++-v3/libsupc++/exception_ptr.h
+++ b/libstdc++-v3/libsupc++/exception_ptr.h
@@ -180,8 +180,9 @@ namespace std
#ifndef _GLIBCXX_EH_PTR_COMPAT
inline
#endif
-exception_ptr::exception_ptr(const exception_ptr& other) _GLIBCXX_NOEXCEPT
-: _M_exception_object(other._M_exception_object)
+exception_ptr::exception_ptr(const exception_ptr& __other)
+ _GLIBCXX_NOEXCEPT
+: _M_exception_object(__other._M_exception_object)
{
if (_M_exception_object)
_M_addref();
@@ -200,9 +201,9 @@ namespace std
inline
#endif
exception_ptr&
-exception_ptr::operator=(const
Re: Problem with static const objects and LTO
On 10/7/20 5:12 PM, H.J. Lu wrote: > On Wed, Oct 7, 2020 at 3:09 PM Jeff Law via Gcc-patches > wrote: >> Adding the testcase... >> >> On 10/7/20 4:08 PM, Jeff Law wrote: >>> On 9/17/20 1:03 PM, Jakub Jelinek wrote: >>> [ ... Big snip, starting over ... ] >>> >>> I may have not explained things too well. So I've put together a small >>> example that folks can play with to show the underlying issue. >>> >>> >>> There's a static library libfu.a. In this static library we have a hunk >>> of local static data (utf8_sb_map) and two functions. One function puts >>> the address utf8_sb_map into a structure (rpl_regcomp), the other >>> verifies that the address stored in the structure is the same as the >>> address of utf8_sb_map (rpl_regfree). >>> >>> >>> That static library is linked into DSO libdso.so. The DSO sources >>> define a single function xregcomp which calls rpl_regcomp, but >>> references nothing else from the static library. Since libfu.a was >>> linked into the library we actually get a *copy* of rpl_regcomp in the >>> DSO. In fact, we get a copy of the entire .o file from libfu.a, which >>> matches traditional linkage models where the .o file is an atomic unit >>> for linking. >>> >>> >>> The main program calls xregcomp which is defined in the DSO and calls >>> rpl_regfree. The main program links against libdso.so and libfu.a. >>> Because it links libfu.a, it gets a copy of rpl_regfree, but *only* >>> rpl_regfree. That copy of rpl_regfree references a new and distinct >>> copy of utf8_sb_map. Naturally the address of utf8_sb_map in the main >>> program is different from the one in libdso.so and the test aborts. >>> >>> >>> Without LTO the main program would still reference rpl_regfree, but the >>> main program would not have its own copy. rpl_regfree and rpl_regcomp >>> would both be satisfied by the DSO (which remember has a complete copy >>> of the .o file from libfu.a). Thus there would be only one utf8_sb_map >>> as well and naturally the program will exit normally. >>> >>> >>> So I've got a bunch of thoughts here, but will defer sharing them >>> immediately so as not to unduly influence anyone. >>> >>> >>> I don't have a sense of how pervasive this issue is. I know it affects >>> man-db, but there could well be others. > This is: > > https://sourceware.org/bugzilla/show_bug.cgi?id=26530 > https://sourceware.org/bugzilla/show_bug.cgi?id=26314 I'm highly confident the fix for 26314 is already in my trees -- in fact I'm the one that originally reported that to Nick to begin with and much of that text is mine :-) I will check 26530. Thanks for the pointers. jeff
Re: Problem with static const objects and LTO
On Wed, Oct 7, 2020 at 3:09 PM Jeff Law via Gcc-patches wrote: > > Adding the testcase... > > On 10/7/20 4:08 PM, Jeff Law wrote: > > On 9/17/20 1:03 PM, Jakub Jelinek wrote: > > [ ... Big snip, starting over ... ] > > > > I may have not explained things too well. So I've put together a small > > example that folks can play with to show the underlying issue. > > > > > > There's a static library libfu.a. In this static library we have a hunk > > of local static data (utf8_sb_map) and two functions. One function puts > > the address utf8_sb_map into a structure (rpl_regcomp), the other > > verifies that the address stored in the structure is the same as the > > address of utf8_sb_map (rpl_regfree). > > > > > > That static library is linked into DSO libdso.so. The DSO sources > > define a single function xregcomp which calls rpl_regcomp, but > > references nothing else from the static library. Since libfu.a was > > linked into the library we actually get a *copy* of rpl_regcomp in the > > DSO. In fact, we get a copy of the entire .o file from libfu.a, which > > matches traditional linkage models where the .o file is an atomic unit > > for linking. > > > > > > The main program calls xregcomp which is defined in the DSO and calls > > rpl_regfree. The main program links against libdso.so and libfu.a. > > Because it links libfu.a, it gets a copy of rpl_regfree, but *only* > > rpl_regfree. That copy of rpl_regfree references a new and distinct > > copy of utf8_sb_map. Naturally the address of utf8_sb_map in the main > > program is different from the one in libdso.so and the test aborts. > > > > > > Without LTO the main program would still reference rpl_regfree, but the > > main program would not have its own copy. rpl_regfree and rpl_regcomp > > would both be satisfied by the DSO (which remember has a complete copy > > of the .o file from libfu.a). Thus there would be only one utf8_sb_map > > as well and naturally the program will exit normally. > > > > > > So I've got a bunch of thoughts here, but will defer sharing them > > immediately so as not to unduly influence anyone. > > > > > > I don't have a sense of how pervasive this issue is. I know it affects > > man-db, but there could well be others. This is: https://sourceware.org/bugzilla/show_bug.cgi?id=26530 https://sourceware.org/bugzilla/show_bug.cgi?id=26314 -- H.J.
[PATCH 6/8] [RS6000] rs6000_rtx_costs multi-insn constants
This small patch to rs6000_rtx_costs considerably improves code
generated for large constants in 64-bit code, teaching gcc that it is
better to load a constant from memory than to generate a sequence of
up to five dependent instructions. Note that the rs6000 backend does
generate large constants as loads from memory at expand time but
optimisation passes replace them with SETs of the value due to not
having correct costs.
PR 94393
* config/rs6000/rs6000.c (rs6000_rtx_costs): Cost multi-insn
constants.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 15a806fe307..76aedbfae6f 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21343,7 +21343,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
switch (code)
{
- /* On the RS/6000, if it is valid in the insn, it is free. */
+ /* (reg) is costed at zero by rtlanal.c:rtx_cost. That sets a
+baseline for rtx costs: If a constant is valid in an insn,
+it is free. */
case CONST_INT:
if (((outer_code == SET
|| outer_code == PLUS
@@ -21404,6 +21406,17 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
case CONST_DOUBLE:
case CONST_WIDE_INT:
+ /* Subtract one insn here for consistency with the above code
+that returns one less than the actual number of insns for
+SETs. Don't subtract one for other than SETs, because other
+operations will require the constant to be loaded to a
+register before performing the operation. All special cases
+for codes other than SET must be handled before we get
+here. */
+ *total = COSTS_N_INSNS (num_insns_constant (x, mode)
+ - (outer_code == SET ? 1 : 0));
+ return true;
+
case CONST:
case HIGH:
case SYMBOL_REF:
[PATCH 8/8] [RS6000] rs6000_rtx_costs for !speed
When optimizing for size we shouldn't be using metrics based on speed
or vice-versa. rtlanal.c:get_full_rtx_cost wants both speed and size
metric from rs6000_rtx_costs independent of the global optimize_size.
Note that the patch changes param_simultaneous_prefetches,
param_l1_cache_size, param_l1_cache_line_size and param_l2_cache_size,
which were previously all set to zero for optimize_size. I think that
was a bug. Those params are a function of the processor.
* config/rs6000/rs6000.h (rs6000_cost): Don't declare.
(struct processor_costs): Move to..
* config/rs6000/rs6000.c: ..here.
(rs6000_cost): Make static.
(rs6000_option_override_internal): Ignore optimize_size when
setting up rs6000_cost.
(rs6000_insn_cost): Take into account optimize_size here
instead.
(rs6000_emit_parity): Likewise.
(rs6000_rtx_costs): Don't use rs6000_cost when !speed.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index d455aa52427..14ecbad5df4 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -497,7 +497,26 @@ rs6000_store_data_bypass_p (rtx_insn *out_insn, rtx_insn
*in_insn)
/* Processor costs (relative to an add) */
-const struct processor_costs *rs6000_cost;
+struct processor_costs {
+ const int mulsi; /* cost of SImode multiplication. */
+ const int mulsi_const; /* cost of SImode multiplication by constant. */
+ const int mulsi_const9; /* cost of SImode mult by short constant. */
+ const int muldi; /* cost of DImode multiplication. */
+ const int divsi; /* cost of SImode division. */
+ const int divdi; /* cost of DImode division. */
+ const int fp; /* cost of simple SFmode and DFmode insns. */
+ const int dmul;/* cost of DFmode multiplication (and fmadd). */
+ const int sdiv;/* cost of SFmode division (fdivs). */
+ const int ddiv;/* cost of DFmode division (fdiv). */
+ const int cache_line_size;/* cache line size in bytes. */
+ const int l1_cache_size; /* size of l1 cache, in kilobytes. */
+ const int l2_cache_size; /* size of l2 cache, in kilobytes. */
+ const int simultaneous_prefetches; /* number of parallel prefetch
+ operations. */
+ const int sfdf_convert; /* cost of SF->DF conversion. */
+};
+
+static const struct processor_costs *rs6000_cost;
/* Instruction size costs on 32bit processors. */
static const
@@ -4618,131 +4637,128 @@ rs6000_option_override_internal (bool global_init_p)
}
/* Initialize rs6000_cost with the appropriate target costs. */
- if (optimize_size)
-rs6000_cost = TARGET_POWERPC64 ? _cost : _cost;
- else
-switch (rs6000_tune)
- {
- case PROCESSOR_RS64A:
- rs6000_cost = _cost;
- break;
+ switch (rs6000_tune)
+{
+case PROCESSOR_RS64A:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_MPCCORE:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_MPCCORE:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC403:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC403:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC405:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC405:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC440:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC440:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC476:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC476:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC601:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC601:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC603:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC603:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC604:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC604:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC604e:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC604e:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC620:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC620:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC630:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC630:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_CELL:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_CELL:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC750:
- case PROCESSOR_PPC7400:
- rs6000_cost = _cost;
- break;
+case PROCESSOR_PPC750:
+case PROCESSOR_PPC7400:
+ rs6000_cost = _cost;
+ break;
- case PROCESSOR_PPC7450:
- rs6000_cost = _cost;
-
[PATCH 0/8] [RS6000] rs6000_rtx_costs V2
This is a revised version of the patch series posted at https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553919.html Patch 8/8 in particular changes rather a lot more than the original !speed changes. Bootstrapped and regression tested powerpc64le-linux and powerpc64-linux biarch. Alan Modra (8): [RS6000] rs6000_rtx_costs comment [RS6000] rs6000_rtx_costs for AND [RS6000] rs6000_rtx_costs tidy AND [RS6000] rs6000_rtx_costs tidy break/return [RS6000] rs6000_rtx_costs cost IOR [RS6000] rs6000_rtx_costs multi-insn constants [RS6000] rs6000_rtx_costs reduce cost for SETs [RS6000] rs6000_rtx_costs for !speed gcc/config/rs6000/rs6000.c | 516 + gcc/config/rs6000/rs6000.h | 23 -- 2 files changed, 350 insertions(+), 189 deletions(-)
[PATCH 7/8] [RS6000] rs6000_rtx_costs reduce cost for SETs
The aim of this patch is to make rtx_costs for SETs closer to
insn_cost for SETs. One visible effect on powerpc code is increased
if-conversion.
* config/rs6000/rs6000.c (rs6000_rtx_costs): Reduce cost of SET
operands.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 76aedbfae6f..d455aa52427 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21684,6 +21684,35 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
}
return false;
+case SET:
+ /* On entry the value in *TOTAL is the number of general purpose
+regs being set, multiplied by COSTS_N_INSNS (1). Handle
+costing of set operands specially since in most cases we have
+an instruction rather than just a piece of RTL and should
+return a cost comparable to insn_cost. That's a little
+complicated because in some cases the cost of SET operands is
+non-zero, see point 5 above and cost of PLUS for example, and
+in others it is zero, for example for (set (reg) (reg)).
+But (set (reg) (reg)) has the same insn_cost as
+(set (reg) (plus (reg) (reg))). Hack around this by
+subtracting COSTS_N_INSNS (1) from the operand cost in cases
+were we add at least COSTS_N_INSNS (1) for some operation.
+However, don't do so for constants. Constants might cost
+more than zero when they require more than one instruction,
+and we do want the cost of extra instructions. */
+ {
+ rtx_code src_code = GET_CODE (SET_SRC (x));
+ if (src_code == CONST_INT
+ || src_code == CONST_DOUBLE
+ || src_code == CONST_WIDE_INT)
+ return false;
+ int set_cost = (rtx_cost (SET_SRC (x), mode, SET, 1, speed)
+ + rtx_cost (SET_DEST (x), mode, SET, 0, speed));
+ if (set_cost >= COSTS_N_INSNS (1))
+ *total += set_cost - COSTS_N_INSNS (1);
+ return true;
+ }
+
default:
return false;
}
[PATCH 2/8] [RS6000] rs6000_rtx_costs for AND
The existing "case AND" in this function is not sufficient for
optabs.c:avoid_expensive_constant usage, where the AND is passed in
outer_code. We'd like to cost AND of rs6000_is_valid_and_mask
or rs6000_is_valid_2insn_and variety there, so that those masks aren't
seen as expensive (ie. better to load to a reg then AND).
* config/rs6000/rs6000.c (rs6000_rtx_costs): Combine CONST_INT
AND handling with IOR/XOR. Move costing for AND with
rs6000_is_valid_and_mask or rs6000_is_valid_2insn_and to
CONST_INT.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 97180bb3819..e870ba0039a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21264,16 +21264,13 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
|| outer_code == MINUS)
&& (satisfies_constraint_I (x)
|| satisfies_constraint_L (x)))
- || (outer_code == AND
- && (satisfies_constraint_K (x)
- || (mode == SImode
- ? satisfies_constraint_L (x)
- : satisfies_constraint_J (x
- || ((outer_code == IOR || outer_code == XOR)
+ || ((outer_code == AND || outer_code == IOR || outer_code == XOR)
&& (satisfies_constraint_K (x)
|| (mode == SImode
? satisfies_constraint_L (x)
: satisfies_constraint_J (x
+ || (outer_code == AND
+ && rs6000_is_valid_and_mask (x, mode))
|| outer_code == ASHIFT
|| outer_code == ASHIFTRT
|| outer_code == LSHIFTRT
@@ -21310,7 +21307,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
|| outer_code == IOR
|| outer_code == XOR)
&& (INTVAL (x)
- & ~ (unsigned HOST_WIDE_INT) 0x) == 0))
+ & ~ (unsigned HOST_WIDE_INT) 0x) == 0)
+ || (outer_code == AND
+ && rs6000_is_valid_2insn_and (x, mode)))
{
*total = COSTS_N_INSNS (1);
return true;
@@ -21448,26 +21447,6 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
*total += COSTS_N_INSNS (1);
return true;
}
-
- /* rotate-and-mask (no rotate), andi., andis.: 1 insn. */
- HOST_WIDE_INT val = INTVAL (XEXP (x, 1));
- if (rs6000_is_valid_and_mask (XEXP (x, 1), mode)
- || (val & 0x) == val
- || (val & 0x) == val
- || ((val & 0x) == 0 && mode == SImode))
- {
- *total = rtx_cost (left, mode, AND, 0, speed);
- *total += COSTS_N_INSNS (1);
- return true;
- }
-
- /* 2 insns. */
- if (rs6000_is_valid_2insn_and (XEXP (x, 1), mode))
- {
- *total = rtx_cost (left, mode, AND, 0, speed);
- *total += COSTS_N_INSNS (2);
- return true;
- }
}
*total = COSTS_N_INSNS (1);
[PATCH 4/8] [RS6000] rs6000_rtx_costs tidy break/return
Most cases use "return false" rather than breaking out of the switch. Do so in all cases. * config/rs6000/rs6000.c (rs6000_rtx_costs): Tidy break/return. diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index bc5e51aa5ce..383d2901c9f 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -21371,7 +21371,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code, *total = rs6000_cost->fp; else *total = rs6000_cost->dmul; - break; + return false; case DIV: case MOD: @@ -21539,7 +21539,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code, *total = rs6000_cost->fp; return false; } - break; + return false; case NE: case EQ: @@ -21577,13 +21577,11 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code, *total = 0; return true; } - break; + return false; default: - break; + return false; } - - return false; } /* Debug form of r6000_rtx_costs that is selected if -mdebug=cost. */
[PATCH 1/8] [RS6000] rs6000_rtx_costs comment
This lays out the ground rules for following patches. * config/rs6000/rs6000.c (rs6000_rtx_costs): Expand comment. diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index b58eeae2b98..97180bb3819 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -21208,7 +21208,46 @@ rs6000_cannot_copy_insn_p (rtx_insn *insn) /* Compute a (partial) cost for rtx X. Return true if the complete cost has been computed, and false if subexpressions should be - scanned. In either case, *TOTAL contains the cost result. */ + scanned. In either case, *TOTAL contains the cost result. + + 1) Calls from places like optabs.c:avoid_expensive_constant will + come here with OUTER_CODE set to an operation such as AND with X + being a CONST_INT or other CONSTANT_P type. This will be compared + against set_src_cost, where we'll come here with OUTER_CODE as SET + and X the same constant. + + 2) Calls from places like combine.c:distribute_and_simplify_rtx are + asking whether a possibly quite complex SET_SRC can be implemented + more cheaply than some other logically equivalent SET_SRC. + + 3) Calls from places like ifcvt.c:default_noce_conversion_profitable_p + will come here via seq_cost which calls set_rtx_cost on single set + insns. set_rtx_cost passes the pattern of a SET insn in X with + OUTER_CODE as INSN. The overall cost should be comparable to + rs6000_insn_cost since the code is comparing one insn sequence + (some of which may be costed by insn_cost) against another sequence. + Note the difference between set_rtx_cost, which costs the entire + SET, and set_src_cost, which costs just the SET_SRC. + + 4) Calls from places like cprop.c:try_replace_reg will also come + here via set_rtx_cost, with X either a valid pattern of a SET or + one where some registers have been replaced with constants. The + replacements may make the SET invalid, for example if + (set (reg1) (and (reg2) (const_int 0xfff))) + has reg2 replaced as + (set (reg1) (and (symbol_ref) (const_int 0xfff))) + then the replacement can't be implemented in one instruction and + really the cost should be higher by one instruction. However, + the cost for invalid insns doesn't matter much except that a + higher cost may lead to their rejection earlier. + + 5) fwprop.c:should_replace_address puts yet another wrinkle on this + function, where we prefer an address calculation that is more + complex yet has the same address_cost. In this case "more + complex" is determined by having a higher set_src_cost. So for + example, if we want a plain (reg) address to be replaced with + (plus (reg) (const)) when possible then PLUS needs to cost more + than zero here. */ static bool rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
[PATCH 5/8] [RS6000] rs6000_rtx_costs cost IOR
* config/rs6000/rs6000.c (rotate_insert_cost): New function.
(rs6000_rtx_costs): Cost IOR.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 383d2901c9f..15a806fe307 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21206,6 +21206,91 @@ rs6000_cannot_copy_insn_p (rtx_insn *insn)
&& get_attr_cannot_copy (insn);
}
+/* Handle rtx_costs for scalar integer rotate and insert insns. */
+
+static bool
+rotate_insert_cost (rtx left, rtx right, machine_mode mode, bool speed,
+ int *total)
+{
+ if (GET_CODE (right) == AND
+ && CONST_INT_P (XEXP (right, 1))
+ && UINTVAL (XEXP (left, 1)) + UINTVAL (XEXP (right, 1)) + 1 == 0)
+{
+ rtx leftop = XEXP (left, 0);
+ rtx rightop = XEXP (right, 0);
+
+ /* rotlsi3_insert_5. */
+ if (REG_P (leftop)
+ && REG_P (rightop)
+ && mode == SImode
+ && UINTVAL (XEXP (left, 1)) != 0
+ && UINTVAL (XEXP (right, 1)) != 0
+ && rs6000_is_valid_mask (XEXP (left, 1), NULL, NULL, mode))
+ return true;
+ /* rotldi3_insert_6. */
+ if (REG_P (leftop)
+ && REG_P (rightop)
+ && mode == DImode
+ && exact_log2 (-UINTVAL (XEXP (left, 1))) > 0)
+ return true;
+ /* rotldi3_insert_7. */
+ if (REG_P (leftop)
+ && REG_P (rightop)
+ && mode == DImode
+ && exact_log2 (-UINTVAL (XEXP (right, 1))) > 0)
+ return true;
+
+ rtx mask = 0;
+ rtx shift = leftop;
+ rtx_code shift_code = GET_CODE (shift);
+ /* rotl3_insert. */
+ if (shift_code == ROTATE
+ || shift_code == ASHIFT
+ || shift_code == LSHIFTRT)
+ mask = right;
+ else
+ {
+ shift = rightop;
+ shift_code = GET_CODE (shift);
+ /* rotl3_insert_2. */
+ if (shift_code == ROTATE
+ || shift_code == ASHIFT
+ || shift_code == LSHIFTRT)
+ mask = left;
+ }
+ if (mask
+ && CONST_INT_P (XEXP (shift, 1))
+ && rs6000_is_valid_insert_mask (XEXP (mask, 1), shift, mode))
+ {
+ *total += rtx_cost (XEXP (shift, 0), mode, shift_code, 0, speed);
+ *total += rtx_cost (XEXP (mask, 0), mode, AND, 0, speed);
+ return true;
+ }
+}
+ /* rotl3_insert_3. */
+ if (GET_CODE (right) == ASHIFT
+ && CONST_INT_P (XEXP (right, 1))
+ && (INTVAL (XEXP (right, 1))
+ == exact_log2 (UINTVAL (XEXP (left, 1)) + 1)))
+{
+ *total += rtx_cost (XEXP (left, 0), mode, AND, 0, speed);
+ *total += rtx_cost (XEXP (right, 0), mode, ASHIFT, 0, speed);
+ return true;
+}
+ /* rotl3_insert_4. */
+ if (GET_CODE (right) == LSHIFTRT
+ && CONST_INT_P (XEXP (right, 1))
+ && mode == SImode
+ && (INTVAL (XEXP (right, 1))
+ + exact_log2 (-UINTVAL (XEXP (left, 1 == 32)
+{
+ *total += rtx_cost (XEXP (left, 0), mode, AND, 0, speed);
+ *total += rtx_cost (XEXP (right, 0), mode, LSHIFTRT, 0, speed);
+ return true;
+}
+ return false;
+}
+
/* Compute a (partial) cost for rtx X. Return true if the complete
cost has been computed, and false if subexpressions should be
scanned. In either case, *TOTAL contains the cost result.
@@ -21253,7 +21338,7 @@ static bool
rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
int opno ATTRIBUTE_UNUSED, int *total, bool speed)
{
- rtx right;
+ rtx left, right;
int code = GET_CODE (x);
switch (code)
@@ -21435,7 +21520,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
right = XEXP (x, 1);
if (CONST_INT_P (right))
{
- rtx left = XEXP (x, 0);
+ left = XEXP (x, 0);
rtx_code left_code = GET_CODE (left);
/* rotate-and-mask: 1 insn. */
@@ -21452,9 +21537,16 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
return false;
case IOR:
- /* FIXME */
*total = COSTS_N_INSNS (1);
- return true;
+ left = XEXP (x, 0);
+ if (GET_CODE (left) == AND
+ && CONST_INT_P (XEXP (left, 1)))
+ {
+ right = XEXP (x, 1);
+ if (rotate_insert_cost (left, right, mode, speed, total))
+ return true;
+ }
+ return false;
case CLZ:
case XOR:
[PATCH 3/8] [RS6000] rs6000_rtx_costs tidy AND
* config/rs6000/rs6000.c (rs6000_rtx_costs): Tidy AND code.
Don't avoid recursion on const_int shift count.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index e870ba0039a..bc5e51aa5ce 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21253,6 +21253,7 @@ static bool
rs6000_rtx_costs (rtx x, machine_mode mode, int outer_code,
int opno ATTRIBUTE_UNUSED, int *total, bool speed)
{
+ rtx right;
int code = GET_CODE (x);
switch (code)
@@ -21430,7 +21431,9 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
return false;
case AND:
- if (CONST_INT_P (XEXP (x, 1)))
+ *total = COSTS_N_INSNS (1);
+ right = XEXP (x, 1);
+ if (CONST_INT_P (right))
{
rtx left = XEXP (x, 0);
rtx_code left_code = GET_CODE (left);
@@ -21439,17 +21442,13 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
if ((left_code == ROTATE
|| left_code == ASHIFT
|| left_code == LSHIFTRT)
- && rs6000_is_valid_shift_mask (XEXP (x, 1), left, mode))
+ && rs6000_is_valid_shift_mask (right, left, mode))
{
- *total = rtx_cost (XEXP (left, 0), mode, left_code, 0, speed);
- if (!CONST_INT_P (XEXP (left, 1)))
- *total += rtx_cost (XEXP (left, 1), SImode, left_code, 1,
speed);
- *total += COSTS_N_INSNS (1);
+ *total += rtx_cost (XEXP (left, 0), mode, left_code, 0, speed);
+ *total += rtx_cost (XEXP (left, 1), SImode, left_code, 1, speed);
return true;
}
}
-
- *total = COSTS_N_INSNS (1);
return false;
case IOR:
testsuite: [arm] add -mthumb to arm/cortex-m55 tests
The recently added arm/cortex-m55-* tests fail if the compiler does not default to thumb code. This patch adds -mthumb to the dg-additional-options, which fixes the problem in most cases. When running the tests with an incompatible -march= value, the tests also fail because it conflicts with -mcpu=cortex-m55. I tried to add -march=armv8.1-m.main+XXX but found it awkward to get the right value for XXX to match what each test is adding to cortex-m55, so I propose to consider adding -mthumb as good enough. Maybe we can/should add dg-skip-if for -march? OK with a suitable changelog? Thanks, Christophe cortex-m55-thumb.patch1 Description: Binary data
Re: Problem with static const objects and LTO
Adding the testcase... On 10/7/20 4:08 PM, Jeff Law wrote: > On 9/17/20 1:03 PM, Jakub Jelinek wrote: > [ ... Big snip, starting over ... ] > > I may have not explained things too well. So I've put together a small > example that folks can play with to show the underlying issue. > > > There's a static library libfu.a. In this static library we have a hunk > of local static data (utf8_sb_map) and two functions. One function puts > the address utf8_sb_map into a structure (rpl_regcomp), the other > verifies that the address stored in the structure is the same as the > address of utf8_sb_map (rpl_regfree). > > > That static library is linked into DSO libdso.so. The DSO sources > define a single function xregcomp which calls rpl_regcomp, but > references nothing else from the static library. Since libfu.a was > linked into the library we actually get a *copy* of rpl_regcomp in the > DSO. In fact, we get a copy of the entire .o file from libfu.a, which > matches traditional linkage models where the .o file is an atomic unit > for linking. > > > The main program calls xregcomp which is defined in the DSO and calls > rpl_regfree. The main program links against libdso.so and libfu.a. > Because it links libfu.a, it gets a copy of rpl_regfree, but *only* > rpl_regfree. That copy of rpl_regfree references a new and distinct > copy of utf8_sb_map. Naturally the address of utf8_sb_map in the main > program is different from the one in libdso.so and the test aborts. > > > Without LTO the main program would still reference rpl_regfree, but the > main program would not have its own copy. rpl_regfree and rpl_regcomp > would both be satisfied by the DSO (which remember has a complete copy > of the .o file from libfu.a). Thus there would be only one utf8_sb_map > as well and naturally the program will exit normally. > > > So I've got a bunch of thoughts here, but will defer sharing them > immediately so as not to unduly influence anyone. > > > I don't have a sense of how pervasive this issue is. I know it affects > man-db, but there could well be others. > > > > > jeff > t.tar.gz Description: application/gzip
Re: Problem with static const objects and LTO
On 9/17/20 1:03 PM, Jakub Jelinek wrote: [ ... Big snip, starting over ... ] I may have not explained things too well. So I've put together a small example that folks can play with to show the underlying issue. There's a static library libfu.a. In this static library we have a hunk of local static data (utf8_sb_map) and two functions. One function puts the address utf8_sb_map into a structure (rpl_regcomp), the other verifies that the address stored in the structure is the same as the address of utf8_sb_map (rpl_regfree). That static library is linked into DSO libdso.so. The DSO sources define a single function xregcomp which calls rpl_regcomp, but references nothing else from the static library. Since libfu.a was linked into the library we actually get a *copy* of rpl_regcomp in the DSO. In fact, we get a copy of the entire .o file from libfu.a, which matches traditional linkage models where the .o file is an atomic unit for linking. The main program calls xregcomp which is defined in the DSO and calls rpl_regfree. The main program links against libdso.so and libfu.a. Because it links libfu.a, it gets a copy of rpl_regfree, but *only* rpl_regfree. That copy of rpl_regfree references a new and distinct copy of utf8_sb_map. Naturally the address of utf8_sb_map in the main program is different from the one in libdso.so and the test aborts. Without LTO the main program would still reference rpl_regfree, but the main program would not have its own copy. rpl_regfree and rpl_regcomp would both be satisfied by the DSO (which remember has a complete copy of the .o file from libfu.a). Thus there would be only one utf8_sb_map as well and naturally the program will exit normally. So I've got a bunch of thoughts here, but will defer sharing them immediately so as not to unduly influence anyone. I don't have a sense of how pervasive this issue is. I know it affects man-db, but there could well be others. jeff
[PATCH] libstdc++: Implement C++20 features for
From: Thomas Rodgers New ctors and ::view() accessor for - * basic_stingbuf * basic_istringstream * basic_ostringstream * basic_stringstreamm New ::get_allocator() accessor for basic_stringbuf. libstdc++-v3/ChangeLog: * acinclude.m4 (glibcxx_SUBDIRS): Add src/c++20. * config/abi/pre/gnu.ver: Update GLIBCXX_3.4.29 for the addition of - basic_stringbuf::basic_stringbuf(allocator const&), basic_stringbuf::basic_stringbuf(openmode, allocator const&), basic_stringbuf::basic_stringbuf(basic_string&&, openmode), basic_stringbuf::basic_stringbuf(basic_stringbuf&&, allocator const&), basic_stringbuf::get_allocator(), basic_stringbuf::view(), basic_istringstream::basic_istringstream(basic_string&&, openmode), basic_istringstream::basic_istringstream(openmode, allocator const&), basic_istringstream::view(), basic_ostringstream::basic_ostringstream(basic_string&&, openmode), basic_ostringstream::basic_ostringstream(openmode, allocator const&), basic_ostringstream::view(), basic_stringstream::basic_stringstream(basic_string&&, openmode), basic_stringstream::basic_stringstream(openmode, allocator const&), basic_stringstream::view(). * configure: Regenerate. * include/std/sstream: (basic_stringbuf::basic_stringbuf(allocator const&)): New constructor. (basic_stringbuf::basic_stringbuf(openmode, allocator const&)): Likewise. (basic_stringbuf::basic_stringbuf(basic_string&&, openmode)): Likewise. (basic_stringbuf::basic_stringbuf(basic_stringbuf&&, allocator const&)): Likewise. (basic_stringbuf::get_allocator()): New method. (basic_stringbuf::view()): Likewise. (basic_istringstream::basic_istringstream(basic_string&&, openmode)): New constructor. (basic_istringstream::basic_istringstream(openmode, allocator const&)): Likewise (basic_istringstream::view()): New method. (basic_ostringstream::basic_ostringstream(basic_string&&, openmode)): New constructor. (basic_ostringstream::basic_ostringstream(openmode, allocator const&)): Likewise (basic_ostringstream::view()): New method. (basic_stringstream::basic_stringstream(basic_string&&, openmode)): New constructor. (basic_stringstream::basic_stringstream(openmode, allocator const&)): Likewise (basic_stringstream::view()): New method. * src/Makefile.in: Add c++20 directory. * src/Makefile.am: Regenerate. * src/c++20/Makefile.am: Add makefile for new sub-directory. * src/c++20/Makefile.in: Generate. * src/c++20/sstream-inst.cc: New file defining explicit instantiations for basic_stringbuf, basic_istringstream, basic_ostringstream, and basic_stringstream member functions added in C++20. * testsuite/27_io/basic_stringbuf/cons/char/2.cc: New test. * testsuite/27_io/basic_stringbuf/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringbuf/view/char/2.cc: Likewise. * testsuite/27_io/basic_stringbuf/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_istringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_istringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_istringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_istringstream/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_stringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_stringstream/view/wchar_t/2.cc: Likewise. --- .topdeps | 1 + .topmsg | 2 + libstdc++-v3/acinclude.m4 | 2 +- libstdc++-v3/config/abi/pre/gnu.ver | 45 ++ libstdc++-v3/configure| 16 +- libstdc++-v3/include/std/sstream | 196 + libstdc++-v3/src/Makefile.am | 12 +- libstdc++-v3/src/Makefile.in | 14 +- libstdc++-v3/src/c++20/Makefile.am| 105 +++ libstdc++-v3/src/c++20/Makefile.in| 735 ++ libstdc++-v3/src/c++20/sstream-inst.cc| 110 +++ .../27_io/basic_istringstream/cons/char/1.cc | 85 ++ .../basic_istringstream/cons/wchar_t/1.cc | 85 ++ .../27_io/basic_istringstream/view/char/1.cc | 35 + .../basic_istringstream/view/wchar_t/1.cc | 36 +
Re: [PATCH 2b/5] RS6000 add 128-bit Integer Operations
On Mon, 2020-10-05 at 11:52 -0700, Carl Love wrote: > Will and Segher: > > This is the rest of the second patch which adds the 128-bit integer > support for divide, modulo, shift, compare of 128-bit > integers instructions and builtin support. > > In the last round of changes, the flag for the 128-bit operations was > removed. Per Will's comments, the BU_P10_128BIT_* builtin definitions > can be removed. Instead we can just use P10V_BUILTIN. Similarly for > the BU_P10_P builtin definition. The commit log was updated to reflect > the change. There were a few change log entries for the 128-bit > operations flag that needed removing. As well as other fixes noted by > Will. > > The changes are all name changes not functional changes. > > No regression failures were found when run on a P9. > > Please let me know if this is ready for mainline. > >Carl > > > > gcc/ChangeLog > > 2020-10/05 Carl Love > * config/rs6000/altivec.h (vec_signextq, vec_dive, vec_mod): Add define > for new builtins. > * config/rs6000/altivec.md (UNSPEC_VMULEUD, UNSPEC_VMULESD, > UNSPEC_VMULOUD, UNSPEC_VMULOSD): New unspecs. > (altivec_eqv1ti, altivec_gtv1ti, altivec_gtuv1ti, altivec_vmuleud, > altivec_vmuloud, altivec_vmulesd, altivec_vmulosd, altivec_vrlq, > altivec_vrlqmi, altivec_vrlqmi_inst, altivec_vrlqnm, > altivec_vrlqnm_inst, altivec_vslq, altivec_vsrq, altivec_vsraq, > altivec_vcmpequt_p, altivec_vcmpgtst_p, altivec_vcmpgtut_p): New > define_insn. > (vec_widen_umult_even_v2di, vec_widen_smult_even_v2di, > vec_widen_umult_odd_v2di, vec_widen_smult_odd_v2di, altivec_vrlqmi, > altivec_vrlqnm): New define_expands. > * config/rs6000/rs6000-builtin.def (VCMPEQUT_P, VCMPGTST_P, > VCMPGTUT_P): Add macro expansions. > (VCMPGTUT, VCMPGTST, VCMPEQUT, CMPNET, CMPGE_1TI, > CMPGE_U1TI, CMPLE_1TI, CMPLE_U1TI, VNOR_V1TI_UNS, VNOR_V1TI, VCMPNET_P, > VCMPAET_P, VSIGNEXTSD2Q, VMULEUD, VMULESD, VMULOUD, VMULOSD, VRLQ, > VSLQ, VSRQ, VSRAQ, VRLQNM, DIV_V1TI, UDIV_V1TI, DIVES_V1TI, DIVEU_V1TI, > MODS_V1TI, MODU_V1TI, VRLQMI): New macro expansions. > (VRLQ, VSLQ, VSRQ, VSRAQ, DIVE, MOD, SIGNEXT): New overload expansions. > * config/rs6000/rs6000-call.c (P10_BUILTIN_VCMPEQUT, > P10V_BUILTIN_CMPGE_1TI, P10V_BUILTIN_CMPGE_U1TI, > P10V_BUILTIN_VCMPGTUT, P10V_BUILTIN_VCMPGTST, > P10V_BUILTIN_CMPLE_1TI, P10V_BUILTIN_VCMPLE_U1TI, > P10V_BUILTIN_128BIT_DIV_V1TI, P10V_BUILTIN_128BIT_UDIV_V1TI, > P10V_BUILTIN_128BIT_VMULESD, P10V_BUILTIN_128BIT_VMULEUD, > P10V_BUILTIN_128BIT_VMULOSD, P10V_BUILTIN_128BIT_VMULOUD, Just sniff-checked a few. Don't see the P10V_BUILTIN_128BIT_* entries below. > P10V_BUILTIN_VNOR_V1TI, P10V_BUILTIN_VNOR_V1TI_UNS, > P10V_BUILTIN_128BIT_VRLQ, P10V_BUILTIN_128BIT_VRLQMI, > P10V_BUILTIN_128BIT_VRLQNM, P10V_BUILTIN_128BIT_VSLQ, > P10V_BUILTIN_128BIT_VSRQ, P10V_BUILTIN_128BIT_VSRAQ, > P10V_BUILTIN_VCMPGTUT_P, P10V_BUILTIN_VCMPGTST_P, > P10V_BUILTIN_VCMPEQUT_P, P10V_BUILTIN_VCMPGTUT_P, > P10V_BUILTIN_VCMPGTST_P, P10V_BUILTIN_CMPNET, > P10V_BUILTIN_VCMPNET_P, P10V_BUILTIN_VCMPAET_P, > P10V_BUILTIN_128BIT_VSIGNEXTSD2Q, P10V_BUILTIN_128BIT_DIVES_V1TI, > P10V_BUILTIN_128BIT_MODS_V1TI, P10V_BUILTIN_128BIT_MODU_V1TI): > New overloaded definitions. > (rs6000_gimple_fold_builtin) [P10V_BUILTIN_VCMPEQUT, > P10_BUILTIN_CMPNET, P10_BUILTIN_CMPGE_1TI, > P10_BUILTIN_CMPGE_U1TI, P10_BUILTIN_VCMPGTUT, > P10_BUILTIN_VCMPGTST, P10_BUILTIN_CMPLE_1TI, > P10_BUILTIN_CMPLE_U1TI]: New case statements. > (rs6000_init_builtins) [bool_V1TI_type_node, int_ftype_int_v1ti_v1ti]: > New assignments. > (altivec_init_builtins): New E_V1TImode case statement. > (builtin_function_type)[P10_BUILTIN_128BIT_VMULEUD, > P10_BUILTIN_128BIT_VMULOUD, P10_BUILTIN_128BIT_DIVEU_V1TI, > P10_BUILTIN_128BIT_MODU_V1TI, P10_BUILTIN_CMPGE_U1TI, > P10_BUILTIN_VCMPGTUT, P10_BUILTIN_VCMPEQUT]: New case statements. I don't see these keywords below. Possibly P10V_* > * config/rs6000/r6000.c (rs6000_handle_altivec_attribute)[E_TImode, > E_V1TImode]: New case statements. > * config/rs6000/r6000.h (RS6000_BTM_TI_VECTOR_OPS): New defines. > (rs6000_builtin_type_index): New enum value RS6000_BTI_bool_V1TI. > * config/rs6000/vector.md (vector_gtv1ti,vector_nltv1ti, > vector_gtuv1ti, vector_nltuv1ti, vector_ngtv1ti, vector_ngtuv1ti, > vector_eq_v1ti_p, vector_ne_v1ti_p, vector_ae_v1ti_p, > vector_gt_v1ti_p, vector_gtu_v1ti_p, vrotlv1ti3, vashlv1ti3, > vlshrv1ti3, vashrv1ti3): New define_expands. > * config/rs6000/vsx.md (UNSPEC_VSX_DIVSQ, UNSPEC_VSX_DIVUQ, > UNSPEC_VSX_DIVESQ, UNSPEC_VSX_DIVEUQ, UNSPEC_VSX_MODSQ, >
libgo patch committed: Stop using -fno-section-anchors on AIX
This patch by Clément Chigot changes the libgo configure script to
stop using -fno-section-anchors on AIX. This option is no longer
needed. There is no crash without it since at least gcc-9.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
45376dc0f426c0fc39c5ee77937c928c27fab77a
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index 15f01a0519b..930339e9b44 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-762b74a56f7ca102a5b8da1c9d0ffce592caa46b
+2563706e4ead80d6906d66ae23c8915c360583ad
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
diff --git a/libgo/configure.ac b/libgo/configure.ac
index f15f8d830bb..9a10d3305ab 100644
--- a/libgo/configure.ac
+++ b/libgo/configure.ac
@@ -33,10 +33,6 @@ AC_SUBST(CFLAGS)
case ${host} in
*-*-aix*)
-# static hash tables crashes on AIX when libgo is built with O2
-CFLAGS="$CFLAGS -fno-section-anchors"
-GOCFLAGS="$GOCFLAGS -fno-section-anchors"
-
# Check default architecture for FAT library creation
if test -z "`$CC -x c -E /dev/null -g3 -o - | grep 64BIT`" ; then
AIX_EXTRA_ARCH='64'
Re: [PATCH 2a/5] rs6000, vec_rlnm builtin fix arguments
On Mon, 2020-10-05 at 11:52 -0700, Carl Love wrote: > Will, Segher: > > > > The following changes were made from the previous version: > > Per Will's comments, I split the bug fix from patch 2 into a separate > patch. This patch is the bug fix for the vec_rlnm builtin. I recommend trying to keep a clean paragraph appropriate for the commit log, separate from the history of the patch. i.e. "This patch fixes an error in how the vec_rlnm() builtin parameters were handled." > > Regression tests reran on Power 9 LE with no regression errors. > > Please let me know if it looks OK to commit to mainline. > > Carl > > -- > > > gcc/ChangeLog > > 2020-10-05 Carl Love > > * config/rs6000/altivec.h (vec_rlnm): Fix bug in argument generation. Is there any testcase impact? (If this doesn't fix an existing test, may be worth adding one..) LGTM, Thanks -Will > --- > gcc/config/rs6000/altivec.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h > index 8a2dcda0144..f7720d136c9 100644 > --- a/gcc/config/rs6000/altivec.h > +++ b/gcc/config/rs6000/altivec.h > @@ -183,7 +183,7 @@ > #define vec_recipdiv __builtin_vec_recipdiv > #define vec_rlmi __builtin_vec_rlmi > #define vec_vrlnm __builtin_vec_rlnm > -#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((c)<<8)|(b))) > +#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((b)<<8)|(c))) > #define vec_rsqrt __builtin_vec_rsqrt > #define vec_rsqrte __builtin_vec_rsqrte > #define vec_signed __builtin_vec_vsigned
Re: [PATCH 1/5] RS6000 Add 128-bit Binary Integer sign extend operations
On Mon, 2020-10-05 at 11:51 -0700, Carl Love wrote:
> Will, Segher:
>
> Patch 1, adds the 128-bit sign extension instruction support and
> corresponding builtin support.
>
> I updated the change log per the comments from Will.
>
> Patch has been retested on Power 9 LE.
>
> Pet me know if it is ready to commit to mainline.
>
> Carl
>
> ---
>
>
> gcc/ChangeLog
>
> 2020-10-05 Carl Love
> * config/rs6000/altivec.h (vec_signextll, vec_signexti): Add define
> for new builtins.
> * config/rs6000/rs6000-builtin.def (VSIGNEXTI, VSIGNEXTLL): Add
> overloaded builtin definitions.
> (VSIGNEXTSB2W, VSIGNEXTSH2W, VSIGNEXTSB2D, VSIGNEXTSH2D,VSIGNEXTSW2D):
> Add builtin expansions.
> * config/rs6000-call.c (P9V_BUILTIN_VEC_VSIGNEXTI,
> P9V_BUILTIN_VEC_VSIGNEXTLL): Add overloaded argument definitions.
> * config/rs6000/vsx.md: Make define_insn vsx_sign_extend_si_v2di
> visible.
> * doc/extend.texi: Add documentation for the vec_signexti and
> vec_signextll builtins.
>
> gcc/testsuite/ChangeLog
>
> 2020-10-05 Carl Love
> * gcc.target/powerpc/p9-sign_extend-runnable.c: New test case.
> ---
> gcc/config/rs6000/altivec.h | 3 +
> gcc/config/rs6000/rs6000-builtin.def | 9 ++
> gcc/config/rs6000/rs6000-call.c | 13 ++
> gcc/config/rs6000/vsx.md | 2 +-
> gcc/doc/extend.texi | 15 ++
> .../powerpc/p9-sign_extend-runnable.c | 128 ++
> 6 files changed, 169 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/p9-sign_extend-runnable.c
>
> diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h
> index f7720d136c9..cfa5eda4cd5 100644
> --- a/gcc/config/rs6000/altivec.h
> +++ b/gcc/config/rs6000/altivec.h
> @@ -494,6 +494,9 @@
>
> #define vec_xlx __builtin_vec_vextulx
> #define vec_xrx __builtin_vec_vexturx
> +#define vec_signexti __builtin_vec_vsignexti
> +#define vec_signextll __builtin_vec_vsignextll
> +
> #endif
Can probably drop that blank line.
>
> /* Predicates.
> diff --git a/gcc/config/rs6000/rs6000-builtin.def
> b/gcc/config/rs6000/rs6000-builtin.def
> index e91a48ddf5f..4c2e9460949 100644
> --- a/gcc/config/rs6000/rs6000-builtin.def
> +++ b/gcc/config/rs6000/rs6000-builtin.def
> @@ -2715,6 +2715,8 @@ BU_P9V_OVERLOAD_1 (VPRTYBD, "vprtybd")
> BU_P9V_OVERLOAD_1 (VPRTYBQ, "vprtybq")
> BU_P9V_OVERLOAD_1 (VPRTYBW, "vprtybw")
> BU_P9V_OVERLOAD_1 (VPARITY_LSBB, "vparity_lsbb")
> +BU_P9V_OVERLOAD_1 (VSIGNEXTI,"vsignexti")
> +BU_P9V_OVERLOAD_1 (VSIGNEXTLL, "vsignextll")
>
> /* 2 argument functions added in ISA 3.0 (power9). */
> BU_P9_2 (CMPRB, "byte_in_range",CONST, cmprb)
> @@ -2726,6 +2728,13 @@ BU_P9_OVERLOAD_2 (CMPRB, "byte_in_range")
> BU_P9_OVERLOAD_2 (CMPRB2,"byte_in_either_range")
> BU_P9_OVERLOAD_2 (CMPEQB,"byte_in_set")
>
> +/* Sign extend builtins that work on ISA 3.0, but not defined until ISA 3.1.
> */
I have mixed feelings about straddling the ISA 3.0 and 3.1 ; but not
sure how to properly improve. (I defer).
The rest LGTM,
Thanks
-Will
> +BU_P9V_AV_1 (VSIGNEXTSB2W, "vsignextsb2w", CONST,
> vsx_sign_extend_qi_v4si)
> +BU_P9V_AV_1 (VSIGNEXTSH2W, "vsignextsh2w", CONST,
> vsx_sign_extend_hi_v4si)
> +BU_P9V_AV_1 (VSIGNEXTSB2D, "vsignextsb2d", CONST,
> vsx_sign_extend_qi_v2di)
> +BU_P9V_AV_1 (VSIGNEXTSH2D, "vsignextsh2d", CONST,
> vsx_sign_extend_hi_v2di)
> +BU_P9V_AV_1 (VSIGNEXTSW2D, "vsignextsw2d", CONST,
> vsx_sign_extend_si_v2di)
> +
> /* Builtins for scalar instructions added in ISA 3.1 (power10). */
> BU_P10_MISC_2 (CFUGED, "cfuged", CONST, cfuged)
> BU_P10_MISC_2 (CNTLZDM, "cntlzdm", CONST, cntlzdm)
> diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
> index a8b520834c7..9e514a01012 100644
> --- a/gcc/config/rs6000/rs6000-call.c
> +++ b/gcc/config/rs6000/rs6000-call.c
> @@ -5527,6 +5527,19 @@ const struct altivec_builtin_types
> altivec_overloaded_builtins[] = {
> RS6000_BTI_unsigned_V2DI, RS6000_BTI_unsigned_V2DI,
> RS6000_BTI_INTSI, RS6000_BTI_INTSI },
>
> + /* Sign extend builtins that work work on ISA 3.0, not added until ISA 3.1
> */
> + { P9V_BUILTIN_VEC_VSIGNEXTI, P9V_BUILTIN_VSIGNEXTSB2W,
> +RS6000_BTI_V4SI, RS6000_BTI_V16QI, 0, 0 },
> + { P9V_BUILTIN_VEC_VSIGNEXTI, P9V_BUILTIN_VSIGNEXTSH2W,
> +RS6000_BTI_V4SI, RS6000_BTI_V8HI, 0, 0 },
> +
> + { P9V_BUILTIN_VEC_VSIGNEXTLL, P9V_BUILTIN_VSIGNEXTSB2D,
> +RS6000_BTI_V2DI, RS6000_BTI_V16QI, 0, 0 },
> + { P9V_BUILTIN_VEC_VSIGNEXTLL, P9V_BUILTIN_VSIGNEXTSH2D,
> +RS6000_BTI_V2DI, RS6000_BTI_V8HI, 0, 0 },
> + { P9V_BUILTIN_VEC_VSIGNEXTLL, P9V_BUILTIN_VSIGNEXTSW2D,
> +RS6000_BTI_V2DI, RS6000_BTI_V4SI, 0, 0 },
> +
>
Re: [PATCH] c++: Fix P0846 (ADL and function templates) in template [PR97010]
On 9/10/20 6:15 PM, Marek Polacek wrote:
To quickly recap, P0846 says that a name is also considered to refer to
a template if it is an unqualified-id followed by a < and name lookup
finds either one or more functions or finds nothing.
In a template, when parsing a function call that has type-dependent
arguments, we can't perform ADL right away so we set KOENIG_LOOKUP_P in
the call to remember to do it when instantiating the call
(tsubst_copy_and_build/CALL_EXPR). When the called function is a
function template, we represent the call with a TEMPLATE_ID_EXPR;
usually the operand is an OVERLOAD.
In the P0846 case though, the operand can be an IDENTIFIER_NODE, when
name lookup found nothing when parsing the template name. But we
weren't handling this correctly in tsubst_copy_and_build. First
we need to pass the FUNCTION_P argument from to
, otherwise we give a bogus error. And then in
we need to perform ADL. The rest of the changes is to
give better errors when ADL didn't find anything.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
I think I'd like to backport to 10 too.
OK for both.
gcc/cp/ChangeLog:
PR c++/97010
* pt.c (tsubst_copy_and_build) : Call
tsubst_copy_and_build explicitly instead of using the RECUR macro.
Handle a TEMPLATE_ID_EXPR with an IDENTIFIER_NODE as its operand.
: Perform ADL for a TEMPLATE_ID_EXPR with an
IDENTIFIER_NODE as its operand.
gcc/testsuite/ChangeLog:
PR c++/97010
* g++.dg/cpp2a/fn-template21.C: New test.
* g++.dg/cpp2a/fn-template22.C: New test.
---
gcc/cp/pt.c| 37 --
gcc/testsuite/g++.dg/cpp2a/fn-template21.C | 24 ++
gcc/testsuite/g++.dg/cpp2a/fn-template22.C | 25 +++
3 files changed, 77 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/fn-template21.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/fn-template22.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 30c6735dede..566e24f9bf3 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -19241,7 +19241,8 @@ out:
}
/* Like tsubst but deals with expressions and performs semantic
- analysis. FUNCTION_P is true if T is the "F" in "F (ARGS)". */
+ analysis. FUNCTION_P is true if T is the "F" in "F (ARGS)" or
+ "F (ARGS)". */
tree
tsubst_copy_and_build (tree t,
@@ -19323,7 +19324,10 @@ tsubst_copy_and_build (tree t,
case TEMPLATE_ID_EXPR:
{
tree object;
- tree templ = RECUR (TREE_OPERAND (t, 0));
+ tree templ = tsubst_copy_and_build (TREE_OPERAND (t, 0), args,
+ complain, in_decl,
+ function_p,
+ integral_constant_expression_p);
tree targs = TREE_OPERAND (t, 1);
if (targs)
@@ -19370,13 +19374,21 @@ tsubst_copy_and_build (tree t,
}
else
object = NULL_TREE;
- templ = lookup_template_function (templ, targs);
+
+ tree tid = lookup_template_function (templ, targs);
if (object)
- RETURN (build3 (COMPONENT_REF, TREE_TYPE (templ),
-object, templ, NULL_TREE));
+ RETURN (build3 (COMPONENT_REF, TREE_TYPE (tid),
+object, tid, NULL_TREE));
+ else if (identifier_p (templ))
+ {
+ /* C++20 P0846: we can encounter an IDENTIFIER_NODE here when
+ name lookup found nothing when parsing the template name. */
+ gcc_assert (cxx_dialect >= cxx20 || seen_error ());
+ RETURN (tid);
+ }
else
- RETURN (baselink_for_fns (templ));
+ RETURN (baselink_for_fns (tid));
}
case INDIRECT_REF:
@@ -19967,14 +19979,17 @@ tsubst_copy_and_build (tree t,
/* We do not perform argument-dependent lookup if normal
lookup finds a non-function, in accordance with the
- expected resolution of DR 218. */
+ resolution of DR 218. */
if (koenig_p
&& ((is_overloaded_fn (function)
/* If lookup found a member function, the Koenig lookup is
not appropriate, even if an unqualified-name was used
to denote the function. */
&& !DECL_FUNCTION_MEMBER_P (get_first_fn (function)))
- || identifier_p (function))
+ || identifier_p (function)
+ /* C++20 P0846: Lookup found nothing. */
+ || (TREE_CODE (function) == TEMPLATE_ID_EXPR
+ && identifier_p (TREE_OPERAND (function, 0
/* Only do this when substitution turns a dependent call
into a non-dependent call. */
&& type_dependent_expression_p_push (t)
@@ -19982,9 +19997,13 @@ tsubst_copy_and_build (tree t,
function = perform_koenig_lookup (function, call_args, tf_none);
Re: [PING][PATCH] correct handling of indices into arrays with elements larger than 1 (PR c++/96511)
On 10/7/20 4:11 PM, Martin Sebor wrote:
On 10/7/20 1:28 PM, Jason Merrill wrote:
On 10/7/20 11:19 AM, Martin Sebor wrote:
On 10/7/20 9:07 AM, Jason Merrill wrote:
On 10/7/20 10:42 AM, Martin Sebor wrote:
On 10/7/20 8:26 AM, Jason Merrill wrote:
On 9/28/20 6:01 PM, Martin Sebor wrote:
On 9/25/20 11:17 PM, Jason Merrill wrote:
On 9/22/20 4:05 PM, Martin Sebor wrote:
The rebased and retested patches are attached.
On 9/21/20 3:17 PM, Martin Sebor wrote:
Ping:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553906.html
(I'm working on rebasing the patch on top of the latest trunk
which
has changed some of the same code but it'd be helpful to get a
go-
ahead on substance the changes. I don't expect the rebase to
require any substantive modifications.)
Martin
On 9/14/20 4:01 PM, Martin Sebor wrote:
On 9/4/20 11:14 AM, Jason Merrill wrote:
On 9/3/20 2:44 PM, Martin Sebor wrote:
On 9/1/20 1:22 PM, Jason Merrill wrote:
On 8/11/20 12:19 PM, Martin Sebor via Gcc-patches wrote:
-Wplacement-new handles array indices and pointer offsets
the same:
by adjusting them by the size of the element. That's
correct for
the latter but wrong for the former, causing false
positives when
the element size is greater than one.
In addition, the warning doesn't even attempt to handle
arrays of
arrays. I'm not sure if I forgot or if I simply didn't
think of
it.
The attached patch corrects these oversights by replacing
most
of the -Wplacement-new code with a call to
compute_objsize which
handles all this correctly (plus more), and is also
better tested.
But even compute_objsize has bugs: it trips up while
converting
wide_int to offset_int for some pointer offset ranges.
Since
handling the C++ IL required changes in this area the
patch also
fixes that.
For review purposes, the patch affects just the middle end.
The C++ diff pretty much just removes code from the front
end.
The C++ changes are OK.
Thank you for looking at the rest as well.
-compute_objsize (tree ptr, int ostype, access_ref *pref,
- bitmap *visited, const vr_values *rvals
/* = NULL */)
+compute_objsize (tree ptr, int ostype, access_ref *pref,
bitmap *visited,
+ const vr_values *rvals)
This reformatting seems unnecessary, and I prefer to keep
the comment about the default argument.
This overload doesn't take a default argument. (There was
a stray
declaration of a similar function at the top of the file
that had
one. I've removed it.)
Ah, true.
- if (!size || TREE_CODE (size) != INTEGER_CST)
- return false;
>...
You change some failure cases in compute_objsize to return
success with a maximum range, while others continue to
return failure. This needs commentary about the design
rationale.
This is too much for a comment in the code but the
background is
this: compute_objsize initially returned the object size as
a constant.
Recently, I have enhanced it to return a range to improve
warnings for
allocated objects. With that, a failure can be turned into
success by
having the function set the range to that of the largest
object. That
should simplify the function's callers and could even improve
the detection of some invalid accesses. Once this change
is made
it might even be possible to change its return type to void.
The change that caught your eye is necessary to make the
function
a drop-in replacement for the C++ front end code which
makes this
same assumption. Without it, a number of test cases that
exercise
VLAs fail in g++.dg/warn/Wplacement-new-size-5.C. For
example:
void f (int n)
{
char a[n];
new (a - 1) int ();
}
Changing any of the other places isn't necessary for
existing tests
to pass (and I didn't want to introduce too much churn).
But I do
want to change the rest of the function along the same
lines at some
point.
Please do change the other places to be consistent; better
to have more churn than to leave the function half-updated.
That can be a separate patch if you prefer, but let's do it
now rather than later.
I've made most of these changes in the other patch (also
attached).
I'm quite happy with the result but it turned out to be a lot
more
work than either of us expected, mostly due to the amount of
testing.
I've left a couple of failing cases in place mainly as reminders
to handle them better (which means I also didn't change the
caller
to avoid testing for failures). I've also added TODO notes with
reminders to handle some of the new codes more completely.
+ special_array_member sam{ };
sam is always set by component_ref_size, so I don't think
it's necessary to initialize it at the declaration.
I find initializing pass-by-pointer local variables helpful
but
I don't insist on it.
@@ -187,7 +187,7 @@ decl_init_size (tree decl, bool min)
tree last_type = TREE_TYPE (last);
if (TREE_CODE (last_type) != ARRAY_TYPE
|| TYPE_SIZE (last_type))
- return
Re: [PATCH] c++: ICE in dependent_type_p with constrained auto [PR97052]
On 10/7/20 10:46 AM, Patrick Palka wrote:
On Tue, 6 Oct 2020, Jason Merrill wrote:
On 10/5/20 4:30 PM, Patrick Palka wrote:
On Wed, 30 Sep 2020, Jason Merrill wrote:
On 9/29/20 5:01 PM, Patrick Palka wrote:
This patch fixes an "unguarded" call to coerce_template_parms in
build_standard_check: processing_template_decl could be zero if we
we get here during processing of the first 'auto' parameter of an
abbreviated function template. In the testcase below, this leads to an
ICE when coerce_template_parms substitutes into C's dependent default
template argument.
Bootstrapped and regtested on x86_64-pc-linux-gnu and tested by building
cmcstl2 and range-v3. Does this look OK for trunk?
This looks OK, but is there a place higher in the call stack where we
should
have already set processing_template_decl?
The call stack at that point is:
build_variable_check
build_concept_check
build_type_constraint
finish_type_constraints
cp_parser_placeholder_type_specifier
cp_parser_simple_type_specifier
...
So it seems the most natural place to set processing_template_decl would
be in build_type_constraint, around the call to build_concept_check,
since that's where we create the WILDCARD_DECL that eventually reaches
coerce_template_parms.
And in order to additionally avoid a similar ICE when processing the
type constraint of a non-templated variable, we also need to guard the
call to build_concept check in make_constrained_placeholder_type. The
testcase below now contains such an example.
Setting the flag in cp_parser_placeholder_type_specifier would cover both of
those, right?
That doesn't quite work unfortunately, because if we set
processing_template_decl to non-zero around the call to
make_constrained_auto within cp_parser_placeholder_type_specifier, then
that will skew the level of the 'auto' parm built by make_auto_1,
causing the auto deduction of the non-templated variable to later fail
it seems.
Ah. Then your patch is OK.
So something like this perhaps:
-- >8 --
Subject: [PATCH] c++: ICE in dependent_type_p with constrained auto
[PR97052]
This patch fixes an "unguarded" call to coerce_template_parms in
build_standard_check: processing_template_decl could be zero if we
get here during processing of the first 'auto' parameter of an
abbreviated function template, or if we're processing the type
constraint of a non-templated variable. In the testcase below, this
leads to an ICE when coerce_template_parms instantiates C's dependent
default template argument.
gcc/cp/ChangeLog:
PR c++/97052
* constraint.cc (build_type_constraint): Temporarily increment
processing_template_decl before calling build_concept_check.
* pt.c (make_constrained_placeholder_type): Likewise.
gcc/testsuite/ChangeLog:
PR c++/97052
* g++.dg/cpp2a/concepts-defarg2: New test.
---
gcc/cp/constraint.cc | 2 ++
gcc/cp/pt.c | 2 ++
gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C | 13 +
3 files changed, 17 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index d49957a6c4a..050b55ce092 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1427,7 +1427,9 @@ tree
build_type_constraint (tree decl, tree args, tsubst_flags_t complain)
{
tree wildcard = build_nt (WILDCARD_DECL);
+ ++processing_template_decl;
tree check = build_concept_check (decl, wildcard, args, complain);
+ --processing_template_decl;
if (check == error_mark_node)
return error_mark_node;
return unpack_concept_check (check);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 72efecff37f..efdd017a4d5 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -27914,7 +27914,9 @@ make_constrained_placeholder_type (tree type, tree
con, tree args)
tree expr = tmpl;
if (TREE_CODE (con) == FUNCTION_DECL)
expr = ovl_make (tmpl);
+ ++processing_template_decl;
expr = build_concept_check (expr, type, args, tf_warning_or_error);
+ --processing_template_decl;
PLACEHOLDER_TYPE_CONSTRAINTS (type) = expr;
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C
b/gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C
new file mode 100644
index 000..a63ca4e133d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C
@@ -0,0 +1,13 @@
+// PR c++/97052
+// { dg-do compile { target c++20 } }
+
+template
+concept C = true;
+
+constexpr bool f(C auto) {
+ return true;
+}
+
+static_assert(f(0));
+
+C auto x = 0;
Re: [PING][PATCH] correct handling of indices into arrays with elements larger than 1 (PR c++/96511)
On 10/7/20 1:28 PM, Jason Merrill wrote:
On 10/7/20 11:19 AM, Martin Sebor wrote:
On 10/7/20 9:07 AM, Jason Merrill wrote:
On 10/7/20 10:42 AM, Martin Sebor wrote:
On 10/7/20 8:26 AM, Jason Merrill wrote:
On 9/28/20 6:01 PM, Martin Sebor wrote:
On 9/25/20 11:17 PM, Jason Merrill wrote:
On 9/22/20 4:05 PM, Martin Sebor wrote:
The rebased and retested patches are attached.
On 9/21/20 3:17 PM, Martin Sebor wrote:
Ping:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553906.html
(I'm working on rebasing the patch on top of the latest trunk
which
has changed some of the same code but it'd be helpful to get a go-
ahead on substance the changes. I don't expect the rebase to
require any substantive modifications.)
Martin
On 9/14/20 4:01 PM, Martin Sebor wrote:
On 9/4/20 11:14 AM, Jason Merrill wrote:
On 9/3/20 2:44 PM, Martin Sebor wrote:
On 9/1/20 1:22 PM, Jason Merrill wrote:
On 8/11/20 12:19 PM, Martin Sebor via Gcc-patches wrote:
-Wplacement-new handles array indices and pointer offsets
the same:
by adjusting them by the size of the element. That's
correct for
the latter but wrong for the former, causing false
positives when
the element size is greater than one.
In addition, the warning doesn't even attempt to handle
arrays of
arrays. I'm not sure if I forgot or if I simply didn't
think of
it.
The attached patch corrects these oversights by replacing
most
of the -Wplacement-new code with a call to compute_objsize
which
handles all this correctly (plus more), and is also better
tested.
But even compute_objsize has bugs: it trips up while
converting
wide_int to offset_int for some pointer offset ranges. Since
handling the C++ IL required changes in this area the
patch also
fixes that.
For review purposes, the patch affects just the middle end.
The C++ diff pretty much just removes code from the front
end.
The C++ changes are OK.
Thank you for looking at the rest as well.
-compute_objsize (tree ptr, int ostype, access_ref *pref,
- bitmap *visited, const vr_values *rvals
/* = NULL */)
+compute_objsize (tree ptr, int ostype, access_ref *pref,
bitmap *visited,
+ const vr_values *rvals)
This reformatting seems unnecessary, and I prefer to keep
the comment about the default argument.
This overload doesn't take a default argument. (There was a
stray
declaration of a similar function at the top of the file
that had
one. I've removed it.)
Ah, true.
- if (!size || TREE_CODE (size) != INTEGER_CST)
- return false;
>...
You change some failure cases in compute_objsize to return
success with a maximum range, while others continue to
return failure. This needs commentary about the design
rationale.
This is too much for a comment in the code but the
background is
this: compute_objsize initially returned the object size as
a constant.
Recently, I have enhanced it to return a range to improve
warnings for
allocated objects. With that, a failure can be turned into
success by
having the function set the range to that of the largest
object. That
should simplify the function's callers and could even improve
the detection of some invalid accesses. Once this change is
made
it might even be possible to change its return type to void.
The change that caught your eye is necessary to make the
function
a drop-in replacement for the C++ front end code which makes
this
same assumption. Without it, a number of test cases that
exercise
VLAs fail in g++.dg/warn/Wplacement-new-size-5.C. For example:
void f (int n)
{
char a[n];
new (a - 1) int ();
}
Changing any of the other places isn't necessary for
existing tests
to pass (and I didn't want to introduce too much churn).
But I do
want to change the rest of the function along the same lines
at some
point.
Please do change the other places to be consistent; better to
have more churn than to leave the function half-updated.
That can be a separate patch if you prefer, but let's do it
now rather than later.
I've made most of these changes in the other patch (also
attached).
I'm quite happy with the result but it turned out to be a lot
more
work than either of us expected, mostly due to the amount of
testing.
I've left a couple of failing cases in place mainly as reminders
to handle them better (which means I also didn't change the
caller
to avoid testing for failures). I've also added TODO notes with
reminders to handle some of the new codes more completely.
+ special_array_member sam{ };
sam is always set by component_ref_size, so I don't think
it's necessary to initialize it at the declaration.
I find initializing pass-by-pointer local variables helpful but
I don't insist on it.
@@ -187,7 +187,7 @@ decl_init_size (tree decl, bool min)
tree last_type = TREE_TYPE (last);
if (TREE_CODE (last_type) != ARRAY_TYPE
|| TYPE_SIZE (last_type))
- return size;
+ return size ? size :
Re: [PATCH] c++: Set the constraints of a class type sooner [PR96229]
On 9/29/20 10:26 AM, Patrick Palka wrote:
In the testcase below, during processing (at parse time) of Y's base
class X, convert_template_argument calls is_compatible_template_arg
to check if the template argument Y is no more constrained than the
parameter P. But at this point we haven't yet set Y's constraints, so
get_normalized_constraints_from_decl yields NULL_TREE as the normal form
and it caches this result in the normalized_map.
We set Y's constraints later in cp_parser_class_specifier_1 but the
stale normal form in the normalized_map remains. This ultimately causes
us to miss the constraint failure for Y because according to the
cached normal form, it's not constrained.
This patch fixes this issue by moving up the call to
associate_classtype_constraints so that we set constraints before we
begin processing its bases.
Tested on x86_64-pc-linux-gnu, and also on the cmcstlv2 and range-v3
libraries. Does this look OK to commit?
OK for trunk and 10.
gcc/cp/ChangeLog:
PR c++/96229
* parser.c (cp_parser_class_specifier_1): Move call to
associate_classtype_constraints from here to ...
(cp_parser_class_head): ... here, before we process bases.
* pt.c (is_compatible_template_arg): Correct documentation to
say "argument is _no_ more constrained than the parameter".
gcc/testsuite/ChangeLog:
PR c++/96229
* g++.dg/cpp2a/concepts-class2.C: New test.
---
gcc/cp/parser.c | 8
gcc/cp/pt.c | 7 ---
gcc/testsuite/g++.dg/cpp2a/concepts-class2.C | 11 +++
3 files changed, 19 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-class2.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 8905833fbd6..b44bdf21e1d 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -23978,10 +23978,6 @@ cp_parser_class_specifier_1 (cp_parser* parser)
= parser->in_unbraced_linkage_specification_p;
parser->in_unbraced_linkage_specification_p = false;
- // Associate constraints with the type.
- if (flag_concepts)
-type = associate_classtype_constraints (type);
-
/* Start the class. */
if (nested_name_specifier_p)
{
@@ -24749,6 +24745,10 @@ cp_parser_class_head (cp_parser* parser,
fixup_attribute_variants (type);
}
+ /* Associate constraints with the type. */
+ if (flag_concepts)
+type = associate_classtype_constraints (type);
+
/* We will have entered the scope containing the class; the names of
base classes should be looked up in that context. For example:
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 199fe658f71..96ad2025893 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -8126,9 +8126,10 @@ canonicalize_expr_argument (tree arg, tsubst_flags_t
complain)
return canon;
}
-// A template declaration can be substituted for a constrained
-// template template parameter only when the argument is more
-// constrained than the parameter.
+/* A template declaration can be substituted for a constrained
+ template template parameter only when the argument is no more
+ constrained than the parameter. */
+
static bool
is_compatible_template_arg (tree parm, tree arg)
{
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-class2.C
b/gcc/testsuite/g++.dg/cpp2a/concepts-class2.C
new file mode 100644
index 000..0ed9eb0a386
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-class2.C
@@ -0,0 +1,11 @@
+// PR c++/96229
+// { dg-do compile { target c++20 } }
+
+template concept Int = requires { T{0}; };
+template class P> struct X{ };
+template struct Y : X { };
+ struct Z{ };
+ struct W{ int i; };
+
+Y z; // { dg-error "constraint" }
+Y w;
Re: [PATCH v2] c++: Verify 'this' of NS member functions in constexpr [PR97230]
On 10/7/20 2:48 PM, Marek Polacek wrote:
On Tue, Oct 06, 2020 at 01:06:37AM -0400, Jason Merrill via Gcc-patches wrote:
On 10/1/20 1:08 PM, Marek Polacek wrote:
@@ -2496,8 +2502,72 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
new_obj = NULL_TREE;
}
}
+ /* Verify that the object we're invoking the function on is sane. */
+ else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fun)
+ /* maybe_add_lambda_conv_op creates a null 'this' pointer. */
+ && !LAMBDA_TYPE_P (CP_DECL_CONTEXT (fun)))
Let's look at lambda_static_thunk_p of ctx->call->fundef->decl instead; we
don't want to allow calling a lambda op() with a null object from other
contexts.
OK, fixed.
+{
+ tree thisarg = TREE_VEC_ELT (new_call.bindings, 0);
+ if (integer_zerop (thisarg))
+ {
+ if (!ctx->quiet)
+ error_at (loc, "member call on null pointer is not allowed "
+ "in a constant expression");
+ *non_constant_p = true;
+ result = error_mark_node;
+ }
+ else
+ {
+ STRIP_NOPS (thisarg);
+ if (TREE_CODE (thisarg) == ADDR_EXPR)
+ thisarg = TREE_OPERAND (thisarg, 0);
+ /* Detect out-of-bounds accesses. */
+ if (TREE_CODE (thisarg) == ARRAY_REF)
+ {
+ eval_and_check_array_index (ctx, thisarg, /*allow_one_past*/false,
+ non_constant_p, overflow_p);
+ if (*non_constant_p)
+ result = error_mark_node;
+ }
+ /* Detect using an inactive member of a union. */
+ else if (TREE_CODE (thisarg) == COMPONENT_REF)
+ {
+ cxx_eval_component_reference (ctx, thisarg, /*lval*/false,
+ non_constant_p, overflow_p);
+ if (*non_constant_p)
+ result = error_mark_node;
+ }
+ /* Detect other invalid accesses like
- tree result = NULL_TREE;
+ X x;
+ ()[1].foo();
+
+where we'll end up with p+ 1. */
Is there any way to combine this POINTER_PLUS_EXPR handling with
cxx_fold_pointer_plus_expression or cxx_fold_indirect_ref?
cxx_fold_pointer_plus_expression looked like a place where the new checking
could go but in the end it didn't work. E.g., it STRIP_NOPS the lhs of the
POINTER_PLUS_EXPR so any casts would get lost.
+ else if (TREE_CODE (thisarg) == POINTER_PLUS_EXPR)
+ {
+ tree op0 = TREE_OPERAND (thisarg, 0);
+ /* This shouldn't trigger if we're accessing a base class of
+the object in question. */
+ if (TREE_CODE (op0) == ADDR_EXPR
+ && DECL_P (TREE_OPERAND (op0, 0)))
+ {
+ if (!ctx->quiet)
+ {
+ tree op1 = TREE_OPERAND (thisarg, 1);
+ if (integer_onep (op1))
+ error_at (loc, "cannot dereference one-past-the-end "
+ "pointer in a constant expression");
+ else
+ error_at (loc, "cannot access element %qE of a "
+ "non-array object in a constant expression",
+ op1);
+ }
+ *non_constant_p = true;
+ result = error_mark_node;
+ }
+ }
+ }
+}
These checks seem like the requirement that "A reference shall be
initialized to refer to a valid object or function.", which matches with the
change to describe the object argument as a reference rather than a pointer.
Let's split this out into a separate function that expresses this, and
perhaps then also use it to check the initializer of a reference variable.
I've moved it to a new function called cxx_verify_object_argument.
But checking the initializers of reference variables is still not handled
in this patch, because I don't know how to do that. For something like
constexpr const int = ()[1].m;
store_init_value calls cxx_constant_init for the init, which is
(const int &) &( + 4)->m
It could also be something like
(const int &) [3].m
That could also be an object argument for a member function, right? I'm
not sure what point you're making.
Maybe cxx_eval_constant_expression/NOP_EXPR should set some flag so that
we perform some checking when we encounter a POINTER_PLUS_EXPR or an
ARRAY_REF when evaluating op0 of the NOP_EXPR?
Hmm, I suppose it might make sense to check for a valid object when we
see a conversion to reference rather than wait for initialization.
I've attached constexpr-ref13.C which is a testcase I played with; we
diagnose no errors there.
And let's call it from cxx_bind_parameters_in_call.
Done.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
This PR points out that when we're invoking a
[PATCH] MIPS/libphobos: Fix switchcontext.S assembly for MIPS I ISA
Correct MIPS I assembly build errors in switchcontext.S: .../libphobos/libdruntime/config/mips/switchcontext.S: Assembler messages: .../libphobos/libdruntime/config/mips/switchcontext.S:50: Error: opcode not supported on this processor: mips1 (mips1) `sdc1 $f20,(0*8-((6*8+4+(-6*8+4&7($sp)' etc., due to the use of the MIPS II LDC1 and SDC1 hardware instructions for FP register load and store operations. Instead use the L.D and S.D generic assembly instructions, which are strict aliases for the LDC1 and SDC1 instructions respectively and produce identical machine code where the assembly for the MIPS II or a higher ISA has been requested, however they become assembly macros and expand to compatible sequences of LWC1 and SWC1 hardware instructions where the assembly for the MIPS I ISA is in effect. libphobos/ * libdruntime/config/mips/switchcontext.S [__mips_hard_float]: Use L.D and S.D generic assembly instructions rather than LDC1 and SDC1 MIPS II hardware instructions. --- Hi, Noticed in a build of a MIPS I toolchain. I have no way to run MIPS regression-testing right now, however in `libopcodes' the L.D and S.D instructions are strict aliases valid for the MIPS II and higher ISAs, and just to double-check that I have built MIPS32r2 GCC with and without the change applied and verified with `objdump' that the respective target objects produced are identical. OK to apply to trunk, and -- as a fatal compilation error -- to backport to active release branches? Maciej --- libphobos/libdruntime/config/mips/switchcontext.S | 24 +++--- 1 file changed, 12 insertions(+), 12 deletions(-) gcc-libphobos-mips1.diff Index: gcc/libphobos/libdruntime/config/mips/switchcontext.S === --- gcc.orig/libphobos/libdruntime/config/mips/switchcontext.S +++ gcc/libphobos/libdruntime/config/mips/switchcontext.S @@ -47,12 +47,12 @@ see the files COPYING3 and COPYING.RUNTI #ifdef __mips_hard_float #define ALIGN8(val) (val + (-val & 7)) #define BELOW (ALIGN8(6 * 8 + 4)) -sdc1 $f20, (0 * 8 - BELOW)($sp) -sdc1 $f22, (1 * 8 - BELOW)($sp) -sdc1 $f24, (2 * 8 - BELOW)($sp) -sdc1 $f26, (3 * 8 - BELOW)($sp) -sdc1 $f28, (4 * 8 - BELOW)($sp) -sdc1 $f30, (5 * 8 - BELOW)($sp) +s.d $f20, (0 * 8 - BELOW)($sp) +s.d $f22, (1 * 8 - BELOW)($sp) +s.d $f24, (2 * 8 - BELOW)($sp) +s.d $f26, (3 * 8 - BELOW)($sp) +s.d $f28, (4 * 8 - BELOW)($sp) +s.d $f30, (5 * 8 - BELOW)($sp) #endif sw $ra, -4($sp) @@ -72,12 +72,12 @@ see the files COPYING3 and COPYING.RUNTI move $sp, $a1 #ifdef __mips_hard_float -ldc1 $f20, (0 * 8 - BELOW)($sp) -ldc1 $f22, (1 * 8 - BELOW)($sp) -ldc1 $f24, (2 * 8 - BELOW)($sp) -ldc1 $f26, (3 * 8 - BELOW)($sp) -ldc1 $f28, (4 * 8 - BELOW)($sp) -ldc1 $f30, (5 * 8 - BELOW)($sp) +l.d $f20, (0 * 8 - BELOW)($sp) +l.d $f22, (1 * 8 - BELOW)($sp) +l.d $f24, (2 * 8 - BELOW)($sp) +l.d $f26, (3 * 8 - BELOW)($sp) +l.d $f28, (4 * 8 - BELOW)($sp) +l.d $f30, (5 * 8 - BELOW)($sp) #endif lw $ra, -4($sp)
Re: [PATCH] libstdc++: Implement C++20 features for
On 07/10/20 09:55 -0700, Thomas Rodgers wrote: From: Thomas Rodgers New ctors and ::view() accessor for - * basic_stingbuf * basic_istringstream * basic_ostringstream * basic_stringstreamm New ::get_allocator() accessor for basic_stringbuf. libstdc++-v3/ChangeLog: * acinclude.m4 (glibcxx_SUBDIRS): Add src/c++20. * config/abi/pre/gnu.ver: Update GLIBCXX_3.4.29 for the addition of - basic_stringbuf::basic_stringbuf(allocator const&), basic_stringbuf::basic_stringbuf(openmode, allocator const&), basic_stringbuf::basic_stringbuf(basic_string&&, openmode), basic_stringbuf::basic_stringbuf(basic_stringbuf&&, allocator const&), basic_stringbuf::get_allocator(), basic_stringbuf::view(), basic_istringstream::basic_istringstream(basic_string&&, openmode), basic_istringstream::basic_istringstream(openmode, allocator const&), basic_istringstream::view(), basic_ostringstream::basic_ostringstream(basic_string&&, openmode), basic_ostringstream::basic_ostringstream(openmode, allocator const&), basic_ostringstream::view(), basic_stringstream::basic_stringstream(basic_string&&, openmode), basic_stringstream::basic_stringstream(openmode, allocator const&), basic_stringstream::view(). * configure: Regenerate. * include/std/sstream: (basic_stringbuf::basic_stringbuf(allocator const&)): New constructor. (basic_stringbuf::basic_stringbuf(openmode, allocator const&)): Likewise. (basic_stringbuf::basic_stringbuf(basic_string&&, openmode)): Likewise. (basic_stringbuf::basic_stringbuf(basic_stringbuf&&, allocator const&)): Likewise. (basic_stringbuf::get_allocator()): New method. (basic_stringbuf::view()): Likewise. (basic_istringstream::basic_istringstream(basic_string&&, openmode)): New constructor. (basic_istringstream::basic_istringstream(openmode, allocator const&)): Likewise (basic_istringstream::view()): New method. (basic_ostringstream::basic_ostringstream(basic_string&&, openmode)): New constructor. (basic_ostringstream::basic_ostringstream(openmode, allocator const&)): Likewise (basic_ostringstream::view()): New method. (basic_stringstream::basic_stringstream(basic_string&&, openmode)): New constructor. (basic_stringstream::basic_stringstream(openmode, allocator const&)): Likewise (basic_stringstream::view()): New method. * src/Makefile.in: Add c++20 directory. * src/Makefile.am: Regenerate. * src/c++20/Makefile.am: Add makefile for new sub-directory. * src/c++20/Makefile.in: Generate. * src/c++20/sstream-inst.cc: New file defining explicit instantiations for basic_stringbuf, basic_istringstream, basic_ostringstream, and basic_stringstream member functions added in C++20. * testsuite/27_io/basic_stringbuf/cons/char/2.cc: New test. * testsuite/27_io/basic_stringbuf/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringbuf/view/char/2.cc: Likewise. * testsuite/27_io/basic_stringbuf/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_istringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_istringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_istringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_istringstream/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_stringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_stringstream/view/wchar_t/2.cc: Likewise. --- .topdeps | 1 + .topmsg | 2 + libstdc++-v3/acinclude.m4 | 2 +- libstdc++-v3/config/abi/pre/gnu.ver | 60 ++ libstdc++-v3/configure| 16 +- libstdc++-v3/include/std/sstream | 198 + libstdc++-v3/src/Makefile.am | 12 +- libstdc++-v3/src/Makefile.in | 14 +- libstdc++-v3/src/c++20/Makefile.am| 105 +++ libstdc++-v3/src/c++20/Makefile.in| 735 ++ libstdc++-v3/src/c++20/sstream-inst.cc| 110 +++ .../27_io/basic_istringstream/cons/char/1.cc | 84 ++ .../basic_istringstream/cons/wchar_t/1.cc | 84 ++ .../27_io/basic_istringstream/view/char/1.cc | 34 + .../basic_istringstream/view/wchar_t/1.cc | 35 +
Re: [PING][PATCH] correct handling of indices into arrays with elements larger than 1 (PR c++/96511)
On 10/7/20 11:19 AM, Martin Sebor wrote:
On 10/7/20 9:07 AM, Jason Merrill wrote:
On 10/7/20 10:42 AM, Martin Sebor wrote:
On 10/7/20 8:26 AM, Jason Merrill wrote:
On 9/28/20 6:01 PM, Martin Sebor wrote:
On 9/25/20 11:17 PM, Jason Merrill wrote:
On 9/22/20 4:05 PM, Martin Sebor wrote:
The rebased and retested patches are attached.
On 9/21/20 3:17 PM, Martin Sebor wrote:
Ping:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553906.html
(I'm working on rebasing the patch on top of the latest trunk which
has changed some of the same code but it'd be helpful to get a go-
ahead on substance the changes. I don't expect the rebase to
require any substantive modifications.)
Martin
On 9/14/20 4:01 PM, Martin Sebor wrote:
On 9/4/20 11:14 AM, Jason Merrill wrote:
On 9/3/20 2:44 PM, Martin Sebor wrote:
On 9/1/20 1:22 PM, Jason Merrill wrote:
On 8/11/20 12:19 PM, Martin Sebor via Gcc-patches wrote:
-Wplacement-new handles array indices and pointer offsets
the same:
by adjusting them by the size of the element. That's
correct for
the latter but wrong for the former, causing false
positives when
the element size is greater than one.
In addition, the warning doesn't even attempt to handle
arrays of
arrays. I'm not sure if I forgot or if I simply didn't
think of
it.
The attached patch corrects these oversights by replacing most
of the -Wplacement-new code with a call to compute_objsize
which
handles all this correctly (plus more), and is also better
tested.
But even compute_objsize has bugs: it trips up while
converting
wide_int to offset_int for some pointer offset ranges. Since
handling the C++ IL required changes in this area the patch
also
fixes that.
For review purposes, the patch affects just the middle end.
The C++ diff pretty much just removes code from the front end.
The C++ changes are OK.
Thank you for looking at the rest as well.
-compute_objsize (tree ptr, int ostype, access_ref *pref,
- bitmap *visited, const vr_values *rvals /*
= NULL */)
+compute_objsize (tree ptr, int ostype, access_ref *pref,
bitmap *visited,
+ const vr_values *rvals)
This reformatting seems unnecessary, and I prefer to keep
the comment about the default argument.
This overload doesn't take a default argument. (There was a
stray
declaration of a similar function at the top of the file that
had
one. I've removed it.)
Ah, true.
- if (!size || TREE_CODE (size) != INTEGER_CST)
- return false;
>...
You change some failure cases in compute_objsize to return
success with a maximum range, while others continue to
return failure. This needs commentary about the design
rationale.
This is too much for a comment in the code but the background is
this: compute_objsize initially returned the object size as a
constant.
Recently, I have enhanced it to return a range to improve
warnings for
allocated objects. With that, a failure can be turned into
success by
having the function set the range to that of the largest
object. That
should simplify the function's callers and could even improve
the detection of some invalid accesses. Once this change is
made
it might even be possible to change its return type to void.
The change that caught your eye is necessary to make the
function
a drop-in replacement for the C++ front end code which makes
this
same assumption. Without it, a number of test cases that
exercise
VLAs fail in g++.dg/warn/Wplacement-new-size-5.C. For example:
void f (int n)
{
char a[n];
new (a - 1) int ();
}
Changing any of the other places isn't necessary for existing
tests
to pass (and I didn't want to introduce too much churn). But
I do
want to change the rest of the function along the same lines
at some
point.
Please do change the other places to be consistent; better to
have more churn than to leave the function half-updated. That
can be a separate patch if you prefer, but let's do it now
rather than later.
I've made most of these changes in the other patch (also
attached).
I'm quite happy with the result but it turned out to be a lot more
work than either of us expected, mostly due to the amount of
testing.
I've left a couple of failing cases in place mainly as reminders
to handle them better (which means I also didn't change the caller
to avoid testing for failures). I've also added TODO notes with
reminders to handle some of the new codes more completely.
+ special_array_member sam{ };
sam is always set by component_ref_size, so I don't think
it's necessary to initialize it at the declaration.
I find initializing pass-by-pointer local variables helpful but
I don't insist on it.
@@ -187,7 +187,7 @@ decl_init_size (tree decl, bool min)
tree last_type = TREE_TYPE (last);
if (TREE_CODE (last_type) != ARRAY_TYPE
|| TYPE_SIZE (last_type))
- return size;
+ return size ? size : TYPE_SIZE_UNIT (type);
This change seems to violate the
libgo patch committed: Handle go1.10+ correctly in match.sh
This libgo patch by Nikhil Benesch handles go1.10+ correctly in match.sh. The same issue will arise with Go 1.100, but that is a long ways off. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. Ian 33137fb1974e4b4aa5c82fbcecc2d9e2d854c394 diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index c5c02aa2392..15f01a0519b 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -1,4 +1,4 @@ -613e530547549f4220c4571ea913acbe5fa56f72 +762b74a56f7ca102a5b8da1c9d0ffce592caa46b The first line of this file holds the git revision number of the last merge done from the gofrontend repository. diff --git a/libgo/match.sh b/libgo/match.sh index 6f7b368f8bf..04db8d2c0d2 100755 --- a/libgo/match.sh +++ b/libgo/match.sh @@ -151,18 +151,18 @@ for f in $gofiles; do fi match=false ;; - $goos | $goarch | $cgotag | $cmdlinetag | "gccgo" | go1.[0-9]) + $goos | $goarch | $cgotag | $cmdlinetag | "gccgo" | go1.[0-9] | go1.[0-9][0-9]) match=true ;; - "!"$goos | "!"$goarch | "!"$cgotag | "!"$cmdlinetag | "!gccgo" | "!"go1.[0-9]) + "!"$goos | "!"$goarch | "!"$cgotag | "!"$cmdlinetag | "!gccgo" | "!"go1.[0-9] | "!"go1.[0-9][0-9]) ;; *,*) cmatch=true for ctag in `echo $tag | sed -e 's/,/ /g'`; do case $ctag in - $goos | $goarch | $cgotag | $cmdlinetag | "gccgo" | go1.[0-9]) + $goos | $goarch | $cgotag | $cmdlinetag | "gccgo" | go1.[0-9] | go1.[0-9][0-9]) ;; - "!"$goos | "!"$goarch | "!"$cgotag | "!"$cmdlinetag | "!gccgo" | "!"go1.[0-9]) + "!"$goos | "!"$goarch | "!"$cgotag | "!"$cmdlinetag | "!gccgo" | "!"go1.[0-9] | "!"go1.[0-9][0-9]) cmatch=false ;; "!"*) diff --git a/libgo/testsuite/gotest b/libgo/testsuite/gotest index 5bb27ec1631..0fd64194360 100755 --- a/libgo/testsuite/gotest +++ b/libgo/testsuite/gotest @@ -342,18 +342,18 @@ x) fi match=false ;; - $goos | $goarch | cgo | gccgo | go1.[0-9]) + $goos | $goarch | cgo | gccgo | go1.[0-9] | go1.[0-9][0-9]) match=true ;; - "!"$goos | "!"$goarch | "!cgo" | "!gccgo" | "!"go1.[0-9]) + "!"$goos | "!"$goarch | "!cgo" | "!gccgo" | "!"go1.[0-9] | "!"go1.[0-9][0-9]) ;; *,*) cmatch=true for ctag in `echo $tag | sed -e 's/,/ /g'`; do case $ctag in - $goos | $goarch | cgo | gccgo | go1.[0-9]) + $goos | $goarch | cgo | gccgo | go1.[0-9] | go1.[0-9][0-9]) ;; - "!"$goos | "!"$goarch | "!cgo" | "!gccgo" | "!"go1.[0-9]) + "!"$goos | "!"$goarch | "!cgo" | "!gccgo" | "!"go1.[0-9] | "!"go1.[0-9][0-9]) cmatch=false ;; "!"*)
Re: [PATCH v2] c++: Verify 'this' of NS member functions in constexpr [PR97230]
On Tue, Oct 06, 2020 at 01:06:37AM -0400, Jason Merrill via Gcc-patches wrote:
> On 10/1/20 1:08 PM, Marek Polacek wrote:
> > @@ -2496,8 +2502,72 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
> > tree t,
> > new_obj = NULL_TREE;
> > }
> > }
> > + /* Verify that the object we're invoking the function on is sane. */
> > + else if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fun)
> > + /* maybe_add_lambda_conv_op creates a null 'this' pointer. */
> > + && !LAMBDA_TYPE_P (CP_DECL_CONTEXT (fun)))
>
> Let's look at lambda_static_thunk_p of ctx->call->fundef->decl instead; we
> don't want to allow calling a lambda op() with a null object from other
> contexts.
OK, fixed.
> > +{
> > + tree thisarg = TREE_VEC_ELT (new_call.bindings, 0);
> > + if (integer_zerop (thisarg))
> > + {
> > + if (!ctx->quiet)
> > + error_at (loc, "member call on null pointer is not allowed "
> > + "in a constant expression");
> > + *non_constant_p = true;
> > + result = error_mark_node;
> > + }
> > + else
> > + {
> > + STRIP_NOPS (thisarg);
> > + if (TREE_CODE (thisarg) == ADDR_EXPR)
> > + thisarg = TREE_OPERAND (thisarg, 0);
> > + /* Detect out-of-bounds accesses. */
> > + if (TREE_CODE (thisarg) == ARRAY_REF)
> > + {
> > + eval_and_check_array_index (ctx, thisarg, /*allow_one_past*/false,
> > + non_constant_p, overflow_p);
> > + if (*non_constant_p)
> > + result = error_mark_node;
> > + }
> > + /* Detect using an inactive member of a union. */
> > + else if (TREE_CODE (thisarg) == COMPONENT_REF)
> > + {
> > + cxx_eval_component_reference (ctx, thisarg, /*lval*/false,
> > + non_constant_p, overflow_p);
> > + if (*non_constant_p)
> > + result = error_mark_node;
> > + }
> > + /* Detect other invalid accesses like
> > - tree result = NULL_TREE;
> > + X x;
> > + ()[1].foo();
> > +
> > +where we'll end up with p+ 1. */
>
> Is there any way to combine this POINTER_PLUS_EXPR handling with
> cxx_fold_pointer_plus_expression or cxx_fold_indirect_ref?
cxx_fold_pointer_plus_expression looked like a place where the new checking
could go but in the end it didn't work. E.g., it STRIP_NOPS the lhs of the
POINTER_PLUS_EXPR so any casts would get lost.
> > + else if (TREE_CODE (thisarg) == POINTER_PLUS_EXPR)
> > + {
> > + tree op0 = TREE_OPERAND (thisarg, 0);
> > + /* This shouldn't trigger if we're accessing a base class of
> > +the object in question. */
> > + if (TREE_CODE (op0) == ADDR_EXPR
> > + && DECL_P (TREE_OPERAND (op0, 0)))
> > + {
> > + if (!ctx->quiet)
> > + {
> > + tree op1 = TREE_OPERAND (thisarg, 1);
> > + if (integer_onep (op1))
> > + error_at (loc, "cannot dereference one-past-the-end "
> > + "pointer in a constant expression");
> > + else
> > + error_at (loc, "cannot access element %qE of a "
> > + "non-array object in a constant expression",
> > + op1);
> > + }
> > + *non_constant_p = true;
> > + result = error_mark_node;
> > + }
> > + }
> > + }
> > +}
>
> These checks seem like the requirement that "A reference shall be
> initialized to refer to a valid object or function.", which matches with the
> change to describe the object argument as a reference rather than a pointer.
> Let's split this out into a separate function that expresses this, and
> perhaps then also use it to check the initializer of a reference variable.
I've moved it to a new function called cxx_verify_object_argument.
But checking the initializers of reference variables is still not handled
in this patch, because I don't know how to do that. For something like
constexpr const int = ()[1].m;
store_init_value calls cxx_constant_init for the init, which is
(const int &) &( + 4)->m
It could also be something like
(const int &) [3].m
Maybe cxx_eval_constant_expression/NOP_EXPR should set some flag so that
we perform some checking when we encounter a POINTER_PLUS_EXPR or an
ARRAY_REF when evaluating op0 of the NOP_EXPR?
I've attached constexpr-ref13.C which is a testcase I played with; we
diagnose no errors there.
> And let's call it from cxx_bind_parameters_in_call.
Done.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
This PR points out that when we're invoking a non-static member function
on a null instance during constant evaluation, we should reject.
cxx_eval_call_expression calls cxx_bind_parameters_in_call which
evaluates function arguments, but it won't detect problems like these.
Well, ok, so
[PATCH, rs6000] rename BU_P10_MISC_2 define to BU_P10_POWERPC64_MISC_2
Hi, Rename our BU_P10_MISC_2 built-in define macro to be BU_P10_POWERPC64_MISC_2. This more accurately reflects that the macro includes the RS6000_BTM_POWERPC64 entry that is not present in the other BU_P10_MISC macros, and matches the style we used for the P7 equivalent. Should be entirely cosmetic, no codegen changes. A regtest is underway just in case. OK for trunk? Thanks, -Will gcc/ChangeLog: * gcc/config/rs6000/rs6000-builtin.def (BU_P10_MISC_2): Rename to BU_P10_POWERPC64_MISC_2. (CFUGED,CNTLZDM,CNTTZDM,PDEPD,PEXTD): Call renamed macro. diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def index e91a48ddf5fe..3eb55f0ae434 100644 --- a/gcc/config/rs6000/rs6000-builtin.def +++ b/gcc/config/rs6000/rs6000-builtin.def @@ -1109,11 +1109,11 @@ RS6000_BTM_P10, /* MASK */ \ (RS6000_BTC_ ## ATTR/* ATTR */ \ | RS6000_BTC_UNARY), \ CODE_FOR_ ## ICODE) /* ICODE */ -#define BU_P10_MISC_2(ENUM, NAME, ATTR, ICODE) \ +#define BU_P10_POWERPC64_MISC_2(ENUM, NAME, ATTR, ICODE) \ RS6000_BUILTIN_2 (P10_BUILTIN_ ## ENUM, /* ENUM */ \ "__builtin_" NAME, /* NAME */ \ RS6000_BTM_P10 \ | RS6000_BTM_POWERPC64, /* MASK */ \ (RS6000_BTC_ ## ATTR/* ATTR */ \ @@ -2725,15 +2725,15 @@ BU_P9_64BIT_2 (CMPEQB, "byte_in_set", CONST, cmpeqb) BU_P9_OVERLOAD_2 (CMPRB, "byte_in_range") BU_P9_OVERLOAD_2 (CMPRB2, "byte_in_either_range") BU_P9_OVERLOAD_2 (CMPEQB, "byte_in_set") /* Builtins for scalar instructions added in ISA 3.1 (power10). */ -BU_P10_MISC_2 (CFUGED, "cfuged", CONST, cfuged) -BU_P10_MISC_2 (CNTLZDM, "cntlzdm", CONST, cntlzdm) -BU_P10_MISC_2 (CNTTZDM, "cnttzdm", CONST, cnttzdm) -BU_P10_MISC_2 (PDEPD, "pdepd", CONST, pdepd) -BU_P10_MISC_2 (PEXTD, "pextd", CONST, pextd) +BU_P10_POWERPC64_MISC_2 (CFUGED, "cfuged", CONST, cfuged) +BU_P10_POWERPC64_MISC_2 (CNTLZDM, "cntlzdm", CONST, cntlzdm) +BU_P10_POWERPC64_MISC_2 (CNTTZDM, "cnttzdm", CONST, cnttzdm) +BU_P10_POWERPC64_MISC_2 (PDEPD, "pdepd", CONST, pdepd) +BU_P10_POWERPC64_MISC_2 (PEXTD, "pextd", CONST, pextd) /* Builtins for vector instructions added in ISA 3.1 (power10). */ BU_P10V_AV_2 (VCLRLB, "vclrlb", CONST, vclrlb) BU_P10V_AV_2 (VCLRRB, "vclrrb", CONST, vclrrb) BU_P10V_AV_2 (VCFUGED, "vcfuged", CONST, vcfuged)
Re: make sincos take type from intrinsic formal, not from result assignment
On Oct 6, 2020, Richard Biener wrote:
> So how about that mathfn_type helper instead of hard-wring this logic
> in sincos()?
Like this?
Regstrapped on x86_64-linux-gnu. Ok to install?
I'm a little unhappy with the duplication of the CASE_MATHFN* sequence,
that ought to be kept in sync, , and considered turning that whole
sequence into a #define used in both places, but that would bloat the
patch further and make it less readable, so I figured I'd propose this
one first. Please let me know if you agree this additional change would
make it better.
take type from intrinsic in sincos pass
From: Alexandre Oliva
This is a first step towards enabling the sincos optimization in Ada.
The issue this patch solves is that sincos takes the type to be looked
up with mathfn_built_in from variables or temporaries passed as
arguments to SIN and COS intrinsics. In Ada, different float types
may be used but, despite their representation equivalence, their
distinctness causes the optimization to be skipped, because they are
not the types that mathfn_built_in expects.
This patch introduces a function that maps intrinsics to the type
they're associated with, and uses that type, obtained from the
intrinsics used in calls to be optimized, to look up the correspoding
CEXPI intrinsic.
For the sake of defensive programming, when using the type obtained
from the intrinsic, it now checks that, if different types are found
for the used argument, or for other calls that use it, that the types
are interchangeable.
for gcc/ChangeLog
* builtins.c (mathfn_built_in_type): New.
* builtins.h (mathfn_built_in_type): Declare.
* tree-ssa-math-opts.c (execute_cse_sincos_1): Use it to
obtain the type expected by the intrinsic.
---
gcc/builtins.c | 147 ++
gcc/builtins.h |1
gcc/tree-ssa-math-opts.c | 17 -
3 files changed, 162 insertions(+), 3 deletions(-)
diff --git a/gcc/builtins.c b/gcc/builtins.c
index f91266e4..5649242 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -2160,6 +2160,7 @@ mathfn_built_in_2 (tree type, combined_fn fn)
switch (fn)
{
+ /* Copied to mathfn_built_in_type, please keep in sync. */
CASE_MATHFN (ACOS)
CASE_MATHFN (ACOSH)
CASE_MATHFN (ASIN)
@@ -2278,6 +2279,10 @@ mathfn_built_in_2 (tree type, combined_fn fn)
return END_BUILTINS;
}
+#undef CASE_MATHFN
+#undef CASE_MATHFN_FLOATN
+#undef CASE_MATHFN_REENT
+
/* Return mathematic function equivalent to FN but operating directly on TYPE,
if available. If IMPLICIT_P is true use the implicit builtin declaration,
otherwise use the explicit declaration. If we can't do the conversion,
@@ -2313,6 +2318,148 @@ mathfn_built_in (tree type, enum built_in_function fn)
return mathfn_built_in_1 (type, as_combined_fn (fn), /*implicit=*/ 1);
}
+/* Return the type associated with a built in function, i.e., the one
+ to be passed to mathfn_built_in to get the type-specific
+ function. */
+
+tree
+mathfn_built_in_type (combined_fn fn)
+{
+#define CASE_MATHFN(MATHFN)\
+ case BUILT_IN_##MATHFN: \
+return double_type_node; \
+ case BUILT_IN_##MATHFN##F: \
+return float_type_node;\
+ case BUILT_IN_##MATHFN##L: \
+return long_double_type_node;
+
+#define CASE_MATHFN_FLOATN(MATHFN) \
+ CASE_MATHFN(MATHFN) \
+ case BUILT_IN_##MATHFN##F16: \
+return float16_type_node; \
+ case BUILT_IN_##MATHFN##F32: \
+return float32_type_node; \
+ case BUILT_IN_##MATHFN##F64: \
+return float64_type_node; \
+ case BUILT_IN_##MATHFN##F128:\
+return float128_type_node; \
+ case BUILT_IN_##MATHFN##F32X:\
+return float32x_type_node; \
+ case BUILT_IN_##MATHFN##F64X:\
+return float64x_type_node; \
+ case BUILT_IN_##MATHFN##F128X: \
+return float128x_type_node;
+
+/* Similar to above, but appends _R after any F/L suffix. */
+#define CASE_MATHFN_REENT(MATHFN) \
+ case BUILT_IN_##MATHFN##_R: \
+return double_type_node; \
+ case BUILT_IN_##MATHFN##F_R: \
+return float_type_node;\
+ case BUILT_IN_##MATHFN##L_R: \
+return long_double_type_node;
+
+ switch (fn)
+{
+ /* Copied from mathfn_built_in_2, please keep in sync. */
+CASE_MATHFN (ACOS)
+CASE_MATHFN (ACOSH)
+CASE_MATHFN (ASIN)
+CASE_MATHFN (ASINH)
+CASE_MATHFN (ATAN)
+CASE_MATHFN (ATAN2)
+CASE_MATHFN (ATANH)
+CASE_MATHFN (CBRT)
+CASE_MATHFN_FLOATN (CEIL)
+CASE_MATHFN (CEXPI)
+CASE_MATHFN_FLOATN
Re: fixing commits?
On 10/7/20 10:43 AM, Andrew MacLeod via Gcc-patches wrote: > When I broke r11-3684 temporarily yesterday, it was fixed in the next > commit. > > But it does mean that for anyone bisecting a problem, theres a broken > commit. > > Is there a blessed way to fix that? or does it stay broken forever? > All I need to do is change a single line in gimple-range.h... > > or do we just let sleeping dogs lie? Rewriting history isn't allowed or advisable. Yea, it sucks that there's a commit in the tree that causes problems, but rewriting history would likely have a wider impact than bisection problems related to that commit. So leave it as-is. jeff
[PATCH] libstdc++: Implement C++20 features for
From: Thomas Rodgers New ctors and ::view() accessor for - * basic_stingbuf * basic_istringstream * basic_ostringstream * basic_stringstreamm New ::get_allocator() accessor for basic_stringbuf. libstdc++-v3/ChangeLog: * acinclude.m4 (glibcxx_SUBDIRS): Add src/c++20. * config/abi/pre/gnu.ver: Update GLIBCXX_3.4.29 for the addition of - basic_stringbuf::basic_stringbuf(allocator const&), basic_stringbuf::basic_stringbuf(openmode, allocator const&), basic_stringbuf::basic_stringbuf(basic_string&&, openmode), basic_stringbuf::basic_stringbuf(basic_stringbuf&&, allocator const&), basic_stringbuf::get_allocator(), basic_stringbuf::view(), basic_istringstream::basic_istringstream(basic_string&&, openmode), basic_istringstream::basic_istringstream(openmode, allocator const&), basic_istringstream::view(), basic_ostringstream::basic_ostringstream(basic_string&&, openmode), basic_ostringstream::basic_ostringstream(openmode, allocator const&), basic_ostringstream::view(), basic_stringstream::basic_stringstream(basic_string&&, openmode), basic_stringstream::basic_stringstream(openmode, allocator const&), basic_stringstream::view(). * configure: Regenerate. * include/std/sstream: (basic_stringbuf::basic_stringbuf(allocator const&)): New constructor. (basic_stringbuf::basic_stringbuf(openmode, allocator const&)): Likewise. (basic_stringbuf::basic_stringbuf(basic_string&&, openmode)): Likewise. (basic_stringbuf::basic_stringbuf(basic_stringbuf&&, allocator const&)): Likewise. (basic_stringbuf::get_allocator()): New method. (basic_stringbuf::view()): Likewise. (basic_istringstream::basic_istringstream(basic_string&&, openmode)): New constructor. (basic_istringstream::basic_istringstream(openmode, allocator const&)): Likewise (basic_istringstream::view()): New method. (basic_ostringstream::basic_ostringstream(basic_string&&, openmode)): New constructor. (basic_ostringstream::basic_ostringstream(openmode, allocator const&)): Likewise (basic_ostringstream::view()): New method. (basic_stringstream::basic_stringstream(basic_string&&, openmode)): New constructor. (basic_stringstream::basic_stringstream(openmode, allocator const&)): Likewise (basic_stringstream::view()): New method. * src/Makefile.in: Add c++20 directory. * src/Makefile.am: Regenerate. * src/c++20/Makefile.am: Add makefile for new sub-directory. * src/c++20/Makefile.in: Generate. * src/c++20/sstream-inst.cc: New file defining explicit instantiations for basic_stringbuf, basic_istringstream, basic_ostringstream, and basic_stringstream member functions added in C++20. * testsuite/27_io/basic_stringbuf/cons/char/2.cc: New test. * testsuite/27_io/basic_stringbuf/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringbuf/view/char/2.cc: Likewise. * testsuite/27_io/basic_stringbuf/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_istringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_istringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_istringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_istringstream/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_ostringstream/view/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringstream/cons/char/2.cc: Likewise. * testsuite/27_io/basic_stringstream/cons/wchar_t/2.cc: Likewise. * testsuite/27_io/basic_stringstream/view/char/2.cc: Likewise. * testsuite/27_io/basic_stringstream/view/wchar_t/2.cc: Likewise. --- .topdeps | 1 + .topmsg | 2 + libstdc++-v3/acinclude.m4 | 2 +- libstdc++-v3/config/abi/pre/gnu.ver | 60 ++ libstdc++-v3/configure| 16 +- libstdc++-v3/include/std/sstream | 198 + libstdc++-v3/src/Makefile.am | 12 +- libstdc++-v3/src/Makefile.in | 14 +- libstdc++-v3/src/c++20/Makefile.am| 105 +++ libstdc++-v3/src/c++20/Makefile.in| 735 ++ libstdc++-v3/src/c++20/sstream-inst.cc| 110 +++ .../27_io/basic_istringstream/cons/char/1.cc | 84 ++ .../basic_istringstream/cons/wchar_t/1.cc | 84 ++ .../27_io/basic_istringstream/view/char/1.cc | 34 + .../basic_istringstream/view/wchar_t/1.cc | 35 +
fixing commits?
When I broke r11-3684 temporarily yesterday, it was fixed in the next commit. But it does mean that for anyone bisecting a problem, theres a broken commit. Is there a blessed way to fix that? or does it stay broken forever? All I need to do is change a single line in gimple-range.h... or do we just let sleeping dogs lie? Andrew
Re: [patch] Add an if-exists-then-else spec function
Hello Armin, > On 1 Oct 2020, at 18:20, Armin Brauns via Gcc-patches > wrote: > > > could you please make sure to update the documentation around > gcc/doc/invoke.texi:31574 accordingly? Oh, sure. > There's already a pending patch to make it more complete at > https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553321.html, but > there shouldn't be any major conflicts between the two. Understood, thanks for the heads-up. I’ll post an updated version once I’m done retesting it together with a couple of changes addressing comments on other patches. Thanks for your feedback! Olivier
Re: [PATCH] libstdc++: Make ranges::construct_at constexpr-friendly [PR95788]
On Wed, 30 Sep 2020, Patrick Palka wrote:
> This rewrites ranges::construct_at in terms of std::construct_at so
> that we can piggy back on the compiler's existing support for
> recognizing placement new within std::construct_at during constexpr
> evaluation instead of having to additionally teach the compiler about
> ranges::construct_at.
>
> While we're here, we should also make ranges::construct_at conditionally
> noexcept like std::construct_at.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/95788
> * include/bits/ranges_uninitialized.h:
> (__construct_at_fn::operator()): Just call std::construct_at.
> Declare it conditionally noexcept.
> * testsuite/20_util/specialized_algorithms/construct_at/95788.cc:
> New test.
Here's an updated patch that std::-qualifies the calls to declval in its
function signature and adjusts formatting accordingly:
-- >8 --
Subject: [PATCH] libstdc++: Make ranges::construct_at constexpr-friendly
[PR95788]
This rewrites ranges::construct_at in terms of std::construct_at so
that we can piggyback on the compiler's existing support for
intercepting placement new within std::construct_at during constexpr
evaluation, instead of having to additionally teach the compiler about
ranges::construct_at.
While we're making changes to ranges::construct_at, this patch also
declares it conditionally noexcept and qualifies the calls to declval in
its requires-clause.
libstdc++-v3/ChangeLog:
PR libstdc++/95788
* include/bits/ranges_uninitialized.h:
(__construct_at_fn::operator()): Rewrite in terms of
std::construct_at. Declare it conditionally noexcept. Qualify
calls to declval in its requires-clause.
* testsuite/20_util/specialized_algorithms/construct_at/95788.cc:
New test.
---
.../include/bits/ranges_uninitialized.h | 10 +++--
.../construct_at/95788.cc | 41 +++
2 files changed, 48 insertions(+), 3 deletions(-)
create mode 100644
libstdc++-v3/testsuite/20_util/specialized_algorithms/construct_at/95788.cc
diff --git a/libstdc++-v3/include/bits/ranges_uninitialized.h
b/libstdc++-v3/include/bits/ranges_uninitialized.h
index d758078fc03..25e664de753 100644
--- a/libstdc++-v3/include/bits/ranges_uninitialized.h
+++ b/libstdc++-v3/include/bits/ranges_uninitialized.h
@@ -493,12 +493,16 @@ namespace ranges
struct __construct_at_fn
{
template
- requires requires { ::new (declval()) _Tp(declval<_Args>()...); }
+ requires requires {
+ ::new (std::declval()) _Tp(std::declval<_Args>()...);
+ }
constexpr _Tp*
operator()(_Tp* __location, _Args&&... __args) const
+ noexcept(noexcept(std::construct_at(__location,
+ std::forward<_Args>(__args)...)))
{
- return ::new (__detail::__voidify(*__location))
-_Tp(std::forward<_Args>(__args)...);
+ return std::construct_at(__location,
+std::forward<_Args>(__args)...);
}
};
diff --git
a/libstdc++-v3/testsuite/20_util/specialized_algorithms/construct_at/95788.cc
b/libstdc++-v3/testsuite/20_util/specialized_algorithms/construct_at/95788.cc
new file mode 100644
index 000..895332b302b
--- /dev/null
+++
b/libstdc++-v3/testsuite/20_util/specialized_algorithms/construct_at/95788.cc
@@ -0,0 +1,41 @@
+// 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
+//
Re: [PUSHED] Hybrid EVRP and testcases
On 10/7/20 11:19 AM, Aldy Hernandez wrote: On 10/7/20 8:10 AM, Richard Biener wrote: > On Tue, Oct 6, 2020 at 7:06 PM Andrew MacLeod via Gcc-patches > wrote: >> >> I have now checked in the hybrid EVRP pass. >> >> We have resolved all the issue we are aware of with a full Fedora build, >> but if any more issues arise, please let us know. (And Im sure you will :-) >> >> I made some minor tweaks. the option to the new -fevrp-mode flag are now: >> >> legacy : classic EVRP mode >> ranger : Ranger only mode >> *legacy-first : Query ranges with EVRP first, and if that fails try >> the ranger* >> ranger-first : Query the ranger first, then evrp >> ranger-trace : Ranger-only mode plus Show range tracing info in the dump >> ranger-debug : Ranger-only mode, and also include all cache debugging info >> trace : Hybrid mode with range tracing info >> debug : Hybrid mode with cache debugging as well as tracing >> >> The default is still *legacy-first*. > > We'll have to keep -fevrp-mode forever so can you instead make it a --param > since I hope it is transitional only? It certainly shouldn't be a > user-visible flag, should it? Sounds reasonable. Attached is a patch to do so. It basically moves the code from common.opt to params.opt. I also removed the undocumented modifier, as all --params are supposed to be for internal use only. OK? * common.opt (-fevrp-mode): Rename and move... * params.opt (--param=evrp-mode): ...here. * gimple-range.h (DEBUG_RANGE_CACHE): Use param_evrp_mode instead of flag_evrp_mode. * gimple-ssa-evrp.c (rvrp_folder): Same. (hybrid_folder): Same. (execute_early_vrp): Same. OK. thanks. Andrew
Re: [PING][PATCH] correct handling of indices into arrays with elements larger than 1 (PR c++/96511)
On 10/7/20 9:07 AM, Jason Merrill wrote:
On 10/7/20 10:42 AM, Martin Sebor wrote:
On 10/7/20 8:26 AM, Jason Merrill wrote:
On 9/28/20 6:01 PM, Martin Sebor wrote:
On 9/25/20 11:17 PM, Jason Merrill wrote:
On 9/22/20 4:05 PM, Martin Sebor wrote:
The rebased and retested patches are attached.
On 9/21/20 3:17 PM, Martin Sebor wrote:
Ping:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553906.html
(I'm working on rebasing the patch on top of the latest trunk which
has changed some of the same code but it'd be helpful to get a go-
ahead on substance the changes. I don't expect the rebase to
require any substantive modifications.)
Martin
On 9/14/20 4:01 PM, Martin Sebor wrote:
On 9/4/20 11:14 AM, Jason Merrill wrote:
On 9/3/20 2:44 PM, Martin Sebor wrote:
On 9/1/20 1:22 PM, Jason Merrill wrote:
On 8/11/20 12:19 PM, Martin Sebor via Gcc-patches wrote:
-Wplacement-new handles array indices and pointer offsets
the same:
by adjusting them by the size of the element. That's
correct for
the latter but wrong for the former, causing false positives
when
the element size is greater than one.
In addition, the warning doesn't even attempt to handle
arrays of
arrays. I'm not sure if I forgot or if I simply didn't
think of
it.
The attached patch corrects these oversights by replacing most
of the -Wplacement-new code with a call to compute_objsize
which
handles all this correctly (plus more), and is also better
tested.
But even compute_objsize has bugs: it trips up while converting
wide_int to offset_int for some pointer offset ranges. Since
handling the C++ IL required changes in this area the patch
also
fixes that.
For review purposes, the patch affects just the middle end.
The C++ diff pretty much just removes code from the front end.
The C++ changes are OK.
Thank you for looking at the rest as well.
-compute_objsize (tree ptr, int ostype, access_ref *pref,
- bitmap *visited, const vr_values *rvals /*
= NULL */)
+compute_objsize (tree ptr, int ostype, access_ref *pref,
bitmap *visited,
+ const vr_values *rvals)
This reformatting seems unnecessary, and I prefer to keep the
comment about the default argument.
This overload doesn't take a default argument. (There was a
stray
declaration of a similar function at the top of the file that had
one. I've removed it.)
Ah, true.
- if (!size || TREE_CODE (size) != INTEGER_CST)
- return false;
>...
You change some failure cases in compute_objsize to return
success with a maximum range, while others continue to return
failure. This needs commentary about the design rationale.
This is too much for a comment in the code but the background is
this: compute_objsize initially returned the object size as a
constant.
Recently, I have enhanced it to return a range to improve
warnings for
allocated objects. With that, a failure can be turned into
success by
having the function set the range to that of the largest
object. That
should simplify the function's callers and could even improve
the detection of some invalid accesses. Once this change is made
it might even be possible to change its return type to void.
The change that caught your eye is necessary to make the function
a drop-in replacement for the C++ front end code which makes this
same assumption. Without it, a number of test cases that
exercise
VLAs fail in g++.dg/warn/Wplacement-new-size-5.C. For example:
void f (int n)
{
char a[n];
new (a - 1) int ();
}
Changing any of the other places isn't necessary for existing
tests
to pass (and I didn't want to introduce too much churn). But
I do
want to change the rest of the function along the same lines
at some
point.
Please do change the other places to be consistent; better to
have more churn than to leave the function half-updated. That
can be a separate patch if you prefer, but let's do it now
rather than later.
I've made most of these changes in the other patch (also attached).
I'm quite happy with the result but it turned out to be a lot more
work than either of us expected, mostly due to the amount of
testing.
I've left a couple of failing cases in place mainly as reminders
to handle them better (which means I also didn't change the caller
to avoid testing for failures). I've also added TODO notes with
reminders to handle some of the new codes more completely.
+ special_array_member sam{ };
sam is always set by component_ref_size, so I don't think
it's necessary to initialize it at the declaration.
I find initializing pass-by-pointer local variables helpful but
I don't insist on it.
@@ -187,7 +187,7 @@ decl_init_size (tree decl, bool min)
tree last_type = TREE_TYPE (last);
if (TREE_CODE (last_type) != ARRAY_TYPE
|| TYPE_SIZE (last_type))
- return size;
+ return size ? size : TYPE_SIZE_UNIT (type);
This change seems to violate the comment for the function.
By my reading (and writing)
Re: [PUSHED] Hybrid EVRP and testcases
On 10/7/20 8:10 AM, Richard Biener wrote:
> On Tue, Oct 6, 2020 at 7:06 PM Andrew MacLeod via Gcc-patches
> wrote:
>>
>> I have now checked in the hybrid EVRP pass.
>>
>> We have resolved all the issue we are aware of with a full Fedora build,
>> but if any more issues arise, please let us know. (And Im sure you
will :-)
>>
>> I made some minor tweaks. the option to the new -fevrp-mode flag
are now:
>>
>> legacy : classic EVRP mode
>> ranger : Ranger only mode
>> *legacy-first : Query ranges with EVRP first, and if that fails try
>> the ranger*
>> ranger-first : Query the ranger first, then evrp
>> ranger-trace : Ranger-only mode plus Show range tracing info in
the dump
>> ranger-debug : Ranger-only mode, and also include all cache
debugging info
>> trace: Hybrid mode with range tracing info
>> debug : Hybrid mode with cache debugging as well as tracing
>>
>> The default is still *legacy-first*.
>
> We'll have to keep -fevrp-mode forever so can you instead make it a
--param
> since I hope it is transitional only? It certainly shouldn't be a
> user-visible flag, should it?
Sounds reasonable.
Attached is a patch to do so. It basically moves the code from
common.opt to params.opt. I also removed the undocumented modifier, as
all --params are supposed to be for internal use only.
OK?
* common.opt (-fevrp-mode): Rename and move...
* params.opt (--param=evrp-mode): ...here.
* gimple-range.h (DEBUG_RANGE_CACHE): Use param_evrp_mode instead
of flag_evrp_mode.
* gimple-ssa-evrp.c (rvrp_folder): Same.
(hybrid_folder): Same.
(execute_early_vrp): Same.
---
gcc/common.opt| 31 ---
gcc/gimple-range.h| 2 +-
gcc/gimple-ssa-evrp.c | 8
gcc/params.opt| 31 +++
4 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/gcc/common.opt b/gcc/common.opt
index e2bd90c450d..7e789d1c47f 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2870,37 +2870,6 @@ ftree-vrp
Common Report Var(flag_tree_vrp) Init(0) Optimization
Perform Value Range Propagation on trees.
-fevrp-mode=
-Common Undocumented Joined RejectNegative Enum(evrp_mode)
Var(flag_evrp_mode) Init(EVRP_MODE_EVRP_FIRST) Optimization
--fevrp-mode=[legacy|ranger|legacy-first|ranger-first|ranger-trace|ranger-debug|trace|debug]
Specifies the mode Early VRP should operate in.
-
-Enum
-Name(evrp_mode) Type(enum evrp_mode) UnknownError(unknown evrp mode %qs)
-
-EnumValue
-Enum(evrp_mode) String(legacy) Value(EVRP_MODE_EVRP_ONLY)
-
-EnumValue
-Enum(evrp_mode) String(ranger) Value(EVRP_MODE_RVRP_ONLY)
-
-EnumValue
-Enum(evrp_mode) String(legacy-first) Value(EVRP_MODE_EVRP_FIRST)
-
-EnumValue
-Enum(evrp_mode) String(ranger-first) Value(EVRP_MODE_RVRP_FIRST)
-
-EnumValue
-Enum(evrp_mode) String(ranger-trace) Value(EVRP_MODE_RVRP_TRACE)
-
-EnumValue
-Enum(evrp_mode) String(ranger-debug) Value(EVRP_MODE_RVRP_DEBUG)
-
-EnumValue
-Enum(evrp_mode) String(trace) Value(EVRP_MODE_TRACE)
-
-EnumValue
-Enum(evrp_mode) String(debug) Value(EVRP_MODE_DEBUG)
-
fsplit-paths
Common Report Var(flag_split_paths) Init(0) Optimization
Split paths leading to loop backedges.
diff --git a/gcc/gimple-range.h b/gcc/gimple-range.h
index 4d35e72795f..041dc7c2a97 100644
--- a/gcc/gimple-range.h
+++ b/gcc/gimple-range.h
@@ -165,6 +165,6 @@ private:
};
// Flag to enable debugging the various internal Caches.
-#define DEBUG_RANGE_CACHE (dump_file && (flag_evrp_mode & EVRP_MODE_DEBUG))
+#define DEBUG_RANGE_CACHE (dump_file && (param_evrp_mode &
EVRP_MODE_DEBUG))
#endif // GCC_GIMPLE_RANGE_STMT_H
diff --git a/gcc/gimple-ssa-evrp.c b/gcc/gimple-ssa-evrp.c
index 6be32d7a3f6..363e2ab6816 100644
--- a/gcc/gimple-ssa-evrp.c
+++ b/gcc/gimple-ssa-evrp.c
@@ -118,7 +118,7 @@ public:
rvrp_folder () : substitute_and_fold_engine (), m_simplifier ()
{
-if (flag_evrp_mode & EVRP_MODE_TRACE)
+if (param_evrp_mode & EVRP_MODE_TRACE)
m_ranger = new trace_ranger ();
else
m_ranger = new gimple_ranger ();
@@ -175,7 +175,7 @@ class hybrid_folder : public evrp_folder
public:
hybrid_folder (bool evrp_first)
{
-if (flag_evrp_mode & EVRP_MODE_TRACE)
+if (param_evrp_mode & EVRP_MODE_TRACE)
m_ranger = new trace_ranger ();
else
m_ranger = new gimple_ranger ();
@@ -307,8 +307,8 @@ execute_early_vrp ()
scev_initialize ();
calculate_dominance_info (CDI_DOMINATORS);
- // only the last 2 bits matter for choosing the folder.
- switch (flag_evrp_mode & EVRP_MODE_RVRP_FIRST)
+ // Only the last 2 bits matter for choosing the folder.
+ switch (param_evrp_mode & EVRP_MODE_RVRP_FIRST)
{
case EVRP_MODE_EVRP_ONLY:
{
diff --git a/gcc/params.opt b/gcc/params.opt
index 6f308a10da0..d770c55143b 100644
--- a/gcc/params.opt
+++ b/gcc/params.opt
@@ -102,6 +102,37 @@ Maximum size (in bytes) of objects tracked bytewise
Re: [PING][PATCH] correct handling of indices into arrays with elements larger than 1 (PR c++/96511)
On 10/7/20 10:42 AM, Martin Sebor wrote:
On 10/7/20 8:26 AM, Jason Merrill wrote:
On 9/28/20 6:01 PM, Martin Sebor wrote:
On 9/25/20 11:17 PM, Jason Merrill wrote:
On 9/22/20 4:05 PM, Martin Sebor wrote:
The rebased and retested patches are attached.
On 9/21/20 3:17 PM, Martin Sebor wrote:
Ping:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553906.html
(I'm working on rebasing the patch on top of the latest trunk which
has changed some of the same code but it'd be helpful to get a go-
ahead on substance the changes. I don't expect the rebase to
require any substantive modifications.)
Martin
On 9/14/20 4:01 PM, Martin Sebor wrote:
On 9/4/20 11:14 AM, Jason Merrill wrote:
On 9/3/20 2:44 PM, Martin Sebor wrote:
On 9/1/20 1:22 PM, Jason Merrill wrote:
On 8/11/20 12:19 PM, Martin Sebor via Gcc-patches wrote:
-Wplacement-new handles array indices and pointer offsets the
same:
by adjusting them by the size of the element. That's correct
for
the latter but wrong for the former, causing false positives
when
the element size is greater than one.
In addition, the warning doesn't even attempt to handle
arrays of
arrays. I'm not sure if I forgot or if I simply didn't think of
it.
The attached patch corrects these oversights by replacing most
of the -Wplacement-new code with a call to compute_objsize which
handles all this correctly (plus more), and is also better
tested.
But even compute_objsize has bugs: it trips up while converting
wide_int to offset_int for some pointer offset ranges. Since
handling the C++ IL required changes in this area the patch also
fixes that.
For review purposes, the patch affects just the middle end.
The C++ diff pretty much just removes code from the front end.
The C++ changes are OK.
Thank you for looking at the rest as well.
-compute_objsize (tree ptr, int ostype, access_ref *pref,
- bitmap *visited, const vr_values *rvals /* =
NULL */)
+compute_objsize (tree ptr, int ostype, access_ref *pref,
bitmap *visited,
+ const vr_values *rvals)
This reformatting seems unnecessary, and I prefer to keep the
comment about the default argument.
This overload doesn't take a default argument. (There was a stray
declaration of a similar function at the top of the file that had
one. I've removed it.)
Ah, true.
- if (!size || TREE_CODE (size) != INTEGER_CST)
- return false;
>...
You change some failure cases in compute_objsize to return
success with a maximum range, while others continue to return
failure. This needs commentary about the design rationale.
This is too much for a comment in the code but the background is
this: compute_objsize initially returned the object size as a
constant.
Recently, I have enhanced it to return a range to improve
warnings for
allocated objects. With that, a failure can be turned into
success by
having the function set the range to that of the largest
object. That
should simplify the function's callers and could even improve
the detection of some invalid accesses. Once this change is made
it might even be possible to change its return type to void.
The change that caught your eye is necessary to make the function
a drop-in replacement for the C++ front end code which makes this
same assumption. Without it, a number of test cases that exercise
VLAs fail in g++.dg/warn/Wplacement-new-size-5.C. For example:
void f (int n)
{
char a[n];
new (a - 1) int ();
}
Changing any of the other places isn't necessary for existing
tests
to pass (and I didn't want to introduce too much churn). But I do
want to change the rest of the function along the same lines at
some
point.
Please do change the other places to be consistent; better to
have more churn than to leave the function half-updated. That
can be a separate patch if you prefer, but let's do it now
rather than later.
I've made most of these changes in the other patch (also attached).
I'm quite happy with the result but it turned out to be a lot more
work than either of us expected, mostly due to the amount of
testing.
I've left a couple of failing cases in place mainly as reminders
to handle them better (which means I also didn't change the caller
to avoid testing for failures). I've also added TODO notes with
reminders to handle some of the new codes more completely.
+ special_array_member sam{ };
sam is always set by component_ref_size, so I don't think it's
necessary to initialize it at the declaration.
I find initializing pass-by-pointer local variables helpful but
I don't insist on it.
@@ -187,7 +187,7 @@ decl_init_size (tree decl, bool min)
tree last_type = TREE_TYPE (last);
if (TREE_CODE (last_type) != ARRAY_TYPE
|| TYPE_SIZE (last_type))
- return size;
+ return size ? size : TYPE_SIZE_UNIT (type);
This change seems to violate the comment for the function.
By my reading (and writing) the change is covered by the first
sentence:
Re: [PATCH] divmod: Match and expand DIVMOD even in some cases of constant divisor [PR97282]
On Tue, 6 Oct 2020 at 11:42, Christophe Lyon wrote:
>
> Hi Jakub,
>
> On Tue, 6 Oct 2020 at 10:13, Richard Biener wrote:
> >
> > On Tue, 6 Oct 2020, Jakub Jelinek wrote:
> >
> > > Hi!
> > >
> > > As written in the comment, tree-ssa-math-opts.c wouldn't create a DIVMOD
> > > ifn call for division + modulo by constant for the fear that during
> > > expansion we could generate better code for those cases.
> > > If the divisoris a power of two, that is certainly the case always,
> > > but otherwise expand_divmod can punt in many cases, e.g. if the division
> > > type's precision is above HOST_BITS_PER_WIDE_INT, we don't even call
> > > choose_multiplier, because it works on HOST_WIDE_INTs (true, something
> > > we should fix eventually now that we have wide_ints), or if pre/post shift
> > > is larger than BITS_PER_WORD.
> > >
> > > So, the following patch recognizes DIVMOD with constant last argument even
> > > when it is unclear if expand_divmod will be able to optimize it, and then
> > > during DIVMOD expansion if the divisor is constant attempts to expand it
> > > as
> > > division + modulo and if they actually don't contain any libcalls or
> > > division/modulo, they are kept as is, otherwise that sequence is thrown
> > > away
> > > and divmod optab or libcall is used.
> > >
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > OK.
> >
> > Richard.
> >
> > > 2020-10-06 Jakub Jelinek
> > >
> > > PR rtl-optimization/97282
> > > * tree-ssa-math-opts.c (divmod_candidate_p): Don't return false for
> > > constant op2 if it is not a power of two and the type has precision
> > > larger than HOST_BITS_PER_WIDE_INT or BITS_PER_WORD.
> > > * internal-fn.c (contains_call_div_mod): New function.
> > > (expand_DIVMOD): If last argument is a constant, try to expand it as
> > > TRUNC_DIV_EXPR followed by TRUNC_MOD_EXPR, but if the sequence
> > > contains any calls or {,U}{DIV,MOD} rtxes, throw it away and use
> > > divmod optab or divmod libfunc.
> > >
>
> This patch causes ICEs on arm while building newlib or glibc
>
> For instance with newlib when compiling vfwprintf.o:
> during RTL pass: expand
> In file included from
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/newlib/newlib/libc/stdio/vfprintf.c:153:
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/newlib/newlib/libc/include/stdio.h:
> In function '_vfprintf_r':
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/newlib/newlib/libc/include/stdio.h:503:9:
> internal compiler error: in int_mode_for_mode, at stor-layout.c:404
> 503 | int _vfprintf_r (struct _reent *, FILE *__restrict, const
> char *__restrict, __VALIST)
> | ^~~
> 0xaed4e3 int_mode_for_mode(machine_mode)
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/stor-layout.c:404
> 0x7ff73d emit_move_via_integer
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/expr.c:3425
> 0x808f2d emit_move_insn_1(rtx_def*, rtx_def*)
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/expr.c:3793
> 0x8092d7 emit_move_insn(rtx_def*, rtx_def*)
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/expr.c:3935
> 0x6e703f emit_library_call_value_1(int, rtx_def*, rtx_def*,
> libcall_type, machine_mode, int, std::pair*)
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/calls.c:5601
> 0xdff642 emit_library_call_value
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/rtl.h:4258
> 0xdff642 arm_expand_divmod_libfunc
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/arm/arm.c:33256
> 0x8c69af expand_DIVMOD
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/internal-fn.c:3084
> 0x7021b7 expand_call_stmt
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:2612
> 0x7021b7 expand_gimple_stmt_1
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:3686
> 0x7021b7 expand_gimple_stmt
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:3851
> 0x702cfd expand_gimple_basic_block
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:5892
> 0x70533e execute
>
> /tmp/1435347_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cfgexpand.c:6576
>
I have just filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97322
to track this.
Thanks
> Christophe
>
>
>
> > > * gcc.target/i386/pr97282.c: New test.
> > >
> > > --- gcc/tree-ssa-math-opts.c.jj 2020-10-01 10:40:10.104755999 +0200
> > > +++ gcc/tree-ssa-math-opts.c 2020-10-05 13:51:54.476628287 +0200
> > > @@ -3567,9 +3567,24 @@ divmod_candidate_p (gassign *stmt)
> > >
> > >/* Disable the transform if either is a constant, since
> > > division-by-constant
> > > may have specialized expansion. */
> > > - if (CONSTANT_CLASS_P (op1) || CONSTANT_CLASS_P
[PATCH] tree-optimization/97307 - improve sinking of loads
This improves the heuristics finding a sink location for loads that does
not cross any store.
Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
2020-10-07 Richard Biener
PR tree-optimization/97307
* tree-ssa-sink.c (statement_sink_location): Change heuristic
for not skipping stores to look for virtual definitions
rather than uses.
* gcc.dg/tree-ssa/ssa-sink-17.c: New testcase.
* gcc.dg/vect/pr65947-3.c: XFAIL.
---
gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-17.c | 15 +++
gcc/testsuite/gcc.dg/vect/pr65947-3.c | 5 ++-
gcc/tree-ssa-sink.c | 45 +++--
3 files changed, 43 insertions(+), 22 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-17.c
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-17.c
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-17.c
new file mode 100644
index 000..cf2e2a0f766
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-17.c
@@ -0,0 +1,15 @@
+/* PR tree-optimization/97307 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-sink-details" } */
+
+int pure_f(int a, int b) __attribute__((pure));
+int my_f(int a, int b)
+{
+ int x = pure_f(a, b);
+ if (a > 0)
+return x;
+ return a;
+}
+
+/* We should sink the call to pure_f to the if block. */
+/* { dg-final { scan-tree-dump "Sinking # VUSE" "sink" } } */
diff --git a/gcc/testsuite/gcc.dg/vect/pr65947-3.c
b/gcc/testsuite/gcc.dg/vect/pr65947-3.c
index 6b4077e1a62..8a2608cf0f1 100644
--- a/gcc/testsuite/gcc.dg/vect/pr65947-3.c
+++ b/gcc/testsuite/gcc.dg/vect/pr65947-3.c
@@ -51,6 +51,9 @@ main (void)
return 0;
}
-/* { dg-final { scan-tree-dump-times "LOOP VECTORIZED" 2 "vect" } } */
+/* XFAILed because of the fix for PR97307 which sinks the load of a[i],
preventing
+ if-conversion to happen. */
+/* { dg-final { scan-tree-dump-times "LOOP VECTORIZED" 2 "vect" { xfail *-*-*
} } } */
+/* { dg-final { scan-tree-dump-times "LOOP VECTORIZED" 1 "vect" } } */
/* { dg-final { scan-tree-dump-times "optimizing condition reduction with
FOLD_EXTRACT_LAST" 2 "vect" { target vect_fold_extract_last } } } */
/* { dg-final { scan-tree-dump-not "condition expression based on integer
induction." "vect" } } */
diff --git a/gcc/tree-ssa-sink.c b/gcc/tree-ssa-sink.c
index 4cc5195f2f8..ba8e5577e9c 100644
--- a/gcc/tree-ssa-sink.c
+++ b/gcc/tree-ssa-sink.c
@@ -369,10 +369,9 @@ statement_sink_location (gimple *stmt, basic_block frombb,
return false;
/* If this is a load then do not sink past any stores.
-??? This is overly simple but cheap. We basically look
-for an existing load with the same VUSE in the path to one
-of the sink candidate blocks and we adjust commondom to the
-nearest to commondom. */
+Look for virtual definitions in the path from frombb to the sink
+location computed from the real uses and if found, adjust
+that it a common dominator. */
if (gimple_vuse (stmt))
{
/* Do not sink loads from hard registers. */
@@ -383,29 +382,33 @@ statement_sink_location (gimple *stmt, basic_block frombb,
imm_use_iterator imm_iter;
use_operand_p use_p;
- basic_block found = NULL;
FOR_EACH_IMM_USE_FAST (use_p, imm_iter, gimple_vuse (stmt))
{
gimple *use_stmt = USE_STMT (use_p);
basic_block bb = gimple_bb (use_stmt);
- /* For PHI nodes the block we know sth about
-is the incoming block with the use. */
+ /* For PHI nodes the block we know sth about is the incoming block
+with the use. */
if (gimple_code (use_stmt) == GIMPLE_PHI)
- bb = EDGE_PRED (bb, PHI_ARG_INDEX_FROM_USE (use_p))->src;
- /* Any dominator of commondom would be ok with
-adjusting commondom to that block. */
- bb = nearest_common_dominator (CDI_DOMINATORS, bb, commondom);
- if (!found)
- found = bb;
- else if (dominated_by_p (CDI_DOMINATORS, bb, found))
- found = bb;
- /* If we can't improve, stop. */
- if (found == commondom)
- break;
+ {
+ /* In case the PHI node post-dominates the current insert
location
+we can disregard it. */
+ if (commondom != bb
+ && dominated_by_p (CDI_POST_DOMINATORS, commondom, bb))
+ continue;
+ bb = EDGE_PRED (bb, PHI_ARG_INDEX_FROM_USE (use_p))->src;
+ }
+ else if (!gimple_vdef (use_stmt))
+ continue;
+ /* If the use is not dominated by the path entry it is not on
+the path. */
+ if (!dominated_by_p (CDI_DOMINATORS, bb, frombb))
+ continue;
+ /* There is no easy way to
Re: [PATCH] c++: Set the constraints of a class type sooner [PR96229]
On Tue, 29 Sep 2020, Patrick Palka wrote:
> In the testcase below, during processing (at parse time) of Y's base
> class X, convert_template_argument calls is_compatible_template_arg
> to check if the template argument Y is no more constrained than the
> parameter P. But at this point we haven't yet set Y's constraints, so
> get_normalized_constraints_from_decl yields NULL_TREE as the normal form
> and it caches this result in the normalized_map.
>
> We set Y's constraints later in cp_parser_class_specifier_1 but the
> stale normal form in the normalized_map remains. This ultimately causes
> us to miss the constraint failure for Y because according to the
> cached normal form, it's not constrained.
>
> This patch fixes this issue by moving up the call to
> associate_classtype_constraints so that we set constraints before we
> begin processing its bases.
>
> Tested on x86_64-pc-linux-gnu, and also on the cmcstlv2 and range-v3
> libraries. Does this look OK to commit?
>
> gcc/cp/ChangeLog:
>
> PR c++/96229
> * parser.c (cp_parser_class_specifier_1): Move call to
> associate_classtype_constraints from here to ...
> (cp_parser_class_head): ... here, before we process bases.
> * pt.c (is_compatible_template_arg): Correct documentation to
> say "argument is _no_ more constrained than the parameter".
>
> gcc/testsuite/ChangeLog:
>
> PR c++/96229
> * g++.dg/cpp2a/concepts-class2.C: New test.
Ping
> ---
> gcc/cp/parser.c | 8
> gcc/cp/pt.c | 7 ---
> gcc/testsuite/g++.dg/cpp2a/concepts-class2.C | 11 +++
> 3 files changed, 19 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-class2.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 8905833fbd6..b44bdf21e1d 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -23978,10 +23978,6 @@ cp_parser_class_specifier_1 (cp_parser* parser)
> = parser->in_unbraced_linkage_specification_p;
>parser->in_unbraced_linkage_specification_p = false;
>
> - // Associate constraints with the type.
> - if (flag_concepts)
> -type = associate_classtype_constraints (type);
> -
>/* Start the class. */
>if (nested_name_specifier_p)
> {
> @@ -24749,6 +24745,10 @@ cp_parser_class_head (cp_parser* parser,
>fixup_attribute_variants (type);
> }
>
> + /* Associate constraints with the type. */
> + if (flag_concepts)
> +type = associate_classtype_constraints (type);
> +
>/* We will have entered the scope containing the class; the names of
> base classes should be looked up in that context. For example:
>
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 199fe658f71..96ad2025893 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -8126,9 +8126,10 @@ canonicalize_expr_argument (tree arg, tsubst_flags_t
> complain)
>return canon;
> }
>
> -// A template declaration can be substituted for a constrained
> -// template template parameter only when the argument is more
> -// constrained than the parameter.
> +/* A template declaration can be substituted for a constrained
> + template template parameter only when the argument is no more
> + constrained than the parameter. */
> +
> static bool
> is_compatible_template_arg (tree parm, tree arg)
> {
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-class2.C
> b/gcc/testsuite/g++.dg/cpp2a/concepts-class2.C
> new file mode 100644
> index 000..0ed9eb0a386
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-class2.C
> @@ -0,0 +1,11 @@
> +// PR c++/96229
> +// { dg-do compile { target c++20 } }
> +
> +template concept Int = requires { T{0}; };
> +template class P> struct X{ };
> +template struct Y : X { };
> + struct Z{ };
> + struct W{ int i; };
> +
> +Y z; // { dg-error "constraint" }
> +Y w;
> --
> 2.28.0.618.g9bc233ae1c
>
>
Re: [PATCH] c++: ICE in dependent_type_p with constrained auto [PR97052]
On Tue, 6 Oct 2020, Jason Merrill wrote:
> On 10/5/20 4:30 PM, Patrick Palka wrote:
> > On Wed, 30 Sep 2020, Jason Merrill wrote:
> >
> > > On 9/29/20 5:01 PM, Patrick Palka wrote:
> > > > This patch fixes an "unguarded" call to coerce_template_parms in
> > > > build_standard_check: processing_template_decl could be zero if we
> > > > we get here during processing of the first 'auto' parameter of an
> > > > abbreviated function template. In the testcase below, this leads to an
> > > > ICE when coerce_template_parms substitutes into C's dependent default
> > > > template argument.
> > > >
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu and tested by building
> > > > cmcstl2 and range-v3. Does this look OK for trunk?
> > >
> > > This looks OK, but is there a place higher in the call stack where we
> > > should
> > > have already set processing_template_decl?
> >
> > The call stack at that point is:
> >
> >build_variable_check
> >build_concept_check
> >build_type_constraint
> >finish_type_constraints
> >cp_parser_placeholder_type_specifier
> >cp_parser_simple_type_specifier
> >...
> >
> > So it seems the most natural place to set processing_template_decl would
> > be in build_type_constraint, around the call to build_concept_check,
> > since that's where we create the WILDCARD_DECL that eventually reaches
> > coerce_template_parms.
> >
> > And in order to additionally avoid a similar ICE when processing the
> > type constraint of a non-templated variable, we also need to guard the
> > call to build_concept check in make_constrained_placeholder_type. The
> > testcase below now contains such an example.
>
> Setting the flag in cp_parser_placeholder_type_specifier would cover both of
> those, right?
That doesn't quite work unfortunately, because if we set
processing_template_decl to non-zero around the call to
make_constrained_auto within cp_parser_placeholder_type_specifier, then
that will skew the level of the 'auto' parm built by make_auto_1,
causing the auto deduction of the non-templated variable to later fail
it seems.
>
> > So something like this perhaps:
> >
> > -- >8 --
> >
> > Subject: [PATCH] c++: ICE in dependent_type_p with constrained auto
> > [PR97052]
> >
> > This patch fixes an "unguarded" call to coerce_template_parms in
> > build_standard_check: processing_template_decl could be zero if we
> > get here during processing of the first 'auto' parameter of an
> > abbreviated function template, or if we're processing the type
> > constraint of a non-templated variable. In the testcase below, this
> > leads to an ICE when coerce_template_parms instantiates C's dependent
> > default template argument.
> >
> > gcc/cp/ChangeLog:
> >
> > PR c++/97052
> > * constraint.cc (build_type_constraint): Temporarily increment
> > processing_template_decl before calling build_concept_check.
> > * pt.c (make_constrained_placeholder_type): Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/97052
> > * g++.dg/cpp2a/concepts-defarg2: New test.
> > ---
> > gcc/cp/constraint.cc | 2 ++
> > gcc/cp/pt.c | 2 ++
> > gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C | 13 +
> > 3 files changed, 17 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C
> >
> > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > index d49957a6c4a..050b55ce092 100644
> > --- a/gcc/cp/constraint.cc
> > +++ b/gcc/cp/constraint.cc
> > @@ -1427,7 +1427,9 @@ tree
> > build_type_constraint (tree decl, tree args, tsubst_flags_t complain)
> > {
> > tree wildcard = build_nt (WILDCARD_DECL);
> > + ++processing_template_decl;
> > tree check = build_concept_check (decl, wildcard, args, complain);
> > + --processing_template_decl;
> > if (check == error_mark_node)
> > return error_mark_node;
> > return unpack_concept_check (check);
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index 72efecff37f..efdd017a4d5 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -27914,7 +27914,9 @@ make_constrained_placeholder_type (tree type, tree
> > con, tree args)
> > tree expr = tmpl;
> > if (TREE_CODE (con) == FUNCTION_DECL)
> > expr = ovl_make (tmpl);
> > + ++processing_template_decl;
> > expr = build_concept_check (expr, type, args, tf_warning_or_error);
> > + --processing_template_decl;
> > PLACEHOLDER_TYPE_CONSTRAINTS (type) = expr;
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C
> > new file mode 100644
> > index 000..a63ca4e133d
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-defarg2.C
> > @@ -0,0 +1,13 @@
> > +// PR c++/97052
> > +// { dg-do compile { target c++20 } }
> > +
> > +template
> > +concept C = true;
> > +
> > +constexpr bool f(C auto) {
> > + return true;
> > +}
> > +
> > +static_assert(f(0));
> >
Re: [PING][PATCH] correct handling of indices into arrays with elements larger than 1 (PR c++/96511)
On 10/7/20 8:26 AM, Jason Merrill wrote:
On 9/28/20 6:01 PM, Martin Sebor wrote:
On 9/25/20 11:17 PM, Jason Merrill wrote:
On 9/22/20 4:05 PM, Martin Sebor wrote:
The rebased and retested patches are attached.
On 9/21/20 3:17 PM, Martin Sebor wrote:
Ping:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553906.html
(I'm working on rebasing the patch on top of the latest trunk which
has changed some of the same code but it'd be helpful to get a go-
ahead on substance the changes. I don't expect the rebase to
require any substantive modifications.)
Martin
On 9/14/20 4:01 PM, Martin Sebor wrote:
On 9/4/20 11:14 AM, Jason Merrill wrote:
On 9/3/20 2:44 PM, Martin Sebor wrote:
On 9/1/20 1:22 PM, Jason Merrill wrote:
On 8/11/20 12:19 PM, Martin Sebor via Gcc-patches wrote:
-Wplacement-new handles array indices and pointer offsets the
same:
by adjusting them by the size of the element. That's correct for
the latter but wrong for the former, causing false positives when
the element size is greater than one.
In addition, the warning doesn't even attempt to handle arrays of
arrays. I'm not sure if I forgot or if I simply didn't think of
it.
The attached patch corrects these oversights by replacing most
of the -Wplacement-new code with a call to compute_objsize which
handles all this correctly (plus more), and is also better
tested.
But even compute_objsize has bugs: it trips up while converting
wide_int to offset_int for some pointer offset ranges. Since
handling the C++ IL required changes in this area the patch also
fixes that.
For review purposes, the patch affects just the middle end.
The C++ diff pretty much just removes code from the front end.
The C++ changes are OK.
Thank you for looking at the rest as well.
-compute_objsize (tree ptr, int ostype, access_ref *pref,
- bitmap *visited, const vr_values *rvals /* =
NULL */)
+compute_objsize (tree ptr, int ostype, access_ref *pref,
bitmap *visited,
+ const vr_values *rvals)
This reformatting seems unnecessary, and I prefer to keep the
comment about the default argument.
This overload doesn't take a default argument. (There was a stray
declaration of a similar function at the top of the file that had
one. I've removed it.)
Ah, true.
- if (!size || TREE_CODE (size) != INTEGER_CST)
- return false;
>...
You change some failure cases in compute_objsize to return
success with a maximum range, while others continue to return
failure. This needs commentary about the design rationale.
This is too much for a comment in the code but the background is
this: compute_objsize initially returned the object size as a
constant.
Recently, I have enhanced it to return a range to improve
warnings for
allocated objects. With that, a failure can be turned into
success by
having the function set the range to that of the largest object.
That
should simplify the function's callers and could even improve
the detection of some invalid accesses. Once this change is made
it might even be possible to change its return type to void.
The change that caught your eye is necessary to make the function
a drop-in replacement for the C++ front end code which makes this
same assumption. Without it, a number of test cases that exercise
VLAs fail in g++.dg/warn/Wplacement-new-size-5.C. For example:
void f (int n)
{
char a[n];
new (a - 1) int ();
}
Changing any of the other places isn't necessary for existing tests
to pass (and I didn't want to introduce too much churn). But I do
want to change the rest of the function along the same lines at
some
point.
Please do change the other places to be consistent; better to
have more churn than to leave the function half-updated. That
can be a separate patch if you prefer, but let's do it now rather
than later.
I've made most of these changes in the other patch (also attached).
I'm quite happy with the result but it turned out to be a lot more
work than either of us expected, mostly due to the amount of testing.
I've left a couple of failing cases in place mainly as reminders
to handle them better (which means I also didn't change the caller
to avoid testing for failures). I've also added TODO notes with
reminders to handle some of the new codes more completely.
+ special_array_member sam{ };
sam is always set by component_ref_size, so I don't think it's
necessary to initialize it at the declaration.
I find initializing pass-by-pointer local variables helpful but
I don't insist on it.
@@ -187,7 +187,7 @@ decl_init_size (tree decl, bool min)
tree last_type = TREE_TYPE (last);
if (TREE_CODE (last_type) != ARRAY_TYPE
|| TYPE_SIZE (last_type))
- return size;
+ return size ? size : TYPE_SIZE_UNIT (type);
This change seems to violate the comment for the function.
By my reading (and writing) the change is covered by the first
sentence:
Returns the size of the object designated by DECL
Re: [PING][PATCH] correct handling of indices into arrays with elements larger than 1 (PR c++/96511)
On 9/28/20 6:01 PM, Martin Sebor wrote:
On 9/25/20 11:17 PM, Jason Merrill wrote:
On 9/22/20 4:05 PM, Martin Sebor wrote:
The rebased and retested patches are attached.
On 9/21/20 3:17 PM, Martin Sebor wrote:
Ping:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553906.html
(I'm working on rebasing the patch on top of the latest trunk which
has changed some of the same code but it'd be helpful to get a go-
ahead on substance the changes. I don't expect the rebase to
require any substantive modifications.)
Martin
On 9/14/20 4:01 PM, Martin Sebor wrote:
On 9/4/20 11:14 AM, Jason Merrill wrote:
On 9/3/20 2:44 PM, Martin Sebor wrote:
On 9/1/20 1:22 PM, Jason Merrill wrote:
On 8/11/20 12:19 PM, Martin Sebor via Gcc-patches wrote:
-Wplacement-new handles array indices and pointer offsets the
same:
by adjusting them by the size of the element. That's correct for
the latter but wrong for the former, causing false positives when
the element size is greater than one.
In addition, the warning doesn't even attempt to handle arrays of
arrays. I'm not sure if I forgot or if I simply didn't think of
it.
The attached patch corrects these oversights by replacing most
of the -Wplacement-new code with a call to compute_objsize which
handles all this correctly (plus more), and is also better tested.
But even compute_objsize has bugs: it trips up while converting
wide_int to offset_int for some pointer offset ranges. Since
handling the C++ IL required changes in this area the patch also
fixes that.
For review purposes, the patch affects just the middle end.
The C++ diff pretty much just removes code from the front end.
The C++ changes are OK.
Thank you for looking at the rest as well.
-compute_objsize (tree ptr, int ostype, access_ref *pref,
- bitmap *visited, const vr_values *rvals /* =
NULL */)
+compute_objsize (tree ptr, int ostype, access_ref *pref,
bitmap *visited,
+ const vr_values *rvals)
This reformatting seems unnecessary, and I prefer to keep the
comment about the default argument.
This overload doesn't take a default argument. (There was a stray
declaration of a similar function at the top of the file that had
one. I've removed it.)
Ah, true.
- if (!size || TREE_CODE (size) != INTEGER_CST)
- return false;
>...
You change some failure cases in compute_objsize to return
success with a maximum range, while others continue to return
failure. This needs commentary about the design rationale.
This is too much for a comment in the code but the background is
this: compute_objsize initially returned the object size as a
constant.
Recently, I have enhanced it to return a range to improve
warnings for
allocated objects. With that, a failure can be turned into
success by
having the function set the range to that of the largest object.
That
should simplify the function's callers and could even improve
the detection of some invalid accesses. Once this change is made
it might even be possible to change its return type to void.
The change that caught your eye is necessary to make the function
a drop-in replacement for the C++ front end code which makes this
same assumption. Without it, a number of test cases that exercise
VLAs fail in g++.dg/warn/Wplacement-new-size-5.C. For example:
void f (int n)
{
char a[n];
new (a - 1) int ();
}
Changing any of the other places isn't necessary for existing tests
to pass (and I didn't want to introduce too much churn). But I do
want to change the rest of the function along the same lines at some
point.
Please do change the other places to be consistent; better to have
more churn than to leave the function half-updated. That can be a
separate patch if you prefer, but let's do it now rather than later.
I've made most of these changes in the other patch (also attached).
I'm quite happy with the result but it turned out to be a lot more
work than either of us expected, mostly due to the amount of testing.
I've left a couple of failing cases in place mainly as reminders
to handle them better (which means I also didn't change the caller
to avoid testing for failures). I've also added TODO notes with
reminders to handle some of the new codes more completely.
+ special_array_member sam{ };
sam is always set by component_ref_size, so I don't think it's
necessary to initialize it at the declaration.
I find initializing pass-by-pointer local variables helpful but
I don't insist on it.
@@ -187,7 +187,7 @@ decl_init_size (tree decl, bool min)
tree last_type = TREE_TYPE (last);
if (TREE_CODE (last_type) != ARRAY_TYPE
|| TYPE_SIZE (last_type))
- return size;
+ return size ? size : TYPE_SIZE_UNIT (type);
This change seems to violate the comment for the function.
By my reading (and writing) the change is covered by the first
sentence:
Returns the size of the object designated by DECL considering
its initializer if it either
Re: [PUSHED] Hybrid EVRP and testcases
On 10/7/20 2:10 AM, Richard Biener wrote: On Tue, Oct 6, 2020 at 7:06 PM Andrew MacLeod via Gcc-patches wrote: I have now checked in the hybrid EVRP pass. We have resolved all the issue we are aware of with a full Fedora build, but if any more issues arise, please let us know. (And Im sure you will :-) I made some minor tweaks. the option to the new -fevrp-mode flag are now: legacy : classic EVRP mode ranger : Ranger only mode *legacy-first : Query ranges with EVRP first, and if that fails try the ranger* ranger-first : Query the ranger first, then evrp ranger-trace : Ranger-only mode plus Show range tracing info in the dump ranger-debug : Ranger-only mode, and also include all cache debugging info trace: Hybrid mode with range tracing info debug : Hybrid mode with cache debugging as well as tracing The default is still *legacy-first*. We'll have to keep -fevrp-mode forever so can you instead make it a --param since I hope it is transitional only? It certainly shouldn't be a user-visible flag, should it? Richard. doesnt need to be user visible, just to give us the ability to turn things on and off. Sure. I'll try to get to that shortly.
[PUSHED] Off by one final fix.
On 10/6/20 2:59 PM, Andrew MacLeod via Gcc-patches wrote:
On 10/6/20 2:41 PM, Andreas Schwab wrote:
On Okt 06 2020, Andrew MacLeod via Gcc-patches wrote:
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 7031a823138..02a952bf81f 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -668,13 +668,12 @@ irange_allocator::allocate (unsigned num_pairs)
if (num_pairs < 2)
num_pairs = 2;
- struct newir {
- irange range;
- tree mem[2];
- };
- size_t nbytes = (sizeof (newir) + sizeof (tree) * 2 * (num_pairs
- 1));
- struct newir *r = (newir *) obstack_alloc (_obstack, nbytes);
- return new (r) irange (r->mem, num_pairs);
+ size_t nbytes = sizeof (tree) * 2 * num_pairs;
+
+ // Allocate the irnge and required memory for the vector
Typo: irange
Andreas.
Ha. Its all good now. THIS is actually the final final FINAL patch
which is going thru testing.
And its now in.
2020-10-06 Andrew MacLeod
* value-range.h (irange_allocator::allocate): Allocate in two hunks
instead of using the variably-sized trailing array approach.
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 7031a823138..63c96204cda 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -668,13 +668,12 @@ irange_allocator::allocate (unsigned num_pairs)
if (num_pairs < 2)
num_pairs = 2;
- struct newir {
-irange range;
-tree mem[2];
- };
- size_t nbytes = (sizeof (newir) + sizeof (tree) * 2 * (num_pairs - 1));
- struct newir *r = (newir *) obstack_alloc (_obstack, nbytes);
- return new (r) irange (r->mem, num_pairs);
+ size_t nbytes = sizeof (tree) * 2 * num_pairs;
+
+ // Allocate the irange and required memory for the vector.
+ void *r = obstack_alloc (_obstack, sizeof (irange));
+ tree *mem = (tree *) obstack_alloc (_obstack, nbytes);
+ return new (r) irange (mem, num_pairs);
}
inline irange *
[committed] analyzer: handle C++ argument numbers and "this" [PR97116]
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r11-3701-g2f7c50b7091c09d665aaf27173aacf34c9904e4c.
gcc/analyzer/ChangeLog:
PR analyzer/97116
* sm-malloc.cc (method_p): New.
(describe_argument_index): New.
(inform_nonnull_attribute): Use describe_argument_index.
(possible_null_arg::describe_final_event): Likewise.
(null_arg::describe_final_event): Likewise.
gcc/testsuite/ChangeLog:
PR analyzer/97116
* g++.dg/analyzer/pr97116.C: New test.
---
gcc/analyzer/sm-malloc.cc | 61 +++--
gcc/testsuite/g++.dg/analyzer/pr97116.C | 39
2 files changed, 86 insertions(+), 14 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/analyzer/pr97116.C
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index 6293d7885cd..fd12a358176 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -562,15 +562,40 @@ public:
};
+/* Return true if FNDECL is a C++ method. */
+
+static bool
+method_p (tree fndecl)
+{
+ return TREE_CODE (TREE_TYPE (fndecl)) == METHOD_TYPE;
+}
+
+/* Return a 1-based description of ARG_IDX (0-based) of FNDECL.
+ Compare with %P in the C++ FE (implemented in cp/error.c: parm_to_string
+ as called from cp_printer). */
+
+static label_text
+describe_argument_index (tree fndecl, int arg_idx)
+{
+ if (method_p (fndecl))
+if (arg_idx == 0)
+ return label_text::borrow ("'this'");
+ pretty_printer pp;
+ pp_printf (, "%u", arg_idx + 1 - method_p (fndecl));
+ return label_text::take (xstrdup (pp_formatted_text ()));
+}
+
/* Subroutine for use by possible_null_arg::emit and null_arg::emit.
Issue a note informing that the pertinent argument must be non-NULL. */
static void
inform_nonnull_attribute (tree fndecl, int arg_idx)
{
+ label_text arg_desc = describe_argument_index (fndecl, arg_idx);
inform (DECL_SOURCE_LOCATION (fndecl),
- "argument %u of %qD must be non-null",
- arg_idx + 1, fndecl);
+ "argument %s of %qD must be non-null",
+ arg_desc.m_buffer, fndecl);
+ arg_desc.maybe_free ();
/* Ideally we would use the location of the parm and underline the
attribute also - but we don't have the location_t values at this point
in the middle-end.
@@ -618,15 +643,19 @@ public:
label_text describe_final_event (const evdesc::final_event ) FINAL
OVERRIDE
{
+label_text arg_desc = describe_argument_index (m_fndecl, m_arg_idx);
+label_text result;
if (m_origin_of_unchecked_event.known_p ())
- return ev.formatted_print ("argument %u (%qE) from %@ could be NULL"
-" where non-null expected",
-m_arg_idx + 1, ev.m_expr,
-_origin_of_unchecked_event);
+ result = ev.formatted_print ("argument %s (%qE) from %@ could be NULL"
+ " where non-null expected",
+ arg_desc.m_buffer, ev.m_expr,
+ _origin_of_unchecked_event);
else
- return ev.formatted_print ("argument %u (%qE) could be NULL"
-" where non-null expected",
-m_arg_idx + 1, ev.m_expr);
+ result = ev.formatted_print ("argument %s (%qE) could be NULL"
+ " where non-null expected",
+ arg_desc.m_buffer, ev.m_expr);
+arg_desc.maybe_free ();
+return result;
}
private:
@@ -714,13 +743,17 @@ public:
label_text describe_final_event (const evdesc::final_event ) FINAL
OVERRIDE
{
+label_text arg_desc = describe_argument_index (m_fndecl, m_arg_idx);
+label_text result;
if (zerop (ev.m_expr))
- return ev.formatted_print ("argument %u NULL where non-null expected",
-m_arg_idx + 1);
+ result = ev.formatted_print ("argument %s NULL where non-null expected",
+ arg_desc.m_buffer);
else
- return ev.formatted_print ("argument %u (%qE) NULL"
-" where non-null expected",
-m_arg_idx + 1, ev.m_expr);
+ result = ev.formatted_print ("argument %s (%qE) NULL"
+ " where non-null expected",
+ arg_desc.m_buffer, ev.m_expr);
+arg_desc.maybe_free ();
+return result;
}
private:
diff --git a/gcc/testsuite/g++.dg/analyzer/pr97116.C
b/gcc/testsuite/g++.dg/analyzer/pr97116.C
new file mode 100644
index 000..d8e08a73172
--- /dev/null
+++ b/gcc/testsuite/g++.dg/analyzer/pr97116.C
@@ -0,0 +1,39 @@
+#include
+#include
+
+struct foo
+{
+ foo (int i) : m_i (i) {} // { dg-message "argument 'this' of
'foo::foo\\(int\\)' must be non-null" "note" }
+
+ int get () const { return m_i; } // { dg-message "argument 'this' of
[committed] Add -fdiagnostics-path-format=separate-events to -fdiagnostics-plain-output
The path-printing default of -fdiagnostics-path-format=inline-events
interacted poorly with -fdiagnostics-plain-output, so it makes most
sense to add -fdiagnostics-path-format=separate-events to
-fdiagnostics-plain-output.
Seen when adding an experimental analyzer plugin to gcc.dg/plugin.exp.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as 7345c89ecb1a31ce96c6789bffc7183268a040b3.
gcc/ChangeLog:
* doc/invoke.texi (-fdiagnostics-plain-output): Add
-fdiagnostics-path-format=separate-events to list of
options injected by -fdiagnostics-plain-output.
* opts-common.c (decode_cmdline_options_to_array): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/analyzer/analyzer.exp (DEFAULT_CXXFLAGS): Remove
-fdiagnostics-path-format=separate-events.
* gcc.dg/analyzer/analyzer.exp (DEFAULT_CFLAGS): Likewise.
* gcc.dg/plugin/diagnostic-path-format-default.c: Rename to...
* gcc.dg/plugin/diagnostic-path-format-plain.c: ...this. Remove
dg-options directive. Copy remainder of test from
diagnostic-path-format-separate-events.c.
* gcc.dg/plugin/diagnostic-test-paths-2.c: Add
-fdiagnostics-path-format=inline-events to options.
Fix expected output for location of conditional within "for" loop.
* gcc.dg/plugin/plugin.exp (plugin_test_list): Update for
renaming.
* gfortran.dg/analyzer/analyzer.exp (DEFAULT_FFLAGS): Remove
-fdiagnostics-path-format=separate-events.
---
gcc/doc/invoke.texi | 3 +-
gcc/opts-common.c | 1 +
gcc/testsuite/g++.dg/analyzer/analyzer.exp| 2 +-
gcc/testsuite/gcc.dg/analyzer/analyzer.exp| 2 +-
.../plugin/diagnostic-path-format-default.c | 142 --
.../plugin/diagnostic-path-format-plain.c | 42 ++
.../gcc.dg/plugin/diagnostic-test-paths-2.c | 8 +-
gcc/testsuite/gcc.dg/plugin/plugin.exp| 2 +-
.../gfortran.dg/analyzer/analyzer.exp | 2 +-
9 files changed, 53 insertions(+), 151 deletions(-)
delete mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-default.c
create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-plain.c
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index f623467b763..d8bc4cc3267 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4345,7 +4345,8 @@ options:
@gccoptlist{-fno-diagnostics-show-caret @gol
-fno-diagnostics-show-line-numbers @gol
-fdiagnostics-color=never @gol
--fdiagnostics-urls=never}
+-fdiagnostics-urls=never @gol
+-fdiagnostics-path-format=separate-events}
In the future, if GCC changes the default appearance of its diagnostics, the
corresponding option to disable the new behavior will be added to this list.
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 237e4ce454e..8ec8c1ec1a8 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -1000,6 +1000,7 @@ decode_cmdline_options_to_array (unsigned int argc, const
char **argv,
"-fno-diagnostics-show-line-numbers",
"-fdiagnostics-color=never",
"-fdiagnostics-urls=never",
+ "-fdiagnostics-path-format=separate-events",
};
const int num_expanded = ARRAY_SIZE (expanded_args);
opt_array_len += num_expanded - 1;
diff --git a/gcc/testsuite/g++.dg/analyzer/analyzer.exp
b/gcc/testsuite/g++.dg/analyzer/analyzer.exp
index 60262f678ee..78edff2d11b 100644
--- a/gcc/testsuite/g++.dg/analyzer/analyzer.exp
+++ b/gcc/testsuite/g++.dg/analyzer/analyzer.exp
@@ -29,7 +29,7 @@ if [info exists DEFAULT_CXXFLAGS] then {
}
# If a testcase doesn't have special options, use these.
-set DEFAULT_CXXFLAGS " -fanalyzer -fdiagnostics-path-format=separate-events
-Wanalyzer-too-complex -fanalyzer-call-summaries"
+set DEFAULT_CXXFLAGS " -fanalyzer -Wanalyzer-too-complex
-fanalyzer-call-summaries"
# Initialize `dg'.
dg-init
diff --git a/gcc/testsuite/gcc.dg/analyzer/analyzer.exp
b/gcc/testsuite/gcc.dg/analyzer/analyzer.exp
index ac9c49511aa..d72fef37ede 100644
--- a/gcc/testsuite/gcc.dg/analyzer/analyzer.exp
+++ b/gcc/testsuite/gcc.dg/analyzer/analyzer.exp
@@ -30,7 +30,7 @@ if [info exists DEFAULT_CFLAGS] then {
}
# If a testcase doesn't have special options, use these.
-set DEFAULT_CFLAGS "-fanalyzer -fdiagnostics-path-format=separate-events
-Wanalyzer-too-complex -fanalyzer-call-summaries"
+set DEFAULT_CFLAGS "-fanalyzer -Wanalyzer-too-complex
-fanalyzer-call-summaries"
# Initialize `dg'.
dg-init
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-default.c
b/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-default.c
deleted file mode 100644
index 5712dbd6472..000
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-path-format-default.c
+++ /dev/null
@@ -1,142 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-fdiagnostics-show-caret" } */
-
-#include
-
-void *wrapped_malloc (size_t size)
-{
c++: block-scope externs get an alias [PR95677,PR31775,PR95677]
This patch improves block-scope extern handling by always injecting a
hidden copy into the enclosing namespace (or using a match already
there). This hidden copy will be revealed if the user explicitly
declares it later. We can get from the DECL_LOCAL_DECL_P local extern
to the alias via DECL_LOCAL_DECL_ALIAS. This fixes several bugs and
removes the kludgy per-function extern_decl_map. We only do this
pushing for non-dependent local externs -- dependent ones will be
pushed during instantiation.
User code that expected to be able to handle incompatible local
externs in different block-scopes will no longer work. That code is
ill-formed. (always was, despite what 31775 claimed). I had to
adjust a number of testcases that fell into this.
I tried using DECL_VALUE_EXPR, but that didn't work out. Due to
constexpr requirements we have to do the replacement very late (it
happens in the gimplifier). Consider:
extern int l[]; // #1
constexpr bool foo ()
{
extern int l[3]; // this does not complete the type of decl #1
constexpr int *p = [2]; // ok
return !p;
}
This requirement, coupled with our use of the common folding machinery
makes pr97306 hard to fix, as we end up with an expression containing
the two different decls for 'l', and only the c++ FE knows how to
reconcile those. I punted on this.
gcc/cp/
* cp-tree.h (struct language_function): Delete extern_decl_map.
(DECL_LOCAL_DECL_ALIAS): New.
* name-lookup.h (is_local_extern): Delete.
* name-lookup.c (set_local_extern_decl_linkage): Replace with ...
(push_local_extern_decl): ... this new function.
(do_pushdecl): Call new function after pushing new decl. Unhide
hidden non-functions.
(is_local_extern): Delete.
* decl.c (layout_var_decl): Do not allow VLA local externs.
* decl2.c (mark_used): Also mark DECL_LOCAL_DECL_ALIAS. Drop old
local-extern treatment.
* parser.c (cp_parser_oacc_declare): Deal with local extern aliases.
* pt.c (tsubst_expr): Adjust local extern instantiation.
* cp-gimplify.c (cp_genericize_r): Remap DECL_LOCAL_DECLs.
gcc/testsuite/
* g++.dg/cpp0x/lambda/lambda-sfinae1.C: Avoid ill-formed local extern
* g++.dg/init/pr42844.C: Add expected error.
* g++.dg/lookup/extern-redecl1.C: Likewise.
* g++.dg/lookup/koenig15.C: Avoid ill-formed.
* g++.dg/lto/pr95677.C: New.
* g++.dg/other/nested-extern-1.C: Correct expected behabviour.
* g++.dg/other/nested-extern-2.C: Likewise.
* g++.dg/other/nested-extern.cc: Split ...
* g++.dg/other/nested-extern-1.cc: ... here ...
* g++.dg/other/nested-extern-2.cc: ... here.
* g++.dg/template/scope5.C: Avoid ill-formed
* g++.old-deja/g++.law/missed-error2.C: Allow extension.
* g++.old-deja/g++.pt/crash3.C: Add expected error.
pushing to trunk
nathan
--
Nathan Sidwell
diff --git c/gcc/cp/cp-gimplify.c w/gcc/cp/cp-gimplify.c
index 07549828dc9..44c9d249b15 100644
--- c/gcc/cp/cp-gimplify.c
+++ w/gcc/cp/cp-gimplify.c
@@ -980,21 +980,17 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
/* Map block scope extern declarations to visible declarations with the
same name and type in outer scopes if any. */
- if (cp_function_chain->extern_decl_map
- && VAR_OR_FUNCTION_DECL_P (stmt)
- && DECL_EXTERNAL (stmt))
-{
- struct cxx_int_tree_map *h, in;
- in.uid = DECL_UID (stmt);
- h = cp_function_chain->extern_decl_map->find_with_hash (, in.uid);
- if (h)
- {
- *stmt_p = h->to;
- TREE_USED (h->to) |= TREE_USED (stmt);
- *walk_subtrees = 0;
- return NULL;
- }
-}
+ if (VAR_OR_FUNCTION_DECL_P (stmt) && DECL_LOCAL_DECL_P (stmt))
+if (tree alias = DECL_LOCAL_DECL_ALIAS (stmt))
+ {
+ if (alias != error_mark_node)
+ {
+ *stmt_p = alias;
+ TREE_USED (alias) |= TREE_USED (stmt);
+ }
+ *walk_subtrees = 0;
+ return NULL;
+ }
if (TREE_CODE (stmt) == INTEGER_CST
&& TYPE_REF_P (TREE_TYPE (stmt))
diff --git c/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h
index e5a2ff20223..467256117ec 100644
--- c/gcc/cp/cp-tree.h
+++ w/gcc/cp/cp-tree.h
@@ -1926,7 +1926,6 @@ struct GTY(()) language_function {
/* Tracking possibly infinite loops. This is a vec only because
vec doesn't work with gtype. */
vec *infinite_loops;
- hash_table *extern_decl_map;
};
/* The current C++-specific per-function global variables. */
@@ -2697,6 +2696,7 @@ struct GTY(()) lang_decl_min {
In a lambda-capture proxy VAR_DECL, this is DECL_CAPTURED_VARIABLE.
In a function-scope TREE_STATIC VAR_DECL or IMPLICIT_TYPEDEF_P TYPE_DECL,
this is DECL_DISCRIMINATOR.
+ In a DECL_LOCAL_DECL_P decl, this is the namespace decl it aliases.
Otherwise, in a class-scope DECL, this is DECL_ACCESS. */
tree access;
};
@@ -4023,6 +4023,10 @@ more_aggr_init_expr_args_p (const
c++: Rename DECL_BUILTIN_P to DECL_UNDECLARED_BUILTIN_P
I realized I'd misnamed DECL_BUILTIN_P, it's only true of compiler
builtins unless and until the user declares them -- at that point
they're real decls, and will have a location in the user's source.
(BUILT_IN_FN and friends still work though). This renames them so
future-me is not confused as to why the predicate becomes false.
gcc/cp/
* cp-tree.h (DECL_BUILTIN_P): Rename to ...
(DECL_UNDECLARED_BUILTIN_P): ... here.
* decl.c (duplicate_decls): Adjust.
* name-lookup.c (anticipated_builtin_p): Adjust.
(do_nonmember_using_decl): Likewise.
libcc1/
* libcp1plugin.cc (supplement_binding): Rename
DECL_BUILTIN_P.
pushing to trunk
nathan
--
Nathan Sidwell
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c9ad75117ad..22700844fac 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4035,8 +4035,9 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter)
#define FNDECL_USED_AUTO(NODE) \
TREE_LANG_FLAG_2 (FUNCTION_DECL_CHECK (NODE))
-/* True if NODE is a builtin decl. */
-#define DECL_BUILTIN_P(NODE) \
+/* True if NODE is an undeclared builtin decl. As soon as the user
+ declares it, the location will be updated. */
+#define DECL_UNDECLARED_BUILTIN_P(NODE) \
(DECL_SOURCE_LOCATION(NODE) == BUILTINS_LOCATION)
/* True for artificial decls added for OpenMP privatized non-static
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index ac5fd92a162..ee9190d8d0c 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -1467,7 +1467,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden)
/* Check for redeclaration and other discrepancies. */
if (TREE_CODE (olddecl) == FUNCTION_DECL
- && DECL_BUILTIN_P (olddecl))
+ && DECL_UNDECLARED_BUILTIN_P (olddecl))
{
if (TREE_CODE (newdecl) != FUNCTION_DECL)
{
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 2f42ad0d532..5e714503e4b 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2134,7 +2134,7 @@ anticipated_builtin_p (tree ovl)
{
return (TREE_CODE (ovl) == OVERLOAD
&& OVL_HIDDEN_P (ovl)
- && DECL_BUILTIN_P (OVL_FUNCTION (ovl)));
+ && DECL_UNDECLARED_BUILTIN_P (OVL_FUNCTION (ovl)));
}
/* BINDING records an existing declaration for a name in the current scope.
@@ -3987,7 +3987,7 @@ do_nonmember_using_decl (name_lookup , bool fn_scope_p,
}
else if (old.using_p ())
continue; /* This is a using decl. */
- else if (old.hidden_p () && DECL_BUILTIN_P (old_fn))
+ else if (old.hidden_p () && DECL_UNDECLARED_BUILTIN_P (old_fn))
continue; /* This is an anticipated builtin. */
else if (!matching_fn_p (new_fn, old_fn))
continue; /* Parameters do not match. */
diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
index 98174250585..e7ab325d7df 100644
--- a/libcc1/libcp1plugin.cc
+++ b/libcc1/libcp1plugin.cc
@@ -353,7 +353,7 @@ supplement_binding (cxx_binding *binding, tree decl)
/* If TARGET_BVAL is anticipated but has not yet been
declared, pretend it is not there at all. */
|| (TREE_CODE (target_bval) == FUNCTION_DECL
- && DECL_BUILTIN_P (target_bval)))
+ && DECL_UNDECLARED_BUILTIN_P (target_bval)))
binding->value = decl;
else if (TREE_CODE (target_bval) == TYPE_DECL
&& DECL_ARTIFICIAL (target_bval)
Re: GCC 10 backports
On 10/1/20 9:18 PM, Martin Liška wrote:
I'm going to install the following 3 tested backports.
Martin
One more patch that I've tested.
Martin
>From d957172d0d43cdcf8dc098ae493c193d6678ecfd Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Mon, 5 Oct 2020 18:03:08 +0200
Subject: [PATCH] lto: fix LTO debug sections copying.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
readelf -S prints:
There are 81999 section headers, starting at offset 0x1f488060:
Section Headers:
[Nr] Name TypeAddress OffSize ES Flg Lk Inf Al
[ 0] NULL 00 01404f 00 81998 0 0
[ 1] .groupGROUP 40 08 04 81995 105027 4
...
[81995] .symtab SYMTAB d5d9298 2db310 18 81997 105026 8
[81996] .symtab_shndx SYMTAB SECTION INDICES d8b45a8 079dd8 04 81995 0 4
[81997] .strtab STRTAB d92e380 80460c 00 0 0 1
...
Looking at the documentation:
Table 7–15 ELF sh_link and sh_info Interpretation
sh_type - sh_link
SHT_SYMTAB - The section header index of the associated string table.
SHT_SYMTAB_SHNDX - The section header index of the associated symbol table.
As seen, sh_link of a SHT_SYMTAB always points to a .strtab and readelf
confirms that.
So we need to use reverse mapping taken from
[81996] .symtab_shndx SYMTAB SECTION INDICES d8b45a8 079dd8 04 81995 0 4
where sh_link points to 81995.
libiberty/ChangeLog:
PR lto/97290
* simple-object-elf.c (simple_object_elf_copy_lto_debug_sections):
Use sh_link of a .symtab_shndx section.
(cherry picked from commit 190c04ba36d9c6c3dce41f12012aa97c6d7f22f5)
---
libiberty/simple-object-elf.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/libiberty/simple-object-elf.c b/libiberty/simple-object-elf.c
index 7c9d492f6a4..37e73348cb7 100644
--- a/libiberty/simple-object-elf.c
+++ b/libiberty/simple-object-elf.c
@@ -1191,7 +1191,7 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
unsigned int sh_link;
sh_link = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
shdr, sh_link, Elf_Word);
- symtab_indices_shndx[sh_link - 1] = i;
+ symtab_indices_shndx[sh_link - 1] = i - 1;
/* Always discard the extended index sections, after
copying it will not be needed. This way we don't need to
update it and deal with the ordering constraints of
@@ -1372,19 +1372,22 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
{
unsigned entsize = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
shdr, sh_entsize, Elf_Addr);
- unsigned strtab = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
- shdr, sh_link, Elf_Word);
size_t prevailing_name_idx = 0;
unsigned char *ent;
unsigned *shndx_table = NULL;
/* Read the section index table if present. */
if (symtab_indices_shndx[i - 1] != 0)
{
- unsigned char *sidxhdr = shdrs + (strtab - 1) * shdr_size;
+ unsigned char *sidxhdr = shdrs + symtab_indices_shndx[i - 1] * shdr_size;
off_t sidxoff = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
sidxhdr, sh_offset, Elf_Addr);
size_t sidxsz = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
sidxhdr, sh_size, Elf_Addr);
+ unsigned int shndx_type
+ = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
+ sidxhdr, sh_type, Elf_Word);
+ if (shndx_type != SHT_SYMTAB_SHNDX)
+ return "Wrong section type of a SYMTAB SECTION INDICES section";
shndx_table = (unsigned *)XNEWVEC (char, sidxsz);
simple_object_internal_read (sobj->descriptor,
sobj->offset + sidxoff,
--
2.28.0
c++: Adding exception specs can changed dependentness
Making an exception variant can cause a non-dependent function type to become dependent (since c++17 eh-specs are part of the type). The same is (possibly?) true for adding a late return type. Fixed thusly. My upcoming local extern-decl changes have a test case that covers this (which was how I found it). gcc/cp/ * tree.c (build_cp_fntype_variant): Clear TYPE_DEPENDENT_P_VALID if necessary. nathan -- Nathan Sidwell diff --git i/gcc/cp/tree.c w/gcc/cp/tree.c index 8b7c6798ee9..074fa0c025e 100644 --- i/gcc/cp/tree.c +++ w/gcc/cp/tree.c @@ -2638,6 +2638,9 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual, /* Need to build a new variant. */ v = build_variant_type_copy (type); + if (!TYPE_DEPENDENT_P (v)) +/* We no longer know that it's not type-dependent. */ +TYPE_DEPENDENT_P_VALID (v) = false; TYPE_RAISES_EXCEPTIONS (v) = raises; TYPE_HAS_LATE_RETURN_TYPE (v) = late; switch (rqual)
[committed] amdgcn: Use scalar instructions for addptrdi3
This patch adds an extra alternative to the existing addptrdi3 pattern.
It permits safe 64-bit addition in scalar registers, as well as vector
registers. This is especially useful because the result of addptr
typically gets fed to instructions that expect the base address to be in
a scalar register. It was only the way it was because vector
instructions can specify a custom register to clobber.
Hopefully this will help prevent unnecessary register moves for address
calculations.
Andrew
amdgcn: Use scalar instructions for addptrdi3
Allow addptr to use SPGRs as well as VGPRs for pointers. This ought to
prevent some unnecessary copying back and forth.
gcc/ChangeLog:
* config/gcn/gcn.md (unspec): Add UNSPEC_ADDPTR.
(addptrdi3): Add SGPR alternative.
diff --git a/gcc/config/gcn/gcn.md b/gcc/config/gcn/gcn.md
index 0e73fea93cf..763e77008ad 100644
--- a/gcc/config/gcn/gcn.md
+++ b/gcc/config/gcn/gcn.md
@@ -67,6 +67,7 @@ (define_c_enum "unspecv" [
UNSPECV_ICACHE_INV])
(define_c_enum "unspec" [
+ UNSPEC_ADDPTR
UNSPEC_VECTOR
UNSPEC_BPERMUTE
UNSPEC_SGPRBASE
@@ -1219,29 +1220,47 @@ (define_insn "addcsi3_scalar_zero"
; "addptr" is the same as "add" except that it must not write to VCC or SCC
; as a side-effect. Unfortunately GCN does not have a suitable instruction
-; for this, so we use a custom VOP3 add with CC_SAVE_REG as a temp.
-; Note that it is not safe to save/clobber/restore SCC because doing so will
-; break data-flow analysis, so this must use vector registers.
+; for this, so we use CC_SAVE_REG as a temp.
+; Note that it is not safe to save/clobber/restore as separate insns because
+; doing so will break data-flow analysis, so this must use multiple
+; instructions in one insn.
;
; The "v0" should be just "v", but somehow the "0" helps LRA not loop forever
; on testcase pr54713-2.c with -O0. It's only an optimization hint anyway.
+;
+; The SGPR alternative is preferred as it is typically used with mov_sgprbase.
(define_insn "addptrdi3"
- [(set (match_operand:DI 0 "register_operand" "= v")
- (plus:DI (match_operand:DI 1 "register_operand" " v0")
- (match_operand:DI 2 "nonmemory_operand" "vDA")))]
+ [(set (match_operand:DI 0 "register_operand" "= v, Sg")
+(unspec:DI [
+ (plus:DI (match_operand:DI 1 "register_operand" "^v0,Sg0")
+ (match_operand:DI 2 "nonmemory_operand" "vDA,SgDB"))]
+ UNSPEC_ADDPTR))]
""
{
-rtx new_operands[4] = { operands[0], operands[1], operands[2],
- gen_rtx_REG (DImode, CC_SAVE_REG) };
+if (which_alternative == 0)
+ {
+ rtx new_operands[4] = { operands[0], operands[1], operands[2],
+gen_rtx_REG (DImode, CC_SAVE_REG) };
-output_asm_insn ("v_add%^_u32 %L0, %3, %L2, %L1", new_operands);
-output_asm_insn ("v_addc%^_u32 %H0, %3, %H2, %H1, %3", new_operands);
+ output_asm_insn ("v_add%^_u32\t%L0, %3, %L2, %L1", new_operands);
+ output_asm_insn ("v_addc%^_u32\t%H0, %3, %H2, %H1, %3", new_operands);
+ }
+else
+ {
+ rtx new_operands[4] = { operands[0], operands[1], operands[2],
+gen_rtx_REG (BImode, CC_SAVE_REG) };
+
+ output_asm_insn ("s_mov_b32\t%3, scc", new_operands);
+ output_asm_insn ("s_add_u32\t%L0, %L1, %L2", new_operands);
+ output_asm_insn ("s_addc_u32\t%H0, %H1, %H2", new_operands);
+ output_asm_insn ("s_cmpk_lg_u32\t%3, 0", new_operands);
+ }
return "";
}
- [(set_attr "type" "vmult")
- (set_attr "length" "16")])
+ [(set_attr "type" "vmult,mult")
+ (set_attr "length" "16,24")])
;; }}}
;; {{{ ALU special cases: Minus
Re: [PATCH] IPA: merge profiles more sensitively
On 10/7/20 1:23 PM, Jan Hubicka wrote:
During WPA we merge 2 functions where one is built with -O3
-fprofile-use while the other one uses -O0. We end up with:
prevailing node:
BitwiseCast (const double aFrom)
{
long unsigned int temp;
long unsigned int D.4528;
:
BitwiseCast (aFrom_2(D), );
_4 = temp;
temp ={v} {CLOBBER};
:
:
return _4;
}
merged node:
BitwiseCast (const double aFrom)
{
[count: 1509]:
_3 = VIEW_CONVERT_EXPR(aFrom_2(D));
return _3;
}
What happens is that we copy dst->count = src->count.ipa() but
we skip merging of BBs (a different CFG). Then we end up in situation
where profile of a ENTRY_BB_FOR_FN == uninitialized while
dst->count.quality is precise.
In usual case there of diverging CFGs both functions have non-trivial
profile, one is ipa and other is guessed (so count.initialized_p returns
false)
In this case one TU is compiled with -O0, so we miss profile at all
(if I see correctly).
In this case we do want to copy the count which will lead in scaling the
local profile by the known entry frequency. This is arranged by
fixup_cfg that does this kind of transitions.
All right, here we end up with
profile_count num = node->count;
profile_count den = ENTRY_BLOCK_PTR_FOR_FN (cfun)->count;
(gdb) p num.debug()
1509 (precise)
$7 = void
(gdb) p den.debug()
1509 (precise)
$8 = void
bool scale = num.initialized_p () && !(num == den);
and so scale == false;
Thus no scaling happens and e.g. BB 2 misses profile (due to -O0):
(gdb) p debug_bb(bb)
:
BitwiseCast (aFrom_2(D), );
_4 = temp;
temp ={v} {CLOBBER};
and we later ICE here:
Breakpoint 1, profile_count::to_frequency (this=0x773f8058,
fun=0x773ee000) at ../../gcc/profile-count.c:273
273 gcc_assert (REG_BR_PROB_BASE == BB_FREQ_MAX
(gdb) bt
#0 profile_count::to_frequency (this=0x773f8058, fun=0x773ee000) at
../../gcc/profile-count.c:273
#1 0x00d72c4d in regstat_bb_compute_ri (bb=0x773f8000,
live=0x2839f60) at ../../gcc/regstat.c:200
#2 0x00d73005 in regstat_compute_ri () at ../../gcc/regstat.c:253
#3 0x00be1a7d in ira (f=) at ../../gcc/ira.c:5294
#4 (anonymous namespace)::pass_ira::execute (this=) at
../../gcc/ira.c:5666
#5 0x00d23b45 in execute_one_pass (pass=0x26ef850) at
../../gcc/passes.c:2502
#6 0x00d23e6a in execute_pass_list_1 (pass=0x26ef850) at
../../gcc/passes.c:2590
#7 0x00d23e9b in execute_pass_list_1 (pass=0x26ee5d0) at
../../gcc/passes.c:2591
#8 0x00d23ef3 in execute_pass_list (fn=0x773ee000, pass=0x26ea890)
at ../../gcc/passes.c:2601
#9 0x0090a7d7 in cgraph_node::expand (this=0x773ede10) at
../../gcc/cgraphunit.c:2300
#10 0x0090b232 in output_in_order () at ../../gcc/cgraphunit.c:2578
#11 0x0090b869 in symbol_table::compile (this=0x775d6100) at
../../gcc/cgraphunit.c:2819
#12 0x0083df7d in lto_main () at ../../gcc/lto/lto.c:653
#13 0x00deda92 in compile_file () at ../../gcc/toplev.c:458
#14 0x00df0c7a in do_compile () at ../../gcc/toplev.c:2278
#15 0x00df0f63 in toplev::main (this=0x7fffd9fe, argc=19,
argv=0x26c26f0) at ../../gcc/toplev.c:2417
#16 0x008196af in main (argc=19, argv=0x7fffdb08) at
../../gcc/main.c:39
So what do you suggest to fix it :P?
Is this the bug that riggers Firefox ICE?
What goes wrong later?
Honza
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
gcc/ChangeLog:
PR ipa/97295
* ipa-utils.c (ipa_merge_profiles):
---
gcc/ipa-utils.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
index 23e7f714306..fe2149069be 100644
--- a/gcc/ipa-utils.c
+++ b/gcc/ipa-utils.c
@@ -437,10 +437,7 @@ ipa_merge_profiles (struct cgraph_node *dst,
else if (dst->count.ipa ().initialized_p ())
;
else if (src->count.ipa ().initialized_p ())
-{
- copy_counts = true;
- dst->count = src->count.ipa ();
-}
+copy_counts = true;
/* If no updating needed return early. */
if (dst->count == orig_count)
@@ -620,6 +617,9 @@ ipa_merge_profiles (struct cgraph_node *dst,
bool dstscale = !copy_counts
&& dstnum.initialized_p () && !(dstnum == dstden);
+ if (copy_counts)
+ dst->count = src->count.ipa ();
+
/* TODO: merge also statement histograms. */
FOR_ALL_BB_FN (srcbb, srccfun)
{
--
2.28.0
Re: [PATCH] debug: Pass --gdwarf-N to assembler if fixed gas is detected during configure
Hi, On Tue, 2020-10-06 at 16:57 -0400, Jason Merrill wrote: > All three of these patches (Jakub's, and your two) look good to me, > except that your add_filepath_AT_string patch is missing comments on > some of the new functions. I added documentation for the two new functions missing comments before pushing. Thanks, Mark
Re: libgfortran caf API help needed: Fixing fnspec strings in trans-decl
Hi Andre, On 10/7/20 12:21 PM, Andre Vehreschild wrote: I am confused. Given that on 2. Oct. 2020 you committed changes to the caf-fn_spec strings means: you don't need anymore help on this? Do you mean the change by the patch "Perforate fnspec attribute strings"? (https://gcc.gnu.org/g:762cca0023c9bdbd762c44f33a954845bbccd568) That one just added spaces for the updated fnspec format. Otherwise: See gcc/attr-fnspec.h for a description. I suggest also to have a look at the commit log at https://gcc.gnu.org/g:2595f25cdaf4f16d04a1078a487b2ecc126cae29 and at the PR https://gcc.gnu.org/PR92123 for gotchas. Tobias - Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
Re: [PATCH] IPA: merge profiles more sensitively
> During WPA we merge 2 functions where one is built with -O3
> -fprofile-use while the other one uses -O0. We end up with:
>
> prevailing node:
>
> BitwiseCast (const double aFrom)
> {
> long unsigned int temp;
> long unsigned int D.4528;
>
>:
> BitwiseCast (aFrom_2(D), );
> _4 = temp;
> temp ={v} {CLOBBER};
>
>:
> :
> return _4;
>
> }
>
> merged node:
> BitwiseCast (const double aFrom)
> {
>[count: 1509]:
> _3 = VIEW_CONVERT_EXPR(aFrom_2(D));
> return _3;
> }
>
> What happens is that we copy dst->count = src->count.ipa() but
> we skip merging of BBs (a different CFG). Then we end up in situation
> where profile of a ENTRY_BB_FOR_FN == uninitialized while
> dst->count.quality is precise.
In usual case there of diverging CFGs both functions have non-trivial
profile, one is ipa and other is guessed (so count.initialized_p returns
false)
In this case we do want to copy the count which will lead in scaling the
local profile by the known entry frequency. This is arranged by
fixup_cfg that does this kind of transitions.
Is this the bug that riggers Firefox ICE?
What goes wrong later?
Honza
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> PR ipa/97295
> * ipa-utils.c (ipa_merge_profiles):
> ---
> gcc/ipa-utils.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
> index 23e7f714306..fe2149069be 100644
> --- a/gcc/ipa-utils.c
> +++ b/gcc/ipa-utils.c
> @@ -437,10 +437,7 @@ ipa_merge_profiles (struct cgraph_node *dst,
>else if (dst->count.ipa ().initialized_p ())
> ;
>else if (src->count.ipa ().initialized_p ())
> -{
> - copy_counts = true;
> - dst->count = src->count.ipa ();
> -}
> +copy_counts = true;
>/* If no updating needed return early. */
>if (dst->count == orig_count)
> @@ -620,6 +617,9 @@ ipa_merge_profiles (struct cgraph_node *dst,
>bool dstscale = !copy_counts
> && dstnum.initialized_p () && !(dstnum == dstden);
> + if (copy_counts)
> + dst->count = src->count.ipa ();
> +
>/* TODO: merge also statement histograms. */
>FOR_ALL_BB_FN (srcbb, srccfun)
> {
> --
> 2.28.0
>
Re: [PATCH] IPA: merge profiles more sensitively
On 10/7/20 1:18 PM, Martin Liška wrote:
* ipa-utils.c (ipa_merge_profiles):
I forgot to fill up the ChangeLog entry. Fixed.
Martin
>From a5d5d23175d336883972ebef66380cbc4f1b6d54 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Wed, 7 Oct 2020 12:30:27 +0200
Subject: [PATCH] IPA: merge profiles more sensitively
During WPA we merge 2 functions where one is built with -O3
-fprofile-use while the other one uses -O0. We end up with:
prevailing node:
BitwiseCast (const double aFrom)
{
long unsigned int temp;
long unsigned int D.4528;
:
BitwiseCast (aFrom_2(D), );
_4 = temp;
temp ={v} {CLOBBER};
:
:
return _4;
}
merged node:
BitwiseCast (const double aFrom)
{
[count: 1509]:
_3 = VIEW_CONVERT_EXPR(aFrom_2(D));
return _3;
}
What happens is that we copy dst->count = src->count.ipa() but
we skip merging of BBs (a different CFG). Then we end up in situation
where profile of a ENTRY_BB_FOR_FN == uninitialized while
dst->count.quality is precise.
gcc/ChangeLog:
PR ipa/97295
* ipa-utils.c (ipa_merge_profiles): Assign dst->count only
when CFG matches.
---
gcc/ipa-utils.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
index 23e7f714306..fe2149069be 100644
--- a/gcc/ipa-utils.c
+++ b/gcc/ipa-utils.c
@@ -437,10 +437,7 @@ ipa_merge_profiles (struct cgraph_node *dst,
else if (dst->count.ipa ().initialized_p ())
;
else if (src->count.ipa ().initialized_p ())
-{
- copy_counts = true;
- dst->count = src->count.ipa ();
-}
+copy_counts = true;
/* If no updating needed return early. */
if (dst->count == orig_count)
@@ -620,6 +617,9 @@ ipa_merge_profiles (struct cgraph_node *dst,
bool dstscale = !copy_counts
&& dstnum.initialized_p () && !(dstnum == dstden);
+ if (copy_counts)
+ dst->count = src->count.ipa ();
+
/* TODO: merge also statement histograms. */
FOR_ALL_BB_FN (srcbb, srccfun)
{
--
2.28.0
Re: [PATCH] IPA: merge profiles more sensitively
On 10/7/20 1:18 PM, Martin Liška wrote: * ipa-utils.c (ipa_merge_profiles): I forgot to fill up the ChangeLog entry. Fixed. Martin
[PATCH] IPA: merge profiles more sensitively
During WPA we merge 2 functions where one is built with -O3
-fprofile-use while the other one uses -O0. We end up with:
prevailing node:
BitwiseCast (const double aFrom)
{
long unsigned int temp;
long unsigned int D.4528;
:
BitwiseCast (aFrom_2(D), );
_4 = temp;
temp ={v} {CLOBBER};
:
:
return _4;
}
merged node:
BitwiseCast (const double aFrom)
{
[count: 1509]:
_3 = VIEW_CONVERT_EXPR(aFrom_2(D));
return _3;
}
What happens is that we copy dst->count = src->count.ipa() but
we skip merging of BBs (a different CFG). Then we end up in situation
where profile of a ENTRY_BB_FOR_FN == uninitialized while
dst->count.quality is precise.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
gcc/ChangeLog:
PR ipa/97295
* ipa-utils.c (ipa_merge_profiles):
---
gcc/ipa-utils.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
index 23e7f714306..fe2149069be 100644
--- a/gcc/ipa-utils.c
+++ b/gcc/ipa-utils.c
@@ -437,10 +437,7 @@ ipa_merge_profiles (struct cgraph_node *dst,
else if (dst->count.ipa ().initialized_p ())
;
else if (src->count.ipa ().initialized_p ())
-{
- copy_counts = true;
- dst->count = src->count.ipa ();
-}
+copy_counts = true;
/* If no updating needed return early. */
if (dst->count == orig_count)
@@ -620,6 +617,9 @@ ipa_merge_profiles (struct cgraph_node *dst,
bool dstscale = !copy_counts
&& dstnum.initialized_p () && !(dstnum == dstden);
+ if (copy_counts)
+ dst->count = src->count.ipa ();
+
/* TODO: merge also statement histograms. */
FOR_ALL_BB_FN (srcbb, srccfun)
{
--
2.28.0
RE: [PATCH v2] arm: [MVE[ Add vqdmlashq intrinsics
Hi Christophe > -Original Message- > From: Gcc-patches On Behalf Of > Christophe Lyon via Gcc-patches > Sent: 06 October 2020 16:59 > To: gcc-patches@gcc.gnu.org > Subject: [PATCH v2] arm: [MVE[ Add vqdmlashq intrinsics > > This patch adds: > vqdmlashq_m_n_s16 > vqdmlashq_m_n_s32 > vqdmlashq_m_n_s8 > vqdmlashq_n_s16 > vqdmlashq_n_s32 > vqdmlashq_n_s8 > > v2: rebased after Srinath's reorganization patch > Ok. Thanks, Kyrill > 2020-10-05 Christophe Lyon > > gcc/ > PR target/96914 > * config/arm/arm_mve.h (vqdmlashq, vqdmlashq_m): Define. > * config/arm/arm_mve_builtins.def (vqdmlashq_n_s) > (vqdmlashq_m_n_s,): New. > * config/arm/unspecs.md (VQDMLASHQ_N_S, VQDMLASHQ_M_N_S): > New > unspecs. > * config/arm/iterators.md (VQDMLASHQ_N_S, > VQDMLASHQ_M_N_S): New > attributes. > (VQDMLASHQ_N): New iterator. > * config/arm/mve.md (mve_vqdmlashq_n_, mve_vqdmlashq_m_n_s): > New > patterns. > > gcc/tetsuite/ > PR target/96914 > * gcc.target/arm/mve/intrinsics/vqdmlashq_m_n_s16.c: New test. > * gcc.target/arm/mve/intrinsics/vqdmlashq_m_n_s32.c: New test. > * gcc.target/arm/mve/intrinsics/vqdmlashq_m_n_s8.c: New test. > * gcc.target/arm/mve/intrinsics/vqdmlashq_n_s16.c: New test. > * gcc.target/arm/mve/intrinsics/vqdmlashq_n_s32.c: New test. > * gcc.target/arm/mve/intrinsics/vqdmlashq_n_s8.c: New test. > --- > gcc/config/arm/arm_mve.h | 116 > + > gcc/config/arm/arm_mve_builtins.def| 2 + > gcc/config/arm/iterators.md| 3 + > gcc/config/arm/mve.md | 33 ++ > gcc/config/arm/unspecs.md | 2 + > .../arm/mve/intrinsics/vqdmlashq_m_n_s16.c | 23 > .../arm/mve/intrinsics/vqdmlashq_m_n_s32.c | 23 > .../arm/mve/intrinsics/vqdmlashq_m_n_s8.c | 23 > .../arm/mve/intrinsics/vqdmlashq_n_s16.c | 21 > .../arm/mve/intrinsics/vqdmlashq_n_s32.c | 21 > .../gcc.target/arm/mve/intrinsics/vqdmlashq_n_s8.c | 21 > 11 files changed, 288 insertions(+) > create mode 100644 > gcc/testsuite/gcc.target/arm/mve/intrinsics/vqdmlashq_m_n_s16.c > create mode 100644 > gcc/testsuite/gcc.target/arm/mve/intrinsics/vqdmlashq_m_n_s32.c > create mode 100644 > gcc/testsuite/gcc.target/arm/mve/intrinsics/vqdmlashq_m_n_s8.c > create mode 100644 > gcc/testsuite/gcc.target/arm/mve/intrinsics/vqdmlashq_n_s16.c > create mode 100644 > gcc/testsuite/gcc.target/arm/mve/intrinsics/vqdmlashq_n_s32.c > create mode 100644 > gcc/testsuite/gcc.target/arm/mve/intrinsics/vqdmlashq_n_s8.c > > diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h > index d9bfb203..7626ad1 100644 > --- a/gcc/config/arm/arm_mve.h > +++ b/gcc/config/arm/arm_mve.h > @@ -141,6 +141,7 @@ > #define vrev64q_m(__inactive, __a, __p) __arm_vrev64q_m(__inactive, __a, > __p) > #define vqrdmlashq(__a, __b, __c) __arm_vqrdmlashq(__a, __b, __c) > #define vqrdmlahq(__a, __b, __c) __arm_vqrdmlahq(__a, __b, __c) > +#define vqdmlashq(__a, __b, __c) __arm_vqdmlashq(__a, __b, __c) > #define vqdmlahq(__a, __b, __c) __arm_vqdmlahq(__a, __b, __c) > #define vmvnq_m(__inactive, __a, __p) __arm_vmvnq_m(__inactive, __a, > __p) > #define vmlasq(__a, __b, __c) __arm_vmlasq(__a, __b, __c) > @@ -260,6 +261,7 @@ > #define vorrq_m(__inactive, __a, __b, __p) __arm_vorrq_m(__inactive, __a, > __b, __p) > #define vqaddq_m(__inactive, __a, __b, __p) __arm_vqaddq_m(__inactive, > __a, __b, __p) > #define vqdmladhq_m(__inactive, __a, __b, __p) > __arm_vqdmladhq_m(__inactive, __a, __b, __p) > +#define vqdmlashq_m(__a, __b, __c, __p) __arm_vqdmlashq_m(__a, __b, > __c, __p) > #define vqdmladhxq_m(__inactive, __a, __b, __p) > __arm_vqdmladhxq_m(__inactive, __a, __b, __p) > #define vqdmlahq_m(__a, __b, __c, __p) __arm_vqdmlahq_m(__a, __b, __c, > __p) > #define vqdmlsdhq_m(__inactive, __a, __b, __p) > __arm_vqdmlsdhq_m(__inactive, __a, __b, __p) > @@ -1307,6 +1309,7 @@ > #define vqdmlsdhxq_s8(__inactive, __a, __b) > __arm_vqdmlsdhxq_s8(__inactive, __a, __b) > #define vqdmlsdhq_s8(__inactive, __a, __b) > __arm_vqdmlsdhq_s8(__inactive, __a, __b) > #define vqdmlahq_n_s8(__a, __b, __c) __arm_vqdmlahq_n_s8(__a, __b, __c) > +#define vqdmlashq_n_s8(__a, __b, __c) __arm_vqdmlashq_n_s8(__a, __b, > __c) > #define vqdmladhxq_s8(__inactive, __a, __b) > __arm_vqdmladhxq_s8(__inactive, __a, __b) > #define vqdmladhq_s8(__inactive, __a, __b) > __arm_vqdmladhq_s8(__inactive, __a, __b) > #define vmlsdavaxq_s8(__a, __b, __c) __arm_vmlsdavaxq_s8(__a, __b, __c) > @@ -1391,6 +1394,7 @@ > #define vqrdmladhq_s16(__inactive, __a, __b) > __arm_vqrdmladhq_s16(__inactive, __a, __b) > #define vqdmlsdhxq_s16(__inactive, __a, __b) > __arm_vqdmlsdhxq_s16(__inactive, __a, __b) > #define vqdmlsdhq_s16(__inactive, __a, __b) > __arm_vqdmlsdhq_s16(__inactive, __a, __b)
[PATCH, wwwdocs] gcc-11/changes: C++11 is now required to build GCC
This summarizes GCC 11's change in build requirements from C++98 to C++11, for the release notes. I've put it in the Caveats immediately below the "The default mode for C++ is..." change hence the wording. I've based it on the change to gcc/doc/install.texi in the GCC source tree, which was 5329b59a2e13dabbe2038af0fe2e3cf5fc7f98ed there. Validates. OK to commit? --- htdocs/gcc-11/changes.html | 5 + 1 file changed, 5 insertions(+) diff --git a/htdocs/gcc-11/changes.html b/htdocs/gcc-11/changes.html index e2a32e51..e33abe44 100644 --- a/htdocs/gcc-11/changes.html +++ b/htdocs/gcc-11/changes.html @@ -35,6 +35,11 @@ a work-in-progress. features with -fno-new-ttp-matching. + When building GCC itself, the host compiler must now support C++11, +rather than C++98. In particular bootstrapping GCC 11 using an older +version of GCC requires a binary of GCC 4.8 or later, rather than of +GCC 3.4 or later as was the case for bootstrapping GCC 10. + Naming and location of auxiliary and dump output files changed. If you compile multiple input files in a single command, if you enable Link Time Optimization, or if you use -dumpbase, -- 2.26.2
Re: libgfortran caf API help needed: Fixing fnspec strings in trans-decl
Hi,
I am confused. Given that on 2. Oct. 2020 you committed changes to the
caf-fn_spec strings means: you don't need anymore help on this?
- Andre
On Wed, 30 Sep 2020 18:12:31 +0200
Jan Hubicka wrote:
> Hi,
> this patch contains basic fixup of the fnspec strings for caf, however I
> am quite sure I need help on this (short of dropping them all).
>
> I first assumed that we have missing "." for return values since most
> strings had as many letters as parametrs, but it is not true.
> I tried to check the strings with reality. For example:
>
>
> void
> _gfortran_caf_co_sum (gfc_descriptor_t *a __attribute__ ((unused)),
> int result_image __attribute__ ((unused)),
> int *stat, char *errmsg __attribute__ ((unused)),
> size_t errmsg_len __attribute__ ((unused)))
> {
> if (stat)
> *stat = 0;
> }
>
>
> Should have fnspec
> ".XXWXX"
> First dot represents return value, then X is for unused parameters and W
> is for stat pointer we write into.
>
> However I am not sure why the pointers are part ofthe API, if they are
> meant to be used later, we need to specify them so things remain ABI
> compatible.
>
> It is declared as:
> get_identifier (PREFIX("caf_co_sum")), "W.WW",
> Which correclty specifies stat as W, but I am not sure what does the
> else.
>
> I would apprechiate help from someone who knows the API to correct the
> strings. Basicaly all strings starting with "W" or "R" are wrong since
> they miss the return value specifier.
>
> An alternative would be to simply drop all of those if we are unsure
> what they do, but it seems lame.
>
> Honza
>
> diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
> index 2be9df40d2c..59ea891915e 100644
> --- a/gcc/fortran/trans-decl.c
> +++ b/gcc/fortran/trans-decl.c
> @@ -3514,8 +3514,8 @@ gfc_build_intrinsic_function_decls (void)
>DECL_PURE_P (gfor_fndecl_si_kind) = 1;
>TREE_NOTHROW (gfor_fndecl_si_kind) = 1;
>
> - gfor_fndecl_sr_kind = gfc_build_library_function_decl_with_spec (
> - get_identifier (PREFIX("selected_real_kind2008")), ".RR",
> + gfor_fndecl_sr_kind = gfc_build_library_function_decl (
> + get_identifier (PREFIX("selected_real_kind2008")),
> gfc_int4_type_node, 3, pvoid_type_node, pvoid_type_node,
> pvoid_type_node);
>DECL_PURE_P (gfor_fndecl_sr_kind) = 1;
> @@ -3841,50 +3841,50 @@ gfc_build_builtin_function_decls (void)
> get_identifier (PREFIX("caf_num_images")), integer_type_node,
> 2, integer_type_node, integer_type_node);
>
> - gfor_fndecl_caf_register = gfc_build_library_function_decl_with_spec (
> - get_identifier (PREFIX("caf_register")), "RRR", void_type_node,
> 7,
> + gfor_fndecl_caf_register = gfc_build_library_function_decl (
> + get_identifier (PREFIX("caf_register")), void_type_node, 7,
> size_type_node, integer_type_node, ppvoid_type_node, pvoid_type_node,
> pint_type, pchar_type_node, size_type_node);
>
>gfor_fndecl_caf_deregister = gfc_build_library_function_decl_with_spec
> (
> - get_identifier (PREFIX("caf_deregister")), "WRWWR", void_type_node,
> 5,
> + get_identifier (PREFIX("caf_deregister")), ".W.WW.", void_type_node,
> 5, ppvoid_type_node, integer_type_node, pint_type, pchar_type_node,
> size_type_node);
>
> - gfor_fndecl_caf_get = gfc_build_library_function_decl_with_spec (
> - get_identifier (PREFIX("caf_get")), ".R.RRWRRRW", void_type_node, 10,
> + gfor_fndecl_caf_get = gfc_build_library_function_decl (
> + get_identifier (PREFIX("caf_get")), void_type_node, 10,
> pvoid_type_node, size_type_node, integer_type_node, pvoid_type_node,
> pvoid_type_node, pvoid_type_node, integer_type_node,
> integer_type_node, boolean_type_node, pint_type);
>
> - gfor_fndecl_caf_send = gfc_build_library_function_decl_with_spec (
> - get_identifier (PREFIX("caf_send")), ".R.RRWR", void_type_node,
> 11,
> + gfor_fndecl_caf_send = gfc_build_library_function_decl (
> + get_identifier (PREFIX("caf_send")), void_type_node, 11,
> pvoid_type_node, size_type_node, integer_type_node, pvoid_type_node,
> pvoid_type_node, pvoid_type_node, integer_type_node,
> integer_type_node, boolean_type_node, pint_type, pvoid_type_node);
>
> - gfor_fndecl_caf_sendget = gfc_build_library_function_decl_with_spec (
> - get_identifier (PREFIX("caf_sendget")), ".R..RR",
> + gfor_fndecl_caf_sendget = gfc_build_library_function_decl (
> + get_identifier (PREFIX("caf_sendget")),
> void_type_node, 14, pvoid_type_node, size_type_node,
> integer_type_node, pvoid_type_node, pvoid_type_node, pvoid_type_node,
> size_type_node, integer_type_node, pvoid_type_node, pvoid_type_node,
> integer_type_node, integer_type_node, boolean_type_node, integer_type_node);
>
> - gfor_fndecl_caf_get_by_ref = gfc_build_library_function_decl_with_spec
> (
> - get_identifier (PREFIX("caf_get_by_ref")), ".RWRWR",
> void_type_node,
> +
[PATCH] arc: Use separate predicated patterns for mpyd(u)
From: Claudiu Zissulescu The compiler can match mpyd.eq r0,r1,r0 as a predicated instruction, which is incorrect. The mpyd(u) instruction takes as input two 32-bit registers, returning into a double 64-bit even-odd register pair. For the predicated case, the ARC instruction decoder expects the destination register to be the same as the first input register. In the big-endian case the result is swaped in the destination register pair, however, the instruction encoding remains the same. Refurbish the mpyd(u) patterns to take into account the above observation. Permission to apply this patch to master, gcc10 and gcc9 branches. Cheers, Claudiu -xx-xx Claudiu Zissulescu * testsuite/gcc.target/arc/pmpyd.c: New test. * testsuite/gcc.target/arc/tmac-1.c: Update. * config/arc/arc.md (mpyd_arcv2hs): New template pattern. (*pmpyd_arcv2hs): Likewise. (*pmpyd_imm_arcv2hs): Likewise. (mpyd_arcv2hs): Moved into above template. (mpyd_imm_arcv2hs): Moved into above template. (mpydu_arcv2hs): Likewise. (mpydu_imm_arcv2hs): Likewise. (su_optab): New optab prefix for sign/zero-extending operations. Signed-off-by: Claudiu Zissulescu --- gcc/config/arc/arc.md | 101 +- gcc/testsuite/gcc.target/arc/pmpyd.c | 15 gcc/testsuite/gcc.target/arc/tmac-1.c | 2 +- 3 files changed, 67 insertions(+), 51 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arc/pmpyd.c diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index 1720e8cd2f6f..d4d9f59a3eac 100644 --- a/gcc/config/arc/arc.md +++ b/gcc/config/arc/arc.md @@ -894,6 +894,8 @@ archs4x, archs4xd" (define_code_iterator SEZ [sign_extend zero_extend]) (define_code_attr SEZ_prefix [(sign_extend "sex") (zero_extend "ext")]) +; Optab prefix for sign/zero-extending operations +(define_code_attr su_optab [(sign_extend "") (zero_extend "u")]) (define_insn "*xt_cmp0_noout" [(set (match_operand 0 "cc_set_register" "") @@ -6436,66 +6438,65 @@ archs4x, archs4xd" (set_attr "predicable" "no") (set_attr "cond" "nocond")]) -(define_insn "mpyd_arcv2hs" - [(set (match_operand:DI 0 "even_register_operand""=Rcr, r") - (mult:DI (sign_extend:DI (match_operand:SI 1 "register_operand" " 0, c")) -(sign_extend:DI (match_operand:SI 2 "register_operand" " c, c" +(define_insn "mpyd_arcv2hs" + [(set (match_operand:DI 0 "even_register_operand" "=r") + (mult:DI (SEZ:DI (match_operand:SI 1 "register_operand" "r")) +(SEZ:DI (match_operand:SI 2 "register_operand" "r" (set (reg:DI ARCV2_ACC) (mult:DI - (sign_extend:DI (match_dup 1)) - (sign_extend:DI (match_dup 2] + (SEZ:DI (match_dup 1)) + (SEZ:DI (match_dup 2] "TARGET_PLUS_MACD" - "mpyd%? %0,%1,%2" - [(set_attr "length" "4,4") - (set_attr "iscompact" "false") - (set_attr "type" "multi") - (set_attr "predicable" "yes,no") - (set_attr "cond" "canuse,nocond")]) - -(define_insn "mpyd_imm_arcv2hs" - [(set (match_operand:DI 0 "even_register_operand""=Rcr, r,r,Rcr, r") - (mult:DI (sign_extend:DI (match_operand:SI 1 "register_operand" " 0, c,0, 0, c")) -(match_operand 2 "immediate_operand" " L, L,I,Cal,Cal"))) + "mpyd%?\\t%0,%1,%2" + [(set_attr "length" "4") + (set_attr "iscompact" "false") + (set_attr "type" "multi") + (set_attr "predicable" "no")]) + +(define_insn "*pmpyd_arcv2hs" + [(set (match_operand:DI 0 "even_register_operand" "=r") + (mult:DI +(SEZ:DI (match_operand:SI 1 "even_register_operand" "%0")) +(SEZ:DI (match_operand:SI 2 "register_operand" "r" (set (reg:DI ARCV2_ACC) - (mult:DI (sign_extend:DI (match_dup 1)) -(match_dup 2)))] + (mult:DI + (SEZ:DI (match_dup 1)) + (SEZ:DI (match_dup 2] "TARGET_PLUS_MACD" - "mpyd%? %0,%1,%2" - [(set_attr "length" "4,4,4,8,8") - (set_attr "iscompact" "false") - (set_attr "type" "multi") - (set_attr "predicable" "yes,no,no,yes,no") - (set_attr "cond" "canuse,nocond,nocond,canuse_limm,nocond")]) - -(define_insn "mpydu_arcv2hs" - [(set (match_operand:DI 0 "even_register_operand""=Rcr, r") - (mult:DI (zero_extend:DI (match_operand:SI 1 "register_operand" " 0, c")) -(zero_extend:DI (match_operand:SI 2 "register_operand" " c, c" + "mpyd%?\\t%0,%1,%2" + [(set_attr "length" "4") + (set_attr "iscompact" "false") + (set_attr "type" "multi") + (set_attr "predicable" "yes")]) + +(define_insn "mpyd_imm_arcv2hs" + [(set (match_operand:DI 0 "even_register_operand""=r,r, r") + (mult:DI (SEZ:DI (match_operand:SI 1 "register_operand" "r,0, r")) +(match_operand 2"immediate_operand" "L,I,Cal"))) (set (reg:DI ARCV2_ACC) -
[PATCH] bootstrap/95582 - adjust what types we consider boolean for vectorization
This excludes BOOLEAN_TYPE which are signed or have TYPE_PRECISION != 1
from being handled as VECTOR_BOOLEAN_TYPE_P. This fixes a bootstrap
ICE with LTO and PGO in the Ada frontend.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2020-10-07 Richard Biener
PR bootstrap/95582
* tree-vectorizer.h (VECT_SCALAR_BOOLEAN_TYPE_P): Handle
BOOLEAN_TYPE the same as INTEGER_TYPE and ENUMERAL_TYPE.
---
gcc/tree-vectorizer.h | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 37b091558fd..e03d41ce982 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -1383,11 +1383,9 @@ struct gather_scatter_info {
VECTOR_BOOLEAN_TYPE_P. */
#define VECT_SCALAR_BOOLEAN_TYPE_P(TYPE) \
- (TREE_CODE (TYPE) == BOOLEAN_TYPE\
- || ((TREE_CODE (TYPE) == INTEGER_TYPE \
- || TREE_CODE (TYPE) == ENUMERAL_TYPE) \
- && TYPE_PRECISION (TYPE) == 1 \
- && TYPE_UNSIGNED (TYPE)))
+ (INTEGRAL_TYPE_P (TYPE) \
+ && TYPE_PRECISION (TYPE) == 1\
+ && TYPE_UNSIGNED (TYPE))
static inline bool
nested_in_vect_loop_p (class loop *loop, stmt_vec_info stmt_info)
--
2.26.2
Re: [PATCH] ipa-prop: Fix multiple-target speculation resolution
On 10/1/20 7:39 PM, Martin Jambor wrote: OK for trunk? Approved. Martin
Re: [PATCH] Add if-chain to switch conversion pass.
On Tue, Oct 6, 2020 at 4:03 PM Martin Liška wrote: > > On 10/6/20 9:47 AM, Richard Biener wrote: > > But is it really extensible with the current implementation? I doubt so. > > I must agree with the statement. So let's make the pass properly. > I would need a help with the algorithm where I'm planning to do the following > steps: > > 1) for each BB ending with a gcond, parse index variable and it's VR; > I'll support: > a) index == 123 ([123, 123]) > b) 1 <= index && index <= 9 ([1, 9]) > c) index == 123 || index == 12345 ([123, 123] [12345, 12345]) > d) index != 1 ([1, 1]) > e) index != 1 && index != 5 ([1, 1] [5, 5]) > > 2) switch range edge is identified, e.g. true_edge for 1e, while false_edge > for 1a > > 3) I want to support forward code hoisting, so for each condition BB we need > to identify > if the block contains only stmts without a side-effect > > 4) we can ignore BBs with condition variables that has small number of > potential > case switches > > 5) for each condition variable we can iterate bottom up in dominator order > and try > to find a BB predecessor chain (in first phase no BB in between such > "condition" BBs) > that uses a same condition variable > > 6) the chain will be converted to a switch statement > 7) code hoisting must be done in order to move a gimple statements and fix > potential > gphis that can be collapsed > > Is it something feasible that can work? As said I'd have a BB-local pass over BBs recording the index variable and the range covered by the BBs gcond, plus recording how many excess stmts there are for eventual code motion. Only after that BB-local pass start to group BBs in a walk from dominated to dominating BBs looking for common indexes and building a case vector. The main thing is to avoid repeatedly analyzing BBs conditions (so the first pass could be also a on-demand precompute thing) and making the case vector build optimal. > Thanks, > > Martin >
Re: [PATCH 1/4] system_data_types.7: Add '__int128'
* Michael Kerrisk: > Hi Florian, > > On 10/5/20 9:12 AM, Florian Weimer wrote: >> * Paul Eggert: >> >>> On 10/2/20 12:01 PM, Alejandro Colomar wrote: If you propose not to document the stdint types either, >>> >>> This is not a stdint.h issue. __int128 is not in stdint.h and is not a >>> system data type in any real sense; it's purely a compiler >>> issue. Besides, do we start repeating the GCC manual too, while we're >>> at it? At some point we need to restrain ourselves and stay within the >>> scope of the man pages. >> >> The manual pages also duplicate the glibc manual, and as far as I know, >> it's what programmers actually read. (Downstream, we receive many more >> man-pages bugs than glibc or GCC manual bugs.) Most developers use >> distributions > > I presume by most you mean "Debian + Ubuntu"? (The certainly > reflects what I see.) Yes, exactly. And other distributions inspired by the Debian interpretation of the DFSG. >> which do not ship the glibc or GCC manual for licensing >> policy reasons, so the GNU manuals are not installed locally. > > I hadn't quite clicked to this point. So, Debian (and thus > Ubuntu) do not ship the glibc manual because of GNU FDL. That's > unfortunate. >From Debian's point of view, only GFDL plus Invariant Sections is problematic, but both the glibc and GCC manuals have them. Plain GFDL would still be awkward for upstream, but fine from a policy standpoint. Thanks, Florian -- Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill
Re: [PATCH] lto: fix LTO debug sections copying.
On 10/6/20 7:34 PM, Ian Lance Taylor wrote: On Tue, Oct 6, 2020 at 3:20 AM Martin Liška wrote: On 10/6/20 10:00 AM, Richard Biener wrote: On Tue, Oct 6, 2020 at 9:01 AM Martin Liška wrote: On 10/5/20 6:34 PM, Ian Lance Taylor wrote: On Mon, Oct 5, 2020 at 9:09 AM Martin Liška wrote: The previous patch was not correct. This one should be. Ready for master? I don't understand why this code uses symtab_indices_shndx at all. There should only be one SHT_SYMTAB_SHNDX section. There shouldn't be any need for the symtab_indices_shndx vector. Well, the question is if we can have multiple .symtab sections in one ELF file? Theoretically yes, so we should also handle SHT_SYMTAB_SHNDX sections. Note that the original usage of the SHT_SYMTAB_SHNDX section was motivated by PR81968 which is about Solaris ld. It wasn't my code but I suppose this way the implementation was "easiest". There should be exactly one symtab / shndx section. Rainer authored this support. If we expect at maximum one SHT_SYMTAB_SHNDX section section, then I'm suggesting an updated version of the patch. It's what Ian offered. This is OK with me with one minor change. + return "Multiple SYMTAB SECTION INDICES sections"; I think simply "More than one SHT_SYMTAB_SHNDX section". SYMTAB SECTION INDICES doesn't mean anything to me, and at least people can do a web search for SHT_SYMTAB_SHNDX. Ok, so I decided to install the first version of the patch. Thanks for review, Martin Thanks. Ian
Re: [PATCH 1/4] system_data_types.7: Add '__int128'
Hi Florian, On 10/5/20 9:12 AM, Florian Weimer wrote: > * Paul Eggert: > >> On 10/2/20 12:01 PM, Alejandro Colomar wrote: >>> If you propose not to document the stdint types either, >> >> This is not a stdint.h issue. __int128 is not in stdint.h and is not a >> system data type in any real sense; it's purely a compiler >> issue. Besides, do we start repeating the GCC manual too, while we're >> at it? At some point we need to restrain ourselves and stay within the >> scope of the man pages. > > The manual pages also duplicate the glibc manual, and as far as I know, > it's what programmers actually read. (Downstream, we receive many more > man-pages bugs than glibc or GCC manual bugs.) Most developers use > distributions I presume by most you mean "Debian + Ubuntu"? (The certainly reflects what I see.) > which do not ship the glibc or GCC manual for licensing > policy reasons, so the GNU manuals are not installed locally. I hadn't quite clicked to this point. So, Debian (and thus Ubuntu) do not ship the glibc manual because of GNU FDL. That's unfortunate. (Many years ago, IIRC, there were still one of two GNU FDL licensed pages in man-pages, and I rewrote / removed the pages to eliminate the problem for Debian--and I was happy also at the same time to reduce the number of licenses in man-pages.) >> PS. Have you ever tried to use __int128 in real code? I have, to my >> regret. It's a portability and bug minefield and should not be used >> unless you really know what you're doing, which most people do not. > > Doesn't this suggest we need improved documentation? Well, yes. But I'm also not convinced man-pages is the right place for it. Thanks, Michael -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/
[PATCH][GCC] arm: Fix wrong code generated for mve scatter store with writeback intrinsics with -O2 (PR97271).
Hello,
This patch fixes (PR97271) the wrong code-gen for mve scatter store with
writeback intrinsics with -O2.
$cat bug.c
#include "arm_mve.h"
void
foo (uint32x4_t * addr, const int offset, int32x4_t value)
{
vstrwq_scatter_base_wb_s32 (addr, 8, value);
}
$ arm-none-eabi-gcc bug.c -S -O2 -march=armv8.1-m.main+mve -mfloat-abi=hard -o
-
Without this patch:
...
foo:
vldrw.32q3, [r0]
vstrw.u32 q0, [q3, #8]! ---> (A)
vldr.64 d4, .L3
vldr.64 d5, .L3+8
vldrw.32q3, [r0]
vstrw.u32 q2, [q3, #8]! ---> (B)
bx lr
...
With this patch:
...
foo:
vldrw.32q3, [r0]
vstrw.u32 q0, [q3, #8]! --> (C)
vstrw.32q3, [r0]
bx lr
...
Without this patch 2 vstrw assembly instructions (A and B) are generated for
vstrwq_scatter_base_wb_s32
intrinsic where as fix generates only one vstrw assembly instruction (C).
Bootstrapped on arm-none-linux-gnueabihf and regression tested on arm-none-eabi
and found no regressions.
Ok for master? Ok for GCC-10 branch?
Regards,
Srinath.
gcc/ChangeLog:
2020-10-06 Srinath Parvathaneni
PR target/97291
* config/arm/arm-builtins.c (arm_strsbwbs_qualifiers): Modify array.
(arm_strsbwbu_qualifiers): Likewise.
(arm_strsbwbs_p_qualifiers): Likewise.
(arm_strsbwbu_p_qualifiers): Likewise.
* config/arm/arm_mve.h (__arm_vstrdq_scatter_base_wb_s64): Modify
function definition.
(__arm_vstrdq_scatter_base_wb_u64): Likewise.
(__arm_vstrdq_scatter_base_wb_p_s64): Likewise.
(__arm_vstrdq_scatter_base_wb_p_u64): Likewise.
(__arm_vstrwq_scatter_base_wb_p_s32): Likewise.
(__arm_vstrwq_scatter_base_wb_p_u32): Likewise.
(__arm_vstrwq_scatter_base_wb_s32): Likewise.
(__arm_vstrwq_scatter_base_wb_u32): Likewise.
(__arm_vstrwq_scatter_base_wb_f32): Likewise.
(__arm_vstrwq_scatter_base_wb_p_f32): Likewise.
* config/arm/arm_mve_builtins.def (vstrwq_scatter_base_wb_add_u): Remove
expansion for the builtin.
(vstrwq_scatter_base_wb_add_s): Likewise.
(vstrwq_scatter_base_wb_add_f): Likewise.
(vstrdq_scatter_base_wb_add_u): Likewise.
(vstrdq_scatter_base_wb_add_s): Likewise.
(vstrwq_scatter_base_wb_p_add_u): Likewise.
(vstrwq_scatter_base_wb_p_add_s): Likewise.
(vstrwq_scatter_base_wb_p_add_f): Likewise.
(vstrdq_scatter_base_wb_p_add_u): Likewise.
(vstrdq_scatter_base_wb_p_add_s): Likewise.
* config/arm/mve.md (mve_vstrwq_scatter_base_wb_v4si): Remove
expand.
(mve_vstrwq_scatter_base_wb_add_v4si): Likewise.
(mve_vstrwq_scatter_base_wb_v4si_insn): Rename pattern to ...
(mve_vstrwq_scatter_base_wb_v4si): This.
(mve_vstrwq_scatter_base_wb_p_v4si): Remove expand.
(mve_vstrwq_scatter_base_wb_p_add_v4si): Likewise.
(mve_vstrwq_scatter_base_wb_p_v4si_insn): Rename pattern to ...
(mve_vstrwq_scatter_base_wb_p_v4si): This.
(mve_vstrwq_scatter_base_wb_fv4sf): Remove expand.
(mve_vstrwq_scatter_base_wb_add_fv4sf): Likewise.
(mve_vstrwq_scatter_base_wb_fv4sf_insn): Rename pattern to ...
(mve_vstrwq_scatter_base_wb_fv4sf): This.
(mve_vstrwq_scatter_base_wb_p_fv4sf): Remove expand.
(mve_vstrwq_scatter_base_wb_p_add_fv4sf): Likewise.
(mve_vstrwq_scatter_base_wb_p_fv4sf_insn): Rename pattern to ...
(mve_vstrwq_scatter_base_wb_p_fv4sf): This.
(mve_vstrdq_scatter_base_wb_v2di): Remove expand.
(mve_vstrdq_scatter_base_wb_add_v2di): Likewise.
(mve_vstrdq_scatter_base_wb_v2di_insn): Rename pattern to ...
(mve_vstrdq_scatter_base_wb_v2di): This.
(mve_vstrdq_scatter_base_wb_p_v2di): Remove expand.
(mve_vstrdq_scatter_base_wb_p_add_v2di): Likewise.
(mve_vstrdq_scatter_base_wb_p_v2di_insn): Rename pattern to ...
(mve_vstrdq_scatter_base_wb_p_v2di): This.
gcc/testsuite/ChangeLog:
PR target/97291
* gcc.target/arm/mve/intrinsics/vstrdq_scatter_base_wb_p_s64.c: Modify.
* gcc.target/arm/mve/intrinsics/vstrdq_scatter_base_wb_p_u64.c:
Likewise.
* gcc.target/arm/mve/intrinsics/vstrdq_scatter_base_wb_s64.c: Likewise.
* gcc.target/arm/mve/intrinsics/vstrdq_scatter_base_wb_u64.c: Likewise.
* gcc.target/arm/mve/intrinsics/vstrwq_scatter_base_wb_f32.c: Likewise.
* gcc.target/arm/mve/intrinsics/vstrwq_scatter_base_wb_p_f32.c:
Likewise.
* gcc.target/arm/mve/intrinsics/vstrwq_scatter_base_wb_p_s32.c:
Likewise.
* gcc.target/arm/mve/intrinsics/vstrwq_scatter_base_wb_p_u32.c:
Likewise.
* gcc.target/arm/mve/intrinsics/vstrwq_scatter_base_wb_s32.c: Likewise.
* gcc.target/arm/mve/intrinsics/vstrwq_scatter_base_wb_u32.c: Likewise.
### Attachment also inlined for ease of
Re: [patch][DOC]PR97309--improve documentation of -fallow-store-data-races
On Tue, 6 Oct 2020, Qing Zhao wrote:
> Richard,
>
> On behalf of John Henning, I am sending the following simple documentation
> patch
> for an official approval. Hopefully this patch can get into gcc11.
>
> Per John, the wording of the documentation change was based on the discussion
> between him and you on 25-Aug-2020.
>
> I created a gcc bugzilla bug
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97309
> to record this issue.
>
> Okay to commit?
OK.
Richard.
> thanks.
>
> Qing
>
> ChangeLog:
>
>
> 2020-10-06 John Henning
>
> * gcc/doc/invoke.texi: Improve documentation of -fallow-store-data-races
>
> ---
> gcc/doc/invoke.texi | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 7c81d7f41bd..926ee1ff28e 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -11621,7 +11621,18 @@ Do not remove unused C++ allocations in dead code
> elimination.
>
> @item -fallow-store-data-races
> @opindex fallow-store-data-races
> -Allow the compiler to introduce new data races on stores.
> +Allow the compiler to perform optimizations that may introduce new data races
> +on stores, without proving that the variable cannot be concurrently accessed
> +by other threads. Does not affect optimization of local data. It is safe to
> +use this option if it is known that global data will not be accessed by
> +multiple threads.
> +
> +Examples of optimizations enabled by @option{-fallow-store-data-races}
> include
> +hoisting or if-conversions that may cause a value that was already in memory
> +to be re-written with that same value. Such re-writing is safe in a single
> +threaded context but may be unsafe in a multi-threaded context. Note that on
> +some processors, if-conversions may be required in order to enable
> +vectorization.
>
> Enabled at level @option{-Ofast}.
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
Re: [PUSHED] Hybrid EVRP and testcases
On Tue, Oct 6, 2020 at 7:06 PM Andrew MacLeod via Gcc-patches
wrote:
>
> I have now checked in the hybrid EVRP pass.
>
> We have resolved all the issue we are aware of with a full Fedora build,
> but if any more issues arise, please let us know. (And Im sure you will :-)
>
> I made some minor tweaks. the option to the new -fevrp-mode flag are now:
>
> legacy : classic EVRP mode
> ranger : Ranger only mode
> *legacy-first : Query ranges with EVRP first, and if that fails try
> the ranger*
> ranger-first : Query the ranger first, then evrp
> ranger-trace : Ranger-only mode plus Show range tracing info in the dump
> ranger-debug : Ranger-only mode, and also include all cache debugging info
> trace: Hybrid mode with range tracing info
> debug : Hybrid mode with cache debugging as well as tracing
>
> The default is still *legacy-first*.
We'll have to keep -fevrp-mode forever so can you instead make it a --param
since I hope it is transitional only? It certainly shouldn't be a
user-visible flag, should it?
Richard.
>
>
> If there is a compilation problem, and the problem goes away with
> -fevrp-mode=legacy
> the ranger is to blame, and let us know asap.
>
>
> Attached is the patch which was applied.
>
> ---
>
> These are the initial test case differences, and the detailed analysis
> is below:
>
> 1) We now XPASS analyzer/pr94851-1.c.
> 2) -disable-tree-evrp added to gcc.dg/pr81192.c
> 3) -disable-tree-evrp added to gcc.dg/tree-ssa/pr77445-2.c
> 4) -disable-tree-evrp added to tree-ssa/ssa-dom-thread-6.c
> 5) -disable-tree-evrp added to tree-ssa/ssa-dom-thread-7.c
>
>
> mostly it is one of 2 things:
>
> 1) we propagate a constant into into PHI that wasnt happening before,
> EVRP didn't handle anything other than single entry blocks well.
> 2) switches are processed in a lot more detail, which again propagate a
> lot of values into PHIs, and it then triggers more threading.
>
> Last minute update... It also turns out the analyzer xpass may be noise.
> we did change the IL, but it sounds like there are other analyzer things
> at play at the moment :-)
>
>
> 1) We now XPASS analyzer/pr94851-1.c.
>
>
> It was xfailing with:
> In function ‘pamark’:
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:43:13:
> warning: leak of ‘p’ [CWE-401] [-Wanalyzer-malloc-leak]
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:29:6: note:
> (1) following ‘false’ branch (when ‘p’ is NULL)...
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:32:23: note:
> (2) ...to here
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:32:23: note:
> (3) allocated here
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:32:8: note:
> (4) assuming ‘p’ is non-NULL
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:32:8: note:
> (5) following ‘false’ branch (when ‘p’ is non-NULL)...
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:35:15: note:
> (6) ...to here
> /gcc/master/gcc/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c:43:13: note:
> (7) ‘p’ leaks here; was allocated at (3)
>
> now we produce:
> XPASS: gcc.dg/analyzer/pr94851-1.c bogus leak (test for bogus messages,
> line 43)
>
>
> THe reason is in the IL:
>:
># p_9 = PHI
># last_11 = PHI
>if (p_9 != 0B)
> goto ; [INV]
>else
> goto ; [INV] --> This outgoing edge
>
> :
>_3 = p_9->m_name;
>_4 = (char) _32;
>if (_3 != _4)
> goto ; [INV]
>else
> goto ; [INV]
>
> :
># p_2 = PHI <<< THis PHI node
># last_17 = PHI
>if (p_2 != 0B)
> goto ; [INV]
>else
> goto ; [INV]
>
> :
>printf ("over writing mark %c\n", _32);
>goto ; [INV]
>
>
> The ranger propagates the p_9 == 0 from the else branch into the PHI
> argument on edge 4->6
>:
># p_2 = PHI <0B(4), p_9(5)>
>
> which the threaders can then bypass the print in bb7 on one path, and
> that seems to resolve the current issue.
>
> THe IL produced by the time we get to .optimized is identical, we just
> clear it up early enough for the analyzer to use now.
>
> ---
>
> 2) -fdisable-tree-evrp added to gcc.dg/pr81192.c to enable the test to pass
>
> new version of evrp sees
> :
>if (j_8(D) != 2147483647)
> goto ; [50.00%]
>else
> goto ; [50.00%]
> :
>iftmp.2_11 = j_8(D) + 1;
> :
># iftmp.2_12 = PHI
>
> EVRP now recognizes a constant can be propagated into the 3->5 edge and
> produces
># iftmp.2_12 = PHI <2147483647(3), iftmp.2_11(4)>
> which causes the situation being tested to dissappear before we get to
> PRE. */
>
>
Re: [PATCH][tree-ssa-loop-ch] Add missing NULL test for dump_file
On Wed, 7 Oct 2020, Tom de Vries wrote:
> Hi,
>
> If we change gimple_can_duplicate_bb_p to return false instead of true, we run
> into a segfault in ch_base::copy_headers due to using dump_file while it's
> NULL:
> ...
> if (!gimple_duplicate_sese_region (entry, exit, bbs, n_bbs, copied_bbs,
> true))
>{
> fprintf (dump_file, "Duplication failed.\n");
> continue;
>}
> ...
>
> Fix this by adding the missing dump_file != NULL test.
>
> Tested by rebuilding lto1 and rerunning the failing test-case.
>
> [ Not committing this as obvious because I'm not sure about the
> (dump_flags & TDF_DETAILS) bit. ]
>
> OK for trunk?
OK.
Richard.
> Thanks,
> - Tom
>
> [tree-ssa-loop-ch] Add missing NULL test for dump_file
>
> gcc/ChangeLog:
>
> 2020-10-07 Tom de Vries
>
> * tree-ssa-loop-ch.c (ch_base::copy_headers): Add missing NULL test
> for dump_file.
>
> ---
> gcc/tree-ssa-loop-ch.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-ssa-loop-ch.c b/gcc/tree-ssa-loop-ch.c
> index b86acf7c39d..1f3d9321a55 100644
> --- a/gcc/tree-ssa-loop-ch.c
> +++ b/gcc/tree-ssa-loop-ch.c
> @@ -425,7 +425,8 @@ ch_base::copy_headers (function *fun)
>if (!gimple_duplicate_sese_region (entry, exit, bbs, n_bbs, copied_bbs,
>true))
> {
> - fprintf (dump_file, "Duplication failed.\n");
> + if (dump_file && (dump_flags & TDF_DETAILS))
> + fprintf (dump_file, "Duplication failed.\n");
> continue;
> }
>copied.safe_push (std::make_pair (entry, loop));
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
