[arm] Fix off-by-1 issue with stack returns

On 32-bit platforms, float64 stack returns take 2 stack slots. In the
implemention of the kArmPeek instruction we assume that provided slot
index points to the first stack slot. However, due to an off-by-1 issue
the provided slot index pointed to the second stack slot. This CL fixes
the problem and generalizes an existing test which reproduces it.

R=v8-arm-ports@googlegroups.com

Change-Id: Ibb2fd8275cf912da064e2f863c2d64d2526caaac
Reviewed-on: https://chromium-review.googlesource.com/839761Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50291}

src/compiler/arm/instruction-selector-arm.cc

@@ -1578,17 +1578,22 @@ void InstructionSelector::EmitPrepareResults(ZoneVector<PushParameter>* results,
   int reverse_slot = 0;
   for (PushParameter output : *results) {
     if (!output.location.IsCallerFrameSlot()) continue;
-    reverse_slot += output.location.GetSizeInPointers();
+    ++reverse_slot;
     // Skip any alignment holes in nodes.
-    if (output.node == nullptr) continue;
-    DCHECK(!descriptor->IsCFunctionCall());
-    if (output.location.GetType() == MachineType::Float32()) {
-      MarkAsFloat32(output.node);
-    } else if (output.location.GetType() == MachineType::Float64()) {
-      MarkAsFloat64(output.node);
+    if (output.node != nullptr) {
+      DCHECK(!descriptor->IsCFunctionCall());
+      if (output.location.GetType() == MachineType::Float32()) {
+        MarkAsFloat32(output.node);
+      } else if (output.location.GetType() == MachineType::Float64()) {
+        MarkAsFloat64(output.node);
+      }
+      InstructionOperand result = g.DefineAsRegister(output.node);
+      Emit(kArmPeek | MiscField::encode(reverse_slot), result);
+    }
+    if (output.location.GetType() == MachineType::Float64()) {
+      // Float64 require an implicit second slot.
+      ++reverse_slot;
     }
-    InstructionOperand result = g.DefineAsRegister(output.node);
-    Emit(kArmPeek | MiscField::encode(reverse_slot), result);
   }
 }
 

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

@@ -477,14 +477,13 @@ TEST(ReturnMultipleRandom) {
   }
 }
 
-TEST(ReturnLastValue) {
+void ReturnLastValue(MachineType type) {
   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;
+  const int return_count = num_registers(type) + 2;
 
-  CallDescriptor* desc =
-      CreateMonoCallDescriptor(&zone, return_count, 0, MachineType::Int32());
+  CallDescriptor* desc = CreateMonoCallDescriptor(&zone, return_count, 0, type);
 
   HandleAndZoneScope handles;
   RawMachineAssembler m(handles.main_isolate(),
@@ -495,7 +494,7 @@ TEST(ReturnLastValue) {
   std::unique_ptr<Node* []> returns(new Node*[return_count]);
 
   for (int i = 0; i < return_count; ++i) {
-    returns[i] = m.Int32Constant(i);
+    returns[i] = Constant(m, type, i);
   }
 
   m.Return(return_count, returns.get());
@@ -511,10 +510,16 @@ TEST(ReturnLastValue) {
 
   Node* call = mt.AddNode(mt.common()->Call(desc), 1, &code_node);
 
-  mt.Return(mt.AddNode(mt.common()->Projection(return_count - 1), call));
+  mt.Return(ToInt32(
+      mt, type, mt.AddNode(mt.common()->Projection(return_count - 1), call)));
 
   CHECK_EQ(expect, mt.Call());
 }
+
+TEST(ReturnLastValueInt32) { ReturnLastValue(MachineType::Int32()); }
+TEST(ReturnLastValueFloat32) { ReturnLastValue(MachineType::Float32()); }
+TEST(ReturnLastValueFloat64) { ReturnLastValue(MachineType::Float64()); }
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8