Author: rjmccall Date: Sun Apr 1 14:04:30 2018 New Revision: 328942 URL: http://llvm.org/viewvc/llvm-project?rev=328942&view=rev Log: Fix a major swiftcall ABI bug with trivial C++ class types.
The problem with the previous logic was that there might not be any explicit copy/move constructor declarations, e.g. if the type is trivial and we've never type-checked a copy of it. Relying on Sema's computation seems much more reliable. Also, I believe Richard's recommendation is exactly the rule we use now on the Itanium ABI, modulo the trivial_abi attribute (which this change of course fixes our handling of in Swift). This does mean that we have a less portable rule for deciding indirectness for swiftcall. I would prefer it if we just applied the Itanium rule universally under swiftcall, but in the meantime, I need to fix this bug. This only arises when defining functions with class-type arguments in C++, as we do in the Swift runtime. It doesn't affect normal Swift operation because we don't import code as C++. Modified: cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp Modified: cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp?rev=328942&r1=328941&r2=328942&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp (original) +++ cfe/trunk/lib/CodeGen/SwiftCallingConv.cpp Sun Apr 1 14:04:30 2018 @@ -742,22 +742,10 @@ void swiftcall::legalizeVectorType(CodeG bool swiftcall::shouldPassCXXRecordIndirectly(CodeGenModule &CGM, const CXXRecordDecl *record) { - // Following a recommendation from Richard Smith, pass a C++ type - // indirectly only if the destructor is non-trivial or *all* of the - // copy/move constructors are deleted or non-trivial. - - if (record->hasNonTrivialDestructor()) - return true; - - // It would be nice if this were summarized on the CXXRecordDecl. - for (auto ctor : record->ctors()) { - if (ctor->isCopyOrMoveConstructor() && !ctor->isDeleted() && - ctor->isTrivial()) { - return false; - } - } - - return true; + // FIXME: should we not rely on the standard computation in Sema, just in + // case we want to diverge from the platform ABI (e.g. on targets where + // that uses the MSVC rule)? + return !record->canPassInRegisters(); } static ABIArgInfo classifyExpandedType(SwiftAggLowering &lowering, Modified: cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp?rev=328942&r1=328941&r2=328942&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp (original) +++ cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp Sun Apr 1 14:04:30 2018 @@ -113,3 +113,13 @@ TEST(struct_indirect_1) // Should not be byval. // CHECK-LABEL: define {{.*}} void @take_struct_indirect_1({{.*}}*{{( %.*)?}}) + +// Do a simple standalone test here of a function definition to ensure that +// we don't have problems due to failure to eagerly synthesize a copy +// constructor declaration. +class struct_trivial { + int x; +}; +// CHECK-LABEL define void @test_struct_trivial(i32{{( %.*)?}}) +extern "C" SWIFTCALL +void test_struct_trivial(struct_trivial triv) {} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits