[Beignet] [PATCH V2] GBE: optimize IMM handling for SEL/SEL_CMP/CMP.

[Song, Ruiling]

Other part seems ok for me.

-----Original Message-----
From: Gong, Zhigang 
Sent: Friday, May 23, 2014 11:29 AM
To: Song, Ruiling; Zhigang Gong
Cc: beignet at lists.freedesktop.org
Subject: RE: [Beignet] [PATCH V2] GBE: optimize IMM handling for SEL/SEL_CMP/CMP.

I just thought it could switch two operands easily.
You are right, OP_ORD should not be added into the imm handling function.
As it use both operands as src0 and src1.
I will fix that, any other comment on this patch?

> -----Original Message-----
> From: Song, Ruiling
> Sent: Friday, May 23, 2014 11:25 AM
> To: Zhigang Gong; Gong, Zhigang
> Cc: beignet at lists.freedesktop.org
> Subject: RE: [Beignet] [PATCH V2] GBE: optimize IMM handling for 
> SEL/SEL_CMP/CMP.
> 
> > +    }
> > +    // If it's a compare instruction, theoritically, we can easily 
> > + revert the
> condition code to
> > +    // switch the two operands. But we can't do that for float due 
> > + to the
> NaN's exist.
> > +    // For a normal select instruction, we can always inverse the
> predication to switch the two
> > +    // operands' position.
> > +    else if (OCL_OPTIMIZE_IMMEDIATE && dag0 != NULL &&
> > +             dag0->insn.getOpcode() == OP_LOADI &&
> canGetRegisterFromImmediate(dag0->insn) &&
> > +             ((dag.insn.isMemberOf<CompareInstruction>() && type !=
> TYPE_FLOAT && type != TYPE_DOUBLE) ||
> > +              (dag.insn.isMemberOf<SelectInstruction>()) ||
> > +              (dag.insn.getOpcode() == OP_ORD))) {
> Why do we need to optimize for OP_ORD here?
> 
>     } else if(opcode == OP_ORD) {
>           sel.push();
>             sel.CMP(GEN_CONDITIONAL_EQ, src0, src0, tmpDst);
>             sel.curr.predicate = GEN_PREDICATE_NORMAL;
>             sel.curr.flagGen = 1;
>             sel.CMP(GEN_CONDITIONAL_EQ, src1, src1, tmpDst);
>           sel.pop();
> 
> seems that OP_ORD cannot accept immediate operand.
> 
> > +      const auto &childInsn = cast<LoadImmInstruction>(dag0->insn);
> > +      src0 = this->selReg(dag.insn.getSrc(src1Index), type);
> > +      src1 = getRegisterFromImmediate(childInsn.getImmediate(), type);
> > +      inverse = true;
> > +      if (dag1) dag1->isRoot = 1;