Re: [PATCH, MIPS] fix MIPS16 jump table overflow
On Fri, Aug 24, 2012 at 10:46 PM, Richard Sandiford wrote: > Andrew Pinski writes: >> On Fri, Aug 24, 2012 at 10:08 PM, Andrew Pinski wrote: >>> On Wed, Aug 22, 2012 at 7:15 PM, Sandra Loosemore >>> wrote: On 08/21/2012 02:23 PM, Richard Sandiford wrote: > > > Would be nice to add a compile test for -mabi=64 just to make sure > that Pmode == DImode works. A copy of an existing test like > code-readable-1.c would be fine. I'm having problems with this part -- it seems like every combination of options with -mabi=64 I've tried with code-readable-1.c complains about something-or-another being incompatible. The closest I've come is "-mabi=64 -march=mips64 -msoft-float", which is accepted by the mipsisa32r2-sde-elf >>> >>> Did you test this at all on a mips64-*-* target? After this change >>> n64 jump tables are broken. >>> Before CASE_VECTOR_MODE was DImode for n64 . >>> >>> See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54371 for why it fails. >>> gpdword produces a double word for n64. >>> >>> For EABI64 it is ok to load 32bits because that the addresses are >>> 32bits but for n64, it is not ok. The load addresses are normally >>> above the 32bits boundary. >> >> I am testing a patch which changes CASE_VECTOR_MODE to be: >> #define CASE_VECTOR_MODE ((mips_abi == ABI_64) ? DImode : SImode) > > I think it should be: > > #define CASE_VECTOR_MODE \ > (TARGET_MIPS16_SHORT_JUMP_TABLES ? SImode : ptr_mode) > > #define CASE_VECTOR_SHORTEN_MODE(MIN, MAX, BODY) \ > (!TARGET_MIPS16_SHORT_JUMP_TABLES ? ptr_mode \ >: (MIN) >= -32768 && (MAX) < 32768 : HImode \ >: SImode) > > The point being that the TARGET_MIPS16_SHORT_JUMP_TABLES entries > are relative, so SImode would be correct there even for n64. > > I'd missed that CASE_VECTOR_MODE applied to tablejump as well > as casesi, sorry. I am testing the above patch right now. Thanks, Andrew
Re: [PATCH, MIPS] fix MIPS16 jump table overflow
Andrew Pinski writes: > On Fri, Aug 24, 2012 at 10:08 PM, Andrew Pinski wrote: >> On Wed, Aug 22, 2012 at 7:15 PM, Sandra Loosemore >> wrote: >>> On 08/21/2012 02:23 PM, Richard Sandiford wrote: Would be nice to add a compile test for -mabi=64 just to make sure that Pmode == DImode works. A copy of an existing test like code-readable-1.c would be fine. >>> >>> >>> I'm having problems with this part -- it seems like every combination of >>> options with -mabi=64 I've tried with code-readable-1.c complains about >>> something-or-another being incompatible. The closest I've come is "-mabi=64 >>> -march=mips64 -msoft-float", which is accepted by the mipsisa32r2-sde-elf >> >> Did you test this at all on a mips64-*-* target? After this change >> n64 jump tables are broken. >> Before CASE_VECTOR_MODE was DImode for n64 . >> >> See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54371 for why it fails. >> gpdword produces a double word for n64. >> >> For EABI64 it is ok to load 32bits because that the addresses are >> 32bits but for n64, it is not ok. The load addresses are normally >> above the 32bits boundary. > > I am testing a patch which changes CASE_VECTOR_MODE to be: > #define CASE_VECTOR_MODE ((mips_abi == ABI_64) ? DImode : SImode) I think it should be: #define CASE_VECTOR_MODE \ (TARGET_MIPS16_SHORT_JUMP_TABLES ? SImode : ptr_mode) #define CASE_VECTOR_SHORTEN_MODE(MIN, MAX, BODY) \ (!TARGET_MIPS16_SHORT_JUMP_TABLES ? ptr_mode \ : (MIN) >= -32768 && (MAX) < 32768 : HImode \ : SImode) The point being that the TARGET_MIPS16_SHORT_JUMP_TABLES entries are relative, so SImode would be correct there even for n64. I'd missed that CASE_VECTOR_MODE applied to tablejump as well as casesi, sorry. Richard
[PATCH] skipping tree.h in gdbinit.in
Hello Everyone, When I was GDBing cc1, the skip tree.h line was giving me error. When I replaced it with "skip file tree.h" it seem to work fine. Here is a patch to fix it. Thanks, Balaji V. Iyer. Index: gcc/gdbinit.in === --- gcc/gdbinit.in (revision 190623) +++ gcc/gdbinit.in (working copy) @@ -208,4 +208,4 @@ # These are used in accessor macros. # Note that this is added at the end because older gdb versions # do not understand the 'skip' command. -skip "tree.h" +skip file tree.h Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 190623) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,7 @@ +2012-08-25 Balaji V. Iyer + + * gdbinit.in (skip "tree.h"): Replaced with skip file tree.h. +
Re: [PATCH, MIPS] fix MIPS16 jump table overflow
On Fri, Aug 24, 2012 at 10:08 PM, Andrew Pinski wrote: > On Wed, Aug 22, 2012 at 7:15 PM, Sandra Loosemore > wrote: >> On 08/21/2012 02:23 PM, Richard Sandiford wrote: >>> >>> >>> Would be nice to add a compile test for -mabi=64 just to make sure >>> that Pmode == DImode works. A copy of an existing test like >>> code-readable-1.c would be fine. >> >> >> I'm having problems with this part -- it seems like every combination of >> options with -mabi=64 I've tried with code-readable-1.c complains about >> something-or-another being incompatible. The closest I've come is "-mabi=64 >> -march=mips64 -msoft-float", which is accepted by the mipsisa32r2-sde-elf > > Did you test this at all on a mips64-*-* target? After this change > n64 jump tables are broken. > Before CASE_VECTOR_MODE was DImode for n64 . > > See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54371 for why it fails. > gpdword produces a double word for n64. > > For EABI64 it is ok to load 32bits because that the addresses are > 32bits but for n64, it is not ok. The load addresses are normally > above the 32bits boundary. I am testing a patch which changes CASE_VECTOR_MODE to be: #define CASE_VECTOR_MODE ((mips_abi == ABI_64) ? DImode : SImode) Thanks, Andrew Pinski
Re: [PATCH, MIPS] fix MIPS16 jump table overflow
On Wed, Aug 22, 2012 at 7:15 PM, Sandra Loosemore wrote: > On 08/21/2012 02:23 PM, Richard Sandiford wrote: >> >> >> Would be nice to add a compile test for -mabi=64 just to make sure >> that Pmode == DImode works. A copy of an existing test like >> code-readable-1.c would be fine. > > > I'm having problems with this part -- it seems like every combination of > options with -mabi=64 I've tried with code-readable-1.c complains about > something-or-another being incompatible. The closest I've come is "-mabi=64 > -march=mips64 -msoft-float", which is accepted by the mipsisa32r2-sde-elf Did you test this at all on a mips64-*-* target? After this change n64 jump tables are broken. Before CASE_VECTOR_MODE was DImode for n64 . See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54371 for why it fails. gpdword produces a double word for n64. For EABI64 it is ok to load 32bits because that the addresses are 32bits but for n64, it is not ok. The load addresses are normally above the 32bits boundary. Thanks, Andrew Pinski
C++ PATCH for default template argument access control SFINAE
I noticed that the earlier work on access control SFINAE didn't handle default template arguments; we weren't checking their access against the right declarations at all. In order to do that, we need to generate the DECL to compare against in fn_type_unification, when we still know what accesses we need to check. Doing this complicated detection of excessive deduction recursion somewhat, since we really want to avoid treating excess recursion as a SFINAE failure; once we hit that error, we can't really generate any FUNCTION_DECLs. Tested x86_64-pc-linux-gnu, applying to trunk. commit 6bcfa6523137462da4d11ab418ddeec7fe265c82 Author: Jason Merrill Date: Sun Aug 5 11:23:56 2012 -0400 PR c++/51213 (again) * pt.c (deduction_tsubst_fntype): Remove. (fn_type_unification): Check deduction depth and call instantiate_template here. Handle default argument access checks. (determine_specialization): Suppress access control. (tsubst_decl): Check for excessive deduction depth. (recheck_decl_substitution): Make sure access control is on. (type_unification_real): Don't mess with access deferring here. (get_bindings): Adjust for fn_type_unification return type. * call.c (enum rejection_reason_code): Drop rr_template_instantiation. (template_instantiation_rejection): Remove. (struct rejection_reason): Change targs to num_targs. (template_unification_rejection, print_z_candidate): Adjust. (add_template_candidate_real): Adjust for fn_type_unification change. * class.c (resolve_address_of_overloaded_function): Likewise. * cp-tree.h: Adjust declaration. diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 148ef8f..3915738 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -451,7 +451,6 @@ enum rejection_reason_code { rr_arg_conversion, rr_bad_arg_conversion, rr_template_unification, - rr_template_instantiation, rr_invalid_copy }; @@ -485,7 +484,7 @@ struct rejection_reason { struct { tree tmpl; tree explicit_targs; - tree targs; + int num_targs; const tree *args; unsigned int nargs; tree return_type; @@ -688,7 +687,7 @@ template_unification_rejection (tree tmpl, tree explicit_targs, tree targs, struct rejection_reason *r = alloc_rejection (rr_template_unification); r->u.template_unification.tmpl = tmpl; r->u.template_unification.explicit_targs = explicit_targs; - r->u.template_unification.targs = targs; + r->u.template_unification.num_targs = TREE_VEC_LENGTH (targs); /* Copy args to our own storage. */ memcpy (args1, args, args_n_bytes); r->u.template_unification.args = args1; @@ -706,15 +705,6 @@ template_unification_error_rejection (void) } static struct rejection_reason * -template_instantiation_rejection (tree tmpl, tree targs) -{ - struct rejection_reason *r = alloc_rejection (rr_template_instantiation); - r->u.template_instantiation.tmpl = tmpl; - r->u.template_instantiation.targs = targs; - return r; -} - -static struct rejection_reason * invalid_copy_with_fn_template_rejection (void) { struct rejection_reason *r = alloc_rejection (rr_invalid_copy); @@ -2873,7 +2863,6 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl, unsigned int ia, ix; tree arg; struct z_candidate *cand; - int i; tree fn; struct rejection_reason *reason = NULL; int errs; @@ -2920,12 +2909,12 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl, gcc_assert (ia == nargs_without_in_chrg); errs = errorcount+sorrycount; - i = fn_type_unification (tmpl, explicit_targs, targs, - args_without_in_chrg, - nargs_without_in_chrg, - return_type, strict, flags, false); + fn = fn_type_unification (tmpl, explicit_targs, targs, + args_without_in_chrg, + nargs_without_in_chrg, + return_type, strict, flags, false); - if (i != 0) + if (fn == error_mark_node) { /* Don't repeat unification later if it already resulted in errors. */ if (errorcount+sorrycount == errs) @@ -2938,13 +2927,6 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl, goto fail; } - fn = instantiate_template (tmpl, targs, tf_none); - if (fn == error_mark_node) -{ - reason = template_instantiation_rejection (tmpl, targs); - goto fail; -} - /* In [class.copy]: A member function template is never instantiated to perform the @@ -3239,7 +3221,8 @@ print_z_candidate (location_t loc, const char *msgstr, inform (cloc, " template argument deduction/substitution failed:"); fn_type_unification (r->u.template_unification.tmpl, r->u.template_unification.explicit_targs, - r->u.template_unification.targs, + (make_tree_vec +(r->u.template_unification.num_targs)), r->u.template_unification.args, r->u.template_unification.nargs, r->u.template_unification.retur
Re: [C++Patch] PR 51421
OK. Jason
PATCH to print_node to avoid ICE on TREE_VEC
Now that TREE_LANG_FLAG_0 doesn't work for TREE_VEC, we shouldn't try to use it. Tested x86_64-pc-linux-gnu, applying to trunk as obvious. commit 3a2398e3485f19d74cd9ea74301ef9da62680be8 Author: Jason Merrill Date: Fri Aug 24 16:37:51 2012 -0400 * print-tree.c (print_node): Don't check TREE_LANG_FLAG_* on TREE_VEC or SSA_NAME. diff --git a/gcc/print-tree.c b/gcc/print-tree.c index d40fb08..27fb72f 100644 --- a/gcc/print-tree.c +++ b/gcc/print-tree.c @@ -363,20 +363,24 @@ print_node (FILE *file, const char *prefix, tree node, int indent) fputs (" deprecated", file); if (TREE_VISITED (node)) fputs (" visited", file); - if (TREE_LANG_FLAG_0 (node)) -fputs (" tree_0", file); - if (TREE_LANG_FLAG_1 (node)) -fputs (" tree_1", file); - if (TREE_LANG_FLAG_2 (node)) -fputs (" tree_2", file); - if (TREE_LANG_FLAG_3 (node)) -fputs (" tree_3", file); - if (TREE_LANG_FLAG_4 (node)) -fputs (" tree_4", file); - if (TREE_LANG_FLAG_5 (node)) -fputs (" tree_5", file); - if (TREE_LANG_FLAG_6 (node)) -fputs (" tree_6", file); + + if (code != TREE_VEC && code != SSA_NAME) +{ + if (TREE_LANG_FLAG_0 (node)) + fputs (" tree_0", file); + if (TREE_LANG_FLAG_1 (node)) + fputs (" tree_1", file); + if (TREE_LANG_FLAG_2 (node)) + fputs (" tree_2", file); + if (TREE_LANG_FLAG_3 (node)) + fputs (" tree_3", file); + if (TREE_LANG_FLAG_4 (node)) + fputs (" tree_4", file); + if (TREE_LANG_FLAG_5 (node)) + fputs (" tree_5", file); + if (TREE_LANG_FLAG_6 (node)) + fputs (" tree_6", file); +} /* DECL_ nodes have additional attributes. */
C++ PATCH to improve template diagnostic locations
Another change I have been working on was causing some error messages to be associated with different lines. To maintain good locations, I've changed a few places to make sure that some expressions have a location, and use those locations more consistently during instantiation. Since this means that default argument instantiation errors should now be associated with the line that the default argument is on, I've added an explanatory note to indicate what triggered the instantiation. Tested x86_64-pc-linux-gnu, applying to trunk. commit 08a0ed0f115e56657103342f27c0455b74b4db6e Author: Jason Merrill Date: Mon Aug 13 19:33:27 2012 -0400 * pt.c (tsubst_default_argument): Indicate where the default argument is being instantiated for. (tsubst_expr): Restore previous location. (tsubst_copy_and_build): Set and restore location. * call.c (build_new_method_call_1): Remember location of call. * semantics.c (finish_call_expr): Here too. * parser.c (cp_parser_omp_for_loop): Remember the location of the increment expression. diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 5d5899f..148ef8f 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -7537,6 +7537,7 @@ build_new_method_call_1 (tree instance, tree fns, VEC(tree,gc) **args, build_min (COMPONENT_REF, TREE_TYPE (CALL_EXPR_FN (call)), orig_instance, orig_fns, NULL_TREE), orig_args)); + SET_EXPR_LOCATION (call, input_location); call = convert_from_reference (call); if (cast_to_void) call = build_nop (void_type_node, call); diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index a7f12ba..0f897c9 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -26603,6 +26603,8 @@ cp_parser_omp_for_loop (cp_parser *parser, tree clauses, tree *par_clauses) incr = cp_parser_omp_for_incr (parser, real_decl); else incr = cp_parser_expression (parser, false, NULL); + if (CAN_HAVE_LOCATION_P (incr) && !EXPR_HAS_LOCATION (incr)) + SET_EXPR_LOCATION (incr, input_location); } if (!cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)) diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index eff0b4d..6c9d143 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -9609,6 +9609,7 @@ tsubst_default_argument (tree fn, tree type, tree arg) { tree saved_class_ptr = NULL_TREE; tree saved_class_ref = NULL_TREE; + int errs = errorcount + sorrycount; /* This can happen in invalid code. */ if (TREE_CODE (arg) == DEFAULT_ARG) @@ -9656,6 +9657,10 @@ tsubst_default_argument (tree fn, tree type, tree arg) cp_function_chain->x_current_class_ref = saved_class_ref; } + if (errorcount+sorrycount > errs) +inform (input_location, + " when instantiating default argument for call to %D", fn); + /* Make sure the default argument is reasonable. */ arg = check_default_argument (type, arg); @@ -12496,15 +12501,19 @@ static tree tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl, bool integral_constant_expression_p) { +#define RETURN(EXP) do { r = (EXP); goto out; } while(0) #define RECUR(NODE)\ tsubst_expr ((NODE), args, complain, in_decl, \ integral_constant_expression_p) tree stmt, tmp; + tree r; + location_t loc; if (t == NULL_TREE || t == error_mark_node) return t; + loc = input_location; if (EXPR_HAS_LOCATION (t)) input_location = EXPR_LOCATION (t); if (STATEMENT_CODE_P (TREE_CODE (t))) @@ -13016,42 +13025,46 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl, stmt = build_transaction_expr (EXPR_LOCATION (t), RECUR (TRANSACTION_EXPR_BODY (t)), flags, NULL_TREE); -return stmt; +RETURN (stmt); } } break; case MUST_NOT_THROW_EXPR: - return build_must_not_throw_expr (RECUR (TREE_OPERAND (t, 0)), - RECUR (MUST_NOT_THROW_COND (t))); + RETURN (build_must_not_throw_expr (RECUR (TREE_OPERAND (t, 0)), + RECUR (MUST_NOT_THROW_COND (t; case EXPR_PACK_EXPANSION: error ("invalid use of pack expansion expression"); - return error_mark_node; + RETURN (error_mark_node); case NONTYPE_ARGUMENT_PACK: error ("use %<...%> to expand argument pack"); - return error_mark_node; + RETURN (error_mark_node); case COMPOUND_EXPR: tmp = RECUR (TREE_OPERAND (t, 0)); if (tmp == NULL_TREE) /* If the first operand was a statement, we're done with it. */ - return RECUR (TREE_OPERAND (t, 1)); - return build_x_compound_expr (EXPR_LOCATION (t), tmp, + RETURN (RECUR (TREE_OPERAND (t, 1))); + RETURN (build_x_compound_expr (EXPR_LOCATION (t), tmp, RECUR (TREE_OPERAND (t, 1)), -complain); +complain)); default: gcc_assert (!STATEMENT_CODE_P (TREE_CODE (t))); - return tsubst_copy_and_build (t, args, complain, in_decl, + RETURN (tsubst_copy_and_build (t, args, compl
Re: [doc] Fix typo in gty.texi
On Thu, 23 Aug 2012, Mingjie Xing wrote: > A very small patch. OK? > > 2012-08-23 Mingjie Xing > > * doc/gty.texi: Fix typo. By the way, changes of this kind, you can just commit and post the patch; they do not require approval. (When in doubt feel free to ask, of course.) Gerald
Re: [google] Modification of gcov pmu format to reduce gcda size bloat (issue 6427063)
On Fri, Aug 24, 2012 at 3:56 PM, wrote: > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h > File gcc/gcov-io.h (right): > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode688 > gcc/gcov-io.h:688: gcov_unsigned_t index; /* The corresponding string > table index */ > On 2012/08/24 22:30:30, davidxl wrote: >> >> Is this field necessary? > > > For the purposes of gcov_dump output we wanted to output not only the > string table entry but also its index in the string table just in case > there were any problems with ordering that might map filetags to the > wrong filename. Because of this and the fact that gcov.c and gcov-dump.c > read one entry at a time, it seemed better to have the index be > self-contained within the struct to avoid extra logic. That is fine -- but you probably need to add assertion check when the string table is read in. > > Also, since the string table entry has to be written with its > corresponding index for gcov-dump, the gcov_write_string_table_entry > function could have a similar syntax to the ll/brm writing functions. > > > http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode689 > gcc/gcov-io.h:689: char* filename; /* The filename that belongs > at this index */ > On 2012/08/24 22:30:30, davidxl wrote: >> >> Can this field name be generalized? > > > Generalized in what way? I used this name because it was used before in > the old gcda format. Since it is string table, it can be something like str_ or strval_ etc. > > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c > File libgcc/pmu-profile.c (right): > > http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c#newcode83 > libgcc/pmu-profile.c:83: > On 2012/08/24 22:30:30, davidxl wrote: >> >> Who is the caller to this interface? > > >> It should be declared in gcov-io.h > > > The only current caller of this is > https://critique.corp.google.com/#review/31972005. This is also the same > for the other two ll/brm writing functions. Should I declare those in > gcov-io.h as well? Any utilities functions that may be be used by clients other than gcov tool should be in the common header. David > > http://codereview.appspot.com/6427063/
[patch] Make every GIMPLE switch have a default case, always
Hello, This patch restores the old invariant that every GIMPLE switch has a default case. This invariant is only broken by the SJLJ exception dispatch code, and it's resulted in some code accepting a switch without a default while others still assume there is _always_ a default case. The patch enforces the invariant, fixes some fall-out, and cleans up the code in a couple of places. It makes the follow-up work on switch code generation that I still have planned a bit easier. Bootstrapped&tested on x86_64-unknown-linux-gnu (with Java to torture the exception handling code) and did a non-bootstrap build (including Java again) with --enable-sjlj-exceptions. OK for trunk? Ciao! Steven 00_gimple_switch_default.diff Description: Binary data
Re: [google] Modification of gcov pmu format to reduce gcda size bloat (issue 6427063)
http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h File gcc/gcov-io.h (right): http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode688 gcc/gcov-io.h:688: gcov_unsigned_t index; /* The corresponding string table index */ On 2012/08/24 22:30:30, davidxl wrote: Is this field necessary? For the purposes of gcov_dump output we wanted to output not only the string table entry but also its index in the string table just in case there were any problems with ordering that might map filetags to the wrong filename. Because of this and the fact that gcov.c and gcov-dump.c read one entry at a time, it seemed better to have the index be self-contained within the struct to avoid extra logic. Also, since the string table entry has to be written with its corresponding index for gcov-dump, the gcov_write_string_table_entry function could have a similar syntax to the ll/brm writing functions. http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode689 gcc/gcov-io.h:689: char* filename; /* The filename that belongs at this index */ On 2012/08/24 22:30:30, davidxl wrote: Can this field name be generalized? Generalized in what way? I used this name because it was used before in the old gcda format. http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c File libgcc/pmu-profile.c (right): http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c#newcode83 libgcc/pmu-profile.c:83: On 2012/08/24 22:30:30, davidxl wrote: Who is the caller to this interface? It should be declared in gcov-io.h The only current caller of this is https://critique.corp.google.com/#review/31972005. This is also the same for the other two ll/brm writing functions. Should I declare those in gcov-io.h as well? http://codereview.appspot.com/6427063/
Re: [PATCH] Fix emit_conditional_add and documentation for add@var{mode}cc
Forgot to attach the patch. -- Andrew On Fri, Aug 24, 2012 at 3:42 PM, Andrew Pinski wrote: > Hi, > I decided to split this patch from the other patch which uses > emit_conditional_add in expand as that part of the patch needs some > work. This part of the patch can be applied separately and it fixes a > few things dealing with conditional adds. > > First the documentation is wrong for the pattern as we do the addition > if operand 0 is true rather than false. > Then emit_conditional_add is wrong as you cannot switch around op2 and op3. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > Thanks, > Andrew Pinski > > ChangeLog: > * optabs.c (emit_conditional_add): Correct comment about the arguments. > Remove code which might swap op2 and op3 since they cannot be swapped. > * doc/md.texi (add@var{mode}cc): Fix document about how the arguments are > used. Index: doc/md.texi === --- doc/md.texi (revision 190657) +++ doc/md.texi (working copy) @@ -5250,7 +5250,7 @@ define these patterns. @item @samp{add@var{mode}cc} Similar to @samp{mov@var{mode}cc} but for conditional addition. Conditionally move operand 2 or (operands 2 + operand 3) into operand 0 according to the -comparison in operand 1. If the comparison is true, operand 2 is moved into +comparison in operand 1. If the comparison is false, operand 2 is moved into operand 0, otherwise (operand 2 + operand 3) is moved. @cindex @code{cstore@var{mode}4} instruction pattern Index: optabs.c === --- optabs.c(revision 190657) +++ optabs.c(working copy) @@ -4584,7 +4584,7 @@ can_conditionally_move_p (enum machine_m the mode to use should they be constants. If it is VOIDmode, they cannot both be constants. - OP2 should be stored in TARGET if the comparison is true, otherwise OP2+OP3 + OP2 should be stored in TARGET if the comparison is false, otherwise OP2+OP3 should be stored there. MODE is the mode to use should they be constants. If it is VOIDmode, they cannot both be constants. @@ -4598,7 +4598,6 @@ emit_conditional_add (rtx target, enum r { rtx tem, comparison, last; enum insn_code icode; - enum rtx_code reversed; /* If one operand is constant, make it the second one. Only do this if the other operand is not constant as well. */ @@ -4622,16 +4621,6 @@ emit_conditional_add (rtx target, enum r if (cmode == VOIDmode) cmode = GET_MODE (op0); - if (swap_commutative_operands_p (op2, op3) - && ((reversed = reversed_comparison_code_parts (code, op0, op1, NULL)) - != UNKNOWN)) -{ - tem = op2; - op2 = op3; - op3 = tem; - code = reversed; -} - if (mode == VOIDmode) mode = GET_MODE (op2);
[PATCH] Fix emit_conditional_add and documentation for add@var{mode}cc
Hi, I decided to split this patch from the other patch which uses emit_conditional_add in expand as that part of the patch needs some work. This part of the patch can be applied separately and it fixes a few things dealing with conditional adds. First the documentation is wrong for the pattern as we do the addition if operand 0 is true rather than false. Then emit_conditional_add is wrong as you cannot switch around op2 and op3. OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. Thanks, Andrew Pinski ChangeLog: * optabs.c (emit_conditional_add): Correct comment about the arguments. Remove code which might swap op2 and op3 since they cannot be swapped. * doc/md.texi (add@var{mode}cc): Fix document about how the arguments are used.
Re: [google] Modification of gcov pmu format to reduce gcda size bloat (issue 6427063)
http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h File gcc/gcov-io.h (right): http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode688 gcc/gcov-io.h:688: gcov_unsigned_t index; /* The corresponding string table index */ Is this field necessary? http://codereview.appspot.com/6427063/diff/11002/gcc/gcov-io.h#newcode689 gcc/gcov-io.h:689: char* filename; /* The filename that belongs at this index */ Can this field name be generalized? http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c File libgcc/pmu-profile.c (right): http://codereview.appspot.com/6427063/diff/11002/libgcc/pmu-profile.c#newcode83 libgcc/pmu-profile.c:83: Who is the caller to this interface? It should be declared in gcov-io.h http://codereview.appspot.com/6427063/
Re: [google] Modification of gcov pmu format to reduce gcda size bloat (issue 6427063)
On 2012/08/24 21:51:24, cmang wrote: Fixed formatting issues. The gcov.c is still not uploaded properly. === --- gcc/gcov.c (revision 190359) +++ gcc/gcov.c (working copy) @@ -222,6 +222,7 @@ typedef struct pmu_data { ll_infos_t ll_infos; brm_infos_t brm_infos; + string_table_t string_table; } pmu_data_t; /* Describes a single line of source. Contains a chain of basic blocks @@ -975,6 +976,7 @@ release_structures (void) function_t *fn; ll_infos_t *ll_infos = &pmu_global_info.ll_infos; brm_infos_t *brm_infos = &pmu_global_info.brm_infos; + string_table_t *string_table = &pmu_global_info.string_table; for (ix = n_sources; ix--;) { @@ -1008,8 +1010,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < ll_infos->ll_count; ++i) { - if (ll_infos->ll_array[i]->filename) -XDELETE (ll_infos->ll_array[i]->filename); XDELETE (ll_infos->ll_array[i]); } /* delete the array itself */ @@ -1026,8 +1026,6 @@ release_structures (void) /* delete each element */ for (i = 0; i < brm_infos->brm_count; ++i) { - if (brm_infos->brm_array[i]->filename) -XDELETE (brm_infos->brm_array[i]->filename); XDELETE (brm_infos->brm_array[i]); } /* delete the array itself */ @@ -1035,6 +1033,22 @@ release_structures (void) brm_infos->brm_array = NULL; brm_infos->brm_count = 0; } + + /* Cleanup PMU string table entries. */ + if (string_table->st_count) +{ + unsigned i; + + /* delete each element */ + for (i = 0; i < string_table->st_count; ++i) +{ + XDELETE (string_table->st_array[i]); +} Remove {}. http://codereview.appspot.com/6427063/
[C++Patch] PR 51421
Hi, a rather simple issue. In mainline we have machinery to detect broken uses of auto variables before deduction, for which mark_used is designed to error out and return false to the caller. However, in the specific cases pointed out in this PR the error recovery is not Ok, because mark_used doesn't return false after error and anyway the caller - finish_id_expression - doesn't check the return value. Tested x86_64-linux. Thanks, Paolo. // /cp 2012-08-25 Paolo Carlini PR c++/51421 * decl2.c (mark_used): Consistently return false after errors about uses before deduction of auto. * semantics.c (finish_id_expression): Check mark_used return value and return error_mark_node in case of failure. /testsuite 2012-08-25 Paolo Carlini PR c++/51421 * g++.dg/cpp0x/auto34.C: New. Index: testsuite/g++.dg/cpp0x/auto34.C === --- testsuite/g++.dg/cpp0x/auto34.C (revision 0) +++ testsuite/g++.dg/cpp0x/auto34.C (revision 0) @@ -0,0 +1,18 @@ +// PR c++/51421 +// { dg-do compile { target c++11 } } + +int foo1(int); + +void bar1() +{ + auto i = foo1(i); // { dg-error "before deduction" } +} + +struct A {}; + +A foo2(A); + +void bar2() +{ + auto a = foo2(a); // { dg-error "before deduction" } +} Index: cp/decl2.c === --- cp/decl2.c (revision 190649) +++ cp/decl2.c (working copy) @@ -4238,7 +4238,10 @@ mark_used (tree decl) || DECL_THUNK_P (decl)) { if (!processing_template_decl && type_uses_auto (TREE_TYPE (decl))) - error ("use of %qD before deduction of %", decl); + { + error ("use of %qD before deduction of %", decl); + return false; + } return true; } @@ -4284,7 +4287,10 @@ mark_used (tree decl) } if (type_uses_auto (TREE_TYPE (decl))) -error ("use of %qD before deduction of %", decl); +{ + error ("use of %qD before deduction of %", decl); + return false; +} /* If we don't need a value, then we don't need to synthesize DECL. */ if (cp_unevaluated_operand != 0) Index: cp/semantics.c === --- cp/semantics.c (revision 190649) +++ cp/semantics.c (working copy) @@ -3220,11 +3220,12 @@ finish_id_expression (tree id_expression, /* Mark variable-like entities as used. Functions are similarly marked either below or after overload resolution. */ - if (TREE_CODE (decl) == VAR_DECL - || TREE_CODE (decl) == PARM_DECL - || TREE_CODE (decl) == CONST_DECL - || TREE_CODE (decl) == RESULT_DECL) - mark_used (decl); + if ((TREE_CODE (decl) == VAR_DECL + || TREE_CODE (decl) == PARM_DECL + || TREE_CODE (decl) == CONST_DECL + || TREE_CODE (decl) == RESULT_DECL) + && !mark_used (decl)) + return error_mark_node; /* Only certain kinds of names are allowed in constant expression. Template parameters have already
[google] Modification of gcov pmu format to reduce gcda size bloat (issue6427063)
Fixed formatting issues. The patch should be applied to google/main Tested with crosstools. 2012-08-24 Chris Manghane * libgcc/pmu-profile.c (gcov_write_load_latency_infos): Removed unused function. (gcov_write_branch_mispredict_infos): Ditto. (destroy_load_latency_infos): Removed static keyword. (init_pmu_branch_mispredict): Ditto. (gcov_write_ll_line): Ditto, plus replaced filename field with filetag. (gcov_write_branch_mispredict_line): Ditto. (gcov_write_string_table_entry): New function. (gcov_write_tool_header): Removed static keyword. * gcc/gcov.c (release_structures): Removed filename field from PMU structures. (filter_pmu_data_lines): Added PMU string table support. (process_pmu_profile): Ditto. * gcc/gcov-io.c (gcov_read_pmu_load_latency_info): Replaced filename field with filetag. (gcov_read_pmu_branch_mispredict_info): Ditto. (gcov_read_pmu_string_table_entry): New Function. (print_load_latency_line): Replaced filename field with filetag. (print_branch_mispredict_line): Ditto. (print_string_table_entry): New function. * gcc/gcov-io.h (GCOV_TAG_PMU_LOAD_LATENCY_LENGTH): Replaced filename field with filetag (GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH): Ditto. (GCOV_TAG_PMU_STRING_TABLE_ENTRY): Added new tag. (GCOV_TAG_PMU_STRING_TABLE_ENTRY_LENGTH): Ditto. (gcov_pmu_load_latency_info): Replaced filename field with filetag. (gcov_pmu_branch_mispredict_info): Ditto. (gcov_pmu_string_table_entry): New struct. (gcov_pmu_string_table): New struct. * gcc/gcov-dump.c (tag_pmu_load_latency_info): Removed PMU filename field. (tag_pmu_branch_mispredict_info): Ditto. (tag_pmu_string_table_entry): New function. Index: libgcc/pmu-profile.c === --- libgcc/pmu-profile.c(revision 190362) +++ libgcc/pmu-profile.c(working copy) @@ -1,3 +1,4 @@ + /* Performance monitoring unit (PMU) profiler. If available, use an external tool to collect hardware performance counter data and write it in the .gcda files. @@ -74,13 +75,13 @@ static void destroy_load_latency_infos (void *info static void destroy_branch_mispredict_infos (void *info); static gcov_unsigned_t gcov_tag_pmu_tool_header_length (gcov_pmu_tool_header_t *header); -static void gcov_write_tool_header (gcov_pmu_tool_header_t *header); -static void gcov_write_load_latency_infos (void *info); -static void gcov_write_branch_mispredict_infos (void *info); -static void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); -static void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t +void gcov_write_tool_header (gcov_pmu_tool_header_t *header); +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info); +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info); +void gcov_write_string_table_entry (const gcov_pmu_st_entry_t *st_entry); + /* Convert a fractional PCT to an unsigned integer after muliplying by 100. */ @@ -156,11 +157,11 @@ init_pmu_branch_mispredict (void) /* Write the load latency information LL_INFO into the gcda file. */ -static void +void gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_LOAD_LATENCY_LENGTH (ll_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_LOAD_LATENCY_INFO, + GCOV_TAG_PMU_LOAD_LATENCY_LENGTH()); gcov_write_unsigned (ll_info->counts); gcov_write_unsigned (ll_info->self); gcov_write_unsigned (ll_info->cum); @@ -174,27 +175,38 @@ gcov_write_ll_line (const gcov_pmu_ll_info_t *ll_i gcov_write_counter (ll_info->code_addr); gcov_write_unsigned (ll_info->line); gcov_write_unsigned (ll_info->discriminator); - gcov_write_string (ll_info->filename); + gcov_write_unsigned (ll_info->filetag); } /* Write the branch mispredict information BRM_INFO into the gcda file. */ -static void +void gcov_write_branch_mispredict_line (const gcov_pmu_brm_info_t *brm_info) { - gcov_unsigned_t len = GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH ( - brm_info->filename); - gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, len); + gcov_write_tag_length (GCOV_TAG_PMU_BRANCH_MISPREDICT_INFO, + GCOV_TAG_PMU_BRANCH_MISPREDICT_LENGTH()); gcov_write_unsigned (brm_info->counts); gcov_write_unsigned (brm_info->self); gcov_write_unsigned (brm_info->cum); gcov_write_counter (brm_info->code_addr); gcov_write_unsigned (brm_info->line); gcov_write_unsigned (brm_info->discriminator); - gcov_write_string (brm_info->filename); + gcov_write_unsigned
Re: Merge C++ conversion into trunk (5/6 - double_int rewrite)
On Aug 12, 2012, at 1:15 PM, Diego Novillo wrote: > + Second, the GCC conding conventions prefer explicit conversion, Spelling... coding
Re: [PATCH] Don't ICE if COMPOUND_LITERAL_EXPR's DECL_INITIAL isn't CONSTRUCTOR (PR c/54363)
On Fri, 24 Aug 2012, Jakub Jelinek wrote: > Hi! > > On this testcase, we ICE in optimize_compound_literals_in_ctor > because init isn't CONSTRUCTOR, but the recursive call relies on it > being a CONSTRUCTOR. > > Either we can add that check as the patch does, making the optimization > tiny bit more robust (the rest of the gimplifier handles non-CONSTRUCTOR > DECL_INITIAL of COMPOUND_LITERAL_EXPR just fine), or the C FE would need to > be somehow fixed to always emit a CONSTRUCTOR. > > Joseph, what do you prefer here? I prefer the gimplifier approach here (allowing COMPOUND_LITERAL_EXPRs to use anything that would be a valid initializer for the relevant type). -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] Fix ICE on invalid (PR c/54355)
On Fri, 24 Aug 2012, Jakub Jelinek wrote: > 2012-08-23 Jakub Jelinek > > PR c/54355 > * c-decl.c (c_parser_label): Pass true as nested and fix up comments > for nested and empty_ok arguments in the call to > c_parser_declaration_or_fndef. > > * gcc.dg/pr54355.c: New test. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [google] Modification of gcov pmu format to reduce gcda size bloat (issue 6427063)
I don't see any code where a string table is declared nor created ... David On Fri, Aug 24, 2012 at 2:26 PM, Chris Manghane wrote: > Also, what did you mean by there being missing string table management code? > > > On Fri, Aug 24, 2012 at 2:23 PM, wrote: >> >> >> Ok, I fixed most of the problem you found. I'm not sure what happened >> with gcov.c, but I'll try uploading it again. >> >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c >> File gcc/gcov-io.c (right): >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode280 >> gcc/gcov-io.c:280: gcov_read_pmu_string_table_entry >> (gcov_pmu_st_entry_t* st_entry, >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> Fix format: >> >> >>> ..entry_t *st_entry, >> >> >> Done. >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode281 >> gcc/gcov-io.c:281: gcov_unsigned_t len ATTRIBUTE_UNUSED) >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> Why having an unused parameter? Can it be used in assertion check? >> >> >> I'm following the format of the pmu_branch_mispredict/load_latency_info >> function defined above. Should I remove the ATTRIBUTE_UNUSED symbol from >> them as well? >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode830 >> gcc/gcov-io.c:830: print_pmu_string_table_entry (FILE *fp, const >> gcov_pmu_st_entry_t* st_entry, >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> Fix format. >> >> >> Done. >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode831 >> gcc/gcov-io.c:831: const enum print_newline newline) { >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> '{' goes to the new line. >> >> >> Done. >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h >> File gcc/gcov-io.h (right): >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode699 >> gcc/gcov-io.h:699: Used for bookkeeping. */ >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> typo. >> >> >> Sorry, I don't notice the typo here. This line is copied from >> load_latency/branch_mispredict_infos. >> >> >> http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode916 >> gcc/gcov-io.h:916: const enum print_newline); >> On 2012/08/24 20:42:03, davidxl wrote: >>> >>> Fix indentation. >> >> >> Done. >> >> http://codereview.appspot.com/6427063/ > >
[PATCH] Don't ICE if COMPOUND_LITERAL_EXPR's DECL_INITIAL isn't CONSTRUCTOR (PR c/54363)
Hi! On this testcase, we ICE in optimize_compound_literals_in_ctor because init isn't CONSTRUCTOR, but the recursive call relies on it being a CONSTRUCTOR. Either we can add that check as the patch does, making the optimization tiny bit more robust (the rest of the gimplifier handles non-CONSTRUCTOR DECL_INITIAL of COMPOUND_LITERAL_EXPR just fine), or the C FE would need to be somehow fixed to always emit a CONSTRUCTOR. Joseph, what do you prefer here? Bootstrapped/regtested on x86_64-linux and i686-linux. 2012-08-24 Jakub Jelinek PR c/54363 * gimplify.c (optimize_compound_literals_in_ctor): Only recurse if init is a CONSTRUCTOR. * gcc.dg/pr54363.c: New test. --- gcc/gimplify.c.jj 2012-08-15 10:55:33.0 +0200 +++ gcc/gimplify.c 2012-08-24 11:01:55.253815273 +0200 @@ -3857,7 +3857,8 @@ optimize_compound_literals_in_ctor (tree if (!TREE_ADDRESSABLE (value) && !TREE_ADDRESSABLE (decl) - && init) + && init + && TREE_CODE (init) == CONSTRUCTOR) newval = optimize_compound_literals_in_ctor (init); } if (newval == value) --- gcc/testsuite/gcc.dg/pr54363.c.jj 2012-08-24 11:34:49.857494287 +0200 +++ gcc/testsuite/gcc.dg/pr54363.c 2012-08-24 11:36:20.943061045 +0200 @@ -0,0 +1,12 @@ +/* PR c/54363 */ +/* { dg-do compile } */ +/* { dg-options "-std=gnu99" } */ + +struct S { char **a; }; + +void +test (void) +{ + struct S b = { .a = (char **) { "a", "b" } }; /* { dg-warning "(initialization|excess elements)" } */ + struct S c = { .a = (char *[]) { "a", "b" } }; +} Jakub
[C PATCH] Fix ICE on invalid (PR c/54355)
Hi! When the c_parser_declaration_or_fndef call has been added for error handling of invalid label followed by decl, the comments for the two arguments were swapped, and I believe nested needs to be passed as true, this is inside of another function. Testing revealed that empty_ok=true is also desirable, for the 20031223-1.c testcase. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2012-08-23 Jakub Jelinek PR c/54355 * c-decl.c (c_parser_label): Pass true as nested and fix up comments for nested and empty_ok arguments in the call to c_parser_declaration_or_fndef. * gcc.dg/pr54355.c: New test. --- gcc/c/c-parser.c.jj 2012-08-17 17:42:25.315617706 +0200 +++ gcc/c/c-parser.c2012-08-24 20:38:57.445151457 +0200 @@ -4327,7 +4327,7 @@ c_parser_label (c_parser *parser) "a declaration is not a statement"); c_parser_declaration_or_fndef (parser, /*fndef_ok*/ false, /*static_assert_ok*/ true, -/*nested*/ true, /*empty_ok*/ false, +/*empty_ok*/ true, /*nested*/ true, /*start_attr_ok*/ true, NULL); } } --- gcc/testsuite/gcc.dg/pr54355.c.jj 2012-08-24 12:14:25.973223541 +0200 +++ gcc/testsuite/gcc.dg/pr54355.c 2012-08-24 12:14:25.973223541 +0200 @@ -0,0 +1,11 @@ +/* PR c/54355 */ +/* { dg-do compile } */ + +void +foo (int i) +{ + switch (i) + { + case 0: T x > /* { dg-error "(label|unknown type|expected)" } */ + } +} /* { dg-error "expected" } */ Jakub
Re: [google] Modification of gcov pmu format to reduce gcda size bloat (issue 6427063)
Ok, I fixed most of the problem you found. I'm not sure what happened with gcov.c, but I'll try uploading it again. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c File gcc/gcov-io.c (right): http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode280 gcc/gcov-io.c:280: gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t* st_entry, On 2012/08/24 20:42:03, davidxl wrote: Fix format: ..entry_t *st_entry, Done. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode281 gcc/gcov-io.c:281: gcov_unsigned_t len ATTRIBUTE_UNUSED) On 2012/08/24 20:42:03, davidxl wrote: Why having an unused parameter? Can it be used in assertion check? I'm following the format of the pmu_branch_mispredict/load_latency_info function defined above. Should I remove the ATTRIBUTE_UNUSED symbol from them as well? http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode830 gcc/gcov-io.c:830: print_pmu_string_table_entry (FILE *fp, const gcov_pmu_st_entry_t* st_entry, On 2012/08/24 20:42:03, davidxl wrote: Fix format. Done. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode831 gcc/gcov-io.c:831: const enum print_newline newline) { On 2012/08/24 20:42:03, davidxl wrote: '{' goes to the new line. Done. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h File gcc/gcov-io.h (right): http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode699 gcc/gcov-io.h:699: Used for bookkeeping. */ On 2012/08/24 20:42:03, davidxl wrote: typo. Sorry, I don't notice the typo here. This line is copied from load_latency/branch_mispredict_infos. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode916 gcc/gcov-io.h:916: const enum print_newline); On 2012/08/24 20:42:03, davidxl wrote: Fix indentation. Done. http://codereview.appspot.com/6427063/
Re: [PATCH, testsuite] No short enum in tree-ssa test
On 08/23/2012 08:05 PM, Joey Ye wrote: > Ssa-dom-thread-3.c has following code to trigger a warning on ARM. Add > -fno-short-enums to suppress it. > struct tree_base > { > enum tree_code code:16; > }; > > OK to trunk and 4.7? OK. Janis > 2012-08-15 Joey Ye > > * gcc.dg/tree-ssa/ssa-dom-thread-3.c: Add -fno-short-enums. > > Index: testsuite/gcc.dg/tree-ssa/ssa-dom-thread-3.c > === > --- testsuite/gcc.dg/tree-ssa/ssa-dom-thread-3.c (revision 190337) > +++ testsuite/gcc.dg/tree-ssa/ssa-dom-thread-3.c (working copy) > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-dom1-details" } */ > +/* { dg-options "-O2 -fdump-tree-dom1-details -fno-short-enums" } */ > extern void abort (void) __attribute__ ((__noreturn__)); > union tree_node; > typedef union tree_node *tree; > > > > > > >
Re: VxWorks Patches Back from the Dead!
Hi Robert, If you are going to defer, then: On Fri, Aug 24, 2012 at 1:20 PM, rbmj wrote: > diff --git a/fixincludes/fixinc.in b/fixincludes/fixinc.in > index e73aed9..de7be35 100755 > --- a/fixincludes/fixinc.in > +++ b/fixincludes/fixinc.in > @@ -128,6 +128,18 @@ fi > > # # # # # # # # # # # # # # # # # # # # # > # > +# Check to see if the machine_name fix needs to be disabled. > +# > + > +case "${target_canonical}" in > +*-*-vxworks*) > +machine_name_override="OVERRIDE" replace this line with: test -f ${MACRO_LIST} && rm -f ${MACRO_LIST} The remaining part of the patch to this file is not necessary. > +;; > +esac > Yes, my earlier solution wasn't quite the simplest. Driving the screw with > the proverbial golden hammer... Boy I'm glad I never do that... :( ') Cheers - Bruce
Re: [google] Modification of gcov pmu format to reduce gcda size bloat (issue 6427063)
Where is the string table management code? The gcov.c file is not properly uploaded either. David http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c File gcc/gcov-io.c (right): http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode280 gcc/gcov-io.c:280: gcov_read_pmu_string_table_entry (gcov_pmu_st_entry_t* st_entry, Fix format: ..entry_t *st_entry, http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode281 gcc/gcov-io.c:281: gcov_unsigned_t len ATTRIBUTE_UNUSED) Why having an unused parameter? Can it be used in assertion check? http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode830 gcc/gcov-io.c:830: print_pmu_string_table_entry (FILE *fp, const gcov_pmu_st_entry_t* st_entry, Fix format. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.c#newcode831 gcc/gcov-io.c:831: const enum print_newline newline) { '{' goes to the new line. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h File gcc/gcov-io.h (right): http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode699 gcc/gcov-io.h:699: Used for bookkeeping. */ typo. http://codereview.appspot.com/6427063/diff/5001/gcc/gcov-io.h#newcode916 gcc/gcov-io.h:916: const enum print_newline); Fix indentation. http://codereview.appspot.com/6427063/
Re: VxWorks Patches Back from the Dead!
I have two candidate patches. I've tested both and either can supersede the original 0001-fixincludes-machine_name patch. The first is the original proposed sed expression: --- fixincludes/mkfixinc.sh |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fixincludes/mkfixinc.sh b/fixincludes/mkfixinc.sh index 89e8ab7..a9f7c30 100755 --- a/fixincludes/mkfixinc.sh +++ b/fixincludes/mkfixinc.sh @@ -15,7 +15,6 @@ case $machine in i?86-*-mingw32* | \ x86_64-*-mingw32* | \ i?86-*-interix* | \ -*-*-vxworks* | \ powerpc-*-eabisim* | \ powerpc-*-eabi*| \ powerpc-*-rtems* | \ @@ -26,6 +25,12 @@ case $machine in (echo "#! /bin/sh" ; echo "exit 0" ) > ${target} ;; +*-*-vxworks* ) +# Platforms for which the machine_name fix breaks things +sed '/if test -s .{MACRO_LIST}/s/$/ \&\& false/' \ + ${srcdir}/fixinc.in > ${target} || exit 1 +;; + *) cat < ${srcdir}/fixinc.in > ${target} || exit 1 ;; -- The second is adding a target-dependent override to fixinc.in directly (IMHO more complex, but also more complete): --- fixincludes/fixinc.in | 14 +- fixincludes/mkfixinc.sh |1 - 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/fixincludes/fixinc.in b/fixincludes/fixinc.in index e73aed9..de7be35 100755 --- a/fixincludes/fixinc.in +++ b/fixincludes/fixinc.in @@ -128,6 +128,18 @@ fi # # # # # # # # # # # # # # # # # # # # # # +# Check to see if the machine_name fix needs to be disabled. +# + +case "${target_canonical}" in +*-*-vxworks*) +machine_name_override="OVERRIDE" +;; +esac + + +# # # # # # # # # # # # # # # # # # # # # +# # In the file macro_list are listed all the predefined # macros that are not in the C89 reserved namespace (the reserved # namespace is all identifiers beginnning with two underscores or one @@ -137,7 +149,7 @@ fi # Note dependency on ASCII. \012 = newline. # tr ' ' '\n' is, alas, not portable. -if test -s ${MACRO_LIST} +if test -s ${MACRO_LIST} && test -z "${machine_name_override}" then if test $VERBOSE -gt 0; then echo "Forbidden identifiers: `tr '\012' ' ' < ${MACRO_LIST}`" diff --git a/fixincludes/mkfixinc.sh b/fixincludes/mkfixinc.sh index 89e8ab7..6653fed 100755 --- a/fixincludes/mkfixinc.sh +++ b/fixincludes/mkfixinc.sh @@ -15,7 +15,6 @@ case $machine in i?86-*-mingw32* | \ x86_64-*-mingw32* | \ i?86-*-interix* | \ -*-*-vxworks* | \ powerpc-*-eabisim* | \ powerpc-*-eabi*| \ powerpc-*-rtems* | \ -- Both work for me, so whichever one you like wins. On 8/24/2012 3:48 PM, Bruce Korb wrote: I will approve any reasonably simple solution. :) Yes, my earlier solution wasn't quite the simplest. Driving the screw with the proverbial golden hammer... Thank you for doing this, by the way! No problem. Thank you for the (significantly larger) amount of work that you (both individual and collective) do :D -- Robert Mason
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Aug 24, 2012, at 5:35 AM, Jakub Jelinek wrote: > You haven't built your compiler with --disable-bootstrap, so you aren't > seeing what Mike is complaining about. Actually, I'm not using disable Just a normal cross compile.
Re: VxWorks Patches Back from the Dead!
On 08/24/12 11:50, rbmj wrote: On 8/22/2012 8:52 PM, Bruce Korb wrote: However I think it might be simpler to tweak mkfixinc.sh to sed '/if test -s .{MACRO_LIST}/s/$/ && false/' \ ${srcdir}/fixinc.in > ${target} for vxworks rather than all that configury rigmarole. That would eliminate changes to gcc/configure.ac and gcc/Makefile.in. OK. One question though: Why not just have a case statement inside fixinc.in? e.g. case ${target_canonical} in *-*-vxworks*) # Disable the machine name fix as it breaks things machine_name_override='OVERRIDE' ;; esac if test -s ${MACRO_LIST} && test -z "${machine_name_override}" There are many ways of accomplishing the same thing. I just thought of the fact that the mkfixinc.sh script constructed the result script either by spinning it from thin air or by copying a source file, so why not just edit that source file when it wasn't doing the right thing? Your solution is fine, too. You could also make the sed expression simpler: sed '/^if test -s .{MACRO_LIST}/s/if .*/if false/' sed '/^if test -s .{MACRO_LIST}/,/^fi/d' or replace ``machine_name_override='OVERRIDE'' with ``test -f ${MACRO_LIST} && rm -f ${MACRO_LIST}'' I will approve any reasonably simple solution. :) Thank you for doing this, by the way! Regards, Bruce
Sync include/plugin-api.h with src/
Hi, This patch was approved by Ian: http://sourceware.org/ml/binutils/2012-08/msg00447.html I have synced plugin-api.h with the following patch: * plugin-api.h (ld_plugin_allow_unique_segment_for_sections): New interface. (ld_plugin_unique_segment_for_sections): New interface. (LDPT_ALLOW_UNIQUE_SEGMENT_FOR_SECTIONS): New enum val. (LDPT_UNIQUE_SEGMENT_FOR_SECTIONS): New enum val. (tv_allow_unique_segment_for_sections): New member. (tv_unique_segment_for_sections): New member. Index: plugin-api.h === --- plugin-api.h(revision 190650) +++ plugin-api.h(working copy) @@ -318,6 +318,33 @@ typedef enum ld_plugin_status (*ld_plugin_allow_section_ordering) (void); +/* The linker's interface for specifying that a subset of sections is + to be mapped to a unique segment. If the plugin wants to call + unique_segment_for_sections, it must call this function from a + claim_file_handler or when it is first loaded. */ + +typedef +enum ld_plugin_status +(*ld_plugin_allow_unique_segment_for_sections) (void); + +/* The linker's interface for specifying that a specific set of sections + must be mapped to a unique segment. ELF segments do not have names + and the NAME is used as the name of the newly created output section + that is then placed in the unique PT_LOAD segment. FLAGS is used to + specify if any additional segment flags need to be set. For instance, + a specific segment flag can be set to identify this segment. Unsetting + segment flags that would be set by default is not possible. The + parameter SEGMENT_ALIGNMENT when non-zero will override the default. */ + +typedef +enum ld_plugin_status +(*ld_plugin_unique_segment_for_sections) ( +const char* segment_name, +uint64_t segment_flags, +uint64_t segment_alignment, +const struct ld_plugin_section * section_list, +unsigned int num_sections); + enum ld_plugin_level { LDPL_INFO, @@ -355,7 +382,9 @@ enum ld_plugin_tag LDPT_GET_INPUT_SECTION_CONTENTS, LDPT_UPDATE_SECTION_ORDER, LDPT_ALLOW_SECTION_ORDERING, - LDPT_GET_SYMBOLS_V2 + LDPT_GET_SYMBOLS_V2, + LDPT_ALLOW_UNIQUE_SEGMENT_FOR_SECTIONS, + LDPT_UNIQUE_SEGMENT_FOR_SECTIONS }; /* The plugin transfer vector. */ @@ -385,6 +414,8 @@ struct ld_plugin_tv ld_plugin_get_input_section_contents tv_get_input_section_contents; ld_plugin_update_section_order tv_update_section_order; ld_plugin_allow_section_ordering tv_allow_section_ordering; +ld_plugin_allow_unique_segment_for_sections tv_allow_unique_segment_for_sections; +ld_plugin_unique_segment_for_sections tv_unique_segment_for_sections; } tv_u; }; Thanks, -Sri.
Re: [Fortran] PR37336 - FIINAL patch [1/n]: Implement the finalization wrapper subroutine
Dear Alessandro, Alessandro Fanfarillo wrote: there are some problems with the final-wrapper-v2.diff patch; I get Internal Error at (1): gfc_code2string(): Bad code for every test case that I use; in attachment final2.f90. Fixed by the patch below. However, note that the current patch only implement the wrapper function - it doesn't handle calling the wrapper function. That's requires a follow up patch. (That was the reason that I did not do extensive test.) The patch is a complete module.c patch, not incrementally based on the previous patch. Tobias diff --git a/gcc/fortran/module.c b/gcc/fortran/module.c index a4ff199..3e636cd 100644 --- a/gcc/fortran/module.c +++ b/gcc/fortran/module.c @@ -1840,17 +1840,18 @@ typedef enum AB_ELEMENTAL, AB_PURE, AB_RECURSIVE, AB_GENERIC, AB_ALWAYS_EXPLICIT, AB_CRAY_POINTER, AB_CRAY_POINTEE, AB_THREADPRIVATE, AB_ALLOC_COMP, AB_POINTER_COMP, AB_PROC_POINTER_COMP, AB_PRIVATE_COMP, - AB_VALUE, AB_VOLATILE, AB_PROTECTED, AB_LOCK_COMP, + AB_VALUE, AB_VOLATILE, AB_PROTECTED, AB_LOCK_COMP, AB_FINAL_COMP, AB_IS_BIND_C, AB_IS_C_INTEROP, AB_IS_ISO_C, AB_ABSTRACT, AB_ZERO_COMP, AB_IS_CLASS, AB_PROCEDURE, AB_PROC_POINTER, AB_ASYNCHRONOUS, AB_CODIMENSION, AB_COARRAY_COMP, AB_VTYPE, AB_VTAB, AB_CONTIGUOUS, AB_CLASS_POINTER, - AB_IMPLICIT_PURE + AB_IMPLICIT_PURE, AB_ARTIFICIAL } ab_attribute; static const mstring attr_bits[] = { minit ("ALLOCATABLE", AB_ALLOCATABLE), +minit ("ARTIFICIAL", AB_ARTIFICIAL), minit ("ASYNCHRONOUS", AB_ASYNCHRONOUS), minit ("DIMENSION", AB_DIMENSION), minit ("CODIMENSION", AB_CODIMENSION), @@ -1883,6 +1884,7 @@ static const mstring attr_bits[] = minit ("VALUE", AB_VALUE), minit ("ALLOC_COMP", AB_ALLOC_COMP), minit ("COARRAY_COMP", AB_COARRAY_COMP), +minit ("FINAL_COMP", AB_FINAL_COMP), minit ("LOCK_COMP", AB_LOCK_COMP), minit ("POINTER_COMP", AB_POINTER_COMP), minit ("PROC_POINTER_COMP", AB_PROC_POINTER_COMP), @@ -1975,6 +1977,8 @@ mio_symbol_attribute (symbol_attribute *attr) { if (attr->allocatable) MIO_NAME (ab_attribute) (AB_ALLOCATABLE, attr_bits); + if (attr->artificial) + MIO_NAME (ab_attribute) (AB_ARTIFICIAL, attr_bits); if (attr->asynchronous) MIO_NAME (ab_attribute) (AB_ASYNCHRONOUS, attr_bits); if (attr->dimension) @@ -2057,6 +2061,8 @@ mio_symbol_attribute (symbol_attribute *attr) MIO_NAME (ab_attribute) (AB_PRIVATE_COMP, attr_bits); if (attr->coarray_comp) MIO_NAME (ab_attribute) (AB_COARRAY_COMP, attr_bits); + if (attr->final_comp) + MIO_NAME (ab_attribute) (AB_FINAL_COMP, attr_bits); if (attr->lock_comp) MIO_NAME (ab_attribute) (AB_LOCK_COMP, attr_bits); if (attr->zero_comp) @@ -2090,6 +2096,9 @@ mio_symbol_attribute (symbol_attribute *attr) case AB_ALLOCATABLE: attr->allocatable = 1; break; + case AB_ARTIFICIAL: + attr->artificial = 1; + break; case AB_ASYNCHRONOUS: attr->asynchronous = 1; break; @@ -2198,6 +2207,9 @@ mio_symbol_attribute (symbol_attribute *attr) case AB_COARRAY_COMP: attr->coarray_comp = 1; break; + case AB_FINAL_COMP: + attr->final_comp = 1; + break; case AB_LOCK_COMP: attr->lock_comp = 1; break;
C++ PATCH to remove get_bindings special case
While looking at another issue, I found the special case in get_bindings for unifying the template with itself odd; the places where we have been using it can just use coerce_template_parms instead. Tested x86_64-pc-linux-gnu, applying to trunk. commit 521c93c5aef7a96f9506ade3fb4759ac1cc7fdc0 Author: Jason Merrill Date: Sun Aug 12 22:13:51 2012 -0400 * pt.c (resolve_overloaded_unification): Use coerce_template_parms instead of get_bindings. (resolve_nondeduced_context): Likewise. diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 580a3d4..eff0b4d 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -15246,10 +15246,12 @@ resolve_overloaded_unification (tree tparms, if (TREE_CODE (fn) != TEMPLATE_DECL) continue; - ++processing_template_decl; - subargs = get_bindings (fn, DECL_TEMPLATE_RESULT (fn), - expl_subargs, /*check_ret=*/false); - if (subargs && !any_dependent_template_arguments_p (subargs)) + subargs = coerce_template_parms (DECL_INNERMOST_TEMPLATE_PARMS (fn), + expl_subargs, NULL_TREE, tf_none, + /*require_all_args=*/true, + /*use_default_args=*/true); + if (subargs != error_mark_node + && !any_dependent_template_arguments_p (subargs)) { elem = tsubst (TREE_TYPE (fn), subargs, tf_none, NULL_TREE); if (try_one_overload (tparms, targs, tempargs, parm, @@ -15262,7 +15264,6 @@ resolve_overloaded_unification (tree tparms, } else if (subargs) ++ok; - --processing_template_decl; } /* If no templates (or more than one) are fully resolved by the explicit arguments, this template-id is a non-deduced context; it @@ -15367,10 +15368,12 @@ resolve_nondeduced_context (tree orig_expr) if (TREE_CODE (fn) != TEMPLATE_DECL) continue; - ++processing_template_decl; - subargs = get_bindings (fn, DECL_TEMPLATE_RESULT (fn), - expl_subargs, /*check_ret=*/false); - if (subargs && !any_dependent_template_arguments_p (subargs)) + subargs = coerce_template_parms (DECL_INNERMOST_TEMPLATE_PARMS (fn), + expl_subargs, NULL_TREE, tf_none, + /*require_all_args=*/true, + /*use_default_args=*/true); + if (subargs != error_mark_node + && !any_dependent_template_arguments_p (subargs)) { elem = instantiate_template (fn, subargs, tf_none); if (elem == error_mark_node) @@ -15384,7 +15387,6 @@ resolve_nondeduced_context (tree orig_expr) ++good; } } - --processing_template_decl; } if (good == 1) { @@ -15435,6 +15437,9 @@ try_one_overload (tree tparms, tree tempargs; int i; + if (arg == error_mark_node) +return 0; + /* [temp.deduct.type] A template-argument can be deduced from a pointer to function or pointer to member function argument if the set of overloaded functions does not contain function templates and at most @@ -17129,40 +17134,13 @@ get_bindings (tree fn, tree decl, tree explicit_args, bool check_rettype) { int ntparms = DECL_NTPARMS (fn); tree targs = make_tree_vec (ntparms); - tree decl_type; + tree decl_type = TREE_TYPE (decl); tree decl_arg_types; tree *args; unsigned int nargs, ix; tree arg; - /* Substitute the explicit template arguments into the type of DECL. - The call to fn_type_unification will handle substitution into the - FN. */ - decl_type = TREE_TYPE (decl); - if (explicit_args && decl == DECL_TEMPLATE_RESULT (fn)) -{ - tree tmpl; - tree converted_args; - - if (DECL_TEMPLATE_INFO (decl)) - tmpl = DECL_TI_TEMPLATE (decl); - else - /* We can get here for some invalid specializations. */ - return NULL_TREE; - - converted_args - = coerce_template_parms (DECL_INNERMOST_TEMPLATE_PARMS (tmpl), - explicit_args, NULL_TREE, - tf_none, - /*require_all_args=*/false, - /*use_default_args=*/false); - if (converted_args == error_mark_node) - return NULL_TREE; - - decl_type = tsubst (decl_type, converted_args, tf_none, NULL_TREE); - if (decl_type == error_mark_node) - return NULL_TREE; -} + gcc_assert (decl != DECL_TEMPLATE_RESULT (fn)); /* Never do unification on the 'this' parameter. */ decl_arg_types = skip_artificial_parms_for (decl,
Minor C++ PATCH to specialization matching
Another patch I'm working on revealed a couple of latent issues in specialization matching. First, check_specialization_namespace takes the template as an argument, not the specialization; fixing this improves the error location on spec25.C. Second, allowing determine_specialization to match a member of an uninstantiated class was leading to an abort in other code that relied on that not happening, since it's ill-formed. Tested x86_64-pc-linux-gnu, applying to trunk. commit 47e1174262229c446ff4cdb9a01e131ed269e7d3 Author: Jason Merrill Date: Sun Aug 12 22:25:11 2012 -0400 * pt.c (register_specialization): Correct argument to check_specialization_namespace. (determine_specialization): Don't consider members of unspecialized types. diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index ad81bab..580a3d4 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -735,7 +735,7 @@ end_explicit_instantiation (void) processing_explicit_instantiation = false; } -/* An explicit specialization or partial specialization TMPL is being +/* An explicit specialization or partial specialization of TMPL is being declared. Check that the namespace in which the specialization is occurring is permissible. Returns false iff it is invalid to specialize TMPL in the current namespace. */ @@ -1407,7 +1407,7 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend, DECL_SOURCE_LOCATION (clone) = DECL_SOURCE_LOCATION (fn); } - check_specialization_namespace (fn); + check_specialization_namespace (tmpl); return fn; } @@ -1804,6 +1804,16 @@ determine_specialization (tree template_id, if (template_id == error_mark_node || decl == error_mark_node) return error_mark_node; + /* We shouldn't be specializing a member template of an + unspecialized class template; we already gave an error in + check_specialization_scope, now avoid crashing. */ + if (template_count && DECL_CLASS_SCOPE_P (decl) + && template_class_depth (DECL_CONTEXT (decl)) > 0) +{ + gcc_assert (errorcount); + return error_mark_node; +} + fns = TREE_OPERAND (template_id, 0); explicit_targs = TREE_OPERAND (template_id, 1); diff --git a/gcc/testsuite/g++.dg/template/spec25.C b/gcc/testsuite/g++.dg/template/spec25.C index 3f641fe..385d19a 100644 --- a/gcc/testsuite/g++.dg/template/spec25.C +++ b/gcc/testsuite/g++.dg/template/spec25.C @@ -1,10 +1,10 @@ namespace N { template struct S { -void f() {} +void f() {} // { dg-error "definition" } }; } namespace K { - template <> void N::S::f() {} // { dg-error "namespace|definition" } + template <> void N::S::f() {} // { dg-error "different namespace" } }
Re: VxWorks Patches Back from the Dead!
On 8/22/2012 8:52 PM, Bruce Korb wrote: However I think it might be simpler to tweak mkfixinc.sh to sed '/if test -s .{MACRO_LIST}/s/$/ && false/' \ ${srcdir}/fixinc.in > ${target} for vxworks rather than all that configury rigmarole. That would eliminate changes to gcc/configure.ac and gcc/Makefile.in. OK. One question though: Why not just have a case statement inside fixinc.in? Just running the script through sed seems like a kludgier solution. e.g. case ${target_canonical} in *-*-vxworks*) # Disable the machine name fix as it breaks things machine_name_override='OVERRIDE' ;; esac if test -s ${MACRO_LIST} && test -z "${machine_name_override}" -- Robert Mason
Re: Re-implement VEC_* to be member functions of vec_t
On 2012-08-23 23:08 , Diego Novillo wrote: I've tested this patch on x86_64 and ppc64 with all languages plus ada, go and obj-c++. I am going to be offline for several days starting on Saturday, so I will not commit it until I return. I've also done memory and time comparisons to make sure I didn't change behaviour. No differences. Diego.
Re: [PATCH] New command line switch -fada-spec-parent
On Fri, Aug 24, 2012 at 6:33 PM, Thomas Quinot wrote: > * common.opt (-fada-spec-parent): Define new command line switch. Why here instead of in c-family/c.opt? Ciao! Steven
Re: Re-implement VEC_* to be member functions of vec_t
On Fri, Aug 24, 2012 at 11:08 AM, Diego Novillo wrote: > On 2012-08-24 12:03 , Gabriel Dos Reis wrote: > >> I would just use C++ standard function `at()' (e.g. as found in vector) >> for this. > > > Sure. For regular functions, using default-valued arguments would be fine. > But I think the mechanism would be much more transparent if the compiler did > the heavy lifting. > > 1- Add a class/function attribute that makes the compiler add 3 hidden >args for the caller location. > > 2- During code generation, the compiler fills in these values at call >sites. > > 3- The callee accesses these values by referencing specially named >arguments (much like 'this') or via __builtin_* accessors. >I think I prefer using __builtin_*. I agree. We need to find a way to solve 1. that does not introduce ABI problems for operators. Your idea of backtrace would be a solution, but JSM appears to indicate it entails a non-trivial amount of non-portability... -- Gaby
[PATCH] New command line switch -fada-spec-parent
The following proposed change adds a new command line switch -fada-spec-parent allowing the user to specify a parent unit for all units generated by -fdump-ada-spec. * common.opt (-fada-spec-parent): Define new command line switch. * c-family/c-ada-spec.c (get_ada_package): When -fada-spec-parent is specified, generate binding spec as a child of the specified unit. * doc/invoke.texi: Document -fada-spec-parent. Tested on x86_64-linux. diff --git a/gcc/c-family/c-ada-spec.c b/gcc/c-family/c-ada-spec.c index d18e78d..250078c 100644 --- a/gcc/c-family/c-ada-spec.c +++ b/gcc/c-family/c-ada-spec.c @@ -945,15 +945,26 @@ get_ada_package (const char *file) char *res; const char *s; int i; + int plen; s = strstr (file, "/include/"); if (s) base = s + 9; else base = lbasename (file); - res = XNEWVEC (char, strlen (base) + 1); - for (i = 0; *base; base++, i++) + if (ada_specs_parent == NULL) +plen = 0; + else +plen = strlen(ada_specs_parent) + 1; + + res = XNEWVEC (char, plen + strlen (base) + 1); + if (ada_specs_parent != NULL) { +strcpy(res, ada_specs_parent); +res [plen - 1] = '.'; + } + + for (i = plen; *base; base++, i++) switch (*base) { case '+': @@ -965,7 +976,7 @@ get_ada_package (const char *file) case '_': case '/': case '\\': - res [i] = (i == 0 || res [i - 1] == '_') ? 'u' : '_'; + res [i] = (i == 0 || res [i - 1] == '.' || res [i - 1] == '_') ? 'u' : '_'; break; default: @@ -3365,7 +3376,10 @@ dump_ads (const char *source_file, ads_name = xstrdup (pkg_name); for (s = ads_name; *s; s++) -*s = TOLOWER (*s); +if (*s == '.') + *s = '-'; +else + *s = TOLOWER (*s); ads_name = reconcat (ads_name, ads_name, ".ads", NULL); diff --git a/gcc/common.opt b/gcc/common.opt index 1c7c4c6..f0fcf5e 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1031,6 +1031,10 @@ fdump- Common Joined RejectNegative Var(common_deferred_options) Defer -fdump- Dump various compiler internals to a file +fada-spec-parent= +Common RejectNegative Joined Var(ada_specs_parent) +-fada-spec-parent=unit Dump Ada specs as child units of given parent + fdump-final-insns Driver RejectNegative diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 5725f7b..97ac5c3 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -166,7 +166,7 @@ in the following sections. -pipe -pass-exit-codes @gol -x @var{language} -v -### --help@r{[}=@var{class}@r{[},@dots{}@r{]]} --target-help @gol --version -wrapper @@@var{file} -fplugin=@var{file} -fplugin-arg-@var{name}=@var{arg} @gol --fdump-ada-spec@r{[}-slim@r{]} -fdump-go-spec=@var{file}} +-fdump-ada-spec@r{[}-slim@r{]} -fada-spec-parent=@var{arg} -fdump-go-spec=@var{file}} @item C Language Options @xref{C Dialect Options,,Options Controlling C Dialect}. -- Thomas Quinot, Ph.D. ** qui...@adacore.com ** Senior Software Engineer AdaCore -- Paris, France -- New York, USA
Re: failed attempt: retain identifier length from frontend to backend
> "Dimitris" == Dimitrios Apostolou writes: Dimitris> [...] since I broke things like the static assert in Dimitris> libcpp/identifiers.c, that I don't even understand: Dimitris> /* We don't need a proxy since the hash table's identifier comes first Dimitris> in cpp_hashnode. However, in case this is ever changed, we have a Dimitris> static assertion for it. */ Dimitris> -extern char proxy_assertion_broken[offsetof (struct cpp_hashnode, ident) == 0 ? 1 : -1]; This assertion is because the implementation of cpp_forall_identifiers relies on the layout of cpp_hashnode: void cpp_forall_identifiers (cpp_reader *pfile, cpp_cb cb, void *v) { ht_forall (pfile->hash_table, (ht_cb) cb, v); } The idea is that since the identifier comes first, we can walk the hash table directly using ht_forall, relying on an implicit cast to convert the ht_identifier* to a cpp_hashnode*. This is somewhat bogus -- casts of functions are almost never good. (In gdb at least we tend to be more pedantic about this kind of thing. But this code has been in libcpp a long time...) If the struct were laid out differently, then cpp_forall_identifiers would need an intermediate function to do the conversion and then call 'cb'. Tom
Re: Re-implement VEC_* to be member functions of vec_t
On 2012-08-24 12:03 , Gabriel Dos Reis wrote: I would just use C++ standard function `at()' (e.g. as found in vector) for this. Sure. For regular functions, using default-valued arguments would be fine. But I think the mechanism would be much more transparent if the compiler did the heavy lifting. 1- Add a class/function attribute that makes the compiler add 3 hidden args for the caller location. 2- During code generation, the compiler fills in these values at call sites. 3- The callee accesses these values by referencing specially named arguments (much like 'this') or via __builtin_* accessors. I think I prefer using __builtin_*. Diego.
Re: Re-implement VEC_* to be member functions of vec_t
On Fri, Aug 24, 2012 at 10:50 AM, Marc Glisse wrote: > On Thu, 23 Aug 2012, Diego Novillo wrote: > >> There is another issue that I need to address and I'm not quite >> sure how to go about it: with the macro-based API, we make use of >> pre-processor trickery to insert __FILE__, __LINE__ and >> __FUNCTION__ into the argument list of functions. >> >> When I change VEC_pop(V) with V->pop(), the macro expansion no >> longer exists and we lose the caller references. Richi, I >> understand that your __builtin_FILE patch would allow me to >> declare default values for these arguments? Something like: >> >> T vec_t::pop(const char *file_ = __builtin_FILE, >> unsigned line_ = __builtin_LINE, >> const char *function_ = __builtin_FUNCTION) >> >> which would then be evaluated at the call site and get the right >> values. Is that more or less how your solution works? >> >> If so, then we could get away with that in most cases. However, >> we would still have the problem of operator functions (e.g., >> vec_t::operator[]). > > > Hello, > > it seems like you have found a better solution, but I'll just mention > quickly a way to handle operator[]: > > struct debug_int > { > debug_int (int i_, const char *file_ = __builtin_FILE ()) > : i (i_), file (file_) {} > int i; > const char *file; > operator int () const { return i; } > }; > > struct vec { > ... > T operator[] (debug_int arg) const { > gcc_assert (...); > } > ... > }; > > vec v; > v[3]; > > This gets the information into the function. Extracting it in gcc_assert is > a bit painful and forces the name arg to be the same everywhere :-( > But at least the callers are not uglified. Did not we discourage implicit conversions? I would just use C++ standard function `at()' (e.g. as found in vector) for this. -- Gaby
Re: Re-implement VEC_* to be member functions of vec_t
On Thu, 23 Aug 2012, Diego Novillo wrote: There is another issue that I need to address and I'm not quite sure how to go about it: with the macro-based API, we make use of pre-processor trickery to insert __FILE__, __LINE__ and __FUNCTION__ into the argument list of functions. When I change VEC_pop(V) with V->pop(), the macro expansion no longer exists and we lose the caller references. Richi, I understand that your __builtin_FILE patch would allow me to declare default values for these arguments? Something like: T vec_t::pop(const char *file_ = __builtin_FILE, unsigned line_ = __builtin_LINE, const char *function_ = __builtin_FUNCTION) which would then be evaluated at the call site and get the right values. Is that more or less how your solution works? If so, then we could get away with that in most cases. However, we would still have the problem of operator functions (e.g., vec_t::operator[]). Hello, it seems like you have found a better solution, but I'll just mention quickly a way to handle operator[]: struct debug_int { debug_int (int i_, const char *file_ = __builtin_FILE ()) : i (i_), file (file_) {} int i; const char *file; operator int () const { return i; } }; struct vec { ... T operator[] (debug_int arg) const { gcc_assert (...); } ... }; vec v; v[3]; This gets the information into the function. Extracting it in gcc_assert is a bit painful and forces the name arg to be the same everywhere :-( But at least the callers are not uglified. -- Marc Glisse
[PATCH][Revised2] skip gcc.target/i386/asm-dialect-1.c on darwin
Currently the new gcc.target/i386/asm-dialect-1.c testcase is failing on darwin due the absence of support for -masm=intel on that target. The attached patch skips this test on darwin. Tested on x86_64-apple-darwin12... http://gcc.gnu.org/ml/gcc-testresults/2012-08/msg02042.html Okay for gcc trunk? Jack 2012-08-24 Jack Howarth PR target/54255 * gcc.target/i386/asm-dialect-1.c: Skip on darwin. Index: gcc/testsuite/gcc.target/i386/asm-dialect-1.c warth@bromo ~]$ vi === --- gcc/testsuite/gcc.target/i386/asm-dialect-1.c (revision 190647) +++ gcc/testsuite/gcc.target/i386/asm-dialect-1.c (working copy) @@ -1,4 +1,5 @@ /* { dg-options "-masm=intel" } */ +/* { dg-skip-if "No support for -masm=intel" { *-*-darwin* } } */ extern void abort (void);
Re: Speedups/Cleanups: End of GSOC patch collection
> "Dodji" == Dodji Seketeli writes: Dodji> With these changes, the libcpp parts look OK to me if they still Dodji> boostrap post c++ conversion. I am not a maintainer so I a deferring to Dodji> Tom and the other maintainers. I agree. Dodji, thanks for looking at this. Dmitris, thanks for writing it -- patch ok with that change. Tom
Re: [PATCH, x86-Atom] Enabling look-ahead scheduling feature for Atom processors
> OK. > Checked in http://gcc.gnu.org/ml/gcc-cvs/2012-08/msg00626.html Thanks, K
[Patch, fortran] [5/5] PR 45586: Use the right type in scalar to array assignments
Patch number 3 handled scalar assignments like: scalar_some_type = some_type(...) Patch number 4 handled array assignments like: array_some_type = (/some_type :: .../) This takes care of assignments like: array_some_type = some_type(...) i.e. scalar to array assignments. As all the infrastructure has been installed previously, this simply sets it up properly. OK? 2012-08-22 Mikael Morin * trans-array.c (gfc_add_loop_ss_code): Use RESTRICTED field. * trans-expr.c (gfc_trans_assignment): Set RESTRICTED field. diff --git a/trans-array.c b/trans-array.c index f5051ff..432fc72 100644 --- a/trans-array.c +++ b/trans-array.c @@ -2489,6 +2489,7 @@ gfc_add_loop_ss_code (gfc_loopinfo * loop, gfc_ss * ss, bool subscript, /* Scalar expression. Evaluate this now. This includes elemental dimension indices, but not array section bounds. */ gfc_init_se (&se, NULL); + se.want_restricted_types = ss_info->restricted; gfc_conv_expr (&se, expr); gfc_add_block_to_block (&outer_loop->pre, &se.pre); diff --git a/trans-expr.c b/trans-expr.c index 2a60087..6af753a 100644 --- a/trans-expr.c +++ b/trans-expr.c @@ -7258,8 +7258,11 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, /* Walk the rhs. */ rss = gfc_walk_expr (expr2); if (rss == gfc_ss_terminator) - /* The rhs is scalar. Add a ss for the expression. */ - rss = gfc_get_scalar_ss (gfc_ss_terminator, expr2); + { + /* The rhs is scalar. Add a ss for the expression. */ + rss = gfc_get_scalar_ss (gfc_ss_terminator, expr2); + gfc_ss_set_restricted (rss, !gfc_expr_attr (expr1).target); + } else if (rss->next == gfc_ss_terminator && rss->info->type == GFC_SS_CONSTRUCTOR) gfc_ss_set_restricted (rss, !(gfc_expr_attr (expr1).target));
[Patch, fortran] [4/5] PR 45586: Use the right type to build array constructors.
With array constructors, there is the same type problem as with structure constructor: we don't know whether we want the non-restricted or the restricted variant. Array constructors are translated from somewhere deep inside the scalarizer, so we have to pass the information there down to gfc_conv_expr (which is now able to handle it thanks to the previous patch). A new flag is added to the gfc_ss_info structure to store that information. As most of the time the restrict variant is needed, the flag is set to true by default. A new function is added (gfc_ss_set_restricted) to set it to false in the one (for now) case it is useful. As before, new flags have to be added to function prototypes to transfer that flag's information. The call graph is as follows (to be seen with a fixed font): gfc_add_loop_ss_code [set restricted] \ +-> trans_array_constructor \ +-> gfc_trans_array_constructor_value <--+ |\ | | +--+ |\ | +-> gfc_trans_array_constructor_subarray \ \ +---+-> gfc_trans_array_ctor_element \ +-> gfc_conv_expr [propagate restricted] gfc_trans_array_ctor_element is changed to use gfc_trans_scalar_assign, which is able to handle incompatible types thanks to patch number 1. OK? 2012-08-22 Mikael Morin * trans.h (struct gfc_ss_info): New field RESTRICTED. * trans-array.h (gfc_ss_set_restricted): New declaration. * trans-expr.c (gfc_trans_assignment_1): Call gfc_ss_set_restricted. * trans-array.c (gfc_ss_set_restricted): New function. (gfc_get_array_ss, gfc_get_scalar_ss, gfc_get_temp_ss): Set the RESTRICTED field by default. (gfc_trans_array_ctor_element): Use gfc_trans_scalar_assign. (gfc_trans_array_constructor_subarray, gfc_trans_array_constructor_value): Add argument RESTRICTED. Pass it down. (trans_array_constructor): Update call to gfc_trans_array_constructor_value. Choose the variant type that is wanted. diff --git a/trans-array.c b/trans-array.c index 217d7b8..f5051ff 100644 --- a/trans-array.c +++ b/trans-array.c @@ -557,6 +557,7 @@ gfc_get_array_ss (gfc_ss *next, gfc_expr *expr, int dimen, gfc_ss_type type) ss_info->refcount++; ss_info->type = type; ss_info->expr = expr; + ss_info->restricted = 1; ss = gfc_get_ss (); ss->info = ss_info; @@ -583,6 +584,7 @@ gfc_get_temp_ss (tree type, tree string_length, int dimen) ss_info->type = GFC_SS_TEMP; ss_info->string_length = string_length; ss_info->data.temp.type = type; + ss_info->restricted = 1; ss = gfc_get_ss (); ss->info = ss_info; @@ -593,7 +595,7 @@ gfc_get_temp_ss (tree type, tree string_length, int dimen) return ss; } - + /* Creates and initializes a scalar type gfc_ss struct. */ @@ -607,6 +609,7 @@ gfc_get_scalar_ss (gfc_ss *next, gfc_expr *expr) ss_info->refcount++; ss_info->type = GFC_SS_SCALAR; ss_info->expr = expr; + ss_info->restricted = 1; ss = gfc_get_ss (); ss->info = ss_info; @@ -616,6 +619,23 @@ gfc_get_scalar_ss (gfc_ss *next, gfc_expr *expr) } +/* Sets SS's restricted attribute if needed according to WANT_RESTRICTED. + WANT_RESTRICTED is typically the lhs's target attribute in an assignment. */ + +void +gfc_ss_set_restricted (gfc_ss *ss, bool want_restricted) +{ + gcc_assert (ss != gfc_ss_terminator); + + if (!want_restricted + && (ss->info->expr->ts.type == BT_DERIVED + || ss->info->expr->ts.type == BT_CLASS)) +ss->info->restricted = 0; + else +ss->info->restricted = 1; +} + + /* Free all the SS associated with a loop. */ void @@ -1432,9 +1452,14 @@ gfc_trans_array_ctor_element (stmtblock_t * pblock, tree desc, } else { - /* TODO: Should the frontend already have done this conversion? */ - se->expr = fold_convert (TREE_TYPE (tmp), se->expr); - gfc_add_modify (&se->pre, tmp, se->expr); + gfc_se tmp_se; + tree code; + + gfc_init_se (&tmp_se, NULL); + tmp_se.expr = tmp; + code = gfc_trans_scalar_assign (&tmp_se, se, expr->ts, true, false, + false); + gfc_add_expr_to_block (pblock, code); } gfc_add_block_to_block (pblock, &se->pre); @@ -1450,7 +1475,7 @@ gfc_trans_array_constructor_subarray (stmtblock_t * pblock, tree type ATTRIBUTE_UNUSED, tree desc, gfc_expr * expr, tree * poffset, tree * offsetvar, - bool dynamic) + bool dynamic, bool restricted) { gfc_se se; gfc_ss *ss; @@ -1464,6 +1489,7 @@ gfc_trans_array_constructor_subarray (stmtblock_t *
[Patch, fortran] [3/5] PR 45586: Use the right type to build structure constructors.
This patch makes the LTO failures go away. It propagates the information that we want (non-)restrict types from gfc_trans_assignment_1 (where we have access to the LHS) down to gfc_conv_structure (where the type specialization happens). The call graph is roughly as follows (to be seen with a fixed font): gfc_trans_assignment_1 [set restricted] \ +-> gfc_conv_expr <--+---+ \ \ \ +-> gfc_conv_structure [use restricted] <+-|-+ | \\| \| +-> gfc_conv_initializer + + \ | +-> gfc_conv_array_initializer ---+ To avoid polluting every function (and every caller) with a restricted flag I have added it to gfc_se, which has already a good deal of request specification flags. Unfortunately, gfc_conv_initializer and gfc_conv_array_initializer don't have a gfc_se arg, so they don't avoid the restricted argument. To avoid changing all gfc_conv_initializer callers it is made a wrapper around the function with the restricted argument. I didn't do the same for gfc_conv_array_initializer as it has a single caller, so the interface change is harmless/non-invasive. As I had to update the declaration I moved it from gfortran.h to trans-array.h by the way. OK? 2012-08-22 Mikael Morin * trans.h (struct gfc_se): New flag want_restricted_types. * trans-expr.c (gfc_trans_assignment_1): Set the want_restricted_types field. (gfc_conv_structure): Use the new field to choose the variant type that is wanted. (gfc_conv_initializer): Make it a wrapper around the old function renamed to... (conv_initializer_1): ... this. Add the RESTRICTED argument. Pass it down. * gfortran.h (gfc_conv_array_initializer): Move declaration... * trans-array.h (gfc_conv_array_initializer): ... here. Add a boolean argument. * trans-array.c (gfc_conv_array_initializer): Add the RESTRICTED argument. Pass it down. diff --git a/gfortran.h b/gfortran.h index 4c8a856..c11cb12 100644 --- a/gfortran.h +++ b/gfortran.h @@ -2837,7 +2837,6 @@ gfc_try gfc_array_size (gfc_expr *, mpz_t *); gfc_try gfc_array_dimen_size (gfc_expr *, int, mpz_t *); gfc_try gfc_array_ref_shape (gfc_array_ref *, mpz_t *); gfc_array_ref *gfc_find_array_ref (gfc_expr *); -tree gfc_conv_array_initializer (tree type, gfc_expr *); gfc_try spec_size (gfc_array_spec *, mpz_t *); gfc_try spec_dimen_size (gfc_array_spec *, int, mpz_t *); int gfc_is_compile_time_shape (gfc_array_spec *); diff --git a/trans-array.c b/trans-array.c index c350c3b..217d7b8 100644 --- a/trans-array.c +++ b/trans-array.c @@ -5309,7 +5309,7 @@ gfc_array_deallocate (tree descriptor, tree pstat, tree errmsg, tree errlen, We assume the frontend already did any expansions and conversions. */ tree -gfc_conv_array_initializer (tree type, gfc_expr * expr) +gfc_conv_array_initializer (tree type, gfc_expr * expr, bool restricted) { gfc_constructor *c; tree tmp; @@ -5329,9 +5329,10 @@ gfc_conv_array_initializer (tree type, gfc_expr * expr) case EXPR_CONSTANT: case EXPR_STRUCTURE: /* A single scalar or derived type value. Create an array with all - elements equal to that value. */ +elements equal to that value. */ gfc_init_se (&se, NULL); - + se.want_restricted_types = restricted; + if (expr->expr_type == EXPR_CONSTANT) gfc_conv_constant (&se, expr); else @@ -5398,7 +5399,8 @@ gfc_conv_array_initializer (tree type, gfc_expr * expr) else range = NULL; - gfc_init_se (&se, NULL); + gfc_init_se (&se, NULL); + se.want_restricted_types = restricted; switch (c->expr->expr_type) { case EXPR_CONSTANT: diff --git a/trans-array.h b/trans-array.h index de03202..8d071b9 100644 --- a/trans-array.h +++ b/trans-array.h @@ -137,6 +137,8 @@ void gfc_conv_array_parameter (gfc_se *, gfc_expr *, bool, const gfc_symbol *, const char *, tree *); /* Evaluate and transpose a matrix expression. */ void gfc_conv_array_transpose (gfc_se *, gfc_expr *); +/* Creates a middle-end array constructor from a constant expression. */ +tree gfc_conv_array_initializer (tree, gfc_expr *, bool); /* These work with both descriptors and descriptorless arrays. */ tree gfc_conv_array_data (tree); diff --git a/trans-expr.c b/trans-expr.c index 9dab898..38c17a1 100644 --- a/trans-expr.c +++ b/trans-expr.c @@ -5166,12 +5166,13 @@ gfc_conv_array_constructor_expr (gfc_se * se, gfc_expr * expr) /* Build a static initializer. EXPR is the expression for the initial value. - The other parameters describe the variable of the component bei
[Patch, fortran] [2/5] PR 45586: Use distinct types to have per field qualified types.
This patch comes from Richi. Self explanatory. OK? 2012-08-22 Richard Guenther PR fortran/45586 * trans-expr.c (gfc_nonrestricted_type): Make the non-restrict type distinct from the original type. diff --git a/trans-types.c b/trans-types.c index a6e5d99..189f597 100644 --- a/trans-types.c +++ b/trans-types.c @@ -2026,7 +2026,8 @@ gfc_nonrestricted_type (tree t) ret = t; else { - ret = build_variant_type_copy (t); + ret = build_distinct_type_copy (t); + TYPE_CANONICAL (ret) = TYPE_CANONICAL (t); TREE_TYPE (ret) = elemtype; if (TYPE_LANG_SPECIFIC (t) && GFC_TYPE_ARRAY_DATAPTR_TYPE (t)) @@ -2070,7 +2071,8 @@ gfc_nonrestricted_type (tree t) } if (!field) break; - ret = build_variant_type_copy (t); + ret = build_distinct_type_copy (t); + TYPE_CANONICAL (ret) = TYPE_CANONICAL (t); TYPE_FIELDS (ret) = NULL_TREE; /* Here we make sure that as soon as we know we have to copy
[Patch, fortran] [1/5] PR 45586: VIEW_CONVERT_EXPR wrapping
This patch avoids fold_convert bombing when the types are not variants of the same base type. It is necessary to avoid regressing with the next patch. It tries to take the VIEW_CONVERT_EXPR path only when it is necessary and use the usual fold_convert otherwise. I use gfc_nonrestricted_type in one assertion, so I had to make it public For what it's worth, I had another version of this patch which tried harder to not use VIEW_CONVERT_EXPR by carefully avoiding copying the data pointer (which is overwritten just after anyway). However, as I didn't want to pull in the scalarizer to assign arrays, I couldn't avoid VIEW_CONVERT_EXPR in all cases, so I finally preferred this (simpler) patch. OK? 2012-08-22 Mikael Morin PR fortran/45586 * trans-expr.c (gfc_trans_scalar_assign): Wrap in a VIEW_CONVERT_EXPR node if the types don't match. * trans-types.c (gfc_nonrestricted_type): Make non-static. * trans.h (gfc_nonrestricted_type): New declaration. diff --git a/trans-expr.c b/trans-expr.c index ebaa238..9dab898 100644 --- a/trans-expr.c +++ b/trans-expr.c @@ -6396,8 +6396,24 @@ gfc_trans_scalar_assign (gfc_se * lse, gfc_se * rse, gfc_typespec ts, gfc_add_block_to_block (&block, &rse->pre); gfc_add_block_to_block (&block, &lse->pre); - gfc_add_modify (&block, lse->expr, - fold_convert (TREE_TYPE (lse->expr), rse->expr)); + tree converted = NULL_TREE; + if (TYPE_MAIN_VARIANT (TREE_TYPE (lse->expr)) + != TYPE_MAIN_VARIANT (TREE_TYPE (rse->expr)) + && !POINTER_TYPE_P (TREE_TYPE (lse->expr)) + && !POINTER_TYPE_P (TREE_TYPE (rse->expr))) + { + gcc_assert (TYPE_CANONICAL (TREE_TYPE (lse->expr)) + == TYPE_CANONICAL (TREE_TYPE (rse->expr)) + && gfc_nonrestricted_type (TREE_TYPE (lse->expr)) +== gfc_nonrestricted_type (TREE_TYPE (rse->expr))); + /* fold_convert won't like this. Let's bypass it. */ + converted = fold_build1_loc (input_location, VIEW_CONVERT_EXPR, + TREE_TYPE (lse->expr), rse->expr); + } + else + converted = fold_convert (TREE_TYPE (lse->expr), rse->expr); + + gfc_add_modify (&block, lse->expr, converted); /* Do a deep copy if the rhs is a variable, if it is not the same as the lhs. */ diff --git a/trans-types.c b/trans-types.c index 3286a5a..a6e5d99 100644 --- a/trans-types.c +++ b/trans-types.c @@ -1924,7 +1924,6 @@ gfc_build_pointer_type (gfc_symbol * sym, tree type) return build_pointer_type (type); } -static tree gfc_nonrestricted_type (tree t); /* Given two record or union type nodes TO and FROM, ensure that all fields in FROM have a corresponding field in TO, their type being nonrestrict variants. This accepts a TO @@ -1973,7 +1972,7 @@ mirror_fields (tree to, tree from) /* Given a type T, returns a different type of the same structure, except that all types it refers to (recursively) are always non-restrict qualified types. */ -static tree +tree gfc_nonrestricted_type (tree t) { tree ret = t; diff --git a/trans.h b/trans.h index 9818ceb..56b6c2f 100644 --- a/trans.h +++ b/trans.h @@ -639,6 +639,7 @@ tree getdecls (void); /* In trans-types.c. */ struct array_descr_info; bool gfc_get_array_descr_info (const_tree, struct array_descr_info *); +tree gfc_nonrestricted_type (tree); /* In trans-openmp.c */ bool gfc_omp_privatize_by_reference (const_tree);
[Patch, fortran] [0/5] PR 45586: restrict vs. non-restrict type compatibility hell
Hello, here come several patches to fix the infamous PR 45586. The main issue is the middle-end expecting variant types to share the fields, thus one field cannot be restrict qualified in one type, and not restrict qualified in one of its variants. The fix, as per Richi's suggestion, makes the types not be variants of each other, but this raises type compatibility problems, as fold_convert triggers an assertion if the types are not variants of the same base type. The fix for that (suggested by Richi again) wraps the expression in a VIEW_CONVERT_EXPR. The above is not enough to make LTO happy. There are problems with structure constructors, where we use the restricted type, but the variable to assign to is a target, thus has a non-restrict type. The fix for that propagates the information that we don't want restrict qualification from gfc_trans_assignment down to gfc_conv_structure. The same applies to array constructors. The patch is split as follows: [1/5]: Add the VIEW_CONVERT_EXPR wrapping. [2/5]: Make target vs. non-target variant types distinct. [3/5]: Use the target information to assign from structure constructors. [4/5]: Use the target information to assign from array constructors. [5/5]: Use the target information to assign a scalar structure to an array. More details in the follow-up mails. Regression tested on amd64-linux. OK for trunk? Mikael 2012-08-18 Mikael Morin PR fortran/45586 * gfortran.dg/restrict_type_compat_1.f90: New test. ! { dg-do compile } ! { dg-options "-fdump-tree-original" } ! ! PR fortran/45586 ! Test restricted vs. non-restricted (or target vs. non-target) type ! compatibility in assignments type :: t integer :: i integer, allocatable :: a(:) real :: r end type t type(t), target :: x, xx(1) type(t) :: y, yy(1) x = t( 1, null(), -1.0) y = t(-1, null(), 1.0) x = y! VIEW_CONVERT_EXPR y = x! VIEW_CONVERT_EXPR x = t( 1, (/3/), -1.0) y = t(-1, (/4/), 1.0) x = func() ! VIEW_CONVERT_EXPR y = func() xx = x yy = y xx = t(1, null(), -1.0) yy = t(-1, null(), 1.0) xx = y! VIEW_CONVERT_EXPR yy = x! VIEW_CONVERT_EXPR xx = yy ! VIEW_CONVERT_EXPR yy = xx ! VIEW_CONVERT_EXPR xx = t( 1, (/3/), -1.0) yy = t(-1, (/4/), 1.0) xx = (/x/) yy = (/y/) xx = (/t( 1, null(), -1.0)/) yy = (/t(-1, null(), 1.0)/) xx = (/y/)! VIEW_CONVERT_EXPR yy = (/x/)! VIEW_CONVERT_EXPR xx = (/yy/) ! VIEW_CONVERT_EXPR yy = (/xx/) ! VIEW_CONVERT_EXPR xx = (/t( 1, (/3/), -1.0)/) yy = (/t(-1, (/4/), 1.0)/) xx = func() ! VIEW_CONVERT_EXPR yy = func() xx = (/func()/) ! VIEW_CONVERT_EXPR yy = (/func()/) contains function func() result(res) type(t) :: res res = t(2, (/5/), -2.0) end function func end ! { dg-final { scan-tree-dump-times "VIEW_CONVERT_EXPR" 13 "original" } } ! { dg-final { cleanup-tree-dump "original" } }
Re: [PATCH][Revised] skip gcc.target/i386/asm-dialect-1.c on darwin
Jack Howarth writes: > === > --- gcc/testsuite/gcc.target/i386/asm-dialect-1.c (revision 190647) > +++ gcc/testsuite/gcc.target/i386/asm-dialect-1.c (working copy) > @@ -1,4 +1,5 @@ > /* { dg-options "-masm=intel" } */ > +/* { dg-skip-if "No support for -masm=intel" { *-*-darwin* } { "*" } { "" } > } */ You still explicitly list the default include-opts and exclude-opts for dg-skip-if. Please leave them off completely. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [Fortran] PR37336 - FIINAL patch [1/n]: Implement the finalization wrapper subroutine
Dear Tobias, there are some problems with the final-wrapper-v2.diff patch; I get the following error final2.f90:71.15: end module test 1 Internal Error at (1): gfc_code2string(): Bad code for every test case that I use; in attachment final2.f90. Regards Alessandro 2012/8/19 Tobias Burnus : > Dear all, > > attached is a slightly updated patch: > > * Call finalizers of nonallocatable, nonpointer components > * Generate FINAL wrapper for abstract types which have a finalizer. (The > allocatable components are deallocated in the first type (abstract or not) > which has a finalizer, i.e. abstract + finalizer or first nonabstract type.) > > I had to disable some resolve warning; I did so by introducing an > attr.artificial. I used it to also fix PR 51632, where we errored out for > __def_init and __copy where there were coarray components. > > Build and regtested on x86-64-linux. > OK for the trunk? > > Tobias -- final2.f90 Description: Binary data
Re: Reproducible gcc builds, gfortran, and -grecord-gcc-switches
On Fri, 24 Aug 2012, Simon Baldwin wrote: > gcc/ChangeLog > 2012-08-24 Simon Baldwin > > * dwarf2out.c (gen_producer_string): Omit command line switch if > CL_NO_DWARF_RECORD flag set. > * opts.h (CL_NO_DWARF_RECORD): New. > * opt-functions.awk (switch_flags): Add NoDWARFRecord. > * doc/options.texi: Document NoDWARFRecord option flag. > > gcc/fortran/ChangeLog > 2012-08-24 Simon Baldwin > > * lang.opt (-cpp=): Mark flag NoDWARFRecord. OK. -- Joseph S. Myers jos...@codesourcery.com
[PATCH][Revised] skip gcc.target/i386/asm-dialect-1.c on darwin
Currently the new gcc.target/i386/asm-dialect-1.c testcase is failing on darwin due the absence of support for -masm=intel on that target. The attached patch skips this test on darwin. Tested on x86_64-apple-darwin12... http://gcc.gnu.org/ml/gcc-testresults/2012-08/msg02042.html Okay for gcc trunk? Jack 2012-08-24 Jack Howarth PR target/54255 * gcc.target/i386/asm-dialect-1.c: Skip on darwin. Index: gcc/testsuite/gcc.target/i386/asm-dialect-1.c warth@bromo ~]$ vi === --- gcc/testsuite/gcc.target/i386/asm-dialect-1.c (revision 190647) +++ gcc/testsuite/gcc.target/i386/asm-dialect-1.c (working copy) @@ -1,4 +1,5 @@ /* { dg-options "-masm=intel" } */ +/* { dg-skip-if "No support for -masm=intel" { *-*-darwin* } { "*" } { "" } } */ extern void abort (void);
Re: [AArch64] Do not mix statements with declarations
On 2012-08-17 11:32 , Sofiane Naci wrote: Hi, I've just committed the attached patch on the AArch64 branch to fix a style issue related to mixing statements with declarations. Thanks Sofiane - r190486 | sofiane | 2012-08-17 16:26:47 +0100 (Fri, 17 Aug 2012) | 7 lines 2012-08-17 Marcus Shawcroft [AArch64] Do not mix statements and declarations. * config/aarch64/aarch64.c (aarch64_simd_lane_bounds): Do not mix statements with declarations. This is not necessary now. Code and declarations can be mixed. Diego.
Re: [PATCH] skip gcc.target/i386/pr53249.c on darwin
On Fri, Aug 24, 2012 at 7:13 AM, Rainer Orth wrote: > Jack Howarth writes: > >>Currently the new testcase for gcc.target/i386/pr53249.c is failing >> on darwin due to the absence of -mx32 support on that target. The following >> patch skips this testcase on darwin. Tested on x86_64-apple-darwin12... > > This also fails on Solaris/x86 (cf. PR testsuite/53365) and > i686-unknown-linux-gnu. I'd strongly prefer if HJ could devise a real > fix instead of just skipping the test on an explicit list of systems. > x32 code generation is mostly platform independent, except for TLS support. We can limit x32 TLS tests to Linux. -- H.J.
Re: [PATCH] skip gcc.target/i386/pr53249.c on darwin
Jack Howarth writes: >Currently the new testcase for gcc.target/i386/pr53249.c is failing > on darwin due to the absence of -mx32 support on that target. The following > patch skips this testcase on darwin. Tested on x86_64-apple-darwin12... This also fails on Solaris/x86 (cf. PR testsuite/53365) and i686-unknown-linux-gnu. I'd strongly prefer if HJ could devise a real fix instead of just skipping the test on an explicit list of systems. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Another ping: Reorganized documentation for warnings -- attempt 2
On 2012-08-22 20:56 , David Stone wrote: I hope this can get looked at, thanks. http://gcc.gnu.org/ml/gcc-patches/2012-06/msg01208.html Please send your patches as unified or context diffs (see http://gcc.gnu.org/contribute.html). You say that you have sent your copyright paperwork. Is the whole process finished? Only when the final paperwork is filed with the FSF can we start accepting significant changes. Thanks. Diego.
Re: [PATCH] skip gcc.target/i386/asm-dialect-1.c on darwin
Jack Howarth writes: > === > --- gcc/testsuite/gcc.target/i386/asm-dialect-1.c (revision 190647) > +++ gcc/testsuite/gcc.target/i386/asm-dialect-1.c (working copy) > @@ -1,4 +1,5 @@ > /* { dg-options "-masm=intel" } */ > +/* { dg-skip-if "" { *-*-darwin* } { "*" } { "" } } */ Please add a comment explaining why this is skipped (that's what the dg-skip-if comment field is for) and omit the default arguments. I'll leave the actual approval to a darwin maintainer. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [doc] Fix typo in gty.texi
On 2012-08-23 06:55 , 陳韋任 (Wei-Ren Chen) wrote: I think this is much clear than before. The word "modes" in "CFG takes various different modes" means different forms, GIMPLE or RTL, right? Right. Diego.
[PATCH] skip gcc.target/i386/pr53249.c on darwin
Currently the new testcase for gcc.target/i386/pr53249.c is failing on darwin due to the absence of -mx32 support on that target. The following patch skips this testcase on darwin. Tested on x86_64-apple-darwin12... http://gcc.gnu.org/ml/gcc-testresults/2012-08/msg02042.html Okay for gcc trunk? Jack 2012-08-24 Dominique d'Humieres Jack Howarth PR target/54257 * gcc.target/i386/pr53249.c: Skip on darwin. Index: gcc/testsuite/gcc.target/i386/pr53249.c === --- gcc/testsuite/gcc.target/i386/pr53249.c (revision 190647) +++ gcc/testsuite/gcc.target/i386/pr53249.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do compile { target { ! { ia32 } } } } */ +/* { dg-do compile { target { { ! { ia32 } } && { ! *-*-darwin* } } } } */ /* { dg-options "-O2 -mx32 -ftls-model=initial-exec -maddress-mode=short" } */ struct gomp_task
[PATCH] skip gcc.target/i386/asm-dialect-1.c on darwin
Currently the new gcc.target/i386/asm-dialect-1.c testcase is failing on darwin due the absence of support for -masm=intel on that target. The attached patch skips this test on darwin. Tested on x86_64-apple-darwin12... http://gcc.gnu.org/ml/gcc-testresults/2012-08/msg02042.html Okay for gcc trunk? Jack 2012-08-24 Jack Howarth PR target/54255 * gcc.target/i386/asm-dialect-1.c: Skip on darwin. Index: gcc/testsuite/gcc.target/i386/asm-dialect-1.c warth@bromo ~]$ vi === --- gcc/testsuite/gcc.target/i386/asm-dialect-1.c (revision 190647) +++ gcc/testsuite/gcc.target/i386/asm-dialect-1.c (working copy) @@ -1,4 +1,5 @@ /* { dg-options "-masm=intel" } */ +/* { dg-skip-if "" { *-*-darwin* } { "*" } { "" } } */ extern void abort (void);
Re: [PATCH] Set current_function_decl in {push,pop}_cfun and push_struct_function
Hi, On Wed, Aug 22, 2012 at 03:37:48PM +0200, Richard Guenther wrote: > On Wed, Aug 22, 2012 at 3:04 PM, Martin Jambor wrote: > > On Tue, Aug 21, 2012 at 01:30:47PM +0200, Richard Guenther wrote: > >> On Tue, Aug 21, 2012 at 1:27 PM, Martin Jambor wrote: > >> > On Wed, Aug 15, 2012 at 05:21:04PM +0200, Martin Jambor wrote: > >> >> Hi, > >> >> > >> >> On Fri, Aug 10, 2012 at 04:57:41PM +0200, Eric Botcazou wrote: > >> >> > > - ada/gcc-interface/utils.c:rest_of_subprog_body_compilation calls > >> >> > > dump_function which in turns calls dump_function_to_file which > >> >> > > calls > >> >> > > push_cfun. But Ada front end has its idea of the > >> >> > > current_function_decl and there is no cfun which is an > >> >> > > inconsistency > >> >> > > which makes push_cfun assert fail. I "solved" it by temporarily > >> >> > > setting current_function_decl to NULL_TREE. It's just dumping > >> >> > > and I > >> >> > > thought that dump_function should be considered middle-end and > >> >> > > thus > >> >> > > middle-end invariants should apply. > >> >> > > >> >> > If you think that calling dump_function from > >> >> > rest_of_subprog_body_compilation > >> >> > is a layering violation, I don't have a problem with replacing it > >> >> > with a more > >> >> > "manual" scheme like the one in c-family/c-gimplify.c:c_genericize, > >> >> > provided > >> >> > that this yields roughly the same output. > >> >> > >> >> Richi suggested on IRC that I remove the push/pop_cfun calls from > >> >> dump_function_to_file. The only problem seems to be > >> >> dump_histograms_for_stmt > >> > > >> > Yesterday I actually tried and it is not the only problem. Another > >> > one is dump_function_to_file->dump_bb->maybe_hot_bb_p which uses cfun > >> > to read profile_status. There may be others, this one just blew up > >> > first when I set cfun to NULL. And in future someone is quite likely > >> > to need cfun to dump something new too. > >> > > >> > At the same time, re-implementing dumping > >> > c-family/c-gimplify.c:c_genericize when dump_function suffices seems > >> > ugly to me. > >> > > >> > So I am going to declare dump_function a front-end interface and use > >> > set_cfun in my original patch in dump_function_to_file like we do in > >> > other such functions. > >> > > >> > I hope that will be OK. Thanks, > >> > >> Setting cfun has side-effects of switching target stuff which might have > >> code-generation side-effects because of implementation issues we have > >> with target/optimize attributes. So I don't think cfun should be changed > >> just for dumping. > >> > >> Can you instead just set current_function_decl and access > >> struct function via DECL_STRUCT_FUNCTION in the dumpers then? > >> After all, it it is a front-end interface, the frontend way of saying > >> "this is the current function" is to set current_function_decl, not the > >> middle-end cfun. > >> > > > > Like the following? Tested and bootstrapped on x86_64-linux, it does > > help avoid the ada hunk in my previous patch. OK for trunk? > > Ok if nobody complains - btw, I think you miss to restore > current_function_decl > here: > > > > *** /tmp/HcgoTd_tree-cfg.c Wed Aug 22 15:02:30 2012 > > --- gcc/tree-cfg.c Wed Aug 22 11:53:02 2012 > > *** move_sese_region_to_fn (struct function > > *** 6632,6650 > > */ > > > > void > > ! dump_function_to_file (tree fn, FILE *file, int flags) > > { > > ! tree arg, var; > > struct function *dsf; > > bool ignore_topmost_bind = false, any_var = false; > > basic_block bb; > > tree chain; > > ! bool tmclone = TREE_CODE (fn) == FUNCTION_DECL && decl_is_tm_clone (fn); > > > > ! fprintf (file, "%s %s(", current_function_name (), > > ! tmclone ? "[tm-clone] " : ""); > > > > ! arg = DECL_ARGUMENTS (fn); > > while (arg) > > { > > print_generic_expr (file, TREE_TYPE (arg), dump_flags); > > --- 6632,6652 > > */ > > > > void > > ! dump_function_to_file (tree fndecl, FILE *file, int flags) > > { > > ! tree arg, var, old_current_fndecl = current_function_decl; > > struct function *dsf; > > bool ignore_topmost_bind = false, any_var = false; > > basic_block bb; > > tree chain; > > ! bool tmclone = (TREE_CODE (fndecl) == FUNCTION_DECL > > ! && decl_is_tm_clone (fndecl)); > > ! struct function *fun = DECL_STRUCT_FUNCTION (fndecl); > > > > ! current_function_decl = fndecl; > > ! fprintf (file, "%s %s(", function_name (fun), tmclone ? "[tm-clone] " : > > ""); > > > > ! arg = DECL_ARGUMENTS (fndecl); > > while (arg) > > { > > print_generic_expr (file, TREE_TYPE (arg), dump_flags); > > *** dump_function_to_file (tree fn, FILE *fi > > *** 6659,6689 > > fprintf (file, ")\n"); > > > > if (flags & TDF_VERBOSE) > > ! print_node (file, "", fn, 2); > > > > ! dsf = DECL_STRUCT_FUNCTION (fn); > > if (dsf && (flags & TDF_EH)) >
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 2012-08-24 08:35 , Jakub Jelinek wrote: You haven't built your compiler with --disable-bootstrap, so you aren't seeing what Mike is complaining about. Ah, Mike failed to mention that detail. Mike, it is unlikely that I will be able to work on a fix before I leave. It does not look like a difficult fix in any case. Would you mind preparing a patch? If not, I'll get to it after I return. Thanks. Diego.
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Fri, Aug 24, 2012 at 08:30:36AM -0400, Diego Novillo wrote: > On 2012-08-23 16:54 , Mike Stump wrote: > >On Aug 12, 2012, at 1:04 PM, Diego Novillo wrote: > >>Other than the bootstrap change, the patches make no functional > >>changes to the compiler. Everything should build as it does now in > >>trunk. > > > >In my gcc/Makefile, I see: > > > >CFLAGS = -g CXXFLAGS = -g -O2 > > Odd. I see: > > CFLAGS = -g > CXXFLAGS = -g > > in stage1-gcc/ > > In gcc/ I see both set to -g -O2, of course. I would've noticed -g > -O2 in the stage1 build because I am constantly debugging that > binary. You haven't built your compiler with --disable-bootstrap, so you aren't seeing what Mike is complaining about. Jakub
Re: Merge C++ conversion into trunk (0/6 - Overview)
On 2012-08-23 16:54 , Mike Stump wrote: On Aug 12, 2012, at 1:04 PM, Diego Novillo wrote: Other than the bootstrap change, the patches make no functional changes to the compiler. Everything should build as it does now in trunk. In my gcc/Makefile, I see: CFLAGS = -g CXXFLAGS = -g -O2 Odd. I see: CFLAGS = -g CXXFLAGS = -g in stage1-gcc/ In gcc/ I see both set to -g -O2, of course. I would've noticed -g -O2 in the stage1 build because I am constantly debugging that binary. Diego.
Re: Ping: [PATCH] Enable bbro for -Os
On Wed, Aug 22, 2012 at 8:49 AM, Zhenqiang Chen wrote: >> The patch is to enable bbro for -Os. When optimizing for size, it >> * avoid duplicating block. >> * keep its original order if there is no chance to fall through. >> * ignore edge frequency and probability. >> * handle predecessor first if its index is smaller to break long trace. You do this by inserting the index as a key. I don't fully understand this change. You're assuming that a block with a lower index has a lower pre-order number in the CFG's DFS spanning tree, IIUC (i.e. the blocks are numbered sequentially)? I'm not sure that's always true. I think you should add an explanation for this heuristic. >> * only connect Trace n with Trace n + 1 to reduce long jump. ... >> * bb-reorder.c (connect_better_edge_p): New added. >> (find_traces_1_round): When optimizing for size, ignore edge >> frequency >> and probability, and handle all in one round. >> (bb_to_key): Use bb->index as key for size. >> (better_edge_p): The smaller bb index is better for size. >> (connect_traces): Connect block n with block n + 1; >> connect trace m with trace m + 1 if falling through. >> (copy_bb_p): Avoid duplicating blocks. >> (gate_handle_reorder_blocks): Enable bbro when optimizing for -Os. This probably fixes PR54364. > @@ -1169,6 +1272,10 @@ copy_bb_p (const_basic_block bb, int code_may_grow) >int max_size = uncond_jump_length; >rtx insn; > > + /* Avoid duplicating blocks for size. */ > + if (optimize_function_for_size_p (cfun)) > +return false; > + >if (!bb->frequency) > return false; This shouldn't be necessary, due to the CODE_MAY_GROW argument, and this change should result in a code size increase because jumps to conditional jumps aren't removed anymore. What did you make this change for, do you have a test case where code size increases if you allow copy_bb_p to return true? Ciao! Steven
Re: [Patch,AVR] PR54222: Add fixed point support
2012/8/23 Georg-Johann Lay : > Denis Chertykov wrote: >> 2012/8/13 Georg-Johann Lay: >>> Denis Chertykov wrote: 2012/8/11 Georg-Johann Lay: > Weddington, Eric schrieb: >>> From: Georg-Johann Lay >>> >>> >>> The first step would be to bisect and find the patch that lead to >>> PR53923. It was not a change in the avr BE, so the question goes >>> to the authors of the respective patch. >>> >>> Up to now I didn't even try to bisect; that would take years on the >>> host that I have available... >>> My only real concern is that this is a major feature addition and the AVR port is currently broken. >>> I don't know if it's the avr port or some parts of the middle end that >>> don't cooperate with avr. >> I would really, really love to see fixed point support added in, >> especially since I know that Sean has worked on it for quite a while, >> and you've also done a lot of work in getting the patches in shape to >> get them committed. >> >> But, if the AVR port is currently broken (by whomever, and whatever >> patch) and a major feature like this can't be tested to make sure it >> doesn't break anything else in the AVR backend, then I'm hesitant to >> approve (even though I really want to approve). > I don't understand enough of DF to fix PR53923. The insn that leads > to the ICE is (in df-problems.c:dead_debug_insert_temp): > Today I have updated GCC svn tree and successfully compiled avr-gcc. The libgcc2-mulsc3.c from also compiled without bugs. Denis. PS: May be I'm doing something wrong ? (I had too long vacations) >>> I am configuring with --target=avr --disable-nls --with-dwarf2 >>> --enable-languages=c,c++ --enable-target-optspace=yes >>> --enable-checking=yes,rtl >>> >>> Build GCC is "gcc version 4.3.2". >>> Build and host are i686-pc-linux-gnu. >>> >>> Maybe it's different on a 64-bit computer, but I only have 32-bit host. >>> >> >> I have debugging PR53923 and on my opinion it's not an AVR port bug. >> Please commit fixed point support. >> >> Denis. > > Hi, here is an updated patch. > > Some functions are reworked and there is some code clean up. > > The test results look good, there are no additional regressions. > > The new test cases in gcc.dg/fixed-point pass except some convert-*.c for > two reasons: > > * Some test cases have a loss of precision and therefore fail. > One fail is that 0x3fffc000 is compared against > 0x4000 and thus fails. Presumably its a rounding > error from float. I'd say this is not critical. > > * PR54330: This leads to wrong code for __satfractudadq and the > wrong code is already present in .expand. From the distance > this looks like a middle-end or tree-ssa problem. > > The new patch implements TARGET_BUILD_BUILTIN_VA_LIST. > Rationale is that avr-fixed.md adjust some modes bit these > changes are not reflected by the built-in macros made by gcc. > This leads to wrong code in libgcc because it deduces the > type layout from these built-in defines. Thus, the respective > nodes must be patches *before* built-in macros are emit. > > The changes to LIB2FUNCS_EXCLUDE currently have no effects, > this needs http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01580.html > which is currently under review. > > Ok to install? > Please commit. Denis.
Re: VxWorks Patches Back from the Dead!
Il 23/08/2012 21:37, rbmj ha scritto: >> In gcc/gcov-io.c, the call to open() only has two arguments. This >> is fine, as long as the system open() is standards compliant. So you have to add another fixincludes hack, adding a macro indirection like the one you have for ioctl: #define open(a, b, ...) __open(a, b , ##__VA_ARGS__, 0660) #define __open(a, b, c, ...) (open)(a, b, c) > Also forgot to note: I've seen passing the extra argument > unconditionally (even though it's for a read-only open) other places in > GCC sources, so that seems to be accepted practice. It doesn't really seem to be the case, they look more like cut-and-paste: With: ./c-family/c-pch.c: fd = open (name, O_RDONLY | O_BINARY, 0666); ./mips-tfile.c: in_fd = open (object_name, O_RDONLY, 0666); ./gcc.c: desc = open (filename, O_RDONLY, 0); Without: ./mips-tdump.c: tfile_fd = open (argv[optind], O_RDONLY); ./ggc-page.c: G.dev_zero_fd = open ("/dev/zero", O_RDONLY); ./lto/lto.c: fd = open (file_data->file_name, O_RDONLY|O_BINARY); ./ggc-zone.c: G.dev_zero_fd = open ("/dev/zero", O_RDONLY); ./gcov-io.c: fd = open (name, O_RDONLY); ./collect2-aix.c: ldfile->fd = open (filename, O_RDONLY); ./java/resource.c: fd = open (filename, O_RDONLY | O_BINARY); ./java/win32-host.c: return open (filename, oflag); ./java/jcf-io.c: fd = open (zipfile, O_RDONLY | O_BINARY); ./java/jcf-io.c: int fd = open (filename, O_RDONLY | O_BINARY); ./gcc.c: int fd = open (cmpfile[i], O_RDONLY); ./config/rs6000/driver-rs6000.c: fd = open ("/proc/self/auxv", O_RDONLY); ./config/rs6000/driver-rs6000.c: fd = open ("/proc/self/auxv", O_RDONLY); ./config/alpha/host-osf.c: procfd = open (pname, O_RDONLY); Paolo
Re: Reproducible gcc builds, gfortran, and -grecord-gcc-switches
On 21 August 2012 17:18, Joseph S. Myers wrote: > > On Tue, 21 Aug 2012, Simon Baldwin wrote: > > > Index: gcc/doc/options.texi > > === > > --- gcc/doc/options.texi (revision 190535) > > +++ gcc/doc/options.texi (working copy) > > @@ -468,4 +468,8 @@ of @option{-@var{opt}}, if not explicitl > > specify several different languages. Each @var{language} must have > > been declared by an earlier @code{Language} record. @xref{Option file > > format}. > > + > > +@item NoDWARFRecord > > +The option is added to the list of those omitted from the producer > > string > > +written by @option{-grecord-gcc-switches}. > > Remove "added to the list of those" (which seems unnecessarily verbose). > > > +@item @samp{nodwarfrecord} > > +Display only those options that are marked for addition to the list of > > +options omitted from @option{-grecord-gcc-switches}. > > I don't think there's any need for special --help support for options with > this flag; this flag is really an implementation detail. (Thus, I think > all the opts.c changes are unnecessary.) Thanks, revised and shorter version below. Please take another look when ready. -- Omit OPT_cpp_ from the DWARF producer string in gfortran. Gfortran uses -cpp= internally, and with -grecord_gcc_switches this command line switch is stored by default in object files. This causes problems with build and packaging systems that care about gcc binary reproducibility and file checksums; the temporary file is different on each compiler invocation. Fixed by adding a new opt marker NoDWARFRecord and associated flag, filtering options for this this setting when writing the producer string, and setting this flag for fortran -cpp= Tested for fortran (suppresses -cpp=...) and c (no effect). gcc/ChangeLog 2012-08-24 Simon Baldwin * dwarf2out.c (gen_producer_string): Omit command line switch if CL_NO_DWARF_RECORD flag set. * opts.h (CL_NO_DWARF_RECORD): New. * opt-functions.awk (switch_flags): Add NoDWARFRecord. * doc/options.texi: Document NoDWARFRecord option flag. gcc/fortran/ChangeLog 2012-08-24 Simon Baldwin * lang.opt (-cpp=): Mark flag NoDWARFRecord. Index: gcc/doc/options.texi === --- gcc/doc/options.texi(revision 190642) +++ gcc/doc/options.texi(working copy) @@ -468,4 +468,8 @@ of @option{-@var{opt}}, if not explicitl specify several different languages. Each @var{language} must have been declared by an earlier @code{Language} record. @xref{Option file format}. + +@item NoDWARFRecord +The option is omitted from the producer string written by +@option{-grecord-gcc-switches}. @end table Index: gcc/dwarf2out.c === --- gcc/dwarf2out.c (revision 190642) +++ gcc/dwarf2out.c (working copy) @@ -18138,6 +18138,9 @@ gen_producer_string (void) /* Ignore these. */ continue; default: +if (cl_options[save_decoded_options[j].opt_index].flags + & CL_NO_DWARF_RECORD) + continue; gcc_checking_assert (save_decoded_options[j].canonical_option[0][0] == '-'); switch (save_decoded_options[j].canonical_option[0][1]) Index: gcc/opts.h === --- gcc/opts.h (revision 190642) +++ gcc/opts.h (working copy) @@ -145,6 +145,7 @@ extern const unsigned int cl_lang_count; #define CL_JOINED (1U << 22) /* If takes joined argument. */ #define CL_SEPARATE(1U << 23) /* If takes a separate argument. */ #define CL_UNDOCUMENTED(1U << 24) /* Do not output with --help. */ +#define CL_NO_DWARF_RECORD (1U << 25) /* Do not add to producer string. */ /* Flags for an enumerated option argument. */ #define CL_ENUM_CANONICAL (1 << 0) /* Canonical for this value. */ Index: gcc/fortran/lang.opt === --- gcc/fortran/lang.opt(revision 190642) +++ gcc/fortran/lang.opt(working copy) @@ -295,7 +295,7 @@ Fortran Negative(nocpp) Enable preprocessing cpp= -Fortran Joined Negative(nocpp) Undocumented +Fortran Joined Negative(nocpp) Undocumented NoDWARFRecord ; Internal option generated by specs from -cpp. nocpp Index: gcc/opt-functions.awk === --- gcc/opt-functions.awk (revision 190642) +++ gcc/opt-functions.awk (working copy) @@ -103,6 +103,7 @@ function switch_flags (flags) test_flag("JoinedOrMissing", flags, " | CL_JOINED") \ test_flag("Separate", flags, " | CL_SEPARATE") \ test_flag("Undocumented", flags, " | CL_UNDOCUMENTED") \ + test_flag("NoDWARFRecord", flags, " | CL_NO_DWARF_RECORD") \ test_flag("
Re: Re-implement VEC_* to be member functions of vec_t
On Thu, 23 Aug 2012, Diego Novillo wrote: > I think I would like to explore the idea of implement a stack > unwinder that's used by gcc_assert(). This way: (a) we do not > need to uglify all the APIs with these extra arguments, (b) we > can control how much of the call stack we show on an assertion. > > Would that be something difficult to implement? I don't think we > need something as generic as libunwind. Thoughts? Stack unwinding is heavily host-specific. For example, on glibc hosts you can use backtrace () - but for many architectures this requires you to build with unwind tables enabled. To convert the addresses to function names you can use backtrace_symbols () - but that requires the program to be linked with -rdynamic so that the symbols are in the dynamic symbol table. (A plugin-enabled compiler will be linked with -rdynamic anyway.) Other hosts may or may not have such interfaces. In the presence of inlining, such interfaces may give less helpful results - and in any case they seem unlikely to give source line numbers, just function names. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Changes in mode switching
Thank you for testing. > With commenting out "if (i != mode)" of the hunk I changed type of transp and added this checking because if we reset transp[mode], then later in the loop FOR_EACH_BB (bb) sbitmap_not (kill[bb->index], transp[i][bb->index]); we set kill of the bb for that mode and thereby force insertion mode switching for the mode in succeeding blocks in any case. Regards, Vladimir 2012/8/24 Kaz Kojima : >> I've tried the patch on sh4-unknown-linux-gnu. I see new failures >> with it: > > Here is a reduced test case for sh4-unknown-linux-gnu. > > volatile double gd[32]; > volatile float gf[32]; > > int main () > { > int i; > > for (i = 0; i < 32; i++) > gd[i] = i * 4, gf[i] = i; > > for (i = 0; i < 32; i++) > if (gd[i] != i * 4 > || gf[i] != i) > abort (); > exit (0); > } > > The problem occurs at the second loop. With the patch, the only > mode switching is done at just before gf[i] != i. > OTOH the original compiler inserts mode switchings both at before > gd[i] != i * 4 and gf[i] != i. > With commenting out "if (i != mode)" of the hunk > > @@ -530,10 +535,16 @@ optimize_mode_switching (void) > last_mode = mode; > ptr = new_seginfo (mode, insn, bb->index, live_now); > add_seginfo (info + bb->index, ptr); > - RESET_BIT (transp[bb->index], j); > + for (i = 0 ; i < max_num_modes; i++) > + if (i != mode) > + RESET_BIT (transp[i][bb->index], j); > ... > > it looks all new failures go away. > > Regards, > kaz
[patch] obvious: don't dump out-of-ssa partitions for virtual operands
... because they're not partitioned anyway, so they just confuse the dumps: Will bootstrap&test, and commit if nothing strange shows up. Ciao! Steven * tree-ssa-live.c (dump_var_map): Do not dump the partition map of virtual operands. Index: tree-ssa-live.c === --- tree-ssa-live.c (revision 190601) +++ tree-ssa-live.c (working copy) @@ -1140,7 +1140,8 @@ dump_var_map (FILE *f, var_map map) else p = x; - if (ssa_name (p) == NULL_TREE) + if (ssa_name (p) == NULL_TREE + || virtual_operand_p (ssa_name (p))) continue; t = 0;
Re: [PATCH] Changes in mode switching
> I've tried the patch on sh4-unknown-linux-gnu. I see new failures > with it: Here is a reduced test case for sh4-unknown-linux-gnu. volatile double gd[32]; volatile float gf[32]; int main () { int i; for (i = 0; i < 32; i++) gd[i] = i * 4, gf[i] = i; for (i = 0; i < 32; i++) if (gd[i] != i * 4 || gf[i] != i) abort (); exit (0); } The problem occurs at the second loop. With the patch, the only mode switching is done at just before gf[i] != i. OTOH the original compiler inserts mode switchings both at before gd[i] != i * 4 and gf[i] != i. With commenting out "if (i != mode)" of the hunk @@ -530,10 +535,16 @@ optimize_mode_switching (void) last_mode = mode; ptr = new_seginfo (mode, insn, bb->index, live_now); add_seginfo (info + bb->index, ptr); - RESET_BIT (transp[bb->index], j); + for (i = 0 ; i < max_num_modes; i++) + if (i != mode) + RESET_BIT (transp[i][bb->index], j); ... it looks all new failures go away. Regards, kaz
Re: PATCH: Properly handle arg_pointer and frame_pointer in DWARF output
On Sat, Apr 28, 2012 at 08:11:14AM -0700, H.J. Lu wrote: > arg_pointer and frame_pointer are handled as special cases in > based_loc_descr. > > (plus:DI (reg/f:DI 16 argp) > (const_int -20 [0xffec])) > > is perfectly valid when Pmode == DImode and DWARF2_ADDR_SIZE is 32bit > with ptr_mode == SImode. This patch fixes ICE on the 2 testcases here. > OK for trunk? Ok. I must say I don't like these ugly hacks for wider Pmode than DWARF2_ADDR_SIZE very much, and they keep being added in lots of places, but I can live with this change. > 2012-04-06 H.J. Lu > > PR debug/52857 > * dwarf2out.c (mem_loc_descriptor): Allow arg_pointer_rtx and > frame_pointer_rtx for based_loc_descr. > > gcc/testsuite/ > > 2012-04-06 H.J. Lu > > PR debug/52857 > * gcc.target/i386/pr52857-1.c: New. > * gcc.target/i386/pr52857-2.c: Likewise. > > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c > index ca88fc5..515a824 100644 > --- a/gcc/dwarf2out.c > +++ b/gcc/dwarf2out.c > @@ -11655,6 +11657,8 @@ mem_loc_descriptor (rtx rtl, enum machine_mode mode, > case REG: >if (GET_MODE_CLASS (mode) != MODE_INT > || (GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE > + && rtl != arg_pointer_rtx > + && rtl != frame_pointer_rtx > #ifdef POINTERS_EXTEND_UNSIGNED > && (mode != Pmode || mem_mode == VOIDmode) > #endif > @@ -11927,7 +11931,9 @@ mem_loc_descriptor (rtx rtl, enum machine_mode mode, > case PLUS: > plus: >if (is_based_loc (rtl) > - && GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE > + && (GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE > + || XEXP (rtl, 0) == arg_pointer_rtx > + || XEXP (rtl, 0) == frame_pointer_rtx) > && GET_MODE_CLASS (mode) == MODE_INT) > mem_loc_result = based_loc_descr (XEXP (rtl, 0), > INTVAL (XEXP (rtl, 1)), > diff --git a/gcc/testsuite/gcc.target/i386/pr52857-1.c > b/gcc/testsuite/gcc.target/i386/pr52857-1.c > new file mode 100644 > index 000..16fd78f > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr52857-1.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile { target { ! { ia32 } } } } */ > +/* { dg-options "-g -O -mx32 -maddress-mode=long" } */ > + > +extern void get_BID128 (int *); > +void > +__bid128_div (void) > +{ > + int res; > + get_BID128 (&res); > +} > diff --git a/gcc/testsuite/gcc.target/i386/pr52857-2.c > b/gcc/testsuite/gcc.target/i386/pr52857-2.c > new file mode 100644 > index 000..879240a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr52857-2.c > @@ -0,0 +1,8 @@ > +/* { dg-do compile { target { ! { ia32 } } } } */ > +/* { dg-options "-g -O -mx32 -maddress-mode=long" } */ > + > +void uw_init_context_1 (void *); > +void _Unwind_ForcedUnwind (void) > +{ > + uw_init_context_1 (__builtin_dwarf_cfa ()); > +} Jakub
Re: Merge C++ conversion into trunk (0/6 - Overview)
On Aug 24, 2012, at 12:24 AM, Paolo Bonzini wrote: > Agreed, patch is preapproved. This is not really done to aid debugging > though, it is to avoid optimization bugs when compiling stage1. Ah, but building a non-bootstrap compiler from the top-level builds -O2 and when built from the gcc subtree, builds -O0. Ever wonder why? It isn't to avoid code-gen errors in CC. it is to make the developers life easier. I know, so much of gcc's history is lost to time and at times, not handed down to the new kids. The bad bits, just fade away. The useful things, for example, this, will live on, as some of use still know about and use the feature. When it breaks, we complain.
Re: [PATCH, ARM] Constant vector permute for the Neon vext insn
On 24 August 2012 10:40, Richard Earnshaw wrote: > > Has this been tested for big-endian? > > R. No. I'll give a look at it and let you know. Christophe.
Re: [PATCH, ARM] Constant vector permute for the Neon vext insn
On 24/08/12 08:45, Christophe Lyon wrote: > Hi, > > The patch below enables GCC for ARM to implement relevant constant > vector permutations using the Neon vext instruction, by extending the > support currently in place for vrev, vzip, vunzip and vtrn. > > For the cases where vext and vrev would lead to the same result, I > have chosen to keep using vrev to avoid updating the testsuite when > both are equivalent (1 cycle) or when vrev is faster (1 cycle when > operating on Qn vs 2 cycles for vext). > > Tested with qemu-arm on arm-none-linux-gnueabi. > > Christophe. > > 2012-08-23 Christophe Lyon > > gcc/ > * config/arm/arm.c (arm_evpc_neon_vext): New > function. > (arm_expand_vec_perm_const_1): Add call to > arm_evpc_neon_vext. > > gcc/testsuite/ > * gcc.target/arm/neon-vext.c: New tests.= > Has this been tested for big-endian? R.
[PATCH, ARM] Constant vector permute for the Neon vext insn
Hi, The patch below enables GCC for ARM to implement relevant constant vector permutations using the Neon vext instruction, by extending the support currently in place for vrev, vzip, vunzip and vtrn. For the cases where vext and vrev would lead to the same result, I have chosen to keep using vrev to avoid updating the testsuite when both are equivalent (1 cycle) or when vrev is faster (1 cycle when operating on Qn vs 2 cycles for vext). Tested with qemu-arm on arm-none-linux-gnueabi. Christophe. 2012-08-23 Christophe Lyon gcc/ * config/arm/arm.c (arm_evpc_neon_vext): New function. (arm_expand_vec_perm_const_1): Add call to arm_evpc_neon_vext. gcc/testsuite/ * gcc.target/arm/neon-vext.c: New tests. gcc-vec-permute-vext.patch Description: Binary data
Re: [PATCH, x86-Atom] Enabling look-ahead scheduling feature for Atom processors
On Fri, Aug 24, 2012 at 9:22 AM, Igor Zamyatin wrote: > Following change enables look ahead feature in the code scheduler for > Atom processors. This gives quite reasonable gain for some benchmarks > for mobile market. > > Overall compile time increase for SPEC2000 is about 1%. > > Regtested for x86_64 and also bootstrapped with "--with-arch=core2 > --with-cpu=atom" > > 2012-08-23 Yuri Rumyantsev > > * config/i386/i386.c (ia32_multipass_dfa_lookahead) : Add > case for Atom processor. OK. Thanks, Uros.
Re: Merge C++ conversion into trunk (0/6 - Overview)
Il 23/08/2012 22:54, Mike Stump ha scritto: > > # Remove the -O2: for historical reasons, unless bootstrapping we prefer > > # optimizations to be activated explicitly by the toplevel. > > case "$CC" in > */prev-gcc/xgcc*) ;; > *) CFLAGS=`echo $CFLAGS | sed "s/-O[[s0-9]]* *//" ` ;; > esac > AC_SUBST(CFLAGS) > > in configure.ac does this. I think if CXXFLAGS is also so done, we'd gain > parity. Agreed, patch is preapproved. This is not really done to aid debugging though, it is to avoid optimization bugs when compiling stage1. Paolo
[PATCH, x86-Atom] Enabling look-ahead scheduling feature for Atom processors
Hi! Following change enables look ahead feature in the code scheduler for Atom processors. This gives quite reasonable gain for some benchmarks for mobile market. Overall compile time increase for SPEC2000 is about 1%. Regtested for x86_64 and also bootstrapped with "--with-arch=core2 --with-cpu=atom" Ok for trunk? Thanks, Igor Changelog: 2012-08-23 Yuri Rumyantsev * config/i386/i386.c (ia32_multipass_dfa_lookahead) : Add case for Atom processor. diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 976bbb4..331e29a 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -24103,6 +24103,7 @@ ia32_multipass_dfa_lookahead (void) case PROCESSOR_CORE2_64: case PROCESSOR_COREI7_32: case PROCESSOR_COREI7_64: +case PROCESSOR_ATOM: /* Generally, we want haifa-sched:max_issue() to look ahead as far as many instructions can be executed on a cycle, i.e., issue_rate. I wonder why tuning for many CPUs does not do this. */