[Beignet] [PATCH V2] Support multi-source CL program

[Zhigang Gong]

Pushed. Thanks for the patch and the review comments.

On Wed, Jun 19, 2013 at 12:56:08PM +0800, He Junyan wrote:
> Reviewed, OK
> Thanks
> 
> On 06/19/2013 10:30 AM, Ruiling Song wrote:
> >Also fix several errors on clBuildProgram.
> >
> >v2: minor refine of cl_program_release_sources()
> >
> >Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> >---
> >  src/cl_api.c     |    5 ++++-
> >  src/cl_program.c |   38 +++++++++++++++++++++-----------------
> >  src/cl_program.h |    3 +--
> >  3 files changed, 26 insertions(+), 20 deletions(-)
> >
> >diff --git a/src/cl_api.c b/src/cl_api.c
> >index f14bee4..d8787cd 100644
> >--- a/src/cl_api.c
> >+++ b/src/cl_api.c
> >@@ -696,6 +696,7 @@ clBuildProgram(cl_program            program,
> >    INVALID_VALUE_IF (num_devices > 1);
> >    INVALID_VALUE_IF (num_devices == 0 && device_list != NULL);
> >    INVALID_VALUE_IF (num_devices != 0 && device_list == NULL);
> >+  INVALID_VALUE_IF (pfn_notify  == 0 && user_data   != NULL);
> >    /* Everything is easy. We only support one device anyway */
> >    if (num_devices != 0) {
> >@@ -706,7 +707,9 @@ clBuildProgram(cl_program            program,
> >    /* TODO support create program from binary */
> >    assert(program->source_type == FROM_LLVM ||
> >           program->source_type == FROM_SOURCE);
> >-  cl_program_build(program, options);
> >+  if((err = cl_program_build(program, options)) != CL_SUCCESS) {
> >+    goto error;
> >+  }
> >    program->is_built = CL_TRUE;
> >    if (pfn_notify) pfn_notify(program, user_data);
> >diff --git a/src/cl_program.c b/src/cl_program.c
> >index 6acf31f..1b49b84 100644
> >--- a/src/cl_program.c
> >+++ b/src/cl_program.c
> >@@ -36,13 +36,10 @@
> >  static void
> >  cl_program_release_sources(cl_program p)
> >  {
> >-  size_t i;
> >-  if (p->sources == NULL) return;
> >-  for (i = 0; i < p->src_n; ++i)
> >-    if (p->sources[i]) cl_free(p->sources[i]);
> >-  cl_free(p->sources);
> >-  p->sources = NULL;
> >-  p->src_n = 0;
> >+  if (p->source) {
> >+    cl_free(p->source);
> >+    p->source = NULL;
> >+  }
> >  }
> >  LOCAL void
> >@@ -234,27 +231,36 @@ cl_program_create_from_source(cl_context ctx,
> >    cl_program program = NULL;
> >    cl_int err = CL_SUCCESS;
> >    cl_uint i;
> >-
> >+  int32_t * lens = NULL;
> >+  int32_t len_total = 0;
> >    assert(ctx);
> >-
> >+  char * p = NULL;
> >    // the real compilation step will be done at build time since we do not have
> >    // yet the compilation options
> >    program = cl_program_new(ctx);
> >-  TRY_ALLOC (program->sources, cl_calloc(count, sizeof(char*)));
> >+  TRY_ALLOC (lens, cl_calloc(count, sizeof(int32_t)));
> >    for (i = 0; i < (int) count; ++i) {
> >      size_t len;
> >      if (lengths == NULL || lengths[i] == 0)
> >        len = strlen(strings[i]);
> >      else
> >        len = lengths[i];
> >-    TRY_ALLOC (program->sources[i], cl_calloc(len+1, sizeof(char)));
> >-    memcpy(program->sources[i], strings[i], len);
> >-    program->sources[i][len] = 0;
> >+    lens[i] = len;
> >+    len_total += len;
> >    }
> >-  program->src_n = count;
> >+  TRY_ALLOC(program->source, cl_calloc(len_total+1, sizeof(char)));
> >+  p = program->source;
> >+  for (i = 0; i < (int) count; ++i) {
> >+    memcpy(p, strings[i], lens[i]);
> >+    p += lens[i];
> >+  }
> >+  *p = '\0';
> >+
> >    program->source_type = FROM_SOURCE;
> >  exit:
> >+  cl_free(lens);
> >+  lens = NULL;
> >    if (errcode_ret)
> >      *errcode_ret = err;
> >    return program;
> >@@ -270,9 +276,7 @@ cl_program_build(cl_program p, const char *options)
> >    cl_int err = CL_SUCCESS;
> >    if (p->source_type == FROM_SOURCE) {
> >-    /* XXX support multiple sources later */
> >-    FATAL_IF (p->src_n != 1, "Only ONE source file supported");
> >-    p->opaque = gbe_program_new_from_source(p->sources[0], 0, options, NULL, NULL);
> >+    p->opaque = gbe_program_new_from_source(p->source, 0, options, NULL, NULL);
> >      if (UNLIKELY(p->opaque == NULL)) {
> >        err = CL_INVALID_PROGRAM;
> >        goto error;
> >diff --git a/src/cl_program.h b/src/cl_program.h
> >index 161d858..3e3edab 100644
> >--- a/src/cl_program.h
> >+++ b/src/cl_program.h
> >@@ -47,8 +47,7 @@ struct _cl_program {
> >    cl_context ctx;         /* Its parent context */
> >    char *bin;              /* The program copied verbatim */
> >    size_t bin_sz;          /* Its size in memory */
> >-  char **sources;         /* Program sources */
> >-  size_t src_n;           /* Number of sources */
> >+  char *source;           /* Program sources */
> >    uint32_t ker_n;         /* Number of declared kernels */
> >    uint32_t source_type:2; /* Built from binary, source or LLVM */
> >    uint32_t is_built:1;    /* Did we call clBuildProgram on it? */
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet