[PATCH] D102975: [HIP] Check compatibility of -fgpu-sanitize with offload arch

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.
Herald added a subscriber: foad.


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.h:114-115
+  /// specified and valid.
+  std::tuple<Optional<std::string>, Optional<std::string>,
+             Optional<llvm::StringMap<bool>>>
+  getParsedTargetID(const llvm::opt::ArgList &DriverArgs) const;
----------------
I'd use a struct instead. It makes things more readable in all the other other places where we don't have the exact return type spelled out.


================
Comment at: clang/lib/Driver/ToolChains/HIP.cpp:473
+
+  auto &FeatureMap = std::get<2>(ParsedTargetID).getValue();
+  // Sanitizer is not supported with xnack-.
----------------
What if FeatureMap is `None`?
Considering that it's derived from user input, this should be properly checked and diagnosed, if we didn't get the feature map.
At the very least there should be an assert, if we ensure somewhere else that we always get a valid FeatureMap here.


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

https://reviews.llvm.org/D102975