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] Update generic and bleeding-edge CPUs #96584

Merged
merged 5 commits into from
Jul 2, 2024

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Jun 25, 2024

This updates the list of features in 'generic' and 'bleeding-edge' CPUs in the backend to match

bool WebAssemblyTargetInfo::initFeatureMap(
llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
const std::vector<std::string> &FeaturesVec) const {
auto addGenericFeatures = [&]() {
Features["multivalue"] = true;
Features["mutable-globals"] = true;
Features["reference-types"] = true;
Features["sign-ext"] = true;
};
auto addBleedingEdgeFeatures = [&]() {
addGenericFeatures();
Features["atomics"] = true;
Features["bulk-memory"] = true;
Features["exception-handling"] = true;
Features["extended-const"] = true;
Features["half-precision"] = true;
Features["multimemory"] = true;
Features["nontrapping-fptoint"] = true;
Features["tail-call"] = true;
setSIMDLevel(Features, RelaxedSIMD, true);
};
if (CPU == "generic") {
addGenericFeatures();
} else if (CPU == "bleeding-edge") {
addBleedingEdgeFeatures();
}
return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
}

This updates existing CodeGen tests in a way that, if a test has separate RUN lines for a reference-types test and a non-reference-types test, I added -mattr=-reference-types to the no-reftype test's RUN command line. I didn't delete existing -mattr=+reference-types lines in reftype tests because having it helps readability.

Also, when tests is not really about reference-types but they have to updated because they happen to contain call_indirect lines because now call_indirect will take __indirect_function_table as an argument, I just added the table argument to the expected output.

target-features-cpus.ll has been updated reflecting the newly added features.

This updates the list of features in 'generic' and 'bleeding-edge' CPUs
in the backend to match
https://github.com/llvm/llvm-project/blob/4e0a0eae58f7a6998866719f7eb970096a2a52e9/clang/lib/Basic/Targets/WebAssembly.cpp#L150-L175.

It is hard to add a backend test for this, given that we don't have a
convenient way of testing the list of features included in each cpu like
the preprocessor test in Clang:
https://github.com/llvm/llvm-project/blob/4e0a0eae58f7a6998866719f7eb970096a2a52e9/clang/test/Preprocessor/wasm-target-features.c#L158-L208
And the current features for 'generic' and 'bleeding-edge' don't have a
separate backend test anyway.
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-webassembly

Author: Heejin Ahn (aheejin)

Changes

This updates the list of features in 'generic' and 'bleeding-edge' CPUs in the backend to match

bool WebAssemblyTargetInfo::initFeatureMap(
llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
const std::vector<std::string> &FeaturesVec) const {
auto addGenericFeatures = [&]() {
Features["multivalue"] = true;
Features["mutable-globals"] = true;
Features["reference-types"] = true;
Features["sign-ext"] = true;
};
auto addBleedingEdgeFeatures = [&]() {
addGenericFeatures();
Features["atomics"] = true;
Features["bulk-memory"] = true;
Features["exception-handling"] = true;
Features["extended-const"] = true;
Features["half-precision"] = true;
Features["multimemory"] = true;
Features["nontrapping-fptoint"] = true;
Features["tail-call"] = true;
setSIMDLevel(Features, RelaxedSIMD, true);
};
if (CPU == "generic") {
addGenericFeatures();
} else if (CPU == "bleeding-edge") {
addBleedingEdgeFeatures();
}
.

It is hard to add a backend test for this, given that we don't have a convenient way of testing the list of features included in each cpu like the preprocessor test in Clang:

// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm32-unknown-unknown -mcpu=generic \
// RUN: | FileCheck %s -check-prefix=GENERIC-INCLUDE
// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm64-unknown-unknown -mcpu=generic \
// RUN: | FileCheck %s -check-prefix=GENERIC-INCLUDE
//
// 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 \
// RUN: -target wasm32-unknown-unknown -mcpu=generic \
// RUN: | FileCheck %s -check-prefix=GENERIC
// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm64-unknown-unknown -mcpu=generic \
// RUN: | FileCheck %s -check-prefix=GENERIC
//
// GENERIC-NOT: #define __wasm_atomics__ 1{{$}}
// GENERIC-NOT: #define __wasm_bulk_memory__ 1{{$}}
// GENERIC-NOT: #define __wasm_exception_handling__ 1{{$}}
// GENERIC-NOT: #define __wasm_extended_const__ 1{{$}}
// GENERIC-NOT: #define __wasm_half_precision__ 1{{$}}
// GENERIC-NOT: #define __wasm_multimemory__ 1{{$}}
// GENERIC-NOT: #define __wasm_nontrapping_fptoint__ 1{{$}}
// GENERIC-NOT: #define __wasm_relaxed_simd__ 1{{$}}
// GENERIC-NOT: #define __wasm_simd128__ 1{{$}}
// GENERIC-NOT: #define __wasm_tail_call__ 1{{$}}
// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm32-unknown-unknown -mcpu=bleeding-edge \
// RUN: | FileCheck %s -check-prefix=BLEEDING-EDGE-INCLUDE
// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm64-unknown-unknown -mcpu=bleeding-edge \
// RUN: | FileCheck %s -check-prefix=BLEEDING-EDGE-INCLUDE
//
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_atomics__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_bulk_memory__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_exception_handling__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_extended_const__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_half_precision__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_multimemory__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_multivalue__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_mutable_globals__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_nontrapping_fptoint__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_reference_types__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_relaxed_simd__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_sign_ext__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_simd128__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_tail_call__ 1{{$}}
And the current features for 'generic' and 'bleeding-edge' don't have a separate backend test anyway.


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

1 Files Affected:

  • (modified) llvm/lib/Target/WebAssembly/WebAssembly.td (+8-4)
diff --git a/llvm/lib/Target/WebAssembly/WebAssembly.td b/llvm/lib/Target/WebAssembly/WebAssembly.td
index 0dd674426e9e8..97618617ff82f 100644
--- a/llvm/lib/Target/WebAssembly/WebAssembly.td
+++ b/llvm/lib/Target/WebAssembly/WebAssembly.td
@@ -110,13 +110,17 @@ def : ProcessorModel<"mvp", NoSchedModel, []>;
 // consideration given to available support in relevant engines and tools, and
 // the importance of the features.
 def : ProcessorModel<"generic", NoSchedModel,
-                      [FeatureSignExt, FeatureMutableGlobals]>;
+                      [FeatureMultivalue, FeatureMutableGlobals,
+                       FeatureReferenceTypes, FeatureSignExt]>;
 
 // Latest and greatest experimental version of WebAssembly. Bugs included!
 def : ProcessorModel<"bleeding-edge", NoSchedModel,
-                      [FeatureSIMD128, FeatureAtomics,
-                       FeatureNontrappingFPToInt, FeatureSignExt,
-                       FeatureMutableGlobals, FeatureBulkMemory,
+                      [FeatureAtomics, FeatureBulkMemory,
+                       FeatureExceptionHandling, FeatureExtendedConst,
+                       FeatureHalfPrecision, FeatureMultiMemory,
+                       FeatureMultivalue, FeatureMutableGlobals,
+                       FeatureNontrappingFPToInt, FeatureRelaxedSIMD,
+                       FeatureReferenceTypes, FeatureSIMD128, FeatureSignExt,
                        FeatureTailCall]>;
 
 //===----------------------------------------------------------------------===//

Copy link
Collaborator

@tlively tlively left a comment

Choose a reason for hiding this comment

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

LGTM!

@llvmbot llvmbot added the mc Machine (object) code label Jun 25, 2024
@aheejin
Copy link
Member Author

aheejin commented Jun 25, 2024

Sorry, apparently there are tests to be updated 😅

3186ad7 updates tests in a way that, if a test has separate RUN lines for a reference-types test and a non-reference-types test, I added -mattr=-reference-types to the no-reftype test's RUN command line. I didn't delete existing -mattr=+reference-types lines in reftype tests because having it helps readability.

Also, when tests is not really about reference-types but they have to updated because they happen to contain call_indirect lines because now call_indirect will take __indirect_function_table as an argument, I just added the table argument to the expected output.

I think https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/WebAssembly/target-features.ll better be updated separately... Will upload a PR for that and land this after that one lands.

@aheejin aheejin marked this pull request as draft June 25, 2024 22:09
@tlively
Copy link
Collaborator

tlively commented Jun 25, 2024

Test changes so far look good 👍

@aheejin aheejin marked this pull request as ready for review June 26, 2024 20:35
@aheejin
Copy link
Member Author

aheejin commented Jul 2, 2024

Will land this; please let me know if you have any other comments.

@aheejin aheejin merged commit fb6e024 into llvm:main Jul 2, 2024
7 checks passed
@aheejin aheejin deleted the llc_default branch July 2, 2024 02:12
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
This updates the list of features in 'generic' and 'bleeding-edge' CPUs
in the backend to match

https://github.com/llvm/llvm-project/blob/4e0a0eae58f7a6998866719f7eb970096a2a52e9/clang/lib/Basic/Targets/WebAssembly.cpp#L150-L178

This updates existing CodeGen tests in a way that, if a test has
separate RUN lines for a reference-types test and a non-reference-types
test, I added -mattr=-reference-types to the no-reftype test's RUN
command line. I didn't delete existing -mattr=+reference-types lines in
reftype tests because having it helps readability.

Also, when tests is not really about reference-types but they have to
updated because they happen to contain call_indirect lines because now
call_indirect will take __indirect_function_table as an argument, I just
added the table argument to the expected output.

`target-features-cpus.ll` has been updated reflecting the newly added
features.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
This updates the list of features in 'generic' and 'bleeding-edge' CPUs
in the backend to match

https://github.com/llvm/llvm-project/blob/4e0a0eae58f7a6998866719f7eb970096a2a52e9/clang/lib/Basic/Targets/WebAssembly.cpp#L150-L178

This updates existing CodeGen tests in a way that, if a test has
separate RUN lines for a reference-types test and a non-reference-types
test, I added -mattr=-reference-types to the no-reftype test's RUN
command line. I didn't delete existing -mattr=+reference-types lines in
reftype tests because having it helps readability.

Also, when tests is not really about reference-types but they have to
updated because they happen to contain call_indirect lines because now
call_indirect will take __indirect_function_table as an argument, I just
added the table argument to the expected output.

`target-features-cpus.ll` has been updated reflecting the newly added
features.
@CryZe
Copy link

CryZe commented Oct 18, 2024

Some feedback: This broke the majority of the JavaScript bundlers (webpack and co.) which are commonly built on top of webassemblyjs which is very unmaintained and does not support reference-types. I'm not sure, but it may be worth reverting.

@dschuff
Copy link
Member

dschuff commented Oct 18, 2024

Can you say more about what the bundlers are doing, and how they are doing it?
I don't want to be breaking people, but also we surely can't freeze LLVM forever just because some other tool is unmaintained. We are actually planning to enable bulk memory and nontrapping-fptoint soon as well.

@dschuff
Copy link
Member

dschuff commented Oct 18, 2024

Interestingly, it looks like the decoder in webassemblyjs already decodes the table index as an LEB.

@dschuff
Copy link
Member

dschuff commented Oct 18, 2024

actually... sorry, let's have this conversation on WebAssembly/tool-conventions#233 rather than forking it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:WebAssembly mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants