Revert "[wasm-simd] Fix scalar lowering of kParameter"
This reverts commit e8832647b6
.
Reason for revert: Causes flaky fails on the tree, reverting as this test should be deterministic pass/fail.
https://logs.chromium.org/logs/v8/buildbucket/cr-buildbucket.appspot.com/8889903130443940000/+/steps/Check_-_nosse3__flakes_/0/logs/simd-call/0
Original change's description:
> [wasm-simd] Fix scalar lowering of kParameter
>
> Lowers the call descriptor of a wasm function if it contains simd.
>
> Also fixes a couple of issues with the lowering of kParameter:
> - the old_index == new_index check is incorrect, it would only work if
> the s128 parameter is the first parameter
> - the old_index was also not adjusted to account for Parameter[0] being
> the wasm instance object
> - new_index needs to be adjusted to account for the instance object too
>
> These fixes make it more similar to the lowering of kParameter in
> int64-lowering.c.
>
> Also add a new mjsunit test to exercise this logic.
>
> Bug: v8:10154
> Change-Id: Ia767a464c26a6a78fd931eab9e6897890a0904e8
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2020521
> Commit-Queue: Zhi An Ng <zhin@chromium.org>
> Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#66032}
TBR=gdeepti@chromium.org,ahaas@chromium.org,zhin@chromium.org
Change-Id: I69589e2331c857c0f197ac53b8fb8a241376c632
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:10154
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2028830
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66034}
This commit is contained in:
parent
8580537587
commit
1b5a3178f8
@ -909,36 +909,29 @@ void SimdScalarLowering::LowerNode(Node* node) {
|
|||||||
}
|
}
|
||||||
case IrOpcode::kParameter: {
|
case IrOpcode::kParameter: {
|
||||||
DCHECK_EQ(1, node->InputCount());
|
DCHECK_EQ(1, node->InputCount());
|
||||||
int param_count = static_cast<int>(signature()->parameter_count());
|
|
||||||
// Only exchange the node if the parameter count actually changed. We do
|
// Only exchange the node if the parameter count actually changed. We do
|
||||||
// not even have to do the default lowering because the start node,
|
// not even have to do the default lowering because the the start node,
|
||||||
// the only input of a parameter node, only changes if the parameter count
|
// the only input of a parameter node, only changes if the parameter count
|
||||||
// changes.
|
// changes.
|
||||||
if (GetParameterCountAfterLowering() != param_count) {
|
if (GetParameterCountAfterLowering() !=
|
||||||
|
static_cast<int>(signature()->parameter_count())) {
|
||||||
int old_index = ParameterIndexOf(node->op());
|
int old_index = ParameterIndexOf(node->op());
|
||||||
// Parameter index 0 is the instance parameter, we will use old_index to
|
|
||||||
// index into the function signature, so we need to decrease it by 1.
|
|
||||||
--old_index;
|
|
||||||
int new_index =
|
int new_index =
|
||||||
GetParameterIndexAfterLoweringSimd128(signature(), old_index);
|
GetParameterIndexAfterLoweringSimd128(signature(), old_index);
|
||||||
// Similarly, the index into function signature needs to account for the
|
if (old_index == new_index) {
|
||||||
// instance parameter, so increase it by 1.
|
NodeProperties::ChangeOp(node, common()->Parameter(new_index));
|
||||||
++new_index;
|
|
||||||
NodeProperties::ChangeOp(node, common()->Parameter(new_index));
|
|
||||||
|
|
||||||
if (old_index < 0) {
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
DCHECK(old_index < param_count);
|
|
||||||
|
|
||||||
if (signature()->GetParam(old_index) ==
|
|
||||||
MachineRepresentation::kSimd128) {
|
|
||||||
Node* new_node[kNumLanes32];
|
Node* new_node[kNumLanes32];
|
||||||
|
for (int i = 0; i < kNumLanes32; ++i) {
|
||||||
|
new_node[i] = nullptr;
|
||||||
|
}
|
||||||
new_node[0] = node;
|
new_node[0] = node;
|
||||||
for (int i = 1; i < kNumLanes32; ++i) {
|
if (signature()->GetParam(old_index) ==
|
||||||
new_node[i] = graph()->NewNode(common()->Parameter(new_index + i),
|
MachineRepresentation::kSimd128) {
|
||||||
graph()->start());
|
for (int i = 1; i < kNumLanes32; ++i) {
|
||||||
|
new_node[i] = graph()->NewNode(common()->Parameter(new_index + i),
|
||||||
|
graph()->start());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
ReplaceNode(node, new_node, kNumLanes32);
|
ReplaceNode(node, new_node, kNumLanes32);
|
||||||
}
|
}
|
||||||
|
@ -6932,11 +6932,6 @@ wasm::WasmCompilationResult ExecuteTurbofanWasmCompilation(
|
|||||||
call_descriptor = GetI32WasmCallDescriptor(&zone, call_descriptor);
|
call_descriptor = GetI32WasmCallDescriptor(&zone, call_descriptor);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (ContainsSimd(func_body.sig) &&
|
|
||||||
(!CpuFeatures::SupportsWasmSimd128() || env->lower_simd)) {
|
|
||||||
call_descriptor = GetI32WasmCallDescriptorForSimd(&zone, call_descriptor);
|
|
||||||
}
|
|
||||||
|
|
||||||
Pipeline::GenerateCodeForWasmFunction(
|
Pipeline::GenerateCodeForWasmFunction(
|
||||||
&info, wasm_engine, mcgraph, call_descriptor, source_positions,
|
&info, wasm_engine, mcgraph, call_descriptor, source_positions,
|
||||||
node_origins, func_body, env->module, func_index);
|
node_origins, func_body, env->module, func_index);
|
||||||
|
@ -1,66 +0,0 @@
|
|||||||
// Copyright 2020 the V8 project authors. All rights reserved.
|
|
||||||
// Use of this source code is governed by a BSD-style license that can be
|
|
||||||
// found in the LICENSE file.
|
|
||||||
|
|
||||||
// Flags: --experimental-wasm-simd
|
|
||||||
|
|
||||||
load('test/mjsunit/wasm/wasm-module-builder.js');
|
|
||||||
|
|
||||||
// Tests function calls containing s128 parameters. It also checks that
|
|
||||||
// lowering of simd calls are correct, so we create 2 functions with s128
|
|
||||||
// arguments: function 2 has a single s128 parameter, function 3 has a i32 then
|
|
||||||
// s128, to ensure that the arguments in different indices are correctly lowered.
|
|
||||||
(function TestSimd128Params() {
|
|
||||||
const builder = new WasmModuleBuilder();
|
|
||||||
builder.addImportedMemory('m', 'imported_mem', 1, 2);
|
|
||||||
|
|
||||||
builder
|
|
||||||
.addFunction("main", makeSig([], []))
|
|
||||||
.addBodyWithEnd([
|
|
||||||
kExprI32Const, 0,
|
|
||||||
kSimdPrefix, kExprS128LoadMem, 0, 0,
|
|
||||||
kExprCallFunction, 0x01,
|
|
||||||
kExprEnd,
|
|
||||||
]);
|
|
||||||
|
|
||||||
// Writes s128 argument to memory starting byte 16.
|
|
||||||
builder
|
|
||||||
.addFunction("function2", makeSig([kWasmS128], []))
|
|
||||||
.addBodyWithEnd([
|
|
||||||
kExprI32Const, 16,
|
|
||||||
kExprLocalGet, 0,
|
|
||||||
kSimdPrefix, kExprS128StoreMem, 0, 0,
|
|
||||||
kExprI32Const, 9, // This constant doesn't matter.
|
|
||||||
kExprLocalGet, 0,
|
|
||||||
kExprCallFunction, 0x02,
|
|
||||||
kExprEnd,
|
|
||||||
]);
|
|
||||||
|
|
||||||
// Writes s128 argument to memory starting byte 32.
|
|
||||||
builder
|
|
||||||
.addFunction("function3", makeSig([kWasmI32, kWasmS128], []))
|
|
||||||
.addBodyWithEnd([
|
|
||||||
kExprI32Const, 32,
|
|
||||||
kExprLocalGet, 1,
|
|
||||||
kSimdPrefix, kExprS128StoreMem, 0, 0,
|
|
||||||
kExprEnd,
|
|
||||||
]);
|
|
||||||
|
|
||||||
builder.addExport('main', 0);
|
|
||||||
var memory = new WebAssembly.Memory({initial:1, maximum:2});
|
|
||||||
const instance = builder.instantiate({m: {imported_mem: memory}});
|
|
||||||
|
|
||||||
const arr = new Uint8Array(memory.buffer);
|
|
||||||
// Fill the initial memory with some values, this is read by main and passed
|
|
||||||
// as arguments to function2, and then to function3.
|
|
||||||
for (let i = 0; i < 16; i++) {
|
|
||||||
arr[i] = i * 2;
|
|
||||||
}
|
|
||||||
|
|
||||||
instance.exports.main();
|
|
||||||
|
|
||||||
for (let i = 0; i < 16; i++) {
|
|
||||||
assertEquals(arr[i], arr[i+16]);
|
|
||||||
assertEquals(arr[i], arr[i+32]);
|
|
||||||
}
|
|
||||||
})();
|
|
Loading…
Reference in New Issue
Block a user