| Message ID | 20170216202624.8535-1-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
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]) >
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 >
> -----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
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])
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