[Beignet] [PATCH v2] use table to define and query binary headers.

Guo, Yejun yejun.guo at intel.com
Tue Oct 20 19:59:02 PDT 2015


Btw, other parts LGTM

-----Original Message-----
From: Guo, Yejun 
Sent: Wednesday, October 21, 2015 10:35 AM
To: Luo, Xionghu; beignet at lists.freedesktop.org
Cc: Luo, Xionghu
Subject: RE: [Beignet] [PATCH v2] use table to define and query binary headers.

For  the two enum, just define the *_MAX, it will '+1' automatically.

+typedef enum _BINARY_HEADER_INDEX {
+  BHI_SPIR = 0,
+  BHI_COMPIRED_OBJECT = 1,
+  BHI_LIBRARY = 2,
+  BHI_GEN_BINARY = 3, 
+  BHI_MAX,
+}BINARY_HEADER_INDEX;


+  enum GEN_BINARY_HEADER_INDEX {
+    GBHI_BYT = 0,
+    GBHI_IVB = 1,
+    GBHI_HSW = 2,
+    GBHI_CHV = 3,
+    GBHI_BDW = 4,
+    GBHI_SKL = 5,
+    GBHI_MAX,
+  };
-----Original Message-----
From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of xionghu.luo at intel.com
Sent: Wednesday, October 21, 2015 8:44 AM
To: beignet at lists.freedesktop.org
Cc: Luo, Xionghu
Subject: [Beignet] [PATCH v2] use table to define and query binary headers.

From: Luo Xionghu <xionghu.luo at intel.com>

currently, we support create program from 4 types of binary: SPIR(BITCODE),
LLVM Compiled Object, LLVM Library and GEN Binary. The detailed formats are
listed in code.
also use table to match or fill gen binary header in backend.

v2: use enum to replace the magic number.

Signed-off-by: Luo Xionghu <xionghu.luo at intel.com>
---
 backend/src/backend/gen_program.cpp | 126 ++++++++++++++++++++++--------------
 src/cl_program.c                    |  57 ++++++++--------
 src/cl_program.h                    |   8 +++
 3 files changed, 117 insertions(+), 74 deletions(-)

diff --git a/backend/src/backend/gen_program.cpp b/backend/src/backend/gen_program.cpp
index 233dfe9..bc9a9fb 100644
--- a/backend/src/backend/gen_program.cpp
+++ b/backend/src/backend/gen_program.cpp
@@ -204,37 +204,72 @@ namespace gbe {
 #endif
   }
 
