Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WebAssembly] Re-enable reference types by default #93261

Merged
merged 2 commits into from
Jun 20, 2024
Merged

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented May 24, 2024

Now that we are about to upgrade emsdk's default node to v18.20.3 (emscripten-core/emsdk#1387), we can re-enable reference-types by default again. This effectively reverts #90792.

Now that we are about to upgrade emsdk's default node to v18.20.3
(emscripten-core/emsdk#1387), we can re-enable
reference-types by default again. This effectively reverts llvm#90792.
@aheejin aheejin requested review from sbc100 and dschuff May 24, 2024 01:07
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:WebAssembly clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 24, 2024
@aheejin aheejin changed the title Re-enable reference types by default [WebAssembly] Re-enable reference types by default May 24, 2024
@llvmbot
Copy link
Member

llvmbot commented May 24, 2024

@llvm/pr-subscribers-backend-webassembly

@llvm/pr-subscribers-clang

Author: Heejin Ahn (aheejin)

Changes

Now that we are about to upgrade emsdk's default node to v18.20.3 (emscripten-core/emsdk#1387), we can re-enable reference-types by default again. This effectively reverts #90792.


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+4-4)
  • (modified) clang/lib/Basic/Targets/WebAssembly.cpp (+1-1)
  • (modified) clang/test/Preprocessor/wasm-target-features.c (+1-1)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2b35e2162ab5b..dfc1af8c4be8a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -881,10 +881,10 @@ AIX Support
 WebAssembly Support
 ^^^^^^^^^^^^^^^^^^^
 
-The -mcpu=generic configuration now enables multivalue feature, which is
-standardized and available in all major engines. Enabling multivalue here only
-enables the language feature but does not turn on the multivalue ABI (this
-enables non-ABI uses of multivalue, like exnref).
+The -mcpu=generic configuration now enables multivalue and reference-types.These
+proposals are standardized and available in all major engines. Enabling
+multivalue here only enables the language feature but does not turn on the
+multivalue ABI (this enables non-ABI uses of multivalue, like exnref).
 
 AVR Support
 ^^^^^^^^^^^
diff --git a/clang/lib/Basic/Targets/WebAssembly.cpp b/clang/lib/Basic/Targets/WebAssembly.cpp
index 5a000314a72ce..1e565f0a5319f 100644
--- a/clang/lib/Basic/Targets/WebAssembly.cpp
+++ b/clang/lib/Basic/Targets/WebAssembly.cpp
@@ -153,6 +153,7 @@ bool WebAssemblyTargetInfo::initFeatureMap(
   auto addGenericFeatures = [&]() {
     Features["multivalue"] = true;
     Features["mutable-globals"] = true;
+    Features["reference-types"] = true;
     Features["sign-ext"] = true;
   };
   auto addBleedingEdgeFeatures = [&]() {
@@ -164,7 +165,6 @@ bool WebAssemblyTargetInfo::initFeatureMap(
     Features["half-precision"] = true;
     Features["multimemory"] = true;
     Features["nontrapping-fptoint"] = true;
-    Features["reference-types"] = true;
     Features["tail-call"] = true;
     setSIMDLevel(Features, RelaxedSIMD, true);
   };
diff --git a/clang/test/Preprocessor/wasm-target-features.c b/clang/test/Preprocessor/wasm-target-features.c
index 9d49e3af603f8..d5539163b3bf5 100644
--- a/clang/test/Preprocessor/wasm-target-features.c
+++ b/clang/test/Preprocessor/wasm-target-features.c
@@ -164,6 +164,7 @@
 //
 // GENERIC-INCLUDE-DAG: #define __wasm_multivalue__ 1{{$}}
 // GENERIC-INCLUDE-DAG: #define __wasm_mutable_globals__ 1{{$}}
+// GENERIC-INCLUDE-DAG: #define __wasm_reference_types__ 1{{$}}
 // GENERIC-INCLUDE-DAG: #define __wasm_sign_ext__ 1{{$}}
 //
 // RUN: %clang -E -dM %s -o - 2>&1 \
@@ -180,7 +181,6 @@
 // GENERIC-NOT: #define __wasm_half_precision__ 1{{$}}
 // GENERIC-NOT: #define __wasm_multimemory__ 1{{$}}
 // GENERIC-NOT: #define __wasm_nontrapping_fptoint__ 1{{$}}
-// GENERIC-NOT: #define __wasm_reference_types__ 1{{$}}
 // GENERIC-NOT: #define __wasm_relaxed_simd__ 1{{$}}
 // GENERIC-NOT: #define __wasm_simd128__ 1{{$}}
 // GENERIC-NOT: #define __wasm_tail_call__ 1{{$}}

@aheejin
Copy link
Member Author

aheejin commented May 24, 2024

This should land after the node-updating PRs land and the node version in Chromium CI is updated.

@aheejin
Copy link
Member Author

aheejin commented Jun 20, 2024

Given that the node version in Chromium CI has been updated successfully (https://chromium-review.googlesource.com/c/emscripten-releases/+/5503423), I'm gonna land this.

@aheejin aheejin merged commit 6e38df3 into llvm:main Jun 20, 2024
9 checks passed
@aheejin aheejin deleted the reftypes branch June 20, 2024 20:03
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
Now that we are about to upgrade emsdk's default node to v18.20.3
(emscripten-core/emsdk#1387), we can re-enable
reference-types by default again. This effectively reverts llvm#90792.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:WebAssembly clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants