[Mesa-dev] [PATCH] intel/eu: print bytes instead of 32 bit hex value

[Matt Turner]

Cool. This looks pretty good to me. A few comments inline.

On Wed, Aug 15, 2018 at 2:00 PM Sagar Ghuge <sagar.ghuge at intel.com> wrote:
>
> INTEL_DEBUG=hex prints 32 bit hex value
> and due to endianness of CPU byte order is
> reversed. In order to disassemble binary
> files, print each byte instead of 32 bit hex
> value.

Let's get your editor configured to line wrap at the correct length
(these lines are too short).

If you use vim, you should be able to automatically line wrap to the
appropriate length by highlighting the lines and then giving the
command 'gq'

> Signed-off-by: Sagar Ghuge <sagar.ghuge at intel.com>
> ---
>  src/intel/compiler/brw_eu.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c
> index 6ef0a6a577..223e561dff 100644
> --- a/src/intel/compiler/brw_eu.c
> +++ b/src/intel/compiler/brw_eu.c
> @@ -365,9 +365,14 @@ brw_disassemble(const struct gen_device_info *devinfo,
>        if (compacted) {
>           brw_compact_inst *compacted = (void *)insn;
>          if (dump_hex) {
> -           fprintf(out, "0x%08x 0x%08x                       ",
> -                   ((uint32_t *)insn)[1],
> -                   ((uint32_t *)insn)[0]);
> +           unsigned char * insn_ptr = ((unsigned char *)&insn[0]);
> +           for (int i = 0 ; i < 8; i = i + 4) {
> +              fprintf(out, "%02x %02x %02x %02x ",
> +                      insn_ptr[i],
> +                      insn_ptr[i + 1],
> +                      insn_ptr[i + 2],
> +                      insn_ptr[i + 3]);
> +           }

I like printing the spaces between the bytes. That really shows more
clearly that this is a byte array and not subject to any endianness
issues.

One suggestion: let's print some blank spaces after the compacted
instruction hex so that the disassembled instruction vertically aligns
with uncompacted instructions. Currently we get disassembly that looks
like

01 0b 1d 20 00 7c 02 00 mov(8)          g124<1>F        g2.3<0,1,0>F
01 00 60 00 e8 3a a0 2f 5c 00 00 00 00 00 00 00 mov(8)
g125<1>F        g2.7<0,1,0>F

Also, we don't use tabs in i965. When editing old lines that had tabs,
let's take the opportunity to remove them.

My ~/.vimrc has

autocmd BufNewFile,BufRead /home/mattst88/projects/mesa/* set
expandtab tabstop=8 softtabstop=3 shiftwidth=3
autocmd BufNewFile,BufRead
/home/mattst88/projects/mesa/src/glsl/glcpp/* set noexpandtab
tabstop=8 softtabstop=8 shiftwidth=8
autocmd BufNewFile,BufRead
/home/mattst88/projects/mesa/src/glsl/glsl_parser.yy set noexpandtab
tabstop=8 shiftwidth=8
autocmd BufNewFile,BufRead /home/mattst88/projects/piglit/* set
noexpandtab tabstop=8 softtabstop=8 shiftwidth=8

to configure it appropriately for my Mesa and piglit directories.

With those couple of small nits fixed, this will earn my review.