nice catch, that's an oversight. I just fixed it and pushed this patch. Thanks.
On Tue, Jan 28, 2014 at 02:53:04AM +0000, Yang, Rong R wrote: > One comment. > > -----Original Message----- > From: [email protected] > [mailto:[email protected]] On Behalf Of Zhigang Gong > Sent: Monday, January 27, 2014 5:22 PM > To: [email protected] > Cc: Gong, Zhigang > Subject: [Beignet] [PATCH] GBE: fixed the out-of-range JMPI. > > For the conditional jump distance out of S15 range [-32768, 32767], we need > to use an inverted jmp followed by a add ip, ip, distance to implement. A > little hacky as we need to change the nop instruction to add instruction > manually. > > There is an optimization method which we can insert a ADD instruction on > demand. But that will need some extra analysis for all the branching > instruction. And need to adjust the distance for those branch instruction's > start point and end point contains this instruction. > > After this patch, the luxrender's slg4 could render the scene "alloy" > correctly. > > Signed-off-by: Zhigang Gong <[email protected]> > --- > backend/src/backend/gen_context.cpp | 6 +++--- > backend/src/backend/gen_encoder.cpp | 30 +++++++++++++++++++++++++++--- > 2 files changed, 30 insertions(+), 6 deletions(-) > > diff --git a/backend/src/backend/gen_context.cpp > b/backend/src/backend/gen_context.cpp > index 893de34..d30369d 100644 > --- a/backend/src/backend/gen_context.cpp > +++ b/backend/src/backend/gen_context.cpp > @@ -880,8 +880,8 @@ namespace gbe > NOT_IMPLEMENTED; > p->curr.execWidth = 1; > p->curr.noMask = 1; > + jip0 = p->n_instruction(); > p->JMPI(GenRegister::immud(0)); > - jip0 = p->n_instruction() - 1; > p->pop(); > > p->curr.predicate = GEN_PREDICATE_NONE; @@ -905,8 +905,8 @@ namespace > gbe > NOT_IMPLEMENTED; > p->curr.execWidth = 1; > p->curr.noMask = 1; > + jip1 = p->n_instruction(); > p->JMPI(GenRegister::immud(0)); > - jip1 = p->n_instruction() - 1; > p->pop(); > > p->curr.predicate = GEN_PREDICATE_NONE; @@ -1457,7 +1457,7 @@ > namespace gbe > p->curr.noMask = 1; > int jip = -(int)(p->n_instruction() - loop_start + 1) * 2; > p->JMPI(zero); > - p->patchJMPI(p->n_instruction()-1, jip); > + p->patchJMPI(p->n_instruction()-2, jip); > p->pop(); > // end of loop > } > diff --git a/backend/src/backend/gen_encoder.cpp > b/backend/src/backend/gen_encoder.cpp > index c372e36..1b79077 100644 > --- a/backend/src/backend/gen_encoder.cpp > +++ b/backend/src/backend/gen_encoder.cpp > @@ -1050,13 +1050,37 @@ namespace gbe > > void GenEncoder::JMPI(GenRegister src) { > alu2(this, GEN_OPCODE_JMPI, GenRegister::ip(), GenRegister::ip(), src); > + NOP(); > } > > void GenEncoder::patchJMPI(uint32_t insnID, int32_t jumpDistance) { > GenInstruction &insn = this->store[insnID]; > - assert(insnID < this->store.size()); > - assert(insn.header.opcode == GEN_OPCODE_JMPI); > - this->setSrc1(&insn, GenRegister::immd(jumpDistance)); > + GBE_ASSERT(insnID < this->store.size()); > + GBE_ASSERT(insn.header.opcode == GEN_OPCODE_JMPI); > + if ( (jumpDistance > -32767 && jumpDistance < 32768) > + || (insn.header.predicate_control == GEN_PREDICATE_NONE)) { > > >>>>>> If predicate_control is GEN_PREDICATE_NONE and jumpDistance is out of > >>>>>> range, also can't use jmpi. > > + this->setSrc1(&insn, GenRegister::immd(jumpDistance)); > + } else { > + // For the conditional jump distance out of S15 range, we need to use > an > + // inverted jmp followed by a add ip, ip, distance to implement. > + // A little hacky as we need to change the nop instruction to add > + // instruction manually. > + // FIXME there is an optimization method which we can insert a > + // ADD instruction on demand. But that will need some extra analysis > + // for all the branching instruction. And need to adjust the distance > + // for those branch instruction's start point and end point contains > + // this instruction. > + insn.header.predicate_inverse ^= 1; > + this->setSrc1(&insn, GenRegister::immd(2)); > + GenInstruction &insn2 = this->store[insnID+1]; > + GBE_ASSERT(insn2.header.opcode == GEN_OPCODE_NOP); > + GBE_ASSERT(insnID < this->store.size()); > + insn2.header.predicate_control = GEN_PREDICATE_NONE; > + insn2.header.opcode = GEN_OPCODE_ADD; > + this->setDst(&insn2, GenRegister::ip()); > + this->setSrc0(&insn2, GenRegister::ip()); > + this->setSrc1(&insn2, GenRegister::immd(jumpDistance * 8)); > + } > } > > void GenEncoder::CMP(uint32_t conditional, GenRegister src0, GenRegister > src1) { > -- > 1.7.9.5 > > _______________________________________________ > Beignet mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/beignet > _______________________________________________ > Beignet mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
