feat: initial support for Clang HIP

[GZGavinZhao]

Fixes #2044.

Still needs to distinguished between different ROCm versions, due to implicit dependencies that may be introduced by $HIP_DEVICE_LIB_PATH/*.bc and header files located at $HIP_PATH/include/hip/**/*.h No need to concern about header files, but for the bitcode libraries I'm still thinking.

I currently have an experimental support for detecting and adding the bitcode libraries into extra_hash_files. If the implementation is undesired, then my idea is to just ignore bitcode libraries and explicitly mention in the docs to add the bitcode libraries to SCCACHE_EXTRA_FILES if the user is worried about erroneous cache hits.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 12.90323% with 243 lines in your changes are missing coverage. Please review.

Project coverage is 43.09%. Comparing base (0cc0c62) to head (b84eeaa).
Report is 7 commits behind head on main.

Files	Patch %	Lines
tests/system.rs	5.26%	176 Missing and 4 partials ⚠️
src/compiler/c.rs	5.55%	34 Missing ⚠️
src/compiler/clang.rs	48.93%	1 Missing and 23 partials ⚠️
src/compiler/gcc.rs	0.00%	2 Missing and 1 partial ⚠️
src/compiler/compiler.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2045       +/-   ##
===========================================
+ Coverage   30.91%   43.09%   +12.18%     
===========================================
  Files          53       53               
  Lines       20112    20576      +464     
  Branches     9755     9930      +175     
===========================================
+ Hits         6217     8868     +2651     
+ Misses       7922     7893       -29     
+ Partials     5973     3815     -2158     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[GZGavinZhao]

An implementation questions:

As mentioned earlier, I want to include changes in the $HIP_DEVICE_LIB_PATH/*.bc bitcode files into account. Currently my way of doing this is to add them to extra_hash_files.

However, this presents a problem: the location of those files can be determined by both the environment variable and command-line arguments, but there doesn't seem to be a function where I have access to both of them. Ideally I want to add those files to extra_hash_files inside parse_arguments. However, I don't have access to env_vars inside that function.

While I do have access to env_vars in generate_hash_key, at this point the arguments have already been parsed into parsed_args and I have to iterate through the entire parsed_args.common_args to check. It also just feels wrong that those bitcode are not included inside extra_hash_files and I have to manually take them into account when hashing.

Would it be accepted if parse_arguments (defined in src/compiler/gcc.rs) can take another parameter env_vars: Vec<(OsString, OsString)>? I'm also open to other ways to achieve the same result.

[GZGavinZhao]

Hi @sylvestre or @Xuanwo, could one of you provide some advice on how my problem above might be solved?

[sylvestre]

HIP_DEVICE_LIB_PATH and HIP_PATH are standard and expected env variables in this ecosystem? Sorry, i don't know much here

[GZGavinZhao]

Sorry, please ignore my previous comment. I figured the current implementation is good enough.

Feature-wise this is complete. I just need to write up docs and add E2E tests in tests/.

[GZGavinZhao]

I'll leave it Vec<String> instead of Vec<OsString> for now because I had a little trouble figuring out how to concat two OsString.

[GZGavinZhao]

I still need to write up docs, but I would appreciate feedback on whether you guys are okay with the logic to find and hash bitcode libraries (these changes are in src/compiler/c.rs).

[sylvestre]

please avoid "let mut" and the following
move the work into a function

[GZGavinZhao]

Fixed, please check if the current version is acceptable.

[sylvestre]

please move this into a function

[sylvestre]

actually, would it be possible to move all this block into a function?

[GZGavinZhao]

Yes, I'm thinking of something like fn search_hip_device_libs(args: &ParsedArguments, env_vars: &[(OsString, OsString)]) -> Vec<PathBuf>. Where shall I put this function, in src/util.rs?

[sylvestre]

This file is fine

[sylvestre]

Thanks!