public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
@ 2012-07-18 23:45 Allen Martin
  2012-07-18 23:45 ` [U-Boot] [PATCH v4 2/2] arm: work around assembler bug Allen Martin
  2012-07-19  3:11 ` [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version Mike Frysinger
  0 siblings, 2 replies; 18+ messages in thread
From: Allen Martin @ 2012-07-18 23:45 UTC (permalink / raw)
  To: u-boot

Modeled after gcc-version, add function to get binutils version.

Signed-off-by: Allen Martin <amartin@nvidia.com>
---
 config.mk                 |    1 +
 tools/binutils-version.sh |   20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)
 create mode 100755 tools/binutils-version.sh

diff --git a/config.mk b/config.mk
index 3dcea6a..919b77d 100644
--- a/config.mk
+++ b/config.mk
@@ -128,6 +128,7 @@ endif
 # cc-version
 # Usage gcc-ver := $(call cc-version)
 cc-version = $(shell $(SHELL) $(SRCTREE)/tools/gcc-version.sh $(CC))
+binutils-version = $(shell $(SHELL) $(SRCTREE)/tools/binutils-version.sh $(AS))
 
 #
 # Include the make variables (CC, etc...)
diff --git a/tools/binutils-version.sh b/tools/binutils-version.sh
new file mode 100755
index 0000000..d4d9eb4
--- /dev/null
+++ b/tools/binutils-version.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+#
+# binutils-version [-p] gas-command
+#
+# Prints the binutils version of `gas-command' in a canonical 4-digit form
+# such as `0222' for binutils 2.22
+#
+
+gas="$*"
+
+if [ ${#gas} -eq 0 ]; then
+	echo "Error: No assembler specified."
+	printf "Usage:\n\t$0 <gas-command>\n"
+	exit 1
+fi
+
+MAJOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 1)
+MINOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 2)
+
+printf "%02d%02d\\n" $MAJOR $MINOR
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 2/2] arm: work around assembler bug
  2012-07-18 23:45 [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version Allen Martin
@ 2012-07-18 23:45 ` Allen Martin
  2012-07-19  6:06   ` Albert ARIBAUD
  2012-10-04  8:47   ` Albert ARIBAUD
  2012-07-19  3:11 ` [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version Mike Frysinger
  1 sibling, 2 replies; 18+ messages in thread
From: Allen Martin @ 2012-07-18 23:45 UTC (permalink / raw)
  To: u-boot

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 v4:
 -removed warning print
changes for v3:
 -split shell line to improve readability
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 |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index 3f4453a..24b9d7c 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -87,3 +87,21 @@ endif
 ifndef CONFIG_NAND_SPL
 LDFLAGS_u-boot += -pie
 endif
+
+#
+# FIXME: 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)
+endif
+ifeq ($(GAS_BUG_12532),y)
+PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
+endif
+endif
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-07-18 23:45 [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version Allen Martin
  2012-07-18 23:45 ` [U-Boot] [PATCH v4 2/2] arm: work around assembler bug Allen Martin
@ 2012-07-19  3:11 ` Mike Frysinger
  2012-07-19 15:08   ` Tom Rini
  1 sibling, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2012-07-19  3:11 UTC (permalink / raw)
  To: u-boot

On Wednesday 18 July 2012 19:45:52 Allen Martin wrote:
> +MAJOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 1)
> +MINOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 2)
> +
> +printf "%02d%02d\\n" $MAJOR $MINOR

can be replaced with a single awk script:

$gas --version | awk '{
	gsub(/[.]/, " ", $NF)
	$0 = $NF
	printf "%02d%02d\n", $1, $2
	exit
}'
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120718/5e6e9409/attachment.pgp>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 2/2] arm: work around assembler bug
  2012-07-18 23:45 ` [U-Boot] [PATCH v4 2/2] arm: work around assembler bug Allen Martin
@ 2012-07-19  6:06   ` Albert ARIBAUD
  2012-07-19 15:08     ` Tom Rini
  2012-10-04  8:47   ` Albert ARIBAUD
  1 sibling, 1 reply; 18+ messages in thread
From: Albert ARIBAUD @ 2012-07-19  6:06 UTC (permalink / raw)
  To: u-boot

Hi Allen,

On Wed, 18 Jul 2012 16:45:53 -0700, Allen Martin <amartin@nvidia.com> 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 v4:
>  -removed warning print
> changes for v3:
>  -split shell line to improve readability
> 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 |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> index 3f4453a..24b9d7c 100644
> --- a/arch/arm/config.mk
> +++ b/arch/arm/config.mk
> @@ -87,3 +87,21 @@ endif
>  ifndef CONFIG_NAND_SPL
>  LDFLAGS_u-boot += -pie
>  endif
> +
> +#
> +# FIXME: 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)
> +endif
> +ifeq ($(GAS_BUG_12532),y)
> +PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
> +endif
> +endif

Can previous reviewers ack or test this? I would like to have it in the ARM
master branch in time for 12.07.

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-07-19  3:11 ` [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version Mike Frysinger
@ 2012-07-19 15:08   ` Tom Rini
  2012-07-19 15:21     ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Rini @ 2012-07-19 15:08 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/18/2012 08:11 PM, Mike Frysinger wrote:
> On Wednesday 18 July 2012 19:45:52 Allen Martin wrote:
>> +MAJOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . 
>> -f 1) +MINOR=$($gas --version | head -1 | awk '{print $NF}' |
>> cut -d . -f 2) + +printf "%02d%02d\\n" $MAJOR $MINOR
> 
> can be replaced with a single awk script:
> 
> $gas --version | awk '{ gsub(/[.]/, " ", $NF) $0 = $NF printf 
> "%02d%02d\n", $1, $2 exit }'

That looks much longer and we call this once so a few execs is noise.

- -- 
Tom

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJQCCLaAAoJENk4IS6UOR1Wag8P/0/s9VVHGj60Q2MqaJPWST1H
8SVj8SMNBbeG2WHadkY+iB815MoToK+8oz5WMXhDA8PLThXdljBaQgbIjur2xyo5
QXRPFwxk3oIQxiy/fKR9ubt1eg9jJSrnZM6tTdWpOANYeqZo5w9S4MKoUr+mT9cf
FzXIhKOoX9YRxKZsqhMYgTxO85sMg7FUn8OEK3q1GJdbqVF5lSSAKSG/WHDbv3wV
/VIbgskp2OjtOlq1GHtmEQw77d1aYhHkwSKaRiYnFKimez+GeIq++ap9bdt2x7Dn
87THqKfeB62xdID9kCIDf9tUPKeFBf/mN606Vreg6s00DxZOmtn20CW/WErxIdxx
7b6r49KXPKqA553e08BChbopvQEYM9F/5MIkPqEX71ilR8p9WJ56IuJNJNbSMFMZ
YP96niRroVEV8KBk9FAPIqgNVZ40Fz0TCJSurs2ESHBaVlH9ZF9b6YjOXjIulFwa
WImS4xwXcsfV0PzryVxsptX2TEq8t0p9gXt7oKCAjvER8nnGjLz+B7VuNl1avBrC
6xXyBSpXhOMgpTTPleC7ymo9D8NUJFfetDwi2jNVxcnVIT/tiN3eqFWbapdqWeds
pfocE+E2VIZF2g1BizMuUYuPkw/9NJBqp42vS8lr/FVdaePzIOZo/DSDl95/Vlr6
U6BFALNyEJK0WmnXaOIV
=6YIv
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 2/2] arm: work around assembler bug
  2012-07-19  6:06   ` Albert ARIBAUD
@ 2012-07-19 15:08     ` Tom Rini
  2012-08-01 17:34       ` Allen Martin
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Rini @ 2012-07-19 15:08 UTC (permalink / raw)
  To: u-boot

On 07/18/2012 11:06 PM, Albert ARIBAUD wrote:
> Hi Allen,
> 
> On Wed, 18 Jul 2012 16:45:53 -0700, Allen Martin <amartin@nvidia.com> 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 v4:
>>  -removed warning print
>> changes for v3:
>>  -split shell line to improve readability
>> 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 |   18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
>> index 3f4453a..24b9d7c 100644
>> --- a/arch/arm/config.mk
>> +++ b/arch/arm/config.mk
>> @@ -87,3 +87,21 @@ endif
>>  ifndef CONFIG_NAND_SPL
>>  LDFLAGS_u-boot += -pie
>>  endif
>> +
>> +#
>> +# FIXME: 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)
>> +endif
>> +ifeq ($(GAS_BUG_12532),y)
>> +PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
>> +endif
>> +endif
> 
> Can previous reviewers ack or test this? I would like to have it in the ARM
> master branch in time for 12.07.

Acked-by: Tom Rini <trini@ti.com>

-- 
Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-07-19 15:08   ` Tom Rini
@ 2012-07-19 15:21     ` Mike Frysinger
  2012-07-19 15:38       ` Tom Rini
  0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2012-07-19 15:21 UTC (permalink / raw)
  To: u-boot

On Thursday 19 July 2012 11:08:10 Tom Rini wrote:
> On 07/18/2012 08:11 PM, Mike Frysinger wrote:
> > On Wednesday 18 July 2012 19:45:52 Allen Martin wrote:
> >> +MAJOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 1)
> >> +MINOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 2)
> >> +
> >> +printf "%02d%02d\\n" $MAJOR $MINOR
> > 
> > can be replaced with a single awk script:
> > 
> > $gas --version | awk '{ gsub(/[.]/, " ", $NF) $0 = $NF printf
> > "%02d%02d\n", $1, $2 exit }'
> 
> That looks much longer and we call this once so a few execs is noise.

here's a shorter version:
$gas --version | awk '{ gsub(/[.]/, " ", $NF); $0 = $NF; printf "%02d%02d\n", $1, $2; exit }'
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120719/6d999c93/attachment.pgp>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-07-19 15:21     ` Mike Frysinger
@ 2012-07-19 15:38       ` Tom Rini
  2012-07-19 16:43         ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Rini @ 2012-07-19 15:38 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/19/2012 08:21 AM, Mike Frysinger wrote:
> On Thursday 19 July 2012 11:08:10 Tom Rini wrote:
>> On 07/18/2012 08:11 PM, Mike Frysinger wrote:
>>> On Wednesday 18 July 2012 19:45:52 Allen Martin wrote:
>>>> +MAJOR=$($gas --version | head -1 | awk '{print $NF}' | cut
>>>> -d . -f 1) +MINOR=$($gas --version | head -1 | awk '{print
>>>> $NF}' | cut -d . -f 2) + +printf "%02d%02d\\n" $MAJOR $MINOR
>>> 
>>> can be replaced with a single awk script:
>>> 
>>> $gas --version | awk '{ gsub(/[.]/, " ", $NF) $0 = $NF printf 
>>> "%02d%02d\n", $1, $2 exit }'
>> 
>> That looks much longer and we call this once so a few execs is
>> noise.
> 
> here's a shorter version: $gas --version | awk '{ gsub(/[.]/, " ",
> $NF); $0 = $NF; printf "%02d%02d\n", $1, $2; exit }'

And still over 80 chars before we assign it to a variable.  I could
get it to 77 chars with all whitespace removed.

- -- 
Tom


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJQCCn/AAoJENk4IS6UOR1WFJwP/2pv0lUXUJVJZk1Wb1w2ln2G
MtjUg1ntwrgQmKCb5D67VNCHN2sjtiWi0HGNZDCvHqRz9AGN7BX/Dz8Jx0hKoqf3
S92J7VpKJ+MGpBVpWxlZMNY5FlBnkiVpUCkIQefhz5sUqq1fA2PjCs82MRadr9WP
KmpfDXZ6OBRl7hKqeHVaYaqfCjxaaXqmPLrXh1VXUjA6oYKv0kZcvW9H2kMPyaTd
+shYx0z/TI6UXHmw0CmcoDvPsy9cIBAtqtuqOeP/YP1sIHq1UQBIehfD4ji4JZAl
wWKymSJwtcIl6hCV+IV3/wGczIdvRfyraa8mN5/MrVWLbgiytW/OOnix7mYlw4ov
ysXz/5pg7dPkaHxczvANpVO7PkzRJZKMQhjyuZmVDSDdEzPks9QHWt3miRErXjMc
kkD2LaKkV9hBBSqD6+/vfd45zLI125UDfkkRLLwr67bHcrGkdBbojOXE357vQx7N
ELp2FPOwfXkBOw//P0KhlVtc9T1Li+LqqfjqzbYQkPYbHzcAal7SPERzxION+pFO
00M+uYwtxPDm/wvmk033VTZNaLdvLHU6Zg1wkKwj484MZ/x+ptFwDvTwppRw/Kme
4JuhjxWGwQFu9tkV0gWFd2D2uYZzbDt5VnZkntp1BxyXEHJ3FvvUvMryr8ZqEAIS
Ss/IDqUlIsctLDiDziSD
=9xj8
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-07-19 15:38       ` Tom Rini
@ 2012-07-19 16:43         ` Mike Frysinger
  2012-07-19 16:54           ` Tom Rini
  0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2012-07-19 16:43 UTC (permalink / raw)
  To: u-boot

On Thursday 19 July 2012 11:38:39 Tom Rini wrote:
> On 07/19/2012 08:21 AM, Mike Frysinger wrote:
> > On Thursday 19 July 2012 11:08:10 Tom Rini wrote:
> >> On 07/18/2012 08:11 PM, Mike Frysinger wrote:
> >>> On Wednesday 18 July 2012 19:45:52 Allen Martin wrote:
> >>>> +MAJOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 1)
> >>>> +MINOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 2)
> >>>> +
> >>>> +printf "%02d%02d\\n" $MAJOR $MINOR
> >>> 
> >>> can be replaced with a single awk script:
> >>> 
> >>> $gas --version | awk '{ gsub(/[.]/, " ", $NF) $0 = $NF printf
> >>> "%02d%02d\n", $1, $2 exit }'
> >> 
> >> That looks much longer and we call this once so a few execs is
> >> noise.
> > 
> > here's a shorter version: $gas --version | awk '{ gsub(/[.]/, " ",
> > $NF); $0 = $NF; printf "%02d%02d\n", $1, $2; exit }'
> 
> And still over 80 chars before we assign it to a variable.  I could
> get it to 77 chars with all whitespace removed.

which is why i unrolled it to make it readable.  i don't know what metrics 
you're using here, but i don't think the awk version is "longer" by really any 
of them.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120719/b721b0fc/attachment.pgp>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-07-19 16:43         ` Mike Frysinger
@ 2012-07-19 16:54           ` Tom Rini
  2012-08-01 22:31             ` Mike Frysinger
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Rini @ 2012-07-19 16:54 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/19/2012 09:43 AM, Mike Frysinger wrote:
> On Thursday 19 July 2012 11:38:39 Tom Rini wrote:
>> On 07/19/2012 08:21 AM, Mike Frysinger wrote:
>>> On Thursday 19 July 2012 11:08:10 Tom Rini wrote:
>>>> On 07/18/2012 08:11 PM, Mike Frysinger wrote:
>>>>> On Wednesday 18 July 2012 19:45:52 Allen Martin wrote:
>>>>>> +MAJOR=$($gas --version | head -1 | awk '{print $NF}' |
>>>>>> cut -d . -f 1) +MINOR=$($gas --version | head -1 | awk
>>>>>> '{print $NF}' | cut -d . -f 2) + +printf "%02d%02d\\n"
>>>>>> $MAJOR $MINOR
>>>>> 
>>>>> can be replaced with a single awk script:
>>>>> 
>>>>> $gas --version | awk '{ gsub(/[.]/, " ", $NF) $0 = $NF
>>>>> printf "%02d%02d\n", $1, $2 exit }'
>>>> 
>>>> That looks much longer and we call this once so a few execs
>>>> is noise.
>>> 
>>> here's a shorter version: $gas --version | awk '{ gsub(/[.]/, "
>>> ", $NF); $0 = $NF; printf "%02d%02d\n", $1, $2; exit }'
>> 
>> And still over 80 chars before we assign it to a variable.  I
>> could get it to 77 chars with all whitespace removed.
> 
> which is why i unrolled it to make it readable.  i don't know what
> metrics you're using here, but i don't think the awk version is
> "longer" by really any of them.

The metric of 'wc -c' and "what fits in a single line, unwrapped on an
80x24 terminal."  awk is great and awesome, don't get me wrong, but
it's not doing the job as compactly as the original.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJQCDvNAAoJENk4IS6UOR1WzRsP/ikEjUCPX593aZxLqHh7oGZO
novK94mu/ThO8pApYs17Lhh77wh9Bn/lbRrqwjRrFZAKqGN6gPYpknvvmxD31AkV
K72xz19ut7S/zBAVZIgMcnuHDvz5LEEBZBRjinmKM+7nMRMvaqi2rEhs+PUj00xP
IPeQvqttfAB1iblVhU+07at6vhsKRM2fXS45crXztGXYfeYzT90hMM4NrGXuiJ+G
4aav8FK4nHM+DgOAjzwYlZ7Ty1okm0F7mAwM9+nJE5WiUPI8G9nPzcWt3IeVa6JF
XmRkaZH7cTAz/qrWjbVKd7rLnZ75jqoez1Wv4FVescrJ6Mu5oH7QUXfciYJrGwQ8
VHBBvI+Hf1W2YICBHTeO9wdfMOuVl7Jj6K7+CunczJ7qA5VjUHyr0Q3gCnr3UoTW
9tUblOT39zRLIs57IWN7cio2RdIPdzd1N5sGh3S1UxGhM+dnUTeM3faVxoI8LJyU
/Fb+kmv1LhMPPMjqxyzbFROjGZVXx0T4K5ERqrZ+k4jP/r3t6QOAMsJhvypZHkQf
WeMyldYIwlbeflMYKjxA729C/CdN0MJttYMbnBRhcmWM/VJR41Cuh2GSOhN635Z3
N3IYxb9V8txAfq308rEp9Hwj1oJ9DjnEw3xBvjBLYCcFncJSER6dMajuFK6qXqay
PR+q3ZquMwPWcYlWQRyk
=sxrC
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 2/2] arm: work around assembler bug
  2012-07-19 15:08     ` Tom Rini
@ 2012-08-01 17:34       ` Allen Martin
  0 siblings, 0 replies; 18+ messages in thread
From: Allen Martin @ 2012-08-01 17:34 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 19, 2012 at 08:08:31AM -0700, Tom Rini wrote:
> On 07/18/2012 11:06 PM, Albert ARIBAUD wrote:
> > Hi Allen,
> > 
> > On Wed, 18 Jul 2012 16:45:53 -0700, Allen Martin <amartin@nvidia.com> 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>


> > Can previous reviewers ack or test this? I would like to have it in the ARM
> > master branch in time for 12.07.
> 
> Acked-by: Tom Rini <trini@ti.com>

Hi Albert, just checking on the status of applying this to
u-boot-arm.  I have a patch series to enable thumb for tegra that
needs this.  I'll probably just keep a copy of this in that series
until it goes in.

thanks,
-Allen

-- 
nvpublic

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-07-19 16:54           ` Tom Rini
@ 2012-08-01 22:31             ` Mike Frysinger
  2012-08-01 22:46               ` Allen Martin
  0 siblings, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2012-08-01 22:31 UTC (permalink / raw)
  To: u-boot

On Thursday 19 July 2012 12:54:37 Tom Rini wrote:
> On 07/19/2012 09:43 AM, Mike Frysinger wrote:
> > On Thursday 19 July 2012 11:38:39 Tom Rini wrote:
> >> On 07/19/2012 08:21 AM, Mike Frysinger wrote:
> >>> On Thursday 19 July 2012 11:08:10 Tom Rini wrote:
> >>>> On 07/18/2012 08:11 PM, Mike Frysinger wrote:
> >>>>> On Wednesday 18 July 2012 19:45:52 Allen Martin wrote:
> >>>>>> +MAJOR=$($gas --version | head -1 | awk '{print $NF}' |
> >>>>>> cut -d . -f 1) +MINOR=$($gas --version | head -1 | awk
> >>>>>> '{print $NF}' | cut -d . -f 2) + +printf "%02d%02d\\n"
> >>>>>> $MAJOR $MINOR
> >>>>> 
> >>>>> can be replaced with a single awk script:
> >>>>> 
> >>>>> $gas --version | awk '{ gsub(/[.]/, " ", $NF) $0 = $NF
> >>>>> printf "%02d%02d\n", $1, $2 exit }'
> >>>> 
> >>>> That looks much longer and we call this once so a few execs
> >>>> is noise.
> >>> 
> >>> here's a shorter version: $gas --version | awk '{ gsub(/[.]/, "
> >>> ", $NF); $0 = $NF; printf "%02d%02d\n", $1, $2; exit }'
> >> 
> >> And still over 80 chars before we assign it to a variable.  I
> >> could get it to 77 chars with all whitespace removed.
> > 
> > which is why i unrolled it to make it readable.  i don't know what
> > metrics you're using here, but i don't think the awk version is
> > "longer" by really any of them.
> 
> The metric of 'wc -c' and "what fits in a single line, unwrapped on an
> 80x24 terminal."  awk is great and awesome, don't get me wrong, but
> it's not doing the job as compactly as the original.

obviously i disagree.  i find the awk version "better" in just about every way.  
maybe someone else will jump in with their favorite bike.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120801/0cc4bed0/attachment.pgp>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-08-01 22:31             ` Mike Frysinger
@ 2012-08-01 22:46               ` Allen Martin
  2012-08-01 22:55                 ` Graeme Russ
  2012-08-01 23:01                 ` Mike Frysinger
  0 siblings, 2 replies; 18+ messages in thread
From: Allen Martin @ 2012-08-01 22:46 UTC (permalink / raw)
  To: u-boot

On Wed, Aug 01, 2012 at 03:31:37PM -0700, Mike Frysinger wrote:
> On Thursday 19 July 2012 12:54:37 Tom Rini wrote:
> > On 07/19/2012 09:43 AM, Mike Frysinger wrote:
> > > On Thursday 19 July 2012 11:38:39 Tom Rini wrote:
> > >> On 07/19/2012 08:21 AM, Mike Frysinger wrote:
> > >>> On Thursday 19 July 2012 11:08:10 Tom Rini wrote:
> > >>>> On 07/18/2012 08:11 PM, Mike Frysinger wrote:
> > >>>>> On Wednesday 18 July 2012 19:45:52 Allen Martin wrote:
> > >>>>>> +MAJOR=$($gas --version | head -1 | awk '{print $NF}' |
> > >>>>>> cut -d . -f 1) +MINOR=$($gas --version | head -1 | awk
> > >>>>>> '{print $NF}' | cut -d . -f 2) + +printf "%02d%02d\\n"
> > >>>>>> $MAJOR $MINOR
> > >>>>> 
> > >>>>> can be replaced with a single awk script:
> > >>>>> 
> > >>>>> $gas --version | awk '{ gsub(/[.]/, " ", $NF) $0 = $NF
> > >>>>> printf "%02d%02d\n", $1, $2 exit }'
> > >>>> 
> > >>>> That looks much longer and we call this once so a few execs
> > >>>> is noise.
> > >>> 
> > >>> here's a shorter version: $gas --version | awk '{ gsub(/[.]/, "
> > >>> ", $NF); $0 = $NF; printf "%02d%02d\n", $1, $2; exit }'
> > >> 
> > >> And still over 80 chars before we assign it to a variable.  I
> > >> could get it to 77 chars with all whitespace removed.
> > > 
> > > which is why i unrolled it to make it readable.  i don't know what
> > > metrics you're using here, but i don't think the awk version is
> > > "longer" by really any of them.
> > 
> > The metric of 'wc -c' and "what fits in a single line, unwrapped on an
> > 80x24 terminal."  awk is great and awesome, don't get me wrong, but
> > it's not doing the job as compactly as the original.
> 
> obviously i disagree.  i find the awk version "better" in just about every way.  
> maybe someone else will jump in with their favorite bike.

As the original author I don't really care either way, I only care
about working around the assembler bug so I can turn on thumb for
tegra.  But maybe I'll rewrite it in prolog just to mess with you guys
:^)

-Allen
-- 
nvpublic

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-08-01 22:46               ` Allen Martin
@ 2012-08-01 22:55                 ` Graeme Russ
  2012-08-01 23:01                 ` Mike Frysinger
  1 sibling, 0 replies; 18+ messages in thread
From: Graeme Russ @ 2012-08-01 22:55 UTC (permalink / raw)
  To: u-boot

Hi Allen,

On Thu, Aug 2, 2012 at 8:46 AM, Allen Martin <amartin@nvidia.com> wrote:

[snip]

> As the original author I don't really care either way, I only care
> about working around the assembler bug so I can turn on thumb for
> tegra.  But maybe I'll rewrite it in prolog just to mess with you guys
> :^)

Honestly, I prefer the original version - It clearly shows what the code
is doing. I'm not an awk god, and I find it really difficult to figure
out what half of the fancy scripts littered about the Makefiles actually
do. If it doesn't impact on performance, I prefer clarity (to non awk
god-like creatures) the compactness.

Just my $0.02 worth

Regards,

Graeme

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-08-01 22:46               ` Allen Martin
  2012-08-01 22:55                 ` Graeme Russ
@ 2012-08-01 23:01                 ` Mike Frysinger
  2012-08-02 17:12                   ` Allen Martin
  1 sibling, 1 reply; 18+ messages in thread
From: Mike Frysinger @ 2012-08-01 23:01 UTC (permalink / raw)
  To: u-boot

On Wednesday 01 August 2012 18:46:18 Allen Martin wrote:
> But maybe I'll rewrite it in prolog just to mess with you guys

i'd ack it if it were written in bf
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120801/1350ba27/attachment.pgp>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-08-01 23:01                 ` Mike Frysinger
@ 2012-08-02 17:12                   ` Allen Martin
  2012-08-02 19:30                     ` Wolfgang Denk
  0 siblings, 1 reply; 18+ messages in thread
From: Allen Martin @ 2012-08-02 17:12 UTC (permalink / raw)
  To: u-boot

On Wed, Aug 01, 2012 at 04:01:41PM -0700, Mike Frysinger wrote:
> On Wednesday 01 August 2012 18:46:18 Allen Martin wrote:
> > But maybe I'll rewrite it in prolog just to mess with you guys
> 
> i'd ack it if it were written in bf
> -mike
> 

Challenge accepted

#!/bin/sh
#
# binutils-version [-p] gas-command
#
# Prints the binutils version of `gas-command' in a canonical 4-digit
#form
# such as `0222' for binutils 2.22
#

gas="$*"

if [ ${#gas} -eq 0 ]; then
        echo "Error: No assembler specified."
        printf "Usage:\n\t$0 <gas-command>\n"
        exit 1
fi

tmp=$(mktemp)
echo '>>++++[<++++++++>-]>>+++++[>+++++++++<-]>+>,[>>++++[<++++++++>-]>>+++++[>\
+++++++++<-]>+>,]<<<<<<[<<<<<<]>>>>>>[<[<+>-]>[<<-<+>>>-]<<<[>>>+<<<-]>[>+<[-]]\
>>>>>>>>]<<<<<<[<<<<<<]>>>>>[>>>>>>]><<<<<<<<<<<<[-]<[-]++++++[>++++++++<-]>.>>\
>>>>.>>>>>>>>>>>>.>>>>>>.' > $tmp
$gas --version | bf -n $tmp
rm $tmp

-Allen
-- 
nvpublic

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version
  2012-08-02 17:12                   ` Allen Martin
@ 2012-08-02 19:30                     ` Wolfgang Denk
  0 siblings, 0 replies; 18+ messages in thread
From: Wolfgang Denk @ 2012-08-02 19:30 UTC (permalink / raw)
  To: u-boot

Dear Allen Martin,

In message <20120802171230.GC7791@nvidia.com> you wrote:
>
> Challenge accepted
...
> tmp=$(mktemp)
> echo '>>++++[<++++++++>-]>>+++++[>+++++++++<-]>+>,[>>++++[<++++++++>-]>>+++++[>\
> +++++++++<-]>+>,]<<<<<<[<<<<<<]>>>>>>[<[<+>-]>[<<-<+>>>-]<<<[>>>+<<<-]>[>+<[-]]\
> >>>>>>>>]<<<<<<[<<<<<<]>>>>>[>>>>>>]><<<<<<<<<<<<[-]<[-]++++++[>++++++++<-]>.>>\
> >>>>.>>>>>>>>>>>>.>>>>>>.' > $tmp

Thanks.  Registered as entry # 1 for the IOUCC.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
On my planet, to rest is to rest -- to cease using energy. To me,  it
is  quite  illogical to run up and down on green grass, using energy,
instead of saving it.
	-- Spock, "Shore Leave", stardate 3025.2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH v4 2/2] arm: work around assembler bug
  2012-07-18 23:45 ` [U-Boot] [PATCH v4 2/2] arm: work around assembler bug Allen Martin
  2012-07-19  6:06   ` Albert ARIBAUD
@ 2012-10-04  8:47   ` Albert ARIBAUD
  1 sibling, 0 replies; 18+ messages in thread
From: Albert ARIBAUD @ 2012-10-04  8:47 UTC (permalink / raw)
  To: u-boot

Hi Allen,

On Wed, 18 Jul 2012 16:45:53 -0700, Allen Martin <amartin@nvidia.com>
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 v4:
>  -removed warning print
> changes for v3:
>  -split shell line to improve readability
> 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 |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> index 3f4453a..24b9d7c 100644
> --- a/arch/arm/config.mk
> +++ b/arch/arm/config.mk
> @@ -87,3 +87,21 @@ endif
>  ifndef CONFIG_NAND_SPL
>  LDFLAGS_u-boot += -pie
>  endif
> +
> +#
> +# FIXME: 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)
> +endif
> +ifeq ($(GAS_BUG_12532),y)
> +PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
> +endif
> +endif

Applied to u-boot-arm/master, thanks!

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-10-04  8:47 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-18 23:45 [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version Allen Martin
2012-07-18 23:45 ` [U-Boot] [PATCH v4 2/2] arm: work around assembler bug Allen Martin
2012-07-19  6:06   ` Albert ARIBAUD
2012-07-19 15:08     ` Tom Rini
2012-08-01 17:34       ` Allen Martin
2012-10-04  8:47   ` Albert ARIBAUD
2012-07-19  3:11 ` [U-Boot] [PATCH v4 1/2] tools, config.mk: add binutils-version Mike Frysinger
2012-07-19 15:08   ` Tom Rini
2012-07-19 15:21     ` Mike Frysinger
2012-07-19 15:38       ` Tom Rini
2012-07-19 16:43         ` Mike Frysinger
2012-07-19 16:54           ` Tom Rini
2012-08-01 22:31             ` Mike Frysinger
2012-08-01 22:46               ` Allen Martin
2012-08-01 22:55                 ` Graeme Russ
2012-08-01 23:01                 ` Mike Frysinger
2012-08-02 17:12                   ` Allen Martin
2012-08-02 19:30                     ` Wolfgang Denk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox