C++ PATCH for c++/65974 (deprecated virtual)
When a vtable uses a deprecated virtual function, that's outside the user's control and so shouldn't be warned about. Tested x86_64-pc-linux-gnu, applying to trunk. commit 3e78f42251b5a82bc3aa2c00b85e40abb54fc70f Author: Jason Merrill Date: Fri Aug 14 17:41:24 2015 +0100 PR c++/65974 * decl2.c (mark_vtable_entries): Suppress -Wdeprecated. diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index f5d1e52..b0368ae 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -1742,6 +1742,9 @@ mark_vtable_entries (tree decl) tree fnaddr; unsigned HOST_WIDE_INT idx; + /* It's OK for the vtable to refer to deprecated virtual functions. */ + warning_sentinel w(warn_deprecated_decl); + FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (DECL_INITIAL (decl)), idx, fnaddr) { diff --git a/gcc/testsuite/g++.dg/warn/deprecated-9.C b/gcc/testsuite/g++.dg/warn/deprecated-9.C new file mode 100644 index 000..fc861ee --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/deprecated-9.C @@ -0,0 +1,16 @@ +// PR c++/65974 +// { dg-options "-Wdeprecated" } + +struct S { +void bar(); + +__attribute__((deprecated("use bar() instead."))) +virtual void foo(); +}; + +void S::foo() { bar(); } + +int main() +{ +return 0; +}
Re: [PR64164] drop copyrename, integrate into expand
FAIL: gcc.target/aarch64/target_attr_crypto_ice_1.c (internal compiler error) In file included from /opt/gcc/gcc-20150815/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_1.c:4:0: /opt/gcc/gcc-20150815/Build/gcc/include/arm_neon.h: In function 'test_vsha1cq_u32': /opt/gcc/gcc-20150815/Build/gcc/include/arm_neon.h:21076:10: internal compiler error: in expand_expr_real_1, at expr.c:9532 0x7f060b expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) ../../gcc/expr.c:9532 0xdb1027 expand_normal ../../gcc/expr.h:261 0xdb1027 aarch64_simd_expand_args ../../gcc/config/aarch64/aarch64-builtins.c:944 0xdb1027 aarch64_simd_expand_builtin(int, tree_node*, rtx_def*) ../../gcc/config/aarch64/aarch64-builtins.c:1118 0x6cc667 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int) ../../gcc/builtins.c:5931 0x7ecab7 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) ../../gcc/expr.c:10360 0x7f8547 store_expr_with_bounds(tree_node*, rtx_def*, int, bool, tree_node*) ../../gcc/expr.c:5398 0x7fa9d3 expand_assignment(tree_node*, tree_node*, bool) ../../gcc/expr.c:5170 0x6f435f expand_call_stmt ../../gcc/cfgexpand.c:2621 0x6f435f expand_gimple_stmt_1 ../../gcc/cfgexpand.c:3510 0x6f435f expand_gimple_stmt ../../gcc/cfgexpand.c:3671 0x6f69c7 expand_gimple_tailcall ../../gcc/cfgexpand.c:3718 0x6f69c7 expand_gimple_basic_block ../../gcc/cfgexpand.c:5651 0x6fc777 execute ../../gcc/cfgexpand.c:6260 Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [patch] Remove useless variables, class Gt, and struct CompLast
On Sat, Aug 15, 2015 at 2:47 AM, Kai Zhao wrote: > Hi, > > There are some useless variables, class Gt, and struct CompLast in > > testsuite/25_algorithms/* > > This patch is to remove those useless variables, class Gt and struct > CompLast. > > The patch is attached. > > commit d13ea592473ccbd29276908782877156e669b28a > Author: Kai Zhao > Date: Sat Aug 15 02:23:14 2015 +0800 > > 2015-08-15 Kai Zhao > > testsuite/25_algorithms/nth_element/3.cc: Remove useless variables, > class Gt, and struct CompLast. > > testsuite/25_algorithms/partial_sort/2.cc: Likewise. > testsuite/25_algorithms/partial_sort_copy/2.cc: Likewise. > testsuite/25_algorithms/sort/1.cc: Likewise. > testsuite/25_algorithms/stable_sort/2.cc: Likewise. > The previous format is wrong, I should add "*" before the changed files. There are some useless variables, class Gt, and struct CompLast in testsuite/25_algorithms/* This patch is to remove those useless variables, class Gt and struct CompLast. The patch is attached. commit d13ea592473ccbd29276908782877156e669b28a Author: Kai Zhao Date: Sat Aug 15 02:23:14 2015 +0800 2015-08-15 Kai Zhao * testsuite/25_algorithms/nth_element/3.cc: Remove useless variables, class Gt, and struct CompLast. * testsuite/25_algorithms/partial_sort/2.cc: Likewise. * testsuite/25_algorithms/partial_sort_copy/2.cc: Likewise. * testsuite/25_algorithms/sort/1.cc: Likewise. * testsuite/25_algorithms/stable_sort/2.cc: Likewise. Thanks, Kai commit d13ea592473ccbd29276908782877156e669b28a Author: Kai Zhao Date: Sat Aug 15 02:23:14 2015 +0800 2015-08-15 Kai Zhao * testsuite/25_algorithms/nth_element/3.cc: Remove useless variables, class Gt, and struct CompLast. * testsuite/25_algorithms/partial_sort/2.cc: Likewise. * testsuite/25_algorithms/partial_sort_copy/2.cc: Likewise. * testsuite/25_algorithms/sort/1.cc: Likewise. * testsuite/25_algorithms/stable_sort/2.cc: Likewise. diff --git a/libstdc++-v3/testsuite/25_algorithms/nth_element/3.cc b/libstdc++-v3/testsuite/25_algorithms/nth_element/3.cc index 29a0450..4f9bc8a 100644 --- a/libstdc++-v3/testsuite/25_algorithms/nth_element/3.cc +++ b/libstdc++-v3/testsuite/25_algorithms/nth_element/3.cc @@ -26,8 +26,6 @@ const int A[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, const int B[] = {10, 20, 1, 11, 2, 12, 3, 13, 4, 14, 5, 15, 6, 16, 7, 17, 8, 18, 9, 19}; const int C[] = {20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; const int N = sizeof(A) / sizeof(int); -const int logN = 3; // ln(N) rounded up -const int P = 7; // comparison predicate for stable_sort: order by rightmost digit struct CompLast @@ -37,28 +35,6 @@ struct CompLast { return x % 10 < y % 10; } }; -// This functor has the equivalent functionality of std::geater<>, -// but there is no dependency on and it also tracks the -// number of invocations since creation. -class Gt -{ -public: - static int count() { return itsCount; } - static void reset() { itsCount = 0; } - - bool - operator()(const int& x, const int& y) - { -++itsCount; -return x > y; - } - -private: -static int itsCount; -}; - -int Gt::itsCount = 0; - // 25.3.2 nth_element() void test05() diff --git a/libstdc++-v3/testsuite/25_algorithms/partial_sort/2.cc b/libstdc++-v3/testsuite/25_algorithms/partial_sort/2.cc index 4ec511a..f68cca5 100644 --- a/libstdc++-v3/testsuite/25_algorithms/partial_sort/2.cc +++ b/libstdc++-v3/testsuite/25_algorithms/partial_sort/2.cc @@ -26,39 +26,19 @@ const int A[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, const int B[] = {10, 20, 1, 11, 2, 12, 3, 13, 4, 14, 5, 15, 6, 16, 7, 17, 8, 18, 9, 19}; const int C[] = {20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; const int N = sizeof(A) / sizeof(int); -const int logN = 3; // ln(N) rounded up const int P = 7; -// comparison predicate for stable_sort: order by rightmost digit -struct CompLast -{ - bool - operator()(const int x, const int y) - { return x % 10 < y % 10; } -}; - // This functor has the equivalent functionality of std::geater<>, // but there is no dependency on and it also tracks the // number of invocations since creation. class Gt { public: - static int count() { return itsCount; } - static void reset() { itsCount = 0; } - bool operator()(const int& x, const int& y) - { -++itsCount; -return x > y; - } - -private: -static int itsCount; + { return x > y; } }; -int Gt::itsCount = 0; - // 25.3.1.3 partial_sort() void test03() @@ -71,7 +51,6 @@ test03() VERIFY(std::equal(s1, s1 + P, A)); Gt gt; -gt.reset(); std::partial_sort(s1, s1 + P, s1 + N, gt); VERIFY(std::equal(s1, s1 + P, C)); } diff --git a/libstdc++-v3/testsuite/25_algorithms/partial_sort_copy/2.cc b/libstdc++-v3/testsuite/25_algorithms/partial_sort_copy/2.cc index 3659666..5943148 100644
Re: [PATCH][MIPS] Fix register renaming in the interrupt handlers
Robert Suchanek writes: >> > You also need to do the same thing for TARGET_HARD_REGNO_SCRATCH_OK, >> > to stop peephole2 from using unsaved registers as scratch registers. >> > >> > I should dig out my patches to clean up this interface. It's just >> > too brittle to have two macros that say what registers can be used >> > after reload. > > Indeed. I naively thought that there would be a single place that needed > an update and overlooked this hook. > >> Gosh, I don't like the interface of them at all. I have interrupt functions >> and I want all the goodness out of gcc and I want gcc to just know that it >> can't use registers it doesn't want to save for any reason that the port >> marked as saved by the calling api of the function. :-( Very few ports (3) >> define TARGET_HARD_REGNO_SCRATCH_OK and many port (19 or so) don't define >> this but do seem to have interrupt functions. The existing documentation >> seems not give be enough information to let me decide if I need to define it >> or not. I read through all the ports that do define it, and none of them >> shed any light on to allow me to decide if my port needs to or not. >> >> So, first question is, are the 16 other ports that have interrupt functions >> and don't define this just buggy (or non-optimal)? How can I tell if I need >> to or not? A single example of that does cause a port to see it and a single >> example of why a port would not need to define it would be instructive. The port is only buggy if they have define_peephole2s with match_scratches and those match_scratches require a register that would be saved by an interrupt function. In other cases defining T_H_R_S_O would have no effect (but still be a good idea for future proofing -- obviously someone who adds a new define_peephole2 is unlikely to be thinking about inerrupt functions). >> Along the lines of TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS, I'd be >> willing to explain the required abi of each function, and then I just want >> gcc to just work. All functions are normal, except for interrupt functions, >> all allocatable register need to be saved. I think this is a universally >> true thing, and it just happens to be true on my port as well. >> >> Anyway, I'd love to see gcc improved in this area. >> >> This one is a pet peeve of mine as I've seen what happens to software when a >> single register that should have been saved, wasn't. It was slightly >> annoying to track down and find. Fixing was trivial. > > The cleanup as Richard suggested would probably be a good start. It's > interesting that this kind of bug wasn't discovered earlier and likely > existing for several years. I think the underlying problem is that target-independent code knows which set of registers are call(ee)-saved under the ABI, but doesn't know about function-specific variations. The idea was to expose that directly as a new register set in the main function struct. We can then get rid of all definitions of TARGET_HARD_REGNO_SCRATCH_OK and most definitions of HARD_REGNO_RENAME_OK. (Some renames are invalid for other reasons.) The main problem is that we then have three sets of registers: - the ABI call-clobbered set - the call-clobbered set for the current function - the set of registers that are clobbered by an already-compiled function (for -fipa-ra) I think that's valid, but you obviously have to be very careful to use the right one. Especially when it comes to cached derived information. Thanks, Richard
Demangler patch committed: Fix constructor names with ABI tags
The symbol _ZNSt8ios_base7failureB5cxx11C1EPKcRKSt10error_code, which appears in libstdc++, was being demangled as std::ios_base::failure[abi:cxx11]::cxx11(char const*, std::error_code const&) That is clearly incorrect: std::ios_base::failure does not have a method cxx11, and anyhow if you look closely at the mangled name you will see that it is supposed to be a constructor. This patch fixes the demangler to generate the correct demangling, namely std::ios_base::failure[abi:cxx11]::failure(char const*, std::error_code const&) Bootstrapped and ran libiberty and libstdc++-v3 tests on x86_64-unknown-linux-gnu. Committed to mainline. Ian 2015-08-15 Ian Lance Taylor * cp-demangle.c (d_abi_tags): Preserve di->last_name across any ABI tags. Index: cp-demangle.c === --- cp-demangle.c (revision 226846) +++ cp-demangle.c (working copy) @@ -1306,7 +1306,12 @@ d_encoding (struct d_info *di, int top_l static struct demangle_component * d_abi_tags (struct d_info *di, struct demangle_component *dc) { + struct demangle_component *hold_last_name; char peek; + + /* Preserve the last name, so the ABI tag doesn't clobber it. */ + hold_last_name = di->last_name; + while (peek = d_peek_char (di), peek == 'B') { @@ -1315,6 +1320,9 @@ d_abi_tags (struct d_info *di, struct de tag = d_source_name (di); dc = d_make_comp (di, DEMANGLE_COMPONENT_TAGGED_NAME, dc, tag); } + + di->last_name = hold_last_name; + return dc; } Index: testsuite/demangle-expected === --- testsuite/demangle-expected (revision 226846) +++ testsuite/demangle-expected (working copy) @@ -4389,3 +4389,9 @@ f(std::string[abi:foo], std::string[abi: --format=gnu-v3 _Z18IndirectExternCallIPU7stdcallU7regparmILi3EEFviiEiEvT_T0_S3_ void IndirectExternCall stdcall*)(int, int), int>(void ( regparm<3> stdcall*)(int, int), int, void ( regparm<3> stdcall*)(int, int)) +# +# ABI tags used to confuse the constructor name calculation. +--format=gnu-v3 --no-params +_ZNSt8ios_base7failureB5cxx11C1EPKcRKSt10error_code +std::ios_base::failure[abi:cxx11]::failure(char const*, std::error_code const&) +std::ios_base::failure[abi:cxx11]::failure
Re: Demangler patch committed: Fix constructor names with ABI tags
Hi Ian, On 08/15/2015 03:23 PM, Ian Lance Taylor wrote: The symbol _ZNSt8ios_base7failureB5cxx11C1EPKcRKSt10error_code, which appears in libstdc++, was being demangled as std::ios_base::failure[abi:cxx11]::cxx11(char const*, std::error_code const&) That is clearly incorrect: std::ios_base::failure does not have a method cxx11, and anyhow if you look closely at the mangled name you will see that it is supposed to be a constructor. This patch fixes the demangler to generate the correct demangling, namely std::ios_base::failure[abi:cxx11]::failure(char const*, std::error_code const&) Bootstrapped and ran libiberty and libstdc++-v3 tests on x86_64-unknown-linux-gnu. Committed to mainline. At the moment I can't really further investigate myself (sorry about that) but I suspect this issue may be related to c++/63887. Can you double check? Thanks, Paolo.
[gomp4] Middle end bits for routines
I've committed this to the gomp4 branch. It extends the 'oacc function' attribute's dimension handling to deal with routines. With the latter we now use TREE_PURPOSE of the dimension list to indicate whether the routine may or may not spawn a partitioned loop on a particular axis. I had to tweak the validate_dims hook to tell it what the outermost such axis is (or -1 for a non-routine). The patch was complicated y the rather baroque handling of the routine directive in the C and C++ frontends. Now I see that handling is based on a misinterpretation of the specification. I'll clean that up shortly. nathan 2015-08-15 Nathan Sidwell * config/nvptx/nvptx.c (nvptx_reorg): Examine TREE_PURPOSE of dimensions. (nvptx_record_offload_symbol): Adjust. (nvptx_validate_dims): Adjust. * omp-low.c (replace_oacc_fn_attrib): Detect if we're the first attribute. (set_oacc_fn_attrub): Swap FN and CLAUSE parameters for consistency. (build_oacc_routine_dims): New. (expand_omp_target): Adjust set_oacc_fn_attrib call. (execute_oacc_transform): Deal with routine dimensions. (default_goacc_validate_dims): Add FN_LEVEL parameter. * omp-low.h (replace_oacc_fn_attrib, build_oacc_routine_dims): Declare. * target.def (validate_dims): Add FN_LEVEL arg. * targhooks.h (default_goacc_validate_dims): Adjust. * doc/tm.texi: Rebuilt. testsuite/ * c-c++-common/goacc/routine-4.c: Adjust expected error. Delete bogus tests. fortran/ * f95-lang.c (gfc_attribs): oacc function attribute can take list. * gfortran.h (struct symbol_attribute): Add more bits to oacc_routine field. * openmp.c (gfc_oacc_routine_dims): New. (gfc_match_oacc_routins): Call it. * trans-decl.c: Include gomp-constants.h. (add_attributes_to_decl): Create oacc function dimension data. cp/ * parser.c (cp_parser_oacc_routine_check_parallelism): Delete. (cp_parser_oacc_routine): Don't check parallelism here. (cp_parser_late_parssing_oacc_routine): Use build_oacc_routine_dims. c/ * c-parser.c (c_parser_oacc_routine): Don't check loop axes here, use build_oacc_routine_dims. c-family/ * c-common.c (c_common_attribs): oacc function can take list. Index: gcc/c/c-parser.c === --- gcc/c/c-parser.c (revision 226860) +++ gcc/c/c-parser.c (working copy) @@ -13291,7 +13291,6 @@ static void c_parser_oacc_routine (c_parser *parser, enum pragma_context context) { tree name = NULL_TREE; - location_t here = c_parser_peek_token (parser)->location; c_parser_consume_pragma (parser); @@ -13329,30 +13328,6 @@ c_parser_oacc_routine (c_parser *parser, "#pragma acc routine", OACC_ROUTINE_CLAUSE_DEVICE_TYPE_MASK); - /* Check of the presence if gang, worker, vector and seq clauses, and - throw an error if more than one of those clauses is specified. */ - int parallelism = 0; - tree c; - - for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c)) -switch (OMP_CLAUSE_CODE (c)) - { - case OMP_CLAUSE_GANG: - case OMP_CLAUSE_WORKER: - case OMP_CLAUSE_VECTOR: - case OMP_CLAUSE_SEQ: - ++parallelism; - break; - default: - break; - } - - if (parallelism > 1) -{ - error_at (here, "invalid combination of gang, worker, vector or seq for" - "%<#pragma acc routine%>"); -} - if (name) { TREE_CHAIN (name) = clauses; @@ -13422,14 +13397,16 @@ c_finish_oacc_routine (c_parser *parser, return; } + /* Process for function attrib */ + tree dims = build_oacc_routine_dims (clauses); + replace_oacc_fn_attrib (fndecl, dims); + + /* Also attach as a declare. */ if (clauses != NULL_TREE) clauses = tree_cons (NULL_TREE, clauses, NULL_TREE); - clauses = build_tree_list (get_identifier ("omp declare target"), - clauses); - TREE_CHAIN (clauses) = DECL_ATTRIBUTES (fndecl); DECL_ATTRIBUTES (fndecl) -= tree_cons (get_identifier ("oacc function"), - NULL_TREE, clauses); += tree_cons (get_identifier ("omp declare target"), + clauses, DECL_ATTRIBUTES (fndecl)); } /* OpenACC 2.0: Index: gcc/c-family/c-common.c === --- gcc/c-family/c-common.c (revision 226860) +++ gcc/c-family/c-common.c (working copy) @@ -823,7 +823,7 @@ const struct attribute_spec c_common_att { "bnd_instrument", 0, 0, true, false, false, handle_bnd_instrument, false }, { "oacc declare", 0, -1, true, false, false, NULL, false }, - { "oacc function", 0, 0, true, false, false, NULL, false }, + { "oacc function", 0, -1, true, false, false, NULL, false }, { NULL, 0, 0, false, false, false, NULL, false } }; Index: gcc/config/nvptx/nvptx.c === --- gcc/config/nvptx/nvptx.c (revision 226860) +++ gcc/config/nvptx/nvptx.c (working copy) @@ -3079,9 +3079,10 @@ nvptx_reorg (void) for (ix = 0
Re: [PATCH][MIPS] Fix register renaming in the interrupt handlers
On Aug 15, 2015, at 3:32 AM, Richard Sandiford wrote: > > The port is only buggy if they have define_peephole2s with match_scratches > and those match_scratches require a register that would be saved by > an interrupt function. In other cases defining T_H_R_S_O would have > no effect (but still be a good idea for future proofing -- obviously > someone who adds a new define_peephole2 is unlikely to be thinking > about inerrupt functions). Yeah, that was my reading of the code after I posted as well. My port was buggy. :-( I think all the other ports like likely buggy or suboptimal. > The main problem is that we then have three sets of registers: > - the ABI call-clobbered set > - the call-clobbered set for the current function > - the set of registers that are clobbered by an already-compiled function > (for -fipa-ra) > > I think that's valid, but you obviously have to be very careful to use > the right one. Especially when it comes to cached derived information. No. Being very careful isn’t the right solution. It should be impossible to use the wrong one. Something like static inline call_used (regno, call_abi = 0) { return call_used[call_abi][regno]; } where callers that want the call_used for a given function would call call_used (regno, fndecl) and fndecl can convert over to a call_abi or possibly something like fndecl->call_used[regno] where most callers can just use cfun->call_used[regnp] is the right one. It is fine for ports without interrupt functions (because call_abi defaults to 0), and it is the right one to use on ports with interrupt functions. In the later, the interrupt function will have its call_abi set appropriately. Ports that already have and manage call_abi today will notice they even like the new scheme. And the TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS code I think could benefit from this as well.
Re: Demangler patch committed: Fix constructor names with ABI tags
... I think your patch fixes c++/63887. The libiberty testsuite passes with the below applied. Paolo. // Index: testsuite/demangle-expected === --- testsuite/demangle-expected (revision 226910) +++ testsuite/demangle-expected (working copy) @@ -4390,8 +4390,13 @@ f(std::string[abi:foo], std::string[abi:foo]) _Z18IndirectExternCallIPU7stdcallU7regparmILi3EEFviiEiEvT_T0_S3_ void IndirectExternCall stdcall*)(int, int), int>(void ( regparm<3> stdcall*)(int, int), int, void ( regparm<3> stdcall*)(int, int)) # -# ABI tags used to confuse the constructor name calculation. +# ABI tags used to confuse the constructor and destructor name calculation. --format=gnu-v3 --no-params _ZNSt8ios_base7failureB5cxx11C1EPKcRKSt10error_code std::ios_base::failure[abi:cxx11]::failure(char const*, std::error_code const&) std::ios_base::failure[abi:cxx11]::failure +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63887 +--format=gnu-v3 --no-params +_ZNSt8time_getB5cxx11D1Ev +std::time_get[abi:cxx11]::~time_get() +std::time_get[abi:cxx11]::~time_get
Re: [PATCH] Remove pointless -fPIC warning on Windows platforms
On Aug 14, 2015, at 3:24 PM, Paolo Bonzini wrote: > There are plenty of targets that do not require -fPIC because they always > generate position independent code, but none of them feels the need to > complain with the user about an unnecessary but perfectly valid option, > on each and every .c->.o rule, and without a way to disable it. :-) I think darwin had, or has that same bug. I kinda like the notion that -fPIC is ignored on such a platform, not complained about.
Re: [PATCH][MIPS] Fix register renaming in the interrupt handlers
Mike Stump writes: > On Aug 15, 2015, at 3:32 AM, Richard Sandiford > wrote: >> >> The port is only buggy if they have define_peephole2s with match_scratches >> and those match_scratches require a register that would be saved by >> an interrupt function. In other cases defining T_H_R_S_O would have >> no effect (but still be a good idea for future proofing -- obviously >> someone who adds a new define_peephole2 is unlikely to be thinking >> about inerrupt functions). > > Yeah, that was my reading of the code after I posted as well. My port > was buggy. :-( I think all the other ports like likely buggy or > suboptimal. Suboptimal how? >> The main problem is that we then have three sets of registers: >> - the ABI call-clobbered set >> - the call-clobbered set for the current function >> - the set of registers that are clobbered by an already-compiled function >> (for -fipa-ra) >> >> I think that's valid, but you obviously have to be very careful to use >> the right one. Especially when it comes to cached derived information. > > No. Being very careful isn’t the right solution. It should be > impossible to use the wrong one. > > Something like static inline call_used (regno, call_abi = 0) { return > call_used[call_abi][regno]; } where callers that want the call_used for > a given function would call call_used (regno, fndecl) and fndecl can > convert over to a call_abi or possibly something like > fndecl->call_used[regno] where most callers can just use > cfun->call_used[regnp] is the right one. It is fine for ports without > interrupt functions (because call_abi defaults to 0), and it is the > right one to use on ports with interrupt functions. In the later, the > interrupt function will have its call_abi set appropriately. Ports that > already have and manage call_abi today will notice they even like the > new scheme. And the TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS > code I think could benefit from this as well. I don't see how that helps. The default argument creates exactly the same difference as thw first two in my list. There are many times when you don't know which function is being called and so can't pass a decl. The third in my list is still there unless you disable -fipa-ra. Richard
Re: [GOOGLE] Reset lambda scope information when popping module for LIPO
ok. David On Fri, Aug 14, 2015 at 11:13 PM, Teresa Johnson wrote: > This patch resets the lambda scope based sequence numbering used to assign > numbers to lambdas during parsing when we pop a module scope. This resets > the numbering for subsequent modules imported during LIPO compilation. > > It also renames cp_clear_deferred_fns to reset_parsing_state, as that > routine now (and has for awhile) encompasses more than just deferred > function clearing. > > Passes regression tests. Ok for google 4_9? > > 2015-08-14 Teresa Johnson > > Google ref b/23176787. > * cp/cp-lang.c: Rename clear_deferred_fns to reset_parsing_state. > * cp/cp-tree.h: Ditto, declare clear_lambda_scope. > * cp/decl2.c (cp_reset_parsing_state): Ditto, call clear_lambda_scope. > * cp/parser.c (clear_lambda_scope): New function. > * l-ipo.c (pop_module_scope): Rename clear_deferred_fns to > reset_parsing_state. > * langhooks-def.h: Ditto. > * langhooks.h: Ditto. > > Index: cp/cp-lang.c > === > --- cp/cp-lang.c(revision 226875) > +++ cp/cp-lang.c(working copy) > @@ -109,8 +109,8 @@ static tree get_template_argument_pack_elems_folde > #define LANG_HOOKS_COPY_LANG_TYPE cp_lipo_copy_lang_type > #undef LANG_HOOKS_PROCESS_PENDING_DECLS > #define LANG_HOOKS_PROCESS_PENDING_DECLS cp_process_pending_declarations > -#undef LANG_HOOKS_CLEAR_DEFFERED_FNS > -#define LANG_HOOKS_CLEAR_DEFFERED_FNS cp_clear_deferred_fns > +#undef LANG_HOOKS_RESET_PARSING_STATE > +#define LANG_HOOKS_RESET_PARSING_STATE cp_reset_parsing_state > #undef LANG_HOOKS_IS_GENERATED_TYPE > #define LANG_HOOKS_IS_GENERATED_TYPE cp_is_compiler_generated_type > #undef LANG_HOOKS_CMP_LANG_TYPE > Index: cp/cp-tree.h > === > --- cp/cp-tree.h(revision 226875) > +++ cp/cp-tree.h(working copy) > @@ -4385,6 +4385,7 @@ extern int cp_unevaluated_operand; > extern tree cp_convert_range_for (tree, tree, tree, bool); > extern bool parsing_nsdmi (void); > extern void inject_this_parameter (tree, cp_cv_quals); > +extern void clear_lambda_scope (void); > > /* in pt.c */ > > @@ -5342,7 +5343,7 @@ extern void cplus_decl_attributes (tree *, tree, > extern void finish_anon_union (tree); > extern void cp_write_global_declarations (void); > extern void cp_process_pending_declarations (location_t); > -extern void cp_clear_deferred_fns (void); > +extern void cp_reset_parsing_state (void); > extern void cp_clear_constexpr_hashtable(void); > extern void cp_clear_conv_type_map (void); > extern tree coerce_new_type(tree); > Index: cp/decl2.c > === > --- cp/decl2.c (revision 226875) > +++ cp/decl2.c (working copy) > @@ -4178,10 +4178,10 @@ no_linkage_error (tree decl) >"to declare function %q#D with linkage", t, decl); > } > > -/* Clear the list of deferred functions. */ > +/* Reset the parsing state for the next module. */ > > void > -cp_clear_deferred_fns (void) > +cp_reset_parsing_state (void) > { >vec_free (deferred_fns); >deferred_fns = NULL; > @@ -4192,6 +4192,7 @@ void >clear_pending_templates (); >reset_anon_name (); >reset_temp_count (); > + clear_lambda_scope (); > } > > /* Collect declarations from all namespaces relevant to SOURCE_FILE. */ > Index: cp/parser.c > === > --- cp/parser.c (revision 226875) > +++ cp/parser.c (working copy) > @@ -8705,6 +8705,16 @@ finish_lambda_scope (void) >lambda_scope_stack->pop (); > } > > +void > +clear_lambda_scope (void) > +{ > + if (!lambda_scope_stack) > +return; > + gcc_assert(lambda_scope_stack->is_empty()); > + lambda_scope = NULL_TREE; > + lambda_count = 0; > +} > + > /* Parse a lambda expression. > > lambda-expression: > Index: l-ipo.c > === > --- l-ipo.c (revision 226875) > +++ l-ipo.c (working copy) > @@ -414,7 +414,7 @@ pop_module_scope (void) >at_eof = 1; >cgraph_process_same_body_aliases (); >lang_hooks.l_ipo.process_pending_decls (input_location); > - lang_hooks.l_ipo.clear_deferred_fns (); > + lang_hooks.l_ipo.reset_parsing_state (); >at_eof = 0; > >is_last = is_last_module (current_module_id); > Index: langhooks-def.h > === > --- langhooks-def.h (revision 226875) > +++ langhooks-def.h (working copy) > @@ -214,7 +214,7 @@ extern tree lhd_make_node (enum tree_code); > #define LANG_HOOKS_DUP_LANG_TYPE lhd_do_nothing_t_t > #define LANG_HOOKS_COPY_LANG_TYPE lhd_do_nothing_t_t > #define LANG_HOOKS_PROCESS_PENDING_DECLS lhd_do_nothing_u > -#define LAN
RE: [Patch,tree-optimization]: Add new path Splitting pass on tree ssa representation
All: Please find the updated patch with suggestion and feedback incorporated. Thanks Jeff and Richard for the review comments. Following changes were done based on the feedback on RFC comments. and the review for the previous patch. 1. Both tracer and path splitting pass are separate passes so that two instances of the pass will run in the end, one doing path splitting and one doing tracing, at different times in the optimization pipeline. 2. Transform code is shared for tracer and path splitting pass. The common code in extracted in a given function transform_duplicate And place the function in tracer.c and the path splitting pass uses the transform code. 3. Analysis for the basic block population and traversing the basic block using the Fibonacci heap is commonly used. This cannot be Factored out into new function as the tracer pass does more analysis based on the profile and the different heuristics is used in tracer And path splitting pass. 4. The include headers is minimal and presence of what is required for the path splitting pass. 5. The earlier patch does the SSA updating with replace function to preserve the SSA representation required to move the loop latch node same as join Block to its predecessors and the loop latch node is just forward block. Such replace function are not required as suggested by the Jeff. Such replace Function goes away with this patch and the transformed code is factored into a given function which is shared between tracer and path splitting pass. Bootstrapping with i386 and Microblaze target works fine. No regression is seen in Deja GNU tests for Microblaze. There are lesser failures. Mibench/EEMBC benchmarks were run for Microblaze target and the gain of 9.3% is seen in rgbcmy_lite the EEMBC benchmarks. SPEC 2000 benchmarks were run with i386 target and the following performance number is achieved. INT benchmarks with path splitting(ratio) Vs INT benchmarks without path splitting(ratio) = 3661.225091 vs 3621.520572 FP benchmarks with path splitting(ratio) Vs FP benchmarks without path splitting(ratio ) = 4339.986209 vs 4339.775527 Maximum gains achieved with 252.eon INT benchmarks = 9.03%. ChangeLog: 2015-08-15 Ajit Agarwal * gcc/Makefile.in: Add the build of the new file tree-ssa-path-split.c * gcc/common.opt (ftree-path-split): Add the new flag. * gcc/opts.c (OPT_ftree_path_split) : Add an entry for Path splitting pass with optimization flag greater and equal to O2. * gcc/passes.def (path_split): add new path splitting pass. * gcc/timevar.def (TV_TREE_PATH_SPLIT): New. * gcc/tree-pass.h (make_pass_path_split): New declaration. * gcc/tree-ssa-path-split.c: New. * gcc/tracer.c (transform_duplicate): New. * gcc/testsuite/gcc.dg/tree-ssa/path-split-2.c: New. * gcc/testsuite/gcc.dg/path-split-1.c: New. * gcc/doc/invoke.texi (ftree-path-split): Document. (fdump-tree-path_split): Document. Signed-off-by:Ajit Agarwal ajit...@xilinx.com. Thanks & Regards Ajit -Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-ow...@gcc.gnu.org] On Behalf Of Ajit Kumar Agarwal Sent: Wednesday, July 29, 2015 10:13 AM To: Richard Biener; Jeff Law Cc: GCC Patches; Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: RE: [Patch,tree-optimization]: Add new path Splitting pass on tree ssa representation -Original Message- From: Richard Biener [mailto:richard.guent...@gmail.com] Sent: Thursday, July 16, 2015 4:30 PM To: Ajit Kumar Agarwal Cc: l...@redhat.com; GCC Patches; Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch,tree-optimization]: Add new path Splitting pass on tree ssa representation On Tue, Jul 7, 2015 at 3:22 PM, Ajit Kumar Agarwal wrote: > > > -Original Message- > From: Richard Biener [mailto:richard.guent...@gmail.com] > Sent: Tuesday, July 07, 2015 2:21 PM > To: Ajit Kumar Agarwal > Cc: l...@redhat.com; GCC Patches; Vinod Kathail; Shail Aditya Gupta; > Vidhumouli Hunsigida; Nagaraju Mekala > Subject: Re: [Patch,tree-optimization]: Add new path Splitting pass on > tree ssa representation > > On Sat, Jul 4, 2015 at 2:39 PM, Ajit Kumar Agarwal > wrote: >> >> >> -Original Message- >> From: Richard Biener [mailto:richard.guent...@gmail.com] >> Sent: Tuesday, June 30, 2015 4:42 PM >> To: Ajit Kumar Agarwal >> Cc: l...@redhat.com; GCC Patches; Vinod Kathail; Shail Aditya Gupta; >> Vidhumouli Hunsigida; Nagaraju Mekala >> Subject: Re: [Patch,tree-optimization]: Add new path Splitting pass >> on tree ssa representation >> >> On Tue, Jun 30, 2015 at 10:16 AM, Ajit Kumar Agarwal >> wrote: >>> All: >>> >>> The below patch added a new path Splitting optimization pass on SSA >>> representation. The Path Splitting optimization Pass moves the join >>> block of if-then-else same as loop latch
Re: [PATCH][MIPS] Fix register renaming in the interrupt handlers
On Aug 15, 2015, at 9:19 AM, Richard Sandiford wrote: > Mike Stump writes: >> On Aug 15, 2015, at 3:32 AM, Richard Sandiford >> wrote: >>> >>> The port is only buggy if they have define_peephole2s with match_scratches >>> and those match_scratches require a register that would be saved by >>> an interrupt function. In other cases defining T_H_R_S_O would have >>> no effect (but still be a good idea for future proofing -- obviously >>> someone who adds a new define_peephole2 is unlikely to be thinking >>> about inerrupt functions). >> >> Yeah, that was my reading of the code after I posted as well. My port >> was buggy. :-( I think all the other ports like likely buggy or >> suboptimal. > > Suboptimal how? Inability to use some registers. Hum, maybe that can’t happen for other reasons. > I don't see how that helps. Maybe I’m envisioning something you aren’t proposing. Anyway, a nice fix for this has code like: /* If this is a CALL_INSN, all call used registers are stored with unknown values. */ if (CALL_P (insn)) { for (i = FIRST_PSEUDO_REGISTER - 1; i >= 0; i--) { if (call_used_regs[i]) /* Reset the information about this register. */ reg_mode[i] = VOIDmode; } } being updated so that at least for some calls to an interrupt function, the mode of the reg isn’t set to VOIDmode. I didn’t choose this for any specific reason, I just grabbed a random use of call_used_regs that likely is wrong or less than optimal. In the type of fix I envision, it would/could address this. > The third in my list is still there unless you disable -fipa-ra. Maybe I was railing against something you’re not proposing. Anyway, safe to defer til we have more detail.
[gomp4] routine pragma c parsing
I've committed this patch to rectify the misinterpretation of the specification. The pragma comes in two forms: #pragma acc routine fn-decl-or-defn and #pragma acc routine (name) The latter form names a function already in scope, which makes it fairly straight forwards. However, the current implentation was such that 'name' need not be in scope -- in essence permitting forward reference to an undeclared entity. No other part of C permits this, so such an interpretation of the openacc spec must be wrong. I think the confusion comes from interpreting The routine directive with a name may appear anywhere that a function prototype is allowed and applies to the function in that scope with that name, but must appear before any definition or use of that function. to mean the function could be declared after the pragma. But that's not what it is saying -- merely that the pragma must appear before definition or use (we don't check that constraint, it's immaterial to this immplementation), not declaration. And the usual interpretation of 'in that scope' is an already declared object. I'll fix up the C++ parser next and then commit my testcases. nathan 2015-08-15 Nathan Sidwell c/ * c-parser.c (struct c_parser): Remove oacc_routines field. (c_parser_declaration_or_fndef): Remove 'named' parameter. Adjust all callers. Adjust c_finish_oacc_routine calls. (c_parser_oacc_routine): Reimplement. (c_finish_oacc_routine): Reimplement. testsuite/ * c-c++-common/goacc/dtype-1.c: Remove bogus routine tests. Index: gcc/c/c-parser.c === --- gcc/c/c-parser.c (revision 226911) +++ gcc/c/c-parser.c (working copy) @@ -225,10 +225,6 @@ typedef struct GTY(()) c_parser { /* Buffer to hold all the tokens from parsing the vector attribute for the SIMD-enabled functions (formerly known as elemental functions). */ vec *cilk_simd_fn_tokens; - - /* OpenACC specific parser information. */ - - vec *oacc_routines; } c_parser; @@ -1171,7 +1167,7 @@ static void c_parser_external_declaratio static void c_parser_asm_definition (c_parser *); static void c_parser_declaration_or_fndef (c_parser *, bool, bool, bool, bool, bool, tree *, vec, - tree, bool); + tree); static void c_parser_static_assert_declaration_no_semi (c_parser *); static void c_parser_static_assert_declaration (c_parser *); static void c_parser_declspecs (c_parser *, struct c_declspecs *, bool, bool, @@ -1261,8 +1257,7 @@ static bool c_parser_pragma (c_parser *, static bool c_parser_omp_target (c_parser *, enum pragma_context); static void c_parser_omp_end_declare_target (c_parser *); static void c_parser_omp_declare (c_parser *, enum pragma_context); -static void c_parser_oacc_routine (c_parser *parser, enum pragma_context - context); +static void c_parser_oacc_routine (c_parser *parser, enum pragma_context); static void c_parser_oacc_declare (c_parser *parser); /* These Objective-C parser functions are only ever called when @@ -1448,7 +1443,7 @@ c_parser_external_declaration (c_parser only tell which after parsing the declaration specifiers, if any, and the first declarator. */ c_parser_declaration_or_fndef (parser, true, true, true, false, true, - NULL, vNULL, NULL_TREE, false); + NULL, vNULL, NULL_TREE); break; } } @@ -1767,7 +1762,7 @@ finish_oacc_declare (tree fnbody, tree d static void c_finish_omp_declare_simd (c_parser *, tree, tree, vec); -static void c_finish_oacc_routine (c_parser *, tree, tree, bool); +static void c_finish_oacc_routine (c_parser *, tree, tree); /* Parse a declaration or function definition (C90 6.5, 6.7.1, C99 6.7, 6.9.1). If FNDEF_OK is true, a function definition is @@ -1846,7 +1841,7 @@ c_parser_declaration_or_fndef (c_parser bool nested, bool start_attr_ok, tree *objc_foreach_object_declaration, vec omp_declare_simd_clauses, - tree oacc_routine_clauses, bool oacc_routine_named) + tree oacc_routine_clauses) { struct c_declspecs *specs; tree prefix_attrs; @@ -2024,9 +2019,8 @@ c_parser_declaration_or_fndef (c_parser || !vec_safe_is_empty (parser->cilk_simd_fn_tokens)) c_finish_omp_declare_simd (parser, NULL_TREE, NULL_TREE, omp_declare_simd_clauses); - else - c_finish_oacc_routine (parser, NULL_TREE, - oacc_routine_clauses, oacc_routine_named); + if (oacc_routine_clauses) + c_finish_oacc_routine (parser, NULL_TREE, oacc_routine_clauses); c_parser_skip_to_end_of_block_or_statement (parser); return; } @@ -2123,9 +2117,9 @@ c_parser_declaration_or_fndef (c_parser || !vec_safe_is_empty (parser->cilk_simd_fn_tokens)) c_finish_omp_declare_simd (parser, d, NULL_TREE, omp_declare_simd_clauses); - else - c_finish_oacc_routine (parser, d, oacc_routine_clauses, - oacc_rou