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
