Closed Bug 1782770 Opened 6 months ago Closed 6 months ago

Transfer all type definitions to Metadata

Categories

(Core :: JavaScript: WebAssembly, task, P2)

task

Tracking

()

RESOLVED FIXED
105 Branch
Tracking Status
firefox105 --- fixed

People

(Reporter: rhunt, Assigned: rhunt)

References

Details

Attachments

(3 files)

Metadata stores a vector of TypeDef that comes from the type section of the module. We currently only transfer a type if it's a struct type, array type, or else a function type that cannot fit in an immediate for call_indirect signature checks.

Because we filter out some types, this means we need to renumber the type index space to match. This is a bit hacky, and is difficult to do consistently.

The original reason for only bringing function types that cannot fit in an immediate was to reduce the size of metadata as most function types can fit in an immediate. However, I discovered that we make a copies of function types for FuncExport and FuncImport metadata. This can create multiple copies of the original function type, e.g. when multiple exported functions reference the same type definition.

If we instead transfer all types and store the index of the function type in FuncExport/FuncImport we can reduce total metadata size, reduce allocations, and remove type renumbering.

Metadata stores a vector of TypeDef that comes from the type section of the module.
We currently only transfer a type if it's a struct type, array type, or else a
function type that cannot fit in an immediate for call_indirect signature checks.

Because we filter out some types, this means we need to renumber the type index
space to match. This is a bit hacky, and is difficult to do consistently.

The original reason for only bringing function types that cannot fit in an
immediate was to reduce the size of metadata as most function types can fit in an
immediate. However, I discovered that we make a copies of function types for
FuncExport and FuncImport metadata. This can create multiple copies of the original
function type, e.g. when multiple exported functions reference the same type
definition.

If we instead transfer all types and store the index of the function type in
FuncExport/FuncImport we can reduce total metadata size, reduce allocations,
and remove type renumbering.

This commit does this by:

  1. Transferring all type definitions to Metadata (WasmGenerator.cpp)
  2. Removing the typeRenumbering vector
  3. Replacing FuncType with typeIndex on FuncImport/FuncExport
  4. Updating all users of FuncImport/FuncExport to get the FuncType using
    the typeIndex and Metadata. This the bulk of the changes.

ValType needed to be pointer size to support (rtt) types. Now that
they have been removed, we can revert to 32-bit on all platforms.

Depends on D153498

Now that we transfer all type definitions to Metadata, we can
remove the special debugging case where we would transfer all
function types. Instead, we can just transfer the funcTypeIndex
and find the function type in Metadata.

Depends on D153499

Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/autoland/rev/a881b6ab4795
wasm: Transfer all type definitions to Metadata. r=yury
https://hg.mozilla.org/integration/autoland/rev/c4eca5ce1a52
wasm: Make ValType a uint32_t now that rtt's are gone. r=yury
https://hg.mozilla.org/integration/autoland/rev/dfb8854e3ac0
wasm: Don't make extra copy of FuncType for debugging. r=yury

Backed out for causing wasm related spidermonkey build bustages.

Push with failures

Failure log

Backout link

