[wasm-gc] Fix WasmJSFunction polymorphic spec-inlining
{WasmInternalFunction::external} might return a function that is not a WasmExportedFunction, at which point the code in ProcessTypeFeedback fails. See crrev.com/c/3277878 for context. Bug: v8:12436, v8:12166 Change-Id: I09ef96df3fc051586044dd9c2ce88d6aeeb34b9f Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3306391 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Manos Koukoutos <manoskouk@chromium.org> Cr-Commit-Position: refs/heads/main@{#78139}
This commit is contained in:
parent
2d34bb3de6
commit
91ac9163d6
@ -1256,8 +1256,9 @@ std::vector<CallSiteFeedback> ProcessTypeFeedback(
|
||||
if (value.IsWasmInternalFunction() &&
|
||||
WasmExportedFunction::IsWasmExportedFunction(
|
||||
WasmInternalFunction::cast(value).external())) {
|
||||
// Monomorphic. Mark the target for inlining if it's defined in the
|
||||
// same module.
|
||||
// Monomorphic, and the internal function points to a wasm-generated
|
||||
// external function (WasmExportedFunction). Mark the target for inlining
|
||||
// if it's defined in the same module.
|
||||
WasmExportedFunction target = WasmExportedFunction::cast(
|
||||
WasmInternalFunction::cast(value).external());
|
||||
if (target.instance() == *instance &&
|
||||
@ -1288,16 +1289,25 @@ std::vector<CallSiteFeedback> ProcessTypeFeedback(
|
||||
double frequency = static_cast<double>(this_count) / total_count;
|
||||
if (frequency > best_frequency) best_frequency = frequency;
|
||||
if (frequency < 0.8) continue;
|
||||
Object maybe_target = polymorphic.get(j);
|
||||
if (!maybe_target.IsWasmInternalFunction()) {
|
||||
|
||||
// We reject this polymorphic entry if:
|
||||
// - it is not defined,
|
||||
// - it is not a wasm-defined function (WasmExportedFunction)
|
||||
// - it was not defined in this module.
|
||||
if (!polymorphic.get(j).IsWasmInternalFunction()) continue;
|
||||
WasmInternalFunction internal =
|
||||
WasmInternalFunction::cast(polymorphic.get(j));
|
||||
if (!WasmExportedFunction::IsWasmExportedFunction(
|
||||
internal.external())) {
|
||||
continue;
|
||||
}
|
||||
WasmExportedFunction target = WasmExportedFunction::cast(
|
||||
WasmInternalFunction::cast(polymorphic.get(j)).external());
|
||||
WasmExportedFunction target =
|
||||
WasmExportedFunction::cast(internal.external());
|
||||
if (target.instance() != *instance ||
|
||||
target.function_index() < imported_functions) {
|
||||
continue;
|
||||
}
|
||||
|
||||
found_target = target.function_index();
|
||||
found_count = static_cast<int>(this_count);
|
||||
if (FLAG_trace_wasm_speculative_inlining) {
|
||||
@ -1317,6 +1327,10 @@ std::vector<CallSiteFeedback> ProcessTypeFeedback(
|
||||
// If we fall through to here, then this call isn't eligible for inlining.
|
||||
// Possible reasons: uninitialized or megamorphic feedback; or monomorphic
|
||||
// or polymorphic that didn't meet our requirements.
|
||||
if (FLAG_trace_wasm_speculative_inlining) {
|
||||
PrintF("[Function #%d call_ref #%d *not* inlineable]\n", func_index,
|
||||
i / 2);
|
||||
}
|
||||
result[i / 2] = {-1, -1};
|
||||
}
|
||||
return result;
|
||||
|
@ -3,8 +3,9 @@
|
||||
// found in the LICENSE file.
|
||||
|
||||
// Flags: --wasm-speculative-inlining --experimental-wasm-return-call
|
||||
// Flags: --experimental-wasm-typed-funcref --allow-natives-syntax
|
||||
// Flags: --experimental-wasm-typed-funcref --experimental-wasm-type-reflection
|
||||
// Flags: --no-wasm-tier-up --wasm-dynamic-tiering --wasm-tiering-budget=100
|
||||
// Flags: --allow-natives-syntax
|
||||
|
||||
// These tests check if functions are speculatively inlined as expected. We do
|
||||
// not check automatically which functions are inlined. To get more insight, run
|
||||
@ -70,6 +71,7 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
|
||||
.exportAs("main");
|
||||
|
||||
let instance = builder.instantiate();
|
||||
|
||||
// Run 'main' until it is tiered-up.
|
||||
while (%IsLiftoffFunction(instance.exports.main)) {
|
||||
assertEquals(14, instance.exports.main(10, 1));
|
||||
@ -184,8 +186,50 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
|
||||
|
||||
// Run 'main' until it is tiered-up.
|
||||
while (%IsLiftoffFunction(instance2.exports.main)) {
|
||||
instance2.exports.main(0, instance1.exports.f1);
|
||||
assertEquals(1, instance2.exports.main(0, instance1.exports.f1));
|
||||
}
|
||||
// The function f1 defined in another module should not be inlined.
|
||||
instance2.exports.main(0, instance1.exports.f1);
|
||||
assertEquals(1, instance2.exports.main(0, instance1.exports.f1));
|
||||
})();
|
||||
|
||||
// Check that we handle WasmJSFunctions properly and do not inline them, both
|
||||
// in the monomorphic and polymorphic case.
|
||||
(function CallRefWasmJsFunction() {
|
||||
print(arguments.callee.name);
|
||||
|
||||
let f1 = new WebAssembly.Function({parameters: ["i32"], results: ["i32"]},
|
||||
x => x + 1);
|
||||
let f2 = new WebAssembly.Function({parameters: ["i32"], results: ["i32"]},
|
||||
x => x * 2);
|
||||
|
||||
let instance2 = function() {
|
||||
let builder = new WasmModuleBuilder();
|
||||
|
||||
let sig = builder.addType(kSig_i_i);
|
||||
|
||||
builder.addFunction("main", makeSig(
|
||||
[kWasmI32, wasmRefType(sig), wasmRefType(sig)], [kWasmI32]))
|
||||
.addBody([kExprLocalGet, 0, kExprLocalGet, 1, kExprCallRef,
|
||||
kExprLocalGet, 0, kExprLocalGet, 2, kExprCallRef,
|
||||
kExprI32Add])
|
||||
.exportFunc();
|
||||
|
||||
return builder.instantiate({});
|
||||
}();
|
||||
|
||||
var i = 0;
|
||||
// Run 'main' until it is tiered-up. The first argument should try to be
|
||||
// spec-inlined monomorphically. We pass f2 to the second argument 80% of the
|
||||
// time, so it should try to be spec-inlined polymorphically.
|
||||
while (%IsLiftoffFunction(instance2.exports.main)) {
|
||||
if (i % 5 == 0) {
|
||||
assertEquals(12, instance2.exports.main(5, f1, f1));
|
||||
} else {
|
||||
assertEquals(16, instance2.exports.main(5, f1, f2));
|
||||
}
|
||||
i++;
|
||||
}
|
||||
// WebAssembly.Function objects should not be inlined.
|
||||
assertEquals(16, instance2.exports.main(5, f1, f2));
|
||||
assertEquals(12, instance2.exports.main(5, f1, f1));
|
||||
})();
|
||||
|
Loading…
Reference in New Issue
Block a user