[PATCHv2] configure: libatomic check

Message ID 20170216202624.8535-1-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Feb. 16, 2017, 8:26 p.m.
upcoming patch ip fragmentation example fails to with
gcc 4.8.4 on linking __atomic_compare_exchange_16 functions
on x86. For some version of gcc both -latomic and -mcx16
needs to be provided in that case. For clang only -mcx16. That
options are set internally for platform but do not set for examples.
This patch unhides setting this options, make them common and printed
on configure log.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

---
 v2: unhide options and make frag code compile.
 CI now happy:
 https://travis-ci.org/muvarov/odp/builds/202380977


 configure.ac                           | 16 ----------
 platform/linux-generic/m4/configure.m4 | 57 ++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 16 deletions(-)

-- 
2.11.0.295.gd7dffce

Comments

Maxim Uvarov Feb. 17, 2017, 7:01 p.m. | #1
I need somebody to review this patch. ip frag example required this
change to compile in our env. CI things. Looks like ip frag is ready to
be merged and this patch is show stopper this that.

Thank you,
Maxim.

On 02/16/17 23:26, Maxim Uvarov wrote:
> upcoming patch ip fragmentation example fails to with

> gcc 4.8.4 on linking __atomic_compare_exchange_16 functions

> on x86. For some version of gcc both -latomic and -mcx16

> needs to be provided in that case. For clang only -mcx16. That

> options are set internally for platform but do not set for examples.

> This patch unhides setting this options, make them common and printed

> on configure log.

> 

> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

> ---

>  v2: unhide options and make frag code compile.

>  CI now happy:

>  https://travis-ci.org/muvarov/odp/builds/202380977

> 

> 

>  configure.ac                           | 16 ----------

>  platform/linux-generic/m4/configure.m4 | 57 ++++++++++++++++++++++++++++++++++

>  2 files changed, 57 insertions(+), 16 deletions(-)

> 

> diff --git a/configure.ac b/configure.ac

> index 6153efd2..a514f6be 100644

> --- a/configure.ac

> +++ b/configure.ac

> @@ -314,22 +314,6 @@ ODP_CFLAGS="$ODP_CFLAGS -std=c99"

>  # Extra flags for example to suppress certain warning types

>  ODP_CFLAGS="$ODP_CFLAGS $ODP_CFLAGS_EXTRA"

>  

> -#########################################################################

> -# Check if compiler supports cmpxchng16

> -##########################################################################

> -if test "${CC}" != "gcc" -o ${CC_VERSION_MAJOR} -ge 5; then

> -   my_save_cflags="$CFLAGS"

> -

> -   CFLAGS=-mcx16

> -   AC_MSG_CHECKING([whether CC supports -mcx16])

> -   AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])],

> -	    [AC_MSG_RESULT([yes])]

> -	    [ODP_CFLAGS="$ODP_CFLAGS $CFLAGS"],

> -	    [AC_MSG_RESULT([no])]

> -	    )

> -   CFLAGS="$my_save_cflags"

> -fi

> -

>  ##########################################################################

>  # Default include setup

>  ##########################################################################

> diff --git a/platform/linux-generic/m4/configure.m4 b/platform/linux-generic/m4/configure.m4

> index d3e5528c..92172d4b 100644

> --- a/platform/linux-generic/m4/configure.m4

> +++ b/platform/linux-generic/m4/configure.m4

> @@ -28,6 +28,63 @@ AC_LINK_IFELSE(

>      echo "Use newer version. For gcc > 4.7.0"

>      exit -1)

>  

> +#########################################################################

> +# Check if compiler supports cmpxchng16

> +##########################################################################

> +my_save_cflags="$CFLAGS"

> +

> +CFLAGS=-mcx16

> +AC_MSG_CHECKING([whether CC supports -mcx16])

> +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])],

> +    [AC_MSG_RESULT([yes])]

> +    [MCX16_CFLAGS="-mcx16"],

> +    [AC_MSG_RESULT([no])]

> +    )

> +CFLAGS="$my_save_cflags"

> +

> +#########################################################################

> +# Check if compiler supports __atomic_compare_exchange

> +##########################################################################

> +my_save_cflags="$CFLAGS"

> +LD_LIBATOMIC=""

> +

> +AC_MSG_CHECKING(if libatomic required)

