Page MenuHomePhabricator

[HIP] Fix default output file for -E
ClosedPublic

Authored by yaxunl on Fri, Oct 2, 5:28 AM.

Details

Summary

By convention the default output file for -E is "-" (stdout).
This is expected by tools like ccache, which uses output
of -E to determine if a file and its dependence has changed.

Currently clang does not use stdout as default output file for -E
for HIP, which causes ccache not working.

This patch fixes that.

Diff Detail

Event Timeline

yaxunl requested review of this revision.Fri, Oct 2, 5:28 AM
yaxunl created this revision.
tra accepted this revision.Fri, Oct 2, 12:10 PM
tra added inline comments.
clang/test/Driver/hip-output-file-name.hip
13–15

What does it mean for -E to be used when we compile for host and multiple devices. I believe for CUDA clang errors out unless there's only one sub-compilation. What does HIP do when it's run with -E -o - ?

Looks like CUDA (and, maybe HIP, too) has a bug there. -E will run preprocess on all subcompilations. -E -o - will error out claiming that you can't use -o for multiple output files, even though -### shows the same -o - in all subcompilations in both cases.

This revision is now accepted and ready to land.Fri, Oct 2, 12:10 PM
yaxunl added inline comments.Fri, Oct 2, 1:18 PM
clang/test/Driver/hip-output-file-name.hip
13–15

HIP will emit a clang-offload-bundler file in textual format. Clang is able to consume this file by unbundling it. I think ccache uses it to generate a hash to check if a file and its dependent header files have changed. Since this bundled file is in text format, ccache is able to use it to generate a hash which covers both host dependence and device dependence.

tra added a comment.Fri, Oct 2, 1:49 PM

LGTM.

clang/test/Driver/hip-output-file-name.hip
13–15

That would explain why I sometimes get a screenfull of noise when I do something like that with HIP. :-/
My expectation of -E [-o -] is that I get the text of preprocessed source. In this regard, CUDA's concatenate all preprocessd outputs is a bit better than HIPs' "here's a binary blob with preprocessed output inside".

This is a bit of a mess that's beyond the scope of this CL and should be dealt with separately.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSun, Oct 4, 7:09 PM