[PATCH] D93587: [hip] Fix HIP version parsing.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:91
 
-void RocmInstallationDetector::ParseHIPVersionFile(llvm::StringRef V) {
+bool RocmInstallationDetector::parseHIPVersionFile(llvm::StringRef V) {
   SmallVector<StringRef, 4> VersionParts;
----------------
A comment describing expected input format would be helpful here.


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:91
 
-void RocmInstallationDetector::ParseHIPVersionFile(llvm::StringRef V) {
+bool RocmInstallationDetector::parseHIPVersionFile(llvm::StringRef V) {
   SmallVector<StringRef, 4> VersionParts;
----------------
tra wrote:
> A comment describing expected input format would be helpful here.
A comment that it returns `true` on failure to parse would be helpful.


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:97
   for (auto Part : VersionParts) {
     auto Splits = Part.split('=');
+    if (Splits.first == "HIP_VERSION_MAJOR") {
----------------
`Part.trim().split()` (or, maybe, `rtrim()`if you only care about EOL) would save you trimming in each individual case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93587