[wasm][multi-return] Fix problem with unused stack returns

There was an issue when the caller of a function with multiple returns
did not use all values which were returned over the stack. The caller
used only the used returns to calculate the offsets on the stack,
whereas the callee used all returns to calculate the offsets.

With this CL also the caller uses all returns to calculate the stack
offsets and thereby agrees again with the callee on the location of
all returns.

In addition I fixed an issue on x64: A quad word is reserved on the
stack frame to spill callee-saved FP registers, which is not pointer
size.

R=titzer@chromium.org

Change-Id: Ibe56b4b57e4b6e59071a868805b1237412344f93
Reviewed-on: https://chromium-review.googlesource.com/824043Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50193}

src/compiler/instruction-selector.cc

@@ -693,20 +693,27 @@ void InstructionSelector::InitializeCallBuffer(Node* call, CallBuffer* buffer,
                                                bool is_tail_call,
                                                int stack_param_delta) {
   OperandGenerator g(this);
-  DCHECK_LE(call->op()->ValueOutputCount(),
-            static_cast<int>(buffer->descriptor->ReturnCount()));
+  size_t ret_count = buffer->descriptor->ReturnCount();
+  DCHECK_LE(call->op()->ValueOutputCount(), ret_count);
   DCHECK_EQ(
       call->op()->ValueInputCount(),
       static_cast<int>(buffer->input_count() + buffer->frame_state_count()));
 
-  if (buffer->descriptor->ReturnCount() > 0) {
+  if (ret_count > 0) {
     // Collect the projections that represent multiple outputs from this call.
-    if (buffer->descriptor->ReturnCount() == 1) {
+    if (ret_count == 1) {
       PushParameter result = {call, buffer->descriptor->GetReturnLocation(0)};
       buffer->output_nodes.push_back(result);
     } else {
-      buffer->output_nodes.resize(buffer->descriptor->ReturnCount());
+      buffer->output_nodes.resize(ret_count);
       int stack_count = 0;
+      for (size_t i = 0; i < ret_count; ++i) {
+        LinkageLocation location = buffer->descriptor->GetReturnLocation(i);
+        buffer->output_nodes[i] = PushParameter(nullptr, location);
+        if (location.IsCallerFrameSlot()) {
+          stack_count += location.GetSizeInPointers();
+        }
+      }
       for (Edge const edge : call->use_edges()) {
         if (!NodeProperties::IsValueEdge(edge)) continue;
         Node* node = edge.from();
@@ -715,13 +722,7 @@ void InstructionSelector::InitializeCallBuffer(Node* call, CallBuffer* buffer,
 
         DCHECK_LT(index, buffer->output_nodes.size());
         DCHECK(!buffer->output_nodes[index].node);
-        PushParameter result = {node,
-                                buffer->descriptor->GetReturnLocation(index)};
-        buffer->output_nodes[index] = result;
-
-        if (result.location.IsCallerFrameSlot()) {
-          stack_count += result.location.GetSizeInPointers();
-        }
+        buffer->output_nodes[index].node = node;
       }
       frame_->EnsureReturnSlots(stack_count);
     }

src/compiler/x64/code-generator-x64.cc

@@ -3089,7 +3089,8 @@ void CodeGenerator::AssembleConstructFrame() {
 
     // Skip callee-saved and return slots, which are created below.
     shrink_slots -= base::bits::CountPopulation(saves);
-    shrink_slots -= base::bits::CountPopulation(saves_fp);
+    shrink_slots -=
+        base::bits::CountPopulation(saves_fp) * (kQuadWordSize / kPointerSize);
     shrink_slots -= frame()->GetReturnSlotCount();
     if (shrink_slots > 0) {
       __ subq(rsp, Immediate(shrink_slots * kPointerSize));

test/cctest/compiler/test-multiple-return.cc

@@ -477,6 +477,44 @@ TEST(ReturnMultipleRandom) {
   }
 }
 
+TEST(ReturnLastValue) {
+  v8::internal::AccountingAllocator allocator;
+  Zone zone(&allocator, ZONE_NAME);
+  // Let 2 returns be on the stack.
+  const int return_count = num_registers(MachineType::Int32()) + 2;
+
+  CallDescriptor* desc =
+      CreateMonoCallDescriptor(&zone, return_count, 0, MachineType::Int32());
+
+  HandleAndZoneScope handles;
+  RawMachineAssembler m(handles.main_isolate(),
+                        new (handles.main_zone()) Graph(handles.main_zone()),
+                        desc, MachineType::PointerRepresentation(),
+                        InstructionSelector::SupportedMachineOperatorFlags());
+
+  std::unique_ptr<Node* []> returns(new Node*[return_count]);
+
+  for (int i = 0; i < return_count; ++i) {
+    returns[i] = m.Int32Constant(i);
+  }
+
+  m.Return(return_count, returns.get());
+
+  CompilationInfo info(ArrayVector("testing"), handles.main_zone(), Code::STUB);
+  Handle<Code> code = Pipeline::GenerateCodeForTesting(
+      &info, handles.main_isolate(), desc, m.graph(), m.Export());
+
+  // Generate caller.
+  int expect = return_count - 1;
+  RawMachineAssemblerTester<int32_t> mt;
+  Node* code_node = mt.HeapConstant(code);
+
+  Node* call = mt.AddNode(mt.common()->Call(desc), 1, &code_node);
+
+  mt.Return(mt.AddNode(mt.common()->Projection(return_count - 1), call));
+
+  CHECK_EQ(expect, mt.Call());
+}
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8