Skip to content

Commit 5f2fd84

Browse files
andykaylorandykaylor
committed
Incorporate review feedback
1 parent dc7ae0c commit 5f2fd84

File tree

3 files changed

+84
-63
lines changed

3 files changed

+84
-63
lines changed

‎clang/lib/CIR/CodeGen/CIRGenClass.cpp‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,7 @@ void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,
222222

223223
const CXXRecordDecl *classDecl = cd->getParent();
224224

225-
// This code doesn't use range-based iteration because we may need to emit
226-
// code between the virtual base initializers and the non-virtual base or
227-
// between the non-virtual base initializers and the member initializers.
228-
CXXConstructorDecl::init_const_iterator b = cd->init_begin(),
229-
e = cd->init_end();
230-
231-
// Virtual base initializers first, if any. They aren't needed if:
225+
// Virtual base initializers aren't needed if:
232226
// - This is a base ctor variant
233227
// - There are no vbases
234228
// - The class is abstract, so a complete object of it cannot be constructed
@@ -245,9 +239,28 @@ void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,
245239
return;
246240
}
247241

242+
// Create three separate ranges for the different types of initializers.
243+
auto allInits = cd->inits();
244+
245+
// Find the boundaries between the three groups.
246+
auto virtualBaseEnd = std::find_if(
247+
allInits.begin(), allInits.end(), [](const CXXCtorInitializer *Init) {
248+
return !(Init->isBaseInitializer() && Init->isBaseVirtual());
249+
});
250+
251+
auto nonVirtualBaseEnd = std::find_if(virtualBaseEnd, allInits.end(),
252+
[](const CXXCtorInitializer *Init) {
253+
return !Init->isBaseInitializer();
254+
});
255+
256+
// Create the three ranges.
257+
auto virtualBaseInits = llvm::make_range(allInits.begin(), virtualBaseEnd);
258+
auto nonVirtualBaseInits =
259+
llvm::make_range(virtualBaseEnd, nonVirtualBaseEnd);
260+
auto memberInits = llvm::make_range(nonVirtualBaseEnd, allInits.end());
261+
248262
const mlir::Value oldThisValue = cxxThisValue;
249263

