@cpp calls pass records by value and read them back from registers as each C++ ABI does

README.md

@@ -729,7 +729,8 @@ declare function make_square(side: s32): Square;
 - **Slots** are clang's: the destructor (slot 0 under MSVC, 0 and 1 under the Itanium ABI) and methods that aren't public are slots left out. A method overriding a base's is not declared again: the `@cpp` class's override keeps the base's slot.
 - **Overloads** keep the C++ name, each with its own slot (or symbol), in whatever order the ABI puts the slots: a call picks the overload by its arguments, as C++ does (`shape.scale(2)`, `shape.scale(0.5)`). A method named as a field gets `_2`, `_3`, ...
 - **A pointer or a reference** to an object of a bound class is the class (`| null` for a pointer); a reference to a struct is a `Reference` to its tuple.
-- **A record returned by value** is a tuple with its layout, `@cppnontrivial` when it has a destructor or a copy constructor.
+- **A record returned by value** is a tuple with its layout, `@cppnontrivial` when it has a destructor or a copy constructor, or (Itanium) is more than 16 bytes; under the Itanium ABI one with floating-point fields is skipped.
+- **A record passed by value** to a virtual method is its own type (a tuple), which tslang passes as the C++ ABI does (docs/cpp-classes.md, "Structs passed by value"); one that isn't trivially copyable, a union, and (Itanium) one with floating-point fields are skipped, and so is a non-virtual method taking one.
 - **Fields** are declared when they are where a `@cpp` class puts them (after the vtable pointer and the base's storage).
 - **Left out, with a `// skipped:` line:** `inline` methods (no symbol to link to), struct parameters passed by value, and a non-virtual method returning a struct. Static methods and operators are left out silently.
 - **Not bound at all:** templates, classes with several bases or a virtual one, and 32-bit targets. A pointer to such a class stays `Opaque`.

docs/cpp-classes.md

@@ -79,8 +79,11 @@ errors and not warnings.
 | `-mm=rc` / `-mm=own` | retains and releases write a count in the word before an object, inside the C++ object |
 | a 32-bit x86 MSVC target | C++ member functions are `__thiscall` there (`this` in ECX) |
 | a virtual method returning a tuple, on a target other than MSVC and Itanium x86-64 / AArch64 | each ABI returns records its own way (see below) |
-| on Itanium, a virtual method returning a trivial tuple other than one or two 8-byte integer or pointer fields | SysV / AAPCS64 pack it into registers by its fields, which a tuple isn't (see below) |
-| a non-virtual C++ method returning a tuple | not supported yet: its symbol would have to be declared with the hidden result pointer |
+| on Itanium, a virtual method returning a trivial tuple with floating-point fields, unmarked | SysV / AAPCS64 return it in SSE registers / as an HFA, which isn't done (see below) |
+| on Itanium, a call of a method returning a trivial tuple of more than 16 bytes, unmarked | it is returned in memory: mark the method `@cppnontrivial` (see below) |
+| on Itanium, a call passing a tuple with floating-point fields by value | SysV / AAPCS64 pass it in SSE registers / as an HFA, which isn't done (see below) |
+| a non-virtual C++ method returning or taking a tuple | not supported yet: its symbol would have to be declared as each ABI passes records |
+| a method implementing a C++ method (with a body) taking a tuple | not supported yet: its body would have to read it as the ABI passes it |
 
 A generic or static class can't be `@cpp` either.
 
@@ -141,18 +144,43 @@ Writing one by hand:
 
   - a record with a destructor or a copy constructor - `RefCountPtr<T>` - comes back through a
     hidden pointer passed *before* `this`, as the call's `sret` argument (in RDI on x86-64, X8 on
-    AArch64). A tuple can't say the C++ type has them, so mark the method `@cppnontrivial`. (A
-    trivial record of more than 16 bytes is returned the same way, so `@cppnontrivial` is right
-    for it too.) MSVC ignores the mark;
-  - a trivial record of one or two 8-byte integer or pointer fields - `nvrhi::Object` - comes
-    back in RAX:RDX / X0:X1, as tslang returns such a tuple: no mark;
-  - any other trivial record of up to 16 bytes is packed into registers by the SysV / AAPCS64
-    classification, which isn't done: such a method is an error, as is any method returning a
-    tuple on another Itanium target (32-bit x86, say).
+    AArch64). A tuple can't say the C++ type has them, so mark the method `@cppnontrivial`. A
+    trivial record of more than 16 bytes is returned the same way, so mark that one too (a call
+    of it unmarked is an error). MSVC ignores the mark;
+  - a trivial record of up to 16 bytes of integers and pointers - `nvrhi::Object`, a struct of
+    three `bool`s, one of three `uint32_t`s - comes back in RAX:RDX / X0:X1, packed as the record
+    is in memory: the call reads the two registers back as the record. No mark;
+  - one with floating-point fields comes back in SSE registers (SysV) or as an HFA (AAPCS64),
+    which isn't done: such a method is an error, as is any method returning a tuple on another
+    Itanium target (32-bit x86, say).
 
   The record isn't destroyed: a `RefCountPtr` the method returns holds a reference, which the
   caller gives back with `Release()`. A method stored in a variable and called later takes the
   hidden pointer as an argument; call it directly.
