[torque] allow untagged builtin parameters

For this, all Torque stub-linkage builtins use TFC instead of TFS,
with a custom descriptor added to interface-descriptors.h

To avoid having complex logic in the generated code, the new class
TorqueInterfaceDescriptor contains the logic to create a
CallInterfaceDescriptor from a signature consisting of TNode types.

As an example and test, this CL ports StringCharAt to Torque.

Bug: v8:7793
Change-Id: I8339d2ad6e4f908ebdc3b8d30244e4bcbd974f21
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1798427Reviewed-by: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64148}

BUILD.gn

@@ -1092,6 +1092,7 @@ action("run_torque") {
 
   outputs = [
     "$target_gen_dir/torque-generated/builtin-definitions-tq.h",
+    "$target_gen_dir/torque-generated/interface-descriptors-tq.inc",
     "$target_gen_dir/torque-generated/field-offsets-tq.h",
     "$target_gen_dir/torque-generated/class-verifiers-tq.cc",
     "$target_gen_dir/torque-generated/class-verifiers-tq.h",

src/builtins/builtins-definitions.h

@@ -102,7 +102,6 @@ namespace internal {
   ASM(ResumeGeneratorTrampoline, ResumeGenerator)                              \
                                                                                \
   /* String helpers */                                                         \
-  TFC(StringCharAt, StringAt)                                                  \
   TFC(StringCodePointAt, StringAt)                                             \
   TFC(StringFromCodePointAt, StringAtAsString)                                 \
   TFC(StringEqual, Compare)                                                    \

src/builtins/builtins-string-gen.cc

@@ -760,19 +760,6 @@ TF_BUILTIN(StringGreaterThanOrEqual, StringBuiltinsAssembler) {
                                      Operation::kGreaterThanOrEqual);
 }
 
-TF_BUILTIN(StringCharAt, StringBuiltinsAssembler) {
-  TNode<String> receiver = CAST(Parameter(Descriptor::kReceiver));
-  TNode<IntPtrT> position =
-      UncheckedCast<IntPtrT>(Parameter(Descriptor::kPosition));
-
-  // Load the character code at the {position} from the {receiver}.
-  TNode<Int32T> code = StringCharCodeAt(receiver, position);
-
-  // And return the single character string with only that {code}
-  TNode<String> result = StringFromSingleCharCode(code);
-  Return(result);
-}
-
 TF_BUILTIN(StringCodePointAt, StringBuiltinsAssembler) {
   Node* receiver = Parameter(Descriptor::kReceiver);
   Node* position = Parameter(Descriptor::kPosition);

src/builtins/string.tq

@@ -188,4 +188,12 @@ namespace string {
       left: String, right: JSAny): String {
     return left + ToStringImpl(context, ToPrimitiveDefault(right));
   }
+
+  builtin StringCharAt(implicit context: Context)(
+      receiver: String, position: intptr): String {
+    // Load the character code at the {position} from the {receiver}.
+    const code: int32 = StringCharCodeAt(receiver, position);
+    // And return the single character string with only that {code}
+    return StringFromSingleCharCode(code);
+  }
 }

src/codegen/interface-descriptors.h

@@ -9,12 +9,17 @@
 
 #include "src/codegen/machine-type.h"
 #include "src/codegen/register-arch.h"
+#include "src/codegen/tnode.h"
 #include "src/common/globals.h"
 #include "src/execution/isolate.h"
 
 namespace v8 {
 namespace internal {
 
+#define TORQUE_BUILTIN_LIST_TFC(V)                                            \
+  BUILTIN_LIST_FROM_TORQUE(IGNORE_BUILTIN, IGNORE_BUILTIN, V, IGNORE_BUILTIN, \
+                           IGNORE_BUILTIN, IGNORE_BUILTIN)
+
 #define INTERFACE_DESCRIPTOR_LIST(V)  \
   V(Abort)                            \
   V(Allocate)                         \
@@ -90,7 +95,8 @@ namespace internal {
   V(WasmTableGet)                     \
   V(WasmTableSet)                     \
   V(WasmThrow)                        \
-  BUILTIN_LIST_TFS(V)
+  BUILTIN_LIST_TFS(V)                 \
+  TORQUE_BUILTIN_LIST_TFC(V)
 
 class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
  public:
@@ -487,6 +493,46 @@ class V8_EXPORT_PRIVATE VoidDescriptor : public CallInterfaceDescriptor {
   DECLARE_DESCRIPTOR(VoidDescriptor, CallInterfaceDescriptor)
 };
 
+// This class is subclassed by Torque-generated call interface descriptors.
+template <int parameter_count>
+class TorqueInterfaceDescriptor : public CallInterfaceDescriptor {
+ public:
+  static constexpr int kDescriptorFlags = CallInterfaceDescriptorData::kNoFlags;
+  static constexpr int kParameterCount = parameter_count;
+  enum ParameterIndices { kContext = kParameterCount };
+  template <int i>
+  static ParameterIndices ParameterIndex() {
+    STATIC_ASSERT(0 <= i && i < kParameterCount);
+    return static_cast<ParameterIndices>(i);
+  }
+  static constexpr int kReturnCount = 1;
+
+  using CallInterfaceDescriptor::CallInterfaceDescriptor;
+
+ protected:
+  static const int kRegisterParams =
+      kParameterCount > kMaxTFSBuiltinRegisterParams
+          ? kMaxTFSBuiltinRegisterParams
+          : kParameterCount;
+  static const int kStackParams = kParameterCount - kRegisterParams;
+  virtual MachineType ReturnType() = 0;
+  virtual std::array<MachineType, kParameterCount> ParameterTypes() = 0;
+  void InitializePlatformSpecific(CallInterfaceDescriptorData* data) override {
+    DefaultInitializePlatformSpecific(data, kRegisterParams);
+  }
+  void InitializePlatformIndependent(
+      CallInterfaceDescriptorData* data) override {
+    std::vector<MachineType> machine_types = {ReturnType()};
+    auto parameter_types = ParameterTypes();
+    machine_types.insert(machine_types.end(), parameter_types.begin(),
+                         parameter_types.end());
+    DCHECK_EQ(kReturnCount + kParameterCount, machine_types.size());
+    data->InitializePlatformIndependent(Flags(kDescriptorFlags), kReturnCount,
+                                        kParameterCount, machine_types.data(),
+                                        static_cast<int>(machine_types.size()));
+  }
+};
+
 // Dummy descriptor used to mark builtins that don't yet have their proper
 // descriptor associated.
 using DummyDescriptor = VoidDescriptor;
@@ -1309,6 +1355,11 @@ class CloneObjectWithVectorDescriptor final : public CallInterfaceDescriptor {
 BUILTIN_LIST_TFS(DEFINE_TFS_BUILTIN_DESCRIPTOR)
 #undef DEFINE_TFS_BUILTIN_DESCRIPTOR
 
+// This file contains interface descriptor class definitions for builtins
+// defined in Torque. It is included here because the class definitions need to
+// precede the definition of name##Descriptor::key() below.
+#include "torque-generated/interface-descriptors-tq.inc"
+
 #undef DECLARE_DEFAULT_DESCRIPTOR
 #undef DECLARE_DESCRIPTOR_WITH_BASE
 #undef DECLARE_DESCRIPTOR

src/compiler/machine-graph-verifier.cc

@@ -1007,6 +1007,13 @@ class MachineRepresentationChecker {
         return IsAnyCompressed(actual);
       case MachineRepresentation::kTaggedSigned:
       case MachineRepresentation::kTaggedPointer:
+        // TODO(tebbi): At the moment, the machine graph doesn't contain
+        // reliable information if a node is kTaggedSigned, kTaggedPointer or
+        // kTagged, and often this is context-dependent. We should at least
+        // check for obvious violations: kTaggedSigned where we expect
+        // kTaggedPointer and the other way around, but at the moment, this
+        // happens in dead code.
+        return IsAnyTagged(actual);
       case MachineRepresentation::kCompressedSigned:
       case MachineRepresentation::kCompressedPointer:
       case MachineRepresentation::kFloat32:

src/torque/declaration-visitor.cc

@@ -93,20 +93,6 @@ Builtin* DeclarationVisitor::CreateBuiltin(BuiltinDeclaration* decl,
     }
   }
 
-  if (TorqueBuiltinDeclaration::DynamicCast(decl)) {
-    for (size_t i = 0; i < signature.types().size(); ++i) {
-      const Type* type = signature.types()[i];
-      if (!type->IsSubtypeOf(TypeOracle::GetTaggedType())) {
-        const Identifier* id = signature.parameter_names.size() > i
-                                   ? signature.parameter_names[i]
-                                   : nullptr;
-        Error("Untagged argument ", id ? (id->value + " ") : "", "at position ",
-              i, " to builtin ", decl->name, " is not supported.")
-            .Position(id ? id->pos : decl->pos);
-      }
-    }
-  }
-
   if (const StructType* struct_type =
           StructType::DynamicCast(signature.return_type)) {
     Error("Builtins ", decl->name, " cannot return structs ",

src/torque/implementation-visitor.cc

@@ -526,7 +526,6 @@ void ImplementationVisitor::Visit(Builtin* builtin) {
     source_out() << "  USE(" << parameter0 << ");\n";
 
     for (size_t i = 1; i < signature.parameter_names.size(); ++i) {
-      const std::string& parameter_name = signature.parameter_names[i]->value;
       const Type* type = signature.types()[i];
       const bool mark_as_used = signature.implicit_count > i;
       std::string var = AddParameter(i, builtin, &parameters, &parameter_types,
@@ -534,8 +533,8 @@ void ImplementationVisitor::Visit(Builtin* builtin) {
       source_out() << "  " << type->GetGeneratedTypeName() << " " << var
                    << " = "
                    << "UncheckedCast<" << type->GetGeneratedTNodeTypeName()
-                   << ">(Parameter(Descriptor::k"
-                   << CamelifyString(parameter_name) << "));\n";
+                   << ">(Parameter(Descriptor::ParameterIndex<" << (i - 1)
+                   << ">()));\n";
       source_out() << "  USE(" << var << ");\n";
     }
   }
@@ -2670,13 +2669,34 @@ void ImplementationVisitor::Visit(Declarable* declarable) {
   }
 }
 
-void ImplementationVisitor::GenerateBuiltinDefinitions(
+std::string MachineTypeString(const Type* type) {
+  if (type->IsSubtypeOf(TypeOracle::GetSmiType())) {
+    return "MachineType::TaggedSigned()";
+  }
+  if (type->IsSubtypeOf(TypeOracle::GetHeapObjectType())) {
+    return "MachineType::TaggedPointer()";
+  }
+  if (type->IsSubtypeOf(TypeOracle::GetTaggedType())) {
+    return "MachineType::AnyTagged()";
+  }
+  return "MachineTypeOf<" + type->GetGeneratedTNodeTypeName() + ">::value";
+}
+
+void ImplementationVisitor::GenerateBuiltinDefinitionsAndInterfaceDescriptors(
     const std::string& output_directory) {
-  std::stringstream new_contents_stream;
-  std::string file_name = "builtin-definitions-tq.h";
+  std::stringstream builtin_definitions;
+  std::string builtin_definitions_file_name = "builtin-definitions-tq.h";
+
+  // This file contains plain interface descriptor definitions and has to be
+  // included in the middle of interface-descriptors.h. Thus it is not a normal
+  // header file and uses the .inc suffix instead of the .h suffix.
+  std::stringstream interface_descriptors;
+  std::string interface_descriptors_file_name = "interface-descriptors-tq.inc";
   {
-    IncludeGuardScope include_guard(new_contents_stream, file_name);
-    new_contents_stream
+    IncludeGuardScope builtin_definitions_include_guard(
+        builtin_definitions, builtin_definitions_file_name);
+
+    builtin_definitions
         << "\n"
            "#define BUILTIN_LIST_FROM_TORQUE(CPP, TFJ, TFC, TFS, TFH, "
            "ASM) "
@@ -2684,40 +2704,67 @@ void ImplementationVisitor::GenerateBuiltinDefinitions(
     for (auto& declarable : GlobalContext::AllDeclarables()) {
       Builtin* builtin = Builtin::DynamicCast(declarable.get());
       if (!builtin || builtin->IsExternal()) continue;
-      size_t firstParameterIndex = 1;
-      bool declareParameters = true;
       if (builtin->IsStub()) {
-        new_contents_stream << "TFS(" << builtin->ExternalName();
+        builtin_definitions << "TFC(" << builtin->ExternalName() << ", "
+                            << builtin->ExternalName();
+        std::string descriptor_name = builtin->ExternalName() + "Descriptor";
+        constexpr size_t kFirstNonContextParameter = 1;
+        size_t parameter_count =
+            builtin->parameter_names().size() - kFirstNonContextParameter;
+
+        interface_descriptors << "class " << descriptor_name
+                              << " : public TorqueInterfaceDescriptor<"
+                              << parameter_count << "> {\n";
+        interface_descriptors << "  DECLARE_DESCRIPTOR_WITH_BASE("
+                              << descriptor_name
+                              << ", TorqueInterfaceDescriptor)\n";
+
+        interface_descriptors << "  MachineType ReturnType() override {\n";
+        interface_descriptors
+            << "    return "
+            << MachineTypeString(builtin->signature().return_type) << ";\n";
+        interface_descriptors << "  }\n";
+
+        interface_descriptors << "  std::array<MachineType, " << parameter_count
+                              << "> ParameterTypes() override {\n";
+        interface_descriptors << "    return {";
+        for (size_t i = kFirstNonContextParameter;
+             i < builtin->parameter_names().size(); ++i) {
+          bool last = i + 1 == builtin->parameter_names().size();
+          const Type* type = builtin->signature().parameter_types.types[i];
+          interface_descriptors << MachineTypeString(type)
+                                << (last ? "" : ", ");
+        }
+        interface_descriptors << "};\n";
+
+        interface_descriptors << "  }\n";
+        interface_descriptors << "};\n\n";
       } else {
-        new_contents_stream << "TFJ(" << builtin->ExternalName();
+        builtin_definitions << "TFJ(" << builtin->ExternalName();
         if (builtin->IsVarArgsJavaScript()) {
-          new_contents_stream
+          builtin_definitions
               << ", SharedFunctionInfo::kDontAdaptArgumentsSentinel";
-          declareParameters = false;
         } else {
           DCHECK(builtin->IsFixedArgsJavaScript());
           // FixedArg javascript builtins need to offer the parameter
           // count.
           int parameter_count =
               static_cast<int>(builtin->signature().ExplicitCount());
-          new_contents_stream << ", " << parameter_count;
+          builtin_definitions << ", " << parameter_count;
           // And the receiver is explicitly declared.
-          new_contents_stream << ", kReceiver";
-          firstParameterIndex = builtin->signature().implicit_count;
-        }
-      }
-      if (declareParameters) {
-        for (size_t i = firstParameterIndex;
-             i < builtin->parameter_names().size(); ++i) {
-          Identifier* parameter = builtin->parameter_names()[i];
-          new_contents_stream << ", k" << CamelifyString(parameter->value);
+          builtin_definitions << ", kReceiver";
+          for (size_t i = builtin->signature().implicit_count;
+               i < builtin->parameter_names().size(); ++i) {
+            Identifier* parameter = builtin->parameter_names()[i];
+            builtin_definitions << ", k" << CamelifyString(parameter->value);
+          }
         }
       }
-      new_contents_stream << ") \\\n";
+      builtin_definitions << ") \\\n";
     }
-    new_contents_stream << "\n";
+    builtin_definitions << "\n";
 
-    new_contents_stream
+    builtin_definitions
         << "#define TORQUE_FUNCTION_POINTER_TYPE_TO_BUILTIN_MAP(V) \\\n";
     for (const BuiltinPointerType* type :
          TypeOracle::AllBuiltinPointerTypes()) {
@@ -2728,13 +2775,15 @@ void ImplementationVisitor::GenerateBuiltinDefinitions(
             SourcePosition{CurrentSourceFile::Get(), {-1, -1}, {-1, -1}});
         ReportError("unable to find any builtin with type \"", *type, "\"");
       }
-      new_contents_stream << "  V(" << type->function_pointer_type_id() << ","
+      builtin_definitions << "  V(" << type->function_pointer_type_id() << ","
                           << example_builtin->ExternalName() << ")\\\n";
     }
-    new_contents_stream << "\n";
+    builtin_definitions << "\n";
   }
-  std::string new_contents(new_contents_stream.str());
-  WriteFile(output_directory + "/" + file_name, new_contents);
+  WriteFile(output_directory + "/" + builtin_definitions_file_name,
+            builtin_definitions.str());
+  WriteFile(output_directory + "/" + interface_descriptors_file_name,
+            interface_descriptors.str());
 }
 
 namespace {

src/torque/implementation-visitor.h

@@ -343,7 +343,8 @@ bool IsCompatibleSignature(const Signature& sig, const TypeVector& types,
 
 class ImplementationVisitor {
  public:
-  void GenerateBuiltinDefinitions(const std::string& output_directory);
+  void GenerateBuiltinDefinitionsAndInterfaceDescriptors(
+      const std::string& output_directory);
   void GenerateClassFieldOffsets(const std::string& output_directory);
   void GeneratePrintDefinitions(const std::string& output_directory);
   void GenerateClassDefinitions(const std::string& output_directory);

src/torque/torque-compiler.cc

@@ -79,7 +79,8 @@ void CompileCurrentAst(TorqueCompilerOptions options) {
 
   ReportAllUnusedMacros();
 
-  implementation_visitor.GenerateBuiltinDefinitions(output_directory);
+  implementation_visitor.GenerateBuiltinDefinitionsAndInterfaceDescriptors(
+      output_directory);
   implementation_visitor.GenerateClassFieldOffsets(output_directory);
   implementation_visitor.GeneratePrintDefinitions(output_directory);
   implementation_visitor.GenerateClassDefinitions(output_directory);

test/cctest/heap/heap-tester.h

@@ -102,6 +102,7 @@ class HeapTester {
   // test-heap.cc
   static AllocationResult AllocateByteArrayForTest(Heap* heap, int length,
                                                    AllocationType allocation);
+  static bool CodeEnsureLinearAllocationArea(Heap* heap, int size_in_bytes);
 
   // test-mark-compact.cc
   static AllocationResult AllocateMapForTest(v8::internal::Isolate* isolate);

test/cctest/heap/test-heap.cc

@@ -5337,6 +5337,11 @@ AllocationResult HeapTester::AllocateByteArrayForTest(
   return result;
 }
 
+bool HeapTester::CodeEnsureLinearAllocationArea(Heap* heap, int size_in_bytes) {
+  return heap->code_space()->EnsureLinearAllocationArea(
+      size_in_bytes, AllocationOrigin::kRuntime);
+}
+
 HEAP_TEST(Regress587004) {
   ManualGCScope manual_gc_scope;
 #ifdef VERIFY_HEAP
@@ -6788,11 +6793,13 @@ TEST(CodeObjectRegistry) {
   HandleScope outer_scope(heap->isolate());
   Address code2_address;
   {
+    // Ensure that both code objects end up on the same page.
+    CHECK(HeapTester::CodeEnsureLinearAllocationArea(
+        heap, kMaxRegularHeapObjectSize));
     code1 = DummyOptimizedCode(isolate);
     Handle<Code> code2 = DummyOptimizedCode(isolate);
     code2_address = code2->address();
-    // If this check breaks, change the allocation to ensure that both code
-    // objects are on the same page.
+
     CHECK_EQ(MemoryChunk::FromHeapObject(*code1),
              MemoryChunk::FromHeapObject(*code2));
     CHECK(MemoryChunk::FromHeapObject(*code1)->Contains(code1->address()));