> +AC_LINK_IFELSE(

> +    [AC_LANG_SOURCE(

> +      [[int main() {

> +        unsigned __int128 x = 0, y = 0;

> +        y = __atomic_load_n(&x, 0);

> +        __atomic_store_n(&x, y, 0);

> +        __atomic_compare_exchange_n(&x, &y, x, 0, 0, 0);

> +        return 0;

> +        }

> +    ]])],

> +    AC_MSG_RESULT(yes)

> +    LD_LIBATOMIC="-latomic"

> +    echo "adding -lbatomic",

> +    AC_MSG_RESULT(no))

> +

> +CFLAGS="$my_save_cflags $LD_LIBATOMIC $MCX16_CFLAGS"

> +AC_MSG_CHECKING([CFLAGS=$CFLAGS])

> +AC_LINK_IFELSE(

> +    [AC_LANG_SOURCE(

> +      [[int main() {

> +        unsigned __int128 x = 0, y = 0;

> +        y = __atomic_load_n(&x, 0);

> +        __atomic_store_n(&x, y, 0);

> +        __atomic_compare_exchange_n(&x, &y, x, 0, 0, 0);

> +        return 0;

> +        }

> +    ]])],

> +    AC_MSG_RESULT(yes),

> +    AC_MSG_RESULT(no)

> +    echo "atomic operations compilation fail."

> +    exit -1)

> +

> +# Restore LDFLAGS

> +CFLAGS="$my_save_cflags $MCX16_CFLAGS"

> +LDFLAGS="$LDFLAGS $LD_LIBATOMIC"

> +

>  m4_include([platform/linux-generic/m4/odp_pthread.m4])

>  m4_include([platform/linux-generic/m4/odp_openssl.m4])

>  m4_include([platform/linux-generic/m4/odp_pcap.m4])

>
Christophe Milard Feb. 20, 2017, 4:10 p.m. | #2
Hi,

If these flags are required for compiling the examples, maybe, they
should not be moved to the platform m4.
But the problem is maybe more: Why do the examples (which are supposed
to be compilable by all) requires this?

So maybe this change is good, and will trigger the example question...

This seems to fix an issue, anyway,  so I am positive to apply this patch

Christophe

On 16 February 2017 at 21:26, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> upcoming patch ip fragmentation example fails to with

> gcc 4.8.4 on linking __atomic_compare_exchange_16 functions

> on x86. For some version of gcc both -latomic and -mcx16

> needs to be provided in that case. For clang only -mcx16. That

> options are set internally for platform but do not set for examples.

> This patch unhides setting this options, make them common and printed

> on configure log.


Reviewed-by: Christophe Milard <christophe.milard@linaro.org>


>

> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

> ---

>  v2: unhide options and make frag code compile.

>  CI now happy:

>  https://travis-ci.org/muvarov/odp/builds/202380977

>

>

>  configure.ac                           | 16 ----------

>  platform/linux-generic/m4/configure.m4 | 57 ++++++++++++++++++++++++++++++++++

>  2 files changed, 57 insertions(+), 16 deletions(-)

>

> diff --git a/configure.ac b/configure.ac

> index 6153efd2..a514f6be 100644

> --- a/configure.ac

> +++ b/configure.ac

> @@ -314,22 +314,6 @@ ODP_CFLAGS="$ODP_CFLAGS -std=c99"

>  # Extra flags for example to suppress certain warning types

>  ODP_CFLAGS="$ODP_CFLAGS $ODP_CFLAGS_EXTRA"

>

> -#########################################################################

> -# Check if compiler supports cmpxchng16

> -##########################################################################

> -if test "${CC}" != "gcc" -o ${CC_VERSION_MAJOR} -ge 5; then

> -   my_save_cflags="$CFLAGS"

> -

> -   CFLAGS=-mcx16

> -   AC_MSG_CHECKING([whether CC supports -mcx16])

> -   AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])],

> -           [AC_MSG_RESULT([yes])]

> -           [ODP_CFLAGS="$ODP_CFLAGS $CFLAGS"],

> -           [AC_MSG_RESULT([no])]

> -           )

> -   CFLAGS="$my_save_cflags"

> -fi

> -

>  ##########################################################################

>  # Default include setup

>  ##########################################################################

> diff --git a/platform/linux-generic/m4/configure.m4 b/platform/linux-generic/m4/configure.m4

> index d3e5528c..92172d4b 100644

> --- a/platform/linux-generic/m4/configure.m4

> +++ b/platform/linux-generic/m4/configure.m4

> @@ -28,6 +28,63 @@ AC_LINK_IFELSE(

>      echo "Use newer version. For gcc > 4.7.0"

>      exit -1)

>

> +#########################################################################

> +# Check if compiler supports cmpxchng16

> +##########################################################################

> +my_save_cflags="$CFLAGS"

> +

> +CFLAGS=-mcx16

> +AC_MSG_CHECKING([whether CC supports -mcx16])

> +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])],

> +    [AC_MSG_RESULT([yes])]

> +    [MCX16_CFLAGS="-mcx16"],

> +    [AC_MSG_RESULT([no])]

> +    )

> +CFLAGS="$my_save_cflags"

> +

> +#########################################################################

> +# Check if compiler supports __atomic_compare_exchange

> +##########################################################################

> +my_save_cflags="$CFLAGS"

> +LD_LIBATOMIC=""

> +

> +AC_MSG_CHECKING(if libatomic required)

> +AC_LINK_IFELSE(

> +    [AC_LANG_SOURCE(

> +      [[int main() {

> +        unsigned __int128 x = 0, y = 0;

> +        y = __atomic_load_n(&x, 0);

> +        __atomic_store_n(&x, y, 0);

> +        __atomic_compare_exchange_n(&x, &y, x, 0, 0, 0);

> +        return 0;

> +        }

> +    ]])],

> +    AC_MSG_RESULT(yes)

> +    LD_LIBATOMIC="-latomic"

> +    echo "adding -lbatomic",

> +    AC_MSG_RESULT(no))

> +

> +CFLAGS="$my_save_cflags $LD_LIBATOMIC $MCX16_CFLAGS"

> +AC_MSG_CHECKING([CFLAGS=$CFLAGS])

> +AC_LINK_IFELSE(

> +    [AC_LANG_SOURCE(

> +      [[int main() {

> +        unsigned __int128 x = 0, y = 0;

> +        y = __atomic_load_n(&x, 0);

> +        __atomic_store_n(&x, y, 0);

> +        __atomic_compare_exchange_n(&x, &y, x, 0, 0, 0);

> +        return 0;

> +        }

> +    ]])],

> +    AC_MSG_RESULT(yes),

> +    AC_MSG_RESULT(no)

> +    echo "atomic operations compilation fail."

> +    exit -1)

> +

> +# Restore LDFLAGS

> +CFLAGS="$my_save_cflags $MCX16_CFLAGS"

> +LDFLAGS="$LDFLAGS $LD_LIBATOMIC"

> +

>  m4_include([platform/linux-generic/m4/odp_pthread.m4])

>  m4_include([platform/linux-generic/m4/odp_openssl.m4])

>  m4_include([platform/linux-generic/m4/odp_pcap.m4])

> --

> 2.11.0.295.gd7dffce

>
Savolainen, Petri (Nokia - FI/Espoo) Feb. 21, 2017, 8:11 a.m. | #3
> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> Christophe Milard

> Sent: Monday, February 20, 2017 6:10 PM

> To: Maxim Uvarov <maxim.uvarov@linaro.org>

> Cc: LNG ODP Mailman List <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [PATCHv2] configure: libatomic check

> 

> Hi,

> 

> If these flags are required for compiling the examples, maybe, they

> should not be moved to the platform m4.

> But the problem is maybe more: Why do the examples (which are supposed

> to be compilable by all) requires this?

> 

> So maybe this change is good, and will trigger the example question...

> 

> This seems to fix an issue, anyway,  so I am positive to apply this patch

> 

> Christophe

> 

> On 16 February 2017 at 21:26, Maxim Uvarov <maxim.uvarov@linaro.org>

> wrote:

> > upcoming patch ip fragmentation example fails to with

> > gcc 4.8.4 on linking __atomic_compare_exchange_16 functions

> > on x86. For some version of gcc both -latomic and -mcx16

> > needs to be provided in that case. For clang only -mcx16. That

> > options are set internally for platform but do not set for examples.

> > This patch unhides setting this options, make them common and printed

> > on configure log.

> 

> Reviewed-by: Christophe Milard <christophe.milard@linaro.org>



