richard.townsend.arm added inline comments.
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1058
+
+ // 1. For return types <= 16 bytes, use the C return semantics.
+
Microsoft have updated the spec since this was written, it now says to check
for aggregate-ness, t
richard.townsend.arm added inline comments.
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1090
+FI.getReturnInfo().setSRetAfterThis(isInstanceMethod);
+FI.getReturnInfo().setInReg(isAArch64 && isIndirectReturn);
Based on https://reviews.llvm.org/D60348 -
richard.townsend.arm added inline comments.
Comment at: lib/Sema/SemaDeclCXX.cpp:5908
+// The checks below ensure that thare no user provided constructors.
+// For AArch64, we use the C++14 definition of an aggregate, so we also
+// check for:
I think
richard.townsend.arm added inline comments.
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1074
+ if (!RD->hasTrivialCopyAssignment())
+return true;
+ return false;
Should this function also check for user-provided constructors?
CHANGES SINCE LAST ACTION
h
richard.townsend.arm added inline comments.
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1074
+ if (!RD->hasTrivialCopyAssignment())
+return true;
+ return false;
richard.townsend.arm wrote:
> Should this function also check for user-provided constructors?
I
richard.townsend.arm added a comment.
Confirmed just now that the current diff (196774) does work, but it'd be good
to correct user-provided constructors and trivial destructors before this lands.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60349/new/
https://reviews.llvm.org/D60349
richard.townsend.arm added a comment.
The current diff (196894) seems to have the same `std::setw` issue as the
previous one. Here's what it's trying to compile:
_MRTIMP2 _Smanip __cdecl setw(streamsize wide)
{ // manipulator to set width
return (_Smanip(&swfun, wide));
}
Her
richard.townsend.arm added a comment.
Apologies, I meant to write "here's what Clang is trying to call" in the
previous comment. It's clear from looking at MSVC's output that MSVC expects to
return indirectly via X0, implying that `_Smanip` is not aggregate by MSVC's
definition.
CHANGES SINCE
richard.townsend.arm added inline comments.
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1080
+ if (const auto *Constructor = dyn_cast(RD))
+if (Constructor->isUserProvided())
+ return true;
So I think that the problem with this new check is that it does
richard.townsend.arm added inline comments.
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106
+
+FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD));
I'm not sure what the IsSizeGreaterThan128 check is doing here - if the return
type is over
richard.townsend.arm added inline comments.
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106
+
+FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD));
efriedma wrote:
> richard.townsend.arm wrote:
> > I'm not sure what the IsSizeGreaterThan128
richard.townsend.arm added inline comments.
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106
+
+FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD));
efriedma wrote:
> richard.townsend.arm wrote:
> > efriedma wrote:
> > > richard.townsend.arm
richard.townsend.arm added a comment.
And with those modifications, everything seems to work correctly. I'd be fine
with the LLVM portion landing at this stage, but one more rev and another test
cycle for this patch would be ideal.
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:
richard.townsend.arm added a comment.
Just completed testing... everything seems to be working correctly. So long as
the all the tests pass, let's commit! :D
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60349/new/
https://reviews.llvm.org/D60349
14 matches
Mail list logo
