[Atomics] use TFJ builtins for atomic add, sub, and, or, and xor

src/builtins/builtins-sharedarraybuffer-gen.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 "src/builtins/builtins-utils-gen.h"
 #include "src/builtins/builtins.h"
 #include "src/code-stub-assembler.h"
 #include "src/objects.h"
 
 namespace v8 {
 namespace internal {
 
 using compiler::Node;
 
 class SharedArrayBufferBuiltinsAssembler : public CodeStubAssembler {
  public:
   explicit SharedArrayBufferBuiltinsAssembler(
       compiler::CodeAssemblerState* state)
       : CodeStubAssembler(state) {}
 
  protected:
+  typedef Node* (CodeAssembler::*AssemblerFunction)(MachineType type,
+                                                    Node* base, Node* offset,
+                                                    Node* value);
   void ValidateSharedTypedArray(Node* tagged, Node* context,
                                 Node** out_instance_type,
                                 Node** out_backing_store);
   Node* ConvertTaggedAtomicIndexToWord32(Node* tagged, Node* context,
                                          Node** number_index);
   void ValidateAtomicIndex(Node* index_word, Node* array_length_word,
                            Node* context);
+  void AtomicBinopBuiltinCommon(Node* array, Node* index, Node* value,
+                                Node* context, AssemblerFunction function,
+                                Runtime::FunctionId runtime_function);
 };
 
 void SharedArrayBufferBuiltinsAssembler::ValidateSharedTypedArray(
     Node* tagged, Node* context, Node** out_instance_type,
     Node** out_backing_store) {
   Label not_float_or_clamped(this), invalid(this);
 
   // Fail if it is not a heap object.
   GotoIf(TaggedIsSmi(tagged), &invalid);
 
@@ skipping 98 matching lines @@
   Unreachable();
 
   BIND(&check_passed);
 }
 
 TF_BUILTIN(AtomicsLoad, SharedArrayBufferBuiltinsAssembler) {
   Node* array = Parameter(Descriptor::kArray);
   Node* index = Parameter(Descriptor::kIndex);
   Node* context = Parameter(Descriptor::kContext);
 
+  Node* index_integer;
+  Node* index_word32 =
+      ConvertTaggedAtomicIndexToWord32(index, context, &index_integer);
+
   Node* instance_type;
   Node* backing_store;
   ValidateSharedTypedArray(array, context, &instance_type, &backing_store);
 
-  Node* index_integer;
-  Node* index_word32 =
-      ConvertTaggedAtomicIndexToWord32(index, context, &index_integer);
   Node* array_length_word32 = TruncateTaggedToWord32(
       context, LoadObjectField(array, JSTypedArray::kLengthOffset));
   ValidateAtomicIndex(index_word32, array_length_word32, context);
   Node* index_word = ChangeUint32ToWord(index_word32);
 
   Label i8(this), u8(this), i16(this), u16(this), i32(this), u32(this),
       other(this);
   int32_t case_values[] = {
       FIXED_INT8_ARRAY_TYPE,   FIXED_UINT8_ARRAY_TYPE, FIXED_INT16_ARRAY_TYPE,
       FIXED_UINT16_ARRAY_TYPE, FIXED_INT32_ARRAY_TYPE, FIXED_UINT32_ARRAY_TYPE,
@@ skipping 32 matching lines @@
   BIND(&other);
   Unreachable();
 }
 
 TF_BUILTIN(AtomicsStore, SharedArrayBufferBuiltinsAssembler) {
   Node* array = Parameter(Descriptor::kArray);
   Node* index = Parameter(Descriptor::kIndex);
   Node* value = Parameter(Descriptor::kValue);
   Node* context = Parameter(Descriptor::kContext);
 
+  // The value_integer needs to be computed before the validations as the
+  // ToInteger function can be potentially modified in JS to invalidate the
+  // conditions. This is just a no-cost safety measure as SABs can't be neutered
+  // or shrunk.
+  Node* value_integer = ToInteger(context, value);

[Jarin, 2017/04/11 08:23:16]

I believe this is incorrect because the spec says the ToInteger has to be called
after the validation and this is observable (if you supply an invalid index, the
tointeger should not be called). See
https://tc39.github.io/ecmascript_sharedmem/shmem.html#Atomics.ReadModifyWrite

Could you write tests that check the order complies with the spec?

+  Node* value_word32 = TruncateTaggedToWord32(context, value_integer);
+
+  Node* index_integer;
+  Node* index_word32 =
+      ConvertTaggedAtomicIndexToWord32(index, context, &index_integer);
+
   Node* instance_type;
   Node* backing_store;
   ValidateSharedTypedArray(array, context, &instance_type, &backing_store);
 
-  Node* index_integer;
-  Node* index_word32 =
-      ConvertTaggedAtomicIndexToWord32(index, context, &index_integer);
   Node* array_length_word32 = TruncateTaggedToWord32(
       context, LoadObjectField(array, JSTypedArray::kLengthOffset));
   ValidateAtomicIndex(index_word32, array_length_word32, context);
   Node* index_word = ChangeUint32ToWord(index_word32);
 
-  Node* value_integer = ToInteger(context, value);
-  Node* value_word32 = TruncateTaggedToWord32(context, value_integer);
-
   Label u8(this), u16(this), u32(this), other(this);
   int32_t case_values[] = {
       FIXED_INT8_ARRAY_TYPE,   FIXED_UINT8_ARRAY_TYPE, FIXED_INT16_ARRAY_TYPE,
       FIXED_UINT16_ARRAY_TYPE, FIXED_INT32_ARRAY_TYPE, FIXED_UINT32_ARRAY_TYPE,
   };
   Label* case_labels[] = {
       &u8, &u8, &u16, &u16, &u32, &u32,
   };
   Switch(instance_type, &other, case_values, case_labels,
          arraysize(case_labels));
@@ skipping 17 matching lines @@
   BIND(&other);
   Unreachable();
 }
 
 TF_BUILTIN(AtomicsExchange, SharedArrayBufferBuiltinsAssembler) {
   Node* array = Parameter(Descriptor::kArray);
   Node* index = Parameter(Descriptor::kIndex);
   Node* value = Parameter(Descriptor::kValue);
   Node* context = Parameter(Descriptor::kContext);
 
+  // The value_integer needs to be computed before the validations as the
+  // ToInteger function can be potentially modified in JS to invalidate the
+  // conditions. This is just a no-cost safety measure as SABs can't be neutered
+  // or shrunk.
+  Node* value_integer = ToInteger(context, value);
+
+  Node* index_integer;
+  Node* index_word32 =
+      ConvertTaggedAtomicIndexToWord32(index, context, &index_integer);
+
   Node* instance_type;
   Node* backing_store;
   ValidateSharedTypedArray(array, context, &instance_type, &backing_store);
 
-  Node* index_integer;
-  Node* index_word32 =
-      ConvertTaggedAtomicIndexToWord32(index, context, &index_integer);
   Node* array_length_word32 = TruncateTaggedToWord32(
       context, LoadObjectField(array, JSTypedArray::kLengthOffset));
   ValidateAtomicIndex(index_word32, array_length_word32, context);
 
-  Node* value_integer = ToInteger(context, value);
-
 #if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64
   Return(CallRuntime(Runtime::kAtomicsExchange, context, array, index_integer,
                      value_integer));
 #else
   Node* index_word = ChangeUint32ToWord(index_word32);
 
   Node* value_word32 = TruncateTaggedToWord32(context, value_integer);
 
   Label i8(this), u8(this), i16(this), u16(this), i32(this), u32(this),
       other(this);
@@ skipping 39 matching lines @@
 #endif  // V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64
 }
 
 TF_BUILTIN(AtomicsCompareExchange, SharedArrayBufferBuiltinsAssembler) {
   Node* array = Parameter(Descriptor::kArray);
   Node* index = Parameter(Descriptor::kIndex);
   Node* old_value = Parameter(Descriptor::kOldValue);
   Node* new_value = Parameter(Descriptor::kNewValue);
   Node* context = Parameter(Descriptor::kContext);
 
+  // The value_integers needs to be computed before the validations as the
+  // ToInteger function can be potentially modified in JS to invalidate the
+  // conditions. This is just a no-cost safety measure as SABs can't be neutered
+  // or shrunk.
+  Node* old_value_integer = ToInteger(context, old_value);
+  Node* new_value_integer = ToInteger(context, new_value);
+
+  Node* index_integer;
+  Node* index_word32 =
+      ConvertTaggedAtomicIndexToWord32(index, context, &index_integer);
+
   Node* instance_type;
   Node* backing_store;
   ValidateSharedTypedArray(array, context, &instance_type, &backing_store);
 
-  Node* index_integer;
-  Node* index_word32 =
-      ConvertTaggedAtomicIndexToWord32(index, context, &index_integer);
   Node* array_length_word32 = TruncateTaggedToWord32(
       context, LoadObjectField(array, JSTypedArray::kLengthOffset));
   ValidateAtomicIndex(index_word32, array_length_word32, context);
 
-  Node* old_value_integer = ToInteger(context, old_value);
-
-  Node* new_value_integer = ToInteger(context, new_value);
-
 #if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64 || V8_TARGET_ARCH_PPC64 || \
     V8_TARGET_ARCH_PPC || V8_TARGET_ARCH_S390 || V8_TARGET_ARCH_S390X
   Return(CallRuntime(Runtime::kAtomicsCompareExchange, context, array,
                      index_integer, old_value_integer, new_value_integer));
 #else
   Node* index_word = ChangeUint32ToWord(index_word32);
 
   Node* old_value_word32 = TruncateTaggedToWord32(context, old_value_integer);
 
   Node* new_value_word32 = TruncateTaggedToWord32(context, new_value_integer);
@@ skipping 40 matching lines @@
       MachineType::Uint32(), backing_store, WordShl(index_word, 2),
       old_value_word32, new_value_word32)));
 
   // This shouldn't happen, we've already validated the type.
   BIND(&other);
   Unreachable();
 #endif  // V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64 || V8_TARGET_ARCH_PPC64
         // || V8_TARGET_ARCH_PPC || V8_TARGET_ARCH_S390 || V8_TARGET_ARCH_S390X
 }
 
