[PATCH] D127142: [HIP] Link with clang_rt.builtins

[Yaxun Liu via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:485
                   "amdhip64.lib"});
+  CmdArgs.push_back(Args.MakeArgString("clang_rt.builtins-" +
+                                       getTriple().getArchName() + ".lib"));
----------------
MaskRay wrote:
> Note that the path is different with LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on. See D107799
will use getCompilerRT to get the compiler-rt builtin lib name so that it is always correct


================
Comment at: clang/test/Driver/hip-runtime-libs-msvc.hip:10
 
+// CHECK: "-libpath:{{.*lib.*windows}}"
 // CHECK: "-libpath:{{.*Inputs.*rocm.*lib}}" "amdhip64.lib"
----------------
tra wrote:
> What are we matching here? A more verbose pattern or some comments would be helpful.
this is for checking lib path for compiler lib.

As we will use getCompilerRT to get the complete path of compiler-rt builtin lib, this part will be changed. We will use a clang option to print the compiler-rt lib path and use it as a reference to check compiler-rt lib used by HIP.


================
Comment at: clang/test/Driver/hip-runtime-libs-msvc.hip:12
 // CHECK: "-libpath:{{.*Inputs.*rocm.*lib}}" "amdhip64.lib"
+// CHECK: "clang_rt.builtins-x86_64.lib"
----------------
tra wrote:
> `CHECK-SAME`?
will do


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

https://reviews.llvm.org/D127142