[hip] Fix HIP version parsing.

[hliao]

Summary

• 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.

[hliao]

PING

[tra]

clang/lib/Driver/ToolChains/AMDGPU.cpp

91–93

A comment describing expected input format would be helpful here.

91–93

A comment that it returns true on failure to parse would be helpful.

98–106

Part.trim().split() (or, maybe, rtrim()if you only care about EOL) would save you trimming in each individual case.

[yaxunl]

Pls address Artem's comments. LGTM otherwise.

[hliao]

clang/lib/Driver/ToolChains/AMDGPU.cpp

98–106

sure, as that file is auto-gen so far, we could simplify the unnecessary parsing so far. The EOL is the only issue so far.

[hliao]

Revise following reviewers' comments.

[tra]

LGTM overall.

clang/lib/Driver/ToolChains/AMDGPU.cpp

92

It still does not desribe what we expect to see in that file.
E.g.:

HIP_VERSION_MAJOR=1
HIP_VERSION_MINOR=2
HIP_VERSION_PATCH=3

100

Perhaps we could use StringSwitch here:

int &Value = llvm::StringSwitch<int&>(Splits.first)
    .Case("HIP_VERSION_MAJOR", Major)
    .Case("HIP_VERSION_MINOR", Minor)
    .Case("HIP_VERSION_PATCH", VersionPatch)
if (Splits.second.getAsInteger(0, Value))
      return true;

clang/lib/Driver/ToolChains/AMDGPU.cpp

if (BaseName == "ocml") {
       llvm::Twine GfxName = Twine("gfx") + IsaVersionNumber;
       SmallString<8> Tmp;
       LibDeviceMap.insert(
         std::make_pair(GfxName.toStringRef(Tmp), FilePath.str()));
     }
   }
 }
 
-void RocmInstallationDetector::ParseHIPVersionFile(llvm::StringRef V) {
+// Parse and extract version numbers from `.hipVersion`. Return `true` if
+// the parsing fails.
+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")
+    auto Splits = Part.rtrim().split('=');
+    if (Splits.first == "HIP_VERSION_MAJOR") {
+      if (Splits.second.getAsInteger(0, Major))
+        return true;
+    } else if (Splits.first == "HIP_VERSION_MINOR") {
+      if (Splits.second.getAsInteger(0, Minor))
+        return true;
+    } else if (Splits.first == "HIP_VERSION_PATCH")
       VersionPatch = Splits.second.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.
 SmallVector<RocmInstallationDetector::Candidate, 4>
 RocmInstallationDetector::getInstallationPathCandidates() {
   SmallVector<Candidate, 4> Candidates;
   if (!RocmPathArg.empty()) {
     Candidates.emplace_back(RocmPathArg.str());
for (const auto &Candidate : Candidates) {
     llvm::sys::path::append(LibPath, "lib");
 
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> VersionFile =
         FS.getBufferForFile(BinPath + "/.hipVersion");
     if (!VersionFile && Candidate.StrictChecking)
       continue;
 
     if (HIPVersionArg.empty() && VersionFile)
-      ParseHIPVersionFile((*VersionFile)->getBuffer());
+      if (parseHIPVersionFile((*VersionFile)->getBuffer()))
+        continue;
 
     HasHIPRuntime = true;
     return;
   }
   HasHIPRuntime = false;
 }
 
 void RocmInstallationDetector::print(raw_ostream &OS) const {

clang/lib/Driver/ToolChains/ROCm.h

private:
   bool allGenericLibsValid() const {
     return !OCML.empty() && !OCKL.empty() && !OpenCL.empty() && !HIP.empty() &&
            WavefrontSize64.isValid() && FiniteOnly.isValid() &&
            UnsafeMath.isValid() && DenormalsAreZero.isValid() &&
            CorrectlyRoundedSqrt.isValid();
   }
 
   void scanLibDevicePath(llvm::StringRef Path);
-  void ParseHIPVersionFile(llvm::StringRef V);
+  bool parseHIPVersionFile(llvm::StringRef V);
   SmallVector<Candidate, 4> getInstallationPathCandidates();
 
 public:
   RocmInstallationDetector(const Driver &D, const llvm::Triple &HostTriple,
                            const llvm::opt::ArgList &Args,
                            bool DetectHIPRuntime = true,
                            bool DetectDeviceLib = false);
 

clang/test/Driver/Inputs/rocm/bin/.hipVersion

 # 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