Bug 240594 - graphics/rawtherapee: drop GCC dependency after 11.2 and 12.0 are discontinued? (March 2020?)
Summary: graphics/rawtherapee: drop GCC dependency after 11.2 and 12.0 are discontinue...
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Matthias Andree
URL:
Keywords:
Depends on: 230888
Blocks:
  Show dependency treegraph
 
Reported: 2019-09-15 21:52 UTC by rozhuk.im
Modified: 2019-09-30 20:14 UTC (History)
2 users (show)

See Also:
mandree: maintainer-feedback+
mandree: merge-quarterly-


Attachments
patch (1.38 KB, patch)
2019-09-15 21:52 UTC, rozhuk.im
mandree: maintainer-approval-
Details | Diff
add option (2.21 KB, patch)
2019-09-20 17:13 UTC, rozhuk.im
mandree: maintainer-approval-
Details | Diff
conf + build log (114.16 KB, application/gzip)
2019-09-20 23:09 UTC, rozhuk.im
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description rozhuk.im 2019-09-15 21:52:55 UTC
Created attachment 207513 [details]
patch

Remove unnecessary gcc dep.
Comment 1 Matthias Andree freebsd_committer 2019-09-16 22:08:35 UTC
Rejected. We need LTO to get reasonable cross-procedural optimization, and 
https://svnweb.freebsd.org/ports?view=revision&revision=512005 is pretty clear on why I chose LTO.  Getting LTO and OpenMP and whatnot to work with clang and all its dependencies isn't trivial, I've tried, and gave up on it.

So if someone comes up with a patch that maintains portability across ALL architectures and releases AND proves that the code doesn't get less efficient, I'll reconsider. Until then it's no, we'll have one GCC 9+ everywhere.
Comment 2 rozhuk.im 2019-09-16 22:14:09 UTC
Can gcc9 be optional? Default option?

Personally I do not need any features and optimizations for this app, I use it time to time, and I will lose more time to compile gcc9.
Comment 3 Matthias Andree freebsd_committer 2019-09-16 22:36:41 UTC
Read my previous comment for requirements on when a patch would be acceptable.
Comment 4 rozhuk.im 2019-09-17 09:41:02 UTC
As user of this software I have different priorities.

> We need LTO to get reasonable cross-procedural optimization, and 
> https://svnweb.freebsd.org/ports?view=revision&revision=512005
> is pretty clear on why I chose LTO.

Not sure that all other few users of this port will agree with that :)
To reduce "conversion on a Sony ARW file" time from 25 to 17s on unknown HW, witch personally I never use and have no plans to use.

> So if someone comes up with a patch that maintains portability across ALL
> architectures and releases AND proves that the code doesn't get less
> efficient, I'll reconsider. Until then it's no, we'll have one GCC 9+ everywhere.

No one will do that, it is waste of time.
Comment 5 rozhuk.im 2019-09-17 09:42:42 UTC
Prefer to keep this patch for users that not agree with maintainer aggressive optimization politic.
Comment 6 Matthias Andree freebsd_committer 2019-09-17 17:11:39 UTC
I provided one data point that shows that LTO isn't academic. Other RAW files or filters may have different timing properies, but inter-procedural optimization is important and yields non-trivial savings on user CPU time, and we need to optimize for the majority.

The conditions for opening the PR again are laid out clearly in comment #1. Unless and until I see a patch that fulfills all the requirements, this PR remains closed.  

Discussion and search is possible on/for closed PRs, Bugzilla doesn't archive or lock out followups or patches.
Comment 7 rozhuk.im 2019-09-17 20:16:34 UTC
1. Your data is not academic: at least you does not provide results for "ALL architectures and releases AND proves that the code doesn't get less efficient".
Only one test on unknown system, with one kind of input data.

2. As user of this app I have another priorities: I do not care how slow/fast it work, I do not want waste my time to compile it for long time.

At this point of view I ask you to add option to able to compile this port with system clang. It is WIN-WIN solution.
I can rework patch to add this option.
Comment 8 Matthias Andree freebsd_committer 2019-09-17 20:55:12 UTC
I am not going to trade individual build time optimization for an at-scale run-time loss.  We have binary packages these days.

If you have a patch that fulfills my criteria, I will consider it.
Comment 9 rozhuk.im 2019-09-20 17:13:24 UTC
(In reply to Matthias Andree from comment #8)

I start this because this port fail to build on mine system:
==============================================================================
[20/279] : && /usr/local/bin/cmake -E remove rtexif/librtexif.a && gcc-ar9 qc rtexif/librtexif.a  rtexif/CMakeFiles/rtexif.dir/rtexif.cc.o rtexif/CMakeFiles/rtexif.dir/stdattribs.cc.o rtexif/CMakeFiles/rtexif.dir/nikonattribs.cc.o rtexif/CMakeFiles/rtexif.dir/canonattribs.cc.o rtexif/CMakeFiles/rtexif.dir/pentaxattribs.cc.o rtexif/CMakeFiles/rtexif.dir/fujiattribs.cc.o rtexif/CMakeFiles/rtexif.dir/sonyminoltaattribs.cc.o rtexif/CMakeFiles/rtexif.dir/olympusattribs.cc.o rtexif/CMakeFiles/rtexif.dir/kodakattribs.cc.o rtexif/CMakeFiles/rtexif.dir/panasonicattribs.cc.o && gcc-ranlib9 rtexif/librtexif.a && :
FAILED: rtexif/librtexif.a 
: && /usr/local/bin/cmake -E remove rtexif/librtexif.a && gcc-ar9 qc rtexif/librtexif.a  rtexif/CMakeFiles/rtexif.dir/rtexif.cc.o rtexif/CMakeFiles/rtexif.dir/stdattribs.cc.o rtexif/CMakeFiles/rtexif.dir/nikonattribs.cc.o rtexif/CMakeFiles/rtexif.dir/canonattribs.cc.o rtexif/CMakeFiles/rtexif.dir/pentaxattribs.cc.o rtexif/CMakeFiles/rtexif.dir/fujiattribs.cc.o rtexif/CMakeFiles/rtexif.dir/sonyminoltaattribs.cc.o rtexif/CMakeFiles/rtexif.dir/olympusattribs.cc.o rtexif/CMakeFiles/rtexif.dir/kodakattribs.cc.o rtexif/CMakeFiles/rtexif.dir/panasonicattribs.cc.o && gcc-ranlib9 rtexif/librtexif.a && :
ar: unrecognized option `--plugin'
usage:  ar -d [-Tjsvz] archive file ...
	ar -m [-Tjsvz] archive file ...
	ar -m [-Tabijsvz] position archive file ...
	ar -p [-Tv] archive [file ...]
	ar -q [-TcDjsUvz] archive file ...
	ar -r [-TcDjsUuvz] archive file ...
	ar -r [-TabcDijsUuvz] position archive file ...
	ar -s [-jz] archive
	ar -t [-Tv] archive [file ...]
	ar -x [-CTouv] archive [file ...]
	ar -V
==============================================================================

Thats why I try to remove gcc crap and it build ok.
Please add option to able to build without gcc from my patch.
Comment 10 rozhuk.im 2019-09-20 17:13:54 UTC
Created attachment 207661 [details]
add option
Comment 11 Matthias Andree freebsd_committer 2019-09-20 22:12:41 UTC
(In reply to rozhuk.im from comment #9)
1. do not set maintainer feedback to anything but "?" unless you are the maintainer. 

2. Something appears to be wrong on your system. I have tested the port on 11.2 i386 and amd64 and on 12.0 amd64 (with poudriere) and on 12.0 amd64 in a live system, and none showed such an issue. Also, why is "ar" complaining about an unknown plugin option if we invoke gcc9-ar?  That looks "not quite right".

Please share some details on the system, how you build, possibly the entire build log (feel free to xz it and mail it to me off-list), /etc/src.conf and /etc/make.conf and port options you've set.
Comment 12 Matthias Andree freebsd_committer 2019-09-20 22:14:39 UTC
Comment on attachment 207661 [details]
add option

Re your new patch, we're getting closer, but the compiler flag must remain set even without LTO, and we shouldn't make LTO optional but get it working even with the base compiler.  Please revise.
Comment 13 Matthias Andree freebsd_committer 2019-09-20 22:48:30 UTC
(In reply to Matthias Andree from comment #11)
To be more specific, your installation appears to use /usr/bin/ar instead of ${LOCALBASE}/bin/ar (or /usr/local/bin/ar) from the binutils port.
I guess if you're installing with a nondefault PREFIX you may need to make sure that ${LOCALBASE}/bin is in PATH.
Comment 14 rozhuk.im 2019-09-20 23:09:04 UTC
(In reply to Matthias Andree from comment #12)

> make LTO optional but get it working even with the base compiler.

Looks like I have no LTO in base compiler:

    /usr/local/libexec/ccache/cc   -O2 -pipe -O2 -pipe -DSTRIP_FBSDID -D_FORTIFY_SOURCE=2 -mretpoline  -I/usr/local/include -fPIC -flto=16 -fuse-linker-plugin -fno-fat-lto-objects -O3 -funroll-loops -msse2 -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -o CMakeFiles/cmTC_58ba1.dir/testCCompiler.c.o   -c testCCompiler.c
    cc: error: unsupported argument '16' to option 'flto='
    cc: warning: optimization flag '-fuse-linker-plugin' is not supported [-Wignored-optimization-argument]
    cc: warning: optimization flag '-fno-fat-lto-objects' is not supported [-Wignored-optimization-argument]

I can rename option to CLANGBASE = "Build with clang in base, without LTO"


> I guess if you're installing with a nondefault PREFIX you may need to make sure that ${LOCALBASE}/bin is in PATH.

I never change PREFIX.


> Please share some details on the system

ok.
Comment 15 rozhuk.im 2019-09-20 23:09:35 UTC
Created attachment 207665 [details]
conf + build log
Comment 16 Matthias Andree freebsd_committer 2019-09-20 23:38:56 UTC
1a. What ${ARCH} are you building on and for?
1b. What exact FreeBSD version are you building on and for?

2. Please use my original port and in the Makefile, change this line:
_LTO_FLAGS=     -flto=${MAKE_JOBS_NUMBER} -fuse-linker-plugin -fno-fat-lto-objects

to only read:
_LTO_FLAGS=     -flto=${MAKE_JOBS_NUMBER}

and see if ar still complains about unsupported --plugin - if it does, that would  show that GCC behaved differently than documented. http://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html
Comment 17 Matthias Andree freebsd_committer 2019-09-20 23:46:59 UTC
(In reply to rozhuk.im from comment #14)
For clang, I guess you need to use -flto=thin instead, and possibly use lld for linking on arch/platforms that don't support linker plugins.
Comment 18 Matthias Andree freebsd_committer 2019-09-29 09:28:33 UTC
Well... I have a version that works with LTO and without GCC/binutils on 12.0 amd64, but its performance is considerably slower with base clang than it is with GCC9. 

For one of my test cases, processing time rises substantially (50% user time on CPU) on rawtherapee-cli development of one file with denoising, demosaic, wavelet, and other stuff.

GCC 9.2.0:
        7,76 real        49,79 user        19,29 sys
        7,64 real        49,46 user        19,46 sys
        7,94 real        50,10 user        20,09 sys

base clang 6.0.1:
        9,45 real        76,74 user        17,40 sys
This also causes OpenMP deprecation warnings at run-time.

LLVM 9.0 miscompiles the program, it gets SIGBUS somewhere in rtengine::wavelet_decomposition::level_coeffs (this=0x24a5ca, level=0)
    at /usr/ports.svn/graphics/rawtherapee/work/rawtherapee-5.7/rtengine/cplx_wavelet_dec.h:66
66	        return wavelet_decomp[level]->subbands();

LLVM 8.0 doesn't even configure rawtherapee because it somehow forces /usr/bin/ld which barfs on LTO.

So given that situation, I need to isolate the package from potential compiler bugs as the base compiler gets updated, and I will leave the port as it is for now (with GCC dependency) so that a known good version gets branched into 2019Q4 (albeit with heavy requisites) and we can revisit things after 2019Q4 has branched - but that will need LLVM/clang bug fixes first.

Rawtherapee has always been a port that unmasks lots of bugs in our toolchains, and it continues to do that.
Comment 19 commit-hook freebsd_committer 2019-09-29 09:35:52 UTC
A commit references this bug:

Author: mandree
Date: Sun Sep 29 09:35:33 UTC 2019
New revision: 513210
URL: https://svnweb.freebsd.org/changeset/ports/513210

Log:
  Drop default arguments from _LTO_FLAGS.

  Context:
  PR: 240594

Changes:
  head/graphics/rawtherapee/Makefile
Comment 20 Matthias Andree freebsd_committer 2019-09-29 12:18:44 UTC
One more data point, 11.2-i386 will barf on missing __atomic_load().
Comment 21 Matthias Andree freebsd_committer 2019-09-29 12:18:59 UTC
(In reply to Matthias Andree from comment #20)
...with clang, that is.
Comment 22 Matthias Andree freebsd_committer 2019-09-29 15:33:37 UTC
Same story with FreeBSD 12.0 i386, also fails on __atomic_* from clang6-in-base, so it would either have to use a newer LLVM (which either fails in the configure phase, or miscompiles rawtherapee, and is similarly heaviweight), or we're back to GCC anyhow.

In the end that was all incomplete testing on the submitter's part. 
"Works for me" is not enough to submit a patch!

So this whole waste of time and experimentation showed that GCC 9 is the only sane option that doesn't lead to combinatorial explosion and endless textwalls of .if ARCH/CPU/VERSION/COMPILER... in the Makefile.

We can revisit things once those FreeBSD versions with clang 6 in base have gone out of support, i. e. around March 2020.

This is the summary of findings and requirements:
- LTO with clang 6 as base compiler still requires BINUTILS (so we have ar/nm/ranlib that support LTO).
- FreeBSD 11.2 and 12.0 with clang 6 emit calls to __atomic_load and whatnot and fail a link on i386, so need to use GCC.


So my final ruling is that until 11.2 and later 12.0 are both EOL, we use USE_GCC no matter what.

Please refrain from further comments about heaviweight dependencies, or options, or other half-baked approaches. We're through.
Comment 23 rozhuk.im 2019-09-30 11:58:06 UTC
(In reply to Matthias Andree from comment #22)

Did you try build without LTO and with clang on 11.2 and 12.0?
Comment 24 Matthias Andree freebsd_committer 2019-09-30 18:10:25 UTC
I did on previous versions, and the i386 atomic ops issue is independent of LTO and will also occur without LTO.  clang 6 doesn't cut it.

I've wasted half the week-end trying to find a compromise that will permit us to get rid of the GCC without endless .if .else .but .stillnotworking chains and I found none. I will not spend any more time until all supported releases have working clang versions.

You have ignored all pleas to stop posting unless you have a full solution that doesn't regress as can be seen through the comments, I have enough now.  I have better ways to spend my time than for atypical use of an application on underpowered hardware or builders.

I have now unsubscribed from this PR and will revisit it on my own schedule in March 2020 the soonest because I don't want to hear any more input from you and will ignore it.