[WebAssembly] Define call-indirect-overlong and bulk-memory-opt features #117087
Conversation
This defines some new target features. These are subsets of existing features
that reflect implementation concerns:
- "call-indirect-overlong" - implied by "reference-types"; just the overlong
encoding for the `call_indirect` immediate, and not the actual reference
types.
- "bulk-memory-opt" - implied by "bulk-memory": just `memory.copy` and
`memory.fill`, and not the other instructions in the bulk-memory
proposal.
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-webassembly @llvm/pr-subscribers-clang Author: Dan Gohman (sunfishcode) ChangesThis defines some new target features. These are subsets of existing features that reflect implementation concerns:
This is split out from #112035. Patch is 37.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117087.diff 31 Files Affected:
diff --git a/clang/lib/Basic/Targets/WebAssembly.cpp b/clang/lib/Basic/Targets/WebAssembly.cpp
index 0b380bdf835ffb..ba323940a8a86c 100644
--- a/clang/lib/Basic/Targets/WebAssembly.cpp
+++ b/clang/lib/Basic/Targets/WebAssembly.cpp
@@ -47,6 +47,8 @@ bool WebAssemblyTargetInfo::hasFeature(StringRef Feature) const {
return llvm::StringSwitch<bool>(Feature)
.Case("atomics", HasAtomics)
.Case("bulk-memory", HasBulkMemory)
+ .Case("bulk-memory-opt", HasBulkMemoryOpt)
+ .Case("call-indirect-overlong", HasCallIndirectOverlong)
.Case("exception-handling", HasExceptionHandling)
.Case("extended-const", HasExtendedConst)
.Case("fp16", HasFP16)
@@ -79,6 +81,8 @@ void WebAssemblyTargetInfo::getTargetDefines(const LangOptions &Opts,
Builder.defineMacro("__wasm_atomics__");
if (HasBulkMemory)
Builder.defineMacro("__wasm_bulk_memory__");
+ if (HasBulkMemoryOpt)
+ Builder.defineMacro("__wasm_bulk_memory_opt__");
if (HasExceptionHandling)
Builder.defineMacro("__wasm_exception_handling__");
if (HasExtendedConst)
@@ -155,6 +159,8 @@ bool WebAssemblyTargetInfo::initFeatureMap(
const std::vector<std::string> &FeaturesVec) const {
auto addGenericFeatures = [&]() {
Features["bulk-memory"] = true;
+ Features["bulk-memory-opt"] = true;
+ Features["call-indirect-overlong"] = true;
Features["multivalue"] = true;
Features["mutable-globals"] = true;
Features["nontrapping-fptoint"] = true;
@@ -200,6 +206,14 @@ bool WebAssemblyTargetInfo::handleTargetFeatures(
HasBulkMemory = false;
continue;
}
+ if (Feature == "+bulk-memory-opt") {
+ HasBulkMemoryOpt = true;
+ continue;
+ }
+ if (Feature == "-bulk-memory-opt") {
+ HasBulkMemoryOpt = false;
+ continue;
+ }
if (Feature == "+exception-handling") {
HasExceptionHandling = true;
continue;
@@ -265,6 +279,14 @@ bool WebAssemblyTargetInfo::handleTargetFeatures(
HasReferenceTypes = false;
continue;
}
+ if (Feature == "+call-indirect-overlong") {
+ HasCallIndirectOverlong = true;
+ continue;
+ }
+ if (Feature == "-call-indirect-overlong") {
+ HasCallIndirectOverlong = false;
+ continue;
+ }
if (Feature == "+relaxed-simd") {
SIMDLevel = std::max(SIMDLevel, RelaxedSIMD);
continue;
@@ -310,6 +332,18 @@ bool WebAssemblyTargetInfo::handleTargetFeatures(
<< Feature << "-target-feature";
return false;
}
+
+ // The reference-types feature included the change to `call_indirect`
+ // encodings to support overlong immediates.
+ if (HasReferenceTypes) {
+ HasCallIndirectOverlong = true;
+ }
+
+ // bulk-memory-opt is a subset of bulk-memory.
+ if (HasBulkMemory) {
+ HasBulkMemoryOpt = true;
+ }
+
return true;
}
diff --git a/clang/lib/Basic/Targets/WebAssembly.h b/clang/lib/Basic/Targets/WebAssembly.h
index 6c2fe8049ff47a..09da9d60dc5a34 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -55,6 +55,7 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : public TargetInfo {
bool HasAtomics = false;
bool HasBulkMemory = false;
+ bool HasBulkMemoryOpt = false;
bool HasExceptionHandling = false;
bool HasExtendedConst = false;
bool HasFP16 = false;
@@ -63,6 +64,7 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : public TargetInfo {
bool HasMutableGlobals = false;
bool HasNontrappingFPToInt = false;
bool HasReferenceTypes = false;
+ bool HasCallIndirectOverlong = false;
bool HasSignExt = false;
bool HasTailCall = false;
bool HasWideArithmetic = false;
diff --git a/lld/test/wasm/compress-relocs.ll b/lld/test/wasm/compress-relocs.ll
index f1faab754cb765..cea9f3476e996a 100644
--- a/lld/test/wasm/compress-relocs.ll
+++ b/lld/test/wasm/compress-relocs.ll
@@ -1,5 +1,5 @@
; RUN: llc -filetype=obj %s -o %t.o
-; RUN: llvm-mc -mattr=+reference-types -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/call-indirect.s -o %t2.o
+; RUN: llvm-mc -mattr=+call-indirect-overlong -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/call-indirect.s -o %t2.o
; RUN: wasm-ld --export-dynamic -o %t.wasm %t2.o %t.o
; RUN: obj2yaml %t.wasm | FileCheck %s
; RUN: wasm-ld --export-dynamic -O2 -o %t-opt.wasm %t2.o %t.o
diff --git a/lld/test/wasm/import-table-explicit.s b/lld/test/wasm/import-table-explicit.s
index 1dc21beba06294..701b7a1dc3e165 100644
--- a/lld/test/wasm/import-table-explicit.s
+++ b/lld/test/wasm/import-table-explicit.s
@@ -1,4 +1,4 @@
-# RUN: llvm-mc -mattr=+reference-types -filetype=obj -triple=wasm32-unknown-unknown %s -o %t.o
+# RUN: llvm-mc -mattr=+call-indirect-overlong -filetype=obj -triple=wasm32-unknown-unknown %s -o %t.o
# RUN: wasm-ld --import-table -o %t.wasm %t.o
# RUN: obj2yaml %t.wasm | FileCheck %s
diff --git a/lld/test/wasm/invalid-mvp-table-use.s b/lld/test/wasm/invalid-mvp-table-use.s
index b4f12a7eeb9a48..58c472e29d1ad4 100644
--- a/lld/test/wasm/invalid-mvp-table-use.s
+++ b/lld/test/wasm/invalid-mvp-table-use.s
@@ -1,7 +1,7 @@
# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
#
# If any table is defined or declared besides the __indirect_function_table,
-# the compilation unit should be compiled with -mattr=+reference-types,
+# the compilation unit should be compiled with -mattr=+call-indirect-overlong,
# causing symbol table entries to be emitted for all tables.
# RUN: not wasm-ld --no-entry %t.o -o %t.wasm 2>&1 | FileCheck -check-prefix=CHECK-ERR %s
diff --git a/lld/test/wasm/lto/Inputs/libcall-archive.ll b/lld/test/wasm/lto/Inputs/libcall-archive.ll
index 7d8c34196dfe49..30764af83e6739 100644
--- a/lld/test/wasm/lto/Inputs/libcall-archive.ll
+++ b/lld/test/wasm/lto/Inputs/libcall-archive.ll
@@ -5,4 +5,4 @@ define void @memcpy() #0 {
ret void
}
-attributes #0 = { "target-features"="-bulk-memory" }
+attributes #0 = { "target-features"="-bulk-memory,-bulk-memory-opt" }
diff --git a/lld/test/wasm/lto/libcall-archive.ll b/lld/test/wasm/lto/libcall-archive.ll
index 5c46d2f7ed7838..0cee9a5de29f61 100644
--- a/lld/test/wasm/lto/libcall-archive.ll
+++ b/lld/test/wasm/lto/libcall-archive.ll
@@ -16,7 +16,7 @@ entry:
declare void @llvm.memcpy.p0.p0.i64(ptr nocapture, ptr nocapture, i64, i1)
-attributes #0 = { "target-features"="-bulk-memory" }
+attributes #0 = { "target-features"="-bulk-memory,-bulk-memory-opt" }
; CHECK: - Type: CUSTOM
; CHECK-NEXT: Name: name
diff --git a/lld/test/wasm/lto/stub-library-libcall.s b/lld/test/wasm/lto/stub-library-libcall.s
index d65983c0cf5bf5..40e15933f7bc39 100644
--- a/lld/test/wasm/lto/stub-library-libcall.s
+++ b/lld/test/wasm/lto/stub-library-libcall.s
@@ -2,7 +2,7 @@
# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t_main.o %t/main.s
# RUN: llvm-as %S/Inputs/foo.ll -o %t_foo.o
# RUN: llvm-as %S/Inputs/libcall.ll -o %t_libcall.o
-# RUN: wasm-ld -mllvm -mattr=-bulk-memory %t_main.o %t_libcall.o %t_foo.o %p/Inputs/stub.so -o %t.wasm
+# RUN: wasm-ld -mllvm -mattr=-bulk-memory,-bulk-memory-opt %t_main.o %t_libcall.o %t_foo.o %p/Inputs/stub.so -o %t.wasm
# RUN: obj2yaml %t.wasm | FileCheck %s
# The function `func_with_libcall` will generate an undefined reference to
@@ -12,7 +12,7 @@
# If %t_foo.o is not included in the link we get an undefined symbol reported
# to the dependency of memcpy on the foo export:
-# RUN: not wasm-ld -mllvm -mattr=-bulk-memory %t_main.o %t_libcall.o %p/Inputs/stub.so -o %t.wasm 2>&1 | FileCheck --check-prefix=MISSING %s
+# RUN: not wasm-ld -mllvm -mattr=-bulk-memory,-bulk-memory-opt %t_main.o %t_libcall.o %p/Inputs/stub.so -o %t.wasm 2>&1 | FileCheck --check-prefix=MISSING %s
# MISSING: stub.so: undefined symbol: foo. Required by memcpy
#--- main.s
diff --git a/lld/test/wasm/multi-table.s b/lld/test/wasm/multi-table.s
index bf905ac748f9fb..3129093a25c7fb 100644
--- a/lld/test/wasm/multi-table.s
+++ b/lld/test/wasm/multi-table.s
@@ -26,7 +26,7 @@ call_indirect_explicit_tables:
call_indirect table_b, () -> ()
end_function
-# RT-MVP: wasm-ld: error: object file not built with 'reference-types' feature conflicts with import of table table_a by file
+# RT-MVP: wasm-ld: error: object file not built with 'call-indirect-overlong' feature conflicts with import of table table_a by file
# CHECK: --- !WASM
# CHECK-NEXT: FileHeader:
diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index fd06788457966a..53331aa58d2d37 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -255,10 +255,11 @@ static void setRelocs(const std::vector<T *> &chunks,
}
}
-// An object file can have two approaches to tables. With the reference-types
-// feature enabled, input files that define or use tables declare the tables
-// using symbols, and record each use with a relocation. This way when the
-// linker combines inputs, it can collate the tables used by the inputs,
+// An object file can have two approaches to tables. With the
+// call-indirect-overlong feature enabled (explicitly, or implied by the
+// reference-types feature), input files that define or use tables declare the
+// tables using symbols, and record each use with a relocation. This way when
+// the linker combines inputs, it can collate the tables used by the inputs,
// assigning them distinct table numbers, and renumber all the uses as
// appropriate. At the same time, the linker has special logic to build the
// indirect function table if it is needed.
@@ -284,7 +285,7 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded(
return;
// It's possible for an input to define tables and also use the indirect
- // function table, but forget to compile with -mattr=+reference-types.
+ // function table, but forget to compile with -mattr=+call-indirect-overlong.
// For these newer files, we require symbols for all tables, and
// relocations for all of their uses.
if (tableSymbolCount != 0) {
diff --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp
index 1454c3324af989..747c203b996674 100644
--- a/lld/wasm/SyntheticSections.cpp
+++ b/lld/wasm/SyntheticSections.cpp
@@ -326,7 +326,7 @@ void TableSection::addTable(InputTable *table) {
// to assign table number 0 to the indirect function table.
for (const auto *culprit : out.importSec->importedSymbols) {
if (isa<UndefinedTable>(culprit)) {
- error("object file not built with 'reference-types' feature "
+ error("object file not built with 'call-indirect-overlong' feature "
"conflicts with import of table " +
culprit->getName() + " by file " +
toString(culprit->getFile()));
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index 10451600050ca7..f693ef3dbf962b 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -276,7 +276,18 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
: MCTargetAsmParser(Options, STI, MII), Parser(Parser),
Lexer(Parser.getLexer()), Is64(STI.getTargetTriple().isArch64Bit()),
TC(Parser, MII, Is64), SkipTypeCheck(Options.MCNoTypeCheck) {
- setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
+ FeatureBitset FBS = ComputeAvailableFeatures(STI.getFeatureBits());
+
+ // bulk-memory implies bulk-memory-opt
+ if (FBS.test(WebAssembly::FeatureBulkMemory)) {
+ FBS.set(WebAssembly::FeatureBulkMemoryOpt);
+ }
+ // reference-types implies call-indirect-overlong
+ if (FBS.test(WebAssembly::FeatureReferenceTypes)) {
+ FBS.set(WebAssembly::FeatureCallIndirectOverlong);
+ }
+
+ setAvailableFeatures(FBS);
// Don't type check if this is inline asm, since that is a naked sequence of
// instructions without a function/locals decl.
auto &SM = Parser.getSourceManager();
@@ -291,7 +302,8 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
DefaultFunctionTable = getOrCreateFunctionTableSymbol(
getContext(), "__indirect_function_table", Is64);
- if (!STI->checkFeatures("+reference-types"))
+ if (!STI->checkFeatures("+call-indirect-overlong") &&
+ !STI->checkFeatures("+reference-types"))
DefaultFunctionTable->setOmitFromLinkingSection();
}
@@ -531,11 +543,13 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
}
bool parseFunctionTableOperand(std::unique_ptr<WebAssemblyOperand> *Op) {
- if (STI->checkFeatures("+reference-types")) {
- // If the reference-types feature is enabled, there is an explicit table
- // operand. To allow the same assembly to be compiled with or without
- // reference types, we allow the operand to be omitted, in which case we
- // default to __indirect_function_table.
+ if (STI->checkFeatures("+call-indirect-overlong") ||
+ STI->checkFeatures("+reference-types")) {
+ // If the call-indirect-overlong feature is enabled, or implied by the
+ // reference-types feature, there is an explicit table operand. To allow
+ // the same assembly to be compiled with or without
+ // call-indirect-overlong, we allow the operand to be omitted, in which
+ // case we default to __indirect_function_table.
auto &Tok = Lexer.getTok();
if (Tok.is(AsmToken::Identifier)) {
auto *Sym =
diff --git a/llvm/lib/Target/WebAssembly/WebAssembly.td b/llvm/lib/Target/WebAssembly/WebAssembly.td
index 88628f2a793545..d393e3e00309f3 100644
--- a/llvm/lib/Target/WebAssembly/WebAssembly.td
+++ b/llvm/lib/Target/WebAssembly/WebAssembly.td
@@ -29,6 +29,10 @@ def FeatureBulkMemory :
SubtargetFeature<"bulk-memory", "HasBulkMemory", "true",
"Enable bulk memory operations">;
+def FeatureBulkMemoryOpt :
+ SubtargetFeature<"bulk-memory-opt", "HasBulkMemoryOpt", "true",
+ "Enable bulk memory optimization operations">;
+
def FeatureExceptionHandling :
SubtargetFeature<"exception-handling", "HasExceptionHandling", "true",
"Enable Wasm exception handling">;
@@ -63,6 +67,10 @@ def FeatureReferenceTypes :
SubtargetFeature<"reference-types", "HasReferenceTypes", "true",
"Enable reference types">;
+def FeatureCallIndirectOverlong :
+ SubtargetFeature<"call-indirect-overlong", "HasCallIndirectOverlong", "true",
+ "Enable overlong encoding for call_indirect immediates">;
+
def FeatureRelaxedSIMD :
SubtargetFeature<"relaxed-simd", "SIMDLevel", "RelaxedSIMD",
"Enable relaxed-simd instructions">;
@@ -114,19 +122,20 @@ def : ProcessorModel<"mvp", NoSchedModel, []>;
// consideration given to available support in relevant engines and tools, and
// the importance of the features.
def : ProcessorModel<"generic", NoSchedModel,
- [FeatureBulkMemory, FeatureMultivalue,
- FeatureMutableGlobals, FeatureNontrappingFPToInt,
- FeatureReferenceTypes, FeatureSignExt]>;
+ [FeatureBulkMemory, FeatureBulkMemoryOpt,
+ FeatureMultivalue, FeatureMutableGlobals,
+ FeatureNontrappingFPToInt, FeatureReferenceTypes,
+ FeatureCallIndirectOverlong, FeatureSignExt]>;
// Latest and greatest experimental version of WebAssembly. Bugs included!
def : ProcessorModel<"bleeding-edge", NoSchedModel,
- [FeatureAtomics, FeatureBulkMemory,
+ [FeatureAtomics, FeatureBulkMemory, FeatureBulkMemoryOpt,
FeatureExceptionHandling, FeatureExtendedConst,
FeatureFP16, FeatureMultiMemory,
FeatureMultivalue, FeatureMutableGlobals,
FeatureNontrappingFPToInt, FeatureRelaxedSIMD,
- FeatureReferenceTypes, FeatureSIMD128, FeatureSignExt,
- FeatureTailCall]>;
+ FeatureReferenceTypes, FeatureCallIndirectOverlong,
+ FeatureSIMD128, FeatureSignExt, FeatureTailCall]>;
//===----------------------------------------------------------------------===//
// Target Declaration
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
index 558aaa38096f7e..210a35e1462aca 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
@@ -895,7 +895,7 @@ bool WebAssemblyFastISel::selectCall(const Instruction *I) {
// The table into which this call_indirect indexes.
MCSymbolWasm *Table = WebAssembly::getOrCreateFunctionTableSymbol(
MF->getContext(), Subtarget);
- if (Subtarget->hasReferenceTypes()) {
+ if (Subtarget->hasCallIndirectOverlong()) {
MIB.addSym(Table);
} else {
// Otherwise for the MVP there is at most one table whose number is 0, but
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index 2d00889407ff48..1fcbbcd523da52 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -768,7 +768,7 @@ LowerCallResults(MachineInstr &CallResults, DebugLoc DL, MachineBasicBlock *BB,
MF.getContext(), Subtarget)
: WebAssembly::getOrCreateFunctionTableSymbol(
MF.getContext(), Subtarget);
- if (Subtarget->hasReferenceTypes()) {
+ if (Subtarget->hasCallIndirectOverlong()) {
MIB.addSym(Table);
} else {
// For the MVP there is at most one table whose number is 0, but we can't
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td b/llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td
index 0772afb039f820..79d6f21517e5d4 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td
@@ -11,13 +11,13 @@
///
//===----------------------------------------------------------------------===//
-// Instruction requiring HasBulkMemory and the bulk memory prefix byte
+// Instruction requiring HasBulkMemoryOpt and the bulk memory prefix byte
multiclass BULK_I<dag oops_r, dag iops_r, dag oops_s, dag iops_s,
list<dag> pattern_r, string asmstr_r = "",
string asmstr_s = "", bits<32> simdop = -1> {
defm "" : I<oops_r, iops_r, oops_s, iops_s, pattern_r, asmstr_r, asmstr_s,
!or(0xfc00, !and(0xff, simdop))>,
- Requires<[HasBulkMemory]>;
+ Requires<[HasBulkMemoryOpt]>;
}
// Bespoke types and nodes for bulk memory ops
@@ -89,14 +89,14 @@ defm CPY_A#B : I<(outs), (ins i32imm_op:$src_idx, i32imm_op:$dst_idx,
rc:$dst, rc:$src, rc:$len
)],
"", "", 0>,
- Requires<[HasBulkMemory]>;
+ Requires<[HasBulkMemoryOpt]>;
let usesCustomInserter = 1, isCodeGenOnly = 1, mayStore = 1 in
defm SET_A#B : I<(outs), (ins i32imm_op:$idx, rc:$dst, I32:$value, rc:$size),
(outs), (ins i32imm_op:$idx),
[(wasm_memset (i32 imm:$idx), rc:$dst, I32:$value, rc:$size)],
"", "", 0>,
- Requires<[HasBulkMemory]>;
+ Requires<[HasBulkMemoryOpt]>;
}
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
index b3ea499c4f915e..bd587d8897d86c 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
@@ -30,6 +30,10 @@ def HasBulkMemory :
Predicate<"Subtarget->hasBulkMemory()">,
AssemblerPredicate<(all_of FeatureBulkMemory), "bulk-memory">;
+def HasBulkMemoryOpt :
+ Predicate<"Subtarget->hasBulkMemoryOpt()">,
+ AssemblerPredicate<(all_of FeatureBulkMemoryOpt), "bulk-memory-opt">;
+
def HasExceptionHandling :
Predicate<"Subtarget->hasExceptionHandling()">,
AssemblerPredicate<(all_of FeatureExceptionHandling), "...
[truncated]
|
|
Can you explain why you want |
|
The short answer is that's what the Lime1 CPU calls it 😄 .
I included call-indirect-overlong in my original Lime1 proposal because of the simplicity of it. I expected it's easy for mvp-level engines to add support for it. And the more engines support it, the fewer users will see obscure binary decoding errors in cases where a toolchain tries to use an overlong and an engine doesn't recognize it. Concerning naming, from Lime1's perspective, call-indirect-overlong is just a language feature. It's not inherently for call-indirect relocations or multi-table separate compilation strategies. Engines should just accept |
I see, so in practice the effect on LLVM is that you get a relocation at each call_indirect site but we don't need this relocation of the wide encoding for any particular reason. It seems like a lot of steps and complexity just to force some binary decoders to support an otherwise unused feature.. but you think its worth the effort? |
LLVM is already emitting those relocations in the default "generic" CPU today. If that's an undesirable default, perhaps we should make LLVM not emit these relocations by default, even when the target CPU says it can. This is independent of Lime1 or this PR though.
I guessed that since the "generic" CPU is already out there forcing various binary decoders to support this feature, it wouldn't be controversial for "lime1" to do it too. |
I do think its reasonable for all those decoders to support overlong LEBs. But I think I don't see the advantage that adding this new pseudo feature offers. Wouldn't it be simpler to simply not add it? |
I see it more as exposing existing complexity, rather than creating new complexity. It's already surprising that enabling the reference-types feature changes how LLVM isel and MC handle indirect calls, and changes the encoding for call indirects, when no actual reference types are involved. This new call-indirect-overlongs flag just exposes that behavior change as a distinct feature. Adding call-indirect-overlongs to Lime1 simplifies plausible scenarios. Perhaps some users will run "generic" code in their Lime1-supporting engines, in cases where that works. Perhaps some users will use Lime1 and compile some things with reference-types enabled (even when it doesn't need to be, because Makefiles are complex). Perhaps a future Lime2 will include exception-handling, and then include more of reference-types, in order to support |
| if (HasBulkMemoryOpt) | ||
| Builder.defineMacro("__wasm_bulk_memory_opt__"); |
There was a problem hiding this comment.
You don't need this for call-indirect-overlong?
There was a problem hiding this comment.
I couldn't think of a reason C/C++ code would need to know about call-indirect overlongs. I'm not opposed to adding it if we think there might be a reason though.
Co-authored-by: Heejin Ahn <[email protected]>
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Co-authored-by: Heejin Ahn <[email protected]>
| @@ -1,7 +1,7 @@ | |||
| ; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mcpu=mvp -mattr=+bulk-memory | FileCheck %s --check-prefixes CHECK,BULK-MEM | |||
| ; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mcpu=mvp -mattr=-bulk-memory | FileCheck %s --check-prefixes CHECK,NO-BULK-MEM | |||
| ; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mcpu=mvp -mattr=+bulk-memory-opt | FileCheck %s --check-prefixes CHECK,BULK-MEM | |||
There was a problem hiding this comment.
Do we need to replace the current -mattr=+bulk-memory with +bulk-memory-opt in all the tests where it doesn't matter? I am not strongly opinionated whether we should or should not add the non-Wasm-proposal features, but I'm not sure whether they should be used in all the tests instead of the old features.
There was a problem hiding this comment.
I'm ok either way. I've now changed bulk-memory.ll and bulk-memory64.ll back to -mattr=+bulk-memory.
Co-authored-by: Heejin Ahn <[email protected]>
Co-authored-by: Heejin Ahn <[email protected]>
Co-authored-by: Heejin Ahn <[email protected]>
This defines some new target features. These are subsets of existing features that reflect implementation concerns:
"call-indirect-overlong" - implied by "reference-types"; just the overlong encoding for the
call_indirectimmediate, and not the actual reference types."bulk-memory-opt" - implied by "bulk-memory": just
memory.copyandmemory.fill, and not the other instructions in the bulk-memory proposal.This is split out from #112035.