[U-Boot] [PATCH 2/7] HACK: rearrange link order for thumb

[Allen Martin]

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.russ at gmail.com> wrote:
> > Hi Allen
> > 
> > On Tue, Jul 10, 2012 at 10:45 AM, Allen Martin <amartin at 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 <amartin at 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