[PATCH] D68157: [X86][ABI] Keep empty class argument passing by value compatible with GCC.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added a reviewer: craig.topper.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68157

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/empty-class.cpp


Index: clang/test/CodeGen/empty-class.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGen/empty-class.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64 | FileCheck --check-prefix=X64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686 | FileCheck --check-prefix=X32 %s
+
+class Empty {};
+
+void bar(Empty *);
+
+// X64-LABEL: _Z3foo5Empty
+// X64-SAME: %class.Empty* byval(%class.Empty) align 8 %e
+// X64-NOT: alloca
+// X32-LABEL: _Z3foo5Empty
+// X32-SAME: %class.Empty* byval(%class.Empty) align 4 %e
+// X32-NOT: alloca
+void foo(Empty e) {
+  bar(&e);
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===================================================================
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1670,8 +1670,12 @@
       return getIndirectResult(Ty, true, State);
 
     // Ignore empty structs/unions on non-Windows.
-    if (!IsWin32StructABI && isEmptyRecord(getContext(), Ty, true))
+    if (!IsWin32StructABI && isEmptyRecord(getContext(), Ty, true)) {
+      // For compatibility with GCC, treat it like a struct with a single char.
+      if (getContext().getLangOpts().CPlusPlus)
+        return getIndirectResult(Ty, /*ByVal=*/true, State);
       return ABIArgInfo::getIgnore();
+    }
 
     llvm::LLVMContext &LLVMContext = getVMContext();
     llvm::IntegerType *Int32 = llvm::Type::getInt32Ty(LLVMContext);
@@ -2799,6 +2803,16 @@
     if (RD->hasFlexibleArrayMember())
       return;
 
+    // According to the resolution to A-5 in
+    // https://itanium-cxx-abi.github.io/cxx-abi/cxx-open.html, empty class by
+    // value should be treated as a struct containing a single character.
+    // That's aligned with record layout in AST and compatible with gcc.
+    // Check https://godbolt.org/z/D1u2MV for the difference in codegen.
+    if (getContext().getLangOpts().CPlusPlus)
+      if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD))
+        if (CXXRD->isEmpty())
+          return;
+
     const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD);
 
     // Reset Lo class, this will be recomputed.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68157.222229.patch
Type: text/x-patch
Size: 2180 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190927/822f8470/attachment-0001.bin>