[PATCH] D154077: [HIP] Fix version detection for old HIP-PATH

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:471
          {std::string(SharePath) + "/hip/version",
+          std::string(ParentSharePath) + "/hip/version",
           std::string(BinPath) + "/.hipVersion"}) {
----------------
tra wrote:
> We seem to be rather inconsistent about how we handle paths.
> 
> Above, we use `llvm::sys::path::append`, but here we revert to just appending a path as a string. 
> 
> I think we should be using llvm::sys::path API consistently. It's unfortunate that the API does not provide a string-returning function to append elements.
> ```
> auto Append = [](SmallVectorImpl<char> &path, const Twine &a,
>                                          const Twine &b = "",
>                                          const Twine &c = "",
>                                          const Twine &d = "") {
>     SmallVectorImpl<char> newpath = path;
>     llvm::sys::path::append(newpath, a,b,c,d);
>     return newpath; 
> }
> for (const auto &VersionFilePath :
>          {Append(SharePath, "hip", "version"),
>           Append(ParentSharePath,  "hip", "version"),
>           Append(BinPath, ".hipVersion")}) {
>   ...
> }
> ```
> 
will do. thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154077/new/

https://reviews.llvm.org/D154077