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 > Subject: [Beignet] [PATCH 1/4] prepare gen9 sends binary format and enable the > ASM dump for sends > > Signed-off-by: Guo, Yejun > --- > 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 000..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 G
Re: [Beignet] [PATCH 1/4] prepare gen9 sends binary format and enable the ASM dump for sends
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 > Subject: [Beignet] [PATCH 1/4] prepare gen9 sends binary format and > enable the ASM dump for sends > > Signed-off-by: Guo, Yejun > --- > 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_V
Re: [Beignet] [PATCH 1/4] prepare gen9 sends binary format and enable the ASM dump for sends
> -Original Message- > From: Guo, Yejun > Sent: Thursday, November 24, 2016 10:21 AM > To: Song, Ruiling ; beignet@lists.freedesktop.org > Subject: RE: [Beignet] [PATCH 1/4] prepare gen9 sends binary format and enable > the ASM dump for sends > > thanks, and two inline comments, right? yes > > > +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? It's better to send your V2 to fix both issues. > ___ Beignet mailing list Beignet@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/beignet
Re: [Beignet] [PATCH 1/4] prepare gen9 sends binary format and enable the ASM dump for sends
ok -Original Message- From: Song, Ruiling Sent: Thursday, November 24, 2016 10:33 AM To: Guo, Yejun; beignet@lists.freedesktop.org Subject: RE: [Beignet] [PATCH 1/4] prepare gen9 sends binary format and enable the ASM dump for sends > -Original Message- > From: Guo, Yejun > Sent: Thursday, November 24, 2016 10:21 AM > To: Song, Ruiling ; > beignet@lists.freedesktop.org > Subject: RE: [Beignet] [PATCH 1/4] prepare gen9 sends binary format > and enable the ASM dump for sends > > thanks, and two inline comments, right? yes > > > +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? It's better to send your V2 to fix both issues. > ___ Beignet mailing list Beignet@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/beignet