|Summary:||graphics/milton: fix build with GCC-based architectures, please portlint|
|Product:||Ports & Packages||Reporter:||Piotr Kubaj <pkubaj>|
|Component:||Individual Port(s)||Assignee:||Alexey Dokuchaev <danfe>|
|Severity:||Affects Only Me||CC:||danfe, linimon|
Description Piotr Kubaj 2019-05-21 12:53:10 UTC
Created attachment 204508 [details] patch Move USE_GITHUB and INSTALLS_ICONS to please portlint. Add -DNO_WARN_X86_INTRINSICS -maltivec -mvsx to CXXFLAGS on powerpc64 to use the new Intel SIMD software translation mode in GCC. Add ifdefs for a couple of E* macros to files/patch-src_shadergen.cc. Errors: /wrkdirs/usr/ports/graphics/milton/work/milton-1.6.0/src/shadergen.cc:364:13: error: 'ENODATA' was not declared in this scope case ENODATA: str = "No message is available on the STREAM head read queue (POSIX.1)"; break; ^~~~~~~ /wrkdirs/usr/ports/graphics/milton/work/milton-1.6.0/src/shadergen.cc:364:13: note: suggested alternative: 'ENOTTY' case ENODATA: str = "No message is available on the STREAM head read queue (POSIX.1)"; break; ^~~~~~~ ENOTTY
Comment 1 Mark Linimon 2019-05-29 21:27:52 UTC
This builds for me, so once the maintainer approves, consider this mentor approval.
Comment 2 Alexey Dokuchaev 2019-06-04 15:53:59 UTC
Thanks for the patch. Let me see if I can reproduce the errors on my Mac mini G4 here (I'm particularly curious about those EFOOBAR #ifdef's). > Move USE_GITHUB and INSTALLS_ICONS to please portlint. Actually, portlint(1) warning is bogus here: GitHub-related (that is, fetch-related) knobs are deliberately grouped together.
Comment 3 Piotr Kubaj 2019-06-04 15:59:52 UTC
(In reply to Alexey Dokuchaev from comment #2) Just note that you won't be able to run the binary that is compiled with this patch. VSX was introduced in POWER8. However there's currently no way to set IGNORE depending on the POWER CPU generation.
Comment 4 Mark Linimon 2019-06-21 11:40:05 UTC
Created attachment 205260 [details] patch to graphics/milton/Makefile Here's the slightly different patch that I tested.
Comment 5 Alexey Dokuchaev 2019-06-21 11:44:19 UTC
I'm a bit reluctant to pull heavy stuff like LLVM, which can be a pain on low-end hardware like my Mac mini G4, I need to think more about this.
Comment 6 Piotr Kubaj 2019-06-21 11:50:03 UTC
(In reply to Mark Linimon from comment #4) Why does using LLVM here help?
Comment 7 Alexey Dokuchaev 2019-06-24 09:33:37 UTC
I suspect because of unguarded inclusion of x86-specific headers in src/system_includes.h: > #include <xmmintrin.h> > #include <emmintrin.h> ... and pulling LLVM/Clang allows to handle them on !x86. This, of course, is very bogus solution. No more patches are needed at this point, I'm on it with my Mac mini G4. :-)
Comment 8 commit-hook 2019-06-24 10:59:16 UTC
A commit references this bug: Author: danfe Date: Mon Jun 24 10:58:22 UTC 2019 New revision: 505022 URL: https://svnweb.freebsd.org/changeset/ports/505022 Log: - Update `graphics/milton' to version 1.6.2 - It's written in C++11, so pull the right compiler - Don't try to decode errnos which are never returned on FreeBSD and thus might not be defined with some compilers - Fix the build on !x86 by removing unused intrinsic headers - Add some missing USE_* components reported by stage Q/A PR: 238021 Changes: head/graphics/milton/Makefile head/graphics/milton/distinfo head/graphics/milton/files/patch-src_system__includes.h
Comment 9 Alexey Dokuchaev 2019-06-24 11:08:54 UTC
OK, now lets talk more about AltiVec, VSX, and POWER8. Piotr, I have a few questions: 1) What is -DNO_WARN_X86_INTRINSICS for and why do we need it? Is it still needed after ports r505022? 2) Unfortunately, the program segfaults for me on powerpc32 even without -maltivec -mvsx, so I cannot test it. Did you observe some real, measurable benefit when building it with these options? 3) You've said that VSX requires POWER8. But AltiVec doesn't, right? Also, AltiVec is available on PPC32. This suggests the following: > CXXFLAGS_powerpc= -maltivec > CXXFLAGS_powerpc64= -maltivec -mvsx To make sure that generated packages are working on pre-POWER8 64-bit machines (like Apple's G5), these can be controlled by the option which is off by default. Unfortunately, as I'm mentioned, I cannot do proper run-time tests, so I might need some help here.
Comment 10 Piotr Kubaj 2019-06-24 11:16:30 UTC
(In reply to Alexey Dokuchaev from comment #9) It looks like removing includes fixed it and this patch is not necessary now.