[clang] Implement [[msvc::no_unique_address]] (PR #65675)
MaskRay wrote: The supposed doc fix (for `doc-clang-html`?) #67195 caused `error: Record `NoUniqueAddress', field `Documentation' exists but does not have a list value`. I have reverted #67195. https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang closed https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
amykhuang wrote: Are there any other comments here? I think I've addressed all the existing ones. https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang resolved https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang updated
https://github.com/llvm/llvm-project/pull/65675
>From 481337fde54dc4d7f68a604952a963c99913675d Mon Sep 17 00:00:00 2001
From: Amy Huang
Date: Fri, 21 Jul 2023 16:30:30 -0700
Subject: [PATCH 1/6] Implement [[msvc::no_unique_address]]
This attribute should match the behavior of MSVC's [[msvc::no_unique_address]]
attribute.
Bug: https://github.com/llvm/llvm-project/issues/49358
Differential Revision: https://reviews.llvm.org/D157762
---
clang/include/clang/Basic/Attr.td | 8 +-
clang/include/clang/Basic/AttrDocs.td | 4 +
clang/lib/AST/Decl.cpp | 8 +
clang/lib/AST/RecordLayoutBuilder.cpp | 191 +--
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 1 +
clang/lib/Sema/SemaDeclAttr.cpp | 17 +
clang/test/Layout/ms-no-unique-address.cpp | 338
7 files changed, 540 insertions(+), 27 deletions(-)
create mode 100644 clang/test/Layout/ms-no-unique-address.cpp
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 08ae2087cfe70eb..383be8318fc2145 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext ) const {
if (!CXXRD->isEmpty())
return false;
+ // MS ABI: nonzero if class type with class type fields
+ if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() &&
+ llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) {
+return Field->getType()->getAs();
+ })) {
+return false;
+ }
+
// 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
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8afd88ae7be27b3..2152e69732d65c9 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) {}
+ MicrosoftRecordLayoutBuilder(const ASTContext ,
+ EmptySubobjectMap *EmptySubobjects)
+ : Context(Context), EmptySubobjects(EmptySubobjects) {}
+
private:
MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
void operator=(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2562,7 +2565,11 @@ struct MicrosoftRecordLayoutBuilder {
void layoutNonVirtualBase(const CXXRecordDecl *RD,
const CXXRecordDecl *BaseDecl,
const ASTRecordLayout ,
-const ASTRecordLayout *);
+const ASTRecordLayout *,
+BaseSubobjectInfo *Base);
+ BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD,
+ bool IsVirtual,
+ BaseSubobjectInfo *Derived);
void injectVFPtr(const CXXRecordDecl *RD);
void injectVBPtr(const CXXRecordDecl *RD);
/// Lays out the fields of the record. Also rounds size up to
@@ -2595,6 +2602,12 @@ struct MicrosoftRecordLayoutBuilder {
llvm::SmallPtrSetImpl ,
const CXXRecordDecl *RD) const;
const ASTContext
+ EmptySubobjectMap
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -3317,6 +3353,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const
{
Builder.EndsWithZeroSizedObject, Builder.LeadsWithZeroSizedBase,
Builder.Bases, Builder.VBases);
} else {
+ MicrosoftRecordLayoutBuilder Builder(*this, /*EmptySubobjects*/ nullptr);
shafik wrote:
see
[bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -3317,6 +3353,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const
{
Builder.EndsWithZeroSizedObject, Builder.LeadsWithZeroSizedBase,
Builder.Bases, Builder.VBases);
} else {
+ MicrosoftRecordLayoutBuilder Builder(*this, /*EmptySubobjects*/ nullptr);
shafik wrote:
```suggestion
MicrosoftRecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/nullptr);
```
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
amykhuang wrote: > > We should consider whether we want to support `__msvc_no_unique_address__` > > or similar as an alternative spelling #61196 > > I think `[[__msvc__::__no_unique_address__]]` would be better. This is how > the clang and gnu attributes are handled too. I'm also fine with handling > this in a follow-up patch. Sounds good, I can do this in a separate patch https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang resolved https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang resolved https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -55,7 +55,7 @@ CXX11(unlikely) // CHECK: likely: 201803L // CHECK: maybe_unused: 201603L // ITANIUM: no_unique_address: 201803L -// WINDOWS: no_unique_address: 0 +// WINDOWS: no_unique_address: 201803L amykhuang wrote: Tried this and the inheritance part wasn't quite working (think that the NoUniqueAddress should be a class, but in that case, not sure how to parse the two attributes). I did a slightly different version where we have three attributes with no inheritance (MS, Itanium, and general) and just parse the MS and Itanium versions as the general version in SemaDeclAttr. It at least passes the tests that were broken before. https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang resolved https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/cor3ntin edited https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -55,7 +55,7 @@ CXX11(unlikely) // CHECK: likely: 201803L // CHECK: maybe_unused: 201603L // ITANIUM: no_unique_address: 201803L -// WINDOWS: no_unique_address: 0 +// WINDOWS: no_unique_address: 201803L cor3ntin wrote: don't we expect `__has_cpp_attribute(no_unique_address)` to be 0 on windows and `__has_cpp_attribute(msvc::no_unique_address)` to be... some value? https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -8368,6 +8368,20 @@ static void handleNoMergeAttr(Sema , Decl *D, const
ParsedAttr ) {
D->addAttr(NoMergeAttr::Create(S.Context, AL));
}
+static void handleNoUniqueAddressAttr(Sema , Decl *D, const ParsedAttr ) {
+ NoUniqueAddressAttr TmpAttr(S.Context, AL);
+ if (S.getLangOpts().MSVCCompat) {
+if (TmpAttr.isStandard()) {
+ S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
+ return;
+}
+ } else if (TmpAttr.isMSVC()) {
cor3ntin wrote:
```suggestion
if (S.getLangOpts().MSVCCompat && TmpAttr.isStandard()) {
S.Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
return;
} else if (TmpAttr.isMSVC()) {
```
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/cor3ntin commented: There are some whitespace-only that should be reverted + 1 small nitpick. And I have a question about `__has_cpp_attribute` https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -743,6 +743,7 @@ void CGRecordLowering::calculateZeroInit() {
void CGRecordLowering::clipTailPadding() {
std::vector::iterator Prior = Members.begin();
CharUnits Tail = getSize(Prior->Data);
+
mstorsjo wrote:
Nit: The total PR sums up to adding a spurious empty line here, while this file
otherwise no longer has any functional changes.
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -2866,8 +2871,10 @@
MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
for (const CXXBaseSpecifier : RD->bases()) {
if (Base.isVirtual())
continue;
+
const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
const ASTRecordLayout = Context.getASTRecordLayout(BaseDecl);
+
mstorsjo wrote:
Nit: After adding and removing things here, the total result of the PR ends up
in adding new spurious empty lines here (and right above, and below on line
2949 and 2967).
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang updated
https://github.com/llvm/llvm-project/pull/65675:
>From 923a43cd6386f6e57023fd8928eed0dc0ab04d57 Mon Sep 17 00:00:00 2001
From: Amy Huang
Date: Fri, 21 Jul 2023 16:30:30 -0700
Subject: [PATCH 1/3] Implement [[msvc::no_unique_address]]
This attribute should match the behavior of MSVC's [[msvc::no_unique_address]]
attribute.
Bug: https://github.com/llvm/llvm-project/issues/49358
Differential Revision: https://reviews.llvm.org/D157762
---
clang/include/clang/Basic/Attr.td | 8 +-
clang/include/clang/Basic/AttrDocs.td | 4 +
clang/lib/AST/Decl.cpp | 8 +
clang/lib/AST/RecordLayoutBuilder.cpp | 191 +--
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 1 +
clang/lib/Sema/SemaDeclAttr.cpp | 17 +
clang/test/Layout/ms-no-unique-address.cpp | 338
7 files changed, 540 insertions(+), 27 deletions(-)
create mode 100644 clang/test/Layout/ms-no-unique-address.cpp
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..d1770330070a519 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext ) const {
if (!CXXRD->isEmpty())
return false;
+ // MS ABI: nonzero if class type with class type fields
+ if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() &&
+ llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) {
+return Field->getType()->getAs();
+ })) {
+return false;
+ }
+
// 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
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8afd88ae7be27b3..2152e69732d65c9 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) {}
+ MicrosoftRecordLayoutBuilder(const ASTContext ,
+ EmptySubobjectMap *EmptySubobjects)
+ : Context(Context), EmptySubobjects(EmptySubobjects) {}
+
private:
MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
void operator=(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2562,7 +2565,11 @@ struct MicrosoftRecordLayoutBuilder {
void layoutNonVirtualBase(const CXXRecordDecl *RD,
const CXXRecordDecl *BaseDecl,
const ASTRecordLayout ,
-const ASTRecordLayout *);
+const ASTRecordLayout *,
+BaseSubobjectInfo *Base);
+ BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD,
+ bool IsVirtual,
+ BaseSubobjectInfo *Derived);
void injectVFPtr(const CXXRecordDecl *RD);
void injectVBPtr(const CXXRecordDecl *RD);
/// Lays out the fields of the record. Also rounds size up to
@@ -2595,6 +2602,12 @@ struct MicrosoftRecordLayoutBuilder {
llvm::SmallPtrSetImpl ,
const CXXRecordDecl *RD) const;
const ASTContext
+ EmptySubobjectMap
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang updated
https://github.com/llvm/llvm-project/pull/65675:
>From 923a43cd6386f6e57023fd8928eed0dc0ab04d57 Mon Sep 17 00:00:00 2001
From: Amy Huang
Date: Fri, 21 Jul 2023 16:30:30 -0700
Subject: [PATCH 1/3] Implement [[msvc::no_unique_address]]
This attribute should match the behavior of MSVC's [[msvc::no_unique_address]]
attribute.
Bug: https://github.com/llvm/llvm-project/issues/49358
Differential Revision: https://reviews.llvm.org/D157762
---
clang/include/clang/Basic/Attr.td | 8 +-
clang/include/clang/Basic/AttrDocs.td | 4 +
clang/lib/AST/Decl.cpp | 8 +
clang/lib/AST/RecordLayoutBuilder.cpp | 191 +--
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 1 +
clang/lib/Sema/SemaDeclAttr.cpp | 17 +
clang/test/Layout/ms-no-unique-address.cpp | 338
7 files changed, 540 insertions(+), 27 deletions(-)
create mode 100644 clang/test/Layout/ms-no-unique-address.cpp
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..d1770330070a519 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext ) const {
if (!CXXRD->isEmpty())
return false;
+ // MS ABI: nonzero if class type with class type fields
+ if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() &&
+ llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) {
+return Field->getType()->getAs();
+ })) {
+return false;
+ }
+
// 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
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8afd88ae7be27b3..2152e69732d65c9 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) {}
+ MicrosoftRecordLayoutBuilder(const ASTContext ,
+ EmptySubobjectMap *EmptySubobjects)
+ : Context(Context), EmptySubobjects(EmptySubobjects) {}
+
private:
MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
void operator=(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2562,7 +2565,11 @@ struct MicrosoftRecordLayoutBuilder {
void layoutNonVirtualBase(const CXXRecordDecl *RD,
const CXXRecordDecl *BaseDecl,
const ASTRecordLayout ,
-const ASTRecordLayout *);
+const ASTRecordLayout *,
+BaseSubobjectInfo *Base);
+ BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD,
+ bool IsVirtual,
+ BaseSubobjectInfo *Derived);
void injectVFPtr(const CXXRecordDecl *RD);
void injectVBPtr(const CXXRecordDecl *RD);
/// Lays out the fields of the record. Also rounds size up to
@@ -2595,6 +2602,12 @@ struct MicrosoftRecordLayoutBuilder {
llvm::SmallPtrSetImpl ,
const CXXRecordDecl *RD) const;
const ASTContext
+ EmptySubobjectMap
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang updated
https://github.com/llvm/llvm-project/pull/65675:
>From 923a43cd6386f6e57023fd8928eed0dc0ab04d57 Mon Sep 17 00:00:00 2001
From: Amy Huang
Date: Fri, 21 Jul 2023 16:30:30 -0700
Subject: [PATCH 1/3] Implement [[msvc::no_unique_address]]
This attribute should match the behavior of MSVC's [[msvc::no_unique_address]]
attribute.
Bug: https://github.com/llvm/llvm-project/issues/49358
Differential Revision: https://reviews.llvm.org/D157762
---
clang/include/clang/Basic/Attr.td | 8 +-
clang/include/clang/Basic/AttrDocs.td | 4 +
clang/lib/AST/Decl.cpp | 8 +
clang/lib/AST/RecordLayoutBuilder.cpp | 191 +--
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 1 +
clang/lib/Sema/SemaDeclAttr.cpp | 17 +
clang/test/Layout/ms-no-unique-address.cpp | 338
7 files changed, 540 insertions(+), 27 deletions(-)
create mode 100644 clang/test/Layout/ms-no-unique-address.cpp
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..d1770330070a519 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext ) const {
if (!CXXRD->isEmpty())
return false;
+ // MS ABI: nonzero if class type with class type fields
+ if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() &&
+ llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) {
+return Field->getType()->getAs();
+ })) {
+return false;
+ }
+
// 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
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8afd88ae7be27b3..2152e69732d65c9 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) {}
+ MicrosoftRecordLayoutBuilder(const ASTContext ,
+ EmptySubobjectMap *EmptySubobjects)
+ : Context(Context), EmptySubobjects(EmptySubobjects) {}
+
private:
MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
void operator=(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2562,7 +2565,11 @@ struct MicrosoftRecordLayoutBuilder {
void layoutNonVirtualBase(const CXXRecordDecl *RD,
const CXXRecordDecl *BaseDecl,
const ASTRecordLayout ,
-const ASTRecordLayout *);
+const ASTRecordLayout *,
+BaseSubobjectInfo *Base);
+ BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD,
+ bool IsVirtual,
+ BaseSubobjectInfo *Derived);
void injectVFPtr(const CXXRecordDecl *RD);
void injectVBPtr(const CXXRecordDecl *RD);
/// Lays out the fields of the record. Also rounds size up to
@@ -2595,6 +2602,12 @@ struct MicrosoftRecordLayoutBuilder {
llvm::SmallPtrSetImpl ,
const CXXRecordDecl *RD) const;
const ASTContext
+ EmptySubobjectMap
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext ) const {
if (!CXXRD->isEmpty())
return false;
+ // MS ABI: nonzero if class type with class type fields
amykhuang wrote:
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
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang resolved https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang resolved https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -2999,17 +3139,17 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const
FieldDecl *FD) {
auto NewSize = Context.toCharUnitsFromBits(
llvm::alignDown(FieldBitOffset, Context.toBits(Info.Alignment)) +
Context.toBits(Info.Size));
-Size = std::max(Size, NewSize);
+DataSize = Size = std::max(Size, NewSize);
amykhuang wrote:
Removed cascading assignments
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang resolved https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
AaronBallman wrote: > 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/ . Sorry for the confusion, yes, I was mostly worried about RecordLayoutBuilder.cpp and whether you spotted any concerns with the changes. https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
llvmbot wrote:
@llvm/pr-subscribers-clang
Changes
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: https://github.com/llvm/llvm-project/issues/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:
- (modified) clang/include/clang/Basic/Attr.td (+5-3)
- (modified) clang/include/clang/Basic/AttrDocs.td (+4)
- (modified) clang/lib/AST/Decl.cpp (+7-3)
- (modified) clang/lib/AST/RecordLayoutBuilder.cpp (+56-10)
- (modified) clang/lib/CodeGen/CGRecordLayoutBuilder.cpp (+1)
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+17)
- (added) clang/test/Layout/ms-no-unique-address.cpp (+338)
- (modified) clang/test/Preprocessor/has_attribute.cpp (+1-1)
- (modified) clang/test/SemaCXX/cxx2a-no-unique-address.cpp (+1-1)
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 ) 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) {}
+ MicrosoftRecordLayoutBuilder(const ASTContext ,
+ EmptySubobjectMap *EmptySubobjects)
+ : Context(Context), EmptySubobjects(EmptySubobjects) {}
+
private:
MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
void operator=(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2595,6 +2598,12 @@ struct MicrosoftRecordLayoutBuilder {
llvm::SmallPtrSetImpl ,
const CXXRecordDecl *RD) const;
const ASTContext
+ 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 : RD->bases()) {
-if (Base.isVirtual())
- continue;
const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
const ASTRecordLayout
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/llvmbot labeled https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang updated
https://github.com/llvm/llvm-project/pull/65675:
>From 923a43cd6386f6e57023fd8928eed0dc0ab04d57 Mon Sep 17 00:00:00 2001
From: Amy Huang
Date: Fri, 21 Jul 2023 16:30:30 -0700
Subject: [PATCH 1/2] Implement [[msvc::no_unique_address]]
This attribute should match the behavior of MSVC's [[msvc::no_unique_address]]
attribute.
Bug: https://github.com/llvm/llvm-project/issues/49358
Differential Revision: https://reviews.llvm.org/D157762
---
clang/include/clang/Basic/Attr.td | 8 +-
clang/include/clang/Basic/AttrDocs.td | 4 +
clang/lib/AST/Decl.cpp | 8 +
clang/lib/AST/RecordLayoutBuilder.cpp | 191 +--
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 1 +
clang/lib/Sema/SemaDeclAttr.cpp | 17 +
clang/test/Layout/ms-no-unique-address.cpp | 338
7 files changed, 540 insertions(+), 27 deletions(-)
create mode 100644 clang/test/Layout/ms-no-unique-address.cpp
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..d1770330070a519 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext ) const {
if (!CXXRD->isEmpty())
return false;
+ // MS ABI: nonzero if class type with class type fields
+ if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() &&
+ llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) {
+return Field->getType()->getAs();
+ })) {
+return false;
+ }
+
// 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
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8afd88ae7be27b3..2152e69732d65c9 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) {}
+ MicrosoftRecordLayoutBuilder(const ASTContext ,
+ EmptySubobjectMap *EmptySubobjects)
+ : Context(Context), EmptySubobjects(EmptySubobjects) {}
+
private:
MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
void operator=(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2562,7 +2565,11 @@ struct MicrosoftRecordLayoutBuilder {
void layoutNonVirtualBase(const CXXRecordDecl *RD,
const CXXRecordDecl *BaseDecl,
const ASTRecordLayout ,
-const ASTRecordLayout *);
+const ASTRecordLayout *,
+BaseSubobjectInfo *Base);
+ BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD,
+ bool IsVirtual,
+ BaseSubobjectInfo *Derived);
void injectVFPtr(const CXXRecordDecl *RD);
void injectVBPtr(const CXXRecordDecl *RD);
/// Lays out the fields of the record. Also rounds size up to
@@ -2595,6 +2602,12 @@ struct MicrosoftRecordLayoutBuilder {
llvm::SmallPtrSetImpl ,
const CXXRecordDecl *RD) const;
const ASTContext
+ EmptySubobjectMap
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext ) const {
if (!CXXRD->isEmpty())
return false;
+ // MS ABI: nonzero if class type with class type fields
erichkeane wrote:
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?
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -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>]>,
erichkeane wrote:
```suggestion
let Accessors = [Accessor<"isStandard", [CXX11<"", "no_unique_address",
201803>]>,
```
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/erichkeane commented: The `RecordLayoutBuilder` is not something I have any knowledge about, but I shared two comments. https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -2937,14 +2964,97 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
BaseOffset = CharUnits::Zero();
} else {
// Otherwise, lay the base out at the end of the MDC.
- BaseOffset = Size = Size.alignTo(Info.Alignment);
+ BaseOffset = DataSize = Size = Size.alignTo(Info.Alignment);
}
+
+// Place in EmptySubobjects map but don't check the position? MSVC seems to
+// not allow fields to overlap at the end of a virtual base, but they can
+// overlap with other bass.
+EmptySubobjects->CanPlaceBaseAtOffset(Base, BaseOffset);
}
+
Bases.insert(std::make_pair(BaseDecl, BaseOffset));
Size += BaseLayout.getNonVirtualSize();
+ DataSize = Size;
PreviousBaseLayout =
}
+BaseSubobjectInfo *MicrosoftRecordLayoutBuilder::computeBaseSubobjectInfo(
+const CXXRecordDecl *RD, bool IsVirtual, BaseSubobjectInfo *Derived) {
+ // This is copied directly from
ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo.
+ BaseSubobjectInfo *Info;
+
+ if (IsVirtual) {
+// Check if we already have info about this virtual base.
+BaseSubobjectInfo * = VirtualBaseInfo[RD];
+if (InfoSlot) {
+ assert(InfoSlot->Class == RD && "Wrong class for virtual base info!");
+ return InfoSlot;
+}
+
+// We don't, create it.
+InfoSlot = new (BaseSubobjectInfoAllocator.Allocate()) BaseSubobjectInfo;
+Info = InfoSlot;
+ } else {
+Info = new (BaseSubobjectInfoAllocator.Allocate()) BaseSubobjectInfo;
+ }
+
+ Info->Class = RD;
+ Info->IsVirtual = IsVirtual;
+ Info->Derived = nullptr;
+ Info->PrimaryVirtualBaseInfo = nullptr;
+
+ const CXXRecordDecl *PrimaryVirtualBase = nullptr;
+ BaseSubobjectInfo *PrimaryVirtualBaseInfo = nullptr;
+
+ // Check if this base has a primary virtual base.
efriedma-quic wrote:
Are primary virtual bases actually an thing in the MSVC ABI? It seems like
PrimaryVirtualBase and PrimaryVirtualBaseInfo aren't actually used anywhere.
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
efriedma-quic wrote: 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/ . https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/AaronBallman commented: 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. https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/AaronBallman review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/AaronBallman review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/AaronBallman review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/cjdb requested changes to this pull request. https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -2999,17 +3139,17 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const
FieldDecl *FD) {
auto NewSize = Context.toCharUnitsFromBits(
llvm::alignDown(FieldBitOffset, Context.toBits(Info.Alignment)) +
Context.toBits(Info.Size));
-Size = std::max(Size, NewSize);
+DataSize = Size = std::max(Size, NewSize);
cjdb wrote:
I'd prefer it if we didn't have cascading assignment please.
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/cjdb edited https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/rnk review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -2937,14 +2964,97 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
BaseOffset = CharUnits::Zero();
} else {
// Otherwise, lay the base out at the end of the MDC.
- BaseOffset = Size = Size.alignTo(Info.Alignment);
+ BaseOffset = DataSize = Size = Size.alignTo(Info.Alignment);
}
+
+// Place in EmptySubobjects map but don't check the position? MSVC seems to
+// not allow fields to overlap at the end of a virtual base, but they can
+// overlap with other bass.
+EmptySubobjects->CanPlaceBaseAtOffset(Base, BaseOffset);
}
+
Bases.insert(std::make_pair(BaseDecl, BaseOffset));
Size += BaseLayout.getNonVirtualSize();
+ DataSize = Size;
PreviousBaseLayout =
}
+BaseSubobjectInfo *MicrosoftRecordLayoutBuilder::computeBaseSubobjectInfo(
+const CXXRecordDecl *RD, bool IsVirtual, BaseSubobjectInfo *Derived) {
+ // This is copied directly from
ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo.
amykhuang wrote:
Actually going to remove this altogether. Makes it less consistent with MSVC's
behavior but MSVC appears to avoid overlapping fields with bases only when
no_unique_address is used, which seems weird.
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
philnik777 wrote: > We should consider whether we want to support `__msvc_no_unique_address__` or > similar as an alternative spelling #61196 I think `[[__msvc__::__no_unique_address__]]` would be better. This is how the clang and gnu attributes are handled too. https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
cor3ntin wrote: We should consider whether we want to support __msvc_no_unique_address__ or similar https://github.com/llvm/llvm-project/issues/61196 https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -3055,7 +3195,7 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const
CXXRecordDecl *RD) {
// It is possible that there were no fields or bases located after vbptr,
// so the size was not adjusted before.
if (Size < FieldStart)
- Size = FieldStart;
+ DataSize = Size = FieldStart;
amykhuang wrote:
I realized I can reduce the places that DataSize is set, and I think it's more
readable this way?
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang resolved https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -2937,14 +2964,97 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
BaseOffset = CharUnits::Zero();
} else {
// Otherwise, lay the base out at the end of the MDC.
- BaseOffset = Size = Size.alignTo(Info.Alignment);
+ BaseOffset = DataSize = Size = Size.alignTo(Info.Alignment);
}
+
+// Place in EmptySubobjects map but don't check the position? MSVC seems to
+// not allow fields to overlap at the end of a virtual base, but they can
+// overlap with other bass.
+EmptySubobjects->CanPlaceBaseAtOffset(Base, BaseOffset);
}
+
Bases.insert(std::make_pair(BaseDecl, BaseOffset));
Size += BaseLayout.getNonVirtualSize();
+ DataSize = Size;
PreviousBaseLayout =
}
+BaseSubobjectInfo *MicrosoftRecordLayoutBuilder::computeBaseSubobjectInfo(
+const CXXRecordDecl *RD, bool IsVirtual, BaseSubobjectInfo *Derived) {
+ // This is copied directly from
ItaniumRecordLayoutBuilder::ComputeBaseSubobjectInfo.
rnk wrote:
Can we structure this in a way that avoids the duplication?
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -3055,7 +3195,7 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const
CXXRecordDecl *RD) {
// It is possible that there were no fields or bases located after vbptr,
// so the size was not adjusted before.
if (Size < FieldStart)
- Size = FieldStart;
+ DataSize = Size = FieldStart;
rnk wrote:
If we need to track DataSize, should we create an abstraction for updating both
members?
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext ) const {
if (!CXXRD->isEmpty())
return false;
+ // MS ABI: nonzero if class type with class type fields
+ if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() &&
+ llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) {
+return Field->getType()->getAs();
+ })) {
+return false;
+ }
+
// 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
rnk wrote:
I think you've addressed this FIXME. Perhaps you can early return true if !MS
ABI, and then return a result based on the any_of check.
https://github.com/llvm/llvm-project/pull/65675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang edited https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/github-actions[bot] labeled https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/github-actions[bot] labeled https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang labeled https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang review_requested https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang created
https://github.com/llvm/llvm-project/pull/65675:
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: https://github.com/llvm/llvm-project/issues/49358
>From 923a43cd6386f6e57023fd8928eed0dc0ab04d57 Mon Sep 17 00:00:00 2001
From: Amy Huang
Date: Fri, 21 Jul 2023 16:30:30 -0700
Subject: [PATCH] Implement [[msvc::no_unique_address]]
This attribute should match the behavior of MSVC's [[msvc::no_unique_address]]
attribute.
Bug: https://github.com/llvm/llvm-project/issues/49358
Differential Revision: https://reviews.llvm.org/D157762
---
clang/include/clang/Basic/Attr.td | 8 +-
clang/include/clang/Basic/AttrDocs.td | 4 +
clang/lib/AST/Decl.cpp | 8 +
clang/lib/AST/RecordLayoutBuilder.cpp | 191 +--
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 1 +
clang/lib/Sema/SemaDeclAttr.cpp | 17 +
clang/test/Layout/ms-no-unique-address.cpp | 338
7 files changed, 540 insertions(+), 27 deletions(-)
create mode 100644 clang/test/Layout/ms-no-unique-address.cpp
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..d1770330070a519 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4505,6 +4505,14 @@ bool FieldDecl::isZeroSize(const ASTContext ) const {
if (!CXXRD->isEmpty())
return false;
+ // MS ABI: nonzero if class type with class type fields
+ if (Ctx.getTargetInfo().getCXXABI().isMicrosoft() &&
+ llvm::any_of(CXXRD->fields(), [](const FieldDecl *Field) {
+return Field->getType()->getAs();
+ })) {
+return false;
+ }
+
// 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
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8afd88ae7be27b3..2152e69732d65c9 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) {}
+ MicrosoftRecordLayoutBuilder(const ASTContext ,
+ EmptySubobjectMap *EmptySubobjects)
+ : Context(Context), EmptySubobjects(EmptySubobjects) {}
+
private:
MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
void operator=(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2562,7 +2565,11 @@ struct MicrosoftRecordLayoutBuilder {
void layoutNonVirtualBase(const CXXRecordDecl *RD,
const CXXRecordDecl *BaseDecl,
const ASTRecordLayout ,
-const ASTRecordLayout *);
+const ASTRecordLayout *,
+BaseSubobjectInfo *Base);
+ BaseSubobjectInfo *computeBaseSubobjectInfo(const CXXRecordDecl *RD,
+ bool IsVirtual,
+ BaseSubobjectInfo *Derived);
void injectVFPtr(const CXXRecordDecl *RD);
void injectVBPtr(const
[clang] Implement [[msvc::no_unique_address]] (PR #65675)
https://github.com/amykhuang labeled https://github.com/llvm/llvm-project/pull/65675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
