[PATCH] D96835: [HIP] Support device sanitizer

[Artem Belevich via Phabricator via cfe-commits]

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

Nice. LGTM with few minor nits.



================
Comment at: clang/lib/Driver/ToolChain.cpp:1185
+ToolChain::getHIPDeviceLibs(const ArgList &DriverArgs) const {
+  return llvm::SmallVector<std::string, 12>();
+}
----------------
Nit: It could be just `return {}`;


================
Comment at: clang/lib/Driver/ToolChains/HIP.cpp:376
+  if (DriverArgs.hasArg(options::OPT_nogpulib))
+    return BCLibs;
+  ArgStringList LibraryPaths;
----------------
I'd explicitly return `{}` to make it obvious that it's an empty list and move `BClibs` down to where we use it first.


================
Comment at: clang/test/Driver/hip-sanitize-options.hip:25
+// RUN:   -fsanitize=address -fgpu-sanitize \
+// RUN:   -nogpuinc --rocm-path=%S/Inputs/rocm-invalid \
+// RUN:   %s 2>&1 | FileCheck -check-prefixes=FAIL %s
----------------
I'd add a comment describing what exactly is invalid about `rocm-invalid`. Maybe add it to a README in the directory.

Or, If the purpose is very specific, rename the directory to reflect it. E.g. if it just to test handling of installations w/o asan bitcode, call it `rocm-noasan`.


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

https://reviews.llvm.org/D96835