[cctest] Clarify that tests for sync instructions are simulator specific

Some tests were recently added to test-simulator-arm.cc, however this file is
meant for tests that are specific to the simulator and therefore are not written
to work on hardware. While this sounds surprising, the reason is that our simulation
of synchronisation instructions is more conservative than on hardware.

To make this more clear, this patch renames the "test-simulator-arm{,64}.cc"
files to "test-sync-primitives-arm{,64}.cc", and moves the vneg and vabs tests
into "test-assembler-arm.cc" which is were tests that are garanteed to work in
either native or simulated environments live.

Finally, take the opportunity to share a little bit of code.

Bug: v8:6963
Change-Id: Ifb85d3671c823b9bba73d09f419536b089a4e87c
Reviewed-on: https://chromium-review.googlesource.com/749387Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Pierre Langlois <pierre.langlois@arm.com>
Cr-Commit-Position: refs/heads/master@{#49073}

test/cctest/BUILD.gn

@@ -249,6 +249,8 @@ v8_source_set("cctest_sources") {
 
   if (v8_current_cpu == "arm") {
     sources += [  ### gcmole(arch:arm) ###
+      "assembler-helper-arm.cc",
+      "assembler-helper-arm.h",
       "test-assembler-arm.cc",
       "test-code-stubs-arm.cc",
       "test-code-stubs.cc",
@@ -256,7 +258,7 @@ v8_source_set("cctest_sources") {
       "test-disasm-arm.cc",
       "test-macro-assembler-arm.cc",
       "test-run-wasm-relocation-arm.cc",
-      "test-simulator-arm.cc",
+      "test-sync-primitives-arm.cc",
     ]
   } else if (v8_current_cpu == "arm64") {
     sources += [  ### gcmole(arch:arm64) ###
@@ -269,7 +271,7 @@ v8_source_set("cctest_sources") {
       "test-javascript-arm64.cc",
       "test-js-arm64-variables.cc",
       "test-run-wasm-relocation-arm64.cc",
-      "test-simulator-arm64.cc",
+      "test-sync-primitives-arm64.cc",
       "test-utils-arm64.cc",
       "test-utils-arm64.h",
     ]

test/cctest/assembler-helper-arm.cc

+// Copyright 2017 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 "test/cctest/assembler-helper-arm.h"
+
+#include "src/v8.h"
+#include "test/cctest/cctest.h"
+
+#include "src/isolate-inl.h"
+
+namespace v8 {
+namespace internal {
+
+Address AssembleCode(std::function<void(Assembler&)> assemble) {
+  Isolate* isolate = CcTest::i_isolate();
+  Assembler assm(isolate, nullptr, 0);
+
+  assemble(assm);
+  assm.bx(lr);
+
+  CodeDesc desc;
+  assm.GetCode(isolate, &desc);
+  Handle<Code> code =
+      isolate->factory()->NewCode(desc, Code::STUB, Handle<Code>());
+  if (FLAG_print_code) {
+    code->Print();
+  }
+  return code->entry();
+}
+
+}  // namespace internal
+}  // namespace v8

test/cctest/assembler-helper-arm.h

+// Copyright 2017 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.
+
+#ifndef V8_CCTEST_ASSEMBLER_HELPER_ARM_H_
+#define V8_CCTEST_ASSEMBLER_HELPER_ARM_H_
+
+#include <functional>
+
+#include "src/macro-assembler.h"
+
+namespace v8 {
+namespace internal {
+
+// These function prototypes have 5 arguments since they are used with the
+// CALL_GENERATED_CODE macro.
+typedef Object* (*F_iiiii)(int x, int p1, int p2, int p3, int p4);
+typedef Object* (*F_piiii)(void* p0, int p1, int p2, int p3, int p4);
+typedef Object* (*F_ppiii)(void* p0, void* p1, int p2, int p3, int p4);
+typedef Object* (*F_pppii)(void* p0, void* p1, void* p2, int p3, int p4);
+typedef Object* (*F_ippii)(int p0, void* p1, void* p2, int p3, int p4);
+
+Address AssembleCode(std::function<void(Assembler&)> assemble);
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_CCTEST_ASSEMBLER_HELPER_ARM_H_

test/cctest/cctest.gyp

@@ -244,6 +244,8 @@
       'test-run-wasm-relocation-x64.cc',
     ],
     'cctest_sources_arm': [  ### gcmole(arch:arm) ###
+      'assembler-helper-arm.cc',
+      'assembler-helper-arm.h',
       'test-assembler-arm.cc',
       'test-code-stubs.cc',
       'test-code-stubs.h',
@@ -251,7 +253,7 @@
       'test-disasm-arm.cc',
       'test-macro-assembler-arm.cc',
       'test-run-wasm-relocation-arm.cc',
-      'test-simulator-arm.cc',
+      'test-sync-primitives-arm.cc',
     ],
     'cctest_sources_arm64': [  ### gcmole(arch:arm64) ###
       'test-utils-arm64.cc',
@@ -265,7 +267,7 @@
       'test-javascript-arm64.cc',
       'test-js-arm64-variables.cc',
       'test-run-wasm-relocation-arm64.cc',
-      'test-simulator-arm64.cc',
+      'test-sync-primitives-arm64.cc',
     ],
     'cctest_sources_s390': [  ### gcmole(arch:s390) ###
       'test-assembler-s390.cc',

test/cctest/test-simulator-arm.cc → test/cctest/test-sync-primitives-arm.cc

@@ -26,8 +26,8 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include "src/v8.h"
+#include "test/cctest/assembler-helper-arm.h"
 #include "test/cctest/cctest.h"
-#include "test/cctest/compiler/value-helper.h"
 
 #include "src/arm/simulator-arm.h"
 #include "src/assembler-inl.h"
@@ -38,14 +38,25 @@
 namespace v8 {
 namespace internal {
 
+// These tests rely on the behaviour specific to the simulator so we cannot
+// expect the same results on real hardware. The reason for this is that our
+// simulation of synchronisation primitives is more conservative than the
+// reality.
+// For example:
+//   ldrex r1, [r2] ; Load acquire at address r2; r2 is now marked as exclusive.
+//   ldr r0, [r4]   ; This is a normal load, and at a different address.
+//                  ; However, any memory accesses can potentially clear the
+//                  ; exclusivity (See ARM DDI 0406C.c A3.4.5). This is unlikely
+//                  ; on real hardware but to be conservative, the simulator
+//                  ; always does it.
+//   strex r3, r1, [r2] ; As a result, this will always fail in the simulator
+//                      ; but will likely succeed on hardware.
+#if defined(USE_SIMULATOR)
+
 #ifndef V8_TARGET_LITTLE_ENDIAN
 #error Expected ARM to be little-endian
 #endif
 
-// Define these function prototypes to match JSEntryFunction in execution.cc.
-typedef Object* (*F_iiiii)(int p0, int p1, int p2, int p3, int p4);
-typedef Object* (*F_piiii)(void* p0, int p1, int p2, int p3, int p4);
-
 #define __ assm.
 
 namespace {
@@ -168,22 +179,6 @@ void AssembleMemoryAccess(Assembler* assembler, MemoryAccess access,
   }
 }
 
-Address AssembleCode(std::function<void(Assembler&)> assemble) {
-  Isolate* isolate = CcTest::i_isolate();
-  Assembler assm(isolate, nullptr, 0);
-
-  assemble(assm);
-
-  __ bx(lr);
-
-  CodeDesc desc;
-  assm.GetCode(isolate, &desc);
-  Handle<Code> code =
-      isolate->factory()->NewCode(desc, Code::STUB, Handle<Code>());
-  return code->entry();
-}
-
-#if defined(USE_SIMULATOR)
 void AssembleLoadExcl(Assembler* assembler, MemoryAccess access,
                       Register value_reg, Register addr_reg) {
   DCHECK(access.kind == MemoryAccess::Kind::LoadExcl);
@@ -228,32 +223,9 @@ void TestInvalidateExclusiveAccess(TestData initial_data, MemoryAccess access1,
       break;
   }
 }
-#endif
-
-std::vector<Float32> Float32Inputs() {
-  std::vector<Float32> inputs;
-  FOR_FLOAT32_INPUTS(f) {
-    inputs.push_back(Float32::FromBits(bit_cast<uint32_t>(*f)));
-  }
-  FOR_UINT32_INPUTS(bits) { inputs.push_back(Float32::FromBits(*bits)); }
-  return inputs;
-}
-
-std::vector<Float64> Float64Inputs() {
-  std::vector<Float64> inputs;
-  FOR_FLOAT64_INPUTS(f) {
-    inputs.push_back(Float64::FromBits(bit_cast<uint64_t>(*f)));
-  }
-  FOR_UINT64_INPUTS(bits) { inputs.push_back(Float64::FromBits(*bits)); }
-  return inputs;
-}
 
 }  // namespace
 
-// TODO(rodolph.perfetta@arm.com): Enable this test for native hardware, see
-// http://crbug.com/v8/6963.
-#if defined(USE_SIMULATOR)
-
 TEST(simulator_invalidate_exclusive_access) {
   using Kind = MemoryAccess::Kind;
   using Size = MemoryAccess::Size;
@@ -290,10 +262,10 @@ TEST(simulator_invalidate_exclusive_access) {
                                 0, TestData(7));
 }
 
-#endif  // USE_SIMULATOR
+namespace {
 
-static int ExecuteMemoryAccess(Isolate* isolate, TestData* test_data,
-                               MemoryAccess access) {
+int ExecuteMemoryAccess(Isolate* isolate, TestData* test_data,
+                        MemoryAccess access) {
   HandleScope scope(isolate);
   F_piiii f = FUNCTION_CAST<F_piiii>(AssembleCode([&](Assembler& assm) {
     AssembleMemoryAccess(&assm, access, r0, r2, r1);
@@ -303,6 +275,8 @@ static int ExecuteMemoryAccess(Isolate* isolate, TestData* test_data,
       CALL_GENERATED_CODE(isolate, f, test_data, 0, 0, 0, 0));
 }
 
+}  // namespace
+
 class MemoryAccessThread : public v8::base::Thread {
  public:
   MemoryAccessThread()
@@ -370,10 +344,6 @@ class MemoryAccessThread : public v8::base::Thread {
   v8::Isolate* isolate_;
 };
 
-// TODO(rodolph.perfetta@arm.com): Enable this test for native hardware, see
-// http://crbug.com/v8/6963.
-#if defined(USE_SIMULATOR)
-
 TEST(simulator_invalidate_exclusive_access_threaded) {
   using Kind = MemoryAccess::Kind;
   using Size = MemoryAccess::Size;
@@ -422,87 +392,9 @@ TEST(simulator_invalidate_exclusive_access_threaded) {
   thread.Join();
 }
 
-#endif  // USE_SIMULATOR
-
-TEST(simulator_vabs_32) {
-  Isolate* isolate = CcTest::i_isolate();
-  HandleScope scope(isolate);
-
-  F_iiiii f = FUNCTION_CAST<F_iiiii>(AssembleCode([](Assembler& assm) {
-    __ vmov(s0, r0);
-    __ vabs(s0, s0);
-    __ vmov(r0, s0);
-  }));
-
-  for (Float32 f32 : Float32Inputs()) {
-    Float32 res = Float32::FromBits(reinterpret_cast<uint32_t>(
-        CALL_GENERATED_CODE(isolate, f, f32.get_bits(), 0, 0, 0, 0)));
-    Float32 exp = Float32::FromBits(f32.get_bits() & ~(1 << 31));
-    CHECK_EQ(exp.get_bits(), res.get_bits());
-  }
-}
-
-TEST(simulator_vabs_64) {
-  Isolate* isolate = CcTest::i_isolate();
-  HandleScope scope(isolate);
-
-  F_iiiii f = FUNCTION_CAST<F_iiiii>(AssembleCode([](Assembler& assm) {
-    __ vmov(d0, r0, r1);
-    __ vabs(d0, d0);
-    __ vmov(r1, r0, d0);
-  }));
-
-  for (Float64 f64 : Float64Inputs()) {
-    uint32_t p0 = static_cast<uint32_t>(f64.get_bits());
-    uint32_t p1 = static_cast<uint32_t>(f64.get_bits() >> 32);
-    uint32_t res = reinterpret_cast<uint32_t>(
-        CALL_GENERATED_CODE(isolate, f, p0, p1, 0, 0, 0));
-    Float64 exp = Float64::FromBits(f64.get_bits() & ~(1ull << 63));
-    // We just get back the top word, so only compare that one.
-    CHECK_EQ(exp.get_bits() >> 32, res);
-  }
-}
-
-TEST(simulator_vneg_32) {
-  Isolate* isolate = CcTest::i_isolate();
-  HandleScope scope(isolate);
-
-  F_iiiii f = FUNCTION_CAST<F_iiiii>(AssembleCode([](Assembler& assm) {
-    __ vmov(s0, r0);
-    __ vneg(s0, s0);
-    __ vmov(r0, s0);
-  }));
-
-  for (Float32 f32 : Float32Inputs()) {
-    Float32 res = Float32::FromBits(reinterpret_cast<uint32_t>(
-        CALL_GENERATED_CODE(isolate, f, f32.get_bits(), 0, 0, 0, 0)));
-    Float32 exp = Float32::FromBits(f32.get_bits() ^ (1 << 31));
-    CHECK_EQ(exp.get_bits(), res.get_bits());
-  }
-}
-
-TEST(simulator_vneg_64) {
-  Isolate* isolate = CcTest::i_isolate();
-  HandleScope scope(isolate);
-
-  F_iiiii f = FUNCTION_CAST<F_iiiii>(AssembleCode([](Assembler& assm) {
-    __ vmov(d0, r0, r1);
-    __ vneg(d0, d0);
-    __ vmov(r1, r0, d0);
-  }));
-
-  for (Float64 f64 : Float64Inputs()) {
-    uint32_t p0 = static_cast<uint32_t>(f64.get_bits());
-    uint32_t p1 = static_cast<uint32_t>(f64.get_bits() >> 32);
-    uint32_t res = reinterpret_cast<uint32_t>(
-        CALL_GENERATED_CODE(isolate, f, p0, p1, 0, 0, 0));
-    Float64 exp = Float64::FromBits(f64.get_bits() ^ (1ull << 63));
-    // We just get back the top word, so only compare that one.
-    CHECK_EQ(exp.get_bits() >> 32, res);
-  }
-}
-
 #undef __
 
+#endif  // defined(USE_SIMULATOR)
+
 }  // namespace internal
 }  // namespace v8

test/cctest/test-simulator-arm64.cc → test/cctest/test-sync-primitives-arm64.cc

@@ -36,6 +36,19 @@
 namespace v8 {
 namespace internal {
 
+// These tests rely on the behaviour specific to the simulator so we cannot
+// expect the same results on real hardware. The reason for this is that our
+// simulation of synchronisation primitives is more conservative than the
+// reality.
+// For example:
+//   ldxr x1, [x2] ; Load acquire at address x2; x2 is now marked as exclusive.
+//   ldr x0, [x4]  ; This is a normal load, and at a different address.
+//                 ; However, any memory accesses can potentially clear the
+//                 ; exclusivity (See ARM DDI 0487B.a B2.9.5). This is unlikely
+//                 ; on real hardware but to be conservative, the simulator
+//                 ; always does it.
+//   stxr w3, x1, [x2] ; As a result, this will always fail in the simulator but
+//                     ; will likely succeed on hardware.
 #if defined(USE_SIMULATOR)
 
 #ifndef V8_TARGET_LITTLE_ENDIAN
@@ -80,9 +93,11 @@ struct TestData {
   int dummy;
 };
 
-static void AssembleMemoryAccess(MacroAssembler* assembler, MemoryAccess access,
-                                 Register dest_reg, Register value_reg,
-                                 Register addr_reg) {
+namespace {
+
+void AssembleMemoryAccess(MacroAssembler* assembler, MemoryAccess access,
+                          Register dest_reg, Register value_reg,
+                          Register addr_reg) {
   MacroAssembler& masm = *assembler;
   __ Add(addr_reg, x0, Operand(access.offset));
 
@@ -162,22 +177,22 @@ static void AssembleMemoryAccess(MacroAssembler* assembler, MemoryAccess access,
   }
 }
 
-static void AssembleLoadExcl(MacroAssembler* assembler, MemoryAccess access,
-                             Register value_reg, Register addr_reg) {
+void AssembleLoadExcl(MacroAssembler* assembler, MemoryAccess access,
+                      Register value_reg, Register addr_reg) {
   DCHECK(access.kind == MemoryAccess::Kind::LoadExcl);
   AssembleMemoryAccess(assembler, access, no_reg, value_reg, addr_reg);
 }
 
-static void AssembleStoreExcl(MacroAssembler* assembler, MemoryAccess access,
-                              Register dest_reg, Register value_reg,
-                              Register addr_reg) {
+void AssembleStoreExcl(MacroAssembler* assembler, MemoryAccess access,
+                       Register dest_reg, Register value_reg,
+                       Register addr_reg) {
   DCHECK(access.kind == MemoryAccess::Kind::StoreExcl);
   AssembleMemoryAccess(assembler, access, dest_reg, value_reg, addr_reg);
 }
 
-static void TestInvalidateExclusiveAccess(
-    TestData initial_data, MemoryAccess access1, MemoryAccess access2,
-    MemoryAccess access3, int expected_res, TestData expected_data) {
+void TestInvalidateExclusiveAccess(TestData initial_data, MemoryAccess access1,
+                                   MemoryAccess access2, MemoryAccess access3,
+                                   int expected_res, TestData expected_data) {
   Isolate* isolate = CcTest::i_isolate();
   HandleScope scope(isolate);
   MacroAssembler masm(isolate, nullptr, 0,
@@ -192,6 +207,7 @@ static void TestInvalidateExclusiveAccess(
   masm.GetCode(isolate, &desc);
   Handle<Code> code =
       isolate->factory()->NewCode(desc, Code::STUB, Handle<Code>());
+
   TestData t = initial_data;
   Simulator::CallArgument args[] = {
       Simulator::CallArgument(reinterpret_cast<uintptr_t>(&t)),
@@ -215,6 +231,8 @@ static void TestInvalidateExclusiveAccess(
   }
 }
 
+}  // namespace
+
 TEST(simulator_invalidate_exclusive_access) {
   using Kind = MemoryAccess::Kind;
   using Size = MemoryAccess::Size;
@@ -251,8 +269,10 @@ TEST(simulator_invalidate_exclusive_access) {
                                 0, TestData(7));
 }
 
-static int ExecuteMemoryAccess(Isolate* isolate, TestData* test_data,
-                               MemoryAccess access) {
+namespace {
+
+int ExecuteMemoryAccess(Isolate* isolate, TestData* test_data,
+                        MemoryAccess access) {
   HandleScope scope(isolate);
   MacroAssembler masm(isolate, nullptr, 0,
                       v8::internal::CodeObjectRequired::kYes);
@@ -270,6 +290,8 @@ static int ExecuteMemoryAccess(Isolate* isolate, TestData* test_data,
   return Simulator::current(isolate)->wreg(0);
 }
 
+}  // namespace
+
 class MemoryAccessThread : public v8::base::Thread {
  public:
   MemoryAccessThread()