diff mbox

[v2,1/1] openpgm: disable on AVR32

Message ID 1383329548-8528-1-git-send-email-alexander.lukichev@gmail.com
State Superseded
Headers show

Commit Message

Alexander Lukichev Nov. 1, 2013, 6:12 p.m. UTC
openpgm doesn't build correctly on AVR32 using
gcc-4.2.2-avr32-2.1.5 toolchain: it is configured to call
intrinsic atomic functions not provided by the toolchain,
so they are propagated as unresolved external symbols in the
built openpgm libraries. This breaks programs that try to link
openpgm, because they do not know where to get those either. For
instance, it breaks building zeromq tests when PGM support is
selected.

This commit disables openpgm on AVR32 due to apparent absence of
interest in this package on that architecture and it breaking too
many test builds.

Fixes http://autobuild.buildroot.net/results/5a3261109ea63ba17375003eabd8b5d88757865f/
(at least)

Signed-off-by: Alexander Lukichev <alexander.lukichev@gmail.com>
---
v2:
  - removed KConfig comment that openpgm is broken on AVR32,
    commented instead in its Config.in
  - added package name which disables PGM support for zeromq on
    AVR32
---
 package/openpgm/Config.in | 2 ++
 package/zeromq/Config.in  | 1 +
 2 files changed, 3 insertions(+)

Comments

Thomas Petazzoni Nov. 1, 2013, 6:20 p.m. UTC | #1
Dear Alexander Lukichev,

On Fri,  1 Nov 2013 20:12:28 +0200, Alexander Lukichev wrote:

> +	depends on !BR2_avr32 # incorrectly configures for the toolchain

This comment is somewhat misleading. What really happens is that
openpgm uses compiler intrinsics that did not exist in gcc 4.2. So
maybe the test should actually be on BR2_GCC_VERSION_4_2_2_AVR32_2_1_5
instead of BR2_avr32 (even though it's admittedly quite the same thing,
since AVR32 will never use any other version that this heavily patched
gcc 4.2).

I've added Simon Dawson in the CC list, who is the main contributor to
the AVR32 support in Buildroot.

Best regards,

Thomas
Alexander Lukichev Nov. 1, 2013, 8:21 p.m. UTC | #2
Hi Thomas, Simon,

The compiler (that particular toolchain) has those intrinsic functions but
is able to generate them only if the exact CPU sub architecture is
specified, not just AVR32 (their implementations differ for each of the two
sub architectures). This is impossible to specify unless we introduce it in
the buildroot configuration system (like several flavors of ARM). And this,
in turn, seems too much to make just one package happy.

I tried to disable using them in the package code by playing with
configure.ac but unsuccessfully: the code used instead some thread
functions that were not provided by the toolchain (at least, not with the
constants defined in configure). In the end, it seemed too much trouble
since nobody showed interest in the package, especially on this
architecture, and it breaks builds for AVR32.

I did not know how to explain all this concisely in openpgm's Config.in,
sorry :-)

Best regards,
  Alexander Lukichev
Alexander Lukichev Nov. 2, 2013, 7:16 a.m. UTC | #3
Hi,

2013/11/1 Alexander Lukichev <alexander.lukichev@gmail.com>

> The compiler (that particular toolchain) has those intrinsic functions

> but is able to generate them only if the exact CPU sub architecture

> is specified, not just AVR32 (their implementations differ for each of

> the two sub architectures)
....
> I tried to disable using them in the package code by playing with
> configure.ac but unsuccessfully: the code used instead some thread
> functions that were not provided by the toolchain (at least, not with
> the constants defined in configure). In the end, it seemed too much
...
> I did not know how to explain all this concisely in openpgm's Config.in,
> sorry :-)

So, since in both cases the project was configured incorrectly for building
by
that toolchain, the comment I have put in Config.in did not seem misleading
to me.

--
Best regards,
  Alexander Lukichev
Simon Dawson Nov. 2, 2013, 8:03 a.m. UTC | #4
Hi Alexander.

On 1 November 2013 20:21, Alexander Lukichev
<alexander.lukichev@gmail.com> wrote:
> The compiler (that particular toolchain) has those intrinsic functions but
> is able to generate them only if the exact CPU sub architecture is
> specified, not just AVR32 (their implementations differ for each of the two
> sub architectures). This is impossible to specify unless we introduce it in
> the buildroot configuration system (like several flavors of ARM). And this,
> in turn, seems too much to make just one package happy.

I might be wrong, but is it not the case that only one of the two
micro-architectures --- specifically, avr32b as used by the ap7 core
--- is relevant for Buildroot? (I'm not aware of a Linux port for the
avr32a architecture, as used by the uc3 core.)

If so, then it seems to me that it would be well worth calling the
compiler with the precise micro-architecture specified. This might be
expected to have a positive impact beyond just the openpgm package.

Simon.
Thomas Petazzoni Nov. 2, 2013, 11:26 a.m. UTC | #5
Dear Simon Dawson,

On Sat, 2 Nov 2013 08:03:51 +0000, Simon Dawson wrote:
> Hi Alexander.
> 
> On 1 November 2013 20:21, Alexander Lukichev
> <alexander.lukichev@gmail.com> wrote:
> > The compiler (that particular toolchain) has those intrinsic functions but
> > is able to generate them only if the exact CPU sub architecture is
> > specified, not just AVR32 (their implementations differ for each of the two
> > sub architectures). This is impossible to specify unless we introduce it in
> > the buildroot configuration system (like several flavors of ARM). And this,
> > in turn, seems too much to make just one package happy.
> 
> I might be wrong, but is it not the case that only one of the two
> micro-architectures --- specifically, avr32b as used by the ap7 core
> --- is relevant for Buildroot? (I'm not aware of a Linux port for the
> avr32a architecture, as used by the uc3 core.)
> 
> If so, then it seems to me that it would be well worth calling the
> compiler with the precise micro-architecture specified. This might be
> expected to have a positive impact beyond just the openpgm package.

I did a minimal test case, and even by specifying -march and -mpart,
sync_add_and_fetch_2 remains an undefined symbol. Interestingly, if you
use sync_add_and_fetch() on a pointer to a 4 bytes value, there's no
problem, but if it's a pointer to a 2 bytes value, then the intrinsics
uses sync_add_and_fetch_2, which doesn't work.

Test case with a pointer to a 4 bytes value
===========================================

$ cat toto.c 
int foo(void) {
	unsigned int *ptr;
	return __sync_add_and_fetch(ptr, 2);
}
$ avr32-linux-gcc -march=ap -mpart=ap7200 -fPIC -c toto.c 
$ avr32-linux-gcc -march=ap -mpart=ap7200 -fPIC -shared -Wl,-soname,libtoto.so.1 -o libtoto.so.1 toto.o 
$ avr32-linux-readelf -a libtoto.so.1 | grep sync_add
$

 => no problem

Test case with a pointer to a 2 bytes value
===========================================

$ cat toto.c 
int foo(void) {
	unsigned short *ptr;
	return __sync_add_and_fetch(ptr, 2);
}
$ avr32-linux-gcc -march=ap -mpart=ap7200 -fPIC -c toto.c 
$ avr32-linux-gcc -march=ap -mpart=ap7200 -fPIC -shared -Wl,-soname,libtoto.so.1 -o libtoto.so.1 toto.o 
$ avr32-linux-readelf -a libtoto.so.1 | grep sync_add
0000149c  00000b29 R_AVR32_GLOB_DAT  00000000   __sync_add_and_fetch_2 + 0
    11: 00000000     0 NOTYPE  GLOBAL DEFAULT  UND __sync_add_and_fetch_2
    43: 00000000     0 NOTYPE  GLOBAL DEFAULT  UND __sync_add_and_fetch_2
$

 => unresolved symbol

Any idea?

Best regards,

Thomas
Simon Dawson Nov. 2, 2013, 4:51 p.m. UTC | #6
Hi Thomas, Alexander.

