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

[Artem Belevich via Phabricator via cfe-commits]

tra added a reviewer: MaskRay.
tra added a comment.
Herald added a subscriber: StephenFan.

Looks OK syntax-wise.  Library path should probably be fixed, though it appears to be a somewhat separate  issue and could be done in its own patch, if the required change is not trivial.



================
Comment at: clang/lib/Driver/ToolChains/Linux.cpp:684-690
   CmdArgs.append(
       {Args.MakeArgString(StringRef("-L") + RocmInstallation.getLibPath()),
        "-rpath", Args.MakeArgString(RocmInstallation.getLibPath())});
 
   CmdArgs.push_back("-lamdhip64");
+  CmdArgs.push_back(
+      Args.MakeArgString("-lclang_rt.builtins-" + getTriple().getArchName()));
----------------
Nit: Collapse all of these into a single `append()`


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


================
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"
----------------
`CHECK-SAME`?


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

https://reviews.llvm.org/D127142