[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: majnemer, rjmccall.

I discovered that function pointers inside a RecordType that had
its type-determination done in a function whose signature matched
said function pointer resulted in the function pointer type being
emitted empty.  This resulted in information being lost that is
interesting to certain optimizations.

See: https://godbolt.org/g/EegViY

This patch accomplishes this in 2 different situations:
1- When the function itself is being emitted, first convert all

  the return/parameter types to ensure that they are available when
  completing the function type.  This should not result in any additional
  work besides completing parameter types that previously were not completed.

2- When the function type is being evaluated, defer conversion of the record

  type until it is no longer dependent. 


https://reviews.llvm.org/D49403

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenTypes.cpp
  test/CodeGen/func_ptr_recursive_layout.c
  test/CodeGenCXX/pr18962.cpp


Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -756,6 +756,13 @@
 
   unsigned CC = ClangCallConvToLLVMCallConv(info.getCC());
 
+  // Prime the type-cache with the types for this function early.  This enables
+  // parameter types that are dependent on this function's type to be properly
+  // emitted.
+  ConvertType(resultType);
+  for (const CanQualType &QT : argTypes)
+    ConvertType(QT);
+
   // Construct the function info.  We co-allocate the ArgInfos.
   FI = CGFunctionInfo::create(CC, instanceMethod, chainCall, info,
                               paramInfos, resultType, argTypes, required);
Index: lib/CodeGen/CodeGenTypes.cpp
===================================================================
--- lib/CodeGen/CodeGenTypes.cpp
+++ lib/CodeGen/CodeGenTypes.cpp
@@ -169,6 +169,14 @@
   if (const auto *AT = CGT.getContext().getAsArrayType(T))
     return isSafeToConvert(AT->getElementType(), CGT, AlreadyChecked);
 
+  // If this type has a function pointer that is in the process of being laid
+  // out, delay emitting this RecordDecl until that function type is finished.
+  if (T->isFunctionPointerType())
+    return isSafeToConvert(T->getPointeeType(), CGT, AlreadyChecked);
+
+  if (T->isFunctionType())
+    return !CGT.isRecordBeingLaidOut(T.getCanonicalType().getTypePtr());
+
   // Otherwise, there is no concern about transforming this.  We only care about
   // things that are contained by-value in a structure that can have another 
   // structure as a member.
Index: test/CodeGen/func_ptr_recursive_layout.c
===================================================================
--- test/CodeGen/func_ptr_recursive_layout.c
+++ test/CodeGen/func_ptr_recursive_layout.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -emit-llvm -o - %s | FileCheck %s
+
+struct has_fp;
+typedef void (*fp)(const struct has_fp *f);
+struct has_fp {
+  fp i;
+};
+void func(const struct has_fp *f);
+
+void usage() {
+  (void)func;
+}
+
+struct has_fp2;
+typedef void (*fp2)(const struct has_fp2 *f);
+struct has_fp2 {
+  fp2 i;
+};
+void func2(const struct has_fp2 *f) {}
+
+// CHECK-DAG: %struct.has_fp = type { void (%struct.has_fp*)* }
+// CHECK-DAG: %struct.has_fp2 = type { void (%struct.has_fp2*)* }
+
Index: test/CodeGenCXX/pr18962.cpp
===================================================================
--- test/CodeGenCXX/pr18962.cpp
+++ test/CodeGenCXX/pr18962.cpp
@@ -25,7 +25,7 @@
 }
 
 // We end up using an opaque type for 'append' to avoid circular references.
-// CHECK: %class.A = type { {}* }
+// CHECK: %class.A = type { void (%class.A*)* }
 // CHECK: %class.C = type <{ %class.D*, %class.B, [3 x i8] }>
 // CHECK: %class.D = type { %class.C.base, [3 x i8] }
 // CHECK: %class.C.base = type <{ %class.D*, %class.B }>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49403.155768.patch
Type: text/x-patch
Size: 2870 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180716/de2e7cdd/attachment-0001.bin>