On 2 November 2013 11:26, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> I did a minimal test case, and even by specifying -march and -mpart,
> sync_add_and_fetch_2 remains an undefined symbol. Interestingly, if you
> use sync_add_and_fetch() on a pointer to a 4 bytes value, there's no
> problem, but if it's a pointer to a 2 bytes value, then the intrinsics
> uses sync_add_and_fetch_2, which doesn't work.

Interesting. Alexander: are we missing a trick here? How did you get
the compiler to use the correct intrinsics?

Simon.
Alexander Lukichev Nov. 3, 2013, 11:24 a.m. UTC | #7
Hi Simon, Thomas,

11/02/2013 06:51 PM, Simon Dawson wrote:
> Interesting. Alexander: are we missing a trick here? How did you get
> the compiler to use the correct intrinsics?

I do not remember. I'll take a look tomorrow on my faster laptop. It
may even be that I have mixed myself up then.
Alexander Lukichev Nov. 6, 2013, 5:27 p.m. UTC | #8
Hi Simon, Thomas,

2013/11/2 Simon Dawson <spdawson@gmail.com>

> On 2 November 2013 11:26, Thomas Petazzoni wrote:
> > I did a minimal test case, and even by specifying -march and -mpart,
> > sync_add_and_fetch_2 remains an undefined symbol. Interestingly, if you
> > use sync_add_and_fetch() on a pointer to a 4 bytes value, there's no
> > problem, but if it's a pointer to a 2 bytes value, then the intrinsics
> > uses sync_add_and_fetch_2, which doesn't work.
>
> Interesting. Alexander: are we missing a trick here? How did you get
> the compiler to use the correct intrinsics?
>
>
No, you don't. I must have hallucinated the whole thing.

I have now tried to check with a locally built toolchain, and it also could
not
generate 2-byte versions of those functions, for all 3 -march= alternatives
(ap,
ucr1, ucr2; I remember that I used ucr2 previously). Nor could I find the
code
for them in GCC sources (I remember vaguely that I stumbled upon it during
my
initial attempts to pacify openpgm).

In addition, the document http://www.atmel.com/Images/doc32074.pdf on page 8
indeed claims to support 4-byte (int) versions of those functions, saying
nothing
about other data types.

I apologize both for the delay and for making other people wasting time on
this.

--
Best regards,
  Alexander Lukichev
diff mbox

Patch

diff --git a/package/openpgm/Config.in b/package/openpgm/Config.in
index cae74f7..ef0f336 100644
--- a/package/openpgm/Config.in
+++ b/package/openpgm/Config.in
@@ -1,5 +1,6 @@ 
 config BR2_PACKAGE_OPENPGM
 	bool "openpgm"
+	depends on !BR2_avr32 # incorrectly configures for the toolchain
 	depends on BR2_TOOLCHAIN_HAS_THREADS
 	depends on BR2_INET_IPV6
 	depends on BR2_USE_WCHAR
@@ -14,3 +15,4 @@  config BR2_PACKAGE_OPENPGM
 
 comment "openpgm needs a toolchain w/ wchar, threads, IPv6"
 	depends on !(BR2_TOOLCHAIN_HAS_THREADS && BR2_INET_IPV6 && BR2_USE_WCHAR)
+	depends on !BR2_avr32
diff --git a/package/zeromq/Config.in b/package/zeromq/Config.in
index 42e13d2..a74ce88 100644
--- a/package/zeromq/Config.in
+++ b/package/zeromq/Config.in
@@ -30,6 +30,7 @@  config BR2_PACKAGE_ZEROMQ
 config BR2_PACKAGE_ZEROMQ_PGM
 	bool "PGM/EPGM support"
 	depends on BR2_PACKAGE_ZEROMQ
+	depends on !BR2_avr32 # openpgm
 	select BR2_PACKAGE_OPENPGM
 	help
 	  Add support for Pragmatic General Multicast protocol (RFC 3208)