KhronosGroup / WebCL-conformance

WebCL conformance tests
20 stars 15 forks source link

Invalid assumptions in testing createContext #63

Closed toaarnio closed 10 years ago

toaarnio commented 10 years ago

In functionalityTesting/basics/createContext.html, the following code fragment is causing false negatives on systems with more than one WebCLPlatform:

var defaultDevice = wtu.getDevices(webCLPlatform, webcl.DEVICE_TYPE_DEFAULT)[0];
if (contextDevices[0].getInfo(webcl.DEVICE_TYPE) !== defaultDevice.getInfo(webcl.DEVICE_TYPE)) {
  testFailed("Failed in validating the number of devices, when no argument is passed.");
  return;
}

Here's the problem: webCLPlatform is the platform selected by the user; let's assume the user has selected platform C. The default device of platform C is then compared against the device that was previously selected by webcl.createContext(), which is by no means guaranteed to be on platform C. This will cause a test failure with no valid reason.

I would propose to fix this by simply removing the above code fragment, because it doesn't seem to be adding a whole lot of value. Another alternative is to loop over all platforms, get the default device from each, and compare contextDevices[0] against those.

shilpashri commented 10 years ago

There is no mention about the platform associated with context to be created for webcl.createContext() call in the spec.

OpenCL spec says " properties can be NULL in which case the platform that is selected is implementation-defined" [http://www.khronos.org/registry/cl/sdk/1.1/docs/man/xhtml/clCreateContext.html] I guess its worth mentioning default behavior in WebCL spec when no param is passed.

SharathKamathK commented 10 years ago

Also if its "implementation-defined" in the Spec, we can remove these checks from test case.

toaarnio commented 10 years ago

Yes, I agree these checks should be removed.

SharathKamathK commented 10 years ago

Can we have this "implementation-defined" somewhere in the spec ?

toaarnio commented 10 years ago

Sure. See Khronos Bugzilla at http://www.khronos.org/bugzilla/show_bug.cgi?id=1171, tentative spec changes at https://github.com/KhronosGroup/WebCL/compare/7b3883a2f4131a236ec30e19ca5dd72a9c7a60ee...master.

shilpashri commented 10 years ago

Fixed by 398791e3730f9ced784a131cbec0d4ae4c75f6c9 . Please review.

toaarnio commented 10 years ago

Appears to be fixed.