250-
// Initialize virtual bases.
251264
auto emitInitializer = [&](CXXCtorInitializer *baseInit) {
252265
if (cgm.getCodeGenOpts().StrictVTablePointers &&
253266
cgm.getCodeGenOpts().OptimizationLevel > 0 &&
@@ -260,22 +273,19 @@ void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,
260273
emitBaseInitializer(getLoc(cd->getBeginLoc()), classDecl, baseInit);
261274
};
262275

263-
for (; b != e && (*b)->isBaseInitializer() && (*b)->isBaseVirtual(); b++) {
276+
// Process virtual base initializers.
277+
for (CXXCtorInitializer *virtualBaseInit : virtualBaseInits) {
264278
if (!constructVBases)
265279
continue;
266-
emitInitializer(*b);
280+
emitInitializer(virtualBaseInit);
267281
}
268282

269-
// The loop above and the loop below could obviously be merged in their
270-
// current form, but when we implement support for the MS C++ ABI, we will
271-
// need to insert a branch after the last virtual base initializer, so
272-
// separate loops will be useful then. The missing code is covered by the
273-
// "virtual base without variants" diagnostic above.
283+
assert(!cir::MissingFeatures::msabi());
274284

275-
// Handle non-virtual base initializers.
276-
for (; b != e && (*b)->isBaseInitializer(); b++) {
277-
assert(!(*b)->isBaseVirtual());
278-
emitInitializer(*b);
285+
// Then, non-virtual base initializers.
286+
for (CXXCtorInitializer *nonVirtualBaseInit : nonVirtualBaseInits) {
287+
assert(!nonVirtualBaseInit->isBaseVirtual());
288+
emitInitializer(nonVirtualBaseInit);
279289
}
280290

281291
cxxThisValue = oldThisValue;
@@ -289,8 +299,7 @@ void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,
289299
// lowering or optimization phases to keep the memory accesses more
290300
// explicit. For now, we don't insert memcpy at all.
291301
assert(!cir::MissingFeatures::ctorMemcpyizer());
292-
for (; b != e; b++) {
293-
CXXCtorInitializer *member = (*b);
302+
for (CXXCtorInitializer *member : memberInits) {
294303
assert(!member->isBaseInitializer());
295304
assert(member->isAnyMemberInitializer() &&
296305
"Delegating initializer on non-delegating constructor");

‎clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp‎

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ struct CIRRecordLowering final {
4141
// member type that ensures correct rounding.
4242
struct MemberInfo final {
4343
CharUnits offset;
44-
enum class InfoKind { VFPtr, Field, Base, VBase, Scissor } kind;
44+
enum class InfoKind { VFPtr, Field, Base, VBase } kind;
4545
mlir::Type data;
4646
union {
4747
const FieldDecl *fieldDecl;
@@ -934,44 +934,22 @@ void CIRRecordLowering::accumulateBases() {
934934
}
935935

936936
void CIRRecordLowering::accumulateVBases() {
937-
CharUnits scissorOffset = astRecordLayout.getNonVirtualSize();
938-
// In the itanium ABI, it's possible to place a vbase at a dsize that is
939-
// smaller than the nvsize. Here we check to see if such a base is placed
940-
// before the nvsize and set the scissor offset to that, instead of the
941-
// nvsize.
942-
if (isOverlappingVBaseABI()) {
943-
for (const auto &base : cxxRecordDecl->vbases()) {
944-
const CXXRecordDecl *baseDecl = base.getType()->getAsCXXRecordDecl();
945-
if (baseDecl->isEmpty())
946-
continue;
947-
// If the vbase is a primary virtual base of some base, then it doesn't
948-
// get its own storage location but instead lives inside of that base.
949-
if (astContext.isNearlyEmpty(baseDecl) &&
950-
!hasOwnStorage(cxxRecordDecl, baseDecl))
951-
continue;
952-
scissorOffset = std::min(scissorOffset,
953-
astRecordLayout.getVBaseClassOffset(baseDecl));
954-
}
955-
}
956-
members.push_back(MemberInfo(scissorOffset, MemberInfo::InfoKind::Scissor,
957-
mlir::Type{}, cxxRecordDecl));
958937
for (const auto &base : cxxRecordDecl->vbases()) {
959938
const CXXRecordDecl *baseDecl = base.getType()->getAsCXXRecordDecl();
960-
if (baseDecl->isEmpty())
939+
if (isEmptyRecordForLayout(astContext, base.getType()))
961940
continue;
962941
CharUnits offset = astRecordLayout.getVBaseClassOffset(baseDecl);
963942
// If the vbase is a primary virtual base of some base, then it doesn't
964943
// get its own storage location but instead lives inside of that base.
965-
if (isOverlappingVBaseABI() && astContext.isNearlyEmpty(baseDecl) &&
944+
if (isOverlappingVBaseABI() &&
945+
astContext.isNearlyEmpty(baseDecl) &&
966946
!hasOwnStorage(cxxRecordDecl, baseDecl)) {
967-
members.push_back(
968-
MemberInfo(offset, MemberInfo::InfoKind::VBase, nullptr, baseDecl));
947+
members.push_back(MemberInfo(offset, MemberInfo::InfoKind::VBase, nullptr,
948+
baseDecl));
969949
continue;
970950
}
971951
// If we've got a vtordisp, add it as a storage type.
972-
if (astRecordLayout.getVBaseOffsetsMap()
973-
.find(baseDecl)
974-
->second.hasVtorDisp())
952+
if (astRecordLayout.getVBaseOffsetsMap().find(baseDecl)->second.hasVtorDisp())
975953
members.push_back(makeStorageInfo(offset - CharUnits::fromQuantity(4),
976954
getUIntNType(32)));
977955
members.push_back(MemberInfo(offset, MemberInfo::InfoKind::VBase,

‎clang/lib/CodeGen/CGClass.cpp‎

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,11 @@ CharUnits CodeGenModule::computeNonVirtualBaseClassOffset(
180180
// Get the layout.
181181
const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
182182

183-
const auto *BaseDecl = Base->getType()->castAsCXXRecordDecl();
183+
const auto *BaseDecl =
184+
cast<CXXRecordDecl>(
185+
Base->getType()->castAs<RecordType>()->getOriginalDecl())
186+
->getDefinitionOrSelf();
187+
184188
// Add the offset.
185189
Offset += Layout.getBaseClassOffset(BaseDecl);
186190

@@ -298,7 +302,9 @@ Address CodeGenFunction::GetAddressOfBaseClass(
298302
// *start* with a step down to the correct virtual base subobject,
299303
// and hence will not require any further steps.
300304
if ((*Start)->isVirtual()) {
301-
VBase = (*Start)->getType()->castAsCXXRecordDecl();
305+
VBase = cast<CXXRecordDecl>(
306+
(*Start)->getType()->castAs<RecordType>()->getOriginalDecl())
307+
->getDefinitionOrSelf();
302308
++Start;
303309
}
304310

@@ -553,7 +559,10 @@ static void EmitBaseInitializer(CodeGenFunction &CGF,
553559

554560
Address ThisPtr = CGF.LoadCXXThisAddress();
555561

556-
const auto *BaseClassDecl = BaseInit->getBaseClass()->castAsCXXRecordDecl();
562+
const Type *BaseType = BaseInit->getBaseClass();
563+
const auto *BaseClassDecl =
564+
cast<CXXRecordDecl>(BaseType->castAs<RecordType>()->getOriginalDecl())
565+
->getDefinitionOrSelf();
557566

558567
bool isBaseVirtual = BaseInit->isBaseVirtual();
559568

@@ -1258,7 +1267,10 @@ namespace {
12581267

12591268
static bool isInitializerOfDynamicClass(const CXXCtorInitializer *BaseInit) {
12601269
const Type *BaseType = BaseInit->getBaseClass();
1261-
return BaseType->castAsCXXRecordDecl()->isDynamicClass();
1270+
const auto *BaseClassDecl =
1271+
cast<CXXRecordDecl>(BaseType->castAs<RecordType>()->getOriginalDecl())
1272+
->getDefinitionOrSelf();
1273+
return BaseClassDecl->isDynamicClass();
12621274
}
12631275

12641276
/// EmitCtorPrologue - This routine generates necessary code to initialize
@@ -1365,7 +1377,10 @@ HasTrivialDestructorBody(ASTContext &Context,
13651377
if (I.isVirtual())
13661378
continue;
13671379

1368-
const auto *NonVirtualBase = I.getType()->castAsCXXRecordDecl();
1380+
const CXXRecordDecl *NonVirtualBase =
1381+
cast<CXXRecordDecl>(
1382+
I.getType()->castAs<RecordType>()->getOriginalDecl())
1383+
->getDefinitionOrSelf();
13691384
if (!HasTrivialDestructorBody(Context, NonVirtualBase,
13701385
MostDerivedClassDecl))
13711386
return false;
@@ -1374,7 +1389,10 @@ HasTrivialDestructorBody(ASTContext &Context,
13741389
if (BaseClassDecl == MostDerivedClassDecl) {
13751390
// Check virtual bases.
13761391
for (const auto &I : BaseClassDecl->vbases()) {
1377-
const auto *VirtualBase = I.getType()->castAsCXXRecordDecl();
1392+
const auto *VirtualBase =
1393+
cast<CXXRecordDecl>(
1394+
I.getType()->castAs<RecordType>()->getOriginalDecl())
1395+
->getDefinitionOrSelf();
13781396
if (!HasTrivialDestructorBody(Context, VirtualBase,
13791397
MostDerivedClassDecl))
13801398
return false;
@@ -1390,10 +1408,13 @@ FieldHasTrivialDestructorBody(ASTContext &Context,
13901408
{
13911409
QualType FieldBaseElementType = Context.getBaseElementType(Field->getType());
13921410

1393-
auto *FieldClassDecl = FieldBaseElementType->getAsCXXRecordDecl();
1394-
if (!FieldClassDecl)
1411+
const RecordType *RT = FieldBaseElementType->getAs<RecordType>();
1412+
if (!RT)
13951413
return true;
13961414

1415+
auto *FieldClassDecl =
1416+
cast<CXXRecordDecl>(RT->getOriginalDecl())->getDefinitionOrSelf();
1417+
13971418
// The destructor for an implicit anonymous union member is never invoked.
13981419
if (FieldClassDecl->isUnion() && FieldClassDecl->isAnonymousStructOrUnion())
13991420
return true;
@@ -1886,7 +1907,11 @@ void CodeGenFunction::EnterDtorCleanups(const CXXDestructorDecl *DD,
18861907
// We push them in the forward order so that they'll be popped in
18871908
// the reverse order.
18881909
for (const auto &Base : ClassDecl->vbases()) {
1889-
auto *BaseClassDecl = Base.getType()->castAsCXXRecordDecl();
1910+
auto *BaseClassDecl =
1911+
cast<CXXRecordDecl>(
1912+
Base.getType()->castAs<RecordType>()->getOriginalDecl())
1913+
->getDefinitionOrSelf();
1914+
18901915
if (BaseClassDecl->hasTrivialDestructor()) {
18911916
// Under SanitizeMemoryUseAfterDtor, poison the trivial base class
18921917
// memory. For non-trival base classes the same is done in the class
@@ -2105,7 +2130,10 @@ void CodeGenFunction::EmitCXXAggrConstructorCall(const CXXConstructorDecl *ctor,
21052130
void CodeGenFunction::destroyCXXObject(CodeGenFunction &CGF,
21062131
Address addr,
21072132
QualType type) {
2108-
const CXXDestructorDecl *dtor = type->castAsCXXRecordDecl()->getDestructor();
2133+
const RecordType *rtype = type->castAs<RecordType>();
2134+
const auto *record =
2135+
cast<CXXRecordDecl>(rtype->getOriginalDecl())->getDefinitionOrSelf();
2136+
const CXXDestructorDecl *dtor = record->getDestructor();
21092137
assert(!dtor->isTrivial());
21102138
CGF.EmitCXXDestructorCall(dtor, Dtor_Complete, /*for vbase*/ false,
21112139
/*Delegating=*/false, addr, type);
@@ -2624,7 +2652,10 @@ void CodeGenFunction::getVTablePointers(BaseSubobject Base,
26242652

26252653
// Traverse bases.
26262654
for (const auto &I : RD->bases()) {
2627-
auto *BaseDecl = I.getType()->castAsCXXRecordDecl();
2655+
auto *BaseDecl = cast<CXXRecordDecl>(
2656+
I.getType()->castAs<RecordType>()->getOriginalDecl())
2657+
->getDefinitionOrSelf();
2658+
26282659
// Ignore classes without a vtable.
26292660
if (!BaseDecl->isDynamicClass())
26302661
continue;
@@ -2819,10 +2850,13 @@ void CodeGenFunction::EmitVTablePtrCheckForCast(QualType T, Address Derived,
28192850
if (!getLangOpts().CPlusPlus)
28202851
return;
28212852

2822-
const auto *ClassDecl = T->getAsCXXRecordDecl();
2823-
if (!ClassDecl)
2853+
auto *ClassTy = T->getAs<RecordType>();
2854+
if (!ClassTy)
28242855
return;
28252856

2857+
const auto *ClassDecl =
2858+
cast<CXXRecordDecl>(ClassTy->getOriginalDecl())->getDefinitionOrSelf();
2859+
28262860
if (!ClassDecl->isCompleteDefinition() || !ClassDecl->isDynamicClass())
28272861
return;
28282862

0 commit comments

Comments
 (0)