Created attachment 207513 [details] patch Remove unnecessary gcc dep.
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.
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.
Read my previous comment for requirements on when a patch would be acceptable.
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.
Prefer to keep this patch for users that not agree with maintainer aggressive optimization politic.
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.
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.
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.
(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.
Created attachment 207661 [details] add option
(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 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.
(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.
(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.
Created attachment 207665 [details] conf + build log
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
(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.
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.
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
One more data point, 11.2-i386 will barf on missing __atomic_load().
(In reply to Matthias Andree from comment #20) ...with clang, that is.
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.
(In reply to Matthias Andree from comment #22) Did you try build without LTO and with clang on 11.2 and 12.0?
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.
Thank you for you time, but some miscommunication happen. I only want fast build from source, because I use it time to time and do not care how fast/slow app work. I still think that you can add option "NOLTO for dummies", and if there is some problems with this option on non amd64 arch - then mark it as only amd64.
As an update, RT as of r526615 crashes with SIGSEGV if built with 12.1-RELEASE-p2 amd64's clang, but not under USE_GCC...
After further attempts, I find that the SEGV came from mixing in an OpenMP fftw3w-float lib. Fixing that, I find that clang-compiled rawtherapee crashes in so many diverse places under OpenMP that OpenMP isn't workable with clang, and rawtherapee isn't workable without OpenMP performance-wise. I give up. Rawtherapee will stick to USE_GCC. Please refrain from further requests to avoid GCC unless - you have a patch that - works on all supported Tier 1 platforms/architectures at least with the highest-numbered formal FreeBSD release (today, this would be 12.1-RELEASE-p2) - works with OpenMP enabled - passes my self-test suite with nontrivial test image processing files (including wavelet denoise and whatnot)
*** Bug 251041 has been marked as a duplicate of this bug. ***
A commit references this bug: Author: mandree Date: Thu Nov 12 22:41:57 UTC 2020 New revision: 554991 URL: https://svnweb.freebsd.org/changeset/ports/554991 Log: graphics/rawtherapee: build stability improvements - on some systems, the base binutils's ar does not support --plugin. Use BINARY_ALIAS to make sure the port/package binutils's ar and ranlib are used. [1] - while here, disable TCMALLOC on FreeBSD 11, which appears to cause strange errors in the run-time self-tests -> bump PORTREVISION to flush out old packages - while here, rearrange a bit per portclippy's suggestions PR: 251041 [1] PR: 240594 comment #9 [1] Reported by: rozhuk.im@gmail.com [1] Changes: head/graphics/rawtherapee/Makefile