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

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM in general with a minor suggestion.



================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:471
          {std::string(SharePath) + "/hip/version",
+          std::string(ParentSharePath) + "/hip/version",
           std::string(BinPath) + "/.hipVersion"}) {
----------------
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")}) {
  ...
}
```



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

https://reviews.llvm.org/D154077