> -----Original Message-----
> From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of
> rander
> Sent: Monday, February 27, 2017 12:25 PM
> To: beignet@lists.freedesktop.org
> Cc: Wang, Rander <rander.w...@intel.com>
> Subject: [Beignet] [PATCH] Backend: for BDW and after, According to BSpec no
> need to split CMP when src is DW DF
>
> Signed-off-by: rander <rander.w...@intel.com>
> ---
> backend/src/backend/gen8_encoder.cpp | 10 ++++++++++
> backend/src/backend/gen8_encoder.hpp | 1 +
> backend/src/backend/gen9_encoder.cpp | 5 +++++
> backend/src/backend/gen9_encoder.hpp | 1 +
> backend/src/backend/gen_encoder.cpp | 7 ++++++-
> backend/src/backend/gen_encoder.hpp | 1 +
> 6 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/backend/src/backend/gen8_encoder.cpp
> b/backend/src/backend/gen8_encoder.cpp
> index a33fbac..bb4fdb0 100644
> --- a/backend/src/backend/gen8_encoder.cpp
> +++ b/backend/src/backend/gen8_encoder.cpp
> @@ -38,6 +38,7 @@ static const uint32_t untypedRWMask[] = {
> namespace gbe
> {
> extern bool compactAlu3(GenEncoder *p, uint32_t opcode, GenRegister dst,
> GenRegister src0, GenRegister src1, GenRegister src2);
> +
> void Gen8Encoder::setHeader(GenNativeInstruction *insn) {
> Gen8NativeInstruction *gen8_insn = &insn->gen8_insn;
> if (this->curr.execWidth == 8)
> @@ -883,4 +884,13 @@ namespace gbe
> msg_length,
> response_length);
> }
> +
> + /* for BDW and after, no need to split CMP when src is DW*/
> + bool Gen8Encoder::needToSplitCmpBySrcType(GenEncoder *p, GenRegister
> src0, GenRegister src1) {
I am a little confusing, in the comment you said no need to split if src is DW.
Why do you still return "true" for GEN_TYPE_F?
> + if (src0.type == GEN_TYPE_F)
> + return true;
> + if (src1.type == GEN_TYPE_F)
> + return true;
> + return false;
> + }
> } /* End of the name space. */
> diff --git a/backend/src/backend/gen8_encoder.hpp
> b/backend/src/backend/gen8_encoder.hpp
> index fa62a8d..51c079c 100644
> --- a/backend/src/backend/gen8_encoder.hpp
> +++ b/backend/src/backend/gen8_encoder.hpp
> @@ -83,6 +83,7 @@ namespace gbe
> virtual void OBREADA64(GenRegister dst, GenRegister header, uint32_t bti,
> uint32_t elemSize);
> /*! A64 OBlock write */
> virtual void OBWRITEA64(GenRegister header, uint32_t bti, uint32_t
> elemSize);
> + virtual bool needToSplitCmpBySrcType(GenEncoder *p, GenRegister src0,
> GenRegister src1);
> };
> }
> #endif /* __GBE_GEN8_ENCODER_HPP__ */
> diff --git a/backend/src/backend/gen9_encoder.cpp
> b/backend/src/backend/gen9_encoder.cpp
> index b37fd98..f2b9274 100644
> --- a/backend/src/backend/gen9_encoder.cpp
> +++ b/backend/src/backend/gen9_encoder.cpp
> @@ -301,4 +301,9 @@ namespace gbe
> response_length);
> }
> }
> +
> + bool Gen9Encoder::needToSplitCmpBySrcType(GenEncoder *p, GenRegister
> src0, GenRegister src1)
> + {
> + return false;
> + }
> } /* End of the name space. */
> diff --git a/backend/src/backend/gen9_encoder.hpp
> b/backend/src/backend/gen9_encoder.hpp
> index 2eaa538..69b7490 100644
> --- a/backend/src/backend/gen9_encoder.hpp
> +++ b/backend/src/backend/gen9_encoder.hpp
> @@ -56,6 +56,7 @@ namespace gbe
> virtual void ATOMIC(GenRegister dst, uint32_t function, GenRegister addr,
> GenRegister data, GenRegister bti, uint32_t srcNum, bool useSends);
> virtual void OBWRITE(GenRegister header, GenRegister data, uint32_t bti,
> uint32_t ow_size, bool useSends);
> virtual void MBWRITE(GenRegister header, GenRegister data, uint32_t bti,
> uint32_t data_size, bool useSends);
> + virtual bool needToSplitCmpBySrcType(GenEncoder *p, GenRegister src0,
> GenRegister src1);
> };
> }
> #endif /* __GBE_GEN9_ENCODER_HPP__ */
> diff --git a/backend/src/backend/gen_encoder.cpp
> b/backend/src/backend/gen_encoder.cpp
> index 03ce0e2..296a0c5 100644
> --- a/backend/src/backend/gen_encoder.cpp
> +++ b/backend/src/backend/gen_encoder.cpp
> @@ -192,6 +192,10 @@ namespace gbe
> if (isSrcDstDiffSpan(dst, src0) == true) return true;
> if (isSrcDstDiffSpan(dst, src1) == true) return true;
I would like to see needToSplitCmpBySrcType be called here.
That is needToSplitCmp() will do all kind of check. That is conform to its name.
>
> + return false;
> + }
> +
> + bool GenEncoder::needToSplitCmpBySrcType(GenEncoder *p, GenRegister
> src0, GenRegister src1) {
> if (src0.type == GEN_TYPE_D || src0.type == GEN_TYPE_UD || src0.type ==
> GEN_TYPE_F)
> return true;
> if (src1.type == GEN_TYPE_D || src1.type == GEN_TYPE_UD || src1.type ==
> GEN_TYPE_F)
> @@ -199,6 +203,7 @@ namespace gbe
> return false;
> }
>
> +
> void GenEncoder::setMessageDescriptor(GenNativeInstruction *inst, enum
> GenMessageTarget sfid,
> unsigned msg_length, unsigned
> response_length,
> bool header_present, bool
> end_of_thread)
> @@ -1041,7 +1046,7 @@ namespace gbe
> }
>
> void GenEncoder::CMP(uint32_t conditional, GenRegister src0, GenRegister
> src1, GenRegister dst) {
> - if (needToSplitCmp(this, src0, src1, dst) == false) {
Please remove the call to needToSplitCmpBySrcType() here. I think move it into
needToSplitCmp() is more clear.
> + if ((needToSplitCmp(this, src0, src1, dst) |
> needToSplitCmpBySrcType(this,
> src0, src1))== false) {
> if(!GenRegister::isNull(dst) && compactAlu2(this, GEN_OPCODE_CMP, dst,
> src0, src1, conditional, false)) {
> return;
> }
> diff --git a/backend/src/backend/gen_encoder.hpp
> b/backend/src/backend/gen_encoder.hpp
> index 3e45c81..040b94a 100644
> --- a/backend/src/backend/gen_encoder.hpp
> +++ b/backend/src/backend/gen_encoder.hpp
> @@ -162,6 +162,7 @@ namespace gbe
> void BRD(GenRegister src);
> /*! Compare instructions */
> void CMP(uint32_t conditional, GenRegister src0, GenRegister src1,
> GenRegister dst = GenRegister::null());
> + virtual bool needToSplitCmpBySrcType(GenEncoder *p, GenRegister src0,
> GenRegister src1);
> /*! Select with embedded compare (like sel.le ...) */
> void SEL_CMP(uint32_t conditional, GenRegister dst, GenRegister src0,
> GenRegister src1);
> /*! EOT is used to finish GPGPU threads */
> --
> 2.7.4
>
> _______________________________________________
> Beignet mailing list
> Beignet@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/beignet
_______________________________________________
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet
