[clang] [AArch64] Fix Windows va_arg handling for larger structs

[mstorsjo]

Summary

Aggregate types over 16 bytes are passed by reference.

Contrary to the x86_64 ABI, smaller structs with an odd (non power
of two) are padded and passed in registers.

I noted that I could clean up the rest of the test, sharing check patterns
that are identical, in a separate patch later.

[rnk]

clang/lib/CodeGen/TargetInfo.cpp

6115

Do we need to worry about H[VF]As passed to variadic functions, or is there a special case when passing those to variadics?

[rnk]

lgtm

clang/lib/CodeGen/TargetInfo.cpp

6115

After looking at D100365, I'm assuming the answer is yes. It would be surprising if an HFA passed to a variadic function is passed in integer registers. I suppose it could be passed directly in memory instead of indirectly, but that also seems unlikely.

[mstorsjo]

clang/lib/CodeGen/TargetInfo.cpp

6115

Looks like we don't handle them correctly either, both as fixed and variable arguments in vararg functions - I'll leave this patch uncommitted until I've got that one figured out to see if it should be amended to this one (probably) or warrants a separate patch.

clang/lib/CodeGen/TargetInfo.cpp

Address AArch64ABIInfo::EmitDarwinVAArg(Address VAListAddr, QualType Ty,
   auto TyInfo = getContext().getTypeInfoInChars(Ty);
 
   // Arguments bigger than 16 bytes which aren't homogeneous
   // aggregates should be passed indirectly.
   bool IsIndirect = false;
   if (TyInfo.Width.getQuantity() > 16) {
     const Type *Base = nullptr;
     uint64_t Members = 0;
     IsIndirect = !isHomogeneousAggregate(Ty, Base, Members);
   }
 
   return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect,
                           TyInfo, SlotSize, /*AllowHigherAlign*/ true);
 }
 
 Address AArch64ABIInfo::EmitMSVAArg(CodeGenFunction &CGF, Address VAListAddr,
                                     QualType Ty) const {
-  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*indirect*/ false,
+  bool IsIndirect = false;
+
+  // Composites larger than 16 bytes are passed by reference.
+  if (isAggregateTypeForABI(Ty) && getContext().getTypeSize(Ty) > 128)
+    IsIndirect = true;
+
+  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect,
                           CGF.getContext().getTypeInfoInChars(Ty),
                           CharUnits::fromQuantity(8),
                           /*allowHigherAlign*/ false);
 }
 
 //===----------------------------------------------------------------------===//
 // ARM ABI Implementation
 //===----------------------------------------------------------------------===//

clang/test/CodeGen/ms_abi_aarch64.c

-// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm < %s | FileCheck -check-prefix=LINUX %s
-// RUN: %clang_cc1 -triple aarch64-pc-win32 -emit-llvm < %s | FileCheck -check-prefix=WIN64 %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm < %s | FileCheck -check-prefixes=LINUX,COMMON %s
+// RUN: %clang_cc1 -triple aarch64-pc-win32 -emit-llvm < %s | FileCheck -check-prefixes=WIN64,COMMON %s
+
+struct small_odd {
+  char a, b, c;
+};
+
+struct larger {
+  int a, b, c, d, e;
+};
 
 void __attribute__((ms_abi)) f1(void);
 void f2(void);
 void f3(void) {
   // LINUX-LABEL: define{{.*}} void @f3()
   // WIN64-LABEL: define dso_local void @f3()
   f1();
   // LINUX: call win64cc void @f1()
void __attribute__((ms_abi)) f4(int a, ...) {
   // LINUX-NEXT: store i8* %[[AP_VAL]], i8** %[[AP2:.*]]
   // WIN64: %[[AP_VAL:.*]] = load i8*, i8** %[[AP]]
   // WIN64-NEXT: store i8* %[[AP_VAL]], i8** %[[AP2:.*]]
   __builtin_ms_va_end(ap);
   // LINUX: call void @llvm.va_end
   // WIN64: call void @llvm.va_end
 }
 
+void __attribute__((ms_abi)) f4_2(int a, ...) {
+  // LINUX-LABEL: define{{.*}} win64cc void @f4_2
+  // WIN64-LABEL: define dso_local void @f4_2
+  __builtin_ms_va_list ap;
+  __builtin_ms_va_start(ap, a);
+  // COMMON: %[[AP:.*]] = alloca i8*
+  // COMMON: call void @llvm.va_start
+  struct small_odd s1 = __builtin_va_arg(ap, struct small_odd);
+  // COMMON: %[[AP_CUR:.*]] = load i8*, i8** %[[AP]]
+  // COMMON-NEXT: %[[AP_NEXT:.*]] = getelementptr inbounds i8, i8* %[[AP_CUR]], i64 8
+  // COMMON-NEXT: store i8* %[[AP_NEXT]], i8** %[[AP]]
+  // COMMON-NEXT: bitcast i8* %[[AP_CUR]] to %struct.small_odd*
+  struct larger s2 = __builtin_va_arg(ap, struct larger);
+  // COMMON: %[[AP_CUR2:.*]] = load i8*, i8** %[[AP]]
+  // COMMON-NEXT: %[[AP_NEXT3:.*]] = getelementptr inbounds i8, i8* %[[AP_CUR2]], i64 8
+  // COMMON-NEXT: store i8* %[[AP_NEXT3]], i8** %[[AP]]
+  // COMMON-NEXT: bitcast i8* %[[AP_CUR2]] to %struct.larger**
+  __builtin_ms_va_end(ap);
+}
+
 // Let's verify that normal va_lists work right on Win64, too.
 void f5(int a, ...) {
   // WIN64-LABEL: define dso_local void @f5
   __builtin_va_list ap;
   __builtin_va_start(ap, a);
   // WIN64: %[[AP:.*]] = alloca i8*
   // WIN64: call void @llvm.va_start
   int b = __builtin_va_arg(ap, int);