Re: [PATCH] Fix pr57637
On 22 July 2013 21:16, Eric Botcazou ebotca...@adacore.com wrote: And if df_live is non-zero, do we need update df_lr's IN and OUT? I think we need another patch to make all these consistency. Possibly, but this would belong to another patch. I nevertheless think that we should set the bit in the GEN set because we'll be testing the GEN set now. The patch is OK with this change if it passes the usual testing. Thanks. Bootstrap on x86-64, ARM chrome book and Pandaboard. No make check regression on x86-64 and Pandaboard. The patch is committed with the ChangeLog updated according to your comments. -Zhenqiang ChangeLog 2013-07-22 Zhenqiang Chen zhenqiang.c...@linaro.org * function.c (move_insn_for_shrink_wrap): check gen of df_live if it exists, otherwise (-O1) give up searching. Capital letter at the beginning, and I'd expand a little on it, something like: * function.c (move_insn_for_shrink_wrap): Also check the GEN set of the LIVE problem for the liveness analysis if it exists, otherwise give up. gcc/testsuite/ChangeLog 2013-07-22 Zhenqiang Chen zhenqiang.c...@linaro.org * gcc.target/arm/pr57637.c: New added. New testcase -- Eric Botcazou
Re: [Patch, Fortran, OOP] PR 57306: ICE on valid with class pointer initialization
2013/7/30 Janus Weil ja...@gcc.gnu.org: The attached new version should do the right thing now. At least it shows the correct dump for the original test case as well as yours. It is currently being regtested. unfortunately it shows a couple of runtime problems with type-bound operators: FAIL: gfortran.dg/class_defined_operator_1.f03 -O0 execution test FAIL: gfortran.dg/typebound_operator_13.f03 -O0 execution test FAIL: gfortran.dg/typebound_operator_7.f03 -O0 execution test FAIL: gfortran.dg/typebound_operator_8.f03 -O0 execution test FAIL: gfortran.dg/typebound_operator_9.f03 -O0 execution test The attached update fixes it, and thus should hopefully be regression-free. It also renames 'gfc_class_null_initializer' to 'gfc_class_initializer', since it now also does other initializations beside EXPR_NULL. Will do another regtest to make sure it's clean. No failures observed. As a test case I'm using now Tobias' extended version (attached). New ChangeLog below. Ok for trunk? Cheers, Janus 2013-07-30 Janus Weil ja...@gcc.gnu.org PR fortran/57306 * class.c (gfc_class_null_initializer): Rename to 'gfc_class_initializer'. Treat non-NULL init-exprs. * gfortran.h (gfc_class_null_initializer): Update prototype. * trans-decl.c (gfc_get_symbol_decl): Treat class pointers. * trans-expr.c (gfc_conv_initializer): Ditto. (gfc_trans_subcomponent_assign): Renamed gfc_class_null_initializer. 2013-07-30 Janus Weil ja...@gcc.gnu.org PR fortran/57306 * gfortran.dg/pointer_init_8.f90: New. pr57306_v4.diff Description: Binary data pointer_init_8.f90 Description: Binary data
Re: C++ coding conventions: namespaces, references and getters (was Re: [PATCH 2/2] Introduce beginnings of a pipeline class.)
Hi,
thanks for the email I was supposed to write.
On Mon, Jul 29, 2013 at 02:20:02PM -0400, David Malcolm wrote:
On Thu, 2013-07-25 at 15:08 +0200, Martin Jambor wrote:
Hi,
I don't know why it's me again but again I do have a few comments.
Thanks for looking over the patch.
One global remark first: If we are going to start using the gcc
namespace (I understand it you need for isolation of symbols once you
use gcc as library, right?), I'm wondering whether mixing using
namespace gcc and explicit identifier qualifications is a good idea.
We have never had a discussion about namespace conventions but I'd
suggest that you add the using directive at the top of all gcc source
files that need it and never use explicit gcc:: qualification (unless
there is a reason for it, of course, like in symbol definitions).
But perhaps someone else with more C++ experience disagrees?
http://gcc.gnu.org/codingconventions.html#Namespace_Use says:
Namespaces are encouraged. All separable libraries should have a
unique global namespace.
[...snip...]
Header files should have neither using directives nor namespace-scope
using declarations.
and the rationale doc says:
Using them within an implementation file can help conciseness.
However, there doesn't seem to be a discussion on the merits of the
various forms of using directives.
These aren't the GCC coding conventions, but the Google conventions:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces
forbid using directives of the form:
using NAMESPACE;
but suggest using directives of the form:
using NAMESPACE::NAME;
to pick out individual names from a namespace, though *not* in global
scope in a header (to avoid polluting the global namespace).
I like this approach - how about using it for frequently used names in
a .c/.cc file, keeping the names alphabetizing by fully qualified
path, so e.g.:
#include foo.h
...
#include bar.h
using gcc::context;
using gcc::pass_manager;
...etc, individually grabbing the names we'll be needing
// code follows
and thus avoiding grabbing whole namespaces, whilst keeping the code
concise.
I don't know. Before your patches there were no namespaces and the
one flat global namespace is cluttered, all symbols from included
header files are there. I thought you just wanted to move (some part
of) this to the gcc namespace so that users of gcc-as-library will not
clash with it. So I guess it depends on what you want to move to the
gcc namespace. This enumeration approach would be viable if there
were only a few things in it, we can't expect people to put tens or
hundreds of such using directives to their sources.
In any event, I do not consider this to be really important, I was
just curious. One day there will be great bikeshedding about division
of our current flat namespace, I certainly hope that day has not come
yet, though ;-)
I may want to violate that rule within gtype-desc.c, as per:
http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01262.html
to simplify handling of GTY and gcc::
A few more comments inline:
Likewise
[...]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index b82c2e0..dc489fb 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -194,6 +194,8 @@ along with GCC; see the file COPYING3. If not see
#include except.h
#include cfgloop.h
#include regset.h /* FIXME: For reg_obstack. */
+#include context.h
+#include pipeline.h
/* Queue of cgraph nodes scheduled to be added into cgraph. This is a
secondary queue used during optimization to accommodate passes that
@@ -478,6 +480,7 @@ cgraph_finalize_function (tree decl, bool nested)
void
cgraph_add_new_function (tree fndecl, bool lowered)
{
+ gcc::pipeline passes = g-get_passes ();
struct cgraph_node *node;
switch (cgraph_state)
{
@@ -508,7 +511,7 @@ cgraph_add_new_function (tree fndecl, bool lowered)
push_cfun (DECL_STRUCT_FUNCTION (fndecl));
gimple_register_cfg_hooks ();
bitmap_obstack_initialize (NULL);
- execute_pass_list (all_lowering_passes);
+ execute_pass_list (passes.all_lowering_passes);
execute_pass_list (pass_early_local_passes.pass.sub);
bitmap_obstack_release (NULL);
pop_cfun ();
@@ -640,7 +643,7 @@ analyze_function (struct cgraph_node *node)
gimple_register_cfg_hooks ();
bitmap_obstack_initialize (NULL);
- execute_pass_list (all_lowering_passes);
+ execute_pass_list (g-get_passes ().all_lowering_passes);
free_dominance_info (CDI_POST_DOMINATORS);
free_dominance_info (CDI_DOMINATORS);
compact_blocks ();
@@ -1588,7 +1591,7 @@ expand_function (struct cgraph_node *node)
/* Signal the start of passes. */
invoke_plugin_callbacks (PLUGIN_ALL_PASSES_START, NULL);
- execute_pass_list (all_passes);
+
Re: C++ coding conventions: namespaces, references and getters (was Re: [PATCH 2/2] Introduce beginnings of a pipeline class.)
Hi, On Mon, Jul 29, 2013 at 09:02:53PM +0200, Oleg Endo wrote: On Mon, 2013-07-29 at 14:20 -0400, David Malcolm wrote: The same here and at a few other places. It may be just me not being used to references... nevertheless, if someone really wants to use them like this, at least make them const and you will save a night of frantic debugging to someone, probably to yourself. But I strongly prefer pointers... it's hard to describe how strongly I prefer them. OK. How do others feel? As I said above, I like the above idiom, since it puts the assertion of non-NULLness in a single place. I'm voting for references. References can be seen as yet another software structuring tool that instantly communicate some properties such as you mentioned above. In addition to that it's also a hint of ownership, i.e. if I get an object from somewhere I know that I better not even think about whether to delete it or not. well, let me stress again that we should think about this in the context of GCC. In GCC, we are used to C semantics of the dot operator and have a lot of existing code that we will continue to use and mix with new code with the same assumption. Putting a reference where none has been before might result in silent and hard to spot erroneous modifications. At the same time, we are used to lookup whether the pointer we got can be NULL or not if necessary. Moreover, in GCC, NULL-dereferences are not particularly dangerous and are easy to debug. In the context of GCC, there will be no ownership hints of this kind simply because new code will co-exist with tons of old code which uses pointers and will not give any such hints and so you can't assume you could free its pointers. Let me not even mention GC. Therefore, I'd avoid non const references where we can use pointers. Thanks, Martin
Re: [PATCH][AArch64] Fix FAIL: gcc.target/aarch64/cmn.c scan-assembler cmn\tw[0-9]
On 29/07/13 14:58, Kyrylo Tkachov wrote: Hi all, Now that combine emits the canonical form for (eq (neg x) (y)) instead of (eq (x) (neg y)), this patch fixes up the corresponding pattern in aarch64 to match that. This enables combine to properly generate the cmn instruction where appropriate. Tested aarch64-none-elf on model. Ok for trunk? No, this is wrong for inequalities, since in reality it's the second operand that's inverted. You'll need to use CC_SWP mode and then arrange for this to be correctly picked by select_cc_mode. R. Thanks, Kyrill 2013-07-29 Kyrylo Tkachov kyrylo.tkac...@arm.com * config/aarch64/aarch64.md (compare_negmode): Match canonical RTL form.= aarch64-cmn.patch N¬n‡r¥ªíÂ)emçhÂyhi×¢w^™©Ý
Re: [patch, mips] Size savings for MIPS16 switch statements
On Tue, 23 Jul 2013, Steve Ellcey wrote: While doing some space optimization work with mips16 I found that using a larger case threshold value could shrink the code. I did testing on some libraries like libpng and libjpeg as well as some test cases I wrote and came up with 10 as the best value for space savings in mips16 mode. I did some testing of mips32 code as well and found that this change did not help with that code so I restricted the change to mips16 only. Tested on mips-mti-elf target, OK for checkin? 2013-07-23 Steve Ellcey sell...@mips.com * config/mips/mips.c (mips_case_values_threshold): New. (TARGET_CASE_VALUES_THRESHOLD): Define. This change has caused regressions I believe, with the mips-linux-gnu target and the MIPS32/o32 multilib: FAIL: gcc.target/mips/code-readable-1.c -Os scan-assembler \tla\t FAIL: gcc.target/mips/code-readable-1.c -Os scan-assembler \t\\.half\t FAIL: gcc.target/mips/code-readable-2.c -Os scan-assembler \t\\.word\t[^\n]*L FAIL: gcc.target/mips/code-readable-3.c -Os scan-assembler %hi\\([^)]*L FAIL: gcc.target/mips/code-readable-3.c -Os scan-assembler %lo\\([^)]*L FAIL: gcc.target/mips/code-readable-4.c -Os scan-assembler \t\\.half\t FAIL: gcc.target/mips/code-readable-4.c -Os scan-assembler \tla\t -- it may be that the tests have to be disabled at -Os just like e.g. code-readable-1.c already is at -O0. Maciej
Re: [PATCH] Add atomic type qualifier
On Mon, 29 Jul 2013, Andrew MacLeod wrote: Ive been poking at this today, and Im wondering what you think of the idea of adding a flag to MODIFY_EXPR, #define MODIFY_EXPR_IS_COMPOUND(NODE) MODIFY_EXPR_CHECK(NODE)-base.asm_written_flag and set that in the MODIFY_EXPR node when we create it from the x op= y form in the front end. That flag seems to be free for expressions. My suggestion is that the IR generated by the front end should make it completely explicit what may need retrying with a compare-and-exchange, rather than relying on non-obvious details to reconstruct the semantics required at gimplification time - there are too many transformations (folding etc.) that may happen on existing trees and no clear way to be confident that you can still identify all the operands accurately after such transformations. That is, an ATOMIC_COMPOUND_MODIFY_EXPR or similar, whose operands are: the LHS of the assignment; a temporary variable, old in C11 footnote 113; the RHS; and the old op val expression complete with the conversion to the type of the LHS. Gimplification would then (carry out the effects of stabilize_reference on the LHS and save_expr on the RHS and) do old = LHS; followed by the do-while compare-exchange loop. A flag on the expression could indicate that the floating-point semantics are required. I'd guess back ends would need to provide three insn patterns, corresponding to feholdexcept, feclearexcept and feupdateenv, that there'd be corresponding built-in functions for these used at gimplification time, and that a target hook would give the type used for fenv_t by these built-in functions (*not* necessarily the same as the fenv_t used by any implementation of the functions in libm). The target should also be able to declare that there's no support for floating-point exceptions (e.g. for soft-float) and so floating-point cases don't need any special handling. -- Joseph S. Myers jos...@codesourcery.com
Fix ICE when profiles are mismatched
Hi, as noticed by Martin we ICE when value profiles get mismatch only in some values. This seems to be happening on Firefox 8 for weird reason where we optimize out sym == sym2 comparsion at profile-use but not at profile-generate time. We are still looking into that problem, but this patch fixes the obvoius ICE. Bootstrapped/regtested x86_64-linux, comitted. Honza Index: ChangeLog === --- ChangeLog (revision 201332) +++ ChangeLog (working copy) @@ -1,3 +1,9 @@ +2013-07-30 Jan Hubicka j...@suse.cz + Martin Liska marxin.li...@gmail.com + + * profile.c (compute_value_histograms): Do not ICE when + there is mismatch only on some counters. + 2013-07-30 Zhenqiang Chen zhenqiang.c...@linaro.org PR rtl-optimization/57637 Index: profile.c === --- profile.c (revision 201250) +++ profile.c (working copy) @@ -885,12 +885,16 @@ compute_value_histograms (histogram_valu t = (int) hist-type; aact_count = act_count[t]; - act_count[t] += hist-n_counters; + if (act_count[t]) +act_count[t] += hist-n_counters; gimple_add_histogram_value (cfun, stmt, hist); hist-hvalue.counters = XNEWVEC (gcov_type, hist-n_counters); for (j = 0; j hist-n_counters; j++) - hist-hvalue.counters[j] = aact_count[j]; +if (aact_count[t]) + hist-hvalue.counters[j] = aact_count[j]; + else + hist-hvalue.counters[j] = 0; } for (t = 0; t GCOV_N_VALUE_COUNTERS; t++)
Re: [Patch] Refractor Thompson matcher
On Mon, Jul 29, 2013 at 4:26 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Ah, excellent. As usual, let's wait a day or so for further comments and then please commit the whole thing. Commited. -- Tim Shen
Re: [PATCH] Enable non-complex math builtins from C99 for Bionic
Just to confirm that the patch successfully regtested on x86_64-unknown-linux-gnu. thanks, Alexander 2013/7/29 Alexander Ivchenko aivch...@gmail.com: 2013/7/28 Michael Eager ea...@eagerm.com: On 07/27/13 15:18, Alexander Ivchenko wrote: Hi Joseph, thanks for your comments. I updated the patch: 2013/7/9 Joseph S. Myers jos...@codesourcery.com: * It looks rather like microblaze*-*-* don't use elfos.h, so meaning semantics aren't preserved for those (non-Linux) targets either. Now, I don't know if there's a good reason for not using that file (ask the architecture maintainer), but in any case semantics should be preserved. I don't know why microblaze does not include elfos.h. It looks like it should, to be consistent with other targets. This would require some cleanup and verification. Your patch adds the following to microblaze.h, duplicating the change to elfos.h: +/* microblaze-unknown-elf target has no support of C99 runtime */ +#undef TARGET_LIBC_HAS_FUNCTION +#define TARGET_LIBC_HAS_FUNCTION no_c99_libc_has_function I'm assuming that this means that no other change to microblaze is needed and the question about elfos.h is moot. Yes, with this change in my patch the semantics for microblaze-unknown-elf is preserved. As for microblaze-unknown-linux-gnu case - the linux_android_libc_has_function version of TARGET_LIBC_HAS_FUNCTION from linux.h will be used, so the semantics is preserved as well. --Alexander
Re: [PATCH] Add atomic type qualifier
On 07/30/2013 07:41 AM, Joseph S. Myers wrote: On Mon, 29 Jul 2013, Andrew MacLeod wrote: Ive been poking at this today, and Im wondering what you think of the idea of adding a flag to MODIFY_EXPR, #define MODIFY_EXPR_IS_COMPOUND(NODE) MODIFY_EXPR_CHECK(NODE)-base.asm_written_flag and set that in the MODIFY_EXPR node when we create it from the x op= y form in the front end. That flag seems to be free for expressions. My suggestion is that the IR generated by the front end should make it completely explicit what may need retrying with a compare-and-exchange, rather than relying on non-obvious details to reconstruct the semantics required at gimplification time - there are too many transformations (folding etc.) that may happen on existing trees and no clear way to be confident that you can still identify all the operands accurately after such transformations. That is, an ATOMIC_COMPOUND_MODIFY_EXPR or similar, whose operands are: the LHS of the assignment; a temporary variable, old in C11 footnote 113; the RHS; and the old op val expression complete with the conversion to the type of the LHS. Gimplification would then (carry out the effects of stabilize_reference on the LHS and save_expr on the RHS and) do old = LHS; followed by the do-while compare-exchange loop. In fact, after thinking about it overnight, I came to similar conclusions... I believe it requires new builtin(s) for these operations. Something like __atomic_compound_assign (atomic_expr, enum atomic_operation_type, blah, blah,...) A call to this builtin would be generated right from the parser when it sees the op= expression, and the built-in can then travel throughout gimple as a normal atomic built-in operation like the rest. During expansion to RTL it can be turned into whatever sequence we happen to need. This is what happens currently with the various __atomic_fetch_op and __atomic_op_fetch. In fact, they are a subset of required operations, so I should be able to combine the implementation of those with this new one. Is C++ planning to match these behaviours in the atomic library? It would need to access this builtin as well so that the C++ template code can invoke it. A flag on the expression could indicate that the floating-point semantics are required. I'd guess back ends would need to provide three insn patterns, corresponding to feholdexcept, feclearexcept and feupdateenv, that there'd be corresponding built-in functions for these used at gimplification time, and that a target hook would give the type used for fenv_t by these built-in functions (*not* necessarily the same as the fenv_t used by any implementation of the functions in libm). The target should also be able to declare that there's no support for floating-point exceptions (e.g. for soft-float) and so floating-point cases don't need any special handling. I think the fact that it requires floating point sematics should be determinable from the types of the expressions involved. If there is a floating point somewhere, then we'll need to utilize the patterns. we'll still have the types, although it would certainly be easy enough to add a flag to the builtin... and maybe thats the way to go after all. THis also means that for the 3 floating point operations all we need are RTL insn patterns, no buitin. And as with the other atomics, if the pattern doesnt exist, we just wont emit it. we could add a warning easily enough in this case. I think we're somewhere good now :-) I guess I'll do the same thing for normal references to an atomic variable... issue the atomic load or atomic store directly from the parser... Andrew
[C++ Patch] PR 57947
Hi,
this issue, error recovery in C++98 mode, is a bit tricky: as far as I
can see, it boils down to the fact that in C++98 mode we don't want to
handle specially std::initializer_list. Otherwise, after the error
message we crash in convert_like_real where we try to handle ck_list, @
line 6056. With the below we emit the exact same diagnostic we emit for
init_list or any other non special name.
Tested x86_64-linux.
Thanks,
Paolo.
/cp
2013-07-30 Paolo Carlini paolo.carl...@oracle.com
PR c++/57947
* call.c (add_list_candidates): Only handle specially totype with
TYPE_HAS_LIST_CTOR (totype) set if cxx_dialect != cxx98.
/testsuite
2013-07-30 Paolo Carlini paolo.carl...@oracle.com
PR c++/57947
* g++.dg/parse/crash63.C: New.
Index: cp/call.c
===
--- cp/call.c (revision 201331)
+++ cp/call.c (working copy)
@@ -3370,7 +3370,8 @@ add_list_candidates (tree fns, tree first_arg,
;
/* If the class has a list ctor, try passing the list as a single
argument first, but only consider list ctors. */
- else if (TYPE_HAS_LIST_CTOR (totype))
+ else if (TYPE_HAS_LIST_CTOR (totype)
+ cxx_dialect != cxx98)
{
flags |= LOOKUP_LIST_ONLY;
args = make_tree_vector_single (init_list);
Index: testsuite/g++.dg/parse/crash63.C
===
--- testsuite/g++.dg/parse/crash63.C(revision 0)
+++ testsuite/g++.dg/parse/crash63.C(working copy)
@@ -0,0 +1,10 @@
+// PR c++/57947
+// { dg-options -std=c++98 }
+
+namespace std
+{
+ template class E class initializer_list {};
+ template int N struct D { D(initializer_listint) {} };
+ D0 d {1, 2, 3}; // { dg-error constructor|no matching }
+ // { dg-warning initializer list { target *-*-* } 8 }
+}
Re: [PATCH] Add atomic type qualifier
On Tue, 30 Jul 2013, Andrew MacLeod wrote: A flag on the expression could indicate that the floating-point semantics are required. I'd guess back ends would need to provide three insn patterns, corresponding to feholdexcept, feclearexcept and feupdateenv, that there'd be corresponding built-in functions for these used at gimplification time, and that a target hook would give the type used for fenv_t by these built-in functions (*not* necessarily the same as the fenv_t used by any implementation of the functions in libm). The target should also be able to declare that there's no support for floating-point exceptions (e.g. for soft-float) and so floating-point cases don't need any special handling. I think the fact that it requires floating point sematics should be determinable from the types of the expressions involved. If there is a My reasoning for such a flag is: The gimplifier shouldn't need to know the details of the semantics for operand promotions, how various arithmetic on complex numbers is carried out, and how a result is converted back to the destination type for the store. Even if it's the language-specific gimplification code rather than the language-independent gimplifier, we still want those details to be in one place (currently build_binary_op for the binary operation semantics), shared by atomic and non-atomic operations. Hence my suggestion that the operands to the new built-in function / operation do not include the unmodified RHS, and do not directly specify the operation in question. Instead, one operand is an expression of the form (convert-to-LHS-type) (old op val) where old and val are the temporary variables given in C11 footnote 113 and probably allocated by the front end (val initialized once, old initialized once but then potentially changing each time through the loop). The trees for that expression would be generated by build_binary_op and contain everything required for the semantics of e.g. complex arithmetic. It's true that in the case where floating-point issues arise, floating-point types will be involved somewhere in this expression (and otherwise, they will not be - though the initializer for val might itself have involved floating-point arithmetic), but you'd need to search recursively for them; having a flag seems simpler. THis also means that for the 3 floating point operations all we need are RTL insn patterns, no buitin. And as with the other atomics, if the pattern I think something will also be needed to specify allocation of the fenv_t temporary (whether in memory or registers). doesnt exist, we just wont emit it. we could add a warning easily enough in this case. Note there's a difference between no need to emit it, no warning should be given (soft float) and need to emit it but patterns not yet written so warning should be given (hard float but patterns not yet implemented for the architecture). -- Joseph S. Myers jos...@codesourcery.com
C++ PATCH for c++/58022 (bogus abstract class error)
Fallout from when I started checking for abstract classes as function
parameter types; if we see an incomplete type when we check for
abstractness, we save it to a list and check it again later when it's
complete. But we shouldn't do that in SFINAE context. This change is
already on the trunk, as part of a larger patch.
Tested x86_64-pc-linux-gnu, applying to 4.8.
commit 911dd30618250216d5eadca07b98a264d14f422f
Author: Jason Merrill ja...@redhat.com
Date: Mon Jul 29 16:52:53 2013 -0400
PR c++/58022
* typeck2.c (abstract_virtuals_error_sfinae): Don't remember
lookup in SFINAE context.
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 9b7db62..9c9f075 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -262,7 +262,7 @@ abstract_virtuals_error_sfinae (tree decl, tree type, tsubst_flags_t complain)
so that we can check again once it is completed. This makes sense
only for objects for which we have a declaration or at least a
name. */
- if (!COMPLETE_TYPE_P (type))
+ if (!COMPLETE_TYPE_P (type) (complain tf_error))
{
void **slot;
struct pending_abstract_type *pat;
diff --git a/gcc/testsuite/g++.dg/template/abstract1.C b/gcc/testsuite/g++.dg/template/abstract1.C
new file mode 100644
index 000..20bbf5a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/abstract1.C
@@ -0,0 +1,12 @@
+// PR c++/58022
+
+template class T struct A { };
+template class T AT operator (AT, T);
+template class T class foo;
+template class T Achar operator(Achar o, const fooT l);
+template class T class foo {
+friend Achar operator T (Achar o, const fooT l);
+};
+class bar;
+foobar fb;
+class bar { virtual void baz()=0; };
C++ PATCH for c++/57901 (bogus constexpr error)
We need to use break_out_target_exprs for unsharing the constexpr body,
because unshare_expr doesn't unshare TARGET_EXPRs.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.8.
commit 405d150804256f94e63101f35535098c9e74438b
Author: Jason Merrill ja...@redhat.com
Date: Mon Jul 29 16:12:56 2013 -0400
PR c++/57901
* semantics.c (build_data_member_initialization, constexpr_fn_retval):
Use break_out_target_exprs instead of unshare_expr.
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index f68d386..acdd178 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -6016,7 +6016,7 @@ build_data_member_initialization (tree t, vecconstructor_elt, va_gc **vec)
|| TREE_CODE (t) == MODIFY_EXPR)
{
member = TREE_OPERAND (t, 0);
- init = unshare_expr (TREE_OPERAND (t, 1));
+ init = break_out_target_exprs (TREE_OPERAND (t, 1));
}
else if (TREE_CODE (t) == CALL_EXPR)
{
@@ -6024,7 +6024,7 @@ build_data_member_initialization (tree t, vecconstructor_elt, va_gc **vec)
/* We don't use build_cplus_new here because it complains about
abstract bases. Leaving the call unwrapped means that it has the
wrong type, but cxx_eval_constant_expression doesn't care. */
- init = unshare_expr (t);
+ init = break_out_target_exprs (t);
}
else if (TREE_CODE (t) == DECL_EXPR)
/* Declaring a temporary, don't add it to the CONSTRUCTOR. */
@@ -6261,7 +6261,7 @@ constexpr_fn_retval (tree body)
}
case RETURN_EXPR:
- return unshare_expr (TREE_OPERAND (body, 0));
+ return break_out_target_exprs (TREE_OPERAND (body, 0));
case DECL_EXPR:
if (TREE_CODE (DECL_EXPR_DECL (body)) == USING_DECL)
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-value4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-value4.C
new file mode 100644
index 000..1fc3738
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-value4.C
@@ -0,0 +1,16 @@
+// PR c++/57901
+// { dg-require-effective-target c++11 }
+
+struct Z {
+Z() = default;
+Z(Z const) = default;
+constexpr Z(Z) {} /* non-trivial (constexpr) move ctor */
+};
+
+templatetypename T
+constexpr int fn0(T v) { return 0; }
+templatetypename T
+constexpr int fn (T v) { return fn0(v); }
+
+constexpr auto t0 = fn0(Z()); // OK!
+constexpr auto t = fn (Z()); // error! (GCC 4.8.1)
[PATCH, AArch64] Add secondary reload for immediates into FP_REGS
Our movdi_aarch64 pattern allows moving a constant into an FP_REG,
but has the constraint Dd, which is stricter than the one for
moving a constant into a CORE_REG. This is due to restricted values
allowed for MOVI instructions.
Due to the predicate for the pattern allowing any constant that is
valid for the CORE_REGs, we can run into situations where IRA/reload
has decided to use FP_REGs but the value is not actually valid for
MOVI.
This patch introduces a secondary reload to handle this case.
Supplied with testcase that highlighted original problem.
Tested on Linux GNU regressions.
OK for trunk?
Cheers,
Ian
2013-07-30 Ian Bolton ian.bol...@arm.com
gcc/
* config/aarch64/aarch64.c (aarch64_secondary_reload)): Handle
constant into FP_REGs that is not valid for MOVI.
testsuite/
* gcc.target/aarch64/movdi_1.c: New test.diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 9941d7c..f16988e 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -4070,6 +4070,15 @@ aarch64_secondary_reload (bool in_p ATTRIBUTE_UNUSED,
rtx x,
if (rclass == FP_REGS (mode == TImode || mode == TFmode) CONSTANT_P(x))
return CORE_REGS;
+ /* Only a subset of the DImode immediate values valid for CORE_REGS are
+ valid for FP_REGS. Where we have an immediate value that isn't valid
+ for FP_REGS, and RCLASS is FP_REGS, we return CORE_REGS to cause the
+ value to be generated into there first and later copied to FP_REGS to be
+ used. */
+ if (rclass == FP_REGS mode == DImode CONST_INT_P (x)
+ !aarch64_simd_imm_scalar_p (x, GET_MODE (x)))
+return CORE_REGS;
+
return NO_REGS;
}
diff --git a/gcc/testsuite/gcc.target/aarch64/movdi_1.c
b/gcc/testsuite/gcc.target/aarch64/movdi_1.c
new file mode 100644
index 000..1decd99
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/movdi_1.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -fno-inline } */
+
+#include arm_neon.h
+
+void
+foo (uint64_t *a)
+{
+ uint64x1_t val18;
+ uint32x2_t val19;
+ uint64x1_t val20;
+ val19 = vcreate_u32 (0x80004cf3dffbUL);
+ val20 = vrsra_n_u64 (val18, vreinterpret_u64_u32 (val19), 34);
+ vst1_u64 (a, val20);
+}
Re: [C++ Patch] PR 57947
How about just returning false from is_std_init_list in C++98 mode? Jason
[SPARC] Remove superfluous memory barrier for atomics with TSO
If you compile the following C++ code at -O for Linux or Solaris:
int exchange (int *loc, int val)
{
return __atomic_exchange_4 (loc, val, __ATOMIC_SEQ_CST);
}
you get in the assembly file:
_Z8exchangePii:
.LLFB0:
mov %o0, %g1
membar 2
mov %o1, %o0
swap[%g1], %o0
jmp %o7+8
nop
membar 2 is membar #StoreLoad. Now Linux and Solaris default to TSO, which
means that the swap is effectively preceded by membar #StoreStore; moreover
swap is atomic and both a load and a store, so this preceding membar is also a
membar #StoreLoad; in the end, the generated membar is superfluous.
This is confirmed by the last sentence below from the V9 manual:
8.4.4.3 Total Store Order (TSO)
[...]
The rules for TSO are:
— Loads are blocking and ordered with respect to earlier loads.
— Stores are ordered with respect to stores.
— Atomic load-stores are ordered with respect to loads and stores.
The attached patchlet implements the missing bits to eliminate the membar, very
similarly to what Richard did for loads and the PSO model. Since it can only
affect the 3 hardware atomic primitives, I'd like to apply it on all active
branches. Any objections?
2013-07-30 Eric Botcazou ebotca...@adacore.com
* config/sparc/sparc.c (sparc_emit_membar_for_model) SMM_TSO: Add
the implied StoreLoad barrier for atomic operations if before.
--
Eric Botcazou
Index: config/sparc/sparc.c
===
--- config/sparc/sparc.c (revision 201177)
+++ config/sparc/sparc.c (working copy)
@@ -11318,6 +11319,11 @@ sparc_emit_membar_for_model (enum memmod
/* Total Store Ordering: all memory transactions with store semantics
are followed by an implied StoreStore. */
implied |= StoreStore;
+
+ /* If we're not looking for a raw barrer (before+after), then atomic
+ operations get the benefit of being both load and store. */
+ if (load_store == 3 before_after == 1)
+ implied |= StoreLoad;
/* FALLTHRU */
case SMM_PSO:
Re: [PATCH] Add atomic type qualifier
On 07/30/2013 09:16 AM, Joseph S. Myers wrot My reasoning for such a flag is: Hence my suggestion that the operands to the new built-in function / operation do not include the unmodified RHS, and do not directly specify the operation in question. Instead, one operand is an expression of the form (convert-to-LHS-type) (old op val) where old and val are the temporary variables given in C11 footnote 113 and probably allocated by the front end (val initialized once, old initialized once but then potentially changing each time through the loop). The trees for that expression would be generated by build_binary_op and contain everything required for the semantics of e.g. complex arithmetic. It's true that in the case where floating-point issues arise, floating-point types will be involved somewhere in this expression (and otherwise, they will not be - though the initializer for val might itself have involved floating-point arithmetic), but you'd need to search recursively for them; having a flag seems simpler. I agree. THis also means that for the 3 floating point operations all we need are RTL insn patterns, no buitin. And as with the other atomics, if the pattern I think something will also be needed to specify allocation of the fenv_t temporary (whether in memory or registers). doesnt exist, we just wont emit it. we could add a warning easily enough in this case. Note there's a difference between no need to emit it, no warning should be given (soft float) and need to emit it but patterns not yet written so warning should be given (hard float but patterns not yet implemented for the architecture). In fact, the flag could be the presence of the fenv_t variable.. Null for that variable field in the builtin indicate you don't need the patterns emitted. I';ll get back to you in a bit with the actual built-in's format once I poke around the existing one and see how I can leverage it. I rewrote all that code last year and it ought to be pretty simple to add new operand support. It better be anyway :-) Amndrew
Symtab cleanups 8/17: abstract functions
Hi,
symtab nodes are (ab)used to prevent removal of debug info from functions that
appear as
abstract origins of other functions. I am not sure if this is the best way to
do so,
but the code is fundamentaly broken on few places and it seems to make sense to
get it working
before thinking what would be really proper representation (for LTO the fact
that debug
info has nodes comes handy)
This is first part of the patch that makes us to handle abstract function
correctly
in the symtab.
Bootstrapped/regtested x86_64-linux, will commit it shortly.
Honza
* lto-symtab.c (lto_symtab_merge_symbols): Merge duplicated nodes for
abstract functions.
* cgraph.h (symtab_real_symbol_p): Abstract declarations are not real
symbols.
Index: lto-symtab.c
===
--- lto-symtab.c(revision 201250)
+++ lto-symtab.c(working copy)
@@ -599,6 +599,13 @@ lto_symtab_merge_symbols (void)
(cnode2 = cgraph_get_node (node-symbol.decl))
cnode2 != cnode)
lto_cgraph_replace_node (cnode2, cnode);
+
+ /* Abstract functions may have duplicated cgraph nodes attached;
+remove them. */
+ else if (cnode DECL_ABSTRACT (cnode-symbol.decl)
+ (cnode2 = cgraph_get_node (node-symbol.decl))
+ cnode2 != cnode)
+ cgraph_remove_node (cnode2);
symtab_insert_node_to_hashtable ((symtab_node)node);
}
}
Index: cgraph.h
===
--- cgraph.h(revision 201250)
+++ cgraph.h(working copy)
@@ -1347,13 +1347,13 @@ symtab_real_symbol_p (symtab_node node)
{
struct cgraph_node *cnode;
+ if (DECL_ABSTRACT (node-symbol.decl))
+return false;
if (!is_a cgraph_node (node))
return true;
cnode = cgraph (node);
if (cnode-global.inlined_to)
return false;
- if (cnode-abstract_and_needed)
-return false;
return true;
}
#endif /* GCC_CGRAPH_H */
[ubsan] Add -static-libubsan
This adds missing -static-libubsan option. It's already handled in
the gcc.c via various macros.
Tested x86_64-pc-linux-gnu, applying to ubsan branch.
diff --git a/gcc/ChangeLog.ubsan b/gcc/ChangeLog.ubsan
index 3d15c19..47f81b4 100644
--- a/gcc/ChangeLog.ubsan
+++ b/gcc/ChangeLog.ubsan
@@ -1,3 +1,8 @@
+2013-07-29 Marek Polacek pola...@redhat.com
+
+ * common.opt (static-libubsan): New option.
+ * doc/invoke.texi: Document -static-libubsan.
+
2013-07-24 Marek Polacek pola...@redhat.com
* ubsan.c (struct ubsan_typedesc): Improve comment.
diff --git a/gcc/common.opt b/gcc/common.opt
index 123d593..fe6bc9c 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2580,6 +2580,9 @@ Driver
static-libtsan
Driver
+static-libubsan
+Driver
+
symbolic
Driver
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 3596c6c..5dd9a62 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -452,7 +452,7 @@ Objective-C and Objective-C++ Dialects}.
@gccoptlist{@var{object-file-name} -l@var{library} @gol
-nostartfiles -nodefaultlibs -nostdlib -pie -rdynamic @gol
-s -static -static-libgcc -static-libstdc++ @gol
--static-libasan -static-libtsan @gol
+-static-libasan -static-libtsan -static-libubsan @gol
-shared -shared-libgcc -symbolic @gol
-T @var{script} -Wl,@var{option} -Xlinker @var{option} @gol
-u @var{symbol}}
@@ -10102,6 +10102,15 @@ option is not used, then this links against the shared
version of
driver to link @file{libtsan} statically, without necessarily linking
other libraries statically.
+@item -static-libubsan
+When the @option{-fsanitize=undefined} option is used to link a program,
+the GCC driver automatically links against @option{libubsan}. If
+@file{libubsan} is available as a shared library, and the @option{-static}
+option is not used, then this links against the shared version of
+@file{libubsan}. The @option{-static-libubsan} option directs the GCC
+driver to link @file{libubsan} statically, without necessarily linking
+other libraries statically.
+
@item -static-libstdc++
When the @command{g++} program is used to link a C++ program, it
normally automatically links against @option{libstdc++}. If
Marek
[ubsan] Don't try to sanitize shifts outside of functions
This is something I've stumbled upon when trying to bootstrap with
-fsanitize=undefined (doesn't work so far...). It's pretty serious
stuff, but clang doesn't handle it either... We errored on e.g.
enum e { x = 1 1 };
because of the instrumentation. Fixed by doing the instrumentation only
when current_function_decl != 0. Perhaps this should be revised later on.
Tested x86_64-pc-linux-gnu, applying to ubsan branch.
diff --git a/gcc/c/ChangeLog.ubsan b/gcc/c/ChangeLog.ubsan
index 58e931f..11d167f 100644
--- a/gcc/c/ChangeLog.ubsan
+++ b/gcc/c/ChangeLog.ubsan
@@ -1,3 +1,8 @@
+2013-07-30 Marek Polacek pola...@redhat.com
+
+ * c-typeck.c (build_binary_op): Sanitize only when
+ current_function_decl is not zero.
+
2013-07-21 Marek Polacek pola...@redhat.com
* c-typeck.c (build_binary_op): Call c_fully_fold on both
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index d0f4b0d..7257166 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -10488,7 +10488,8 @@ build_binary_op (location_t location, enum tree_code
code,
return error_mark_node;
}
- if (flag_sanitize SANITIZE_UNDEFINED)
+ if (flag_sanitize SANITIZE_UNDEFINED
+ current_function_decl != 0)
{
/* OP0 and/or OP1 might have side-effects. */
op0 = c_save_expr (op0);
diff --git a/gcc/cp/ChangeLog.ubsan b/gcc/cp/ChangeLog.ubsan
index 0ab2870..f37ce94 100644
--- a/gcc/cp/ChangeLog.ubsan
+++ b/gcc/cp/ChangeLog.ubsan
@@ -1,3 +1,8 @@
+2013-07-30 Marek Polacek pola...@redhat.com
+
+ * typeck.c (cp_build_binary_op): Sanitize only when
+ current_function_decl is not zero.
+
2013-07-05 Marek Polacek pola...@redhat.com
* typeck.c (cp_build_binary_op): Add division by zero and shift
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 7790830..ebe27e8 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4885,6 +4885,7 @@ cp_build_binary_op (location_t location,
if ((flag_sanitize SANITIZE_UNDEFINED)
!processing_template_decl
+ current_function_decl != 0
(doing_div_or_mod || doing_shift))
{
/* OP0 and/or OP1 might have side-effects. */
diff --git a/gcc/testsuite/ChangeLog.ubsan b/gcc/testsuite/ChangeLog.ubsan
index 453bd61..f3f18f9 100644
--- a/gcc/testsuite/ChangeLog.ubsan
+++ b/gcc/testsuite/ChangeLog.ubsan
@@ -1,3 +1,7 @@
+2013-07-30 Marek Polacek pola...@redhat.com
+
+ * c-c++-common/ubsan/const-expr.c: New test.
+
2013-07-22 Marek Polacek pola...@redhat.com
* c-c++-common/ubsan/div-by-zero-3.c: Add more testing.
diff --git a/gcc/testsuite/c-c++-common/ubsan/const-expr-1.c
b/gcc/testsuite/c-c++-common/ubsan/const-expr-1.c
new file mode 100644
index 000..f474ec6
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/const-expr-1.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options -fsanitize=shift -w } */
+
+enum e { A = 1 1, B, };
+const int arr[] = {
+ 1 2,
+ 1 3,
+};
+
+int
+bar (int a, int b)
+{
+ return a b;
+}
+
+int
+foo (void)
+{
+ int i = 1;
+ int vla[B 3];
+ return bar (A, (i = 6, i + 2));
+}
Marek
[ubsan] Add bootstrap-ubsan.mk
This adds the bootstrap-ubsan.mk file so that --with-build-config=bootstrap-ubsan be possible. I doesn't work yet, though :(. Tested x86_64-pc-linux-gnu, applying to ubsan branch. diff --git a/config/ChangeLog.ubsan b/config/ChangeLog.ubsan new file mode 100644 index 000..f7a2125 --- /dev/null +++ b/config/ChangeLog.ubsan @@ -0,0 +1,3 @@ +2013-07-30 Marek Polacek pola...@redhat.com + + * bootstrap-ubsan.mk: New. diff --git a/config/bootstrap-ubsan.mk b/config/bootstrap-ubsan.mk new file mode 100644 index 000..10543f6 --- /dev/null +++ b/config/bootstrap-ubsan.mk @@ -0,0 +1,7 @@ +# This option enables -fsanitize=undefined for stage2 and stage3. + +STAGE2_CFLAGS += -fsanitize=undefined +STAGE3_CFLAGS += -fsanitize=undefined +POSTSTAGE1_LDFLAGS += -fsanitize=undefined -static-libubsan \ + -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/ubsan/ \ + -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/ubsan/.libs Marek
[ubsan] Use build_constructor_va where possible
This is only a cleanup; build_constructor_va is much more convenient
than build_constructor when we know the number of elements in
a constructor.
Tested x86_64-pc-linux-gnu, applying to ubsan branch.
diff --git a/gcc/ChangeLog.ubsan b/gcc/ChangeLog.ubsan
index 47f81b4..311a15c 100644
--- a/gcc/ChangeLog.ubsan
+++ b/gcc/ChangeLog.ubsan
@@ -1,3 +1,9 @@
+2013-07-30 Marek Polacek pola...@redhat.com
+
+ * ubsan.c (ubsan_source_location): Use build_constructor_va
+ instead of build_constructor.
+ (ubsan_type_descriptor): Likewise.
+
2013-07-29 Marek Polacek pola...@redhat.com
* common.opt (static-libubsan): New option.
diff --git a/gcc/ubsan.c b/gcc/ubsan.c
index 0bd1b96..1996225 100644
--- a/gcc/ubsan.c
+++ b/gcc/ubsan.c
@@ -254,13 +254,10 @@ ubsan_source_location (location_t loc)
{
expanded_location xloc;
tree type = ubsan_source_location_type ();
- vecconstructor_elt, va_gc *v;
xloc = expand_location (loc);
/* Fill in the values from LOC. */
- vec_alloc (v, 3);
- tree ctor = build_constructor (type, v);
size_t len = strlen (xloc.file);
tree str = build_string (len + 1, xloc.file);
TREE_TYPE (str) = build_array_type (char_type_node,
@@ -268,11 +265,11 @@ ubsan_source_location (location_t loc)
TREE_READONLY (str) = 1;
TREE_STATIC (str) = 1;
str = build_fold_addr_expr_loc (loc, str);
- CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, str);
- CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, build_int_cst (unsigned_type_node,
- xloc.line));
- CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, build_int_cst (unsigned_type_node,
- xloc.column));
+ tree ctor = build_constructor_va (type, 3, NULL_TREE, str, NULL_TREE,
+ build_int_cst (unsigned_type_node,
+ xloc.line), NULL_TREE,
+ build_int_cst (unsigned_type_node,
+ xloc.column));
TREE_CONSTANT (ctor) = 1;
TREE_STATIC (ctor) = 1;
@@ -312,7 +309,6 @@ ubsan_type_descriptor (tree type)
return (*slot)-decl;
tree dtype = ubsan_type_descriptor_type ();
- vecconstructor_elt, va_gc *v;
const char *tname;
unsigned short tkind, tinfo;
@@ -346,20 +342,17 @@ ubsan_type_descriptor (tree type)
DECL_IGNORED_P (decl) = 1;
DECL_EXTERNAL (decl) = 0;
- vec_alloc (v, 3);
- tree ctor = build_constructor (dtype, v);
size_t len = strlen (tname);
tree str = build_string (len + 1, tname);
TREE_TYPE (str) = build_array_type (char_type_node,
build_index_type (size_int (len)));
TREE_READONLY (str) = 1;
TREE_STATIC (str) = 1;
- CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, build_int_cst
(short_unsigned_type_node,
- tkind));
- CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, build_int_cst
(short_unsigned_type_node,
- tinfo));
- CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, str);
-
+ tree ctor = build_constructor_va (dtype, 3, NULL_TREE,
+ build_int_cst (short_unsigned_type_node,
+ tkind), NULL_TREE,
+ build_int_cst (short_unsigned_type_node,
+ tinfo), NULL_TREE, str);
TREE_CONSTANT (ctor) = 1;
TREE_STATIC (ctor) = 1;
DECL_INITIAL (decl) = ctor;
Marek
Re: [C++ Patch] PR 57947
Hi,
On 07/30/2013 03:30 PM, Jason Merrill wrote:
How about just returning false from is_std_init_list in C++98 mode?
Sure. It works great.
Thanks,
Paolo.
/cp
2013-07-30 Paolo Carlini paolo.carl...@oracle.com
PR c++/57947
* call.c (is_std_init_list): Return false if cxx_dialect == cxx98.
/testsuite
2013-07-30 Paolo Carlini paolo.carl...@oracle.com
PR c++/57947
* g++.dg/parse/crash63.C: New.
Index: cp/call.c
===
--- cp/call.c (revision 201331)
+++ cp/call.c (working copy)
@@ -9396,6 +9396,8 @@ is_std_init_list (tree type)
/* Look through typedefs. */
if (!TYPE_P (type))
return false;
+ if (cxx_dialect == cxx98)
+return false;
type = TYPE_MAIN_VARIANT (type);
return (CLASS_TYPE_P (type)
CP_TYPE_CONTEXT (type) == std_node
Index: testsuite/g++.dg/parse/crash63.C
===
--- testsuite/g++.dg/parse/crash63.C(revision 0)
+++ testsuite/g++.dg/parse/crash63.C(working copy)
@@ -0,0 +1,10 @@
+// PR c++/57947
+// { dg-options -std=c++98 }
+
+namespace std
+{
+ template class E class initializer_list {};
+ template int N struct D { D(initializer_listint) {} };
+ D0 d {1, 2, 3}; // { dg-error constructor|no matching }
+ // { dg-warning initializer list { target *-*-* } 8 }
+}
RE: [PATCH] Add a new option -ftree-bitfield-merge (patch / doc inside)
Thank you for the reply. I am in the process of modifying the patch according to some comments received. Currently I am considering the usage of DECL_BIT_FIELD_REPRESENTATIVE. I see that they can be used during analysis phase for deciding which accesses can be merged - only accesses with same representative will be merged. I have more dilemmas with usage of representatives for lowering. If my understanding is correct bit field representative can only be associated to a field declaration, and not to a BIT_FIELD_REF node. As a consequence optimization must use COMPONENT_REF to model new bit field access (which should be an equivalent of several merged accesses). To use COMPONENT_REF a new field declaration with appropriate bit size (equal to sum of bit sizes of all merged bit field accesses) must be created and then corresponding bit field representative could be attached. Is my understanding correct? Is creating a new field declaration for every set of merged bit field accesses acceptable? Regards, Zoran Jovanovic From: Richard Biener [richard.guent...@gmail.com] Sent: Thursday, July 18, 2013 11:31 AM To: Zoran Jovanovic; gcc-patches@gcc.gnu.org Cc: Petar Jovanovic Subject: Re: [PATCH] Add a new option -ftree-bitfield-merge (patch / doc inside) Zoran Jovanovic zoran.jovano...@imgtec.com wrote: Hello, This patch adds new optimization pass that combines several adjacent bit field accesses that copy values from one memory location to another into single bit field access. Example: Original code: unnamed-unsigned:3 D.1351; unnamed-unsigned:9 D.1350; unnamed-unsigned:7 D.1349; D.1349_2 = p1_1(D)-f1; p2_3(D)-f1 = D.1349_2; D.1350_4 = p1_1(D)-f2; p2_3(D)-f2 = D.1350_4; D.1351_5 = p1_1(D)-f3; p2_3(D)-f3 = D.1351_5; Optimized code: unnamed-unsigned:19 D.1358; D.1358_10 = BIT_FIELD_REF *p1_1(D), 19, 13; BIT_FIELD_REF *p2_3(D), 19, 13 = D.1358_10; Algorithm works on basic block level and consists of following 3 major steps: 1. Go trough basic block statements list. If there are statement pairs that implement copy of bit field content from one memory location to another record statements pointers and other necessary data in corresponding data structure. 2. Identify records that represent adjacent bit field accesses and mark them as merged. 3. Modify trees accordingly. All this should use BITFIELD_REPRESENTATIVE both to decide what accesses are related and for the lowering. This makes sure to honor the appropriate memory models. In theory only lowering is necessary and FRE and DSE will do the job of optimizing - also properly accounting for alias issues that Joseph mentions. The lowering and analysis is strongly related to SRA So I don't believe we want a new pass for this. Richard.
[ubsan] Rename obsolete variable
Apparently I forgot to check rs6000.h when doing the flag_asan - flag_sanitize change. This broke bootstrap on ppc. Tested powerpc64-unknown-linux-gnu, applying to ubsan branch. diff --git a/gcc/ChangeLog.ubsan b/gcc/ChangeLog.ubsan index 311a15c..ac584ff 100644 --- a/gcc/ChangeLog.ubsan +++ b/gcc/ChangeLog.ubsan @@ -1,5 +1,10 @@ 2013-07-30 Marek Polacek pola...@redhat.com + * config/rs6000/rs6000.h (FRAME_GROWS_DOWNWARD): Use flag_sanitize + instead of flag_asan. + +2013-07-30 Marek Polacek pola...@redhat.com + * ubsan.c (ubsan_source_location): Use build_constructor_va instead of build_constructor. (ubsan_type_descriptor): Likewise. diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index e5a6abd..f89b20d 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -1498,7 +1498,8 @@ extern enum reg_class rs6000_constraints[RS6000_CONSTRAINT_MAX]; On the RS/6000, we grow upwards, from the area after the outgoing arguments. */ -#define FRAME_GROWS_DOWNWARD (flag_stack_protect != 0 || flag_asan != 0) +#define FRAME_GROWS_DOWNWARD (flag_stack_protect != 0 \ + || (flag_sanitize SANITIZE_ADDRESS) != 0) /* Size of the outgoing register save area */ #define RS6000_REG_SAVE ((DEFAULT_ABI == ABI_AIX \ Marek
[patch, ARM] Add mulhi3 expander.
When we have the DSP multiply extension (armv5e and up for ARM or T2),
then a 16-bit multiply is best done with smulbb rather than mul. This
saves us doing redundant extends and may even be a faster instruction on
some cores. Unfortunately, expand does not try this operation by
default; but it's trivial to add to the back-end. Since the value
produced is extended, we represent this by expanding into the widening
mul version and then subreg-ing the result.
* arm.md (mulhi3): New expand pattern.
R.--- gcc/config/arm/arm.md (revision 201327)
+++ gcc/config/arm/arm.md (local)
@@ -1725,6 +1725,20 @@ (define_expand subdf3
;; Multiplication insns
+(define_expand mulhi3
+ [(set (match_operand:HI 0 s_register_operand )
+ (mult:HI (match_operand:HI 1 s_register_operand )
+(match_operand:HI 2 s_register_operand )))]
+ TARGET_DSP_MULTIPLY
+
+ {
+rtx result = gen_reg_rtx (SImode);
+emit_insn (gen_mulhisi3 (result, operands[1], operands[2]));
+emit_move_insn (operands[0], gen_lowpart (HImode, result));
+DONE;
+ }
+)
+
(define_expand mulsi3
[(set (match_operand:SI 0 s_register_operand )
(mult:SI (match_operand:SI 2 s_register_operand )
[patch, ARM] require 64-bit hw-int for all arm targets
Most arm target configs now require a 64-bit HW-int. Unfortunately a
few of the older, less commonly used config targets do not. The code in
arm.c now pretty much requires that a 64-bit HW-int is used, especially
for vectorization to work.
This patch makes 64-bit HW-int the default for all arm configs and
cleans up the configure script accordingly.
* config.gcc (arm): Require 64-bit host-wide-int for all ARM
target configs.
R.--- gcc/config.gcc (revision 201327)
+++ gcc/config.gcc (local)
@@ -330,6 +330,7 @@ arm*-*-*)
target_type_format_char='%'
c_target_objs=arm-c.o
cxx_target_objs=arm-c.o
+ need_64bit_hwint=yes
extra_options=${extra_options} arm/arm-tables.opt
;;
avr-*-*)
@@ -943,10 +944,6 @@ arm*-*-linux-*)# ARM GNU/Linux with E
tmake_file=$tmake_file arm/t-linux-androideabi
;;
esac
- # The BPABI long long divmod functions return a 128-bit value in
- # registers r0-r3. Correctly modeling that requires the use of
- # TImode.
- need_64bit_hwint=yes
# The EABI requires the use of __cxa_atexit.
default_use_cxa_atexit=yes
with_tls=${with_tls:-gnu}
@@ -955,10 +952,6 @@ arm*-*-uclinux*eabi*) # ARM ucLinux
tm_file=dbxelf.h elfos.h arm/unknown-elf.h arm/elf.h arm/linux-gas.h
arm/uclinux-elf.h glibc-stdint.h
tmake_file=arm/t-arm arm/t-arm-elf arm/t-bpabi
tm_file=$tm_file arm/bpabi.h arm/uclinux-eabi.h arm/aout.h
vxworks-dummy.h arm/arm.h
- # The BPABI long long divmod functions return a 128-bit value in
- # registers r0-r3. Correctly modeling that requires the use of
- # TImode.
- need_64bit_hwint=yes
# The EABI requires the use of __cxa_atexit.
default_use_cxa_atexit=yes
;;
@@ -967,10 +960,6 @@ arm*-*-eabi* | arm*-*-symbianelf* | arm*
arm*eb-*-eabi*)
tm_defines=${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1
esac
- # The BPABI long long divmod functions return a 128-bit value in
- # registers r0-r3. Correctly modeling that requires the use of
- # TImode.
- need_64bit_hwint=yes
default_use_cxa_atexit=yes
tm_file=dbxelf.h elfos.h arm/unknown-elf.h arm/elf.h arm/bpabi.h
tmake_file=arm/t-arm arm/t-arm-elf
Re: [PATCH v2 06/18] convert the C++ front end to automatic dependencies
On Mon, Jul 29, 2013 at 11:24 AM, Tom Tromey tro...@redhat.com wrote: This converts the C++ front end. This renames g++spec.o to cp/g++spec.o for uniformity. This lets us remove an explicit rule. This patch does not remove various *_H macros from cp/Make-lang.in. These are still needed by ObjC++. They're removed by a later patch. * Make-lang.in (g++spec.o): Remove. (CFLAGS-cp/g++spec.o): New variable. (GXX_OBJS): Reference cp/g++spec.o. (cc1plus-checksum.o, cp/lex.o, cp/cp-array-notation.o) (cp/cp-lang.o, cp/decl.o, cp/decl2.o, cp/cp-objcp-common.o) (cp/typeck2.o, cp/typeck.o, cp/class.o, cp/call.o) (cp/friend.o, cp/init.o, cp/method.o, cp/cvt.o, cp/search.o) (cp/tree.o, cp/ptree.o, cp/rtti.o, cp/except.o, cp/expr.o) (cp/pt.o, cp/error.o, cp/repo.o, cp/semantics.o, cp/dump.o) (cp/optimize.o, cp/mangle.o, cp/parser.o, cp/cp-gimplify.o) (cp/name-lookup.o, cp/cxx-pretty-print.o): Remove. --- gcc/cp/Make-lang.in | 95 ++--- 1 file changed, 2 insertions(+), 93 deletions(-) diff --git a/gcc/cp/Make-lang.in b/gcc/cp/Make-lang.in index da8e87b..43b7003 100644 --- a/gcc/cp/Make-lang.in +++ b/gcc/cp/Make-lang.in @@ -51,13 +51,10 @@ c++: cc1plus$(exeext) # Tell GNU make to ignore these if they exist. .PHONY: c++ -g++spec.o: $(srcdir)/cp/g++spec.c $(SYSTEM_H) coretypes.h $(TM_H) $(GCC_H) \ -$(CONFIG_H) $(OPTS_H) - $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(DRIVER_DEFINES) \ - $(INCLUDES) $(srcdir)/cp/g++spec.c +CFLAGS-cp/g++spec.o += $(DRIVER_DEFINES) # Create the compiler driver for g++. -GXX_OBJS = $(GCC_OBJS) g++spec.o +GXX_OBJS = $(GCC_OBJS) cp/g++spec.o xg++$(exeext): $(GXX_OBJS) $(EXTRA_GCC_OBJS) libcommon-target.a $(LIBDEPS) +$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \ $(GXX_OBJS) $(EXTRA_GCC_OBJS) libcommon-target.a \ @@ -96,8 +93,6 @@ cc1plus-checksum.c : build/genchecksum$(build_exeext) checksum-options \ checksum-options cc1plus-checksum.c.tmp \ $(srcdir)/../move-if-change cc1plus-checksum.c.tmp cc1plus-checksum.c -cc1plus-checksum.o : cc1plus-checksum.c $(CONFIG_H) $(SYSTEM_H) - cc1plus$(exeext): $(CXX_OBJS) cc1plus-checksum.o $(BACKEND) $(LIBDEPS) +$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \ $(CXX_OBJS) cc1plus-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS) @@ -261,89 +256,3 @@ CXX_TREE_H = $(TREE_H) cp/name-lookup.h cp/cp-tree.h $(C_COMMON_H) \ $(srcdir)/../include/hashtab.h CXX_PARSER_H = tree.h $(CXX_TREE_H) c-family/c-pragma.h cp/parser.h CXX_PRETTY_PRINT_H = cp/cxx-pretty-print.h $(C_PRETTY_PRINT_H) - -cp/lex.o: cp/lex.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) \ - $(C_PRAGMA_H) input.h cp/operators.def $(TM_P_H) \ - c-family/c-objc.h -cp/cp-array-notation.o: cp/cp-array-notation.c $(CONFIG_H) $(SYSTEM_H) \ - coretypes.h $(TREE_H) $(CXX_TREE_H) $(DIAGNOSTIC_H) tree-iterator.h vec.h \ - $(GIMPLE_H) c-family/array-notation-common.o $(C_COMMON_H) -cp/cp-lang.o: cp/cp-lang.c $(CXX_TREE_H) $(TM_H) debug.h langhooks.h \ - $(LANGHOOKS_DEF_H) $(C_COMMON_H) gtype-cp.h gt-cp-cp-lang.h \ - cp/cp-objcp-common.h $(EXPR_H) $(TARGET_H) $(CXX_PARSER_H) -cp/decl.o: cp/decl.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) cp/decl.h \ - output.h toplev.h $(HASHTAB_H) $(RTL_H) \ - cp/operators.def $(TM_P_H) $(TREE_INLINE_H) $(DIAGNOSTIC_H) $(C_PRAGMA_H) \ - debug.h gt-cp-decl.h $(TIMEVAR_H) $(TARGET_H) $(PLUGIN_H) \ - intl.h tree-iterator.h pointer-set.h $(SPLAY_TREE_H) \ - c-family/c-objc.h -cp/decl2.o: cp/decl2.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) cp/decl.h \ - toplev.h $(C_COMMON_H) gt-cp-decl2.h $(CGRAPH_H) \ - $(C_PRAGMA_H) dumpfile.h intl.h $(TARGET_H) $(GIMPLE_H) pointer-set.h \ - $(SPLAY_TREE_H) c-family/c-ada-spec.h \ - c-family/c-objc.h -cp/cp-objcp-common.o : cp/cp-objcp-common.c $(CONFIG_H) $(SYSTEM_H) \ - coretypes.h $(TM_H) $(TREE_H) $(CXX_TREE_H) $(C_COMMON_H) \ - langhooks.h $(LANGHOOKS_DEF_H) $(DIAGNOSTIC_H) debug.h \ - $(CXX_PRETTY_PRINT_H) cp/cp-objcp-common.h gt-cp-cp-objcp-common.h -cp/typeck2.o: cp/typeck2.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) \ - $(TM_P_H) $(DIAGNOSTIC_CORE_H) gt-cp-typeck2.h $(REAL_H) intl.h -cp/typeck.o: cp/typeck.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) $(PARAMS_H) \ - toplev.h $(DIAGNOSTIC_H) convert.h $(C_COMMON_H) $(TARGET_H) \ - c-family/c-objc.h -cp/class.o: cp/class.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) toplev.h \ - $(TARGET_H) convert.h $(CGRAPH_H) dumpfile.h gt-cp-class.h \ - $(SPLAY_TREE_H) pointer-set.h $(HASH_TABLE_H) -cp/call.o: cp/call.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) toplev.h \ - $(DIAGNOSTIC_CORE_H) intl.h gt-cp-call.h convert.h $(TARGET_H) langhooks.h \ - $(TIMEVAR_H) c-family/c-objc.h -cp/friend.o: cp/friend.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H) -cp/init.o: cp/init.c $(CXX_TREE_H) $(TM_H) $(FLAGS_H)
Re: [C++ Patch] PR 57947
OK.
[PATCH 0/5] Atomic type qualifier
On 07/26/2013 01:15 PM, Andrew MacLeod wrote: This patch adds an atomic type qualifier to GCC. It can be accessed via __attribute__((atomic)) or in C11 mode via the _Atomic keyword. What it does: * All the __atomic builtins now expect an atomic qualified value for the atomic variable. Non-atomic values can still be passed in, but they are not guaranteed to work if the original type is not compatible with the atomic type No fears. At the moment, every target's atomic types line up with the unsigned type of the same size, so like magic, its all good for existing code. * There is a new target hook atomic_align_for_mode which a target can use to override the default alignment when the atomic variation requires something different. There may be other attributes eventually, but for now alignment is the only thing supported. I considered size, but the effort to do that is what drove me to the re-architecture project :-P At least the mechanism is now in place for overrides when atomic requirements aren't just the default it use to be.(I introduced atomicQI_type_node, atomicHI_type_node, atomicSI_type_node, atomicDI_type_node, atomicTI_type_node, ). I tested this by aligning all shorts to 32 byte boundaries and bootstrapping/running all the tests. * I went through the front ends trying to treat atomic qualifier mostly like a volatile, since thats is the basic behaviour one expects. In the backend, it sets the TREE_IS_VOLATILE bit so behaviour ought to be the same as before, plus they are/will be always used in __atomic_built-in functions. * I changed the libstdc++ atomic implementation so that all the atomic classes use __attribute__((atomic)) on their data members, ensuring that if anyone overrides the atomic type, it should work fine with C++ atomics. It also served as a good test that I had the type set up properly... getting TYPE_CANONICAL() correct throughout the C++ compiler was, well..., lets just say painful. * I changed 2 of the atomic test sets, one uses __attribute__((atomic)) to ensure the attribute compiles ok, and the other uses _Atomic and --std=c11 to ensure that compiles OK. What it doesn't do: * It doesn't implement the C11 expression expansion into atomic built-ins. ie, you can't write: _Atomic int x; x = 0; and have the result be an atomic operation calling __atomic_store (x, 0). That will be in a follow on patch. So none of the expression expansion from C11 is included yet. This just enables that. * It doesn't do a full set of checks when _Atomic is used in invalid ways. I don't know the standard nor the front end well enough.. Im hoping someone else will pitch in with that eventually, assuming someone cares enough :-) There are a couple of errors issued, but that is it. This bootstraps on x86_64, and passes all the testsuites with no new regressions. I didnt split it up any more than this because most of it is inter-related... plus this is already split out from a much larger set of changes :-) I did try to at least organize the ordering, all the long boring stuff is at the end :-) Both front end and back end stuff in here. I think I caught it all, but have a look. There is time to find any remaining issues I may have missed. Andrew I split the original patch into some smaller hunks, and cleaned up a few bit and pieces here and there... following:
Re: [patch, mips] Size savings for MIPS16 switch statements
On Tue, 2013-07-30 at 11:18 +0100, Maciej W. Rozycki wrote: -- it may be that the tests have to be disabled at -Os just like e.g. code-readable-1.c already is at -O0. Maciej Sorry about that, not sure why I didn't notice the failures. Rather then skipping the tests for -Os I was thinking it might be better to increase the size of the switch statements. Here is a patch I have tested to fix these failures. Richard, does this look OK for checkin? Steve Ellcey sell...@mips.com 2013-07-30 Steve Ellcey sell...@mips.com * gcc.target/mips/code-readable-1.c: Increase switch size. * gcc.target/mips/code-readable-2.c: Ditto. * gcc.target/mips/code-readable-3.c: Ditto. * gcc.target/mips/code-readable-4.c: Ditto. diff --git a/gcc/testsuite/gcc.target/mips/code-readable-1.c b/gcc/testsuite/gcc.target/mips/code-readable-1.c index 34c2c0a..b3e864d 100644 --- a/gcc/testsuite/gcc.target/mips/code-readable-1.c +++ b/gcc/testsuite/gcc.target/mips/code-readable-1.c @@ -8,6 +8,10 @@ volatile int x4; volatile int x5; volatile int x6; volatile int x7; +volatile int x8; +volatile int x9; +volatile int x10; +volatile int x11; MIPS16 int foo (int i, volatile *x) @@ -21,6 +25,10 @@ foo (int i, volatile *x) case 5: return x5 + x[4]; case 6: return x6 + x[5]; case 7: return x7 + x[6]; +case 8: return x8 + x[7]; +case 9: return x9 + x[8]; +case 10: return x10 + x[9]; +case 11: return x11 + x[10]; default: return 0; } } diff --git a/gcc/testsuite/gcc.target/mips/code-readable-2.c b/gcc/testsuite/gcc.target/mips/code-readable-2.c index 71aeb13..3d32504 100644 --- a/gcc/testsuite/gcc.target/mips/code-readable-2.c +++ b/gcc/testsuite/gcc.target/mips/code-readable-2.c @@ -7,6 +7,10 @@ volatile int x4; volatile int x5; volatile int x6; volatile int x7; +volatile int x8; +volatile int x9; +volatile int x10; +volatile int x11; MIPS16 int foo (int i, volatile *x) @@ -20,6 +24,10 @@ foo (int i, volatile *x) case 5: return x5 + x[4]; case 6: return x6 + x[5]; case 7: return x7 + x[6]; +case 8: return x8 + x[7]; +case 9: return x9 + x[8]; +case 10: return x10 + x[9]; +case 11: return x11 + x[10]; default: return 0; } } diff --git a/gcc/testsuite/gcc.target/mips/code-readable-3.c b/gcc/testsuite/gcc.target/mips/code-readable-3.c index fc78505..aaf1874 100644 --- a/gcc/testsuite/gcc.target/mips/code-readable-3.c +++ b/gcc/testsuite/gcc.target/mips/code-readable-3.c @@ -7,6 +7,10 @@ volatile int x4; volatile int x5; volatile int x6; volatile int x7; +volatile int x8; +volatile int x9; +volatile int x10; +volatile int x11; MIPS16 int foo (int i, volatile *x) @@ -20,6 +24,10 @@ foo (int i, volatile *x) case 5: return x5 + x[4]; case 6: return x6 + x[5]; case 7: return x7 + x[6]; +case 8: return x8 + x[7]; +case 9: return x9 + x[8]; +case 10: return x10 + x[9]; +case 11: return x11 + x[10]; default: return 0; } } diff --git a/gcc/testsuite/gcc.target/mips/code-readable-4.c b/gcc/testsuite/gcc.target/mips/code-readable-4.c index ae8ff8a..4db89f8 100644 --- a/gcc/testsuite/gcc.target/mips/code-readable-4.c +++ b/gcc/testsuite/gcc.target/mips/code-readable-4.c @@ -8,6 +8,10 @@ volatile int x4; volatile int x5; volatile int x6; volatile int x7; +volatile int x8; +volatile int x9; +volatile int x10; +volatile int x11; MIPS16 int foo (int i, volatile *x) @@ -21,6 +25,10 @@ foo (int i, volatile *x) case 5: return x5 + x[4]; case 6: return x6 + x[5]; case 7: return x7 + x[6]; +case 8: return x8 + x[7]; +case 9: return x9 + x[8]; +case 10: return x10 + x[9]; +case 11: return x11 + x[10]; default: return 0; } }
Re: [ubsan] Add bootstrap-ubsan.mk
On Tue, Jul 30, 2013 at 04:34:14PM +0200, Marek Polacek wrote:
This adds the bootstrap-ubsan.mk file so that
--with-build-config=bootstrap-ubsan be possible. I doesn't work yet,
though :(.
One of the reasons is that we use -Werror and e.g. on the following
testcase
static int x;
void
foo (void)
{
int o = 1;
x = x o;
}
with -O -fsanitize=undefined we generate:
c.c:6:9: warning: ‘x.2’ is used uninitialized in this function [-Wuninitialized]
x = x o;
while with -O -fno-sanitize=undefined there isn't such warning. When
sanitizing,
in .uninit1 we have
int x.3;
int x.2;
bb 2:
x.3_3 = x.2_1(D) 1;
x = x.3_3;
and when no sanitizing
int x.1;
int x.0;
bb 2:
x.0_2 = x;
x.1_3 = x.0_2 1;
x = x.1_3;
The warning comes from warn_uninitialized_vars. The warning here seems
incorrect, but I'm afraid there isn't much to do about this on the
ubsan side of things.
Marek
Re: [Patch] Refractor Thompson matcher
On 07/30/2013 02:07 PM, Tim Shen wrote: On Mon, Jul 29, 2013 at 4:26 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Ah, excellent. As usual, let's wait a day or so for further comments and then please commit the whole thing. Commited. The bootstrap is currently broken due to this commit. I'm quickly moving inline a few functions in the *non* template _BFSMatcher which were out of line. When you consider the design stable enough please take care of exporting from the *.so the large non-template functions. Thanks, Paolo.
[PATCH 2/5] Atomic type qualifier - Add atomic type to tree node
This patch adds an atomic qualifier to the type node. it sets up
atomic{QHSDT}I_type_node types upon startup and used the alignment
target hook to override alignment if need be.
Whenever new atomic types are created, they will inherit this alignment
setting if they would otherwise be aligned to a lower boundry.
The middle end is also taught to treat decls that are atomic much the
way we handle volatile... ie, don't really try to optimize this thing.
Adnrew
* tree.h (struct tree_base): Add atomic_flag field.
(TYPE_ATOMIC): New accessor macro.
(enum cv_qualifier): Add TYPE_QUAL_ATOMIC.
(TYPE_QUALS, TYPE_QUALS_NO_ADDR_SPACE): Add TYPE_QUAL_ATOMIC.
(TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC): New macro.
(enum tree_index): Add TI_ATOMIC{QHSDT}I_TYPE.
(atomic{QHSDT}I_type_node): Add new type nodes.
* emit-rtl.c (set_mem_attributes_minus_bitpos): Atomics are volatile.
* function.c (assign_stack_temp_for_type): Atomics are volatile.
* print-tree.c (print_node): Print atomic qualifier.
* tree-pretty-print.c (dump_generic_node): Print atomic type attribute.
* tree.c (set_type_quals): Set TYPE_ATOMIC.
(find_atomic_core_type): New. Function to get atomic type from size.
(build_qualified_type): Tweak for atomic qualifier overrides.
(build_atomic_variant): New. Build atomic variant node.
(build_common_tree_nodes): Build atomic{QHSDT}I_type_node, allowing
for override with target hook.
* alias.c (objects_must_conflict_p): Treat atomics as volatile.
* calls.c (expand_call): Treat atomics as volatile.
Index: gcc/tree.h
===
*** gcc/tree.h (revision 201248)
--- gcc/tree.h (working copy)
*** struct GTY(()) tree_base {
*** 457,463
unsigned packed_flag : 1;
unsigned user_align : 1;
unsigned nameless_flag : 1;
! unsigned spare0 : 4;
unsigned spare1 : 8;
--- 457,464
unsigned packed_flag : 1;
unsigned user_align : 1;
unsigned nameless_flag : 1;
! unsigned atomic_flag : 1;
! unsigned spare0 : 3;
unsigned spare1 : 8;
*** extern enum machine_mode vector_type_mod
*** 2205,2210
--- 2206,2214
/* Nonzero in a type considered volatile as a whole. */
#define TYPE_VOLATILE(NODE) (TYPE_CHECK (NODE)-base.volatile_flag)
+ /* Nonzero in a type considered atomic as a whole. */
+ #define TYPE_ATOMIC(NODE) (TYPE_CHECK (NODE)-base.u.bits.atomic_flag)
+
/* Means this type is const-qualified. */
#define TYPE_READONLY(NODE) (TYPE_CHECK (NODE)-base.readonly_flag)
*** enum cv_qualifier
*** 2226,2232
TYPE_UNQUALIFIED = 0x0,
TYPE_QUAL_CONST= 0x1,
TYPE_QUAL_VOLATILE = 0x2,
! TYPE_QUAL_RESTRICT = 0x4
};
/* Encode/decode the named memory support as part of the qualifier. If more
--- 2230,2237
TYPE_UNQUALIFIED = 0x0,
TYPE_QUAL_CONST= 0x1,
TYPE_QUAL_VOLATILE = 0x2,
! TYPE_QUAL_RESTRICT = 0x4,
! TYPE_QUAL_ATOMIC = 0x8
};
/* Encode/decode the named memory support as part of the qualifier. If more
*** enum cv_qualifier
*** 2245,2250
--- 2250,2256
#define TYPE_QUALS(NODE) \
((int) ((TYPE_READONLY (NODE) * TYPE_QUAL_CONST) \
| (TYPE_VOLATILE (NODE) * TYPE_QUAL_VOLATILE) \
+ | (TYPE_ATOMIC (NODE) * TYPE_QUAL_ATOMIC) \
| (TYPE_RESTRICT (NODE) * TYPE_QUAL_RESTRICT) \
| (ENCODE_QUAL_ADDR_SPACE (TYPE_ADDR_SPACE (NODE)
*** enum cv_qualifier
*** 2252,2259
--- 2258,2273
#define TYPE_QUALS_NO_ADDR_SPACE(NODE)\
((int) ((TYPE_READONLY (NODE) * TYPE_QUAL_CONST) \
| (TYPE_VOLATILE (NODE) * TYPE_QUAL_VOLATILE) \
+ | (TYPE_ATOMIC (NODE) * TYPE_QUAL_ATOMIC) \
+ | (TYPE_RESTRICT (NODE) * TYPE_QUAL_RESTRICT)))
+ /* The same as TYPE_QUALS without the address space and atomic
+qualifications. */
+ #define TYPE_QUALS_NO_ADDR_SPACE_NO_ATOMIC(NODE) \
+ ((int) ((TYPE_READONLY (NODE) * TYPE_QUAL_CONST) \
+ | (TYPE_VOLATILE (NODE) * TYPE_QUAL_VOLATILE) \
| (TYPE_RESTRICT (NODE) * TYPE_QUAL_RESTRICT)))
+
/* These flags are available for each language front end to use internally. */
#define TYPE_LANG_FLAG_0(NODE) (TYPE_CHECK (NODE)-type_common.lang_flag_0)
#define TYPE_LANG_FLAG_1(NODE) (TYPE_CHECK (NODE)-type_common.lang_flag_1)
*** enum tree_index
*** 4178,4183
--- 4192,4203
TI_UINTDI_TYPE,
TI_UINTTI_TYPE,
+ TI_ATOMICQI_TYPE,
+ TI_ATOMICHI_TYPE,
+ TI_ATOMICSI_TYPE,
+ TI_ATOMICDI_TYPE,
+ TI_ATOMICTI_TYPE,
+
TI_UINT16_TYPE,
TI_UINT32_TYPE,
TI_UINT64_TYPE,
*** extern GTY(()) tree global_trees[TI_MAX]
*** 4334,4339
--- 4354,4365
#define unsigned_intDI_type_node global_trees[TI_UINTDI_TYPE]
#define unsigned_intTI_type_node global_trees[TI_UINTTI_TYPE]
+ #define atomicQI_type_node global_trees[TI_ATOMICQI_TYPE]
+ #define
[PATCH 3/5] Atomic type qualifier - front end changes
Teach the front ends to handle __attribute__((atomic)) and set the
ATOMIC_TYPE bit in the type tree.
Also add RID_ATOMIC and recognize the _Atomic keyword, and treat it like
__attribute__((atomic)) was specified.
Places that set TREE_THIS_VOLATILE and TREE_SIDE_EFFECTS for volatile
symbols also need to set these fields for atomic variables. All through
the middle end we have always treated atomics as volatile in order to
avoid any kind of optimization.
c-family
* c-common.h (enum rid): Add RID_ATOMIC.
* c-common.c (struct c_common_resword c_common_r): Add _Atomic.
(struct attribute_spec c_common_att): Add atomic attribute.
(handle_atomic_attribute): New. Add atomic qualifier to type.
(sync_resolve_params): Use MAIN_VARIANT as cast for the non-atomic
parameters.
(keyword_is_type_qualifier): Add RID_ATOMIC;
* c-format.c (check_format_types): Add atomic as a qualifer check.
* c-pretty-print.c (pp_c_cv_qualifiers): Handle atomic qualifier.
c
* c-tree.h (struct c_declspecs): Add atomic_p field.
* c-aux-info.c (gen_type): Handle atomic qualifier.
* c-decl.c (shadow_tag_warned): Add atomic_p to declspecs check.
(quals_from_declspecs): Add atomic_p to declspecs check.
(grokdeclarator): Check atomic and warn of duplicate or errors.
(build_null_declspecs): Handle atomic_p.
(declspecs_add_qual): Handle RID_ATOMIC.
* c-parser.c (c_token_starts_typename): Add RID_ATOMIC.
(c_token_is_qualifier, c_token_starts_declspecs): Add RID_ATOMIC.
(c_parser_declspecs, c_parser_attribute_any_word): Add RID_ATOMIC.
* c-typeck.c (build_indirect_ref): Treat atomic as volatile.
(build_array_ref, convert_for_assignment): Treat atomic as volatile.
objc
* objc-act.c (objc_push_parm): Treat atomic as volatile.
cp
* cp-tree.h (CP_TYPE_ATOMIC_P): New macro.
(enum cp_decl_spec): Add ds_atomic.
* class.c (build_simple_base_path): Treat atomic as volatile.
* cvt.c (diagnose_ref_binding): Handle atomic.
(convert_from_reference, convert_to_void): Treat atomic as volatile.
* decl.c (grokfndecl): Treat atomic as volatile.
(build_ptrmemfunc_type): Set TYPE_ATOMIC.
(grokdeclarator): handle atomic qualifier.
* mangle.c (dump_substitution_candidates): Add atomic to the qualifier
list.
* parser.c (cp_parser_type_specifier): Handle RID_ATOMIC.
(cp_parser_cv_qualifier_seq_opt): Handle RID_ATOMIC.
(set_and_check_decl_spec_loc): Add atomic to decl_spec_names[].
* pt.c (check_cv_quals_for_unify): Add TYPE_QUAL_ATOMIC to check.
* rtti.c (qualifier_flags): Set atomic qualifier flag.
* semantics.c (non_const_var_error): Check CP_TYPE_ATOMIC_P too.
* tree.c (cp_build_qualified_type_real): Add TYPE_QUAL_ATOMIC.
(cv_unqualified): Add TYPE_QUAL_ATOMIC to mask.
* typeck.c (build_class_member_access_expr): Treat atomic as volatile.
(cp_build_indirect_ref, cp_build_array_ref): Treat atomic as volatile.
(check_return_expr, cp_type_quals): Treat atomic as volatile.
(cv_qualified_p): Add TYPE_QUAL_ATOMIC to mask.
Index: gcc/c-family/c-common.h
===
*** gcc/c-family/c-common.h (revision 201248)
--- gcc/c-family/c-common.h (working copy)
*** enum rid
*** 66,72
RID_UNSIGNED, RID_LONG,RID_CONST, RID_EXTERN,
RID_REGISTER, RID_TYPEDEF, RID_SHORT, RID_INLINE,
RID_VOLATILE, RID_SIGNED, RID_AUTO, RID_RESTRICT,
! RID_NORETURN,
/* C extensions */
RID_COMPLEX, RID_THREAD, RID_SAT,
--- 66,72
RID_UNSIGNED, RID_LONG,RID_CONST, RID_EXTERN,
RID_REGISTER, RID_TYPEDEF, RID_SHORT, RID_INLINE,
RID_VOLATILE, RID_SIGNED, RID_AUTO, RID_RESTRICT,
! RID_NORETURN, RID_ATOMIC,
/* C extensions */
RID_COMPLEX, RID_THREAD, RID_SAT,
Index: gcc/c-family/c-common.c
===
*** gcc/c-family/c-common.c (revision 201248)
--- gcc/c-family/c-common.c (working copy)
*** static tree handle_unused_attribute (tre
*** 325,330
--- 325,331
static tree handle_externally_visible_attribute (tree *, tree, tree, int,
bool *);
static tree handle_const_attribute (tree *, tree, tree, int, bool *);
+ static tree handle_atomic_attribute (tree *, tree, tree, int, bool *);
static tree handle_transparent_union_attribute (tree *, tree, tree,
int, bool *);
static tree handle_constructor_attribute (tree *, tree, tree, int, bool *);
*** const struct c_common_resword c_common_r
*** 401,406
--- 402,408
{
{ _Alignas, RID_ALIGNAS, D_CONLY },
{ _Alignof, RID_ALIGNOF, D_CONLY },
+ { _Atomic, RID_ATOMIC,D_CONLY },
{ _Bool, RID_BOOL, D_CONLY },
{ _Complex, RID_COMPLEX, 0 },
{ _Imaginary, RID_IMAGINARY, D_CONLY },
*** const struct attribute_spec c_common_att
*** 637,642
--- 639,646
/* The same comments as for noreturn attributes apply to const ones. */
{ const, 0, 0, true, false, false,
Re: [Patch] Refractor Thompson matcher
.. no, too many changes, please simply revert the commit ASAP and next time please test more carefully before posting. Thanks, Paolo.
[PATCH 4/5] Atomic type qualifier - Change built-in function types
This patch changes all the __Atomic built-in functions to take a pointer
to an atomic volatile object instead of just a volatile object.
This will allow the atomic operations to work with any requested
alignment change for a target.
I am not adding support for the __sync built-ins to expect the atomic
qualifier as they are in deprecation mode.
fortran
* types.def (BT_ATOMIC_PTR, BT_CONST_ATOMIC_PTR): New
primitive data types for volatile atomic pointers.
(BT_FN_VOID_APTR): Renamed from BT_FN_VOID_VPTR.
(BT_FN_VOID_VPTR_INT, BT_FN_BOOL_VPTR_INT,
BT_FN_BOOL_SIZE_CONST_VPTR): Renamed to APTR variant.
(BT_FN_I{1,2,4,8,16}_CONST_APTR_INT): New.
(BT_FN_I{1,2,4,8,16}_APTR_I{1,2,4,8,16}_INT): New.
(BT_FN_VOID_APTR_I{1,2,4,8,16}_INT): New.
(BT_FN_VOID_SIZE_VPTR_PTR_INT, BT_FN_VOID_SIZE_CONST_VPTR_PTR_INT,
BT_FN_VOID_SIZE_VPTR_PTR_PTR_INT,
BT_FN_BOOL_VPTR_PTR_I{1,2,4,8,16}_BOOL_INT_INT): Renamed to APTR
variant.
gcc
* builtin-types.def (BT_ATOMIC_PTR, BT_CONST_ATOMIC_PTR): New
primitive data types for volatile atomic pointers.
(BT_FN_VOID_APTR): Renamed from BT_FN_VOID_VPTR.
(BT_FN_VOID_VPTR_INT, BT_FN_BOOL_VPTR_INT,
BT_FN_BOOL_SIZE_CONST_VPTR): Renamed to APTR variant.
(BT_FN_I{1,2,4,8,16}_CONST_APTR_INT): New.
(BT_FN_I{1,2,4,8,16}_APTR_I{1,2,4,8,16}_INT): New.
(BT_FN_VOID_APTR_I{1,2,4,8,16}_INT): New.
(BT_FN_VOID_SIZE_VPTR_PTR_INT, BT_FN_VOID_SIZE_CONST_VPTR_PTR_INT,
BT_FN_VOID_SIZE_VPTR_PTR_PTR_INT,
BT_FN_BOOL_VPTR_PTR_I{1,2,4,8,16}_BOOL_INT_INT): Renamed to APTR
variant.
* sync-builtins.def: Change all __atomic builtins to use the APTR
atomic pointer variant for the first parameter instead of VPTR..
doc
* extend.texi: Add atomic pointer note to __atomic built-ins.
Index: gcc/fortran/types.def
===
*** gcc/fortran/types.def (revision 201248)
--- gcc/fortran/types.def (working copy)
*** DEF_PRIMITIVE_TYPE (BT_CONST_VOLATILE_PT
*** 74,79
--- 74,87
build_pointer_type
(build_qualified_type (void_type_node,
TYPE_QUAL_VOLATILE|TYPE_QUAL_CONST)))
+ DEF_PRIMITIVE_TYPE (BT_ATOMIC_PTR,
+ build_pointer_type
+ (build_qualified_type (void_type_node,
+ TYPE_QUAL_VOLATILE|TYPE_QUAL_ATOMIC)))
+ DEF_PRIMITIVE_TYPE (BT_CONST_ATOMIC_PTR,
+ build_pointer_type
+ (build_qualified_type (void_type_node,
+ TYPE_QUAL_VOLATILE|TYPE_QUAL_CONST|TYPE_QUAL_ATOMIC)))
DEF_POINTER_TYPE (BT_PTR_LONG, BT_LONG)
DEF_POINTER_TYPE (BT_PTR_ULONGLONG, BT_ULONGLONG)
DEF_POINTER_TYPE (BT_PTR_PTR, BT_PTR)
*** DEF_FUNCTION_TYPE_2 (BT_FN_I8_CONST_VPTR
*** 113,122
BT_INT)
DEF_FUNCTION_TYPE_2 (BT_FN_I16_CONST_VPTR_INT, BT_I16, BT_CONST_VOLATILE_PTR,
BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_VOID_VPTR_INT, BT_VOID, BT_VOLATILE_PTR, BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_BOOL_VPTR_INT, BT_BOOL, BT_VOLATILE_PTR, BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_BOOL_SIZE_CONST_VPTR, BT_BOOL, BT_SIZE,
! BT_CONST_VOLATILE_PTR)
DEF_POINTER_TYPE (BT_PTR_FN_VOID_PTR_PTR, BT_FN_VOID_PTR_PTR)
--- 121,140
BT_INT)
DEF_FUNCTION_TYPE_2 (BT_FN_I16_CONST_VPTR_INT, BT_I16, BT_CONST_VOLATILE_PTR,
BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_I1_CONST_APTR_INT, BT_I1, BT_CONST_ATOMIC_PTR,
! BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_I2_CONST_APTR_INT, BT_I2, BT_CONST_ATOMIC_PTR,
! BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_I4_CONST_APTR_INT, BT_I4, BT_CONST_ATOMIC_PTR,
! BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_I8_CONST_APTR_INT, BT_I8, BT_CONST_ATOMIC_PTR,
! BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_I16_CONST_APTR_INT, BT_I16, BT_CONST_ATOMIC_PTR,
! BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_VOID_APTR_INT, BT_VOID, BT_ATOMIC_PTR, BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_BOOL_APTR_INT, BT_BOOL, BT_ATOMIC_PTR, BT_INT)
! DEF_FUNCTION_TYPE_2 (BT_FN_BOOL_SIZE_CONST_APTR, BT_BOOL, BT_SIZE,
! BT_CONST_ATOMIC_PTR)
DEF_POINTER_TYPE (BT_PTR_FN_VOID_PTR_PTR, BT_FN_VOID_PTR_PTR)
*** DEF_FUNCTION_TYPE_3 (BT_FN_I2_VPTR_I2_IN
*** 144,169
DEF_FUNCTION_TYPE_3 (BT_FN_I4_VPTR_I4_INT, BT_I4, BT_VOLATILE_PTR, BT_I4, BT_INT)
DEF_FUNCTION_TYPE_3 (BT_FN_I8_VPTR_I8_INT, BT_I8, BT_VOLATILE_PTR, BT_I8, BT_INT)
DEF_FUNCTION_TYPE_3 (BT_FN_I16_VPTR_I16_INT, BT_I16, BT_VOLATILE_PTR, BT_I16, BT_INT)
DEF_FUNCTION_TYPE_3 (BT_FN_VOID_VPTR_I1_INT, BT_VOID, BT_VOLATILE_PTR, BT_I1, BT_INT)
DEF_FUNCTION_TYPE_3 (BT_FN_VOID_VPTR_I2_INT, BT_VOID, BT_VOLATILE_PTR, BT_I2, BT_INT)
DEF_FUNCTION_TYPE_3 (BT_FN_VOID_VPTR_I4_INT, BT_VOID, BT_VOLATILE_PTR, BT_I4, BT_INT)
DEF_FUNCTION_TYPE_3 (BT_FN_VOID_VPTR_I8_INT, BT_VOID, BT_VOLATILE_PTR, BT_I8, BT_INT)
DEF_FUNCTION_TYPE_3 (BT_FN_VOID_VPTR_I16_INT, BT_VOID, BT_VOLATILE_PTR, BT_I16, BT_INT)
[PATCH 1/5] Atomic type qualifier - Add type alignment override hook
This patch adds a target hook which a target can use to override the
alignment of the lock-free atomic type for a given mode.
Andrew
* hooks.c (hook_uint_mode_0): Return 0 unit hook.
* hooks.h (hook_uint_mode_0): Prototype.
* target.def (atomic_align_for_mode): define hook.
* tm.texi (TARGET_ATOMIC_TYPE_FOR_MODE): Define.
* doc/tm.texi.in (TARGET_ATOMIC_TYPE_FOR_MODE): Add.
Index: gcc/hooks.c
===
*** gcc/hooks.c (revision 201248)
--- gcc/hooks.c (working copy)
*** hook_rtx_tree_int_null (tree a ATTRIBUTE
*** 352,357
--- 352,364
return NULL;
}
+ /* Generic hook that takes a machine mode and returns an unsigned int 0. */
+ unsigned int
+ hook_uint_mode_0 (enum machine_mode m ATTRIBUTE_UNUSED)
+ {
+ return 0;
+ }
+
/* Generic hook that takes three trees and returns the last one as is. */
tree
hook_tree_tree_tree_tree_3rd_identity (tree a ATTRIBUTE_UNUSED,
Index: gcc/hooks.h
===
*** gcc/hooks.h (revision 201248)
--- gcc/hooks.h (working copy)
*** extern tree hook_tree_tree_tree_tree_3rd
*** 89,94
--- 89,95
extern tree hook_tree_tree_int_treep_bool_null (tree, int, tree *, bool);
extern unsigned hook_uint_void_0 (void);
+ extern unsigned int hook_uint_mode_0 (enum machine_mode);
extern bool default_can_output_mi_thunk_no_vcall (const_tree, HOST_WIDE_INT,
HOST_WIDE_INT, const_tree);
Index: gcc/target.def
===
*** gcc/target.def (revision 201248)
--- gcc/target.def (working copy)
*** DEFHOOKPOD
*** 5116,5122
@code{atomic_test_and_set} is not exactly 1, i.e. the\
@code{bool} @code{true}.,
unsigned char, 1)
!
/* Leave the boolean fields at the end. */
/* True if we can create zeroed data by switching to a BSS section
--- 5116,5134
@code{atomic_test_and_set} is not exactly 1, i.e. the\
@code{bool} @code{true}.,
unsigned char, 1)
!
! /* Return an unsigned int representing the alignment (in bits) of the atomic
!type which maps to machine MODE. This allows alignment to be overridden
!as needed. */
! DEFHOOK
! (atomic_align_for_mode,
! If defined, this function returns an appropriate alignment in bits for an\
! atomic object of machine_mode @var{mode}. If 0 is returned then the\
! default alignment for the specified mode is used. ,
! unsigned int, (enum machine_mode mode),
! hook_uint_mode_0)
!
!
/* Leave the boolean fields at the end. */
/* True if we can create zeroed data by switching to a BSS section
Index: gcc/doc/tm.texi
===
*** gcc/doc/tm.texi (revision 201248)
--- gcc/doc/tm.texi (working copy)
*** It returns true if the target supports G
*** 11375,11377
--- 11375,11381
The support includes the assembler, linker and dynamic linker.
The default value of this hook is based on target's libc.
@end deftypefn
+
+ @deftypefn {Target Hook} {unsigned int} TARGET_ATOMIC_ALIGN_FOR_MODE (enum machine_mode @var{mode})
+ If defined, this function returns an appropriate alignment in bits for an atomic object of machine_mode @var{mode}. If 0 is returned then the default alignment for the specified mode is used.
+ @end deftypefn
Index: gcc/doc/tm.texi.in
===
*** gcc/doc/tm.texi.in (revision 201248)
--- gcc/doc/tm.texi.in (working copy)
*** and the associated definitions of those
*** 8415,8417
--- 8415,8419
@hook TARGET_ATOMIC_TEST_AND_SET_TRUEVAL
@hook TARGET_HAS_IFUNC_P
+
+ @hook TARGET_ATOMIC_ALIGN_FOR_MODE
[PATCH 5/5] Atomic type qualifier - Use atomic objects
This patch turns on the atomic qualifier for all C++ atomic objects
It also modifies a few testcases to ensure __attribute__((atomic)) and
_Atomic are recognized by the compiler and compile as expected.
libstdc++-v3
* include/bits/atomic_base.h (struct __atomic_base): Add
__attribute__((atomic) to member data element.
(struct __atomic_base_PTp*): Add __attribute__((atomic)) to member
data element.
* include/std/atomic (struct atomic): Add __attribute__((atomic)) to
member data element.
testsuite
* gcc.dg/atomic-exchange-{1-5}.c: Change atomic var to use
__attribute__((atomic)).
* gcc.dg/atomic-op-{1-5}.c: Add --std=c11 and change atomic var to
use _Atomic keyword.
Index: libstdc++-v3/include/bits/atomic_base.h
===
*** libstdc++-v3/include/bits/atomic_base.h (revision 201248)
--- libstdc++-v3/include/bits/atomic_base.h (working copy)
*** _GLIBCXX_BEGIN_NAMESPACE_VERSION
*** 346,361
// atomic_char32_t char32_t
// atomic_wchar_t wchar_t
//
! // NB: Assuming _ITp is an integral scalar type that is 1, 2, 4, or
! // 8 bytes, since that is what GCC built-in functions for atomic
// memory access expect.
templatetypename _ITp
struct __atomic_base
{
private:
! typedef _ITp __int_type;
! __int_type _M_i;
public:
__atomic_base() noexcept = default;
--- 346,362
// atomic_char32_t char32_t
// atomic_wchar_t wchar_t
//
! // NB: Assuming _ITp is an integral scalar type that is 1, 2, 4, 8, or
! // 16 bytes, since that is what GCC built-in functions for atomic
// memory access expect.
templatetypename _ITp
struct __atomic_base
{
private:
! typedef _ITp __int_type;
! typedef _ITp __attribute__ ((atomic)) __atomic_int_type;
! __atomic_int_type _M_i;
public:
__atomic_base() noexcept = default;
*** _GLIBCXX_BEGIN_NAMESPACE_VERSION
*** 669,677
struct __atomic_base_PTp*
{
private:
! typedef _PTp* __pointer_type;
! __pointer_type _M_p;
// Factored out to facilitate explicit specialization.
constexpr ptrdiff_t
--- 670,679
struct __atomic_base_PTp*
{
private:
! typedef _PTp* __pointer_type;
! typedef _PTp* __attribute ((atomic)) __atomic_pointer_type;
! __atomic_pointer_type _M_p;
// Factored out to facilitate explicit specialization.
constexpr ptrdiff_t
Index: libstdc++-v3/include/std/atomic
===
*** libstdc++-v3/include/std/atomic (revision 201248)
--- libstdc++-v3/include/std/atomic (working copy)
*** _GLIBCXX_BEGIN_NAMESPACE_VERSION
*** 161,167
struct atomic
{
private:
! _Tp _M_i;
public:
atomic() noexcept = default;
--- 161,167
struct atomic
{
private:
! _Tp __attribute ((atomic)) _M_i;
public:
atomic() noexcept = default;
Index: gcc/testsuite/gcc.dg/atomic-exchange-1.c
===
*** gcc/testsuite/gcc.dg/atomic-exchange-1.c (revision 201248)
--- gcc/testsuite/gcc.dg/atomic-exchange-1.c (working copy)
***
*** 7,13
extern void abort(void);
! char v, count, ret;
main ()
{
--- 7,14
extern void abort(void);
! char __attribute__ ((atomic)) v;
! char count, ret;
main ()
{
Index: gcc/testsuite/gcc.dg/atomic-exchange-2.c
===
*** gcc/testsuite/gcc.dg/atomic-exchange-2.c (revision 201248)
--- gcc/testsuite/gcc.dg/atomic-exchange-2.c (working copy)
***
*** 7,13
extern void abort(void);
! short v, count, ret;
main ()
{
--- 7,14
extern void abort(void);
! short __attribute__ ((atomic)) v;
! short count, ret;
main ()
{
Index: gcc/testsuite/gcc.dg/atomic-exchange-3.c
===
*** gcc/testsuite/gcc.dg/atomic-exchange-3.c (revision 201248)
--- gcc/testsuite/gcc.dg/atomic-exchange-3.c (working copy)
***
*** 7,13
extern void abort(void);
! int v, count, ret;
main ()
{
--- 7,14
extern void abort(void);
! int __attribute__ ((atomic)) v;
! int count, ret;
main ()
{
Index: gcc/testsuite/gcc.dg/atomic-exchange-4.c
===
*** gcc/testsuite/gcc.dg/atomic-exchange-4.c (revision 201248)
--- gcc/testsuite/gcc.dg/atomic-exchange-4.c (working copy)
***
*** 9,15
extern void abort(void);
! long long v, count, ret;
main ()
{
--- 9,16
extern void abort(void);
! long long __attribute__ ((atomic)) v;
! long long count, ret;
main ()
{
Index:
Re: [PATCH, libgcc] Fix licenses on several files
On Sun, Jul 28, 2013 at 3:03 PM, Maxim Kuvyrkov ma...@kugelworks.com wrote: While verifying license compliance for GCC and its libraries I noticed that several libgcc files that end up in the final library are licensed under GPL-3.0+ instead of GPL-3.0-with-GCC-exception. This is, obviously, was not the intention of developers who just copied wrong boilerplate text, and this patch fixes the oversights. Just to avoid any possible fallout from this issue ... Marcus, did you and ARM intend to license config/aarch64/sfp-machine.h and config/aarch64/sync-cache.c under GPL-3.0-with-GCC-exception? Sriram, did you and Google intend to license config/i386/cpuinfo.c under GPL-3.0-with-GCC-exception? Yes, the intention is to license under GPL-3.0-with-GCC-exception. Thanks, Sri Richard, did you and Red Hat intend to license config/ia64/unwind-ia64.h under GPL-3.0-with-GCC-exception? DJ, did you and Red Hat intend to license config/mips/vr4120-div.S under GPL-3.0-with-GCC-exception? Once confirmed, I will apply this patch to all active branches: trunk, gcc-4_8-branch and gcc-4_7-branch. Thank you, -- Maxim Kuvyrkov www.kugelworks.com
[C++ Patch] PR 57673
Hi,
we currently fail to parse this rather simple NSDMI usage (note that
wrapping the whole initializer in parentheses works around the problem
because cp_parser_cache_group handles the special case).
Simply detecting that we are parsing an ellipsis in an nsdmi and not
ending the main while loop in that case appears to work fine. If we
think we must be stricter we may consider keeping memory that the
previous token was a sizeof or... something else.
Tested x86_64-linux.
Thanks,
Paolo.
/cp
2013-07-30 Paolo Carlini paolo.carl...@oracle.com
PR c++/57673
* parser.c (cp_parser_cache_defarg): In an NSDMI don't stop when
token-type == CPP_ELLIPSIS.
/testsuite
2013-07-30 Paolo Carlini paolo.carl...@oracle.com
PR c++/57673
* g++.dg/cpp0x/nsdmi-sizeof.C: New.
Index: cp/parser.c
===
--- cp/parser.c (revision 201343)
+++ cp/parser.c (working copy)
@@ -24145,7 +24145,9 @@ cp_parser_cache_defarg (cp_parser *parser, bool ns
case CPP_SEMICOLON:
case CPP_CLOSE_BRACE:
case CPP_CLOSE_SQUARE:
- if (depth == 0)
+ if (depth == 0
+ /* Handle correctly int n = sizeof ... ( p ); */
+ !(nsdmi token-type == CPP_ELLIPSIS))
done = true;
/* Update DEPTH, if necessary. */
else if (token-type == CPP_CLOSE_PAREN
Index: testsuite/g++.dg/cpp0x/nsdmi-sizeof.C
===
--- testsuite/g++.dg/cpp0x/nsdmi-sizeof.C (revision 0)
+++ testsuite/g++.dg/cpp0x/nsdmi-sizeof.C (working copy)
@@ -0,0 +1,7 @@
+// PR c++/57673
+// { dg-do compile { target c++11 } }
+
+template int ... p
+struct d {
+ int n = sizeof ... ( p );
+};
Re: msp430 port
On 07/25/2013 05:36 PM, DJ Delorie wrote:
I tried to reproduce the original bugs that led to these patterns, but
was unable. Testsuite results are the same with and without, and
eembc code size doesn't change.
So, I'm removing these patterns from the port.
The remaining (subreg...) patterns are just optimizations.
So it's a little unclear to me which patterns you removed. Your message
has two patterns attached with comments they were to work around reload
issues. Are these the patterns you removed:
; This pattern is identical to the truncsipsi2 pattern except
; that it uses a SUBREG instead of a TRUNC. It is needed in
; order to prevent reload from converting (set:SI (SUBREG:PSI (SI)))
; into (SET:PSI (PSI)).
;
; Note: using POPM.A #1 is two bytes smaller than using POPX.A
(define_insn movsipsi2
[(set (match_operand:PSI0 register_operand =r)
(subreg:PSI (match_operand:SI 1 register_operand r) 0))]
TARGET_LARGE
PUSH.W %H1 { PUSH.W %1 { POPM.A #1, %0
)
; This pattern is needed in order to avoid reload problems.
; It takes an SI pair of registers, adds a value to them, and
; then converts them into a single PSI register.
(define_insn addsipsi3
[(set (subreg:SI (match_operand:PSI 0 register_operand =r) 0)
(plus:SI (match_operand:SI1 register_operand 0)
(match_operand 2 general_operand rmi)))
(clobber (reg:CC CARRY))
]
ADD.W\t%L2, %L0 { ADDC.W\t%H2, %H0 { PUSH.W %H0 { PUSH.W %L0 { POPM.A #1,
%0
)
With respect to optimizations, I was looking over the old mn102 port and
found the following you might want to consider.
First, if you can handle ASHIFT directly in PSImode, that's advantageous
for address calculations. Second, I had a fair number of combiner
patterns to try and combine things enough that we could eliminate
extensions in address arithmetic. Here's the patterns for reference:
;; These are special combiner patterns to improve array/pointer accesses.
;;
;; A typical sequence involves extending an integer/char, shifting it left
;; a few times, then truncating the value to PSImode.
;;
;; This first pattern combines the shifting truncation operations, by
;; itself it is a win because the shifts end up occurring in PSImode instead
;; of SImode. However, it has the secondary effect of giving us the
;; opportunity to match patterns which allow us to remove the initial
;; extension completely, which is a big win.
(define_insn
[(set (match_operand:PSI 0 general_operand =d,d,a)
(truncate:PSI
(ashift:SI (match_operand:SI 1 general_operand d,m,m)
(match_operand:HI 2 const_int_operand i,i,i]
*
{
int count = INTVAL (operands[2]);
if (which_alternative == 0)
output_asm_insn (\jsr ___truncsipsi2_%1_%0\, operands);
else if (which_alternative == 1)
output_asm_insn (\movx %A1,%0\, operands);
else
output_asm_insn (\ mov %1,%0\, operands);
while (count)
{
output_asm_insn (\add %0,%0\, operands);
count--;
}
return \\;
}
[(set_attr cc clobber)])
;; Similarly, except that we also have zero/sign extension of the
;; original operand. */
(define_insn
[(set (match_operand:PSI 0 general_operand =d,d)
(truncate:PSI
(ashift:SI
(zero_extend:SI (match_operand:HI 1 general_operand 0,dim))
(match_operand:HI 2 const_int_operand i,i]
*
{
int count = INTVAL (operands[2]);
/* First extend operand 1 to PSImode. */
if (which_alternative == 0)
output_asm_insn (\extxu %0\, operands);
else
output_asm_insn (\mov %1,%0\;extxu %0\, operands);
/* Now do the shifting. */
while (count)
{
output_asm_insn (\add %0,%0\, operands);
count--;
}
return \\;
}
[(set_attr cc clobber)])
(define_insn
[(set (match_operand:PSI 0 general_operand =d,d,d)
(truncate:PSI
(ashift:SI
(sign_extend:SI (match_operand:HI 1 general_operand 0,di,m))
(match_operand:HI 2 const_int_operand i,i,i]
*
{
int count = INTVAL (operands[2]);
/* First extend operand 1 to PSImode. */
if (which_alternative == 0)
output_asm_insn (\extx %0\, operands);
else if (which_alternative == 1)
output_asm_insn (\mov %1,%0\;extx %0\, operands);
else
output_asm_insn (\mov %1,%0\, operands);
/* Now do the shifting. */
while (count)
{
output_asm_insn (\add %0,%0\, operands);
count--;
}
return \\;
}
[(set_attr cc clobber)])
(define_insn
[(set (match_operand:PSI 0 general_operand =d,d,d)
(truncate:PSI
(ashift:SI
(sign_extend:SI
(zero_extend:HI (match_operand:QI 1 general_operand 0,di,m)))
(match_operand:HI 2 const_int_operand i,i,i]
*
{
int count = INTVAL (operands[2]);
/* First extend operand 1 to PSImode. */
if (which_alternative == 0)
output_asm_insn (\extxbu %0\, operands);
else if (which_alternative ==
Re: msp430 port
On 07/23/2013 04:08 PM, DJ Delorie wrote:
Index: gcc/cfgexpand.c
===
--- gcc/cfgexpand.c (revision 201184)
+++ gcc/cfgexpand.c (working copy)
I thought I already approved this. Go ahead and install it.
Index: gcc/simplify-rtx.c
===
--- gcc/simplify-rtx.c (revision 201184)
+++ gcc/simplify-rtx.c (working copy)
Testcase? Explanation? This seems like you're papering over a bug.
Index: gcc/dwarf2cfi.c
===
--- gcc/dwarf2cfi.c (revision 201184)
+++ gcc/dwarf2cfi.c (working copy)
@@ -277,12 +277,19 @@ expand_builtin_init_dwarf_reg_sizes (tre
{
if (save_mode == VOIDmode)
continue;
wrote_return_column = true;
}
size = GET_MODE_SIZE (save_mode);
+
+ /* Entries in the dwarf_reg_size_table must be big enough to hold an
_Unwind_Word
+ even if this is bigger than reg_raw_mode. This can happen on
targets where the
+ pointer size is larger than the integer size, and not a
power-of-two. (Eg MSP430). */
+ if (size GET_MODE_SIZE (targetm.unwind_word_mode ()))
+size = GET_MODE_SIZE (targetm.unwind_word_mode ());
So was there a discussion of this patch? Presumably BITS_PER_WORD is 16
and you've got 20 bit pointers which causes this problem, right?
I think you should go ahead and install this patch as well.
+; Note: using POPM.A #1 is two bytes smaller than using POPX.A
+
+(define_insn movsipsi2
+ [(set (match_operand:PSI0 register_operand =r)
+ (subreg:PSI (match_operand:SI 1 register_operand r) 0))]
+ TARGET_LARGE
+ PUSH.W %H1 { PUSH.W %1 { POPM.A #1, %0
+)
So how is this different fram
(set (psi) (truncate:psi (si)))?
+)
+
+; This pattern is needed in order to avoid reload problems.
+; It takes an SI pair of registers, adds a value to them, and
+; then converts them into a single PSI register.
+
+(define_insn addsipsi3
+ [(set (subreg:SI (match_operand:PSI 0 register_operand =r) 0)
+ (plus:SI (match_operand:SI1 register_operand 0)
+(match_operand 2 general_operand rmi)))
+ (clobber (reg:CC CARRY))
+ ]
+
+ ADD.W\t%L2, %L0 { ADDC.W\t%H2, %H0 { PUSH.W %H0 { PUSH.W %L0 { POPM.A #1,
%0
+
Is it possible the insns which resulted in adding this pattern came from
trying to zero/sign extend the frame pointer, then the frame pointer
gets eliminated and turns into a (plus (sp) (const_int)?
I dealt with this by having a special alternative to the
zero_extendpsisi2 and extendpsi_si2 patterns. I used a constraint which
matched sp + offset. Feels cleaner as we don't have the subreg in there.
Jeff
Re: [PATCH] libgcc/MIPS: Fill in delay slots of (some) MIPS16 call stubs
On Mon, 29 Jul 2013, Maciej W. Rozycki wrote: These stubs are I believe not really covered in our testing, because they require a mixed standard-MIPS/MIPS16 environment. What's the barrier to testing a mixed environment? The normal *-linux-gnu configurations have no MIPS16 multilibs, so you should be able to test it on a plain mips-linux-gnu configuration using --target_flags unix/-mips16. FWIW I've been using the mips64-linux-gnu equivalent (--target_flags unix/-mabi=32/-mips16) without problems. Or if you don't want to test on GNU/Linux, you should be able to use something like mips64-elf configured with whichever --with-arch= you like (and an appropriate simulator). Long time since I tried that though. I prefer testing on GNU/Linux because it also covers libgcc.so symbol visibility and has better libgfortran support. We don't have specific coverage, but something in the testsuite might happen to pull one or more of these thunks indeed. It is as I feared, the coverage is sparse. The only call/return stubs pulled across the testsuite are: __mips16_call_stub_sf_5 __mips16_call_stub_sc_0 __mips16_call_stub_df_0 __mips16_call_stub_df_1 __mips16_call_stub_df_2 __mips16_call_stub_df_10 (there are some call stubs too and some PIC stubs, but they all use different code). Of these only the first suffers from the old GAS shortcoming: __mips16_call_stub_sf_5: 0: 03e09021moves2,ra 4: 44846000mtc1a0,$f12 8: 44857000mtc1a1,$f14 c: 0040f809jalrv0 10: 0040c821movet9,v0 14: 4402mfc1v0,$f0 18: 0248jr s2 1c: nop as only stubs that return a single float result are affected (ones that return a single complex or a double result are not, because they use more than one CP1 move in their epilogues and old GAS is capable enough to schedule the last move into the JR's delay slot itself). libgcc/ * config/mips/mips16.S (DELAYf): Alias to DELAYt for the MIPS IV ISA and up. OK, thanks, but please do run it through the testsuite first. I'll see if I can do it -- the MIPS/Linux tree I used for recent MIPS32r2 MADD.fmt testing has no MIPS16 multilibs configured, so it might happen to just work with -mips16 passed as an extra option (otherwise MIPS16 libs would be automagically picked instead). I'll check if binaries executeed really pulled any of the thunks concerned. No regressions seen, applied now. Thanks for your review. Maciej
Re: [patch, mips] Size savings for MIPS16 switch statements
On Tue, 30 Jul 2013, Steve Ellcey wrote: -- it may be that the tests have to be disabled at -Os just like e.g. code-readable-1.c already is at -O0. Sorry about that, not sure why I didn't notice the failures. Rather then skipping the tests for -Os I was thinking it might be better to increase the size of the switch statements. Here is a patch I have tested to fix these failures. That sounds even better to me -- perhaps you can take the opportunity and test the new threshold too? I.e. another test case for MIPS16 and -Os only that makes sure the code produced is switched between the two models when the size of a switch statement crosses the boundary? Maciej
Re: msp430 port
Index: gcc/cfgexpand.c
===
--- gcc/cfgexpand.c (revision 201184)
+++ gcc/cfgexpand.c (working copy)
I thought I already approved this. Go ahead and install it.
Sorry, I forgot to split out those three patches when I diffed.
Index: gcc/simplify-rtx.c
===
--- gcc/simplify-rtx.c (revision 201184)
+++ gcc/simplify-rtx.c (working copy)
Testcase? Explanation? This seems like you're papering over a bug.
Likewise.
Index: gcc/dwarf2cfi.c
So was there a discussion of this patch? Presumably BITS_PER_WORD is 16
and you've got 20 bit pointers which causes this problem, right?
I don't recall any discussion. Right, pointers are bigger than words.
I think you should go ahead and install this patch as well.
Ok.
+; Note: using POPM.A #1 is two bytes smaller than using POPX.A
+
+(define_insn movsipsi2
+ [(set (match_operand:PSI0 register_operand =r)
+ (subreg:PSI (match_operand:SI 1 register_operand r) 0))]
+ TARGET_LARGE
+ PUSH.W %H1 { PUSH.W %1 { POPM.A #1, %0
+)
So how is this different fram
(set (psi) (truncate:psi (si)))?
It should be the same, of course. I typically look at what gcc *is*
generating, not what it *should* be generating, though ;-)
GCC does a lot of silly things these days.
+; This pattern is needed in order to avoid reload problems.
+; It takes an SI pair of registers, adds a value to them, and
+; then converts them into a single PSI register.
+
+(define_insn addsipsi3
+ [(set (subreg:SI (match_operand:PSI 0 register_operand =r) 0)
+ (plus:SI (match_operand:SI1 register_operand 0)
+(match_operand 2 general_operand rmi)))
+ (clobber (reg:CC CARRY))
+ ]
+
+ ADD.W\t%L2, %L0 { ADDC.W\t%H2, %H0 { PUSH.W %H0 { PUSH.W %L0 { POPM.A
#1, %0
+
Is it possible the insns which resulted in adding this pattern came from
trying to zero/sign extend the frame pointer, then the frame pointer
gets eliminated and turns into a (plus (sp) (const_int)?
It's possible that's one case, but *all* pointer math happens in
SImode since the pointer_plus change went in. So this pattern happens
a lot.
[Google] Fix profiledbootstrap failure
Hello This change allows to complete profiledbootstrap on the google gcc-4.8 branch, tested with make bootstrap with no new regressions. OK for google 4.8? thanks, Dinar. profiledbootstrap-fix.patch Description: Binary data
Re: C++ coding conventions: namespaces, references and getters (was Re: [PATCH 2/2] Introduce beginnings of a pipeline class.)
On Tue, 2013-07-30 at 11:30 +0200, Martin Jambor wrote: Hi, On Mon, Jul 29, 2013 at 09:02:53PM +0200, Oleg Endo wrote: On Mon, 2013-07-29 at 14:20 -0400, David Malcolm wrote: The same here and at a few other places. It may be just me not being used to references... nevertheless, if someone really wants to use them like this, at least make them const and you will save a night of frantic debugging to someone, probably to yourself. But I strongly prefer pointers... it's hard to describe how strongly I prefer them. OK. How do others feel? As I said above, I like the above idiom, since it puts the assertion of non-NULLness in a single place. I'm voting for references. References can be seen as yet another software structuring tool that instantly communicate some properties such as you mentioned above. In addition to that it's also a hint of ownership, i.e. if I get an object from somewhere I know that I better not even think about whether to delete it or not. well, let me stress again that we should think about this in the context of GCC. In GCC, we are used to C semantics of the dot operator How are the dot operator semantics in C different from the dot operator semantics in C++? and have a lot of existing code that we will continue to use and mix with new code with the same assumption. Putting a reference where none has been before might result in silent and hard to spot erroneous modifications. Sorry, I have difficulty understanding your reasoning here. Could you provide an example for Putting a reference where none has been before might result in silent and hard to spot erroneous modifications.? Cheers, Oleg
Re: msp430 port
So it's a little unclear to me which patterns you removed. Your message has two patterns attached with comments they were to work around reload issues. Are these the patterns you removed? Yes. I was unable to reproduce the reload problems to which those comments referred. First, if you can handle ASHIFT directly in PSImode, that's advantageous for address calculations. Yup, I have patterns for 1 and 2 bit shifts, which are all that really happen. They look remarkably like your patterns, but use a subreg on 0 instead of a truncate on the expression (again, that's what combine was looking for, so that's what I put in).
Re: [Google] Fix profiledbootstrap failure
cc'ing Rong and David since this came from LIPO support. The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on by default) without removing the parameter itself. What is the failure mode you see from this code? Thanks, Teresa On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov di...@kugelworks.com wrote: Hello This change allows to complete profiledbootstrap on the google gcc-4.8 branch, tested with make bootstrap with no new regressions. OK for google 4.8? thanks, Dinar. -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
[Committed] Introduce beginnings of gcc::pass_manager class (was Re: [PATCH 01/11] Introduce beginnings of a pipeline class.)
On Mon, 2013-07-29 at 13:56 -0600, Jeff Law wrote: On 07/26/2013 09:04 AM, David Malcolm wrote: [...snip quote of ChangeLog...] So as has been discussed elsewhere I'd like to see pipeline changed to pass_manager. WRT references. Not being a C++ guy, I'd always mentally equated references with pointers. I've done a little reading and it seems that references actually convey more information, which I'm generally a fan of -- with the caveat that they're pointers that act more like values, which might get confusing. At least in the immediate term, I think we should stick with pointers until we have a clearer sense of whether or not we want to be using references in this way. I doubt it's terribly important, but the non-nullness ought to be expressable via an attribute. With the pipeline-pass_manager change and using pointers instead of references on the return type, this patch is fine. Pre-approved with those changes. Thanks. I've committed the attached to trunk as r201351, having done the following: * rebased against trunk * renamed class pipeline and file pipeline.h to class pass_manager and file pass_manager.h * replaced uses of references with pointers * context.c (context::context): Fixed missing space in pass_manager constructor invocation noted by Martin Jambor in http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01155.html * successfully bootstrapped the new patch on x86_64-unknown-linux-gnu and reran the testsuite: all testcases showed the same results as an unpatched build (relative to r201317). * verified that the patch builds on an updated svn checkout. Dave From d396b8c8d4382fb0337ad7813fca03dc563b96d2 Mon Sep 17 00:00:00 2001 From: David Malcolm dmalc...@redhat.com Date: Mon, 22 Jul 2013 14:06:26 -0400 Subject: [PATCH 01/14] Introduce beginnings of a pass_manager class. This patch introduces a gcc::pass_manager class and moves various non-GTY globals relating to pass management into it. The gcc::context gains its first field: a pointer to the gcc::pass_manager instance. It was previously sent as: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01090.html as part of: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01088.html and Martin Jambor raised some stylistic concerns about it: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01155.html I'm reposting it here so that people can see how it fits into the later patches. gcc/ * Makefile.in (PASS_MANAGER_H): New. (lto-cgraph.o): Depend on CONTEXT_H and PASS_MANAGER_H. (passes.o): Likewise. (statistics.o): Likewise. (cgraphunit.o): Likewise. (context.o): Depend on PASS_MANAGER_H. * pass_manager.h: New. * cgraphunit.c (cgraph_add_new_function): Update for moves of globals to fields of pass_manager. (analyze_function): Likewise. (expand_function): Likewise. (ipa_passes): Likewise. (compile): Likewise. * context.c (context::context): New. * context.h (context::context): New. (context::get_passes): New. (context::passes_): New. * lto-cgraph.c (input_node): Update for moves of globals to fields of pass_manager. * passes.c (all_passes): Remove, in favor of a field of the same name within the new class pass_manager. (all_small_ipa_passes): Likewise. (all_lowering_passes): Likewise. (all_regular_ipa_passes): Likewise. (all_late_ipa_passes): Likewise. (all_lto_gen_passes): Likewise. (passes_by_id): Likewise. (passes_by_id_size): Likewise. (gcc_pass_lists): Remove, in favor of pass_lists field within the new class pass_manager. (set_pass_for_id): Convert to... (pass_manager::set_pass_for_id): ...method. (get_pass_for_id): Convert to... (pass_manager::get_pass_for_id): ...method. (register_one_dump_file): Move body of implementation into... (pass_manager::register_one_dump_file): ...here. (register_dump_files_1): Convert to... (pass_manager::register_dump_files_1): ...method. (register_dump_files): Convert to... (pass_manager::register_dump_files): ...method. (create_pass_tab): Update for moves of globals to fields of pass_manager. (dump_passes): Move body of implementation into... (pass_manager::dump_passes): ...here. (register_pass): Move body of implementation into... (pass_manager::register_pass): ...here. (init_optimization_passes): Convert into... (pass_manager::pass_manager): ...constructor for new pass_manager class, and initialize the pass_lists array. (check_profile_consistency): Update for moves of globals to fields of pass_manager. (dump_profile_report): Move body of implementation into... (pass_manager::dump_profile_report): ...here. (ipa_write_summaries_1): Update for moves of pass lists from being globals to fields of pass_manager. (ipa_write_optimization_summaries): Likewise. (ipa_read_summaries): Likewise. (ipa_read_optimization_summaries): Likewise. (execute_all_ipa_stmt_fixups): Likewise. * statistics.c (statistics_fini): Update for moves of globals to fields of pass_manager. * toplev.c (general_init): Replace call to
Re: [Google] Fix profiledbootstrap failure
I need to understand why this affects profile bootstrap -- is this due to file name conflict? The fix is wrong -- please do not remove the parameter. If it is a problem, a better fix is to change the default parameter value to 0. David On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com wrote: cc'ing Rong and David since this came from LIPO support. The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on by default) without removing the parameter itself. What is the failure mode you see from this code? Thanks, Teresa On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov di...@kugelworks.com wrote: Hello This change allows to complete profiledbootstrap on the google gcc-4.8 branch, tested with make bootstrap with no new regressions. OK for google 4.8? thanks, Dinar. -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: [patch, mips] Size savings for MIPS16 switch statements
Steve Ellcey sell...@mips.com writes: 2013-07-30 Steve Ellcey sell...@mips.com * gcc.target/mips/code-readable-1.c: Increase switch size. * gcc.target/mips/code-readable-2.c: Ditto. * gcc.target/mips/code-readable-3.c: Ditto. * gcc.target/mips/code-readable-4.c: Ditto. OK, thanks. Richard
Re: [C++ Patch] PR 57673
OK. Jason
Re: [Google] Fix profiledbootstrap failure
We have seen the issue before. It does fail the profile boostrap as it reads a wrong gcda file. I thought it had been fixed. (The fix was as David mentioned, setting the default value of the parameter to 0). -Rong On Tue, Jul 30, 2013 at 12:02 PM, Xinliang David Li davi...@google.com wrote: I need to understand why this affects profile bootstrap -- is this due to file name conflict? The fix is wrong -- please do not remove the parameter. If it is a problem, a better fix is to change the default parameter value to 0. David On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com wrote: cc'ing Rong and David since this came from LIPO support. The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on by default) without removing the parameter itself. What is the failure mode you see from this code? Thanks, Teresa On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov di...@kugelworks.com wrote: Hello This change allows to complete profiledbootstrap on the google gcc-4.8 branch, tested with make bootstrap with no new regressions. OK for google 4.8? thanks, Dinar. -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: [Google] Fix profiledbootstrap failure
I need to understand why this affects profile bootstrap -- is this due to file name conflict? Yes, It is simple. During the profiledbootstrap on x86_64 platform for libiberty the compiler picks an incorrect profile from the current directory(non-pic version), while compiling pic version, and profiledbootstrap fails. Here is log: if [ x-fpic != x ]; then \ /home/dinar/bugz/x86_64/build-gcc-4_8-svn-orig/./prev-gcc/xgcc -B/home/dinar/bugz/x86_64/bisect/build-gcc-4_8-svn-orig/./prev-gcc/ -B/tmp/x86_64-unknown-linux-gnu/bin/ -B/tmp/x86_64-unknown-linux-gnu/bin/ -B/tmp/x86_64-unknown-linux-gnu/lib/ -isystem /tmp/x86_64-unknown-linux-gnu/include -isystem /tmp/x86_64-unknown-linux-gnu/sys-include-c -DHAVE_CONFIG_H -g -O2 -fprofile-use -I. -I../../gcc-4_8-svn-orig/libiberty/../include -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic -fpic ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c -o pic/dwarfnames.o; \ else true; fi yes ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: error: the control flow of function ‘get_DW_CFA_name’ does not match its profile data (counter ‘arcs’) [-Werror=coverage-mismatch] ^ ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: note: use -Wno-error=coverage-mismatch to tolerate the mismatch but performance may drop if the function is hot ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c: In function ‘get_DW_ATE_name’: ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: error: the control flow of function ‘get_DW_ATE_name’ does not match its profile data (counter ‘arcs’) [-Werror=coverage-mismatch] ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: note: use -Wno-error=coverage-mismatch to tolerate the mismatch but performance may drop if the function is hot ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c: In function ‘get_DW_OP_name’: ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: error: the control flow of function ‘get_DW_OP_name’ does not match its profile data (counter ‘arcs’) [-Werror=coverage-mismatch] ../../gcc-4_8-svn-orig/libiberty/dwarfnames.c:75:0: note: use -Wno-error=coverage-mismatch to tolerate the mismatch but performance may drop if the function is hot thanks, Dinar. On Tue, Jul 30, 2013 at 11:02 PM, Xinliang David Li davi...@google.com wrote: I need to understand why this affects profile bootstrap -- is this due to file name conflict? The fix is wrong -- please do not remove the parameter. If it is a problem, a better fix is to change the default parameter value to 0. David On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com wrote: cc'ing Rong and David since this came from LIPO support. The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on by default) without removing the parameter itself. What is the failure mode you see from this code? Thanks, Teresa On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov di...@kugelworks.com wrote: Hello This change allows to complete profiledbootstrap on the google gcc-4.8 branch, tested with make bootstrap with no new regressions. OK for google 4.8? thanks, Dinar. -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: [Google] Fix profiledbootstrap failure
Ok. Rong, can you help commit the parameter default setting patch? thanks, David On Tue, Jul 30, 2013 at 12:48 PM, Rong Xu x...@google.com wrote: We have seen the issue before. It does fail the profile boostrap as it reads a wrong gcda file. I thought it had been fixed. (The fix was as David mentioned, setting the default value of the parameter to 0). -Rong On Tue, Jul 30, 2013 at 12:02 PM, Xinliang David Li davi...@google.com wrote: I need to understand why this affects profile bootstrap -- is this due to file name conflict? The fix is wrong -- please do not remove the parameter. If it is a problem, a better fix is to change the default parameter value to 0. David On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com wrote: cc'ing Rong and David since this came from LIPO support. The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on by default) without removing the parameter itself. What is the failure mode you see from this code? Thanks, Teresa On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov di...@kugelworks.com wrote: Hello This change allows to complete profiledbootstrap on the google gcc-4.8 branch, tested with make bootstrap with no new regressions. OK for google 4.8? thanks, Dinar. -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: [Google] Fix profiledbootstrap failure
Will do. The patch was in gcc-4_7 by Dehao. r194713 | dehao | 2012-12-24 16:49:06 -0800 (Mon, 24 Dec 2012) | 5 lines Fix the profiled bootstrap: 1. Set the default value of gcov-debug to be 0. 2. Merge profile summaries from different instrumented binaries. On Tue, Jul 30, 2013 at 12:58 PM, Xinliang David Li davi...@google.com wrote: Ok. Rong, can you help commit the parameter default setting patch? thanks, David On Tue, Jul 30, 2013 at 12:48 PM, Rong Xu x...@google.com wrote: We have seen the issue before. It does fail the profile boostrap as it reads a wrong gcda file. I thought it had been fixed. (The fix was as David mentioned, setting the default value of the parameter to 0). -Rong On Tue, Jul 30, 2013 at 12:02 PM, Xinliang David Li davi...@google.com wrote: I need to understand why this affects profile bootstrap -- is this due to file name conflict? The fix is wrong -- please do not remove the parameter. If it is a problem, a better fix is to change the default parameter value to 0. David On Tue, Jul 30, 2013 at 11:56 AM, Teresa Johnson tejohn...@google.com wrote: cc'ing Rong and David since this came from LIPO support. The patch as-is removes the one use of PARAM_GCOV_DEBUG (which is on by default) without removing the parameter itself. What is the failure mode you see from this code? Thanks, Teresa On Tue, Jul 30, 2013 at 11:50 AM, Dinar Temirbulatov di...@kugelworks.com wrote: Hello This change allows to complete profiledbootstrap on the google gcc-4.8 branch, tested with make bootstrap with no new regressions. OK for google 4.8? thanks, Dinar. -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: msp430 port
On 07/23/2013 12:08 PM, DJ Delorie wrote:
===
--- gcc/simplify-rtx.c(revision 201184)
+++ gcc/simplify-rtx.c(working copy)
@@ -5884,12 +5884,23 @@ simplify_immed_subreg (enum machine_mode
/* Simplify SUBREG:OUTERMODE(OP:INNERMODE, BYTE)
Return 0 if no simplifications are possible. */
rtx
simplify_subreg (enum machine_mode outermode, rtx op,
enum machine_mode innermode, unsigned int byte)
{
+ /* FIXME: hack to allow building of newlib/libc.a for msp430/430x/large
multilib.
+ The problem is the var-tracking is generating paradoxical SUBREGs. Not
sure why... */
+ if (!(GET_MODE (op) == innermode
+|| GET_MODE (op) == VOIDmode)
+ || (innermode == VOIDmode)
+ )
+{
+ debug_rtx (op);
+ return NULL_RTX;
+}
Jeff asked about this one.
Certainly this sort of hack could never go in as-is.
+(define_predicate msp_volatile_memory_operand
+ (and (match_code mem)
+ (match_test (memory_address_addr_space_p (GET_MODE (op), XEXP (op,
0), MEM_ADDR_SPACE (op)
+)
The name could be better here. Alternately,
+; TRUE for any valid general operand. We do this because
+; general_operand refuses to match volatile memory refs.
+
+(define_predicate msp_general_operand
+ (ior (match_operand 0 general_operand)
+ (match_operand 0 msp_volatile_memory_operand))
+)
... define these as
(define_predicate msp_general_operand
(match_code mem,reg,subreg,const_int,const,symbol_ref,label_ref
{
int save_volatile_ok = volatile_ok;
volatile_ok = 1;
int ret = general_operand (op, mode);
volatile_ok = save_volatile_ok;
return ret;
})
That said, why do they exist?
+; TRUE for constants which are bit positions for zero_extract
+(define_predicate msp430_bitpos
+ (and (match_code const_int)
+ (match_test ( INTVAL (op) = 0
+ INTVAL (op) = 15
IN_RANGE?
+;; This is nasty. Operand0 is bogus. It is only there so that we can get a
+;; mode for the %B0 to work. We should use operand1 for this, but that does
+;; not have a mode.
+;;
+;; Operand1 is actually a register, but we cannot accept (REG...) because the
+;; cprop_hardreg pass can and will renumber registers even inside
+;; unspec_volatiles. So we take an integer register number parameter and
+;; fudge it to be a register name when we generate the assembler.
+;;
+;; The pushm pattern does not have this problem because of all of the
+;; frame info cruft attached to it, so cprop_hardreg leaves it alone.
+(define_insn popm
+ [(unspec_volatile [(match_operand 0 register_operand r)
+ (match_operand 1 immediate_operand i)
+ (match_operand 2 immediate_operand i)] UNS_POPM)]
+
+ POPM%B0\t%2, r%D1
+ )
That's because the insn is mis-defined. Even with unspec_volatile, you must
respect the form of rtl. Above, operand 1 is positioned as an input, which is
wrong. It must be positioned as an output, e.g.
[(set (match_operand 0 register_operand =r)
(unspec_volatile [(match_operand 1 immediate_operand i)]
UNS_POPM))]
POPM%B0\t%1, %0
+;; The next two patterns are here to support a feature of how GCC
implements
+;; varargs. When a function uses varargs and the *second* to last named
+;; argument is split between argument registers and the stack, gcc expects
the
+;; callee to allocate space on the stack that can contain the register-based
+;; part of the argument. This space *has* to be just before the remaining
+;; arguments (ie the ones that are fully on the stack).
Ug. We ought to have been able to convince the compiler to copy the other
direction. I.e. create storage for that argument as if it were a regular local
variable, spill the register portion into it, and also copy in the fragment
from the stack.
That sort of solution seems better than fiddling the location of the return
address in both directions.
+; This pattern is needed in order to avoid reload problems.
+; It takes an SI pair of registers, adds a value to them, and
+; then converts them into a single PSI register.
+
+(define_insn addsipsi3
+ [(set (subreg:SI (match_operand:PSI 0 register_operand =r) 0)
+ (plus:SI (match_operand:SI1 register_operand 0)
+ (match_operand 2 general_operand rmi)))
+ (clobber (reg:CC CARRY))
Do you really need the subreg here? Or would you be better off with
a truncate pattern on the RHS? Alternately, Jeff's solution.
+(define_split
+ [(set (match_operand:SI 0 msp430_nonsubreg_operand =rm)
+ (plus:SI (match_operand:SI 1 nonimmediate_operand %0)
+ (match_operand:SI 2 general_operand rmi)))
No constraints on splits.
+ (clobber (reg:CC CARRY))
+ ]
+
+ [(parallel [(set (match_operand:HI 3 nonimmediate_operand =rm)
Nor here.
+;; Alternatives 2 and 3 are to handle cases generated by
Re: msp430 port
On 07/30/2013 12:47 PM, DJ Delorie wrote:
+; Note: using POPM.A #1 is two bytes smaller than using POPX.A
+
+(define_insn movsipsi2
+ [(set (match_operand:PSI0 register_operand =r)
+ (subreg:PSI (match_operand:SI 1 register_operand r) 0))]
+ TARGET_LARGE
+ PUSH.W %H1 { PUSH.W %1 { POPM.A #1, %0
+)
So how is this different fram
(set (psi) (truncate:psi (si)))?
It should be the same, of course. I typically look at what gcc *is*
generating, not what it *should* be generating, though ;-)
GCC does a lot of silly things these days.
Well, you need to show why GCC is generating this and we need to
evaluate whether or not it's safe. I'm very leery of anything using a
PSImode SUBREG on a port where truncation is not a nop.
+; This pattern is needed in order to avoid reload problems.
+; It takes an SI pair of registers, adds a value to them, and
+; then converts them into a single PSI register.
+
+(define_insn addsipsi3
+ [(set (subreg:SI (match_operand:PSI 0 register_operand =r) 0)
+ (plus:SI (match_operand:SI1 register_operand 0)
+(match_operand 2 general_operand rmi)))
+ (clobber (reg:CC CARRY))
+ ]
+
+ ADD.W\t%L2, %L0 { ADDC.W\t%H2, %H0 { PUSH.W %H0 { PUSH.W %L0 { POPM.A #1,
%0
+
Is it possible the insns which resulted in adding this pattern came from
trying to zero/sign extend the frame pointer, then the frame pointer
gets eliminated and turns into a (plus (sp) (const_int)?
It's possible that's one case, but *all* pointer math happens in
SImode since the pointer_plus change went in. So this pattern happens
a lot.
I'm very aware that all pointer math happens in SImode. My point is
you need to find out *why* pattern was needed, explain it to this list
so that we can evaluate if it's the right solution. I brought up the
issue from the mn102 port because there's some reasonable chance it's
the same problem. If it is the same problem, then we can discuss which
is the better solution or if we hate them both, look at what it might
take to fix reload.
jeff
Re: msp430 port
On 07/30/2013 12:51 PM, DJ Delorie wrote: First, if you can handle ASHIFT directly in PSImode, that's advantageous for address calculations. Yup, I have patterns for 1 and 2 bit shifts, which are all that really happen. They look remarkably like your patterns, but use a subreg on 0 instead of a truncate on the expression (again, that's what combine was looking for, so that's what I put in). And to that I would say, let's evaluate why combine was using the subreg rather than a truncate. That seems wrong if we've defined TRULY_NOOP_TRUNCATION properly. jeff
[google gcc-4_8] Force cmd-line match for option -ansi in LIPO use
The following patch forces the command line match for -ansi option
in LIPO use build. Otherwise, it gets various undefined symbol errors.
This is exposed in LIPO random grouping test.
Tested with google internal benchmarks and gcc regression test.
2013-07-30 Rong Xu x...@google.com
* gcc/coverage.c (force_matching_cg_opts): require
cmd line match on -ansi option in LIPO use build.
Index: gcc/coverage.c
===
--- gcc/coverage.c (revision 201219)
+++ gcc/coverage.c (working copy)
@@ -263,6 +263,7 @@ static struct opt_desc force_matching_cg_opts[] =
{ -fsized-delete, -fno-sized-delete, false },
{ -frtti, -fno-rtti, true },
{ -fstrict-aliasing, -fno-strict-aliasing, true },
+{ -ansi, , false },
{ NULL, NULL, false }
};
Re: [google gcc-4_8] Force cmd-line match for option -ansi in LIPO use
On Tue, Jul 30, 2013 at 1:44 PM, Rong Xu x...@google.com wrote:
The following patch forces the command line match for -ansi option
in LIPO use build. Otherwise, it gets various undefined symbol errors.
Parsing error as you have clarified.
This is exposed in LIPO random grouping test.
Tested with google internal benchmarks and gcc regression test.
2013-07-30 Rong Xu x...@google.com
* gcc/coverage.c (force_matching_cg_opts): require
cmd line match on -ansi option in LIPO use build.
Index: gcc/coverage.c
===
--- gcc/coverage.c (revision 201219)
+++ gcc/coverage.c (working copy)
@@ -263,6 +263,7 @@ static struct opt_desc force_matching_cg_opts[] =
{ -fsized-delete, -fno-sized-delete, false },
{ -frtti, -fno-rtti, true },
{ -fstrict-aliasing, -fno-strict-aliasing, true },
+{ -ansi, , false },
{ NULL, NULL, false }
};
Ok for google branch.
David
Re: [GOOGLE] Refactor AutoFDO
I have some preliminary comments. Mostly just related to code style
and missing documentation.
David
#define DEFAULT_AUTO_PROFILE_FILE fbdata.afdo
struct SourceLocation
Is using Upper case in struct names allowed?
{
tree func_decl;
unsigned lineno;
};
typedef std::vectorconst char * StringVector;
typedef std::vectorSourceLocation InlineStack;
typedef std::mapunsigned, gcov_type TargetMap;
Add short description of each new types.
struct ProfileInfo
{
gcov_type count;
TargetMap target_map;
};
struct StringCompare
{
bool operator() (const char* a, const char* b) const
'*' should bind to name.
{ return strcmp (a, b) 0; }
};
class StringMap {
public:
static StringMap *Create();
int GetIndex (const char *name) const;
int GetIndexByDecl (tree decl) const;
const char *GetName (int index) const;
private:
StringMap () {}
bool Read ();
typedef std::mapconst char *, unsigned, StringCompare StringIndexMap;
StringVector vector_;
StringIndexMap map_;
};
Add some documentation on the new type, indicating what is 'index'.
class Symbol {
The name 'Symbol' is too generic -- can cause conflicts in the future
unless namespace is used. ALso missing documentation.
public:
typedef std::vectorSymbol * SymbolStack;
Fix indentation problems.
/* Read the profile and create a symbol with head count as HEAD_COUNT.
Recursively read callsites to create nested symbols too. STACK is used
to track the recursive creation process. */
static const Symbol *ReadSymbol (SymbolStack *stack, gcov_type head_count);
/* Recursively deallocate all callsites (nested symbols). */
~Symbol ();
/* Accessors. */
unsigned name () const { return name_; }
gcov_type total_count () const { return total_count_; }
gcov_type head_count () const { return head_count_; }
/* Recursively traverse STACK starting from LEVEL to find the corresponding
symbol. */
const Symbol *GetSymbol (const InlineStack stack, unsigned level) const;
/* Return the profile info for LOC. */
bool GetProfileInfo (location_t loc, ProfileInfo *info) const;
private:
Symbol (unsigned name, gcov_type head_count)
: name_(name), total_count_(0), head_count_(head_count) {}
const Symbol *GetSymbolByDecl (unsigned lineno, tree decl) const;
typedef std::mapgcov_type, const Symbol * CallsiteMap;
Need documentation for this map.
typedef std::mapunsigned, ProfileInfo PositionCountMap;
Need documentation.
/* Symbol name index in the string map. */
unsigned name_;
/* The total sampled count. */
gcov_type total_count_;
/* The total sampled count in the head bb. */
gcov_type head_count_;
/* Map from callsite location to callee symbol. */
CallsiteMap callsites;
/* Map from source location to count and instruction number. */
PositionCountMap pos_counts;
};
class SymbolMap {
Need documentation.
public:
static SymbolMap *Create ()
{
SymbolMap *map = new SymbolMap ();
if (map-Read ())
return map;
delete map;
return NULL;
}
~SymbolMap ();
const Symbol *GetSymbolByDecl (tree decl) const;
bool GetProfileInfo (gimple stmt, ProfileInfo *info) const;
gcov_type GetCallsiteTotalCount (struct cgraph_edge *edge) const;
Missing documentation for the interfaces
private:
typedef std::mapunsigned, const Symbol * NameSymbolMap;
map from what to symbol?
SymbolMap () {}
bool Read ();
const Symbol *GetSymbolByInlineStack (const InlineStack stack) const;
Missing documentation for the interfaces
NameSymbolMap map_;
};
class ModuleMap {
Need documentation.
On Tue, Jul 30, 2013 at 11:03 AM, Dehao Chen de...@google.com wrote:
I just rebased the CL to head and updated the patch.
Thanks,
Dehao
On Tue, Jul 30, 2013 at 10:09 AM, Xinliang David Li davi...@google.com
wrote:
I can not apply the patch cleanly in my v17 gcc client -- there is
some problem in auto-profile.c.
David
On Mon, Jul 29, 2013 at 7:52 PM, Dehao Chen de...@google.com wrote:
This patch refactors AutoFDO to use:
1. C++ to rewrite the whole thing.
2. Use tree instead of hashtable to represent the profile.
3. Make AutoFDO standalone: keep changes to other modules minimum.
Bootstrapped and passed regression test and benchmark test.
OK for google-4_8 branch?
Thanks,
Dehao
http://codereview.appspot.com/12079043
Re: [Patch] Refractor Thompson matcher
Hi again, I reverted the commit and tested that mainline is fine again. Just to clarify how we normally handle these issues in v3: *temporarily*, to avoid the linkage issues which broke the bootstrap today, all the non-template functions must be inline, even if large. In the past normally we had the definitions of such functions in *.tcc files, thus explicitly inline, with FIXME comments. I think it makes sense to do this for regex too. In any case, as soon as the design stabilizes all such functions shall be exported by the dynamic library, thus in practice all the definitions will be moved to src/c++11/*.cc files. Thanks, Paolo
Re: [SPARC] Remove superfluous memory barrier for atomics with TSO
On 07/30/2013 03:31 AM, Eric Botcazou wrote: 2013-07-30 Eric Botcazou ebotca...@adacore.com * config/sparc/sparc.c (sparc_emit_membar_for_model) SMM_TSO: Add the implied StoreLoad barrier for atomic operations if before. Looks good. r~
Re: [Patch] Refractor Thompson matcher
On Wed, Jul 31, 2013 at 6:10 AM, Paolo Carlini paolo.carl...@oracle.com wrote: I reverted the commit and tested that mainline is fine again. Sorry for the accident! Just to clarify how we normally handle these issues in v3: *temporarily*, to avoid the linkage issues which broke the bootstrap today, all the non-template functions must be inline, even if large. In the past normally we had the definitions of such functions in *.tcc files, thus explicitly inline, with FIXME comments. I think it makes sense to do this for regex too. In any case, as soon as the design stabilizes all such functions shall be exported by the dynamic library, thus in practice all the definitions will be moved to src/c++11/*.cc files. I see. So I include regex in different files and then compile them together, it broke. I've make every non-templated function in this commit inline. Now it compiles. Sorry again for this commit. Should I commit again? -- Tim Shen
Re: [Patch] Refractor Thompson matcher
On Wed, Jul 31, 2013 at 6:44 AM, Tim Shen timshe...@gmail.com wrote: On Wed, Jul 31, 2013 at 6:10 AM, Paolo Carlini paolo.carl...@oracle.com wrote: I reverted the commit and tested that mainline is fine again. Sorry for the accident! Just to clarify how we normally handle these issues in v3: *temporarily*, to avoid the linkage issues which broke the bootstrap today, all the non-template functions must be inline, even if large. In the past normally we had the definitions of such functions in *.tcc files, thus explicitly inline, with FIXME comments. I think it makes sense to do this for regex too. In any case, as soon as the design stabilizes all such functions shall be exported by the dynamic library, thus in practice all the definitions will be moved to src/c++11/*.cc files. I see. So I include regex in different files and then compile them together, it broke. I've make every non-templated function in this commit inline. Now it compiles. Sorry again for this commit. Should I commit again? -- Tim Shen -- Tim Shen inline.patch Description: Binary data
Re: [Patch] Refractor Thompson matcher
Hi, On 07/31/2013 12:44 AM, Tim Shen wrote: I see. So I include regex in different files and then compile them together, it broke. I've make every non-templated function in this commit inline. Now it compiles. Sorry again for this commit. Did you actually bootstrap test the patch? Because you didn't last time, right? Should I commit again? Please add FIXME comments to all the non-template functions in *.tcc files (which will be moved to *.cc files). To repeat: the inlines in *.tcc files must be all and only non-templates, as a temporary hack. Thanks, Paolo.
Re: [Patch] Refractor Thompson matcher
On Wed, Jul 31, 2013 at 7:03 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Did you actually bootstrap test the patch? Because you didn't last time, right? I compiled it and run the 28_regex testsuite, nothing wrong happened because there're all single-file testcases in it; but I ignored the duplicated definition problem. It'll never happen in the future. Sorry again. Please add FIXME comments to all the non-template functions in *.tcc files (which will be moved to *.cc files). To repeat: the inlines in *.tcc files must be all and only non-templates, as a temporary hack. Added. -- Tim Shen
Re: [Patch] Refractor Thompson matcher
Hi, On 07/31/2013 01:11 AM, Tim Shen wrote: On Wed, Jul 31, 2013 at 7:03 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Did you actually bootstrap test the patch? Because you didn't last time, right? I compiled it and run the 28_regex testsuite, nothing wrong happened because there're all single-file testcases in it; but I ignored the duplicated definition problem. It'll never happen in the future. Sorry again. Earlier, even without bootstrapping you should have noticed that the library didn't build. It's not about excuses, it's about following a policy, which, should be obvious now, we are following for a reason: all posted patches must be boostrapped tested. Please add FIXME comments to all the non-template functions in *.tcc files (which will be moved to *.cc files). To repeat: the inlines in *.tcc files must be all and only non-templates, as a temporary hack. Added. Please post the complete patch you intend to commit. Part of the GCC policy is also that all the patches must be posted complete, exactly as would be committed upon approval. Paolo.
MIPS tests (nan-legacy.c and nans-legacy.c) failing
Maciej, When I run two of your new tests in gcc.target/mips (nan-legacy.c and nans-legacy.c), they are failing because my GCC is putting out .word 4294967295 instead of .word -1 like the test is expecting. I believe they are equivalent (0x) but I am not sure what it is about my targets (mips-mti-elf and mips-mti-linux-gnu) that would make this different from yours. Should the tests be modified to allow either output? Steve Ellcey sell...@mips.com
Re: [Patch] Refractor Thompson matcher
On Wed, Jul 31, 2013 at 7:18 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Please post the complete patch you intend to commit. Part of the GCC policy is also that all the patches must be posted complete, exactly as would be committed upon approval. Here it is. -- Tim Shen bfs.patch Description: Binary data
Re: [Patch] Refractor Thompson matcher
On 07/31/2013 01:26 AM, Tim Shen wrote: On Wed, Jul 31, 2013 at 7:18 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Please post the complete patch you intend to commit. Part of the GCC policy is also that all the patches must be posted complete, exactly as would be committed upon approval. Here it is. Ok, thanks. Normally the ChangeLog entries too are wrapped to 80 columns. Thanks again, Paolo.
Re: [PATCH 02/11] Generate pass-instances.def
On Mon, 2013-07-29 at 14:01 -0600, Jeff Law wrote: On 07/26/2013 09:04 AM, David Malcolm wrote: Introduce a new gen-pass-instances.awk script, and use it at build time to make a pass-instances.def from passes.def. An example of the result can be seen at: http://dmalcolm.fedorapeople.org/gcc/2013-07-25/pass-instances.def The generated pass-instances.def contains similar content to passes.def, but the pass instances within it are explicitly numbered, so that e.g. the third instance of: NEXT_PASS (pass_copy_prop) becomes: NEXT_PASS (pass_copy_prop, 3) This is needed by a subsequent patch so that we can create fields within the pipeline class for each pass instance, where we need unique field names to avoid a syntax error. For example, all 8 instances of pass_copy_prop will need different names. e.g. opt_pass *pass_copy_prop_1; ... opt_pass *pass_copy_prop_8; I have successfully tested the script with gawk, with gawk using the -c compatibility option to turn off gawk extensions, and with busybox awk (versions tested were gawk-4.0.1 and busybox-1.19.4). This patch replaces a previous attempt at this: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg00686.html which converted multi-instance passes to using a new NEXT_PASS_NUM macro, requiring the instance numbering within passes.def to be maintained by hand. In the new approach, the instance numbers are generated automatically, and are visible at build time, giving the uniqueness needed by later patches, whilst avoiding manual maintenance work, and also making it easy to see the instance numbering (by inspecting the generated pass-instances.def). gcc/ * Makefile.in (pass-instances.def): New. (passes.o): Replace dependency on passes.def with one on pass-instances.def * gen-pass-instances.awk: New. * passes.c (pipeline::pipeline): Use pass-instances.def rather than passes.def, updating local definition of NEXT_PASS macro to add an extra NUM parameter (currently unused). My awk-fu isn't all that great. I'm going to assume this works and that if it breaks, you own it :-) Fine for the trunk. Thanks. Committed to trunk as r201359, having double-checked that it bootstrapped by itself on top of what had gone before, and that the testsuite results were unaffected by it (on x86_64-unknown-linux-gnu).
Re: msp430 port
[nickc added for comments about the bits he wrote]
... define these as
(define_predicate msp_general_operand
(match_code mem,reg,subreg,const_int,const,symbol_ref,label_ref
{
int save_volatile_ok = volatile_ok;
volatile_ok = 1;
int ret = general_operand (op, mode);
volatile_ok = save_volatile_ok;
return ret;
})
That said, why do they exist?
Because gcc refuses to optimize operations with volatile memory
references, even when the target has opcodes that follow the volatile
memory rules. set bit in memory for example. I've had to do
something like this for every port I've written, for the same reason,
despite arguing against gcc's pedantry about volatile memory accesses.
+;; The next two patterns are here to support a feature of how GCC
implements
+;; varargs. When a function uses varargs and the *second* to last named
+;; argument is split between argument registers and the stack, gcc expects
the
+;; callee to allocate space on the stack that can contain the
register-based
+;; part of the argument. This space *has* to be just before the remaining
+;; arguments (ie the ones that are fully on the stack).
Ug. We ought to have been able to convince the compiler to copy the other
direction. I.e. create storage for that argument as if it were a regular
local
variable, spill the register portion into it, and also copy in the fragment
from the stack.
That sort of solution seems better than fiddling the location of the return
address in both directions.
Nick, this one is yours.
No constraints on splits.
Nor here.
Fixed. Perhaps gen* could error on those? I know they're ignored, I
just keep forgetting that splits are special.
(alternately, why can't they be included in the matching logic?)
+;; Alternatives 2 and 3 are to handle cases generated by reload.
+(define_insn subpsi3
+ [(set (match_operand:PSI0 nonimmediate_operand =r, rm,
?r, ?r)
+ (minus:PSI (match_operand:PSI 1 general_operand 0, 0, !r,
!i)
+ (match_operand:PSI 2 general_operand rLs, rmi, rmi,
r)))
+ (clobber (reg:CC CARRY))
Under what conditions does reload generate 2 3?
We don't handle these cases for i386, so I question why
you'd have to handle them here.
Hmmm... I took those alternatives out, and was still able to build
everything and test with no regressions. Very strange. Anyway, I'll
take those out.
+(define_insn *bicmode_cg
+ [(set (match_operand:QHI 0 msp_nonimmediate_operand =rYs,m)
+ (and:QHI (match_operand:QHI 1 msp_general_operand 0,0)
+(match_operand 2 msp430_inv_constgen_operator n,n)))]
+
+ @
+ BIC%x0%B0\t#%I2, %0
+ BIC%X0%B0\t#%I2, %0
+)
This really should be an alternative of
+(define_insn andmode3
+ [(set (match_operand:QHI 0 msp_nonimmediate_operand =rYs,rm)
+ (and:QHI (match_operand:QHI 1 msp_nonimmediate_operand %0,0)
+(match_operand:QHI 2 msp_general_operand riYs,rmi)))
+ (clobber (reg:CC CARRY))
+ ]
+
+ @
+ AND%x0%B0\t%2, %0
+ AND%X0%B0\t%2, %0
+)
... this.
The first doesn't have a clobber, though...
I suppose shrink-wrapping results in larger prologues and epilogues
in general? Otherwise I'd suggest using simple_return here.
../../gcc-trunkb/gcc/function.c: In function 'vecedge_def*, va_heap, vl_ptr
convert_jumps_to_returns(basic_block_def*, bool, vecedge_def*, va_heap,
vl_ptr)':
../../gcc-trunkb/gcc/function.c:5765: error:
'emit_use_return_register_into_block' was not declared in this scope
../../gcc-trunkb/gcc/function.c:5767: error: 'emit_return_into_block' was not
declared in this scope
../../gcc-trunkb/gcc/function.c:5797: error:
'emit_use_return_register_into_block' was not declared in this scope
../../gcc-trunkb/gcc/function.c: In function 'basic_block_def*
emit_return_for_exit(edge_def*, bool)':
../../gcc-trunkb/gcc/function.c:5839: error: 'emit_return_into_block' was not
declared in this scope
../../gcc-trunkb/gcc/function.c: In function 'void
thread_prologue_and_epilogue_insns()':
../../gcc-trunkb/gcc/function.c:6594: error: 'emit_return_into_block' was not
declared in this scope
../../gcc-trunkb/gcc/function.c:6625: error: 'emit_return_into_block' was not
declared in this scope
Do you find these reversed patterns actually used? In the deep
dark past one needed to provide them, but I thought we'd fixed the
middle-end not to generate them anymore...
It's not that kind of reverse. The MSP430 doesn't have a full
complement of comparisons; sometimes we need to use a reversed-operand
comparison to make up for a missing one:
; TRUE for comparisons we need to reverse.
(define_predicate msp430_reversible_cmp_operator
(match_code gt,gtu,le,leu))
+; TRUE for constants which are bit positions for zero_extract
+(define_predicate msp430_bitpos
+ (and (match_code const_int)
+ (match_test ( INTVAL (op) = 0
+
RE: [arm-embedded] Patch to define multilibs for arm embedded-4_8-branch
OK - Joey -Original Message- From: Terry Guo Sent: Wednesday, July 24, 2013 16:15 To: Joey Ye Cc: gcc-patches@gcc.gnu.org Subject: [arm-embedded] Patch to define multilibs for arm embedded-4_8- branch Hi Joey, This patch is to define multilibs for recently created embedded-4_8-branch. Is it OK to commit? BR, Terry 2013-07-24 Terry Guo terry@arm.com * configure.ac (with_multilib_list): Export its value. * Makefile.in (with_multilib_list): Import it from configure files. * configure: Regenerated. * config/arm/t-mlibs: New files to define multilibs. * config.gcc: Use above multilib fragment.
Re: [Patch] Refractor Thompson matcher
I found some other wired problems in that patch after committing. Probably need more time to work on it, so now I revert it :( -- Tim Shen
