@cppvtable: methods returning a record through MSVC's hidden pointer

docs/cppvtable-classes.md

@@ -60,6 +60,7 @@ these are errors and not warnings.
 | `instanceof`, a cast to or from `any`, a cast to an interface | each asks the object through slot 0, which is a C++ method |
 | `-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 method returning a tuple, on a non-MSVC target | the Itanium ABI returns records otherwise (see below) |
 
 A generic or static class can't be `@cppvtable` either.
 
@@ -74,14 +75,27 @@ A generic or static class can't be `@cppvtable` either.
   TypeScript name. MSVC groups overloads in the vtable, and not always in declaration order:
   check the slot numbers in the layout report.
 - **Struct returned by value.** A C++ method that returns a class or struct by value (for
-  example `nvrhi::RefCountPtr<T>`) returns it through a hidden pointer. Under MSVC that pointer
-  is passed after `this`, and the method returns it. Declare the method with that parameter:
+  example `nvrhi::RefCountPtr<T>` or `nvrhi::Object`) returns a record. Declare its return type
+  as a tuple with the record's layout:
 
   ```ts
-  createBuffer(ret: Ref<Opaque | null>, desc: Ref<BufferDesc>): Opaque;  // RefCountPtr<IBuffer>
+  type RefCountPtr = [pointer: Opaque | null];          // nvrhi::RefCountPtr<T>
+  createBuffer(desc: Ref<BufferDesc>): RefCountPtr;     // const handle = device.createBuffer(Ref(desc))
   ```
 
-  The pointer it writes holds a reference, which the caller gives back with `Release()`.
+  MSVC returns a record from an instance method through a hidden pointer, whatever its size. The
+  pointer is passed after `this`, and the method returns it as well. A call to such a method
+  passes a local for the result and reads it back, so the call's value is the record. This is
+  clang's `MicrosoftCXXABI::classifyReturnType`: "MSVC always returns structs indirectly from C++
+  instance methods". A method returning a class (`getLayout(): IBindingLayout`, a C++ pointer) or
+  a number still returns it in a register.
+
+  The record isn't destroyed: a `RefCountPtr` the method returns holds a reference, which the
+  caller gives back with `Release()`. The Itanium ABI passes the pointer before `this`, and only
+  for some records, so on a non-MSVC target such a method is an error. A method stored in a
+  variable and called later takes the hidden pointer as its first argument; call it directly.
+  tslang pre-alpha94 and earlier need the hidden pointer declared as a parameter:
+  `createBuffer(ret: Ref<Opaque | null>, desc: Ref<BufferDesc>): Opaque`.
 - **Structs by reference** (`const BufferDesc&`) are a `Ref<T>` to a named tuple with the C++
   layout. That layout depends on the C++ build: `std::string`, for example, is 32 bytes with the
   release CRT and 40 with the debug CRT.

tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h

@@ -151,6 +151,10 @@ struct MethodInfo
     mlir_ts::AccessLevel accessLevel;
     // @dllname: exported symbol name, empty when the method keeps funcName
     std::string dllName;
+    // A @cppvtable method returning a C++ record: the type it is declared with (funcType is the
+    // one its slot is called with, the hidden result pointer after `this`, see
+    // mlirGenClassCppVTableHiddenReturn); null for any other method.
+    mlir_ts::FunctionType declaredFuncType;
 };
 
 struct GenericMethodInfo

tslang/include/TypeScript/TargetInfo.h

@@ -32,6 +32,12 @@ struct TargetInfo
     // convention a class method is called with, so a @cppvtable class can't call them.
     bool memberCallsUseThiscall = false;
 
+    // MSVC returns a C++ record (struct, class) from an instance method through a hidden pointer,
+    // passed after `this` and returned in turn (clang's MicrosoftCXXABI::classifyReturnType): the
+    // convention of a @cppvtable method declared to return a tuple. The Itanium ABI passes it
+    // before `this`, and only for some records.
+    bool hiddenReturnPointerAfterThis = false;
+
     static TargetInfo fromTriple(const llvm::Triple &target, const llvm::Triple &host)
     {
         TargetInfo info;
@@ -55,6 +61,7 @@ struct TargetInfo
         info.supportsInProcessJit =
             target.getArch() == host.getArch() && target.getOS() == host.getOS();
         info.memberCallsUseThiscall = msvc && x86_32;
+        info.hiddenReturnPointerAfterThis = msvc;
 
         return info;
     }

tslang/lib/TypeScript/DeclarationPrinter.cpp

@@ -662,11 +662,13 @@ namespace typescript
                 os << "abstract ";
             }
 
+            // a @cppvtable method returning a record: as declared, the importer lowers it again
+            auto funcType = method.declaredFuncType ? method.declaredFuncType : method.funcType;
             printMethod(
                 method.isStatic,
                 method.name,
-                method.funcType.getParams(),
-                method.funcType.getNumResults() > 0 ? method.funcType.getResult(0) : mlir::Type(),
+                funcType.getParams(),
+                funcType.getNumResults() > 0 ? funcType.getResult(0) : mlir::Type(),
                 classType->classType);
 
             newline();

tslang/lib/TypeScript/MLIRGenAccessCall.cpp

@@ -1751,6 +1751,28 @@ namespace mlirgen
                     getFunctionType(calledBoundFuncType.getInputs(), calledBoundFuncType.getResults(), calledBoundFuncType.isVarArg());
                 auto thisValue = builder.create<mlir_ts::GetThisOp>(location, calledFuncType.getInput(0), funcRefValue);
                 auto unboundFuncRefValue = builder.create<mlir_ts::GetMethodOp>(location, calledFuncType, funcRefValue);
+
+                // a @cppvtable method returning a record (mlirGenClassCppVTableHiddenReturn): it
+                // writes it to a local, passed after `this`, which is the call's value
+                if (auto resultType = cppVTableHiddenReturnType(funcRefValue))
+                {
+                    auto resultSlot = builder.create<mlir_ts::VariableOp>(location, mlir_ts::RefType::get(resultType),
+                                                                         mlir::Value(), builder.getBoolAttr(false),
+                                                                         builder.getIndexAttr(0));
+                    SmallVector<mlir::Value, 4> slotOperands;
+                    slotOperands.push_back(resultSlot);
+                    slotOperands.append(operands.begin(), operands.end());
+                    auto callResult = mlirGenCallFunction(location, calledFuncType, unboundFuncRefValue, thisValue, slotOperands, genContext);
+                    if (mlir::failed(callResult))
+                    {
+                        value = mlir::failure();
+                        return;
+                    }
+
+                    value = V(builder.create<mlir_ts::LoadOp>(location, resultType, resultSlot));
+                    return;
+                }
+
                 value = mlirGenCallFunction(location, calledFuncType, unboundFuncRefValue, thisValue, operands, genContext);
             })
             .Case<mlir_ts::ExtensionFunctionType>([&](auto calledExtentFuncType) {

tslang/lib/TypeScript/MLIRGenClasses.cpp

@@ -369,6 +369,53 @@ namespace mlirgen
         return newClassPtr;
     }
 
+    // MSVC returns a C++ record (struct, class - RefCountPtr<T> too) from an instance method through
+    // a hidden pointer, passed after `this` and returned in turn (clang's
+    // MicrosoftCXXABI::classifyReturnType: "MSVC always returns structs indirectly from C++ instance
+    // methods"). A @cppvtable method declared to return a tuple returns that record: its slot is
+    // called as (this, Ref<T>, args...) -> Ref<T>, the type kept in the MethodInfo, and a call
+    // (mlirGenCall) passes a local of T and reads it back. The declared type stays for
+    // DeclarationPrinter, so an importer lowers it again.
+    mlir::LogicalResult MLIRGenImpl::mlirGenClassCppVTableHiddenReturn(mlir::Location location, ClassInfo::TypePtr newClassPtr,
+                                                          StringRef funcName)
+    {
+        if (!newClassPtr->isCppVTable)
+        {
+            return mlir::success();
+        }
+
+        auto method = llvm::find_if(newClassPtr->methods, [&](auto &method) { return method.funcName == funcName; });
+        if (method == newClassPtr->methods.end() || method->declaredFuncType)
+        {
+            return mlir::success();
+        }
+
+        auto funcType = method->funcType;
+        if (funcType.getNumResults() != 1 || !isa<mlir_ts::TupleType>(funcType.getResult(0)) || funcType.getNumInputs() < 1)
+        {
+            return mlir::success();
+        }
+
+        if (!compileOptions.targetInfo.hiddenReturnPointerAfterThis)
+        {
+            emitError(location) << "a method of a @" CPP_VTABLE " class returning a struct needs the MSVC ABI, which returns it "
+                                << "through a pointer after 'this'";
+            return mlir::failure();
+        }
+
+        auto resultType = funcType.getResult(0);
+        auto resultRefType = mlir_ts::RefType::get(resultType);
+        SmallVector<mlir::Type> inputs;
+        inputs.push_back(funcType.getInput(0));
+        inputs.push_back(resultRefType);
+        inputs.append(funcType.getInputs().begin() + 1, funcType.getInputs().end());
+
+        method->declaredFuncType = funcType;
+        method->funcType = getFunctionType(inputs, {resultRefType}, funcType.isVarArg());
+        cppVTableHiddenReturns[funcName] = resultType;
+        return mlir::success();
+    }
+
     // A @cppvtable class only names the slots of a vtable made by C++, so everything that would add
     // a slot, a field initializer or code of its own is an error, and so is a hierarchy mixing it
     // with ordinary classes: a slot number off by one calls the wrong C++ method, with no error.
@@ -2402,6 +2449,11 @@ genContext);
             classMethodMemberInfo.setFuncOp(funcOp);
             if (classMethodMemberInfo.registerClassMethodMember(loc(funcLikeDeclaration), orderWeight, accessLevel))
             {
+                if (mlir::failed(mlirGenClassCppVTableHiddenReturn(loc(funcLikeDeclaration), newClassPtr, funcOp.getName())))
+                {
+                    return mlir::failure();
+                }
+
                 funcLikeDeclaration->processed = true;
                 return mlir::success();
             }

tslang/lib/TypeScript/MLIRGenExpressions.cpp

@@ -1075,6 +1075,11 @@ namespace mlirgen
 
         SmallVector<mlir::Value, 4> operands;
         auto offsetArgs = isa<mlir_ts::BoundFunctionType>(funcType) || isa<mlir_ts::ExtensionFunctionType>(funcType) ? 1 : 0;
+        // a @cppvtable method returning a record: the arguments follow its hidden result pointer
+        if (cppVTableHiddenReturnType(funcResult))
+        {
+            offsetArgs = 2;
+        }
         if (mlir::failed(mlirGenOperands(callExpression->arguments, operands, funcResult.getType(), genContext, offsetArgs, noReceiverTypesForGenericCall)))
         {
             return mlir::failure();

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -5335,6 +5335,22 @@ class MLIRGenImpl
         return mlir::success();
     }
 
+    // The record a call of funcRef returns through a hidden pointer: funcRef is a @cppvtable method
+    // declared to return a tuple (mlirGenClassCppVTableHiddenReturn); null for any other callee.
+    mlir::Type cppVTableHiddenReturnType(mlir::Value funcRef)
+    {
+        if (auto virtualSymbolRef = funcRef.getDefiningOp<mlir_ts::ThisVirtualSymbolRefOp>())
+        {
+            auto it = cppVTableHiddenReturns.find(virtualSymbolRef.getIdentifier());
+            if (it != cppVTableHiddenReturns.end())
+            {
+                return it->second;
+            }
+        }
+
+        return mlir::Type();
+    }
+
     // A @cppvtable class's objects are C++ objects: their slot 0 is a C++ method (the destructor),
     // not `.instanceOf`, and the class has no RTTI, so nothing may ask one what it is - not
     // `instanceof`, nor boxing into `any` (unboxed by asking), nor an interface (made by asking).
@@ -11201,6 +11217,9 @@ class MLIRGenImpl
         return mlir::failure();
     }
 
+    mlir::LogicalResult mlirGenClassCppVTableHiddenReturn(mlir::Location location, ClassInfo::TypePtr newClassPtr,
+                                                          StringRef funcName);
+
     mlir::LogicalResult mlirGenClassCppVTableCheck(mlir::Location location,
                                                    ClassLikeDeclaration classDeclarationAST,
                                                    ClassInfo::TypePtr newClassPtr, const GenContext &genContext);
@@ -14038,6 +14057,9 @@ class MLIRGenImpl
 
     llvm::ScopedHashTable<StringRef, ClassInfo::TypePtr> fullNameClassesMap;
 
+    // the @cppvtable methods returning a C++ record (by function name): the record's type
+    llvm::StringMap<mlir::Type> cppVTableHiddenReturns;
+
     llvm::ScopedHashTable<StringRef, GenericClassInfo::TypePtr> fullNameGenericClassesMap;
 
     llvm::ScopedHashTable<StringRef, InterfaceInfo::TypePtr> fullNameInterfacesMap;

tslang/test/tester/CMakeLists.txt

@@ -2730,6 +2730,23 @@ add_test(NAME test-compile-cppvtable-memory-model-none
 set_tests_properties(test-compile-cppvtable-memory-model-none
                      PROPERTIES FAIL_REGULAR_EXPRESSION "error:|Stack dump|Assertion failed")
 
+# A method returning a record (tests/00cppvtable_hidden_return.ts) is MSVC's: the Itanium ABI
+# returns a record through a pointer before `this`, and only some records, so it is an error
+# anywhere else and its tests run on Windows only.
+if (WIN32)
+    tslang_add_test(NAME test-compile-00-cppvtable-hidden-return COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00cppvtable_hidden_return.ts")
+    tslang_add_test(NAME test-jit-00-cppvtable-hidden-return COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00cppvtable_hidden_return.ts")
+    tslang_add_test(NAME test-compile-shared-export-import-cppvtable-hidden-return COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_cppvtable_hidden_return.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_cppvtable_hidden_return.ts")
+    tslang_add_test(NAME test-jit-shared-export-import-cppvtable-hidden-return COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_cppvtable_hidden_return.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_cppvtable_hidden_return.ts")
+endif()
+add_test(NAME test-compile-cppvtable-hidden-return-target
+         COMMAND $<TARGET_FILE:tslang> --emit=obj --no-default-lib -mm=none -mtriple=x86_64-unknown-linux-gnu
+                 "${PROJECT_SOURCE_DIR}/test/tester/cppvtable/hidden_return_target.ts"
+                 -o "${CMAKE_CURRENT_BINARY_DIR}/cppvtable-hidden-return-target.obj")
+set_tests_properties(test-compile-cppvtable-hidden-return-target
+                     PROPERTIES PASS_REGULAR_EXPRESSION "hidden_return_target\\.ts:7:5: error: a method of a @cppvtable class returning a struct needs the MSVC ABI"
+                                FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed")
+
 # MSVC member functions are __thiscall on 32-bit x86, which a class method call isn't.
 add_test(NAME test-compile-cppvtable-thiscall-target
          COMMAND $<TARGET_FILE:tslang> --emit=obj --no-default-lib -mm=none -mtriple=i686-pc-windows-msvc

tslang/test/tester/cppvtable/hidden_return_target.ts

@@ -0,0 +1,12 @@
+// Compiled for Linux: the Itanium ABI returns a record from a method otherwise than MSVC, through
+// a pointer before `this`, and only some records.
+type Pair = [first: s32, second: s32];
+
+@cppvtable
+class Methods {
+    pair(x: s32): Pair;
+}
+
+function main() {
+    print("done.");
+}

tslang/test/tester/tests/00cppvtable_hidden_return.ts

@@ -0,0 +1,71 @@
+// A @cppvtable method declared to return a tuple returns a C++ record (a struct, a class such as
+// RefCountPtr<T>) by value. MSVC returns a record from an instance method through a hidden pointer,
+// passed after `this` and returned in turn, so the slot is called as fn(this, &result, args) and the
+// call's value is what fn wrote there. The "C++ methods" here are written in that shape.
+
+declare function memcpy(dst: Opaque, src: Opaque, count: index): Opaque;
+
+type Pair = [first: s32, second: s32];        // 8 bytes: still a hidden pointer, as an instance method's
+type Triple = [a: u64, b: u64, c: u64];       // 24 bytes
+
+@cppvtable
+class Methods {
+    value: s32;
+    pair(x: s32): Pair;                         // slot 0
+    triple(): Triple;                           // slot 1
+    plain(x: s32, y: s32): s32;                 // slot 2: a number, returned as before
+}
+
+@cppvtable
+class DerivedMethods extends Methods {
+    otherPair(x: s32): Pair;                    // slot 3
+}
+
+type ObjectData = [vtable: Opaque, value: s32];
+
+function slotPair(self: Opaque, result: Ref<Pair>, x: s32): Ref<Pair> {
+    let value: Pair = [x + Deref(<Ref<ObjectData>>self).value, x * 2];
+    memcpy(<Opaque>result, <Opaque>Ref(value), sizeof<Pair>());
+    return result;
+}
+
+function slotTriple(self: Opaque, result: Ref<Triple>): Ref<Triple> {
+    let value: Triple = [1, 2, 0x123456789];
+    memcpy(<Opaque>result, <Opaque>Ref(value), sizeof<Triple>());
+    return result;
+}
+
+function slotPlain(self: Opaque, x: s32, y: s32): s32 {
+    return x * y;
+}
+
+function slotOtherPair(self: Opaque, result: Ref<Pair>, x: s32): Ref<Pair> {
+    let value: Pair = [x, -x];
+    memcpy(<Opaque>result, <Opaque>Ref(value), sizeof<Pair>());
+    return result;
+}
+
+function main() {
+    let vtable: Opaque[] = [<Opaque>slotPair, <Opaque>slotTriple, <Opaque>slotPlain, <Opaque>slotOtherPair];
+    let data: ObjectData = [<Opaque>Ref(vtable[0]), 100];
+    const methods = <Methods><Opaque>Ref(data);
+
+    const pair = methods.pair(5);
+    assert(pair.first == 105 && pair.second == 10, "a record returned through the hidden pointer");
+
+    const triple = methods.triple();
+    assert(triple.a == 1 && triple.b == 2 && triple.c == 0x123456789, "a bigger record");
+
+    assert(methods.plain(6, 7) == 42, "a number is still returned in a register, after them");
+    assert(methods.pair(1).second == 2, "a record read from the call's value");
+
+    // a base class's method called through a derived class
+    const derived = <DerivedMethods><Opaque>Ref(data);
+    const inherited = derived.pair(3);
+    assert(inherited.first == 103 && inherited.second == 6, "an inherited method returning a record");
+    const other = derived.otherPair(4);
+    assert(other.first == 4 && other.second == -4, "and the derived class's own");
+    assert(derived.plain(2, 3) == 6, "a number through the derived class");
+
+    print("done.");
+}

tslang/test/tester/tests/export_cppvtable_hidden_return.ts

@@ -0,0 +1,24 @@
+namespace M {
+
+    // A @cppvtable method returning a record (00cppvtable_hidden_return.ts), exported from one
+    // module and called from another: the declaration embedded in the binary (__decls) keeps the
+    // declared tuple, so the importer passes the hidden result pointer too. MSVC only.
+
+    export type Pair = [first: s32, second: s32];
+
+    @cppvtable
+    export class Base {
+        value: s32;
+        first(): s32;        // slot 0
+    }
+
+    @cppvtable
+    export class Derived extends Base {
+        pair(x: s32): Pair;  // slot 1: a record, returned through a pointer after `this`
+    }
+
+    // called through this module's numbering
+    export function callPair(object: Derived, x: s32): Pair {
+        return object.pair(x);
+    }
+}