[Beignet] [PATCH 2/2] Skip spill/unspill instruction when trying to do spill.

[Song, Ruiling]

Good question, let me explain a little more.
For a spilled virtual register, it will not have a dedicated physical register for it anymore. And its real data lies in scratch memory.
When a virtual register is used, it will be always reloaded from scratch memory. when it is updated, it will be written to scratch memory immediately.
The content of the reserved register is only valid for one instruction(not including spill/unspill instruction). In the next instruction, it will be used to store other data.

From: zhigang gong [mailto:zhigang.gong at gmail.com]
Sent: Friday, August 09, 2013 5:02 PM
To: Song, Ruiling
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH 2/2] Skip spill/unspill instruction when trying to do spill.

On Fri, Aug 9, 2013 at 4:38 PM, Song, Ruiling <ruiling.song at intel.com<mailto:ruiling.song at intel.com>> wrote:
When I do a spill, I directly construct a physical register, see below:
spill->src(0) =GenRegister(GEN_GENERAL_REGISTER_FILE, dstStart + dstID, 0,
                                              selReg.type, selReg.vstride, selReg.width, selReg.hstride);
and also allocate one physical register from reserved register pool, change the spilled virtual register to physical register:
dst.physical =1; dst.nr<http://dst.nr> = dstStart+dstID; dst.subnr = 0;
so, in later emit code, when they call ra->genReg(), I will directly return the physical register; see GenRegAllocator::Opaque::genReg()
  Sounds good. But if my understanding is correct, then after spill or unspill, the spilledReg will become a physical
  register:
            // change nr/subnr, keep other register settings
           src.nr<http://src.nr> = srcStart+srcID; src.subnr=0; src.physical=1;

            // change nr/subnr, keep other register settings
            dst.physical =1; dst.nr<http://dst.nr> = dstStart+dstID; dst.subnr = 0;
   And the most important side effect is that the reserved registers will be used.
   Then latter, if we want to spill another register, then it may use the reserved registers again but those registers are already used by the
   previous spilled register.

   Is it a problem?


From: beignet-bounces+ruiling.song=intel.com at lists.freedesktop.org<mailto:intel.com at lists.freedesktop.org> [mailto:beignet-bounces+ruiling.song<mailto:beignet-bounces%2Bruiling.song>=intel.com at lists.freedesktop.org<mailto:intel.com at lists.freedesktop.org>] On Behalf Of zhigang gong
Sent: Friday, August 09, 2013 4:29 PM
To: Song, Ruiling
Cc: beignet at lists.freedesktop.org<mailto:beignet at lists.freedesktop.org>
Subject: Re: [Beignet] [PATCH 2/2] Skip spill/unspill instruction when trying to do spill.

Could you tell something more about how could a spilledReg be a physical register?
This is not very clear for me. If a virtual register is to be spilled, then it will
always be a virtual register, and will be unspilled when it is used. When it may
become a physical register?


On Fri, Aug 9, 2013 at 1:23 PM, Ruiling Song <ruiling.song at intel.com<mailto:ruiling.song at intel.com>> wrote:
We can only spill virtual registers, should skip physical register.
This fix random failure of compiler_box_blur when do spilling.

Signed-off-by: Ruiling Song <ruiling.song at intel.com<mailto:ruiling.song at intel.com>>
---
 backend/src/backend/gen_insn_selection.cpp |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
index 3610051..5cafa98 100644
--- a/backend/src/backend/gen_insn_selection.cpp
+++ b/backend/src/backend/gen_insn_selection.cpp
@@ -628,12 +628,15 @@ namespace gbe

     for (auto &block : blockList)
       for (auto &insn : block.insnList) {
+        // spill / unspill insn should be skipped when do spilling
+        if(insn.opcode == SEL_OP_SPILL_REG || insn.opcode == SEL_OP_UNSPILL_REG) continue;
+
         const uint32_t srcNum = insn.srcNum, dstNum = insn.dstNum;

         for (uint32_t srcID = 0; srcID < srcNum; ++srcID) {
           const GenRegister selReg = insn.src(srcID);
           const ir::Register reg = selReg.reg();
-          if(selReg.file == GEN_GENERAL_REGISTER_FILE && reg == spilledReg) {
+          if(reg == spilledReg && selReg.file == GEN_GENERAL_REGISTER_FILE && selReg.physical == 0) {
             GBE_ASSERT(srcID < 5);
             SelectionInstruction *unspill = this->create(SEL_OP_UNSPILL_REG, 1, 0);
             unspill->state  = GenInstructionState(simdWidth);
@@ -652,7 +655,7 @@ namespace gbe
         for (uint32_t dstID = 0; dstID < dstNum; ++dstID) {
           const GenRegister selReg = insn.dst(dstID);
           const ir::Register reg = selReg.reg();
-          if(selReg.file == GEN_GENERAL_REGISTER_FILE && reg == spilledReg) {
+          if(reg == spilledReg && selReg.file == GEN_GENERAL_REGISTER_FILE && selReg.physical == 0) {
             GBE_ASSERT(dstID < 5);
             SelectionInstruction *spill = this->create(SEL_OP_SPILL_REG, 0, 1);
             spill->state  = GenInstructionState(simdWidth);
--
1.7.9.5

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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/beignet/attachments/20130812/b42443e4/attachment.html>