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

Yang, Rong R rong.r.yang at intel.com
Wed Oct 21 00:56:38 PDT 2015


Push it with change  BHI_MAX and GBHI_MAX manually. 

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Guo, Yejun
> Sent: Wednesday, October 21, 2015 10:59
> To: Luo, Xionghu; beignet at lists.freedesktop.org
> Cc: Luo, Xionghu
> Subject: Re: [Beignet] [PATCH v2] use table to define and query binary
> headers.
> 
> 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
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list