[Beignet] [PATCH 2/2] GBE: Fixed a bug and release 2 or 3 simdWidth register space.

[Song, Ruiling]

Very good finding, the patch LGTM.

-----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 Zhigang Gong
Sent: Friday, August 09, 2013 10:36 AM
To: beignet at lists.freedesktop.org
Cc: Zhigang Gong
Subject: [Beignet] [PATCH 2/2] GBE: Fixed a bug and release 2 or 3 simdWidth register space.

This patch fix two issues. One is for the sel_cmp pattern matching.
We should not set the sel_cmp instruction state to physicalFlag, as sel_cmp will never use a flag register. And as it set the physicalFlag and leave the flagIndex to zero. Then it just increase the virtual register 0's interval to as long as the last sel_cmp instruction, thus the virtual register 0 will never be freed.

Another issue is that when we allocate special registers. We are not always allocate them on demand. For example, the 3 local id registers are always allocated. Thus maybe some of the registers are not used at all. So the interval's end point will not get a chance to set to a proper value and it will never be released. Now just init the end point to 0. And latter, if it's used, it will be set to a proper value. Otherwise, it will be zero, and will be deallocated when do expiering.

This patch could fix(work around) a long standing bug:
When disable the pre allocation instruction scheduling by export OCL_PRE_ALLOC_INSN_SCHEDULE=0 And run the case:
utests/utest_run compiler_menger_sponge_no_shadow it fails.

I spent almost one day to track down that it's related to the register allocation. But I haven't root caused that where is the actual buggy code. I doubt the register allocation, but I reviewed the code very careful, and haven't found anything wrong. Now the last suspect is in the register interval handling.

Anyway, by apply these patch to release two registers to the pool which may change the register allocation/expieration, thus work around that bug. We may still need to spend some time to investigate the root cause the failure in the future.

Signed-off-by: Zhigang Gong <zhigang.gong at linux.intel.com>
---
 backend/src/backend/gen_insn_selection.cpp |    1 -
 backend/src/backend/gen_reg_allocation.cpp |    1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
index a361ab3..d40fbfe 100644
--- a/backend/src/backend/gen_insn_selection.cpp
+++ b/backend/src/backend/gen_insn_selection.cpp
@@ -1670,7 +1670,6 @@ namespace gbe
       sel.push();
         sel.curr.predicate = GEN_PREDICATE_NONE;
         sel.curr.execWidth = simdWidth;
-        sel.curr.physicalFlag = 0;
         sel.SEL_CMP(genCmp, tmp, src0, src1);
       sel.pop();
 
diff --git a/backend/src/backend/gen_reg_allocation.cpp b/backend/src/backend/gen_reg_allocation.cpp
index ccbc0da..5b243c4 100644
--- a/backend/src/backend/gen_reg_allocation.cpp
+++ b/backend/src/backend/gen_reg_allocation.cpp
@@ -136,6 +136,7 @@ namespace gbe
       const uint32_t offset = GEN_REG_SIZE + curbeOffset + subOffset;
       RA.insert(std::make_pair(reg, offset));
       this->intervals[reg].minID = 0;
+      this->intervals[reg].maxID = 0;
     }
   }
 
--
1.7.9.5

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