[PATCH] D98867: [HIP] Fix ROCm detection

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 2 inline comments as done.
yaxunl added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:267-268
+    // rocm-{major}.{minor}.{subMinor}[-{build}]
+    auto Loc = StringRef(VerStr).rfind('_');
+    if (Loc != StringRef::npos)
+      VerStr[Loc] = '.';
----------------
tra wrote:
> You don't need `StringRef` here. `VerStr.rfind()` will do.
will fix


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:269
+    if (Loc != StringRef::npos)
+      VerStr[Loc] = '.';
+    V.tryParse(VerStr);
----------------
tra wrote:
> This only deals with a single `.` in the version. It's likely that we will have to deal with a patch release at some point, so you may want to replace all instances.
> 
The ROCm directory follows rocm-{major}.{minor}.{subMinor}[-{build}] format, where major, minor and path (subMinor) are delimited by '.', only the optional build number is delimited by '-'. I have a comment about that at line 266.


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

https://reviews.llvm.org/D98867