Bug 228009 - [MAINTAINER] math/superlu: take maintainership, switch to flang
Summary: [MAINTAINER] math/superlu: take maintainership, switch to flang
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: amd64 Any
: --- Affects Many People
Assignee: Yuri Victorovich
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-06 05:55 UTC by robert.ayrapetyan
Modified: 2018-05-08 06:35 UTC (History)
1 user (show)

See Also:


Attachments
necessary changes for switching to flang (657 bytes, patch)
2018-05-06 05:55 UTC, robert.ayrapetyan
no flags Details | Diff
poudriere log for a patched port (265.95 KB, text/plain)
2018-05-06 21:50 UTC, robert.ayrapetyan
no flags Details
necessary changes for switching to flang (867 bytes, patch)
2018-05-07 05:59 UTC, robert.ayrapetyan
no flags Details | Diff
poudriere log for a patched port (170.81 KB, text/plain)
2018-05-07 06:00 UTC, robert.ayrapetyan
no flags Details
Updated patch for flang (1.20 KB, patch)
2018-05-07 06:24 UTC, Yuri Victorovich
no flags Details | Diff
Updated patch for flang (1.37 KB, patch)
2018-05-07 06:56 UTC, Yuri Victorovich
no flags Details | Diff
patch with a for loop for non-flang archs (1.33 KB, patch)
2018-05-07 20:18 UTC, robert.ayrapetyan
no flags Details | Diff
patch (1.36 KB, patch)
2018-05-08 06:35 UTC, robert.ayrapetyan
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description robert.ayrapetyan 2018-05-06 05:55:35 UTC
Created attachment 193074 [details]
necessary changes for switching to flang

There are lot of known problems with gfortran when clang is involved, e.g.:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=196862

This patch switches to using flang compiler (on amd64 only) and resolves gcc linking issues.

Note:
- patched "Makefile" contains a "ditry" arch check, I believe it's the best available option now until "flang" will be set by default for all amd64 ports in fortran.mk.
Comment 1 robert.ayrapetyan 2018-05-06 21:50:43 UTC
Created attachment 193121 [details]
poudriere log for a patched port
Comment 2 Yuri Victorovich freebsd_committer 2018-05-06 23:58:15 UTC
Committed change of maintainership.

----

I didn't commit USES=fortran:flang:
USES=fortran:flang fails in build due to the need of USES=fortran because of the lapack library use. flang currently can only be used when no gfortran is used in dependencies. Additionally, this wasn't the right way to add flang. The right way is through the port option. You can see an example here math/R/Makefile.
Comment 3 commit-hook freebsd_committer 2018-05-06 23:58:33 UTC
A commit references this bug:

Author: yuri
Date: Sun May  6 23:58:10 UTC 2018
New revision: 469258
URL: https://svnweb.freebsd.org/changeset/ports/469258

Log:
  math/superlu: robert.ayrapetyan@gmail.com takes maintainership

  Also removed the dysfunctional TEST option and added the do-test target
  that enables the 'make test'.

  PR:		228009
  Submitted by:	robert.ayrapetyan@gmail.com

Changes:
  head/math/superlu/Makefile
Comment 4 robert.ayrapetyan 2018-05-07 00:11:35 UTC
I've created multiple commits (including one for lapack project https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=228007), so if it will be accepted there this port will build fine. If you don't mind I'll reopen this issue again if they accept a patch.

Using flang through the option is not a best choice either - it's weird to show such an option on a non-amd64 platforms. Also, optional compilation will produce a flang\gfortran dependencies hell in all related ports.

To avoid all sort of conflicts I think the best and the only way is using flang by default on adm64 platform. Ideally it should be placed into fortran.mk, but it's too early for now. 

I would rather work on all lapack-dependent ports one by one.

Thanks!
Comment 5 Yuri Victorovich freebsd_committer 2018-05-07 00:18:18 UTC
The FLANG option won't appear on other platforms, see how math/R does it.

> To avoid all sort of conflicts I think the best and the only way is using flang by default on adm64 platform. Ideally it should be placed into fortran.mk, but it's too early for now. 

