[Beignet] [PATCH 2/5] GBE: don't assert even if we fail to compile kernel at the backend stage.

Zhigang Gong zhigang.gong at intel.com
Thu Nov 12 00:47:02 PST 2015


We should not assert even if the application triggers a internal limitation
such as lack of scratch space. We should return error to the application and
let the application to make further decision.

Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
---
 backend/src/backend/context.cpp            |  3 +--
 backend/src/backend/gen_context.hpp        |  1 +
 backend/src/backend/gen_program.cpp        |  2 +-
 backend/src/backend/gen_reg_allocation.cpp | 18 ++++++++++++------
 backend/src/backend/program.cpp            | 22 +++++++++++++++-------
 5 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/backend/src/backend/context.cpp b/backend/src/backend/context.cpp
index 51d643e..47d8a45 100644
--- a/backend/src/backend/context.cpp
+++ b/backend/src/backend/context.cpp
@@ -229,8 +229,7 @@ namespace gbe
       // We have a valid offset now
       return aligned;
     }
-    GBE_ASSERT( !assertFail );
-    return 0;
+    return -1;
   }
 
   void SimpleAllocator::deallocate(int32_t offset)
diff --git a/backend/src/backend/gen_context.hpp b/backend/src/backend/gen_context.hpp
index 155b68e..c622236 100644
--- a/backend/src/backend/gen_context.hpp
+++ b/backend/src/backend/gen_context.hpp
@@ -49,6 +49,7 @@ namespace gbe
     REGISTER_ALLOCATION_FAIL,
     REGISTER_SPILL_EXCEED_THRESHOLD,
     REGISTER_SPILL_FAIL,
+    REGISTER_SPILL_NO_SPACE,
     OUT_OF_RANGE_IF_ENDIF,
   } CompileErrorCode;
 
diff --git a/backend/src/backend/gen_program.cpp b/backend/src/backend/gen_program.cpp
index bb22542..a12ab39 100644
--- a/backend/src/backend/gen_program.cpp
+++ b/backend/src/backend/gen_program.cpp
@@ -197,7 +197,7 @@ namespace gbe {
         GBE_ASSERT(!(ctx->getErrCode() == OUT_OF_RANGE_IF_ENDIF && ctx->getIFENDIFFix()));
     }
 
-    GBE_ASSERTM(kernel != NULL, "Fail to compile kernel, may need to increase reserved registers for spilling.");
+    //GBE_ASSERTM(kernel != NULL, "Fail to compile kernel, may need to increase reserved registers for spilling.");
     return kernel;
 #else
     return NULL;
diff --git a/backend/src/backend/gen_reg_allocation.cpp b/backend/src/backend/gen_reg_allocation.cpp
index a9338c5..13c856e 100644
--- a/backend/src/backend/gen_reg_allocation.cpp
+++ b/backend/src/backend/gen_reg_allocation.cpp
@@ -192,7 +192,7 @@ namespace gbe
     INLINE bool spillReg(GenRegInterval interval, bool isAllocated = false);
     INLINE bool spillReg(ir::Register reg, bool isAllocated = false);
     INLINE bool vectorCanSpill(SelectionVector *vector);
-    INLINE void allocateScratchForSpilled();
+    INLINE bool allocateScratchForSpilled();
     void allocateCurbePayload(void);
 
     /*! replace specified source/dst register with temporary register and update interval */
@@ -788,7 +788,10 @@ namespace gbe
           return false;
         }
       }
-      allocateScratchForSpilled();
+      if (!allocateScratchForSpilled()) {
+        ctx.errCode = REGISTER_SPILL_NO_SPACE;
+        return false;
+      }
       bool success = selection.spillRegs(spilledRegs, reservedReg);
       if (!success) {
         ctx.errCode = REGISTER_SPILL_FAIL;
@@ -799,7 +802,7 @@ namespace gbe
     return true;
   }
 
-  INLINE void GenRegAllocator::Opaque::allocateScratchForSpilled()
+  INLINE bool GenRegAllocator::Opaque::allocateScratchForSpilled()
   {
     const uint32_t regNum = spilledRegs.size();
     this->starting.resize(regNum);
@@ -833,7 +836,10 @@ namespace gbe
       ir::RegisterFamily family = ctx.sel->getRegisterFamily(cur->reg);
       it->second.addr = ctx.allocateScratchMem(getFamilySize(family)
                                              * ctx.getSimdWidth());
-      }
+      if (it->second.addr == -1)
+        return false;
+    }
+    return true;
   }
 
   INLINE bool GenRegAllocator::Opaque::expireReg(ir::Register reg)
