Bug 207425

Summary: math/R Add some optimization related options to Makefile
Product: Ports & Packages Reporter: Fernando Herrero Carrón <nq1n407ba>
Component: Individual Port(s)Assignee: Kurt Jaeger <pi>
Status: Closed FIXED    
Severity: Affects Some People CC: pi, thierry
Priority: --- Flags: pi: maintainer-feedback-
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Add options for LTO, OpenMP and BLAS/LAPACK of choice
none
Add new options to math/R
none
Adds performance related options and fix compilation on 9.3 none

Description Fernando Herrero Carrón 2016-02-22 21:46:23 UTC
Created attachment 167306 [details]
Add options for LTO, OpenMP and BLAS/LAPACK of choice

There are a couple of enhancements that can be made to math/R regarding optimization: one would be to enable LTO and OpenMP whenever the compiler supports it, another one would be to allow linking to OpenBLAS besides ATLAS or R's internal BLAS implementation. The patch was inspired by math/octave's handling of BLAS.

Please fin attached a patch to add these options to the user.
Comment 1 Fernando Herrero Carrón 2016-05-29 15:08:40 UTC
Created attachment 170792 [details]
Add new options to math/R

Add some new options to the port: build using LTO, OpenMP and link with different BLAS libraries. Additionally fix some warnings reported by poudriere testport (x11 missing modules mainly).
Comment 2 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-04 20:38:07 UTC
testbuilds@work
Comment 3 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-05 03:20:59 UTC
Building with LTO selected fails on 10.3amd:

http://people.freebsd.org/~pi/logs/math__R-103-1465089577.txt

Building fails on 9.3a, can you investigate ?

http://people.freebsd.org/~pi/logs/math__R-93a-1465090000.txt
Comment 4 Fernando Herrero Carrón 2016-06-05 10:14:03 UTC
Sure, I'll have a look at it. The port is compiling fine for me on 10.3/amd64. First thing that I notice in the logs is that gcc is version 5 while I am compiling with 4.8. I'll check that.
Comment 5 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-05 10:19:35 UTC
Thanks for the pointer to gcc. I have

DEFAULT_VERSIONS=gcc=5

in my make.conf for the 10.3a poudriere, that's why. I'm surprised it turns out to cause trouble.
Comment 6 Fernando Herrero Carrón 2016-06-05 10:57:17 UTC
Ok, that solves one of my questions. Somehow gcc5 seems to not find/install the lto plugin. I'll do some research on that. As to 9.3, this seems an issue with older FreeBSD versions having limited support for long double math [1], thus the configure script finds support for long double type, but the compiler does not find expl nor longl. There is a a configure option to disable long double support, I'll add that for older versions. What I wonder now is how the original port is compiling on 9.3. I don't see the long-double option disabled anywhere.


[1] https://lists.freebsd.org/pipermail/freebsd-hackers/2009-March/028030.html
Comment 7 Fernando Herrero Carrón 2016-06-11 09:16:53 UTC
Looking at svn log /usr/src/lib/msun/src/math.h, I have found the following commit which brings an actual implementation for expl (logl should have been added in a previous commit):


----------
r271779 | tijl | 2014-09-18 17:10:22 +0200 (jue, 18 sep 2014) | 10 lines

MFC r257770 r257818 r257823 r260066 r260067 r260089 r260145 r268587 r268588
    r268589 r268590 r268593 r268597 r269758 r270845 r270847 r270893 r270932
    r270947 r271147

Merge libm work by kargl, bde and das from the past few months.
Besides optimisations and small bug fixes this includes new implementations
for C99 functions expl, coshl, sinhl, tanhl, erfl and erfcl.
----------

Now checking the OS version for that commit with svn cat -r r271779 sys/sys/param.h:

#define __FreeBSD_version 1000716       /* Master, propagated to newvers */


How should I handle this in the port so that long double support is disabled in older FreeBSD? Should I check for OS version 1000716 or rather a different one?

Thanks!
Comment 8 Fernando Herrero Carrón 2016-06-24 22:24:26 UTC
Created attachment 171764 [details]
Adds performance related options and fix compilation on 9.3

Fixes build issues on 9.3 with Long Double data type, plus adds the options from previous patches.
Comment 9 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-25 17:39:03 UTC
Testbuilds@work
Comment 10 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-25 19:10:44 UTC
Committed, thanks. I was busy the last few days, sorry for the delay.
Comment 11 commit-hook freebsd_committer freebsd_triage 2016-06-25 19:10:58 UTC
A commit references this bug:

Author: pi
Date: Sat Jun 25 19:10:20 UTC 2016
New revision: 417477
URL: https://svnweb.freebsd.org/changeset/ports/417477

Log:
  math/R: Add optimization related options to Makefile

  There are a couple of enhancements that can be made to math/R
  regarding optimization: one would be to enable LTO and OpenMP
  whenever the compiler supports it, another one would be to allow
  linking to OpenBLAS besides ATLAS or R's internal BLAS implementation.
  The patch was inspired by math/octave's handling of BLAS.

  PR:		207425
  Submitted by:	elferdo@gmail.com
  Approved by:	bf (maintainer timeout)

Changes:
  head/math/R/Makefile
  head/math/R/pkg-plist
Comment 12 Thierry Thomas freebsd_committer freebsd_triage 2016-06-26 12:00:02 UTC
There is a problem with these changes: the default package does not include any more the shared lib (only if the option RBLAS is selecte"d, which is not the default).

That breaks the depending ports which need this lib, e.g. math/rkward-kde4.

See http://beefy6.nyi.freebsd.org/data/101amd64-default/417508/logs/rkward-kde4-0.6.5.log
Comment 13 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-26 12:07:38 UTC
Thanks for the pointer, I'll check if RBLAS can be made the default.
Comment 14 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-26 12:22:07 UTC
A quick glance: looks like the LIBR option should be set per default, which would fix the build problem of rkward-kde4 ?
Comment 15 Fernando Herrero Carrón 2016-06-26 12:31:55 UTC
My bad, sorry! I can take the fix if you will.

As it stands right now, handling those two slave ports in math/R is somewhat painful. I'll try to rewrite the Makefile, but I wonder if they are useful at all...
Comment 16 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-26 13:42:42 UTC
(In reply to Thierry Thomas from comment #12)
test-build rkward-kde4 with LIBR set in math/R. So probably this is enough ?
Comment 17 Thierry Thomas freebsd_committer freebsd_triage 2016-06-26 13:59:01 UTC
(In reply to Kurt Jaeger from comment #16)

Yes, it is. Could it be set as DEFAULT?
Comment 18 commit-hook freebsd_committer freebsd_triage 2016-06-26 14:03:49 UTC
A commit references this bug:

Author: pi
Date: Sun Jun 26 14:03:10 UTC 2016
New revision: 417525
URL: https://svnweb.freebsd.org/changeset/ports/417525

Log:
  math/R: add LIBR to OPTIONS_DEFAULT to allow math/rkward-kde4 to build

  PR:		207425
  Reported by:	thierry

Changes:
  head/math/R/Makefile
Comment 19 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-26 14:04:27 UTC
Fixed, thanks!