improved Exception handling in CLProgram and CLKernel.

introduced CLResource interface for releasable resources.
CLProgram is now rebuildable.

8 files changed, 194 insertions, 57 deletions

diff --git a/src/com/mbien/opencl/CLBuffer.java b/src/com/mbien/opencl/CLBuffer.java
index 42b9586d..f640ac1d 100644
--- a/src/com/mbien/opencl/CLBuffer.java
+++ b/src/com/mbien/opencl/CLBuffer.java
@@ -13,7 +13,7 @@ import static com.mbien.opencl.CLException.*;
  *
  * @author Michael Bien
  */
-public class CLBuffer<B extends Buffer> {
+public class CLBuffer<B extends Buffer> implements CLResource {
 
     public final B buffer;
     public final long ID;
diff --git a/src/com/mbien/opencl/CLCommandQueue.java b/src/com/mbien/opencl/CLCommandQueue.java
index 991b0088..d488d1c4 100644
--- a/src/com/mbien/opencl/CLCommandQueue.java
+++ b/src/com/mbien/opencl/CLCommandQueue.java
@@ -15,7 +15,7 @@ import static com.mbien.opencl.CLException.*;
  * perform appropriate synchronization.
  * @author Michael Bien
  */
-public class CLCommandQueue {
+public class CLCommandQueue implements CLResource {
 
     public final long ID;
     private final CLContext context;
@@ -146,9 +146,9 @@ public class CLCommandQueue {
     }
 */
 
-    public CLCommandQueue putNDRangeKernel(CLKernel kernel, int workDimension, long globalWorkOffset, long globalWorkSize, long localWorkSize) {
+    public CLCommandQueue put1DRangeKernel(CLKernel kernel, long globalWorkOffset, long globalWorkSize, long localWorkSize) {
         return this.putNDRangeKernel(
-                kernel, workDimension,
+                kernel, 1,
                 globalWorkOffset==0 ? null : new long[] {globalWorkOffset},
                 globalWorkSize  ==0 ? null : new long[] {globalWorkSize  },
                 localWorkSize   ==0 ? null : new long[] {localWorkSize   }  );
diff --git a/src/com/mbien/opencl/CLContext.java b/src/com/mbien/opencl/CLContext.java
index 757e0498..04df962b 100644
--- a/src/com/mbien/opencl/CLContext.java
+++ b/src/com/mbien/opencl/CLContext.java
@@ -29,7 +29,7 @@ import static com.sun.gluegen.runtime.BufferFactory.*;
  * specified in the context.
  * @author Michael Bien
  */
-public final class CLContext {
+public final class CLContext implements CLResource {
 
     final CL cl;
     public final long ID;
@@ -180,7 +180,10 @@ public final class CLContext {
      * @throws IOException when a IOException occurred while reading or closing the stream.
      */
     public CLProgram createProgram(InputStream sources) throws IOException {
-        
+
+        if(sources == null)
+            throw new IllegalArgumentException("input stream for program sources must not be null");
+
         BufferedReader reader = new BufferedReader(new InputStreamReader(sources));
         StringBuilder sb = new StringBuilder();
 
diff --git a/src/com/mbien/opencl/CLException.java b/src/com/mbien/opencl/CLException.java
index c080b164..db4426f8 100644
--- a/src/com/mbien/opencl/CLException.java
+++ b/src/com/mbien/opencl/CLException.java
@@ -6,20 +6,23 @@ package com.mbien.opencl;
  */
 public class CLException extends RuntimeException {
 
-    public CLException(Throwable cause) {
-        super(cause);
-    }
-
-    public CLException(String message, Throwable cause) {
-        super(message, cause);
-    }
-
-    public CLException(String message) {
-        super(message);
-    }
+    public final int errorcode;
+
+//    public CLException(Throwable cause) {
+//        super(cause);
+//    }
+//
+//    public CLException(String message, Throwable cause) {
+//        super(message, cause);
+//    }
+//
+//    public CLException(String message) {
+//        super(message);
+//    }
 
     public CLException(int error, String message) {
         super(identifyError(error) + ": " + message);
+        errorcode = error;
     }
 
     public static final void checkForError(int status, String message) {
diff --git a/src/com/mbien/opencl/CLKernel.java b/src/com/mbien/opencl/CLKernel.java
index a4ee9f89..cadc635d 100644
--- a/src/com/mbien/opencl/CLKernel.java
+++ b/src/com/mbien/opencl/CLKernel.java
@@ -8,13 +8,18 @@ import java.nio.ByteOrder;
 import static com.mbien.opencl.CLException.*;
 
 /**
- *
+ * High level abstraction for an OpenCL Kernel.
+ * "A kernel is a function declared in a program. A kernel is identified by the __kernel qualifier
+ * applied to any function in a program. A kernel object encapsulates the specific __kernel
+ * function declared in a program and the argument values to be used when executing this
+ * __kernel function."
  * @author Michael Bien
  */
-public class CLKernel {
+public class CLKernel implements CLResource {
 
     public final long ID;
     public final String name;
+    public final int numArgs;
 
     private final CLProgram program;
     private final CL cl;
@@ -26,6 +31,7 @@ public class CLKernel {
 
         long[] longArray = new long[1];
 
+        // get function name
         int ret = cl.clGetKernelInfo(ID, CL.CL_KERNEL_FUNCTION_NAME, 0, null, longArray, 0);
         checkForError(ret, "error while asking for kernel function name");
 
@@ -36,38 +42,53 @@ public class CLKernel {
 
         this.name = new String(bb.array(), 0, bb.capacity()).trim();
 
+        // get number of arguments
+        ret = cl.clGetKernelInfo(ID, CL.CL_KERNEL_NUM_ARGS, 0, null, longArray, 0);
+        checkForError(ret, "error while asking for number of function arguments.");
+
+        numArgs = (int)longArray[0];
+
     }
 
     public CLKernel setArg(int argumentIndex, CLBuffer<?> value) {
-        int ret = cl.clSetKernelArg(ID, argumentIndex, CPU.is32Bit()?4:8, wrap(value.ID));
-        checkForError(ret, "error on clSetKernelArg");
+        setArgument(argumentIndex, CPU.is32Bit()?4:8, wrap(value.ID));
         return this;
     }
 
     public CLKernel setArg(int argumentIndex, int value) {
-        int ret = cl.clSetKernelArg(ID, argumentIndex, 4, wrap(value));
-        checkForError(ret, "error on clSetKernelArg");
+        setArgument(argumentIndex, 4, wrap(value));
         return this;
     }
 
     public CLKernel setArg(int argumentIndex, long value) {
-        int ret = cl.clSetKernelArg(ID, argumentIndex, 8, wrap(value));
-        checkForError(ret, "error on clSetKernelArg");
+        setArgument(argumentIndex, 8, wrap(value));
         return this;
     }
 
     public CLKernel setArg(int argumentIndex, float value) {
-        int ret = cl.clSetKernelArg(ID, argumentIndex, 4, wrap(value));
-        checkForError(ret, "error on clSetKernelArg");
+        setArgument(argumentIndex, 4, wrap(value));
         return this;
     }
 
     public CLKernel setArg(int argumentIndex, double value) {
-        int ret = cl.clSetKernelArg(ID, argumentIndex, 8, wrap(value));
-        checkForError(ret, "error on clSetKernelArg");
+        setArgument(argumentIndex, 8, wrap(value));
         return this;
     }
 
+    private final void setArgument(int argumentIndex, int size, Buffer value) {
+        if(argumentIndex >= numArgs || argumentIndex < 0) {
+            throw new IndexOutOfBoundsException("kernel "+ toString() +" has "+numArgs+
+                    " arguments, can not set argument number "+argumentIndex);
+        }
+        if(!program.isExecutable()) {
+            throw new IllegalStateException("can not set program" +
+                    " arguments for a not excecutable program. "+program);
+        }
+
+        int ret = cl.clSetKernelArg(ID, argumentIndex, size, value);
+        checkForError(ret, "error on clSetKernelArg");
+    }
+
     private final Buffer wrap(float value) {
         return BufferFactory.newDirectByteBuffer(4).putFloat(value).rewind();
     }
diff --git a/src/com/mbien/opencl/CLProgram.java b/src/com/mbien/opencl/CLProgram.java
index 5b00898f..57c242ba 100644
--- a/src/com/mbien/opencl/CLProgram.java
+++ b/src/com/mbien/opencl/CLProgram.java
@@ -12,23 +12,23 @@ import static com.mbien.opencl.CLException.*;
  *
  * @author Michael Bien
  */
-public class CLProgram {
+public class CLProgram implements CLResource {
     
     public final CLContext context;
     public final long ID;
     
     private final CL cl;
 
-    private final Map<String, CLKernel> kernels;
-    
+    private Map<String, CLKernel> kernels;
+    private Map<CLDevice, Status> buildStatusMap;
+
+    private boolean executable;
 
     CLProgram(CLContext context, String src, long contextID) {
         
         this.cl = context.cl;
         this.context = context;
 
-        this.kernels = new HashMap<String, CLKernel>();
-
         int[] intArray = new int[1];
         // Create the program
         ID = cl.clCreateProgramWithSource(contextID, 1, new String[] {src}, new long[]{src.length()}, 0, intArray, 0);
@@ -37,19 +37,49 @@ public class CLProgram {
 
     private final void initKernels() {
 
-        if(kernels.isEmpty()) {
-            int[] intArray = new int[1];
-            int ret = cl.clCreateKernelsInProgram(ID, 0, null, 0, intArray, 0);
-            checkForError(ret, "can not create kernels for program");
+        if(kernels == null) {
 
-            long[] kernelIDs = new long[intArray[0]];
-            ret = cl.clCreateKernelsInProgram(ID, kernelIDs.length, kernelIDs, 0, null, 0);
+            int[] numKernels = new int[1];
+            int ret = cl.clCreateKernelsInProgram(ID, 0, null, 0, numKernels, 0);
             checkForError(ret, "can not create kernels for program");
 
-            for (int i = 0; i < intArray[0]; i++) {
-                CLKernel kernel = new CLKernel(this, kernelIDs[i]);
-                kernels.put(kernel.name, kernel);
+            if(numKernels[0] > 0) {
+                HashMap<String, CLKernel> map = new HashMap<String, CLKernel>();
+
+                long[] kernelIDs = new long[numKernels[0]];
+                ret = cl.clCreateKernelsInProgram(ID, kernelIDs.length, kernelIDs, 0, null, 0);
+                checkForError(ret, "can not create kernels for program");
+
+                for (int i = 0; i < kernelIDs.length; i++) {
+                    CLKernel kernel = new CLKernel(this, kernelIDs[i]);
+                    map.put(kernel.name, kernel);
+                }
+                this.kernels = map;
+            }else{
+                initBuildStatus();
+                if(!isExecutable()) {
+                    // It is illegal to create kernels from a not executable program.
+                    // For consistency between AMD and NVIDIA drivers throw an exception at this point.
+                    throw new CLException(CL.CL_INVALID_PROGRAM_EXECUTABLE,
+                            "can not initialize kernels, program is not executable. status: "+buildStatusMap);
+                }
+            }
+        }
+    }
+
+    private final void initBuildStatus() {
+
+        if(buildStatusMap == null) {
+            Map<CLDevice, Status> map = new HashMap<CLDevice, Status>();
+            CLDevice[] devices = getCLDevices();
+            for (CLDevice device : devices) {
+                Status status = getBuildStatus(device);
+                if(status == Status.BUILD_SUCCESS) {
+                    executable = true;
+                }
+                map.put(device, status);
             }
+            this.buildStatusMap = Collections.unmodifiableMap(map);
         }
     }
 
@@ -125,12 +155,19 @@ public class CLProgram {
     }
 
     /**
-     * Builds this program for the given devices and with the specified build options.
+     * Builds this program for the given devices and with the specified build options. In case this program was
+     * already built and there are kernels associated with this program they will be released first before rebuild.
      * @return this
      * @param devices A list of devices this program should be build on or null for all devices of its context.
      */
     public CLProgram build(CLDevice[] devices, String options) {
 
+        if(kernels != null) {
+            //No changes to the program executable are allowed while there are
+            //kernel objects associated with a program object.
+            releaseKernels();
+        }
+
         long[] deviceIDs = null;
         if(devices != null) {
             deviceIDs = new long[devices.length];
@@ -139,6 +176,10 @@ public class CLProgram {
             }
         }
 
+        // invalidate build status
+        buildStatusMap = null;
+        executable = false;
+
         // Build the program
         int ret = cl.clBuildProgram(ID, deviceIDs, options, null, null);
 
@@ -154,21 +195,25 @@ public class CLProgram {
     }
 
     /**
-     * Releases this program.
+     * Releases this program with its kernels.
      */
     public void release() {
 
-        if(!kernels.isEmpty()) {
+        releaseKernels();
+
+        int ret = cl.clReleaseProgram(ID);
+        context.onProgramReleased(this);
+        checkForError(ret, "can not release program");
+    }
+
+    private void releaseKernels() {
+        if(kernels != null) {
             String[] names = kernels.keySet().toArray(new String[kernels.size()]);
             for (String name : names) {
                 kernels.get(name).release();
             }
+            kernels = null;
         }
-
-        int ret = cl.clReleaseProgram(ID);
-        context.onProgramReleased(this);
-        checkForError(ret, "can not release program");
-        
     }
 
     /**
@@ -234,12 +279,17 @@ public class CLProgram {
      * Returns the build status enum of this program for each device as Map.
      */
     public Map<CLDevice,Status> getBuildStatus() {
-        Map<CLDevice,Status> statusMap = new HashMap<CLDevice, Status>();
-        CLDevice[] devices = getCLDevices();
-        for (CLDevice device : devices) {
-            statusMap.put(device, getBuildStatus(device));
-        }
-        return Collections.unmodifiableMap(statusMap);
+        initBuildStatus();
+        return buildStatusMap;
+    }
+
+    /**
+     * Returns true if the build status 'BUILD_SUCCESS' for at least one device
+     * of this program exists.
+     */
+    public boolean isExecutable() {
+        initBuildStatus();
+        return executable;
     }
 
     /**
@@ -298,6 +348,12 @@ public class CLProgram {
     }
 
     @Override
+    public String toString() {
+        return "CLProgram [id: " + ID
+                       + " status: "+getBuildStatus()+"]";
+    }
+
+    @Override
     public boolean equals(Object obj) {
         if (obj == null) {
             return false;
diff --git a/src/com/mbien/opencl/CLResource.java b/src/com/mbien/opencl/CLResource.java
new file mode 100644
index 00000000..4f0d9d01
--- /dev/null
+++ b/src/com/mbien/opencl/CLResource.java
@@ -0,0 +1,14 @@
+package com.mbien.opencl;
+
+/**
+ * Releasable OpenCL resource.
+ * @author Michael Bien
+ */
+public interface CLResource {
+
+    /**
+     * Releases the OpenCL resource.
+     */
+    public void release();
+
+}
diff --git a/test/com/mbien/opencl/HighLevelBindingTest.java b/test/com/mbien/opencl/HighLevelBindingTest.java
index edeedc6b..2a8807ad 100644
--- a/test/com/mbien/opencl/HighLevelBindingTest.java
+++ b/test/com/mbien/opencl/HighLevelBindingTest.java
@@ -225,5 +225,45 @@ public class HighLevelBindingTest {
         out.println("results are valid");
 
     }
+
+    @Test
+    public void rebuildProgramTest() throws IOException {
+
+        out.println(" - - - highLevelTest; rebuild program test - - - ");
+
+        CLContext context = CLContext.create();
+        CLProgram program = context.createProgram(getClass().getResourceAsStream("testkernels.cl"));
+
+        try{
+            program.getCLKernels();
+            fail("expected exception but got none :(");
+        }catch(CLException ex) {
+            out.println("got expected exception:\n"+ex.getMessage());
+            assertTrue(ex.errorcode == CL.CL_INVALID_PROGRAM_EXECUTABLE);
+        }
+
+        program.build();
+        assertTrue(program.isExecutable());
+        out.println(program.getBuildStatus());
+
+        Map<String, CLKernel> kernels = program.getCLKernels();
+        assertNotNull(kernels);
+        assertTrue("kernel map is empty", kernels.size() > 0);
+
+        // rebuild
+        // 1. release kernels (internally)
+        // 2. build program
+        program.build();
+        assertTrue(program.isExecutable());
+        out.println(program.getBuildStatus());
+
+        // try again with rebuilt program
+        kernels = program.getCLKernels();
+        assertNotNull(kernels);
+        assertTrue("kernel map is empty", kernels.size() > 0);
+        assertTrue(kernels.size() > 0);
+
+        context.release();
+    }
     
 }