When compared to gcc-4.6.2, gcc-4.7.1 shows about a 10-20% performance regression when inserting into a google dense hashmap in a tight loop. A small sample program is attached and was compiled with '-O3' and on a linux x86_64 box (using -std=gnu++0x, as required by sparsehash). Potentially related, the generated binary under gcc-4.7 is also over 50% bigger.
Created attachment 29561 [details] Sample program using dense hashmap
I also see this same perf regression on trunk (as of r195725).
Instead of expecting everyone who looks at this bug to find and download the dense hash map code please provide preprocessed source as requested in the bug reporting guidelines that you were prominently asked to read before you entered this bug report: http://gcc.gnu.org/bugs/
Created attachment 29565 [details] Preprocessed test program source
Ah, sorry. Just attached preprocessed sources now (which had to be compressed to it in the 1K size limit).
thanks
I was able to bisect this to http://gcc.gnu.org/viewcvs?view=revision&revision=172430. I haven't had time to dig in further, but that rev looks like it has plenty of inlining changes, which makes sense.
Thanks for bisecting it
Created attachment 29864 [details] Simple test case Here's an even simpler test case. It seems that there's a fairly serious regression with inlining. Even adding the attribute always_inline doesn't seem to restore performance. However, manually inlining the code with a macro restores performance to gcc 4.6 level
I did several attempts at bisecting this. Whenever I (hackily) reverted the change which caused the regression, it just popped up a few hundred revs later. This seems to be gcc assigning new weights to functions, which determines whether the functions is early inlined or not. So I *think* this is really an early inlining v. late inlining issue, and we happened to get lucky with the weights that 4.6 selected (I don't think there is a really effective way for gcc to predict how subsequent optimizations will/can benefit by early inlining). Also, as far as I can see, there is no way to force early inlining (other than switching to macros).
The reduced testcase has been slowed down by r176072, aka PR45437. If you want to speed it up by source code changes, q += (unsigned int) f(i); would do instead of q += f(i);, or you can do q = q + f(i);. Before that bugfix, the C++ FE produced (void) (q = (int) ((unsigned int) f (i) + (unsigned int) q)) which is generally wrong (say if q was addressable and f could modify it), after it we generate (void) (q = TARGET_EXPR <D.2078, f (i)>;, (int) ((long unsigned int) q + D.2078);) The question is why the narrowing of the operation (to be done on unsigned int rather than long unsigned int) isn't performed here, that is something we do right now solely in the FE. There are PRs (PR45397 and PR47477) about lack of type demotion (and promotion) on GIMPLE, but don't know what the current state of that is.
Thank you, Jakub.
Created attachment 30312 [details] gcc49-pr56493.patch Untested patch that in the FE restores the performance on the simplified testcase. Haven't tried the original. Of course, having this optimization be performed in GIMPLE will be much more worthwhile, but likely not backportable.
According to Comment #0 and Comment #9, this PR should be confirmed as a regression from 4.6.
We are using that #c13 patch successfully for almost 18 months now in Fedora. As type promotion/demotion patch is not (sadly) going to happen for GCC 5, I still think it would be worthwhile to apply it temporarily, until type promotion/demotion is written.
(In reply to Jakub Jelinek from comment #15) > We are using that #c13 patch successfully for almost 18 months now in Fedora. > As type promotion/demotion patch is not (sadly) going to happen for GCC 5, I > still think it would be worthwhile to apply it temporarily, until type > promotion/demotion is written. Works for me.
Btw, I think that (int) ((long unsigned int) q + D.2078) to (int) ((unsigned int) q + (unsigned int) D.2078) doesn't look like an always profitable pattern on GIMPLE (more stmts). Also take into consideration what targets PROMOTE_MODE (mode-of-q/D.2078) do and prefer types according to that. The vectorizer prefers single type sizes throughout computations to avoid re-packing.
(In reply to Richard Biener from comment #17) > Btw, I think that > > (int) ((long unsigned int) q + D.2078) > > to > > (int) ((unsigned int) q + (unsigned int) D.2078) > > doesn't look like an always profitable pattern on GIMPLE (more stmts). Also > take into consideration what targets PROMOTE_MODE (mode-of-q/D.2078) do > and prefer types according to that. > > The vectorizer prefers single type sizes throughout computations to avoid > re-packing. Which is why the proposal was to do demotion early and promote back depending on machine description (and, perhaps on separate IL copy, in between ifcvt and vectorizer also promote to decrease re-packing). The demotion would help with avoiding unnecessary computations (e.g. those affecting just the high bits), canonicalizing the IL and in some cases allowing bigger parts of computations with the same type bitsizes, and promotion would be an attempt to help with sub-word arithmetics on word only arithmetics targets, etc. Dunno why Kai has stopped working on that :(.
(In reply to Jakub Jelinek from comment #18) > (In reply to Richard Biener from comment #17) > > Btw, I think that > > > > (int) ((long unsigned int) q + D.2078) > > > > to > > > > (int) ((unsigned int) q + (unsigned int) D.2078) > > > > doesn't look like an always profitable pattern on GIMPLE (more stmts). Also > > take into consideration what targets PROMOTE_MODE (mode-of-q/D.2078) do > > and prefer types according to that. > > > > The vectorizer prefers single type sizes throughout computations to avoid > > re-packing. > > Which is why the proposal was to do demotion early and promote back > depending on machine description (and, perhaps on separate IL copy, in > between ifcvt and vectorizer also promote to decrease re-packing). The > demotion would help with avoiding unnecessary computations (e.g. those > affecting just the high bits), canonicalizing the IL and in some cases > allowing bigger parts of computations with the same type bitsizes, and > promotion would be an attempt to help with sub-word arithmetics on word only > arithmetics targets, etc. > Dunno why Kai has stopped working on that :(. Because I pushed back hard. He ended up with multiple passes here and there, and mostly motivated things by better folding with less conversions in the IL. I also think you can't really separate demotion and promotion (demotion is just making range information a little bit more "explicit" in the IL, and thus IMHO tied to VRP if it runs). Promotion (which can of course also be effective demotion if legal) is to make things aligned with the target.
Author: jakub Date: Thu Dec 4 09:46:45 2014 New Revision: 218345 URL: https://gcc.gnu.org/viewcvs?rev=218345&root=gcc&view=rev Log: PR c++/56493 * convert.c (convert_to_real, convert_to_expr, convert_to_complex): Handle COMPOUND_EXPR. * c-c++-common/pr56493.c: New test. Added: trunk/gcc/testsuite/c-c++-common/pr56493.c Modified: trunk/gcc/ChangeLog trunk/gcc/convert.c trunk/gcc/testsuite/ChangeLog
Author: jakub Date: Thu Dec 4 09:47:54 2014 New Revision: 218346 URL: https://gcc.gnu.org/viewcvs?rev=218346&root=gcc&view=rev Log: PR c++/56493 * convert.c (convert_to_real, convert_to_expr, convert_to_complex): Handle COMPOUND_EXPR. * c-c++-common/pr56493.c: New test. Added: branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/pr56493.c Modified: branches/gcc-4_9-branch/gcc/ChangeLog branches/gcc-4_9-branch/gcc/convert.c branches/gcc-4_9-branch/gcc/testsuite/ChangeLog
Author: jakub Date: Thu Dec 4 09:48:54 2014 New Revision: 218347 URL: https://gcc.gnu.org/viewcvs?rev=218347&root=gcc&view=rev Log: PR c++/56493 * convert.c (convert_to_real, convert_to_expr, convert_to_complex): Handle COMPOUND_EXPR. * c-c++-common/pr56493.c: New test. Added: branches/gcc-4_8-branch/gcc/testsuite/c-c++-common/pr56493.c Modified: branches/gcc-4_8-branch/gcc/ChangeLog branches/gcc-4_8-branch/gcc/convert.c branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
Fixed.