Fix Objective-C++ Sret of non-trivial data types on Windows ARM64

[hmelder]

Linked to gnustep/libobjc2#289.

More information can be found in issue: #88273.

My solution involves creating a new message-send function for this calling convention when targeting MSVC. Additional information is available in the libobjc2 pull request.

I am unsure whether we should check for a runtime version where objc_msgSend_stret2_np is guaranteed to be present or leave it as is, considering it remains a critical bug. What are your thoughts about this @davidchisnall?

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Hugo Melder (hmelder)

Changes

Linked to gnustep/libobjc2#289.

More information can be found in issue: #88273.

My solution involves creating a new message-send function for this calling convention when targeting MSVC. Additional information is available in the libobjc2 pull request.

I am unsure whether we should check for a runtime version where objc_msgSend_stret2_np is guaranteed to be present or leave it as is, considering it remains a critical bug. What are your thoughts about this @davidchisnall?

---

Full diff: https://github.com/llvm/llvm-project/pull/88671.diff

4 Files Affected:

• (modified) clang/lib/CodeGen/CGCall.cpp (+5)
• (modified) clang/lib/CodeGen/CGObjCGNU.cpp (+17-4)
• (modified) clang/lib/CodeGen/CodeGenModule.h (+3)
• (added) clang/test/CodeGenObjCXX/msabi-stret-arm64.mm (+77)

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 7a0bc6fa77b889..5ea26b0f594815 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1585,6 +1585,11 @@ bool CodeGenModule::ReturnTypeUsesSRet(const CGFunctionInfo &FI) {
   return RI.isIndirect() || (RI.isInAlloca() && RI.getInAllocaSRet());
 }
 
+bool CodeGenModule::ReturnTypeHasInReg(const CGFunctionInfo &FI) {
+  const auto &RI = FI.getReturnInfo();
+  return RI.getInReg();
+}
+
 bool CodeGenModule::ReturnSlotInterferesWithArgs(const CGFunctionInfo &FI) {
   return ReturnTypeUsesSRet(FI) &&
          getTargetCodeGenInfo().doesReturnSlotInterfereWithArgs();
diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 4e7f777ba1d916..5faecc6af33556 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -2911,12 +2911,25 @@ CGObjCGNU::GenerateMessageSend(CodeGenFunction &CGF,
                                       "objc_msgSend_fpret")
                 .getCallee();
       } else if (CGM.ReturnTypeUsesSRet(MSI.CallInfo)) {
+        StringRef name = "objc_msgSend_stret";
+
+        // The address of the memory block is be passed in x8 for POD type,
+        // or in x0 for non-POD type (marked as inreg).
+        bool shouldCheckForInReg =
+            CGM.getContext()
+                .getTargetInfo()
+                .getTriple()
+                .isWindowsMSVCEnvironment() &&
+            CGM.getContext().getTargetInfo().getTriple().isAArch64();
+        if (shouldCheckForInReg && CGM.ReturnTypeHasInReg(MSI.CallInfo)) {
+          name = "objc_msgSend_stret2_np";
+        }
+
         // The actual types here don't matter - we're going to bitcast the
         // function anyway
-        imp =
-            CGM.CreateRuntimeFunction(llvm::FunctionType::get(IdTy, IdTy, true),
-                                      "objc_msgSend_stret")
-                .getCallee();
+        imp = CGM.CreateRuntimeFunction(
+                     llvm::FunctionType::get(IdTy, IdTy, true), name)
+                  .getCallee();
       } else {
         imp = CGM.CreateRuntimeFunction(
                      llvm::FunctionType::get(IdTy, IdTy, true), "objc_msgSend")
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 1cc447765e2c97..be43a18fc60856 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1241,6 +1241,9 @@ class CodeGenModule : public CodeGenTypeCache {
   /// Return true iff the given type uses 'sret' when used as a return type.
   bool ReturnTypeUsesSRet(const CGFunctionInfo &FI);
 
+  /// Return true iff the given type has `inreg` set.
+  bool ReturnTypeHasInReg(const CGFunctionInfo &FI);
+
   /// Return true iff the given type uses an argument slot when 'sret' is used
   /// as a return type.
   bool ReturnSlotInterferesWithArgs(const CGFunctionInfo &FI);
diff --git a/clang/test/CodeGenObjCXX/msabi-stret-arm64.mm b/clang/test/CodeGenObjCXX/msabi-stret-arm64.mm
new file mode 100644
index 00000000000000..0a17b5862b338e
--- /dev/null
+++ b/clang/test/CodeGenObjCXX/msabi-stret-arm64.mm
@@ -0,0 +1,77 @@
+// RUN: %clang_cc1 -triple aarch64-pc-windows-msvc -fobjc-runtime=gnustep-2.2 -fobjc-dispatch-method=non-legacy -emit-llvm -o - %s | FileCheck %s
+
+// Pass and return for type size <= 8 bytes.
+struct S1 {
+  int a[2];
+};
+
+// Pass and return hfa <= 8 bytes
+struct F1 {
+  float a[2];
+};
+
+// Pass and return for type size > 16 bytes.
+struct S2 {
+  int a[5];
+};
+
+// Pass and return aggregate (of size < 16 bytes) with non-trivial destructor.
+// Sret and inreg: Returned in x0
+struct S3 {
+  int a[3];
+  ~S3();
+};
+S3::~S3() {
+}
+
+
+@interface MsgTest { id isa; } @end
+@implementation MsgTest 
+- (S1) smallS1 {
+  S1 x;
+  x.a[0] = 0;
+  x.a[1] = 1;
+  return x;
+
+}
+- (F1) smallF1 {
+  F1 x;
+  x.a[0] = 0.2f;
+  x.a[1] = 0.5f;
+  return x;
+}
+- (S2) stretS2 {
+  S2 x;
+  for (int i = 0; i < 5; i++) {
+    x.a[i] = i;
+  }
+  return x;
+}
+- (S3) stretInRegS3 {
+  S3 x;
+  for (int i = 0; i < 3; i++) {
+    x.a[i] = i;
+  }
+  return x;
+}
++ (S3) msgTestStretInRegS3 {
+  S3 x;
+  for (int i = 0; i < 3; i++) {
+    x.a[i] = i;
+  }
+  return x;
+}
+@end
+
+void test0(MsgTest *t) {
+    // CHECK: call {{.*}} @objc_msgSend
+    S1 ret = [t smallS1];
+    // CHECK: call {{.*}} @objc_msgSend
+    F1 ret2 = [t smallF1];
+    // CHECK: call {{.*}} @objc_msgSend_stret
+    S2 ret3 = [t stretS2];
+    // CHECK: call {{.*}} @objc_msgSend_stret2_np
+    S3 ret4 = [t stretInRegS3];
+    // CHECK: call {{.*}} @objc_msgSend_stret2_np
+    S3 ret5 = [MsgTest msgTestStretInRegS3];
+}

[hmelder]

Seems like the formatting error in buildkite is unrelated to this PR.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[hmelder]

@davidchisnall thank you for reviewing the PR. Do you have merge permissions?

[efriedma-quic]

@davidchisnall Is this patch waiting for something? (I got a request to check on the progress of this patch.)