[task 2022-08-11T22:04:10.312Z] gmake[4]: Entering directory '/builds/worker/workspace/obj-spider/js/src/wasm'
[task 2022-08-11T22:04:10.313Z] /builds/worker/fetches/clang/bin/clang++ --sysroot /builds/worker/fetches/sysroot-x86_64-linux-gnu -std=gnu++17 -o WasmDebugFrame.o -c  -I/builds/worker/workspace/obj-spider/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fstack-clash-protection -ftrivial-auto-var-init=pattern -DDEBUG=1 -DWASM_SUPPORTS_HUGE_MEMORY -DJS_CACHEIR_SPEW -DJS_STRUCTURED_SPEW -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/builds/worker/checkouts/gecko/js/src/wasm -I/builds/worker/workspace/obj-spider/js/src/wasm -I/builds/worker/workspace/obj-spider/js/src -I/builds/worker/checkouts/gecko/js/src -I/builds/worker/workspace/obj-spider/dist/include -I/builds/worker/workspace/obj-spider/dist/include/nspr -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-spider/js/src/js-confdefs.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wdeprecated-this-capture -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wno-error=tautological-type-limit-compare -Wunreachable-code -Wunreachable-code-return -Wunused-but-set-parameter -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wc++2a-compat -Wenum-compare-conditional -Wno-ambiguous-reversed-operator -Wno-error=deprecated -Wno-error=deprecated-anon-enum-enum-conversion -Wno-error=deprecated-enum-enum-conversion -Wno-error=deprecated-enum-float-conversion -Wno-error=deprecated-pragma -Wno-error=deprecated-this-capture -Wno-error=deprecated-volatile -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=free-nonheap-object -Wno-error=return-std-move -Wno-error=atomic-alignment -Wno-error=deprecated-copy -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-psabi -Wthread-safety -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fPIC -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -gdwarf-4 -Xclang -load -Xclang /builds/worker/workspace/obj-spider/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Werror -Werror=format -fno-strict-aliasing -ffp-contract=off  -MD -MP -MF .deps/WasmDebugFrame.o.pp   /builds/worker/checkouts/gecko/js/src/wasm/WasmDebugFrame.cpp
[task 2022-08-11T22:04:10.313Z] In file included from /builds/worker/checkouts/gecko/js/src/wasm/WasmDebugFrame.cpp:21:
[task 2022-08-11T22:04:10.313Z] In file included from /builds/worker/checkouts/gecko/js/src/vm/EnvironmentObject.h:17:
[task 2022-08-11T22:04:10.313Z] In file included from /builds/worker/checkouts/gecko/js/src/vm/GlobalObject.h:33:
[task 2022-08-11T22:04:10.313Z] In file included from /builds/worker/checkouts/gecko/js/src/vm/JSContext.h:36:
[task 2022-08-11T22:04:10.313Z] In file included from /builds/worker/checkouts/gecko/js/src/vm/Runtime.h:72:
[task 2022-08-11T22:04:10.313Z] In file included from /builds/worker/checkouts/gecko/js/src/vm/Scope.h:38:
[task 2022-08-11T22:04:10.313Z] In file included from /builds/worker/checkouts/gecko/js/src/wasm/WasmJS.h:41:
[task 2022-08-11T22:04:10.313Z] In file included from /builds/worker/checkouts/gecko/js/src/wasm/WasmCodegenTypes.h:31:
[task 2022-08-11T22:04:10.314Z] In file included from /builds/worker/checkouts/gecko/js/src/wasm/WasmInstanceData.h:27:
[task 2022-08-11T22:04:10.314Z] /builds/worker/checkouts/gecko/js/src/wasm/WasmInstance.h:317:26: error: inline function 'js::wasm::Instance::metadata' is not defined [-Werror,-Wundefined-inline]
[task 2022-08-11T22:04:10.314Z]   inline const Metadata& metadata() const;
[task 2022-08-11T22:04:10.314Z]                          ^
[task 2022-08-11T22:04:10.314Z] /builds/worker/checkouts/gecko/js/src/wasm/WasmDebugFrame.cpp:138:19: note: used here
[task 2022-08-11T22:04:10.314Z]       instance()->metadata().debugFuncType(funcIndex()).results());
[task 2022-08-11T22:04:10.314Z]                   ^
[task 2022-08-11T22:04:10.314Z] 1 error generated.
[task 2022-08-11T22:04:10.314Z] gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:669: WasmDebugFrame.o] Error 1
[task 2022-08-11T22:04:10.314Z] gmake[4]: Leaving directory '/builds/worker/workspace/obj-spider/js/src/wasm'
[task 2022-08-11T22:04:10.314Z] gmake[4]: *** Waiting for unfinished jobs....
[task 2022-08-11T22:04:10.318Z] gmake[4]: Entering directory '/builds/worker/workspace/obj-spider/js/src/gc'
Flags: needinfo?(rhunt)

Non-unified builds..

Flags: needinfo?(rhunt)
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/autoland/rev/c867a71812f8
wasm: Transfer all type definitions to Metadata. r=yury
https://hg.mozilla.org/integration/autoland/rev/4f2d7c625c8d
wasm: Make ValType a uint32_t now that rtt's are gone. r=yury
https://hg.mozilla.org/integration/autoland/rev/115c87e6a8bd
wasm: Don't make extra copy of FuncType for debugging. r=yury
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
You need to log in before you can comment on or make changes to this bug.