[Mesa-dev] [PATCH 09/10] i965/gen8: Make disassembly function match brw's signature.

[Kenneth Graunke]

On 05/13/2014 02:52 PM, Matt Turner wrote:
> gen8_dump_compile will be called indirectly by code common used by
> generations before and after the gen8 instruction format change.
> ---
>  src/mesa/drivers/dri/i965/gen8_fs_generator.cpp   | 4 ++--
>  src/mesa/drivers/dri/i965/gen8_generator.cpp      | 7 ++++---
>  src/mesa/drivers/dri/i965/gen8_generator.h        | 6 ++++--
>  src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp | 4 ++--
>  4 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp b/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp
> index de06a97..d25da81 100644
> --- a/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp
> @@ -1293,7 +1293,7 @@ gen8_fs_generator::generate_code(exec_list *instructions)
>        }
>  
>        if (unlikely(INTEL_DEBUG & DEBUG_WM)) {
> -         disassemble(stderr, last_native_inst_offset, next_inst_offset);
> +         gen8_dump_compile(brw, store, last_native_inst_offset, next_inst_offset, stderr);

I would much rather see both functions named "disassemble" than
"dump_compile."  I think it's a better description of what the function
does, and is really easy term to remember and search for.

Perhaps:
brw_disasm                  -> gen4_disassemble_inst
gen8_disassemble            -> gen8_disassemble_inst
brw_dump_compile            -> gen4_disassemble
gen8_generator::disassemble -> gen8_disassemble

Or disassemble_instruction/disassemble_program?

What do you think?  I'm fine with leaving your patch as-is and renaming
various functions in a follow-up patch or two.

>  
>           foreach_list(node, &cfg->block_list) {
>              bblock_link *link = (bblock_link *)node;
> @@ -1327,7 +1327,7 @@ gen8_fs_generator::generate_code(exec_list *instructions)
>      * case you're doing that.
>      */
>     if (0 && unlikely(INTEL_DEBUG & DEBUG_WM)) {
> -      disassemble(stderr, 0, next_inst_offset);
> +      gen8_dump_compile(brw, store, 0, next_inst_offset, stderr);
>     }
>  }
>  
> diff --git a/src/mesa/drivers/dri/i965/gen8_generator.cpp b/src/mesa/drivers/dri/i965/gen8_generator.cpp
> index faca9c0..774df18 100644
> --- a/src/mesa/drivers/dri/i965/gen8_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/gen8_generator.cpp
> @@ -620,13 +620,14 @@ gen8_generator::HALT()
>     return inst;
>  }
>  
> -void
> -gen8_generator::disassemble(FILE *out, int start, int end)
> +extern "C" void
> +gen8_dump_compile(struct brw_context *brw, void *assembly,
> +                  int start,int end, FILE *out)
>  {
>     bool dump_hex = false;
>  
>     for (int offset = start; offset < end; offset += 16) {
> -      gen8_instruction *inst = &store[offset / 16];
> +      gen8_instruction *inst = &((gen8_instruction *)assembly)[offset / 16];
>        fprintf(stderr, "0x%08x: ", offset);
>  
>        if (dump_hex) {
> diff --git a/src/mesa/drivers/dri/i965/gen8_generator.h b/src/mesa/drivers/dri/i965/gen8_generator.h
> index b144809..b6ed24c 100644
> --- a/src/mesa/drivers/dri/i965/gen8_generator.h
> +++ b/src/mesa/drivers/dri/i965/gen8_generator.h
> @@ -117,8 +117,6 @@ public:
>     gen8_instruction *NOP();
>     /** @} */
>  
> -   void disassemble(FILE *out, int start, int end);
> -
>  protected:
>     gen8_instruction *alu3(unsigned opcode,
>                            struct brw_reg dst,
> @@ -196,3 +194,7 @@ protected:
>  
>     void *mem_ctx;
>  };
> +
> +extern "C" void
> +gen8_dump_compile(struct brw_context *brw, void *assembly,
> +                  int start,int end, FILE *out);
> diff --git a/src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp b/src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp
> index 1d83312..0c54e6d 100644
> --- a/src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp
> @@ -910,7 +910,7 @@ gen8_vec4_generator::generate_code(exec_list *instructions)
>        }
>  
>        if (unlikely(debug_flag)) {
> -         disassemble(stderr, last_native_inst_offset, next_inst_offset);
> +         gen8_dump_compile(brw, store, last_native_inst_offset, next_inst_offset, stderr);
>        }
>  
>        last_native_inst_offset = next_inst_offset;
> @@ -928,7 +928,7 @@ gen8_vec4_generator::generate_code(exec_list *instructions)
>      * case you're doing that.
>      */
>     if (0 && unlikely(debug_flag)) {
> -      disassemble(stderr, 0, next_inst_offset);
> +      gen8_dump_compile(brw, store, 0, next_inst_offset, stderr);
>     }
>  }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140513/2235b8c9/attachment.sig>