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

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added reviewers: yaxunl, tra.
Herald added subscribers: kerbowa, nhaehnle, jvesely.
hliao requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Need trimming before parsing major or minor version numbers. This's required due to the different line ending on Windows.
- In addition, the integer conversion may fail due to invalid char. Return that parsing function return `true` when the parsing fails.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93587

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/ROCm.h
  clang/test/Driver/Inputs/rocm/bin/.hipVersion


Index: clang/test/Driver/Inputs/rocm/bin/.hipVersion
===================================================================
--- clang/test/Driver/Inputs/rocm/bin/.hipVersion
+++ clang/test/Driver/Inputs/rocm/bin/.hipVersion
@@ -1,4 +1,6 @@
 # Auto-generated by cmake
-HIP_VERSION_MAJOR=3
+# NOTE: The trailing whitespace is added on purpose to verify that these
+# whitespaces are trimmed before paring.
+HIP_VERSION_MAJOR=3 
 HIP_VERSION_MINOR=6
 HIP_VERSION_PATCH=20214-a2917cd
Index: clang/lib/Driver/ToolChains/ROCm.h
===================================================================
--- clang/lib/Driver/ToolChains/ROCm.h
+++ clang/lib/Driver/ToolChains/ROCm.h
@@ -103,7 +103,7 @@
   }
 
   void scanLibDevicePath(llvm::StringRef Path);
-  void ParseHIPVersionFile(llvm::StringRef V);
+  bool parseHIPVersionFile(llvm::StringRef V);
   SmallVector<Candidate, 4> getInstallationPathCandidates();
 
 public:
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===================================================================
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -88,23 +88,28 @@
   }
 }
 
-void RocmInstallationDetector::ParseHIPVersionFile(llvm::StringRef V) {
+bool RocmInstallationDetector::parseHIPVersionFile(llvm::StringRef V) {
   SmallVector<StringRef, 4> VersionParts;
   V.split(VersionParts, '\n');
-  unsigned Major;
-  unsigned Minor;
+  unsigned Major = ~0U;
+  unsigned Minor = ~0U;
   for (auto Part : VersionParts) {
     auto Splits = Part.split('=');
-    if (Splits.first == "HIP_VERSION_MAJOR")
-      Splits.second.getAsInteger(0, Major);
-    else if (Splits.first == "HIP_VERSION_MINOR")
-      Splits.second.getAsInteger(0, Minor);
-    else if (Splits.first == "HIP_VERSION_PATCH")
-      VersionPatch = Splits.second.str();
+    if (Splits.first == "HIP_VERSION_MAJOR") {
+      if (Splits.second.trim().getAsInteger(0, Major))
+        return true;
+    } else if (Splits.first == "HIP_VERSION_MINOR") {
+      if (Splits.second.trim().getAsInteger(0, Minor))
+        return true;
+    } else if (Splits.first == "HIP_VERSION_PATCH")
+      VersionPatch = Splits.second.trim().str();
   }
+  if (Major == ~0U || Minor == ~0U)
+    return true;
   VersionMajorMinor = llvm::VersionTuple(Major, Minor);
   DetectedVersion =
       (Twine(Major) + "." + Twine(Minor) + "." + VersionPatch).str();
+  return false;
 }
 
 // For candidate specified by --rocm-path we do not do strict check.
@@ -290,7 +295,8 @@
       continue;
 
     if (HIPVersionArg.empty() && VersionFile)
-      ParseHIPVersionFile((*VersionFile)->getBuffer());
+      if (parseHIPVersionFile((*VersionFile)->getBuffer()))
+        continue;
 
     HasHIPRuntime = true;
     return;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D93587.312951.patch
Type: text/x-patch
Size: 2730 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201219/60b79491/attachment.bin>