Re: [c++-concepts] code review
On Sat, Jun 8, 2013 at 8:34 AM, Andrew Sutton wrote: >> template >> tree >> extract_goals (proof_state& s) >> ... >> return extract_goals(s); >> >> but I suppose STL style is OK, too. > > > Huh. I didn't realize that could be inlined. Neat. We do -- we have been doing so for quite some time now, including devirtualization and inlining of functions with internal linkage. The reason why I tend to prefer the function argument style over template argument-style is where I don't want to specify everything, but rather prefer type deduction. Which in my case is almost aways. There is one thing though: C++03 does not allow using a function with internal linkage or no linkage as template arguments. If you put the function in an unnamed namespace you would expect GCC to treat is as if it was of internal linkage for many purposes including automatic inlining, but it doesn't :-( For example, you lose the "defined but not used warning", and the "used but not defined" warnings :-(( -- Gaby
Re: [c++-concepts] code review
On Sun, Jun 9, 2013 at 3:34 PM, Oleg Endo wrote: > On Thu, 2013-06-06 at 16:29 -0400, Jason Merrill wrote: >> On 06/06/2013 01:47 PM, Andrew Sutton wrote: >> > I never did understand why this happens. Compiling with GCC-4.6, I get >> > these errors originating in logic.cc from an include of . >> > This is what I get: >> > >> > /usr/include/c++/4.6/cstdlib:76:8: error: attempt to use poisoned "calloc" >> >> Ah, I see: adding the include gets the mentions of malloc in before the >> names are poisoned. This change is OK. >> > > I ran into the same issue when I started using C++ std:: stuff in the SH > backend code last year. I posted a patch, but somehow it didn't go > anywhere... > > http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00880.html > > The workaround was to include as the first include in sh.c. > Would it be possible to have the change above also in trunk? > > Cheers, > Oleg > I strongly suggest prefering over for GCC source code base. The reason is that it brings us very little: 1. Not all compilers implement C++93/C++03 semantics on all platforms; in fact even GCC didn't on solaris platforms for example. So, from bootstrapping purposes, we better be on the safer side. 2. C++11 says that the implementation is free to define names in both namespaces (global and std.) If we ever accept C++11 in 5-10 years, we better have something can withstand the evolution. So, my advice is for GCC source code to forget about the headers for the most part. I can see an instance where or would make a difference but given point (1) above, no it doesn't. Just use the traditional headers and be done with it. Maybe I should have included this in our C++ coding standards, but I don't know how Benjamin, Lawrence, and Diego fee about it. -- Gaby
[patch] Fix two small issues
The first change works around a compiler error in in c++1y mode, I think it's a front end bug (reported as PR 57573) but is easy to solve with this change. The second changes a test to avoid calling try_lock() when the calling thread already owns the mutex, but moving the try_lock() call into a new thread. * include/std/mutex (call_once): Remove parentheses to fix error in c++1y and gnu++1y mode. * testsuite/30_threads/mutex/try_lock/2.cc: Call try_lock() in new thread to avoid undefined behaviour. Tested x86_64-linux, committed to trunk. commit a548855f6c161971cd34b973dc9dc3c5b5663112 Author: Jonathan Wakely Date: Sun Jun 9 16:54:20 2013 +0100 * include/std/mutex (call_once): Remove parentheses to fix error in c++1y and gnu++1y mode. * testsuite/30_threads/mutex/try_lock/2.cc: Call try_lock() in new thread to avoid undefined behaviour. diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex index 3c666c1..cdd05a3 100644 --- a/libstdc++-v3/include/std/mutex +++ b/libstdc++-v3/include/std/mutex @@ -783,7 +783,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __set_once_functor_lock_ptr(&__functor_lock); #endif - int __e = __gthread_once(&(__once._M_once), &__once_proxy); + int __e = __gthread_once(&__once._M_once, &__once_proxy); #ifndef _GLIBCXX_HAVE_TLS if (__functor_lock) diff --git a/libstdc++-v3/testsuite/30_threads/mutex/try_lock/2.cc b/libstdc++-v3/testsuite/30_threads/mutex/try_lock/2.cc index bb3fcd4..f2a6723 100644 --- a/libstdc++-v3/testsuite/30_threads/mutex/try_lock/2.cc +++ b/libstdc++-v3/testsuite/30_threads/mutex/try_lock/2.cc @@ -24,6 +24,7 @@ #include +#include #include #include @@ -38,15 +39,18 @@ int main() m.lock(); bool b; - try - { - b = m.try_lock(); - VERIFY( !b ); - } - catch (const std::system_error& e) - { - VERIFY( false ); - } + std::thread t([&] { +try + { +b = m.try_lock(); + } +catch (const std::system_error& e) + { +VERIFY( false ); + } + }); + t.join(); + VERIFY( !b ); m.unlock(); }
[C++ Patch] PR 38634
Hi, in this old issue, we ICE after having reported an error. The problem seems that start_preparsed_function doesn't directly inform the caller that push_template_decl failed. If we adjust for that and pass back a boolean to cp_parser_function_definition_from_specifiers_and_declarator via start_function, then the parser knows how to clean up, ie, skip the entire function, etc. Patch works well in terms of testsuite, I would ask you to double check in particular the pop_nested_class call which I added before the early return from start_preparsed_function to compensate for the preceding push_nested_class. Tested x86_64-linux. Thanks, Paolo. /cp 2013-06-11 Paolo Carlini PR c++/38634 * decl.c (start_preparsed_function): Return a bool, false if push_template_decl fails. (start_function): Adjust. * cp-tree.h: Update. /testsuite 2013-06-11 Paolo Carlini PR c++/38634 * g++.dg/template/crash116.C: New. Index: cp/cp-tree.h === --- cp/cp-tree.h(revision 199868) +++ cp/cp-tree.h(working copy) @@ -5202,8 +5202,9 @@ extern void finish_enum_value_list(tree); extern void finish_enum(tree); extern void build_enumerator (tree, tree, tree, location_t); extern tree lookup_enumerator (tree, tree); -extern void start_preparsed_function (tree, tree, int); -extern int start_function (cp_decl_specifier_seq *, const cp_declarator *, tree); +extern bool start_preparsed_function (tree, tree, int); +extern bool start_function (cp_decl_specifier_seq *, +const cp_declarator *, tree); extern tree begin_function_body(void); extern void finish_function_body (tree); extern tree outer_curly_brace_block(tree); Index: cp/decl.c === --- cp/decl.c (revision 199868) +++ cp/decl.c (working copy) @@ -13008,7 +13008,7 @@ check_function_type (tree decl, tree current_funct error_mark_node if the function has never been defined, or a BLOCK if the function has been defined somewhere. */ -void +bool start_preparsed_function (tree decl1, tree attrs, int flags) { tree ctype = NULL_TREE; @@ -13107,8 +13107,13 @@ start_preparsed_function (tree decl1, tree attrs, { /* FIXME: Handle error_mark_node more gracefully. */ tree newdecl1 = push_template_decl (decl1); - if (newdecl1 != error_mark_node) - decl1 = newdecl1; + if (newdecl1 == error_mark_node) + { + if (ctype || DECL_STATIC_FUNCTION_P (decl1)) + pop_nested_class (); + return false; + } + decl1 = newdecl1; } /* We are now in the scope of the function being defined. */ @@ -13219,7 +13224,7 @@ start_preparsed_function (tree decl1, tree attrs, /* This function may already have been parsed, in which case just return; our caller will skip over the body without parsing. */ if (DECL_INITIAL (decl1) != error_mark_node) -return; +return true; /* Initialize RTL machinery. We cannot do this until CURRENT_FUNCTION_DECL and DECL_RESULT are set up. We do this @@ -13381,17 +13386,19 @@ start_preparsed_function (tree decl1, tree attrs, start_fname_decls (); store_parm_decls (current_function_parms); + + return true; } /* Like start_preparsed_function, except that instead of a FUNCTION_DECL, this function takes DECLSPECS and DECLARATOR. - Returns 1 on success. If the DECLARATOR is not suitable for a function - (it defines a datum instead), we return 0, which tells - yyparse to report a parse error. */ + Returns true on success. If the DECLARATOR is not suitable + for a function, we return false, which tells the parser to + skip the entire function. */ -int +bool start_function (cp_decl_specifier_seq *declspecs, const cp_declarator *declarator, tree attrs) @@ -13400,13 +13407,13 @@ start_function (cp_decl_specifier_seq *declspecs, decl1 = grokdeclarator (declarator, declspecs, FUNCDEF, 1, &attrs); if (decl1 == error_mark_node) -return 0; +return false; /* If the declarator is not suitable for a function definition, cause a syntax error. */ if (decl1 == NULL_TREE || TREE_CODE (decl1) != FUNCTION_DECL) { error ("invalid function declaration"); - return 0; + return false; } if (DECL_MAIN_P (decl1)) @@ -13415,9 +13422,7 @@ start_function (cp_decl_specifier_seq *declspecs, gcc_assert (same_type_p (TREE_TYPE (TREE_TYPE (decl1)), integer_type_node)); - start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT); - - return 1; +
[PATCH] Add a couple of dialect and warning options regarding Objective-C instance variable scope
Hello, First, let me say that I have consciously broken most of the rules mentioned about patch submission at gcc.gnu.org but I have done so in order to spare myself from wasting time to provide a proper patch in case the implemented functionality is not deemed worthy of approval and adoption into GCC. If any of the implemented switches prove to be welcome I'll be more than happy to split them into separate patches, add test-cases and add ChangLog entries as needed. Two of these switches are related to a feature request I submitted a while ago, Bug 56044 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56044). I won't reproduce the entire argument here since it is available in the feature request. The relevant functionality in the patch comes in the form of two switches: -Wshadow-ivars which controls the "local declaration of ‘somevar’ hides instance variable" warning which curiously is enabled by default instead of being controlled at least by -Wshadow. The patch changes it so that this warning can be enabled and disabled specifically through -Wshadow-ivars as well as with all other shadowing-related warnings through -Wshadow. The reason for the extra switch is that, while searching through the Internet for a solution to this problem I have found out that other people are inconvenienced by this particular warning as well so it might be useful to be able to turn it off while keeping all the other shadowing-related warnings enabled. -flocal-ivars which when true, as it is by default, treats instance variables as having local scope. If false (-fno-local-ivars) instance variables must always be referred to as self->ivarname and references of ivarname resolve to the local or global scope as usual. I've also taken the opportunity of adding another switch unrelated to the above but related to instance variables: -fivar-visibility which can be set to either private, protected (the default), public and package. This sets the default instance variable visibility which normally is implicitly protected. My use-case for it is basically to be able to set it to public and thus effectively disable this visibility mechanism altogether which I find no use for and therefore have to circumvent. I'm not sure if anyone else feels the same way towards this but I figured it was worth a try. I'm attaching a preliminary patch against the current revision in case anyone wants to have a look. The changes are very small and any blatant mistakes should be immediately obvious. I have to admit to having virtually no knowledge of the internals of GCC but I have tried to keep in line with formatting guidelines and general style as well as looking up the particulars of the way options are handled in the available documentation to avoid blind copy-pasting. I have also tried to test the functionality both in my own (relatively large, or at least not too small) project and with small test programs and everything works as expected. Finallly, I tried running the tests too but these fail to complete both in the patched and unpatched version, possibly due to the way I've configured GCC. Dimitris Index: gcc/c-family/c.opt === --- gcc/c-family/c.opt (revision 199867) +++ gcc/c-family/c.opt (working copy) @@ -661,6 +661,10 @@ Wselector ObjC ObjC++ Var(warn_selector) Warning Warn if a selector has multiple methods +Wshadow-ivar +ObjC ObjC++ Var(warn_shadow_ivar) Init(-1) Warning +Warn if a local declaration hides an instance variable + Wsequence-point C ObjC C++ ObjC++ Var(warn_sequence_point) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) Warn about possible violations of sequence point rules @@ -1018,6 +1022,29 @@ fnil-receivers ObjC ObjC++ Var(flag_nil_receivers) Init(1) Assume that receivers of Objective-C messages may be nil +flocal-ivars +ObjC ObjC++ Var(flag_local_ivars) Init(1) +Allow access to instance variables as if they were local declarations within instance method implementations. + +fivar-visibility= +ObjC ObjC++ Joined RejectNegative Enum(ivar_visibility) Var(default_ivar_visibility) Init(IVAR_VISIBILITY_PROTECTED) +-fvisibility=[private|protected|public|package] Set the default symbol visibility + +Enum +Name(ivar_visibility) Type(enum ivar_visibility) UnknownError(unrecognized ivar visibility value %qs) + +EnumValue +Enum(ivar_visibility) String(private) Value(IVAR_VISIBILITY_PRIVATE) + +EnumValue +Enum(ivar_visibility) String(protected) Value(IVAR_VISIBILITY_PROTECTED) + +EnumValue +Enum(ivar_visibility) String(public) Value(IVAR_VISIBILITY_PUBLIC) + +EnumValue +Enum(ivar_visibility) String(package) Value(IVAR_VISIBILITY_PACKAGE) + fnonansi-builtins C++ ObjC++ Var(flag_no_nonansi_builtin, 0) Index: gcc/flag-types.h === --- gcc/flag-types.h (revision 199867) +++ gcc/flag-types.h (working copy) @@ -104,6 +104,16 @@ enum symbol_vis
[x86] Remove 2 builtins
Hello, this patch removes 2 builtins that are undocumented, unused, and have confusing semantics. Bootstrap+testsuite on x86_64-linux-gnu. 2013-06-10 Marc Glisse PR target/57224 * config/i386/i386.c (enum ix86_builtins, bdesc_args): Remove IX86_BUILTIN_CMPNGTSS and IX86_BUILTIN_CMPNGESS. -- Marc GlisseIndex: config/i386/i386.c === --- config/i386/i386.c (revision 199867) +++ config/i386/i386.c (working copy) @@ -25903,22 +25903,20 @@ enum ix86_builtins IX86_BUILTIN_CMPNGTPS, IX86_BUILTIN_CMPNGEPS, IX86_BUILTIN_CMPORDPS, IX86_BUILTIN_CMPUNORDPS, IX86_BUILTIN_CMPEQSS, IX86_BUILTIN_CMPLTSS, IX86_BUILTIN_CMPLESS, IX86_BUILTIN_CMPNEQSS, IX86_BUILTIN_CMPNLTSS, IX86_BUILTIN_CMPNLESS, - IX86_BUILTIN_CMPNGTSS, - IX86_BUILTIN_CMPNGESS, IX86_BUILTIN_CMPORDSS, IX86_BUILTIN_CMPUNORDSS, IX86_BUILTIN_COMIEQSS, IX86_BUILTIN_COMILTSS, IX86_BUILTIN_COMILESS, IX86_BUILTIN_COMIGTSS, IX86_BUILTIN_COMIGESS, IX86_BUILTIN_COMINEQSS, IX86_BUILTIN_UCOMIEQSS, @@ -27541,22 +27539,20 @@ static const struct builtin_description { OPTION_MASK_ISA_SSE, CODE_FOR_sse_maskcmpv4sf3, "__builtin_ia32_cmpngtps", IX86_BUILTIN_CMPNGTPS, UNGE, (int) V4SF_FTYPE_V4SF_V4SF_SWAP }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_maskcmpv4sf3, "__builtin_ia32_cmpngeps", IX86_BUILTIN_CMPNGEPS, UNGT, (int) V4SF_FTYPE_V4SF_V4SF_SWAP}, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_maskcmpv4sf3, "__builtin_ia32_cmpordps", IX86_BUILTIN_CMPORDPS, ORDERED, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpeqss", IX86_BUILTIN_CMPEQSS, EQ, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpltss", IX86_BUILTIN_CMPLTSS, LT, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpless", IX86_BUILTIN_CMPLESS, LE, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpunordss", IX86_BUILTIN_CMPUNORDSS, UNORDERED, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpneqss", IX86_BUILTIN_CMPNEQSS, NE, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpnltss", IX86_BUILTIN_CMPNLTSS, UNGE, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpnless", IX86_BUILTIN_CMPNLESS, UNGT, (int) V4SF_FTYPE_V4SF_V4SF }, - { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpngtss", IX86_BUILTIN_CMPNGTSS, UNGE, (int) V4SF_FTYPE_V4SF_V4SF_SWAP }, - { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpngess", IX86_BUILTIN_CMPNGESS, UNGT, (int) V4SF_FTYPE_V4SF_V4SF_SWAP }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmmaskcmpv4sf3, "__builtin_ia32_cmpordss", IX86_BUILTIN_CMPORDSS, ORDERED, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sminv4sf3, "__builtin_ia32_minps", IX86_BUILTIN_MINPS, UNKNOWN, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_smaxv4sf3, "__builtin_ia32_maxps", IX86_BUILTIN_MAXPS, UNKNOWN, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmsminv4sf3, "__builtin_ia32_minss", IX86_BUILTIN_MINSS, UNKNOWN, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_vmsmaxv4sf3, "__builtin_ia32_maxss", IX86_BUILTIN_MAXSS, UNKNOWN, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_andv4sf3, "__builtin_ia32_andps", IX86_BUILTIN_ANDPS, UNKNOWN, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_sse_andnotv4sf3, "__builtin_ia32_andnps", IX86_BUILTIN_ANDNPS, UNKNOWN, (int) V4SF_FTYPE_V4SF_V4SF }, { OPTION_MASK_ISA_SSE, CODE_FOR_iorv4sf3, "__builtin_ia32_orps", IX86_BUILTIN_ORPS, UNKNOWN, (int) V4SF_FTYPE_V4SF_V4SF },
Re: [c++-concepts] code review
On Thu, 2013-06-06 at 16:29 -0400, Jason Merrill wrote: > On 06/06/2013 01:47 PM, Andrew Sutton wrote: > > I never did understand why this happens. Compiling with GCC-4.6, I get > > these errors originating in logic.cc from an include of . > > This is what I get: > > > > /usr/include/c++/4.6/cstdlib:76:8: error: attempt to use poisoned "calloc" > > Ah, I see: adding the include gets the mentions of malloc in before the > names are poisoned. This change is OK. > I ran into the same issue when I started using C++ std:: stuff in the SH backend code last year. I posted a patch, but somehow it didn't go anywhere... http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00880.html The workaround was to include as the first include in sh.c. Would it be possible to have the change above also in trunk? Cheers, Oleg
More forwprop for vectors
Hello, just adapting yet another function so it also works with vectors. It seemed convenient to add a new macro. The name sucks (it doesn't match the semantics of INTEGRAL_TYPE_P), but I didn't want to name it INTEGER_SCALAR_OR_VECTOR_CONSTANT_P and didn't have any good idea for a short name. Bootstrap+testsuite on x86_64-linux-gnu. 2013-06-10 Marc Glisse * tree.h (INTEGRAL_CST_P): New macro. * tree-ssa-forwprop.c (simplify_bitwise_binary, associate_plusminus): Also apply the transformations to vectors. -- Marc GlisseIndex: tree.h === --- tree.h (revision 199867) +++ tree.h (working copy) @@ -1021,20 +1021,26 @@ extern void omp_clause_range_check_faile #define COMPLEX_FLOAT_TYPE_P(TYPE) \ (TREE_CODE (TYPE) == COMPLEX_TYPE\ && TREE_CODE (TREE_TYPE (TYPE)) == REAL_TYPE) /* Nonzero if TYPE represents a vector integer type. */ #define VECTOR_INTEGER_TYPE_P(TYPE)\ (VECTOR_TYPE_P (TYPE)\ && TREE_CODE (TREE_TYPE (TYPE)) == INTEGER_TYPE) +/* Nonzero if NODE represents a scalar or vector integer constant. */ + +#define INTEGRAL_CST_P(NODE) \ + (TREE_CODE (NODE) == INTEGER_CST \ + || (TREE_CODE (NODE) == VECTOR_CST \ + && VECTOR_INTEGER_TYPE_P (TREE_TYPE (NODE /* Nonzero if TYPE represents a vector floating-point type. */ #define VECTOR_FLOAT_TYPE_P(TYPE) \ (VECTOR_TYPE_P (TYPE)\ && TREE_CODE (TREE_TYPE (TYPE)) == REAL_TYPE) /* Nonzero if TYPE represents a floating-point type, including complex and vector floating-point types. The vector and complex check does not use the previous two macros to enable early folding. */ Index: tree-ssa-forwprop.c === --- tree-ssa-forwprop.c (revision 199867) +++ tree-ssa-forwprop.c (working copy) @@ -1971,22 +1971,22 @@ simplify_bitwise_binary (gimple_stmt_ite gimple_assign_set_rhs2 (stmt, b); gimple_assign_set_rhs_code (stmt, def1_code); update_stmt (stmt); return true; } } /* (a | CST1) & CST2 -> (a & CST2) | (CST1 & CST2). */ if (code == BIT_AND_EXPR && def1_code == BIT_IOR_EXPR - && TREE_CODE (arg2) == INTEGER_CST - && TREE_CODE (def1_arg2) == INTEGER_CST) + && INTEGRAL_CST_P (arg2) + && INTEGRAL_CST_P (def1_arg2)) { tree cst = fold_build2 (BIT_AND_EXPR, TREE_TYPE (arg2), arg2, def1_arg2); tree tem; gimple newop; if (integer_zerop (cst)) { gimple_assign_set_rhs1 (stmt, def1_arg1); update_stmt (stmt); return true; @@ -2002,34 +2002,33 @@ simplify_bitwise_binary (gimple_stmt_ite gimple_assign_set_rhs_code (stmt, BIT_IOR_EXPR); update_stmt (stmt); return true; } /* Combine successive equal operations with constants. */ if ((code == BIT_AND_EXPR || code == BIT_IOR_EXPR || code == BIT_XOR_EXPR) && def1_code == code - && TREE_CODE (arg2) == INTEGER_CST - && TREE_CODE (def1_arg2) == INTEGER_CST) + && INTEGRAL_CST_P (arg2) + && INTEGRAL_CST_P (def1_arg2)) { tree cst = fold_build2 (code, TREE_TYPE (arg2), arg2, def1_arg2); gimple_assign_set_rhs1 (stmt, def1_arg1); gimple_assign_set_rhs2 (stmt, cst); update_stmt (stmt); return true; } /* Canonicalize X ^ ~0 to ~X. */ if (code == BIT_XOR_EXPR - && TREE_CODE (arg2) == INTEGER_CST && integer_all_onesp (arg2)) { gimple_assign_set_rhs_with_ops (gsi, BIT_NOT_EXPR, arg1, NULL_TREE); gcc_assert (gsi_stmt (*gsi) == stmt); update_stmt (stmt); return true; } /* Try simple folding for X op !X, and X op X. */ res = simplify_bitwise_binary_1 (code, TREE_TYPE (arg1), arg1, arg2); @@ -2472,66 +2471,68 @@ associate_plusminus (gimple_stmt_iterato && code != def_code) { /* (A +- B) -+ B -> A. */ code = TREE_CODE (def_rhs1); rhs1 = def_rhs1; rhs2 = NULL_TREE; gimple_assign_set_rhs_with_ops (gsi, code, rhs1, NULL_TREE); gcc_assert (gsi_stmt (*gsi) == stmt); gimple_set_modified (stmt, true); } - else if (TREE_CODE (rhs2) == INTEGER_CST - && TREE_CODE (def_rhs1) == INTEGER_CST) + else if (INTEGRAL_CST_P (rhs2) + && INTEGRAL_CST_P (def_rhs1)) { /* (CST +- A) +- CST -> CST +- A. */ tree cst = fold_binary (code, TREE_TYPE (rhs1), def_rhs1, rhs2); if (cst &&
Document __builtin_isinf_sign more precisely
Hello, this patch documents that __builtin_isinf_sign returns +-1 for +-Inf. This builtin was created so it could be used by a libc that has a stronger guarantee than the standard, and for glibc that means returning +-1, which is what the code already does. 2013-06-10 Marc Glisse PR middle-end/57219 * doc/extend.texi (__builtin_isinf_sign): Restrict the return values to -1, 0 and 1. -- Marc GlisseIndex: doc/extend.texi === --- doc/extend.texi (revision 199867) +++ doc/extend.texi (working copy) @@ -8603,22 +8603,23 @@ Similar to @code{__builtin_inf}, except Similar to @code{__builtin_inf}, except the return type is @code{float}. This function is suitable for implementing the ISO C99 macro @code{INFINITY}. @end deftypefn @deftypefn {Built-in Function} {long double} __builtin_infl (void) Similar to @code{__builtin_inf}, except the return type is @code{long double}. @end deftypefn @deftypefn {Built-in Function} int __builtin_isinf_sign (...) -Similar to @code{isinf}, except the return value is negative for -an argument of @code{-Inf}. Note while the parameter list is an +Similar to @code{isinf}, except the return value is -1 for +an argument of @code{-Inf} and 1 for an argument of @code{+Inf}. +Note while the parameter list is an ellipsis, this function only accepts exactly one floating-point argument. GCC treats this parameter as type-generic, which means it does not do default promotion from float to double. @end deftypefn @deftypefn {Built-in Function} double __builtin_nan (const char *str) This is an implementation of the ISO C99 function @code{nan}. Since ISO C99 defines this function in terms of @code{strtod}, which we do not implement, a description of the parsing is in order. The string
[C++ testcase, committed] PR 37404
Hi tested x86_64-linux, committed to mainline. Thanks, Paolo. 2013-06-09 Paolo Carlini PR c++/37404 * g++.dg/other/vararg-4.C: New. Index: g++.dg/other/vararg-4.C === --- g++.dg/other/vararg-4.C (revision 0) +++ g++.dg/other/vararg-4.C (working copy) @@ -0,0 +1,12 @@ +// PR c++/37404 + +typedef __builtin_va_list __gnuc_va_list; +typedef __gnuc_va_list va_list; + +template struct S { static void foo () { } }; +template +struct S { static void foo () { T(); } }; + +int main () { + S::foo(); +}
Re: [PATCH] Fix TARGET_READ_MODIFY_WRITE peephole2s (PR target/57568)
On Sun, Jun 9, 2013 at 10:57 AM, Jakub Jelinek wrote: > These two peephole2s misbehave if it sees e.g. > regN = mem > regN = regN + regN > mem = regN > CC = regN != 0 > because transforming it into > mem = mem + regN ; CC = mem != 0 > is wrong, I forgot to verify the second operand of the > plusminuslogic_operator doesn't overlap the first operand. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk/4.8 and perhaps also 4.7 (where it is just latent)? > > 2013-06-09 Jakub Jelinek > > PR target/57568 > * config/i386/i386.md (TARGET_READ_MODIFY_WRITE peepholes): Ensure > that operands[2] doesn't overlap with operands[0]. > > * gcc.c-torture/execute/pr57568.c: New test. The patch is OK everywhere. Thanks, Uros.
Remove self-assignments
Hello, this patch removes some self-assignments. I don't know if this is the best way, but it passes a bootstrap and the testsuite on x86_64-linux-gnu. 2013-06-10 Marc Glisse PR tree-optimization/57361 gcc/ * tree-ssa-dse.c (dse_possible_dead_store_p): Handle self-assignment. gcc/testsuite/ * gcc.dg/tree-ssa/pr57361.c: New file. -- Marc GlisseIndex: testsuite/gcc.dg/tree-ssa/pr57361.c === --- testsuite/gcc.dg/tree-ssa/pr57361.c (revision 0) +++ testsuite/gcc.dg/tree-ssa/pr57361.c (revision 0) @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fdump-tree-dse1-details" } */ + +struct A { int x; double y; }; +void f (struct A *a) { + *a = *a; +} + +/* { dg-final { scan-tree-dump "Deleted dead store" "dse1"} } */ Property changes on: testsuite/gcc.dg/tree-ssa/pr57361.c ___ Added: svn:keywords + Author Date Id Revision URL Added: svn:eol-style + native Index: tree-ssa-dse.c === --- tree-ssa-dse.c (revision 199867) +++ tree-ssa-dse.c (working copy) @@ -77,20 +77,29 @@ static void dse_enter_block (struct dom_ Return TRUE if the above conditions are met, otherwise FALSE. */ static bool dse_possible_dead_store_p (gimple stmt, gimple *use_stmt) { gimple temp; unsigned cnt = 0; *use_stmt = NULL; + /* Self-assignments are zombies. */ + if (gimple_assign_rhs_code (stmt) == TREE_CODE (gimple_assign_lhs (stmt)) + && operand_equal_p (gimple_assign_rhs1 (stmt), + gimple_assign_lhs (stmt), 0)) +{ + *use_stmt = stmt; + return true; +} + /* Find the first dominated statement that clobbers (part of) the memory stmt stores to with no intermediate statement that may use part of the memory stmt stores. That is, find a store that may prove stmt to be a dead store. */ temp = stmt; do { gimple use_stmt, defvar_def; imm_use_iterator ui; bool fail = false;
Re: *PING* / Re: [Patch, Fortran] Finalize nonallocatables with INTENT(out)
Tobias Burnus writes: > --- /dev/null 2013-06-06 09:52:08.544104880 +0200 > +++ gcc/gcc/testsuite/gfortran.dg/finalize_10.f90 2013-06-03 > 12:32:38.763008261 +0200 > @@ -0,0 +1,39 @@ > +! { dg-do compile } > +! { dg-options "-fdump-tree-original" } > +! > +! PR fortran/37336 > +! > +! Finalize nonallocatable INTENT(OUT) > +! > +module m > + type t > + end type t > + type t2 > + contains > +final :: fini > + end type t2 > +contains > + elemental subroutine fini(var) > +type(t2), intent(inout) :: var > + end subroutine fini > +end module m > + > +subroutine foo(x,y,aa,bb) > + use m > + class(t), intent(out) :: x(:),y > + type(t2), intent(out) :: aa(:),bb > +end subroutine foo > + > +! Finalize CLASS + set default init > +! { dg-final { scan-tree-dump-times "y->_vptr->_final \\(&desc.\[0-9\]+, > y->_vptr->_size, 0\\);" 1 "original" } } > +! { dg-final { scan-tree-dump-times "__builtin_memcpy \\(\\(void .\\) > y->_data, \\(void .\\) y->_vptr->_def_init, \\(unsigned long\\) > y->_vptr->_size\\);" 1 "original" } } That doesn't match. (void) __builtin_memcpy ((void *) y->_data, (void *) y->_vptr->_def_init, (character(kind=4)) y->_vptr->_size); Appears to be a 32/64 bit issue. 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."
[C++] Fix __builtin_shuffle
Hello, when porting __builtin_shuffle from C to C++, I ignored all the C++ specificities and added some extra bugs. This should improve things a bit. Bootstrap+testsuite on x86_64-linux-gnu. 2013-06-10 Marc Glisse PR c++/57509 gcc/c-family/ * c-common.h (c_build_vec_perm_expr): New complain argument. * c-common.c (c_build_vec_perm_expr): Likewise. Use save_expr also in C++. gcc/cp/ * typeck.c (cp_build_vec_perm_expr): New function. * cp-tree.h: Declare it. * parser.c (cp_parser_postfix_expression): Call it. * pt.c (tsubst_copy): Handle VEC_PERM_EXPR. (tsubst_copy_and_build): Likewise. gcc/testsuite/ * g++.dg/ext/pr57509.C: New file. -- Marc GlisseIndex: c-family/c-common.c === --- c-family/c-common.c (revision 199867) +++ c-family/c-common.c (working copy) @@ -2253,95 +2253,103 @@ vector_types_convertible_p (const_tree t and have vector types, V0 has the same type as V1, and the number of elements of V0, V1, MASK is the same. In case V1 is a NULL_TREE it is assumed that __builtin_shuffle was called with two arguments. In this case implementation passes the first argument twice in order to share the same tree code. This fact could enable the mask-values being twice the vector length. This is an implementation accident and this semantics is not guaranteed to the user. */ tree -c_build_vec_perm_expr (location_t loc, tree v0, tree v1, tree mask) +c_build_vec_perm_expr (location_t loc, tree v0, tree v1, tree mask, + bool complain) { tree ret; bool wrap = true; bool maybe_const = false; bool two_arguments = false; if (v1 == NULL_TREE) { two_arguments = true; v1 = v0; } if (v0 == error_mark_node || v1 == error_mark_node || mask == error_mark_node) return error_mark_node; if (TREE_CODE (TREE_TYPE (mask)) != VECTOR_TYPE || TREE_CODE (TREE_TYPE (TREE_TYPE (mask))) != INTEGER_TYPE) { - error_at (loc, "__builtin_shuffle last argument must " -"be an integer vector"); + if (complain) + error_at (loc, "__builtin_shuffle last argument must " + "be an integer vector"); return error_mark_node; } if (TREE_CODE (TREE_TYPE (v0)) != VECTOR_TYPE || TREE_CODE (TREE_TYPE (v1)) != VECTOR_TYPE) { - error_at (loc, "__builtin_shuffle arguments must be vectors"); + if (complain) + error_at (loc, "__builtin_shuffle arguments must be vectors"); return error_mark_node; } if (TYPE_MAIN_VARIANT (TREE_TYPE (v0)) != TYPE_MAIN_VARIANT (TREE_TYPE (v1))) { - error_at (loc, "__builtin_shuffle argument vectors must be of " -"the same type"); + if (complain) + error_at (loc, "__builtin_shuffle argument vectors must be of " + "the same type"); return error_mark_node; } if (TYPE_VECTOR_SUBPARTS (TREE_TYPE (v0)) != TYPE_VECTOR_SUBPARTS (TREE_TYPE (mask)) && TYPE_VECTOR_SUBPARTS (TREE_TYPE (v1)) != TYPE_VECTOR_SUBPARTS (TREE_TYPE (mask))) { - error_at (loc, "__builtin_shuffle number of elements of the " -"argument vector(s) and the mask vector should " -"be the same"); + if (complain) + error_at (loc, "__builtin_shuffle number of elements of the " + "argument vector(s) and the mask vector should " + "be the same"); return error_mark_node; } if (GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (v0 != GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (mask) { - error_at (loc, "__builtin_shuffle argument vector(s) inner type " -"must have the same size as inner type of the mask"); + if (complain) + error_at (loc, "__builtin_shuffle argument vector(s) inner type " + "must have the same size as inner type of the mask"); return error_mark_node; } if (!c_dialect_cxx ()) { /* Avoid C_MAYBE_CONST_EXPRs inside VEC_PERM_EXPR. */ v0 = c_fully_fold (v0, false, &maybe_const); wrap &= maybe_const; if (two_arguments) v1 = v0 = save_expr (v0); else { v1 = c_fully_fold (v1, false, &maybe_const); wrap &= maybe_const; } mask = c_fully_fold (mask, false, &maybe_const); wrap &= maybe_const; } + else if (two_arguments) +v1 = v0 = save_expr (v0); ret = build3_loc (loc, VEC_PERM_EXPR, TREE_TYPE (v0), v0, v1, mask); if (!c_dialect_cxx () && !wrap) ret = c_wrap_maybe_const (ret, true); return ret; } /* Like tree.c:get_narrower, but retain conversion from C++0x scoped enum Index: c-family/c-common.h
Re: *PING* / Re: [Patch, Fortran] Finalize nonallocatables with INTENT(out)
Dear Tobias, The test gfortran.dg/finalize_10.f90 fails in 32 bit mode (see http://gcc.gnu.org/ml/gcc-testresults/2013-06/msg00842.html FAIL: gfortran.dg/finalize_10.f90 -O scan-tree-dump-times original "__builtin_memcpy ((void .) y->_data, (void .) y->_vptr->_def_init, (unsigned long) y->_vptr->_size);" 1) because "unsigned long" is replaced with "unsigned int". The following patch fixes it --- ../_clean/gcc/testsuite/gfortran.dg/finalize_10.f90 2013-06-08 21:50:32.0 +0200 +++ gcc/testsuite/gfortran.dg/finalize_10.f90 2013-06-09 11:33:12.0 +0200 @@ -26,7 +26,8 @@ end subroutine foo ! Finalize CLASS + set default init ! { dg-final { scan-tree-dump-times "y->_vptr->_final \\(&desc.\[0-9\]+, y->_vptr->_size, 0\\);" 1 "original" } } -! { dg-final { scan-tree-dump-times "__builtin_memcpy \\(\\(void .\\) y->_data, \\(void .\\) y->_vptr->_def_init, \\(unsigned long\\) y->_vptr->_size\\);" 1 "original" } } +! { dg-final { scan-tree-dump-times "__builtin_memcpy \\(\\(void .\\) y->_data, \\(void .\\) y->_vptr->_def_init, \\(unsigned long\\) y->_vptr->_size\\);" 1 "original" { target lp64 } } } +! { dg-final { scan-tree-dump-times "__builtin_memcpy \\(\\(void .\\) y->_data, \\(void .\\) y->_vptr->_def_init, \\(unsigned int\\) y->_vptr->_size\\);" 1 "original" { target ilp32 } } } ! { dg-final { scan-tree-dump-times "x->_vptr->_final \\(&x->_data, x->_vptr->_size, 0\\);" 1 "original" } } ! { dg-final { scan-tree-dump-times "x->_vptr->_copy \\(x->_vptr->_def_init, &x->_data\\);" 1 "original" } } I have tried to weaken the test by not using any target and using a regexp of the kind "(int|long)", but I did not succeeded. CAVEAT: I don't know if the targets work for x32. TIA Dominique
[PATCH] Fix TARGET_READ_MODIFY_WRITE peephole2s (PR target/57568)
Hi! These two peephole2s misbehave if it sees e.g. regN = mem regN = regN + regN mem = regN CC = regN != 0 because transforming it into mem = mem + regN ; CC = mem != 0 is wrong, I forgot to verify the second operand of the plusminuslogic_operator doesn't overlap the first operand. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.8 and perhaps also 4.7 (where it is just latent)? 2013-06-09 Jakub Jelinek PR target/57568 * config/i386/i386.md (TARGET_READ_MODIFY_WRITE peepholes): Ensure that operands[2] doesn't overlap with operands[0]. * gcc.c-torture/execute/pr57568.c: New test. --- gcc/config/i386/i386.md.jj 2013-06-03 19:15:34.0 +0200 +++ gcc/config/i386/i386.md 2013-06-08 21:59:43.416954936 +0200 @@ -16591,6 +16591,7 @@ (define_peephole2 "(TARGET_READ_MODIFY_WRITE || optimize_insn_for_size_p ()) && peep2_reg_dead_p (4, operands[0]) && !reg_overlap_mentioned_p (operands[0], operands[1]) + && !reg_overlap_mentioned_p (operands[0], operands[2]) && (mode != QImode || immediate_operand (operands[2], QImode) || q_regs_operand (operands[2], QImode)) @@ -16655,6 +16656,7 @@ (define_peephole2 || immediate_operand (operands[2], SImode) || q_regs_operand (operands[2], SImode)) && !reg_overlap_mentioned_p (operands[0], operands[1]) + && !reg_overlap_mentioned_p (operands[0], operands[2]) && ix86_match_ccmode (peep2_next_insn (3), (GET_CODE (operands[3]) == PLUS || GET_CODE (operands[3]) == MINUS) --- gcc/testsuite/gcc.c-torture/execute/pr57568.c.jj2013-06-08 22:03:38.861005658 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr57568.c 2013-06-08 22:03:19.0 +0200 @@ -0,0 +1,12 @@ +/* PR target/57568 */ + +extern void abort (void); +int a[6][9] = { }, b = 1, *c = &a[3][5]; + +int +main () +{ + if (b && (*c = *c + *c)) +abort (); + return 0; +} Jakub
Re: [Patch, Fortran] PR57508 - Fix ICE/Reject-valid issue with get_temp_from_expr (intrinsic assignment with defined assignment)
Hello, Le 03/06/2013 16:06, Tobias Burnus a écrit : > Dear all, > > Due to copying the attributes, the temporary variable could get marked > as function (attr.function, attr.flavor == FL_PROCEDURE). This either > lead to leaking those attributes into the assembler file - or to cause > an error due to the call to gfc_add_flavor. With this patch, I now > explicitly unset those attribues. (Fund when building ForTrilinos.) > > diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c > index b2e8fdc..655d3c1 100644 > --- a/gcc/fortran/resolve.c > +++ b/gcc/fortran/resolve.c > @@ -9293,8 +9293,12 @@ get_temp_from_expr (gfc_expr *e, gfc_namespace *ns) > } > } > >/* Add the attributes and the arrayspec to the temporary. */ >/* Add the attributes and the arrayspec to the temporary. */ >tmp->n.sym->attr = gfc_expr_attr (e); > + tmp->n.sym->attr.function = 0; > + tmp->n.sym->attr.result = 0; > + tmp->n.sym->attr.flavor = FL_VARIABLE; > + >if (as) > { >tmp->n.sym->as = gfc_copy_array_spec (as); This fixes the problem, but shouldn't the fix be in gfc_expr_attr instead? It seems to me that most symbol attributes don't make sense in any case for non-variables, except for some of the standard ones (allocatable,...) and possibly a couple more. Mikael