3 Replies Latest reply on Nov 3, 2009 5:31 PM by omkaranathan

    Proposal for better CL Headers / bug reports

    Sternenprinz

      1) I would like to propose a better format for the standard header cl.h.

      Reason: The usual building and running problems, including the issue having to depent on OpenCL.dll (/.so/whatever lib format) statically, unable to support more than one platform.

      Solution: Minor changes in the way 'cl.h' declares the functions. Simply declare function pointers instead of function prototypes. Additionally there has to be a 'cl.c' that is able to fill the pointers. 

      Example:

      instead of

       

      extern CL_API_ENTRY cl_int CL_API_CALL

      fn_clReleaseEvent(cl_event /* event */) CL_API_SUFFIX__VERSION_1_0;

      a)


      typedef CL_API_ENTRY cl_int 

      (CL_API_CALL *fn_clReleaseEvent)(cl_event /* event */) CL_API_SUFFIX__VERSION_1_0;

      extern fn_clSomeProc clReleaseEvent;

      or the direct way b)

       

      extern CL_API_ENTRY cl_int 

      (CL_API_CALL *clReleaseEvent)(cl_event /* event */) CL_API_SUFFIX__VERSION_1_0;

      // small changes, big impact...

      // Working on the VC series, it should be no problem to get this working

      // on other major compilers, at least with minor changes

      <IMHO>



      There is no good reason that in the year 2009 Headers have to look like in times of K&R. Nearly all compilers will eat function pointer, i guess that those who dont wont get a special lib from Khronos to link to - i.e. those are out the game anyway. So everybody who is not happy with static linking or with using some specific toolchain has to write its own import header. Is this really nescessary?

      Maybe there is a additional overhead with calling function pointers, but this overhead is a) small and b) inevitable for dynamic binding. On the other hand there is no problem to circumvent this with some preprocessor magic.

       

      </IMHO>

      2) The vector class from cl.hpp has a trivial bug, example:

       

       

      This works

      for (vector = begin,vector!=end;++vector) // <-- note the prefix op

      this not

      for (vector = begin,vector!=end;vector++) // <-- note the postfix op

      Semantically both versions do the same. At least a note that the postfix op is broken would be nice.

      3) The whole SDK does not work with VC90 due to 'Library Hell' reasons. What does work are the prebuilt demos/examples. Solution is -surprise- dynamically linking to OpenCL.dll and throwing away the whole SDK.
      4) I could provide patches/mods for 1 & 2, if -and only if- someone in charge is actually willing to test them and maybe propose them to the committe.

       

        • Proposal for better CL Headers / bug reports
          omkaranathan

          Hi Sternenprinz,

          Do you have a test case to reproduce the second issue? Could you post it?

            • Proposal for better CL Headers / bug reports
              Sternenprinz

              Sure, and i will explain what the bug is.

              Example:

              typedef int vectorType; // just for convinience

              cl::vector< int > devices(1,0); // sample vector, type doesnt matter
              for (cl::vector<vectorType>::iterator device=devices.begin();device!=devices.end();++device)
                std::cout << "prefix iterating devices..." << std::endl;
              std::cout << "prefix iteration finished..." << std::endl;
              for (cl::vector<vectorType>::iterator device=devices.begin();device!=devices.end();device++)
                std::cout << "postfix iterating devices..." << std::endl;
              std::cout << "postfix iteration finished..." << std::endl; // will not be reached

              ... will lock up in the second loop, i.e. looping as if the iterator would not be incremented at all. In fact, it does not get incremented at all.

              Root of this evil is 'cl.hpp' Line 693ff (comments by me).

              void operator++(int x) // looks like postfix ++
              {
                 index_ += x; // OMG!

              // according to standard postfix ops must return a new modified object

              // additionally, value of x is undefined,

              // so this one is -sorry- complete bs

              }

              Better:

              iterator // this is a return value

              operator ++(int x) // postfix increment

              {

              // assuming there is a copy constructor,

              // wich in fact does not exist

                iterator ret(*this);

              index_++; // increment

              return ret; // return copy,

              // so any remaining code in the same expression still gets the old value

              }


              Conclusion:

              Both postfix operators are bogus in two ways: a) x is undefined and likely zero b) even if x would be 1, the postfix operator would be a prefix operator.

              Additionally the prefix ops are also not so standard

               

              Some evil comments by me:

              Those who coded cl.hpp obviously are C++ noobies. E.g. enforcing specific values for the various 'getInfo' methods could be better done with Enumerations instead of a template hell. Templates produce in most cases a code bloat. If there would be an speed advantage, there would be no problem (i really like templates). But there is absolutly no gain, the values get evaluated in OpenCL.dll/.so. A lot code is as mentioned roughly working but strange, i.e. not really how c++ code should look like. In fact, it looks like if some Java developers had an excursion to C++. And more...

               

              Finally:

              Have you any idea why i cant get any kernel compiled with >>GPU<< as target? (XP-SP3/VC90/Strem2.0-Beta4). It just returns Error -11.

              Sample Binaries deployed by the official installer work fine, but sample project (HelloCL) build by me report error -11. Any own projects too, for GUI-based projects 3 console windows get openend before error -11 is reported, console projects just report error -11.

                • Proposal for better CL Headers / bug reports
                  omkaranathan

                   

                  Originally posted by: Sternenprinz

                   

                   

                  Have you any idea why i cant get any kernel compiled with >>GPU<< as target? (XP-SP3/VC90/Strem2.0-Beta4). It just returns Error -11.

                   

                  Sample Binaries deployed by the official installer work fine, but sample project (HelloCL) build by me report error -11. Any own projects too, for GUI-based projects 3 console windows get openend before error -11 is reported, console projects just report error -11.

                   



                  Error -11 is CL_BUILD_PROGRAM_FAILURE, do check the buildlog in case of clBuildProgram failures, using clGetProgramBuildInfo API call.