[Beignet] [PATCH 1/4] prepare gen9 sends binary format and enable the ASM dump for sends

Guo, Yejun yejun.guo at intel.com
Thu Nov 24 02:21:10 UTC 2016


thanks, and two inline comments, right?

> +    string(file, "null");
This is not a good idea. Please parse the instruction bits to know whether it is the null register.
[yejun] yes, when I did something like atomic which has return value, I found it is not always null. So, I fixed this in another patch, locally now. 

> +    format(file, "0x%x", gen9_insn->bits3.ud);
"0x%08x" would be better.
[yejun] agree, should I send a v2 patch, or fix it in the latter patch together with the above issue?

-----Original Message-----
From: Song, Ruiling 
Sent: Wednesday, November 23, 2016 10:15 PM
To: Guo, Yejun; beignet at lists.freedesktop.org
Cc: Guo, Yejun
Subject: RE: [Beignet] [PATCH 1/4] prepare gen9 sends binary format and enable the ASM dump for sends



> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf 
> Of Guo, Yejun
> Sent: Tuesday, November 22, 2016 2:42 PM
> To: beignet at lists.freedesktop.org
> Cc: Guo, Yejun <yejun.guo at intel.com>
> Subject: [Beignet] [PATCH 1/4] prepare gen9 sends binary format and 
> enable the ASM dump for sends
> 
> Signed-off-by: Guo, Yejun <yejun.guo at intel.com>
> ---
>  backend/src/backend/gen/gen_mesa_disasm.c |  28 ++++++--  
> backend/src/backend/gen9_instruction.hpp  | 112
> ++++++++++++++++++++++++++++++
>  backend/src/backend/gen_defs.hpp          |   3 +
>  3 files changed, 139 insertions(+), 4 deletions(-)  create mode 
> 100644 backend/src/backend/gen9_instruction.hpp
> 
> diff --git a/backend/src/backend/gen/gen_mesa_disasm.c
> b/backend/src/backend/gen/gen_mesa_disasm.c
> index c30f168..4f6c35d 100644
> --- a/backend/src/backend/gen/gen_mesa_disasm.c
> +++ b/backend/src/backend/gen/gen_mesa_disasm.c
> @@ -50,6 +50,7 @@
> 
>  #include "backend/gen_defs.hpp"
>  #include "backend/gen7_instruction.hpp"
> +#include "backend/gen9_instruction.hpp"
>  #include "src/cl_device_data.h"
> 
>  static const struct {
> @@ -104,6 +105,7 @@ static const struct {
> 
>    [GEN_OPCODE_SEND] = { .name = "send", .nsrc = 2, .ndst = 1 },
>    [GEN_OPCODE_SENDC] = { .name = "sendc", .nsrc = 2, .ndst = 1 },
> +  [GEN_OPCODE_SENDS] = { .name = "sends", .nsrc = 2, .ndst = 1 },
>    [GEN_OPCODE_NOP] = { .name = "nop", .nsrc = 0, .ndst = 0 },
>    [GEN_OPCODE_JMPI] = { .name = "jmpi", .nsrc = 0, .ndst = 0 },
>    [GEN_OPCODE_BRD] = { .name = "brd", .nsrc = 0, .ndst = 0 }, @@ 
> -1411,7 +1413,8 @@ int gen_disasm (FILE *file, const void *inst, 
> uint32_t deviceID, uint32_t compac
>      }
> 
>    } else if (OPCODE(inst) != GEN_OPCODE_SEND &&
> -             OPCODE(inst) != GEN_OPCODE_SENDC) {
> +             OPCODE(inst) != GEN_OPCODE_SENDC &&
> +             OPCODE(inst) != GEN_OPCODE_SENDS) {
>      err |= control(file, "conditional modifier", conditional_modifier,
>                     COND_DST_OR_MODIFIER(inst), NULL);
>      if (COND_DST_OR_MODIFIER(inst))
> @@ -1426,7 +1429,17 @@ int gen_disasm (FILE *file, const void *inst, 
> uint32_t deviceID, uint32_t compac
>      string(file, ")");
>    }
> 
> -  if (opcode[OPCODE(inst)].nsrc == 3) {
> +  if (OPCODE(inst) == GEN_OPCODE_SENDS) {
> +    const union Gen9NativeInstruction *gen9_insn = (const union
> Gen9NativeInstruction *)inst;
> +    pad(file, 16);
> +    string(file, "null");
This is not a good idea. Please parse the instruction bits to know whether it is the null register.
> +    pad(file, 32);
> +    format(file, "g%d(addLen:%d)", 
> + gen9_insn->bits2.sends.src0_reg_nr,
> gen9_insn->bits3.sends_untyped_rw.src0_length);
> +    pad(file, 48);
> +    format(file, "g%d(dataLen:%d)", 
> + gen9_insn->bits1.sends.src1_reg_nr,
> gen9_insn->bits2.sends.src1_length);
> +    pad(file, 64);
> +    format(file, "0x%x", gen9_insn->bits3.ud);
"0x%08x" would be better.
 
> +  } else if (opcode[OPCODE(inst)].nsrc == 3) {
>      pad(file, 16);
>      err |= dest_3src(file, inst);
> 
> @@ -1469,7 +1482,8 @@ int gen_disasm (FILE *file, const void *inst, 
> uint32_t deviceID, uint32_t compac
>    }
> 
>    if (OPCODE(inst) == GEN_OPCODE_SEND ||
> -      OPCODE(inst) == GEN_OPCODE_SENDC) {
> +      OPCODE(inst) == GEN_OPCODE_SENDC ||
> +      OPCODE(inst) == GEN_OPCODE_SENDS) {
>      enum GenMessageTarget target = COND_DST_OR_MODIFIER(inst);
> 
>      newline(file);
> @@ -1484,7 +1498,13 @@ int gen_disasm (FILE *file, const void *inst, 
> uint32_t deviceID, uint32_t compac
>                       target, &space);
>      }
> 
> -    if (GEN_BITS_FIELD2(inst, bits1.da1.src1_reg_file, bits2.da1.src1_reg_file) ==
> GEN_IMMEDIATE_VALUE) {
> +    int immbti = 0;
> +    if (OPCODE(inst) == GEN_OPCODE_SENDS) {
> +      const union Gen9NativeInstruction *gen9_insn = (const union
> Gen9NativeInstruction *)inst;
> +      immbti = !(gen9_insn->bits2.sends.sel_reg32_desc);
> +    } else
> +      immbti = (GEN_BITS_FIELD2(inst, bits1.da1.src1_reg_file,
> bits2.da1.src1_reg_file) == GEN_IMMEDIATE_VALUE);
> +    if (immbti) {
>        switch (target) {
>          case GEN_SFID_VIDEO_MOTION_EST:
>            format(file, " (bti: %d, msg_type: %d)", diff --git 
> a/backend/src/backend/gen9_instruction.hpp
> b/backend/src/backend/gen9_instruction.hpp
> new file mode 100644
> index 0000000..9d57f08
> --- /dev/null
> +++ b/backend/src/backend/gen9_instruction.hpp
> @@ -0,0 +1,112 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Author: Guo, Yejun <yejun.guo at intel.com>  */
> +
> +
> +#ifndef __GEN9_INSTRUCTION_HPP__
> +#define __GEN9_INSTRUCTION_HPP__
> +
> +union Gen9NativeInstruction
> +{
> +  struct {
> +    struct {
> +      uint32_t opcode:7;
> +      uint32_t pad:1;
> +      uint32_t access_mode:1;
> +      uint32_t dependency_control:2;
> +      uint32_t nib_ctrl:1;
> +      uint32_t quarter_control:2;
> +      uint32_t thread_control:2;
> +      uint32_t predicate_control:4;
> +      uint32_t predicate_inverse:1;
> +      uint32_t execution_size:3;
> +      uint32_t destreg_or_condmod:4;
> +      uint32_t acc_wr_control:1;
> +      uint32_t cmpt_control:1;
> +      uint32_t debug_control:1;
> +      uint32_t saturate:1;
> +    } header;
> +
> +    union {
> +      struct {
> +        uint32_t flag_sub_reg_nr:1;
> +        uint32_t flag_reg_nr:1;
> +        uint32_t mask_control:1;
> +        uint32_t dest_reg_file_0:1;
> +        uint32_t src1_reg_file_0:1;
> +        uint32_t dest_reg_type:4;
> +        uint32_t pad0:3;
> +        uint32_t src1_reg_nr:8;
> +        uint32_t dest_subreg_nr:1;
> +        uint32_t dest_reg_nr:8;
> +        uint32_t pad1:1;
> +        uint32_t pad2:1;    //direct mode is used
> +        uint32_t dest_address_mode:1;
> +      } sends;
> +
> +      uint32_t ud;
> +    }bits1;
> +
> +    union {
> +      struct {
> +        uint32_t src1_length:4;     //exdesc_9_6
> +        uint32_t src0_subreg_nr:1;
> +        uint32_t src0_reg_nr:8;
> +        uint32_t sel_reg32_desc:1;
> +        uint32_t pad0:1;
> +        uint32_t src0_address_mode:1;
> +        uint32_t exdesc_31_16:16;
> +      } sends;
> +
> +      uint32_t ud;
> +    } bits2;
> +
> +    union {
> +      struct {
> +        uint32_t bti:8;
> +        uint32_t rgba:4;
> +        uint32_t simd_mode:2;
> +        uint32_t msg_type:4;
> +        uint32_t category:1;
> +        uint32_t header_present:1;
> +        uint32_t response_length:5;
> +        uint32_t src0_length:4;
> +        uint32_t pad2:2;
> +        uint32_t end_of_thread:1;
> +      } sends_untyped_rw;
> +
> +      struct {
> +        uint32_t bti:8;
> +        uint32_t simd_mode:1;
> +        uint32_t ignored0:1;
> +        uint32_t data_size:2;
> +        uint32_t ignored1:2;
> +        uint32_t msg_type:4;
> +        uint32_t category:1;
> +        uint32_t header_present:1;
> +        uint32_t response_length:5;
> +        uint32_t src0_length:4;
> +        uint32_t pad2:2;
> +        uint32_t end_of_thread:1;
> +      } sends_byte_rw;
> +
> +      uint32_t ud;
> +    } bits3;
> +  };
> +};
> +#endif
> diff --git a/backend/src/backend/gen_defs.hpp
> b/backend/src/backend/gen_defs.hpp
> index a6c6bb0..c34e1bb 100644
> --- a/backend/src/backend/gen_defs.hpp
> +++ b/backend/src/backend/gen_defs.hpp
> @@ -54,6 +54,7 @@
>  #include <stdint.h>
>  #include "backend/gen7_instruction.hpp"
>  #include "backend/gen8_instruction.hpp"
> +#include "backend/gen9_instruction.hpp"
> 
>  
> //////////////////////////////////////////////////////////////////////
> ///////
>  // Gen EU defines
> @@ -148,6 +149,7 @@ enum opcode {
>    GEN_OPCODE_WAIT = 48,
>    GEN_OPCODE_SEND = 49,
>    GEN_OPCODE_SENDC = 50,
> +  GEN_OPCODE_SENDS = 51,
>    GEN_OPCODE_MATH = 56,
>    GEN_OPCODE_ADD = 64,
>    GEN_OPCODE_MUL = 65,
> @@ -559,6 +561,7 @@ union GenNativeInstruction
>    };
>    union Gen7NativeInstruction gen7_insn;
>    union Gen8NativeInstruction gen8_insn;
> +  union Gen9NativeInstruction gen9_insn;
> 
>    //Gen7 & Gen8 common field
>    struct {
> --
> 1.9.1
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list