[PATCH] D154077: [HIP] Fix version detection for old HIP-PATH
Yaxun Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 29 11:28:12 PDT 2023
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
More information about the cfe-commits
mailing list