Date: Sat, 08 Apr 2017 14:55:56 +0200 From: Matthew Rezny <rezny@freebsd.org> To: Jan Beich <jbeich@freebsd.org> Cc: svn-ports-head@freebsd.org, svn-ports-all@freebsd.org, ports-committers@freebsd.org Subject: Re: svn commit: r437953 - in head/lang/beignet: . files Message-ID: <4355650.qKugQOSgqh@workstation.reztek> In-Reply-To: <fuhj-kilw-wny@FreeBSD.org> References: <201704071930.v37JUBGj017948@repo.freebsd.org> <24496938.jsC6qZ9reS@workstation.reztek> <fuhj-kilw-wny@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 07 April 2017 23:34:03 Jan Beich wrote:
> Matthew Rezny <rezny@freebsd.org> writes:
> > On Friday 07 April 2017 22:23:18 Jan Beich wrote:
> >> Matthew Rezny <rezny@FreeBSD.org> writes:
> >> > - Enable OpenCL 2.0 on AMD64
> >>
> >> Isn't this premature before 1.3.2 or bug 217635 fix?
> >
> > There was no clear answer given in that PR as to whether or not version
> > 1.3.1 addressed the issue, but as I understood it the issue in that PR
> > was caused by a lack of OpenCL 2.0 so I do not see how enabling it would
> > be premature.
> DRM_IOCTL_I915_GEM_USERPTR is only available on drm-next but even there is
> half broken. Beignet *requires* it for OpenCL 2.0 but, looking more, the
> default is still 1.2. The patch just limits the ioctl to when an application
> explicitly requests 2.0.
>
> If you want a clear answer look no further than comment 0. OpenCL 2.0
> support upstream *is* the source of the regression. The patch just have a
> different effect on 1.3.0 and 1.3.1 but still required for both on FreeBSD.
>
Given that comments 0 and 1 in 217635 are the reporting of the original issue,
I assume you are referring to comment 0 of 217771, which is not the correct
place to give an answer about 217635. However, that was just a summary of your
testing without any statement of impact on 217635. Clear as mud.
Not long after creating 217771, you posted comment 4 to 217635 that reads
"With bug 217771 in scope comment 0 is limited to OPENCL20=off while comment 1
no longer happens." To me, that says both issues in 217635 are fixed if Beignet
is updated to 1.3.1 and OpenCL 2.0 support is enabled.
I wanted to be absolutely clear on that, hence comment 5; "I was going to
suggest trying 1.3.1 since it was released today but it appears you are ahead
of me. If I understand your last reply correctly, there is no problem and no
need for the patch if beignet is built with OpenCL 2.0 support enabled,
correct?"
That only needed a simple yes or no answer, but more than 3 weeks have passed
without any answer to it in either PR. You seemed to have time to question my
work in other areas while remaining silent on that detail, so I could only
assume that the issue was resolved and you did not feel the need to give
confirmation. Most often I get no direct confirmation when I fix something so the
the silence serves as confirmation.
> >> > +USE_LDCONFIG= ${LOCALBASE}/lib/${PORTNAME}
> >>
> >> Why do you need ldconfig hints for dlopen(3)? libcl.so embeds absolute
> >> paths for libgbe.so and libgbeinterp.so + absolutepath via ocl-icd
> >> config in /usr/local/etc/OpenCL/vendors/intel-beignet.icd
> >>
> >> Please, don't turn portlint into a cargo cult.
> >
> > Nothing to do with portlint, just following what is written in the
> > Porter's
> > Handbook about ports that install shared libraries and USE_LDCONFIG.
>
> Porter's Handook says "Please double-check, often this is not necessary at
> all". For an X11-related example, dri, libva, libvdpau, xorg-server load
> modules under /usr/local/lib/foo without adding it to USE_LDCONFIG e.g.,
>
> /usr/local/lib/xorg/modules/libglamoregl.so
>
I am aware X.org does things it's own way, including module loading and symbol
resolution, but this is not Xorg nor is ocl-icd.
The section on shared libraries starts with "If the port installs one or more
shared libraries, define a USE_LDCONFIG make variable, which will instruct a
bsd.port.mk to run ${LDCONFIG} -m on the directory where the new library is
installed (usually PREFIX/lib) during post-install target to register it into
the shared library cache." which makes it sound like the default should be to
USE_LDCONFIG is there are any .so files installed. There is no statement about
when to install shared libraries without USE_LDCONFIG.
As your incomplete quote follows "The default directory can be overridden by
setting USE_LDCONFIG to a list of directories into which shared libraries are
to be installed. " and ends with " or can be avoided through -rpath or setting
LD_RUN_PATH during linking (see lang/moscow_ml for an example), or through a
shell-wrapper which sets LD_LIBRARY_PATH before invoking the binary, like
www/seamonkey does.", the "not necessary at all" is clearly in reference to
overriding the location with USE_LDCONFIG and not simply the use of
USE_LDCONFIG.
So, the PHB suggests methods to keep it as a simple USE_LDCONFIG=yes, but does
not suggest conditions in which USE_LDCONFIG would be superfluous. That was
your opportunity to fill the void with a better explanation, but instead of
imparting any useful information, you just put a negative spin on a well
intended attempt to follow the written directions.
Fortunately, someone else has followed up with a more useful response;
"Blindly following the PHB (or portlint) is still merely cargo-culting.
USE_LDCONFIG is bogus for *.so's that are not being normally linked by
consumers, but dlopen(3)ed during runtime instead."
Sentence 1 I'm ignoring as the product of this thread.
Sentence 2 is what would have helped to see in the PHB or earlier in this
thread.
> >> > +LLVMVER= ${MESA_LLVM_VER:U39}
> >>
> >> Why not drop LLVMVER variable and use MESA_LLVM_VER directly?
> >>
> >> BUILD_DEPENDS= clang${MESA_LLVM_VER}:devel/llvm${MESA_LLVM_VER} \
> >> [...]
> >> MESA_LLVM_VER?= 39
> >
> > Conversely, why do it that way? I see no benefit, just more lines changed.
>
> ${LLVMVER} != ${MESA_LLVM_VER} cannot happen, so an extra variable may
> confuse the next maintainer.
Using MESA_LLVM_VER throughout would seem more confusing to me than to have
one place where Mesa's variable influences the local LLVMVER. So, this is just
another bit of style nit-picking like where to add whitespace.
Given that your interaction thus far has consisted primarily of useless
criticism, where half might be constructive if some reasoning was provided but
half is just personal opinion/style, it feels like your aim is to cause grief
rather than improve or educate, so I may just begin ignoring you in favor of
those who can provide useful feedback.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4355650.qKugQOSgqh>