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@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-boun...@lists.freedesktop.org] On Behalf > Of Guo, Yejun > Sent: Tuesday, November 22, 2016 2:42 PM > To: beignet@lists.freedesktop.org > Cc: Guo, Yejun <yejun....@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....@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....@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list Beignet@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/beignet