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

[zhigang gong]

On Fri, Aug 9, 2013 at 4:38 PM, Song, Ruiling <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 = 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 = srcStart+srcID; src.subnr=0; src.physical=1;

            // change nr/subnr, keep other register settings
            dst.physical =1; 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:
> beignet-bounces+ruiling.song=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
> *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>
> 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>
> ---
>  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
> http://lists.freedesktop.org/mailman/listinfo/beignet****
>
> ** **
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/beignet/attachments/20130809/7379842d/attachment.html>