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

[Zhigang Gong]

On Fri, May 23, 2014 at 07:19:29AM +0000, Song, Ruiling wrote:
> 
> -  ir::Register Selection::Opaque::replaceDst(SelectionInstruction *insn, uint32_t regID) {
> +  ir::Register Selection::Opaque::replaceDst(SelectionInstruction 
> + *insn, uint32_t regID, ir::Type type, bool needMov) {
> +    tmp = this->reg(ir::getFamily(type));
> +    gr = this->selReg(tmp, type);
> 
> 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.

> 
>          ir::Register tmp;
> -        if (vector->isSrc)
> -          tmp = selection.replaceSrc(vector->insn, regID);
> -        else
> -          tmp = selection.replaceDst(vector->insn, regID);
> +        tmp = this->replaceReg(selection, vector->insn, regID, 
> + vector->isSrc);
> The ir::Type is not set here.
> 
>          const VectorLocation location = std::make_pair(vector, regID);
>          this->vectorMap.insert(std::make_pair(tmp, location));
> -        intervals.push_back(tmp);
> -        intervals[tmp].minID = vector->insn->ID;
> -        intervals[tmp].maxID = vector->insn->ID;
>        }
>      }
>    }
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet