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

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 6 11:23:19 PST 2021


tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM overall.



================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:92
+// Parse and extract version numbers from `.hipVersion`. Return `true` if
+// the parsing fails.
+bool RocmInstallationDetector::parseHIPVersionFile(llvm::StringRef V) {
----------------
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
```


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:100
+    auto Splits = Part.rtrim().split('=');
+    if (Splits.first == "HIP_VERSION_MAJOR") {
+      if (Splits.second.getAsInteger(0, Major))
----------------
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;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93587



More information about the cfe-commits mailing list