From mboxrd@z Thu Jan 1 00:00:00 1970 From: Allen Martin Date: Tue, 17 Jul 2012 12:26:58 -0700 Subject: [U-Boot] [PATCH 2/7] HACK: rearrange link order for thumb In-Reply-To: <20120712204518.732749d7@lilith> References: <1341598142-28873-1-git-send-email-amartin@nvidia.com> <1341598142-28873-3-git-send-email-amartin@nvidia.com> <4FF737F7.8040008@wwwdotorg.org> <20120706203329.GA29103@nvidia.com> <4FF74E30.3000101@wwwdotorg.org> <20120706231719.GE29103@nvidia.com> <20120707121536.4cdf7f35@lilith> <20120710004535.GA2240@nvidia.com> <20120712204518.732749d7@lilith> Message-ID: <20120717192658.GA20487@nvidia.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 wrote: > > Hi Allen > > > > On Tue, Jul 10, 2012 at 10:45 AM, Allen Martin 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 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