[PATCH] D96835: [HIP] Support device sanitizer

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 3 inline comments as done.
yaxunl added inline comments.


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


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


================
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
----------------
tra wrote:
> 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`.
I will add a README to the directory explaining what is invalid, since we may reuse this directory for other invalid usages later.


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

https://reviews.llvm.org/D96835