+- **Structs passed by value.** A parameter of a virtual method that is a trivially copyable
+  struct by value (`nvrhi::TextureSubresourceSet subresources`, `nvrhi::Object texture`) is a
+  tuple with its layout, passed as the C++ ABI passes the struct:
+
+  ```ts
+  type TextureSubresourceSet = [baseMipLevel: u32, numMipLevels: u32, baseArraySlice: u32, numArraySlices: u32];
+  @slot(9) clearTextureFloat(t: ITexture | null, subresources: TextureSubresourceSet, clearColor: Ref<Color>): void;
+  // commandList.clearTextureFloat(texture, [0, 1, 0, 1], Ref(color))
+  ```
+
+  The call copies the tuple into a slot of its own (at least 16 bytes, zeroed), then:
+  - MSVC x64 passes a struct of 1, 2, 4 or 8 bytes in a register, as an integer of its size, and
+    any other by a pointer to the copy (which the callee may change: the caller's tuple doesn't);
+  - SysV x86-64 passes one of up to 16 bytes of integers and pointers in one or two 64-bit
+    registers, when that many of the six remain (else all of it on the stack), and a bigger one on
+    the stack (`byval`);
+  - AAPCS64 passes one of up to 16 bytes in one or two consecutive 64-bit registers (`[N x i64]`,
+    all of it in registers or all on the stack), and a bigger one by a pointer to the copy.
+
+  A struct with a destructor or a copy constructor (which MSVC destroys in the callee, and the
+  Itanium ABI passes by a pointer to a copy the caller destroys) can't be a tuple parameter: take
+  it as a `Ref<T>` in a wrapper of your own, or leave the method out. Nor can a struct with
+  floating-point fields under the Itanium ABI (SSE registers, an HFA's), nor any struct taken by a
+  non-virtual method or by one with a body.
 - **Narrow parameters.** A `bool` is a `boolean`, an `uint8_t` (or an `enum class : uint8_t`) a
   `u8`, an `int16_t` an `s16`: a call of a C++ method extends an argument narrower than 32 bits as
   a C++ caller does (`zeroext`, `signext` for a signed type), which clang's code relies on.

tslang/include/TsClangImporter/TypeMapper.h

@@ -90,6 +90,13 @@ class TypeMapper
     // when C++ can't pass it in registers (a destructor or a copy constructor: @cppnontrivial).
     TsType mapRecordResult(clang::QualType type, bool &nonTrivial);
 
+    // A record a C++ (virtual) method takes by value: the record's own type, which tslang passes
+    // as the target's C++ ABI does (in registers, by a pointer to a copy, on the stack). Skipped
+    // when C++ can't pass it in registers (a destructor or a copy constructor), for a union, for
+    // one with floating-point fields under the Itanium ABI (SSE registers, HFAs: not done), and on
+    // targets other than MSVC x64 and the Itanium ABI on x86-64 and AArch64.
+    TsType mapRecordParameter(clang::QualType type);
+
     // clang's semantic analysis, to complete a class template specialization only ever named as a
     // result (`RefCountPtr<IBuffer> createBuffer(...)` doesn't instantiate it)
     void setSema(clang::Sema *value)

tslang/include/TypeScript/TargetInfo.h

@@ -44,6 +44,19 @@ struct TargetInfo
     // without them comes back in RAX:RDX / X0:X1, as a tuple is returned.
     bool hiddenReturnPointerBeforeThis = false;
 
+    // How the C++ ABI passes a record (a trivially copyable struct, a tuple) to a method by value,
+    // and returns one in registers (LowerToLLVM's lowerCppCallArgs, after clang's
+    // MicrosoftCXXABI / X86_64ABIInfo / AArch64ABIInfo): MSVC x64, the SysV x86-64 one, AAPCS64 -
+    // or none it does that for.
+    enum class CppRecordAbi
+    {
+        None,
+        MicrosoftX64,
+        SysVX86_64,
+        AAPCS64
+    };
+    CppRecordAbi cppRecordAbi = CppRecordAbi::None;
+
     static TargetInfo fromTriple(const llvm::Triple &target, const llvm::Triple &host)
     {
         TargetInfo info;
@@ -70,6 +83,9 @@ struct TargetInfo
         info.hiddenReturnPointerAfterThis = msvc;
         info.hiddenReturnPointerBeforeThis =
             !msvc && (target.getArch() == llvm::Triple::x86_64 || target.getArch() == llvm::Triple::aarch64);
+        info.cppRecordAbi = target.getArch() == llvm::Triple::x86_64 ? (msvc ? CppRecordAbi::MicrosoftX64 : CppRecordAbi::SysVX86_64)
+                            : target.getArch() == llvm::Triple::aarch64 && !msvc ? CppRecordAbi::AAPCS64
+                                                                                  : CppRecordAbi::None;
 
         return info;
     }

tslang/lib/TsClangImporter/HeaderParser.cpp

@@ -674,7 +674,16 @@ class Collector : public clang::RecursiveASTVisitor<Collector>
             auto *parameter = method->getParamDecl(index);
             auto parameterName = parameter->getName().empty() ? "p" + std::to_string(index)
                                                               : escapeReserved(parameter->getName().str());
-            auto type = mapper.map(parameter->getType(), TypeMapper::Use::Parameter);
+            // a record by value, which a virtual method's call passes as the C++ ABI does
+            auto byValueRecord = parameter->getType().getCanonicalType()->getAsRecordDecl() != nullptr;
+            if (byValueRecord && !method->isVirtual())
+            {
+                bound.skipReason = "parameter '" + parameterName + "': a non-virtual method taking a record by value isn't supported yet";
+                return;
+            }
+
+            auto type = byValueRecord ? mapper.mapRecordParameter(parameter->getType())
+                                      : mapper.map(parameter->getType(), TypeMapper::Use::Parameter);
             if (type.skipped())
             {
                 bound.skipReason = "parameter '" + parameterName + "': " + type.skipReason;

tslang/lib/TsClangImporter/TypeMapper.cpp

@@ -309,6 +309,93 @@ std::string TypeMapper::classSkipReason(const clang::CXXRecordDecl *record)
     return reason;
 }
 
+// Whether a type holds floating-point values: itself, a record's fields (its bases' too), an
+// array's elements
+static bool hasFloatingPoint(const clang::ASTContext &context, clang::QualType type)
+{
+    auto canonical = type.getCanonicalType();
+    if (canonical->isRealFloatingType())
+    {
+        return true;
+    }
+
+    if (auto *array = context.getAsArrayType(canonical))
+    {
+        return hasFloatingPoint(context, array->getElementType());
+    }
+
+    auto *record = canonical->getAsRecordDecl();
+    if (!record || !record->getDefinition())
+    {
+        return false;
+    }
+
+    auto *definition = record->getDefinition();
+    if (auto *cxxRecord = llvm::dyn_cast<clang::CXXRecordDecl>(definition))
+    {
+        for (auto &base : cxxRecord->bases())
+        {
+            if (hasFloatingPoint(context, base.getType()))
+            {
+                return true;
+            }
+        }
+    }
+
+    return llvm::any_of(definition->fields(), [&](const clang::FieldDecl *field) { return hasFloatingPoint(context, field->getType()); });
+}
+
+// The targets whose C++ ABI tslang passes records by value with, and returns them in registers
+// (TargetInfo::cppRecordAbi): MSVC x64, the Itanium ABI on x86-64 and AArch64
+static bool hasCppRecordAbi(const clang::ASTContext &context)
+{
+    auto &target = context.getTargetInfo();
+    auto arch = target.getTriple().getArch();
+    return target.getCXXABI().isMicrosoft() ? arch == llvm::Triple::x86_64
+                                            : arch == llvm::Triple::x86_64 || arch == llvm::Triple::aarch64;
+}
+
+TsType TypeMapper::mapRecordParameter(clang::QualType type)
+{
+    auto *record = type.getCanonicalType()->getAsRecordDecl();
+    if (record->isUnion())
+    {
+        return TsType::skip("union passed by value");
+    }
+
+    // a specialization (std::optional<T>) is instantiated for its layout
+    if (!record->getDefinition() && sema)
+    {
+        sema->isCompleteType(clang::SourceLocation(), type.getCanonicalType());
+    }
+
+    auto *definition = record->getDefinition();
+    if (!definition)
+    {
+        return TsType::skip("incomplete struct passed by value");
+    }
+
+    // (MSVC passes one with a destructor in registers too, the callee destroying its copy: of a
+    // RefCountPtr, a reference the caller never added)
+    auto *cxxDefinition = llvm::dyn_cast<clang::CXXRecordDecl>(definition);
+    if (!definition->canPassInRegisters() || (cxxDefinition && !cxxDefinition->isTriviallyCopyable()))
+    {
+        return TsType::skip("struct with a destructor or a copy constructor passed by value");
+    }
+
+    if (!hasCppRecordAbi(context))
+    {
+        return TsType::skip("struct passed by value: not done for this target's C++ ABI");
+    }
+
+    if (!context.getTargetInfo().getCXXABI().isMicrosoft() && hasFloatingPoint(context, type))
+    {
+        return TsType::skip("struct with floating-point fields passed by value");
+    }
+
+    return map(type, Use::Field);
+}
+
 TsType TypeMapper::mapRecordResult(clang::QualType type, bool &nonTrivial)
 {
     auto *record = type.getCanonicalType()->getAsRecordDecl();
@@ -337,7 +424,6 @@ TsType TypeMapper::mapRecordResult(clang::QualType type, bool &nonTrivial)
     uint64_t offset = 0;
     uint64_t maxAlign = 8;
     unsigned index = 0;
-    auto integerRegisterFields = true;
     for (auto *field : definition->fields())
     {
         auto fieldName = field->getName().str();
@@ -383,7 +469,6 @@ TsType TypeMapper::mapRecordResult(clang::QualType type, bool &nonTrivial)
                 return TsType::skip("record returned by value: field '" + fieldName + "': " + type.skipReason);
             }
 
-            integerRegisterFields &= fieldType->isIntegerType() && context.getTypeSize(fieldType) == 64;
         }
 
         auto align = context.getTypeAlign(field->getType());
@@ -406,14 +491,24 @@ TsType TypeMapper::mapRecordResult(clang::QualType type, bool &nonTrivial)
     }
 
     // MSVC returns any record through a hidden pointer, and so does the Itanium ABI one it can't
-    // pass in registers; a trivial one comes back in registers by its fields, which tslang does
-    // for one or two 8-byte integer or pointer fields, on x86-64 and AArch64
+    // pass in registers or of more than 16 bytes (@cppnontrivial, the pointer before `this`); a
+    // trivial one of up to 16 bytes comes back in registers, which tslang reads back as the
+    // record for integers and pointers, on x86-64 and AArch64 - not floating-point fields (SSE
+    // registers, an HFA's)
     if (!context.getTargetInfo().getCXXABI().isMicrosoft() && !nonTrivial)
     {
-        auto arch = context.getTargetInfo().getTriple().getArch();
-        if ((arch != llvm::Triple::x86_64 && arch != llvm::Triple::aarch64) || names.size() > 2 || !integerRegisterFields)
+        if (!hasCppRecordAbi(context))
+        {
+            return TsType::skip("record returned by value: not done for this target's C++ ABI");
+        }
+
+        if (context.getTypeSize(type) > 128)
+        {
+            nonTrivial = true;
+        }
+        else if (hasFloatingPoint(context, type))
         {
-            return TsType::skip("trivial record returned by value in registers by its fields");
+            return TsType::skip("trivial record with floating-point fields returned in registers");
         }
     }