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