[torque] Allow returning pairs from builtins

This would be useful for ForInPrepare. Syntax is unchanged; Torque
should now do the right thing for builtins that return a two-element
struct. More elements than that is still not supported.

Bug: v8:7793
Change-Id: Ic315699402203aba07e906ff6e029834ec0061c6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2596498
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#72171}

src/codegen/interface-descriptors.h

@@ -547,7 +547,7 @@ class V8_EXPORT_PRIVATE VoidDescriptor : public CallInterfaceDescriptor {
 };
 
 // This class is subclassed by Torque-generated call interface descriptors.
-template <int parameter_count, bool has_context_parameter>
+template <int return_count, int parameter_count, bool has_context_parameter>
 class TorqueInterfaceDescriptor : public CallInterfaceDescriptor {
  public:
   static constexpr int kDescriptorFlags =
@@ -560,7 +560,7 @@ class TorqueInterfaceDescriptor : public CallInterfaceDescriptor {
     STATIC_ASSERT(0 <= i && i < kParameterCount);
     return static_cast<ParameterIndices>(i);
   }
-  static constexpr int kReturnCount = 1;
+  static constexpr int kReturnCount = return_count;
 
   using CallInterfaceDescriptor::CallInterfaceDescriptor;
 
@@ -570,14 +570,15 @@ class TorqueInterfaceDescriptor : public CallInterfaceDescriptor {
           ? kMaxTFSBuiltinRegisterParams
           : kParameterCount;
   static const int kStackParams = kParameterCount - kRegisterParams;
-  virtual MachineType ReturnType() = 0;
+  virtual std::vector<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()};
+    std::vector<MachineType> machine_types = ReturnType();
+    DCHECK_EQ(kReturnCount, machine_types.size());
     auto parameter_types = ParameterTypes();
     machine_types.insert(machine_types.end(), parameter_types.begin(),
                          parameter_types.end());

src/torque/cfg.h

@@ -116,13 +116,18 @@ class ControlFlowGraph {
   Block* start() const { return start_; }
   base::Optional<Block*> end() const { return end_; }
   void set_end(Block* end) { end_ = end; }
-  void SetReturnType(const Type* t) {
+  void SetReturnType(TypeVector t) {
     if (!return_type_) {
       return_type_ = t;
       return;
     }
     if (t != *return_type_) {
-      ReportError("expected return type ", **return_type_, " instead of ", *t);
+      std::stringstream message;
+      message << "expected return type ";
+      PrintCommaSeparatedList(message, *return_type_);
+      message << " instead of ";
+      PrintCommaSeparatedList(message, t);
+      ReportError(message.str());
     }
   }
   const std::vector<Block*>& blocks() const { return placed_blocks_; }
@@ -136,7 +141,7 @@ class ControlFlowGraph {
   Block* start_;
   std::vector<Block*> placed_blocks_;
   base::Optional<Block*> end_;
-  base::Optional<const Type*> return_type_;
+  base::Optional<TypeVector> return_type_;
   size_t next_block_id_ = 0;
 };
 

src/torque/csa-generator.cc

@@ -496,23 +496,47 @@ void CSAGenerator::EmitInstruction(const CallBuiltinInstruction& instruction,
     }
     out() << ");\n";
   } else {
-    std::string result_name;
-    if (result_types.size() == 1) {
-      result_name = DefinitionToVariable(instruction.GetValueDefinition(0));
-      decls() << "  TNode<" << result_types[0]->GetGeneratedTNodeTypeName()
-              << "> " << result_name << ";\n";
+    std::vector<std::string> result_names(result_types.size());
+    for (size_t i = 0; i < result_types.size(); ++i) {
+      result_names[i] = DefinitionToVariable(instruction.GetValueDefinition(i));
+      decls() << "  TNode<" << result_types[i]->GetGeneratedTNodeTypeName()
+              << "> " << result_names[i] << ";\n";
     }
+
+    std::string lhs_name;
+    std::string lhs_type;
+    switch (result_types.size()) {
+      case 1:
+        lhs_name = result_names[0];
+        lhs_type = result_types[0]->GetGeneratedTNodeTypeName();
+        break;
+      case 2:
+        // If a builtin returns two values, the return type is represented as a
+        // TNode containing a pair. We need a temporary place to store that
+        // result so we can unpack it into separate TNodes.
+        lhs_name = result_names[0] + "_and_" + result_names[1];
+        lhs_type = "PairT<" + result_types[0]->GetGeneratedTNodeTypeName() +
+                   ", " + result_types[1]->GetGeneratedTNodeTypeName() + ">";
+        decls() << "  TNode<" << lhs_type << "> " << lhs_name << ";\n";
+        break;
+      default:
+        ReportError(
+            "Torque can only call builtins that return one or two values, not ",
+            result_types.size());
+    }
+
     std::string catch_name =
         PreCallableExceptionPreparation(instruction.catch_block);
     Stack<std::string> pre_call_stack = *stack;
 
-    DCHECK_EQ(1, result_types.size());
     std::string generated_type = result_types[0]->GetGeneratedTNodeTypeName();
-    stack->Push(result_name);
-    out() << "    " << result_name << " = ";
-    if (generated_type != "Object") out() << "TORQUE_CAST(";
-    out() << "CodeStubAssembler(state_).CallBuiltin(Builtins::k"
-          << instruction.builtin->ExternalName();
+    for (const std::string& name : result_names) {
+      stack->Push(name);
+    }
+    out() << "    " << lhs_name << " = ";
+    out() << "ca_.CallStub<" << lhs_type
+          << ">(Builtins::CallableFor(ca_.isolate(), Builtins::k"
+          << instruction.builtin->ExternalName() << ")";
     if (!instruction.builtin->signature().HasContextParameter()) {
       // Add dummy context parameter to satisfy the CallBuiltin signature.
       out() << ", TNode<Object>()";
@@ -520,9 +544,15 @@ void CSAGenerator::EmitInstruction(const CallBuiltinInstruction& instruction,
     for (const std::string& argument : arguments) {
       out() << ", " << argument;
     }
-    if (generated_type != "Object") out() << ")";
     out() << ");\n";
 
+    if (result_types.size() > 1) {
+      for (size_t i = 0; i < result_types.size(); ++i) {
+        out() << "    " << result_names[i] << " = ca_.Projection<" << i << ">("
+              << lhs_name << ");\n";
+      }
+    }
+
     PostCallableExceptionPreparation(
         catch_name,
         result_types.size() == 0 ? TypeOracle::GetVoidType() : result_types[0],
@@ -771,7 +801,9 @@ void CSAGenerator::EmitInstruction(const ReturnInstruction& instruction,
   } else {
     out() << "    CodeStubAssembler(state_).Return(";
   }
-  out() << stack->Pop() << ");\n";
+  std::vector<std::string> values = stack->PopMany(instruction.count);
+  PrintCommaSeparatedList(out(), values);
+  out() << ");\n";
 }
 
 void CSAGenerator::EmitInstruction(

src/torque/declaration-visitor.cc

@@ -98,9 +98,11 @@ Builtin* DeclarationVisitor::CreateBuiltin(BuiltinDeclaration* decl,
     }
   }
 
-  if (signature.return_type->StructSupertype()) {
-    Error("Builtins cannot return structs, but the return type is ",
-          *signature.return_type, ".");
+  if (signature.return_type->StructSupertype() && javascript) {
+    Error(
+        "Builtins with JS linkage cannot return structs, but the return type "
+        "is ",
+        *signature.return_type, ".");
   }
 
   if (signature.return_type == TypeOracle::GetVoidType()) {

src/torque/implementation-visitor.cc

@@ -1256,7 +1256,8 @@ const Type* ImplementationVisitor::Visit(ReturnStatement* stmt) {
         SetReturnValue(return_result);
       }
     } else if (current_callable->IsBuiltin()) {
-      assembler().Emit(ReturnInstruction{});
+      assembler().Emit(ReturnInstruction{
+          LoweredSlotCount(current_callable->signature().return_type)});
     } else {
       UNREACHABLE();
     }
@@ -2713,9 +2714,21 @@ VisitResult ImplementationVisitor::GenerateCall(
       return VisitResult::NeverResult();
     } else {
       size_t slot_count = LoweredSlotCount(return_type);
-      DCHECK_LE(slot_count, 1);
-      // TODO(tebbi): Actually, builtins have to return a value, so we should
-      // assert slot_count == 1 here.
+      if (builtin->IsStub()) {
+        if (slot_count < 1 || slot_count > 2) {
+          ReportError(
+              "Builtin with stub linkage is expected to return one or two "
+              "values but returns ",
+              slot_count);
+        }
+      } else {
+        if (slot_count != 1) {
+          ReportError(
+              "Builtin with JS linkage is expected to return one value but "
+              "returns ",
+              slot_count);
+        }
+      }
       return VisitResult(return_type, assembler().TopRange(slot_count));
     }
   } else if (auto* macro = Macro::DynamicCast(callable)) {
@@ -3328,19 +3341,23 @@ void ImplementationVisitor::GenerateBuiltinDefinitionsAndInterfaceDescriptors(
         size_t kFirstNonContextParameter = has_context_parameter ? 1 : 0;
         size_t parameter_count =
             builtin->parameter_names().size() - kFirstNonContextParameter;
+        TypeVector return_types = LowerType(builtin->signature().return_type);
 
         interface_descriptors
             << "class " << descriptor_name
-            << " : public TorqueInterfaceDescriptor<" << parameter_count << ", "
+            << " : public TorqueInterfaceDescriptor<" << return_types.size()
+            << ", " << parameter_count << ", "
             << (has_context_parameter ? "true" : "false") << "> {\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";
+            << "  std::vector<MachineType> ReturnType() override {\n";
+        interface_descriptors << "    return {{";
+        PrintCommaSeparatedList(interface_descriptors, return_types,
+                                MachineTypeString);
+        interface_descriptors << "}};\n";
         interface_descriptors << "  }\n";
 
         interface_descriptors << "  std::array<MachineType, " << parameter_count

src/torque/instructions.cc

@@ -558,12 +558,12 @@ void GotoExternalInstruction::RecomputeDefinitionLocations(
 
 void ReturnInstruction::TypeInstruction(Stack<const Type*>* stack,
                                         ControlFlowGraph* cfg) const {
-  cfg->SetReturnType(stack->Pop());
+  cfg->SetReturnType(stack->PopMany(count));
 }
 
 void ReturnInstruction::RecomputeDefinitionLocations(
     Stack<DefinitionLocation>* locations, Worklist<Block*>* worklist) const {
-  locations->Pop();
+  locations->PopMany(count);
 }
 
 void PrintConstantStringInstruction::TypeInstruction(

src/torque/instructions.h

@@ -578,7 +578,10 @@ struct GotoExternalInstruction : InstructionBase {
 
 struct ReturnInstruction : InstructionBase {
   TORQUE_INSTRUCTION_BOILERPLATE()
+  explicit ReturnInstruction(size_t count) : count(count) {}
   bool IsBlockTerminator() const override { return true; }
+
+  size_t count;  // How many values to return.
 };
 
 struct PrintConstantStringInstruction : InstructionBase {

test/cctest/torque/test-torque.cc

@@ -881,6 +881,23 @@ TEST(TestOffHeapSlice) {
   ft.Call();
 }
 
+TEST(TestCallMultiReturnBuiltin) {
+  CcTest::InitializeVM();
+  Isolate* isolate(CcTest::i_isolate());
+  i::HandleScope scope(isolate);
+  CodeAssemblerTester asm_tester(isolate, 1);
+  TestTorqueAssembler m(asm_tester.state());
+  {
+    Handle<Context> context =
+        Utils::OpenHandle(*v8::Isolate::GetCurrent()->GetCurrentContext());
+    m.TestCallMultiReturnBuiltin(
+        m.UncheckedCast<Context>(m.HeapConstant(context)));
+    m.Return(m.UndefinedConstant());
+  }
+  FunctionTester ft(asm_tester.GenerateCode(), 0);
+  ft.Call();
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

test/torque/test-torque.tq

@@ -1350,4 +1350,21 @@ macro TestOffHeapSlice(ptr: RawPtr<char8>, length: intptr) {
     check(*onHeapSlice.AtIndex(i) == *offHeapSlice.AtIndex(i));
   }
 }
+
+struct TwoValues {
+  a: Smi;
+  b: Map;
+}
+
+builtin ReturnTwoValues(implicit context: Context)(
+    value: Smi, obj: HeapObject): TwoValues {
+  return TwoValues{a: value + 1, b: obj.map};
+}
+
+@export
+macro TestCallMultiReturnBuiltin(implicit context: Context)() {
+  const result = ReturnTwoValues(444, FromConstexpr<String>('hi'));
+  check(result.a == 445);
+  check(result.b == FromConstexpr<String>('hi').map);
+}
 }