Implement [[msvc::no_unique_address]] #65675
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amykhuang
added
platform:windows
clang-cl
amykhuang
requested review from
tru,
rnk,
CaseyCarter,
AaronBallman and
shafik
github-actions
bot
added
clang:frontend
rnk
reviewed
Sep 7, 2023
rnk
reviewed
Sep 7, 2023
|
We should consider whether we want to support |
I think |
cjdb
requested changes
Sep 12, 2023
There was a problem hiding this comment.
Added @erichkeane as attributes code owner, and @efriedma-quic and @rjmccall as codegen code owners.
The attributes changes look good to me. The codegen changes seem reasonable, but are a bit outside my area of expertise for me to feel comfortable signing off on. The only thing I did spot was that this should come with a change to the release notes so users know about the new functionality and its stability guarantees.
|
When you say you want a second opinion on CodeGen changes, do you mean RecordLayoutBuilder.cpp? I don't see any non-whitespace changes to clang/lib/CodeGen/ . |
erichkeane
reviewed
Sep 12, 2023
clang/lib/AST/Decl.cpp
Outdated
| @@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext &Ctx) const { | |||
| if (!CXXRD->isEmpty()) | |||
| return false; | |||
|
|
|||
| // MS ABI: nonzero if class type with class type fields | |||
There was a problem hiding this comment.
Does any differentiation need to be made between the spellings on 4489? Additionally, can you elaborate on the comment here? How can we look at the fields of CXXRD if it is empty?
There was a problem hiding this comment.
As far as I can tell the cases above still apply to MSVC so I think on 4489 it's correct to account for both spellings. Not sure what you mean about the comment? The existing comment above the isEmpty line describes what that checks for
|
@llvm/pr-subscribers-clang ChangesThis implements the [[msvc::no_unique_address]] attribute. There is not ABI compatibility in this patch because the attribute is relatively new and there's still some uncertainty in the MSVC version. Bug: #49358 Also see https://reviews.llvm.org/D157762.Patch is 23.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/65675.diff 9 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index c95db7e8049d47a..23e56cda0f67e9d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1798,11 +1798,13 @@ def ArmMveStrictPolymorphism : TypeAttr, TargetSpecificAttr {
let Documentation = [ArmMveStrictPolymorphismDocs];
}
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr {
- let Spellings = [CXX11<"", "no_unique_address", 201803>];
+def NoUniqueAddress : InheritableAttr {
+ let Spellings = [CXX11<"", "no_unique_address", 201803>,
+ CXX11<"msvc", "no_unique_address", 201803>];
+ let Accessors = [Accessor<"isDefault", [CXX11<"", "no_unique_address", 201803>]>,
+ Accessor<"isMSVC", [CXX11<"msvc", "no_unique_address", 201803>]>];
let Subjects = SubjectList<[NonBitField], ErrorDiag>;
let Documentation = [NoUniqueAddressDocs];
- let SimpleHandler = 1;
}
def ReturnsTwice : InheritableAttr {
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index f11ea89d14bad0d..21e6373611272b5 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -1405,6 +1405,10 @@ Example usage:
``[[no_unique_address]]`` is a standard C++20 attribute. Clang supports its use
in C++11 onwards.
+
+On MSVC targets, ``[[no_unique_address]]`` is ignored; use
+``[[msvc::no_unique_address]]`` instead. Currently there is no guarantee of ABI
+compatibility or stability.
}];
}
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 60c80f2b075336b..c99d5f6c19a15d6 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4507,9 +4507,13 @@ bool FieldDecl::isZeroSize(const ASTContext &Ctx) const {
// Otherwise, [...] the circumstances under which the object has zero size
// are implementation-defined.
- // FIXME: This might be Itanium ABI specific; we don't yet know what the MS
- // ABI will do.
- return true;
+ if (!Ctx.getTargetInfo().getCXXABI().isMicrosoft())
+ return true;
+
+ // MS ABI: nonzero if class type with class type fields
+ return !llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) {
+ return Field->getType()->getAs();
+ });
}
bool FieldDecl::isPotentiallyOverlapping() const {
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8afd88ae7be27b3..2f5b3be413a7b9e 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2545,7 +2545,10 @@ struct MicrosoftRecordLayoutBuilder {
CharUnits Alignment;
};
typedef llvm::DenseMap BaseOffsetsMapTy;
- MicrosoftRecordLayoutBuilder(const ASTContext &Context) : Context(Context) {}
+ MicrosoftRecordLayoutBuilder(const ASTContext &Context,
+ EmptySubobjectMap *EmptySubobjects)
+ : Context(Context), EmptySubobjects(EmptySubobjects) {}
+
private:
MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
void operator=(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2595,6 +2598,12 @@ struct MicrosoftRecordLayoutBuilder {
llvm::SmallPtrSetImpl &HasVtorDispSet,
const CXXRecordDecl *RD) const;
const ASTContext &Context;
+ EmptySubobjectMap *EmptySubobjects;
+ llvm::SpecificBumpPtrAllocator BaseSubobjectInfoAllocator;
+ typedef llvm::DenseMap
+ BaseSubobjectInfoMapTy;
+ BaseSubobjectInfoMapTy VirtualBaseInfo;
+
/// The size of the record being laid out.
CharUnits Size;
/// The non-virtual size of the record layout.
@@ -2864,10 +2873,12 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
bool CheckLeadingLayout = !PrimaryBase;
// Iterate through the bases and lay out the non-virtual ones.
for (const CXXBaseSpecifier &Base : RD->bases()) {
- if (Base.isVirtual())
- continue;
const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl);
+
+ if (Base.isVirtual())
+ continue;
+
// Only lay out bases without extendable VFPtrs on the second pass.
if (BaseLayout.hasExtendableVFPtr()) {
VBPtrOffset = Bases[BaseDecl] + BaseLayout.getNonVirtualSize();
@@ -2908,8 +2919,7 @@ static bool recordUsesEBO(const RecordDecl *RD) {
}
void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
- const CXXRecordDecl *RD,
- const CXXRecordDecl *BaseDecl,
+ const CXXRecordDecl *RD, const CXXRecordDecl *BaseDecl,
const ASTRecordLayout &BaseLayout,
const ASTRecordLayout *&PreviousBaseLayout) {
// Insert padding between two bases if the left first one is zero sized or
@@ -2940,8 +2950,10 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
BaseOffset = Size = Size.alignTo(Info.Alignment);
}
}
+
Bases.insert(std::make_pair(BaseDecl, BaseOffset));
Size += BaseLayout.getNonVirtualSize();
+ DataSize = Size;
PreviousBaseLayout = &BaseLayout;
}
@@ -2956,18 +2968,47 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) {
layoutBitField(FD);
return;
}
+
LastFieldIsNonZeroWidthBitfield = false;
ElementInfo Info = getAdjustedElementInfo(FD);
Alignment = std::max(Alignment, Info.Alignment);
- CharUnits FieldOffset;
- if (UseExternalLayout)
+
+ const CXXRecordDecl *FieldClass = FD->getType()->getAsCXXRecordDecl();
+ bool IsOverlappingEmptyField = FD->isPotentiallyOverlapping() &&
+ FieldClass->isEmpty() &&
+ FieldClass->fields().empty();
+ CharUnits FieldOffset = CharUnits::Zero();
+
+ if (UseExternalLayout) {
FieldOffset =
Context.toCharUnitsFromBits(External.getExternalFieldOffset(FD));
- else if (IsUnion)
+ } else if (IsUnion) {
FieldOffset = CharUnits::Zero();
- else
+ } else if (EmptySubobjects) {
+ if (!IsOverlappingEmptyField)
+ FieldOffset = DataSize.alignTo(Info.Alignment);
+
+ while (!EmptySubobjects->CanPlaceFieldAtOffset(FD, FieldOffset)) {
+ const CXXRecordDecl *ParentClass = cast(FD->getParent());
+ bool HasBases = ParentClass && (!ParentClass->bases().empty() ||
+ !ParentClass->vbases().empty());
+ if (FieldOffset == CharUnits::Zero() && DataSize != CharUnits::Zero() &&
+ HasBases) {
+ // MSVC appears to only do this when there are base classes;
+ // otherwise it overlaps no_unique_address fields in non-zero offsets.
+ FieldOffset = DataSize.alignTo(Info.Alignment);
+ } else {
+ FieldOffset += Info.Alignment;
+ }
+ }
+ } else {
FieldOffset = Size.alignTo(Info.Alignment);
+ }
placeFieldAtOffset(FieldOffset);
+
+ if (!IsOverlappingEmptyField)
+ DataSize = std::max(DataSize, FieldOffset + Info.Size);
+
Size = std::max(Size, FieldOffset + Info.Size);
}
@@ -3013,6 +3054,7 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
Alignment = std::max(Alignment, Info.Alignment);
RemainingBitsInField = Context.toBits(Info.Size) - Width;
}
+ DataSize = Size;
}
void
@@ -3038,6 +3080,7 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) {
Size = FieldOffset;
Alignment = std::max(Alignment, Info.Alignment);
}
+ DataSize = Size;
}
void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) {
@@ -3103,6 +3146,7 @@ void MicrosoftRecordLayoutBuilder::injectVFPtr(const CXXRecordDecl *RD) {
void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
if (!HasVBPtr)
return;
+
// Vtordisps are always 4 bytes (even in 64-bit mode)
CharUnits VtorDispSize = CharUnits::fromQuantity(4);
CharUnits VtorDispAlignment = VtorDispSize;
@@ -3304,8 +3348,9 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const {
const ASTRecordLayout *NewEntry = nullptr;
if (isMsLayout(*this)) {
- MicrosoftRecordLayoutBuilder Builder(*this);
if (const auto *RD = dyn_cast(D)) {
+ EmptySubobjectMap EmptySubobjects(*this, RD);
+ MicrosoftRecordLayoutBuilder Builder(*this, &EmptySubobjects);
Builder.cxxLayout(RD);
NewEntry = new (*this) ASTRecordLayout(
*this, Builder.Size, Builder.Alignment, Builder.Alignment,
@@ -3317,6 +3362,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const {
Builder.EndsWithZeroSizedObject, Builder.LeadsWithZeroSizedBase,
Builder.Bases, Builder.VBases);
} else {
+ MicrosoftRecordLayoutBuilder Builder(*this, /*EmptySubobjects*/ nullptr);
Builder.layout(D);
NewEntry = new (*this) ASTRecordLayout(
*this, Builder.Size, Builder.Alignment, Builder.Alignment,
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 270ff11559417d9..792ff3ce45cf3a3 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -743,6 +743,7 @@ void CGRecordLowering::calculateZeroInit() {
void CGRecordLowering::clipTailPadding() {
std::vector::iterator Prior = Members.begin();
CharUnits Tail = getSize(Prior->Data);
+
for (std::vector::iterator Member = Prior + 1,
MemberEnd = Members.end();
Member != MemberEnd; ++Member) {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 59e0f3e83cfdd80..370dfdeb800b38f 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8368,6 +8368,20 @@ static void handleNoMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->addAttr(NoMergeAttr::Create(S.Context, AL));
}
+static void handleNoUniqueAddressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ NoUniqueAddressAttr TmpAttr(S.Context, AL);
+ if (S.getLangOpts().MSVCCompat) {
+ if (TmpAttr.isDefault()) {
+ S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
+ return;
+ }
+ } else if (TmpAttr.isMSVC()) {
+ S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
+ return;
+ }
+ D->addAttr(NoUniqueAddressAttr::Create(S.Context, AL));
+}
+
static void handleSYCLKernelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
// The 'sycl_kernel' attribute applies only to function templates.
const auto *FD = cast(D);
@@ -9273,6 +9287,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_NoMerge:
handleNoMergeAttr(S, D, AL);
break;
+ case ParsedAttr::AT_NoUniqueAddress:
+ handleNoUniqueAddressAttr(S, D, AL);
+ break;
case ParsedAttr::AT_AvailableOnlyInDefaultEvalMethod:
handleAvailableOnlyInDefaultEvalMethod(S, D, AL);
diff --git a/clang/test/Layout/ms-no-unique-address.cpp b/clang/test/Layout/ms-no-unique-address.cpp
new file mode 100644
index 000000000000000..c5dd80aa5fce4d6
--- /dev/null
+++ b/clang/test/Layout/ms-no-unique-address.cpp
@@ -0,0 +1,338 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-windows-msvc -fms-compatibility -fdump-record-layouts %s | FileCheck %s
+
+namespace Empty {
+ struct A {};
+ struct A2 {};
+ struct A3 { [[msvc::no_unique_address]] A a; };
+ struct alignas(8) A4 {};
+
+ struct B {
+ [[msvc::no_unique_address]] A a;
+ char b;
+ };
+ static_assert(sizeof(B) == 1);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::B
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: 0 | char b
+ // CHECK-NEXT: | [sizeof=1, align=1,
+ // CHECK-NEXT: | nvsize=1, nvalign=1]
+
+ struct C {
+ [[msvc::no_unique_address]] A a;
+ [[msvc::no_unique_address]] A2 a2;
+ char c;
+ };
+ static_assert(sizeof(C) == 1);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::C
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: 0 | struct Empty::A2 a2 (empty)
+ // CHECK-NEXT: 0 | char c
+ // CHECK-NEXT: | [sizeof=1, align=1,
+ // CHECK-NEXT: | nvsize=1, nvalign=1]
+
+ struct D {
+ [[msvc::no_unique_address]] A3 a;
+ int i;
+ };
+ static_assert(sizeof(D) == 8);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::D
+ // CHECK-NEXT: 0 | struct Empty::A3 a (empty)
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: 4 | int i
+ // CHECK-NEXT: | [sizeof=8, align=4,
+ // CHECK-NEXT: | nvsize=8, nvalign=4]
+
+ struct E {
+ [[msvc::no_unique_address]] A a1;
+ [[msvc::no_unique_address]] A a2;
+ char e;
+ };
+ static_assert(sizeof(E) == 2);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::E
+ // CHECK-NEXT: 0 | struct Empty::A a1 (empty)
+ // CHECK-NEXT: 1 | struct Empty::A a2 (empty)
+ // CHECK-NEXT: 0 | char e
+ // CHECK-NEXT: | [sizeof=2, align=1,
+ // CHECK-NEXT: | nvsize=2, nvalign=1]
+
+ struct F {
+ ~F();
+ [[msvc::no_unique_address]] A a1;
+ [[msvc::no_unique_address]] A a2;
+ char f;
+ };
+ static_assert(sizeof(F) == 2);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::F
+ // CHECK-NEXT: 0 | struct Empty::A a1 (empty)
+ // CHECK-NEXT: 1 | struct Empty::A a2 (empty)
+ // CHECK-NEXT: 0 | char f
+ // CHECK-NEXT: | [sizeof=2, align=1,
+ // CHECK-NEXT: | nvsize=2, nvalign=1]
+
+ struct G { [[msvc::no_unique_address]] A a; ~G(); };
+ static_assert(sizeof(G) == 1);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::G
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: | [sizeof=1, align=1,
+ // CHECK-NEXT: | nvsize=1, nvalign=1]
+
+ struct H {
+ [[msvc::no_unique_address]] A a;
+ [[msvc::no_unique_address]] A b;
+ ~H();
+ };
+ static_assert(sizeof(H) == 2);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::H
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: 1 | struct Empty::A b (empty)
+ // CHECK-NEXT: | [sizeof=2, align=1,
+ // CHECK-NEXT: | nvsize=2, nvalign=1]
+
+ struct I {
+ [[msvc::no_unique_address]] A4 a;
+ [[msvc::no_unique_address]] A4 b;
+ };
+ static_assert(sizeof(I) == 16);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::I
+ // CHECK-NEXT: 0 | struct Empty::A4 a (empty)
+ // CHECK-NEXT: 8 | struct Empty::A4 b (empty)
+ // CHECK-NEXT: | [sizeof=16, align=8,
+ // CHECK-NEXT: | nvsize=16, nvalign=8]
+
+ struct J {
+ [[msvc::no_unique_address]] A4 a;
+ A4 b;
+ };
+ static_assert(sizeof(J) == 16);
+
+ // MSVC puts a and b at the same offset.
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::J
+ // CHECK-NEXT: 0 | struct Empty::A4 a (empty)
+ // CHECK-NEXT: 8 | struct Empty::A4 b (empty)
+ // CHECK-NEXT: | [sizeof=16, align=8,
+ // CHECK-NEXT: | nvsize=16, nvalign=8]
+
+ struct K {
+ [[msvc::no_unique_address]] A4 a;
+ [[msvc::no_unique_address]] char c;
+ [[msvc::no_unique_address]] A4 b;
+ };
+ static_assert(sizeof(K) == 16);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::K
+ // CHECK-NEXT: 0 | struct Empty::A4 a (empty)
+ // CHECK-NEXT: 0 | char c
+ // CHECK-NEXT: 8 | struct Empty::A4 b (empty)
+ // CHECK-NEXT: | [sizeof=16, align=8,
+ // CHECK-NEXT: | nvsize=16, nvalign=8]
+
+ struct OversizedEmpty : A {
+ ~OversizedEmpty();
+ [[msvc::no_unique_address]] A a;
+ };
+ static_assert(sizeof(OversizedEmpty) == 1);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::OversizedEmpty
+ // CHECK-NEXT: 0 | struct Empty::A (base) (empty)
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: | [sizeof=1, align=1,
+ // CHECK-NEXT: | nvsize=1, nvalign=1]
+
+ struct HasOversizedEmpty {
+ [[msvc::no_unique_address]] OversizedEmpty m;
+ };
+ static_assert(sizeof(HasOversizedEmpty) == 1);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::HasOversizedEmpty
+ // CHECK-NEXT: 0 | struct Empty::OversizedEmpty m (empty)
+ // CHECK-NEXT: 0 | struct Empty::A (base) (empty)
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: | [sizeof=1, align=1,
+ // CHECK-NEXT: | nvsize=1, nvalign=1]
+
+ struct EmptyWithNonzeroDSize {
+ [[msvc::no_unique_address]] A a;
+ int x;
+ [[msvc::no_unique_address]] A b;
+ int y;
+ [[msvc::no_unique_address]] A c;
+ };
+ static_assert(sizeof(EmptyWithNonzeroDSize) == 8);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::EmptyWithNonzeroDSize
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: 0 | int x
+ // CHECK-NEXT: 1 | struct Empty::A b (empty)
+ // CHECK-NEXT: 4 | int y
+ // CHECK-NEXT: 2 | struct Empty::A c (empty)
+ // CHECK-NEXT: | [sizeof=8, align=4,
+ // CHECK-NEXT: | nvsize=8, nvalign=4]
+
+ struct EmptyWithNonzeroDSizeNonPOD {
+ ~EmptyWithNonzeroDSizeNonPOD();
+ [[msvc::no_unique_address]] A a;
+ int x;
+ [[msvc::no_unique_address]] A b;
+ int y;
+ [[msvc::no_unique_address]] A c;
+ };
+ static_assert(sizeof(EmptyWithNonzeroDSizeNonPOD) == 8);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct Empty::EmptyWithNonzeroDSizeNonPOD
+ // CHECK-NEXT: 0 | struct Empty::A a (empty)
+ // CHECK-NEXT: 0 | int x
+ // CHECK-NEXT: 1 | struct Empty::A b (empty)
+ // CHECK-NEXT: 4 | int y
+ // CHECK-NEXT: 2 | struct Empty::A c (empty)
+ // CHECK-NEXT: | [sizeof=8, align=4,
+ // CHECK-NEXT: | nvsize=8, nvalign=4]
+}
+
+namespace POD {
+ struct A { int n; char c[3]; };
+ struct B { [[msvc::no_unique_address]] A a; char d; };
+ static_assert(sizeof(B) == 12);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct POD::B
+ // CHECK-NEXT: 0 | struct POD::A a
+ // CHECK-NEXT: 0 | int n
+ // CHECK-NEXT: 4 | char[3] c
+ // CHECK-NEXT: 8 | char d
+ // CHECK-NEXT: | [sizeof=12, align=4,
+ // CHECK-NEXT: | nvsize=12, nvalign=4]
+}
+
+namespace NonPOD {
+ struct A { int n; char c[3]; ~A(); };
+ struct B { [[msvc::no_unique_address]] A a; char d; };
+ static_assert(sizeof(B) == 12);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct NonPOD::B
+ // CHECK-NEXT: 0 | struct NonPOD::A a
+ // CHECK-NEXT: 0 | int n
+ // CHECK-NEXT: 4 | char[3] c
+ // CHECK-NEXT: 8 | char d
+ // CHECK-NEXT: | [sizeof=12, align=4,
+ // CHECK-NEXT: | nvsize=12, nvalign=4]
+}
+
+namespace VBases {
+ // The nvsize of an object includes the complete size of its empty subobjects
+ // (although it's unclear why). Ensure this corner case is handled properly.
+ struct Empty {};
+ struct alignas(8) A {}; // dsize 0, nvsize 0, size 8
+ struct B : A { char c; }; // dsize 1, nvsize 8, size 8
+ static_assert(sizeof(B) == 8);
+
+ // CHECK:*** Dumping AST Record Layout
+ // CHECK: 0 | struct VBases::B
+ // CHECK-NEXT: 0 | struct VBases::A (base) (empty)
+ // CHECK-NEXT: 0 | char c
+ // CHECK-NEXT: | [sizeof=8, align=8,
+ // CHECK-NEXT: | nvsize=8, nvalign=8]
+
+ struct V { int n; };
+
+ struct C : B, virtual V ...
|
Sorry for the confusion, yes, I was mostly worried about RecordLayoutBuilder.cpp and whether you spotted any concerns with the changes. |
amykhuang
force-pushed
the
no-unique-address
branch
2 times, most recently
from
87b21d0 to
87631d7
Compare
amykhuang
force-pushed
the
no-unique-address
branch
from
87631d7 to
ca69c68
Compare
Sounds good, I can do this in a separate patch |
shafik
reviewed
Sep 19, 2023
Co-authored-by: Shafik Yaghmour <[email protected]>
|
Are there any other comments here? I think I've addressed all the existing ones. |
amykhuang
added a commit
that referenced
this pull request
Sep 22, 2023
Previous change in #65675 broke the docs generation.
amykhuang
added a commit
to amykhuang/llvm-project
that referenced
this pull request
Sep 22, 2023
This reverts commit 4a55d42. Reverting because this breaks sphinx documentation, and even with it fixed the format of the attribute makes the no_unique_address documentation show up twice.
amykhuang
added a commit
that referenced
this pull request
Sep 22, 2023
amykhuang
added a commit
to amykhuang/llvm-project
that referenced
this pull request
Sep 22, 2023
Change the attribute docs so that there is a separate one for the MSVC attribute. This reverts commit 71f9e76.
amykhuang
added a commit
that referenced
this pull request
Sep 28, 2023
This implements the [[msvc::no_unique_address]] attribute. There is not ABI compatibility in this patch because the attribute is relatively new and there's still some uncertainty in the MSVC version. The recommit changes the attribute definitions so that instead of making two separate attributes for no_unique_address and msvc::no_unique_address, it modifies the attributes tablegen emitter to allow spellings to be target-specific. This reverts commit 71f9e76.
legrosbuffle
pushed a commit
to legrosbuffle/llvm-project
that referenced
this pull request
Sep 29, 2023
…7199) This implements the [[msvc::no_unique_address]] attribute. There is not ABI compatibility in this patch because the attribute is relatively new and there's still some uncertainty in the MSVC version. The recommit changes the attribute definitions so that instead of making two separate attributes for no_unique_address and msvc::no_unique_address, it modifies the attributes tablegen emitter to allow spellings to be target-specific. This reverts commit 71f9e76.
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements the [[msvc::no_unique_address]] attribute.
There is not ABI compatibility in this patch because the attribute is relatively new and there's still some uncertainty in the MSVC version.
Bug: #49358
Also see https://reviews.llvm.org/D157762.