4
\$\begingroup\$

I am learning how to program in opencl and I made a simple program that simply takes an array and adds 1 to every element. I want to run it many times (this is just so that i can benchmark how it does against my cpu, I understand it is a dumb application).

I feel like the way I am doing it right now is inefficient because i seem to be instructing the gpu to do each iteration one by one instead of telling it from the start that it has to do it many times.

Here is my code (Please look for the comment "RELEVANT PART STARTS HERE":

#include <iostream>
#include <CL/cl.hpp>


int main(){
    //get all platforms (drivers)
    std::vector<cl::Platform> all_platforms;
    cl::Platform::get(&all_platforms);
    if(all_platforms.size()==0){
        std::cout<<" No platforms found. Check OpenCL installation!\n";
        exit(1);
    }
    cl::Platform default_platform=all_platforms[0];
    std::cout << "Using platform: "<<default_platform.getInfo<CL_PLATFORM_NAME>()<<"\n";

    //get default device of the default platform
    std::vector<cl::Device> all_devices;
    default_platform.getDevices(CL_DEVICE_TYPE_ALL, &all_devices);
    if(all_devices.size()==0){
        std::cout<<" No devices found. Check OpenCL installation!\n";
        exit(1);
    }
    cl::Device default_device=all_devices[0];
    std::cout<< "Using device: "<<default_device.getInfo<CL_DEVICE_NAME>()<<"\n";


    cl::Context context({default_device});

    cl::Program::Sources sources;

    // kernel calculates for each element C=A+B
    std::string kernel_code=
        "   void kernel simple_add(global  int* A){       "
        "       A[get_global_id(0)]=A[get_global_id(0)]+1;                 "
        "   }                                                                               ";
    sources.push_back({kernel_code.c_str(),kernel_code.length()});

    cl::Program program(context,sources);
    if(program.build({default_device})!=CL_SUCCESS){
        std::cout<<" Error building: "<<program.getBuildInfo<CL_PROGRAM_BUILD_LOG>(default_device)<<"\n";
        exit(1);
    }

    const int N = 1e5   ;

    // create buffers on the device
    cl::Buffer buffer_A(context,CL_MEM_READ_WRITE,sizeof(int)*N);


    int A[N] ;

    for(int i = 0;i<N;i++){
        A[i] = i;
    }

    //create queue to which we will push commands for the device.
    cl::CommandQueue queue(context,default_device);

    queue.enqueueWriteBuffer(buffer_A,CL_TRUE,0,sizeof(int)*N,A);

    int C = 1e5;

    // RELEVANT PART STARTS HERE !!!

    for(int i = 0;i<C;i++){
        cl::Kernel kernel_add=cl::Kernel(program,"simple_add");
        kernel_add.setArg(0,buffer_A);
        queue.enqueueNDRangeKernel(kernel_add,cl::NullRange,cl::NDRange(N),cl::NullRange);
        queue.finish();
    }


    // RELEVANT PART ENDS HERE !!!

    queue.enqueueReadBuffer(buffer_A,CL_TRUE,0,sizeof(int)*N,A);

    printf("%d\n",A[N-4]);

    return 0;
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Sorry, but this is a general review and will not answer your specific question. While this is a simple test program, it is good to develop some habits in writing C or C++ programs.

Complexity
One of the good habits to keep in mind is the Single Responsibility Principle which states

"every module, class, or function should have responsibility over a single part of the functionality provided by the software, ...".

The Single Responsibility Principle is the first pillar in SOLID programming. SOLID programming is a set of principles that generally improve object oriented programming.

The code is too complex to be in one function, main() could be divided into may functions that would make it easier to read and debug. Generally main() should do the following when necessary:
- call a function to process command line arguments.
- call functions to set up for any processing
- call one function to do the processing, this function may call many other functions
- call functions to clean up after processing

In this program there doesn't seem to be any command line arguments so that function isn't necessary. Some possible functions called by main():
- Set Up Platforms
- Set Up Devices
- Set Up Program
- Set Up Queues
- Execute Program
Any or all of these functions can return bool or int to indicate success or failure

Use System Defined Constants to Make Code More Readable
Rather than using return(1); or return(0) in main there are constants defined in cstdlib or stdlib.h that indicate success or failure of a program, these are EXIT_SUCCESS and EXIT_FAILURE. Since they are system defined they are more portable than zero or one. They also make the code self documenting:

    return(EXIT_SUCCESS);

    return(EXIT_FAILURE);

Avoid the Use of the Exit Function in C++ and C
The code is good in that it only calls std::exit() from main(), however, it is generally a bad idea to call std::exit() especially from functions that are not main(). When calling exit from a function other than main no cleanup code will be called and if the program is a program that shouldn't terminate such as an operating system there can be dire side effects. Even calling exit from main may not clean up after processing as explained in this stackoverflow.com question. The accepted practice is to call return from main.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.