Re: [C++ Patch] PR 63985
.. and, to make things more interesting ;) for: for (int a, b, c: arr) we currently give: 63985.C:7:19: error: expected initializer before ‘:’ token 63985.C:7:21: warning: range-based ‘for’ loops only available with -std=c++11 or -std=gnu++11 because, inside cp_parse_init_declarator we are in error mode for range-based after the first comma thus, as a for loop, we complain about the missing initializer; then in cp_parser_for_init_statement we notice the ':' and we give the warning / we think is a range-based. Paolo.
Re: [C++ Patch] PR 63985
Hi again, thus, in order to deal with the various subissues we have got, I prepared the below, which passes testing. As you can see, the diagnostic triggers only once, for the left-most '=' and/or the left-most ','. I suppose that's Ok, I'm not 100% sure about the wording of the error messages though. Otherwise, I'm quite happy with the patch ;) modulo maybe the need to pass around a location_t*, isn't something we do very often. Let me know... Thanks, Paolo. // Index: cp/parser.c === --- cp/parser.c (revision 219042) +++ cp/parser.c (working copy) @@ -2124,7 +2124,8 @@ static tree cp_parser_decltype /* Declarators [gram.dcl.decl] */ static tree cp_parser_init_declarator - (cp_parser *, cp_decl_specifier_seq *, vecdeferred_access_check, va_gc *, bool, bool, int, bool *, tree *); + (cp_parser *, cp_decl_specifier_seq *, vecdeferred_access_check, va_gc *, + bool, bool, int, bool *, tree *, location_t *); static cp_declarator *cp_parser_declarator (cp_parser *, cp_parser_declarator_kind, int *, bool *, bool, bool); static cp_declarator *cp_parser_direct_declarator @@ -11454,6 +11455,8 @@ cp_parser_simple_declaration (cp_parser* parser, cp_decl_specifier_seq decl_specifiers; int declares_class_or_enum; bool saw_declarator; + location_t comma_loc = UNKNOWN_LOCATION; + location_t equal_loc = UNKNOWN_LOCATION; if (maybe_range_for_decl) *maybe_range_for_decl = NULL_TREE; @@ -11528,12 +11531,16 @@ cp_parser_simple_declaration (cp_parser* parser, if (saw_declarator) { - /* If we are processing next declarator, coma is expected */ + /* If we are processing next declarator, comma is expected */ token = cp_lexer_peek_token (parser-lexer); gcc_assert (token-type == CPP_COMMA); cp_lexer_consume_token (parser-lexer); if (maybe_range_for_decl) - *maybe_range_for_decl = error_mark_node; + { + *maybe_range_for_decl = error_mark_node; + if (comma_loc == UNKNOWN_LOCATION) + comma_loc = token-location; + } } else saw_declarator = true; @@ -11545,7 +11552,8 @@ cp_parser_simple_declaration (cp_parser* parser, /*member_p=*/false, declares_class_or_enum, function_definition_p, - maybe_range_for_decl); + maybe_range_for_decl, + equal_loc); /* If an error occurred while parsing tentatively, exit quickly. (That usually happens when in the body of a function; each statement is treated as a declaration-statement until proven @@ -11631,7 +11639,15 @@ cp_parser_simple_declaration (cp_parser* parser, /* Consume the `;'. */ if (!maybe_range_for_decl) - cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON); +cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON); + else if (cp_lexer_next_token_is (parser-lexer, CPP_COLON)) +{ + if (equal_loc != UNKNOWN_LOCATION) + error_at (equal_loc, initializer in range-based %for% loop); + if (comma_loc != UNKNOWN_LOCATION) + error_at (comma_loc, + multiple declarations in range-based %for% loop); +} done: pop_deferring_access_checks (); @@ -16842,8 +16858,13 @@ cp_parser_asm_definition (cp_parser* parser) parsed declaration if it is an uninitialized single declarator not followed by a `;', or to error_mark_node otherwise. Either way, the trailing `;', if present, will not be consumed. If returned, this declarator will be - created with SD_INITIALIZED but will not call cp_finish_decl. */ + created with SD_INITIALIZED but will not call cp_finish_decl. + If MAYBE_RANGE_FOR_DECL is not NULL, and *EQUAL_LOC is equal to + UNKNOWN_LOCATION, and there is an initializer, the pointed location_t + is set to the location of the '=' (or `(', or '{' in C++11) token + introducing the initializer. */ + static tree cp_parser_init_declarator (cp_parser* parser, cp_decl_specifier_seq *decl_specifiers, @@ -16852,7 +16873,8 @@ cp_parser_init_declarator (cp_parser* parser, bool member_p, int declares_class_or_enum, bool* function_definition_p, - tree* maybe_range_for_decl) + tree* maybe_range_for_decl, + location_t* equal_loc) { cp_token *token = NULL, *asm_spec_start_token = NULL, *attributes_start_token = NULL; @@ -16875,6 +16897,7 @@ cp_parser_init_declarator (cp_parser* parser, tree pushed_scope = NULL_TREE; bool range_for_decl_p = false; bool saved_default_arg_ok_p
Re: [arm][patch] fix arm_neon_ok check on !arm_arch7
On 03/12/14 15:03, Andrew Stubbs wrote: The tools have always allowed us to drop down the arch to march=armv5te along with using -mfpu=neon. We are now changing command line behaviour, so an inform in terms of diagnostics to the user would be useful as it states that we don't really have mfpu=neon generating neon code any more because of this particular case. If we are to do this then the original patch is probably not enough as it then doesn't handle the case of TARGET_VFP3 / TARGET_VFP5 / TARGET_NEON_FP16 / TARGET_FP16 / TARGET_FPU_ARMV8 etc. etc. etc. I'll take a look at those shortly. Or, not so shortly. It seems that, on ARM, the arch/CPU setting is basically orthogonal to the FPU setting, and the compiler doesn't even try to match the one to the other. The assembler does the same. In fact, the testcases that James refers to, that have hard-coded -march options, really do emit armv4 code with Neon, say, although most probably don't have vectorizable code. They only work because they're most likely executed on Neon hardware. This means that there's no obvious patch to fix the issue, in the compiler. It's easy to reject Neon for pre-v7 CPUs, but that has consequences, as we've seen. We'd have to have a table of fall-back FPUs or something, and that doesn't seem straight-forward (and anyway, I'm not sure what values to enter into that table). So, I've attacked the problem from the other end, and updated the compiler check. OK to commit? Andrew 2014-12-23 Andrew Stubbs a...@codesourcery.com gcc/testsuite/ * lib/target-supports.exp (check_effective_target_arm_neon_ok_nocache): Don't try to test Neon on ARM architures before v7. Index: gcc/testsuite/lib/target-supports.exp === --- gcc/testsuite/lib/target-supports.exp (revision 219043) +++ gcc/testsuite/lib/target-supports.exp (working copy) @@ -2565,6 +2565,9 @@ if { [check_no_compiler_messages_nocache arm_neon_ok object { #include arm_neon.h int dummy; + #if __ARM_ARCH 7 + #error Architecture too old for NEON. + #endif } $flags] } { set et_arm_neon_flags $flags return 1
Re: [PATCH] Treat a sibling call as though it does a wild read
On Tue, Dec 16, 2014 at 5:17 PM, John David Anglin dave.ang...@bell.net wrote: On 8-Dec-14, at 5:36 PM, Jeff Law wrote: On 12/08/14 15:15, John David Anglin wrote: On 12/8/2014 3:01 PM, Jeff Law wrote: The above is wrong for sibcalls. Sibcall arguments are relative to the incoming argument pointer. Is this always the frame pointer? I don't think it's always the frame pointer. Don't we use an argument pointer for the PA64 runtime? If I recall, it was the only port that had a non-eliminable argument pointer at the time. I don't think PA64 is an argument against this as sibcalls don't work in the PA64 runtime (they are disabled in pa.c) because the argument pointer isn't a fixed register. I guess in theory it could be fixed if it was saved and restored across calls. But there's nothing that says another port in the future won't have similar characteristics as the PA, so while the PA isn't particularly important, it shows there's cases where arguments won't be accessed by the FP. DSE as it stands doesn't look at argument pointer based stores and I suspect they would be deleted with current code. Agreed. I believe that this version addresses the above issues. While there may be some opportunity to optimize the handling of sibling call arguments, I think it is more important to get the overall logic correct. Also, it's obviously a rare situation for the arguments to be pushed to the stack. Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11. This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64388 -- H.J.
[gomp4] Merge trunk r218997 (2014-12-21) into gomp-4_0-branch
Hi! In r219044, I have committed a merge from trunk r218997 (2014-12-21) into gomp-4_0-branch. This includes updating our Fortran front end changes for using plain flag_openacc instead of gfc_option.gfc_flag_openacc, but I have not yet updated most of our changes for the other recent Fortran front end diagnostic changes, such as using %dim% instead of 'dim', or %qs instead of '%s', and so on. Grüße, Thomas pgpivYCnBl2Tu.pgp Description: PGP signature
Re: [C++ Patch] PR 63985
On 12/23/2014 11:30 AM, Paolo Carlini wrote: if (maybe_range_for_decl) - *maybe_range_for_decl = error_mark_node; + { + *maybe_range_for_decl = error_mark_node; + if (*equal_loc == UNKNOWN_LOCATION) + tmp_equal_loc = token-location; Even though the range-for case is the only place we care about the initializer location, I'd rather set *equal_loc whenever equal_loc is non-null. We can also use the local initializer location in place of initializer_start_token-location. And let's call it init_loc rather than equal_loc. Jason
Go patch committed: Copy array in range statement
The Go frontend failed to copy an array in a range statement. The effect was that if the code in the loop changed future elements of the original array, it would incorrectly change the results of the iteration. This patch from Chris Manghane fixes the problem. This issue 34 in the gofrontend issue tracker. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian diff -r 738f975a6972 go/statements.cc --- a/go/statements.cc Fri Dec 19 14:18:12 2014 -0800 +++ b/go/statements.cc Tue Dec 23 10:36:47 2014 -0800 @@ -5558,8 +5558,9 @@ // The loop we generate: // len_temp := len(range) + // range_temp := range // for index_temp = 0; index_temp len_temp; index_temp++ { - // value_temp = range[index_temp] + // value_temp = range_temp[index_temp] // index = index_temp // value = value_temp // original body @@ -5573,9 +5574,11 @@ Block* init = new Block(enclosing, loc); Expression* ref = this-make_range_ref(range_object, range_temp, loc); + range_temp = Statement::make_temporary(NULL, ref, loc); Expression* len_call = this-call_builtin(gogo, len, ref, loc); Temporary_statement* len_temp = Statement::make_temporary(index_temp-type(), len_call, loc); + init-add_statement(range_temp); init-add_statement(len_temp); Expression* zexpr = Expression::make_integer_ul(0, NULL, loc); @@ -5605,7 +5608,7 @@ { iter_init = new Block(body_block, loc); - ref = this-make_range_ref(range_object, range_temp, loc); + ref = Expression::make_temporary_reference(range_temp, loc); Expression* ref2 = Expression::make_temporary_reference(index_temp, loc); Expression* index = Expression::make_index(ref, ref2, NULL, NULL, loc);
nvptx-tools and nvptx-newlib (was: The nvptx port [10/11+] Target files)
Hi! On Mon, 10 Nov 2014 17:19:57 +0100, Bernd Schmidt ber...@codesourcery.com wrote: The scripts (11/11) I've put up on github, along with a hacked up newlib. These are at https://github.com/bernds/nvptx-tools https://github.com/bernds/nvptx-newlib They are likely to migrate to MentorEmbedded from bernds, but that had some permissions problems last week. That has recently been done: https://github.com/MentorEmbedded/nvptx-tools and https://github.com/MentorEmbedded/nvptx-newlib are now available. (I'm aware that we still are to write up how to actually build and test all this.) Grüße, Thomas signature.asc Description: PGP signature
[nvptx-tools, committed] Also install [...]/nvptx-none/bin/ar and [...]/nvptx-none/bin/ranlib.
GCC needs this, if nvptx-none-ar and nvptx-none-ranlib aren't found in $PATH. --- tools/Makefile.in | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git tools/Makefile.in tools/Makefile.in index 6829e29..8dcedbc 100644 --- tools/Makefile.in +++ tools/Makefile.in @@ -45,11 +45,15 @@ install: all for x in $(PROGRAMS); do \ $(INSTALL_PROGRAM) -m 755 $$x $(DESTDIR)$(bindir)/$$x; \ done - rm -f $(DESTDIR)$(prefix)/nvptx-none/bin/as - rm -f $(DESTDIR)$(prefix)/nvptx-none/bin/ld rm -f $(DESTDIR)$(bindir)/nvptx-none-ar rm -f $(DESTDIR)$(bindir)/nvptx-none-ranlib - $(LN_S) $(DESTDIR)$(bindir)/nvptx-none-as $(DESTDIR)$(prefix)/nvptx-none/bin/as - $(LN_S) $(DESTDIR)$(bindir)/nvptx-none-ld $(DESTDIR)$(prefix)/nvptx-none/bin/ld $(LN_S) $(AR) $(DESTDIR)$(bindir)/nvptx-none-ar $(LN_S) $(RANLIB) $(DESTDIR)$(bindir)/nvptx-none-ranlib + rm -f $(DESTDIR)$(prefix)/nvptx-none/bin/ar + rm -f $(DESTDIR)$(prefix)/nvptx-none/bin/as + rm -f $(DESTDIR)$(prefix)/nvptx-none/bin/ld + rm -f $(DESTDIR)$(prefix)/nvptx-none/bin/ranlib + $(LN_S) $(DESTDIR)$(bindir)/nvptx-none-ar $(DESTDIR)$(prefix)/nvptx-none/bin/ar + $(LN_S) $(DESTDIR)$(bindir)/nvptx-none-as $(DESTDIR)$(prefix)/nvptx-none/bin/as + $(LN_S) $(DESTDIR)$(bindir)/nvptx-none-ld $(DESTDIR)$(prefix)/nvptx-none/bin/ld + $(LN_S) $(DESTDIR)$(bindir)/nvptx-none-ranlib $(DESTDIR)$(prefix)/nvptx-none/bin/ranlib -- 1.8.1.1
[nvptx-tools, committed 2/2] Don't link the assembler and linker against CUDA libraries.
..., because that's not needed. --- tools/Makefile.in | 2 +- tools/configure| 4 +--- tools/configure.ac | 6 ++ 3 files changed, 4 insertions(+), 8 deletions(-) diff --git tools/Makefile.in tools/Makefile.in index f007b83..2886fe6 100644 --- tools/Makefile.in +++ tools/Makefile.in @@ -36,7 +36,7 @@ nvptx-none-as: $(srcdir)/nvptx-as.c nvptx-none-run: $(srcdir)/nvptx-run.c $(CXX) $(CPPFLAGS) $(CXXFLAGS) -I$(srcdir)/../include \ $ -o $@ \ - -L ../libiberty -liberty $(LDFLAGS) $(LIBS) + -L ../libiberty -liberty $(LDFLAGS) $(LIBS) -lcuda -lcudart .PHONY: install diff --git tools/configure tools/configure index 7295da1..0f77626 100755 --- tools/configure +++ tools/configure @@ -3133,7 +3133,6 @@ fi if test x$with_cuda_driver_lib != x; then CUDA_DRIVER_LDFLAGS=-L$with_cuda_driver_lib fi -LIBS=$LIBS -lcuda CXXFLAGS=$CXXFLAGS $CUDA_DRIVER_CPPFLAGS LDFLAGS=$LDFLAGS $CUDA_DRIVER_LDFLAGS @@ -3146,7 +3145,7 @@ CFLAGS=$CUDA_DRIVER_CPPFLAGS $CFLAGS cudart_save_LDFLAGS=$LDFLAGS LDFLAGS=$CUDA_DRIVER_LDFLAGS $LDFLAGS cudart_save_LIBS=$LIBS -LIBS=$LIBS -lcudart +LIBS=$LIBS -lcuda -lcudart cat confdefs.h - _ACEOF conftest.$ac_ext /* end confdefs.h. */ @@ -3163,7 +3162,6 @@ const char *p; CUresult r; cuGetErrorString (r, p); cudaDeviceReset (); _ACEOF if ac_fn_c_try_link $LINENO; then : NVPTX_RUN=nvptx-none-run -cudart_save_LIBS=$LIBS fi rm -f core conftest.err conftest.$ac_objext \ conftest$ac_exeext conftest.$ac_ext diff --git tools/configure.ac tools/configure.ac index f871a70..35e8607 100644 --- tools/configure.ac +++ tools/configure.ac @@ -36,7 +36,6 @@ fi if test x$with_cuda_driver_lib != x; then CUDA_DRIVER_LDFLAGS=-L$with_cuda_driver_lib fi -LIBS=$LIBS -lcuda CXXFLAGS=$CXXFLAGS $CUDA_DRIVER_CPPFLAGS LDFLAGS=$LDFLAGS $CUDA_DRIVER_LDFLAGS @@ -48,14 +47,13 @@ CFLAGS=$CUDA_DRIVER_CPPFLAGS $CFLAGS cudart_save_LDFLAGS=$LDFLAGS LDFLAGS=$CUDA_DRIVER_LDFLAGS $LDFLAGS cudart_save_LIBS=$LIBS -LIBS=$LIBS -lcudart +LIBS=$LIBS -lcuda -lcudart AC_LINK_IFELSE([AC_LANG_PROGRAM( [#include cuda.h #include cuda_runtime.h ], [const char *p; CUresult r; cuGetErrorString (r, p); cudaDeviceReset (); ])], -[NVPTX_RUN=nvptx-none-run -cudart_save_LIBS=$LIBS]) +[NVPTX_RUN=nvptx-none-run]) AC_MSG_RESULT($NVPTX_RUN) CFLAGS=$cudart_save_CFLAGS -- 1.8.1.1
[nvptx-tools, committed 1/2] Support standard compiler configuration variables.
Also remove the LIBPATH define, which is not used anywhere. --- tools/Makefile.in | 27 +++ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git tools/Makefile.in tools/Makefile.in index 8dcedbc..f007b83 100644 --- tools/Makefile.in +++ tools/Makefile.in @@ -1,10 +1,13 @@ +CXX = @CXX@ +CPPFLAGS = @CPPFLAGS@ +CXXFLAGS = @CXXFLAGS@ +LDFLAGS = @LDFLAGS@ +LIBS = @LIBS@ + LN_S=@LN_S@ AR = @AR@ RANLIB = @RANLIB@ INSTALL = @INSTALL@ -CXXFLAGS = @CXXFLAGS@ -LDFLAGS = @LDFLAGS@ -LIBS = @LIBS@ INSTALL_PROGRAM = @INSTALL_PROGRAM@ INSTALL_DATA = @INSTALL_DATA@ @@ -21,19 +24,19 @@ PROGRAMS=nvptx-none-ld nvptx-none-as @NVPTX_RUN@ all: $(PROGRAMS) nvptx-none-ld: $(srcdir)/nvptx-ld.c - $(CXX) $(srcdir)/nvptx-ld.c -o nvptx-none-ld -g3 -O $(CXXFLAGS) $(LDFLAGS) \ - -L ../libiberty -liberty $(LIBS) \ - -DLIBPATH=\$(prefix)/nvptx-none/lib\ -I$(srcdir)/../include + $(CXX) $(CPPFLAGS) $(CXXFLAGS) -I$(srcdir)/../include \ + $ -o $@ \ + -L ../libiberty -liberty $(LDFLAGS) $(LIBS) nvptx-none-as: $(srcdir)/nvptx-as.c - $(CXX) $(srcdir)/nvptx-as.c -o nvptx-none-as -g3 -O $(CXXFLAGS) $(LDFLAGS) \ - -L ../libiberty -liberty $(LIBS) \ - -DLIBPATH=\$(prefix)/nvptx-none/lib\ -I$(srcdir)/../include + $(CXX) $(CPPFLAGS) $(CXXFLAGS) -I$(srcdir)/../include \ + $ -o $@ \ + -L ../libiberty -liberty $(LDFLAGS) $(LIBS) nvptx-none-run: $(srcdir)/nvptx-run.c - $(CXX) $(srcdir)/nvptx-run.c -o nvptx-none-run -g3 -O $(CXXFLAGS) $(LDFLAGS) \ - -L ../libiberty -liberty $(LIBS) \ - -DLIBPATH=\$(prefix)/nvptx-none/lib\ -I$(srcdir)/../include + $(CXX) $(CPPFLAGS) $(CXXFLAGS) -I$(srcdir)/../include \ + $ -o $@ \ + -L ../libiberty -liberty $(LDFLAGS) $(LIBS) .PHONY: install -- 1.8.1.1
Re: [PATCH 1/1] gcc/ira-build.c: save a conflict obj compare in ira_flattening
On 12/22/14 18:28, Zhouyi Zhou wrote: Thanks Jeff for reviewing, I forget to mention that I do not have write access to gcc. Can you install for me ? Done. Thanks, Jeff
Re: [PATCH 3/4] Add Visium support to gcc
On 12/22/14 04:14, Eric Botcazou wrote: Revised version after Joseph's comments and latest libgcc changes. gcc/ChangeLog 2014-12-22 Eric Botcazou ebotca...@adacore.com * config.gcc: Add Visium support. * configure.ac: Likewise. * configure: Regenerate. * doc/extend.texi (interrupt attribute): Add Visium. * doc/invoke.texi: Document Visium options. * doc/install.texi: Document Visium target. * doc/md.texi: Document Visium constraints. * common/config/visium: New directory. * config/visium: Likewise. I don't see anything particularly offensive. Actually it looks like a reasonably clean RISC port. I'm a little concerned about the MODES_TIEABLE_P definition, but if it's working, I wouldn't mess with it. Any thoughts on using LRA for this port? Ideally we want to be moving away from reload as much as we can. I didn't look closely, do you need blockage insns in your epilogue/prologue? For the prologue, if you store callee saved registers using the frame pointer, then you need a blockage to ensure those stores don't bubble up before the local stack gets allocated. And you need something analogous in the epilogue. I didn't check your port carefully for this, but I'd advise doing so. Presumably you're going to be the maintainer for this port? If not you, then who will maintain the port (so i can get maintainership officially blessed by the SC). I think this is good to go into the trunk. The blockage issue (if it's an issue) can be resolved as a follow-up. Jeff
Re: #pragma GCC unroll support
Mike Stump mikest...@comcast.net writes: Thanks for doing that. I missed this pragma several times. I didn’t engineer ivdeps and unroll together. Does it sound reasonable to allow both to be used at the same time on the same loop? If so, I can add the two other cases, presently I just handle one of them then the loop. Yes it would be useful, in theory this could allow more vectorizing (although I'm not sure the BB vectorizer would handle it right now) I support using -1 for a directive that says, don’t peel, don’t unroll. As a UI issue, I think this is wrong. I want to to be either 0 or 1, those two seem better. But, not sure which is the right one of the two. Which number says, don’t unroll, I’m smarter than you think. Define a new pragma for these cases? If we have a loop that we know can only be unroll 7 times, and the user says unroll 8, should we unroll it 7 times? Presently I do. The other option, is to ignore the directive when we know it is non-sensicle. I think it's fine to only unroll 7 times, better than not unrolling. Patch looks ok to me from a quick read except for the missing documentation (but I cannot approve anything) -Andi
Re: [patch] New std::string implementation
Hello Jonathan, starting with around r218964, New std::string implementation. the following program does no longer link correctly: cat test1.cc #include string int main() { std::string x; x.erase(x.begin(), x.end()); } g++ test1.cc /tmp/ccgup1FU.o: In function `main': test1.cc:(.text+0x41): undefined reference to `std::__cxx11::basic_stringchar, std::char_traitschar, std::allocatorchar::erase(__gnu_cxx::__normal_iteratorchar*, std::__cxx11::basic_stringchar, std::char_traitschar, std::allocatorchar, __gnu_cxx::__normal_iteratorchar*, std::__cxx11::basic_stringchar, std::char_traitschar, std::allocatorchar)' collect2: error: ld returned 1 exit status This does however not happen at -O1 and above. Any ideas? Thanks, Bernd.
libgo patch committed: Remove race detector calls
The gc Go compiler supports a race detector, and some of the support for that has been copied into libgo. It is useless there because gccgo does not support a race detector, and the calls are normally optimized away. However, this does not occur when the library is built without optimization. This patch by Chris Manghane removes the useless race calls from libgo. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian diff -r 6cbef7b5f57b libgo/runtime/chan.goc --- a/libgo/runtime/chan.gocTue Dec 23 10:38:47 2014 -0800 +++ b/libgo/runtime/chan.gocTue Dec 23 12:30:58 2014 -0800 @@ -6,7 +6,6 @@ #include runtime.h #include arch.h #include go-type.h -#include race.h #include malloc.h #include chan.h @@ -15,7 +14,6 @@ static voiddequeueg(WaitQ*); static SudoG* dequeue(WaitQ*); static voidenqueue(WaitQ*, SudoG*); -static voidracesync(Hchan*, SudoG*); static Hchan* makechan(ChanType *t, int64 hint) @@ -82,6 +80,7 @@ static bool chansend(ChanType *t, Hchan *c, byte *ep, bool block, void *pc) { + USED(pc); SudoG *sg; SudoG mysg; G* gp; @@ -90,9 +89,6 @@ g = runtime_g(); - if(raceenabled) - runtime_racereadobjectpc(ep, t-__element_type, runtime_getcallerpc(t), chansend); - if(c == nil) { USED(t); if(!block) @@ -116,8 +112,6 @@ } runtime_lock(c); - if(raceenabled) - runtime_racereadpc(c, pc, chansend); if(c-closed) goto closed; @@ -126,8 +120,6 @@ sg = dequeue(c-recvq); if(sg != nil) { - if(raceenabled) - racesync(c, sg); runtime_unlock(c); gp = sg-g; @@ -183,11 +175,6 @@ goto asynch; } - if(raceenabled) { - runtime_raceacquire(chanbuf(c, c-sendx)); - runtime_racerelease(chanbuf(c, c-sendx)); - } - runtime_memmove(chanbuf(c, c-sendx), ep, c-elemsize); if(++c-sendx == c-dataqsiz) c-sendx = 0; @@ -225,8 +212,6 @@ if(runtime_gcwaiting()) runtime_gosched(); - // raceenabled: don't need to check ep, as it is always on the stack. - if(debug) runtime_printf(chanrecv: chan=%p\n, c); @@ -256,8 +241,6 @@ sg = dequeue(c-sendq); if(sg != nil) { - if(raceenabled) - racesync(c, sg); runtime_unlock(c); if(ep != nil) @@ -319,11 +302,6 @@ goto asynch; } - if(raceenabled) { - runtime_raceacquire(chanbuf(c, c-recvx)); - runtime_racerelease(chanbuf(c, c-recvx)); - } - if(ep != nil) runtime_memmove(ep, chanbuf(c, c-recvx), c-elemsize); runtime_memclr(chanbuf(c, c-recvx), c-elemsize); @@ -352,8 +330,6 @@ runtime_memclr(ep, c-elemsize); if(received != nil) *received = false; - if(raceenabled) - runtime_raceacquire(c); runtime_unlock(c); if(mysg.releasetime 0) runtime_blockevent(mysg.releasetime - t0, 2); @@ -789,8 +765,6 @@ break; case CaseSend: - if(raceenabled) - runtime_racereadpc(c, runtime_selectgo, chansend); if(c-closed) goto sclose; if(c-dataqsiz 0) { @@ -874,24 +848,11 @@ *cas-receivedp = true; } - if(raceenabled) { - if(cas-kind == CaseRecv cas-sg.elem != nil) - runtime_racewriteobjectpc(cas-sg.elem, c-elemtype, selectgo, chanrecv); - else if(cas-kind == CaseSend) - runtime_racereadobjectpc(cas-sg.elem, c-elemtype, selectgo, chansend); - } - selunlock(sel); goto retc; asyncrecv: // can receive from buffer - if(raceenabled) { - if(cas-sg.elem != nil) - runtime_racewriteobjectpc(cas-sg.elem, c-elemtype, selectgo, chanrecv); - runtime_raceacquire(chanbuf(c, c-recvx)); - runtime_racerelease(chanbuf(c, c-recvx)); - } if(cas-receivedp != nil) *cas-receivedp = true; if(cas-sg.elem != nil) @@ -914,11 +875,6 @@ asyncsend: // can send to buffer - if(raceenabled) { - runtime_raceacquire(chanbuf(c, c-sendx)); - runtime_racerelease(chanbuf(c, c-sendx)); - runtime_racereadobjectpc(cas-sg.elem, c-elemtype, selectgo, chansend); - } runtime_memmove(chanbuf(c, c-sendx), cas-sg.elem, c-elemsize); if(++c-sendx == c-dataqsiz) c-sendx = 0; @@ -937,11 +893,6 @@ syncrecv:
Re: [C++ Patch] PR 63985
Hi, On 12/23/2014 07:13 PM, Jason Merrill wrote: On 12/23/2014 11:30 AM, Paolo Carlini wrote: if (maybe_range_for_decl) -*maybe_range_for_decl = error_mark_node; +{ + *maybe_range_for_decl = error_mark_node; + if (*equal_loc == UNKNOWN_LOCATION) +tmp_equal_loc = token-location; Even though the range-for case is the only place we care about the initializer location, I'd rather set *equal_loc whenever equal_loc is non-null. We can also use the local initializer location in place of initializer_start_token-location. And let's call it init_loc rather than equal_loc. Ok. I think the below is closer, the only detail I want to mention is that it still checks *init_loc == UNKNOWN_LOCATION before assigning, because the location of the error doesn't change if more than one initializer is present. Likewise for the comma. Thanks, Paolo. /// Index: cp/parser.c === --- cp/parser.c (revision 219042) +++ cp/parser.c (working copy) @@ -2124,7 +2124,8 @@ static tree cp_parser_decltype /* Declarators [gram.dcl.decl] */ static tree cp_parser_init_declarator - (cp_parser *, cp_decl_specifier_seq *, vecdeferred_access_check, va_gc *, bool, bool, int, bool *, tree *); + (cp_parser *, cp_decl_specifier_seq *, vecdeferred_access_check, va_gc *, + bool, bool, int, bool *, tree *, location_t *); static cp_declarator *cp_parser_declarator (cp_parser *, cp_parser_declarator_kind, int *, bool *, bool, bool); static cp_declarator *cp_parser_direct_declarator @@ -11454,6 +11455,8 @@ cp_parser_simple_declaration (cp_parser* parser, cp_decl_specifier_seq decl_specifiers; int declares_class_or_enum; bool saw_declarator; + location_t comma_loc = UNKNOWN_LOCATION; + location_t init_loc = UNKNOWN_LOCATION; if (maybe_range_for_decl) *maybe_range_for_decl = NULL_TREE; @@ -11528,12 +11531,16 @@ cp_parser_simple_declaration (cp_parser* parser, if (saw_declarator) { - /* If we are processing next declarator, coma is expected */ + /* If we are processing next declarator, comma is expected */ token = cp_lexer_peek_token (parser-lexer); gcc_assert (token-type == CPP_COMMA); cp_lexer_consume_token (parser-lexer); if (maybe_range_for_decl) - *maybe_range_for_decl = error_mark_node; + { + *maybe_range_for_decl = error_mark_node; + if (comma_loc == UNKNOWN_LOCATION) + comma_loc = token-location; + } } else saw_declarator = true; @@ -11545,7 +11552,8 @@ cp_parser_simple_declaration (cp_parser* parser, /*member_p=*/false, declares_class_or_enum, function_definition_p, - maybe_range_for_decl); + maybe_range_for_decl, + init_loc); /* If an error occurred while parsing tentatively, exit quickly. (That usually happens when in the body of a function; each statement is treated as a declaration-statement until proven @@ -11631,7 +11639,15 @@ cp_parser_simple_declaration (cp_parser* parser, /* Consume the `;'. */ if (!maybe_range_for_decl) - cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON); +cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON); + else if (cp_lexer_next_token_is (parser-lexer, CPP_COLON)) +{ + if (init_loc != UNKNOWN_LOCATION) + error_at (init_loc, initializer in range-based %for% loop); + if (comma_loc != UNKNOWN_LOCATION) + error_at (comma_loc, + multiple declarations in range-based %for% loop); +} done: pop_deferring_access_checks (); @@ -16842,8 +16858,13 @@ cp_parser_asm_definition (cp_parser* parser) parsed declaration if it is an uninitialized single declarator not followed by a `;', or to error_mark_node otherwise. Either way, the trailing `;', if present, will not be consumed. If returned, this declarator will be - created with SD_INITIALIZED but will not call cp_finish_decl. */ + created with SD_INITIALIZED but will not call cp_finish_decl. + If INIT_LOC is not NULL, and *INIT_LOC is equal to UNKNOWN_LOCATION, + and there is an initializer, the pointed location_t is set to the + location of the '=' (or `(', or '{' in C++11) token introducing the + initializer. */ + static tree cp_parser_init_declarator (cp_parser* parser, cp_decl_specifier_seq *decl_specifiers, @@ -16852,7 +16873,8 @@ cp_parser_init_declarator (cp_parser* parser, bool member_p, int declares_class_or_enum, bool* function_definition_p, - tree*
Re: [C++ Patch] PR 63985
OK, thanks. Jason
Re: [PATCH] Treat a sibling call as though it does a wild read
On 2014-12-23 12:32 PM, H.J. Lu wrote: On Tue, Dec 16, 2014 at 5:17 PM, John David Anglin dave.ang...@bell.net wrote: On 8-Dec-14, at 5:36 PM, Jeff Law wrote: On 12/08/14 15:15, John David Anglin wrote: On 12/8/2014 3:01 PM, Jeff Law wrote: The above is wrong for sibcalls. Sibcall arguments are relative to the incoming argument pointer. Is this always the frame pointer? I don't think it's always the frame pointer. Don't we use an argument pointer for the PA64 runtime? If I recall, it was the only port that had a non-eliminable argument pointer at the time. I don't think PA64 is an argument against this as sibcalls don't work in the PA64 runtime (they are disabled in pa.c) because the argument pointer isn't a fixed register. I guess in theory it could be fixed if it was saved and restored across calls. But there's nothing that says another port in the future won't have similar characteristics as the PA, so while the PA isn't particularly important, it shows there's cases where arguments won't be accessed by the FP. DSE as it stands doesn't look at argument pointer based stores and I suspect they would be deleted with current code. Agreed. I believe that this version addresses the above issues. While there may be some opportunity to optimize the handling of sibling call arguments, I think it is more important to get the overall logic correct. Also, it's obviously a rare situation for the arguments to be pushed to the stack. Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11. This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64388 I tend to think the dse behavior expected by the testcase is wrong and this only worked before because the arguments for the call to bar are passed in registers. Dave -- John David Anglin dave.ang...@bell.net
Re: [PATCH] Treat a sibling call as though it does a wild read
On Tue, Dec 23, 2014 at 2:24 PM, John David Anglin dave.ang...@bell.net wrote: On 2014-12-23 12:32 PM, H.J. Lu wrote: On Tue, Dec 16, 2014 at 5:17 PM, John David Anglin dave.ang...@bell.net wrote: On 8-Dec-14, at 5:36 PM, Jeff Law wrote: On 12/08/14 15:15, John David Anglin wrote: On 12/8/2014 3:01 PM, Jeff Law wrote: The above is wrong for sibcalls. Sibcall arguments are relative to the incoming argument pointer. Is this always the frame pointer? I don't think it's always the frame pointer. Don't we use an argument pointer for the PA64 runtime? If I recall, it was the only port that had a non-eliminable argument pointer at the time. I don't think PA64 is an argument against this as sibcalls don't work in the PA64 runtime (they are disabled in pa.c) because the argument pointer isn't a fixed register. I guess in theory it could be fixed if it was saved and restored across calls. But there's nothing that says another port in the future won't have similar characteristics as the PA, so while the PA isn't particularly important, it shows there's cases where arguments won't be accessed by the FP. DSE as it stands doesn't look at argument pointer based stores and I suspect they would be deleted with current code. Agreed. I believe that this version addresses the above issues. While there may be some opportunity to optimize the handling of sibling call arguments, I think it is more important to get the overall logic correct. Also, it's obviously a rare situation for the arguments to be pushed to the stack. Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11. This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64388 I tend to think the dse behavior expected by the testcase is wrong and this only worked before because the arguments for the call to bar are passed in registers. The testcase has /* { dg-do compile { target { { { { { { { { i?86-*-* x86_64-*-* } x32 } || lp 64 } { ! s390*-*-* } } { ! hppa*64*-*-* } } { ! alpha*-*-* } } { { ! powerpc*-*-linux* } || powerpc_elfv2 } { ! nvptx-*-* } } } } } */ In this case, arguments are passed in registers. Isn't the optimization disabled for ia32, which passes arguments on stack, even before your change? -- H.J.
[patch] libstdc++/64389 fix another darwin bootstrap failure
Rather than the patch linked to in the PR comments this does it properly, by compiling locale-inst.cc as C++11 (so the new time_get members are instantiated). Tested powerpc64-linux, committed to trunk. commit 41ecb70e823f723f8c24c2ad227a5d0d567f5af1 Author: Jonathan Wakely jwak...@redhat.com Date: Tue Dec 23 12:26:52 2014 + Compile locale-inst.cc and wlocale-inst.cc as C++11. PR libstdc++/64389 * src/c++11/Makefile.am: Add locale-inst.cc and wlocale-inst.cc. * src/c++11/Makefile.in: Regenerate. * src/c++11/locale-inst.cc: Move from src/c++98/. * src/c++11/wlocale-inst.cc: Likewise. * src/c++11/cxx11-locale-inst.cc: Adjust path to locale-inst.cc. * src/c++11/string-inst.cc: Remove time_get instantiations. * src/c++98/Makefile.am: Remove locale-inst.cc and wlocale-inst.cc. * src/c++98/Makefile.in: Regenerate. * src/c++98/locale-inst.cc: Move to src/c++11/. * src/c++98/wlocale-inst.cc: Likewise. diff --git a/libstdc++-v3/src/c++11/Makefile.am b/libstdc++-v3/src/c++11/Makefile.am index c4345af..829159c 100644 --- a/libstdc++-v3/src/c++11/Makefile.am +++ b/libstdc++-v3/src/c++11/Makefile.am @@ -96,10 +96,12 @@ inst_sources = \ ios-inst.cc \ iostream-inst.cc \ istream-inst.cc \ + locale-inst.cc \ ostream-inst.cc \ sstream-inst.cc \ streambuf-inst.cc \ string-inst.cc \ + wlocale-inst.cc \ wstring-inst.cc else # XTEMPLATE_FLAGS = diff --git a/libstdc++-v3/src/c++11/Makefile.in b/libstdc++-v3/src/c++11/Makefile.in index 2ce23f9..619bf37 100644 --- a/libstdc++-v3/src/c++11/Makefile.in +++ b/libstdc++-v3/src/c++11/Makefile.in @@ -85,9 +85,10 @@ am__objects_3 = chrono.lo condition_variable.lo cow-stdexcept.lo \ @ENABLE_EXTERN_TEMPLATE_TRUE@am__objects_5 = $(am__objects_4) \ @ENABLE_EXTERN_TEMPLATE_TRUE@ ext11-inst.lo fstream-inst.lo \ @ENABLE_EXTERN_TEMPLATE_TRUE@ ios-inst.lo iostream-inst.lo \ -@ENABLE_EXTERN_TEMPLATE_TRUE@ istream-inst.lo ostream-inst.lo \ -@ENABLE_EXTERN_TEMPLATE_TRUE@ sstream-inst.lo streambuf-inst.lo \ -@ENABLE_EXTERN_TEMPLATE_TRUE@ string-inst.lo wstring-inst.lo +@ENABLE_EXTERN_TEMPLATE_TRUE@ istream-inst.lo locale-inst.lo \ +@ENABLE_EXTERN_TEMPLATE_TRUE@ ostream-inst.lo sstream-inst.lo \ +@ENABLE_EXTERN_TEMPLATE_TRUE@ streambuf-inst.lo string-inst.lo \ +@ENABLE_EXTERN_TEMPLATE_TRUE@ wlocale-inst.lo wstring-inst.lo am_libc__11convenience_la_OBJECTS = $(am__objects_3) $(am__objects_5) libc__11convenience_la_OBJECTS = $(am_libc__11convenience_la_OBJECTS) DEFAULT_INCLUDES = -I.@am__isrc@ -I$(top_builddir) @@ -385,10 +386,12 @@ sources = \ @ENABLE_EXTERN_TEMPLATE_TRUE@ ios-inst.cc \ @ENABLE_EXTERN_TEMPLATE_TRUE@ iostream-inst.cc \ @ENABLE_EXTERN_TEMPLATE_TRUE@ istream-inst.cc \ +@ENABLE_EXTERN_TEMPLATE_TRUE@ locale-inst.cc \ @ENABLE_EXTERN_TEMPLATE_TRUE@ ostream-inst.cc \ @ENABLE_EXTERN_TEMPLATE_TRUE@ sstream-inst.cc \ @ENABLE_EXTERN_TEMPLATE_TRUE@ streambuf-inst.cc \ @ENABLE_EXTERN_TEMPLATE_TRUE@ string-inst.cc \ +@ENABLE_EXTERN_TEMPLATE_TRUE@ wlocale-inst.cc \ @ENABLE_EXTERN_TEMPLATE_TRUE@ wstring-inst.cc libc__11convenience_la_SOURCES = $(sources) $(inst_sources) diff --git a/libstdc++-v3/src/c++11/cxx11-locale-inst.cc b/libstdc++-v3/src/c++11/cxx11-locale-inst.cc index 9c1a1c1..93144e8 100644 --- a/libstdc++-v3/src/c++11/cxx11-locale-inst.cc +++ b/libstdc++-v3/src/c++11/cxx11-locale-inst.cc @@ -36,4 +36,4 @@ # define C char # define C_is_char #endif -# include ../c++98/locale-inst.cc +# include locale-inst.cc diff --git a/libstdc++-v3/src/c++11/locale-inst.cc b/libstdc++-v3/src/c++11/locale-inst.cc new file mode 100644 index 000..6cd3616 --- /dev/null +++ b/libstdc++-v3/src/c++11/locale-inst.cc @@ -0,0 +1,413 @@ +// Locale support -*- C++ -*- + +// Copyright (C) 1999-2014 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// Under Section 7 of GPL version 3, you are granted additional +// permissions described in the GCC Runtime Library Exception, version +// 3.1, as published by the Free Software Foundation. + +// You should have received a copy of the GNU General Public License and +// a copy of the GCC Runtime Library Exception along with this program; +// see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +// http://www.gnu.org/licenses/. + +// +// ISO C++ 14882: 22.1 Locales +// + +#ifndef _GLIBCXX_USE_CXX11_ABI +// Instantiations in this file use the old COW std::string ABI unless included +// by another file which defines
Re: [PATCH] Treat a sibling call as though it does a wild read
On 23-Dec-14, at 5:37 PM, H.J. Lu wrote: In this case, arguments are passed in registers. Isn't the optimization disabled for ia32, which passes arguments on stack, even before your change? It's not disabled in dse.c. Possibly, this occurs for some cases for ia32 in ix86_function_ok_for_sibcall. The problem in dse is in how to distinguish stores used for arguments from those used for general manipulation of data. It seems the argument data for the call to bar in the testcase are copied through frame memory on x86_64. We have two situations: /* This field is only used for the processing of const functions. These functions cannot read memory, but they can read the stack because that is where they may get their parms. We need to be this conservative because, like the store motion pass, we don't consider CALL_INSN_FUNCTION_USAGE when processing call insns. Moreover, we need to distinguish two cases: 1. Before reload (register elimination), the stores related to outgoing arguments are stack pointer based and thus deemed of non-constant base in this pass. This requires special handling but also means that the frame pointer based stores need not be killed upon encountering a const function call. 2. After reload, the stores related to outgoing arguments can be either stack pointer or hard frame pointer based. This means that we have no other choice than also killing all the frame pointer based stores upon encountering a const function call. This field is set after reload for const function calls. Having this set is less severe than a wild read, it just means that all the frame related stores are killed rather than all the stores. */ bool frame_read; Case 1 is incorrect for sibling calls as the call arguments are frame or argument pointer based when they are not passed in registers. When we don't have a const function, dse assumes: /* Every other call, including pure functions, may read any memory that is not relative to the frame. */ add_non_frame_wild_read (bb_info); Again, this is incorrect for sibling calls (i.e., dse in general assumes that call arguments are register or stack pointer based before reload). Dave -- John David Anglin dave.ang...@bell.net
Re: [PATCH] Treat a sibling call as though it does a wild read
On Tue, Dec 23, 2014 at 3:55 PM, John David Anglin dave.ang...@bell.net wrote: On 23-Dec-14, at 5:37 PM, H.J. Lu wrote: In this case, arguments are passed in registers. Isn't the optimization disabled for ia32, which passes arguments on stack, even before your change? It's not disabled in dse.c. Possibly, this occurs for some cases for ia32 in ix86_function_ok_for_sibcall. The problem in dse is in how to distinguish stores used for arguments from those used for general manipulation of data. It seems the argument data for the call to bar in the testcase are copied through frame memory on x86_64. We have two situations: /* This field is only used for the processing of const functions. These functions cannot read memory, but they can read the stack because that is where they may get their parms. We need to be this conservative because, like the store motion pass, we don't consider CALL_INSN_FUNCTION_USAGE when processing call insns. Moreover, we need to distinguish two cases: 1. Before reload (register elimination), the stores related to outgoing arguments are stack pointer based and thus deemed of non-constant base in this pass. This requires special handling but also means that the frame pointer based stores need not be killed upon encountering a const function call. 2. After reload, the stores related to outgoing arguments can be either stack pointer or hard frame pointer based. This means that we have no other choice than also killing all the frame pointer based stores upon encountering a const function call. This field is set after reload for const function calls. Having this set is less severe than a wild read, it just means that all the frame related stores are killed rather than all the stores. */ bool frame_read; Case 1 is incorrect for sibling calls as the call arguments are frame or argument pointer based when they are not passed in registers. When we don't have a const function, dse assumes: /* Every other call, including pure functions, may read any memory that is not relative to the frame. */ add_non_frame_wild_read (bb_info); Again, this is incorrect for sibling calls (i.e., dse in general assumes that call arguments are register or stack pointer based before reload). This optimization is done on purpose to fix: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44194 Unless it is also incorrect for x86, why not only disable it for your target without penalizing x86 through a target hook. Thanks. -- H.J.
[PATCH] Update config.sub from upstream config repo
ChangeLog 2014-12-23 James Bowman james.bow...@ftdichip.com * config.sub: Update from upstream config repo. -- James Bowman FTDI Open Source Liaison Index: config.sub === --- config.sub (revision 219020) +++ config.sub (working copy) @@ -2,7 +2,7 @@ # Configuration validation subroutine script. # Copyright 1992-2014 Free Software Foundation, Inc. -timestamp='2014-09-26' +timestamp='2014-12-21' # This file is free software; you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by @@ -260,7 +260,7 @@ | c4x | c8051 | clipper \ | d10v | d30v | dlx | dsp16xx \ | epiphany \ - | fido | fr30 | frv \ + | fido | fr30 | frv | ft32 \ | h8300 | h8500 | hppa | hppa1.[01] | hppa2.0 | hppa2.0[nw] | hppa64 \ | hexagon \ | i370 | i860 | i960 | ia64 \ @@ -313,6 +313,7 @@ | tahoe | tic4x | tic54x | tic55x | tic6x | tic80 | tron \ | ubicom32 \ | v850 | v850e | v850e1 | v850e2 | v850es | v850e2v3 \ + | visium \ | we32k \ | x86 | xc16x | xstormy16 | xtensa \ | z8k | z80) @@ -440,6 +441,7 @@ | ubicom32-* \ | v850-* | v850e-* | v850e1-* | v850es-* | v850e2-* | v850e2v3-* \ | vax-* \ + | visium-* \ | we32k-* \ | x86-* | x86_64-* | xc16x-* | xps100-* \ | xstormy16-* | xtensa*-* \
[C++11] DR1479 - Literal operators and default arguments
I tried this patch a year ago and, as pointed out by NightStrike, got lost in the shuffle. The idea is that default arguments in literal operators would lead to ambiguities and might not act like the uthor intends. The earlier patch errors out. This time I just warn. I'm not sure what we want here actually so some feedback would be nice. I could alternatively error and add a note about default argument. Builds and tests clean on x86_64-linux. OK? Ed
[C++11] DR1479 - Literal operators and default arguments
And now with a patch... I tried this patch a year ago and, as pointed out by NightStrike, got lost in the shuffle. The idea is that default arguments in literal operators would lead to ambiguities and might not act like the uthor intends. The earlier patch errors out. This time I just warn. I'm not sure what we want here actually so some feedback would be nice. I could alternatively error and add a note about default argument. Builds and tests clean on x86_64-linux. OK? Ed cp: 2014-12-23 Edward Smith-Rowland 3dw...@verizon.net DR1479: Literal operators and default arguments * cp-tree.h (check_literal_operator_args): Add new variable. * decl.c (grokfndecl): Adjust call. Report extra error. * typeck.c (check_literal_operator_args): Add and set new return argument indicating defaulted operator argument. of character and build parm pack with correct type and chars. testsuite: 2014-12-23 Edward Smith-Rowland 3dw...@verizon.net DR1479: Literal operators and default arguments * testsuite/g++.dg/cpp0x/udlit-default-arg-neg.C: New. Index: cp/cp-tree.h === --- cp/cp-tree.h(revision 218988) +++ cp/cp-tree.h(working copy) @@ -6286,7 +6286,7 @@ tsubst_flags_t); extern void check_template_keyword (tree); extern bool check_raw_literal_operator (const_tree decl); -extern bool check_literal_operator_args(const_tree, bool *, bool *); +extern bool check_literal_operator_args(const_tree, bool *, bool *, bool *); extern void maybe_warn_about_useless_cast (tree, tree, tsubst_flags_t); extern tree cp_perform_integral_promotions (tree, tsubst_flags_t); Index: cp/decl.c === --- cp/decl.c (revision 218988) +++ cp/decl.c (working copy) @@ -7849,6 +7849,7 @@ return NULL_TREE; else if (UDLIT_OPER_P (DECL_NAME (decl))) { + bool default_arg_p; bool long_long_unsigned_p; bool long_double_p; const char *suffix = NULL; @@ -7861,12 +7862,15 @@ if (DECL_NAMESPACE_SCOPE_P (decl)) { - if (!check_literal_operator_args (decl, long_long_unsigned_p, + if (!check_literal_operator_args (decl, default_arg_p, + long_long_unsigned_p, long_double_p)) { error (%qD has invalid argument list, decl); return NULL_TREE; } + if (default_arg_p) + warning (0, %qD has default argument(s), decl); suffix = UDLIT_OP_SUFFIX (DECL_NAME (decl)); if (long_long_unsigned_p) Index: cp/typeck.c === --- cp/typeck.c (revision 218988) +++ cp/typeck.c (working copy) @@ -9204,15 +9204,21 @@ argument types. */ bool -check_literal_operator_args (const_tree decl, +check_literal_operator_args (const_tree decl, bool *default_arg_p, bool *long_long_unsigned_p, bool *long_double_p) { tree argtypes = TYPE_ARG_TYPES (TREE_TYPE (decl)); + *default_arg_p = false; *long_long_unsigned_p = false; *long_double_p = false; if (processing_template_decl || processing_specialization) -return argtypes == void_list_node; +{ + if (argtypes argtypes != void_list_node TREE_PURPOSE (argtypes)) + *default_arg_p = true; + + return argtypes == void_list_node; +} else { tree argtype; @@ -9224,6 +9230,9 @@ argtype argtype != void_list_node; argtype = TREE_CHAIN (argtype)) { + if (TREE_PURPOSE (argtype)) + *default_arg_p = true; + tree t = TREE_VALUE (argtype); ++arity; @@ -9242,6 +9251,8 @@ argtype = TREE_CHAIN (argtype); if (!argtype) return false; + if (TREE_PURPOSE (argtype)) + *default_arg_p = true; t = TREE_VALUE (argtype); if (maybe_raw_p argtype == void_list_node) return true; @@ -9278,6 +9289,9 @@ if (!argtype) return false; /* Found ellipsis. */ + if (*default_arg_p) + return false; + if (arity != max_arity) return false; Index: testsuite/g++.dg/cpp0x/udlit-default-arg-neg.C === --- testsuite/g++.dg/cpp0x/udlit-default-arg-neg.C (revision 0) +++ testsuite/g++.dg/cpp0x/udlit-default-arg-neg.C (working copy) @@ -0,0 +1,22 @@ +// { dg-options -std=c++11 } + +#include cstddef + +int +operator_a(const char *, std::size_t = 0) // { dg-warning has invalid argument list } +{ + return 1; +} + +int +operator_a(const
Re: [PATCH] Treat a sibling call as though it does a wild read
On 23-Dec-14, at 7:28 PM, H.J. Lu wrote: On Tue, Dec 23, 2014 at 3:55 PM, John David Anglin dave.ang...@bell.net wrote: On 23-Dec-14, at 5:37 PM, H.J. Lu wrote: In this case, arguments are passed in registers. Isn't the optimization disabled for ia32, which passes arguments on stack, even before your change? It's not disabled in dse.c. Possibly, this occurs for some cases for ia32 in ix86_function_ok_for_sibcall. The problem in dse is in how to distinguish stores used for arguments from those used for general manipulation of data. It seems the argument data for the call to bar in the testcase are copied through frame memory on x86_64. We have two situations: /* This field is only used for the processing of const functions. These functions cannot read memory, but they can read the stack because that is where they may get their parms. We need to be this conservative because, like the store motion pass, we don't consider CALL_INSN_FUNCTION_USAGE when processing call insns. Moreover, we need to distinguish two cases: 1. Before reload (register elimination), the stores related to outgoing arguments are stack pointer based and thus deemed of non-constant base in this pass. This requires special handling but also means that the frame pointer based stores need not be killed upon encountering a const function call. 2. After reload, the stores related to outgoing arguments can be either stack pointer or hard frame pointer based. This means that we have no other choice than also killing all the frame pointer based stores upon encountering a const function call. This field is set after reload for const function calls. Having this set is less severe than a wild read, it just means that all the frame related stores are killed rather than all the stores. */ bool frame_read; Case 1 is incorrect for sibling calls as the call arguments are frame or argument pointer based when they are not passed in registers. When we don't have a const function, dse assumes: /* Every other call, including pure functions, may read any memory that is not relative to the frame. */ add_non_frame_wild_read (bb_info); Again, this is incorrect for sibling calls (i.e., dse in general assumes that call arguments are register or stack pointer based before reload). This optimization is done on purpose to fix: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44194 Unless it is also incorrect for x86, why not only disable it for your target without penalizing x86 through a target hook. I tend to think the old code was wrong for x86 but I recognize the lost optimization. This isn't an optimization issue on hppa. It's a wrong code bug. Richard wrote regarding 44194: So we create a stack representation to copy it to the stack temporary (which both are unneeded). We should see that we can avoid the temporary at all as there is no aggregate use of the struct left. At least we should avoid the 2nd temporary. The temporaries are still there and they are the problem. I don't see that this is a target issue. There were nothing more than argument stores in the testcase for the PR 55023 and they were deleted by dse. Dave -- John David Anglin dave.ang...@bell.net
Re: [PATCH] Treat a sibling call as though it does a wild read
On Tue, Dec 23, 2014 at 6:16 PM, John David Anglin dave.ang...@bell.net wrote: On 23-Dec-14, at 7:28 PM, H.J. Lu wrote: On Tue, Dec 23, 2014 at 3:55 PM, John David Anglin dave.ang...@bell.net wrote: On 23-Dec-14, at 5:37 PM, H.J. Lu wrote: In this case, arguments are passed in registers. Isn't the optimization disabled for ia32, which passes arguments on stack, even before your change? It's not disabled in dse.c. Possibly, this occurs for some cases for ia32 in ix86_function_ok_for_sibcall. The problem in dse is in how to distinguish stores used for arguments from those used for general manipulation of data. It seems the argument data for the call to bar in the testcase are copied through frame memory on x86_64. We have two situations: /* This field is only used for the processing of const functions. These functions cannot read memory, but they can read the stack because that is where they may get their parms. We need to be this conservative because, like the store motion pass, we don't consider CALL_INSN_FUNCTION_USAGE when processing call insns. Moreover, we need to distinguish two cases: 1. Before reload (register elimination), the stores related to outgoing arguments are stack pointer based and thus deemed of non-constant base in this pass. This requires special handling but also means that the frame pointer based stores need not be killed upon encountering a const function call. 2. After reload, the stores related to outgoing arguments can be either stack pointer or hard frame pointer based. This means that we have no other choice than also killing all the frame pointer based stores upon encountering a const function call. This field is set after reload for const function calls. Having this set is less severe than a wild read, it just means that all the frame related stores are killed rather than all the stores. */ bool frame_read; Case 1 is incorrect for sibling calls as the call arguments are frame or argument pointer based when they are not passed in registers. When we don't have a const function, dse assumes: /* Every other call, including pure functions, may read any memory that is not relative to the frame. */ add_non_frame_wild_read (bb_info); Again, this is incorrect for sibling calls (i.e., dse in general assumes that call arguments are register or stack pointer based before reload). This optimization is done on purpose to fix: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44194 Unless it is also incorrect for x86, why not only disable it for your target without penalizing x86 through a target hook. I tend to think the old code was wrong for x86 but I recognize the lost optimization. This isn't an optimization issue on hppa. It's a wrong code bug. I run gcc.dg/pr55023.c on x86-64 and x86 without your fix. It works fine. Can you find a testcase, failing on x86-64 and x86, which is fixed by your change? If not, this bug doesn't affect x86 and you should find another way to fix your target problem without introducing a regression on x86. Richard wrote regarding 44194: So we create a stack representation to copy it to the stack temporary (which both are unneeded). We should see that we can avoid the temporary at all as there is no aggregate use of the struct left. At least we should avoid the 2nd temporary. The temporaries are still there and they are the problem. I don't see that this is a target issue. There were nothing more than argument stores in the testcase for the PR 55023 and they were deleted by dse. Thanks. -- H.J.
Re: [PATCH] Treat a sibling call as though it does a wild read
On 12/23/14 19:47, H.J. Lu wrote: On Tue, Dec 23, 2014 at 6:16 PM, John David Anglin dave.ang...@bell.net wrote: On 23-Dec-14, at 7:28 PM, H.J. Lu wrote: On Tue, Dec 23, 2014 at 3:55 PM, John David Anglin dave.ang...@bell.net wrote: On 23-Dec-14, at 5:37 PM, H.J. Lu wrote: In this case, arguments are passed in registers. Isn't the optimization disabled for ia32, which passes arguments on stack, even before your change? It's not disabled in dse.c. Possibly, this occurs for some cases for ia32 in ix86_function_ok_for_sibcall. The problem in dse is in how to distinguish stores used for arguments from those used for general manipulation of data. It seems the argument data for the call to bar in the testcase are copied through frame memory on x86_64. We have two situations: /* This field is only used for the processing of const functions. These functions cannot read memory, but they can read the stack because that is where they may get their parms. We need to be this conservative because, like the store motion pass, we don't consider CALL_INSN_FUNCTION_USAGE when processing call insns. Moreover, we need to distinguish two cases: 1. Before reload (register elimination), the stores related to outgoing arguments are stack pointer based and thus deemed of non-constant base in this pass. This requires special handling but also means that the frame pointer based stores need not be killed upon encountering a const function call. 2. After reload, the stores related to outgoing arguments can be either stack pointer or hard frame pointer based. This means that we have no other choice than also killing all the frame pointer based stores upon encountering a const function call. This field is set after reload for const function calls. Having this set is less severe than a wild read, it just means that all the frame related stores are killed rather than all the stores. */ bool frame_read; Case 1 is incorrect for sibling calls as the call arguments are frame or argument pointer based when they are not passed in registers. When we don't have a const function, dse assumes: /* Every other call, including pure functions, may read any memory that is not relative to the frame. */ add_non_frame_wild_read (bb_info); Again, this is incorrect for sibling calls (i.e., dse in general assumes that call arguments are register or stack pointer based before reload). This optimization is done on purpose to fix: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44194 Unless it is also incorrect for x86, why not only disable it for your target without penalizing x86 through a target hook. I tend to think the old code was wrong for x86 but I recognize the lost optimization. This isn't an optimization issue on hppa. It's a wrong code bug. I run gcc.dg/pr55023.c on x86-64 and x86 without your fix. It works fine. Can you find a testcase, failing on x86-64 and x86, which is fixed by your change? If not, this bug doesn't affect x86 and you should find another way to fix your target problem without introducing a regression on x86. Just to clarify, this is not a target problem, this is a problem that DSE is not designed to handle certain situations WRT argument stores that occur on targets with certain properties. Jeff
Re: [PATCH] Treat a sibling call as though it does a wild read
On Tue, Dec 23, 2014 at 6:59 PM, Jeff Law l...@redhat.com wrote: On 12/23/14 19:47, H.J. Lu wrote: On Tue, Dec 23, 2014 at 6:16 PM, John David Anglin dave.ang...@bell.net wrote: On 23-Dec-14, at 7:28 PM, H.J. Lu wrote: On Tue, Dec 23, 2014 at 3:55 PM, John David Anglin dave.ang...@bell.net wrote: On 23-Dec-14, at 5:37 PM, H.J. Lu wrote: In this case, arguments are passed in registers. Isn't the optimization disabled for ia32, which passes arguments on stack, even before your change? It's not disabled in dse.c. Possibly, this occurs for some cases for ia32 in ix86_function_ok_for_sibcall. The problem in dse is in how to distinguish stores used for arguments from those used for general manipulation of data. It seems the argument data for the call to bar in the testcase are copied through frame memory on x86_64. We have two situations: /* This field is only used for the processing of const functions. These functions cannot read memory, but they can read the stack because that is where they may get their parms. We need to be this conservative because, like the store motion pass, we don't consider CALL_INSN_FUNCTION_USAGE when processing call insns. Moreover, we need to distinguish two cases: 1. Before reload (register elimination), the stores related to outgoing arguments are stack pointer based and thus deemed of non-constant base in this pass. This requires special handling but also means that the frame pointer based stores need not be killed upon encountering a const function call. 2. After reload, the stores related to outgoing arguments can be either stack pointer or hard frame pointer based. This means that we have no other choice than also killing all the frame pointer based stores upon encountering a const function call. This field is set after reload for const function calls. Having this set is less severe than a wild read, it just means that all the frame related stores are killed rather than all the stores. */ bool frame_read; Case 1 is incorrect for sibling calls as the call arguments are frame or argument pointer based when they are not passed in registers. When we don't have a const function, dse assumes: /* Every other call, including pure functions, may read any memory that is not relative to the frame. */ add_non_frame_wild_read (bb_info); Again, this is incorrect for sibling calls (i.e., dse in general assumes that call arguments are register or stack pointer based before reload). This optimization is done on purpose to fix: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44194 Unless it is also incorrect for x86, why not only disable it for your target without penalizing x86 through a target hook. I tend to think the old code was wrong for x86 but I recognize the lost optimization. This isn't an optimization issue on hppa. It's a wrong code bug. I run gcc.dg/pr55023.c on x86-64 and x86 without your fix. It works fine. Can you find a testcase, failing on x86-64 and x86, which is fixed by your change? If not, this bug doesn't affect x86 and you should find another way to fix your target problem without introducing a regression on x86. Just to clarify, this is not a target problem, this is a problem that DSE is not designed to handle certain situations WRT argument stores that occur on targets with certain properties. Why not limit the change to targets with those certain properties? -- H.J.
[PATCH] pr31397 - implement -Wsuggest-override
From: Trevor Saunders tbsaunde+...@tbsaunde.org Hi, comments fixed. bootstrapped on x86_64-linux, new test passes and regtest pending, ok? Trev c-family/ * c.opt (Wsuggest-override): New option. cp/ * class.c (check_for_override): Warn when a virtual function is an override not marked override. gcc/ * doc/invoke.texi: Document -Wsuggest-override. --- gcc/c-family/c.opt| 5 + gcc/cp/class.c| 4 gcc/doc/invoke.texi | 6 +- gcc/testsuite/g++.dg/warn/Wsuggest-override.C | 23 +++ 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wsuggest-override.C diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index f86718b..1f6f793 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -578,6 +578,11 @@ Wsuggest-attribute=format C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning Warn about functions which might be candidates for format attributes +Wsuggest-override +C++ ObjC++ Var(warn_override) Warning +Suggest that the override keyword be used when the declaration of a virtual +function overrides another. + Wswitch C ObjC C++ ObjC++ Var(warn_switch) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) Warn about enumerated switches, with no default, missing a case diff --git a/gcc/cp/class.c b/gcc/cp/class.c index 0ac2124..d2cf4a0 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -2811,6 +2811,10 @@ check_for_override (tree decl, tree ctype) { DECL_VINDEX (decl) = decl; overrides_found = true; + if (warn_override !DECL_OVERRIDE_P (decl) + !DECL_DESTRUCTOR_P (decl)) + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wsuggest_override, + %q+D can be marked override, decl); } if (DECL_VIRTUAL_P (decl)) diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 9f56f42..d0ee093 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -277,7 +277,7 @@ Objective-C and Objective-C++ Dialects}. -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol -Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol --Wsuggest-final-types @gol -Wsuggest-final-methods @gol +-Wsuggest-final-types @gol -Wsuggest-final-methods @gol -Wsuggest-override @gol -Wmissing-format-attribute @gol -Wswitch -Wswitch-default -Wswitch-enum -Wswitch-bool -Wsync-nand @gol -Wsystem-headers -Wtrampolines -Wtrigraphs -Wtype-limits -Wundef @gol @@ -4276,6 +4276,10 @@ class hierarchy graph is more complete. It is recommended to first consider suggestions of @option{-Wsuggest-final-types} and then rebuild with new annotations. +@item -Wsuggest-override +Warn about overriding virtual functions that are not marked with the override +keyword. + @item -Warray-bounds @opindex Wno-array-bounds @opindex Warray-bounds diff --git a/gcc/testsuite/g++.dg/warn/Wsuggest-override.C b/gcc/testsuite/g++.dg/warn/Wsuggest-override.C new file mode 100644 index 000..f820f4b --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wsuggest-override.C @@ -0,0 +1,23 @@ +// { dg-do compile } +// { dg-options -std=c++11 -Wsuggest-override } +struct A +{ + A(); + virtual ~A(); + virtual void f(); + virtual int bar(); + int c(); + operator int(); + virtual operator float(); +}; + +struct B : A +{ + B(); + virtual ~B(); + virtual void f(); // { dg-warning can be marked override } +virtual int bar() override; +int c(); +operator int(); +virtual operator float(); // { dg-warning can be marked override } +}; -- 2.1.3