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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
