On Thu, Jul 12, 2012 at 11:45:18AM -0700, Albert ARIBAUD wrote: > Hi Graeme, > > On Tue, 10 Jul 2012 10:57:39 +1000, Graeme Russ <graeme.r...@gmail.com> wrote: > > Hi Allen > > > > On Tue, Jul 10, 2012 at 10:45 AM, Allen Martin <amar...@nvidia.com> wrote: > > > On Sat, Jul 07, 2012 at 03:15:36AM -0700, Albert ARIBAUD wrote: > > >> Hi Allen, > > >> > > >> On Fri, 6 Jul 2012 16:17:19 -0700, Allen Martin <amar...@nvidia.com> > > >> wrote: > > > > [snip] > > > > >> > And I forgot to mention, the code bloat from disabling the > > >> > optimization is about 400 bytes (185136 -> 185540), so it's not bad, > > >> > but it still seems a shame to disable all short branches because of > > >> > one misoptimized one. > > > > 0.2% be my calcs > > > > >> > > >> Can this not be limited to compiling the object files which are known to > > >> be > > >> sensitive to the problem? > > >> > > > > > > I understand this issue fairly well now. It's a known bug in the > > > assembler that has already been fixed: > > > > > > http://sourceware.org/bugzilla/show_bug.cgi?id=12532 > > > > > > It only impacts preembtable symbols, and since u-boot doesn't have any > > > dynamic loadable objects it's only explictly defined weak symbols that > > > should trigger the bug. > > > > > > I built a new toolchain with binutils 2.22 and verified the bug is no > > > longer present there, and -fno-optimize-sibling-calls is the correct > > > workaround for toolchains that do have the bug, so conditionally > > > disabling the optimization for binutils < 2.22 seems like the right > > > fix. > > > > > > I ran a quick scrub of the u-boot tree and there's 195 instances of > > > __attribute__((weak)) spread across 123 source files, so I think just > > > disabling optimization on the failing object files may be too fragile, > > > as code movement could cause others to crop up. > > > > Adding -fno-optimize-sibling-calls for binutils < 2.22 - 0.2% code size > > increase for people using slightly older tools > > > > Maintain the tweaking of a set of files - someone using binutils >= 2.22 > > adds __attribute__((weak)) to a single function and *BAM* three months > > later someone complains that something broke > > > > I vote option 1 > > > > I do wonder, though, if we should spit out warnings when applying > > workaraounds for older tool-chains? > > I am against this idea: this persistent warning will either worry or > anoy the reader, neither of which is good IMO. OTOH, when in the future > the workaround is removed because the toolchain version it fixes is > considered obsolete, *then* we shall add a warning to let developers > know that they use an *unsupported* binutils version. > > Meanwhile, we could mark the workaround with a FIXME note in the code, > for present and future U-Boot *developers* to notice and remember > what they should do with this workaround. :)
I'm ok with either way, I added the warning at Graeme's suggestion. I'll send another patch series with a FIXME comment and the warning removed and let you guys fight it out :^) -Allen -- nvpublic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
