r328942 - Fix a major swiftcall ABI bug with trivial C++ class types.

[John McCall via cfe-commits]

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) {}