[Mesa-dev] [PATCH v3] nv50/ir: add debug options for shader replacement

[Ilia Mirkin]

Hi Rhys,

I really like this in general, but I have some feedback on the
specific implementation:

1. The shader and its header can be closely related. Please integrate
something that also loads/dumps the shader header (separate file).
This should go into nvc0_program, as that's where the header is
generated. Probably means you have to compute the sha there directly
and feed it into the compiler (see #3 below).
2. Move these functions into nv50_ir_dump.cpp or something.
3. If you're replacing the shader, don't go through the whole
compile/optimize flow - no reason for it. I don't think doing the sha
of the bin code is necessary - for any IR input we should be able to
get a consistent hash of it. So the flow should be "dump; if (replace)
replace; else compile;". I think that should be achievable.
4. Why truncate the sha1?
5. maxGPR = targ->getFileSize() - 1; this also handles the compute
case where you have to limit the number of GPRs based on the number of
CS threads.

Thanks for working on this!

  -ilia

On Tue, May 22, 2018 at 3:46 PM, Rhys Perry <pendingchaos02 at gmail.com> wrote:
> Changes in v2:
> - move "#ifdef DEBUG" from above dumpProgram to above createDumpFilename
> Changes in v3:
> - Fixed messed up patch description and diff
> - Use the checksum of the TGSI instead of the binary if possible
>
> The NV50_PROG_DUMP environment variable specifies a (already created) directory
> to dump both shader binaries and tgsi code. The NV50_PROG_REPLACE environment
> variable specified a (again, already created) directory that is searched to
> find replacement binaries. This is all much like MESA_SHADER_DUMP_PATH and
> MESA_SHADER_READ_PATH except using shortened SHA1 checksums instead of program
> IDs and chip-specific binaries instead of GLSL.
>
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir.cpp | 124 ++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
> index c987da9908..f7fda3a7c6 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.cpp
> @@ -23,6 +23,10 @@
>  #include "codegen/nv50_ir.h"
>  #include "codegen/nv50_ir_target.h"
>  #include "codegen/nv50_ir_driver.h"
> +#ifdef DEBUG
> +#include "tgsi/tgsi_dump.h"
> +#include "util/mesa-sha1.h"
> +#endif
>
>  extern "C" {
>  #include "nouveau_debug.h"
> @@ -1161,6 +1165,121 @@ void Program::releaseValue(Value *value)
>        mem_Symbol.release(value);
>  }
>
> +#ifdef DEBUG
> +static char*
> +createDumpFilename(const char *dir, nv50_ir::Program *prog, const char *ext)
> +{
> +   char* fname = (char*)malloc(strlen(dir)+13+strlen(ext));
> +   if (dir[0] && dir[strlen(dir)-1]=='/')
> +      strcpy(fname, dir);
> +   else
> +      sprintf(fname, "%s/", dir);
> +
> +   unsigned char sha1_bin[20];
> +   char sha1_str[41];
> +   if (prog->driver->bin.sourceRep == PIPE_SHADER_IR_TGSI) {
> +      const tgsi_header* header = (const tgsi_header*)prog->driver->bin.source;
> +      unsigned size = (header->HeaderSize + header->BodySize) * sizeof(tgsi_token);
> +      _mesa_sha1_compute(prog->driver->bin.source, size, sha1_bin);
> +   } else {
> +      _mesa_sha1_compute(prog->code, prog->binSize, sha1_bin);
> +   }
> +   _mesa_sha1_format(sha1_str, sha1_bin);
> +   sha1_str[7] = 0;
> +   strcat(fname, sha1_str);
> +
> +   switch (prog->getType()) {
> +   case nv50_ir::Program::TYPE_VERTEX:
> +      strcat(fname, ".vs");
> +      break;
> +   case nv50_ir::Program::TYPE_TESSELLATION_CONTROL:
> +      strcat(fname, ".tcs");
> +      break;
> +   case nv50_ir::Program::TYPE_TESSELLATION_EVAL:
> +      strcat(fname, ".tes");
> +      break;
> +   case nv50_ir::Program::TYPE_GEOMETRY:
> +      strcat(fname, ".gs");
> +      break;
> +   case nv50_ir::Program::TYPE_FRAGMENT:
> +      strcat(fname, ".fs");
> +      break;
> +   case nv50_ir::Program::TYPE_COMPUTE:
> +      strcat(fname, ".cs");
> +      break;
> +   }
> +
> +   strcat(fname, ext);
> +
> +   return fname;
> +}
> +
> +static void
> +dumpProgram(nv50_ir::Program *prog)
> +{
> +   const char *dump_dir = debug_get_option("NV50_PROG_DUMP", NULL);
> +   if (!dump_dir)
> +      return;
> +
> +   char* fname = createDumpFilename(dump_dir, prog, ".bin");
> +
> +   FILE *fp = fopen(fname, "wb");
> +   if (!fp) {
> +      INFO("Failed to dump code of program %p to %s\n", prog, fname);
> +      return;
> +   }
> +
> +   fwrite(prog->code, prog->binSize, 1, fp);
> +   fclose(fp);
> +
> +   INFO("Dumped code of program %p to %s\n", prog, fname);
> +
> +   free(fname);
> +
> +   if (prog->driver->bin.sourceRep == PIPE_SHADER_IR_TGSI) {
> +      char* fname = createDumpFilename(dump_dir, prog, ".tgsi.txt");
> +      const tgsi_token *tokens = (const tgsi_token *)prog->driver->bin.source;
> +
> +      FILE *fp = fopen(fname, "w");
> +      tgsi_dump_to_file(tokens, 0, fp);
> +      fclose(fp);
> +
> +      INFO("Dumped tgsi of program %p to %s\n", prog, fname);
> +
> +      free(fname);
> +   }
> +}
> +
> +static void
> +replaceProgram(nv50_ir::Program *prog)
> +{
> +   const nv50_ir::Target* targ = prog->getTarget();
> +
> +   const char *replace_dir = debug_get_option("NV50_PROG_REPLACE", NULL);
> +   if (!replace_dir)
> +      return;
> +
> +   char* fname = createDumpFilename(replace_dir, prog, ".bin");
> +
> +   FILE *fp = fopen(fname, "rb");
> +   if (!fp)
> +      return;
> +
> +   FREE(prog->code);
> +   prog->code = (uint32_t*)MALLOC(65536);
> +   prog->binSize = fread(prog->code, 1, 65536, fp);
> +
> +   unsigned maxGPR = targ->getChipset() >= NVISA_GK20A_CHIPSET ? 254 : 62;
> +   prog->maxGPR = MIN2(targ->getFileSize(nv50_ir::FILE_GPR), maxGPR);
> +
> +   fclose(fp);
> +
> +   INFO("Replaced code of program %p with that from %s\n", prog, fname);
> +
> +   free(fname);
> +}
> +#endif
> +
>
>  } // namespace nv50_ir
>
> @@ -1270,6 +1389,11 @@ nv50_ir_generate_code(struct nv50_ir_prog_info *info)
>        goto out;
>     }
>
> +#ifdef DEBUG
> +   nv50_ir::dumpProgram(prog);
> +   nv50_ir::replaceProgram(prog);
> +#endif
> +
>  out:
>     INFO_DBG(prog->dbgFlags, VERBOSE, "nv50_ir_generate_code: ret = %i\n", ret);
>
> --
> 2.14.3
>