[PATCH 0/2] Future warnings not treated as errors
Hello, I have received permission to contribute to GCC, but I do not have commit access. Could one of you please merge these patches for me? -Alex Alex Henrie (1): PR c/65403 - Add tests for -Wno-error= Manuel López-Ibáñez (1): PR c/65403 - Ignore -Wno-error= gcc/opts-common.c | 2 ++ gcc/opts-global.c | 10 +++--- gcc/opts.c | 5 - gcc/opts.h | 2 ++ gcc/testsuite/c-c++-common/pr65403-1.c | 10 ++ gcc/testsuite/c-c++-common/pr65403-2.c | 15 +++ 6 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/pr65403-1.c create mode 100644 gcc/testsuite/c-c++-common/pr65403-2.c -- 2.21.0
[PATCH 1/2] PR c/65403 - Ignore -Wno-error=
From: Manuel López-Ibáñez * opts.c: Ignore -Wno-error= except if there are other diagnostics. --- gcc/opts-common.c | 2 ++ gcc/opts-global.c | 10 +++--- gcc/opts.c| 5 - gcc/opts.h| 2 ++ 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/gcc/opts-common.c b/gcc/opts-common.c index edbb3ac9b6d..36e06e2372a 100644 --- a/gcc/opts-common.c +++ b/gcc/opts-common.c @@ -26,6 +26,8 @@ along with GCC; see the file COPYING3. If not see #include "diagnostic.h" #include "spellcheck.h" +vec ignored_wnoerror_options; + static void prune_options (struct cl_decoded_option **, unsigned int *); /* An option that is undocumented, that takes a joined argument, and diff --git a/gcc/opts-global.c b/gcc/opts-global.c index a5e9ef0237a..0b2550a4855 100644 --- a/gcc/opts-global.c +++ b/gcc/opts-global.c @@ -132,12 +132,16 @@ print_ignored_options (void) { while (!ignored_options.is_empty ()) { - const char *opt; - - opt = ignored_options.pop (); + const char * opt = ignored_options.pop (); warning_at (UNKNOWN_LOCATION, 0, "unrecognized command line option %qs", opt); } + while (!ignored_wnoerror_options.is_empty ()) +{ + const char * opt = ignored_wnoerror_options.pop (); + warning_at (UNKNOWN_LOCATION, 0, + "-Wno-error=%s: no option -W%s", opt, opt); +} } /* Handle an unknown option DECODED, returning true if an error should diff --git a/gcc/opts.c b/gcc/opts.c index 3161e0b6753..4fa79306e30 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -3079,7 +3079,10 @@ enable_warning_as_error (const char *arg, int value, unsigned int lang_mask, strcpy (new_option + 1, arg); option_index = find_opt (new_option, lang_mask); if (option_index == OPT_SPECIAL_unknown) -error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option); +if (value) + error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option); +else + ignored_wnoerror_options.safe_push (arg); else if (!(cl_options[option_index].flags & CL_WARNING)) error_at (loc, "%<-Werror=%s%>: -%s is not an option that controls " "warnings", arg, new_option); diff --git a/gcc/opts.h b/gcc/opts.h index f14d9bcb896..b18504e0bc3 100644 --- a/gcc/opts.h +++ b/gcc/opts.h @@ -456,4 +456,6 @@ extern bool parse_and_check_align_values (const char *flag, bool report_error, location_t loc); +extern vec ignored_wnoerror_options; + #endif -- 2.21.0
[PATCH 2/2] PR c/65403 - Add tests for -Wno-error=
--- gcc/testsuite/c-c++-common/pr65403-1.c | 10 ++ gcc/testsuite/c-c++-common/pr65403-2.c | 15 +++ 2 files changed, 25 insertions(+) create mode 100644 gcc/testsuite/c-c++-common/pr65403-1.c create mode 100644 gcc/testsuite/c-c++-common/pr65403-2.c diff --git a/gcc/testsuite/c-c++-common/pr65403-1.c b/gcc/testsuite/c-c++-common/pr65403-1.c new file mode 100644 index 000..fbe004a1f78 --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr65403-1.c @@ -0,0 +1,10 @@ +/* PR c/65403 */ +/* Test an unrecognized -Wno-error option in the absence of any other + diagnostics. The -Wno-error option should be ignored. */ + +/* { dg-options "-Werror -Wno-error=some-future-warning" } */ + +int main(int argc, char **argv) +{ + return 0; +} diff --git a/gcc/testsuite/c-c++-common/pr65403-2.c b/gcc/testsuite/c-c++-common/pr65403-2.c new file mode 100644 index 000..128d4f694a6 --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr65403-2.c @@ -0,0 +1,15 @@ +/* PR c/65403 */ +/* Test a warning, treated as an error, that some future -Wno-error option + might downgrade back to a warning. The -Wno-error option should produce a + warning in this case. */ + +/* { dg-options "-Wunused-variable -Werror -Wno-error=some-future-warning" } */ + +int main(int argc, char **argv) +{ + int foo; /* { dg-error "unused variable 'foo'" } */ + return 0; +} + +/* { dg-error "no option -Wsome-future-warning" "" { target *-*-* } 0 } */ +/* { dg-message "all warnings being treated as errors" "" { target *-*-* } 0 } */ -- 2.21.0
Re: Implement C++20 constexpr , , and
On 3/18/19 6:18 PM, Jonathan Wakely wrote: On 17/03/19 22:54 -0400, Ed Smith-Rowland via libstdc++ wrote: Greetings, This patch implements C++20 p0202 - Add Constexpr Modifiers to Functions in and Headers and C++20 p1023 - constexpr comparison operators for std::array. The patch is large because of the testsuite additions. Basically, the algorithms and the array comparison operators are all marked constexpr for C++20. This has been built and tested on x86_64-linux. As discussed on IRC< the tests need to be run with -std=gnu++2a to ensure everything we test (which is not exhaustive, but is the best we can do) still works in C++2a mode. The indentation went bad here: template - _FIter1 + _GLIBCXX20_CONSTEXPR + _FIter1 find_first_of(_FIter1, _FIter1, _FIter2, _FIter2, _BinaryPredicate); I would have to look through old emails but I think there's a reason the function objects like _Iter_comp_to_val take their arguments by non-const reference. The _Iter_comp_to_val functions need non-const reference o'loads to accommodate mutable functors. These things now have both const and nonconst o'loads. I'm very surprised that none of the algos that dispatch to __builtin_memove need changes, because those optimizations won't work in constant expressions. I would expect to have to use std::is_constant_evaluated to disable the optimizations when used in constant expressions I'm also actually surprised that __builtin_memove works also. In other strange news, I had to write a std::is_constant_evaluated branch around __builtin_memcmp even though *that* *is* supposed to work for constexpr. Ed
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On Mon, Mar 18, 2019 at 6:58 PM Jeff Law wrote: > > On 3/18/19 6:35 PM, James Hilliard wrote: > > On Mon, Mar 18, 2019 at 5:46 PM Jeff Law wrote: > >> > >> On 3/18/19 5:07 PM, James Hilliard wrote: > >>> On Mon, Mar 18, 2019 at 4:51 PM Jakub Jelinek wrote: > > On Mon, Mar 18, 2019 at 04:41:05PM -0600, James Hilliard wrote: > >> Thanks, but I'm saying that if you look at the code you can see that > >> st is clearly initialized, by the call to lstat. I would like to see > >> an explanation for why you are seeing that warning before changing the > >> code to disable it. Initializing st should not be necessary here. > >> For example, perhaps lstat is a macro when compiling libsanitizer; if > >> that is the underlying problem, then we should fix the macro, not this > >> code. > > Yeah, I'm not sure why the compiler thinks lstat isn't initializing st. > > What should I do to debug this further? > > Guess you should start by telling us which OS it is on (I can't reproduce > this warning on x86_64-linux nor i686-linux with glibc 2.28), looking at > preprocessed source to see what exactly lstat does (e.g. if it is some > macro > or inline function and what exactly it is doing). > >>> I am cross compiling with buildroot master branch using ubuntu 18.10. > >>> I am building gcc 8.3.0 and glibc 2.29 for the cross toolchain. > >>> The build and target systems are both x86_64. > >> Add "-save-temps" to the command line. That will create a .i file, send > >> the .i file along with the command line. > > I added --save-temps to the cflags for the gcc package build. > > Here's the command line log: > > https://gist.githubusercontent.com/jameshilliard/015d972c26d1ec7bbbd1ad2d57d5dd3b/raw/7c848cdfba0967a681401de24ed6f4c86315d2d9/cli.log > > Here's the elf.i fille: > > https://gist.githubusercontent.com/jameshilliard/2a391d7292f3ba7412fe80e166c0f0e3/raw/63eb16ba9eff0b3ee668b16257b0948b2ca894a0/elf.i > Jakub -- I haven't looked at it in any detail, but the first thing that > jumps out at me is the -Og and the message: > > > ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c: In function > > ‘elf_is_symlink’: > > ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21: > > error: ‘st.st_mode’ may be used uninitialized in this function > > [-Werror=maybe-uninitialized] > >return S_ISLNK (st.st_mode); > > It could be the issues we've see with SRA and aggregates that have led > to the discussion around breaking those into a distinct class of uninit > warnings because we don't handle them well. I wouldn't be at all > surprised if it goes away at -O1 or -O2. I did a rebuild with -O2 and it worked. > > jeff
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On 3/18/19 6:35 PM, James Hilliard wrote: > On Mon, Mar 18, 2019 at 5:46 PM Jeff Law wrote: >> >> On 3/18/19 5:07 PM, James Hilliard wrote: >>> On Mon, Mar 18, 2019 at 4:51 PM Jakub Jelinek wrote: On Mon, Mar 18, 2019 at 04:41:05PM -0600, James Hilliard wrote: >> Thanks, but I'm saying that if you look at the code you can see that >> st is clearly initialized, by the call to lstat. I would like to see >> an explanation for why you are seeing that warning before changing the >> code to disable it. Initializing st should not be necessary here. >> For example, perhaps lstat is a macro when compiling libsanitizer; if >> that is the underlying problem, then we should fix the macro, not this >> code. > Yeah, I'm not sure why the compiler thinks lstat isn't initializing st. > What should I do to debug this further? Guess you should start by telling us which OS it is on (I can't reproduce this warning on x86_64-linux nor i686-linux with glibc 2.28), looking at preprocessed source to see what exactly lstat does (e.g. if it is some macro or inline function and what exactly it is doing). >>> I am cross compiling with buildroot master branch using ubuntu 18.10. >>> I am building gcc 8.3.0 and glibc 2.29 for the cross toolchain. >>> The build and target systems are both x86_64. >> Add "-save-temps" to the command line. That will create a .i file, send >> the .i file along with the command line. > I added --save-temps to the cflags for the gcc package build. > Here's the command line log: > https://gist.githubusercontent.com/jameshilliard/015d972c26d1ec7bbbd1ad2d57d5dd3b/raw/7c848cdfba0967a681401de24ed6f4c86315d2d9/cli.log > Here's the elf.i fille: > https://gist.githubusercontent.com/jameshilliard/2a391d7292f3ba7412fe80e166c0f0e3/raw/63eb16ba9eff0b3ee668b16257b0948b2ca894a0/elf.i Jakub -- I haven't looked at it in any detail, but the first thing that jumps out at me is the -Og and the message: > ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c: In function > ‘elf_is_symlink’: > ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21: error: > ‘st.st_mode’ may be used uninitialized in this function > [-Werror=maybe-uninitialized] >return S_ISLNK (st.st_mode); It could be the issues we've see with SRA and aggregates that have led to the discussion around breaking those into a distinct class of uninit warnings because we don't handle them well. I wouldn't be at all surprised if it goes away at -O1 or -O2. jeff
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On Mon, Mar 18, 2019 at 5:46 PM Jeff Law wrote: > > On 3/18/19 5:07 PM, James Hilliard wrote: > > On Mon, Mar 18, 2019 at 4:51 PM Jakub Jelinek wrote: > >> > >> On Mon, Mar 18, 2019 at 04:41:05PM -0600, James Hilliard wrote: > Thanks, but I'm saying that if you look at the code you can see that > st is clearly initialized, by the call to lstat. I would like to see > an explanation for why you are seeing that warning before changing the > code to disable it. Initializing st should not be necessary here. > For example, perhaps lstat is a macro when compiling libsanitizer; if > that is the underlying problem, then we should fix the macro, not this > code. > >>> Yeah, I'm not sure why the compiler thinks lstat isn't initializing st. > >>> What should I do to debug this further? > >> > >> Guess you should start by telling us which OS it is on (I can't reproduce > >> this warning on x86_64-linux nor i686-linux with glibc 2.28), looking at > >> preprocessed source to see what exactly lstat does (e.g. if it is some > >> macro > >> or inline function and what exactly it is doing). > > I am cross compiling with buildroot master branch using ubuntu 18.10. > > I am building gcc 8.3.0 and glibc 2.29 for the cross toolchain. > > The build and target systems are both x86_64. > Add "-save-temps" to the command line. That will create a .i file, send > the .i file along with the command line. I added --save-temps to the cflags for the gcc package build. Here's the command line log: https://gist.githubusercontent.com/jameshilliard/015d972c26d1ec7bbbd1ad2d57d5dd3b/raw/7c848cdfba0967a681401de24ed6f4c86315d2d9/cli.log Here's the elf.i fille: https://gist.githubusercontent.com/jameshilliard/2a391d7292f3ba7412fe80e166c0f0e3/raw/63eb16ba9eff0b3ee668b16257b0948b2ca894a0/elf.i > > jeff
Re: [C PATCH] Fix handling of cv on function return type (PR c/89734)
On Mon, 18 Mar 2019, Jakub Jelinek wrote: > Hi! > > The following patch (the second hunk in particular) should fix following > testcase. c_build_qualified_type used to be called unconditionally at that > spot, it was just the changes to use quals_used instead of type_quals there > that made it conditional, but it needs to be done even if quals_used is 0, > when TYPE_QUALS is non-zero and we need to remove quals. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On 3/18/19 5:07 PM, James Hilliard wrote: > On Mon, Mar 18, 2019 at 4:51 PM Jakub Jelinek wrote: >> >> On Mon, Mar 18, 2019 at 04:41:05PM -0600, James Hilliard wrote: Thanks, but I'm saying that if you look at the code you can see that st is clearly initialized, by the call to lstat. I would like to see an explanation for why you are seeing that warning before changing the code to disable it. Initializing st should not be necessary here. For example, perhaps lstat is a macro when compiling libsanitizer; if that is the underlying problem, then we should fix the macro, not this code. >>> Yeah, I'm not sure why the compiler thinks lstat isn't initializing st. >>> What should I do to debug this further? >> >> Guess you should start by telling us which OS it is on (I can't reproduce >> this warning on x86_64-linux nor i686-linux with glibc 2.28), looking at >> preprocessed source to see what exactly lstat does (e.g. if it is some macro >> or inline function and what exactly it is doing). > I am cross compiling with buildroot master branch using ubuntu 18.10. > I am building gcc 8.3.0 and glibc 2.29 for the cross toolchain. > The build and target systems are both x86_64. Add "-save-temps" to the command line. That will create a .i file, send the .i file along with the command line. jeff
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On Mon, Mar 18, 2019 at 4:51 PM Jakub Jelinek wrote: > > On Mon, Mar 18, 2019 at 04:41:05PM -0600, James Hilliard wrote: > > > Thanks, but I'm saying that if you look at the code you can see that > > > st is clearly initialized, by the call to lstat. I would like to see > > > an explanation for why you are seeing that warning before changing the > > > code to disable it. Initializing st should not be necessary here. > > > For example, perhaps lstat is a macro when compiling libsanitizer; if > > > that is the underlying problem, then we should fix the macro, not this > > > code. > > Yeah, I'm not sure why the compiler thinks lstat isn't initializing st. > > What should I do to debug this further? > > Guess you should start by telling us which OS it is on (I can't reproduce > this warning on x86_64-linux nor i686-linux with glibc 2.28), looking at > preprocessed source to see what exactly lstat does (e.g. if it is some macro > or inline function and what exactly it is doing). I am cross compiling with buildroot master branch using ubuntu 18.10. I am building gcc 8.3.0 and glibc 2.29 for the cross toolchain. The build and target systems are both x86_64. > > Jakub
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On Mon, Mar 18, 2019 at 04:41:05PM -0600, James Hilliard wrote: > > Thanks, but I'm saying that if you look at the code you can see that > > st is clearly initialized, by the call to lstat. I would like to see > > an explanation for why you are seeing that warning before changing the > > code to disable it. Initializing st should not be necessary here. > > For example, perhaps lstat is a macro when compiling libsanitizer; if > > that is the underlying problem, then we should fix the macro, not this > > code. > Yeah, I'm not sure why the compiler thinks lstat isn't initializing st. > What should I do to debug this further? Guess you should start by telling us which OS it is on (I can't reproduce this warning on x86_64-linux nor i686-linux with glibc 2.28), looking at preprocessed source to see what exactly lstat does (e.g. if it is some macro or inline function and what exactly it is doing). Jakub
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On Mon, Mar 18, 2019 at 2:05 PM Ian Lance Taylor wrote: > > On Mon, Mar 18, 2019 at 11:57 AM James Hilliard > wrote: > > > > On Mon, Mar 18, 2019 at 11:19 AM Ian Lance Taylor wrote: > > > > > > On Sun, Mar 17, 2019 at 6:22 PM wrote: > > > > > > > > From: James Hilliard > > > > > > > > Fixes error: ‘st.st_mode’ may be used uninitialized in this function > > > > --- > > > > libbacktrace/elf.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c > > > > index f3988ec02a0..714bfec965d 100644 > > > > --- a/libbacktrace/elf.c > > > > +++ b/libbacktrace/elf.c > > > > @@ -765,7 +765,7 @@ elf_syminfo (struct backtrace_state *state, > > > > uintptr_t addr, > > > > static int > > > > elf_is_symlink (const char *filename) > > > > { > > > > - struct stat st; > > > > + struct stat st={0}; > > > > > > > >if (lstat (filename, ) < 0) > > > > return 0; > > > > > > I can't see why that is needed. The variable is initialized right > > > there on the next non-blank line. If the compiler is giving a > > > warning, then we need to fix the compiler. > > This is the message I get: > > ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c: In > > function ‘elf_is_symlink’: > > ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21: > > error: ‘st.st_mode’ may be used uninitialized in this function > > [-Werror=maybe-uninitialized] > >return S_ISLNK (st.st_mode); > > ^ > > Thanks, but I'm saying that if you look at the code you can see that > st is clearly initialized, by the call to lstat. I would like to see > an explanation for why you are seeing that warning before changing the > code to disable it. Initializing st should not be necessary here. > For example, perhaps lstat is a macro when compiling libsanitizer; if > that is the underlying problem, then we should fix the macro, not this > code. Yeah, I'm not sure why the compiler thinks lstat isn't initializing st. What should I do to debug this further? > > Ian
[PATCH] Fix up handling of "+rm" with TREE_ADDRESSABLE types (PR target/89752)
Hi! For input arguments, we do: /* If we can't make copies, we can only accept memory. */ if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link { if (allows_mem) allows_reg = 0; else { error ("impossible constraint in %"); error ("non-memory input %d must stay in memory", i); return GS_ERROR; } } The following patch does the same thing for output operands as well. For the cases where !allows_mem, that will just result in one extra error explaining what's going on (previously one would get the impossible constraint in % error during vregs pass, now it gets during gimplification + the extra error too), and if allows_mem, it will for the + case make sure we get "=rm" (x) ... : "rm" (x) instead of "=rm" (x) ... : "0" (x) that LRA ICEs on. While the LRA ICE needs to be fixed in any case (one can still reproduce it with __asm volatile ("" : "=rm" (a0), "=rm" (a1) : "0" (a0), "1" (a1)); ), this patch opens the possibility to accept "+rm" (a0) and reject "=rm" (a0) ... "0" (a0) if reload can't prove it is the same address. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2019-03-18 Jakub Jelinek PR target/89752 * gimplify.c (gimplify_asm_expr): For output argument with TREE_ADDRESSABLE type, clear allows_reg if it allows memory, otherwise diagnose error. * g++.dg/ext/asm15.C: Check for particular diagnostic wording. * g++.dg/ext/asm16.C: Likewise. * g++.dg/ext/asm17.C: New test. --- gcc/gimplify.c.jj 2019-03-07 20:45:39.168938360 +0100 +++ gcc/gimplify.c 2019-03-18 16:18:16.515466234 +0100 @@ -6155,6 +6155,19 @@ gimplify_asm_expr (tree *expr_p, gimple_ is_inout = false; } + /* If we can't make copies, we can only accept memory. */ + if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link + { + if (allows_mem) + allows_reg = 0; + else + { + error ("impossible constraint in %"); + error ("non-memory output %d must stay in memory", i); + return GS_ERROR; + } + } + if (!allows_reg && allows_mem) mark_addressable (TREE_VALUE (link)); --- gcc/testsuite/g++.dg/ext/asm15.C.jj 2018-05-06 23:13:33.252652046 +0200 +++ gcc/testsuite/g++.dg/ext/asm15.C2019-03-18 18:00:48.907456236 +0100 @@ -6,5 +6,6 @@ struct S { S (); ~S (); int s; }; void foo (S ) { - __asm volatile ("" : "+r" (s) : : "memory"); // { dg-error "" } + __asm volatile ("" : "+r" (s) : : "memory"); // { dg-error "impossible constraint" } + // { dg-error "must stay in memory" "" { target *-*-* } .-1 } } --- gcc/testsuite/g++.dg/ext/asm16.C.jj 2018-05-06 23:13:33.252652046 +0200 +++ gcc/testsuite/g++.dg/ext/asm16.C2019-03-18 18:00:35.978664187 +0100 @@ -6,5 +6,6 @@ struct S { S (); ~S (); int s[64]; } s; void foo () { - __asm volatile ("" : "=r" (s) : : "memory"); // { dg-error "" } + __asm volatile ("" : "=r" (s) : : "memory"); // { dg-error "impossible constraint" } + // { dg-error "must stay in memory" "" { target *-*-* } .-1 } } --- gcc/testsuite/g++.dg/ext/asm17.C.jj 2019-03-18 17:57:44.409424473 +0100 +++ gcc/testsuite/g++.dg/ext/asm17.C2019-03-18 17:58:32.414651932 +0100 @@ -0,0 +1,11 @@ +// PR target/89752 +// { dg-do compile } + +struct A { A (); ~A (); short c; }; + +void +foo () +{ + A a0, a1; + __asm volatile ("" : "+rm" (a0), "+rm" (a1)); +} Jakub
[C PATCH] Fix handling of cv on function return type (PR c/89734)
Hi! The following patch (the second hunk in particular) should fix following testcase. c_build_qualified_type used to be called unconditionally at that spot, it was just the changes to use quals_used instead of type_quals there that made it conditional, but it needs to be done even if quals_used is 0, when TYPE_QUALS is non-zero and we need to remove quals. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2019-03-18 Jakub Jelinek PR c/89734 * c-decl.c (grokdeclarator): Call c_build_qualified_type on function return type even if quals_used is 0. Formatting fixes. * gcc.dg/pr89734.c: New test. --- gcc/c/c-decl.c.jj 2019-03-11 22:56:44.0 +0100 +++ gcc/c/c-decl.c 2019-03-18 13:55:17.679750733 +0100 @@ -6611,10 +6611,12 @@ grokdeclarator (const struct c_declarato quals_used &= TYPE_QUAL_ATOMIC; if (quals_used && VOID_TYPE_P (type) && really_funcdef) pedwarn (specs_loc, 0, - "function definition has qualified void return type"); + "function definition has qualified void " + "return type"); else warning_at (specs_loc, OPT_Wignored_qualifiers, - "type qualifiers ignored on function return type"); + "type qualifiers ignored on function " + "return type"); /* Ensure an error for restrict on invalid types; the DR#423 resolution is not entirely clear about @@ -6624,8 +6626,7 @@ grokdeclarator (const struct c_declarato && (!POINTER_TYPE_P (type) || !C_TYPE_OBJECT_OR_INCOMPLETE_P (TREE_TYPE (type error_at (loc, "invalid use of %"); - if (quals_used) - type = c_build_qualified_type (type, quals_used); + type = c_build_qualified_type (type, quals_used); } type_quals = TYPE_UNQUALIFIED; --- gcc/testsuite/gcc.dg/pr89734.c.jj 2019-03-18 13:59:14.916942573 +0100 +++ gcc/testsuite/gcc.dg/pr89734.c 2019-03-18 14:02:28.801814072 +0100 @@ -0,0 +1,12 @@ +/* PR c/89734 */ +/* { dg-do compile } */ +/* { dg-options "" } */ + +typedef const int CI; +typedef _Atomic int AI; + +CI foo (void); +const int foo (void); + +AI baz (void); +_Atomic int baz (void); Jakub
Re: [PATCH] [MinGW] Set __USE_MINGW_ACCESS for C++ as well
On 3/3/19 10:41 AM, Johannes Pfau wrote: > We set __USE_MINGW_ACCESS for windows hosts to use MinGWs wrapper > for the access function. The wrapper ensures that access behaves > in the expected way (e.g. for special files, such as nul). > However, we now compile most sources with the C++ compiler and > the __USE_MINGW_ACCESS in CFLAGS is not used there. This causes > GCCs build against newer msvcrt versions with incompatible > access implementations to fail. This patch adds the flag to the > CXXFLAGS for all bootstrap stages. Bootstrapped on > x86_64-mingw64-seh. > > config/ChangeLog: > > 2019-03-02 Johannes Pfau > > * mh-mingw: Also set __USE_MINGW_ACCESS flag for C++ code. > > --- > config/mh-mingw | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/config/mh-mingw b/config/mh-mingw > index bc1d27477d0..a795096f038 100644 > --- a/config/mh-mingw > +++ b/config/mh-mingw > @@ -2,6 +2,11 @@ > # Vista (see PR33281 for details). > BOOT_CFLAGS += -D__USE_MINGW_ACCESS -Wno-pedantic-ms-format > CFLAGS += -D__USE_MINGW_ACCESS > +STAGE1_CXXFLAGS += -D__USE_MINGW_ACCESS > +STAGE2_CXXFLAGS += -D__USE_MINGW_ACCESS > +STAGE3_CXXFLAGS += -D__USE_MINGW_ACCESS > +STAGE4_CXXFLAGS += -D__USE_MINGW_ACCESS > + > # Increase stack limit to a figure based on the Linux default, with 4MB added > # as GCC turns out to need that much more to pass all the limits-* tests. > LDFLAGS += -Wl,--stack,12582912 > Patch looks good, will apply soon. signature.asc Description: OpenPGP digital signature
Re: Implement C++20 constexpr , , and
On 17/03/19 22:54 -0400, Ed Smith-Rowland via libstdc++ wrote: Greetings, This patch implements C++20 p0202 - Add Constexpr Modifiers to Functions in and Headers and C++20 p1023 - constexpr comparison operators for std::array. The patch is large because of the testsuite additions. Basically, the algorithms and the array comparison operators are all marked constexpr for C++20. This has been built and tested on x86_64-linux. As discussed on IRC< the tests need to be run with -std=gnu++2a to ensure everything we test (which is not exhaustive, but is the best we can do) still works in C++2a mode. The indentation went bad here: template -_FIter1 +_GLIBCXX20_CONSTEXPR + _FIter1 find_first_of(_FIter1, _FIter1, _FIter2, _FIter2, _BinaryPredicate); I would have to look through old emails but I think there's a reason the function objects like _Iter_comp_to_val take their arguments by non-const reference. I'm very surprised that none of the algos that dispatch to __builtin_memove need changes, because those optimizations won't work in constant expressions. I would expect to have to use std::is_constant_evaluated to disable the optimizations when used in constant expressions.
Re: Fix a case in which the vector cost model was ignored
On 18 March 2019 10:58:53 CET, Richard Sandiford wrote: >This patch fixes a case in which we vectorised something with a >fully-predicated loop even after the cost model had rejected it. >E.g. the loop in the testcase has the costs: > > Vector inside of loop cost: 27 > Vector prologue cost: 0 > Vector epilogue cost: 0 > Scalar iteration cost: 7 > Scalar outside cost: 6 > Vector outside cost: 0 > prologue iterations: 0 > epilogue iterations: 0 > >and we can see that the loop executes at most three times, but we >decided to vectorise it anyway. > >(The costs here are equal for three iterations, but the same thing >happens even when the vector code is strictly more expensive.) > >The problem is the handling of "/VF" in: > > /* Calculate number of iterations required to make the vector version > profitable, relative to the loop bodies only. The following condition > must hold true: > SIC * niters + SOC > VIC * ((niters-PL_ITERS-EP_ITERS)/VF) + VOC > where > SIC = scalar iteration cost, VIC = vector iteration cost, > VOC = vector outside cost, VF = vectorization factor, > PL_ITERS = prologue iterations, EP_ITERS= epilogue iterations > SOC = scalar outside cost for run time cost model check. */ > >We treat the "/VF" as truncating, but for fully-predicated loops, it's >closer to a ceil division, since fractional iterations are handled by a >full iteration with some predicate bits set to false. > >The easiest fix seemed to be to calculate the minimum number of vector >iterations first, then use that to calculate the minimum number of >scalar >iterations. > >Calculating the minimum number of vector iterations might make sense >for >unpredicated loops too, since calculating the scalar niters directly >doesn't take into account the fact that the VIC multiple has to be an >integer. But the handling of PL_ITERS and EP_ITERS for unpredicated >loops is a bit hand-wavy anyway, so maybe vagueness here cancels out >vagueness there? > >Either way, changing this for unpredicated loops would be much too >invasive for stage 4, so the patch keeps it specific to >fully-predicated >loops (i.e. SVE) for now. There's no functional change for other >targets. > >Tested on aarch64-linux-gnu with and without SVE, and on >x86_64-linux-gnu. >This is a regression introduced by the original cost model patches for >fully-predicated loops, so OK for GCC 9? > >Thanks, >Richard > > >2019-03-18 Richard Sandiford > >gcc/ > * tree-vect-loop.c (vect_estimate_min_profitable_iters): Fix the > calculation of the minimum number of scalar iterations for > fully-predicated loops. > >gcc/testsuite/ > * gcc.target/aarch64/sve/cost_model_1.c: New test. > >Index: gcc/tree-vect-loop.c >=== >--- gcc/tree-vect-loop.c 2019-03-08 18:15:33.668751875 + >+++ gcc/tree-vect-loop.c 2019-03-18 09:55:03.257194326 + >@@ -3600,14 +3600,89 @@ vect_estimate_min_profitable_iters (loop > /* Calculate number of iterations required to make the vector version > profitable, relative to the loop bodies only. The following condition > must hold true: >- SIC * niters + SOC > VIC * ((niters-PL_ITERS-EP_ITERS)/VF) + VOC >+ SIC * niters + SOC > VIC * ((niters - NPEEL) / VF) + VOC > where > SIC = scalar iteration cost, VIC = vector iteration cost, > VOC = vector outside cost, VF = vectorization factor, >- PL_ITERS = prologue iterations, EP_ITERS= epilogue iterations >+ NPEEL = prologue iterations + epilogue iterations, > SOC = scalar outside cost for run time cost model check. */ > >- if ((scalar_single_iter_cost * assumed_vf) > (int) vec_inside_cost) >+ int saving_per_viter = (scalar_single_iter_cost * assumed_vf >+- vec_inside_cost); >+ if (saving_per_viter <= 0) >+{ >+ if (LOOP_VINFO_LOOP (loop_vinfo)->force_vectorize) >+ warning_at (vect_location.get_location_t (), OPT_Wopenmp_simd, >+ "vectorization did not happen for a simd loop"); >+ >+ if (dump_enabled_p ()) >+dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, >+ "cost model: the vector iteration cost = %d " >+ "divided by the scalar iteration cost = %d " >+ "is greater or equal to the vectorization factor = %d" >+ ".\n", >+ vec_inside_cost, scalar_single_iter_cost, assumed_vf); >+ *ret_min_profitable_niters = -1; >+ *ret_min_profitable_estimate = -1; >+ return; >+} >+ >+ /* ??? The "if" arm is written to handle all cases; see below for >what >+ we would do for !LOOP_VINFO_FULLY_MASKED_P. */ >+ if (LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)) >+{ The condition above seems to contain... >+ /* Rewriting the condition above in terms of the number of >+ vector iterations (vniters) rather than the number of >+
Re: [PATCH] improve ifcvt optimization (PR rtl-optimization/89430)
On 3/14/19 10:46 PM, JiangNing OS wrote: > This patch is to fix a missing ifcvt opportunity in back-end. For the simple > case below, > > if (...) > x = a; /* x is memory */ > /* no else */ > > We can generate conditional move and remove the branch as below if the target > cost is acceptable. > > r1 = x > r2 = a > cmp ... > csel r3, r1, r2, cond > x = r3 > > This could be safe if x is a stack variable, and there isn't any address > taken in current function, so the store speculation can be avoided. > > In practice, this optimization can improve a real application performance by > %4 on aarch64. This seems like something that should be addressed for gcc-10 rather than gcc-9. Can we come back to it once gcc-10 development starts? jeff
Re: [wwwdocs] add gcc 9 changes
On 3/18/19 10:59 AM, Martin Sebor wrote: > On 3/17/19 11:24 AM, Sandra Loosemore wrote: >> On 3/4/19 5:28 PM, Martin Sebor wrote: >>> Attached is a patch with (mostly) my changes for GCC 9. To make >>> things easier to find I grouped related changes together within >>> the sections I changed. I put warnings under the same bullet, >>> built-ins, and attributes. >> >> I have a few nit-picky comments... >> >> s/command line option/command-line option/g >> s/command line utilitly/command-line utility/ (typo, not just hyphen) >> s/red-zones/red zones/g > > Fixed. > >> >> Please try to reformat this to avoid lines longer than 80 characters, >> except where it's unavoidable in markup. > > I spotted just a couple of lines in the diff that were exactly > 80 characters long but I'm not sure where else this could be > done. AFAICS, all the long lines I added are because of > the tags. > > There are a few lines in the file that the patch doesn't touch > that are a few characters over 80. Because of the > tags and the overall lack of consistency in indentation (two vs > four spaces) I don't think it matters enough here to spend time > changing. > > Martin > > PS We could fit most lines into 80 characters by introducing some > macro for all the tags. That would make > it easier to insert these links too. E.g., > -falign-functions could be expanded by some > post-processing script into > > href="https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Optimize-Options.html#index-falign-functions;>-falign-functions > > > That way the option links would also continue to work even if > in some later release the options were removed or renamed. (In > GCC 8 I made the links point at the gcc-8.1.0 docs but I haven't > done it here.) I don't know enough about how these files are > processed to do this. > > gcc-9-changes.diff OK jeff
Re: [PATCH] have -Warray-bounds treat [max, min] ranges same as anti-ranges (PR 89720)
On 3/18/19 1:03 PM, Martin Sebor wrote: > I the -Warray-bounds enhancement committed at the beginning > of the GCC 9 window I tried to correctly handle offsets in > MEM_REFs in the [max, min] kind of a range after converting > from sizetype to ptrdiff_type, but I did it wrong. The bug > results in false positives in some unusual use cases that > I didn't consider at the time. > > The attached patch removes this incorrect handling and uses > the conservative anti-range handling for these cases instead. > > Martin > > PS Is there some technical reason why pointer offsets are > represented as the unsigned sizetype when they can be signed? I'm not aware of a conscious decision to treat them as unsigned or a particular target need to do so. It's most likely a historical accident. > > gcc-89720.diff > > PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range with > max < min > > gcc/ChangeLog: > > PR tree-optimization/89720 > * tree-vrp.c (vrp_prop::check_mem_ref): Treat range with max < min > more conservatively, the same as anti-range. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/89720 > * gcc.dg/Warray-bounds-42.c: New test. OK jeff
Re: [patch] Multilib support for amd64 FreeBSD
On 18.03.19 22:22, Jeff Law wrote: On 3/17/19 2:40 PM, Andreas Tobler wrote: Hi all, this patch adds support for multilib on x86_64-unknown-freebsd* The multilibs are 32-bit. This patch is functionality tested on gcc8 and gcc9. Test suite testing is a bit tricky since the FreeBSD dynamic linker handles both, the 32-bit and the 64-bit binaries. So, if I have a 64-bit libgcc in the LD_LIBRARY_PATH the test fails. I tweaked the build tree and removed the 64-bit libgcc to test the 32-bit binaries built. So far it looks good. Need to find a way to extend the testsuite to handle this situation. Any comments about it are welcome. Also, when would be the best time to commit this patch? Afair trunk is in stage4, right? The patch itself only affects x86_64-unknown-freebsd. TIA, Andreas 2019-03-17 Andreas Tobler * config/i386/freebsd64.h: Add bits for 32-bit multilib support. * config/i386/t-freebsd64: New file. * config.gcc: Add the t-freebsd64 for multilib support. stage4 is supposed to be regression fixes only, so it's difficult to make a case for this patch to get into gcc-9. I'd say defer to gcc-10. Ok, fine with me. Once gcc-10 opens, I will commit this patch also to all open and active branches. Thank you for the statement. Andreas
Merge from trunk to gccgo branch
I've merged trunk revision 269780 to the gccgo branch. Ian
Re: [patch] Multilib support for amd64 FreeBSD
On 3/17/19 2:40 PM, Andreas Tobler wrote: > Hi all, > > this patch adds support for multilib on x86_64-unknown-freebsd* > The multilibs are 32-bit. > > This patch is functionality tested on gcc8 and gcc9. Test suite testing > is a bit tricky since the FreeBSD dynamic linker handles both, the > 32-bit and the 64-bit binaries. So, if I have a 64-bit libgcc in the > LD_LIBRARY_PATH the test fails. > I tweaked the build tree and removed the 64-bit libgcc to test the > 32-bit binaries built. So far it looks good. Need to find a way to > extend the testsuite to handle this situation. > > Any comments about it are welcome. > > Also, when would be the best time to commit this patch? Afair trunk is > in stage4, right? > The patch itself only affects x86_64-unknown-freebsd. > > TIA, > Andreas > > 2019-03-17 Andreas Tobler > > * config/i386/freebsd64.h: Add bits for 32-bit multilib support. > * config/i386/t-freebsd64: New file. > * config.gcc: Add the t-freebsd64 for multilib support. > > stage4 is supposed to be regression fixes only, so it's difficult to make a case for this patch to get into gcc-9. I'd say defer to gcc-10. jeff
Re: [PATCH] Add a test for PR c++/89630
On 3/19/19, H.J. Lu wrote: > PR c++/89630 > * g++.dg/cpp0x/pr89630.C: New test. > --- > gcc/testsuite/g++.dg/cpp0x/pr89630.C | 15 +++ > 1 file changed, 15 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr89630.C > > diff --git a/gcc/testsuite/g++.dg/cpp0x/pr89630.C > b/gcc/testsuite/g++.dg/cpp0x/pr89630.C > new file mode 100644 > index 000..56f659f9846 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/pr89630.C > @@ -0,0 +1,15 @@ > +// { dg-do compile { target c++11 } } > +// { dg-additional-options "-mrtm -march=skylake-avx512" { target i?86-*-* > x86_64-*-* } } > + > +template class A; > +template class B; > +template struct C; > +template class D { > + using B::rank_; > + void operator()(typename C>::i); > +}; > + > +template class F { > + using B::rank_; > + void operator()(typename C>::i); > +}; > -- > 2.20.1 > This is the patch I am checking, -- H.J. From 523688c48c9875d73c82eeea3178dd9347d88965 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Tue, 19 Mar 2019 04:29:18 +0800 Subject: [PATCH] Add a test for PR c++/89630 PR c++/89630 * g++.target/i386/pr89630.C: New test. --- gcc/testsuite/g++.target/i386/pr89630.C | 15 +++ 1 file changed, 15 insertions(+) create mode 100644 gcc/testsuite/g++.target/i386/pr89630.C diff --git a/gcc/testsuite/g++.target/i386/pr89630.C b/gcc/testsuite/g++.target/i386/pr89630.C new file mode 100644 index 000..240aa742000 --- /dev/null +++ b/gcc/testsuite/g++.target/i386/pr89630.C @@ -0,0 +1,15 @@ +// { dg-do compile } +// { dg-options "-std=c++14 -mrtm -march=skylake-avx512" } + +template class A; +template class B; +template struct C; +template class D { + using B::rank_; + void operator()(typename C>::i); +}; + +template class F { + using B::rank_; + void operator()(typename C>::i); +}; -- 2.20.1
[PATCH] Add a test for PR c++/89630
PR c++/89630 * g++.dg/cpp0x/pr89630.C: New test. --- gcc/testsuite/g++.dg/cpp0x/pr89630.C | 15 +++ 1 file changed, 15 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr89630.C diff --git a/gcc/testsuite/g++.dg/cpp0x/pr89630.C b/gcc/testsuite/g++.dg/cpp0x/pr89630.C new file mode 100644 index 000..56f659f9846 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/pr89630.C @@ -0,0 +1,15 @@ +// { dg-do compile { target c++11 } } +// { dg-additional-options "-mrtm -march=skylake-avx512" { target i?86-*-* x86_64-*-* } } + +template class A; +template class B; +template struct C; +template class D { + using B::rank_; + void operator()(typename C>::i); +}; + +template class F { + using B::rank_; + void operator()(typename C>::i); +}; -- 2.20.1
libgo patch committed: Update to 1.12.1 release
This patch updates libgo to the 1.12.1 release. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. Ian Index: gcc/go/gofrontend/MERGE === --- gcc/go/gofrontend/MERGE (revision 269713) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -cc70be24502faeffefb66fd0abeb7f20a6c7792a +87945b620b2100d33e27f33e6276a4e4e5890659 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: libgo/MERGE === --- libgo/MERGE (revision 269619) +++ libgo/MERGE (working copy) @@ -1,4 +1,4 @@ -05e77d41914d247a1e7caf37d7125ccaa5a53505 +0380c9ad38843d523d9c9804fe300cb7edd7cd3c The first line of this file holds the git revision number of the last merge done from the master library sources. Index: libgo/VERSION === --- libgo/VERSION (revision 269619) +++ libgo/VERSION (working copy) @@ -1 +1 @@ -go1.12 +go1.12.1 Index: libgo/go/cmd/cgo/ast.go === --- libgo/go/cmd/cgo/ast.go (revision 269619) +++ libgo/go/cmd/cgo/ast.go (working copy) @@ -200,18 +200,6 @@ func (f *File) saveExprs(x interface{}, } case *ast.CallExpr: f.saveCall(x, context) - case *ast.GenDecl: - if x.Tok == token.CONST { - for _, spec := range x.Specs { - vs := spec.(*ast.ValueSpec) - if vs.Type == nil { - for _, name := range spec.(*ast.ValueSpec).Names { - consts[name.Name] = true - } - } - } - } - } } Index: libgo/go/cmd/cgo/gcc.go === --- libgo/go/cmd/cgo/gcc.go (revision 269619) +++ libgo/go/cmd/cgo/gcc.go (working copy) @@ -915,21 +915,16 @@ func (p *Package) rewriteCall(f *File, c needsUnsafe = true } - // Explicitly convert untyped constants to the - // parameter type, to avoid a type mismatch. - if p.isConst(f, arg) { - ptype := p.rewriteUnsafe(param.Go) + // Use "var x T = ..." syntax to explicitly convert untyped + // constants to the parameter type, to avoid a type mismatch. + ptype := p.rewriteUnsafe(param.Go) + + if !p.needsPointerCheck(f, param.Go, args[i]) || param.BadPointer { if ptype != param.Go { needsUnsafe = true } - arg = { - Fun: ptype, - Args: []ast.Expr{arg}, - } - } - - if !p.needsPointerCheck(f, param.Go, args[i]) { - fmt.Fprintf(, "_cgo%d := %s; ", i, gofmtPos(arg, origArg.Pos())) + fmt.Fprintf(, "var _cgo%d %s = %s; ", i, + gofmtLine(ptype), gofmtPos(arg, origArg.Pos())) continue } @@ -1272,47 +1267,6 @@ func (p *Package) isType(t ast.Expr) boo return false } -// isConst reports whether x is an untyped constant expression. -func (p *Package) isConst(f *File, x ast.Expr) bool { - switch x := x.(type) { - case *ast.BasicLit: - return true - case *ast.SelectorExpr: - id, ok := x.X.(*ast.Ident) - if !ok || id.Name != "C" { - return false - } - name := f.Name[x.Sel.Name] - if name != nil { - return name.IsConst() - } - case *ast.Ident: - return x.Name == "nil" || - strings.HasPrefix(x.Name, "_Ciconst_") || - strings.HasPrefix(x.Name, "_Cfconst_") || - strings.HasPrefix(x.Name, "_Csconst_") || - consts[x.Name] - case *ast.UnaryExpr: - return p.isConst(f, x.X) - case *ast.BinaryExpr: - return p.isConst(f, x.X) && p.isConst(f, x.Y) - case *ast.ParenExpr: - return p.isConst(f, x.X) - case *ast.CallExpr: - // Calling the builtin function complex on two untyped - // constants returns an untyped constant. - // TODO: It's possible to construct a case that will - // erroneously succeed if there is a local function - // named "complex", shadowing the builtin, that
Re: [PATCH] RISC-V: Update testcase.
On Mon, Mar 18, 2019 at 3:13 AM Kito Cheng wrote: > gcc/testsuite: > Kito Cheng > ChangeLog > * gcc.target/riscv/arch-1.c: Update testcase. OK. I committed with the ChangeLog entry reword to more accurately describe the change. Jim
Re: [RFC] D support for S/390
Hi! On Mon, Mar 18, 2019 at 10:59:26AM +0100, Iain Buclaw wrote: > On Fri, 15 Mar 2019 at 16:50, Robin Dapp wrote: > > during the last few days I tried to get D running on s390x (apparently > > the first Big Endian platform to try it?). I did not yet go through the > > code systematically and add a version(SystemZ) in every place where it > > might be needed but rather tried to fix test failures as they arose. > > > > HPPA has been somewhat ported, which is BigEndian as well. But I've > not done any testing beyond just the druntime unittests. powerpc64-linux works as well, no failures in the gdc testsuite at all, both -m64 and -m32. No phobos yet though. Segher
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On Mon, Mar 18, 2019 at 11:57 AM James Hilliard wrote: > > On Mon, Mar 18, 2019 at 11:19 AM Ian Lance Taylor wrote: > > > > On Sun, Mar 17, 2019 at 6:22 PM wrote: > > > > > > From: James Hilliard > > > > > > Fixes error: ‘st.st_mode’ may be used uninitialized in this function > > > --- > > > libbacktrace/elf.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c > > > index f3988ec02a0..714bfec965d 100644 > > > --- a/libbacktrace/elf.c > > > +++ b/libbacktrace/elf.c > > > @@ -765,7 +765,7 @@ elf_syminfo (struct backtrace_state *state, uintptr_t > > > addr, > > > static int > > > elf_is_symlink (const char *filename) > > > { > > > - struct stat st; > > > + struct stat st={0}; > > > > > >if (lstat (filename, ) < 0) > > > return 0; > > > > I can't see why that is needed. The variable is initialized right > > there on the next non-blank line. If the compiler is giving a > > warning, then we need to fix the compiler. > This is the message I get: > ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c: In > function ‘elf_is_symlink’: > ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21: > error: ‘st.st_mode’ may be used uninitialized in this function > [-Werror=maybe-uninitialized] >return S_ISLNK (st.st_mode); > ^ Thanks, but I'm saying that if you look at the code you can see that st is clearly initialized, by the call to lstat. I would like to see an explanation for why you are seeing that warning before changing the code to disable it. Initializing st should not be necessary here. For example, perhaps lstat is a macro when compiling libsanitizer; if that is the underlying problem, then we should fix the macro, not this code. Ian
[C++ PATCH] PR c++/89630 - ICE with dependent using-decl as template arg.
I steered Marek wrong on PR 85976; even though these two using-declarations have the same effect, they are not the same declaration, and we don't need to work to treat them as the same like we do for typedefs, so this goes back to the patch he initially sent. If we did need to treat them as the same, we would need to handle them specially in iterative_hash_template_arg as well as here. Tested x86_64-pc-linux-gnu, applying to trunk. * tree.c (cp_tree_equal): Always return false for USING_DECL. --- gcc/cp/tree.c| 9 + gcc/cp/ChangeLog | 3 +++ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index af077e795cf..718eed349c6 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -3661,6 +3661,7 @@ cp_tree_equal (tree t1, tree t2) case TEMPLATE_DECL: case IDENTIFIER_NODE: case SSA_NAME: +case USING_DECL: return false; case BASELINK: @@ -3787,14 +3788,6 @@ cp_tree_equal (tree t1, tree t2) DEFERRED_NOEXCEPT_ARGS (t2))); break; -case USING_DECL: - if (DECL_DEPENDENT_P (t1) && DECL_DEPENDENT_P (t2)) - return (cp_tree_equal (USING_DECL_SCOPE (t1), - USING_DECL_SCOPE (t2)) - && cp_tree_equal (DECL_NAME (t1), - DECL_NAME (t2))); - return false; - case LAMBDA_EXPR: /* Two lambda-expressions are never considered equivalent. */ return false; diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index d4dc5d7146a..76bc8ffaa68 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,5 +1,8 @@ 2019-03-18 Jason Merrill + PR c++/89630 - ICE with dependent using-decl as template arg. + * tree.c (cp_tree_equal): Always return false for USING_DECL. + PR c++/89761 - ICE with sizeof... in pack expansion. * pt.c (argument_pack_element_is_expansion_p): Handle ARGUMENT_PACK_SELECT. base-commit: c821b0ef575ab53abd3037cf288a6b7b40b99d12 -- 2.20.1
[C++ PATCH] PR c++/89761 - ICE with sizeof... in pack expansion.
In this testcase we get confused when looking at the sizeof... because the argument pack for 'args' has been wrapped in an ARGUMENT_PACK_SELECT as part of expanding the fold-expression. We handle this situation a bit lower down in tsubst_pack_expansion, but that doesn't help the call to argument_pack_element_is_expansion_p, which happens earlier. Tested x86_64-pc-linux-gnu, applying to trunk. * pt.c (argument_pack_element_is_expansion_p): Handle ARGUMENT_PACK_SELECT. --- gcc/cp/pt.c | 3 +++ gcc/testsuite/g++.dg/cpp1z/fold10.C | 17 + gcc/cp/ChangeLog| 4 3 files changed, 24 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp1z/fold10.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 7dc6e44cf7b..0acc16d1b92 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -11544,6 +11544,9 @@ make_fnparm_pack (tree spec_parm) static int argument_pack_element_is_expansion_p (tree arg_pack, int i) { + if (TREE_CODE (arg_pack) == ARGUMENT_PACK_SELECT) +/* We're being called before this happens in tsubst_pack_expansion. */ +arg_pack = ARGUMENT_PACK_SELECT_FROM_PACK (arg_pack); tree vec = ARGUMENT_PACK_ARGS (arg_pack); if (i >= TREE_VEC_LENGTH (vec)) return 0; diff --git a/gcc/testsuite/g++.dg/cpp1z/fold10.C b/gcc/testsuite/g++.dg/cpp1z/fold10.C new file mode 100644 index 000..1bd39a05400 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/fold10.C @@ -0,0 +1,17 @@ +// PR c++/89761 +// { dg-do compile { target c++17 } } + +template struct seq {}; +template struct S { +template +constexpr static void call(Args&&...) {} +}; + +template +auto foo (seq, Args&& ...args) { +return (S::call(args), ...); +} + +void bar() { +foo(seq<0,1,2>{}, 1,2,3); +} diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index a3341bd9672..d4dc5d7146a 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,5 +1,9 @@ 2019-03-18 Jason Merrill + PR c++/89761 - ICE with sizeof... in pack expansion. + * pt.c (argument_pack_element_is_expansion_p): Handle + ARGUMENT_PACK_SELECT. + PR c++/89640 - GNU attributes on lambda. * parser.c (cp_parser_lambda_declarator_opt): Allow GNU attributes. base-commit: 4273f1242040771577874bf759fd41b31db5773a -- 2.20.1
[C++ PATCH] PR c++/89640 - GNU attributes on lambda.
My patch for PR 60503 to fix C++11 attribute parsing on lambdas accidentally removed support for GNU attributes. Tested x86_64-pc-linux-gnu, applying to trunk. * parser.c (cp_parser_lambda_declarator_opt): Allow GNU attributes. --- gcc/cp/parser.c | 15 ++- gcc/testsuite/g++.dg/ext/attr-lambda1.C | 9 + gcc/cp/ChangeLog| 3 +++ 3 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ext/attr-lambda1.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index b8a0245ce57..81aff35cad9 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -10790,7 +10790,8 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr) This means an empty parameter list, no attributes, and no exception specification. */ tree param_list = void_list_node; - tree attributes = NULL_TREE; + tree std_attrs = NULL_TREE; + tree gnu_attrs = NULL_TREE; tree exception_spec = NULL_TREE; tree template_param_list = NULL_TREE; tree tx_qual = NULL_TREE; @@ -10849,7 +10850,8 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr) /* In the decl-specifier-seq of the lambda-declarator, each decl-specifier shall either be mutable or constexpr. */ int declares_class_or_enum; - if (cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)) + if (cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer) + && !cp_next_tokens_can_be_gnu_attribute_p (parser)) cp_parser_decl_specifier_seq (parser, CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR, _specs, _class_or_enum); @@ -10866,7 +10868,7 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr) /* Parse optional exception specification. */ exception_spec = cp_parser_exception_specification_opt (parser); - attributes = cp_parser_std_attribute_spec_seq (parser); + std_attrs = cp_parser_std_attribute_spec_seq (parser); /* Parse optional trailing return type. */ if (cp_lexer_next_token_is (parser->lexer, CPP_DEREF)) @@ -10875,6 +10877,9 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr) return_type = cp_parser_trailing_type_id (parser); } + if (cp_next_tokens_can_be_gnu_attribute_p (parser)) + gnu_attrs = cp_parser_gnu_attributes_opt (parser); + /* The function parameters must be in scope all the way until after the trailing-return-type in case of decltype. */ pop_bindings_and_leave_scope (); @@ -10922,11 +10927,11 @@ cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr) exception_spec, return_type, /*requires_clause*/NULL_TREE); -declarator->std_attributes = attributes; +declarator->std_attributes = std_attrs; fco = grokmethod (_type_specs, declarator, - NULL_TREE); + gnu_attrs); if (fco != error_mark_node) { DECL_INITIALIZED_IN_CLASS_P (fco) = 1; diff --git a/gcc/testsuite/g++.dg/ext/attr-lambda1.C b/gcc/testsuite/g++.dg/ext/attr-lambda1.C new file mode 100644 index 000..01470c32233 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-lambda1.C @@ -0,0 +1,9 @@ +// PR c++/89640 +// { dg-options "" } +// { dg-do compile { target c++11 } } + +void test() { +[]() __attribute__((noinline,cold)) { +asm volatile(""); +}(); +} diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index bc3850d3aff..a3341bd9672 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,5 +1,8 @@ 2019-03-18 Jason Merrill + PR c++/89640 - GNU attributes on lambda. + * parser.c (cp_parser_lambda_declarator_opt): Allow GNU attributes. + PR c++/89682 - wrong access error in default argument. * pt.c (tsubst_default_argument): Don't defer access checks. base-commit: 956a881aa78d012361dcca529c0e09c972ef1215 -- 2.20.1
[PATCH] have -Warray-bounds treat [max, min] ranges same as anti-ranges (PR 89720)
I the -Warray-bounds enhancement committed at the beginning of the GCC 9 window I tried to correctly handle offsets in MEM_REFs in the [max, min] kind of a range after converting from sizetype to ptrdiff_type, but I did it wrong. The bug results in false positives in some unusual use cases that I didn't consider at the time. The attached patch removes this incorrect handling and uses the conservative anti-range handling for these cases instead. Martin PS Is there some technical reason why pointer offsets are represented as the unsigned sizetype when they can be signed? PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range with max < min gcc/ChangeLog: PR tree-optimization/89720 * tree-vrp.c (vrp_prop::check_mem_ref): Treat range with max < min more conservatively, the same as anti-range. gcc/testsuite/ChangeLog: PR tree-optimization/89720 * gcc.dg/Warray-bounds-42.c: New test. Index: gcc/tree-vrp.c === --- gcc/tree-vrp.c (revision 269767) +++ gcc/tree-vrp.c (working copy) @@ -4546,9 +4546,9 @@ vrp_prop::check_mem_ref (location_t location, tree const value_range *vr = NULL; /* Determine the offsets and increment OFFRANGE for the bounds of each. - The loop computes the the range of the final offset for expressions - such as (A + i0 + ... + iN)[CSTOFF] where i0 through iN are SSA_NAMEs - in some range. */ + The loop computes the range of the final offset for expressions such + as (A + i0 + ... + iN)[CSTOFF] where i0 through iN are SSA_NAMEs in + some range. */ while (TREE_CODE (arg) == SSA_NAME) { gimple *def = SSA_NAME_DEF_STMT (arg); @@ -4583,26 +4583,21 @@ vrp_prop::check_mem_ref (location_t location, tree if (vr->kind () == VR_RANGE) { - if (tree_int_cst_lt (vr->min (), vr->max ())) + offset_int min + = wi::to_offset (fold_convert (ptrdiff_type_node, vr->min ())); + offset_int max + = wi::to_offset (fold_convert (ptrdiff_type_node, vr->max ())); + if (min < max) { - offset_int min - = wi::to_offset (fold_convert (ptrdiff_type_node, vr->min ())); - offset_int max - = wi::to_offset (fold_convert (ptrdiff_type_node, vr->max ())); - if (min < max) - { - offrange[0] += min; - offrange[1] += max; - } - else - { - offrange[0] += max; - offrange[1] += min; - } + offrange[0] += min; + offrange[1] += max; } else { - /* Conservatively add [-MAXOBJSIZE -1, MAXOBJSIZE] + /* When MIN >= MAX, the offset is effectively in a union + of two ranges: [-MAXOBJSIZE -1, MAX] and [MIN, MAXOBJSIZE]. + Since there is no way to represent such a range across + additions, conservatively add [-MAXOBJSIZE -1, MAXOBJSIZE] to OFFRANGE. */ offrange[0] += arrbounds[0]; offrange[1] += arrbounds[1]; Index: gcc/testsuite/gcc.dg/Warray-bounds-42.c === --- gcc/testsuite/gcc.dg/Warray-bounds-42.c (nonexistent) +++ gcc/testsuite/gcc.dg/Warray-bounds-42.c (working copy) @@ -0,0 +1,26 @@ +/* PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range + with max < min + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +void f (char*, int); + +#if __SIZEOF_POINTER__ == 8 + +void g (__INT64_TYPE__ i) +{ + char a[65536] = ""; + char *p = a + (i & (__INT64_TYPE__)0x3fffLL); + f (p, *(p - 6));/* { dg-bogus "\\\[-Warray-bounds" } */ +} + +#elif __SIZEOF_POINTER__ == 4 + +void h (__INT32_TYPE__ i) +{ + char a[65536] = ""; + char *p = a + (i & (__INT32_TYPE__)0x8fffLL); + f (p, *(p - 6));/* { dg-bogus "\\\[-Warray-bounds" } */ +} + +#endif
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On Mon, Mar 18, 2019 at 11:19 AM Ian Lance Taylor wrote: > > On Sun, Mar 17, 2019 at 6:22 PM wrote: > > > > From: James Hilliard > > > > Fixes error: ‘st.st_mode’ may be used uninitialized in this function > > --- > > libbacktrace/elf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c > > index f3988ec02a0..714bfec965d 100644 > > --- a/libbacktrace/elf.c > > +++ b/libbacktrace/elf.c > > @@ -765,7 +765,7 @@ elf_syminfo (struct backtrace_state *state, uintptr_t > > addr, > > static int > > elf_is_symlink (const char *filename) > > { > > - struct stat st; > > + struct stat st={0}; > > > >if (lstat (filename, ) < 0) > > return 0; > > I can't see why that is needed. The variable is initialized right > there on the next non-blank line. If the compiler is giving a > warning, then we need to fix the compiler. This is the message I get: ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c: In function ‘elf_is_symlink’: ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21: error: ‘st.st_mode’ may be used uninitialized in this function [-Werror=maybe-uninitialized] return S_ISLNK (st.st_mode); ^ > > Ian
[PATCH 3/3] rs6000: Fix altivec-7.c testcase
It currently wants to see lvx insns on AIX, and no lvx insns on Linux. What is really wanted is lvx insns when no VSX, and lxv* insns if VSX. This fixes it. 2019-03-18 Segher Boessenkool gcc/testsuite/ * gcc.target/powerpc/altivec-7.c: Look for lxv* if generating VSX instructions, and lvx if not. --- gcc/testsuite/gcc.target/powerpc/altivec-7.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.target/powerpc/altivec-7.c b/gcc/testsuite/gcc.target/powerpc/altivec-7.c index ebc4a85..42c04a1 100644 --- a/gcc/testsuite/gcc.target/powerpc/altivec-7.c +++ b/gcc/testsuite/gcc.target/powerpc/altivec-7.c @@ -85,8 +85,10 @@ int main () /* { dg-final { scan-assembler-times "vpkpx" 2 } } */ /* { dg-final { scan-assembler-times "vmulesb" 1 } } */ /* { dg-final { scan-assembler-times "vmulosb" 1 } } */ -/* { dg-final { scan-assembler-times {\mlvx\M} 0 { target { powerpc*-*-linux* } } } } */ -/* { dg-final { scan-assembler-times {\mlvx\M} 42 { target { powerpc*-*-aix* } } } } */ +/* { dg-final { scan-assembler-times {\mlvx\M} 42 { target { ! powerpc_vsx } } } } */ +/* { dg-final { scan-assembler-times {\mlxv} 0 { target { ! powerpc_vsx } } } } */ +/* { dg-final { scan-assembler-times {\mlvx\M} 0 { target powerpc_vsx } } } */ +/* { dg-final { scan-assembler-times {\mlxv} 42 { target powerpc_vsx } } } */ /* { dg-final { scan-assembler-times "lvewx" 2 } } */ /* { dg-final { scan-assembler-times "lvxl" 1 } } */ /* { dg-final { scan-assembler-times "vupklsh" 2 } } */ -- 1.8.3.1
[PATCH 1/3] rs6000: Fix pr18096-1.c test
For the big stack frame in the test GCC used to say pr18096-1.c:7:6: error: total size of local objects too large but now it says pr18096-1.c:7:6: error: total size of local objects 2147483647 exceeds maximum 2147483392 Let's just allow both in the test. 2019-03-18 Segher Boessenkool gcc/testsuite/ * gcc.target/powerpc/pr18096-1.c: Allow an error message that says "exceeds" instead of just one that talks about "too large". --- gcc/testsuite/gcc.target/powerpc/pr18096-1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.target/powerpc/pr18096-1.c b/gcc/testsuite/gcc.target/powerpc/pr18096-1.c index 74612f3..73a9ea1 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr18096-1.c +++ b/gcc/testsuite/gcc.target/powerpc/pr18096-1.c @@ -4,7 +4,7 @@ void f(char*); -void mkcatdefs(char *fname) /* { dg-error "too large" "stack frame too large" } */ +void mkcatdefs(char *fname) /* { dg-error "too large|exceeds" "stack frame too large" } */ { char line [2147483647]; f(line); -- 1.8.3.1
[PATCH 2/3] rs6000: Use pointers in bswap testcases
Currently these bswap testcases use global variables, which causes problems with -m32: the memory access is a D-form access, and when combine tries to combine that with the bswap it tries a D-form store with byte reverse. That instruction does not exist, and since combine started with only two insns here it will not try splitting this. This should be improved, but it is not what this test is testing, and the "load" case already uses a pointer, so let's do that for the store case as well. 2019-03-18 Segher Boessenkool gcc/testsuite/ * gcc.target/powerpc/bswap16.c: Use a pointer instead of a global for the "store" test as well. * gcc.target/powerpc/bswap32.c: Ditto. --- gcc/testsuite/gcc.target/powerpc/bswap16.c | 3 +-- gcc/testsuite/gcc.target/powerpc/bswap32.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/gcc/testsuite/gcc.target/powerpc/bswap16.c b/gcc/testsuite/gcc.target/powerpc/bswap16.c index 5eea4f7..89efc811 100644 --- a/gcc/testsuite/gcc.target/powerpc/bswap16.c +++ b/gcc/testsuite/gcc.target/powerpc/bswap16.c @@ -3,6 +3,5 @@ /* { dg-final { scan-assembler "lhbrx" } } */ /* { dg-final { scan-assembler "sthbrx" } } */ -unsigned short us; unsigned int load_bswap16 (unsigned short *p) { return __builtin_bswap16 (*p); } -void store_bswap16 (unsigned int a) { us = __builtin_bswap16 (a); } +void store_bswap16 (unsigned short *p, unsigned int a) { *p = __builtin_bswap16 (a); } diff --git a/gcc/testsuite/gcc.target/powerpc/bswap32.c b/gcc/testsuite/gcc.target/powerpc/bswap32.c index 1b1e189..0d1788f 100644 --- a/gcc/testsuite/gcc.target/powerpc/bswap32.c +++ b/gcc/testsuite/gcc.target/powerpc/bswap32.c @@ -3,6 +3,5 @@ /* { dg-final { scan-assembler "lwbrx" } } */ /* { dg-final { scan-assembler "stwbrx" } } */ -unsigned int ui; unsigned int load_bswap32 (unsigned int *p) { return __builtin_bswap32 (*p); } -void store_bswap32 (unsigned int a) { ui = __builtin_bswap32 (a); } +void store_bswap32 (unsigned int *p, unsigned int a) { *p = __builtin_bswap32 (a); } -- 1.8.3.1
[PATCH 0/3] rs6000: Fix some tests that fail on big-endian
All of them tested on powerpc64-linux {-m32,-m64}. Committing. Segher Segher Boessenkool (3): rs6000: Fix pr18096-1.c test rs6000: Use pointers in bswap testcases rs6000: Fix altivec-7.c testcase gcc/testsuite/gcc.target/powerpc/altivec-7.c | 6 -- gcc/testsuite/gcc.target/powerpc/bswap16.c | 3 +-- gcc/testsuite/gcc.target/powerpc/bswap32.c | 3 +-- gcc/testsuite/gcc.target/powerpc/pr18096-1.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) -- 1.8.3.1
Re: [PATCH] [libbacktrace] Initialize st in elf_is_symlink
On Sun, Mar 17, 2019 at 6:22 PM wrote: > > From: James Hilliard > > Fixes error: ‘st.st_mode’ may be used uninitialized in this function > --- > libbacktrace/elf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c > index f3988ec02a0..714bfec965d 100644 > --- a/libbacktrace/elf.c > +++ b/libbacktrace/elf.c > @@ -765,7 +765,7 @@ elf_syminfo (struct backtrace_state *state, uintptr_t > addr, > static int > elf_is_symlink (const char *filename) > { > - struct stat st; > + struct stat st={0}; > >if (lstat (filename, ) < 0) > return 0; I can't see why that is needed. The variable is initialized right there on the next non-blank line. If the compiler is giving a warning, then we need to fix the compiler. Ian
Re: [wwwdocs] add gcc 9 changes
On 3/17/19 11:24 AM, Sandra Loosemore wrote: On 3/4/19 5:28 PM, Martin Sebor wrote: Attached is a patch with (mostly) my changes for GCC 9. To make things easier to find I grouped related changes together within the sections I changed. I put warnings under the same bullet, built-ins, and attributes. I have a few nit-picky comments... s/command line option/command-line option/g s/command line utilitly/command-line utility/ (typo, not just hyphen) s/red-zones/red zones/g Fixed. Please try to reformat this to avoid lines longer than 80 characters, except where it's unavoidable in markup. I spotted just a couple of lines in the diff that were exactly 80 characters long but I'm not sure where else this could be done. AFAICS, all the long lines I added are because of the tags. There are a few lines in the file that the patch doesn't touch that are a few characters over 80. Because of the tags and the overall lack of consistency in indentation (two vs four spaces) I don't think it matters enough here to spend time changing. Martin PS We could fit most lines into 80 characters by introducing some macro for all the https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Optimize-Options.html#index-falign-functions;>-falign-functions That way the option links would also continue to work even if in some later release the options were removed or renamed. (In GCC 8 I made the links point at the gcc-8.1.0 docs but I haven't done it here.) I don't know enough about how these files are processed to do this. Index: gcc-9/changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/changes.html,v retrieving revision 1.49 diff -u -r1.49 changes.html --- gcc-9/changes.html 28 Feb 2019 21:49:05 - 1.49 +++ gcc-9/changes.html 5 Mar 2019 00:18:18 - @@ -60,8 +60,17 @@ General Improvements +The following GCC command line options have been introduced or improved. +All command line options that take a byte-size argument accept +64-bit integers as well as standard SI and IEC suffixes such as +kb and KiB, MB and MiB, +or GB and GiB denoting the corresponding +multiples of bytes. See +https://gcc.gnu.org/onlinedocs/gcc/Invoking-GCC.html#Invoking-GCC;>Invoking GCC for more. + + A new option, https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-flive-patching;>-flive-patching=[inline-only-static|inline-clone], has been introduced to provide a safe compilation for live-patching. At the same time, provides multiple-level control on the enabled IPA optimizations. @@ -79,9 +88,41 @@ alignment (e.g. -falign-loops=n:m:n2:m2). - A new built-in function, https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fexpect_005fwith_005fprobability;>__builtin_expect_with_probability, - has been added. + New pair of profiling options (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fprofile-filter-files;>-fprofile-filter-files + and https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fprofile-exclude-files;>-fprofile-exclude-files) has been added. + The options help to filter which source files are instrumented. + + + AddressSanitizer generates more compact red-zones for automatic variables. + That helps to reduce memory footprint of a sanitized binary. + +The following built-in functions have been introduced. + + +https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fexpect_005fwith_005fprobability;>__builtin_expect_with_probability to provide branch prediction probability hints to +the optimizer. + + +https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fhas_005fattribute-1;>__builtin_has_attribute determines whether a function, type, or variable has been declared with some +attribute. + + +https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fspeculation_005fsafe_005fvalue-1;>__builtin_speculation_safe_value can be used to help mitigate against unsafe speculative +execution. + + +The following attributes have been introduced. + + +The https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-copy-function-attribute;>copy function attribute has been +added. The attribute can also be applied to type definitions and to +variable declarations. + + +A large number of improvements to code generation have been made, + including but not limited to the following. + Switch expansion has been improved by using a different strategy (jump table, bit test, decision tree) for a subset of switch cases. @@ -106,6 +147,10 @@ can be transformed into 100 * how + 5 (for values defined in the switch statement). + +The following improvements to the gcov command line utilitly + have been made. + The gcov
PING #3 [PATCH] correct handling of offsets in bounds warnings (PR 89350)
Ping: https://gcc.gnu.org/ml/gcc-patches/2019-02/msg02029.html As an aside, I introduced the same mistake/false positive in three places: -Wstringop-overflow in builtins.c, -Warray-bounds in gimple-ssa-warn-restrict.c, and -Warray-bounds in tree-vrp.c. This patch fixes the first two. I'm working on PR 89720 to fix the last one. On 3/11/19 8:52 PM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2019-02/msg02029.html On 3/6/19 2:40 PM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2019-02/msg02029.html (This is marked as a P1 regression.) On 2/26/19 6:32 PM, Martin Sebor wrote: Please disregard the original patch and consider the attached version instead. On 2/26/19 5:03 PM, Martin Sebor wrote: The false positive in PR89350 is due to -Wstringop-overflow trusting that the sizetype offset in POINTER_PLUS_EXPR means the offset is, in fact, unsigned. Avoiding the false positive in the cases when this isn't so is trivial but comes at a cost of false negatives. Avoiding those will, I expect, require enhancing the compute_builtin_object_size() function and that seems risky at this stage so I would like to defer that until stage 1. Except in the instance of memset, the false positives also aren't too serious because the same problem is also diagnosed by the -Warray-bounds warning in the wrestrict pass. Unfortunately, the wrestrict pass only handles copy functions and not memset. With that as background, the attached patch avoids the -Wstringop-overflow false positive by disabling the warning for offsets whose lower bound is positive and upper bound negative. To avoid the false negatives for memset the patch lets the wrestrict pass handle the function (for the bounds checking only). While testing this I noticed that the wrestrict pass makes the same assumption about offsets, so it too is susceptible to similar false positives. The rest of the patch corrects this problem n the wrestrict pass. Because the pass doesn't depend on the compute_builtin_object_size() function as much as -Wstringop-overflow, the fix does not cause false positives (at least none that I came across). Tested on x86_64-linux. Martin
Re: [wwwdocs] My changes to gcc-9/changes.html
On 3/18/19 9:48 AM, Gerald Pfeifer wrote: On Mon, 18 Mar 2019, David Malcolm wrote: Here's a patch for the website to add my changes for GCC 9 (bearing a strong resemblance to my recent blog post) Wow, that. is. a. lot! :-) -Porting to GCC 8 page and the +Porting to GCC 9 page and the Good catch! + + GCC's diagnostics now print a left-margin when printing source code + (via the default https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fdiagnostics-show-caret;>-fdiagnostics-show-caret), + showing line numbers. This can be disabled via https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fno-diagnostics-show-line-numbers;>-fno-diagnostics-show-line-numbers. Can you think of a good way to avoid "print" and "printing" so closely together? A most minor detail, but I think we usually want to avoid such cases if possible? I don't think "left-margin" should be hyphenated here either. I'd rewrite this as something like GCC's diagnostics now print source code with a left margin showing line numbers, configurable with -Sandra
[C++ PATCH] PR c++/89682 - wrong access error in default argument.
Here we were pushing into the right access context, but we were called from a deferred checking context, so didn't end up doing the checks until after we left the access context. Tested x86_64-pc-linux-gnu, applying to trunk. * pt.c (tsubst_default_argument): Don't defer access checks. --- gcc/cp/pt.c | 2 ++ gcc/testsuite/g++.dg/overload/defarg12.C | 14 ++ gcc/cp/ChangeLog | 5 + 3 files changed, 21 insertions(+) create mode 100644 gcc/testsuite/g++.dg/overload/defarg12.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index dc5c24c47a7..7dc6e44cf7b 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -12776,6 +12776,7 @@ tsubst_default_argument (tree fn, int parmnum, tree type, tree arg, rather than in the current class. */ push_to_top_level (); push_access_scope (fn); + push_deferring_access_checks (dk_no_deferred); start_lambda_scope (parm); /* The default argument expression may cause implicitly defined @@ -12799,6 +12800,7 @@ tsubst_default_argument (tree fn, int parmnum, tree type, tree arg, inform (input_location, " when instantiating default argument for call to %qD", fn); + pop_deferring_access_checks (); pop_access_scope (fn); pop_from_top_level (); diff --git a/gcc/testsuite/g++.dg/overload/defarg12.C b/gcc/testsuite/g++.dg/overload/defarg12.C new file mode 100644 index 000..4a2b7e5b1af --- /dev/null +++ b/gcc/testsuite/g++.dg/overload/defarg12.C @@ -0,0 +1,14 @@ +// PR c++/89682 + +template +class C { +class TagType {}; +public: +C(int, TagType = makeTag()); +private: +static TagType makeTag(); +}; + +void test() { +C(1); +} diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index fa569bc1a61..bc3850d3aff 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,8 @@ +2019-03-18 Jason Merrill + + PR c++/89682 - wrong access error in default argument. + * pt.c (tsubst_default_argument): Don't defer access checks. + 2019-03-18 Paolo Carlini PR c++/85014 base-commit: 92bb50f3c7ed732e5734c57da7c6da6f7a5892a9 -- 2.20.1
Re: [wwwdocs] My changes to gcc-9/changes.html
On Mon, 18 Mar 2019, David Malcolm wrote: > Here's a patch for the website to add my changes for GCC 9 (bearing > a strong resemblance to my recent blog post) Wow, that. is. a. lot! :-) > -Porting to GCC 8 page and the > +Porting to GCC 9 page and the Good catch! > + > + GCC's diagnostics now print a left-margin when printing source code > + (via the default href="https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fdiagnostics-show-caret;>-fdiagnostics-show-caret), > + showing line numbers. This can be disabled via href="https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fno-diagnostics-show-line-numbers;>-fno-diagnostics-show-line-numbers. Can you think of a good way to avoid "print" and "printing" so closely together? A most minor detail, but I think we usually want to avoid such cases if possible? > + GCC's diagnostics can also now label regions of the source code to > + show pertinent information, such as the types within an expression. > + > + I'm wondering whether we should perhaps introduce a new class such as "diagnosticsexample" (or shorter)? Logical markup instead of physical markup is the way to go, and right now "blackbg" only appears to be used in gcc-8/changes.html and only for diagnostics, > +A new option href="https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fdiagnostics-format;>-fdiagnostics-format=json > +has been introduced, for emitting diagnostics in a machine-readable > +format. No comma before "for emitting"? > +Numerous improvements have been made to the output of href="https://gcc.gnu.org/onlinedocs/gcc/Developer-Options.html#index-fopt-info;>-fopt-info. Some lines, like this, are really quite long. Indeed some of them, in particular those with longer links, do not fit 80 characters, but usually we try to break up lines to the extent possible then, so before the link in this case. > + The old behavior can be obtained via a new -internals > + suboption of -fopt-info. A concrete usage example might be good here, i.e., how does a suboption look like here? > +If a macro is used with the wrong argument count, the C and C++ front > +ends now show the definition of the macro via a note. "that macro" instead of the "the macro" perhaps, to emphasize a bit more? > +The C++ front end now preserves source locations for longer for > +literals, id-expression and mem-initializer. Comma before "and"? And "for longer" at the end? > +The C++ front end's implementation of href="https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat;>-Wformat > +now shows precise locations within string literals, and underlines > +the pertinent arguments at bogus call sites (the C front end has done > +this since GCC 7). For example: "...C front end has been doing this..."? Thanks for those contributions _and_ taking the time to document them! You may have noticed that nearly everything above is a question. Please take the feedback into account, but do not feel obliged to make all those changes. Okay (with or without some of those changes above). Gerald
Fix two ICEs with C++ destructors and LTO
Hi, these two PRs are about C++ destructors that are not virtual but have virtual alias. The devirtualization machinery walks from alias to symbol and is then confused by not knowing the class symbol belongs to. I think it would make more sense to avoid these walks but that require more of surgery, so for GCC9 I think it is best to stop freeing contexts for desctructors. This does not save any stremaing because THIS pointer of the destructor has the same type. Bootstrapped/regtested x86_64-linux, OK? Honza PR lto/87809 PR lto/89335 * tree.c (free_lang_data_in_decl): Do not free context of C++ destrutors. * g++.dg/lto/pr87089_0.C: New testcase. * g++.dg/lto/pr87089_1.C: New testcase. * g++.dg/lto/pr89335_0.C: New testcase. Index: tree.c === --- tree.c (revision 269704) +++ tree.c (working copy) @@ -5772,10 +5772,16 @@ free_lang_data_in_decl (tree decl, struc not do well with TREE_CHAIN pointers linking them. Also do not drop containing types for virtual methods and tables because - these are needed by devirtualization. */ + these are needed by devirtualization. + C++ destructors are special because C++ frontends sometimes produces + virtual destructor as an alias of non-virtual destructor. In + devirutalization code we always walk through aliases and we need + context to be preserved too. See PR89335 */ if (TREE_CODE (decl) != FIELD_DECL && ((TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL) - || !DECL_VIRTUAL_P (decl))) + || (!DECL_VIRTUAL_P (decl) + && (TREE_CODE (decl) != FUNCTION_DECL + || !DECL_CXX_DESTRUCTOR_P (decl) DECL_CONTEXT (decl) = fld_decl_context (DECL_CONTEXT (decl)); } Index: testsuite/g++.dg/lto/pr87089_0.C === --- testsuite/g++.dg/lto/pr87089_0.C(nonexistent) +++ testsuite/g++.dg/lto/pr87089_0.C(working copy) @@ -0,0 +1,21 @@ +// { dg-lto-do link } +// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" } +namespace itpp { +template void b(a *c) { c[0].~a(); } +class CFix; +template class d { + void e(const char *); + CFix *data; +}; +class CFix { +public: + virtual ~CFix(); +}; +template <> void d::e(const char *) { b(data); } +} // namespace itpp + +int +main (void) +{ + return 0; +} Index: testsuite/g++.dg/lto/pr87089_1.C === --- testsuite/g++.dg/lto/pr87089_1.C(nonexistent) +++ testsuite/g++.dg/lto/pr87089_1.C(working copy) @@ -0,0 +1,12 @@ +namespace itpp { +enum a { b }; +class CFix { +public: + virtual ~CFix(); +}; +template class c : CFix { + ~c() {} +}; +template class c<>; +} // namespace itpp + Index: testsuite/g++.dg/lto/pr89335_0.C === --- testsuite/g++.dg/lto/pr89335_0.C(nonexistent) +++ testsuite/g++.dg/lto/pr89335_0.C(working copy) @@ -0,0 +1,16 @@ +// { dg-lto-do link } +// { dg-lto-options {{-O2 -flto -Wsuggest-final-methods}} } +// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" } +class Container +{ +public: + virtual ~Container (); +}; +class List : public Container // { dg-lto-message "final would enable devirtualization" } +{ +}; +static List cache[256]; +int main (void) +{ + return 0; +}
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
Hi, On Mon, 18 Mar 2019, Richard Biener wrote: > > Or, because an enum with these properties could be modelled as a struct > > containing one member of basetype you could also change > > record_component_aliases(), though that doesn't allow language specific > > behaviour differences. > > No, you can't. I believe that enum X and int being compatible means > that accessing an enum X object via an lvalue of effective type int > is OK _and_ accessing an int object via an lvalue of effective type > enum X is OK. I agree. But the objects type still remains whatever it was: either an enum or an int; type equivalence/identity isn't affected by compatiblity. > That commutativity doesn't hold for struct X { int i; } > vs. int i and those types are not compatible. True, but that's not the important aspect that the aliasing subsetting expresses: if A subsets B or B subsets A, all accesses between an A and a B conflict. If A is the "struct S1 {int}" and B is "int" that's exactly right, an access to the struct and one to an int (_access_, where you don't necessarily know the declared type) conflict. But if we have a C, an "struct S2 {int}" the direction of the subsetting starts to matter: A conflict B, and C conflict B, but !(A conflict C). This is exactly the situation with A and C being different enums as well. The non-transitivity of the 'conflicts' relation is expressed by having a direction in the alias subset relation: everything that is (or contains) and A conflicts with everything that is (or contains) an int, everything that is (or contains) a C conflicts with int, but nothing that is (or contains) an A conflicts with anything that is (or contains) a C (if not for other reasons). > At least unless Joseph corrects me here and that "type X is compatible > with type Y" doesn't imply "type Y is compatible with type X" > (that's not transitivity). That's not transitivity but symmetry, and that of course holds for "compatible". X compat Y, and Z compat Y --> X compat Z would be transitivity and doesn't hold. > Now, we can't currently model precisely this way of non-transitivity > of C's notion of "compatibility". > > enum X { A }; > enum Y { B }; > void *ptr = malloc (4); > *(enum X *)ptr = A; // dynamic type is now X > foo (*(enum Y *)ptr); // undefined, X and Y are not compatible(?) > foo (*(int *)ptr); // OK, X and int are compatible > *(int *)ptr = *(enum X *); // dynamic type is now int > foo (*(enum Y *)ptr); // OK(?), Y and int are compatible I'm pretty sure this can be expressed, in the way I suggested, making X subset int, Y subset int (or superset? As said, I'm always confused), and those above would work. Ciao, Michael.
[wwwdocs] My changes to gcc-9/changes.html
Here's a patch for the website to add my changes for GCC 9 (bearing a strong resemblance to my recent blog post) OK to commit? --- htdocs/gcc-9/changes.html | 310 +- 1 file changed, 305 insertions(+), 5 deletions(-) diff --git a/htdocs/gcc-9/changes.html b/htdocs/gcc-9/changes.html index e696ee1..4bdb67e 100644 --- a/htdocs/gcc-9/changes.html +++ b/htdocs/gcc-9/changes.html @@ -17,7 +17,7 @@ This page is a "brief" summary of some of the huge number of improvements in GCC 9. @@ -72,6 +72,39 @@ a work-in-progress. option completion in a shell. It is intended to be used by Bash-completion. + + GCC's diagnostics now print a left-margin when printing source code + (via the default https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fdiagnostics-show-caret;>-fdiagnostics-show-caret), + showing line numbers. This can be disabled via https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fno-diagnostics-show-line-numbers;>-fno-diagnostics-show-line-numbers. + + + GCC's diagnostics can also now label regions of the source code to + show pertinent information, such as the types within an expression. + + +$ g++ t.cc +t.cc: In function int test(const shape, const shape): +t.cc:15:4: error: no match for operator+ (operand types are boxed_valuedouble and boxed_valuedouble) + 14 | return (width(s1) * height(s1) + | ~~ + | | + | boxed_value[...] + 15 |+ width(s2) * height(s2)); + |^ ~~ + || + |boxed_value[...] + + + + These labels can be disabled via https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fno-diagnostics-show-labels;>-fno-diagnostics-show-labels. + + + +A new option https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html#index-fdiagnostics-format;>-fdiagnostics-format=json +has been introduced, for emitting diagnostics in a machine-readable +format. + + The alignment-related options https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-falign-functions;>-falign-functions, https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-falign-labels;>-falign-labels, https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-falign-loops;>-falign-loops, @@ -122,6 +155,62 @@ foo (int how) AddressSanitizer generates more compact red-zones for automatic variables. That helps to reduce memory footprint of a sanitized binary. + +Numerous improvements have been made to the output of https://gcc.gnu.org/onlinedocs/gcc/Developer-Options.html#index-fopt-info;>-fopt-info. + + Messages are now prefixed with optimized, + missed, or note, rather than the old + behavior of all being prefixed with note. + + + The output from -fopt-info can now contain information + on inlining decisions: + + +$ g++ -c inline.cc -O2 -fopt-info-inline-all +inline.cc:24:11: note: Considering inline candidate void foreach(T, T, void (*)(E)) [with T = char**; E = char*]/2. +inline.cc:24:11: optimized: Inlining void foreach(T, T, void (*)(E)) [with T = char**; E = char*]/2 into int main(int, char**)/1. +inline.cc:19:12: missed: not inlinable: void inline_me(char*)/0 - int std::puts(const char*)/3, function body not available +inline.cc:13:8: optimized: Inlined void inline_me(char*)/4 into int main(int, char**)/1 which now has time 127.363637 and size 11, net change of +0. +Unit growth for small function inlining: 16-16 (0%) + +Inlined 2 calls, eliminated 1 functions + + + + + The output from the vectorizer has been rationalized so that failed + attempts to vectorize a loop are displayed in the form + + +[LOOP-LOCATION]: couldn't vectorize this loop +[PROBLEM-LOCATION]: because of [REASON] + + + rather than an exhaustive log of all decisions made by the vectorizer. + For example: + + +$ gcc -c v.c -O3 -fopt-info-all-vec +v.c:7:3: missed: couldnt vectorize loop +v.c:10:7: missed: statement clobbers memory: __asm__ __volatile__( : : : memory); +v.c:3:6: note: vectorized 0 loops in function. +v.c:10:7: missed: statement clobbers memory: __asm__ __volatile__( : : : memory); + + + + The old behavior can be obtained via a new -internals + suboption of -fopt-info. + + + +A new option, https://gcc.gnu.org/onlinedocs/gcc/Developer-Options.html#index-fsave-optimization-record;>-fsave-optimization-record +has been added, which writes a SRCFILE.opt-record.json.gz +file describing the optimization decisions made by GCC. This is +similar to the output of -fopt-info, but with additional +metadata such as the inlining chain, and profile
Re: [PATCH] Fix PR88945
On Mon, 18 Mar 2019, Richard Biener wrote: > > The following adjusts release_ssa_name to not clear TREE_TYPE but set > it to error_mark_node instead. This avoids ICEs when dumping stmts > with released SSA names which can easily happen when doing -details > dumps and blocks being removed during the PRE-order CFG cleanup > phase which runs before SSA updating. > > It also removes restoring SSA_NAME_VAR in exchange. Not sure > what that new incremental SSA update code was - the code is > this way since forever, we now handle anonymous SSA names > with NULL SSA_NAME_VAR just fine and SSA_NAME_VAR is set > at allocation time (plus we delay releasing when a name is > registered for update). Needed to rip out some other old redundant-with-verify-SSA stuff. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2019-03-18 Richard Biener PR middle-end/88945 * tree-ssanames.c (release_ssa_name_fn): For released SSA names use a TREE_TYPE of error_mark_node to avoid ICEs when dumping basic-blocks that are removed. Remove restoring SSA_NAME_VAR. * tree-outof-ssa.c (eliminate_useless_phis): Remove redundant checking. Index: gcc/tree-ssanames.c === --- gcc/tree-ssanames.c (revision 269754) +++ gcc/tree-ssanames.c (working copy) @@ -595,7 +595,6 @@ release_ssa_name_fn (struct function *fn defining statement. */ if (! SSA_NAME_IN_FREE_LIST (var)) { - tree saved_ssa_name_var = SSA_NAME_VAR (var); int saved_ssa_name_version = SSA_NAME_VERSION (var); use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (var)); @@ -621,13 +620,14 @@ release_ssa_name_fn (struct function *fn /* Restore the version number. */ SSA_NAME_VERSION (var) = saved_ssa_name_version; - /* Hopefully this can go away once we have the new incremental - SSA updating code installed. */ - SET_SSA_NAME_VAR_OR_IDENTIFIER (var, saved_ssa_name_var); - /* Note this SSA_NAME is now in the first list. */ SSA_NAME_IN_FREE_LIST (var) = 1; + /* Put in a non-NULL TREE_TYPE so dumping code will not ICE + if it happens to come along a released SSA name and tries +to inspect its type. */ + TREE_TYPE (var) = error_mark_node; + /* And finally queue it so that it will be put on the free list. */ vec_safe_push (FREE_SSANAMES_QUEUE (fn), var); } Index: gcc/tree-outof-ssa.c === --- gcc/tree-outof-ssa.c(revision 269754) +++ gcc/tree-outof-ssa.c(working copy) @@ -809,26 +809,7 @@ eliminate_useless_phis (void) gphi *phi = gsi.phi (); result = gimple_phi_result (phi); if (virtual_operand_p (result)) - { - /* There should be no arguments which are not virtual, or the -results will be incorrect. */ - if (flag_checking) - for (size_t i = 0; i < gimple_phi_num_args (phi); i++) - { - tree arg = PHI_ARG_DEF (phi, i); - if (TREE_CODE (arg) == SSA_NAME - && !virtual_operand_p (arg)) - { - fprintf (stderr, "Argument of PHI is not virtual ("); - print_generic_expr (stderr, arg, TDF_SLIM); - fprintf (stderr, "), but the result is :"); - print_gimple_stmt (stderr, phi, 0, TDF_SLIM); - internal_error ("SSA corruption"); - } - } - - remove_phi_node (, true); - } + remove_phi_node (, true); else { /* Also remove real PHIs with no uses. */
[gcn commit] Implement circular output buffer
This patch removes the arbitrary 1000-line output limit imposed in the stdout/stderr output mechanism. The implementation is backward compatible with the old counterpart implementation already present in newlib, but I'll be updating that shortly. I've run a GCN testrun with no regressions. Andrew Stubbs Mentor Graphics / CodeSourcery Implement circular print buffer. 2019-03-18 Andrew Stubbs gcc/ * config/gcn/gcn-run.c (struct output): Make next_output unsigned. Extend queue to 1024 entries. Add "consumed" field. (gomp_print_output): Remove print_index parameter. Add final parameter. Change limit to unsigned. Use consumed field to implement circular buffer. Detect interrupted print in final pass. Flush output at the end. (run): Update gomp_print_output usage. (main): Initialize kernargs->output_data.consumed. diff --git a/gcc/config/gcn/gcn-run.c b/gcc/config/gcn/gcn-run.c index 58089843ef8..00a71014c20 100644 --- a/gcc/config/gcn/gcn-run.c +++ b/gcc/config/gcn/gcn-run.c @@ -601,7 +601,7 @@ struct kernargs struct output { int return_value; -int next_output; +unsigned int next_output; struct printf_data { int written; @@ -613,7 +613,8 @@ struct kernargs double dvalue; char text[128]; }; -} queue[1000]; +} queue[1024]; +unsigned int consumed; } output_data; struct heap @@ -624,21 +625,34 @@ struct kernargs }; /* Print any console output from the kernel. - We print all entries from print_index to the next entry without a "written" - flag. Subsequent calls should use the returned print_index value to resume - from the same point. */ + We print all entries from "consumed" to the next entry without a "written" + flag, or "next_output" is reached. The buffer is circular, but the + indices are absolute. It is assumed the kernel will stop writing data + if "next_output" wraps (becomes smaller than "consumed"). */ void -gomp_print_output (struct kernargs *kernargs, int *print_index) +gomp_print_output (struct kernargs *kernargs, bool final) { - int limit = (sizeof (kernargs->output_data.queue) - / sizeof (kernargs->output_data.queue[0])); + unsigned int limit = (sizeof (kernargs->output_data.queue) + / sizeof (kernargs->output_data.queue[0])); - int i; - for (i = *print_index; i < limit; i++) + unsigned int from = __atomic_load_n (>output_data.consumed, + __ATOMIC_ACQUIRE); + unsigned int to = kernargs->output_data.next_output; + + if (from > to) +{ + /* Overflow. */ + if (final) + printf ("GCN print buffer overflowed.\n"); + return; +} + + unsigned int i; + for (i = from; i < to; i++) { - struct printf_data *data = >output_data.queue[i]; + struct printf_data *data = >output_data.queue[i%limit]; - if (!data->written) + if (!data->written && !final) break; switch (data->type) @@ -655,16 +669,16 @@ gomp_print_output (struct kernargs *kernargs, int *print_index) case 3: printf ("%.128s%.128s", data->msg, data->text); break; + default: + printf ("GCN print buffer error!\n"); + break; } data->written = 0; + __atomic_store_n (>output_data.consumed, i+1, + __ATOMIC_RELEASE); } - - if (*print_index < limit && i == limit - && kernargs->output_data.next_output > limit) -printf ("WARNING: GCN print buffer exhausted.\n"); - - *print_index = i; + fflush (stdout); } /* Execute an already-loaded kernel on the device. */ @@ -711,16 +725,15 @@ run (void *kernargs) hsa_fns.hsa_queue_store_write_index_relaxed_fn (queue, index + 1); hsa_fns.hsa_signal_store_relaxed_fn (queue->doorbell_signal, index); /* Kernel running .. */ - int print_index = 0; while (hsa_fns.hsa_signal_wait_relaxed_fn (signal, HSA_SIGNAL_CONDITION_LT, 1, 100, HSA_WAIT_STATE_ACTIVE) != 0) { usleep (1); - gomp_print_output (kernargs, _index); + gomp_print_output (kernargs, false); } - gomp_print_output (kernargs, _index); + gomp_print_output (kernargs, true); if (debug) fprintf (stderr, "Kernel exited\n"); @@ -797,6 +810,7 @@ main (int argc, char *argv[]) for (unsigned i = 0; i < (sizeof (kernargs->output_data.queue) / sizeof (kernargs->output_data.queue[0])); i++) kernargs->output_data.queue[i].written = 0; + kernargs->output_data.consumed = 0; int offset = 0; for (int i = 0; i < kernel_argc; i++) {
Re: Fix a case in which the vector cost model was ignored
Richard Biener writes: > On Mon, Mar 18, 2019 at 10:59 AM Richard Sandiford > wrote: >> >> This patch fixes a case in which we vectorised something with a >> fully-predicated loop even after the cost model had rejected it. >> E.g. the loop in the testcase has the costs: >> >> Vector inside of loop cost: 27 >> Vector prologue cost: 0 >> Vector epilogue cost: 0 >> Scalar iteration cost: 7 >> Scalar outside cost: 6 >> Vector outside cost: 0 >> prologue iterations: 0 >> epilogue iterations: 0 >> >> and we can see that the loop executes at most three times, but we >> decided to vectorise it anyway. >> >> (The costs here are equal for three iterations, but the same thing >> happens even when the vector code is strictly more expensive.) >> >> The problem is the handling of "/VF" in: >> >> /* Calculate number of iterations required to make the vector version >> profitable, relative to the loop bodies only. The following condition >> must hold true: >> SIC * niters + SOC > VIC * ((niters-PL_ITERS-EP_ITERS)/VF) + VOC >> where >> SIC = scalar iteration cost, VIC = vector iteration cost, >> VOC = vector outside cost, VF = vectorization factor, >> PL_ITERS = prologue iterations, EP_ITERS= epilogue iterations >> SOC = scalar outside cost for run time cost model check. */ >> >> We treat the "/VF" as truncating, but for fully-predicated loops, it's >> closer to a ceil division, since fractional iterations are handled by a >> full iteration with some predicate bits set to false. >> >> The easiest fix seemed to be to calculate the minimum number of vector >> iterations first, then use that to calculate the minimum number of scalar >> iterations. >> >> Calculating the minimum number of vector iterations might make sense for >> unpredicated loops too, since calculating the scalar niters directly >> doesn't take into account the fact that the VIC multiple has to be an >> integer. But the handling of PL_ITERS and EP_ITERS for unpredicated >> loops is a bit hand-wavy anyway, so maybe vagueness here cancels out >> vagueness there? > > Well, their estimate if we do not know them statically is. If we statically > know pl/ep iters the formulat should match, no? Yeah, true. I don't see how much we gain by trying to estimate the number of peels for the runtime threshold. The NPEEL-dependent component of VOC is really just SIC * NPEEL, which makes sense given that each peeled iteration is just a normal scalar iteration. VOC also includes extra base overhead on top of that, but once the number of scalar iterations is big enough for the inner-loop saving to compensate for the base overhead, each extra peel counts equally against both sides. So I think we could estimate the minimum number of vector (rather than scalar) iterations without taking the number of peels into account for VOC, just the potential presence of peeling. We could then add a worst-case amount of prologue peeling, an estimate (as in the patch) or the actual runtime amount (which would mean calculating the value even when the vector loop isn't used). > Hopefully for GCC 10 we can even fix the case in PR89754. Yeah, would be nice. :-) Would also be good to be able to compare inner and outer loop vectorisation side-by-side and pick whichever's best. We want that for SVE to avoid e.g. using outer-loop vectorisation for a 3-iteration outer loop and a many-iteration inner loop. Thanks, Richard
[Patch] Bug85667-(x86_64) ms_abi rules aren't followed when returning short structs with float values(32-bit)
Hi All, The attached patch (pr85667.patch) fixes the subjected issue for 32-bit. Please let me know your thoughts on the patch. -- Thanks, Lokesh 85667.patch Description: Binary data
Re: Fix a case in which the vector cost model was ignored
On Mon, Mar 18, 2019 at 10:59 AM Richard Sandiford wrote: > > This patch fixes a case in which we vectorised something with a > fully-predicated loop even after the cost model had rejected it. > E.g. the loop in the testcase has the costs: > > Vector inside of loop cost: 27 > Vector prologue cost: 0 > Vector epilogue cost: 0 > Scalar iteration cost: 7 > Scalar outside cost: 6 > Vector outside cost: 0 > prologue iterations: 0 > epilogue iterations: 0 > > and we can see that the loop executes at most three times, but we > decided to vectorise it anyway. > > (The costs here are equal for three iterations, but the same thing > happens even when the vector code is strictly more expensive.) > > The problem is the handling of "/VF" in: > > /* Calculate number of iterations required to make the vector version > profitable, relative to the loop bodies only. The following condition > must hold true: > SIC * niters + SOC > VIC * ((niters-PL_ITERS-EP_ITERS)/VF) + VOC > where > SIC = scalar iteration cost, VIC = vector iteration cost, > VOC = vector outside cost, VF = vectorization factor, > PL_ITERS = prologue iterations, EP_ITERS= epilogue iterations > SOC = scalar outside cost for run time cost model check. */ > > We treat the "/VF" as truncating, but for fully-predicated loops, it's > closer to a ceil division, since fractional iterations are handled by a > full iteration with some predicate bits set to false. > > The easiest fix seemed to be to calculate the minimum number of vector > iterations first, then use that to calculate the minimum number of scalar > iterations. > > Calculating the minimum number of vector iterations might make sense for > unpredicated loops too, since calculating the scalar niters directly > doesn't take into account the fact that the VIC multiple has to be an > integer. But the handling of PL_ITERS and EP_ITERS for unpredicated > loops is a bit hand-wavy anyway, so maybe vagueness here cancels out > vagueness there? Well, their estimate if we do not know them statically is. If we statically know pl/ep iters the formulat should match, no? Hopefully for GCC 10 we can even fix the case in PR89754. > Either way, changing this for unpredicated loops would be much too > invasive for stage 4, so the patch keeps it specific to fully-predicated > loops (i.e. SVE) for now. There's no functional change for other targets. > > Tested on aarch64-linux-gnu with and without SVE, and on x86_64-linux-gnu. > This is a regression introduced by the original cost model patches for > fully-predicated loops, so OK for GCC 9? OK. Thanks, Richard. > Thanks, > Richard > > > 2019-03-18 Richard Sandiford > > gcc/ > * tree-vect-loop.c (vect_estimate_min_profitable_iters): Fix the > calculation of the minimum number of scalar iterations for > fully-predicated loops. > > gcc/testsuite/ > * gcc.target/aarch64/sve/cost_model_1.c: New test. > > Index: gcc/tree-vect-loop.c > === > --- gcc/tree-vect-loop.c2019-03-08 18:15:33.668751875 + > +++ gcc/tree-vect-loop.c2019-03-18 09:55:03.257194326 + > @@ -3600,14 +3600,89 @@ vect_estimate_min_profitable_iters (loop >/* Calculate number of iterations required to make the vector version > profitable, relative to the loop bodies only. The following condition > must hold true: > - SIC * niters + SOC > VIC * ((niters-PL_ITERS-EP_ITERS)/VF) + VOC > + SIC * niters + SOC > VIC * ((niters - NPEEL) / VF) + VOC > where > SIC = scalar iteration cost, VIC = vector iteration cost, > VOC = vector outside cost, VF = vectorization factor, > - PL_ITERS = prologue iterations, EP_ITERS= epilogue iterations > + NPEEL = prologue iterations + epilogue iterations, > SOC = scalar outside cost for run time cost model check. */ > > - if ((scalar_single_iter_cost * assumed_vf) > (int) vec_inside_cost) > + int saving_per_viter = (scalar_single_iter_cost * assumed_vf > + - vec_inside_cost); > + if (saving_per_viter <= 0) > +{ > + if (LOOP_VINFO_LOOP (loop_vinfo)->force_vectorize) > + warning_at (vect_location.get_location_t (), OPT_Wopenmp_simd, > + "vectorization did not happen for a simd loop"); > + > + if (dump_enabled_p ()) > +dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, > +"cost model: the vector iteration cost = %d " > +"divided by the scalar iteration cost = %d " > +"is greater or equal to the vectorization factor = > %d" > + ".\n", > +vec_inside_cost, scalar_single_iter_cost, > assumed_vf); > + *ret_min_profitable_niters = -1; > + *ret_min_profitable_estimate = -1; > + return; > +} > + > + /* ??? The "if" arm is written to handle
Re: [patch, fortran] Fix PR 68009, wrong prototype in runtime_error
On Sun, Mar 17, 2019 at 2:04 PM Thomas Koenig wrote: > > Hello world, > > this fixes a 7/8/9 regression. The problem is that front-end inlining > of matmul could generate calls to _gfortran_runtime_error which were > called as non-variadic. This fixes the problem by setting the > backend_decl on the resovled symbol, so it always uses the right one. > > Putting it into the resolution stage seems a bit strange, but I tried > several other methods such as putting it into the global symbol table, > and nothing else I tried worked. > > You can check on x86_64 if the patch works by doing > > $ cat nn.f90 > module x > contains >subroutine mm(a,b,c) > real, dimension(:) :: a, c > real, dimension(:,:) :: b > c = matmul(a,b) >end subroutine mm > end module x > > $ gfortran -S -O -fcheck=bounds nn.f90 > > and then looking for code snippets like > > movl$89, %ecx > movq%rdi, %rdx > movl$.LC0, %edi > movl$0, %eax > call_gfortran_runtime_error > > where setting %eax to zero indicates that we are indeed using > varargs, because %eax contains the number of float arguments, > which is zero. > > No test case, because there is not really a good way to check for this. > > So, OK for trunk? > > Regards > > Thomas > > 2019-03-17 Thomas Koenig > > PR fortran/68009 > * iresolve.c: Include trans.h. > (gfc_resolve_fe_runtine_error): Set backend_decl on > resolved_sym. Ok for trunk/7/8. Thanks! -- Janne Blomqvist
[PATCH] Fix PR88945
The following adjusts release_ssa_name to not clear TREE_TYPE but set it to error_mark_node instead. This avoids ICEs when dumping stmts with released SSA names which can easily happen when doing -details dumps and blocks being removed during the PRE-order CFG cleanup phase which runs before SSA updating. It also removes restoring SSA_NAME_VAR in exchange. Not sure what that new incremental SSA update code was - the code is this way since forever, we now handle anonymous SSA names with NULL SSA_NAME_VAR just fine and SSA_NAME_VAR is set at allocation time (plus we delay releasing when a name is registered for update). Bootstrap & regtest running on x86_64-unknown-linux-gnu. Richard. 2019-03-18 Richard Biener PR middle-end/88945 * tree-ssanames.c (release_ssa_name_fn): For released SSA names use a TREE_TYPE of error_mark_node to avoid ICEs when dumping basic-blocks that are removed. Remove restoring SSA_NAME_VAR. Index: gcc/tree-ssanames.c === --- gcc/tree-ssanames.c (revision 269754) +++ gcc/tree-ssanames.c (working copy) @@ -595,7 +595,6 @@ release_ssa_name_fn (struct function *fn defining statement. */ if (! SSA_NAME_IN_FREE_LIST (var)) { - tree saved_ssa_name_var = SSA_NAME_VAR (var); int saved_ssa_name_version = SSA_NAME_VERSION (var); use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (var)); @@ -621,13 +620,14 @@ release_ssa_name_fn (struct function *fn /* Restore the version number. */ SSA_NAME_VERSION (var) = saved_ssa_name_version; - /* Hopefully this can go away once we have the new incremental - SSA updating code installed. */ - SET_SSA_NAME_VAR_OR_IDENTIFIER (var, saved_ssa_name_var); - /* Note this SSA_NAME is now in the first list. */ SSA_NAME_IN_FREE_LIST (var) = 1; + /* Put in a non-NULL TREE_TYPE so dumping code will not ICE + if it happens to come along a released SSA name and tries +to inspect its type. */ + TREE_TYPE (var) = error_mark_node; + /* And finally queue it so that it will be put on the free list. */ vec_safe_push (FREE_SSANAMES_QUEUE (fn), var); }
[PATCH] RISC-V: Update testcase.
From: Kito Cheng gcc.target/riscv/arch-1.c getting failed after r269586, because it wrapping all option names in gcc internal messages with %< and %>, it make option name will print with single quote, and then mis-match the result in the test case. gcc/testsuite: Kito Cheng ChangeLog * gcc.target/riscv/arch-1.c: Update testcase. --- gcc/testsuite/gcc.target/riscv/arch-1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.target/riscv/arch-1.c b/gcc/testsuite/gcc.target/riscv/arch-1.c index 83d5c8a..f12879c 100644 --- a/gcc/testsuite/gcc.target/riscv/arch-1.c +++ b/gcc/testsuite/gcc.target/riscv/arch-1.c @@ -3,4 +3,4 @@ int foo() { } -/* { dg-error ".-march=rv32I: first ISA subset must be `e', `i' or `g'" "" { target *-*-* } 0 } */ +/* { dg-error ".'-march=rv32I': first ISA subset must be `e', `i' or `g'" "" { target *-*-* } 0 } */ -- 1.8.3.1
Re: [RFC] D support for S/390
On Fri, 15 Mar 2019 at 16:50, Robin Dapp wrote: > > Hi, > > during the last few days I tried to get D running on s390x (apparently > the first Big Endian platform to try it?). I did not yet go through the > code systematically and add a version(SystemZ) in every place where it > might be needed but rather tried to fix test failures as they arose. > HPPA has been somewhat ported, which is BigEndian as well. But I've not done any testing beyond just the druntime unittests. I still have an image available locally and a little more free time now, so can kickstart a rebuild and start looking at the other testsuite parts, as the same endian problems should show themselves up there as well. > > After enabling the architecture in the configure files and adding TLS > support (see initial.diff) the test suite showed 200 something failures. > Thanks, I was unsure what to do with the s390 port regarding TLS. > Including big endian handling in some test cases (tests.diff), the > number of failures went down to ~130. > The changes so far look reasonable, although the version conditions are LittleEndian and BigEndian. > Some more involved cases are left: dmd/constfold.c handles > > 'a' ~ "abc" > > but fails because 'a' is treated as int64 and only the first bytes > are memcpy'd into the result buffer. When using a similar logic as > used for > > "abc" ~ a. > > This works but seems a rather hacky approach (cat.diff). > Port::valcpy was introduced precisely for the lack of endian-neutral code in the dmd front-end, so it looks reasonable to me. I'll send it upstream if you have no objection to that. > An even more hacky fix I applied for libdruntime/rt/aaA.d (align.diff) > where algn = 0 is passed to the talign function. I suppose it shouldn't > ever be called with algn = 0 but the alignment should be set somewhere > else initially? > Alignment is written to TypeInfo, I don't think it should ever be zero. That would mean that it isn't being generated by the compiler, or read by the library correctly, so something else is amiss. > Another problem is printing of characters. std.uni.isGraphical returns > false for standard ASCII characters because of the trie traversal or > rather the final lookup in memory via PackedPtr > > cast(inout(T))(cast(U*) origin)[idx] > > This gets the first byte but should get the last byte on Big Endian. A > simple > > + ubyte[] buf = nativeToLittleEndian (origin[idx]); > + auto val = cast(inout(T))(buf.peek!U()); > > helps here, but has two problems: > > - peek!U() apparently does not work in CTFE and subsequently all > compile-time unit tests fail. > - simpleIndex() is called in other places and also does the wrong thing > > return cast(T)((origin[q] >> bits*r) & mask) > > Refraining from peek!... I tried working around it by extracting bytes > and reversing the order but this seems to hacky to create a diff :) > I got it to work for test28.d:test39() but the unit tests still fail. > > Is there a way to debug the compile-time unit tests easily? What's the > preferred method to do "the right thing" even at compile time? Any other > things that should be looked at? Any comments to the diffs so far? > There's pragma(msg) that can be used in user code. i.e: int foo() { return 42; } enum A = foo(); pragma(msg, A); // Prints '42' at compile-time. Otherwise, it's just stepping through CTFE in the compiler front-end (dmd/dinterpret.c, entrypoint is interpret), and using e->toChars() / s->toChars() to get the string representation of intermediate expressions/values. -- Iain
Fix a case in which the vector cost model was ignored
This patch fixes a case in which we vectorised something with a fully-predicated loop even after the cost model had rejected it. E.g. the loop in the testcase has the costs: Vector inside of loop cost: 27 Vector prologue cost: 0 Vector epilogue cost: 0 Scalar iteration cost: 7 Scalar outside cost: 6 Vector outside cost: 0 prologue iterations: 0 epilogue iterations: 0 and we can see that the loop executes at most three times, but we decided to vectorise it anyway. (The costs here are equal for three iterations, but the same thing happens even when the vector code is strictly more expensive.) The problem is the handling of "/VF" in: /* Calculate number of iterations required to make the vector version profitable, relative to the loop bodies only. The following condition must hold true: SIC * niters + SOC > VIC * ((niters-PL_ITERS-EP_ITERS)/VF) + VOC where SIC = scalar iteration cost, VIC = vector iteration cost, VOC = vector outside cost, VF = vectorization factor, PL_ITERS = prologue iterations, EP_ITERS= epilogue iterations SOC = scalar outside cost for run time cost model check. */ We treat the "/VF" as truncating, but for fully-predicated loops, it's closer to a ceil division, since fractional iterations are handled by a full iteration with some predicate bits set to false. The easiest fix seemed to be to calculate the minimum number of vector iterations first, then use that to calculate the minimum number of scalar iterations. Calculating the minimum number of vector iterations might make sense for unpredicated loops too, since calculating the scalar niters directly doesn't take into account the fact that the VIC multiple has to be an integer. But the handling of PL_ITERS and EP_ITERS for unpredicated loops is a bit hand-wavy anyway, so maybe vagueness here cancels out vagueness there? Either way, changing this for unpredicated loops would be much too invasive for stage 4, so the patch keeps it specific to fully-predicated loops (i.e. SVE) for now. There's no functional change for other targets. Tested on aarch64-linux-gnu with and without SVE, and on x86_64-linux-gnu. This is a regression introduced by the original cost model patches for fully-predicated loops, so OK for GCC 9? Thanks, Richard 2019-03-18 Richard Sandiford gcc/ * tree-vect-loop.c (vect_estimate_min_profitable_iters): Fix the calculation of the minimum number of scalar iterations for fully-predicated loops. gcc/testsuite/ * gcc.target/aarch64/sve/cost_model_1.c: New test. Index: gcc/tree-vect-loop.c === --- gcc/tree-vect-loop.c2019-03-08 18:15:33.668751875 + +++ gcc/tree-vect-loop.c2019-03-18 09:55:03.257194326 + @@ -3600,14 +3600,89 @@ vect_estimate_min_profitable_iters (loop /* Calculate number of iterations required to make the vector version profitable, relative to the loop bodies only. The following condition must hold true: - SIC * niters + SOC > VIC * ((niters-PL_ITERS-EP_ITERS)/VF) + VOC + SIC * niters + SOC > VIC * ((niters - NPEEL) / VF) + VOC where SIC = scalar iteration cost, VIC = vector iteration cost, VOC = vector outside cost, VF = vectorization factor, - PL_ITERS = prologue iterations, EP_ITERS= epilogue iterations + NPEEL = prologue iterations + epilogue iterations, SOC = scalar outside cost for run time cost model check. */ - if ((scalar_single_iter_cost * assumed_vf) > (int) vec_inside_cost) + int saving_per_viter = (scalar_single_iter_cost * assumed_vf + - vec_inside_cost); + if (saving_per_viter <= 0) +{ + if (LOOP_VINFO_LOOP (loop_vinfo)->force_vectorize) + warning_at (vect_location.get_location_t (), OPT_Wopenmp_simd, + "vectorization did not happen for a simd loop"); + + if (dump_enabled_p ()) +dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, +"cost model: the vector iteration cost = %d " +"divided by the scalar iteration cost = %d " +"is greater or equal to the vectorization factor = %d" + ".\n", +vec_inside_cost, scalar_single_iter_cost, assumed_vf); + *ret_min_profitable_niters = -1; + *ret_min_profitable_estimate = -1; + return; +} + + /* ??? The "if" arm is written to handle all cases; see below for what + we would do for !LOOP_VINFO_FULLY_MASKED_P. */ + if (LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)) +{ + /* Rewriting the condition above in terms of the number of +vector iterations (vniters) rather than the number of +scalar iterations (niters) gives: + +SIC * (vniters * VF + NPEEL) + SOC > VIC * vniters + VOC + +<==> vniters * (SIC * VF - VIC) > VOC - SIC * NPEEL - SOC + +For integer
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
> On 18 Mar 2019, at 09:12, Richard Biener wrote: > > On Fri, 15 Mar 2019, Jason Merrill wrote: > >> On 3/15/19 9:33 AM, Richard Biener wrote: >>> >>> The following is an attempt to fix PR71598 where C (and C++?) have >>> an implementation-defined compatible integer type for each enum >>> and the TBAA rules mandate that accesses using a compatible type >>> are allowed. >> >> This does not apply to C++; an enum does not alias its underlying type. > > Thus the following different patch, introducing c_get_alias_set and > only doing the special handling for C family frontends (I assume > that at least ObjC is also affected). As far as ObjC is concerned, I’m not aware of any special rule, thus it should behave as per the underlying C impl (therefore if it’s OK for C it is OK for ObjC). Iain > Bootstrap & regtest running on x86_64-unknown-linux-gnu, OK? > > Thanks, > Richard. > > 2019-03-18 Richard Biener > > PR c/71598 > * gimple.c: Include langhooks.h. > (gimple_get_alias_set): Treat enumeral types as the underlying > integer type. > > c/ > * c-tree.h (c_get_alias_set): Declare. > * c-objc-common.h (LANG_HOOKS_GET_ALIAS_SET): Use c_get_alias_set. > * c-objc-common.c (c_get_alias_set): Treat enumeral types > as the underlying integer type. > > * gcc.dg/torture/pr71598-1.c: New testcase. > * gcc.dg/torture/pr71598-2.c: Likewise. > > > Index: gcc/c/c-objc-common.c > === > --- gcc/c/c-objc-common.c (revision 269752) > +++ gcc/c/c-objc-common.c (working copy) > @@ -265,3 +265,22 @@ c_vla_unspec_p (tree x, tree fn ATTRIBUT > { > return c_vla_type_p (x); > } > + > +/* Special routine to get the alias set of T for C. */ > + > +alias_set_type > +c_get_alias_set (tree t) > +{ > + /* Allow aliasing between enumeral types and the underlying > + integer type. This is required since those are compatible types. */ > + if (TREE_CODE (t) == ENUMERAL_TYPE) > +{ > + tree t1 = c_common_type_for_size (tree_to_uhwi (TYPE_SIZE (t)), > + /* short-cut commoning to signed > +type. */ > + false); > + return get_alias_set (t1); > +} > + > + return c_common_get_alias_set (t); > +} > Index: gcc/c/c-objc-common.h > === > --- gcc/c/c-objc-common.h (revision 269752) > +++ gcc/c/c-objc-common.h (working copy) > @@ -43,7 +43,7 @@ along with GCC; see the file COPYING3. > #undef LANG_HOOKS_POST_OPTIONS > #define LANG_HOOKS_POST_OPTIONS c_common_post_options > #undef LANG_HOOKS_GET_ALIAS_SET > -#define LANG_HOOKS_GET_ALIAS_SET c_common_get_alias_set > +#define LANG_HOOKS_GET_ALIAS_SET c_get_alias_set > #undef LANG_HOOKS_PARSE_FILE > #define LANG_HOOKS_PARSE_FILE c_common_parse_file > #undef LANG_HOOKS_FINISH_INCOMPLETE_DECL > Index: gcc/c/c-tree.h > === > --- gcc/c/c-tree.h(revision 269752) > +++ gcc/c/c-tree.h(working copy) > @@ -623,6 +623,7 @@ extern bool c_missing_noreturn_ok_p (tre > extern bool c_warn_unused_global_decl (const_tree); > extern void c_initialize_diagnostics (diagnostic_context *); > extern bool c_vla_unspec_p (tree x, tree fn); > +extern alias_set_type c_get_alias_set (tree); > > /* in c-typeck.c */ > extern int in_alignof; > Index: gcc/gimple.c > === > --- gcc/gimple.c (revision 269752) > +++ gcc/gimple.c (working copy) > @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. > #include "stringpool.h" > #include "attribs.h" > #include "asan.h" > +#include "langhooks.h" > > > /* All the tuples have their operand vector (if present) at the very bottom > @@ -2587,6 +2588,16 @@ gimple_get_alias_set (tree t) > return get_alias_set (t1); > } > > + /* Allow aliasing between enumeral types and the underlying > + integer type. This is required for C since those are > + compatible types. */ > + else if (TREE_CODE (t) == ENUMERAL_TYPE) > +{ > + tree t1 = lang_hooks.types.type_for_size (tree_to_uhwi (TYPE_SIZE (t)), > + false /* short-cut above */); > + return get_alias_set (t1); > +} > + > return -1; > } > > Index: gcc/testsuite/gcc.dg/torture/pr71598-1.c > === > --- gcc/testsuite/gcc.dg/torture/pr71598-1.c (nonexistent) > +++ gcc/testsuite/gcc.dg/torture/pr71598-1.c (working copy) > @@ -0,0 +1,21 @@ > +/* { dg-do run } */ > +/* { dg-additional-options "-fno-short-enums" } */ > + > +enum e1 { c1 }; > + > +__attribute__((noinline,noclone)) > +int f(enum e1 *p, unsigned *q) > +{ > + *p = c1; > + *q = 2; > + return *p; > +} > + > +int main() > +{ > + unsigned x; > + > +
Re: [patch, fortran] Fix PR 68009, wrong prototype in runtime_error
Hi Thomas! On Sun, 17 Mar 2019 13:04:14 +0100, Thomas Koenig wrote: > this fixes a 7/8/9 regression. The problem is that front-end inlining > of matmul could generate calls to _gfortran_runtime_error which were > called as non-variadic. This fixes the problem by setting the > backend_decl on the resovled symbol, so it always uses the right one. Thanks for looking into this. > No test case, because there is not really a good way to check for this. Tested-by: Thomas Schwinge For nvptx target testing, I see: [-FAIL:-]{+PASS:+} gfortran.dg/bounds_check_20.f90 -O (test for excess errors) [-UNRESOLVED:-]{+FAIL:+} gfortran.dg/bounds_check_20.f90 -O [-compilation failed to produce executable-]{+execution test+} ... which before complained about "Inconsistent redefinition of '_gfortran_runtime_error', number of parameters differs". (The "execution test" FAIL now is "Prototype doesn't match for '_gfortran_matmul_r4'", which -- expectedly so -- is not addressed by your patch; not sure if there's a PR open for that.) Your patch generally resolves all the "Prototype doesn't match for '_gfortran_runtime_error'" FAILs, and effects the following progressions: @@ -23499,29 +23500,19 @@ nvptx-run: cuLinkAddData failed: unknown error (CUDA_ERROR_UNKNOWN, 999) , should match Fortran runtime error: Array bound mismatch for dimension 1 of array.* PASS: gfortran.dg/matmul_bounds_10.f90 -O1 (test for excess errors) PASS: gfortran.dg/matmul_bounds_10.f90 -O1 execution test [-FAIL:-]{+PASS:+} gfortran.dg/matmul_bounds_10.f90 -O1 output pattern test,[-is error : Prototype doesn't match for '_gfortran_runtime_error' in 'input file 3 at offset 8110', first defined in 'input file 1 at offset 1805'^M-] [-nvptx-run: cuLinkAddData failed: unknown error (CUDA_ERROR_UNKNOWN, 999)^M-] [-, should match-] Fortran runtime error: Array bound mismatch for dimension 1 of array.* PASS: gfortran.dg/matmul_bounds_10.f90 -O2 (test for excess errors) PASS: gfortran.dg/matmul_bounds_10.f90 -O2 execution test [-FAIL:-]{+PASS:+} gfortran.dg/matmul_bounds_10.f90 -O2 output pattern test,[-is error : Prototype doesn't match for '_gfortran_runtime_error' in 'input file 3 at offset 7382', first defined in 'input file 1 at offset 1805'^M-] [-nvptx-run: cuLinkAddData failed: unknown error (CUDA_ERROR_UNKNOWN, 999)^M-] [-, should match-] Fortran runtime error: Array bound mismatch for dimension 1 of array.* PASS: gfortran.dg/matmul_bounds_10.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) PASS: gfortran.dg/matmul_bounds_10.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions execution test [-FAIL:-]{+PASS:+} gfortran.dg/matmul_bounds_10.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions output pattern test,[-is error : Prototype doesn't match for '_gfortran_runtime_error' in 'input file 3 at offset 6778', first defined in 'input file 1 at offset 1805'^M-] [-nvptx-run: cuLinkAddData failed: unknown error (CUDA_ERROR_UNKNOWN, 999)^M-] [-, should match-] Fortran runtime error: Array bound mismatch for dimension 1 of array.* PASS: gfortran.dg/matmul_bounds_10.f90 -O3 -g (test for excess errors) PASS: gfortran.dg/matmul_bounds_10.f90 -O3 -g execution test [-FAIL:-]{+PASS:+} gfortran.dg/matmul_bounds_10.f90 -O3 -g output pattern test,[-is error : Prototype doesn't match for '_gfortran_runtime_error' in 'input file 3 at offset 6981', first defined in 'input file 1 at offset 1805'^M-] [-nvptx-run: cuLinkAddData failed: unknown error (CUDA_ERROR_UNKNOWN, 999)^M-] [-, should match-] Fortran runtime error: Array bound mismatch for dimension 1 of array.* PASS: gfortran.dg/matmul_bounds_10.f90 -Os (test for excess errors) PASS: gfortran.dg/matmul_bounds_10.f90 -Os execution test [-FAIL:-]{+PASS:+} gfortran.dg/matmul_bounds_10.f90 -Os output pattern test,[-is error : Prototype doesn't match for '_gfortran_runtime_error' in 'input file 3 at offset 7803', first defined in 'input file 1 at offset 1805'^M-] [-nvptx-run: cuLinkAddData failed: unknown error (CUDA_ERROR_UNKNOWN, 999)^M-] [-, should match-] Fortran runtime error: Array bound mismatch for dimension 1 of array.* FAIL: gfortran.dg/matmul_bounds_11.f90 -O0 (test for excess errors) UNRESOLVED: gfortran.dg/matmul_bounds_11.f90 -O0 compilation failed to produce executable FAIL: gfortran.dg/matmul_bounds_11.f90 -O1 (test for excess errors) @@ -23574,29 +23565,19 @@ nvptx-run: cuLinkAddData failed: unknown error (CUDA_ERROR_UNKNOWN, 999) , should match Fortran runtime error: Array bound mismatch for dimension 2 of array.* PASS: gfortran.dg/matmul_bounds_2.f90 -O1 (test for excess errors) PASS: gfortran.dg/matmul_bounds_2.f90 -O1
Re: [PATCH 0/5][ARC] Fix failing tests and use newer macros.
Thanks Andrew for your review, Claudiu On Wed, Mar 6, 2019 at 12:20 PM Claudiu Zissulescu wrote: > > Hi, > > Please find a set of 5 patches as this: > > [ARC] Introduce ADJUST_REG_ALLOC_ORDER. > This patch just cleans the old way of changing the register > allocation order and uses a gcc macro specially made for this task. > > [ARC] Enable code density frame option for elf targets. > The compress instruction for frame are there for a while, but they > are not enabled by default when using Os, just do it. > > [ARC] Define TARGET_HAVE_SPECULATION_SAFE_VALUE. > New macro fixes a dejagnu failure. > > [ARC] Fix tst_movb pattern. > Fixes dejagnu failure. > > [ARC] Refactor deprecated macros. > Old macros replaced with their newer equivalent. > > > Thanks, > Claudiu > > gcc/config/arc/arc-protos.h | 3 +- > gcc/config/arc/arc.c| 101 ++-- > gcc/config/arc/arc.h| 44 ++-- > gcc/config/arc/arc.md | 24 - > gcc/config/arc/arc.opt | 2 +- > gcc/config/arc/elf.h| 4 ++ > gcc/config/arc/linux.h | 4 ++ > 7 files changed, 99 insertions(+), 83 deletions(-) > > -- > 2.20.1 >
Re: [PATCH] Fix PR87561, 416.gamess slowdown
On Fri, 15 Mar 2019, Jan Hubicka wrote: > > > > A previous patch of mine correcting the vectorizer target cost model > > to properly cost scalar FP ops vs. scalar INT ops regressed > > 416.gamess by ~10% on all modern x86 archs. > > > > The following mitigates this in the cost modeling by noticing > > the vectorized loop in question has all loads and stores performed > > strided (built up from scalar loads/stores) and building upon > > the pessimization of strided loads added last year. > > > > The first half is treating strided stores the same as strided > > loads which may make sense (but the latency and dependence > > arguments do not count here). Unfortunately that alone > > doesn't make 416.gamess vectorization fail because we end up > > with TYPE_VECTOR_SUBPARTS == 2 (AVX256 vectorization is rejected > > due to cost reasons already). Now comes the second half > > which is to push it over the edge, adjusting the previous > > pessimization by multiplying with TYPE_VECTOR_SUBPARTS + 1 > > instead of just TYPE_VECTOR_SUBPARTS which makes the biggest > > difference for smaller vectors. > > > > I've benchmarked this on a Haswell machine with SPEC 2006 > > confirming the regression is fixed and re-benchmarked > > appearant regressions with 3 runs confirming that was noise > > and we end up with maybe even a progression there > > (see the bugzilla audit-trail for details). > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > OK for trunk? > > > > Note I'm going to apply as two revisions to allow bisection > > between the two changes, first pushing pessimizing strided > > stores and then adjusting the factor. > > > > Thanks, > > Richard. > > > > 2019-03-15 Richard Biener > > > > PR target/87561 > > * config/i386/i386.c (ix86_add_stmt_cost): Apply strided > > load pessimization to stores as well. > > * config/i386/i386.c (ix86_add_stmt_cost): Pessimize strided > > loads and stores a bit more. > > Looks good to me. Store costs are even more iffy than other costs > because they are not part of dependency chain,so I guess whatever seems > to work best in practice is good. Applied as r269753 and r269754. Please report any issue with this. Richard.
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
On Fri, 15 Mar 2019, Jason Merrill wrote: > On 3/15/19 9:33 AM, Richard Biener wrote: > > > > The following is an attempt to fix PR71598 where C (and C++?) have > > an implementation-defined compatible integer type for each enum > > and the TBAA rules mandate that accesses using a compatible type > > are allowed. > > This does not apply to C++; an enum does not alias its underlying type. Thus the following different patch, introducing c_get_alias_set and only doing the special handling for C family frontends (I assume that at least ObjC is also affected). Bootstrap & regtest running on x86_64-unknown-linux-gnu, OK? Thanks, Richard. 2019-03-18 Richard Biener PR c/71598 * gimple.c: Include langhooks.h. (gimple_get_alias_set): Treat enumeral types as the underlying integer type. c/ * c-tree.h (c_get_alias_set): Declare. * c-objc-common.h (LANG_HOOKS_GET_ALIAS_SET): Use c_get_alias_set. * c-objc-common.c (c_get_alias_set): Treat enumeral types as the underlying integer type. * gcc.dg/torture/pr71598-1.c: New testcase. * gcc.dg/torture/pr71598-2.c: Likewise. Index: gcc/c/c-objc-common.c === --- gcc/c/c-objc-common.c (revision 269752) +++ gcc/c/c-objc-common.c (working copy) @@ -265,3 +265,22 @@ c_vla_unspec_p (tree x, tree fn ATTRIBUT { return c_vla_type_p (x); } + +/* Special routine to get the alias set of T for C. */ + +alias_set_type +c_get_alias_set (tree t) +{ + /* Allow aliasing between enumeral types and the underlying + integer type. This is required since those are compatible types. */ + if (TREE_CODE (t) == ENUMERAL_TYPE) +{ + tree t1 = c_common_type_for_size (tree_to_uhwi (TYPE_SIZE (t)), + /* short-cut commoning to signed + type. */ + false); + return get_alias_set (t1); +} + + return c_common_get_alias_set (t); +} Index: gcc/c/c-objc-common.h === --- gcc/c/c-objc-common.h (revision 269752) +++ gcc/c/c-objc-common.h (working copy) @@ -43,7 +43,7 @@ along with GCC; see the file COPYING3. #undef LANG_HOOKS_POST_OPTIONS #define LANG_HOOKS_POST_OPTIONS c_common_post_options #undef LANG_HOOKS_GET_ALIAS_SET -#define LANG_HOOKS_GET_ALIAS_SET c_common_get_alias_set +#define LANG_HOOKS_GET_ALIAS_SET c_get_alias_set #undef LANG_HOOKS_PARSE_FILE #define LANG_HOOKS_PARSE_FILE c_common_parse_file #undef LANG_HOOKS_FINISH_INCOMPLETE_DECL Index: gcc/c/c-tree.h === --- gcc/c/c-tree.h (revision 269752) +++ gcc/c/c-tree.h (working copy) @@ -623,6 +623,7 @@ extern bool c_missing_noreturn_ok_p (tre extern bool c_warn_unused_global_decl (const_tree); extern void c_initialize_diagnostics (diagnostic_context *); extern bool c_vla_unspec_p (tree x, tree fn); +extern alias_set_type c_get_alias_set (tree); /* in c-typeck.c */ extern int in_alignof; Index: gcc/gimple.c === --- gcc/gimple.c(revision 269752) +++ gcc/gimple.c(working copy) @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. #include "stringpool.h" #include "attribs.h" #include "asan.h" +#include "langhooks.h" /* All the tuples have their operand vector (if present) at the very bottom @@ -2587,6 +2588,16 @@ gimple_get_alias_set (tree t) return get_alias_set (t1); } + /* Allow aliasing between enumeral types and the underlying + integer type. This is required for C since those are + compatible types. */ + else if (TREE_CODE (t) == ENUMERAL_TYPE) +{ + tree t1 = lang_hooks.types.type_for_size (tree_to_uhwi (TYPE_SIZE (t)), + false /* short-cut above */); + return get_alias_set (t1); +} + return -1; } Index: gcc/testsuite/gcc.dg/torture/pr71598-1.c === --- gcc/testsuite/gcc.dg/torture/pr71598-1.c(nonexistent) +++ gcc/testsuite/gcc.dg/torture/pr71598-1.c(working copy) @@ -0,0 +1,21 @@ +/* { dg-do run } */ +/* { dg-additional-options "-fno-short-enums" } */ + +enum e1 { c1 }; + +__attribute__((noinline,noclone)) +int f(enum e1 *p, unsigned *q) +{ + *p = c1; + *q = 2; + return *p; +} + +int main() +{ + unsigned x; + + if (f(, ) != 2) +__builtin_abort(); + return 0; +} Index: gcc/testsuite/gcc.dg/torture/pr71598-2.c === --- gcc/testsuite/gcc.dg/torture/pr71598-2.c(nonexistent) +++ gcc/testsuite/gcc.dg/torture/pr71598-2.c(working copy) @@ -0,0 +1,47 @@ +/* { dg-do run } */ +/* { dg-additional-options "-fshort-enums" } */ + +enum e1 { c1 =
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
On Fri, 15 Mar 2019, Michael Matz wrote: > Hi, > > On Fri, 15 Mar 2019, Michael Matz wrote: > > > I.e. what you touched is the naming of sets (giving them identifiers), > > whereas you should have touched where the relations between the sets are > > established. I _think_ instead of giving enum and basetypes the same > > alias set you should have rather called record_alias_subset(intset, > > enumset) (or the other way around, I'm always confused there :) ). > > Or, because an enum with these properties could be modelled as a struct > containing one member of basetype you could also change > record_component_aliases(), though that doesn't allow language specific > behaviour differences. No, you can't. I believe that enum X and int being compatible means that accessing an enum X object via an lvalue of effective type int is OK _and_ accessing an int object via an lvalue of effective type enum X is OK. That commutativity doesn't hold for struct X { int i; } vs. int i and those types are not compatible. At least unless Joseph corrects me here and that "type X is compatible with type Y" doesn't imply "type Y is compatible with type X" (that's not transitivity). Now, we can't currently model precisely this way of non-transitivity of C's notion of "compatibility". enum X { A }; enum Y { B }; void *ptr = malloc (4); *(enum X *)ptr = A; // dynamic type is now X foo (*(enum Y *)ptr); // undefined, X and Y are not compatible(?) foo (*(int *)ptr); // OK, X and int are compatible *(int *)ptr = *(enum X *); // dynamic type is now int foo (*(enum Y *)ptr); // OK(?), Y and int are compatible Joseph, do you agree that enum and int are to be handled the same way as we handle unsigned vs. signed integer types (though those get an extra bullet in 6.5/7 and so they are probably not compatible). Richard.
Re: Fix PR 86979
On Fri, Mar 15, 2019 at 01:25:57PM +0300, Andrey Belevantsev wrote: > 2019-03-15 Andrey Belevantsev > > PR middle-end/89676 There is a typo in the PR number that I've fixed and added a testcase for this, committed as obvious: 2019-03-18 Jakub Jelinek PR middle-end/86979 * gcc.dg/pr86979.c: New test. --- gcc/testsuite/gcc.dg/pr86979.c.jj 2019-03-18 09:28:00.090554402 +0100 +++ gcc/testsuite/gcc.dg/pr86979.c 2019-03-18 09:27:49.313728712 +0100 @@ -0,0 +1,5 @@ +/* { dg-options "-Og -fPIC -fschedule-insns2 -fselective-scheduling2 -fno-tree-fre --param=max-sched-extend-regions-iters=2" } */ +/* { dg-require-effective-target scheduling } */ +/* { dg-require-effective-target fpic } */ + +#include "../gcc.c-torture/compile/pr69102.c" --- gcc/ChangeLog.jj2019-03-18 09:17:43.669527380 +0100 +++ gcc/ChangeLog 2019-03-18 09:30:15.647361882 +0100 @@ -1,6 +1,6 @@ 2019-03-18 Andrey Belevantsev - PR middle-end/89676 + PR middle-end/86979 * sel-sched.c (compute_av_set_at_bb_end): When we have an ineligible successor, use NULL as its av set. Jakub