public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/7] HACK: rearrange link order for thumb
Date: Thu, 12 Jul 2012 20:45:18 +0200	[thread overview]
Message-ID: <20120712204518.732749d7@lilith> (raw)
In-Reply-To: <CALButCLGn_mWKc7LfTLeC8EdPuGEx29HJwyyJ8_vRBTVbPzofQ@mail.gmail.com>

Hi Graeme,

On Tue, 10 Jul 2012 10:57:39 +1000, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Allen
> 
> On Tue, Jul 10, 2012 at 10:45 AM, Allen Martin <amartin@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@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. :)

> Regards,
> 
> Graeme

Amicalement,
-- 
Albert.

  reply	other threads:[~2012-07-12 18:45 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-06 18:08 [U-Boot] [PATCH 0/7] tegra20: enable thumb Allen Martin
2012-07-06 18:08 ` [U-Boot] [PATCH 1/7] tegra20: remove inline assembly for u32 cast Allen Martin
2012-07-06 18:08 ` [U-Boot] [PATCH 2/7] HACK: rearrange link order for thumb Allen Martin
2012-07-06 19:09   ` Stephen Warren
2012-07-06 20:33     ` Allen Martin
2012-07-06 20:44       ` Stephen Warren
2012-07-06 21:32         ` Allen Martin
2012-07-06 23:04         ` Allen Martin
2012-07-06 23:17         ` Allen Martin
2012-07-07 10:15           ` Albert ARIBAUD
2012-07-07 18:42             ` Allen Martin
2012-07-10  0:45             ` Allen Martin
2012-07-10  0:57               ` Graeme Russ
2012-07-12 18:45                 ` Albert ARIBAUD [this message]
2012-07-17 19:26                   ` Allen Martin
2012-07-06 18:08 ` [U-Boot] [PATCH 3/7] tegra20: enable thumb build Allen Martin
2012-07-06 19:10   ` Stephen Warren
2012-07-06 20:34     ` Allen Martin
2012-07-06 18:08 ` [U-Boot] [PATCH 4/7] arm: add _thumb1_case_uqi to libgcc Allen Martin
2012-07-06 18:09 ` [U-Boot] [PATCH 5/7] arm: add thumb compatible return instructions Allen Martin
2012-07-06 18:09 ` [U-Boot] [PATCH 6/7] arm: use thumb compatible return in arm720t Allen Martin
2012-07-06 18:09 ` [U-Boot] [PATCH 7/7] arm: change arm720t to armv4t Allen Martin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120712204518.732749d7@lilith \
    --to=albert.u.boot@aribaud.net \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox