[meta-networking][PATCH 1/2] grpc: Link with libatomic on clang/x86

[meta-networking][PATCH 1/2] grpc: Link with libatomic on clang/x86

[Khem Raj]

clang does delegate the atomic<double> calls to libatomic on x86 where
as gcc tries to use intrinsics, its debatable who is right, but it does
seem that clang is safe in case pointer is unaligned

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 meta-networking/recipes-devtools/grpc/grpc_1.24.3.bb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meta-networking/recipes-devtools/grpc/grpc_1.24.3.bb b/meta-networking/recipes-devtools/grpc/grpc_1.24.3.bb
index ecc575bcfb..24072c0832 100644
--- a/meta-networking/recipes-devtools/grpc/grpc_1.24.3.bb
+++ b/meta-networking/recipes-devtools/grpc/grpc_1.24.3.bb
@@ -42,6 +42,10 @@ do_configure_prepend_mipsarch() {
     sed -i -e "s/set(_gRPC_ALLTARGETS_LIBRARIES \${CMAKE_DL_LIBS} rt m pthread)/set(_gRPC_ALLTARGETS_LIBRARIES \${CMAKE_DL_LIBS} atomic rt m pthread)/g" ${S}/CMakeLists.txt
 }
 
+do_configure_prepend_toolchain-clang_x86() {
+    sed -i -e "s/set(_gRPC_ALLTARGETS_LIBRARIES \${CMAKE_DL_LIBS} rt m pthread)/set(_gRPC_ALLTARGETS_LIBRARIES \${CMAKE_DL_LIBS} atomic rt m pthread)/g" ${S}/CMakeLists.txt
+}
+
 BBCLASSEXTEND = "native nativesdk"
 
 SYSROOT_DIRS_BLACKLIST_append_class-target = "${libdir}/cmake/grpc"
-- 
2.24.0

[meta-oe][PATCH 2/2] upm: Link with libatomic on clang/x86

[Khem Raj]

This is needed for atomic<double> which clang does not use intrinsic
function for on 32bit x86

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 .../upm/0001-nmea_gps-Link-with-latomic.patch | 27 +++++++++++++++++++
 meta-oe/recipes-extended/upm/upm_git.bb       |  2 ++
 2 files changed, 29 insertions(+)
 create mode 100644 meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch

diff --git a/meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch b/meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch
new file mode 100644
index 0000000000..c16b8dbc5a
--- /dev/null
+++ b/meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch
@@ -0,0 +1,27 @@
+From 996d37fc0b7177ee57788399b9140032d5de2765 Mon Sep 17 00:00:00 2001
+From: Khem Raj <raj.khem@gmail.com>
+Date: Mon, 18 Nov 2019 15:50:02 -0800
+Subject: [PATCH] nmea_gps: Link with latomic
+
+clang/x86 ends up with missing symbols for atomics due to atomic<double>
+with libstdc++, it works ok with libc++
+
+Upstream-Status: Pending
+Signed-off-by: Khem Raj <raj.khem@gmail.com>
+---
+ src/nmea_gps/CMakeLists.txt | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/nmea_gps/CMakeLists.txt b/src/nmea_gps/CMakeLists.txt
+index b3fb6591..5280bfed 100644
+--- a/src/nmea_gps/CMakeLists.txt
++++ b/src/nmea_gps/CMakeLists.txt
+@@ -6,4 +6,4 @@ upm_mixed_module_init (NAME nmea_gps
+     CPP_SRC nmea_gps.cxx
+     FTI_SRC nmea_gps_fti.c
+     CPP_WRAPS_C
+-    REQUIRES mraa utilities-c ${CMAKE_THREAD_LIBS_INIT})
++    REQUIRES mraa utilities-c atomic ${CMAKE_THREAD_LIBS_INIT})
+-- 
+2.24.0
+
diff --git a/meta-oe/recipes-extended/upm/upm_git.bb b/meta-oe/recipes-extended/upm/upm_git.bb
index fb22e83359..dbe97370d5 100644
--- a/meta-oe/recipes-extended/upm/upm_git.bb
+++ b/meta-oe/recipes-extended/upm/upm_git.bb
@@ -15,6 +15,8 @@ SRC_URI = "git://github.com/intel-iot-devkit/${BPN}.git;protocol=http \
            file://0001-Use-stdint-types.patch \
            "
 
+SRC_URI_append_toolchain-clang_x86 = " file://0001-nmea_gps-Link-with-latomic.patch "
+
 S = "${WORKDIR}/git"
 
 # Depends on mraa which only supports x86 and ARM for now
-- 
2.24.0

Re: [meta-oe][PATCH 2/2] upm: Link with libatomic on clang/x86

[André Draszik]

On Mon, 2019-11-18 at 17:16 -0800, Khem Raj wrote:
> This is needed for atomic<double> which clang does not use intrinsic
> function for on 32bit x86
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  .../upm/0001-nmea_gps-Link-with-latomic.patch | 27 +++++++++++++++++++
>  meta-oe/recipes-extended/upm/upm_git.bb       |  2 ++
>  2 files changed, 29 insertions(+)
>  create mode 100644 meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch
> 
> diff --git a/meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch b/meta-oe/recipes-
> extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch
> new file mode 100644
> index 0000000000..c16b8dbc5a
> --- /dev/null
> +++ b/meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch
> @@ -0,0 +1,27 @@
> +From 996d37fc0b7177ee57788399b9140032d5de2765 Mon Sep 17 00:00:00 2001
> +From: Khem Raj <raj.khem@gmail.com>
> +Date: Mon, 18 Nov 2019 15:50:02 -0800
> +Subject: [PATCH] nmea_gps: Link with latomic
> +
> +clang/x86 ends up with missing symbols for atomics due to atomic<double>
> +with libstdc++, it works ok with libc++
> +
> +Upstream-Status: Pending
> +Signed-off-by: Khem Raj <raj.khem@gmail.com>
> +---
> + src/nmea_gps/CMakeLists.txt | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/src/nmea_gps/CMakeLists.txt b/src/nmea_gps/CMakeLists.txt
> +index b3fb6591..5280bfed 100644
> +--- a/src/nmea_gps/CMakeLists.txt
> ++++ b/src/nmea_gps/CMakeLists.txt
> +@@ -6,4 +6,4 @@ upm_mixed_module_init (NAME nmea_gps
> +     CPP_SRC nmea_gps.cxx
> +     FTI_SRC nmea_gps_fti.c
> +     CPP_WRAPS_C
> +-    REQUIRES mraa utilities-c ${CMAKE_THREAD_LIBS_INIT})
> ++    REQUIRES mraa utilities-c atomic ${CMAKE_THREAD_LIBS_INIT})
> +-- 
> +2.24.0
> +
> diff --git a/meta-oe/recipes-extended/upm/upm_git.bb b/meta-oe/recipes-extended/upm/upm_git.bb
> index fb22e83359..dbe97370d5 100644
> --- a/meta-oe/recipes-extended/upm/upm_git.bb
> +++ b/meta-oe/recipes-extended/upm/upm_git.bb
> @@ -15,6 +15,8 @@ SRC_URI = "git://github.com/intel-iot-devkit/${BPN}.git;protocol=http \
>             file://0001-Use-stdint-types.patch \
>             "
>  
> +SRC_URI_append_toolchain-clang_x86 = " file://0001-nmea_gps-Link-with-latomic.patch "
> +
>  S = "${WORKDIR}/git"
>  
>  # Depends on mraa which only supports x86 and ARM for now
> -- 
> 2.24.0
> 

If you're planning to upstream this, I normally use something like this in CMake:

set(ATOMIC_TEST_C_SOURCE "
#include <stdatomic.h>
#include <stdint.h>
atomic_uint_fast64_t x;
atomic_uint_fast64_t y;
int main() {
    return (int)(x + y);
}")


    # libatomic check - some arches need to link against libatomic.so, some don't
    include(CheckCSourceCompiles)
    include(CMakePushCheckState)
    include(CheckLibraryExists)
    cmake_push_check_state()
    check_c_source_compiles("${ATOMIC_TEST_C_SOURCE}" HAVE_ATOMICS_WITHOUT_LIBATOMIC)
    if(NOT HAVE_ATOMICS_WITHOUT_LIBATOMIC)
        check_library_exists(atomic __atomic_load_8 "" LIBATOMIC_EXISTS)
        if(LIBATOMIC_EXISTS)
            set(CMAKE_REQUIRED_LIBRARIES atomic)
            check_c_source_compiles("${ATOMIC_TEST_C_SOURCE}" HAVE_ATOMICS_WITH_LIBATOMIC)
        endif()
        if(HAVE_ATOMICS_WITH_LIBATOMIC)
            list(APPEND EXTRA_LIBRARIES atomic)
        else()
            message(FATAL_ERROR "Could not determine support for atomic operations.")
        endif()
    endif()
    cmake_pop_check_state()


Cheers,
A.

Re: [meta-oe][PATCH 2/2] upm: Link with libatomic on clang/x86

[Khem Raj]

On Thu, Nov 21, 2019 at 11:06 PM André Draszik <git@andred.net> wrote:
>
> On Mon, 2019-11-18 at 17:16 -0800, Khem Raj wrote:
> > This is needed for atomic<double> which clang does not use intrinsic
> > function for on 32bit x86
> >
> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > ---
> >  .../upm/0001-nmea_gps-Link-with-latomic.patch | 27 +++++++++++++++++++
> >  meta-oe/recipes-extended/upm/upm_git.bb       |  2 ++
> >  2 files changed, 29 insertions(+)
> >  create mode 100644 meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch
> >
> > diff --git a/meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch b/meta-oe/recipes-
> > extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch
> > new file mode 100644
> > index 0000000000..c16b8dbc5a
> > --- /dev/null
> > +++ b/meta-oe/recipes-extended/upm/upm/0001-nmea_gps-Link-with-latomic.patch
> > @@ -0,0 +1,27 @@
> > +From 996d37fc0b7177ee57788399b9140032d5de2765 Mon Sep 17 00:00:00 2001
> > +From: Khem Raj <raj.khem@gmail.com>
> > +Date: Mon, 18 Nov 2019 15:50:02 -0800
> > +Subject: [PATCH] nmea_gps: Link with latomic
> > +
> > +clang/x86 ends up with missing symbols for atomics due to atomic<double>
> > +with libstdc++, it works ok with libc++
> > +
> > +Upstream-Status: Pending
> > +Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > +---
> > + src/nmea_gps/CMakeLists.txt | 2 +-
> > + 1 file changed, 1 insertion(+), 1 deletion(-)
> > +
> > +diff --git a/src/nmea_gps/CMakeLists.txt b/src/nmea_gps/CMakeLists.txt
> > +index b3fb6591..5280bfed 100644
> > +--- a/src/nmea_gps/CMakeLists.txt
> > ++++ b/src/nmea_gps/CMakeLists.txt
> > +@@ -6,4 +6,4 @@ upm_mixed_module_init (NAME nmea_gps
> > +     CPP_SRC nmea_gps.cxx
> > +     FTI_SRC nmea_gps_fti.c
> > +     CPP_WRAPS_C
> > +-    REQUIRES mraa utilities-c ${CMAKE_THREAD_LIBS_INIT})
> > ++    REQUIRES mraa utilities-c atomic ${CMAKE_THREAD_LIBS_INIT})
> > +--
> > +2.24.0
> > +
> > diff --git a/meta-oe/recipes-extended/upm/upm_git.bb b/meta-oe/recipes-extended/upm/upm_git.bb
> > index fb22e83359..dbe97370d5 100644
> > --- a/meta-oe/recipes-extended/upm/upm_git.bb
> > +++ b/meta-oe/recipes-extended/upm/upm_git.bb
> > @@ -15,6 +15,8 @@ SRC_URI = "git://github.com/intel-iot-devkit/${BPN}.git;protocol=http \
> >             file://0001-Use-stdint-types.patch \
> >             "
> >
> > +SRC_URI_append_toolchain-clang_x86 = " file://0001-nmea_gps-Link-with-latomic.patch "
> > +
> >  S = "${WORKDIR}/git"
> >
> >  # Depends on mraa which only supports x86 and ARM for now
> > --
> > 2.24.0
> >
>
> If you're planning to upstream this, I normally use something like this in CMake:
>

this is certainly a better fix. I do not plan to work on upstreaming it.

> set(ATOMIC_TEST_C_SOURCE "
> #include <stdatomic.h>
> #include <stdint.h>
> atomic_uint_fast64_t x;
> atomic_uint_fast64_t y;

the real problem is double atomic types which gcc uses intrinsics on
x86/32bit but clang decides to defer it to libatomic
so something like this

atomic<double> x

would be needed as well check

> int main() {
>     return (int)(x + y);
> }")
>
>
>     # libatomic check - some arches need to link against libatomic.so, some don't
>     include(CheckCSourceCompiles)
>     include(CMakePushCheckState)
>     include(CheckLibraryExists)
>     cmake_push_check_state()
>     check_c_source_compiles("${ATOMIC_TEST_C_SOURCE}" HAVE_ATOMICS_WITHOUT_LIBATOMIC)
>     if(NOT HAVE_ATOMICS_WITHOUT_LIBATOMIC)
>         check_library_exists(atomic __atomic_load_8 "" LIBATOMIC_EXISTS)
>         if(LIBATOMIC_EXISTS)
>             set(CMAKE_REQUIRED_LIBRARIES atomic)
>             check_c_source_compiles("${ATOMIC_TEST_C_SOURCE}" HAVE_ATOMICS_WITH_LIBATOMIC)
>         endif()
>         if(HAVE_ATOMICS_WITH_LIBATOMIC)
>             list(APPEND EXTRA_LIBRARIES atomic)
>         else()
>             message(FATAL_ERROR "Could not determine support for atomic operations.")
>         endif()
>     endif()
>     cmake_pop_check_state()
>
>
> Cheers,
> A.
>
>
> --
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-devel