[Beignet] [PATCH 1/2] Multiple register's hstride in suboffset.
Yang, Rong R
rong.r.yang at intel.com
Sun Jan 26 19:35:47 PST 2014
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