Merge pull request #67685 from hyp/eng/hoping-one-day-67674-gets-fixed

[cxx-interop][windows] additional fix for indirect return in methods

Author: hyp

lib/IRGen/GenCall.cpp

@@ -1424,22 +1424,7 @@ void SignatureExpansion::expandExternalSignatureTypes() {
                                   attrKindForExtending(signExt));
   }
 
-  // If we return indirectly, that is the first parameter type.
-  if (returnInfo.isIndirect()) {
-    addIndirectResult();
-  }
-
-  size_t firstParamToLowerNormally = 0;
-
-  // Use a special IR type for passing block pointers.
-  if (FnType->getRepresentation() == SILFunctionTypeRepresentation::Block) {
-    assert(FI.arg_begin()[0].info.isDirect() &&
-           "block pointer not passed directly?");
-    ParamIRTypes.push_back(IGM.ObjCBlockPtrTy);
-    firstParamToLowerNormally = 1;
-  }
-
-  for (auto i : indices(paramTys).slice(firstParamToLowerNormally)) {
+  auto emitArg = [&](size_t i) {
     auto &AI = FI.arg_begin()[i].info;
 
     // Add a padding argument if required.
@@ -1528,8 +1513,35 @@ void SignatureExpansion::expandExternalSignatureTypes() {
     case clang::CodeGen::ABIArgInfo::InAlloca:
       llvm_unreachable("Need to handle InAlloca during signature expansion");
     }
+  };
+
+  size_t firstParamToLowerNormally = 0;
+
+  // If we return indirectly, that is the first parameter type.
+  if (returnInfo.isIndirect()) {
+    if (IGM.Triple.isWindowsMSVCEnvironment() &&
+        FnType->getRepresentation() ==
+            SILFunctionTypeRepresentation::CXXMethod) {
+      // Windows ABI places `this` before the
+      // returned indirect values.
+      emitArg(0);
+      firstParamToLowerNormally = 1;
+      addIndirectResult();
+    } else
+      addIndirectResult();
+  }
+
+  // Use a special IR type for passing block pointers.
+  if (FnType->getRepresentation() == SILFunctionTypeRepresentation::Block) {
+    assert(FI.arg_begin()[0].info.isDirect() &&
+           "block pointer not passed directly?");
+    ParamIRTypes.push_back(IGM.ObjCBlockPtrTy);
+    firstParamToLowerNormally = 1;
   }
 
+  for (auto i : indices(paramTys).slice(firstParamToLowerNormally))
+    emitArg(i);
+
   if (returnInfo.isIndirect() || returnInfo.isIgnore()) {
     ResultIRType = IGM.VoidTy;
   } else {
@@ -2894,6 +2906,15 @@ void CallEmission::emitToUnmappedMemory(Address result) {
   assert(LastArgWritten == 1 && "emitting unnaturally to indirect result");
 
   Args[0] = result.getAddress();
+  if (IGF.IGM.Triple.isWindowsMSVCEnvironment() &&
+      getCallee().getRepresentation() ==
+          SILFunctionTypeRepresentation::CXXMethod &&
+      Args[1] == getCallee().getCXXMethodSelf()) {
+    // C++ methods in MSVC ABI pass `this` before the
+    // indirectly returned value.
+    std::swap(Args[0], Args[1]);
+    assert(!isa<llvm::UndefValue>(Args[1]));
+  }
   SILFunctionConventions FnConv(CurCallee.getSubstFunctionType(),
                                 IGF.getSILModule());
 
@@ -3264,6 +3285,37 @@ void CallEmission::emitToExplosion(Explosion &out, bool isOutlined) {
       emitToMemory(temp, substResultTI, isOutlined);
       return;
     }
+    if (IGF.IGM.Triple.isWindowsMSVCEnvironment() &&
+        getCallee().getRepresentation() ==
+            SILFunctionTypeRepresentation::CXXMethod &&
+        substResultType.isVoid()) {
+      // Some C++ methods return a value but are imported as
+      // returning `Void` (e.g. `operator +=`). In this case
+      // we should allocate the correct temp indirect return
+      // value for it.
+      // FIXME: MSVC ABI hits this as it makes some SIL direct
+      // returns as indirect at IR layer, so fix this for MSVC
+      // first to get this into Swfit 5.9. However, then investigate
+      // if this could also apply to Itanium ABI too.
+      auto fnType = getCallee().getFunctionPointer().getFunctionType();
+      assert(fnType->getNumParams() > 1);
+      auto func = dyn_cast<llvm::Function>(
+          getCallee().getFunctionPointer().getRawPointer());
+      if (func) {
+        // `this` comes before the returned value under the MSVC ABI
+        // so return value is parameter #1.
+        assert(func->hasParamAttribute(1, llvm::Attribute::StructRet));
+        auto resultTy = func->getParamStructRetType(1);
+        auto temp = IGF.createAlloca(resultTy, Alignment(/*safe alignment*/ 16),
+                                     "indirect.result");
+        if (IGF.IGM.getLLVMContext().supportsTypedPointers()) {
+          temp = IGF.Builder.CreateElementBitCast(
+              temp, fnType->getParamType(1)->getNonOpaquePointerElementType());
+        }
+        emitToMemory(temp, substResultTI, isOutlined);
+        return;
+      }
+    }
 
     StackAddress ctemp = substResultTI.allocateStack(IGF, substResultType,
                                                      "call.aggresult");

test/Interop/Cxx/class/method/msvc-abi-return-indirect-trivial-record.swift

@@ -0,0 +1,70 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %target-swift-emit-irgen -I %t/Inputs -enable-experimental-cxx-interop  -Xcc -std=c++17 %t/test.swift -module-name Test | %FileCheck %s
+
+// REQUIRES: OS=windows-msvc
+
+//--- Inputs/module.modulemap
+module MsvcUseVecIt {
+    header "test.h"
+    requires cplusplus
+}
+
+//--- Inputs/test.h
+
+#pragma once
+
+class str {
+    int x;
+};
+
+class It {
+public:
+    int x;
+
+    bool operator ==(const It &other) const ;
+    bool operator !=(const It &other) const ;
+};
+
+template<class T> class vec {
+    int x;
+public:
+    vec(const vec<T> &);
+    ~vec();
+
+    It begin() const;
+    It end() const;
+};
+
+using VecStr = vec<str>;
+
+struct LoadableIntWrapper {
+    int value;
+
+    LoadableIntWrapper operator+=(LoadableIntWrapper rhs) {
+        value += rhs.value;
+        return *this;
+    }
+};
+
+//--- test.swift
+
+import MsvcUseVecIt
+
+public func test(_ result: VecStr) -> CInt {
+    let begin = result.__beginUnsafe()
+    return begin.x
+}
+
+// CHECK: swiftcc i32 @"$s4Test4testys5Int32VSo0014vecstr_yuJCataVF"({{.*}} %[[RESULT:.*]])
+// CHECK: call void @"?begin@?$vec@Vstr@@@@QEBA?AVIt@@XZ"(ptr %[[RESULT]], ptr sret{{.*}}
+
+public func passTempForIndirectRetToVoidCall() {
+    var lhs = LoadableIntWrapper(value: 2)
+    let rhs = LoadableIntWrapper(value: 2)
+    lhs += rhs
+}
+
+// CHECK: void @"$sSo18LoadableIntWrapperV2peoiyyABz_ABtFZ"(ptr
+// CHECK: %[[OPRESULT:.*]] = alloca %struct.LoadableIntWrapper, align 16
+// CHECK: call void @"??YLoadableIntWrapper@@QEAA?AU0@U0@@Z"(ptr {{.*}}, ptr sret(%struct.LoadableIntWrapper) %[[OPRESULT]], i32

test/Interop/Cxx/stdlib/Inputs/module.modulemap

@@ -22,3 +22,9 @@ module StdPair {
   header "std-pair.h"
   requires cplusplus
 }
+
+module MsvcUseVecIt {
+  header "msvc-std-vector-it.h"
+  requires cplusplus
+  export *
+}

test/Interop/Cxx/stdlib/Inputs/msvc-std-vector-it.h

@@ -0,0 +1,12 @@
+#pragma once
+
+#include <vector>
+#include <string>
+
+struct FetchProvidersResult {
+  std::vector<std::string> providers;
+};
+
+inline FetchProvidersResult *f() noexcept {
+    return new FetchProvidersResult();
+}

test/Interop/Cxx/stdlib/msvc-abi-use-vector-iterator.swift

@@ -0,0 +1,17 @@
+// RUN: %target-run-simple-swift(-I %S/Inputs -Xfrontend -enable-experimental-cxx-interop -Xcc -std=c++17)
+
+// REQUIRES: OS=windows-msvc
+// REQUIRES: executable_test
+
+import MsvcUseVecIt
+
+// CHECK:   call void @"?begin@?$vector@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@V?$allocator@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@2@@std@@QEBA?AV?$_Vector_const_iterator@V?$_Vector_val@U?$_Simple_types@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@std@@@std@@@2@XZ"(ptr {{.*}}, ptr noalias nocapture sret
+
+func test() -> Bool {
+    let result = f()
+    let begin = result.pointee.providers.__beginUnsafe()
+    let end = result.pointee.providers.__endUnsafe()
+    return begin != end
+}
+
+let _ = test()