[Beignet] [PATCH 2/2] GBE: optimize CMP instruction encoding.

[Gong, Zhigang]

> -----Original Message-----
> From: Song, Ruiling
> Sent: Friday, May 23, 2014 3:55 PM
> To: Zhigang Gong
> Cc: Gong, Zhigang; beignet at lists.freedesktop.org
> Subject: RE: [Beignet] [PATCH 2/2] GBE: optimize CMP instruction encoding.
> 
> 
> > Seems that you set default type as ir::TYPE_FLOAT for the argument
> > ir::Type type in replaceReg() And you didn't set the type when calling the
> function, except for the CMP case( you set it to ir::TYPE_FLOAT), then if a
> register of type like QWORD. Things will become wrong.
> As to the QWORD, we have never tried to replace a QWORD register so far.
> The registers to be replaced is an element in a vector and all vector are for
> DWORD family.
> > My suggestion is that we don't need to pass an argument ir::Type.
> > We just use the type of the replaced register. We need make sure the register
> type should be right before calling replaceSrc or replaceDst.
> The reason why I prefer to pass in the type is that some times the register to
> be replaced is null register, and we can only get the family from that gen
> register and can't get the original ir::type information which is needed to
> create a new temprary register here.
> 
> And I think as the caller always know the eaxct ir type of the register to be
> replaced, why not just pass in that type and no need to do a family to ir type
> mapping.
> 
> Family to ir type mapping is not a must, we can get the register family, new a
> virtual register, and then retype it to the GEN_TYPE_XX. The type information is
> already in the replaced GenRegister.
I just recalled another reason why I change the API to pass in the type. Actually, the first
draft version was implemented as you suggested.

The null register may just be a GEN_TYPE_F, as the insn selection stage has no idea whether
Or how it will be replaced in the following stage. One possibility is that the caller (reg allocation stage)
may want to replace it to a GEN_TYPE_D if two operands are all GEN_TYPE_D and the caller
want to get the temporary result into a GEN_TYPE_D and use it latter. I tried to use it that way
for some experiment.