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: [email protected] 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: [email protected] > 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; _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
