[Beignet] [PATCH] [PATCH]Fix possible warnings before enable ICC compiler

Lv, Meng meng.lv at intel.com
Wed Aug 6 23:55:40 PDT 2014


Why do we need this weird delete()??
Because we have overload the the "new" operation, it is better to overload a corresponding "delete" operation. For ICC compiler ,if you do not offer the pairs of new and delete, it would warn.
I noticed you remove the "inline", is it because the function too big for ICC?
It seems would introduce warning in ICC compiler, but at last I found the root reason of warning is introducing by the redefine of some data, I would refine this patch.
-----Original Message-----
From: Song, Ruiling 
Sent: Thursday, August 07, 2014 2:09 PM
To: Lv, Meng; beignet at lists.freedesktop.org
Cc: Lv, Meng
Subject: RE: [Beignet] [PATCH] [PATCH]Fix possible warnings before enable ICC compiler

It is better you add one or two lines of commit message to describe what kind of warnings are fixed in you patch.

--- a/backend/src/llvm/llvm_to_gen.cpp
+++ b/backend/src/llvm/llvm_to_gen.cpp
@@ -186,7 +186,10 @@ namespace gbe
 
     // Get the module from its file
     llvm::SMDiagnostic Err;
-    std::auto_ptr<Module> M;
+    //In C++0x std::auto_ptr will be deprecated in favor of std::unique_ptr
+    //std::auto_ptr<Module> M;
+    std::unique_ptr<Module> M;
+

For such kind of fix, it's better you simply remove the old code. Don't leave it here with comment.
For the comment line "In C++0x std::auto_ptr will be deprecated in favor of std::unique_ptr", you can place it in commit message.

     if(fileName){
       // only when module is null, Get the global LLVM context
       llvm::LLVMContext& c = llvm::getGlobalContext(); diff --git a/backend/src/sys/alloc.hpp b/backend/src/sys/alloc.hpp index 8fcb3a7..7189d8b 100644
--- a/backend/src/sys/alloc.hpp
+++ b/backend/src/sys/alloc.hpp
@@ -79,7 +79,8 @@ public: \
   void* operator new(size_t size, void *p) { return p; } \
   void* operator new[](size_t size, void *p) { return p; } \
   void  operator delete(void* ptr) { return gbe::alignedFree(ptr); } \
-  void  operator delete[](void* ptr) { return gbe::alignedFree(ptr); }
+  void  operator delete[](void* ptr) { return gbe::alignedFree(ptr); } 
+ \  void  operator delete(void* ptr,void *p) { }

Why do we need this weird delete()??
 
 /*! Macros to handle allocation position */  #define GBE_NEW(T,...) \ diff --git a/src/cl_api.c b/src/cl_api.c index 177a7e8..d74df40 100644
--- a/src/cl_api.c
+++ b/src/cl_api.c
@@ -90,10 +90,10 @@ handle_events(cl_command_queue queue, cl_int num, const cl_event *wait_list,  }
 
 /* The following code checking overlap is from Appendix of openCL spec 1.1 */ -inline cl_bool check_copy_overlap(const size_t src_offset[3],
-                                  const size_t dst_offset[3],
-                                  const size_t region[3],
-                                  size_t row_pitch, size_t slice_pitch)
+cl_bool check_copy_overlap(const size_t src_offset[3],
+                           const size_t dst_offset[3],
+                           const size_t region[3],
+                           size_t row_pitch, size_t slice_pitch)
 {
   const size_t src_min[] = {src_offset[0], src_offset[1], src_offset[2]};
   const size_t src_max[] = {src_offset[0] + region[0], diff --git a/src/cl_mem.h b/src/cl_mem.h index 4477240..57f38f1 100644

I noticed you remove the "inline", is it because the function too big for ICC?


More information about the Beignet mailing list