diff mbox

clapack: musl patches

Message ID 20150815161506.GA28405@vostro
State Accepted
Headers show

Commit Message

Alex Suykov Aug. 15, 2015, 4:15 p.m. UTC
Fixes
http://autobuild.buildroot.net/results/a94f097193cd5444dee5cd9df9d544ce736a7e7e/
http://autobuild.buildroot.net/results/036014e492f7caf793b92a9822ab6a0b1a54f2e8/

Signed-off-by: Alex Suykov <alex.suykov@gmail.com>
---
 package/clapack/0006-remove-uninit-f2c.patch | 24 ++++++++++++++++++++++++
 package/clapack/0007-off64-t.patch           | 16 ++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 package/clapack/0006-remove-uninit-f2c.patch
 create mode 100644 package/clapack/0007-off64-t.patch

Comments

Yann E. MORIN Aug. 15, 2015, 5:36 p.m. UTC | #1
Alex, All,

On 2015-08-15 19:15 +0300, Alex Suykov spake thusly:
> Fixes
> http://autobuild.buildroot.net/results/a94f097193cd5444dee5cd9df9d544ce736a7e7e/
> http://autobuild.buildroot.net/results/036014e492f7caf793b92a9822ab6a0b1a54f2e8/

Funny! I was looking at the same build failures, and was leaning toward
making clapack depend on !musl...

> Signed-off-by: Alex Suykov <alex.suykov@gmail.com>
> ---
>  package/clapack/0006-remove-uninit-f2c.patch | 24 ++++++++++++++++++++++++
>  package/clapack/0007-off64-t.patch           | 16 ++++++++++++++++
>  2 files changed, 40 insertions(+)
>  create mode 100644 package/clapack/0006-remove-uninit-f2c.patch
>  create mode 100644 package/clapack/0007-off64-t.patch
> 
> diff --git a/package/clapack/0006-remove-uninit-f2c.patch b/package/clapack/0006-remove-uninit-f2c.patch
> new file mode 100644
> index 0000000..251c702
> --- /dev/null
> +++ b/package/clapack/0006-remove-uninit-f2c.patch
> @@ -0,0 +1,24 @@
> +uninit.c includes glibc-specific <fpu_control.h> not provided by musl.
> +Somewhat portable replacement is <fenv.h>, which is available in musl
> +but requires non-trivial changes to uninit.c.
> +
> +f2c uses _uninit_f2c() from uninit.c to implement its -trapuv option,
> +pre-initializiing floating-point variables to NaN and asking FPU to send
> +SIGFPE whenever NaN value is encountered.
> +
> +clapack source has already been run through f2c without -trapuv,
> +so uninit.c code is not used anywhere and can be safely excluded.

Indeed, I could not see any call to _uninit_f2c in any of clapack's own
source code, neither did I in any of the two users of clapack we have in
Buildroot, armadillo and python-numpy.

Googling around only gives references to uninit's own source code (and
this mail of yours, already indexed by Google; scary, isn't it?).

So, it indeed looks sane to remove it entirely...

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

This means I'm now trashing my local patches; thanks! ;-)

Regards,
Yann E. MORIN.

> +Signed-off-by: Alex Suykov <alex.suykov@gmail.com>
> +
> +--- a/F2CLIBS/libf2c/CMakeLists.txt
> ++++ b/F2CLIBS/libf2c/CMakeLists.txt
> +@@ -1,7 +1,7 @@
> + set(MISC 
> +   f77vers.c i77vers.c main.c s_rnge.c abort_.c exit_.c getarg_.c iargc_.c
> +   getenv_.c signal_.c s_stop.c s_paus.c system_.c cabs.c ctype.c
> +-  derf_.c derfc_.c erf_.c erfc_.c sig_die.c uninit.c)
> ++  derf_.c derfc_.c erf_.c erfc_.c sig_die.c)
> + set(POW pow_ci.c pow_dd.c pow_di.c pow_hh.c pow_ii.c pow_ri.c pow_zi.c pow_zz.c)
> + set(CX 	c_abs.c c_cos.c c_div.c c_exp.c c_log.c c_sin.c c_sqrt.c)
> + set(DCX	z_abs.c z_cos.c z_div.c z_exp.c z_log.c z_sin.c z_sqrt.c)
> diff --git a/package/clapack/0007-off64-t.patch b/package/clapack/0007-off64-t.patch
> new file mode 100644
> index 0000000..bf66d3c
> --- /dev/null
> +++ b/package/clapack/0007-off64-t.patch
> @@ -0,0 +1,16 @@
> +musl only provides off64_t, not __off64_t.
> +glibc and uclibc have both defined.
> +
> +Signed-off-by: Alex Suykov <alex.suykov@gmail.com>
> +
> +--- a/F2CLIBS/libf2c/sysdep1.h
> ++++ b/F2CLIBS/libf2c/sysdep1.h
> +@@ -10,7 +10,7 @@
> + 
> + #ifdef __linux__
> + #define USE_LARGEFILE
> +-#define OFF_T __off64_t
> ++#define OFF_T off64_t
> + #endif
> + 
> + #ifdef _AIX43
> -- 
> 2.0.3
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni Aug. 17, 2015, 8:43 p.m. UTC | #2
Dear Alex Suykov,

On Sat, 15 Aug 2015 19:15:06 +0300, Alex Suykov wrote:
> Fixes
> http://autobuild.buildroot.net/results/a94f097193cd5444dee5cd9df9d544ce736a7e7e/
> http://autobuild.buildroot.net/results/036014e492f7caf793b92a9822ab6a0b1a54f2e8/
> 
> Signed-off-by: Alex Suykov <alex.suykov@gmail.com>
> ---
>  package/clapack/0006-remove-uninit-f2c.patch | 24 ++++++++++++++++++++++++
>  package/clapack/0007-off64-t.patch           | 16 ++++++++++++++++
>  2 files changed, 40 insertions(+)
>  create mode 100644 package/clapack/0006-remove-uninit-f2c.patch
>  create mode 100644 package/clapack/0007-off64-t.patch

To be honest, I was a bit hesitant to add the patch that removes
uninit.c from the build, as the entire clapack thing looks a bit
complicated, and doing such a change without upstream's opinion is
dangerous. However, clapack is not that widely used I believe, so let's
see if that proposal breaks anything for anyone.

Alex, could you submit the two patches you have written to the upstream
clapack project?

Thanks!

Thomas
Alex Suykov Aug. 18, 2015, 7:59 p.m. UTC | #3
Mon, Aug 17, 2015 at 10:43:26PM +0200, Thomas Petazzoni wrote:

> To be honest, I was a bit hesitant to add the patch that removes
> uninit.c from the build, as the entire clapack thing looks a bit
> complicated, and doing such a change without upstream's opinion is
> dangerous. However, clapack is not that widely used I believe, so let's
> see if that proposal breaks anything for anyone.

Well, I think it should be ok as long as standalone f2c is not involved.

With standalone f2c things will break, not because of this patch, but
because there will be conflict over libf2c.a. But f2c is not something
I would expect to see in buildroot any time soon.

> Alex, could you submit the two patches you have written to the upstream
> clapack project?

Patches sent upstream, let's see what they will reply.
diff mbox

Patch

diff --git a/package/clapack/0006-remove-uninit-f2c.patch b/package/clapack/0006-remove-uninit-f2c.patch
new file mode 100644
index 0000000..251c702
--- /dev/null
+++ b/package/clapack/0006-remove-uninit-f2c.patch
@@ -0,0 +1,24 @@ 
+uninit.c includes glibc-specific <fpu_control.h> not provided by musl.
+Somewhat portable replacement is <fenv.h>, which is available in musl
+but requires non-trivial changes to uninit.c.
+
+f2c uses _uninit_f2c() from uninit.c to implement its -trapuv option,
+pre-initializiing floating-point variables to NaN and asking FPU to send
+SIGFPE whenever NaN value is encountered.
+
+clapack source has already been run through f2c without -trapuv,
+so uninit.c code is not used anywhere and can be safely excluded.
+
+Signed-off-by: Alex Suykov <alex.suykov@gmail.com>
+
+--- a/F2CLIBS/libf2c/CMakeLists.txt
++++ b/F2CLIBS/libf2c/CMakeLists.txt
+@@ -1,7 +1,7 @@
+ set(MISC 
+   f77vers.c i77vers.c main.c s_rnge.c abort_.c exit_.c getarg_.c iargc_.c
+   getenv_.c signal_.c s_stop.c s_paus.c system_.c cabs.c ctype.c
+-  derf_.c derfc_.c erf_.c erfc_.c sig_die.c uninit.c)
++  derf_.c derfc_.c erf_.c erfc_.c sig_die.c)
+ set(POW pow_ci.c pow_dd.c pow_di.c pow_hh.c pow_ii.c pow_ri.c pow_zi.c pow_zz.c)
+ set(CX 	c_abs.c c_cos.c c_div.c c_exp.c c_log.c c_sin.c c_sqrt.c)
+ set(DCX	z_abs.c z_cos.c z_div.c z_exp.c z_log.c z_sin.c z_sqrt.c)
diff --git a/package/clapack/0007-off64-t.patch b/package/clapack/0007-off64-t.patch
new file mode 100644
index 0000000..bf66d3c
--- /dev/null
+++ b/package/clapack/0007-off64-t.patch
@@ -0,0 +1,16 @@ 
+musl only provides off64_t, not __off64_t.
+glibc and uclibc have both defined.
+
+Signed-off-by: Alex Suykov <alex.suykov@gmail.com>
+
+--- a/F2CLIBS/libf2c/sysdep1.h
++++ b/F2CLIBS/libf2c/sysdep1.h
+@@ -10,7 +10,7 @@
+ 
+ #ifdef __linux__
+ #define USE_LARGEFILE
+-#define OFF_T __off64_t
++#define OFF_T off64_t
+ #endif
+ 
+ #ifdef _AIX43