Skip to content

Conversation

Artem-B
Copy link
Member

@Artem-B Artem-B commented Oct 30, 2023

Code model has no impact on NVPTX as we do not produce any object files, but we need to avoid erroring out on the -mcmodel argument passed to the top-level compilation and propagated to all sub-compilations.

Code model has no impact on NVPTX as we do not produce any object files, but we
need to avoid erroring out on the -mcmodel argument passed to the top-level
compilation and propagated to all sub-compilations.
@Artem-B Artem-B requested review from MaskRay and aeubanks October 30, 2023 22:36
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 30, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 30, 2023

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Artem Belevich (Artem-B)

Changes

Code model has no impact on NVPTX as we do not produce any object files, but we need to avoid erroring out on the -mcmodel argument passed to the top-level compilation and propagated to all sub-compilations.


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

1 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+4)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 43a92adbef64ba8..fb90fcd033b1ac3 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5743,6 +5743,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     } else if (Triple.getArch() == llvm::Triple::x86_64) {
       Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"},
                               CM);
+    } else if (Triple.isNVPTX()) {
+      // NVPTX does not care about the code model and will accept whatever works
+      // for the host.
+      Ok = true;
     }
     if (Ok) {
       CmdArgs.push_back(Args.MakeArgString("-mcmodel=" + CM));

@MaskRay
Copy link
Member

MaskRay commented Oct 30, 2023

The current filtering mechanism is gross. We have code that does the following error checking for a lot of options and there are more and more of them. It is nearly infeasible to enumerate them.

if (!Triple.isXXX()) { err_unsupported_opt_for_target  } else handle

When such an option is used in clang++ --cuda-path=/usr/local/cuda -x cu /dev/null -c -nocudainc -nocudalib $option, we will get an error for NVPTX.

-mcmodel= turned out to be an issue just because we add this option globally.

https://reviews.llvm.org/D105226 introduced a special case to exclude NVPTX for -fbasic-block-sections=, so this has a precedent, but we should figure out a way to handle the target-specific options in a more elegant way.

I do not know whether we should use something similar to -fembed-bitcode's list.

@aeubanks
Copy link
Contributor

aeubanks commented Jan 18, 2024

should this have had a test? I'm trying to do the same thing for -mlarge-data-threshold and am trying to find an appropriate place to add a test

edit: actually #77958 seems to have worked around this

MaskRay added a commit that referenced this pull request Jan 24, 2024
Fix missing test coverage after #70740 #70760

When compiling for CUDA/HIP, the driver creates a cc1 job to compile for
amdgcn/nvptx triple using most options.
Certain target-specific options should be ignored, not lead to an error
(`err_drv_unsupported_opt_for_target`).
aeubanks added a commit to aeubanks/llvm-project that referenced this pull request Mar 20, 2024
The code model doesn't affect the sub-compilation, so don't check it.

Followup to llvm#70740.
aeubanks added a commit that referenced this pull request Mar 21, 2024
The code model doesn't affect the sub-compilation, so don't check it.

Followup to #70740.
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
The code model doesn't affect the sub-compilation, so don't check it.

Followup to llvm#70740.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category cuda
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants