Got it, the previous macro is definitely perfect to hide bugs ;). Thanks. > -----Original Message----- > From: Yang, Rong R [mailto:[email protected]] > Sent: Monday, January 27, 2014 11:36 AM > To: Zhigang Gong > Cc: [email protected] > 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:[email protected]] > Sent: Sunday, January 26, 2014 5:03 PM > To: Yang, Rong R > Cc: [email protected] > 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 <[email protected]> > > --- > > 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 > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/beignet
_______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