-#define BINARY_HEADER_LENGTH 8
-#define IS_GEN_BINARY(binary) (*binary == '\0' && *(binary+1) == 'G'&& *(binary+2) == 'E' &&*(binary+3) == 'N' &&*(binary+4) == 'C')
-#define FILL_GEN_BINARY(binary) do{*binary = '\0'; *(binary+1) = 'G'; *(binary+2) = 'E'; *(binary+3) = 'N'; *(binary+4) = 'C';}while(0)
-#define FILL_DEVICE_ID(binary, src_hw_info) do {*(binary+5) = src_hw_info[0]; *(binary+6) = src_hw_info[1]; *(binary+7) = src_hw_info[2];}while(0)
-#define DEVICE_MATCH(typeA, src_hw_info) ((IS_IVYBRIDGE(typeA) && !strcmp(src_hw_info, "IVB")) ||  \
-                                      (IS_IVYBRIDGE(typeA) && !strcmp(src_hw_info, "BYT")) ||  \
-                                      (IS_BAYTRAIL_T(typeA) && !strcmp(src_hw_info, "BYT")) ||  \
-                                      (IS_HASWELL(typeA) && !strcmp(src_hw_info, "HSW")) ||  \
-                                      (IS_BROADWELL(typeA) && !strcmp(src_hw_info, "BDW")) ||  \
-                                      (IS_CHERRYVIEW(typeA) && !strcmp(src_hw_info, "CHV")) ||  \
-                                      (IS_SKYLAKE(typeA) && !strcmp(src_hw_info, "SKL")) )
+#define GEN_BINARY_HEADER_LENGTH 8
+
+  enum GEN_BINARY_HEADER_INDEX {
+    GBHI_BYT = 0,
+    GBHI_IVB = 1,
+    GBHI_HSW = 2,
+    GBHI_CHV = 3,
+    GBHI_BDW = 4,
+    GBHI_SKL = 5,//remember update GBHI_MAX if add option.
+    GBHI_MAX = GBHI_SKL+1,
+  };
+
+  static const unsigned char gen_binary_header[GBHI_MAX][GEN_BINARY_HEADER_LENGTH]= \
+                                             {{0, 'G','E', 'N', 'C', 'B', 'Y', 'T'},
+                                              {0, 'G','E', 'N', 'C', 'I', 'V', 'B'},
+                                              {0, 'G','E', 'N', 'C', 'H', 'S', 'W'},
+                                              {0, 'G','E', 'N', 'C', 'C', 'H', 'V'},
+                                              {0, 'G','E', 'N', 'C', 'B', 'D', 'W'},
+                                              {0, 'G','E', 'N', 'C', 'S', 'K', 'L'}};
+
+#define FILL_GEN_HEADER(binary, index)  do {int i = 0; do {*(binary+i) = gen_binary_header[index][i]; i++; }while(i < GEN_BINARY_HEADER_LENGTH);}while(0)
+#define FILL_BYT_HEADER(binary) FILL_GEN_HEADER(binary, GBHI_BYT)
+#define FILL_IVB_HEADER(binary) FILL_GEN_HEADER(binary, GBHI_IVB)
+#define FILL_HSW_HEADER(binary) FILL_GEN_HEADER(binary, GBHI_HSW)
+#define FILL_CHV_HEADER(binary) FILL_GEN_HEADER(binary, GBHI_CHV)
+#define FILL_BDW_HEADER(binary) FILL_GEN_HEADER(binary, GBHI_BDW)
+#define FILL_SKL_HEADER(binary) FILL_GEN_HEADER(binary, GBHI_SKL)
+
+  static bool genHeaderCompare(const unsigned char *BufPtr, GEN_BINARY_HEADER_INDEX index)
+  {
+    bool matched = true;
+    for (int i =0; i < GEN_BINARY_HEADER_LENGTH; ++i)
+    {
+      matched = matched && (BufPtr[i] == gen_binary_header[index][i]);
+    }
+    return matched;
+  }
+
+#define MATCH_BYT_HEADER(binary) genHeaderCompare(binary, GBHI_BYT)
+#define MATCH_IVB_HEADER(binary) genHeaderCompare(binary, GBHI_IVB)
+#define MATCH_HSW_HEADER(binary) genHeaderCompare(binary, GBHI_HSW)
+#define MATCH_CHV_HEADER(binary) genHeaderCompare(binary, GBHI_CHV)
+#define MATCH_BDW_HEADER(binary) genHeaderCompare(binary, GBHI_BDW)
+#define MATCH_SKL_HEADER(binary) genHeaderCompare(binary, GBHI_SKL)
+
+#define MATCH_DEVICE(deviceID, binary) ((IS_IVYBRIDGE(deviceID) && MATCH_IVB_HEADER(binary)) ||  \
+                                      (IS_IVYBRIDGE(deviceID) && MATCH_IVB_HEADER(binary)) ||  \
+                                      (IS_BAYTRAIL_T(deviceID) && MATCH_BYT_HEADER(binary)) ||  \
+                                      (IS_HASWELL(deviceID) && MATCH_HSW_HEADER(binary)) ||  \
+                                      (IS_BROADWELL(deviceID) && MATCH_BDW_HEADER(binary)) ||  \
+                                      (IS_CHERRYVIEW(deviceID) && MATCH_CHV_HEADER(binary)) ||  \
+                                      (IS_SKYLAKE(deviceID) && MATCH_SKL_HEADER(binary)) )
 
   static gbe_program genProgramNewFromBinary(uint32_t deviceID, const char *binary, size_t size) {
     using namespace gbe;
     std::string binary_content;
+
+    if(size < GEN_BINARY_HEADER_LENGTH)
+      return NULL;
+
     //the header length is 8 bytes: 1 byte is binary type, 4 bytes are bitcode header, 3  bytes are hw info.
-    char src_hw_info[4]="";
-    src_hw_info[0] = *(binary+5);
-    src_hw_info[1] = *(binary+6);
-    src_hw_info[2] = *(binary+7);
-
-    // check whether is gen binary ('/0GENC')
-    if(!IS_GEN_BINARY(binary)){
-        return NULL;
-    }
-    // check the whether the current device ID match the binary file's.
-    if(!DEVICE_MATCH(deviceID, src_hw_info)){
+    if(!MATCH_DEVICE(deviceID, (unsigned char*)binary)){
       return NULL;
     }
 
-    binary_content.assign(binary+BINARY_HEADER_LENGTH, size-BINARY_HEADER_LENGTH);
+    binary_content.assign(binary+GEN_BINARY_HEADER_LENGTH, size-GEN_BINARY_HEADER_LENGTH);
     GenProgram *program = GBE_NEW(GenProgram, deviceID);
     std::istringstream ifs(binary_content, std::ostringstream::binary);
 
@@ -299,39 +334,31 @@ namespace gbe {
 
       //add header to differetiate from llvm bitcode binary.
       //the header length is 8 bytes: 1 byte is binary type, 4 bytes are bitcode header, 3  bytes are hw info.
-      *binary = (char *)malloc(sizeof(char) * (sz+BINARY_HEADER_LENGTH) );
-      memset(*binary, 0, sizeof(char) * (sz+BINARY_HEADER_LENGTH) );
-      FILL_GEN_BINARY(*binary);
-      char src_hw_info[4]="";
+      *binary = (char *)malloc(sizeof(char) * (sz+GEN_BINARY_HEADER_LENGTH) );
+      if(*binary == NULL)
+        return 0;
+
+      memset(*binary, 0, sizeof(char) * (sz+GEN_BINARY_HEADER_LENGTH) );
       if(IS_IVYBRIDGE(prog->deviceID)){
-        src_hw_info[0]='I';
-        src_hw_info[1]='V';
-        src_hw_info[2]='B';
+        FILL_IVB_HEADER(*binary);
         if(IS_BAYTRAIL_T(prog->deviceID)){
-          src_hw_info[0]='B';
-          src_hw_info[1]='Y';
-          src_hw_info[2]='T';
+        FILL_BYT_HEADER(*binary);
         }
       }else if(IS_HASWELL(prog->deviceID)){
-        src_hw_info[0]='H';
-        src_hw_info[1]='S';
-        src_hw_info[2]='W';
+        FILL_HSW_HEADER(*binary);
       }else if(IS_BROADWELL(prog->deviceID)){
-        src_hw_info[0]='B';
-        src_hw_info[1]='D';
-        src_hw_info[2]='W';
+        FILL_BDW_HEADER(*binary);
       }else if(IS_CHERRYVIEW(prog->deviceID)){
-        src_hw_info[0]='C';
-        src_hw_info[1]='H';
-        src_hw_info[2]='V';
+        FILL_CHV_HEADER(*binary);
       }else if(IS_SKYLAKE(prog->deviceID)){
-        src_hw_info[0]='S';
-        src_hw_info[1]='K';
-        src_hw_info[2]='L';
+        FILL_SKL_HEADER(*binary);
+      }else {
+        free(*binary);
+        *binary = NULL;
+        return 0;
       }
-      FILL_DEVICE_ID(*binary, src_hw_info);
-      memcpy(*binary+BINARY_HEADER_LENGTH, oss.str().c_str(), sz*sizeof(char));
-      return sz+BINARY_HEADER_LENGTH;
+      memcpy(*binary+GEN_BINARY_HEADER_LENGTH, oss.str().c_str(), sz*sizeof(char));
+      return sz+GEN_BINARY_HEADER_LENGTH;
     }else{
 #ifdef GBE_COMPILER_AVAILABLE
       std::string str;
@@ -340,6 +367,9 @@ namespace gbe {
       std::string& bin_str = OS.str();
       int llsz = bin_str.size();
       *binary = (char *)malloc(sizeof(char) * (llsz+1) );
+      if(*binary == NULL)
+        return 0;
+
       *(*binary) = binary_type;
       memcpy(*binary+1, bin_str.c_str(), llsz);
       return llsz+1;
diff --git a/src/cl_program.c b/src/cl_program.c
index 55c1ee8..98b6d51 100644
--- a/src/cl_program.c
+++ b/src/cl_program.c
@@ -166,29 +166,30 @@ error:
   return err;
 }
 
-inline cl_bool isBitcodeWrapper(const unsigned char *BufPtr, const unsigned char *BufEnd)
-{
-  // See if you can find the hidden message in the magic bytes :-).
-  // (Hint: it's a little-endian encoding.)
-  return BufPtr != BufEnd &&
-    BufPtr[0] == 0xDE &&
-    BufPtr[1] == 0xC0 &&
-    BufPtr[2] == 0x17 &&
-    BufPtr[3] == 0x0B;
-}
+#define BINARY_HEADER_LENGTH 5
 
-inline cl_bool isRawBitcode(const unsigned char *BufPtr, const unsigned char *BufEnd)
+static const unsigned char binary_type_header[BHI_MAX][BINARY_HEADER_LENGTH]=  \
+                                              {{'B','C', 0xC0, 0xDE},
+                                               {1, 'B', 'C', 0xC0, 0xDE},
+                                               {2, 'B', 'C', 0xC0, 0xDE},
+                                               {0, 'G','E', 'N', 'C'}};
+
+LOCAL cl_bool headerCompare(const unsigned char *BufPtr, BINARY_HEADER_INDEX index)
 {
-  // These bytes sort of have a hidden message, but it's not in
-  // little-endian this time, and it's a little redundant.
-  return BufPtr != BufEnd &&
-    BufPtr[0] == 'B' &&
-    BufPtr[1] == 'C' &&
-    BufPtr[2] == 0xc0 &&
-    BufPtr[3] == 0xde;
+  bool matched = true;
+  int length = index == BHI_SPIR ? BINARY_HEADER_LENGTH -1 :BINARY_HEADER_LENGTH;
+  int i = 0;
+  for (i = 0; i < length; ++i)
+  {
+    matched = matched && (BufPtr[i] == binary_type_header[index][i]);
+  }
+  return matched;
 }
 
-#define isBitcode(BufPtr,BufEnd)  (isBitcodeWrapper(BufPtr, BufEnd) || isRawBitcode(BufPtr, BufEnd))
+#define isSPIR(BufPtr)      headerCompare(BufPtr, BHI_SPIR)
+#define isLLVM_C_O(BufPtr)  headerCompare(BufPtr, BHI_COMPIRED_OBJECT)
+#define isLLVM_LIB(BufPtr)  headerCompare(BufPtr, BHI_LIBRARY)
+#define isGenBinary(BufPtr) headerCompare(BufPtr, BHI_GEN_BINARY)
 
 LOCAL cl_program
 cl_program_create_from_binary(cl_context             ctx,
@@ -216,7 +217,8 @@ cl_program_create_from_binary(cl_context             ctx,
     goto error;
   }
 
-  if (lengths[0] == 0) {
+  //need at least 4 bytes to check the binary type.
+  if (lengths[0] == 0 || lengths[0] < 4) {
     err = CL_INVALID_VALUE;
     if (binary_status)
       binary_status[0] = CL_INVALID_VALUE;
@@ -229,13 +231,12 @@ cl_program_create_from_binary(cl_context             ctx,
       goto error;
   }
 
-  // TODO:  Need to check the binary format here to return CL_INVALID_BINARY.
   TRY_ALLOC(program->binary, cl_calloc(lengths[0], sizeof(char)));
   memcpy(program->binary, binaries[0], lengths[0]);
   program->binary_sz = lengths[0];
   program->source_type = FROM_BINARY;
 
-  if(isBitcode((unsigned char*)program->binary, (unsigned char*)program->binary+program->binary_sz)) {
+  if(isSPIR((unsigned char*)program->binary)) {
 
     char* typed_binary;
     TRY_ALLOC(typed_binary, cl_calloc(lengths[0]+1, sizeof(char)));
@@ -249,10 +250,10 @@ cl_program_create_from_binary(cl_context             ctx,
     }
 
     program->source_type = FROM_LLVM_SPIR;
-  }else if(isBitcode((unsigned char*)program->binary+1, (unsigned char*)program->binary+program->binary_sz)) {
-    if(*program->binary == 1){
+  }else if(isLLVM_C_O((unsigned char*)program->binary) || isLLVM_LIB((unsigned char*)program->binary)) {
+    if(*program->binary == BHI_COMPIRED_OBJECT){
       program->binary_type = CL_PROGRAM_BINARY_TYPE_COMPILED_OBJECT;
-    }else if(*program->binary == 2){
+    }else if(*program->binary == BHI_LIBRARY){
       program->binary_type = CL_PROGRAM_BINARY_TYPE_LIBRARY;
     }else{
       err= CL_INVALID_BINARY;
@@ -266,7 +267,7 @@ cl_program_create_from_binary(cl_context             ctx,
     }
     program->source_type = FROM_LLVM;
   }
-  else if (*program->binary == 0) {
+  else if (isGenBinary((unsigned char*)program->binary)) {
     program->opaque = interp_program_new_from_binary(program->ctx->device->device_id, program->binary, program->binary_sz);
     if (UNLIKELY(program->opaque == NULL)) {
       err = CL_INVALID_PROGRAM;
@@ -277,6 +278,10 @@ cl_program_create_from_binary(cl_context             ctx,
     TRY (cl_program_load_gen_program, program);
     program->binary_type = CL_PROGRAM_BINARY_TYPE_EXECUTABLE;
   }
+  else {
+    err= CL_INVALID_BINARY;
+    goto error;
+  }
 
   if (binary_status)
     binary_status[0] = CL_SUCCESS;
diff --git a/src/cl_program.h b/src/cl_program.h
index 7af0206..47e7e08 100644
--- a/src/cl_program.h
+++ b/src/cl_program.h
@@ -37,6 +37,14 @@ enum {
   FROM_LLVM_SPIR = 3
 };
 
+typedef enum _BINARY_HEADER_INDEX {
+  BHI_SPIR = 0,
+  BHI_COMPIRED_OBJECT = 1,
+  BHI_LIBRARY = 2,
+  BHI_GEN_BINARY = 3, /*remember update BHI_MAX if add option.*/
+  BHI_MAX = BHI_GEN_BINARY+1,
+}BINARY_HEADER_INDEX;
+
 /* This maps an OCL file containing some kernels */
 struct _cl_program {
   DEFINE_ICD(dispatch)
-- 
1.9.1

_______________________________________________
Beignet mailing list
Beignet at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list