[cleanup] Remove TODO now that wasm-linkage is fixed

- Removes workarounds in test-run-native_calls for ARM and
  adds ARM 32-bit aliasing-aware register allocation.
- Uses wasm::LinkageAllocator instead of custom allocator to avoid
  duplication of this logic.
- Fixes a problem in wasm::LinkageAllocator with high 16 VFP regs,
  and makes member variable naming consistent.

Bug: v8:8015
Change-Id: Ie8bb8bad06bebce2cef3da0f6ad5c59d5f3b3b36
Reviewed-on: https://chromium-review.googlesource.com/1199907Reviewed-by: Ben Titzer <titzer@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55696}

src/wasm/wasm-linkage.h

@@ -40,6 +40,7 @@ constexpr DoubleRegister kFpReturnRegisters[] = {xmm1, xmm2};
 // ===========================================================================
 constexpr Register kGpParamRegisters[] = {r3, r0, r1, r2};
 constexpr Register kGpReturnRegisters[] = {r0, r1};
+// ARM d-registers must be in ascending order for correct allocation.
 constexpr DoubleRegister kFpParamRegisters[] = {d0, d1, d2, d3, d4, d5, d6, d7};
 constexpr DoubleRegister kFpReturnRegisters[] = {d0, d1};
 
@@ -129,9 +130,11 @@ class LinkageAllocator {
 #if V8_TARGET_ARCH_ARM
     switch (rep) {
       case MachineRepresentation::kFloat32:
-        return extra_float_reg >= 0 || fp_offset_ < fp_count_;
+        return extra_float_reg_ >= 0 ||
+               (extra_double_reg_ >= 0 && extra_double_reg_ < 16) ||
+               (fp_offset_ < fp_count_ && fp_regs_[fp_offset_].code() < 16);
       case MachineRepresentation::kFloat64:
-        return extra_double_reg >= 0 || fp_offset_ < fp_count_;
+        return extra_double_reg_ >= 0 || fp_offset_ < fp_count_;
       case MachineRepresentation::kSimd128:
         return ((fp_offset_ + 1) & ~1) + 1 < fp_count_;
       default:
@@ -151,10 +154,10 @@ class LinkageAllocator {
 #if V8_TARGET_ARCH_ARM
     switch (rep) {
       case MachineRepresentation::kFloat32: {
-        // Use the extra S-register if we can.
-        if (extra_float_reg >= 0) {
-          int reg_code = extra_float_reg;
-          extra_float_reg = -1;
+        // Use the extra S-register if there is one.
+        if (extra_float_reg_ >= 0) {
+          int reg_code = extra_float_reg_;
+          extra_float_reg_ = -1;
           return reg_code;
         }
         // Allocate a D-register and split into 2 float registers.
@@ -162,15 +165,15 @@ class LinkageAllocator {
         DCHECK_GT(16, d_reg_code);  // D-registers 16 - 31 can't split.
         int reg_code = d_reg_code * 2;
         // Save the extra S-register.
-        DCHECK_EQ(-1, extra_float_reg);
-        extra_float_reg = reg_code + 1;
+        DCHECK_EQ(-1, extra_float_reg_);
+        extra_float_reg_ = reg_code + 1;
         return reg_code;
       }
       case MachineRepresentation::kFloat64: {
-        // Use an extra D-register if we can.
-        if (extra_double_reg >= 0) {
-          int reg_code = extra_double_reg;
-          extra_double_reg = -1;
+        // Use the extra D-register if there is one.
+        if (extra_double_reg_ >= 0) {
+          int reg_code = extra_double_reg_;
+          extra_double_reg_ = -1;
           return reg_code;
         }
         DCHECK_LT(fp_offset_, fp_count_);
@@ -178,16 +181,16 @@ class LinkageAllocator {
       }
       case MachineRepresentation::kSimd128: {
         // Q-register must be an even-odd pair, so we must try to allocate at
-        // the end, not using extra_double_reg. If we are at an odd D-register,
-        // skip past it (saving it to extra_double_reg).
+        // the end, not using extra_double_reg_. If we are at an odd D-register,
+        // skip past it (saving it to extra_double_reg_).
         DCHECK_LT(((fp_offset_ + 1) & ~1) + 1, fp_count_);
         int d_reg1_code = fp_regs_[fp_offset_++].code();
         if (d_reg1_code % 2 != 0) {
-          // If we're misaligned then extra_double_reg must have been consumed.
-          DCHECK_EQ(-1, extra_double_reg);
+          // If we're misaligned then extra_double_reg_ must have been consumed.
+          DCHECK_EQ(-1, extra_double_reg_);
           int odd_double_reg = d_reg1_code;
           d_reg1_code = fp_regs_[fp_offset_++].code();
-          extra_double_reg = odd_double_reg;
+          extra_double_reg_ = odd_double_reg;
         }
         // Combine the current D-register with the next to form a Q-register.
         int d_reg2_code = fp_regs_[fp_offset_++].code();
@@ -244,8 +247,8 @@ class LinkageAllocator {
   // ARM FP register aliasing may require splitting or merging double registers.
   // Track fragments of registers below fp_offset_ here. There can only be one
   // extra float and double register.
-  int extra_float_reg = -1;
-  int extra_double_reg = -1;
+  int extra_float_reg_ = -1;
+  int extra_double_reg_ = -1;
 #endif
 
   int stack_offset_ = 0;

test/cctest/compiler/test-run-native-calls.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <vector>
+
 #include "src/assembler.h"
 #include "src/codegen.h"
 #include "src/compiler/linkage.h"
@@ -9,6 +11,7 @@
 #include "src/machine-type.h"
 #include "src/objects-inl.h"
 #include "src/register-configuration.h"
+#include "src/wasm/wasm-linkage.h"
 
 #include "test/cctest/cctest.h"
 #include "test/cctest/compiler/codegen-tester.h"
@@ -84,21 +87,12 @@ class RegisterPairs : public Pairs {
               GetRegConfig()->allocatable_general_codes()) {}
 };
 
-
-// Pairs of double registers.
+// Pairs of float registers.
 class Float32RegisterPairs : public Pairs {
  public:
   Float32RegisterPairs()
-      : Pairs(
-            100,
-#if V8_TARGET_ARCH_ARM
-            // TODO(bbudge) Modify wasm linkage to allow use of all float regs.
-            GetRegConfig()->num_allocatable_double_registers() / 2 - 2,
-#else
-            GetRegConfig()->num_allocatable_double_registers(),
-#endif
-            GetRegConfig()->allocatable_double_codes()) {
-  }
+      : Pairs(100, GetRegConfig()->num_allocatable_float_registers(),
+              GetRegConfig()->allocatable_float_codes()) {}
 };
 
 
@@ -112,48 +106,39 @@ class Float64RegisterPairs : public Pairs {
 
 
 // Helper for allocating either an GP or FP reg, or the next stack slot.
-struct Allocator {
-  Allocator(int* gp, int gpc, int* fp, int fpc)
-      : gp_count(gpc),
-        gp_offset(0),
-        gp_regs(gp),
-        fp_count(fpc),
-        fp_offset(0),
-        fp_regs(fp),
-        stack_offset(0) {}
-
-  int gp_count;
-  int gp_offset;
-  int* gp_regs;
-
-  int fp_count;
-  int fp_offset;
-  int* fp_regs;
-
-  int stack_offset;
+class Allocator {
+ public:
+  Allocator(int* gp, int gpc, int* fp, int fpc) : stack_offset_(0) {
+    for (int i = 0; i < gpc; ++i) {
+      gp_.push_back(Register::from_code(gp[i]));
+    }
+    for (int i = 0; i < fpc; ++i) {
+      fp_.push_back(DoubleRegister::from_code(fp[i]));
+    }
+    Reset();
+  }
+
+  int stack_offset() const { return stack_offset_; }
 
   LinkageLocation Next(MachineType type) {
     if (IsFloatingPoint(type.representation())) {
       // Allocate a floating point register/stack location.
-      if (fp_offset < fp_count) {
-        int code = fp_regs[fp_offset++];
-#if V8_TARGET_ARCH_ARM
-        // TODO(bbudge) Modify wasm linkage to allow use of all float regs.
-        if (type.representation() == MachineRepresentation::kFloat32) code *= 2;
-#endif
+      if (reg_allocator_->CanAllocateFP(type.representation())) {
+        int code = reg_allocator_->NextFpReg(type.representation());
         return LinkageLocation::ForRegister(code, type);
       } else {
-        int offset = -1 - stack_offset;
-        stack_offset += StackWords(type);
+        int offset = -1 - stack_offset_;
+        stack_offset_ += StackWords(type);
         return LinkageLocation::ForCallerFrameSlot(offset, type);
       }
     } else {
       // Allocate a general purpose register/stack location.
-      if (gp_offset < gp_count) {
-        return LinkageLocation::ForRegister(gp_regs[gp_offset++], type);
+      if (reg_allocator_->CanAllocateGP()) {
+        int code = reg_allocator_->NextGpReg();
+        return LinkageLocation::ForRegister(code, type);
       } else {
-        int offset = -1 - stack_offset;
-        stack_offset += StackWords(type);
+        int offset = -1 - stack_offset_;
+        stack_offset_ += StackWords(type);
         return LinkageLocation::ForCallerFrameSlot(offset, type);
       }
     }
@@ -163,10 +148,17 @@ struct Allocator {
     return size <= kPointerSize ? 1 : size / kPointerSize;
   }
   void Reset() {
-    fp_offset = 0;
-    gp_offset = 0;
-    stack_offset = 0;
+    stack_offset_ = 0;
+    reg_allocator_.reset(
+        new wasm::LinkageAllocator(gp_.data(), static_cast<int>(gp_.size()),
+                                   fp_.data(), static_cast<int>(fp_.size())));
   }
+
+ private:
+  std::vector<Register> gp_;
+  std::vector<DoubleRegister> fp_;
+  std::unique_ptr<wasm::LinkageAllocator> reg_allocator_;
+  int stack_offset_;
 };
 
 
@@ -197,7 +189,7 @@ class RegisterConfig {
 
     MachineType target_type = MachineType::AnyTagged();
     LinkageLocation target_loc = LinkageLocation::ForAnyRegister();
-    int stack_param_count = params.stack_offset;
+    int stack_param_count = params.stack_offset();
     return new (zone) CallDescriptor(       // --
         CallDescriptor::kCallCodeObject,    // kind
         target_type,                        // target MachineType
@@ -868,7 +860,7 @@ TEST(Float32Select_registers) {
     return;
   }
 
-  int rarray[] = {GetRegConfig()->GetAllocatableDoubleCode(0)};
+  int rarray[] = {GetRegConfig()->GetAllocatableFloatCode(0)};
   ArgsBuffer<float32>::Sig sig(2);
 
   Float32RegisterPairs pairs;
@@ -912,7 +904,7 @@ TEST(Float64Select_registers) {
 
 
 TEST(Float32Select_stack_params_return_reg) {
-  int rarray[] = {GetRegConfig()->GetAllocatableDoubleCode(0)};
+  int rarray[] = {GetRegConfig()->GetAllocatableFloatCode(0)};
   Allocator params(nullptr, 0, nullptr, 0);
   Allocator rets(nullptr, 0, rarray, 1);
   RegisterConfig config(params, rets);