I'm also worried that dependency to 128 bit atomics narrows down the number of target systems that can run odp-linux and examples. I'm sure that 128 bit atomics is supported by x86 and armv8, but how about older mips, powerpc, arm, tilera, etc CPUs and compilers for those? Examples should be more portable than odp-linux implementation. It should run as is on various implementations built with various (older) compilers that are part of vendor SDKs. Odp-linux is in better position (than examples) to have arch dependent #ifdefs and files to use 128 atomics when available, or fall back to locks when not available.

-Petri

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index 6153efd2..a514f6be 100644
--- a/configure.ac
+++ b/configure.ac
@@ -314,22 +314,6 @@  ODP_CFLAGS="$ODP_CFLAGS -std=c99"
 # Extra flags for example to suppress certain warning types
 ODP_CFLAGS="$ODP_CFLAGS $ODP_CFLAGS_EXTRA"
 
-#########################################################################
-# Check if compiler supports cmpxchng16
-##########################################################################
-if test "${CC}" != "gcc" -o ${CC_VERSION_MAJOR} -ge 5; then
-   my_save_cflags="$CFLAGS"
-
-   CFLAGS=-mcx16
-   AC_MSG_CHECKING([whether CC supports -mcx16])
-   AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])],
-	    [AC_MSG_RESULT([yes])]
-	    [ODP_CFLAGS="$ODP_CFLAGS $CFLAGS"],
-	    [AC_MSG_RESULT([no])]
-	    )
-   CFLAGS="$my_save_cflags"
-fi
-
 ##########################################################################
 # Default include setup
 ##########################################################################
diff --git a/platform/linux-generic/m4/configure.m4 b/platform/linux-generic/m4/configure.m4
index d3e5528c..92172d4b 100644
--- a/platform/linux-generic/m4/configure.m4
+++ b/platform/linux-generic/m4/configure.m4
@@ -28,6 +28,63 @@  AC_LINK_IFELSE(
     echo "Use newer version. For gcc > 4.7.0"
     exit -1)
 
+#########################################################################
+# Check if compiler supports cmpxchng16
+##########################################################################
+my_save_cflags="$CFLAGS"
+
+CFLAGS=-mcx16
+AC_MSG_CHECKING([whether CC supports -mcx16])
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])],
+    [AC_MSG_RESULT([yes])]
+    [MCX16_CFLAGS="-mcx16"],
+    [AC_MSG_RESULT([no])]
+    )
+CFLAGS="$my_save_cflags"
+
+#########################################################################
+# Check if compiler supports __atomic_compare_exchange
+##########################################################################
+my_save_cflags="$CFLAGS"
+LD_LIBATOMIC=""
+
+AC_MSG_CHECKING(if libatomic required)
+AC_LINK_IFELSE(
+    [AC_LANG_SOURCE(
+      [[int main() {
+        unsigned __int128 x = 0, y = 0;
+        y = __atomic_load_n(&x, 0);
+        __atomic_store_n(&x, y, 0);
+        __atomic_compare_exchange_n(&x, &y, x, 0, 0, 0);
+        return 0;
+        }
+    ]])],
+    AC_MSG_RESULT(yes)
+    LD_LIBATOMIC="-latomic"
+    echo "adding -lbatomic",
+    AC_MSG_RESULT(no))
+
+CFLAGS="$my_save_cflags $LD_LIBATOMIC $MCX16_CFLAGS"
+AC_MSG_CHECKING([CFLAGS=$CFLAGS])
+AC_LINK_IFELSE(
+    [AC_LANG_SOURCE(
+      [[int main() {
+        unsigned __int128 x = 0, y = 0;
+        y = __atomic_load_n(&x, 0);
+        __atomic_store_n(&x, y, 0);
+        __atomic_compare_exchange_n(&x, &y, x, 0, 0, 0);
+        return 0;
+        }
+    ]])],
+    AC_MSG_RESULT(yes),
+    AC_MSG_RESULT(no)
+    echo "atomic operations compilation fail."
+    exit -1)
+
+# Restore LDFLAGS
+CFLAGS="$my_save_cflags $MCX16_CFLAGS"
+LDFLAGS="$LDFLAGS $LD_LIBATOMIC"
+
 m4_include([platform/linux-generic/m4/odp_pthread.m4])
 m4_include([platform/linux-generic/m4/odp_openssl.m4])
 m4_include([platform/linux-generic/m4/odp_pcap.m4])