[PATCH] D49464: [COFF, ARM64] Mark only POD-type returns as SRET

[Yin Ma via Phabricator via llvm-commits]

yinma added inline comments.


================
Comment at: lib/CodeGen/CGCall.cpp:1988
+    if (getTarget().getTriple().isOSWindows() &&
+        getTarget().getTriple().getArch() == llvm::Triple::aarch64) {
+      const CXXRecordDecl *RD = FI.getReturnType()->getAsCXXRecordDecl();
----------------
efriedma wrote:
> We shouldn't be checking the triple here.  Can we extend CGFunctionInfo to record this instead, then perform the check in AArch64ABIInfo in lib/CodeGen/TargetInfo.cpp?
One thing we can do is to add a new ABIKind in AArch64ABIInfo, and test it as the condition 


================
Comment at: lib/CodeGen/CGCall.cpp:1990
+      const CXXRecordDecl *RD = FI.getReturnType()->getAsCXXRecordDecl();
+      if (RD && RD->isPOD())
+        SRETAttrs.addAttribute(llvm::Attribute::StructRet);
----------------
efriedma wrote:
> Are you sure isPOD() is the right check?  I wouldn't trust the spec; for other Microsoft targets in other similar situations, we usually check whether the class has a trivial copy constructor, or something like that; see MicrosoftCXXABI::getRecordArgABI.
Do you have any information about why isPOD is not the right check? The POD definition is defined as the spec. I am seeing clang has many places to setup  PlainOldData = false. It doesn't look like it is just about testing constructor for me. 


https://reviews.llvm.org/D49464