[WASM SIMD] Implement horizontal add for float and integer types.

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

 // Copyright 2014 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.
 
 #include "src/compiler/code-generator.h"
 
 #include "src/arm/macro-assembler-arm.h"
 #include "src/assembler-inl.h"
 #include "src/compilation-info.h"
 #include "src/compiler/code-generator-impl.h"
@@ skipping 1593 matching lines @@
     }
     case kArmF32x4RecipSqrtApprox: {
       __ vrsqrte(i.OutputSimd128Register(), i.InputSimd128Register(0));
       break;
     }
     case kArmF32x4Add: {
       __ vadd(i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
+    case kArmF32x4AddHoriz: {
+      Simd128Register dst = i.OutputSimd128Register(),
+                      src0 = i.InputSimd128Register(0),
+                      src1 = i.InputSimd128Register(1);
+      // Make sure we don't overwrite source data before it's used.
+      if (dst.is(src0)) {
+        __ vpadd(dst.low(), src0.low(), src0.high());

[georgia.kouveli, 2017/04/20 14:53:06]

The inputs to all the vpadd instructions seem to be in the wrong order,
shouldn't it be high first and low second? (VPADD is not commutative).

The RunF32x4HorizOpTest and other tests will not catch this, as the two inputs
have splatted values, which means the two pairs in dst.high() will have the same
result and the two pairs in dst.low() will also have the same result. It would
help to update the test to have a different expected value in each pair, this
way you would catch any such issues.

[bbudge, 2017/04/21 20:18:58]

I changed the tests to use the permutation test machinery, which assigns unique
(integral) values to each lane of each vector. This eliminates a lot of code and
results in better tests (no more splatted inputs.)

I think there's some confusion in the assembler calls - vpadd stores the first
source operand as Vn, and the second source operand as Vm, which is flipped from
the normal assembler syntax. I'm not sure what to do about that though, as this
matches a lot of existing NEON assembler code. Perhaps another CL to straighten
out the confusion?

+        if (dst.is(src1)) {
+          __ vmov(dst.high(), dst.low());
+        } else {
+          __ vpadd(dst.high(), src1.low(), src1.high());
+        }
+      } else {
+        __ vpadd(dst.high(), src1.low(), src1.high());
+        __ vpadd(dst.low(), src0.low(), src0.high());
+      }
+      break;
+    }
     case kArmF32x4Sub: {
       __ vsub(i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
     case kArmF32x4Mul: {
       __ vmul(i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
@@ skipping 78 matching lines @@
     case kArmI32x4ShrS: {
       __ vshr(NeonS32, i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputInt5(1));
       break;
     }
     case kArmI32x4Add: {
       __ vadd(Neon32, i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
+    case kArmI32x4AddHoriz: {
+      Simd128Register dst = i.OutputSimd128Register(),
+                      src0 = i.InputSimd128Register(0),
+                      src1 = i.InputSimd128Register(1);
+      // Make sure we don't overwrite source data before it's used.

[georgia.kouveli, 2017/04/20 14:53:06]

Might make sense to factor out this code, since it's repeated for all types.

[bbudge, 2017/04/21 20:18:58]

Done. (And I did the same for the repetitious narrowing integer conversions
too.)

+      if (dst.is(src0)) {
+        __ vpadd(Neon32, dst.low(), src0.low(), src0.high());
+        if (dst.is(src1)) {
+          __ vmov(dst.high(), dst.low());
+        } else {
+          __ vpadd(Neon32, dst.high(), src1.low(), src1.high());
+        }
+      } else {
+        __ vpadd(Neon32, dst.high(), src1.low(), src1.high());
+        __ vpadd(Neon32, dst.low(), src0.low(), src0.high());
+      }
+      break;
+    }
     case kArmI32x4Sub: {
       __ vsub(Neon32, i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
     case kArmI32x4Mul: {
       __ vmul(Neon32, i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
@@ skipping 128 matching lines @@
     case kArmI16x8Add: {
       __ vadd(Neon16, i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
     case kArmI16x8AddSaturateS: {
       __ vqadd(NeonS16, i.OutputSimd128Register(), i.InputSimd128Register(0),
                i.InputSimd128Register(1));
       break;
     }
+    case kArmI16x8AddHoriz: {
+      Simd128Register dst = i.OutputSimd128Register(),
+                      src0 = i.InputSimd128Register(0),
+                      src1 = i.InputSimd128Register(1);
+      // Make sure we don't overwrite source data before it's used.
+      if (dst.is(src0)) {
+        __ vpadd(Neon16, dst.low(), src0.low(), src0.high());
+        if (dst.is(src1)) {
+          __ vmov(dst.high(), dst.low());
+        } else {
+          __ vpadd(Neon16, dst.high(), src1.low(), src1.high());
+        }
+      } else {
+        __ vpadd(Neon16, dst.high(), src1.low(), src1.high());
+        __ vpadd(Neon16, dst.low(), src0.low(), src0.high());
+      }
+      break;
+    }
     case kArmI16x8Sub: {
       __ vsub(Neon16, i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
     case kArmI16x8SubSaturateS: {
       __ vqsub(NeonS16, i.OutputSimd128Register(), i.InputSimd128Register(0),
                i.InputSimd128Register(1));
       break;
     }
@@ skipping 148 matching lines @@
     case kArmI8x16Add: {
       __ vadd(Neon8, i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
     case kArmI8x16AddSaturateS: {
       __ vqadd(NeonS8, i.OutputSimd128Register(), i.InputSimd128Register(0),
                i.InputSimd128Register(1));
       break;
     }
+    case kArmI8x16AddHoriz: {
+      Simd128Register dst = i.OutputSimd128Register(),
+                      src0 = i.InputSimd128Register(0),
+                      src1 = i.InputSimd128Register(1);
+      // Make sure we don't overwrite source data before it's used.
+      if (dst.is(src0)) {
+        __ vpadd(Neon8, dst.low(), src0.low(), src0.high());
+        if (dst.is(src1)) {
+          __ vmov(dst.high(), dst.low());
+        } else {
+          __ vpadd(Neon8, dst.high(), src1.low(), src1.high());
+        }
+      } else {
+        __ vpadd(Neon8, dst.high(), src1.low(), src1.high());
+        __ vpadd(Neon8, dst.low(), src0.low(), src0.high());
+      }
+      break;
+    }
     case kArmI8x16Sub: {
       __ vsub(Neon8, i.OutputSimd128Register(), i.InputSimd128Register(0),
               i.InputSimd128Register(1));
       break;
     }
     case kArmI8x16SubSaturateS: {
       __ vqsub(NeonS8, i.OutputSimd128Register(), i.InputSimd128Register(0),
                i.InputSimd128Register(1));
       break;
     }
@@ skipping 1124 matching lines @@
       padding_size -= v8::internal::Assembler::kInstrSize;
     }
   }
 }
 
 #undef __
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8