Skip to content

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Oct 11, 2024

Summary:
The C library for GPUs provides the ability to target regular C/C++
programs by providing the C library and a file containing kernels that
call the main function. This is mostly used for unit tests, this patch
provides a quick way to add them without needing to know the paths. I
currently do this explicitly, but according to the libc++ contributors
we don't want to need to specify these paths manually. See the
discussion in #104515.

I just default to lib/ if the target-specific one isn't found because
the linker will handle giving a reasonable error message if it's not
found. Basically the use-case looks like this.

$ clang test.c --target=amdgcn-amd-amdhsa -mcpu=native -startfiles -stdlib
$ amdhsa-loader a.out
PASS!

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AMDGPU clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 11, 2024

@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Joseph Huber (jhuber6)

Changes

Summary:
The C library for GPUs provides the ability to target regular C/C++
programs by providing the C library and a file containing kernels that
call the main function. This is mostly used for unit tests, this patch
provides a quick way to add them without needing to know the paths. I
currently do this explicitly, but according to the libc++ contributors
we don't want to need to specify these paths manually. See the
discussion in #104515.

I just default to lib/ if the target-specific one isn't found because
the linker will handle giving a reasonable error message if it's not
found. Basically the use-case looks like this.

$ clang test.c --target=amdgcn-amd-amdhsa -mcpu=native -gpustartfiles
$ amdhsa-loader a.out
PASS!

Full diff: https://github.com/llvm/llvm-project/pull/112025.diff

4 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+3)
  • (modified) clang/lib/Driver/ToolChains/AMDGPU.cpp (+9)
  • (modified) clang/lib/Driver/ToolChains/Cuda.cpp (+9)
  • (added) clang/test/Driver/gpustartfiles.c (+7)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index d306c751505e98..b7f7a7cb47f754 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1316,6 +1316,9 @@ defm offload_via_llvm : BoolFOption<"offload-via-llvm",
   BothFlags<[], [ClangOption], " LLVM/Offload as portable offloading runtime.">>;
 }
 
+def gpustartfiles : Flag<["-"], "gpustartfiles">, Group<Link_Group>,
+  HelpText<"Link the GPU C startup utilities automatically, used for testing.">;
+
 // CUDA options
 let Group = cuda_Group in {
 def cuda_include_ptx_EQ : Joined<["--"], "cuda-include-ptx=">,
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 2c85d21ebd738c..9a648be4ea3915 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -648,6 +648,15 @@ void amdgpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
         Args.MakeArgString("-plugin-opt=-mattr=" + llvm::join(Features, ",")));
   }
 
