As already discussed with Loic, CodeSourcery have a GCC patch that implements a new optimization: -fremove-local-statics.
Essentially, it transforms code like this:
int foo (void) { static int a = 1; return a; }
into this:
int foo (void) { int a = 1; return a; }
Admittedly, if the code is written like that you might argue that the auther gets what they deserve, but apparently at least one of the EEMBC benchmarks does have these, so now we all care about it.
This patch was originally submitted, by RedHat, to gcc-patches here: http://gcc.gnu.org/ml/gcc-patches/2008-07/subjects.html#00982
Some discussion later, they decided it would be better to implement the optimization using inter-procedural dead store analysis: http://gcc.gnu.org/ml/gcc-patches/2008-07/msg01602.html
This doesn't seem to have actually been done. Not yet, anyway.
So basically we're left with this patch that does something we want, but not in a way that can go upstream. :(
The question is, should I merge this to Linaro, or not? Loic and I agreed to hold off until I'd done a bit more research and/or tried to upstream it again, but now I think we need to think again.
Andrew
On 29/07/10 17:23, Andrew Stubbs wrote:
This patch was originally submitted, by RedHat, to gcc-patches here: http://gcc.gnu.org/ml/gcc-patches/2008-07/subjects.html#00982
Joseph points out, the patch we have in our tree is not actually the same one posted in the link above. Our patch is a different implementation of the same concept.
Andrew
On 29/07/10 17:23, Andrew Stubbs wrote:
So basically we're left with this patch that does something we want, but not in a way that can go upstream. :(
The question is, should I merge this to Linaro, or not? Loic and I agreed to hold off until I'd done a bit more research and/or tried to upstream it again, but now I think we need to think again.
Ping? Anybody have any thoughts on this. We need to make a decision.
Andrew
Andrew Stubbs ams@codesourcery.com wrote:
Some discussion later, they decided it would be better to implement the optimization using inter-procedural dead store analysis: http://gcc.gnu.org/ml/gcc-patches/2008-07/msg01602.html
I agree that this would be a much nicer way ...
This doesn't seem to have actually been done. Not yet, anyway.
Maybe this is something we should be working on then?
So basically we're left with this patch that does something we want, but not in a way that can go upstream. :(
The question is, should I merge this to Linaro, or not? Loic and I agreed to hold off until I'd done a bit more research and/or tried to upstream it again, but now I think we need to think again.
The one concern I have is that the patch introduces a user-visible construct: the -fremove-local-statics command line option. If we add this now, and users add the flag to their Makefiles, and then it goes away later on, users' builds could break.
On the other hand, we already have the flag in 4.4 anyway, so that risk is there in either case ...
Mit freundlichen Gruessen / Best Regards
Ulrich Weigand
-- Dr. Ulrich Weigand | Phone: +49-7031/16-3727 STSM, GNU compiler and toolchain for Linux on System z and Cell/B.E. IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter | Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht Stuttgart, HRB 243294
Ulrich Weigand wrote:
Andrew Stubbs ams@codesourcery.com wrote:
Some discussion later, they decided it would be better to implement the optimization using inter-procedural dead store analysis: http://gcc.gnu.org/ml/gcc-patches/2008-07/msg01602.html
I agree that this would be a much nicer way ...
Maybe this is something we should be working on then?
So basically we're left with this patch that does something we want, but not in a way that can go upstream. :(
For avoidance of doubt, we (CodeSourcery) aren't attached to the current implementation. If someone wants us to go do a better one, that would be fun. But, on the other hand, the current implementation works, and delivers a significant improvement on EEMBC.
The one concern I have is that the patch introduces a user-visible construct: the -fremove-local-statics command line option. If we add this now, and users add the flag to their Makefiles, and then it goes away later on, users' builds could break.
I think it's reasonable for the option to be named that, no matter what implementation it has. At worst, we could always have a Linaro-specific hack to accept the option and ignore it; certainly, CodeSourcery will support this command-line option in our tools going forward "forever".
Thanks,
linaro-toolchain@lists.linaro.org
-
Andrew Stubbs -
Mark Mitchell -
Ulrich Weigand