[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes
sberg added a comment. see https://bugs.llvm.org/show_bug.cgi?id=37161 "clang-cl triggers ASTContext::getASTRecordLayout Assertion `D && 'Cannot get layout of forward declarations!''" for what appears to be fallout from this change Repository: rL LLVM https://reviews.llvm.org/D45112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329009: [MS] Emit vftable thunks for functions with
incomplete prototypes (authored by rnk, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D45112?vs=140499=140681#toc
Repository:
rL LLVM
https://reviews.llvm.org/D45112
Files:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/lib/CodeGen/CGVTables.h
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/CodeGenTypes.h
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenCXX/ms-thunks-unprototyped-return.cpp
cfe/trunk/test/CodeGenCXX/ms-thunks-unprototyped.cpp
Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -514,8 +514,8 @@
/// correct type, and the caller will bitcast the function to the correct
/// prototype.
const CGFunctionInfo &
-CodeGenTypes::arrangeMSMemberPointerThunk(const CXXMethodDecl *MD) {
- assert(MD->isVirtual() && "only virtual memptrs have thunks");
+CodeGenTypes::arrangeUnprototypedMustTailThunk(const CXXMethodDecl *MD) {
+ assert(MD->isVirtual() && "only methods have thunks");
CanQual FTP = GetFormalType(MD);
CanQualType ArgTys[] = { GetThisType(Context, MD->getParent()) };
return arrangeLLVMFunctionInfo(Context.VoidTy, /*instanceMethod=*/false,
Index: cfe/trunk/lib/CodeGen/CodeGenModule.h
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.h
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h
@@ -800,7 +800,8 @@
ConstantAddress GetAddrOfUuidDescriptor(const CXXUuidofExpr* E);
/// Get the address of the thunk for the given global decl.
- llvm::Constant *GetAddrOfThunk(GlobalDecl GD, const ThunkInfo );
+ llvm::Constant *GetAddrOfThunk(StringRef Name, llvm::Type *FnTy,
+ GlobalDecl GD);
/// Get a reference to the target of VD.
ConstantAddress GetWeakRefReference(const ValueDecl *VD);
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h
@@ -1692,10 +1692,10 @@
void FinishFunction(SourceLocation EndLoc=SourceLocation());
void StartThunk(llvm::Function *Fn, GlobalDecl GD,
- const CGFunctionInfo );
+ const CGFunctionInfo , bool IsUnprototyped);
- void EmitCallAndReturnForThunk(llvm::Constant *Callee,
- const ThunkInfo *Thunk);
+ void EmitCallAndReturnForThunk(llvm::Constant *Callee, const ThunkInfo *Thunk,
+ bool IsUnprototyped);
void FinishThunk();
@@ -1705,7 +1705,8 @@
/// Generate a thunk for the given method.
void generateThunk(llvm::Function *Fn, const CGFunctionInfo ,
- GlobalDecl GD, const ThunkInfo );
+ GlobalDecl GD, const ThunkInfo ,
+ bool IsUnprototyped);
llvm::Function *GenerateVarArgsThunk(llvm::Function *Fn,
const CGFunctionInfo ,
Index: cfe/trunk/lib/CodeGen/CGVTables.cpp
===
--- cfe/trunk/lib/CodeGen/CGVTables.cpp
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp
@@ -31,21 +31,9 @@
CodeGenVTables::CodeGenVTables(CodeGenModule )
: CGM(CGM), VTContext(CGM.getContext().getVTableContext()) {}
-llvm::Constant *CodeGenModule::GetAddrOfThunk(GlobalDecl GD,
- const ThunkInfo ) {
- const CXXMethodDecl *MD = cast(GD.getDecl());
-
- // Compute the mangled name.
- SmallString<256> Name;
- llvm::raw_svector_ostream Out(Name);
- if (const CXXDestructorDecl* DD = dyn_cast(MD))
-getCXXABI().getMangleContext().mangleCXXDtorThunk(DD, GD.getDtorType(),
- Thunk.This, Out);
- else
-getCXXABI().getMangleContext().mangleThunk(MD, Thunk, Out);
-
- llvm::Type *Ty = getTypes().GetFunctionTypeForVTable(GD);
- return GetOrCreateLLVMFunction(Name, Ty, GD, /*ForVTable=*/true,
+llvm::Constant *CodeGenModule::GetAddrOfThunk(StringRef Name, llvm::Type *FnTy,
+ GlobalDecl GD) {
+ return GetOrCreateLLVMFunction(Name, FnTy, GD, /*ForVTable=*/true,
/*DontDefer=*/true, /*IsThunk=*/true);
}
@@ -235,7 +223,8 @@
}
void CodeGenFunction::StartThunk(llvm::Function *Fn, GlobalDecl GD,
- const CGFunctionInfo ) {
+ const CGFunctionInfo ,
+ bool IsUnprototyped) {
assert(!CurGD.getDecl() && "CurGD was already set!");
CurGD = GD;
[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes
This revision was automatically updated to reflect the committed changes.
Closed by commit rC329009: [MS] Emit vftable thunks for functions with
incomplete prototypes (authored by rnk, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45112?vs=140499=140680#toc
Repository:
rC Clang
https://reviews.llvm.org/D45112
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGVTables.cpp
lib/CodeGen/CGVTables.h
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.h
lib/CodeGen/CodeGenTypes.h
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/ms-thunks-unprototyped-return.cpp
test/CodeGenCXX/ms-thunks-unprototyped.cpp
Index: lib/CodeGen/CodeGenTypes.h
===
--- lib/CodeGen/CodeGenTypes.h
+++ lib/CodeGen/CodeGenTypes.h
@@ -313,7 +313,8 @@
const FunctionProtoType *type,
RequiredArgs required,
unsigned numPrefixArgs);
- const CGFunctionInfo (const CXXMethodDecl *MD);
+ const CGFunctionInfo &
+ arrangeUnprototypedMustTailThunk(const CXXMethodDecl *MD);
const CGFunctionInfo (const CXXConstructorDecl *CD,
CXXCtorType CT);
const CGFunctionInfo (const CXXRecordDecl *RD,
Index: lib/CodeGen/CGVTables.h
===
--- lib/CodeGen/CGVTables.h
+++ lib/CodeGen/CGVTables.h
@@ -57,12 +57,10 @@
/// Cache for the deleted virtual member call function.
llvm::Constant *DeletedVirtualFn = nullptr;
- /// emitThunk - Emit a single thunk.
- void emitThunk(GlobalDecl GD, const ThunkInfo , bool ForVTable);
-
- /// maybeEmitThunkForVTable - Emit the given thunk for the vtable if needed by
- /// the ABI.
- void maybeEmitThunkForVTable(GlobalDecl GD, const ThunkInfo );
+ /// Get the address of a thunk and emit it if necessary.
+ llvm::Constant *maybeEmitThunk(GlobalDecl GD,
+ const ThunkInfo ,
+ bool ForVTable);
void addVTableComponent(ConstantArrayBuilder ,
const VTableLayout , unsigned idx,
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1951,7 +1951,8 @@
return cast(GV);
// Create the llvm::Function.
- const CGFunctionInfo = CGM.getTypes().arrangeMSMemberPointerThunk(MD);
+ const CGFunctionInfo =
+ CGM.getTypes().arrangeUnprototypedMustTailThunk(MD);
llvm::FunctionType *ThunkTy = CGM.getTypes().GetFunctionType(FnInfo);
llvm::Function *ThunkFn =
llvm::Function::Create(ThunkTy, llvm::Function::ExternalLinkage,
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -514,8 +514,8 @@
/// correct type, and the caller will bitcast the function to the correct
/// prototype.
const CGFunctionInfo &
-CodeGenTypes::arrangeMSMemberPointerThunk(const CXXMethodDecl *MD) {
- assert(MD->isVirtual() && "only virtual memptrs have thunks");
+CodeGenTypes::arrangeUnprototypedMustTailThunk(const CXXMethodDecl *MD) {
+ assert(MD->isVirtual() && "only methods have thunks");
CanQual FTP = GetFormalType(MD);
CanQualType ArgTys[] = { GetThisType(Context, MD->getParent()) };
return arrangeLLVMFunctionInfo(Context.VoidTy, /*instanceMethod=*/false,
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -800,7 +800,8 @@
ConstantAddress GetAddrOfUuidDescriptor(const CXXUuidofExpr* E);
/// Get the address of the thunk for the given global decl.
- llvm::Constant *GetAddrOfThunk(GlobalDecl GD, const ThunkInfo );
+ llvm::Constant *GetAddrOfThunk(StringRef Name, llvm::Type *FnTy,
+ GlobalDecl GD);
/// Get a reference to the target of VD.
ConstantAddress GetWeakRefReference(const ValueDecl *VD);
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1692,10 +1692,10 @@
void FinishFunction(SourceLocation EndLoc=SourceLocation());
void StartThunk(llvm::Function *Fn, GlobalDecl GD,
- const CGFunctionInfo );
+ const CGFunctionInfo , bool IsUnprototyped);
- void EmitCallAndReturnForThunk(llvm::Constant *Callee,
- const ThunkInfo *Thunk);
+ void EmitCallAndReturnForThunk(llvm::Constant *Callee, const ThunkInfo *Thunk,
+ bool IsUnprototyped);
void FinishThunk();
@@ -1705,7 +1705,8 @@
/// Generate a thunk for the given method.
void
[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes
rnk added a comment. In https://reviews.llvm.org/D45112#1053539, @majnemer wrote: > Does this help PR25641? I think so, these are probably duplicate PRs. I think your analysis on that bug is slightly off, because the TU that provides `ImplCanvas::createColor` *can't* provide the thunks that `ImplBitmapCanvas` needs. It has no way to predict how the offset from the virtual base to the final overrider will change in derived classes. https://reviews.llvm.org/D45112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D45112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes
majnemer added a comment. Does this help PR25641? https://reviews.llvm.org/D45112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes
rnk created this revision.
rnk added reviewers: rjmccall, rsmith, hans.
Herald added a subscriber: Prazek.
The following class hierarchy requires that we be able to emit a
this-adjusting thunk for B::foo in C's vftable:
struct Incomplete;
struct A {
virtual A* foo(Incomplete p) = 0;
};
struct B : virtual A {
void foo(Incomplete p) override;
};
struct C : B { int c; };
This TU is valid, but lacks a definition of 'Incomplete', which makes it
hard to build a thunk for the final overrider, B::foo.
Before this change, Clang gives up attempting to emit the thunk, because
it assumes that if the parameter types are incomplete, it must be
emitting the thunk for optimization purposes. This is untrue for the MS
ABI, where the implementation of B::foo has no idea what thunks C's
vftable may require. Clang needs to emit the thunk without necessarily
having access to the complete prototype of foo.
This change makes Clang emit a musttail variadic call when it needs such
a thunk. I call these "unprototyped" thunks, because they only prototype
the "this" parameter, which must always come first in the MS C++ ABI.
These thunks work, but they create ugly LLVM IR. If the call to the
thunk is devirtualized, it will be a call to a bitcast of a function
pointer. Today, LLVM cannot inline through such a call, but I want to
address that soon, because we also use this pattern for virtual member
pointer thunks.
This change also implements an old FIXME in the code about reusing the
thunk's computed CGFunctionInfo as much as possible. Now we don't end up
computing the thunk's mangled name and arranging it's prototype up to
around three times.
Fixes PR36952
https://reviews.llvm.org/D45112
Files:
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/CGVTables.h
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/CodeGenTypes.h
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/ms-thunks-unprototyped-return.cpp
clang/test/CodeGenCXX/ms-thunks-unprototyped.cpp
Index: clang/test/CodeGenCXX/ms-thunks-unprototyped.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-thunks-unprototyped.cpp
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -fno-rtti-data -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck %s
+
+// In this example, C does not override B::foo, but it needs to emit a thunk to
+// adjust for the relative difference of position between A-in-B and A-in-C.
+
+struct Incomplete;
+template
+struct DoNotInstantiate {
+ typename T::does_not_exist field;
+};
+template
+struct InstantiateLater;
+
+struct A {
+ virtual void foo(Incomplete p) = 0;
+ virtual void bar(DoNotInstantiate p) = 0;
+ virtual int baz(InstantiateLater p) = 0;
+};
+struct B : virtual A {
+ void foo(Incomplete p) override;
+ void bar(DoNotInstantiate p) override;
+ inline int baz(InstantiateLater p) override;
+};
+struct C : B { int c; };
+C c;
+
+// CHECK: @"??_7C@@6B@" = linkonce_odr unnamed_addr constant
+// CHECK-SAME: void (%struct.B*, ...)* @"?foo@B@@W7EAAXUIncomplete@@@Z"
+// CHECK-SAME: void (%struct.B*, ...)* @"?bar@B@@W7EAAXU?$DoNotInstantiate@H@@@Z"
+// CHECK-SAME: i32 (i8*, i32)* @"?baz@B@@W7EAAHU?$InstantiateLater@H@@@Z"
+
+// The thunks should have a -8 adjustment.
+
+// CHECK-LABEL: define linkonce_odr dso_local void @"?foo@B@@W7EAAXUIncomplete@@@Z"(%struct.B* %this, ...)
+// CHECK: %[[THIS_ADJ_i8:[^ ]*]] = getelementptr i8, i8* {{.*}}, i32 -8
+// CHECK: %[[THIS_ADJ:[^ ]*]] = bitcast i8* %[[THIS_ADJ_i8]] to %struct.B*
+// CHECK: musttail call void (%struct.B*, ...) {{.*}}@"?foo@B@@UEAAXUIncomplete@@@Z"
+// CHECK-SAME: (%struct.B* %[[THIS_ADJ]], ...)
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define linkonce_odr dso_local void @"?bar@B@@W7EAAXU?$DoNotInstantiate@H@@@Z"(%struct.B* %this, ...)
+// CHECK: %[[THIS_ADJ_i8:[^ ]*]] = getelementptr i8, i8* {{.*}}, i32 -8
+// CHECK: %[[THIS_ADJ:[^ ]*]] = bitcast i8* %[[THIS_ADJ_i8]] to %struct.B*
+// CHECK: musttail call void (%struct.B*, ...) {{.*}}@"?bar@B@@UEAAXU?$DoNotInstantiate@H@@@Z"
+// CHECK-SAME: (%struct.B* %[[THIS_ADJ]], ...)
+// CHECK-NEXT: ret void
+
+// If we complete the definition later, things work out.
+template struct InstantiateLater { T x; };
+inline int B::baz(InstantiateLater p) { return p.x; }
+
+// CHECK-LABEL: define linkonce_odr dso_local i32 @"?baz@B@@W7EAAHU?$InstantiateLater@H@@@Z"(i8* %this.coerce, i32 %p.coerce)
+// CHECK: = getelementptr i8, i8* {{.*}}, i32 -8
+// CHECK: tail call i32 @"?baz@B@@UEAAHU?$InstantiateLater@H@@@Z"(i8* {{[^,]*}}, i32 {{.*}})
+
+// CHECK-LABEL: define linkonce_odr dso_local i32 @"?baz@B@@UEAAHU?$InstantiateLater@H@@@Z"(i8* %this.coerce, i32 %p.coerce)
Index: clang/test/CodeGenCXX/ms-thunks-unprototyped-return.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-thunks-unprototyped-return.cpp
@@ -0,0 +1,15 @@