It fails to build a large portion of ports.
You should enable it locally, build all fortran ports, and see what breaks.
Comment 6 Yuri Victorovich freebsd_committer 2018-05-07 00:59:16 UTC
IMO, the best strategy is to first identify all flang shortcoming on amd64, and make flang fix them. At some point I've started this, and creates several bug reports for flang. I know that there are quite a few to go. Otherwise, you will soon run into this problem that gfortran6 ports can't be mixed with flang ones. USES=fortran is currently required even when only some dependency is gortran6-based. This will cause conflicts with flang.
Comment 7 Yuri Victorovich freebsd_committer 2018-05-07 01:00:59 UTC
(In reply to Yuri Victorovich from comment #6)

In other words, flang isn't yet mature enough to replace gfortran6 system-wide.
Comment 8 robert.ayrapetyan 2018-05-07 01:02:43 UTC
Agree, there are 174 ports using fortran in ports collection.
Would you accept a patch for OPTION FLANG in this port?
Comment 9 Yuri Victorovich freebsd_committer 2018-05-07 01:05:57 UTC
(In reply to robert.ayrapetyan from comment #8)

Wouldn't such patch conflict with gfortran in blaslapack:gotoblas/blaslapack:atlas/blaslapack:openblas ? They should all be switched first, I think (that is if this is possible).
Comment 10 robert.ayrapetyan 2018-05-07 01:12:00 UTC
They will. I'll request this flag to be added to all BLAS ports. Then when users of e.g. math/mlpack asks about "/lib/libgcc_s.so.1: version GCC_4.6.0" errors in another thread, I will ask them to rebuild chain of ports with FLANG option ON rather than patching make.conf globally or adding things like:
"-L/usr/local/lib/gcc6 -Wl,-rpath,/usr/local/lib:/usr/local/lib/gcc6"
Thanks!
Comment 11 robert.ayrapetyan 2018-05-07 05:59:02 UTC
Created attachment 193130 [details]
necessary changes for switching to flang

A less-destructive flang switch (OPTION).
Comment 12 robert.ayrapetyan 2018-05-07 06:00:17 UTC
Created attachment 193131 [details]
poudriere log for a patched port
Comment 13 robert.ayrapetyan 2018-05-07 06:00:46 UTC
Please review a new patch. Thanks.
Comment 14 Yuri Victorovich freebsd_committer 2018-05-07 06:24:44 UTC
Created attachment 193134 [details]
Updated patch for flang
Comment 15 Yuri Victorovich freebsd_committer 2018-05-07 06:30:39 UTC
It fails on 10.4 i386:
> -- The Fortran compiler identification is unknown
Needs to be investigated.
Comment 16 robert.ayrapetyan 2018-05-07 06:51:33 UTC
(In reply to Yuri Victorovich from comment #15)
You mean on 10.4 amd64, correct? flang is not intended to work on non-amd64 I think...
Also, 
OPTIONS_DEFAULT_amd64=	FLANG
is not a good default choice IMO.
Comment 17 Yuri Victorovich freebsd_committer 2018-05-07 06:56:28 UTC
Created attachment 193139 [details]
Updated patch for flang

Now it works.
Comment 18 robert.ayrapetyan 2018-05-07 07:25:46 UTC
(In reply to Yuri Victorovich from comment #17)
What was wrong with:

OPTIONS_DEFINE_amd64=   FLANG

? It looks way better than:

OPTIONS_EXCLUDE_aarch64=FLANG
OPTIONS_EXCLUDE_armv6=	FLANG
OPTIONS_EXCLUDE_armv7=	FLANG
OPTIONS_EXCLUDE_i386=	FLANG
Comment 19 Yuri Victorovich freebsd_committer 2018-05-07 07:27:14 UTC
(In reply to robert.ayrapetyan from comment #18)

It was failing on non-amd64 platforms, because USES=flang was entirely missing there.
Comment 20 robert.ayrapetyan 2018-05-07 19:11:21 UTC
Hm, excluding multiple archs instead of including just one looks really poor. Could something be patched in options.mk to fix this issue?
Comment 21 Yuri Victorovich freebsd_committer 2018-05-07 19:19:55 UTC
(In reply to robert.ayrapetyan from comment #20)

I know it is ugly, I will replace this with a .for construct. IMO, it's just a reflection of how make and Mk/ scripts work, there is probably no easy way to actually simplify it.
Comment 22 robert.ayrapetyan 2018-05-07 20:18:32 UTC
Created attachment 193164 [details]
patch with a for loop for non-flang archs

Yuri, please find my patch with a for loop.
I've also dropped FLANG from OPTIONS_DEFAULT, seems community has a lot of questions to the quality of this compiler and is not ready yet for accepting it as a default replacement. Thanks.
Comment 23 robert.ayrapetyan 2018-05-08 06:35:37 UTC
Created attachment 193171 [details]
patch

Includes patch for https://bugs.freebsd.org/bugzilla/attachment.cgi?id=193170.

Note: theoretically, flang can be built with any BLAS in case BLAS itself is built with flang. I've tested it with OpenBLAS successfully. But for now let's put all BLAS-es to FLANG_PREVENT, except the base one.
Thanks.