[Beignet] [PATCH 1/3] Fix a vector argument deallocate assert.

[Song, Ruiling]

The whole patchset LGTM.
BTW, it is better if you add a unit test for patch 1.

-----Original Message-----
From: beignet-bounces+ruiling.song=intel.com at lists.freedesktop.org [mailto:beignet-bounces+ruiling.song=intel.com at lists.freedesktop.org] On Behalf Of Yang Rong
Sent: Friday, October 11, 2013 1:50 PM
To: beignet at lists.freedesktop.org
Cc: Yang, Rong R
Subject: [Beignet] [PATCH 1/3] Fix a vector argument deallocate assert.

Vector argument will allocate together but deallocate sepelate, when deallocate will assert. Split the each allocatedBlock in register partitioner to fix it.

Signed-off-by: Yang Rong <rong.r.yang at intel.com>
---
 backend/src/backend/context.cpp            | 28 ++++++++++++++++++++++++++++
 backend/src/backend/context.hpp            |  2 ++
 backend/src/backend/gen_reg_allocation.cpp |  9 +++++----
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/backend/src/backend/context.cpp b/backend/src/backend/context.cpp index cbd38f1..13fa5fa 100644
--- a/backend/src/backend/context.cpp
+++ b/backend/src/backend/context.cpp
@@ -60,6 +60,9 @@ namespace gbe
     /*! Free the given register file piece */
     void deallocate(int16_t offset);
 
+    /*! Spilt a block into 2 blocks */
+    void splitBlock(int16_t offset, int16_t subOffset);
+
   private:
     /*! May need to make that run-time in the future */
     static const int16_t RegisterFileSize = 4*KB; @@ -268,6 +271,27 @@ namespace gbe
     }
   }
 
+  void RegisterFilePartitioner::splitBlock(int16_t offset, int16_t subOffset) {
+    // Retrieve the size in the allocation map
+    auto it = allocatedBlocks.find(offset);
+    GBE_ASSERT(it != allocatedBlocks.end());
+
+    while(subOffset > it->second) {
+      subOffset -= it->second;
+      offset += it->second;
+      it = allocatedBlocks.find(offset);
+      GBE_ASSERT(it != allocatedBlocks.end());
+    }
+
+    if(subOffset == 0)
+      return;
+    int16_t size = it->second;
+    allocatedBlocks.erase(it);
+    // Track the allocation to retrieve the size later
+    allocatedBlocks.insert(std::make_pair(offset, subOffset));
+    allocatedBlocks.insert(std::make_pair(offset + subOffset, size - 
+ subOffset));  }
+
   static int
   alignScratchSize(int size){
     int i = 0;
@@ -328,6 +352,10 @@ namespace gbe
 
   void Context::deallocate(int16_t offset) { partitioner->deallocate(offset); }
 
+  void Context::splitBlock(int16_t offset, int16_t subOffset) {
+    partitioner->splitBlock(offset, subOffset);  }
+
   int32_t Context::allocConstBuf(uint32_t argID) {
      GBE_ASSERT(kernel->args[argID].type == GBE_ARG_CONSTANT_PTR);
 
diff --git a/backend/src/backend/context.hpp b/backend/src/backend/context.hpp index ca2c88d..000612e 100644
--- a/backend/src/backend/context.hpp
+++ b/backend/src/backend/context.hpp
@@ -86,6 +86,8 @@ namespace gbe
     int16_t allocate(int16_t size, int16_t alignment);
     /*! Deallocate previously allocated memory */
     void deallocate(int16_t offset);
+    /*! Spilt a block into 2 blocks, for some registers allocate together but  deallocate seperate */
+    void splitBlock(int16_t offset, int16_t subOffset);
     /* allocate curbe for constant ptr argument */
     int32_t allocConstBuf(uint32_t argID);
     /* allocate a new entry for a specific image's information */ diff --git a/backend/src/backend/gen_reg_allocation.cpp b/backend/src/backend/gen_reg_allocation.cpp
index ab8b7ee..c4cad40 100644
--- a/backend/src/backend/gen_reg_allocation.cpp
+++ b/backend/src/backend/gen_reg_allocation.cpp
@@ -149,15 +149,16 @@ namespace gbe
     const Function &fn = ctx.getFunction();
     GBE_ASSERT(fn.getProfile() == PROFILE_OCL);
     const Function::PushMap &pushMap = fn.getPushMap();
-    for (const auto &pushed : pushMap) {
-      const uint32_t argID = pushed.second.argID;
+    for (auto rit = pushMap.rbegin(); rit != pushMap.rend(); ++rit) {
+      const uint32_t argID = rit->second.argID;
       const FunctionArgument arg = fn.getArg(argID);
 
-      const uint32_t subOffset = pushed.second.offset;
-      const Register reg = pushed.second.getRegister();
+      const uint32_t subOffset = rit->second.offset;
+      const Register reg = rit->second.getRegister();
       auto it = this->ctx.curbeRegs.find(arg.reg);
       assert(it != ctx.curbeRegs.end());
       allocatePayloadReg(reg, it->second, subOffset);
+      ctx.splitBlock(it->second, subOffset);
     }
   }
 
--
1.8.1.2

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