efriedma added inline comments.
================ Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1106 + + FI.getReturnInfo().setInReg(isAArch64 && !IsSizeGreaterThan128(RD)); ---------------- richard.townsend.arm wrote: > efriedma wrote: > > richard.townsend.arm wrote: > > > I'm not sure what the IsSizeGreaterThan128 check is doing here - if the > > > return type is over 128 bits, then it will be indirectly returned in X8 > > > with this check, which is not always what we want (e.g. in > > > https://bugs.llvm.org/show_bug.cgi?id=41135 ostream.cpp, MSVC expects the > > > value returned in X1). Another check of hasMicrosoftABIRestrictions might > > > be OK, but that's also not quite right due to the size check. > > I think the check here is intended to counteract the similar check in > > hasMicrosoftABIRestrictions... but yes, I don't think that works correctly. > > Looks like we're missing a testcase for a non-aggregate type with size > > greater than 128 bits. > Ah, OK. I think the problem is that we need to check whether the return value > meets the rest of aggregate definition (i.e. !hasMicrosoftABIRestrictions), > as well as being more than 128 bits in size. May be worth splitting the logic > up. Will experiment. I think you might get the right behavior by just erasing both IsSizeGreaterThan128 checks. That matches the way the ABI document describes the rules. Haven't tried it, though. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60349/new/ https://reviews.llvm.org/D60349 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
