Date: Tue, 14 Mar 2017 10:07:27 +0000 From: bugzilla-noreply@freebsd.org To: x11@FreeBSD.org Subject: [Bug 217771] lang/beignet: update to 1.3.1 Message-ID: <bug-217771-7141-31kp0d1afM@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-217771-7141@https.bugs.freebsd.org/bugzilla/> References: <bug-217771-7141@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D217771 --- Comment #8 from Matthew Rezny <rezny@freebsd.org> --- (In reply to Jan Beich (mail not working) from comment #7) >OpenCL 2.0 requires LLVM >=3D 3.9 and libdrm >=3D 2.4.67 (i.e. ports r4314= 63 and pkg info libdrm >maybe[1] drm-next). I forgot to adjust at least BUILD_DEPENDS in my versio= n. The LLVM requirement was bumped to 3.9 during the prior update to 1.3.0 and= we have had an up to date libdrm long enough that should be no concern either. >[1] Whether drm_intel_bo_set_softpin_offset() works on FreeBSD 10.3 or 11.= 0 or causes a crash similar to bug 217635 is unknown. > https://cgit.freedesktop.org/mesa/drm/commit/?id=3D8b4d57e7=20=20 That is something that will need to be tested. Given that the only Intel bo= x I have has a chipset with "Gen3" GPU, I cannot test any OpenCL on Intel. >> +OPTIONS_DEFINE=3D FP64 TEST >> +FP64_DESC=3D Double precision (experimental) >I don't like this style of grouping as it doesn't scale. If you have many = >options having to jump between descriptions and definitions when working o= n a >port with many can get tiring, see multimedia/ffmpeg. And I don't like excess whitespace to scroll through, so it's merely a difference of opinion. This port is not ffmpeg and it does have an unwieldy number of options, nor is it expected to. Should there be an explosion of options, whitespace can be added as needed. >> +.if ${ARCH} =3D=3D amd64 >> +PLIST_SUB+=3D OCL20=3D"" >> +.else # ${ARCH} =3D=3D i386 >> +PLIST_SUB+=3D OCL20=3D"@comment " >> +.endif >Conditionals break (mostly) declarative style of makefiles. Try the follow= ing instead > PLIST_SUB+=3D OCL20=3D"${ARCH:Namd64:C/.+/@comment /}" The conditional used it immediately understandable at a glance, the other w= ay is not. Clarity has value greater than conciseness. >> + -@(cd ${TEST_WRKSRC}.utests; . ./setenv.sh; \ >Why do you need to introduce typos? Because I'm not a robot, but an imperfect human? If you mean to ask why did= I retype it, well, I'd already bumped the port version and made some changes (i.e. adding the OCL20 stuff), so the patch wasn't going to apply without abandoning the progress made, copy and paste from the browser doesn't prese= rve the tabs, and that particular section had excess whitespace that would need= to be removed, so retyping was the fastest solution, until I have to explain h= ow a typo could possibly come to be... I did not try to run the tests as I do not expect any to succeed without the requisite hardware. >> +- src_data =3D (uint32_t*)memalign(base_address_alignment, buffer_sz); >> +- if(!src_data) { >> ++ if(posix_memalign((void**)&src_data, base_address_alignment, buffer_= sz)) { >posix_memalign has greater minimal alignment than memalign or aligned_allo= c in >jemalloc and glibc. And neither memalign nor posix_memalign are avail= able on >Windows. Or do you have something against using C11/C++17 features= without >passing corresponding -std=3D ? The intent was the make a patch that could be upstreamed more easily. posix_memalign is part of platforms (that doesn't include Windows). If aligned_alloc is available on all the platforms on which posix_memalign exi= sts, then that is news to me. I am aware it is part of C11, but the support for C++11 (and therefore I assume C11 as well) in GCC (still the favorite on Li= nux) has been lacking every time I have tried to use it, and the last time I had= to use a Microsoft compiler it did not support C99 and left a portion of C++0x stranded in the stdext namespace instead of std, so those are out the quest= ion. If the C11 support in GCC is strong enough, then aligned_alloc would be fin= e, though as you mention it would need the correct -std passed since GCC defau= lts to gnu++98. I do not see any more strict alignment requirements for posix_memalign; they both require the alignment be a power of 2. "As an example of the "supported by the implementation" requirement, POSIX function posix_memalign accepts any alignment that is a power of two and a multiple of sizeof(void *), and POSIX-based implementations of aligned_alloc inherit this requirements." >Try to avoid excessive noise (e.g. line offsets, timestamps) in patches. I had previously been advised to use makepatch when adding and changing patches. Is there a more strict set of criteria for when it is appropriate = than I am aware of? > @@ -29,6 +29,8 @@ > lib/beignet/include/ocl_vload_20.h > lib/beignet/include/ocl_work_group.h > lib/beignet/include/ocl_workitem.h > +%%OCL20%%lib/beignet/beignet_20.bc > +%%OCL20%%lib/beignet/beignet_20.pch > lib/beignet/libcl.so > lib/beignet/libgbe.so > lib/beignet/libgbeinterp.so >Keep sorting consistent with a version where %% macros are expanded. What is inconsistent about the sorting? lib/beignet/b* < lib/beignet/l* --=20 You are receiving this mail because: You are the assignee for the bug.=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-217771-7141-31kp0d1afM>