+#define BINOP_BUILTIN(op)                                       \
+  TF_BUILTIN(Atomics##op, SharedArrayBufferBuiltinsAssembler) { \
+    Node* array = Parameter(Descriptor::kArray);                \
+    Node* index = Parameter(Descriptor::kIndex);                \
+    Node* value = Parameter(Descriptor::kValue);                \
+    Node* context = Parameter(Descriptor::kContext);            \
+    AtomicBinopBuiltinCommon(array, index, value, context,      \
+                             &CodeAssembler::Atomic##op,        \
+                             Runtime::kAtomics##op);            \
+  }
+BINOP_BUILTIN(Add)
+BINOP_BUILTIN(Sub)
+BINOP_BUILTIN(And)
+BINOP_BUILTIN(Or)
+BINOP_BUILTIN(Xor)
+#undef BINOP_BUILTIN
+
+void SharedArrayBufferBuiltinsAssembler::AtomicBinopBuiltinCommon(
+    Node* array, Node* index, Node* value, Node* context,
+    AssemblerFunction function, Runtime::FunctionId runtime_function) {
+  // The value_integer needs to be computed before the validations as the
+  // ToInteger function can be potentially modified in JS to invalidate the
+  // conditions. This is just a no-cost safety measure as SABs can't be neutered
+  // or shrunk.
+  Node* value_integer = ToInteger(context, value);
+
+  Node* index_integer;
+  Node* index_word32 =
+      ConvertTaggedAtomicIndexToWord32(index, context, &index_integer);
+
+  Node* instance_type;
+  Node* backing_store;
+  ValidateSharedTypedArray(array, context, &instance_type, &backing_store);
+
+  Node* array_length_word32 = TruncateTaggedToWord32(
+      context, LoadObjectField(array, JSTypedArray::kLengthOffset));
+  ValidateAtomicIndex(index_word32, array_length_word32, context);
+
+#if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64 || V8_TARGET_ARCH_PPC64 || \
+    V8_TARGET_ARCH_PPC || V8_TARGET_ARCH_S390 || V8_TARGET_ARCH_S390X
+  Return(CallRuntime(runtime_function, context, array, index_integer,
+                     value_integer));
+#else
+  Node* index_word = ChangeUint32ToWord(index_word32);
+
+  Node* value_word32 = TruncateTaggedToWord32(context, value_integer);
+
+  Label i8(this), u8(this), i16(this), u16(this), i32(this), u32(this),
+      other(this);
+  int32_t case_values[] = {
+      FIXED_INT8_ARRAY_TYPE,   FIXED_UINT8_ARRAY_TYPE, FIXED_INT16_ARRAY_TYPE,
+      FIXED_UINT16_ARRAY_TYPE, FIXED_INT32_ARRAY_TYPE, FIXED_UINT32_ARRAY_TYPE,
+  };
+  Label* case_labels[] = {
+      &i8, &u8, &i16, &u16, &i32, &u32,
+  };
+  Switch(instance_type, &other, case_values, case_labels,
+         arraysize(case_labels));
+
+  Bind(&i8);
+  Return(SmiFromWord32((this->*function)(MachineType::Int8(), backing_store,
+                                         index_word, value_word32)));
+
+  Bind(&u8);
+  Return(SmiFromWord32((this->*function)(MachineType::Uint8(), backing_store,
+                                         index_word, value_word32)));
+
+  Bind(&i16);
+  Return(
+      SmiFromWord32((this->*function)(MachineType::Int16(), backing_store,
+                                      WordShl(index_word, 1), value_word32)));
+
+  Bind(&u16);
+  Return(
+      SmiFromWord32((this->*function)(MachineType::Uint16(), backing_store,
+                                      WordShl(index_word, 1), value_word32)));
+
+  Bind(&i32);
+  Return(ChangeInt32ToTagged(
+      (this->*function)(MachineType::Int32(), backing_store,
+                        WordShl(index_word, 2), value_word32)));
+
+  Bind(&u32);
+  Return(ChangeUint32ToTagged(
+      (this->*function)(MachineType::Uint32(), backing_store,
+                        WordShl(index_word, 2), value_word32)));
+
+  // This shouldn't happen, we've already validated the type.
+  Bind(&other);
+  Unreachable();
+#endif  // V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64 || V8_TARGET_ARCH_PPC64
+        // || V8_TARGET_ARCH_PPC || V8_TARGET_ARCH_S390 || V8_TARGET_ARCH_S390X
+}
+
 }  // namespace internal
 }  // namespace v8