| Message ID | 1350071690-11279-1-git-send-email-stefan.froberg@petroprogram.com |
|---|---|
| State | Rejected |
| Headers | show |
On 12/10/12 21:54, Stefan Fröberg wrote: > > Signed-off-by: Stefan Fröberg<stefan.froberg@petroprogram.com> > --- > package/beecrypt/beecrypt.mk | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/package/beecrypt/beecrypt.mk b/package/beecrypt/beecrypt.mk > index d165aee..101d0df 100644 > --- a/package/beecrypt/beecrypt.mk > +++ b/package/beecrypt/beecrypt.mk > @@ -18,6 +18,12 @@ BEECRYPT_CONF_OPT = \ > --without-java \ > --without-python > > +ifeq ($(BR2_GCC_ENABLE_OPENMP),y) > +BEECRYPT_CONF_OPT += --enable-openmp > +else > +BEECRYPT_CONF_OPT += --disable-openmp > +endif > + This doesn't solve the underlying issue that -lgomp isn't included. In addition, it would only enable openmp for internal toolchains, while it may be present for ct-ng or external toolchains as well. I think the automatic detection works well in this case. Regards, Arnout
13.10.2012 12:29, Arnout Vandecappelle kirjoitti: > On 12/10/12 21:54, Stefan Fröberg wrote: >> >> Signed-off-by: Stefan Fröberg<stefan.froberg@petroprogram.com> >> --- >> package/beecrypt/beecrypt.mk | 6 ++++++ >> 1 files changed, 6 insertions(+), 0 deletions(-) >> >> diff --git a/package/beecrypt/beecrypt.mk b/package/beecrypt/beecrypt.mk >> index d165aee..101d0df 100644 >> --- a/package/beecrypt/beecrypt.mk >> +++ b/package/beecrypt/beecrypt.mk >> @@ -18,6 +18,12 @@ BEECRYPT_CONF_OPT = \ >> --without-java \ >> --without-python >> >> +ifeq ($(BR2_GCC_ENABLE_OPENMP),y) >> +BEECRYPT_CONF_OPT += --enable-openmp >> +else >> +BEECRYPT_CONF_OPT += --disable-openmp >> +endif >> + > > This doesn't solve the underlying issue that -lgomp isn't included. In > addition, it would only enable openmp for internal toolchains, while > it may > be present for ct-ng or external toolchains as well. I think the > automatic > detection works well in this case. > Yeah, but how do you test the presence of OpenMP in case of external toolchain ? Checking if /usr/lib/libgomp.so.* are present ? Parsing the output of external toolchain "gcc -v" ? (there is not even configure switch for OpenMP in current gcc) There are just too many variables and corner cases when it comes for testing various features of buildroot external toolchains. And the automatic detection didn't work for Alexander and he was even using the internal toolchain (if Im not wrong he uses ARM version of internal buildroot toolchain ?) Best regards Stefan > Regards, > Arnout
On 13/10/12 15:25, Stefan Fröberg wrote: > And the automatic detection didn't work for Alexander and he was even > using the internal toolchain The automatic detection did work (and it's part of autoconf, BTW). However, beecrypt uses the OPENMP_CFLAGS given by autoconf to determine if OpenMP is available or not - but OPENMP_CFLAGS may be empty if OpenMP is already enabled without any CFLAGS. So it should instead check on ac_cv_prog_c_openmp. Regards, Arnout
13.10.2012 16:29, Arnout Vandecappelle kirjoitti: > On 13/10/12 15:25, Stefan Fröberg wrote: >> And the automatic detection didn't work for Alexander and he was even >> using the internal toolchain > > The automatic detection did work (and it's part of autoconf, BTW). > However, > beecrypt uses the OPENMP_CFLAGS given by autoconf to determine if > OpenMP is > available or not - but *OPENMP_CFLAGS may be empty if OpenMP is > already enabled > without any CFLAGS*. So it should instead check on ac_cv_prog_c_openmp. > > Regards, > Arnout Isn't that ac_cv_prog_c_openmp in configure file the same as AC_OPENMP macro ? And from: https://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Generic-Compiler-Characteristics.html /"If the current language is C, the macro //|AC_OPENMP|//sets the variable //|OPENMP_CFLAGS|//to the C compiler flags needed for supporting OpenMP. /*/|OPENMP_CFLAGS|/**/is set to empty if the compiler already supports OpenMP, if it has no way to activate OpenMP support, or if the user rejects OpenMP support by invoking ‘/**/configure/**/’ with the ‘/**/--disable-openmp/*/*’ option.* //"// / So from that: 1 ) If compiler *supports* OpenMP it set's OPENMP_CFLAGS to empty. No OpenMP for you. 2 ) If compiler has no clue how to activate OpenMP, it will set OPENMP_CFLAGS to empty. No OpenMP for you. 3 ) If user refuses to use OpenMP with --disable-openmp (like in that my patch) it will set OPENMP_CFLAGS to empty. No OpenMP for you. Quick grepping of beecrypt-4.2.1 configure file shows that OPENMP_CFLAGS=ac_cv_prog_c_openmp grep -n ac_cv_prog_c configure configure:17706 OPENMP_CFLAGS=$ac_cv_prog_c_openmp ;; This all sounds soooo messy. Regards: Stefan
2012/10/13 Stefan Fröberg <stefan.froberg@petroprogram.com> > 13.10.2012 16:29, Arnout Vandecappelle kirjoitti: > > On 13/10/12 15:25, Stefan Fröberg wrote: > > And the automatic detection didn't work for Alexander and he was even > using the internal toolchain > > > The automatic detection did work (and it's part of autoconf, BTW). > However, > beecrypt uses the OPENMP_CFLAGS given by autoconf to determine if OpenMP > is > available or not - but *OPENMP_CFLAGS may be empty if OpenMP is already > enabled > without any CFLAGS*. So it should instead check on ac_cv_prog_c_openmp. > > Regards, > Arnout > > > Isn't that ac_cv_prog_c_openmp in configure file the same as AC_OPENMP > macro ? > > And from: > > https://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Generic-Compiler-Characteristics.html > > *"If the current language is C, the macro **AC_OPENMP** sets the variable > **OPENMP_CFLAGS** to the C compiler flags needed for supporting OpenMP. ** > OPENMP_CFLAGS** is set to empty if the compiler already supports OpenMP, > if it has no way to activate OpenMP support, or if the user rejects OpenMP > support by invoking ‘**configure**’ with the ‘**--disable-openmp**’ > option. **"** > * > So from that: > 1 ) If compiler *supports* OpenMP it set's OPENMP_CFLAGS to empty. No > OpenMP for you. > 2 ) If compiler has no clue how to activate OpenMP, it will set > OPENMP_CFLAGS to empty. No OpenMP for you. > 3 ) If user refuses to use OpenMP with --disable-openmp (like in that my > patch) it will set OPENMP_CFLAGS to empty. No OpenMP for you. > > > Quick grepping of beecrypt-4.2.1 configure file shows that > OPENMP_CFLAGS=ac_cv_prog_c_openmp > > grep -n ac_cv_prog_c configure > > configure:17706 OPENMP_CFLAGS=$ac_cv_prog_c_openmp ;; > > This all sounds soooo messy. > > Regards: > Stefan > > > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot > Use my patch please http://pastie.org/pastes/5052617/text
13.10.2012 17:54, Alexander Khryukin kirjoitti: > > > 2012/10/13 Stefan Fröberg <stefan.froberg@petroprogram.com > <mailto:stefan.froberg@petroprogram.com>> > > 13.10.2012 16:29, Arnout Vandecappelle kirjoitti: >> On 13/10/12 15:25, Stefan Fröberg wrote: >>> And the automatic detection didn't work for Alexander and he was >>> even >>> using the internal toolchain >> >> The automatic detection did work (and it's part of autoconf, >> BTW). However, >> beecrypt uses the OPENMP_CFLAGS given by autoconf to determine if >> OpenMP is >> available or not - but *OPENMP_CFLAGS may be empty if OpenMP is >> already enabled >> without any CFLAGS*. So it should instead check on >> ac_cv_prog_c_openmp. >> >> Regards, >> Arnout > > Isn't that ac_cv_prog_c_openmp in configure file the same as > AC_OPENMP macro ? > > And from: > https://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Generic-Compiler-Characteristics.html > > /"If the current language is C, the macro //|AC_OPENMP|//sets the > variable //|OPENMP_CFLAGS|//to the C compiler flags needed for > supporting OpenMP. /*/|OPENMP_CFLAGS|/**/is set to empty if the > compiler already supports OpenMP, if it has no way to activate > OpenMP support, or if the user rejects OpenMP support by invoking > ‘/**/configure/**/’ with the ‘/**/--disable-openmp/*/*’ option.* //"// > / > So from that: > 1 ) If compiler *supports* OpenMP it set's OPENMP_CFLAGS to > empty. No OpenMP for you. > 2 ) If compiler has no clue how to activate OpenMP, it will set > OPENMP_CFLAGS to empty. No OpenMP for you. > 3 ) If user refuses to use OpenMP with --disable-openmp (like in > that my patch) it will set OPENMP_CFLAGS to empty. No OpenMP for you. > > > Quick grepping of beecrypt-4.2.1 configure file shows that > OPENMP_CFLAGS=ac_cv_prog_c_openmp > > grep -n ac_cv_prog_c configure > > configure:17706 OPENMP_CFLAGS=$ac_cv_prog_c_openmp ;; > > This all sounds soooo messy. > > Regards: > Stefan > > > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net <mailto:buildroot@busybox.net> > http://lists.busybox.net/mailman/listinfo/buildroot > > > > Use my patch please > > http://pastie.org/pastes/5052617/text > I would but your patch forces the use of OpenMP in beecrypt, even in case when user's gcc does not have that for his/hers gcc. So it will end to link error for those users. So the beecrypt should have handled the OpenMP support by autodetecting it (and you do have OpenMP in your gcc) in configure process. But it did not. And Im curious to know why. Maybe someone could show what that mysterious AC_OPENMP actually does ... Regards Stefan
On 13/10/12 16:54, Alexander Khryukin wrote: > Use my patch please > > http://pastie.org/pastes/5052617/text That patch adds -lgomp unconditionally, even if OpenMP is not available. Regards, Arnout
diff --git a/package/beecrypt/beecrypt.mk b/package/beecrypt/beecrypt.mk index d165aee..101d0df 100644 --- a/package/beecrypt/beecrypt.mk +++ b/package/beecrypt/beecrypt.mk @@ -18,6 +18,12 @@ BEECRYPT_CONF_OPT = \ --without-java \ --without-python +ifeq ($(BR2_GCC_ENABLE_OPENMP),y) +BEECRYPT_CONF_OPT += --enable-openmp +else +BEECRYPT_CONF_OPT += --disable-openmp +endif + ifeq ($(BR2_PACKAGE_ICU),y) # C++ support needs icu BEECRYPT_DEPENDENCIES += icu
Signed-off-by: Stefan Fröberg <stefan.froberg@petroprogram.com> --- package/beecrypt/beecrypt.mk | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)