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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In https://reviews.llvm.org/D49464#1170686, @efriedma wrote:

> Reid, are you okay with merging something like the current patch for now, and implementing returning the value as a followup?


Yeah.

Please add some test cases that verify that the right thing happens for instance methods returning POD structs. Should the sret parameter be in X1 or X8? Looks like you want X1, so `isPOD` isn't the right check.

  struct IsPOD { int x; };
  struct Foo {
    IsPOD foo() { return IsPOD{3}; }
  };
  int main() {
    return Foo().foo().x;
  }



================
Comment at: include/clang/CodeGen/CGFunctionInfo.h:547
 
+  bool NonPODStructReturn = true;
+
----------------
TomTan wrote:
> Use a bit field like `ReturnsRetained` above?
Yes, please do not grow this struct needlessly.

Please name it something more generic like `SuppressSRet`.


================
Comment at: lib/CodeGen/TargetInfo.cpp:4926
 
+    FI.setNonPODStructReturn(Kind != Win64);
+
----------------
yinma wrote:
> yinma wrote:
> > Do we should limit this to aarch64 windows only? I feel it will impact x86 64bit .
> I just realized Win64 is under AArch64ABIInfo. So this change should be good.
Based on my test case, you should call `setSuppressSRet(true)` whenever `::classifyReturnType` returns true in the MS C++ ABI.


https://reviews.llvm.org/D49464