@@ -1019,7 +1025,7 @@ namespace gbe
   INLINE uint32_t GenRegAllocator::Opaque::allocateReg(GenRegInterval interval,
                                                        uint32_t size,
                                                        uint32_t alignment) {
-    uint32_t grfOffset;
+    int32_t grfOffset;
     // Doing expireGRF too freqently will cause the post register allocation
     // scheduling very hard. As it will cause a very high register conflict rate.
     // The tradeoff here is to reduce the freqency here. And if we are under spilling
@@ -1032,7 +1038,7 @@ namespace gbe
     // and the source is a scalar Dword. If that is the case, the byte register
     // must get 4byte alignment register offset.
     alignment = (alignment + 3) & ~3;
-    while ((grfOffset = ctx.allocate(size, alignment)) == 0) {
+    while ((grfOffset = ctx.allocate(size, alignment)) == -1) {
       const bool success = this->expireGRF(interval);
       if (success == false) {
         if (spillAtInterval(interval, size, alignment) == false)
diff --git a/backend/src/backend/program.cpp b/backend/src/backend/program.cpp
index 472734b..15d6909 100644
--- a/backend/src/backend/program.cpp
+++ b/backend/src/backend/program.cpp
@@ -114,10 +114,12 @@ namespace gbe {
 #ifdef GBE_COMPILER_AVAILABLE
   BVAR(OCL_OUTPUT_GEN_IR, false);
   BVAR(OCL_STRICT_CONFORMANCE, true);
+  BVAR(OCL_OUTPUT_BUILD_LOG, false);
 
   bool Program::buildFromLLVMFile(const char *fileName, const void* module, std::string &error, int optLevel) {
     ir::Unit *unit = new ir::Unit();
     llvm::Module * cloned_module = NULL;
+    bool ret = true;
     if(module){
       cloned_module = llvm::CloneModule((llvm::Module*)module);
     }
@@ -141,12 +143,13 @@ namespace gbe {
       }
     }
     assert(unit->getValid());
-    this->buildFromUnit(*unit, error);
+    if (!this->buildFromUnit(*unit, error))
+      ret = false;
     delete unit;
     if(cloned_module){
       delete (llvm::Module*) cloned_module;
     }
-    return true;
+    return ret;
   }
 
   bool Program::buildFromUnit(const ir::Unit &unit, std::string &error) {
@@ -158,6 +161,13 @@ namespace gbe {
     for (const auto &pair : set) {
       const std::string &name = pair.first;
       Kernel *kernel = this->compileKernel(unit, name, !OCL_STRICT_CONFORMANCE);
+      if (!kernel) {
+        error +=  name;
+        error += ":(GBE): error: failed in Gen backend.\n";
+        if (OCL_OUTPUT_BUILD_LOG)
+          llvm::errs() << error;
+        return false;
+      }
       kernel->setSamplerSet(pair.second->getSamplerSet());
       kernel->setImageSet(pair.second->getImageSet());
       kernel->setPrintfSet(pair.second->getPrintfSet());
@@ -516,8 +526,6 @@ namespace gbe {
   }
 
 #ifdef GBE_COMPILER_AVAILABLE
-  BVAR(OCL_OUTPUT_BUILD_LOG, false);
-
   static bool buildModuleFromSource(const char *source, llvm::Module** out_module, llvm::LLVMContext* llvm_ctx,
                                     std::string dumpLLVMFileName, std::vector<std::string>& options, size_t stringSize, char *err,
                                     size_t *errSize) {
@@ -827,10 +835,10 @@ namespace gbe {
                               stringSize, err, errSize)) {
     // Now build the program from llvm
       size_t clangErrSize = 0;
-      if (err != NULL) {
+      if (err != NULL && *errSize != 0) {
         GBE_ASSERT(errSize != NULL);
-        stringSize -= *errSize;
-        err += *errSize;
+        stringSize = stringSize - *errSize;
+        err = err + *errSize;
         clangErrSize = *errSize;
       }
 
-- 
1.9.1



More information about the Beignet mailing list