[Beignet] [PATCH] GBE: Fixed a bug in byte gather/scatter.

[Zhigang Gong]

Sorry, I made a mistake, this bug is reported by Lv Meng, not yang rong.
So lv meng, please test it and give feedback. Thanks

> -----Original Message-----
> From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com]
> Sent: Thursday, May 23, 2013 11:11 AM
> To: Yang, Rong R (rong.r.yang at intel.com)
> Cc: 'beignet at lists.freedesktop.org'
> Subject: RE: [Beignet] [PATCH] GBE: Fixed a bug in byte gather/scatter.
> 
> Please try this patch with your original kernel which is use a short
element in a
> structure.
> 
> > -----Original Message-----
> > From:
> > beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.org
> > [mailto:beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop
> > .org]
> > On Behalf Of Zhigang Gong
> > Sent: Thursday, May 23, 2013 11:11 AM
> > To: beignet at lists.freedesktop.org
> > Cc: Zhigang Gong
> > Subject: [Beignet] [PATCH] GBE: Fixed a bug in byte gather/scatter.
> >
> > We can't just use the alignment to determine whether use the
> > gather/scatter or not. As for a short type, the compiler may also
> > generate a 4 alignment, thus it will trigger this bug.
> >
> > Signed-off-by: Zhigang Gong <zhigang.gong at linux.intel.com>
> > ---
> >  backend/src/backend/gen_insn_selection.cpp | 18 ++++++++++--------
> >  1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/backend/src/backend/gen_insn_selection.cpp
> > b/backend/src/backend/gen_insn_selection.cpp
> > index 08bc6af..561e32f 100644
> > --- a/backend/src/backend/gen_insn_selection.cpp
> > +++ b/backend/src/backend/gen_insn_selection.cpp
> > @@ -1665,14 +1665,13 @@ namespace gbe
> >
> >      void emitByteGather(Selection::Opaque &sel,
> >                          const ir::LoadInstruction &insn,
> > +                        const uint32_t elemSize,
> >                          GenRegister address,
> >                          GenRegister value,
> >                          uint32_t bti) const
> >      {
> >        using namespace ir;
> >        GBE_ASSERT(insn.getValueNum() == 1);
> > -      const Type type = insn.getValueType();
> > -      const uint32_t elemSize = getByteScatterGatherSize(type);
> >        const uint32_t simdWidth = sel.ctx.getSimdWidth();
> >
> >        // We need a temporary register if we read bytes or words @@
> > -1711,13 +1710,15 @@ namespace gbe
> >                   insn.getAddressSpace() == MEM_PRIVATE ||
> >                   insn.getAddressSpace() == MEM_LOCAL);
> >        GBE_ASSERT(sel.ctx.isScalarReg(insn.getValue(0)) == false);
> > +      const Type type = insn.getValueType();
> > +      const uint32_t elemSize = getByteScatterGatherSize(type);
> >        if (insn.getAddressSpace() == MEM_CONSTANT)
> >          this->emitIndirectMove(sel, insn, address);
> > -      else if (insn.isAligned() == true)
> > +      else if (insn.isAligned() == true && elemSize ==
> > + GEN_BYTE_SCATTER_DWORD)
> >          this->emitUntypedRead(sel, insn, address, space ==
> MEM_LOCAL ?
> > 0xfe : 0x00);
> >        else {
> >          const GenRegister value = sel.selReg(insn.getValue(0));
> > -        this->emitByteGather(sel, insn, address, value, space ==
> > MEM_LOCAL ? 0xfe : 0x01);
> > +        this->emitByteGather(sel, insn, elemSize, address, value,
> > + space == MEM_LOCAL ? 0xfe : 0x01);
> >        }
> >        return true;
> >      }
> > @@ -1745,13 +1746,12 @@ namespace gbe
> >
> >      void emitByteScatter(Selection::Opaque &sel,
> >                           const ir::StoreInstruction &insn,
> > +                         const uint32_t elemSize,
> >                           GenRegister addr,
> >                           GenRegister value,
> >                           uint32_t bti) const
> >      {
> >        using namespace ir;
> > -      const Type type = insn.getValueType();
> > -      const uint32_t elemSize = getByteScatterGatherSize(type);
> >        const uint32_t simdWidth = sel.ctx.getSimdWidth();
> >        const GenRegister dst = value;
> >
> > @@ -1771,12 +1771,14 @@ namespace gbe
> >        using namespace ir;
> >        const AddressSpace space = insn.getAddressSpace();
> >        const uint32_t bti = space == MEM_LOCAL ? 0xfe : 0x01;
> > -      if (insn.isAligned() == true)
> > +      const Type type = insn.getValueType();
> > +      const uint32_t elemSize = getByteScatterGatherSize(type);
> > +      if (insn.isAligned() == true && elemSize ==
> > + GEN_BYTE_SCATTER_DWORD)
> >          this->emitUntypedWrite(sel, insn, bti);
> >        else {
> >          const GenRegister address = sel.selReg(insn.getAddress());
> >          const GenRegister value = sel.selReg(insn.getValue(0));
> > -        this->emitByteScatter(sel, insn, address, value, bti);
> > +        this->emitByteScatter(sel, insn, elemSize, address, value,
> > + bti);
> >        }
> >        return true;
> >      }
> > --
> > 1.7.11.7
> >
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet