[Mesa-dev] [PATCH 08/18] intel/compiler:add function to give option to print offsets into assembly

[Rogovin, Kevin]

I am fine with that suggestion of changing the function signature and to not add a new one; I wanted the changes I made to be as uninvasive as possible which is why I added the function instead of changing an existing one.

-Kevin

-----Original Message-----
From: Matt Turner [mailto:mattst88 at gmail.com] 
Sent: Monday, November 13, 2017 9:42 PM
To: Rogovin, Kevin <kevin.rogovin at intel.com>
Cc: mesa-dev at lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 08/18] intel/compiler:add function to give option to print offsets into assembly

On Mon, Nov 13, 2017 at 5:17 AM,  <kevin.rogovin at intel.com> wrote:
> From: Kevin Rogovin <kevin.rogovin at intel.com>
>
> Signed-off-by: Kevin Rogovin <kevin.rogovin at intel.com>
> ---
>  src/intel/compiler/brw_eu.c | 11 ++++++++++-  
> src/intel/compiler/brw_eu.h |  3 +++
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c 
> index bc297a21b3..8969ae5bda 100644
> --- a/src/intel/compiler/brw_eu.c
> +++ b/src/intel/compiler/brw_eu.c
> @@ -339,6 +339,15 @@ const unsigned *brw_get_program( struct 
> brw_codegen *p,  void  brw_disassemble(const struct gen_device_info 
> *devinfo,
>                  const void *assembly, int start, int end, FILE *out)
> +{
> +   brw_disassemble_print_offset_option(devinfo, assembly, start, end, out,
> +                                       false); }
> +
> +void
> +brw_disassemble_print_offset_option(const struct gen_device_info *devinfo,
> +                                    const void *assembly, int start, int end,
> +                                    FILE *out, bool print_offsets)
>  {
>     bool dump_hex = (INTEL_DEBUG & DEBUG_HEX) != 0;
>
> @@ -346,7 +355,7 @@ brw_disassemble(const struct gen_device_info *devinfo,
>        const brw_inst *insn = assembly + offset;
>        brw_inst uncompacted;
>        bool compacted = brw_inst_cmpt_control(devinfo, insn);
> -      if (0)
> +      if (print_offsets)
>           fprintf(out, "0x%08x: ", offset);
>
>        if (compacted) {
> diff --git a/src/intel/compiler/brw_eu.h b/src/intel/compiler/brw_eu.h 
> index 95503d5513..497cf9e575 100644
> --- a/src/intel/compiler/brw_eu.h
> +++ b/src/intel/compiler/brw_eu.h
> @@ -128,6 +128,9 @@ int brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo,
>                           const struct brw_inst *inst, bool 
> is_compacted);  void brw_disassemble(const struct gen_device_info *devinfo,
>                       const void *assembly, int start, int end, FILE 
> *out);
> +void brw_disassemble_print_offset_option(const struct gen_device_info *devinfo,
> +                                         const void *assembly, int start, int end,
> +                                         FILE *out, bool 
> +print_offsets);

Instead of adding a new function, I'd be in favor of just adding the bool print_offsets parameter to brw_disassemble (before the FILE parameter). Then we can add an INTEL_DEBUG=print_offsets and pass INTEL_DEBUG & DEBUG_PRINT_OFFSETS as the argument in the existing calls.