Skip to content

Fix names of printed ptx and cubin files#3788

Merged
naoyam merged 1 commit intomainfrom
ptx_cubin_filenames
Jan 29, 2025
Merged

Fix names of printed ptx and cubin files#3788
naoyam merged 1 commit intomainfrom
ptx_cubin_filenames

Conversation

@jacobhinkle
Copy link
Collaborator

This uses the function name as the filename. Since #3778 we have been generating filenames like __tmp_.ptx since the kernel_name is not yet filled when cubin and ptx files are output. Now we will get __tmp_nvfuser_none_f0_c0_r0_g0.{cu,cubin,ptx} or if NVFUSER_ENABLE=static_fusion_count is provided
__tmp_nvfuser_1.{cu,cubin,ptx}.

This uses the function name as the filename. Since #3778 we have been
generating filenames like `__tmp_.ptx` since the `kernel_name` is not
yet filled when cubin and ptx files are output. Now we will get
`__tmp_nvfuser_none_f0_c0_r0_g0.{cu,cubin,ptx}` or if
`NVFUSER_ENABLE=static_fusion_count` is provided
`__tmp_nvfuser_1.{cu,cubin,ptx}`.
@jacobhinkle
Copy link
Collaborator Author

!test

@jacobhinkle
Copy link
Collaborator Author

!test --diff

@jacobhinkle
Copy link
Collaborator Author

The codediff job should fail, but we can inspect the artifact to ensure that all the files were created with the proper names.

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
⚡ Recommended focus areas for review

Changed Parameter

The kernel_name parameter has been removed from the compileSource function and replaced with func_name. This change may affect the functionality of the code.

const std::string& func_name,
const bool compile_to_sass,
Renamed File Names

The file names for the compiled kernel's PTX and CUBIN files are now generated using the func_name instead of kernel_name. This may cause issues if the file names are used elsewhere in the code.

        dumpCompiledCodeToFile(compiled_kernel->cubin, func_name, ".cubin");
  }
}

if (!compile_to_sass || isDebugDumpEnabled(DebugDumpOption::Ptx)) {
  compiled_kernel->ptx = compileNvrtcProgramToPtx(program);
  if (isDebugDumpEnabled(DebugDumpOption::Ptx)) {
    compiled_kernel->ptx_filename =
        dumpCompiledCodeToFile(compiled_kernel->ptx, func_name, ".ptx");

@naoyam naoyam merged commit 075f97f into main Jan 29, 2025
50 of 58 checks passed
@naoyam naoyam deleted the ptx_cubin_filenames branch January 29, 2025 17:30
jacobhinkle added a commit that referenced this pull request Jan 31, 2025
In #3788, the filename pattern `__tmp_kernel*.cu` was updated to
`__tmp_*.cu` in `compare_codegen.sh`. That fix was incomplete, because
`run_command.sh` also needs to move the generated files. This fix
implements that change. I think this should finally fix codediff.
jacobhinkle added a commit that referenced this pull request Feb 4, 2025
In #3788, the filename pattern `__tmp_kernel*.cu` was updated to
`__tmp_*.cu` in `compare_codegen.sh`. That fix was incomplete, because
`run_command.sh` also needs to move the generated files. This fix
implements that change. I think this should finally fix codediff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants