Skip to content

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Dec 15, 2024

LLVM's WebAssembly backend supports the noredzone function attribute, but this support was not exposed in Clang. This just mirrors what is done for AArch64, PowerPC, and x86, minus the Apple-specific checks that are irrelevant here.

LLVM's WebAssembly backend supports the noredzone function attribute, but this
support was not exposed in Clang.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Dec 15, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 15, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Alex Rønne Petersen (alexrp)

Changes

LLVM's WebAssembly backend supports the noredzone function attribute, but this support was not exposed in Clang.

Note: I don't have commit access.


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

1 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+3)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 7ef55a33547c50..21117cd71b651e 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2404,6 +2404,9 @@ void Clang::AddWebAssemblyTargetArgs(const ArgList &Args,
   if (!Args.hasArg(options::OPT_fvisibility_EQ,
                    options::OPT_fvisibility_ms_compat))
     CmdArgs.push_back("-fvisibility=hidden");
+
+  if (!Args.hasFlag(options::OPT_mred_zone, options::OPT_mno_red_zone, true))
+    CmdArgs.push_back("-disable-red-zone");
 }
 
 void Clang::AddVETargetArgs(const ArgList &Args, ArgStringList &CmdArgs) const {

@alexrp
Copy link
Member Author

alexrp commented Mar 9, 2025

@sunfishcode would you mind reviewing this one? (I've pinged a few times here but I'm not sure any of the relevant WASM folks are actually seeing it due to the labels...)

@sunfishcode
Copy link
Member

Is there a use case that needs to disable the red zone globally on Wasm? My understanding is that -mno-red-zone exists for compatibility reasons that don't apply to Wasm. I'm hesitant to add flags adding implicit ABI variants in the absence of use cases. At least with the function attribute it's explicit in the code.

@alexrp
Copy link
Member Author

alexrp commented Mar 10, 2025

I don't have any particular use case in mind. I was just going over the targets for which we allow -m(no-)red-zone in Zig and noticed that WASM is a strange outlier in that the LLVM backend supports the option but Clang doesn't expose it. This means that we have to distinguish between whether the option is supported in Zig code (where we go straight to LLVM) vs C/C++ code (zig cc, where we go through the Clang driver). We can do that if we must, but it just seemed easier to expose the functionality that's already there in the backend anyway.

@sunfishcode
Copy link
Member

Makes sense. My instinct here is to wait until we have a concrete use case.

@alexrp alexrp closed this May 5, 2025
@alexrp alexrp deleted the clang-wasm-red-zone branch May 5, 2025 15:40
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants