RE: [PATCH][ARM][thumb1] Reduce lr save for leaf function with non-far jump
Ping > -Original Message- > From: Joey Ye > Sent: Thursday, December 20, 2012 17:53 > To: gcc-patches@gcc.gnu.org; Ramana Radhakrishnan > Cc: Joey Ye > Subject: [PATCH][ARM][thumb1] Reduce lr save for leaf function with non- > far jump > > Current GCC thumb1 has an annoying problem that always assuming far > branch. > So it forces to save lr, even when unnecessarily. The most extreme case > complained by partner is: > > // compiled with "-mthumb -mcpu=cortex-m0 -Os". > void foo() { for (;;); } > => > foo: > push{lr} // Crazy!!! > .L2: > b .L2 > > The reason is that thumb1 far jump is only resolved in the very late > pass > "shorten_branch". Prologue/epilogue pass doesn't actually know a branch > is > far or not from its attribute. It has to conservatively save/restore lr > whenever there is a branch. > > This patch tries to fix it with a simple heuristic, i.e., using function > size to decide if a far jump will likely be used. Function size > information > is meaningful in prologue/epilogue pass. The heuristic uses following > check > to decide if lr should be saved for far jump: > > function_size * 3 >= 2048 // yes: save lr for possible far jump. No: > don't > save lr for far jump > > The scheme has an issue: if some corner case does break above condition, > there is no chance to fix-up but to ICE. But the heuristic condition is > very > conservative. It is base on the worse normal condition that each > instruction > is associated with a 4 byte literal ( (2+4)/2=3, blooming size by 3 > times ). > I can't think of a real case to trigger the ICE. So I think it should > work. > > Other approaches than the heuristic scheme are too expensive to > implement > for this small size/performance issue. I did explored some but none of > them > persuaded myself. > > Tests passed: > * build libgcc, libstdc++, newlib, libm > * make check-gcc with cpu=cortex-m0 > * Small and extreme test cases > > ChangeLog: > > 2012-12-20 Joey Ye > > * config/arm/arm.c(thumb1_final_prescan_insn): > Assert lr save for real far jump. > (thumb_far_jump_used_p): Count instruction size and set > far_jump_used. > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index 327ef22..ad79451 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -21790,6 +21857,11 @@ thumb1_final_prescan_insn (rtx insn) >else if (conds != CONDS_NOCOND) > cfun->machine->thumb1_cc_insn = NULL_RTX; > } > + > +/* Check if unexpected far jump is used. */ > +if (cfun->machine->lr_save_eliminated > +&& get_attr_far_jump (insn) == FAR_JUMP_YES) > + internal_error("Unexpected thumb1 far jump"); > } > > int > @@ -21815,6 +21887,8 @@ static int > thumb_far_jump_used_p (void) > { >rtx insn; > + bool far_jump = false; > + unsigned int func_size = 0; > >/* This test is only important for leaf functions. */ >/* assert (!leaf_function_p ()); */ > @@ -21870,6 +21944,26 @@ thumb_far_jump_used_p (void) > && get_attr_far_jump (insn) == FAR_JUMP_YES > ) > { > + far_jump = true; > + } > + func_size += get_attr_length (insn); > +} > + > + /* Attribute far_jump will always be true for thumb1 before > shorten_branch > + pass. So checking far_jump attribute before shorten_branch isn't > much > + useful. > + > + Following heuristic tries to estimate more accruately if a far > jump > may > + finally be used. The heuristic is very conservative as there is no > chance > + to roll-back the decision of not to use far jump. > + > + Thumb1 long branch offset is -2048 to 2046. The worst case is each > 2-byte > + insn is assiociated with a 4 byte constant pool. Using function > size > + 2048/3 as the threshold is conservative enough. */ > + if (far_jump) > +{ > + if ((func_size * 3) >= 2048) > +{ > /* Record the fact that we have decided that >the function does use far jumps. */ > cfun->machine->far_jump_used = 1; > > >
Re: [google 4-7] adjust single target i-call heursitic (issue7059044)
Need to document the parameter in doc/invoke.texi. Ok with that change. David On Fri, Jan 4, 2013 at 5:28 PM, Rong Xu wrote: > Hi, > > This patch adjusts the single target indirect call promotion heuristics. > (1) it uses bb counts (bb_all), instead of count "all" which only counts > the targets with the same modules as the caller. > (becaue __gcov_indirect_call_callee is file scope statics). > Using overall bb counts makes more sense here. > (2) use a prameter option to control the threshold. > (3) change the default threshold from 75% to 67%. > > Tested with google internal benchmarks and gcc regression test. > > Thanks, > > -Rong > > 2013-01-04 Rong Xu > > * gcc/value-prof.c (gimple_ic_transform_single_targ): > adjust the single_target i_call promotion heuristics. > * gcc/params.def.h: Add new parameter options. > Index: gcc/value-prof.c > === > --- gcc/value-prof.c(revision 194916) > +++ gcc/value-prof.c(working copy) > @@ -1491,16 +1491,20 @@ gimple_ic_transform_single_targ (gimple stmt, hist >gcov_type prob; >gimple modify; >struct cgraph_node *direct_call; > + int perc_threshold, count_threshold; > >val = histogram->hvalue.counters [0]; >count = histogram->hvalue.counters [1]; >all = histogram->hvalue.counters [2]; >gimple_remove_histogram_value (cfun, stmt, histogram); > + bb_all = gimple_bb (stmt)->count; > > - if (4 * count <= 3 * all) > + perc_threshold = PARAM_VALUE > (PARAM_SINGLE_ICALL_PROMOTE_PERCENT_THRESHOLD); > + count_threshold = PARAM_VALUE (PARAM_SINGLE_ICALL_PROMOTE_COUNT_THRESHOLD); > + > + if (100 * count < bb_all * perc_threshold || count < count_threshold) > return false; > > - bb_all = gimple_bb (stmt)->count; >/* The order of CHECK_COUNTER calls is important - > since check_counter can correct the third parameter > and we want to make count <= all <= bb_all. */ > @@ -1508,6 +1512,7 @@ gimple_ic_transform_single_targ (gimple stmt, hist >|| check_counter (stmt, "ic", &count, &all, all)) > return false; > > + all = bb_all; >if (all > 0) > prob = (count * REG_BR_PROB_BASE + all / 2) / all; >else > Index: gcc/params.def > === > --- gcc/params.def (revision 194916) > +++ gcc/params.def (working copy) > @@ -907,6 +907,7 @@ DEFPARAM (PARAM_LOOP_INVARIANT_MAX_BBS_IN_LOOP, > /* Promote indirect call to conditional direct call only > when the percentage of the target count over the total > indirect call count is no smaller than the threshold. */ > +/* For multi-targ indirect_call. */ > DEFPARAM (PARAM_ICALL_PROMOTE_PERCENT_THRESHOLD, > "icall-promote-target-percent-threshold", > "percentage threshold for direct call promotion" > @@ -919,6 +920,19 @@ DEFPARAM (PARAM_ICALL_PROMOTE_COUNT_THRESHOLD, >" of a callee target", > 1, 0, 0) > > +/* For single-targ indirect_call. */ > +DEFPARAM (PARAM_SINGLE_ICALL_PROMOTE_PERCENT_THRESHOLD, > + "single_icall-promote-target-percent-threshold", > + "percentage threshold for direct call promotion" > + " of a callee target", > + 67, 0, 100) > + > +DEFPARAM (PARAM_SINGLE_ICALL_PROMOTE_COUNT_THRESHOLD, > + "single_icall-promote-target_count-threshold", > + "call count threshold for direct call promotion" > + " of a callee target", > + 1, 0, 0) > + > /* 0: do not always inline icall target: > other value: always inline icall target when call count > exceeds this value. > > -- > This patch is available for review at http://codereview.appspot.com/7059044
Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments
On Fri, Jan 4, 2013 at 2:32 PM, Xinliang David Li wrote: > Looks good -- but better with followup : > 1) give a warning when the parameter to the option is not allowed; > 2) add test cases if possible. Made all the changes. Modified the test case to check if the segment splitting API is invoked. The gold linker has a test case to check if the segment API actually splits segments. Thanks, -Sri. > > David > > > On Fri, Jan 4, 2013 at 2:19 PM, Sriraman Tallam wrote: >> Attached new patch. >> >> Thanks, >> -Sri. >> >> On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu wrote: >>> The code looks fine to me. Please consider David's comments about the >>> option name. >>> >>> -Rong >>> >>> On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li >>> wrote: Is it better to change the option to something like: split_segment|nosplit-segment or split_segment=yes|no David On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam wrote: > Hi Rong, > > The following patch modifies the behaviour of the linker plugin to > not create a separate segment for cold sections by default. Separate > segments can be created with the plugin option "segment=cold". Is this > alright to commit? > > Thanks, > -Sri. > > On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam > wrote: >> I have committed this patch. >> >> Thanks, >> -Sri. >> >> On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu wrote: >>> Looks good to me for google/gcc-4_7 branch. >>> >>> Thanks, >>> >>> -Rong >>> >>> >>> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam >>> wrote: Hi Rong, Please review this code. This code allows the function reordering plugin to separate hot and cold code into different ELF segments. This would allow optimizations like mapping the hot code alone to huge pages. With this patch, by default, the plugin maps .text.unlikely sections into a separate ELF segment. This can be turned off with plugin option "--segment=none". The include/plugin-api.h changes are a backport from trunk. Thanks, -Sri. >>> >>> Index: function_reordering_plugin/function_reordering_plugin.c === --- function_reordering_plugin/function_reordering_plugin.c (revision 194878) +++ function_reordering_plugin/function_reordering_plugin.c (working copy) @@ -33,9 +33,13 @@ along with this program; see the file COPYING3. I This plugin dumps the final layout order of the functions in a file called "final_layout.txt". To change the output file, pass the new - file name with --plugin-opt. To dump to stderr instead, just pass - stderr to --plugin-opt. */ + file name with --plugin-opt,file=. To dump to stderr instead, + just pass stderr as the file name. + This plugin also allows placing all functions found cold in a separate + segment. This can be enabled with the linker option: + --plugin-opt,split_segment=yes. */ + #if HAVE_STDINT_H #include #endif @@ -89,9 +93,10 @@ static int is_api_exist = 0; /* The plugin does nothing when no-op is 1. */ static int no_op = 0; -/* The plugin does not create a new segment for unlikely code if - no_segment is set. */ -static int no_segment = 0; +/* The plugin creates a new segment for unlikely code if split_segment + is set. This can be set with the linker option: + "--plugin-opt,split_segment=yes". */ +static int split_segment = 0; /* Copies new output file name out_file */ void get_filename (const char *name) @@ -110,7 +115,7 @@ process_option (const char *name) { const char *option_group = "group="; const char *option_file = "file="; - const char *option_segment = "segment="; + const char *option_segment = "split_segment="; /* Check if option is "group=" */ if (strncmp (name, option_group, strlen (option_group)) == 0) @@ -121,22 +126,26 @@ process_option (const char *name) no_op = 0; return; } - /* Check if option is "file=" */ - if (strncmp (name, option_file, strlen (option_file)) == 0) + else if (strncmp (name, option_file, strlen (option_file)) == 0) { get_filename (name + strlen (option_file)); return; } - - /* Check if options is "segment=none" */ - if (strncmp (name, option_segment, strlen (option_segment)) == 0) + /* Check if options is "split_segment=[yes|no]" */ + else if (strncmp (name, option_segment, strlen (option_segment)) == 0) { - if (strcmp (name + strlen (option_segment), "none") == 0) - no_segment = 1; - else - no_segment = 0; - return; + const char *option_val = name + strlen (option_segment); + if (strcmp (option_val, "no") == 0) + { + split_segment = 0; + return; +
[google 4-7] adjust single target i-call heursitic (issue7059044)
Hi, This patch adjusts the single target indirect call promotion heuristics. (1) it uses bb counts (bb_all), instead of count "all" which only counts the targets with the same modules as the caller. (becaue __gcov_indirect_call_callee is file scope statics). Using overall bb counts makes more sense here. (2) use a prameter option to control the threshold. (3) change the default threshold from 75% to 67%. Tested with google internal benchmarks and gcc regression test. Thanks, -Rong 2013-01-04 Rong Xu * gcc/value-prof.c (gimple_ic_transform_single_targ): adjust the single_target i_call promotion heuristics. * gcc/params.def.h: Add new parameter options. Index: gcc/value-prof.c === --- gcc/value-prof.c(revision 194916) +++ gcc/value-prof.c(working copy) @@ -1491,16 +1491,20 @@ gimple_ic_transform_single_targ (gimple stmt, hist gcov_type prob; gimple modify; struct cgraph_node *direct_call; + int perc_threshold, count_threshold; val = histogram->hvalue.counters [0]; count = histogram->hvalue.counters [1]; all = histogram->hvalue.counters [2]; gimple_remove_histogram_value (cfun, stmt, histogram); + bb_all = gimple_bb (stmt)->count; - if (4 * count <= 3 * all) + perc_threshold = PARAM_VALUE (PARAM_SINGLE_ICALL_PROMOTE_PERCENT_THRESHOLD); + count_threshold = PARAM_VALUE (PARAM_SINGLE_ICALL_PROMOTE_COUNT_THRESHOLD); + + if (100 * count < bb_all * perc_threshold || count < count_threshold) return false; - bb_all = gimple_bb (stmt)->count; /* The order of CHECK_COUNTER calls is important - since check_counter can correct the third parameter and we want to make count <= all <= bb_all. */ @@ -1508,6 +1512,7 @@ gimple_ic_transform_single_targ (gimple stmt, hist || check_counter (stmt, "ic", &count, &all, all)) return false; + all = bb_all; if (all > 0) prob = (count * REG_BR_PROB_BASE + all / 2) / all; else Index: gcc/params.def === --- gcc/params.def (revision 194916) +++ gcc/params.def (working copy) @@ -907,6 +907,7 @@ DEFPARAM (PARAM_LOOP_INVARIANT_MAX_BBS_IN_LOOP, /* Promote indirect call to conditional direct call only when the percentage of the target count over the total indirect call count is no smaller than the threshold. */ +/* For multi-targ indirect_call. */ DEFPARAM (PARAM_ICALL_PROMOTE_PERCENT_THRESHOLD, "icall-promote-target-percent-threshold", "percentage threshold for direct call promotion" @@ -919,6 +920,19 @@ DEFPARAM (PARAM_ICALL_PROMOTE_COUNT_THRESHOLD, " of a callee target", 1, 0, 0) +/* For single-targ indirect_call. */ +DEFPARAM (PARAM_SINGLE_ICALL_PROMOTE_PERCENT_THRESHOLD, + "single_icall-promote-target-percent-threshold", + "percentage threshold for direct call promotion" + " of a callee target", + 67, 0, 100) + +DEFPARAM (PARAM_SINGLE_ICALL_PROMOTE_COUNT_THRESHOLD, + "single_icall-promote-target_count-threshold", + "call count threshold for direct call promotion" + " of a callee target", + 1, 0, 0) + /* 0: do not always inline icall target: other value: always inline icall target when call count exceeds this value. -- This patch is available for review at http://codereview.appspot.com/7059044
Re: [google 4_7] Backport r194909 (<:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
ok. thanks, David On Fri, Jan 4, 2013 at 3:38 PM, Paul Pluzhnikov wrote: > On Fri, Jan 4, 2013 at 3:27 PM, Xinliang David Li wrote: >> I saw only one test case fix patch from Paolo in trunk. Does this >> patch include more local fixes? > > There was an earlier patch which touched g++.dg/parse/error1{1,2}.C: > > r191712 | paolo | 2012-09-25 07:44:52 -0700 (Tue, 25 Sep 2012) | 15 lines > > /cp > 2012-09-25 Paolo Carlini > > PR c++/54526 > * parser.c (cp_parser_template_id): In C++11 mode simply accept > X<::A>. > > /testsuite > 2012-09-25 Paolo Carlini > > PR c++/54526 > * g++.dg/cpp0x/parse2.C: New. > * g++.dg/parse/error11.C: Adjust. > * g++.dg/parse/error12.C: Likewise. > > The r194909 reverted parser.c change, but didn't revert error1{1,2}.C changes. > > Current state of trunk vs. google/gcc-4_7 branch: > > diff -u ${trunk}/gcc/testsuite/g++.dg/parse/error11.C > ${gcc-4_7}/gcc/testsuite/g++.dg/parse/error11.C > @@ -68,4 +68,4 @@ > > // On the first error message, an additional note about the use of > // -fpermissive should be present > -// { dg-message "17:\\(if you use '-fpermissive' or '-std=c\\+\\+11', > or '-std=gnu\\+\\+11' G\\+\\+ will accept your code\\)" "-fpermissive" > { target c++98 } 19 } > +// { dg-message "17:\\(if you use '-fpermissive' G\\+\\+ will accept > your code\\)" "-fpermissive" { target c++98 } 19 } > > > (Minor adjustment to the expected error message). > > error12.C is identical on trunk and google/gcc-4_7. > > Thanks, > -- > Paul Pluzhnikov
Re: [google 4_7] Backport r194909 (<:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
On Fri, Jan 4, 2013 at 3:27 PM, Xinliang David Li wrote: > I saw only one test case fix patch from Paolo in trunk. Does this > patch include more local fixes? There was an earlier patch which touched g++.dg/parse/error1{1,2}.C: r191712 | paolo | 2012-09-25 07:44:52 -0700 (Tue, 25 Sep 2012) | 15 lines /cp 2012-09-25 Paolo Carlini PR c++/54526 * parser.c (cp_parser_template_id): In C++11 mode simply accept X<::A>. /testsuite 2012-09-25 Paolo Carlini PR c++/54526 * g++.dg/cpp0x/parse2.C: New. * g++.dg/parse/error11.C: Adjust. * g++.dg/parse/error12.C: Likewise. The r194909 reverted parser.c change, but didn't revert error1{1,2}.C changes. Current state of trunk vs. google/gcc-4_7 branch: diff -u ${trunk}/gcc/testsuite/g++.dg/parse/error11.C ${gcc-4_7}/gcc/testsuite/g++.dg/parse/error11.C @@ -68,4 +68,4 @@ // On the first error message, an additional note about the use of // -fpermissive should be present -// { dg-message "17:\\(if you use '-fpermissive' or '-std=c\\+\\+11', or '-std=gnu\\+\\+11' G\\+\\+ will accept your code\\)" "-fpermissive" { target c++98 } 19 } +// { dg-message "17:\\(if you use '-fpermissive' G\\+\\+ will accept your code\\)" "-fpermissive" { target c++98 } 19 } (Minor adjustment to the expected error message). error12.C is identical on trunk and google/gcc-4_7. Thanks, -- Paul Pluzhnikov
[Patch, Fortran] Remove the Fortran-only flag -fno-whole-file
This patch "removes" -fno-whole-file. (Actually, it turns it into "Ignore".) Reasoning: * -fwhole-file/-fno-whole-file was added in 4.5 to make the transition easier; -fwhole-file is the default since 4.6. * There are many wrong-code issues and probably also some ICEs with -fno-whole file. * The generated code of -fwhole-file is faster as it allows for inlining. * -fno-whole-file has been deprecated since 4.6 and announced for removal. * Code cleanup is always nice (diff -w): 17 insertions(+), 80 deletions(-) Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias PS: Dominique pointed out that PR 45128 is a -fwhole-file "regression". However, it mainly shows that gfortran needs the new array descriptor to fix such subpointer issues (and other PRs). 2013-01-05 Tobias Burnus * gfortran.h (gfc_option_t): Remove flag_whole_file. * invoke.texi (-fno-whole-file): Remove. * lang.opt (fwhole-file): Change to Ignore. * options.c (gfc_init_options, gfc_post_options, gfc_handle_option): Remove !flag_whole_file handling * parse.c (resolve_all_program_units, translate_all_program_units, gfc_parse_file): Ditto. * resolve.c (resolve_global_procedure): Ditto. * trans-decl.c (gfc_get_symbol_decl, gfc_get_extern_function_decl, gfc_create_module_variable): Ditto. * trans-types.c (gfc_get_derived_type): Ditto. diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 027cab6..b87bf64 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2287,7 +2287,6 @@ typedef struct int flag_init_character; char flag_init_character_value; int flag_align_commons; - int flag_whole_file; int flag_protect_parens; int flag_realloc_lhs; int flag_aggressive_function_elimination; diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi index d7c3219..4ba94e5 100644 --- a/gcc/fortran/invoke.texi +++ b/gcc/fortran/invoke.texi @@ -182,7 +182,7 @@ and warnings}. -finit-real=@var{} @gol -fmax-array-constructor=@var{n} -fmax-stack-var-size=@var{n} -fno-align-commons @gol --fno-automatic -fno-protect-parens -fno-underscoring -fno-whole-file @gol +-fno-automatic -fno-protect-parens -fno-underscoring @gol -fsecond-underscore -fpack-derived -frealloc-lhs -frecursive @gol -frepack-arrays -fshort-enums -fstack-arrays } @@ -1293,22 +1293,6 @@ in the source, even if the names as seen by the linker are mangled to prevent accidental linking between procedures with incompatible interfaces. -@item -fno-whole-file -@opindex @code{fno-whole-file} -This flag causes the compiler to resolve and translate each procedure in -a file separately. - -By default, the whole file is parsed and placed in a single front-end tree. -During resolution, in addition to all the usual checks and fixups, references -to external procedures that are in the same file effect resolution of -that procedure, if not already done, and a check of the interfaces. The -dependences are resolved by changing the order in which the file is -translated into the backend tree. Thus, a procedure that is referenced -is translated before the reference and the duplication of backend tree -declarations eliminated. - -The @option{-fno-whole-file} option is deprecated and may lead to wrong code. - @item -fsecond-underscore @opindex @code{fsecond-underscore} @cindex underscore diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt index 1535187..c885ff3 100644 --- a/gcc/fortran/lang.opt +++ b/gcc/fortran/lang.opt @@ -591,8 +591,8 @@ Fortran Append underscores to externally visible names fwhole-file -Fortran -Compile all program units at once and check all interfaces +Fortran Ignore +Does nothing. Preserved for backward compatibility. fworking-directory Fortran diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c index e05b935..1b2373b 100644 --- a/gcc/fortran/options.c +++ b/gcc/fortran/options.c @@ -126,7 +126,6 @@ gfc_init_options (unsigned int decoded_options_count, gfc_option.flag_real8_kind = 0; gfc_option.flag_dollar_ok = 0; gfc_option.flag_underscoring = 1; - gfc_option.flag_whole_file = 1; gfc_option.flag_f2c = 0; gfc_option.flag_second_underscore = -1; gfc_option.flag_implicit_none = 0; @@ -266,14 +265,6 @@ gfc_post_options (const char **pfilename) sorry ("-fexcess-precision=standard for Fortran"); flag_excess_precision_cmdline = EXCESS_PRECISION_FAST; - /* Whole program needs whole file mode. */ - if (flag_whole_program) -gfc_option.flag_whole_file = 1; - - /* Enable whole-file mode if LTO is in effect. */ - if (flag_lto) -gfc_option.flag_whole_file = 1; - /* Fortran allows associative math - but we cannot reassociate if we want traps or signed zeros. Cf. also flag_protect_parens. */ if (flag_associative_math == -1) @@ -432,9 +423,6 @@ gfc_post_options (const char **pfilename) gfc_option.warn_tabs = 0; } - if (pedantic && gfc_option.flag_whole_file) -gfc_option.flag_whole_file = 2; - /* Optimization imp
Re: [google 4_7] Backport r194909 (<:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
I saw only one test case fix patch from Paolo in trunk. Does this patch include more local fixes? David On Fri, Jan 4, 2013 at 3:03 PM, Paul Pluzhnikov wrote: > On Fri, Jan 4, 2013 at 9:41 AM, Xinliang David Li wrote: >> ok. > > The patch as sent caused some breakage, I had to adjust test cases a bit. > > Submitting attached patch. > > Thanks, > -- > Paul Pluzhnikov
Re: [google 4_7] Backport r194909 (<:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
On Fri, Jan 4, 2013 at 9:41 AM, Xinliang David Li wrote: > ok. The patch as sent caused some breakage, I had to adjust test cases a bit. Submitting attached patch. Thanks, -- Paul Pluzhnikov Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C === --- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194909) +++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy) @@ -31,5 +31,5 @@ }; void foo::x() throw(bar) { - if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda expressions|expected" } parse error + if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda expressions|expected|invalid" } parse error } Index: gcc/testsuite/g++.dg/parse/error12.C === --- gcc/testsuite/g++.dg/parse/error12.C(revision 194909) +++ gcc/testsuite/g++.dg/parse/error12.C(working copy) @@ -8,6 +8,6 @@ template struct Foo {}; -Foo<::B> foo; // { dg-bogus "error" "error in place of warning" } -// { dg-warning "4: '<::' cannot begin a template-argument list" "warning <::" { target *-*-* } 11 } -// { dg-message "4:'<:' is an alternate spelling for '.'. Insert whitespace between '<' and '::'" "note <:" { target *-*-* } 11 } +Foo<::B> foo; // { dg-bogus "error" "error in place of warning" { target c++98 } } +// { dg-warning "4: '<::' cannot begin a template-argument list" "warning <::" { target c++98 } 11 } +// { dg-message "4:'<:' is an alternate spelling for '.'. Insert whitespace between '<' and '::'" "note <:" { target c++98 } 11 } Index: gcc/testsuite/g++.dg/parse/error11.C === --- gcc/testsuite/g++.dg/parse/error11.C(revision 194909) +++ gcc/testsuite/g++.dg/parse/error11.C(working copy) @@ -16,22 +16,22 @@ }; void method(void) { -typename Foo<::B>::template Nested<::B> n; // { dg-error "17:'<::' cannot begin" "17-begin" } -// { dg-message "17:'<:' is an alternate spelling" "17-alt" { target *-*-* } 19 } -// { dg-error "39:'<::' cannot begin" "39-begin" { target *-*-* } 19 } -// { dg-message "39:'<:' is an alternate spelling" "39-alt" { target *-*-* } 19 } +typename Foo<::B>::template Nested<::B> n; // { dg-error "17:'<::' cannot begin" "17-begin" { target c++98 } } +// { dg-message "17:'<:' is an alternate spelling" "17-alt" { target c++98 } 19 } +// { dg-error "39:'<::' cannot begin" "39-begin" { target c++98 } 19 } +// { dg-message "39:'<:' is an alternate spelling" "39-alt" { target c++98 } 19 } n.template Nested::method(); -n.template Nested<::B>::method(); // { dg-error "22:'<::' cannot begin" "error" } -// { dg-message "22:'<:' is an alternate" "note" { target *-*-* } 24 } +n.template Nested<::B>::method(); // { dg-error "22:'<::' cannot begin" "error" { target c++98 } } +// { dg-message "22:'<:' is an alternate" "note" { target c++98 } 24 } Nested::method(); -Nested<::B>::method(); // { dg-error "11:'<::' cannot begin" "error" } -// { dg-message "11:'<:' is an alternate" "note" { target *-*-* } 27 } +Nested<::B>::method(); // { dg-error "11:'<::' cannot begin" "error" { target c++98 } } +// { dg-message "11:'<:' is an alternate" "note" { target c++98 } 27 } } }; template struct Foo2 {}; -template struct Foo2<::B>; // { dg-error "21:'<::' cannot begin" "begin" } -// { dg-message "21:'<:' is an alternate" "alt" { target *-*-* } 33 } +template struct Foo2<::B>; // { dg-error "21:'<::' cannot begin" "begin" { target c++98 } } +// { dg-message "21:'<:' is an alternate" "alt" { target c++98 } 33 } // { dg-message "25:type/value mismatch" "mismatch" { target *-*-* } 33 } // { dg-error "25:expected a constant" "const" { target *-*-* } 33 } @@ -39,11 +39,11 @@ void func(void) { - Foo<::B> f; // { dg-error "cannot begin" "begin" } -// { dg-message "alternate spelling" "alt" { target *-*-* } 42 } + Foo<::B> f; // { dg-error "cannot begin" "begin" { target c++98 } } +// { dg-message "alternate spelling" "alt" { target c++98 } 42 } f.Foo::method(); - f.Foo<::B>::method(); // { dg-error "8:cannot begin" "begin" } -// { dg-message "8:alternate spelling" "alt" { target *-*-* } 45 } + f.Foo<::B>::method(); // { dg-error "8:cannot begin" "begin" { target c++98 } } +// { dg-message "8:alternate spelling" "alt" { target c++98 } 45 } // Check cases where we the token sequence is the correct one, but there // was no digraph or whitespaces in the middle, so we should not emit @@ -63,9 +63,9 @@ Foo[::value] = 0; } -template struct Foo<::B>; // { dg-error "20:'<::' cannot begin" "begin" } -// { dg-message "20:is an alternate" "alt" { target *-*-* } 66 } +template struct Foo<::B>; // { dg-error "20:'<::' cannot begin" "begin" { target c++98 } } +// { dg-message "20:is an alternate" "alt" { target c++98 } 66 } // On the first error message,
Re: [Patch, libfortran] Improve performance of byte swapped IO
Janne Blomqvist writes: > diff --git a/libgfortran/io/file_pos.c b/libgfortran/io/file_pos.c > index c8ecc3a..bf2250a 100644 > --- a/libgfortran/io/file_pos.c > +++ b/libgfortran/io/file_pos.c > @@ -140,15 +140,21 @@ unformatted_backspace (st_parameter_filepos *fpp, > gfc_unit *u) > } >else > { > + uint32_t u32; > + uint64_t u64; > switch (length) > { > case sizeof(GFC_INTEGER_4): > - reverse_memcpy (&m4, p, sizeof (m4)); > + memcpy (&u32, p, sizeof (u32)); > + u32 = __builtin_bswap32 (u32); > + m4 = *(GFC_INTEGER_4*)&u32; Isn't that an aliasing violation? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH] Adding target rdos to GCC
On Fri, Jan 4, 2013 at 2:22 PM, Leif Ekblad wrote: > Change log: > * config/gthr.m4: Added rdos thread header. > * gcc/config/i386/i386.c: Provided a way to define a default setting for > medium memory model and PIC. Provided a way to define a default value for > large-data-threshold. > * gcc/config/i386/i386.h: Defined default value for medium memory model & > PIC. > * gcc/config/i386/rdos.h: Added new file for rdos target definition. > * gcc/config.gcc: Added rdos target > > The change to gthr.m4 requires rebuilding the configure scripts. > > Tested with target rdos and rdos32. Is this ok for trunk? > > Regards, > Leif Ekblad + #ifdef TARGET_SECTION_THRESHOLD + ix86_section_threshold = TARGET_SECTION_THRESHOLD; + #endif You should 1. Add DEFAULT_SECTION_THRESHOLD and set it to 65536. 2. Change the init value of ix86_section_threshold to -1. 3. Set ix86_section_threshold to DEFAULT_SECTION_THRESHOLD if it is -1. -- H.J.
Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments
Looks good -- but better with followup : 1) give a warning when the parameter to the option is not allowed; 2) add test cases if possible. David On Fri, Jan 4, 2013 at 2:19 PM, Sriraman Tallam wrote: > Attached new patch. > > Thanks, > -Sri. > > On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu wrote: >> The code looks fine to me. Please consider David's comments about the >> option name. >> >> -Rong >> >> On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li wrote: >>> Is it better to change the option to something like: >>> >>> split_segment|nosplit-segment >>> or split_segment=yes|no >>> >>> >>> David >>> >>> On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam wrote: Hi Rong, The following patch modifies the behaviour of the linker plugin to not create a separate segment for cold sections by default. Separate segments can be created with the plugin option "segment=cold". Is this alright to commit? Thanks, -Sri. On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam wrote: > I have committed this patch. > > Thanks, > -Sri. > > On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu wrote: >> Looks good to me for google/gcc-4_7 branch. >> >> Thanks, >> >> -Rong >> >> >> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam >> wrote: >>> >>> Hi Rong, >>> >>> Please review this code. This code allows the function reordering >>> plugin to separate hot and cold code into different ELF segments. >>> This would allow optimizations like mapping the hot code alone to huge >>> pages. >>> >>> With this patch, by default, the plugin maps .text.unlikely >>> sections into a separate ELF segment. This can be turned off with >>> plugin option "--segment=none". >>> >>> The include/plugin-api.h changes are a backport from trunk. >>> >>> Thanks, >>> -Sri. >> >>
Re: [RFC PATCH, i386]: Use %r15 for REAL_PIC_OFFSET_TABLE_REGNUM on x86_64
OK. I know I can do that, but I would need to do it for every syscall since every syscall can potentially clobber rbx. Also, it is very strange that it is only one of the inlines the compiler complains about. I have another inline (which uses rbx as input), but that doesn't generate any error: #define RdosUserGateEbxEdiEcxParRetEax(nr, rbx, rdi, rcx, res) do { \ register int _id asm("r14") = nr; \ register typeof(rbx) _rbx asm("rbx") = (rbx); \ register typeof(rdi) _rdi asm("rdi") = (rdi); \ register typeof(rcx) _rcx asm("r8") = (rcx); \ register int _size asm("r12") = (rcx); \ asm volatile ( \ "syscall\n\t" \ "jnc 1f\n\t" \ "xorq %%rax,%%rax\n\t" \ "1: \n\t" \ : "=a" (res) : "r" (_id), "r" (_rbx), "r" (_rdi), "r" (_rcx), "r" (_size) : "rdx", "rsi" \ ); \ } while(0); inline int RdosWriteFile(int Handle, void *Buf, int Size) { int res; RdosUserGateEbxEdiEcxParRetEax(usergate_read_file, Handle, Buf, Size, res); return res; } Why is not this inline causing the problem? Might that be because it will not use rbx, but instead another register? OTOH, the code seems to work and rbx is not assigned to PIC at the point of the call, but to the defined parameter. Regards, Leif Ekblad - Original Message - From: "Jakub Jelinek" To: "Leif Ekblad" Cc: "Uros Bizjak" ; "Richard Henderson" ; "Andi Kleen" ; "Mike Frysinger" ; ; ; "H.J. Lu" Sent: Friday, January 04, 2013 10:42 PM Subject: Re: [RFC PATCH, i386]: Use %r15 for REAL_PIC_OFFSET_TABLE_REGNUM on x86_64 On Fri, Jan 04, 2013 at 10:39:05PM +0100, Leif Ekblad wrote: I just tried the patch, but it seems to produce some problems for me. The other patch which used a 64-bit specific register (r15) instead of rbx was easier to adapt to. The problem for me is that syscalls might clobber higher-half of all 32-bit registers, and that I cannot use the stack to preserve rbx in the call because of the red-zone. Of course you can use the stack, just subtract the red zone size (plus whatever you need) from %rsp first, then save to stack, do syscall, then restore from stack and finally increment %rsp back. Jakub
Re: [AARCH64] Optimize cmp in some cases
On Fri, Jan 4, 2013 at 1:34 AM, Richard Earnshaw wrote: > On 03/01/13 22:39, Andrew Pinski wrote: >> >> Hi, >>For aarch64, we don't CSE some cmp away. This patch fixes the case >> where we are CSE across some basic-blocks like: >> int f(int a, int b) >> { >>if(a> return 1; >>if(a>b) >> return -1; >>return 0; >> } >> --- CUT --- >> To fix this, I implemented the target hook >> TARGET_FIXED_CONDITION_CODE_REGS as there was already code in CSE >> which uses this target hook to find the extra setting of the CC >> registers. >> >> OK? Build and tested on aarch64-thunder-elf (using Cavium's internal >> simulator). Build a cross to aarch64-thunder-linux-gnu and a Canadian >> cross with that one for the native toolchain. >> >> Thanks, >> Andrew Pinski >> >> * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): >> New function. >> (TARGET_FIXED_CONDITION_CODE_REGS): Define. >> >> * gcc.target/aarch64/cmp-1.c: New testcase. >> >> >> fixed_condition_code.diff.txt >> >> >> >> * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): >> New function. >> (TARGET_FIXED_CONDITION_CODE_REGS): Define. >> >> * gcc.target/aarch64/cmp-1.c: New testcase. > > >> Index: config/aarch64/aarch64.c >> === >> --- config/aarch64/aarch64.c(revision 194872) >> +++ config/aarch64/aarch64.c(working copy) >> @@ -3041,6 +3041,16 @@ aarch64_const_double_zero_rtx_p (rtx x) >> return REAL_VALUES_EQUAL (r, dconst0); >> } >> >> +/* Return the fixed registers used for condition codes. */ >> + >> +static bool >> +aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2) >> +{ >> + *p1 = CC_REGNUM; >> + *p2 = -1; > > > Please use INVALID_REGNUM here (as the documentation states). The comment in target.def says: Up to two condition code registers are supported. If there is only one for this target, the int pointed at by the second argument should be set to -1. */ While tm.texi says: arguments point to the hard register numbers used for condition codes. When there is only one such register, as is true on most systems, the integer pointed to by @var{p2} should be set to @code{INVALID_REGNUM}. I had just read the comment in target.def when I was writing this patch which is why I had used -1. I agree INVALID_REGNUM is better. I will send out a patch to fix the comment in target.def later. > Otherwise, OK. > > A backport to the AArch64-4.7 branch would be appreciated. I don't have time to do a backport and to test it. Thanks, Andrew Pinski
[PATCH] Adding target rdos to GCC
Change log: * config/gthr.m4: Added rdos thread header. * gcc/config/i386/i386.c: Provided a way to define a default setting for medium memory model and PIC. Provided a way to define a default value for large-data-threshold. * gcc/config/i386/i386.h: Defined default value for medium memory model & PIC. * gcc/config/i386/rdos.h: Added new file for rdos target definition. * gcc/config.gcc: Added rdos target The change to gthr.m4 requires rebuilding the configure scripts. Tested with target rdos and rdos32. Is this ok for trunk? Regards, Leif Ekblad gcc.diff Description: Binary data
Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments
Attached new patch. Thanks, -Sri. On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu wrote: > The code looks fine to me. Please consider David's comments about the > option name. > > -Rong > > On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li wrote: >> Is it better to change the option to something like: >> >> split_segment|nosplit-segment >> or split_segment=yes|no >> >> >> David >> >> On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam wrote: >>> Hi Rong, >>> >>> The following patch modifies the behaviour of the linker plugin to >>> not create a separate segment for cold sections by default. Separate >>> segments can be created with the plugin option "segment=cold". Is this >>> alright to commit? >>> >>> Thanks, >>> -Sri. >>> >>> On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam >>> wrote: I have committed this patch. Thanks, -Sri. On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu wrote: > Looks good to me for google/gcc-4_7 branch. > > Thanks, > > -Rong > > > On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam > wrote: >> >> Hi Rong, >> >> Please review this code. This code allows the function reordering >> plugin to separate hot and cold code into different ELF segments. >> This would allow optimizations like mapping the hot code alone to huge >> pages. >> >> With this patch, by default, the plugin maps .text.unlikely >> sections into a separate ELF segment. This can be turned off with >> plugin option "--segment=none". >> >> The include/plugin-api.h changes are a backport from trunk. >> >> Thanks, >> -Sri. > > Index: function_reordering_plugin/function_reordering_plugin.c === --- function_reordering_plugin/function_reordering_plugin.c (revision 194878) +++ function_reordering_plugin/function_reordering_plugin.c (working copy) @@ -33,9 +33,13 @@ along with this program; see the file COPYING3. I This plugin dumps the final layout order of the functions in a file called "final_layout.txt". To change the output file, pass the new - file name with --plugin-opt. To dump to stderr instead, just pass - stderr to --plugin-opt. */ + file name with --plugin-opt,file=. To dump to stderr instead, + just pass stderr as the file name. + This plugin also allows placing all functions found cold in a separate + segment. This can be enabled with the linker option: + --plugin-opt,split_segment=yes. */ + #if HAVE_STDINT_H #include #endif @@ -89,9 +93,10 @@ static int is_api_exist = 0; /* The plugin does nothing when no-op is 1. */ static int no_op = 0; -/* The plugin does not create a new segment for unlikely code if - no_segment is set. */ -static int no_segment = 0; +/* The plugin creates a new segment for unlikely code if split_segment + is set. This can be set with the linker option: + "--plugin-opt,split_segment=yes". */ +static int split_segment = 0; /* Copies new output file name out_file */ void get_filename (const char *name) @@ -110,7 +115,7 @@ process_option (const char *name) { const char *option_group = "group="; const char *option_file = "file="; - const char *option_segment = "segment="; + const char *option_segment = "split_segment="; /* Check if option is "group=" */ if (strncmp (name, option_group, strlen (option_group)) == 0) @@ -129,13 +134,14 @@ process_option (const char *name) return; } - /* Check if options is "segment=none" */ + /* Check if options is "split_segment=[yes|no]" */ if (strncmp (name, option_segment, strlen (option_segment)) == 0) { - if (strcmp (name + strlen (option_segment), "none") == 0) - no_segment = 1; - else - no_segment = 0; + const char *option_val = name + strlen (option_segment); + if (strcmp (option_val, "no") == 0) + split_segment = 0; + else if (strcmp (option_val, "yes") == 0) + split_segment = 1; return; } @@ -244,7 +250,10 @@ claim_file_hook (const struct ld_plugin_input_file { /* Inform the linker to prepare for section reordering. */ (*allow_section_ordering) (); - (*allow_unique_segment_for_sections) (); + /* Inform the linker to allow certain sections to be placed in +a separate segment. */ + if (split_segment == 1) +(*allow_unique_segment_for_sections) (); is_ordering_specified = 1; } @@ -335,15 +344,29 @@ all_symbols_read_hook (void) if (out_file != NULL && strcmp (out_file, "stderr") != 0) fclose (fp); - /* Pass the new order of functions to the linker. */ - update_section_order (section_list, unlikely_segment_start); - assert (num_entries >= unlikely_segment_end); - update_section_order (section_list, num_entries - unlikely_segment_end); - /* Map all unlikely code into a new segment. */ - if (no_segment == 0
[Patch, libfortran] Improve performance of byte swapped IO
Hi, currently byte swapped unformatted IO can be quite slow compared to the same code with no byte swapping. There are two major reasons for this: 1) The byte swapping code path resorts to transferring data element by element, leading to a lot of overhead in the IO library. 2) The function used for the actual byte swapping, reverse_memcpy , while able to handle general element sizes, is not particularly fast, especially considering that many CPU's have fast byte swapping instructions (e.g. BSWAP on x86). In order to access these fast byte swapping instructions, gcc provides the __builtin_bswap{16,32,64} builtins, falling back to libgcc code for targets that lack support. The attached patch fixes these issues. For issue (1), the read path uses in-place byte swapping of the data that has been read into the user buffer, while the write path uses a larger temporary buffer (since we are not allowed to modify the user supplied data in this case). For issue(2), the patch uses __builtin_bswap{16,32,64} where appropriate, only falling back to reverse_memcpy for other sizes. With the attached test program run on a tmpfs filesystem to avoid doing actual disk IO, I get the following: - With no byte swapping: Unformatted sequential write/read performance test Record size Write MB/s Read MB/s == 4 52.72384281742220272.721158943820441 8 77.50829689085638697.237815640377221 16 110.26209495334321143.80831184546381 32 173.94872143231535221.89704881197937 64 282.19818562682684373.77854583735541 128 442.22084579742244628.80041029142183 256 636.69620860705299966.37723642576316 512 826.059688407380801380.8835166612221 1024 987.186864651975611763.5990036057208 2048 1047.67215441917102058.0875622043550 4096 1115.58171471348012251.8731832850176 8192 1191.50211509965902283.8893409728184 16384 1417.61109095193912441.0530373866482 32768 1570.44134790460182543.0836384048471 65536 1673.03787065029662651.2182395008308 131072 1697.49442461884452688.2398923155783 262144 1669.63298621458722735.668973292 524288 1594.46699352315522697.7208298823243 - Before patch, with byte swapping: Unformatted sequential write/read performance test Record size Write MB/s Read MB/s == 4 50.57281289368979368.858701306591627 8 58.68851330069031781.591733130441327 16 73.55118848060782096.638995590227665 32 91.593767813989018116.65817140076214 64 107.41379323761915128.32512066346368 128 121.33499652432221147.80777892360237 256 128.99627771476628155.91619889220266 512 135.02742063670030161.30042382365372 1024 137.02276709585524164.11267056940963 2048 138.62774254302394165.22456826188971 4096 139.27695763341924166.34707691429571 8192 147.64584950575932166.59526981475742 16384 147.91235479266419166.77890398940283 32768 150.77029430529927166.90834867503827 65536 151.59474472614465166.84075600288520 131072 155.75202672623249166.96550283835097 262144 155.36506626794849166.78075976148853 524288 155.64305086921487167.44468828946083 - After patch, with byte swapping: Unformatted sequential write/read performance test Record size Write MB/s Read MB/s == 4 49.41477177682136170.808060042286343 8 72.91815640245977293.234093684373946 16 102.72461544178078136.21700026949074 32 160.57240200649090205.97612602315186 64 249.32082957447636331.85515010907363 128 385.71299236810387522.06354804855266 256 535.40608912076459766.59668706247294 512 669.478641203685241006.4275938227961 1024 742.905388955002651187.9846039167674 2048 789.713405573405231333.8411634622269 4096 826.442532047316831395.5536995933605 8192 832.935403161166621361.4621716558986 16384 897.950819770101131469.0940087507722 32768 961.187363080333171533.7736812111871 65536 989.413849084968321564.7013916917260 131072 1003.61137620680401597.4063253370084 262144 980.030676643243961602.3188995993287
Re: [RFC PATCH, i386]: Use %r15 for REAL_PIC_OFFSET_TABLE_REGNUM on x86_64
On Fri, Jan 04, 2013 at 10:39:05PM +0100, Leif Ekblad wrote: > I just tried the patch, but it seems to produce some problems for > me. The other patch which used a 64-bit specific register (r15) > instead of rbx was easier to adapt to. The problem for me is that > syscalls might clobber higher-half of all 32-bit registers, and that > I cannot use the stack to preserve rbx in the call because of the > red-zone. Of course you can use the stack, just subtract the red zone size (plus whatever you need) from %rsp first, then save to stack, do syscall, then restore from stack and finally increment %rsp back. Jakub
Re: [RFC PATCH, i386]: Use %r15 for REAL_PIC_OFFSET_TABLE_REGNUM on x86_64
I just tried the patch, but it seems to produce some problems for me. The other patch which used a 64-bit specific register (r15) instead of rbx was easier to adapt to. The problem for me is that syscalls might clobber higher-half of all 32-bit registers, and that I cannot use the stack to preserve rbx in the call because of the red-zone. Problem code: #define RdosUserGateEdiEcxPar0RetEbx(nr, rdi, rcx, size, res) do { \ register int _id asm("r14") = nr; \ register typeof(rdi) _rdi asm("rdi") = (rdi); \ register typeof(rcx) _rcx asm("r8") = (rcx); \ register int _size asm("r12") = (size); \ asm volatile ( \ "syscall\n\t" \ "jc 1f\n\t" \ "movzx %%bx,%%rax\n\t" \ "jmp 2f\n\t" \ "1: \n\t" \ "xorq %%rax,%%rax\n\t" \ "2: \n\t" \ : "=a" (res) : "r" (_id), "r" (_rdi), "r" (_rcx), "r" (_size) : "rbx", "rdx", "rsi" \ ); \ } while(0); inline volatile int RdosCreateFile(const char *FileName, int Attrib) { int res; int size = strlen(FileName) + 1; RdosUserGateEdiEcxPar0RetEbx(usergate_create_file, FileName, Attrib, size, res); return res; } Error log: $ rdos-gcc test.c -o test.exe In file included from /usr/local/rdos/include/rdos.h:719:0, from test.c:1: /usr/local/rdos/include/rdosgcc.h: In function 'main': /usr/local/rdos/include/rdosgcc.h:236:5: error: PIC register clobbered by 'rbx' in 'asm' RdosUserGateEdiEcxPar0RetEbx(usergate_open_file, FileName, Access, size, res); ^ I would prefer to change PIC register to r15 instead, or alternatively make this an target-option. Regards, Leif Ekblad - Original Message - From: "Uros Bizjak" To: "Richard Henderson" Cc: "Andi Kleen" ; "Mike Frysinger" ; ; "Leif Ekblad" ; "Jakub Jelinek" ; ; "H.J. Lu" Sent: Sunday, December 30, 2012 8:08 PM Subject: Re: [RFC PATCH, i386]: Use %r15 for REAL_PIC_OFFSET_TABLE_REGNUM on x86_64 On Fri, Dec 28, 2012 at 9:27 PM, Richard Henderson wrote: On 12/27/2012 12:08 AM, Uros Bizjak wrote: The alternative approach is changing cpuid definition in cpuid.h (as in attached patch) to preserve %rbx, but we can't detect various code model settings there. Since the change depends on the definition of __PIC__, we unnecessary preserve %rbx also for -mcmodel=small. Certainly we can. We also control the preprocessor defines. All that's needed is that we create one for the code model. Something like attached? I have also included all suggestions (earlyclobber and operand prefix on temporary register). 2012-12-30 Uros Bizjak PR target/55712 * config/i386/i386-c.c (ix86_target_macros_internal): Depending on selected code model, define __code_mode_small__, __code_model_medium__, __code_model_large__, __code_model_32__ or __code_model_kernel__. * config/i386/cpuid.h (__cpuid, __cpuid_count) [__i386__]: Prefix xchg temporary register with %k. Declare temporary register as early clobbered. [__x86_64__]: For medium and large code models, preserve %rbx register. Tested on x86_64-pc-linux-gnu {,-m32}. Uros.
Re: [Patch, Fortran] FINAL (prep patches 3/5): Auxiliary functions for calling the FINAL wrapper
Dear Tobias, s/Argument is neither a pointer/allocatble/Argument is neither a pointer/allocatable/ Why have you all the asserts for se.pre and se.post? Did you have trouble with this or is there some hidden nasty that I am unaware of? Should you not either return rapidly for an intrinsic type or add a gcc_assert to pick up accidental calls? Modulo these points this patch is OK for trunk. Cheers Paul On 31 December 2012 13:21, Tobias Burnus wrote: > This patch adds one auxiliary functions, which will be used when invoking > the finalization wrapper. It is currently unused. > > Build on x86-64-gnu-linux. > OK for the trunk? > > Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] FINAL (prep patches 2/5): Add internal STRIDE intrinsic
Dear Tobias, This one is also OK for trunk. It does cross my mind, however, that we should offer STRIDE as a gcc extension, in anticipation of F201x. Cheers Paul On 31 December 2012 13:17, Tobias Burnus wrote: > The attached patch adds a new - internal only - intrinsic, STRIDE, which > returns the stride of an array with array descriptor; for an "integer :: > array(5,5)", the stride of dim=2 is 5. > > This new intrinsic is only internally available and will be used by the > finalization wrapper to handle noncontiguous arrays. > > Build on x86-64-gnu-linux. > OK for the trunk? > > Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] FINAL (prep patches 1/5): Add _final to intrinsic vtables for CLASS(*)
Dear Tobias, This one is 'obvious' - OK for trunk. Thanks Paul On 31 December 2012 13:16, Tobias Burnus wrote: > This simple patch fixes a wrong indent and adds a _final component to the > virtual tables generated for intrinsic types. > > This patch not only prepares the trunk for finalization support, it also > avoids ABI issues which a later addition would cause. (For intrinsic types, > changing the .mod version number will not reliable force a recompilation.) > > Build on x86-64-gnu-linux. > OK for the trunk? > > Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear Tobias, Committed as revision 194916. I am leaving the PR open to deal with 4.7 just as soon as I have downloaded it! Thanks again for the careful review. Cheers Paul On 2 January 2013 23:48, Tobias Burnus wrote: > Dear Paul, > > First, the new patch is fine from my side. (Although, I think the test case > should also include the vector-section example.) Thanks for working on that > regression. > > > > Paul Richard Thomas wrote: >> >> First of all, thanks for the review! I still owe you my comments on >> FINAL; I got lost in trying to fix these various regressions :-) I >> promise that I'll come back to you first thing tomorrow. > > > I am looking forward to them - and in particular to a patch review of the > FINAL patches. I am also interested in your comment to my LOGICAL in BIND(C) > procedures patch, namely http://gcc.gnu.org/ml/fortran/2012-12/msg00200.html > > >>> It looks mostly okay; however, you do not handle vector sections >>> correctly, >>> which leads to an ICE. Without your patch, one gets: >>> Error: CLASS selector at (1) needs a temporary which is not yet >>> implemented >>> >>> With your patch, it fails as one has: >> >> This was fairly easily fixed - see attached. > > > Thanks. By the way, I have filled a PR to track this "not yet implemented" > issue: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55849 > > >>> I am not quite sure whether the following ICE has the same cause or a >>> different one, but it also ICEs with your patch applied: >>> select type (component => self%cb[4]) >> >> This co-array example was never OK, as far as I can tell. The error >> >> is similar to that of the PR. However, co-arrays were just never >> handled at all again, as far as I can tell. I'll have a go at it >> tomorrow night. > > > I recall that we did add some coarray support for polymorphic variables. At > least with coarray arrays (contrary to coarray scalars) it seems to compile. > But it is very likely that select type never worked with coarrays or coarray > scalars. > > Note that the coindexd > > > select type (component => self%cb[4]) > is invalid (C803; PR55850 (a)). However, the same failure occurs for > noncoindexed valid selector: > select type (component => self%cb) > > > (See also PR 55850 for some other SELECT TYPE issues I found.) > > Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
[PATCH] Remove unnecessaily included limits.h in libgcc2.c
I filed http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55743 (my first upstream gcc bug so be gentle :-) Details are there but the short version is that the limits.h inclusion in libgcc2.c is now a relic because the constants that it brings in are no longer used (since http://repo.or.cz/w/official-gcc.git/blobdiff/49f0f270673c4512c11f72a038b84c321ae5534a..7429c938827aa98bf3b02c4ac89510f4d28ef0b1:/gcc/libgcc2.c ) And this inclusion can break --without-headers bootstrapping (which is how I noticed it). Doko poked me to send the patch to this list for consideration for inclusion in trunk. --- gcc-4.7-4.7.2/src/libgcc/libgcc2.c.orig 2011-11-02 15:26:35.0 + +++ gcc-4.7-4.7.2/src/libgcc/libgcc2.c 2012-12-18 19:33:40.0 + @@ -1676,18 +1676,6 @@ #endif #if defined(L_fixunsxfsi) && LIBGCC2_HAS_XF_MODE -/* Reenable the normal types, in case limits.h needs them. */ -#undef char -#undef short -#undef int -#undef long -#undef unsigned -#undef float -#undef double -#undef MIN -#undef MAX -#include - UWtype __fixunsxfSI (XFtype a) { @@ -1698,18 +1686,6 @@ #endif #if defined(L_fixunsdfsi) && LIBGCC2_HAS_DF_MODE -/* Reenable the normal types, in case limits.h needs them. */ -#undef char -#undef short -#undef int -#undef long -#undef unsigned -#undef float -#undef double -#undef MIN -#undef MAX -#include - UWtype __fixunsdfSI (DFtype a) { @@ -1720,18 +1696,6 @@ #endif #if defined(L_fixunssfsi) && LIBGCC2_HAS_SF_MODE -/* Reenable the normal types, in case limits.h needs them. */ -#undef char -#undef short -#undef int -#undef long -#undef unsigned -#undef float -#undef double -#undef MIN -#undef MAX -#include - UWtype __fixunssfSI (SFtype a) { Wookey -- Principal hats: Linaro, Emdebian, Wookware, Balloonboard, ARM http://wookware.org/
libiberty patch committed: Minor optimization (PR 54800)
PR 54800 points out a minor optimization uncovered by cppcheck. This optimization is not important, but we might as well fix it in case cppcheck comes up with something useful. Tested by Iain Sandoe (thanks!). Committed to mainline. Ian 2013-01-04 Ian Lance Taylor PR other/54800 * simple-object-mach-o.c (simple_object_mach_o_segment): Don't bother to zero out a buffer we are about to set anyhow. Index: simple-object-mach-o.c === --- simple-object-mach-o.c (revision 194911) +++ simple-object-mach-o.c (working copy) @@ -1,5 +1,5 @@ /* simple-object-mach-o.c -- routines to manipulate Mach-O object files. - Copyright 2010, 2011 Free Software Foundation, Inc. + Copyright 2010, 2011, 2013 Free Software Foundation, Inc. Written by Ian Lance Taylor, Google. This program is free software; you can redistribute it and/or modify it @@ -701,12 +701,13 @@ simple_object_mach_o_segment (simple_obj /* Otherwise, make a name like __segment,__section as per the convention in mach-o asm. */ name = &namebuf[0]; - memset (namebuf, 0, MACH_O_NAME_LEN * 2 + 2); memcpy (namebuf, (char *) sechdr + segname_offset, MACH_O_NAME_LEN); + namebuf[MACH_O_NAME_LEN] = '\0'; l = strlen (namebuf); namebuf[l] = ','; memcpy (namebuf + l + 1, (char *) sechdr + sectname_offset, MACH_O_NAME_LEN); + namebuf[l + 1 + MACH_O_NAME_LEN] = '\0'; } simple_object_mach_o_section_info (omr->is_big_endian, is_32, sechdr,
Re: RFA: Fix ICE on PARALLEL returns when expand builtins
> Hmm, how would anyone else get at the "padding" info dealing with the > parallel? This looks broken if we need access to the type :/ Yes, you need to access the type for return values, but that's not really new. > Eric, any comments? Richard's patch seems fine to me, modulo the name of the routine; I'd rather use maybe_emit_group_store or something along these lines. -- Eric Botcazou
[PATCH, i386]: Call convert_to_mode unconditionally.
Hello! As suggested by rth at [1]. 2013-01-04 Uros Bizjak * config/i386/i386.c (ix86_legitimize_address): Call convert_to_mode unconditionally. (ix86_expand_move): Ditto. (ix86_zero_extend_to_Pmode): Ditto. (ix86_expand_call): Ditto. (ix86_expand_special_args_builtin): Ditto. (ix86_expand_builtin): Ditto. Bootstrapped, regression tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN. [1] http://gcc.gnu.org/ml/gcc-patches/2013-01/msg00145.html Uros. Index: config/i386/i386.c === --- config/i386/i386.c (revision 194883) +++ config/i386/i386.c (working copy) @@ -13247,8 +13247,7 @@ ix86_legitimize_address (rtx x, rtx oldx ATTRIBUTE rtx val = force_operand (XEXP (x, 1), temp); if (val != temp) { - if (GET_MODE (val) != Pmode) - val = convert_to_mode (Pmode, val, 1); + val = convert_to_mode (Pmode, val, 1); emit_move_insn (temp, val); } @@ -13262,8 +13261,7 @@ ix86_legitimize_address (rtx x, rtx oldx ATTRIBUTE rtx val = force_operand (XEXP (x, 0), temp); if (val != temp) { - if (GET_MODE (val) != Pmode) - val = convert_to_mode (Pmode, val, 1); + val = convert_to_mode (Pmode, val, 1); emit_move_insn (temp, val); } @@ -15931,8 +15929,7 @@ ix86_expand_move (enum machine_mode mode, rtx oper op1 = force_operand (op1, op0); if (op1 == op0) return; - if (GET_MODE (op1) != mode) - op1 = convert_to_mode (mode, op1, 1); + op1 = convert_to_mode (mode, op1, 1); } else if (TARGET_DLLIMPORT_DECL_ATTRIBUTES && SYMBOL_REF_DLLIMPORT_P (op1)) @@ -16013,8 +16010,7 @@ ix86_expand_move (enum machine_mode mode, rtx oper op1 = legitimize_pic_address (op1, reg); if (op0 == op1) return; - if (GET_MODE (op1) != mode) - op1 = convert_to_mode (mode, op1, 1); + op1 = convert_to_mode (mode, op1, 1); } } } @@ -21650,9 +21646,7 @@ ix86_adjust_counter (rtx countreg, HOST_WIDE_INT v rtx ix86_zero_extend_to_Pmode (rtx exp) { - if (GET_MODE (exp) != Pmode) -exp = convert_to_mode (Pmode, exp, 1); - return force_reg (Pmode, exp); + return force_reg (Pmode, convert_to_mode (Pmode, exp, 1)); } /* Divide COUNTREG by SCALE. */ @@ -23624,9 +23618,7 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx call ? !sibcall_insn_operand (XEXP (fnaddr, 0), word_mode) : !call_insn_operand (XEXP (fnaddr, 0), word_mode)) { - fnaddr = XEXP (fnaddr, 0); - if (GET_MODE (fnaddr) != word_mode) - fnaddr = convert_to_mode (word_mode, fnaddr, 1); + fnaddr = convert_to_mode (word_mode, XEXP (fnaddr, 0), 1); fnaddr = gen_rtx_MEM (QImode, copy_to_mode_reg (word_mode, fnaddr)); } @@ -31276,9 +31268,8 @@ ix86_expand_special_args_builtin (const struct bui gcc_assert (target == 0); if (memory) { - if (GET_MODE (op) != Pmode) - op = convert_to_mode (Pmode, op, 1); - target = gen_rtx_MEM (tmode, force_reg (Pmode, op)); + op = force_reg (Pmode, convert_to_mode (Pmode, op, 1)); + target = gen_rtx_MEM (tmode, op); } else target = force_reg (tmode, op); @@ -31322,9 +31313,8 @@ ix86_expand_special_args_builtin (const struct bui if (i == memory) { /* This must be the memory operand. */ - if (GET_MODE (op) != Pmode) - op = convert_to_mode (Pmode, op, 1); - op = gen_rtx_MEM (mode, force_reg (Pmode, op)); + op = force_reg (Pmode, convert_to_mode (Pmode, op, 1)); + op = gen_rtx_MEM (mode, op); gcc_assert (GET_MODE (op) == mode || GET_MODE (op) == VOIDmode); } @@ -31572,9 +31562,8 @@ ix86_expand_builtin (tree exp, rtx target, rtx sub mode1 = insn_data[icode].operand[1].mode; mode2 = insn_data[icode].operand[2].mode; - if (GET_MODE (op0) != Pmode) - op0 = convert_to_mode (Pmode, op0, 1); - op0 = gen_rtx_MEM (mode1, force_reg (Pmode, op0)); + op0 = force_reg (Pmode, convert_to_mode (Pmode, op0, 1)); + op0 = gen_rtx_MEM (mode1, op0); if (!insn_data[icode].operand[0].predicate (op0, mode0)) op0 = copy_to_mode_reg (mode0, op0); @@ -31605,11 +31594,7 @@ ix86_expand_builtin (tree exp, rtx target, rtx sub op0 = expand_normal (arg0); icode = CODE_FOR_sse2_clflush; if (!insn_data[icode].operand[0].predicate (op0, Pmode)) - { - if (GET_MODE (op0) != Pmode) - op0 = convert_to_mode (Pmode, op0, 1); - op0 = force_reg (Pmode, op0); - } +
Merge from trunk to gccgo branch
I've merged trunk revision 194911 to the gccgo branch. Ian
Re: [Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear Tobias, Thanks for the review. The patched trunk is just now bootstrapping and regtesting prior to commitment. I have added a check for co-indexed selectors in resolve.c and have added tests in select_type_31.f03 for co-indexing and vector indices. I am now turning my attention to FINAL :-) Cheers Paul PS BTW co-arrays fail similarly as selectors in SELECT_TYPE and in ASSOCIATE. Do I assume correctly that the associate variable should be a co-array? On 2 January 2013 23:48, Tobias Burnus wrote: > Dear Paul, > > First, the new patch is fine from my side. (Although, I think the test case > should also include the vector-section example.) Thanks for working on that > regression. > > > > Paul Richard Thomas wrote: >> >> First of all, thanks for the review! I still owe you my comments on >> FINAL; I got lost in trying to fix these various regressions :-) I >> promise that I'll come back to you first thing tomorrow. > > > I am looking forward to them - and in particular to a patch review of the > FINAL patches. I am also interested in your comment to my LOGICAL in BIND(C) > procedures patch, namely http://gcc.gnu.org/ml/fortran/2012-12/msg00200.html > > >>> It looks mostly okay; however, you do not handle vector sections >>> correctly, >>> which leads to an ICE. Without your patch, one gets: >>> Error: CLASS selector at (1) needs a temporary which is not yet >>> implemented >>> >>> With your patch, it fails as one has: >> >> This was fairly easily fixed - see attached. > > > Thanks. By the way, I have filled a PR to track this "not yet implemented" > issue: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55849 > > >>> I am not quite sure whether the following ICE has the same cause or a >>> different one, but it also ICEs with your patch applied: >>> select type (component => self%cb[4]) >> >> This co-array example was never OK, as far as I can tell. The error >> >> is similar to that of the PR. However, co-arrays were just never >> handled at all again, as far as I can tell. I'll have a go at it >> tomorrow night. > > > I recall that we did add some coarray support for polymorphic variables. At > least with coarray arrays (contrary to coarray scalars) it seems to compile. > But it is very likely that select type never worked with coarrays or coarray > scalars. > > Note that the coindexd > > > select type (component => self%cb[4]) > is invalid (C803; PR55850 (a)). However, the same failure occurs for > noncoindexed valid selector: > select type (component => self%cb) > > > (See also PR 55850 for some other SELECT TYPE issues I found.) > > Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [google 4_7] Backport r194909 (<:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
ok. thanks, David On Fri, Jan 4, 2013 at 9:32 AM, Paul Pluzhnikov wrote: > Back-port revision 194909 to google/gcc-4_7 branch: > http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194909 > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54526 > > Google ref: b/7427993 > > Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C > === > --- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194909) > +++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy) > @@ -31,5 +31,5 @@ > }; > void foo::x() throw(bar) > { > - if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda > expressions|expected" } parse error > + if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda > expressions|expected|invalid" } parse error > } > Index: libcpp/lex.c > === > --- libcpp/lex.c(revision 194909) > +++ libcpp/lex.c(working copy) > @@ -2224,6 +2224,17 @@ > { > if (*buffer->cur == ':') > { > + /* C++11 [2.5/3 lex.pptoken], "Otherwise, if the next > +three characters are <:: and the subsequent character > +is neither : nor >, the < is treated as a preprocessor > +token by itself". */ > + if (CPP_OPTION (pfile, cplusplus) > + && (CPP_OPTION (pfile, lang) == CLK_CXX11 > + || CPP_OPTION (pfile, lang) == CLK_GNUCXX11) > + && buffer->cur[1] == ':' > + && buffer->cur[2] != ':' && buffer->cur[2] != '>') > + break; > + > buffer->cur++; > result->flags |= DIGRAPH; > result->type = CPP_OPEN_SQUARE; > > -- > This patch is available for review at http://codereview.appspot.com/7028052
[google 4_7] Backport r194909 (<:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
Back-port revision 194909 to google/gcc-4_7 branch: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194909 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54526 Google ref: b/7427993 Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C === --- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194909) +++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy) @@ -31,5 +31,5 @@ }; void foo::x() throw(bar) { - if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda expressions|expected" } parse error + if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda expressions|expected|invalid" } parse error } Index: libcpp/lex.c === --- libcpp/lex.c(revision 194909) +++ libcpp/lex.c(working copy) @@ -2224,6 +2224,17 @@ { if (*buffer->cur == ':') { + /* C++11 [2.5/3 lex.pptoken], "Otherwise, if the next +three characters are <:: and the subsequent character +is neither : nor >, the < is treated as a preprocessor +token by itself". */ + if (CPP_OPTION (pfile, cplusplus) + && (CPP_OPTION (pfile, lang) == CLK_CXX11 + || CPP_OPTION (pfile, lang) == CLK_GNUCXX11) + && buffer->cur[1] == ':' + && buffer->cur[2] != ':' && buffer->cur[2] != '>') + break; + buffer->cur++; result->flags |= DIGRAPH; result->type = CPP_OPEN_SQUARE; -- This patch is available for review at http://codereview.appspot.com/7028052
Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments
The code looks fine to me. Please consider David's comments about the option name. -Rong On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li wrote: > Is it better to change the option to something like: > > split_segment|nosplit-segment > or split_segment=yes|no > > > David > > On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam wrote: >> Hi Rong, >> >> The following patch modifies the behaviour of the linker plugin to >> not create a separate segment for cold sections by default. Separate >> segments can be created with the plugin option "segment=cold". Is this >> alright to commit? >> >> Thanks, >> -Sri. >> >> On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam >> wrote: >>> I have committed this patch. >>> >>> Thanks, >>> -Sri. >>> >>> On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu wrote: Looks good to me for google/gcc-4_7 branch. Thanks, -Rong On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam wrote: > > Hi Rong, > > Please review this code. This code allows the function reordering > plugin to separate hot and cold code into different ELF segments. > This would allow optimizations like mapping the hot code alone to huge > pages. > > With this patch, by default, the plugin maps .text.unlikely > sections into a separate ELF segment. This can be turned off with > plugin option "--segment=none". > > The include/plugin-api.h changes are a backport from trunk. > > Thanks, > -Sri.
Re: [committed] 2011 and 2012 Copyright year updates
On Fri, Jan 04, 2013 at 08:44:13AM -0800, Andrew Pinski wrote: > On Fri, Jan 4, 2013 at 4:54 AM, Jakub Jelinek wrote: > > I've run a script to notice gcc maintained files with FSF copyright that > > have been modified in 2011 and/or 2012 (according to svn log, ignoring > > r168438 and r184997 commits), but didn't have years 2011 and/or 2012 > > included in Copyright lines. I've kept the preexisting style, so > > where year ranges were used, updated those if needed, if not, kept the > > year lists. > > Can't we just move to ranges of years now that the FSF approves of > them. They even approve of ranges where a file was not touched during > that year. This seems better than listing all the years out. If somebody is willing to do the conversion, sure, but even with some scripting that is going to be lots of work. Even this patch took more than 6 hours of svn log, some scripting and a few hours of manual work, while the conversion would take IMHO more than that. Jakub
C++ PATCH for c++/55877 (wrong linkage for nested classes)
When an anonymous class gets a linkage name from a typedef, we recalculate its visibility; anonymous types have internal linkage, but types with linkage names have external linkage and therefore can have visibility. But we were forgetting to also recalculate the visibility of any nested types, which were also affected by the former anonymity of the enclosing class. Tested x86_64-pc-linux-gnu, applying to trunk, 4.7, 4.6. commit 28beb0d493a46f126f23d15216d4ae9279223514 Author: Jason Merrill Date: Fri Jan 4 10:50:46 2013 -0500 PR c++/55877 * decl.c (reset_type_linkage, bt_reset_linkage): New. (grokdeclarator): Use reset_type_linkage. * name-lookup.c (binding_table_foreach): Handle null table. * tree.c (decl_anon_ns_mem_p): Check TYPE_MAIN_DECL, not TYPE_NAME. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 5c268b1..9640824 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -8513,6 +8513,23 @@ check_var_type (tree identifier, tree type) return type; } +/* Functions for adjusting the visibility of a tagged type and its nested + types when it gets a name for linkage purposes from a typedef. */ + +static void bt_reset_linkage (binding_entry, void *); +static void +reset_type_linkage (tree type) +{ + set_linkage_according_to_type (type, TYPE_MAIN_DECL (type)); + if (CLASS_TYPE_P (type)) +binding_table_foreach (CLASSTYPE_NESTED_UTDS (type), bt_reset_linkage, NULL); +} +static void +bt_reset_linkage (binding_entry b, void */*data*/) +{ + reset_type_linkage (b->type); +} + /* Given declspecs and a declarator (abstract or otherwise), determine the name and type of the object declared and construct a DECL node for it. @@ -10053,8 +10070,7 @@ grokdeclarator (const cp_declarator *declarator, = TYPE_IDENTIFIER (type); /* Adjust linkage now that we aren't anonymous anymore. */ - set_linkage_according_to_type (type, TYPE_MAIN_DECL (type)); - determine_visibility (TYPE_MAIN_DECL (type)); + reset_type_linkage (type); /* FIXME remangle member functions; member functions of a type with external linkage have external linkage. */ diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 87b1f51..754e830 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -251,9 +251,13 @@ binding_table_find (binding_table table, tree name) void binding_table_foreach (binding_table table, bt_foreach_proc proc, void *data) { - const size_t chain_count = table->chain_count; + size_t chain_count; size_t i; + if (!table) +return; + + chain_count = table->chain_count; for (i = 0; i < chain_count; ++i) { binding_entry entry = table->chain[i]; diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index c658582..fcab1a4 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -2404,7 +2404,7 @@ decl_anon_ns_mem_p (const_tree decl) /* Classes and namespaces inside anonymous namespaces have TREE_PUBLIC == 0, so we can shortcut the search. */ else if (TYPE_P (decl)) - return (TREE_PUBLIC (TYPE_NAME (decl)) == 0); + return (TREE_PUBLIC (TYPE_MAIN_DECL (decl)) == 0); else if (TREE_CODE (decl) == NAMESPACE_DECL) return (TREE_PUBLIC (decl) == 0); else diff --git a/gcc/testsuite/g++.dg/ext/visibility/anon11.C b/gcc/testsuite/g++.dg/ext/visibility/anon11.C new file mode 100644 index 000..dfb4f12 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/visibility/anon11.C @@ -0,0 +1,13 @@ +// PR c++/55877 +// { dg-final { scan-assembler-not "\\.local" } } + +typedef struct { + typedef enum { X, Y } A; + typedef struct { } B; + struct C { }; +} D; + +D d; +D::A a; +D::B b; +D::C c;
Re: [committed] 2011 and 2012 Copyright year updates
On Fri, Jan 4, 2013 at 4:54 AM, Jakub Jelinek wrote: > Hi! > > I've run a script to notice gcc maintained files with FSF copyright that > have been modified in 2011 and/or 2012 (according to svn log, ignoring > r168438 and r184997 commits), but didn't have years 2011 and/or 2012 > included in Copyright lines. I've kept the preexisting style, so > where year ranges were used, updated those if needed, if not, kept the > year lists. Can't we just move to ranges of years now that the FSF approves of them. They even approve of ranges where a file was not touched during that year. This seems better than listing all the years out. Thanks, Andrew
Re: [C++ Patch] PR 54526 (again)
OK. Jason
Re: Use libstdc++-raw-cxx.m4 in libjava
On Fri, Jan 4, 2013 at 2:06 AM, Andreas Schwab wrote: > "H.J. Lu" writes: > >> On Thu, Jan 3, 2013 at 10:09 AM, Andreas Schwab >> wrote: >>> "H.J. Lu" writes: >>> diff --git a/libjava/Makefile.am b/libjava/Makefile.am index c6c84e4..dd08a4f 100644 --- a/libjava/Makefile.am +++ b/libjava/Makefile.am @@ -594,7 +594,7 @@ lib_gnu_awt_xlib_la_CPPFLAGS = \ $(AM_CPPFLAGS) \ $(LIBSTDCXX_RAW_CXX_CXXFLAGS) ## The mysterious backslash in the grep pattern is consumed by make. -lib_gnu_awt_xlib_la_LDFLAGS = $(LIBSTDCXX_RAW_CXX_LDLAGS) \ +lib_gnu_awt_xlib_la_LDFLAGS = $(LIBSTDCXX_RAW_CXX_LIBADD) \ @X_PRE_LIBS@ @X_LIBS@ -lX11 @X_EXTRA_LIBS@ \ -rpath $(toolexeclibdir) $(LIBJAVA_LDFLAGS_NOUNDEF) \ -version-info `grep -v '^\#' $(srcdir)/libtool-version` $(LIBGCJ_LD_SYMBOLIC) >>> >>> It is still wrong to use LDFLAGS for libraries to be linked in. >>> All of $(LIBSTDCXX_RAW_CXX_LIBADD) @X_PRE_LIBS@ @X_LIBS@ -lX11 >>> @X_EXTRA_LIBS@ should be on lib_gnu_awt_xlib_la_LDADD. >>> >> >> This was how it was done before my change. If we want to >> make a change, I can submit a separate patch. > > Libraries should never occur before the objects which reference them. > This is in GCC 4.7: lib_gnu_awt_xlib_la_LDFLAGS = ../libstdc++-v3/src/libstdc++.la \ @X_PRE_LIBS@ @X_LIBS@ -lX11 @X_EXTRA_LIBS@ \ -rpath $(toolexeclibdir) $(LIBJAVA_LDFLAGS_NOUNDEF) \ -version-info `grep -v '^\#' $(srcdir)/libtool-version` $(LIBGCJ_LD_SYMBOLIC) lib_gnu_awt_xlib_la_LINK = $(LIBLINK) $(lib_gnu_awt_xlib_la_LDFLAGS) \ $(lib_gnu_awt_xlib_la_version_arg) It does put libraries first. If it is a real bug, it should be fixed separately. -- H.J.
[PATCH] Fix PR55862
The following reverts a change I did during the various PRE cleanups. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied. Richard. 2013-01-04 Richard Biener PR tree-optimization/55862 * tree-ssa-pre.c (phi_translate_1): Valueize SSA names after translating them through PHI nodes. * gcc.dg/torture/pr55862.c: New testcase. Index: gcc/tree-ssa-pre.c === *** gcc/tree-ssa-pre.c (revision 194900) --- gcc/tree-ssa-pre.c (working copy) *** phi_translate_1 (pre_expr expr, bitmap_s *** 1729,1734 --- 1729,1738 edge e = find_edge (pred, gimple_bb (def_stmt)); tree def = PHI_ARG_DEF (def_stmt, e->dest_idx); + /* Valueize it. */ + if (TREE_CODE (def) == SSA_NAME) + def = VN_INFO (def)->valnum; + /* Handle constant. */ if (is_gimple_min_invariant (def)) return get_or_alloc_expr_for_constant (def); Index: gcc/testsuite/gcc.dg/torture/pr55862.c === *** gcc/testsuite/gcc.dg/torture/pr55862.c (revision 0) --- gcc/testsuite/gcc.dg/torture/pr55862.c (working copy) *** *** 0 --- 1,31 + /* { dg-do compile } */ + + int g, a, *b; + + void f(void) + { + int *p; + + if(g) + { + int **k = &p; + + for(; g; p++) + for(a = 0; a < 1; a++) + { + int *c = p; + label2: + if(a < 1) + *c = 0; + } + + goto label1; + + while(g++) + for(*b = 0; *b; b++) + label1: + ; + } + + goto label2; + }
Re: [PATCH] Ensure gimplify_one_sizepos doesn't change something with INTEGER_TYPE into something with e.g. ENUMERAL_TYPE (PR middle-end/55851)
On Fri, 4 Jan 2013, Jakub Jelinek wrote: > Hi! > > As discussed in the PR, as all INTEGRAL_TYPE_P types with the same > sign/precision are usually considered compatible > (useless_type_conversion_p), during gimplify_one_sizepos the gimplifier > can change e.g. an integer expression into e.g. VAR_DECL with ENUMERAL_TYPE, > and when the gimplifier later on passes that to size_binop etc., it can > trigger asserts because ENUMERAL_TYPE isn't considered sizetype-ish enough. > > The following patch (which I don't like too much admittedly) makes sure > for constants we fold it back to INTEGER_TYPE constant, and for VAR_DECLs > add an extra VAR_DECL in such a case. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > Or do you prefer some other way? The other way would be Index: gcc/fold-const.c === --- gcc/fold-const.c(revision 194900) +++ gcc/fold-const.c(working copy) @@ -900,9 +900,9 @@ associate_trees (location_t loc, tree t1 static bool int_binop_types_match_p (enum tree_code code, const_tree type1, const_tree type2) { - if (TREE_CODE (type1) != INTEGER_TYPE && !POINTER_TYPE_P (type1)) + if (!INTEGRAL_TYPE_P (type1) && !POINTER_TYPE_P (type1)) return false; - if (TREE_CODE (type2) != INTEGER_TYPE && !POINTER_TYPE_P (type2)) + if (!INTEGRAL_TYPE_P (type2) && !POINTER_TYPE_P (type2)) return false; switch (code) at least the present check doesn't really verify we use sizetypes for size_*op anymore. I'd agree more with your patch if we'd verify that we have proper [s]sizetype, [s]bitsizetype type arguments to size_*op functions (which of course would be again questionable to some degree). That said - we seem to have moved into a direction that makes the above patch not so questionable. Richard. > 2013-01-04 Jakub Jelinek > > PR middle-end/55851 > * gimplify.c (gimplify_one_sizepos): Ensure gimplify_expr > doesn't turn *expr_p from INTEGER_TYPE expression into e.g. > ENUMERAL_TYPE expression. > > * gcc.c-torture/compile/pr55851.c: New test. > > --- gcc/gimplify.c.jj 2012-12-20 19:13:00.0 +0100 > +++ gcc/gimplify.c2013-01-03 16:16:07.288707387 +0100 > @@ -8180,6 +8180,26 @@ gimplify_one_sizepos (tree *expr_p, gimp >*expr_p = unshare_expr (expr); > >gimplify_expr (expr_p, stmt_p, NULL, is_gimple_val, fb_rvalue); > + > + /* Ensure we don't change an INTEGER_TYPE expression into e.g. > ENUMERAL_TYPE > + expression. */ > + if (TREE_CODE (TREE_TYPE (*expr_p)) != TREE_CODE (TREE_TYPE (expr))) > +{ > + gcc_checking_assert (useless_type_conversion_p (TREE_TYPE (expr), > + TREE_TYPE (*expr_p))); > + if (TREE_CODE (*expr_p) == INTEGER_CST) > + *expr_p = fold_convert (TREE_TYPE (expr), *expr_p); > + else > + { > + tree var = create_tmp_var (TREE_TYPE (expr), NULL); > + tree mod = build2 (INIT_EXPR, TREE_TYPE (var), > + var, unshare_expr (*expr_p)); > + SET_EXPR_LOCATION (mod, EXPR_LOC_OR_HERE (*expr_p)); > + gimplify_and_add (mod, stmt_p); > + ggc_free (mod); > + *expr_p = var; > + } > +} > } > > /* Gimplify the body of statements of FNDECL and return a GIMPLE_BIND node > --- gcc/testsuite/gcc.c-torture/compile/pr55851.c.jj 2013-01-03 > 16:20:19.085284806 +0100 > +++ gcc/testsuite/gcc.c-torture/compile/pr55851.c 2013-01-03 > 16:19:27.698571718 +0100 > @@ -0,0 +1,12 @@ > +/* PR middle-end/55851 */ > + > +enum { A = 1UL, B = -1UL } var = A; > +void foo (char *); > + > +void > +test (void) > +{ > + char vla[1][var]; > + vla[0][0] = 1; > + foo (&vla[0][0]); > +} > > Jakub > > -- Richard Biener SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imend
Re: [Patch, Darwin, ppc] constrain processor usage for m32.
Assembler could be fixed easily (I'll contribute the patch to MacPorts cctools port) and the library is loading and working now. So I suggest disabling the use of 64 bit instructions and registers when targetting PowerPC Darwin 32 bit ABI can be considered working and a patch to gcc shouldn't be necessary. Am 04.01.2013 um 01:02 schrieb Tobias Netzel: Found the problem: The assembler generates a relocation of type LO14, meaning 14 bits that represent bits 2 - 15 of the effective address (lowest two bits are forced to zero), which is used for some types of 64 bit loads and stores. Unfortunately the OS X dynamic linker doesn't support that relocation type any more beginning with 10.4. After looking at the dyld sources It seems LO16 should be used instead. I'll see if I can fix the assembler. Am 01.01.2013 um 22:06 schrieb Mike Stump: On Jan 1, 2013, at 10:03 AM, Tobias Netzel > wrote: Or do you have any other ideas? I don't. I'd grab the .s files (compile with -save-temps) and start stripping things out til it loads, then then last thing stripped was the thing that broke it.
[PATCH] Ensure gimplify_one_sizepos doesn't change something with INTEGER_TYPE into something with e.g. ENUMERAL_TYPE (PR middle-end/55851)
Hi! As discussed in the PR, as all INTEGRAL_TYPE_P types with the same sign/precision are usually considered compatible (useless_type_conversion_p), during gimplify_one_sizepos the gimplifier can change e.g. an integer expression into e.g. VAR_DECL with ENUMERAL_TYPE, and when the gimplifier later on passes that to size_binop etc., it can trigger asserts because ENUMERAL_TYPE isn't considered sizetype-ish enough. The following patch (which I don't like too much admittedly) makes sure for constants we fold it back to INTEGER_TYPE constant, and for VAR_DECLs add an extra VAR_DECL in such a case. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Or do you prefer some other way? 2013-01-04 Jakub Jelinek PR middle-end/55851 * gimplify.c (gimplify_one_sizepos): Ensure gimplify_expr doesn't turn *expr_p from INTEGER_TYPE expression into e.g. ENUMERAL_TYPE expression. * gcc.c-torture/compile/pr55851.c: New test. --- gcc/gimplify.c.jj 2012-12-20 19:13:00.0 +0100 +++ gcc/gimplify.c 2013-01-03 16:16:07.288707387 +0100 @@ -8180,6 +8180,26 @@ gimplify_one_sizepos (tree *expr_p, gimp *expr_p = unshare_expr (expr); gimplify_expr (expr_p, stmt_p, NULL, is_gimple_val, fb_rvalue); + + /* Ensure we don't change an INTEGER_TYPE expression into e.g. ENUMERAL_TYPE + expression. */ + if (TREE_CODE (TREE_TYPE (*expr_p)) != TREE_CODE (TREE_TYPE (expr))) +{ + gcc_checking_assert (useless_type_conversion_p (TREE_TYPE (expr), + TREE_TYPE (*expr_p))); + if (TREE_CODE (*expr_p) == INTEGER_CST) + *expr_p = fold_convert (TREE_TYPE (expr), *expr_p); + else + { + tree var = create_tmp_var (TREE_TYPE (expr), NULL); + tree mod = build2 (INIT_EXPR, TREE_TYPE (var), +var, unshare_expr (*expr_p)); + SET_EXPR_LOCATION (mod, EXPR_LOC_OR_HERE (*expr_p)); + gimplify_and_add (mod, stmt_p); + ggc_free (mod); + *expr_p = var; + } +} } /* Gimplify the body of statements of FNDECL and return a GIMPLE_BIND node --- gcc/testsuite/gcc.c-torture/compile/pr55851.c.jj2013-01-03 16:20:19.085284806 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr55851.c 2013-01-03 16:19:27.698571718 +0100 @@ -0,0 +1,12 @@ +/* PR middle-end/55851 */ + +enum { A = 1UL, B = -1UL } var = A; +void foo (char *); + +void +test (void) +{ + char vla[1][var]; + vla[0][0] = 1; + foo (&vla[0][0]); +} Jakub
[PATCH, PR 55579] Make SRA keep candidated with only debug replacements
Hi, the patch below fixes PR 55579 by not disqualifying a candidate variable when it does not have any real replacements but has ones for debug statements only. analyze_access_subtree has to return the same value regardless of MAY_HAVE_DEBUG_STMTS or we get debug comparison errors. I also have a patch that simply avoids disqualifying candidates with no replacements which means we keep some information about it so that SRA can remove a few more un-needed reads but I suppose that is stage1 material. OK for trunk? Thanks, Martin 2013-01-03 Martin Jambor PR debug/55579 * tree-sra.c (analyze_access_subtree): Return true also after potentially creating a debug-only replacement. testsuite/ * gcc.dg/tree-ssa/pr55579.c: New test. Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr55579.c === --- /dev/null +++ src/gcc/testsuite/gcc.dg/tree-ssa/pr55579.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -g -fdump-tree-esra" } */ + +struct S { int a; char b; char c; short d; }; + +int +foo (int x) +{ + struct S s = { x + 1, x + 2, x + 3, x + 4 }; + char *p = &s.c; + return x; +} + +/* { dg-final { scan-tree-dump "Created a debug-only replacement for s" "esra" } } */ Index: src/gcc/tree-sra.c === --- src.orig/gcc/tree-sra.c +++ src/gcc/tree-sra.c @@ -2197,20 +2197,25 @@ analyze_access_subtree (struct access *r } else { - if (MAY_HAVE_DEBUG_STMTS && allow_replacements + if (allow_replacements && scalar && !root->first_child && (root->grp_scalar_write || root->grp_assignment_write)) { gcc_checking_assert (!root->grp_scalar_read && !root->grp_assignment_read); - root->grp_to_be_debug_replaced = 1; - if (dump_file && (dump_flags & TDF_DETAILS)) + sth_created = true; + if (MAY_HAVE_DEBUG_STMTS) { - fprintf (dump_file, "Marking "); - print_generic_expr (dump_file, root->base, 0); - fprintf (dump_file, " offset: %u, size: %u ", - (unsigned) root->offset, (unsigned) root->size); - fprintf (dump_file, " to be replaced with debug statements.\n"); + root->grp_to_be_debug_replaced = 1; + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, "Marking "); + print_generic_expr (dump_file, root->base, 0); + fprintf (dump_file, " offset: %u, size: %u ", + (unsigned) root->offset, (unsigned) root->size); + fprintf (dump_file, " to be replaced with debug " + "statements.\n"); + } } } @@ -2220,17 +2225,11 @@ analyze_access_subtree (struct access *r root->grp_total_scalarization = 0; } - if (sth_created - && (!hole || root->grp_total_scalarization)) -{ - root->grp_covered = 1; - return true; -} - if (root->grp_write || TREE_CODE (root->base) == PARM_DECL) + if (!hole || root->grp_total_scalarization) +root->grp_covered = 1; + else if (root->grp_write || TREE_CODE (root->base) == PARM_DECL) root->grp_unscalarized_data = 1; /* not covered and written to */ - if (sth_created) -return true; - return false; + return sth_created; } /* Analyze all access trees linked by next_grp by the means of
[committed] 2011 and 2012 Copyright year updates
Hi! I've run a script to notice gcc maintained files with FSF copyright that have been modified in 2011 and/or 2012 (according to svn log, ignoring r168438 and r184997 commits), but didn't have years 2011 and/or 2012 included in Copyright lines. I've kept the preexisting style, so where year ranges were used, updated those if needed, if not, kept the year lists. Jakub Copyright.updates.bz2 Description: BZip2 compressed data
[Patch, Fortran] PR55763 - improve init-data checks for pointers
Fortran 2008 allows: integer :: pointer => init_data and type t integer :: pointer => init_data end type t The current check in gfc_check_assign_symbol was only called for former and for constructors, but not for the type definition. Additionally, BT_CLASS wasn't handled. I also improved the error location. The patch has a downside: One gets some messages twice or trice: Once for resolving the type declaration ("type t") and then for resolving the default initialization via gfc_traverse_ns (ns, resolve_values); Currently, that's unavoidable as one cannot trivially distinguish between a user-supplied "sym->value" and the default constructor. If you think that this is a problem, one can change it, e.g. by setting a sym->attr.value_is_default_init. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias PS: For CLASS pointers, there will be an ICE if one tries to associate a variable to them; that's unchanged by this patch. 2013-01-04 Tobias Burnus PR fortran/55763 * gfortran.h (gfc_check_assign_symbol): Update prototype. * decl.c (add_init_expr_to_sym, do_parm): Update call. * expr.c (gfc_check_assign_symbol): Handle BT_CLASS and improve error location; support components. (gfc_check_pointer_assign): Handle component assignments. * resolve.c (resolve_fl_derived0): Call gfc_check_assign_symbol. (resolve_values): Update call. (resolve_structure_cons): Avoid double diagnostic. 2013-01-04 Tobias Burnus PR fortran/55763 * gfortran.dg/pointer_init_2.f90: Update dg-error. * gfortran.dg/pointer_init_7.f90: New. diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c index fc86efb..5952b70 100644 --- a/gcc/fortran/decl.c +++ b/gcc/fortran/decl.c @@ -1353,14 +1353,14 @@ add_init_expr_to_sym (const char *name, gfc_expr **initp, locus *var_locus) if (sym->ts.type != BT_DERIVED && init->ts.type != BT_DERIVED && sym->ts.type != BT_CLASS && init->ts.type != BT_CLASS && !sym->attr.proc_pointer - && gfc_check_assign_symbol (sym, init) == FAILURE) + && gfc_check_assign_symbol (sym, NULL, init) == FAILURE) return FAILURE; if (sym->ts.type == BT_CHARACTER && sym->ts.u.cl && init->ts.type == BT_CHARACTER) { /* Update symbol character length according initializer. */ - if (gfc_check_assign_symbol (sym, init) == FAILURE) + if (gfc_check_assign_symbol (sym, NULL, init) == FAILURE) return FAILURE; if (sym->ts.u.cl->length == NULL) @@ -6955,7 +6955,7 @@ do_parm (void) goto cleanup; } - if (gfc_check_assign_symbol (sym, init) == FAILURE + if (gfc_check_assign_symbol (sym, NULL, init) == FAILURE || gfc_add_flavor (&sym->attr, FL_PARAMETER, sym->name, NULL) == FAILURE) { m = MATCH_ERROR; diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c index 2610784..146154e 100644 --- a/gcc/fortran/expr.c +++ b/gcc/fortran/expr.c @@ -3291,22 +3291,21 @@ gfc_check_assign (gfc_expr *lvalue, gfc_expr *rvalue, int conform) gfc_try gfc_check_pointer_assign (gfc_expr *lvalue, gfc_expr *rvalue) { - symbol_attribute attr; + symbol_attribute attr, lhs_attr; gfc_ref *ref; bool is_pure, is_implicit_pure, rank_remap; int proc_pointer; - if (lvalue->symtree->n.sym->ts.type == BT_UNKNOWN - && !lvalue->symtree->n.sym->attr.proc_pointer) + lhs_attr = gfc_expr_attr (lvalue); + if (lvalue->ts.type == BT_UNKNOWN && !lhs_attr.proc_pointer) { gfc_error ("Pointer assignment target is not a POINTER at %L", &lvalue->where); return FAILURE; } - if (lvalue->symtree->n.sym->attr.flavor == FL_PROCEDURE - && lvalue->symtree->n.sym->attr.use_assoc - && !lvalue->symtree->n.sym->attr.proc_pointer) + if (lhs_attr.flavor == FL_PROCEDURE && lvalue->symtree->n.sym->attr.use_assoc + && !lhs_attr.proc_pointer) { gfc_error ("'%s' in the pointer assignment at %L cannot be an " "l-value since it is a procedure", @@ -3735,10 +3734,11 @@ gfc_check_pointer_assign (gfc_expr *lvalue, gfc_expr *rvalue) symbol. Used for initialization assignments. */ gfc_try -gfc_check_assign_symbol (gfc_symbol *sym, gfc_expr *rvalue) +gfc_check_assign_symbol (gfc_symbol *sym, gfc_component *comp, gfc_expr *rvalue) { gfc_expr lvalue; gfc_try r; + bool pointer, proc_pointer; memset (&lvalue, '\0', sizeof (gfc_expr)); @@ -3750,9 +3750,27 @@ gfc_check_assign_symbol (gfc_symbol *sym, gfc_expr *rvalue) lvalue.symtree->n.sym = sym; lvalue.where = sym->declared_at; - if (sym->attr.pointer || sym->attr.proc_pointer - || (sym->ts.type == BT_CLASS && CLASS_DATA (sym)->attr.class_pointer - && rvalue->expr_type == EXPR_NULL)) + if (comp) +{ + lvalue.ref = gfc_get_ref (); + lvalue.ref->type = REF_COMPONENT; + lvalue.ref->u.c.component = comp; + lvalue.ref->u.c.sym = sym; + lvalue.ts = comp->ts; + lvalue.rank = comp->as ? comp->as->rank : 0; + lvalue.where = comp-
Re: [Patch, Fortran] PR55763 - reject MOLD with NULL() in init-data expressions
Hello, Le 04/01/2013 00:23, Tobias Burnus a écrit : NULL with MOLD should be rejected as (default) initialization expression. From F2008: R506 null-init is function-reference C512 (R506) The function-reference shall be a reference to the intrinsic function NULL with no arguments. "null-init" occurs twice, as "R505 initialization" in "R505 initialization" and in "R442 component-initialization" (default initialization). Before, integer, pointer :: p => null(x) gave an type error (LHS: integer, RHS: unknown). While class(*), pointer :: p => null(x) was accepted without error diagnostic. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c index 5ed8388..7d49578 100644 --- a/gcc/fortran/decl.c +++ b/gcc/fortran/decl.c @@ -1671,11 +1671,31 @@ match gfc_match_null (gfc_expr **result) { gfc_symbol *sym; - match m; + match m, m2 = MATCH_NO; - m = gfc_match (" null ( )"); - if (m != MATCH_YES) -return m; + if ((m = gfc_match (" null ( )")) == MATCH_ERROR) +return MATCH_ERROR; + + if (m == MATCH_NO) +{ + locus old_loc; + char name[GFC_MAX_SYMBOL_LEN + 1]; + + if ((m2 = gfc_match (" null (", name)) != MATCH_YES) It seems the `name' argument to `gfc_match' is superfluous here. Thanks for the patch. Mikael
Re: [PATCH] Clarify error message (PR middle-end/55859)
On Fri, Jan 04, 2013 at 12:05:49PM +0100, Marek Polacek wrote: > On Fri, Jan 04, 2013 at 11:42:24AM +0100, Jakub Jelinek wrote: > > I wonder what is the point of using %qs with a fixed string, and ' should be > > probably replaced by %< and %>, so perhaps > > "argument to %<-O%> should be a non-negative integer, " > > "%, % or %"); > > So, like this? Yes, thanks. Jakub
Re: [PATCH] Clarify error message (PR middle-end/55859)
On Fri, Jan 04, 2013 at 11:42:24AM +0100, Jakub Jelinek wrote: > I wonder what is the point of using %qs with a fixed string, and ' should be > probably replaced by %< and %>, so perhaps > "argument to %<-O%> should be a non-negative integer, " > "%, % or %"); So, like this? 2013-01-04 Marek Polacek * opts.c (default_options_optimization): Clarify error message. --- gcc/opts.c.mp 2013-01-04 02:48:33.116785922 +0100 +++ gcc/opts.c 2013-01-04 12:00:15.983233160 +0100 @@ -1,7 +1,6 @@ /* Command line option handling. Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, - 2012 - + 2012, 2013 Free Software Foundation, Inc. Contributed by Neil Booth. @@ -542,9 +541,8 @@ default_options_optimization (struct gcc { const int optimize_val = integral_argument (opt->arg); if (optimize_val == -1) - error_at (loc, - "argument to %qs should be a non-negative integer", - "-O"); + error_at (loc, "argument to %<-O%> should be a non-negative " + "integer, %, % or %"); else { opts->x_optimize = optimize_val; Marek
Re: RFA: Fix ICE on PARALLEL returns when expand builtins
On Thu, Jan 3, 2013 at 7:42 PM, Richard Sandiford wrote: > Richard Biener writes: >> On Wed, Jan 2, 2013 at 5:36 PM, Richard Sandiford >> wrote: >>> Richard Biener writes: On Sun, Dec 23, 2012 at 10:43 AM, Richard Sandiford wrote: > Some of the maths builtins can expand to a call followed by a bit > of postprocessing. With 4.8's PARALLEL return optimisations, these > embedded calls might return a PARALLEL of pseudos, but the postprocessing > isn't prepared to deal with that. This leads to an ICE in builtins-53.c > on n32 and n64 mips64-linux-gnu. > > One fix might have been to pass an explicit register target to the > expand routines, but that target's only a hint. This patch instead > adds an avoid_group_rtx function (named after gen_group_rtx) to convert > PARALLELs to pseudos where necessary. > > I wondered whether it was really safe for expand_builtin_int_roundingfn_2 > to pass "target == const0_rtx" as the "ignore" parameter to expand_call, > given that we don't actually ignore the return value ourselves > (even if the caller does). I suppose it is safe though, > since expand_call will always return const0_rtx in that case, > and this code is dealing with integer return values. > > Tested on mips64-linux-gnu. OK to install? Or is there a better way? You didn't add a testcase so I can't check myself >>> >>> It's gcc.dg/builtins-53.c. >>> - but why isn't using force_reg enough here? I can imagine other cases than PARALLELs that are not well handled, no? >>> >>> Not sure either way TBH. Fortunately expanding your own calls seems >>> to be pretty rare. >>> >>> But yeah, having force_reg (or I suppose force_operand) do it sounds >>> good in principle. The problem is that the operation needs the type >>> tree, which the force_* routines don't have. >> >> Hm? force_reg/operand only need a mode. >> >> Index: builtins.c >> === >> *** builtins.c (revision 194787) >> --- builtins.c (working copy) >> *** expand_builtin_int_roundingfn (tree exp, >> *** 2760,2765 >> --- 2760,2766 >> >> /* Truncate the result of floating point optab to integer >>via expand_fix (). */ >> + tmp = force_reg (TYPE_MODE (TREE_TYPE (TREE_TYPE (fallback_fndecl))), >> tmp); >> target = gen_reg_rtx (mode); >> expand_fix (target, tmp, 0); > > What I mean is: force_operand doesn't convert PARALLELs of pseudos > to single pseudos, so this won't work as-is. And we can't make > force_operand do the conversion using the current emit_group_store > machinery because emit_group_store needs access to the type (in order > to work out the padding), which force_operand doesn't have. Hmm, how would anyone else get at the "padding" info dealing with the parallel? This looks broken if we need access to the type :/ Now, why's that rounding function case relevant at all? The rounding fns in question return FP mode values - I seriously doubt there is any "padding" involved (and I can't see how any target would use a multi-register passing here?!) Eh. Eric, any comments? Thanks, Richard. > Richard
Re: [PATCH, PR 55755] Make SRA create less VIEW_CONVERT_EXPRs
On Thu, 3 Jan 2013, Martin Jambor wrote: > Hi, > > the patch below fixes PR 55755 which was in the compiler for years. > The problem is that a replacement of a bit-field can have a larger > TYPE_SIZE than the type of the field and so creating a V_C_E from it > to the field type may result in invalid gimple. We do that when we > scalarize only one side of an assignment and get incompatible types on > both sides and the other (non-scalar) side has a child access in the > access tree (regardless if it is to be scalarize or not). > > When looking at the issue I realized that the last condition is > completely unnecessary (at least now, the first concepts of the "new" > SRA were a bit different) because the subsequent handling of > sub-replacements will do the right thing (load/store them to the > original aggregate) and removing it is indeed the correct thing to > deal with this bug - if both sides are scalarized, size of both will > grow to mode size, if only one, we can avoid the V_C_E. I am a little > worried about the contains_bitfld_comp_ref_p and > contains_vce_or_bfcref_p gurads which are there because of Ada PR > 46349 (which involves aggregate bit-fields) and which might in theory > lead to the same problem but I'm weary of touching it, at least not in > one commit (I'm testing what happens if I remove them right now), and > this patch does not make the current situation any worse. > > In order to make sure we do not mess up when the non-scalar side has > sub-replacements in it, I have added a new testcase. > > The patch has passed bootstrap and testing on x86_64-linux on trunk > and the 4.7 and 4.6 branches. I'd like to commit it to all of them, > perhaps after having it on trunk only for a while. Ok for trunk and branches after a while. Thanks, Richard. > Thanks, > > Martin > > > 2013-01-02 Martin Jambor > > PR tree-optimization/55755 > * tree-sra.c (sra_modify_assign): Do not check that an access has no > children when trying to avoid producing a VIEW_CONVERT_EXPR. > > testsuite/ > * gcc.dg/torture/pr55755.c: New test. > * gcc.dg/tree-ssa/sra-13.c: Likewise. > * gcc.dg/tree-ssa/pr45144.c: Update. > > Index: src/gcc/testsuite/gcc.dg/torture/pr55755.c > === > --- /dev/null > +++ src/gcc/testsuite/gcc.dg/torture/pr55755.c > @@ -0,0 +1,43 @@ > +/* { dg-do run } */ > +/* { dg-require-effective-target int32plus } */ > + > +struct S4 > +{ > + unsigned f0:24; > +} __attribute__((__packed__)); > + > +struct S4 g_10 = { > + 6210831 > +}; > + > +struct S5 > +{ > + int i; > + struct S4 l_8[2]; > +} __attribute__((__packed__)); > + > +int a, b; > + > +struct S4 func_2 (int x) > +{ > + struct S5 l = { > +0, > +{{0}, {0}} > + }; > + l.i = a; > + g_10 = l.l_8[1]; > + for (; x<2; x++) { > +struct S4 tmp = { > + 11936567 > +}; > +l.l_8[x] = tmp; > + } > + b = l.i; > + return g_10; > +} > + > +int main (void) > +{ > + func_2 (0); > + return 0; > +} > Index: src/gcc/testsuite/gcc.dg/tree-ssa/sra-13.c > === > --- /dev/null > +++ src/gcc/testsuite/gcc.dg/tree-ssa/sra-13.c > @@ -0,0 +1,114 @@ > +/* Test that SRA replacement can deal with assignments that have > + sub-replacements on one side and a single scalar replacement on another. > */ > +/* { dg-do run } */ > +/* { dg-options "-O1" } */ > + > +struct A > +{ > + int i1, i2; > +}; > + > +struct B > +{ > + long long int l; > +}; > + > +union U > +{ > + struct A a; > + struct B b; > +}; > + > +int b, gi; > +long gl; > +union U gu1, gu2; > + > +int __attribute__ ((noinline, noclone)) > +foo (void) > +{ > + union U x, y; > + int r; > + > + y = gu1; > + if (b) > +y.b.l = gl; > + > + x = y; > + > + if (!b) > +r = x.a.i1; > + else > +r = 0; > + > + gu2 = x; > + return r; > +} > + > +long long int __attribute__ ((noinline, noclone)) > +bar (void) > +{ > + union U x, y; > + int r; > + > + y = gu1; > + if (b) > +y.a.i1 = gi; > + > + x = y; > + > + if (!b) > +r = x.b.l; > + else > +r = 0; > + > + gu2 = x; > + return r; > +} > + > + > +int > +main (void) > +{ > + int r; > + long long int s; > + > + b = 0; > + gu1.a.i1 = 123; > + gu1.a.i2 = 234; > + r = foo (); > + if (r != 123) > +__builtin_abort (); > + if (gu2.a.i1 != 123) > +__builtin_abort (); > + if (gu2.a.i2 != 234) > +__builtin_abort (); > + > + b = 1; > + gl = 1001; > + gu1.b.l = 1000; > + r = foo (); > + if (r != 0) > +__builtin_abort (); > + if (gu2.b.l != 1001) > +__builtin_abort (); > + > + b = 0; > + gu1.b.l = 2000; > + s = bar (); > + if (s != 2000) > +__builtin_abort (); > + if (gu2.b.l != 2000) > +__builtin_abort (); > + > + b = 1; > + gi = 456; > + gu1.a.i1 = 123; > + gu1.a.i2 = 234; > + s = bar (); > + if (s != 0) > +__builtin_abort (); > + if (gu2.a.i1 != 456)
Re: [PATCH] Clarify error message (PR middle-end/55859)
On Fri, Jan 04, 2013 at 02:59:46AM +0100, Marek Polacek wrote: > This patch clarifies the error message when using e.g. -Obar option, > except non-negative numbers we accept some other levels too. > Bootstrapped on x86_64-linux. Ok for trunk? > > 2013-01-04 Marek Polacek > > PR middle-end/55859 > * opts.c (default_options_optimization): Clarify error message. > > --- gcc/opts.c.mp 2013-01-04 02:48:33.116785922 +0100 > +++ gcc/opts.c2013-01-04 02:48:51.729870314 +0100 > @@ -1,7 +1,6 @@ > /* Command line option handling. > Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, > - 2012 > - > + 2012, 2013 > Free Software Foundation, Inc. > Contributed by Neil Booth. > > @@ -543,7 +542,8 @@ default_options_optimization (struct gcc > const int optimize_val = integral_argument (opt->arg); > if (optimize_val == -1) > error_at (loc, > - "argument to %qs should be a non-negative integer", > + "argument to %qs should be a non-negative integer, " > + "'g', 's', or 'fast'", > "-O"); I wonder what is the point of using %qs with a fixed string, and ' should be probably replaced by %< and %>, so perhaps "argument to %<-O%> should be a non-negative integer, " "%, % or %"); Jakub
[PATCH] Fix PR55863
This fixes PR55863, a folding missed optimization caused by folding -X - 1 to ~X. This makes data dependence analysis fail for offsetted-by-1 accesses (in some cases). Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2013-01-04 Richard Biener PR middle-end/55863 * fold-const.c (split_tree): Undo -X - 1 to ~X folding for reassociation. * gcc.dg/fold-reassoc-2.c: New testcase. Index: gcc/fold-const.c === *** gcc/fold-const.c(revision 194855) --- gcc/fold-const.c(working copy) *** split_tree (tree in, enum tree_code code *** 821,826 --- 821,833 if (neg_var_p) var = negate_expr (var); } + else if (TREE_CODE (in) == BIT_NOT_EXPR + && code == PLUS_EXPR) + { + /* -X - 1 is folded to ~X, undo that here. */ + *minus_litp = build_one_cst (TREE_TYPE (in)); + var = negate_expr (TREE_OPERAND (in, 0)); + } else if (TREE_CONSTANT (in)) *conp = in; else Index: gcc/testsuite/gcc.dg/fold-reassoc-2.c === *** gcc/testsuite/gcc.dg/fold-reassoc-2.c (revision 0) --- gcc/testsuite/gcc.dg/fold-reassoc-2.c (working copy) *** *** 0 --- 1,14 + /* { dg-do compile } */ + /* { dg-options "-O -fdump-tree-original" } */ + + int foo (int i) + { + return (i + 2) - (i + 1); + } + int bar (int i) + { + return (i + 2) + ~i; + } + + /* { dg-final { scan-tree-dump "return 1;" "original" } } */ + /* { dg-final { cleanup-tree-dump "original" } } */
Commit: V850: Only compile callt support functions when CALLT is enabled
Hi Guys, I am checking in the patch below to fix a small problem with libgcc for the V850. The CALLT support functions were being compiled even if the particular multilib concerned did not support the CALLT insn. Normally this would not matter, but the linker will now report an unknown relocation error for these functions due to the way that they are implemented. Tested with no regressions on a v850-elf toolchain. Cheers Nick libgcc/ChangeLog 2013-01-04 Nick Clifton * config/v850/lib1funcs.S: Only provide CALLT support functions if the CALLT instruction is supported. Index: libgcc/config/v850/lib1funcs.S === --- libgcc/config/v850/lib1funcs.S (revision 194883) +++ libgcc/config/v850/lib1funcs.S (working copy) @@ -1764,6 +1764,7 @@ .size __restore_all_interrupt,.-__restore_all_interrupt #endif /* L_save_all_interrupt */ +#if defined __V850_CALLT__ #if defined(__v850e__) || defined(__v850e1__) || defined(__v850e2__) || defined(__v850e2v3__) #ifdef L_callt_save_r2_r29 /* Put these functions into the call table area. */ @@ -2146,6 +2147,7 @@ #endif #endif /* __v850e__ */ +#endif /* __V850_CALLT__ */ /* libgcc2 routines for NEC V850. */ /* Double Integer Arithmetical Operation. */
[Ada] Cleanup order of Nkind declarations in Sinfo
This is an internal cleanup, no functional effect, no test Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Robert Dewar * sinfo.ads: Clean up order of N_xxx subtypes Index: sinfo.ads === --- sinfo.ads (revision 194887) +++ sinfo.ads (working copy) @@ -7638,6 +7638,12 @@ N_Function_Call, N_Procedure_Call_Statement, + -- N_Subexpr, N_Has_Etype, N_Raise_xxx_Error + + N_Raise_Constraint_Error, + N_Raise_Program_Error, + N_Raise_Storage_Error, + -- N_Subexpr, N_Has_Etype N_Explicit_Dereference, @@ -7648,15 +7654,6 @@ N_Null, N_Qualified_Expression, N_Quantified_Expression, - - -- N_Raise_xxx_Error, N_Subexpr, N_Has_Etype - - N_Raise_Constraint_Error, - N_Raise_Program_Error, - N_Raise_Storage_Error, - - -- N_Subexpr, N_Has_Etype - N_Aggregate, N_Allocator, N_Case_Expression,
[RFA/ARM/4.7] Fix PR54974: Thumb literal pools don't handle PC rounding
On 29/11/12 14:42, Matthew Gretton-Dann wrote: On 24 November 2012 00:27, Ramana Radhakrishnan wrote: On Wed, Nov 21, 2012 at 7:59 PM, Matthew Gretton-Dann wrote: [snip] The fix is to decrease the pool_range of all insns by 2 when generating Thumb code. There is no need to change neg_pool_range values as rounding down here will reduce the distance of the literal pool. A comment about this fact around thumb2_pool_range would be appropriate. [snip] Tested arm-none-linux-gnueabi cross, and with the testcase attached to the PR. No added testcase in the patch as this code is sensitive to other code generation and so it is not easy to generate a testcase which will reliably test this condition. OK for trunk, 4.7, and 4.6? Ok for trunk today - please wait a few days before backporting into 4.6 and 4.7 to see what the fallout is like . Watch out for any fallout with the auto-testers. No fallout has been seen with the auto-testers. The attached is what was actually committed as revision 193930 (original patch + requested comment). The attached patch is a backport of the trunk patch to 4.7. Cross tested arm-none-linux-gnueabi with QEMU OK for 4.7? Thanks, Matt 2013-01-04 Matthew Gretton-Dann Backport from mainline. 2012-11-29 Matthew Gretton-Dann PR target/54974 * config/arm/arm.md (thumb2_pool_range, pool_range): Add comment on Thumb pool ranges. (thumb1_extendhisi2): Reduce Thumb pool range. (arm_movdi): Likewise. (thumb1_movdi_insn): Likewise. (thumb1_movsi_insn): Likewise. (pic_load_addr_unified): Likewise. (pic_load_addr_32bit): Likewise. (pic_load_addr_thumb1): Likewise. (thumb1_movhf): Likewise. (arm_movsf_soft_insn): Likewise. (thumb1_movsf_soft_insn): Likewise. (movdf_soft_insn): Likewise. (thumb1_movdf_soft_insn): Likewise. * config/arm/neon.md (*neon_mov): Likewise. (*neon_mov): Likwise. * config/arm/thumb2.md: (*thumb2_movsi_insn): Likewise. (*thumb2_movhi_insn): Likewise. (*thumb2_extendqisi_v6): Likewise. (*thumb2_zero_extendqisi_v6): Likewise. (*thumb2_zero_extendqisi2_v6): Likewise. * config/arm/vfp.md: (*thumb2_movsi_vfp): Likewise. (*movdi_vfp): Likewise. (*movdi_vfp_cortexa8): Likewise. (*thumb2_movsf_vfp): Likewise. (*thumb2_movdf_vfp): Likewise. -- Matthew Gretton-Dann Toolchain Working Group, Linaro Index: gcc/config/arm/arm.md === --- gcc/config/arm/arm.md (revision 194852) +++ gcc/config/arm/arm.md (working copy) @@ -256,6 +256,9 @@ (define_attr "insn_enabled" "no,yes" ; POOL_RANGE is how far away from a constant pool entry that this insn ; can be placed. If the distance is zero, then this insn will never ; reference the pool. +; Note that for Thumb constant pools the PC value is rounded down to the +; nearest multiple of four. Therefore, THUMB2_POOL_RANGE (and POOL_RANGE for +; Thumb insns) should be set to - 2. ; NEG_POOL_RANGE is nonzero for insns that can reference a constant pool entry ; before its address. It is set to - (8 + ). (define_attr "arm_pool_range" "" (const_int 0)) @@ -4833,7 +4836,7 @@ (define_insn "thumb1_extendhisi2" (const_int 2) (const_int 4)) (const_int 4)]) (set_attr "type" "alu_shift,load_byte") - (set_attr "pool_range" "*,1020")] + (set_attr "pool_range" "*,1018")] ) ;; This pattern will only be used when ldsh is not available @@ -5239,7 +5242,7 @@ (define_insn "*arm_movdi" (set_attr "type" "*,*,*,load2,store2") (set_attr "arm_pool_range" "*,*,*,1020,*") (set_attr "arm_neg_pool_range" "*,*,*,1004,*") - (set_attr "thumb2_pool_range" "*,*,*,4096,*") + (set_attr "thumb2_pool_range" "*,*,*,4094,*") (set_attr "thumb2_neg_pool_range" "*,*,*,0,*")] ) @@ -5379,7 +5382,7 @@ (define_insn "*thumb1_movdi_insn" [(set_attr "length" "4,4,6,2,2,6,4,4") (set_attr "type" "*,*,*,load2,store2,load2,store2,*") (set_attr "insn" "*,mov,*,*,*,*,*,mov") - (set_attr "pool_range" "*,*,*,*,*,1020,*,*")] + (set_attr "pool_range" "*,*,*,*,*,1018,*,*")] ) (define_expand "movsi" @@ -5539,7 +5542,7 @@ (define_insn "*thumb1_movsi_insn" mov\\t%0, %1" [(set_attr "length" "2,2,4,4,2,2,2,2,2") (set_attr "type" "*,*,*,*,load1,store1,load1,store1,*") - (set_attr "pool_range" "*,*,*,*,*,*,1020,*,*") + (set_attr "pool_range" "*,*,*,*,*,*,1018,*,*") (set_attr "conds" "set,clob,*,*,nocond,nocond,nocond,nocond,nocond")]) (define_split @@ -5632,7 +5635,7 @@ (define_insn_and_split "pic_load_addr_un (match_dup 2)] UNSPEC_PIC_BASE))] "operands[3] = TARGET_THUMB ? GEN_INT (4) : GEN_INT (8);" [(set_attr "type" "load1,load1,load1") - (set_attr "pool_range" "4096,4096,1024") + (set
Re: Use libstdc++-raw-cxx.m4 in libjava
"H.J. Lu" writes: > On Thu, Jan 3, 2013 at 10:09 AM, Andreas Schwab wrote: >> "H.J. Lu" writes: >> >>> diff --git a/libjava/Makefile.am b/libjava/Makefile.am >>> index c6c84e4..dd08a4f 100644 >>> --- a/libjava/Makefile.am >>> +++ b/libjava/Makefile.am >>> @@ -594,7 +594,7 @@ lib_gnu_awt_xlib_la_CPPFLAGS = \ >>> $(AM_CPPFLAGS) \ >>> $(LIBSTDCXX_RAW_CXX_CXXFLAGS) >>> ## The mysterious backslash in the grep pattern is consumed by make. >>> -lib_gnu_awt_xlib_la_LDFLAGS = $(LIBSTDCXX_RAW_CXX_LDLAGS) \ >>> +lib_gnu_awt_xlib_la_LDFLAGS = $(LIBSTDCXX_RAW_CXX_LIBADD) \ >>> @X_PRE_LIBS@ @X_LIBS@ -lX11 @X_EXTRA_LIBS@ \ >>> -rpath $(toolexeclibdir) $(LIBJAVA_LDFLAGS_NOUNDEF) \ >>> -version-info `grep -v '^\#' $(srcdir)/libtool-version` >>> $(LIBGCJ_LD_SYMBOLIC) >> >> It is still wrong to use LDFLAGS for libraries to be linked in. >> All of $(LIBSTDCXX_RAW_CXX_LIBADD) @X_PRE_LIBS@ @X_LIBS@ -lX11 >> @X_EXTRA_LIBS@ should be on lib_gnu_awt_xlib_la_LDADD. >> > > This was how it was done before my change. If we want to > make a change, I can submit a separate patch. Libraries should never occur before the objects which reference them. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [C++ Patch] PR 54526 (again)
Hi, On 01/03/2013 10:56 PM, Jason Merrill wrote: On 01/03/2013 05:44 AM, Paolo Carlini wrote: + /* C++11 - 2.5 p3, bullet 2. */ Please flesh out this comment some more. Ok, I extended it like this. Thanks, Paolo. / Index: gcc/cp/parser.c === --- gcc/cp/parser.c (revision 194884) +++ gcc/cp/parser.c (working copy) @@ -1,6 +1,6 @@ /* C++ Parser. Copyright (C) 2000, 2001, 2002, 2003, 2004, - 2005, 2007, 2008, 2009, 2010, 2011, 2012 Free Software Foundation, Inc. + 2005, 2007-2013 Free Software Foundation, Inc. Written by Mark Mitchell . This file is part of GCC. @@ -12655,11 +12655,9 @@ cp_parser_template_id (cp_parser *parser, return error_mark_node; } /* Otherwise, emit an error about the invalid digraph, but continue -parsing because we got our argument list. In C++11 do not emit -any error, per 2.5/3. */ - if (cxx_dialect < cxx0x - && permerror (next_token->location, - "%<<::%> cannot begin a template-argument list")) +parsing because we got our argument list. */ + if (permerror (next_token->location, +"%<<::%> cannot begin a template-argument list")) { static bool hint = false; inform (next_token->location, Index: gcc/testsuite/g++.dg/cpp0x/parse2.C === --- gcc/testsuite/g++.dg/cpp0x/parse2.C (revision 194884) +++ gcc/testsuite/g++.dg/cpp0x/parse2.C (working copy) @@ -10,3 +10,6 @@ int main() { X<::A> x; } + +int a; +bool b = 0<::a; Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C === --- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194884) +++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy) @@ -31,5 +31,5 @@ class foo }; void foo::x() throw(bar) { - if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda expressions|expected" } parse error + if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda expressions|expected|invalid" } parse error } Index: libcpp/lex.c === --- libcpp/lex.c(revision 194884) +++ libcpp/lex.c(working copy) @@ -1,6 +1,6 @@ /* CPP Library - lexical analysis. - Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010, - 2011, 2012 Free Software Foundation, Inc. + Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2007-2013 + Free Software Foundation, Inc. Contributed by Per Bothner, 1994-95. Based on CCCP program by Paul Rubin, June 1986 Adapted to ANSI C, Richard Stallman, Jan 1987 @@ -2290,6 +2290,17 @@ _cpp_lex_direct (cpp_reader *pfile) { if (*buffer->cur == ':') { + /* C++11 [2.5/3 lex.pptoken], "Otherwise, if the next +three characters are <:: and the subsequent character +is neither : nor >, the < is treated as a preprocessor +token by itself". */ + if (CPP_OPTION (pfile, cplusplus) + && (CPP_OPTION (pfile, lang) == CLK_CXX11 + || CPP_OPTION (pfile, lang) == CLK_GNUCXX11) + && buffer->cur[1] == ':' + && buffer->cur[2] != ':' && buffer->cur[2] != '>') + break; + buffer->cur++; result->flags |= DIGRAPH; result->type = CPP_OPEN_SQUARE;
Re: [AARCH64] Optimize cmp in some cases
On 03/01/13 22:39, Andrew Pinski wrote: Hi, For aarch64, we don't CSE some cmp away. This patch fixes the case where we are CSE across some basic-blocks like: int f(int a, int b) { if(ab) return -1; return 0; } --- CUT --- To fix this, I implemented the target hook TARGET_FIXED_CONDITION_CODE_REGS as there was already code in CSE which uses this target hook to find the extra setting of the CC registers. OK? Build and tested on aarch64-thunder-elf (using Cavium's internal simulator). Build a cross to aarch64-thunder-linux-gnu and a Canadian cross with that one for the native toolchain. Thanks, Andrew Pinski * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): New function. (TARGET_FIXED_CONDITION_CODE_REGS): Define. * gcc.target/aarch64/cmp-1.c: New testcase. fixed_condition_code.diff.txt * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): New function. (TARGET_FIXED_CONDITION_CODE_REGS): Define. * gcc.target/aarch64/cmp-1.c: New testcase. Index: config/aarch64/aarch64.c === --- config/aarch64/aarch64.c(revision 194872) +++ config/aarch64/aarch64.c(working copy) @@ -3041,6 +3041,16 @@ aarch64_const_double_zero_rtx_p (rtx x) return REAL_VALUES_EQUAL (r, dconst0); } +/* Return the fixed registers used for condition codes. */ + +static bool +aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2) +{ + *p1 = CC_REGNUM; + *p2 = -1; Please use INVALID_REGNUM here (as the documentation states). Otherwise, OK. A backport to the AArch64-4.7 branch would be appreciated. R.
[Ada] Internal fix to Insert_Actions routine in compiler
This patch corrects an obvious latent bug in Insert_Actions, namely that in the case of an expression with actions node, the actions were inserted in reverse order from the calls. As far as we know, this bug is only latent (found from code reading), so no test is required for this fix. Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Robert Dewar * exp_util.ads, exp_util.adb (Insert_Actions): In expression with actions case, new actions are appended to the sequence rather than prepended. Index: exp_util.adb === --- exp_util.adb(revision 194894) +++ exp_util.adb(working copy) @@ -3138,7 +3138,7 @@ and then not Is_Frozen (Current_Scope) then if No (Scope_Stack.Table - (Scope_Stack.Last).Pending_Freeze_Actions) + (Scope_Stack.Last).Pending_Freeze_Actions) then Scope_Stack.Table (Scope_Stack.Last).Pending_Freeze_Actions := Ins_Actions; @@ -3306,13 +3306,13 @@ return; -- Case of appearing within an Expressions_With_Actions node. We --- prepend the actions to the list of actions already there, if +-- append the actions to the list of actions already there, if -- the node has not been analyzed yet. Otherwise find insertion -- location further up the tree. when N_Expression_With_Actions => if not Analyzed (P) then - Prepend_List (Ins_Actions, Actions (P)); + Append_List (Ins_Actions, Actions (P)); return; end if; Index: exp_util.ads === --- exp_util.ads(revision 194887) +++ exp_util.ads(working copy) @@ -75,6 +75,9 @@ --expansion of the N_If_Expression node rewrites the node so that the --actions can be positioned normally. + --For actions coming from expansion of the expression in an expression + --with actions node, the action is appended to the list of actions. + -- Basically what we do is to climb up to the tree looking for the -- proper insertion point, as described by one of the above cases, -- and then insert the appropriate action or actions.
[Ada] Follow on work for tagging of warning switches
Correct some missing cases of setting this switch, document it in usage. Primarily documentation, so no new test needed. Also makes clear that this switch is not available in VMS. No test needed. Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Robert Dewar * gnat_ugn.texi: Document -gnatw.d/w.D (does no apply in VMS mode). * usage.adb: Add lines for -gnatw.d/w.D switches. * warnsw.adb: Minor fixes (some missing cases of setting Warning_Doc_Switch). Reject -gnatw.d and -gnatw.D in VMS mode. Index: usage.adb === --- usage.adb (revision 194894) +++ usage.adb (working copy) @@ -474,6 +474,16 @@ Write_Line (".C* turn off warnings for unrepped components"); Write_Line ("dturn on warnings for implicit dereference"); Write_Line ("D* turn off warnings for implicit dereference"); + + -- Switches -gnatw.d/w.D not available on VMS + + if not OpenVMS_On_Target then + Write_Line +(".d turn on tagging of warnings with -gnatw switch"); + Write_Line +(".D* turn off tagging of warnings with -gnatw switch"); + end if; + Write_Line ("etreat all warnings (but not info) as errors"); Write_Line (".e turn on every optional info/warning " & "(no exceptions)"); Index: gnat_ugn.texi === --- gnat_ugn.texi (revision 194889) +++ gnat_ugn.texi (working copy) @@ -5214,6 +5214,9 @@ switch are @option{-gnatwd} (implicit dereferencing), @option{-gnatwh} (hiding), +@ifclear VMS +@option{-gnatw.d} (tag warnings with -gnatw switch) +@end ifclear @option{-gnatw.h} (holes (gaps) in record layouts) @option{-gnatw.i} (overlapping actuals), @option{-gnatw.k} (redefinition of names in standard), @@ -5362,6 +5365,24 @@ This switch suppresses warnings for implicit dereferences in indexed components, slices, and selected components. +@ifclear vms +@item -gnatw.d +@emph{Activate tagging of warning messages.} +@cindex @option{-gnatw.d} (@command{gcc}) +If this switch is set, then warning messages are tagged, either with +the string ``@option{-gnatw?}'' showing which switch controls the warning, +or with ``[enabled by default]'' if the warning is not under control of a +specific @option{-gnatw?} switch. This mode is off by default, and is not +affected by the use of @code{-gnatwa}. + +@item -gnatw.D +@emph{Deactivate tagging of warning messages.} +@cindex @option{-gnatw.d} (@command{gcc}) +If this switch is set, then warning messages return to the default +mode in which warnings are not tagged as described above for +@code{-gnatw.d}. +@end ifclear + @item -gnatwe @emph{Treat warnings and style checks as errors.} @cindex @option{-gnatwe} (@command{gcc}) Index: warnsw.adb === --- warnsw.adb (revision 194841) +++ warnsw.adb (working copy) @@ -53,11 +53,19 @@ Warn_On_Unrepped_Components := False; when 'd' => -Warning_Doc_Switch := True; +if Open_VMS_On_Target then + return False; +end if; +Warning_Doc_Switch := True; + when 'D' => -Warning_Doc_Switch := False; +if Open_VMS_On_Target then + return False; +end if; +Warning_Doc_Switch := False; + when 'e' => Address_Clause_Overlay_Warnings := True; Check_Unreferenced := True; @@ -68,6 +76,7 @@ Implementation_Unit_Warnings:= True; Ineffective_Inline_Warnings := True; List_Inherited_Aspects := True; +Warning_Doc_Switch := True; Warn_On_Ada_2005_Compatibility := True; Warn_On_Ada_2012_Compatibility := True; Warn_On_All_Unread_Out_Parameters := True; @@ -217,6 +226,7 @@ Implementation_Unit_Warnings:= False; Ineffective_Inline_Warnings := True; List_Inherited_Aspects := False; + Warning_Doc_Switch := False; Warn_On_Ada_2005_Compatibility := True; Warn_On_Ada_2012_Compatibility := True; Warn_On_All_Unread_Out_Parameters := False; @@ -296,6 +306,7 @@ Implementation_Unit_Warnings:= False; Ineffective_Inline_Warnings := False; List_Inherited_Aspects := False; +Warning_Doc_Switch := False; Warn_On_Ada_2005_Compatibility := False; Warn_On_Ada_2012_Compatibility := False; Warn_On_All_Unread_Out
[Ada] Better error message for aspect specification without Ada 2012 mode
This patch improves the error message when compiling a unit with an aspect specification in an older Ada mode, and the first aspect specification is for a boolean aspect whose value is defaulted. Compiling the following: gcc -c r1.ads must yield: r1.ads:2:16: aspect specification is an Ada 2012 feature r1.ads:2:16: unit must be compiled with -gnat2012 switch package R1 is procedure Q with Inline, Pre => False; end R1; Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Ed Schonberg * par-ch13.adb (Aspect_Specifications_Present): In Strict mode, accept an aspect name followed by a comma, indicating a defaulted boolean aspect. Index: par-ch13.adb === --- par-ch13.adb(revision 194841) +++ par-ch13.adb(working copy) @@ -105,6 +105,13 @@ if Token = Tok_Arrow then Result := True; +-- The identifier may be the name of a boolean aspect with a +-- defaulted True value. Further checks when analyzing aspect +-- specification. + +elsif Token = Tok_Comma then + Result := True; + elsif Token = Tok_Apostrophe then Scan; -- past apostrophe
[Ada] Aspect Global
This patch provides the initial implementation of aspect Global. This construct is intended for formal verification proofs. The syntax of the aspect is as follows: global_specification ::= (moded_global_list {, moded_global_list}) | global_list | null moded_global_list::= mode_selector => global_list global_list ::= global_item | (global_item {, global_item}) mode_selector::= Input | Output | In_Out | Contract_In global_item ::= name The semantics of the aspect are as follows: A global_item of a subprogram shall be a stand alone variable object [that is, it is not part of a larger object] or it shall be a state abstraction. Each mode_selector shall occur at most once in a single Global aspect. A function subprogram may not have a mode_selector of Output or In_Out in its Global aspect. Global_items in the same Global aspect shall denote distinct entities. A global item occurring in a Global aspect of a subprogram aspect specification shall not have the same defining_identifier as a formal parameter of the subprogram. A global item of mode in out or out shall not be a Volatile Input state abstraction (see Abstract State Aspect). A global item of mode in or in out shall not be a Volatile Output state abstraction. This patch also corrects the timing of pragma Abstract_State analysis. -- Source -- -- semantics.ads package Semantics with Abstract_State => ((Input_State with Volatile, Input), (Output_State with Volatile, Output)) is type Composite is record Comp : Integer; end record; Var : Composite; type Composite_Array is array (1 .. 5) of Composite; Arr : Composite_Array; procedure Dummy_Proc; procedure OK_1 with Global => (Var, Input_State); procedure Error_1 with Global => (Var.Comp, Arr (2), Dummy_Proc); procedure OK_2 with Global => (Input => (Var, Input_State), Output => Arr); procedure Error_2 with Global => (Input => Var, Contract_In => Input_State, Input => Arr); function OK_3 return Boolean with Global => (Input => Var, Contract_In => Input_State); function Error_3 return Boolean with Global => (In_Out => Arr, Output => Var); procedure Error_4 with Global => (Input => Semantics.Var, Output => Var); function Error_5 (Formal : Boolean) return Boolean with Global => Formal; procedure Error_6 with Global => (In_Out => Input_State); procedure Error_7 with Global => (Output => Input_State); procedure Error_8 with Global => (In_Out => Output_State); procedure Error_9 with Global => (Input => Output_State); procedure Error_10 with Global => Output_State; procedure OK_4 with Global => null; procedure Error_11 with Global => (null, Var); procedure Error_12 with Global => (Var, null, Arr); procedure Error_13 with Global => (Var, null); end Semantics; -- Compilation and output -- $ gcc -c -gnat12 -gnatd.V semantics.ads semantics.ads:19:12: global item must denote variable or state semantics.ads:20:09: global item must denote variable or state semantics.ads:21:09: global item must denote variable or state semantics.ads:30:09: duplicate global mode semantics.ads:37:09: global mode "In_Out" not applicable to functions semantics.ads:38:09: global mode "Output" not applicable to functions semantics.ads:42:19: duplicate global item semantics.ads:44:21: global item cannot reference formal parameter semantics.ads:46:32: global item of mode In_Out or Output cannot reference Volatile Input state semantics.ads:48:32: global item of mode In_Out or Output cannot reference Volatile Input state semantics.ads:50:32: global item of mode In_Out or Input cannot reference Volatile Output state semantics.ads:52:31: global item of mode In_Out or Input cannot reference Volatile Output state semantics.ads:54:21: global item of mode In_Out or Input cannot reference Volatile Output state semantics.ads:58:22: cannot mix null and non-null global items semantics.ads:60:27: cannot mix null and non-null global items semantics.ads:62:27: cannot mix null and non-null global items Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Hristian Kirtchev * aspects.adb, aspects.ads: Add Aspect_Global to all relevant tables. * par-prag.adb: Add pragma Global to the list of pragmas that do not need special processing by the parser. * sem_ch13.adb (Analyze_Aspect_Specifications): Convert aspect Global into a pragma without any form of legality checks. The work is done by Analyze_Pragma. The aspect and pragma are both marked as needing delayed processing. Inser
[Ada] Attribute Loop_Entry and index check generation
This patch delays the generation of index checks for the following cases of Loop_Entry attribute references: Prefix'Loop_Entry (Expr) Prefix'Loop_Entry (Expr1, Expr2, ... ExprN) Even though these constructs appear initially as attribute references, analysis converts them into indexed components to reflect their true semantics. Without this patch, expansion of the indexed components would generate index checks of the following form [constraint_error when not (blah in a'loop_entry'first .. a'loop_entry'last) "index check failed"] and the back end would subsequently fail because it cannot process Loop_Entry. Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Hristian Kirtchev * checks.adb (Generate_Index_Checks): Delay the generation of the check for an indexed component where the prefix mentions Loop_Entry until the attribute has been properly expanded. * exp_ch5.adb (Expand_Loop_Entry_Attributes): Perform minor decoration of the constant that captures the value of Loop_Entry's prefix at the entry point into a loop. Generate index checks for an attribute reference that has been transformed into an indexed component. Index: exp_ch5.adb === --- exp_ch5.adb (revision 194841) +++ exp_ch5.adb (working copy) @@ -1828,11 +1828,29 @@ Object_Definition => New_Reference_To (Typ, Loc), Expression => Relocate_Node (Prefix (LE; + -- Perform minor decoration as this information will be needed for + -- the creation of index checks (if applicable). + + Set_Ekind (Temp, E_Constant); + Set_Etype (Temp, Typ); + -- Replace the original attribute with a reference to the constant Rewrite (LE, New_Reference_To (Temp, Loc)); Set_Etype (LE, Typ); + -- Analysis converts attribute references of the following form + + -- Prefix'Loop_Entry (Expr) + -- Prefix'Loop_Entry (Expr1, Expr2, ... ExprN) + + -- into indexed components for error detection purposes. Generate + -- index checks now that 'Loop_Entry has been properly expanded. + + if Nkind (Parent (LE)) = N_Indexed_Component then +Generate_Index_Checks (Parent (LE)); + end if; + Next_Elmt (LE_Elmt); end loop; Index: checks.adb === --- checks.adb (revision 194848) +++ checks.adb (working copy) @@ -5522,6 +5522,23 @@ or else Index_Checks_Suppressed (Etype (A)) then return; + + -- The indexed component we are dealing with contains 'Loop_Entry in its + -- prefix. This case arises when analysis has determined that constructs + -- such as + + -- Prefix'Loop_Entry (Expr) + -- Prefix'Loop_Entry (Expr1, Expr2, ... ExprN) + + -- require rewriting for error detection purposes. A side effect of this + -- action is the generation of index checks that mention 'Loop_Entry. + -- Delay the generation of the check until 'Loop_Entry has been properly + -- expanded. This is done in Expand_Loop_Entry_Attributes. + + elsif Nkind (Prefix (N)) = N_Attribute_Reference +and then Attribute_Name (Prefix (N)) = Name_Loop_Entry + then + return; end if; -- Generate a raise of constraint error with the appropriate reason and