This is an archive of the discontinued LLVM Phabricator instance.

[CUDA] Fix output name being replaced in device-only mode
ClosedPublic

Authored by jhuber6 on Aug 5 2022, 11:45 AM.

Details

Summary

When performing device only compilation, there was an issue where
cubin outputs were being renamed to cubin despite the user's name.
This is required in a normal compilation flow as the Nvidia tools only
understand specific filenames instead of checking magic bytes for some
unknown reason. We do not want to perform this transformation when the
user is performing device only compilation.

Diff Detail

Event Timeline

jhuber6 created this revision.Aug 5 2022, 11:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2022, 11:45 AM
Herald added subscribers: mattd, yaxunl. · View Herald Transcript
jhuber6 requested review of this revision.Aug 5 2022, 11:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2022, 11:45 AM
tra added inline comments.Aug 5 2022, 1:51 PM
clang/test/Driver/cuda-bindings.cu
148

Ideally we want to test that the output file actually survives the compilation. It's traky to do for in-tree tests as we don't have ptxas. However, we should be able to add a script which would just touch whatever is specified with -o argument and ignore the rest and then actually do run clang with --ptxas-path=/path/to/dummy-ptxas and then verify file existence.

tra accepted this revision.Aug 5 2022, 1:55 PM

I can confirm that this patch fixes the issue.

This revision is now accepted and ready to land.Aug 5 2022, 1:55 PM
jhuber6 added inline comments.Aug 5 2022, 1:56 PM
clang/test/Driver/cuda-bindings.cu
148

Yeah, I wasn't sure if there was a good way. I don't have any CUDA machines to test so I just went with the simplest version.

tra added inline comments.Aug 5 2022, 3:23 PM
clang/test/Driver/cuda-bindings.cu
148

This is a more general problem, not specific to this test -- we have other tests where we assume that if something is printed by cc -###, then it must be real.
I guess it's more of a TODO note for a potential improvement.

This patch is good to go as is.

jhuber6 added inline comments.Aug 5 2022, 3:24 PM
clang/test/Driver/cuda-bindings.cu
148

Okay, I'll land it soon.

This revision was automatically updated to reflect the committed changes.