Re: [PATCH v3] Updated the Fix:
Jason Merrill wrote: On 11/3/19 6:42 AM, Kamlesh Kumar wrote: ChangeLog Entries: --- 2019-11-02 Kamlesh Kumar * cp-demangle.c (d_expr_primary): Handle nullptr demangling. * testsuite/demangle-expected: Added test. This test is failing in at least some places (x86_64-darwin, x86_64-linux) x86_64-apple-darwin16-gcc -DHAVE_CONFIG_H -g -O2 -I.. -I/src-local/gcc-master/libiberty/testsuite/../../include -o test-demangle \ /src-local/gcc-master/libiberty/testsuite/test-demangle.c ../libiberty.a ./test-demangle < /src-local/gcc-master/libiberty/testsuite/demangle-expected FAIL at line 1452, options : in: _Z3fooILPv0EEvPN9enable_ifIXeqT_LDnEEvE4typeE out: void foo<(void*)0>(enable_if<((void*)0)==(decltype(nullptr)), void>::type*) exp: void foo<(void*)0>(enable_if<((void*)0)==((decltype(nullptr))), void>::type*) ./test-demangle: 335 tests, 1 failures Is the extra set of parentheses significant in your expected output? if so, then the demangling is not working … … if not, we should amend the test since it causes early failure of libiberty tests (I guess this goes unnoticed, because they are not integrated into the headline GCC regression tests). thanks Iain
RE: [RFC c-common PATCH] PR c++/40752 - useless -Wconversion with short +=.
Hi Jason, The commit Shorten right-shift again in C++. Back in SVN r131862 richi removed this code to fix PR 34235, but didn't remove the parallel code from the C front-end because the bug had previously been fixed in r44080. This patch copies the code from C again. * typeck.c (cp_build_binary_op): Restore short_shift code. From-SVN: r280128 Is causing ICEs in SPEC2017's leela. internal compiler error: tree check: expected integer_cst, have mult_expr in to_wide, at tree.h:5860 368 | int allblack = ((m_neighbours[i] >> (NBR_SHIFT * BLACK)) & 7) == 4; | ^ 0x6565d3 tree_check_failed(tree_node const*, char const*, int, char const*, ...) ../.././gcc/tree.c:9685 0x11d3d33 tree_int_cst_elt_check(tree_node*, int, char const*, int, char const*) ../.././gcc/tree.h:3478 0x11d3d33 tree_int_cst_elt_check(tree_node const*, int, char const*, int, char const*) ../.././gcc/tree.h:3474 0x11d3d33 wi::to_wide(tree_node const*) ../.././gcc/tree.h:5860 0x11d3d33 tree_int_cst_sgn(tree_node const*) ../.././gcc/tree.c:7382 0x90ad8f cp_build_binary_op(op_location_t const&, tree_code, tree_node*, tree_node*, int) ../.././gcc/cp/typeck.c:5609 0x681547 build_new_op_1 ../.././gcc/cp/call.c:6475 0x681fdf build_new_op(op_location_t const&, tree_code, int, tree_node*, tree_node*, tree_node*, tree_node**, int) ../.././gcc/cp/call.c:6520 0x8f960f build_x_binary_op(op_location_t const&, tree_code, tree_node*, tree_code, tree_node*, tree_code, tree_node**, int) ../.././gcc/cp/typeck.c:4245 0x7dc7cf cp_parser_binary_expression ../.././gcc/cp/parser.c:9648 0x7ddb1f cp_parser_assignment_expression ../.././gcc/cp/parser.c:9783 0x7dde3b cp_parser_expression ../.././gcc/cp/parser.c:9951 0x7f095f cp_parser_primary_expression ../.././gcc/cp/parser.c:5351 0x7fdefb cp_parser_postfix_expression ../.././gcc/cp/parser.c:7203 0x7dc607 cp_parser_binary_expression ../.././gcc/cp/parser.c:9483 0x7ddb1f cp_parser_assignment_expression ../.././gcc/cp/parser.c:9783 0x7dde3b cp_parser_expression ../.././gcc/cp/parser.c:9951 0x7f095f cp_parser_primary_expression ../.././gcc/cp/parser.c:5351 0x7fdefb cp_parser_postfix_expression ../.././gcc/cp/parser.c:7203 0x7dc607 cp_parser_binary_expression ../.././gcc/cp/parser.c:9483 Thanks, Tamar > -Original Message- > From: gcc-patches-ow...@gcc.gnu.org > On Behalf Of Jason Merrill > Sent: Friday, January 10, 2020 6:15 PM > To: gcc-patches List ; Joseph S. Myers > > Cc: Manuel López-Ibáñez > Subject: [RFC c-common PATCH] PR c++/40752 - useless -Wconversion with > short +=. > > Back in 2009 Manuel sent a patch to avoid useless -Wconversion warnings on > compound assignment of types that get promoted to int: > > https://gcc.gnu.org/ml/gcc-patches/2009-08/msg00582.html > > Joseph argued that those warnings are sometimes useful, and that they > should be controlled by a separate flag. So this patch introduces -Warith- > conversion, which is off by default in this patch. > > Joseph, is that default OK with you? If not, can you explain your reasoning > more about why the warnings are desirable? It seems to me that even in > cases where you don't know the ranges involved, it's wrong for e.g. 'mychar > += 1' to warn when 'myint += 1' doesn't. In both cases, the addition might > overflow the range of the target type, but that isn't about the conversion; > the result is the same as if the operation had been done in the > operand/target type rather than the promoted type. > > The change to cp/typeck.c is a placeholder to use if the default setting > changes; even if we end up warning by default for mychar = mychar + 1, I > really don't want to warn about mychar += 1. > > sign.diff is a prerequisite tidying that moves the warnings from > unsafe_conversion_p back into conversion_warning with the others. > > rshift.diff restores the short_shift code to the C++ front end so that C and > C++ give the same warnings with -Warith-conversion. > > Tested x86_64-pc-linux-gnu. Comments?
Re: [wwwdocs] Git transition - how to access private user and vendor branches
Segher Boessenkool wrote: > Hi Richard, > > On Thu, Jan 09, 2020 at 04:50:03PM +, Richard Earnshaw (lists) wrote: >> Given the proposed intention to use non-standard refspecs for private >> and vendor branches I've written some notes on how to use these. >> >> It would be helpful if someone could do some experiments to ensure that >> what I've written works properly for all versions of git, not just the >> one I have installed locally. > > I haven't been able to test it yet, but it does look like it will work. > >> +To fetch any of these you will need to add a fetch refspec to your >> +configuration. For example, to fetch all the IBM vendor branches add to >> +your default upstream pull >> + >> + >> +git config --add remote.origin.fetch >> "+refs/vendors/IBM/heads/*:refs/remotes/origin/IBM/*" >> +git config --add remote.origin.fetch >> "+refs/vendors/IBM/tags/*:refs/tags/IBM/*" >> + > > It may help to show the resulting config file instead of (or in addition > to) git-config commands to manipulate that. The config file is (or can > be) much more readable than those commands (shorter lines, less quoting). > You can also organise it, put in comments, and even *debug it*! And > *understand it* in general! Wow, what a concept :-) > >> + >> +git checkout -b ARM/arm-9-branch origin/ARM/arm-9-branch >> + >> + >> +then change the merge property for the branch to corectly identify the > > (typo) > >> +upstream source >> + >> + >> +git config branch.ARM/arm-9-branch.merge refs/vendors/ARM/heads/arm-9-branch >> + >> + >> +Pull operations will now correctly track the upstream branch. > > Do you also need to set branch..remote? Or is that done correctly > already? Or is that not needed if you have only one remote? > >> +It is also possible to set up push operations so that local changes will be >> pushed to the private namespace. For example, if you mirror your own >> private git information with >> + >> + >> +git config --add remote.origin.fetch >> "+refs/users/my-userid/heads/*:refs/remotes/origin/me/*" >> + >> + >> +then you can also add >> + >> +git config --add remote.origin.push >> "+refs/heads/me/*:refs/users/my-userid/heads/*" >> + >> +and then any push from a branch that begins with me/ will be >> pushed to the private area. > > Will be *force-pushed* to the server. This may not be expected or > wanted. > > Why would people want to name their local branches "me/thing" instead > of just "thing", btw? it’s a way of making things distinct and allows the push rule to be present for them but absent for more dangerous pushes. So if one renames origin to something else e.g. fsf or upstream, and there are no automatic push rules, it’s one more small protection against an accidental push? > You could just use > > push = thing:users//heads/thing > > (no "+", I don't rebase the "thing" branch). Hrm, and maybe make an > alias to push a local branch to the server the first time... Completely > untested: > > [alias] > new-user-branch = !git push $1:users/myname/heads/$1 > > (And yes, I know this doesn't work as written if you have tag names > the same as branch names. That *deserves* punishment :-) ) > > > Segher
Re: Add rough documentation of using git with GCC
On Fri, 10 Jan 2020, Joseph Myers wrote: > And we should also mention configuring your email address for git, if you > haven't used git on that system before or the default address you've > configured for git isn't the one you want for GCC commits. I've now applied this patch to document that. diff --git a/htdocs/gitwrite.html b/htdocs/gitwrite.html index 94449e30..9a460a36 100644 --- a/htdocs/gitwrite.html +++ b/htdocs/gitwrite.html @@ -93,6 +93,19 @@ Host gcc.gnu.org ForwardX11 no +Git needs to know your name and email address. If you have not +already configured those in $HOME/.gitconfig, do: + + +git config --global user.name "Your Name" +git config --global user.email "Your Email Address" + + +If you wish to use a different name or email address for GCC +commits from that in $HOME/.gitconfig, you can configure +that in an individual Git tree using a similar command +without --global. + Write access policies -- Joseph S. Myers jos...@codesourcery.com
[patch, libgfortran] PR90374 Zero width format specifiers.
Hi all,
The attached patch is simple enough. Zero width exponent format
specifier is not allowed with "D" editing.
Also, needed to adjust the width calculated for exponents greater than
999 for the larger kind cases (real 10 and real 16).
Regression tested on x86_64. I will commit as soon as I include in the
test case.
Regards,
Jerry
2020-01-12 Jerry DeLisle
PR libfortran/90374
* io/format.c (parse_format_list): Zero width not allowed with
FMT_D.
* io/write_float.def (build_float_string): Include range of
higher exponent values that require wider width.
diff --git a/libgfortran/io/format.c b/libgfortran/io/format.c
index b42a5593e38..3be861fb19c 100644
--- a/libgfortran/io/format.c
+++ b/libgfortran/io/format.c
@@ -954,7 +954,9 @@ parse_format_list (st_parameter_dt *dtp, bool *seen_dd)
}
tail->u.real.d = fmt->value;
- /* Look for optional exponent */
+ /* Look for optional exponent, not allowed for FMT_D */
+ if (t == FMT_D)
+ break;
u = format_lex (fmt);
if (u != FMT_E)
fmt->saved_token = u;
diff --git a/libgfortran/io/write_float.def b/libgfortran/io/write_float.def
index 75c7942c4c5..8a1be054371 100644
--- a/libgfortran/io/write_float.def
+++ b/libgfortran/io/write_float.def
@@ -497,7 +497,9 @@ build_float_string (st_parameter_dt *dtp, const fnode *f, char *buffer,
else if (f->u.real.e == 0)
{
/* Zero width specified, no leading zeros in exponent */
- if (e > 99 || e < -99)
+ if (e > 999 || e < -999)
+ edigits = 6;
+ else if (e > 99 || e < -99)
edigits = 5;
else if (e > 9 || e < -9)
edigits = 4;
[PATCH] Document configuring your name and email address for git.
[PATCH] Document how to use --reference
From: Andi Kleen By popular demand, I did some updates to the git documentation how to use --reference to save disk space. Also recommend https instead of http (even though both are currently broken) --- htdocs/git.html | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/htdocs/git.html b/htdocs/git.html index 0166ff74..41e2d953 100644 --- a/htdocs/git.html +++ b/htdocs/git.html @@ -43,9 +43,18 @@ check out the GCC sources using the following command: If you are behind a firewall that does not allow the git protocol -through, you can replace git:// with http://. -You should only use the http protocol if -the git protocol does not work; the http protocol has a higher server +through, you can replace git:// with https://. + +When doing multiple clones to different repositories you can avoid +redownloading the whole repository by using --reference. +For example +git clone --reference original-gcc ssh://gcc.gnu.org/git.gcc.git new-gcc +This will also save some disk space. Git will do this automatically when cloning from a local repository on the same file system. It is also possible to do a +shallow checkout with --depth to limit history, but that might limit your +ability to work with existing branches. + +You should only use the https protocol if +the git protocol does not work; the https protocol has a higher server overhead associated with it and will be slower.
Re: [PATCH] Document how to use --reference
On Jan 12 2020, Andi Kleen wrote: > By popular demand, I did some updates to the git documentation how > to use --reference to save disk space. I would advice against using --reference. Nowadays, git worktree is a much safer option. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH] doc: Note that some warnings depend on optimizations (PR 92757)
On Mon, 2 Dec 2019, Jonathan Wakely wrote: > PR driver/92757 > * doc/invoke.texi (Warning Options): Add caveat about some warnings > depending on optimization settings. > > The bug reporter wants this clarified. I'm not entirely convinced it's > necessary, but it doesn't seem to do any harm. > > OK for trunk? Yes, thank you. (I can see how this can help, and even if it only helps one in twenty users, say, that's still worthwhile.) Gerald
Re: [wwwdocs] Git transition - how to access private user and vendor branches
On 1/11/20 10:54 AM, Richard Earnshaw (lists) wrote: On 11/01/2020 15:41, Segher Boessenkool wrote: Hi Richard, On Thu, Jan 09, 2020 at 04:50:03PM +, Richard Earnshaw (lists) wrote: Given the proposed intention to use non-standard refspecs for private and vendor branches I've written some notes on how to use these. It would be helpful if someone could do some experiments to ensure that what I've written works properly for all versions of git, not just the one I have installed locally. I haven't been able to test it yet, but it does look like it will work. +To fetch any of these you will need to add a fetch refspec to your +configuration. For example, to fetch all the IBM vendor branches add to +your default upstream pull + + +git config --add remote.origin.fetch "+refs/vendors/IBM/heads/*:refs/remotes/origin/IBM/*" +git config --add remote.origin.fetch "+refs/vendors/IBM/tags/*:refs/tags/IBM/*" + It may help to show the resulting config file instead of (or in addition to) git-config commands to manipulate that. The config file is (or can be) much more readable than those commands (shorter lines, less quoting). You can also organise it, put in comments, and even *debug it*! And *understand it* in general! Wow, what a concept :-) I wanted to describe it in the 'official' git way. Tweaking the contents of your .git/config file is not really sanctioned, even though we all do it. :-) + +git checkout -b ARM/arm-9-branch origin/ARM/arm-9-branch + + +then change the merge property for the branch to corectly identify the (typo) Already fixed, didn't seem like it was worth reposting just for that. +upstream source + + +git config branch.ARM/arm-9-branch.merge refs/vendors/ARM/heads/arm-9-branch + + +Pull operations will now correctly track the upstream branch. Do you also need to set branch..remote? Or is that done correctly already? Or is that not needed if you have only one remote? Yes. However, on reflection, I'm not sure about that bit anyway. I may remote it entirely. +It is also possible to set up push operations so that local changes will be pushed to the private namespace. For example, if you mirror your own private git information with + + +git config --add remote.origin.fetch "+refs/users/my-userid/heads/*:refs/remotes/origin/me/*" + + +then you can also add + +git config --add remote.origin.push "+refs/heads/me/*:refs/users/my-userid/heads/*" + +and then any push from a branch that begins with me/ will be pushed to the private area. Will be *force-pushed* to the server. This may not be expected or wanted. Why would people want to name their local branches "me/thing" instead of just "thing", btw? You could just use push = thing:users//heads/thing You could do that instead, but then you need a push line for every branch. The me/ trick means that anything named me/* will automatically get sent to the right place if pushed upstream. I'd go with +refs/heads/*:refs/users//heads/* and then use git push origin mybranch I'm unlikely to want to push multiple branches at once. Jason
Re: [PATCH] Document how to use --reference
On Sun, 12 Jan 2020, Andi Kleen wrote: > By popular demand, I did some updates to the git documentation how > to use --reference to save disk space. > > Also recommend https instead of http (even though both are currently > broken) Thank you, Andi. Why https over http in this case? Isn't git itself cryptographically secured? To avoid man in the middle injections via an "alternate" repository presented? Below is a minor update on top of yours that I applied. Hope that's fine. Gerald - Log - commit 8a488efcc0f4265505205aef285ae4a7537ff390 Author: Gerald Pfeifer Date: Sun Jan 12 21:43:59 2020 +0100 Refine the note on local clones, add markup, break a long line. diff --git a/htdocs/git.html b/htdocs/git.html index 41e2d953..7f40c0f6 100644 --- a/htdocs/git.html +++ b/htdocs/git.html @@ -45,17 +45,18 @@ check out the GCC sources using the following command: If you are behind a firewall that does not allow the git protocol through, you can replace git:// with https://. -When doing multiple clones to different repositories you can avoid v-redownloading the whole repository by using --reference. -For example +When doing multiple clones to several local repositories you can avoid +re-downloading everything by using --reference, e.g. + git clone --reference original-gcc ssh://gcc.gnu.org/git.gcc.git new-gcc -This will also save some disk space. Git will do this automatically when cloning from a local repository on the same file system. It is also possible to do a -shallow checkout with --depth to limit history, but that might limit your -ability to work with existing branches. -You should only use the https protocol if -the git protocol does not work; the https protocol has a higher server -overhead associated with it and will be slower. +This will also save disk space. Git will do this automatically when cloning +from a local repository on the same file system. It is also possible to do a +shallow checkout with --depth to limit history, but that might +limit your ability to work with existing branches. + +(Only use the https protocol if the git protocol does not work; https has +a higher server overhead and will be slower.)
Re: [wwwdocs] Add AVR news.
On Fri, 10 Jan 2020, Georg-Johann Lay wrote: > Added the following change to the v10 changes site. Thank you. I applied the patch below to address some issues; please let me know if you have any questions or disagree with any of it. Gerald - Log - commit d65a8b4f8d4d625070b3f8cbb07b13dfe1116ae6 Author: Gerald Pfeifer Date: Sun Jan 12 22:00:38 2020 +0100 Editorial changes to the AVR news section. Use "command-line option" over "command line option". Make references to older versions of GCC consistent. Improve spelling and grammar. diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html index c68e00be..caa9df70 100644 --- a/htdocs/gcc-10/changes.html +++ b/htdocs/gcc-10/changes.html @@ -432,7 +432,7 @@ a work-in-progress. has been added. -A new command line option -nodevicespecs has been added. +A new command-line option -nodevicespecs has been added. It allows to provide a custom device-specs file by means of avr-gcc -nodevicespecs -specs=my-spec-file@@ -441,17 +441,17 @@ a work-in-progress. -mmcu=. See https://gcc.gnu.org/onlinedocs/gcc/AVR-Options.html#index-nodevicespecs";>AVR - command line options for details. -This feature is also available in v9.3+ and v8.4+. + command-line options for details. +This feature is also available in GCC 9.3+ and GCC 8.4+. -New command line options -mdouble=[32,64] and +New command-line options -mdouble=[32,64] and -mlong-double=[32,64] have been added. They allow -to chose the size (in bits) of the double and +to choose the size (in bits) of the double and long double types, respectively. Whether or not the mentioned layouts are available, whether the options act -as a multilib option, and what is the default for either option -is controlled by the new +as a multilib option, and the default for either option +are controlled by the new https://gcc.gnu.org/install/configure.html#avr";>AVR configure options --with-double= and --with-long-double=.
Re: [wwwdocs] Git transition - how to access private user and vendor branches
On Sun, Jan 12, 2020 at 01:31:13PM +, Iain Sandoe wrote: > Segher Boessenkool wrote: > > Why would people want to name their local branches "me/thing" instead > > of just "thing", btw? > > it’s a way of making things distinct and allows the push rule to be present > for them > but absent for more dangerous pushes. That's a weird setting imo. Potentially destroying your own work *is* dangerous :-) Pretty much anything you mess up locally in Git can be easily restored. Restoring remote branches can be much harder. To start with, this requires knowing *what* to restore, which can require direct access to the remote repository, or its backups. So doing an unexpected non-ff push is probably not a good idea. You can also add a "+" manually when you want to overwrite the remote branch, or configure your setup to always do that for certain branches. It all depends on personal preference and work habits, of course. But I think it isn't the best idea to recommend people take up dangerous habits :-) > So if one renames origin to something else > e.g. fsf or upstream, and there are no automatic push rules, it’s one more > small > protection against an accidental push? If you haven't configured push rules for your remote, you get what you have in "push.default" for that remote. Since Git 2.0 the default has been "push.default = simple", and no non-ff pushes are allowed by default anyway? I guess it makes some sense to group together locally the branches you have in users/ on our shared server. But then "me/" is not a great name :-) Segher
Re: [PATCH] Document how to use --reference
On Sun, Jan 12, 2020 at 08:51:28PM +0100, Andreas Schwab wrote: > On Jan 12 2020, Andi Kleen wrote: > > > By popular demand, I did some updates to the git documentation how > > to use --reference to save disk space. > > I would advice against using --reference. Nowadays, git worktree is a > much safer option. Feel free to do further edits. The file probably needs further work anyways. -Andi
Re: [PATCH] Document how to use --reference
On Sun, Jan 12, 2020 at 09:51:22PM +0100, Gerald Pfeifer wrote: > On Sun, 12 Jan 2020, Andi Kleen wrote: > > By popular demand, I did some updates to the git documentation how > > to use --reference to save disk space. > > > > Also recommend https instead of http (even though both are currently > > broken) > > Thank you, Andi. Why https over http in this case? Isn't git itself > cryptographically secured? To avoid man in the middle injections via > an "alternate" repository presented? git is secure once you have the hashes, but I don't think the protocol itself is secure. So a MITM could give you some bogus hashes. > > Below is a minor update on top of yours that I applied. Hope that's > fine. Thanks. -Andi
Re: [PATCH GCC11]Improve uninitialized warning with value range info
-- Sender:Richard Biener Sent At:2020 Jan. 9 (Thu.) 20:01 Recipient:Bin.Cheng Cc:bin.cheng ; GCC Patches Subject:Re: [PATCH GCC11]Improve uninitialized warning with value range info On Thu, Jan 9, 2020 at 11:17 AM Bin.Cheng wrote: > > I am not quite follow here. Do you mean we collect three cases "i < > > j", "i < min(j)", "max(i) < j" then > > call prune_uninit_phi_opnds for all three conditions? > > No, I've meant to somehow arrange that the 'preds' passed to > use_pred_not_overlap_with_undef_path_pred contain all three predicates > rather than just i < j, thus "expand" fully symbolic predicates. Seems this would require non-trivial refactoring of the original code. > > This is another question? because now we simply break out of for loop > > for finding such condition: > > > > - if ((gimple_code (flag_def) == GIMPLE_PHI) > > - && (gimple_bb (flag_def) == gimple_bb (phi)) > > - && find_matching_predicate_in_rest_chains (the_pred, preds, > > -num_preds)) > > - break; > > > > It's always possible that this flag_def can't prune use predicates > > against undefined path predicates, while a later one can prune but is > > skipped? > > I don't follow but I also don't want to understand the code too much ;) > > I'm fine with the idea and if the patch cannot introudce extra bogus warnings > let's go with it. Can you amed the comment before the two find_var_cmp_const > calls? I wonder whether eliding the second sweep when the first didn't find > any predicate it skipped is worth the trouble. Thanks for the comments, I updated the patch as attached. Thanks, bin 2020-01-08 Bin Cheng * tree-ssa-uninit.c (find_var_cmp_const): New function. (use_pred_not_overlap_with_undef_path_pred): Call above. (find_matching_predicate_in_rest_chains): Remove param. 0001-Fix-false-uninitialized-warning-message.patch Description: Binary data
Re: [PATCH v7] Missed function specialization + partial devirtualization
On 2020/1/10 19:08, Jan Hubicka wrote: > OK. You will need to do the obvious updates for Martin's patch > which turned some member functions into static functions. > > Honza Thanks a lot! Rebased & updated, will commit below patch shortly when git push is ready. v8: 1. Rebase to master with Martin's static function (r280043) comments merge. Boostrap/testsuite/SPEC2017 tested pass on Power8-LE. 2. TODO: 2.1. C++ devirt for multiple speculative call targets. 2.2. ipa-icf ipa_merge_profiles refine with COMDAT inline testcase. This patch aims to fix PR69678 caused by PGO indirect call profiling performance issues. The bug that profiling data is never working was fixed by Martin's pull back of topN patches, performance got GEOMEAN ~1% improvement(+24% for 511.povray_r specifically). Still, currently the default profile only generates SINGLE indirect target that called more than 75%. This patch leverages MULTIPLE indirect targets use in LTO-WPA and LTO-LTRANS stage, as a result, function specialization, profiling, partial devirtualization, inlining and cloning could be done successfully based on it. Performance can get improved from 0.70 sec to 0.38 sec on simple tests. Details are: 1. PGO with topn is enabled by default now, but only one indirect target edge will be generated in ipa-profile pass, so add variables to enable multiple speculative edges through passes, speculative_id will record the direct edge index bind to the indirect edge, indirect_call_targets length records how many direct edges owned by the indirect edge, postpone gimple_ic to ipa-profile like default as inline pass will decide whether it is benefit to transform indirect call. 2. Use speculative_id to track and search the reference node matched with the direct edge's callee for multiple targets. Actually, it is the caller's responsibility to handle the direct edges mapped to same indirect edge. speculative_call_info will return one of the direct edge specified, this will leverage current IPA edge process framework mostly. 3. Enable LTO WPA/LTRANS stage multiple indirect call targets analysis for profile full support in ipa passes and cgraph_edge functions. speculative_id can be set by make_speculative id when multiple targets are binded to one indirect edge, and cloned if new edge is cloned. speculative_id is streamed out and stream int by lto like lto_stmt_uid. 4. Create and duplicate all speculative direct edge's call summary in ipa-fnsummary.c with auto_vec. 5. Add 1 in module testcase and 2 cross module testcases. 6. Bootstrap and regression test passed on Power8-LE. No function and performance regression for SPEC2017. gcc/ChangeLog 2020-01-13 Xiong Hu Luo PR ipa/69678 * cgraph.c (symbol_table::create_edge): Init speculative_id and target_prob. (cgraph_edge::make_speculative): Add param for setting speculative_id and target_prob. (cgraph_edge::speculative_call_info): Update comments and find reference by speculative_id for multiple indirect targets. (cgraph_edge::resolve_speculation): Decrease the speculations for indirect edge, drop it's speculative if not direct target left. Update comments. (cgraph_edge::redirect_call_stmt_to_callee): Likewise. (cgraph_node::dump): Print num_speculative_call_targets. (cgraph_node::verify_node): Don't report error if speculative edge not include statement. (cgraph_edge::num_speculative_call_targets_p): New function. * cgraph.h (int common_target_id): Remove. (int common_target_probability): Remove. (num_speculative_call_targets): New variable. (make_speculative): Add param for setting speculative_id. (cgraph_edge::num_speculative_call_targets_p): New declare. (target_prob): New variable. (speculative_id): New variable. * ipa-fnsummary.c (analyze_function_body): Create and duplicate call summaries for multiple speculative call targets. * cgraphclones.c (cgraph_node::create_clone): Clone speculative_id. * ipa-profile.c (struct speculative_call_target): New struct. (class speculative_call_summary): New class. (class speculative_call_summaries): New class. (call_sums): New variable. (ipa_profile_generate_summary): Generate indirect multiple targets summaries. (ipa_profile_write_edge_summary): New function. (ipa_profile_write_summary): Stream out indirect multiple targets summaries. (ipa_profile_dump_all_summaries): New function. (ipa_profile_read_edge_summary): New function. (ipa_profile_read_summary_section): New function. (ipa_profile_read_summary): Stream in indirect multiple targets summaries. (ipa_profile): Generate num_speculative_call_targets from profile summaries. * ipa-ref.h (speculative_id): Ne
[PATCH] Fix typo and avoid possible memory leak
Hi,
Function average_num_loop_insns forgets to free loop body in early return.
Besides, overflow comparison checks 100 (e6) but the return value is
10 (e5), I guess it's unexpected, a typo?
Bootstrapped and regress tested on powerpc64le-linux-gnu.
I guess this should go to GCC11? Is it ok?
BR,
Kewen
gcc/ChangeLog
2020-01-13 Kewen Lin
* cfgloopanal.c (average_num_loop_insns): Free bbs when early return,
fix typo on return value.
diff --git a/gcc/cfgloopanal.c b/gcc/cfgloopanal.c
index 199c20b..65d239a 100644
--- a/gcc/cfgloopanal.c
+++ b/gcc/cfgloopanal.c
@@ -219,7 +219,10 @@ average_num_loop_insns (const class loop *loop)
ninsns += (sreal)binsns * bb->count.to_sreal_scale (loop->header->count);
/* Avoid overflows. */
if (ninsns > 100)
- return 10;
+ {
+ free (bbs);
+ return 100;
+ }
}
free (bbs);
