use table to define and query binary headers.

Submitted by Guo Yejun on Oct. 19, 2015, 3:18 a.m.

Details

Message ID 854E8DBA9F41904AB047E03BB6963AE52294BDAE@SHSMSX101.ccr.corp.intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Guo Yejun Oct. 19, 2015, 3:18 a.m.
-----Original Message-----
From: Beignet [mailto:beignet-bounces@lists.freedesktop.org] On Behalf Of xionghu.luo@intel.com

Sent: Friday, October 16, 2015 1:57 PM
To: beignet@lists.freedesktop.org
Cc: Luo, Xionghu
Subject: [Beignet] [PATCH] use table to define and query binary headers.

From: Luo Xionghu <xionghu.luo@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.

Signed-off-by: Luo Xionghu <xionghu.luo@intel.com>

---
 backend/src/backend/gen_program.cpp | 107 ++++++++++++++++++++++--------------
 src/cl_program.c                    |  51 ++++++++++-------
 2 files changed, 95 insertions(+), 63 deletions(-)



+
+#define MATCH_DEVICE(typeA, binary) ((IS_IVYBRIDGE(typeA) && MATCH_IVB_HEADER(binary)) ||  \
+                                      (IS_IVYBRIDGE(typeA) && MATCH_IVB_HEADER(binary)) ||  \
+                                      (IS_BAYTRAIL_T(typeA) && MATCH_BYT_HEADER(binary)) ||  \
+                                      (IS_HASWELL(typeA) && MATCH_HSW_HEADER(binary)) ||  \
+                                      (IS_BROADWELL(typeA) && MATCH_BDW_HEADER(binary)) ||  \
+                                      (IS_CHERRYVIEW(typeA) && MATCH_CHV_HEADER(binary)) ||  \
+                                      (IS_SKYLAKE(typeA) && MATCH_SKL_HEADER(binary)) )

[yejun] typeA has the meaning as device id, how about use devid instead of typeA directly for easy maintaining?

 

-#define isBitcode(BufPtr,BufEnd)  (isBitcodeWrapper(BufPtr, BufEnd) || isRawBitcode(BufPtr, BufEnd))
+static const unsigned char binary_type_header[5][5]=  \
+                                              {{'B','C', 0xC0, 0xDE},
+                                               {1, 'B', 'C', 0xC0, 0xDE},
+                                               {2, 'B', 'C', 0xC0, 0xDE},
+                                               {0, 'G','E', 'N', 'C'},
+                                               {0}};
+
+#define isSPIR(BufPtr)      binaryCompare4(BufPtr, binary_type_header[0])
+#define isLLVM_C_O(BufPtr)  binaryCompare5(BufPtr, binary_type_header[1])
+#define isLLVM_LIB(BufPtr)  binaryCompare5(BufPtr, binary_type_header[2])
+#define isGenBinary(BufPtr) binaryCompare5(BufPtr, binary_type_header[3])

[Yejun] we can define a meaningful enum instead of magic number, :)
 
 LOCAL cl_program
 cl_program_create_from_binary(cl_context             ctx,
@@ -216,7 +221,7 @@ cl_program_create_from_binary(cl_context             ctx,
     goto error;
   }
 
-  if (lengths[0] == 0) {
+  if (lengths[0] == 0 || lengths[0] < 4) {

[yejun] the min length to know the binary type is 4, maybe we need add a comment here for the magic number 4.

     

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

Patch hide | download patch | download mbox

diff --git a/backend/src/backend/gen_program.cpp b/backend/src/backend/gen_program.cpp
index 233dfe9..28d4ab9 100644
--- a/backend/src/backend/gen_program.cpp
+++ b/backend/src/backend/gen_program.cpp
@@ -205,32 +205,60 @@  namespace gbe {
   }
 
 #define BINARY_HEADER_LENGTH 8

[yejun] how about to change to GEN_BINARY_HEADER_LENGTH? Because the binary has four types: llvm_lib, llvm_co, spir, gen, and the here are for the gen binary.



+static const unsigned char gen_binary_header[7][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'},
+                                              {0}};
+

[Yejun] how about add a new ENUM such as GEN_BINARY_HERDER_INDEX instead of the magic number? For example
enum GEN_BINARY_HERDER_INDEX {
  GBHI_BYT = 0,
 GBHI_IVB = 1,
...
 GBHI_SKL = 5,
 GBHI_MAX = GBHI_SKL
};
And so static const unsigned char gen_binary_header[GBHI_MAX][GEN_BINARY_HEADER_LENGTH] = ...


+#define FILL_GEN_HEADER(binary, index)  do {int i = 0; do {*(binary+i) = gen_binary_header[index][i]; i++; }while(i < BINARY_HEADER_LENGTH);}while(0)
+#define FILL_BYT_HEADER(binary) FILL_GEN_HEADER(binary, 0)
+#define FILL_IVB_HEADER(binary) FILL_GEN_HEADER(binary, 1)
+#define FILL_HSW_HEADER(binary) FILL_GEN_HEADER(binary, 2)
+#define FILL_CHV_HEADER(binary) FILL_GEN_HEADER(binary, 3)
+#define FILL_BDW_HEADER(binary) FILL_GEN_HEADER(binary, 4)
+#define FILL_SKL_HEADER(binary) FILL_GEN_HEADER(binary, 5)

[yejun] use the enum defined above instead of 0/1/2/...

+
+   static bool binaryCompare8(const unsigned char *BufPtr1, const unsigned char* BufPtr2)
+   {
+     return BufPtr1[0] == BufPtr2[0] &&
+       BufPtr1[1] == BufPtr2[1] &&
+       BufPtr1[2] == BufPtr2[2] &&
+       BufPtr1[3] == BufPtr2[3] &&
+       BufPtr1[4] == BufPtr2[4] &&
+       BufPtr1[5] == BufPtr2[5] &&
+       BufPtr1[6] == BufPtr2[6] &&
+       BufPtr1[7] == BufPtr2[7];
+   }
+
+#define MATCH_BYT_HEADER(binary) binaryCompare8(binary, gen_binary_header[0])
+#define MATCH_IVB_HEADER(binary) binaryCompare8(binary, gen_binary_header[1])
+#define MATCH_HSW_HEADER(binary) binaryCompare8(binary, gen_binary_header[2])
+#define MATCH_CHV_HEADER(binary) binaryCompare8(binary, gen_binary_header[3])
+#define MATCH_BDW_HEADER(binary) binaryCompare8(binary, gen_binary_header[4])
+#define MATCH_SKL_HEADER(binary) binaryCompare8(binary, gen_binary_header[5])

[Yejun] change to code like:
#define MATCH_SKL_HEADER(binary) headerCompare(binary, GBHI_SKL)
headerCompare(unsigned char* bin, GEN_BINARY_HERDER_INDEX index)
{
	goldHeader = gen_binary_header[index];
 	//for (int I =0; I < GEN_BINARY_HEADER_LENGTH; ++i)
	{}
}