Opened 8 years ago

Closed 8 years ago

#3004 closed defect (fixed)

OpenCL Version detection broken (at least on OSX)

Reported by: Thilo Borgmann Owned by:
Priority: important Component: build system
Version: git-master Keywords: regression opencl
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:

configure accepts any installed version of OpenCL if any is installed.

In configure, it seems like the block starting in line 4263 is not executed (not mentioned anywhere in config.log, see config.log.unpatched):

{...} &&
{...} &&
{ check_cpp_condition "OpenCL/cl.h" "defined(CL_VERSION_1_2)" ||
  check_cpp_condition "CL/cl.h" "defined(CL_VERSION_1_2)" ||
  die "ERROR: opencl must be installed and version must be 1.2 or compatible"; }

If I reorder the three blocks for opencl, a version prior to 1.2 is found and rejected properly (see config.log.patched).

How to reproduce:
1) Install OpenCL 1.0 or 1.1
2) Run:

./configure --enable-opencl

I can't find a reason why the test is skipped in the unpatched configure and thus I cannot send a patch to really fix it without further assistance. I also don't have a non-Mac machine ready now to test if this is really just a Mac problem.

Attached are a patch for the reordering workaround and two corresponding config.log files.

Attachments (6)

opencl_configure.patch (1.5 KB ) - added by Thilo Borgmann 8 years ago.
Reorder the blocks for opencl.
config.log.unpatched (218.5 KB ) - added by Thilo Borgmann 8 years ago.
Vanilla config.log after "configure --enable-opencl".
config.log.patched (189.3 KB ) - added by Thilo Borgmann 8 years ago.
Patched configure's config.log after "configure --enable-opencl".
patchopencl.diff (938 bytes ) - added by Carl Eugen Hoyos 8 years ago.
0001-configure-fix-logic-for-threads-in-case-of-OpenCL-is.patch (1.3 KB ) - added by Thilo Borgmann 8 years ago.
patchopencl2.diff (892 bytes ) - added by Carl Eugen Hoyos 8 years ago.

Download all attachments as: .zip

Change History (15)

by Thilo Borgmann, 8 years ago

Attachment: opencl_configure.patch added

Reorder the blocks for opencl.

by Thilo Borgmann, 8 years ago

Attachment: config.log.unpatched added

Vanilla config.log after "configure --enable-opencl".

by Thilo Borgmann, 8 years ago

Attachment: config.log.patched added

Patched configure's config.log after "configure --enable-opencl".

comment:1 by Carl Eugen Hoyos, 8 years ago

Keywords: regression added; configure removed
Priority: normalimportant
Version: unspecifiedgit-master

Probably a regression since 1235e91b, related to ticket #2422.

comment:2 by Thilo Borgmann, 8 years ago

Indeed, reverting 1235e91b results in proper OpenCL version check.
I still can't find any reason why 1235e91b does not work as expected.
Thus I'm still lacking an idea for a solution.

comment:3 by Carl Eugen Hoyos, 8 years ago

I can't test atm but isn't replacing } && with } || sufficient to fix the bug?

comment:4 by Thilo Borgmann, 8 years ago

I don't think you want to OR the version condition... "enable_any | | die" does not make sense.

You might verify that any following {} blocks are skipped after the {enable_any ...} block - no need for actually having a CL machine.

I don't see the reason in the logics but I'm not used to so much script magic anyway...

Last edited 8 years ago by Thilo Borgmann (previous) (diff)

by Carl Eugen Hoyos, 8 years ago

Attachment: patchopencl.diff added

comment:5 by Carl Eugen Hoyos, 8 years ago

Untested patch attached.

comment:6 by Thilo Borgmann, 8 years ago

Ok that kind of works. However, I'll send the following patch to devel that uses the same "positive" logic like everywhere else instead.

by Carl Eugen Hoyos, 8 years ago

Attachment: patchopencl2.diff added

comment:7 by Carl Eugen Hoyos, 8 years ago

New untested patch attached.

comment:8 by Thilo Borgmann, 8 years ago

Tested and posted on devel, too.

comment:9 by Carl Eugen Hoyos, 8 years ago

Resolution: fixed
Status: newclosed

Should be fixed.

Note: See TracTickets for help on using tickets.