From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Tue, 10 Jul 2012 11:11:07 -0700 Subject: [U-Boot] [PATCH v2 2/2] arm: work around assembler bug In-Reply-To: <20120710180450.GC6010@nvidia.com> References: <1341939617-26450-1-git-send-email-amartin@nvidia.com> <1341939617-26450-2-git-send-email-amartin@nvidia.com> <20120710172505.GF5053@oliver-linux> <20120710174433.GA6010@nvidia.com> <20120710175034.GH5053@oliver-linux> <20120710180450.GC6010@nvidia.com> Message-ID: <20120710181107.GI5053@oliver-linux> List-Id: 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 11:04:50AM -0700, Allen Martin wrote: > On Tue, Jul 10, 2012 at 10:50:34AM -0700, Tom Rini wrote: > > On Tue, Jul 10, 2012 at 10:44:33AM -0700, Allen Martin wrote: > > > 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 > > > > > --- > > > > > 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. > > > > It should still get evaluated once, I think. Kluding the > > binutils-version script to echo 1 >> /tmp/test every time it's run would > > tell tho for sure :) > > I tried it out, it's obvious from the $(warning) it's getting > reevaluated: > > $ ./MAKEALL -s tegra20 > /home/arm/u-boot/arch/arm/config.mk:102: *** disabling sibling call optimzation because binutils version < 2.22 > /home/arm/u-boot/arch/arm/config.mk:103: *** code size may be slightly larger > Configuring for harmony board... > text data bss dec hex filename > 170334 4284 278352 452970 6e96a ./u-boot > 11676 161 36 11873 2e61 ./spl/u-boot-spl > /home/arm/u-boot/arch/arm/config.mk:102: *** disabling sibling call optimzation because binutils version < 2.22 > /home/arm/u-boot/arch/arm/config.mk:103: *** code size may be slightly larger > /home/arm/u-boot/arch/arm/config.mk:102: *** disabling sibling call optimzation because binutils version < 2.22 > /home/arm/u-boot/arch/arm/config.mk:103: *** code size may be slightly larger > /home/arm/u-boot/arch/arm/config.mk:102: *** disabling sibling call optimzation because binutils version < 2.22 > /home/arm/u-boot/arch/arm/config.mk:103: *** code size may be slightly larger > /home/arm/u-boot/arch/arm/config.mk:102: *** disabling sibling call optimzation because binutils version < 2.22 > /home/arm/u-boot/arch/arm/config.mk:103: *** code size may be slightly larger > /home/arm/u-boot/arch/arm/config.mk:102: *** disabling sibling call optimzation because binutils version < 2.22 > /home/arm/u-boot/arch/arm/config.mk:103: *** code size may be slightly larger > /home/arm/u-boot/arch/arm/config.mk:102: *** disabling sibling call optimzation because binutils version < 2.22 > /home/arm/u-boot/arch/arm/config.mk:103: *** code size may be slightly larger > ... and on and on > > > It makes sense since unless GAS_BUG_125320 is "export"ed to the new > make, it's going to be unset again each the build starts a new > "make -C" Thanks for proving me wrong. Just the white-space / multi line thing please, thanks :) -- Tom