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 <[email protected]> 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 > _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
