Fix build of *86*-linux-android with "--enable-shared"

Fix build of *86*-linux-android with "--enable-shared"

[Alexander Ivchenko]

Hi,

libcilkrts is compiled with "-nostdlib", that means we have to
explicitly specify the pthread library we should link with (e.g. we
don't have such problem with libgomp, because it is C). And, indeed,
"-lpthread" is hard-coded in the Makefile for cilkrts. For Android
this doesn't work, because lpthread is absent and pthreads are part of
libc.

I also noticed, that configure check for
"pthread_{,attr_}[sg]etaffinity_np" always fails, because at the point
where it is placed in configure.ac, "-pthread" is not set. We just
have to put this check after we added "-pthread" to CFLAGS. This patch
addresses this as well.



diff --git a/libcilkrts/ChangeLog b/libcilkrts/ChangeLog
index 3881c82..ab10a0b 100644
--- a/libcilkrts/ChangeLog
+++ b/libcilkrts/ChangeLog
@@ -1,3 +1,15 @@
+2014-08-01  Alexander Ivchenko  <alexander.ivchenko@intel.com>
+
+ * configure.ac: Move pthread affinity test to the place where
+ '-pthread' passed to CFLAGS. Otherwise the test always fails.
+ (XCFLAGS): New variable for correctly passing
+ '-pthread'.
+ (XLDFLAGS): New variable for passing the correct pthread lib.
+ * configure: Regenerate.
+ * Makefile.am (AM_CFLAGS): Add $XCFLAGS.
+ (AM_LDFLAGS): Add $XLDFLAGS.
+ * Makefile.in: Regenerate.
+
 2014-05-21  John Marino  <gnugcc@marino.st>

  * runtime/os-unix.c (__DragonFly__): New target.
diff --git a/libcilkrts/Makefile.am b/libcilkrts/Makefile.am
index 84551c8..70538a2 100644
--- a/libcilkrts/Makefile.am
+++ b/libcilkrts/Makefile.am
@@ -43,9 +43,9 @@ GENERAL_FLAGS = -I$(top_srcdir)/include
-I$(top_srcdir)/runtime -I$(top_srcdir)/
 # Enable Intel Cilk Plus extension
 GENERAL_FLAGS += -fcilkplus

-AM_CFLAGS = $(GENERAL_FLAGS) -std=c99
+AM_CFLAGS = $(XCFLAGS) $(GENERAL_FLAGS) -std=c99
 AM_CPPFLAGS = $(GENERAL_FLAGS)
-AM_LDFLAGS = -lpthread
+AM_LDFLAGS = $(XLDFLAGS)

 # May be used by toolexeclibdir.
 gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER)
@@ -92,7 +92,8 @@ include include/internal/rev.mk

 #libcilkrts_la_LDFLAGS  = -rpath '$(libdir)'
 libcilkrts_la_LDFLAGS = -version-info 5:0:0
-libcilkrts_la_LDFLAGS += -lpthread @lt_cv_dlopen_libs@
+libcilkrts_la_LDFLAGS += @lt_cv_dlopen_libs@
+libcilkrts_la_LDFLAGS += $(AM_LDFLAGS)

 # If we're building on Linux, use the Linux version script
 if LINUX_LINKER_SCRIPT
diff --git a/libcilkrts/configure.ac b/libcilkrts/configure.ac
index fb21505..cc9a39f 100644
--- a/libcilkrts/configure.ac
+++ b/libcilkrts/configure.ac
@@ -164,6 +164,27 @@ AC_SUBST(toolexeclibdir)

 AC_SUBST(lt_cv_dlopen_libs)

+# Check to see if -pthread or -lpthread is needed.  Prefer the former.
+# Note that the CILK_SELF_SPEC in gcc.c may force -pthread.
+# In case the pthread.h system header is not found, this test will fail.
+XCFLAGS=""
+XLDFLAGS=""
+CFLAGS="$CFLAGS -pthread"
+AC_LINK_IFELSE(
+ [AC_LANG_PROGRAM(
+  [#include <pthread.h>
+   void *g(void *d) { return NULL; }],
+  [pthread_t t; pthread_create(&t,NULL,g,NULL);])],
+ [XCFLAGS=" -Wc,-pthread"],
+ [CFLAGS="$save_CFLAGS" LIBS="-lpthread $LIBS"
+  AC_LINK_IFELSE(
+   [AC_LANG_PROGRAM(
+    [#include <pthread.h>
+     void *g(void *d) { return NULL; }],
+    [pthread_t t; pthread_create(&t,NULL,g,NULL);])],
+   [],
+   [AC_MSG_ERROR([Pthreads are required to build libcilkrts])])])
+
 # Check for pthread_{,attr_}[sg]etaffinity_np.
 AC_LINK_IFELSE(
  [AC_LANG_PROGRAM(
@@ -183,24 +204,24 @@ AC_LINK_IFELSE(
   AC_DEFINE(HAVE_PTHREAD_AFFINITY_NP, 1,
 [       Define if pthread_{,attr_}{g,s}etaffinity_np is supported.]))

-# Check to see if -pthread or -lpthread is needed.  Prefer the former.
-# Note that the CILK_SELF_SPEC in gcc.c may force -pthread.
-# In case the pthread.h system header is not found, this test will fail.
-CFLAGS="$CFLAGS -pthread"
-AC_LINK_IFELSE(
- [AC_LANG_PROGRAM(
-  [#include <pthread.h>
-   void *g(void *d) { return NULL; }],
-  [pthread_t t; pthread_create(&t,NULL,g,NULL);])],
- [],
- [CFLAGS="$save_CFLAGS" LIBS="-lpthread $LIBS"
-  AC_LINK_IFELSE(
-   [AC_LANG_PROGRAM(
-    [#include <pthread.h>
-     void *g(void *d) { return NULL; }],
-    [pthread_t t; pthread_create(&t,NULL,g,NULL);])],
-   [],
-   [AC_MSG_ERROR([Pthreads are required to build libcilkrts])])])
+# Every c++ lib is linking by default with -nostdlib, which leads to the
+# fact, that proper pthread library will not be given at link time. We have
+# to hard-code that.
+case "${target}" in
+
+  *android*)
+    XLDFLAGS="$XLDFLAGS -lc"
+     ;;
+  *)
+    XLDFLAGS="$XLDFLAGS -lpthread"
+    ;;
+
+esac
+
+AC_SUBST(XCFLAGS)
+AC_SUBST(XLDFLAGS)
+
+CFLAGS="$save_CFLAGS"

 if test $enable_shared = yes; then
   link_cilkrts="-lcilkrts %{static: $LIBS}"

Is it ok?

--Alexander

Re: Fix build of *86*-linux-android with "--enable-shared"

[Jeff Law]

On 08/04/14 00:08, Alexander Ivchenko wrote:
> Hi,
>
> libcilkrts is compiled with "-nostdlib", that means we have to
> explicitly specify the pthread library we should link with (e.g. we
> don't have such problem with libgomp, because it is C). And, indeed,
> "-lpthread" is hard-coded in the Makefile for cilkrts. For Android
> this doesn't work, because lpthread is absent and pthreads are part of
> libc.
>
> I also noticed, that configure check for
> "pthread_{,attr_}[sg]etaffinity_np" always fails, because at the point
> where it is placed in configure.ac, "-pthread" is not set. We just
> have to put this check after we added "-pthread" to CFLAGS. This patch
> addresses this as well.
>
>
>
> diff --git a/libcilkrts/ChangeLog b/libcilkrts/ChangeLog
> index 3881c82..ab10a0b 100644
> --- a/libcilkrts/ChangeLog
> +++ b/libcilkrts/ChangeLog
> @@ -1,3 +1,15 @@
> +2014-08-01  Alexander Ivchenko  <alexander.ivchenko@intel.com>
> +
> + * configure.ac: Move pthread affinity test to the place where
> + '-pthread' passed to CFLAGS. Otherwise the test always fails.
> + (XCFLAGS): New variable for correctly passing
> + '-pthread'.
> + (XLDFLAGS): New variable for passing the correct pthread lib.
> + * configure: Regenerate.
> + * Makefile.am (AM_CFLAGS): Add $XCFLAGS.
> + (AM_LDFLAGS): Add $XLDFLAGS.
> + * Makefile.in: Regenerate.
So can you confirm that you've bootstrapped this on 
x86_64-unknown-linux-gnu and that there were no regressions?  Also 
double-check the indention in the ChangeLog entry, though it may just be 
your mailer that has mucked that up.

Once the bootstrap and regression test are OK, this is OK.

jeff

Re: Fix build of *86*-linux-android with "--enable-shared"

[Alexander Ivchenko]

Thanks for looking at this.

Bootstrapped and reg-tested on x86_64-unknown-linux-gnu.



2014-08-06 0:18 GMT+04:00 Jeff Law <law@redhat.com>:
> On 08/04/14 00:08, Alexander Ivchenko wrote:
>>
>> Hi,
>>
>> libcilkrts is compiled with "-nostdlib", that means we have to
>> explicitly specify the pthread library we should link with (e.g. we
>> don't have such problem with libgomp, because it is C). And, indeed,
>> "-lpthread" is hard-coded in the Makefile for cilkrts. For Android
>> this doesn't work, because lpthread is absent and pthreads are part of
>> libc.
>>
>> I also noticed, that configure check for
>> "pthread_{,attr_}[sg]etaffinity_np" always fails, because at the point
>> where it is placed in configure.ac, "-pthread" is not set. We just
>> have to put this check after we added "-pthread" to CFLAGS. This patch
>> addresses this as well.
>>
>>
>>
>> diff --git a/libcilkrts/ChangeLog b/libcilkrts/ChangeLog
>> index 3881c82..ab10a0b 100644
>> --- a/libcilkrts/ChangeLog
>> +++ b/libcilkrts/ChangeLog
>> @@ -1,3 +1,15 @@
>> +2014-08-01  Alexander Ivchenko  <alexander.ivchenko@intel.com>
>> +
>> + * configure.ac: Move pthread affinity test to the place where
>> + '-pthread' passed to CFLAGS. Otherwise the test always fails.
>> + (XCFLAGS): New variable for correctly passing
>> + '-pthread'.
>> + (XLDFLAGS): New variable for passing the correct pthread lib.
>> + * configure: Regenerate.
>> + * Makefile.am (AM_CFLAGS): Add $XCFLAGS.
>> + (AM_LDFLAGS): Add $XLDFLAGS.
>> + * Makefile.in: Regenerate.
>
> So can you confirm that you've bootstrapped this on x86_64-unknown-linux-gnu
> and that there were no regressions?  Also double-check the indention in the
> ChangeLog entry, though it may just be your mailer that has mucked that up.
>
> Once the bootstrap and regression test are OK, this is OK.
>
> jeff
>