From mboxrd@z Thu Jan  1 00:00:00 1970
From: Allen Martin <amartin@nvidia.com>
Date: Tue, 10 Jul 2012 10:44:33 -0700
Subject: [U-Boot] [PATCH v2 2/2] arm: work around assembler bug
In-Reply-To: <20120710172505.GF5053@oliver-linux>
References: <1341939617-26450-1-git-send-email-amartin@nvidia.com>
	<1341939617-26450-2-git-send-email-amartin@nvidia.com>
	<20120710172505.GF5053@oliver-linux>
Message-ID: <20120710174433.GA6010@nvidia.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Tue, Jul 10, 2012 at 10:25:05AM -0700, Tom Rini wrote:
> * PGP Signed by an unknown key
> 
> On Tue, Jul 10, 2012 at 10:00:17AM -0700, Allen Martin wrote:
> > Disable sibling call optimization based on binutils version.  This is
> > to work around a bug in the assember in binutils versions < 2.22.
> > Branches to weak symbols can be incorrectly optimized in thumb mode to
> > a short branch (b.n instruction) that won't reach when the symbol gets
> > preempted.
> > 
> > http://sourceware.org/bugzilla/show_bug.cgi?id=12532
> > 
> > Signed-off-by: Allen Martin <amartin@nvidia.com>
> > ---
> > changes for v2:
> >  -changed GAS_BUG_12532 from yes/no to y/n to be consistent
> >  -added additional warning about code size increase
> > ---
> >  arch/arm/config.mk |   21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> > index 3f4453a..bcabf17 100644
> > --- a/arch/arm/config.mk
> > +++ b/arch/arm/config.mk
> > @@ -87,3 +87,24 @@ endif
> >  ifndef CONFIG_NAND_SPL
> >  LDFLAGS_u-boot += -pie
> >  endif
> > +
> > +#
> > +# binutils versions < 2.22 have a bug in the assembler where branches
> > +# to weak symbols can be incorrectly optimized in thumb mode to a
> > +# short branch (b.n instruction) that won't reach when the symbol
> > +# gets preempted
> > +#
> > +# http://sourceware.org/bugzilla/show_bug.cgi?id=12532
> > +#
> > +ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
> > +ifeq ($(GAS_BUG_12532),)
> > +export GAS_BUG_12532:=$(shell if [ $(call binutils-version) -lt 0222 ] ; then echo y; else echo n; fi)
> 
> Please don't squash out whitespace, just break this out into a
> multi-line test ala the others in the toplevel config.mk.

Ok

> > +ifeq ($(GAS_BUG_12532),y)
> > +$(warning *** disabling sibling call optimzation because binutils version < 2.22)
> > +$(warning *** code size may be slightly larger)
> > +endif
> > +endif
> > +ifeq ($(GAS_BUG_12532),y)
> > +PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
> > +endif
> > +endif
> 
> This seems a bit longer than needed.  Just check that it's y once and do
> both things.  Also, I _think_ we should be able to squish this down to:
> ifeq ($(CONFIG_SYS_THUMB_BUILD),y)
> GAS_BUG_12532 := $(shell if [ $(call binutils-version) -lt 0222 ] ; \
> 	then echo y; else echo n; fi)
> ifeq ($(GAS_BUG_125320),y)
> $(warning ...)
> $(warning ...)
> PLATFORM_RELFLAGS += ...
> endif
> endif
> 
> But I haven't tested it.

I wrote it the way I did because I only want the expensive
shell/binutils-version to get evaluated once per "make".  I think what
you proposed would cause it to get re-evaluated every time the build
goes down into a subdir.

-Allen
-- 
nvpublic