[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC353181: [opaque pointer types] Fix the CallInfo passed to
EmitCall in some (authored by jyknight, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D57664?vs=184975=185313#toc
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57664/new/
https://reviews.llvm.org/D57664
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGExprCXX.cpp
lib/CodeGen/CGObjC.cpp
lib/CodeGen/CodeGenTypes.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaOpenMP.cpp
test/CodeGenObjC/getter-property-mismatch.m
Index: test/CodeGenObjC/getter-property-mismatch.m
===
--- test/CodeGenObjC/getter-property-mismatch.m
+++ test/CodeGenObjC/getter-property-mismatch.m
@@ -15,6 +15,4 @@
// CHECK: [[CALL:%.*]] = tail call i8* @objc_getProperty
// CHECK: [[ONE:%.*]] = bitcast i8* [[CALL:%.*]] to [[T1:%.*]]*
-// CHECK: [[TWO:%.*]] = bitcast [[T1]]* [[ONE]] to [[T2:%.*]]*
-// CHECK: ret [[T2]]* [[TWO]]
-
+// CHECK: ret [[T1]]* [[ONE]]
Index: lib/CodeGen/CodeGenTypes.h
===
--- lib/CodeGen/CodeGenTypes.h
+++ lib/CodeGen/CodeGenTypes.h
@@ -182,6 +182,10 @@
/// Convert clang calling convention to LLVM callilng convention.
unsigned ClangCallConvToLLVMCallConv(CallingConv CC);
+ /// Derives the 'this' type for codegen purposes, i.e. ignoring method CVR
+ /// qualification.
+ CanQualType DeriveThisType(const CXXRecordDecl *RD, const CXXMethodDecl *MD);
+
/// ConvertType - Convert type T into a llvm::Type.
llvm::Type *ConvertType(QualType T);
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1566,9 +1566,8 @@
Callee = CGCallee::forDirect(
CGM.getAddrOfCXXStructor(DD, getFromDtorType(Type)), GD);
- CGF.EmitCXXMemberOrOperatorCall(DD, Callee, ReturnValueSlot(),
- This.getPointer(), VTT, VTTTy,
- nullptr, nullptr);
+ CGF.EmitCXXDestructorCall(DD, Callee, This.getPointer(), VTT, VTTTy, nullptr,
+getFromDtorType(Type));
}
void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables ,
@@ -1766,9 +1765,8 @@
CGCallee Callee =
CGCallee::forVirtual(CE, GlobalDecl(Dtor, DtorType), This, Ty);
- CGF.EmitCXXMemberOrOperatorCall(Dtor, Callee, ReturnValueSlot(),
- This.getPointer(), /*ImplicitParam=*/nullptr,
- QualType(), CE, nullptr);
+ CGF.EmitCXXDestructorCall(Dtor, Callee, This.getPointer(), nullptr,
+QualType(), nullptr, getFromDtorType(DtorType));
return nullptr;
}
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -67,10 +67,17 @@
}
/// Derives the 'this' type for codegen purposes, i.e. ignoring method CVR
-/// qualification.
-static CanQualType GetThisType(ASTContext , const CXXRecordDecl *RD,
- const CXXMethodDecl *MD) {
- QualType RecTy = Context.getTagDeclType(RD)->getCanonicalTypeInternal();
+/// qualification. Either or both of RD and MD may be null. A null RD indicates
+/// that there is no meaningful 'this' type, and a null MD can occur when
+/// calling a method pointer.
+CanQualType CodeGenTypes::DeriveThisType(const CXXRecordDecl *RD,
+ const CXXMethodDecl *MD) {
+ QualType RecTy;
+ if (RD)
+RecTy = Context.getTagDeclType(RD)->getCanonicalTypeInternal();
+ else
+RecTy = Context.VoidTy;
+
if (MD)
RecTy = Context.getAddrSpaceQualType(RecTy, MD->getMethodQualifiers().getAddressSpace());
return Context.getPointerType(CanQualType::CreateUnsafe(RecTy));
@@ -235,7 +242,7 @@
/// Arrange the argument and result information for a call to an
/// unknown C++ non-static member function of the given abstract type.
-/// (Zero value of RD means we don't have any meaningful "this" argument type,
+/// (A null RD means we don't have any meaningful "this" argument type,
/// so fall back to a generic pointer type).
/// The member function must be an ordinary function, i.e. not a
/// constructor or destructor.
@@ -246,10 +253,7 @@
SmallVector argTypes;
// Add the 'this' pointer.
- if (RD)
-argTypes.push_back(GetThisType(Context, RD, MD));
- else
-argTypes.push_back(Context.VoidPtrTy);
+ argTypes.push_back(DeriveThisType(RD, MD));
return ::arrangeLLVMFunctionInfo(
*this, true, argTypes,
@@ -303,7 +307,7 @@
SmallVector argTypes;
SmallVector paramInfos;
- argTypes.push_back(GetThisType(Context, MD->getParent(), MD));
+ argTypes.push_back(DeriveThisType(MD->getParent(), MD));
bool
[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.
jyknight marked an inline comment as done. jyknight added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:3837 +// having pointee types). +llvm::FunctionType *IRFuncTyFromInfo = getTypes().GetFunctionType(CallInfo); +assert(IRFuncTy == IRFuncTyFromInfo); dblaikie wrote: > This will be warned as unused in a release build. > > Would this be hideous if it's just all one big assert? > > assert((CallInfo.isVariadic && CallInfo.getArgStruct) || IRFuncTy == > getTypes().GetFunctionType(CallInfo)); > > (I think that's accurate?) Clearer IMO to just put #ifndef NDEBUG around the block, so I'll do that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57664/new/ https://reviews.llvm.org/D57664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.
dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Seems reasonable to me - but if you like you can wait for Richard who might have a more nuanced understanding of the code. (but I'm OK signing off on this if you are & Richard can provide any extra feedback post-commit) Comment at: clang/lib/CodeGen/CGCall.cpp:3837 +// having pointee types). +llvm::FunctionType *IRFuncTyFromInfo = getTypes().GetFunctionType(CallInfo); +assert(IRFuncTy == IRFuncTyFromInfo); This will be warned as unused in a release build. Would this be hideous if it's just all one big assert? assert((CallInfo.isVariadic && CallInfo.getArgStruct) || IRFuncTy == getTypes().GetFunctionType(CallInfo)); (I think that's accurate?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57664/new/ https://reviews.llvm.org/D57664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.
jyknight created this revision.
jyknight added reviewers: dblaikie, rsmith.
Herald added a subscriber: Anastasia.
Herald added a project: clang.
Currently, EmitCall emits a call instruction with a function type
derived from the pointee-type of the callee. This *should* be the same
as the type created from the CallInfo parameter, but in some cases an
incorrect CallInfo was being passed.
All of these fixes were discovered by the addition of the assert in
EmitCall which verifies that the passed-in CallInfo matches the
Callee's function type.
As far as I know, these issues caused no bugs at the moment, as the
correct types were ultimately being emitted. But, some would become
problematic when pointee types are removed.
List of fixes:
- arrangeCXXConstructorCall was passing an incorrect value for the number of
Required args, when calling an inheriting constructor where the inherited
constructor is variadic. (The inheriting constructor doesn't actually get
passed any of the user's args, but the code was calculating it as if it did).
- arrangeFreeFunctionLikeCall was not including the count of the
pass_object_size arguments in the count of required args.
- OpenCL uses other address spaces for the "this" pointer. However,
commonEmitCXXMemberOrOperatorCall was not annotating the address space on the
"this" argument of the call.
- Destructor calls were being created with EmitCXXMemberOrOperatorCall instead
of EmitCXXDestructorCall in a few places. This was a problem because the
calling convention sometimes has destructors returning "this" rather than void,
and the latter function knows about that, and sets up the types properly
(through calling arrangeCXXStructorDeclaration), while the former does not.
- generateObjCGetterBody: the 'objc_getProperty' function returns type 'id',
but was being called as if it returned the particular property's type. (That is
of course the *dynamic* return type, and there's a downcast immediately after.)
- OpenMP user-defined reduction functions (#pragma omp declare reduction) can
be called with a subclass of the declared type. In such case, the call was
being setup as if the function had been actually declared to take the subtype,
rather than the base type.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D57664
Files:
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CodeGenTypes.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/CodeGenObjC/getter-property-mismatch.m
Index: clang/test/CodeGenObjC/getter-property-mismatch.m
===
--- clang/test/CodeGenObjC/getter-property-mismatch.m
+++ clang/test/CodeGenObjC/getter-property-mismatch.m
@@ -15,6 +15,4 @@
// CHECK: [[CALL:%.*]] = tail call i8* @objc_getProperty
// CHECK: [[ONE:%.*]] = bitcast i8* [[CALL:%.*]] to [[T1:%.*]]*
-// CHECK: [[TWO:%.*]] = bitcast [[T1]]* [[ONE]] to [[T2:%.*]]*
-// CHECK: ret [[T2]]* [[TWO]]
-
+// CHECK: ret [[T1]]* [[ONE]]
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -10747,7 +10747,8 @@
return D;
return nullptr;
}))
-return SemaRef.BuildDeclRefExpr(VD, Ty, VK_LValue, Loc);
+return SemaRef.BuildDeclRefExpr(VD, VD->getType().getNonReferenceType(),
+VK_LValue, Loc);
if (auto *VD = filterLookupForUDR(
Lookups, [, Ty, Loc](ValueDecl *D) -> ValueDecl * {
if (!D->isInvalidDecl() &&
@@ -10765,7 +10766,8 @@
/*DiagID=*/0) !=
Sema::AR_inaccessible) {
SemaRef.BuildBasePathArray(Paths, BasePath);
- return SemaRef.BuildDeclRefExpr(VD, Ty, VK_LValue, Loc);
+ return SemaRef.BuildDeclRefExpr(
+ VD, VD->getType().getNonReferenceType(), VK_LValue, Loc);
}
}
}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1566,9 +1566,8 @@
Callee = CGCallee::forDirect(
CGM.getAddrOfCXXStructor(DD, getFromDtorType(Type)), GD);
- CGF.EmitCXXMemberOrOperatorCall(DD, Callee, ReturnValueSlot(),
- This.getPointer(), VTT, VTTTy,
- nullptr, nullptr);
+ CGF.EmitCXXDestructorCall(DD, Callee, This.getPointer(), VTT, VTTTy, nullptr,
+getFromDtorType(Type));
}
void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables ,
@@ -1766,9 +1765,8 @@
CGCallee Callee =
CGCallee::forVirtual(CE, GlobalDecl(Dtor, DtorType), This, Ty);
- CGF.EmitCXXMemberOrOperatorCall(Dtor, Callee, ReturnValueSlot(),
-
