Reapply:

r107173, "fix PR7519: after thrashing around and remembering how all this stuff"
r107216, "fix PR7523, which was caused by the ABI code calling ConvertType instead"

This includes a fix to make ConvertTypeForMem handle the "recursive" case, and call
it as such when lowering function types which have an indirect result.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@107310 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/CGCall.cpp

@@ -61,28 +61,31 @@ static CanQualType GetReturnType(QualType RetTy) {
 }
 
 const CGFunctionInfo &
-CodeGenTypes::getFunctionInfo(CanQual<FunctionNoProtoType> FTNP) {
+CodeGenTypes::getFunctionInfo(CanQual<FunctionNoProtoType> FTNP,
+                              bool IsRecursive) {
   return getFunctionInfo(FTNP->getResultType().getUnqualifiedType(),
                          llvm::SmallVector<CanQualType, 16>(),
-                         FTNP->getExtInfo());
+                         FTNP->getExtInfo(), IsRecursive);
 }
 
 /// \param Args - contains any initial parameters besides those
 ///   in the formal type
 static const CGFunctionInfo &getFunctionInfo(CodeGenTypes &CGT,
                                   llvm::SmallVectorImpl<CanQualType> &ArgTys,
-                                             CanQual<FunctionProtoType> FTP) {
+                                             CanQual<FunctionProtoType> FTP,
+                                             bool IsRecursive = false) {
   // FIXME: Kill copy.
   for (unsigned i = 0, e = FTP->getNumArgs(); i != e; ++i)
     ArgTys.push_back(FTP->getArgType(i));
   CanQualType ResTy = FTP->getResultType().getUnqualifiedType();
-  return CGT.getFunctionInfo(ResTy, ArgTys, FTP->getExtInfo());
+  return CGT.getFunctionInfo(ResTy, ArgTys, FTP->getExtInfo(), IsRecursive);
 }
 
 const CGFunctionInfo &
-CodeGenTypes::getFunctionInfo(CanQual<FunctionProtoType> FTP) {
+CodeGenTypes::getFunctionInfo(CanQual<FunctionProtoType> FTP,
+                              bool IsRecursive) {
   llvm::SmallVector<CanQualType, 16> ArgTys;
-  return ::getFunctionInfo(*this, ArgTys, FTP);
+  return ::getFunctionInfo(*this, ArgTys, FTP, IsRecursive);
 }
 
 static CallingConv getCallingConventionForDecl(const Decl *D) {
@@ -215,7 +218,8 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(QualType ResTy,
 
 const CGFunctionInfo &CodeGenTypes::getFunctionInfo(CanQualType ResTy,
                            const llvm::SmallVectorImpl<CanQualType> &ArgTys,
-                                            const FunctionType::ExtInfo &Info) {
+                                            const FunctionType::ExtInfo &Info,
+                                                    bool IsRecursive) {
 #ifndef NDEBUG
   for (llvm::SmallVectorImpl<CanQualType>::const_iterator
          I = ArgTys.begin(), E = ArgTys.end(); I != E; ++I)
@@ -243,8 +247,17 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(CanQualType ResTy,
   // various situations, pass it in.
   llvm::SmallVector<const llvm::Type *, 8> PreferredArgTypes;
   for (llvm::SmallVectorImpl<CanQualType>::const_iterator
-       I = ArgTys.begin(), E = ArgTys.end(); I != E; ++I)
-    PreferredArgTypes.push_back(ConvertType(*I));
+       I = ArgTys.begin(), E = ArgTys.end(); I != E; ++I) {
+    // If this is being called from the guts of the ConvertType loop, make sure
+    // to call ConvertTypeRecursive so we don't get into issues with cyclic
+    // pointer type structures.
+    const llvm::Type *ArgType;
+    if (IsRecursive)
+      ArgType = ConvertTypeRecursive(*I);
+    else 
+      ArgType = ConvertType(*I);
+    PreferredArgTypes.push_back(ArgType);
+  }
 
   // Compute ABI information.
   getABIInfo().computeInfo(*FI, getContext(), TheModule.getContext(),
@@ -274,7 +287,8 @@ CGFunctionInfo::CGFunctionInfo(unsigned _CallingConvention,
 /***/
 
 void CodeGenTypes::GetExpandedTypes(QualType Ty,
-                                    std::vector<const llvm::Type*> &ArgTys) {
+                                    std::vector<const llvm::Type*> &ArgTys,
+                                    bool IsRecursive) {
   const RecordType *RT = Ty->getAsStructureType();
   assert(RT && "Can only expand structure types.");
   const RecordDecl *RD = RT->getDecl();
@@ -289,9 +303,9 @@ void CodeGenTypes::GetExpandedTypes(QualType Ty,
 
     QualType FT = FD->getType();
     if (CodeGenFunction::hasAggregateLLVMType(FT)) {
-      GetExpandedTypes(FT, ArgTys);
+      GetExpandedTypes(FT, ArgTys, IsRecursive);
     } else {
-      ArgTys.push_back(ConvertType(FT));
+      ArgTys.push_back(ConvertType(FT, IsRecursive));
     }
   }
 }
@@ -554,11 +568,12 @@ const llvm::FunctionType *CodeGenTypes::GetFunctionType(GlobalDecl GD) {
         cast<FunctionDecl>(GD.getDecl())->getType()->getAs<FunctionProtoType>())
     Variadic = FPT->isVariadic();
 
-  return GetFunctionType(FI, Variadic);
+  return GetFunctionType(FI, Variadic, false);
 }
 
 const llvm::FunctionType *
-CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI, bool IsVariadic) {
+CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI, bool IsVariadic,
+                              bool IsRecursive) {
   std::vector<const llvm::Type*> ArgTys;
 
   const llvm::Type *ResultType = 0;
@@ -571,13 +586,13 @@ CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI, bool IsVariadic) {
 
   case ABIArgInfo::Extend:
   case ABIArgInfo::Direct:
-    ResultType = ConvertType(RetTy);
+    ResultType = ConvertType(RetTy, IsRecursive);
     break;
 
   case ABIArgInfo::Indirect: {
     assert(!RetAI.getIndirectAlign() && "Align unused on indirect return.");
     ResultType = llvm::Type::getVoidTy(getLLVMContext());
-    const llvm::Type *STy = ConvertType(RetTy);
+    const llvm::Type *STy = ConvertType(RetTy, IsRecursive);
     ArgTys.push_back(llvm::PointerType::get(STy, RetTy.getAddressSpace()));
     break;
   }
@@ -615,18 +630,18 @@ CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI, bool IsVariadic) {
 
     case ABIArgInfo::Indirect: {
       // indirect arguments are always on the stack, which is addr space #0.
-      const llvm::Type *LTy = ConvertTypeForMem(it->type);
+      const llvm::Type *LTy = ConvertTypeForMem(it->type, IsRecursive);
       ArgTys.push_back(llvm::PointerType::getUnqual(LTy));
       break;
     }
 
     case ABIArgInfo::Extend:
     case ABIArgInfo::Direct:
-      ArgTys.push_back(ConvertType(it->type));
+      ArgTys.push_back(ConvertType(it->type, IsRecursive));
       break;
 
     case ABIArgInfo::Expand:
-      GetExpandedTypes(it->type, ArgTys);
+      GetExpandedTypes(it->type, ArgTys, IsRecursive);
       break;
     }
   }
@@ -639,7 +654,7 @@ CodeGenTypes::GetFunctionTypeForVTable(const CXXMethodDecl *MD) {
   const FunctionProtoType *FPT = MD->getType()->getAs<FunctionProtoType>();
   
   if (!VerifyFuncTypeComplete(FPT))
-    return GetFunctionType(getFunctionInfo(MD), FPT->isVariadic());
+    return GetFunctionType(getFunctionInfo(MD), FPT->isVariadic(), false);
 
   return llvm::OpaqueType::get(getLLVMContext());
 }
@@ -774,7 +789,7 @@ void CodeGenModule::ConstructAttributeList(const CGFunctionInfo &FI,
       // FIXME: This is rather inefficient. Do we ever actually need to do
       // anything here? The result should be just reconstructed on the other
       // side, so extension should be a non-issue.
-      getTypes().GetExpandedTypes(ParamType, Tys);
+      getTypes().GetExpandedTypes(ParamType, Tys, false);
       Index += Tys.size();
       continue;
     }

lib/CodeGen/CodeGenModule.cpp

@@ -872,6 +872,7 @@ CodeGenModule::GetOrCreateLLVMFunction(llvm::StringRef MangledName,
                                   std::vector<const llvm::Type*>(), false);
     IsIncompleteFunction = true;
   }
+  
   llvm::Function *F = llvm::Function::Create(FTy,
                                              llvm::Function::ExternalLinkage,
                                              MangledName, &getModule());
@@ -932,6 +933,7 @@ llvm::Constant *CodeGenModule::GetAddrOfFunction(GlobalDecl GD,
   // If there was no specific requested type, just convert it now.
   if (!Ty)
     Ty = getTypes().ConvertType(cast<ValueDecl>(GD.getDecl())->getType());
+  
   llvm::StringRef MangledName = getMangledName(GD);
   return GetOrCreateLLVMFunction(MangledName, Ty, GD);
 }

lib/CodeGen/CodeGenTypes.cpp

@@ -43,10 +43,15 @@ CodeGenTypes::~CodeGenTypes() {
 }
 
 /// ConvertType - Convert the specified type to its LLVM form.
-const llvm::Type *CodeGenTypes::ConvertType(QualType T) {
-  llvm::PATypeHolder Result = ConvertTypeRecursive(T);
+const llvm::Type *CodeGenTypes::ConvertType(QualType T, bool IsRecursive) {
+  const llvm::Type *RawResult = ConvertTypeRecursive(T);
+  
+  if (IsRecursive || PointersToResolve.empty())
+    return RawResult;
 
-  // Any pointers that were converted defered evaluation of their pointee type,
+  llvm::PATypeHolder Result = RawResult;
+  
+  // Any pointers that were converted deferred evaluation of their pointee type,
   // creating an opaque type instead.  This is in order to avoid problems with
   // circular types.  Loop through all these defered pointees, if any, and
   // resolve them now.
@@ -80,21 +85,12 @@ const llvm::Type *CodeGenTypes::ConvertTypeRecursive(QualType T) {
   return ResultType;
 }
 
-const llvm::Type *CodeGenTypes::ConvertTypeForMemRecursive(QualType T) {
-  const llvm::Type *ResultType = ConvertTypeRecursive(T);
-  if (ResultType->isIntegerTy(1))
-    return llvm::IntegerType::get(getLLVMContext(),
-                                  (unsigned)Context.getTypeSize(T));
-  // FIXME: Should assert that the llvm type and AST type has the same size.
-  return ResultType;
-}
-
 /// ConvertTypeForMem - Convert type T into a llvm::Type.  This differs from
 /// ConvertType in that it is used to convert to the memory representation for
 /// a type.  For example, the scalar representation for _Bool is i1, but the
 /// memory representation is usually i8 or i32, depending on the target.
-const llvm::Type *CodeGenTypes::ConvertTypeForMem(QualType T) {
-  const llvm::Type *R = ConvertType(T);
+const llvm::Type *CodeGenTypes::ConvertTypeForMem(QualType T, bool IsRecursive){
+  const llvm::Type *R = ConvertType(T, IsRecursive);
 
   // If this is a non-bool type, don't map it.
   if (!R->isIntegerTy(1))
@@ -284,7 +280,8 @@ const llvm::Type *CodeGenTypes::ConvertNewType(QualType T) {
     assert(A.getIndexTypeCVRQualifiers() == 0 &&
            "FIXME: We only handle trivial array types so far!");
     // int X[] -> [0 x int]
-    return llvm::ArrayType::get(ConvertTypeForMemRecursive(A.getElementType()), 0);
+    return llvm::ArrayType::get(ConvertTypeForMemRecursive(A.getElementType()),
+                                0);
   }
   case Type::ConstantArray: {
     const ConstantArrayType &A = cast<ConstantArrayType>(Ty);
@@ -299,7 +296,11 @@ const llvm::Type *CodeGenTypes::ConvertNewType(QualType T) {
   }
   case Type::FunctionNoProto:
   case Type::FunctionProto: {
-    // First, check whether we can build the full function type.
+    // First, check whether we can build the full function type.  If the
+    // function type depends on an incomplete type (e.g. a struct or enum), we
+    // cannot lower the function type.  Instead, turn it into an Opaque pointer
+    // and have UpdateCompletedType revisit the function type when/if the opaque
+    // argument type is defined.
     if (const TagType *TT = VerifyFuncTypeComplete(&Ty)) {
       // This function's type depends on an incomplete tag type; make sure
       // we have an opaque type corresponding to the tag type.
@@ -309,17 +310,25 @@ const llvm::Type *CodeGenTypes::ConvertNewType(QualType T) {
       FunctionTypes.insert(std::make_pair(&Ty, ResultType));
       return ResultType;
     }
+    
     // The function type can be built; call the appropriate routines to
     // build it.
-    if (const FunctionProtoType *FPT = dyn_cast<FunctionProtoType>(&Ty))
-      return GetFunctionType(getFunctionInfo(
-                CanQual<FunctionProtoType>::CreateUnsafe(QualType(FPT,0))),
-                             FPT->isVariadic());
-
-    const FunctionNoProtoType *FNPT = cast<FunctionNoProtoType>(&Ty);
-    return GetFunctionType(getFunctionInfo(
-                CanQual<FunctionNoProtoType>::CreateUnsafe(QualType(FNPT,0))),
-                           true);
+    const CGFunctionInfo *FI;
+    bool isVariadic;
+    if (const FunctionProtoType *FPT = dyn_cast<FunctionProtoType>(&Ty)) {
+      FI = &getFunctionInfo(
+                   CanQual<FunctionProtoType>::CreateUnsafe(QualType(FPT, 0)),
+                            true /*Recursive*/);
+      isVariadic = FPT->isVariadic();
+    } else {
+      const FunctionNoProtoType *FNPT = cast<FunctionNoProtoType>(&Ty);
+      FI = &getFunctionInfo(
+                CanQual<FunctionNoProtoType>::CreateUnsafe(QualType(FNPT, 0)),
+                            true /*Recursive*/);
+      isVariadic = true;
+    }
+
+    return GetFunctionType(*FI, isVariadic, true);
   }
 
   case Type::ObjCObject:

lib/CodeGen/CodeGenTypes.h

@@ -106,19 +106,22 @@ public:
   llvm::LLVMContext &getLLVMContext() { return TheModule.getContext(); }
 
   /// ConvertType - Convert type T into a llvm::Type.
-  const llvm::Type *ConvertType(QualType T);
+  const llvm::Type *ConvertType(QualType T, bool IsRecursive = false);
   const llvm::Type *ConvertTypeRecursive(QualType T);
 
   /// ConvertTypeForMem - Convert type T into a llvm::Type.  This differs from
   /// ConvertType in that it is used to convert to the memory representation for
   /// a type.  For example, the scalar representation for _Bool is i1, but the
   /// memory representation is usually i8 or i32, depending on the target.
-  const llvm::Type *ConvertTypeForMem(QualType T);
-  const llvm::Type *ConvertTypeForMemRecursive(QualType T);
+  const llvm::Type *ConvertTypeForMem(QualType T, bool IsRecursive = false);
+  const llvm::Type *ConvertTypeForMemRecursive(QualType T) {
+    return ConvertTypeForMem(T, true);
+  }
 
   /// GetFunctionType - Get the LLVM function type for \arg Info.
   const llvm::FunctionType *GetFunctionType(const CGFunctionInfo &Info,
-                                            bool IsVariadic);
+                                            bool IsVariadic,
+                                            bool IsRecursive = false);
 
   const llvm::FunctionType *GetFunctionType(GlobalDecl GD);
 
@@ -154,8 +157,11 @@ public:
     return getFunctionInfo(Ty->getResultType(), Args,
                            Ty->getExtInfo());
   }
-  const CGFunctionInfo &getFunctionInfo(CanQual<FunctionProtoType> Ty);
-  const CGFunctionInfo &getFunctionInfo(CanQual<FunctionNoProtoType> Ty);
+
+  const CGFunctionInfo &getFunctionInfo(CanQual<FunctionProtoType> Ty,
+                                        bool IsRecursive = false);
+  const CGFunctionInfo &getFunctionInfo(CanQual<FunctionNoProtoType> Ty,
+                                        bool IsRecursive = false);
 
   // getFunctionInfo - Get the function info for a member function.
   const CGFunctionInfo &getFunctionInfo(const CXXRecordDecl *RD,
@@ -176,7 +182,8 @@ public:
   /// \param ArgTys - must all actually be canonical as params
   const CGFunctionInfo &getFunctionInfo(CanQualType RetTy,
                                const llvm::SmallVectorImpl<CanQualType> &ArgTys,
-                                        const FunctionType::ExtInfo &Info);
+                                        const FunctionType::ExtInfo &Info,
+                                        bool IsRecursive = false);
 
   /// \brief Compute a new LLVM record layout object for the given record.
   CGRecordLayout *ComputeRecordLayout(const RecordDecl *D);
@@ -189,7 +196,8 @@ public:  // These are internal details of CGT that shouldn't be used externally.
   /// GetExpandedTypes - Expand the type \arg Ty into the LLVM
   /// argument types it would be passed as on the provided vector \arg
   /// ArgTys. See ABIArgInfo::Expand.
-  void GetExpandedTypes(QualType Ty, std::vector<const llvm::Type*> &ArgTys);
+  void GetExpandedTypes(QualType Ty, std::vector<const llvm::Type*> &ArgTys,
+                        bool IsRecursive);
   
   /// ContainsPointerToDataMember - Return whether the given type contains a
   /// pointer to a data member.

lib/CodeGen/TargetInfo.cpp

@@ -830,8 +830,7 @@ X86_64ABIInfo::Class X86_64ABIInfo::merge(Class Accum, Class Field) {
   return SSE;
 }
 
-void X86_64ABIInfo::classify(QualType Ty,
-                             uint64_t OffsetBase,
+void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase,
                              Class &Lo, Class &Hi) const {
   // FIXME: This code can be simplified by introducing a simple value class for
   // Class pairs with appropriate constructor methods for the various

test/CodeGen/decl.c

@@ -89,3 +89,16 @@ struct test7s { int a; int b; } test7[] = {
 struct test8s { int f0; char f1; } test8g = {};
 
 
+// PR7519
+
+struct S {
+  void (*x) (struct S *);
+};
+
+extern struct S *global_dc;
+void cp_diagnostic_starter(struct S *);
+
+void init_error(void) {
+  global_dc->x = cp_diagnostic_starter;
+}
+

test/CodeGenCXX/x86_64-arguments.cpp

@@ -30,3 +30,18 @@ typedef int s4::* s4_mdp;
 typedef int (s4::*s4_mfp)();
 s4_mdp f4_0(s4_mdp a) { return a; }
 s4_mfp f4_1(s4_mfp a) { return a; }
+
+
+namespace PR7523 {
+struct StringRef {
+  char *a;
+};
+
+void AddKeyword(StringRef, int x);
+
+void foo() {
+  // CHECK: define void @_ZN6PR75233fooEv()
+  // CHECK: call void @_ZN6PR752310AddKeywordENS_9StringRefEi(i8* {{.*}}, i32 4)
+  AddKeyword(StringRef(), 4);
+}
+}