[Beignet] [PATCH 5/8] Backend: Add the MADM function to gen8 encoder.

[He Junyan]

On Tue, Sep 15, 2015 at 05:57:13AM -0700, Matt Turner wrote:
> Date: Tue, 15 Sep 2015 05:57:13 -0700
> From: Matt Turner <mattst88 at gmail.com>
> To: "junyan.he" <junyan.he at inbox.com>
> Cc: "beignet at lists.freedesktop.org" <beignet at lists.freedesktop.org>
> Subject: Re: [Beignet] [PATCH 5/8] Backend: Add the MADM function to gen8
>  encoder.
> 
> On Tue, Sep 15, 2015 at 4:15 AM,  <junyan.he at inbox.com> wrote:
> > From: Junyan He <junyan.he at linux.intel.com>
> >
> > Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> > ---
> >  backend/src/backend/gen8_encoder.cpp | 56 ++++++++++++++++++++++++++++++++++++
> >  backend/src/backend/gen8_encoder.hpp |  2 ++
> >  backend/src/backend/gen_defs.hpp     |  2 ++
> >  3 files changed, 60 insertions(+)
> >
> > diff --git a/backend/src/backend/gen8_encoder.cpp b/backend/src/backend/gen8_encoder.cpp
> > index 0af27a3..002a8b5 100644
> > --- a/backend/src/backend/gen8_encoder.cpp
> > +++ b/backend/src/backend/gen8_encoder.cpp
> > @@ -591,4 +591,60 @@ namespace gbe
> >       this->setSrc0WithAcc(insn, src0, src0Acc);
> >       this->setSrc1WithAcc(insn, src1, src1Acc);
> >    }
> > +
> > +  void Gen8Encoder::MADM(GenRegister dst, GenRegister src0, GenRegister src1, GenRegister src2,
> > +      uint32_t dstAcc, uint32_t src0Acc, uint32_t src1Acc, uint32_t src2Acc)
> > +  {
> > +    GenNativeInstruction *insn = this->next(GEN_OPCODE_MADM);
> > +    Gen8NativeInstruction *gen8_insn = &insn->gen8_insn;
> > +    assert(dst.file == GEN_GENERAL_REGISTER_FILE);
> > +    assert(src0.file == GEN_GENERAL_REGISTER_FILE);
> > +    assert(src1.file == GEN_GENERAL_REGISTER_FILE);
> > +    assert(src2.file == GEN_GENERAL_REGISTER_FILE);
> > +    assert(dst.hstride == GEN_HORIZONTAL_STRIDE_1 || dst.hstride == GEN_HORIZONTAL_STRIDE_0);
> > +    assert(src0.type == GEN_TYPE_DF || src0.type == GEN_TYPE_F);
> > +    assert(src0.type == dst.type);
> > +    assert(src0.type == src1.type);
> > +    assert(src0.type == src2.type);
> > +    int32_t dataType = src0.type == GEN_TYPE_DF ? 3 : 0;
> > +
> > +    this->setHeader(insn);
> > +    gen8_insn->bits1.da3srcacc.dest_reg_nr = dst.nr;
> > +    gen8_insn->bits1.da3srcacc.dest_subreg_nr = dst.subnr / 16;
> > +    gen8_insn->bits1.da3srcacc.dst_specal_acc = dstAcc;
> > +    gen8_insn->bits1.da3srcacc.src_type = dataType;
> > +    gen8_insn->bits1.da3srcacc.dest_type = dataType;
> > +    gen8_insn->header.access_mode = GEN_ALIGN_16;
> > +
> > +    assert(src0.file == GEN_GENERAL_REGISTER_FILE);
> > +    assert(src0.address_mode == GEN_ADDRESS_DIRECT);
> > +    assert(src0.nr < 128);
> > +    gen8_insn->bits2.da3srcacc.src0_specal_acc = src0Acc;
> > +    gen8_insn->bits2.da3srcacc.src0_subreg_nr = src0.subnr / 4 ;
> > +    gen8_insn->bits2.da3srcacc.src0_reg_nr = src0.nr;
> > +    gen8_insn->bits1.da3srcacc.src0_abs = src0.absolute;
> > +    gen8_insn->bits1.da3srcacc.src0_negate = src0.negation;
> > +    gen8_insn->bits2.da3srcacc.src0_rep_ctrl = src0.vstride == GEN_VERTICAL_STRIDE_0;
> > +
> > +    assert(src1.file == GEN_GENERAL_REGISTER_FILE);
> > +    assert(src1.address_mode == GEN_ADDRESS_DIRECT);
> > +    assert(src1.nr < 128);
> > +    gen8_insn->bits2.da3srcacc.src1_specal_acc = src1Acc;
> > +    gen8_insn->bits2.da3srcacc.src1_subreg_nr_low = (src1.subnr / 4) & 0x3;
> > +    gen8_insn->bits3.da3srcacc.src1_subreg_nr_high = (src1.subnr / 4) >> 2;
> > +    gen8_insn->bits2.da3srcacc.src1_rep_ctrl = src1.vstride == GEN_VERTICAL_STRIDE_0;
> > +    gen8_insn->bits3.da3srcacc.src1_reg_nr = src1.nr;
> > +    gen8_insn->bits1.da3srcacc.src1_abs = src1.absolute;
> > +    gen8_insn->bits1.da3srcacc.src1_negate = src1.negation;
> > +
> > +    assert(src2.file == GEN_GENERAL_REGISTER_FILE);
> > +    assert(src2.address_mode == GEN_ADDRESS_DIRECT);
> > +    assert(src2.nr < 128);
> > +    gen8_insn->bits3.da3srcacc.src2_specal_acc = src2Acc;
> > +    gen8_insn->bits3.da3srcacc.src2_subreg_nr = src2.subnr / 4;
> > +    gen8_insn->bits3.da3srcacc.src2_rep_ctrl = src2.vstride == GEN_VERTICAL_STRIDE_0;
> > +    gen8_insn->bits3.da3srcacc.src2_reg_nr = src2.nr;
> > +    gen8_insn->bits1.da3srcacc.src2_abs = src2.absolute;
> > +    gen8_insn->bits1.da3srcacc.src2_negate = src2.negation;
> > +  }
> >  } /* End of the name space. */
> > diff --git a/backend/src/backend/gen8_encoder.hpp b/backend/src/backend/gen8_encoder.hpp
> > index 53ec3d1..8e7939b 100644
> > --- a/backend/src/backend/gen8_encoder.hpp
> > +++ b/backend/src/backend/gen8_encoder.hpp
> > @@ -74,6 +74,8 @@ namespace gbe
> >
> >      void MATH_WITH_ACC(GenRegister dst, uint32_t function, GenRegister src0, GenRegister src1,
> >                         uint32_t dstAcc, uint32_t src0Acc, uint32_t src1Acc);
> > +    void MADM(GenRegister dst, GenRegister src0, GenRegister src1, GenRegister src2,
> > +              uint32_t dstAcc, uint32_t src0Acc, uint32_t src1Acc, uint32_t src2Acc);
> >    };
> >  }
> >  #endif /* __GBE_GEN8_ENCODER_HPP__ */
> > diff --git a/backend/src/backend/gen_defs.hpp b/backend/src/backend/gen_defs.hpp
> > index a1bd8dd..1b550ac 100644
> > --- a/backend/src/backend/gen_defs.hpp
> > +++ b/backend/src/backend/gen_defs.hpp
> > @@ -174,6 +174,8 @@ enum opcode {
> >    GEN_OPCODE_LINE = 89,
> >    GEN_OPCODE_PLN = 90,
> >    GEN_OPCODE_MAD = 91,
> > +  GEN_OPCODE_LRP = 92,
> 
> Unrelated to the main purpose of the patch: Do I understand correctly
> that Beignet does not emit the LRP instruction?
> 
> If not, I'm curious why not? It maps pretty well to the mix() function
> (just reverse the argument order), but it does not handle infinities
> properly [0], which may be a more severe limitation with OpenCL.
> 
> At least with OpenGL shaders, mix() is a pretty common operation so
> LRP is very useful.
I just add LRP here to make the enum consecutive,  LRP really has no
relationship with this patch set.
I will investigate you advice and thanks for your information.

> 
> [0] https://bugs.freedesktop.org/show_bug.cgi?id=89647