[Beignet] [PATCH 1/2] enable scratch memory allocation and read/write

[Song, Ruiling]

[Ruling] Yes, I will refine the code, it is really bad naming. I will make it clear and  remove the OWord block read/write.
 And maybe you can get some hint that why you failed to use the OBlock read/write to implement the spill/unspill.
[Ruiling] No, the failure is not related with OBlock read/write. OBlock read/write also works. The failure have already been fixed by previous patch "Fix a bug in stack calculation". Thanks for your various comments.

From: zhigang gong [mailto:zhigang.gong at gmail.com]
Sent: Wednesday, August 07, 2013 2:14 PM
To: Song, Ruiling
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH 1/2] enable scratch memory allocation and read/write


On Wed, Aug 7, 2013 at 1:44 PM, Song, Ruiling <ruiling.song at intel.com<mailto:ruiling.song at intel.com>> wrote:


From: zhigang gong [mailto:zhigang.gong at gmail.com<mailto:zhigang.gong at gmail.com>]
Sent: Wednesday, August 07, 2013 1:04 PM
To: Song, Ruiling
Cc: beignet at lists.freedesktop.org<mailto:beignet at lists.freedesktop.org>
Subject: Re: [Beignet] [PATCH 1/2] enable scratch memory allocation and read/write

My suggestion here is to seprate the scratch OBlock read/write out. And submit the
Dword Read/Write part firstly, as OBlock read/write is not used right now, Right?
[Ruiling] In fact, I first use OBlock Read/Write for spill implementation. As I met some failure, so, I also implement scratch HBlock. I call it HBlock as it always read multiples of HWord. There are two modes of scratch RW, but I only enable DWORD
  I think you should use Dword to keep align with the spec.  It has reason that although it read data start from Hword alignment,
 but it only use the part of the data according to the data type. For example, if the data type is Dword then it only put the first 8 Dword
 of data to the destination registers.
mode. That is the HBlock in the patch. If you have concern about the oword block rw, I can remove OBlock RW logic.
+  void GenEncoder::SCRATCH_READ_HWORD(GenRegister dst, GenRegister src, uint32_t offset, uint32_t size, uint32_t dst_num)
+  {
+     assert(dst_num == 1 || dst_num ==2);
+     uint32_t block_size = dst_num == 1 ? GEN_SCRATCH_BLOCK_SIZE_1 : GEN_SCRATCH_BLOCK_SIZE_2;
+     GenInstruction *insn = this->next(GEN_OPCODE_SEND);
+     this->setHeader(insn);
+     this->setDst(insn, dst);
+     this->setSrc0(insn, src);
+     this->setSrc1(insn, GenRegister::immud(0));
+      // here dst_num is the register that will be write-back: in terms of 32byte register
+     setScratchMessage(this, insn, offset, block_size, GEN_SCRATCH_DATA_DWORD, GEN_SCRATCH_READ, 1, dst_num);
+  }
+
+  void GenEncoder::SCRATCH_WRITE_OWORD(GenRegister msg, uint32_t size, uint32_t src_num)
+  {
+     GenInstruction *insn = this->next(GEN_OPCODE_SEND);
+     this->setHeader(insn);
+     this->setDst(insn, GenRegister::retype(GenRegister::null(), GEN_TYPE_UD));
+     this->setSrc0(insn, msg);
+     this->setSrc1(insn, GenRegister::immud(0));
+     // here src_num means registers that will be write out: in terms of 32byte register number
+     setOBlockRW(this, insn, 255, size/16, GEN_OBLOCK_WRITE, src_num+1, 0);
+  }
 I'm confused here, you prepared a message header for  Scratch OWord  read/write, but then you send a OWord Block Read
 message. I don't think it's going to work as desired. You can check the  "OWord Block Read/Write" in the spec, you can find
 a restriction there:
   the only surface type allowed is SURFTYPE_BUFFER.
 And here, the surface is a scratch buffer.
[Ruiling] scratch buffer is stateless buffer, i.e. SURFTYPE_BUFFER. It is more preferred to call "OWord block read/write" instead of "scratch OWord block read/write". This is not what written as oword mode under scratch memory read/write.

 You are right, the scratch buffer is one type of SURFTYPE_BUFFER and can be accessed by OWord block read/write.
 What confused me is you are using SCRATCH_WRITE_OWORD as the function name. You know there are real
 Scratch Block Read/Write for OWord which is quite different from this OWord Bloc read. Right? They have different
 message descriptor and different payload/writeback message.

 Ben had implemented the OBlock read/write extension before. And for some reason he removed those implementations.
 If you really want to use OBlock read/write here, it may worth to find what Ben already implemented before and see what we
 can reuse here.  And maybe you can get some hint that why you failed to use the OBlock read/write to implement the spill/unspill.

 Anyway, use scratch_write/read_oword is quite misleading here, and should be corrected. What do you think?

 I'm thinking of is there a way to test the scratch read/write directly?  Maybe it's not so straightforward as it's only
 used when spill/unspill occurs.  And if a kernel triggers spill/unspill, it means the kernel is not very simple and can't
 be a proper unit test case for scratch read/write. Any good idea?
[Ruiling] this is also a problem I met. At last I only can manually call spillReg() to select one virtual register I want, so, I don't have to face a complex kernel. But I don't have good way of verifying scratch RW.
 - Zhigang
--
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/20130807/55b93ced/attachment-0001.html>