+  if (Args.hasArg(options::OPT_gpustartfiles)) {
+    auto IncludePath = getToolChain().getStdlibPath();
+    if (!IncludePath)
+      IncludePath = "/lib";
+    SmallString<128> P(*IncludePath);
+    llvm::sys::path::append(P, "crt1.o");
+    CmdArgs.append({"-lc", "-lm", Args.MakeArgString(P)});
+  }
+
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
   C.addCommand(std::make_unique<Command>(
diff --git a/clang/lib/Driver/ToolChains/Cuda.cpp b/clang/lib/Driver/ToolChains/Cuda.cpp
index 7a70cf1c5694fd..ff96ff989db630 100644
--- a/clang/lib/Driver/ToolChains/Cuda.cpp
+++ b/clang/lib/Driver/ToolChains/Cuda.cpp
@@ -641,6 +641,15 @@ void NVPTX::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   llvm::sys::path::append(DefaultLibPath, CLANG_INSTALL_LIBDIR_BASENAME);
   CmdArgs.push_back(Args.MakeArgString(Twine("-L") + DefaultLibPath));
 
+  if (Args.hasArg(options::OPT_gpustartfiles)) {
+    auto IncludePath = getToolChain().getStdlibPath();
+    if (!IncludePath)
+      IncludePath = "/lib";
+    SmallString<128> P(*IncludePath);
+    llvm::sys::path::append(P, "crt1.o");
+    CmdArgs.append({"-lc", "-lm", Args.MakeArgString(P)});
+  }
+
   C.addCommand(std::make_unique<Command>(
       JA, *this,
       ResponseFileSupport{ResponseFileSupport::RF_Full, llvm::sys::WEM_UTF8,
diff --git a/clang/test/Driver/gpustartfiles.c b/clang/test/Driver/gpustartfiles.c
new file mode 100644
index 00000000000000..c1b7a6fa922df4
--- /dev/null
+++ b/clang/test/Driver/gpustartfiles.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target nvptx64-nvidia-cuda -march=sm_61 -gpustartfiles \
+// RUN:   -nogpulib -nogpuinc -### %s 2>&1 | FileCheck -check-prefix=NVPTX %s
+// NVPTX: clang-nvlink-wrapper{{.*}}"-lc" "-lm" "{{.*}}crt1.o"
+//
+// RUN: %clang -target amdgcn-amd-amdhsa -march=gfx90a -gpustartfiles \
+// RUN:   -nogpulib -nogpuinc -### %s 2>&1 | FileCheck -check-prefix=AMDGPU %s
+// AMDGPU: ld.lld{{.*}}"-lc" "-lm" "{{.*}}crt1.o"

@jhuber6
Copy link
Contributor Author

jhuber6 commented Oct 16, 2024

ping

Summary:
The C library for GPUs provides the ability to target regular C/C++
programs by providing the C library and a file containing kernels that
call the `main` function. This is mostly used for unit tests, this patch
provides a quick way to add them without needing to know the paths. I
currently do this explicitly, but according to the libc++ contributors
we don't want to need to specify these paths manually. See the
discussion in llvm#104515.

I just default to `lib/` if the target-specific one isn't found because
the linker will handle giving a reasonable error message if it's not
found. Basically the use-case looks like this.

```console
$ clang test.c --target=amdgcn-amd-amdhsa -mcpu=native -gpustartfiles
$ amdhsa-loader a.out
PASS!
```
@ldionne
Copy link
Member

ldionne commented Oct 18, 2024

Can you explain again why the compiler isn't providing the C library and the start files implicitly by default, just like it does for non-GPU code?

@jhuber6
Copy link
Contributor Author

jhuber6 commented Oct 18, 2024

Can you explain again why the compiler isn't providing the C library and the start files implicitly by default, just like it does for non-GPU code?

Because the GPU is not a target that wants to provide a C library and start files by default. This is an edge case where I make the GPU look like a normal target for unit testing purposes. If you use a GPU this way, you're basically turning your 1000$+ card into a 10$ Raspberry Pi, it's not something people want by default. Kernels cannot be optimized out, and currently 0.001% of GPU applications in the wild define a main function on the device side so it would cause a linker error.

@jhuber6
Copy link
Contributor Author

jhuber6 commented Oct 25, 2024

ping

@ldionne
Copy link
Member

ldionne commented Oct 25, 2024

Just to clarify, I'm mostly neutral on this. A Clang reviewer should chime in and evaluate the change for what it is. It does simplify the setup of the libc++ test suite for GPU, but it's not required since we have other options to set things up.

if (Args.hasArg(options::OPT_stdlib))
CmdArgs.append({"-lc", "-lm"});
if (Args.hasArg(options::OPT_startfiles)) {
auto IncludePath = getToolChain().getStdlibPath();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no auto

@jhuber6 jhuber6 merged commit d4c4180 into llvm:main Oct 28, 2024
8 checks passed
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
Summary:
The C library for GPUs provides the ability to target regular C/C++
programs by providing the C library and a file containing kernels that
call the `main` function. This is mostly used for unit tests, this patch
provides a quick way to add them without needing to know the paths. I
currently do this explicitly, but according to the libc++ contributors
we don't want to need to specify these paths manually. See the
discussion in llvm#104515.

I just default to `lib/` if the target-specific one isn't found because
the linker will handle giving a reasonable error message if it's not
found. Basically the use-case looks like this.

```console
$ clang test.c --target=amdgcn-amd-amdhsa -mcpu=native -startfiles -stdlib
$ amdhsa-loader a.out
PASS!
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants