saiislam added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:617
+  // Find nvlink and pass it as "--nvlink-command=" argument of 
clang-nvlink-wrapper.
+  auto NvlinkBin = getToolChain().GetProgramPath("nvlink");
+  const char *NvlinkPath =
----------------
Meinersbur wrote:
> [style] [[ 
> https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
>  | LLVM's coding standard does not use almost-always-auto ]].
> 
> It's not immediately obvious here, does `GetProgramPath` look into the 
> BinPath detected by CudaInstallationDetector? I applied the patch locally to 
> http://meinersbur.de:8011/#/builders/1 and it actually does work.
Yes, you are right. `CudaToolChain` constructor initializes binary paths 
obtained from `CudaInstallationDetector` which is used by this call.


================
Comment at: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp:50
+
+static cl::opt<std::string> NvlinkUserPath("nvlink-command",
+                                           cl::desc("path of nvlink binary"),
----------------
tra wrote:
> Nit. Clang already has `--ptxas-path=` option. It may be worth using `-path` 
> suffix here for consistency, too.
changed it to --nvlink-path


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111488

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to