[Beignet] [PATCH 1/2] Fix vector allocation and 64-bit reading

[Xing, Homer]

Thanks.

-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at gmail.com] 
Sent: Monday, August 5, 2013 4:55 PM
To: Song, Ruiling
Cc: Xing, Homer; beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH 1/2] Fix vector allocation and 64-bit reading

As I mentioned before, I ignore the first patch of this patchset.
And pushed the second patch which is a new unit test case.

Thanks for the patch and review comments.

On Mon, Aug 05, 2013 at 03:01:28AM +0000, Song, Ruiling wrote:
> 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 Homer Hsing
> Sent: Wednesday, July 31, 2013 9:37 AM
> To: beignet at lists.freedesktop.org
> Subject: [Beignet] [PATCH 1/2] Fix vector allocation and 64-bit reading
> 
> Vector allocation gave each register DWORD * SIMD_WIDTH.
> When the register is QWORD, allocation is wrong.
> This patch fixes vector allocation.
> 
> This patch also fixes 64-bit reading.
> When reading, temporary registers is next to destination register.
> 
> Signed-off-by: Homer Hsing <homer.xing at intel.com>
> ---
>  backend/src/backend/gen_encoder.cpp        |  4 ++--
>  backend/src/backend/gen_reg_allocation.cpp | 18 ++++++++++++++++--
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/backend/src/backend/gen_encoder.cpp b/backend/src/backend/gen_encoder.cpp
> index f84c6dd..0a2b673 100644
> --- a/backend/src/backend/gen_encoder.cpp
> +++ b/backend/src/backend/gen_encoder.cpp
> @@ -367,10 +367,10 @@ namespace gbe
>  
>    void GenEncoder::READ_FLOAT64(GenRegister dst, GenRegister src, uint32_t bti, uint32_t elemNum) {
>      int w = curr.execWidth;
> -    dst = GenRegister::h2(dst);
>      dst.type = GEN_TYPE_UD;
>      src.type = GEN_TYPE_UD;
> -    GenRegister r = GenRegister::retype(GenRegister::suboffset(src, w*2), GEN_TYPE_UD);
> +    GenRegister r = GenRegister::retype(GenRegister::suboffset(dst, w*2), GEN_TYPE_UD);
> +    dst = GenRegister::h2(dst);
>      GenRegister imm4 = GenRegister::immud(4);
>      GenInstruction *insn;
>      insn = next(GEN_OPCODE_SEND);
> diff --git a/backend/src/backend/gen_reg_allocation.cpp b/backend/src/backend/gen_reg_allocation.cpp
> index e7c96ac..56d9261 100644
> --- a/backend/src/backend/gen_reg_allocation.cpp
> +++ b/backend/src/backend/gen_reg_allocation.cpp
> @@ -475,16 +475,30 @@ namespace gbe
>          const SelectionVector *vector = it->second.first;
>          const uint32_t simdWidth = ctx.getSimdWidth();
>          const uint32_t alignment = simdWidth * sizeof(uint32_t);
> -        const uint32_t size = vector->regNum * alignment;
> +        int num = 0;
> +        for (uint32_t regID = 0; regID < vector->regNum; ++regID) {
> +          const ir::Register reg = vector->reg[regID].reg();
> +          const ir::RegisterFamily f = selection.getRegisterFamily(reg);
> +          if (f == ir::FAMILY_QWORD)
> +            num += 2;
> +          else
> +            num ++;
> +        }
> +        const uint32_t size = num * alignment;
>          uint32_t grfOffset;
>          while ((grfOffset = ctx.allocate(size, alignment)) == 0) {
>            const bool success = this->expireGRF(interval);
>            if (success == false) return false;
>          }
> -        for (uint32_t regID = 0; regID < vector->regNum; ++regID, grfOffset += alignment) {
> +        for (uint32_t regID = 0; regID < vector->regNum; ++regID) {
>            const ir::Register reg = vector->reg[regID].reg();
>            GBE_ASSERT(RA.contains(reg) == false);
>            RA.insert(std::make_pair(reg, grfOffset));
> +          const ir::RegisterFamily f = selection.getRegisterFamily(reg);
> +          if (f == ir::FAMILY_QWORD)
> +            grfOffset += 2 * alignment;
> +          else
> +            grfOffset += alignment;
>          }
>        }
>        // Case 2: This is a regular scalar register, allocate it alone
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet