[Beignet] [PATCH 1/2] Multiple register's hstride in suboffset.
Zhigang Gong
zhigang.gong at linux.intel.com
Sun Jan 26 19:52:39 PST 2014
Got it, the previous macro is definitely perfect to hide bugs ;).
Thanks.
> -----Original Message-----
> From: Yang, Rong R [mailto:rong.r.yang at intel.com]
> Sent: Monday, January 27, 2014 11:36 AM
> To: Zhigang Gong
> Cc: beignet at lists.freedesktop.org
> Subject: RE: [Beignet] [PATCH 1/2] Multiple register's hstride in
suboffset.
>
> In previous suboffset, the second parameter delta means delta multiple
> register's typeSize byte offsets.
> So when register's hstride is not 1, caller should multiple it by hstride.
> In some callers, that known the register's hstride is 2, will multiple the
delta
> with hstride, by hard code.
> But most suboffset's caller don't handle it, so if register's hstride is
not 1, will
> fail.
> Now I move the hstride handling to suboffset, so delta means the delta
> elements in this register, and then must revert the delta that handle in
caller,
> because the hstride is 2, that's why the delta is half.
>
> The fail is cause by function MOV_DF, the register r's hstride is 1, but I
also
> divide it by 2 by mistake. I have send a now patch.
>
> -----Original Message-----
> From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com]
> Sent: Sunday, January 26, 2014 5:03 PM
> To: Yang, Rong R
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH 1/2] Multiple register's hstride in
suboffset.
>
> This patch seems causing a regression:
> compiler_double_2:
> compiler_double_2() [FAILED]
> Error: fabs(((double*)buf_data[1])[i] - cpu_dst[i]) < 1e-4
> at file /home/gongzg/git/fdo/beignet/utests/compiler_double_2.cpp,
> function compiler_double_2, line 42
>
> I just read this patch again and found that you only change the offset
> parameter to half of the previous value for part of the dest/src
registers. Is
> that correct?
>
> On Sun, Jan 26, 2014 at 11:21:14AM +0800, Yang Rong wrote:
> > When register's hstride is not 0 or 1, suboffset will get wrong element.
> > Also change some offsets that already multiple hstride by hard code.
> >
> > Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> > ---
> > backend/src/backend/gen_context.cpp | 26 +++++++++++++-------------
> > backend/src/backend/gen_encoder.cpp | 8 ++++----
> > backend/src/backend/gen_register.hpp | 2 +-
> > 3 files changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/backend/src/backend/gen_context.cpp
> > b/backend/src/backend/gen_context.cpp
> > index ecb8ef3..7d47a1e 100644
> > --- a/backend/src/backend/gen_context.cpp
> > +++ b/backend/src/backend/gen_context.cpp
> > @@ -206,7 +206,7 @@ namespace gbe
> > p->curr.chooseNib(i);
> > p->MOV(xdst, xsrc);
> > xdst = GenRegister::suboffset(xdst, 4);
> > - xsrc = GenRegister::suboffset(xsrc, 8);
> > + xsrc = GenRegister::suboffset(xsrc, 4);
> > }
> > p->pop();
> > break;
> > @@ -1205,10 +1205,10 @@ namespace gbe
> > p->curr.predicate = GEN_PREDICATE_NONE;
> > p->curr.execWidth = 8;
> > p->MOV(dest, src);
> > - p->MOV(GenRegister::suboffset(dest, 4), GenRegister::suboffset(src,
> 8));
> > + p->MOV(GenRegister::suboffset(dest, 4),
> > + GenRegister::suboffset(src, 4));
> > if (execWidth == 16) {
> > - p->MOV(GenRegister::suboffset(dest, 8),
GenRegister::suboffset(src,
> 16));
> > - p->MOV(GenRegister::suboffset(dest, 12),
> GenRegister::suboffset(src, 24));
> > + p->MOV(GenRegister::suboffset(dest, 8),
GenRegister::suboffset(src,
> 8));
> > + p->MOV(GenRegister::suboffset(dest, 12),
> > + GenRegister::suboffset(src, 12));
> > }
> > p->pop();
> > }
> > @@ -1220,13 +1220,13 @@ namespace gbe
> > p->curr.execWidth = 8;
> > p->MOV(dest, src);
> > p->curr.nibControl = 1;
> > - p->MOV(GenRegister::suboffset(dest, 8), GenRegister::suboffset(src,
> 4));
> > + p->MOV(GenRegister::suboffset(dest, 4),
> > + GenRegister::suboffset(src, 4));
> > if (execWidth == 16) {
> > p->curr.quarterControl = 1;
> > p->curr.nibControl = 0;
> > - p->MOV(GenRegister::suboffset(dest, 16),
> GenRegister::suboffset(src, 8));
> > + p->MOV(GenRegister::suboffset(dest, 8),
> > + GenRegister::suboffset(src, 8));
> > p->curr.nibControl = 1;
> > - p->MOV(GenRegister::suboffset(dest, 24),
> GenRegister::suboffset(src, 12));
> > + p->MOV(GenRegister::suboffset(dest, 12),
> > + GenRegister::suboffset(src, 12));
> > }
> > p->pop();
> > }
> > @@ -1238,10 +1238,10 @@ namespace gbe
> > p->curr.predicate = GEN_PREDICATE_NONE;
> > p->curr.execWidth = 8;
> > p->MOV(dest, src);
> > - p->MOV(GenRegister::suboffset(dest, 4), GenRegister::suboffset(src,
> 8));
> > + p->MOV(GenRegister::suboffset(dest, 4),
> > + GenRegister::suboffset(src, 4));
> > if (execWidth == 16) {
> > - p->MOV(GenRegister::suboffset(dest, 8),
GenRegister::suboffset(src,
> 16));
> > - p->MOV(GenRegister::suboffset(dest, 12),
> GenRegister::suboffset(src, 24));
> > + p->MOV(GenRegister::suboffset(dest, 8),
GenRegister::suboffset(src,
> 8));
> > + p->MOV(GenRegister::suboffset(dest, 12),
> > + GenRegister::suboffset(src, 12));
> > }
> > p->pop();
> > }
> > @@ -1253,13 +1253,13 @@ namespace gbe
> > p->curr.execWidth = 8;
> > p->MOV(dest, src);
> > p->curr.nibControl = 1;
> > - p->MOV(GenRegister::suboffset(dest, 8), GenRegister::suboffset(src,
> 4));
> > + p->MOV(GenRegister::suboffset(dest, 4),
> > + GenRegister::suboffset(src, 4));
> > if (execWidth == 16) {
> > p->curr.quarterControl = 1;
> > p->curr.nibControl = 0;
> > - p->MOV(GenRegister::suboffset(dest, 16),
> GenRegister::suboffset(src, 8));
> > + p->MOV(GenRegister::suboffset(dest, 8),
> > + GenRegister::suboffset(src, 8));
> > p->curr.nibControl = 1;
> > - p->MOV(GenRegister::suboffset(dest, 24),
> GenRegister::suboffset(src, 12));
> > + p->MOV(GenRegister::suboffset(dest, 12),
> > + GenRegister::suboffset(src, 12));
> > }
> > p->pop();
> > }
> > diff --git a/backend/src/backend/gen_encoder.cpp
> > b/backend/src/backend/gen_encoder.cpp
> > index c372e36..2ba5fd1 100644
> > --- a/backend/src/backend/gen_encoder.cpp
> > +++ b/backend/src/backend/gen_encoder.cpp
> > @@ -908,26 +908,26 @@ namespace gbe
> > curr.execWidth = 8;
> > curr.predicate = GEN_PREDICATE_NONE;
> > MOV(r0, src0);
> > - MOV(GenRegister::suboffset(r0, 8), GenRegister::suboffset(src0,
4));
> > + MOV(GenRegister::suboffset(r0, 4), GenRegister::suboffset(src0,
> > + 4));
> > curr.predicate = GEN_PREDICATE_NORMAL;
> > curr.quarterControl = 0;
> > curr.nibControl = 0;
> > MOV(dest, r);
> > curr.nibControl = 1;
> > - MOV(GenRegister::suboffset(dest, 4), GenRegister::suboffset(r,
8));
> > + MOV(GenRegister::suboffset(dest, 4), GenRegister::suboffset(r,
> > + 4));
> > pop();
> > if (w == 16) {
> > push();
> > curr.execWidth = 8;
> > curr.predicate = GEN_PREDICATE_NONE;
> > MOV(r0, GenRegister::suboffset(src0, 8));
> > - MOV(GenRegister::suboffset(r0, 8), GenRegister::suboffset(src0,
> 12));
> > + MOV(GenRegister::suboffset(r0, 4),
> > + GenRegister::suboffset(src0, 12));
> > curr.predicate = GEN_PREDICATE_NORMAL;
> > curr.quarterControl = 1;
> > curr.nibControl = 0;
> > MOV(GenRegister::suboffset(dest, 8), r);
> > curr.nibControl = 1;
> > - MOV(GenRegister::suboffset(dest, 12), GenRegister::suboffset(r,
> 8));
> > + MOV(GenRegister::suboffset(dest, 12),
> > + GenRegister::suboffset(r, 4));
> > pop();
> > }
> > }
> > diff --git a/backend/src/backend/gen_register.hpp
> > b/backend/src/backend/gen_register.hpp
> > index 57c78d9..8794318 100644
> > --- a/backend/src/backend/gen_register.hpp
> > +++ b/backend/src/backend/gen_register.hpp
> > @@ -763,7 +763,7 @@ namespace gbe
> >
> > static INLINE GenRegister suboffset(GenRegister reg, uint32_t
delta) {
> > if (reg.hstride != GEN_HORIZONTAL_STRIDE_0) {
> > - reg.subnr += delta * typeSize(reg.type);
> > + reg.subnr += delta * typeSize(reg.type) * hstride_size(reg);
> > reg.nr += reg.subnr / 32;
> > reg.subnr %= 32;
> > }
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list